mirror of
https://github.com/twentyhq/twenty
synced 2026-04-21 13:37:22 +00:00
refactor: use middleware for MCP 405 responses instead of controller handlers
Replace controller-level GET/DELETE handlers with a NestJS middleware that intercepts non-POST requests on /mcp and returns proper 405 Method Not Allowed with a JSON-RPC error body. This keeps the controller focused on business logic, preserves class-level auth guards, and runs before the guard pipeline so rejected methods incur no authentication overhead. Also reverts the protocol version bump to 2024-11-05 — claiming 2025-03-26 without supporting its features (SSE streaming, sessions, batching) would mislead strict clients. https://claude.ai/code/session_011kTnDq6MymZbxfVCBRbbu3
This commit is contained in:
parent
597f623064
commit
318bbf415f
7 changed files with 124 additions and 51 deletions
|
|
@ -17,6 +17,7 @@ import { CoreGraphQLApiModule } from 'src/engine/api/graphql/core-graphql-api.mo
|
|||
import { GraphQLConfigModule } from 'src/engine/api/graphql/graphql-config/graphql-config.module';
|
||||
import { GraphQLConfigService } from 'src/engine/api/graphql/graphql-config/graphql-config.service';
|
||||
import { MetadataGraphQLApiModule } from 'src/engine/api/graphql/metadata-graphql-api.module';
|
||||
import { McpMethodGuardMiddleware } from 'src/engine/api/mcp/middlewares/mcp-method-guard.middleware';
|
||||
import { McpModule } from 'src/engine/api/mcp/mcp.module';
|
||||
import { RestApiModule } from 'src/engine/api/rest/rest-api.module';
|
||||
import { WorkspaceAuthContextMiddleware } from 'src/engine/core-modules/auth/middlewares/workspace-auth-context.middleware';
|
||||
|
|
@ -125,6 +126,10 @@ export class AppModule {
|
|||
)
|
||||
.forRoutes({ path: 'metadata', method: RequestMethod.ALL });
|
||||
|
||||
consumer
|
||||
.apply(McpMethodGuardMiddleware)
|
||||
.forRoutes({ path: 'mcp', method: RequestMethod.ALL });
|
||||
|
||||
for (const method of MIGRATED_REST_METHODS) {
|
||||
consumer
|
||||
.apply(RestCoreMiddleware, WorkspaceAuthContextMiddleware)
|
||||
|
|
|
|||
|
|
@ -1 +1 @@
|
|||
export const MCP_PROTOCOL_VERSION = '2025-03-26';
|
||||
export const MCP_PROTOCOL_VERSION = '2024-11-05';
|
||||
|
|
|
|||
|
|
@ -68,24 +68,6 @@ describe('McpCoreController', () => {
|
|||
expect(controller).toBeDefined();
|
||||
});
|
||||
|
||||
describe('unsupported methods', () => {
|
||||
it('should return 405 payload for GET /mcp', () => {
|
||||
expect(controller.handleUnsupportedGet()).toEqual({
|
||||
error: 'Method Not Allowed',
|
||||
message:
|
||||
'This MCP endpoint supports JSON-RPC over POST only. SSE streaming is not supported on GET /mcp.',
|
||||
});
|
||||
});
|
||||
|
||||
it('should return 405 payload for DELETE /mcp', () => {
|
||||
expect(controller.handleUnsupportedDelete()).toEqual({
|
||||
error: 'Method Not Allowed',
|
||||
message:
|
||||
'This MCP endpoint does not support session termination over DELETE /mcp',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('handleMcpCore', () => {
|
||||
const mockWorkspace = { id: 'workspace-1' } as FlatWorkspace;
|
||||
const mockUser = { id: 'user-1' } as UserEntity;
|
||||
|
|
|
|||
|
|
@ -1,9 +1,6 @@
|
|||
import {
|
||||
Body,
|
||||
Controller,
|
||||
Delete,
|
||||
Get,
|
||||
Header,
|
||||
HttpCode,
|
||||
HttpStatus,
|
||||
Post,
|
||||
|
|
@ -29,39 +26,14 @@ import { AuthUserWorkspaceId } from 'src/engine/decorators/auth/auth-user-worksp
|
|||
import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator';
|
||||
import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator';
|
||||
import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard';
|
||||
import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard';
|
||||
import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
|
||||
|
||||
@Controller('mcp')
|
||||
@UseGuards(McpAuthGuard, WorkspaceAuthGuard, NoPermissionGuard)
|
||||
@UseFilters(RestApiExceptionFilter)
|
||||
export class McpCoreController {
|
||||
constructor(private readonly mcpProtocolService: McpProtocolService) {}
|
||||
|
||||
@UseGuards(PublicEndpointGuard, NoPermissionGuard)
|
||||
@Get()
|
||||
@Header('Allow', 'POST')
|
||||
@HttpCode(HttpStatus.METHOD_NOT_ALLOWED)
|
||||
handleUnsupportedGet() {
|
||||
return {
|
||||
error: 'Method Not Allowed',
|
||||
message:
|
||||
'This MCP endpoint supports JSON-RPC over POST only. SSE streaming is not supported on GET /mcp.',
|
||||
};
|
||||
}
|
||||
|
||||
@UseGuards(PublicEndpointGuard, NoPermissionGuard)
|
||||
@Delete()
|
||||
@Header('Allow', 'POST')
|
||||
@HttpCode(HttpStatus.METHOD_NOT_ALLOWED)
|
||||
handleUnsupportedDelete() {
|
||||
return {
|
||||
error: 'Method Not Allowed',
|
||||
message:
|
||||
'This MCP endpoint does not support session termination over DELETE /mcp',
|
||||
};
|
||||
}
|
||||
|
||||
@UseGuards(McpAuthGuard, WorkspaceAuthGuard, NoPermissionGuard)
|
||||
@Post()
|
||||
@HttpCode(HttpStatus.OK)
|
||||
@UsePipes(
|
||||
|
|
|
|||
|
|
@ -0,0 +1,76 @@
|
|||
import { type Request, type Response } from 'express';
|
||||
|
||||
import { JSON_RPC_ERROR_CODE } from 'src/engine/api/mcp/constants/json-rpc-error-code.const';
|
||||
import { McpMethodGuardMiddleware } from 'src/engine/api/mcp/middlewares/mcp-method-guard.middleware';
|
||||
|
||||
describe('McpMethodGuardMiddleware', () => {
|
||||
let middleware: McpMethodGuardMiddleware;
|
||||
let mockRes: Partial<Response>;
|
||||
let next: jest.Mock;
|
||||
|
||||
beforeEach(() => {
|
||||
middleware = new McpMethodGuardMiddleware();
|
||||
next = jest.fn();
|
||||
mockRes = {
|
||||
setHeader: jest.fn().mockReturnThis(),
|
||||
status: jest.fn().mockReturnThis(),
|
||||
json: jest.fn().mockReturnThis(),
|
||||
};
|
||||
});
|
||||
|
||||
it('should call next() for POST requests', () => {
|
||||
const req = { method: 'POST' } as Request;
|
||||
|
||||
middleware.use(req, mockRes as Response, next);
|
||||
|
||||
expect(next).toHaveBeenCalled();
|
||||
expect(mockRes.status).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return 405 with Allow header for GET requests', () => {
|
||||
const req = { method: 'GET' } as Request;
|
||||
|
||||
middleware.use(req, mockRes as Response, next);
|
||||
|
||||
expect(next).not.toHaveBeenCalled();
|
||||
expect(mockRes.setHeader).toHaveBeenCalledWith('Allow', 'POST');
|
||||
expect(mockRes.status).toHaveBeenCalledWith(405);
|
||||
expect(mockRes.json).toHaveBeenCalledWith({
|
||||
jsonrpc: '2.0',
|
||||
error: {
|
||||
code: JSON_RPC_ERROR_CODE.INVALID_REQUEST,
|
||||
message:
|
||||
'HTTP method GET is not allowed. This MCP endpoint only accepts POST requests.',
|
||||
},
|
||||
id: null,
|
||||
});
|
||||
});
|
||||
|
||||
it('should return 405 with Allow header for DELETE requests', () => {
|
||||
const req = { method: 'DELETE' } as Request;
|
||||
|
||||
middleware.use(req, mockRes as Response, next);
|
||||
|
||||
expect(next).not.toHaveBeenCalled();
|
||||
expect(mockRes.setHeader).toHaveBeenCalledWith('Allow', 'POST');
|
||||
expect(mockRes.status).toHaveBeenCalledWith(405);
|
||||
expect(mockRes.json).toHaveBeenCalledWith({
|
||||
jsonrpc: '2.0',
|
||||
error: {
|
||||
code: JSON_RPC_ERROR_CODE.INVALID_REQUEST,
|
||||
message:
|
||||
'HTTP method DELETE is not allowed. This MCP endpoint only accepts POST requests.',
|
||||
},
|
||||
id: null,
|
||||
});
|
||||
});
|
||||
|
||||
it('should return 405 for PUT requests', () => {
|
||||
const req = { method: 'PUT' } as Request;
|
||||
|
||||
middleware.use(req, mockRes as Response, next);
|
||||
|
||||
expect(next).not.toHaveBeenCalled();
|
||||
expect(mockRes.status).toHaveBeenCalledWith(405);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,30 @@
|
|||
import { Injectable, type NestMiddleware } from '@nestjs/common';
|
||||
|
||||
import { type NextFunction, type Request, type Response } from 'express';
|
||||
|
||||
import { JSON_RPC_ERROR_CODE } from 'src/engine/api/mcp/constants/json-rpc-error-code.const';
|
||||
|
||||
// MCP streamable-http spec (2025-03-26) requires that servers respond with
|
||||
// 405 Method Not Allowed (plus an Allow header) for HTTP methods they do not
|
||||
// support. This middleware runs before guards and controllers so non-POST
|
||||
// requests are rejected without any authentication overhead.
|
||||
@Injectable()
|
||||
export class McpMethodGuardMiddleware implements NestMiddleware {
|
||||
use(req: Request, res: Response, next: NextFunction) {
|
||||
if (req.method === 'POST') {
|
||||
next();
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
res.setHeader('Allow', 'POST');
|
||||
res.status(405).json({
|
||||
jsonrpc: '2.0',
|
||||
error: {
|
||||
code: JSON_RPC_ERROR_CODE.INVALID_REQUEST,
|
||||
message: `HTTP method ${req.method} is not allowed. This MCP endpoint only accepts POST requests.`,
|
||||
},
|
||||
id: null,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -19,21 +19,29 @@ describe('MCP Controller (integration)', () => {
|
|||
.send(JSON.stringify(body));
|
||||
};
|
||||
|
||||
it('should return 405 for GET /mcp', async () => {
|
||||
it('should return 405 with JSON-RPC error for GET /mcp', async () => {
|
||||
await request(baseUrl)
|
||||
.get(endpoint)
|
||||
.expect(405)
|
||||
.expect((res) => {
|
||||
expect(res.headers.allow).toBe('POST');
|
||||
expect(res.body.jsonrpc).toBe('2.0');
|
||||
expect(res.body.error).toBeDefined();
|
||||
expect(res.body.error.code).toBe(-32600);
|
||||
expect(res.body.id).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it('should return 405 for DELETE /mcp', async () => {
|
||||
it('should return 405 with JSON-RPC error for DELETE /mcp', async () => {
|
||||
await request(baseUrl)
|
||||
.delete(endpoint)
|
||||
.expect(405)
|
||||
.expect((res) => {
|
||||
expect(res.headers.allow).toBe('POST');
|
||||
expect(res.body.jsonrpc).toBe('2.0');
|
||||
expect(res.body.error).toBeDefined();
|
||||
expect(res.body.error.code).toBe(-32600);
|
||||
expect(res.body.id).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -57,7 +65,7 @@ describe('MCP Controller (integration)', () => {
|
|||
expect(res.body.id).toBe(123);
|
||||
expect(res.body.jsonrpc).toBe('2.0');
|
||||
expect(res.body.result).toBeDefined();
|
||||
expect(res.body.result.protocolVersion).toBe('2025-03-26');
|
||||
expect(res.body.result.protocolVersion).toBe('2024-11-05');
|
||||
expect(res.body.result.capabilities).toBeDefined();
|
||||
expect(res.body.result.serverInfo).toBeDefined();
|
||||
expect(res.body.result.serverInfo.name).toBe('Twenty MCP Server');
|
||||
|
|
|
|||
Loading…
Reference in a new issue