Skip to content

Conversation

@sahandilshan
Copy link
Contributor

Purpose

Add AI agent that can be contact with mcp server with Auth enabled
image

Copilot AI review requested due to automatic review settings October 18, 2025 13:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a comprehensive MCP (Model Context Protocol) AI agent client with OAuth authentication support. The client provides a web interface for connecting to MCP servers and chatting with AI assistants using WSO2's design system.

Key changes:

  • Complete MCP AI agent client with Next.js, React 19, and TypeScript
  • Full OAuth 2.0 PKCE flow implementation with discovery mechanisms
  • WSO2-branded UI components and design system integration

Reviewed Changes

Copilot reviewed 31 out of 38 changed files in this pull request and generated 1 comment.

File Description
mcp-auth/python/main.py Updated comment to clarify this client's URL in auth settings
mcp-auth/python/jwt_validator.py Updated comment to specify authorization client instead of server
mcp-auth/client/ Complete Next.js client application with OAuth support and WSO2 branding
hotel-booking-agent-autogen-agent-iam/ Updated OpenAPI specs and auth manager comments for consistency
Files not reviewed (1)
  • mcp-auth/client/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@sahandilshan sahandilshan requested a review from Copilot October 18, 2025 13:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 24 out of 31 changed files in this pull request and generated 3 comments.

Files not reviewed (1)
  • mcp-auth/client/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)

mcp-auth/client/app/lib/oauth-utils.ts:1

  • There's a malformed Unicode character on line 130. The character '�' should be '🔍' to match the pattern.
/**

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

setDiscoveryInProgress(serverId);
console.log('🔍 ========================================');
console.log('🔍 Starting OAuth discovery for:', server.url);
console.log('� ========================================');
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same malformed Unicode character issue as in oauth-utils.ts. Line 129 contains '�' instead of '🔍'.

Suggested change
console.log(' ========================================');
console.log('🔍 ========================================');

Copilot uses AI. Check for mistakes.
Comment on lines 465 to 513
// Helper function to resolve JSON Schema $ref references
const resolveSchemaRefs = (schema: any): any => {
if (!schema || typeof schema !== 'object') {
return schema;
}

// If this object has a $ref, resolve it
if (schema.$ref && typeof schema.$ref === 'string') {
const refPath = schema.$ref.split('/');
if (refPath[0] === '#' && refPath[1] === '$defs') {
const defName = refPath[2];
if (schema.$defs && schema.$defs[defName]) {
// Resolve the reference
const resolved = resolveSchemaRefs(schema.$defs[defName]);
// Remove $ref and $defs, keep other properties
const { $ref, $defs, ...rest } = schema;
return { ...resolved, ...rest };
}
}
}

// Recursively resolve refs in the schema
const result: any = Array.isArray(schema) ? [] : {};
for (const key in schema) {
if (key === '$defs') {
// Skip $defs in the output, but keep for resolution
continue;
}
result[key] = resolveSchemaRefs(schema[key]);
}

// If we have $defs at root level, resolve references in properties
if (schema.$defs && schema.properties) {
for (const propKey in result.properties) {
const prop = result.properties[propKey];
if (prop.$ref && typeof prop.$ref === 'string') {
const refPath = prop.$ref.split('/');
if (refPath[0] === '#' && refPath[1] === '$defs') {
const defName = refPath[2];
if (schema.$defs[defName]) {
result.properties[propKey] = resolveSchemaRefs(schema.$defs[defName]);
}
}
}
}
}

return result;
};
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resolveSchemaRefs function is defined inline within the component, making it recreated on every render. Consider moving it outside the component or using useCallback to optimize performance.

Suggested change
// Helper function to resolve JSON Schema $ref references
const resolveSchemaRefs = (schema: any): any => {
if (!schema || typeof schema !== 'object') {
return schema;
}
// If this object has a $ref, resolve it
if (schema.$ref && typeof schema.$ref === 'string') {
const refPath = schema.$ref.split('/');
if (refPath[0] === '#' && refPath[1] === '$defs') {
const defName = refPath[2];
if (schema.$defs && schema.$defs[defName]) {
// Resolve the reference
const resolved = resolveSchemaRefs(schema.$defs[defName]);
// Remove $ref and $defs, keep other properties
const { $ref, $defs, ...rest } = schema;
return { ...resolved, ...rest };
}
}
}
// Recursively resolve refs in the schema
const result: any = Array.isArray(schema) ? [] : {};
for (const key in schema) {
if (key === '$defs') {
// Skip $defs in the output, but keep for resolution
continue;
}
result[key] = resolveSchemaRefs(schema[key]);
}
// If we have $defs at root level, resolve references in properties
if (schema.$defs && schema.properties) {
for (const propKey in result.properties) {
const prop = result.properties[propKey];
if (prop.$ref && typeof prop.$ref === 'string') {
const refPath = prop.$ref.split('/');
if (refPath[0] === '#' && refPath[1] === '$defs') {
const defName = refPath[2];
if (schema.$defs[defName]) {
result.properties[propKey] = resolveSchemaRefs(schema.$defs[defName]);
}
}
}
}
}
return result;
};
// Helper function to resolve JSON Schema $ref references
const resolveSchemaRefs = (schema: any): any => {
if (!schema || typeof schema !== 'object') {
return schema;
}
// If this object has a $ref, resolve it
if (schema.$ref && typeof schema.$ref === 'string') {
const refPath = schema.$ref.split('/');
if (refPath[0] === '#' && refPath[1] === '$defs') {
const defName = refPath[2];
if (schema.$defs && schema.$defs[defName]) {
// Resolve the reference
const resolved = resolveSchemaRefs(schema.$defs[defName]);
// Remove $ref and $defs, keep other properties
const { $ref, $defs, ...rest } = schema;
return { ...resolved, ...rest };
}
}
}
// Recursively resolve refs in the schema
const result: any = Array.isArray(schema) ? [] : {};
for (const key in schema) {
if (key === '$defs') {
// Skip $defs in the output, but keep for resolution
continue;
}
result[key] = resolveSchemaRefs(schema[key]);
}
// If we have $defs at root level, resolve references in properties
if (schema.$defs && schema.properties) {
for (const propKey in result.properties) {
const prop = result.properties[propKey];
if (prop.$ref && typeof prop.$ref === 'string') {
const refPath = prop.$ref.split('/');
if (refPath[0] === '#' && refPath[1] === '$defs') {
const defName = refPath[2];
if (schema.$defs[defName]) {
result.properties[propKey] = resolveSchemaRefs(schema.$defs[defName]);
}
}
}
}
}
return result;
};

Copilot uses AI. Check for mistakes.
Comment on lines +83 to +87
const randomValues = new Uint8Array(length);
crypto.getRandomValues(randomValues);
return Array.from(randomValues)
.map(v => charset[v % charset.length])
.join('');
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The random string generation uses modulo bias which can lead to slightly non-uniform distribution. While not critical for OAuth state/verifier generation, consider using rejection sampling for cryptographically secure randomness.

Suggested change
const randomValues = new Uint8Array(length);
crypto.getRandomValues(randomValues);
return Array.from(randomValues)
.map(v => charset[v % charset.length])
.join('');
const charsetLength = charset.length;
const maxValidByte = Math.floor(256 / charsetLength) * charsetLength;
let result = '';
while (result.length < length) {
const randomBytes = new Uint8Array(length - result.length);
crypto.getRandomValues(randomBytes);
for (let i = 0; i < randomBytes.length && result.length < length; i++) {
const v = randomBytes[i];
if (v < maxValidByte) {
result += charset[v % charsetLength];
}
}
}
return result;

Copilot uses AI. Check for mistakes.
@sahandilshan sahandilshan requested a review from Copilot October 18, 2025 13:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 24 out of 31 changed files in this pull request and generated 3 comments.

Files not reviewed (1)
  • mcp-auth/client/package-lock.json: Language not supported

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


// Connect to MCP server
const connectToMCP = async (server: MCPServerConfig) => {
const newSessionId = `session-${Date.now()}-${Math.random().toString(36).substring(2, 9)}`;
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The session ID generation uses Math.random() which is not cryptographically secure. Consider using crypto.randomUUID() for better security and uniqueness guarantees.

Suggested change
const newSessionId = `session-${Date.now()}-${Math.random().toString(36).substring(2, 9)}`;
const newSessionId = `session-${Date.now()}-${crypto.randomUUID()}`;

Copilot uses AI. Check for mistakes.

const handleSaveAI = () => {
onSaveAIConfig(localAIConfig);
alert('AI settings saved!');
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using alert() for user feedback is not user-friendly. Consider using a toast notification system or inline feedback messages for better UX.

Copilot uses AI. Check for mistakes.
Comment on lines +23 to +25
// Store sessions in memory (in production, use Redis or similar)
const sessions = new Map<string, string>();

Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storing sessions in memory will not scale and will lose data on server restarts. The comment mentions using Redis for production, but this should be implemented or documented more clearly as a production requirement.

Suggested change
// Store sessions in memory (in production, use Redis or similar)
const sessions = new Map<string, string>();
// Use Redis for session storage in production, fallback to in-memory Map for development
import Redis from 'ioredis';
// Session store abstraction
let sessionStore: {
get: (key: string) => Promise<string | undefined>;
set: (key: string, value: string) => Promise<void>;
del: (key: string) => Promise<void>;
};
// Initialize Redis client if in production and REDIS_URL is set
const isProduction = process.env.NODE_ENV === 'production';
const redisUrl = process.env.REDIS_URL;
if (isProduction) {
if (!redisUrl) {
throw new Error('REDIS_URL must be set in production for session storage.');
}
const redis = new Redis(redisUrl);
sessionStore = {
get: async (key: string) => {
const value = await redis.get(key);
return value === null ? undefined : value;
},
set: async (key: string, value: string) => {
await redis.set(key, value);
},
del: async (key: string) => {
await redis.del(key);
}
};
} else {
// In-memory fallback for development
const sessions = new Map<string, string>();
sessionStore = {
get: async (key: string) => sessions.get(key),
set: async (key: string, value: string) => { sessions.set(key, value); },
del: async (key: string) => { sessions.delete(key); }
};
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant