feat(ai): add integration tests for MCP controller and improve JSON-R… (#14047)
…PC validation - Introduced full integration test suite for MCP controller, testing `POST /mcp` with valid and invalid payloads. - Added `@IsDefined` validation to ensure the `method` field is required in JSON-RPC requests. - Applied `RestApiExceptionFilter` to MCP controller for consistent error handling. - Enhanced validation pipe in MCP controller to whitelist and reject non-whitelisted properties. - Consolidated exception filters in SSOAuthController.
This commit is contained in:
@@ -6,6 +6,7 @@ import { type Workspace } from 'src/engine/core-modules/workspace/workspace.enti
|
||||
import { MCP_SERVER_METADATA } from 'src/engine/core-modules/ai/constants/mcp.const';
|
||||
import { AccessTokenService } from 'src/engine/core-modules/auth/token/services/access-token.service';
|
||||
import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service';
|
||||
import { HttpExceptionHandlerService } from 'src/engine/core-modules/exception-handler/http-exception-handler.service';
|
||||
|
||||
import { McpController } from './mcp.controller';
|
||||
|
||||
@@ -33,6 +34,12 @@ describe('McpController', () => {
|
||||
provide: WorkspaceCacheStorageService,
|
||||
useValue: jest.fn(),
|
||||
},
|
||||
{
|
||||
provide: HttpExceptionHandlerService,
|
||||
useValue: {
|
||||
handleError: jest.fn(),
|
||||
},
|
||||
},
|
||||
],
|
||||
}).compile();
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import {
|
||||
Body,
|
||||
Controller,
|
||||
Post,
|
||||
UseFilters,
|
||||
UseGuards,
|
||||
UsePipes,
|
||||
ValidationPipe,
|
||||
@@ -15,21 +16,29 @@ import { AuthUserWorkspaceId } from 'src/engine/decorators/auth/auth-user-worksp
|
||||
import { JsonRpc } from 'src/engine/core-modules/ai/dtos/json-rpc';
|
||||
import { McpService } from 'src/engine/core-modules/ai/services/mcp.service';
|
||||
import { JwtAuthGuard } from 'src/engine/guards/jwt-auth.guard';
|
||||
import { RestApiExceptionFilter } from 'src/engine/api/rest/rest-api-exception.filter';
|
||||
|
||||
@Controller('mcp')
|
||||
@UseGuards(JwtAuthGuard, WorkspaceAuthGuard)
|
||||
@UseFilters(RestApiExceptionFilter)
|
||||
export class McpController {
|
||||
constructor(private readonly mcpService: McpService) {}
|
||||
|
||||
@Post()
|
||||
@UsePipes(new ValidationPipe({ transform: true }))
|
||||
@UsePipes(
|
||||
new ValidationPipe({
|
||||
transform: true,
|
||||
whitelist: true,
|
||||
forbidNonWhitelisted: true,
|
||||
}),
|
||||
)
|
||||
async handleMcpCore(
|
||||
@Body() body: JsonRpc,
|
||||
@AuthWorkspace() workspace: Workspace,
|
||||
@AuthApiKey() apiKey: string | undefined,
|
||||
@AuthUserWorkspaceId() userWorkspaceId: string | undefined,
|
||||
) {
|
||||
return this.mcpService.handleMCPCoreQuery(body, {
|
||||
return await this.mcpService.handleMCPCoreQuery(body, {
|
||||
workspace,
|
||||
userWorkspaceId,
|
||||
apiKey,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import {
|
||||
IsDefined,
|
||||
IsNotEmpty,
|
||||
IsObject,
|
||||
IsOptional,
|
||||
@@ -14,6 +15,7 @@ export class JsonRpc {
|
||||
@Matches(/^2\.0$/, { message: 'jsonrpc must be exactly "2.0"' })
|
||||
jsonrpc = '2.0';
|
||||
|
||||
@IsDefined({ message: 'method is required' })
|
||||
@IsString()
|
||||
@IsNotEmpty()
|
||||
method: string;
|
||||
|
||||
+1
-6
@@ -19,7 +19,6 @@ import {
|
||||
AuthException,
|
||||
AuthExceptionCode,
|
||||
} from 'src/engine/core-modules/auth/auth.exception';
|
||||
import { AuthOAuthExceptionFilter } from 'src/engine/core-modules/auth/filters/auth-oauth-exception.filter';
|
||||
import { AuthRestApiExceptionFilter } from 'src/engine/core-modules/auth/filters/auth-rest-api-exception.filter';
|
||||
import { EnterpriseFeaturesEnabledGuard } from 'src/engine/core-modules/auth/guards/enterprise-features-enabled.guard';
|
||||
import { OIDCAuthGuard } from 'src/engine/core-modules/auth/guards/oidc-auth.guard';
|
||||
@@ -42,6 +41,7 @@ import { workspaceValidator } from 'src/engine/core-modules/workspace/workspace.
|
||||
import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard';
|
||||
|
||||
@Controller('auth')
|
||||
@UseFilters(AuthRestApiExceptionFilter)
|
||||
export class SSOAuthController {
|
||||
constructor(
|
||||
private readonly loginTokenService: LoginTokenService,
|
||||
@@ -58,7 +58,6 @@ export class SSOAuthController {
|
||||
|
||||
@Get('saml/metadata/:identityProviderId')
|
||||
@UseGuards(EnterpriseFeaturesEnabledGuard, PublicEndpointGuard)
|
||||
@UseFilters(AuthRestApiExceptionFilter)
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
async generateMetadata(@Req() req: any): Promise<string | void> {
|
||||
return generateServiceProviderMetadata({
|
||||
@@ -76,7 +75,6 @@ export class SSOAuthController {
|
||||
|
||||
@Get('oidc/login/:identityProviderId')
|
||||
@UseGuards(EnterpriseFeaturesEnabledGuard, OIDCAuthGuard, PublicEndpointGuard)
|
||||
@UseFilters(AuthRestApiExceptionFilter)
|
||||
async oidcAuth() {
|
||||
// As this method is protected by OIDC Auth guard, it will trigger OIDC SSO flow
|
||||
return;
|
||||
@@ -84,7 +82,6 @@ export class SSOAuthController {
|
||||
|
||||
@Get('saml/login/:identityProviderId')
|
||||
@UseGuards(EnterpriseFeaturesEnabledGuard, SAMLAuthGuard, PublicEndpointGuard)
|
||||
@UseFilters(AuthRestApiExceptionFilter)
|
||||
async samlAuth() {
|
||||
// As this method is protected by SAML Auth guard, it will trigger SAML SSO flow
|
||||
return;
|
||||
@@ -92,14 +89,12 @@ export class SSOAuthController {
|
||||
|
||||
@Get('oidc/callback')
|
||||
@UseGuards(EnterpriseFeaturesEnabledGuard, OIDCAuthGuard, PublicEndpointGuard)
|
||||
@UseFilters(AuthOAuthExceptionFilter)
|
||||
async oidcAuthCallback(@Req() req: OIDCRequest, @Res() res: Response) {
|
||||
return await this.authCallback(req, res);
|
||||
}
|
||||
|
||||
@Post('saml/callback/:identityProviderId')
|
||||
@UseGuards(EnterpriseFeaturesEnabledGuard, SAMLAuthGuard, PublicEndpointGuard)
|
||||
@UseFilters(AuthOAuthExceptionFilter)
|
||||
async samlAuthCallback(@Req() req: SAMLRequest, @Res() res: Response) {
|
||||
try {
|
||||
return await this.authCallback(req, res);
|
||||
|
||||
@@ -0,0 +1,169 @@
|
||||
import request from 'supertest';
|
||||
|
||||
/**
|
||||
* Integration tests for MCP core controller
|
||||
*
|
||||
* These tests hit the real Nest app bootstrapped by test/integration/utils/setup-test.ts
|
||||
* and exercise the guarded POST /mcp endpoint using valid/invalid JSON-RPC payloads.
|
||||
*/
|
||||
|
||||
describe('MCP Controller (integration)', () => {
|
||||
const baseUrl = `http://localhost:${APP_PORT}`;
|
||||
const endpoint = '/mcp';
|
||||
|
||||
const postMcp = (body: any, bearer: string = API_KEY_ACCESS_TOKEN) => {
|
||||
return request(baseUrl)
|
||||
.post(endpoint)
|
||||
.set('Authorization', `Bearer ${bearer}`)
|
||||
.set('Content-Type', 'application/json')
|
||||
.send(JSON.stringify(body));
|
||||
};
|
||||
|
||||
it('should respond to ping with a JSON-RPC result envelope', async () => {
|
||||
await postMcp({ jsonrpc: '2.0', method: 'ping', id: '1' })
|
||||
.expect(201)
|
||||
.expect((res) => {
|
||||
expect(res.body).toMatchObject({
|
||||
id: '1',
|
||||
jsonrpc: '2.0',
|
||||
result: {},
|
||||
});
|
||||
expect(res.body.error).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
it('should respond to initialize with server metadata and capabilities', async () => {
|
||||
await postMcp({ jsonrpc: '2.0', method: 'initialize', id: 123 })
|
||||
.expect(201)
|
||||
.expect((res) => {
|
||||
expect(res.body.id).toBe(123);
|
||||
expect(res.body.jsonrpc).toBe('2.0');
|
||||
expect(res.body.result).toBeDefined();
|
||||
// Should include capabilities and server metadata fields merged by wrapJsonRpcResponse
|
||||
expect(res.body.result.capabilities).toBeDefined();
|
||||
expect(Array.isArray(res.body.result.tools)).toBe(true);
|
||||
expect(Array.isArray(res.body.result.resources)).toBe(true);
|
||||
expect(Array.isArray(res.body.result.prompts)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('should validate request body and return 400 for invalid payload (missing method)', async () => {
|
||||
await postMcp({})
|
||||
.expect(400)
|
||||
.expect((res) => {
|
||||
expect(res.body.error).toBe('BadRequestException');
|
||||
});
|
||||
});
|
||||
|
||||
describe('MCP Tools', () => {
|
||||
it('should list available tools with schemas', async () => {
|
||||
const res = await postMcp({
|
||||
jsonrpc: '2.0',
|
||||
method: 'tools/list',
|
||||
id: 'tools-list-1',
|
||||
}).expect(201);
|
||||
|
||||
expect(res.body.id).toBe('tools-list-1');
|
||||
expect(res.body.jsonrpc).toBe('2.0');
|
||||
expect(res.body.result).toBeDefined();
|
||||
expect(res.body.result.capabilities?.tools?.listChanged).toBe(false);
|
||||
expect(Array.isArray(res.body.result.tools)).toBe(true);
|
||||
// In a seeded workspace, there should be at least one tool
|
||||
expect(res.body.result.tools.length).toBeGreaterThanOrEqual(0);
|
||||
// If tools exist, they should have name, description and inputSchema
|
||||
const first = res.body.result.tools[0];
|
||||
|
||||
if (first) {
|
||||
expect(first.name).toBeDefined();
|
||||
expect(first.description).toBeDefined();
|
||||
expect(first.inputSchema).toBeDefined();
|
||||
}
|
||||
});
|
||||
|
||||
it('should return empty result for tools/call without params', async () => {
|
||||
const res = await postMcp({
|
||||
jsonrpc: '2.0',
|
||||
method: 'tools/call',
|
||||
id: 'tools-call-empty',
|
||||
}).expect(201);
|
||||
|
||||
expect(res.body).toMatchObject({
|
||||
id: 'tools-call-empty',
|
||||
jsonrpc: '2.0',
|
||||
result: {},
|
||||
});
|
||||
});
|
||||
|
||||
it('should return error when calling a non-existent tool', async () => {
|
||||
const res = await postMcp({
|
||||
jsonrpc: '2.0',
|
||||
method: 'tools/call',
|
||||
id: 'tools-call-missing',
|
||||
params: { name: 'non_existent_tool', arguments: {} },
|
||||
}).expect(201);
|
||||
|
||||
expect(res.body.id).toBe('tools-call-missing');
|
||||
expect(res.body.jsonrpc).toBe('2.0');
|
||||
expect(res.body.error).toBeDefined();
|
||||
// From McpService error wrapper: code = HttpStatus.NOT_FOUND (404) and message containing tool name
|
||||
expect(res.body.error.code).toBe(404);
|
||||
expect(String(res.body.error.message)).toMatch(/non_existent_tool/);
|
||||
});
|
||||
|
||||
it('should exercise each listed tool with a call and receive a JSON-RPC envelope', async () => {
|
||||
const list = await postMcp({
|
||||
jsonrpc: '2.0',
|
||||
method: 'tools/list',
|
||||
id: 'tools-list-for-calls',
|
||||
}).expect(201);
|
||||
|
||||
const tools: Array<{ name: string }> = list.body.result.tools || [];
|
||||
|
||||
// For each tool, attempt a call with empty arguments. We only assert we get a valid envelope
|
||||
// with either a result or an error. This ensures the MCP controller routes tool calls correctly
|
||||
// for every tool exposed in the workspace, without relying on specific seeded data.
|
||||
for (let i = 0; i < tools.length; i++) {
|
||||
const tool = tools[i];
|
||||
const id = `tool-call-${i}`;
|
||||
const res = await postMcp({
|
||||
jsonrpc: '2.0',
|
||||
method: 'tools/call',
|
||||
id,
|
||||
params: { name: tool.name, arguments: {} },
|
||||
}).expect(201);
|
||||
|
||||
expect(res.body.jsonrpc).toBe('2.0');
|
||||
expect(res.body.id).toBe(id);
|
||||
// Either a structured result or an error is acceptable depending on validation/business rules
|
||||
const hasResultContent = !!res.body?.result?.content;
|
||||
const hasError = !!res.body?.error;
|
||||
|
||||
expect(hasResultContent || hasError).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('should list prompts and resources as empty arrays with listChanged=false', async () => {
|
||||
const prompts = await postMcp({
|
||||
jsonrpc: '2.0',
|
||||
method: 'prompts/list',
|
||||
id: 'prompts-1',
|
||||
}).expect(201);
|
||||
|
||||
expect(prompts.body.result.capabilities?.prompts?.listChanged).toBe(
|
||||
false,
|
||||
);
|
||||
expect(Array.isArray(prompts.body.result.prompts)).toBe(true);
|
||||
|
||||
const resources = await postMcp({
|
||||
jsonrpc: '2.0',
|
||||
method: 'resources/list',
|
||||
id: 'resources-1',
|
||||
}).expect(201);
|
||||
|
||||
expect(resources.body.result.capabilities?.resources?.listChanged).toBe(
|
||||
false,
|
||||
);
|
||||
expect(Array.isArray(resources.body.result.resources)).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user