diff --git a/packages/@n8n/api-types/src/api-keys.ts b/packages/@n8n/api-types/src/api-keys.ts index 7fa40c13d14..5038341974b 100644 --- a/packages/@n8n/api-types/src/api-keys.ts +++ b/packages/@n8n/api-types/src/api-keys.ts @@ -15,3 +15,5 @@ export type ApiKey = { }; export type ApiKeyWithRawValue = ApiKey & { rawApiKey: string }; + +export type ApiKeyAudience = 'public-api' | 'mcp-server-api'; diff --git a/packages/@n8n/db/src/entities/api-key.ts b/packages/@n8n/db/src/entities/api-key.ts index 4c7cfb41ce9..83222fbe752 100644 --- a/packages/@n8n/db/src/entities/api-key.ts +++ b/packages/@n8n/db/src/entities/api-key.ts @@ -1,5 +1,6 @@ import type { ApiKeyScope } from '@n8n/permissions'; import { Column, Entity, Index, ManyToOne, Unique } from '@n8n/typeorm'; +import { ApiKeyAudience } from 'n8n-workflow'; import { JsonColumn, WithTimestampsAndStringId } from './abstract-entity'; import { User } from './user'; @@ -26,4 +27,7 @@ export class ApiKey extends WithTimestampsAndStringId { @Index({ unique: true }) @Column({ type: String }) apiKey: string; + + @Column({ type: String, default: 'public-api' }) + audience: ApiKeyAudience; } diff --git a/packages/@n8n/db/src/migrations/common/1758731786132-AddAudienceColumnToApiKey.ts b/packages/@n8n/db/src/migrations/common/1758731786132-AddAudienceColumnToApiKey.ts new file mode 100644 index 00000000000..6c52d7735b9 --- /dev/null +++ b/packages/@n8n/db/src/migrations/common/1758731786132-AddAudienceColumnToApiKey.ts @@ -0,0 +1,13 @@ +import type { MigrationContext, ReversibleMigration } from '../migration-types'; + +export class AddAudienceColumnToApiKeys1758731786132 implements ReversibleMigration { + async up({ schemaBuilder: { addColumns, column } }: MigrationContext) { + await addColumns('user_api_keys', [ + column('audience').varchar().notNull.default("'public-api'"), + ]); + } + + async down({ schemaBuilder: { dropColumns } }: MigrationContext) { + await dropColumns('user_api_keys', ['audience']); + } +} diff --git a/packages/@n8n/db/src/migrations/mysqldb/index.ts b/packages/@n8n/db/src/migrations/mysqldb/index.ts index 3dd075e4388..86a8263d120 100644 --- a/packages/@n8n/db/src/migrations/mysqldb/index.ts +++ b/packages/@n8n/db/src/migrations/mysqldb/index.ts @@ -1,3 +1,4 @@ +import { AddAudienceColumnToApiKeys1758731786132 } from '../common/1758731786132-AddAudienceColumnToApiKey'; import { AddMfaColumns1690000000030 } from './../common/1690000000040-AddMfaColumns'; import { LinkRoleToProjectRelationTable1753953244168 } from './../common/1753953244168-LinkRoleToProjectRelationTable'; import { InitialMigration1588157391238 } from './1588157391238-InitialMigration'; @@ -203,4 +204,5 @@ export const mysqlMigrations: Migration[] = [ LinkRoleToProjectRelationTable1753953244168, AddTimestampsToRoleAndRoleIndexes1756906557570, AddProjectIdToVariableTable1758794506893, + AddAudienceColumnToApiKeys1758731786132, ]; diff --git a/packages/@n8n/db/src/migrations/postgresdb/index.ts b/packages/@n8n/db/src/migrations/postgresdb/index.ts index e116f90cbc6..d557a9c5df7 100644 --- a/packages/@n8n/db/src/migrations/postgresdb/index.ts +++ b/packages/@n8n/db/src/migrations/postgresdb/index.ts @@ -98,6 +98,7 @@ import { RemoveOldRoleColumn1750252139170 } from '../common/1750252139170-Remove import { CreateDataStoreTables1754475614601 } from '../common/1754475614601-CreateDataStoreTables'; import { ReplaceDataStoreTablesWithDataTables1754475614602 } from '../common/1754475614602-ReplaceDataStoreTablesWithDataTables'; import { AddTimestampsToRoleAndRoleIndexes1756906557570 } from '../common/1756906557570-AddTimestampsToRoleAndRoleIndexes'; +import { AddAudienceColumnToApiKeys1758731786132 } from '../common/1758731786132-AddAudienceColumnToApiKey'; import type { Migration } from '../migration-types'; export const postgresMigrations: Migration[] = [ @@ -201,4 +202,5 @@ export const postgresMigrations: Migration[] = [ LinkRoleToProjectRelationTable1753953244168, AddTimestampsToRoleAndRoleIndexes1756906557570, AddProjectIdToVariableTable1758794506893, + AddAudienceColumnToApiKeys1758731786132, ]; diff --git a/packages/@n8n/db/src/migrations/sqlite/index.ts b/packages/@n8n/db/src/migrations/sqlite/index.ts index 5ed295b09ef..e02222be28e 100644 --- a/packages/@n8n/db/src/migrations/sqlite/index.ts +++ b/packages/@n8n/db/src/migrations/sqlite/index.ts @@ -1,3 +1,4 @@ +import { AddAudienceColumnToApiKeys1758731786132 } from './../common/1758731786132-AddAudienceColumnToApiKey'; import { InitialMigration1588102412422 } from './1588102412422-InitialMigration'; import { WebhookModel1592445003908 } from './1592445003908-WebhookModel'; import { CreateIndexStoppedAt1594825041918 } from './1594825041918-CreateIndexStoppedAt'; @@ -195,6 +196,7 @@ const sqliteMigrations: Migration[] = [ LinkRoleToProjectRelationTable1753953244168, AddTimestampsToRoleAndRoleIndexes1756906557570, AddProjectIdToVariableTable1758794506893, + AddAudienceColumnToApiKeys1758731786132, ]; export { sqliteMigrations }; diff --git a/packages/@n8n/permissions/src/__tests__/__snapshots__/scope-information.test.ts.snap b/packages/@n8n/permissions/src/__tests__/__snapshots__/scope-information.test.ts.snap index 784257adbb6..9be9d2c6434 100644 --- a/packages/@n8n/permissions/src/__tests__/__snapshots__/scope-information.test.ts.snap +++ b/packages/@n8n/permissions/src/__tests__/__snapshots__/scope-information.test.ts.snap @@ -140,6 +140,11 @@ exports[`Scope Information ensure scopes are defined correctly 1`] = ` "workflowTags:*", "role:manage", "role:*", + "mcp:manage", + "mcp:*", + "mcpApiKey:create", + "mcpApiKey:rotate", + "mcpApiKey:*", "*", ] `; diff --git a/packages/@n8n/permissions/src/constants.ee.ts b/packages/@n8n/permissions/src/constants.ee.ts index e4291a85edf..953131ffa3e 100644 --- a/packages/@n8n/permissions/src/constants.ee.ts +++ b/packages/@n8n/permissions/src/constants.ee.ts @@ -31,6 +31,8 @@ export const RESOURCES = { execution: ['delete', 'read', 'retry', 'list', 'get'] as const, workflowTags: ['update', 'list'] as const, role: ['manage'] as const, + mcp: ['manage'] as const, + mcpApiKey: ['create', 'rotate'] as const, } as const; export const API_KEY_RESOURCES = { diff --git a/packages/@n8n/permissions/src/roles/scopes/global-scopes.ee.ts b/packages/@n8n/permissions/src/roles/scopes/global-scopes.ee.ts index 28463fb87bd..bfd7e502058 100644 --- a/packages/@n8n/permissions/src/roles/scopes/global-scopes.ee.ts +++ b/packages/@n8n/permissions/src/roles/scopes/global-scopes.ee.ts @@ -91,6 +91,9 @@ export const GLOBAL_OWNER_SCOPES: Scope[] = [ 'oidc:manage', 'dataStore:list', 'role:manage', + 'mcp:manage', + 'mcpApiKey:create', + 'mcpApiKey:rotate', ]; export const GLOBAL_ADMIN_SCOPES = GLOBAL_OWNER_SCOPES.concat(); @@ -111,4 +114,6 @@ export const GLOBAL_MEMBER_SCOPES: Scope[] = [ 'variable:list', 'variable:read', 'dataStore:list', + 'mcpApiKey:create', + 'mcpApiKey:rotate', ]; diff --git a/packages/@n8n/permissions/src/utilities/__tests__/get-resource-permissions.test.ts b/packages/@n8n/permissions/src/utilities/__tests__/get-resource-permissions.test.ts index 240869d38f5..6ceb8896c89 100644 --- a/packages/@n8n/permissions/src/utilities/__tests__/get-resource-permissions.test.ts +++ b/packages/@n8n/permissions/src/utilities/__tests__/get-resource-permissions.test.ts @@ -35,6 +35,8 @@ describe('permissions', () => { folder: {}, insights: {}, dataStore: {}, + mcp: {}, + mcpApiKey: {}, role: {}, }); }); @@ -103,6 +105,8 @@ describe('permissions', () => { }, saml: {}, oidc: {}, + mcp: {}, + mcpApiKey: {}, securityAudit: {}, sourceControl: {}, tag: { diff --git a/packages/cli/src/auth/__tests__/auth-service-browser-id-whitelist.test.ts b/packages/cli/src/auth/__tests__/auth-service-browser-id-whitelist.test.ts index 0e3fedd44d2..6eea947db08 100644 --- a/packages/cli/src/auth/__tests__/auth-service-browser-id-whitelist.test.ts +++ b/packages/cli/src/auth/__tests__/auth-service-browser-id-whitelist.test.ts @@ -6,7 +6,6 @@ import { AuthService } from '@/auth/auth.service'; import type { MfaService } from '@/mfa/mfa.service'; import type { JwtService } from '@/services/jwt.service'; import type { UrlService } from '@/services/url.service'; -import type { ErrorReporter } from 'n8n-core'; describe('AuthService Browser ID Whitelist', () => { let authService: AuthService; @@ -20,7 +19,6 @@ describe('AuthService Browser ID Whitelist', () => { const userRepository = mock(); const invalidAuthTokenRepository = mock(); const mfaService = mock(); - const errorReporter = mock(); authService = new AuthService( globalConfig, @@ -31,7 +29,6 @@ describe('AuthService Browser ID Whitelist', () => { userRepository, invalidAuthTokenRepository, mfaService, - errorReporter, ); }); diff --git a/packages/cli/src/auth/__tests__/auth.service.test.ts b/packages/cli/src/auth/__tests__/auth.service.test.ts index 21844a05061..f5f7607d736 100644 --- a/packages/cli/src/auth/__tests__/auth.service.test.ts +++ b/packages/cli/src/auth/__tests__/auth.service.test.ts @@ -15,7 +15,6 @@ import { AUTH_COOKIE_NAME } from '@/constants'; import type { MfaService } from '@/mfa/mfa.service'; import { JwtService } from '@/services/jwt.service'; import type { UrlService } from '@/services/url.service'; -import type { ErrorReporter } from 'n8n-core'; describe('AuthService', () => { const browserId = 'test-browser-id'; @@ -36,7 +35,6 @@ describe('AuthService', () => { const userRepository = mock(); const invalidAuthTokenRepository = mock(); const mfaService = mock(); - const errorReporter = mock(); const authService = new AuthService( globalConfig, mock(), @@ -46,7 +44,6 @@ describe('AuthService', () => { userRepository, invalidAuthTokenRepository, mfaService, - errorReporter, ); const now = new Date('2024-02-01T01:23:45.678Z'); diff --git a/packages/cli/src/auth/auth.service.ts b/packages/cli/src/auth/auth.service.ts index 37c4f975873..c03726f7972 100644 --- a/packages/cli/src/auth/auth.service.ts +++ b/packages/cli/src/auth/auth.service.ts @@ -9,7 +9,6 @@ import { createHash } from 'crypto'; import type { NextFunction, Response } from 'express'; import { JsonWebTokenError, TokenExpiredError } from 'jsonwebtoken'; import type { StringValue as TimeUnitValue } from 'ms'; -import { ErrorReporter } from 'n8n-core'; import config from '@/config'; import { AuthError } from '@/errors/response-errors/auth.error'; @@ -48,10 +47,6 @@ interface CreateAuthMiddlewareOptions { * If true, authentication becomes optional in preview mode */ allowSkipPreviewAuth?: boolean; - /** - * If true, the middleware will check for an API key in the Authorization header - */ - apiKeyAuth?: boolean; } @Service() @@ -68,7 +63,6 @@ export class AuthService { private readonly userRepository: UserRepository, private readonly invalidAuthTokenRepository: InvalidAuthTokenRepository, private readonly mfaService: MfaService, - private readonly errorReporter: ErrorReporter, ) { const restEndpoint = globalConfig.endpoints.rest; this.skipBrowserIdCheckEndpoints = [ @@ -89,18 +83,8 @@ export class AuthService { ]; } - createAuthMiddleware({ - allowSkipMFA, - allowSkipPreviewAuth, - apiKeyAuth, - }: CreateAuthMiddlewareOptions) { + createAuthMiddleware({ allowSkipMFA, allowSkipPreviewAuth }: CreateAuthMiddlewareOptions) { return async (req: AuthenticatedRequest, res: Response, next: NextFunction) => { - // If route requests API key authentication, we need to check it first and skip the rest of the auth checks - if (apiKeyAuth) { - await this.checkAPIKey(req, res, next); - return; - } - const token = req.cookies[AUTH_COOKIE_NAME]; if (token) { try { @@ -142,66 +126,6 @@ export class AuthService { }; } - extractAPIKeyFromHeader(headerValue: string) { - if (!headerValue.startsWith('Bearer')) { - throw new AuthError('Invalid authorization header format'); - } - const apiKeyMatch = headerValue.match(/^Bearer\s+(.+)$/i); - if (apiKeyMatch) { - return apiKeyMatch[1]; - } - throw new AuthError('Invalid authorization header format'); - } - - async checkAPIKey(req: AuthenticatedRequest, response: Response, next: NextFunction) { - const headerValue = req.headers['authorization']; - if (!headerValue || typeof headerValue !== 'string') { - response.status(401).json({ status: 'error', message: 'API key is required' }); - return; - } - try { - const apiKey = this.extractAPIKeyFromHeader(headerValue); - - const keyOwner = await this.userRepository.findByApiKey(apiKey); - - if (!keyOwner) { - response.status(401).json({ status: 'error', message: 'Invalid API key' }); - return; - } - - if (keyOwner.disabled) { - response.status(403).json({ status: 'error', message: 'User is disabled' }); - return; - } - req.user = keyOwner; - - // If API key looks like a JWT, verify it to ensure it's not expired - // Legacy API keys (e.g. starting with "n8n_api_") are not JWTs and skip verification - const decoded = this.jwtService.decode(apiKey); - if (decoded) { - try { - this.jwtService.verify(apiKey); - } catch (e) { - if (e instanceof TokenExpiredError || e instanceof JsonWebTokenError) { - response.status(401).json({ status: 'error', message: 'Invalid API key' }); - return; - } - this.errorReporter.error(e); - throw e; - } - } - - next(); - } catch (error) { - if (error instanceof AuthError) { - response.status(401).json({ status: 'error', message: 'Invalid API key' }); - } else { - this.errorReporter.error(error); - response.status(500).json({ status: 'error', message: 'Internal server error' }); - } - } - } - clearCookie(res: Response) { res.clearCookie(AUTH_COOKIE_NAME); } diff --git a/packages/cli/src/controller.registry.ts b/packages/cli/src/controller.registry.ts index 235bf58c5d1..148f53056c6 100644 --- a/packages/cli/src/controller.registry.ts +++ b/packages/cli/src/controller.registry.ts @@ -91,7 +91,6 @@ export class ControllerRegistry { this.authService.createAuthMiddleware({ allowSkipMFA: route.allowSkipMFA, allowSkipPreviewAuth: route.allowSkipPreviewAuth, - apiKeyAuth: route.apiKeyAuth, }), this.lastActiveAtService.middleware.bind(this.lastActiveAtService), ] as RequestHandler[])), diff --git a/packages/cli/src/modules/mcp/__tests__/mcp-server-api-key.service.test.ts b/packages/cli/src/modules/mcp/__tests__/mcp-server-api-key.service.test.ts new file mode 100644 index 00000000000..9a80dcdbff8 --- /dev/null +++ b/packages/cli/src/modules/mcp/__tests__/mcp-server-api-key.service.test.ts @@ -0,0 +1,419 @@ +import { mockInstance } from '@n8n/backend-test-utils'; +import type { User } from '@n8n/db'; +import { ApiKeyRepository, UserRepository } from '@n8n/db'; +import { randomUUID } from 'crypto'; +import type { Request, Response, NextFunction } from 'express'; +import { mock, mockDeep } from 'jest-mock-extended'; +import type { InstanceSettings } from 'n8n-core'; + +import { JwtService } from '@/services/jwt.service'; + +import { McpServerApiKeyService } from '../mcp-api-key.service'; + +const mockReqWith = (authHeader: string | undefined) => { + const req = mockDeep(); + req.header.mockImplementation((name: string) => { + if (name === 'authorization') return authHeader; + return undefined; + }); + return req; +}; + +const instanceSettings = mock({ encryptionKey: 'test-key' }); +const jwtService = new JwtService(instanceSettings, mock()); + +let userRepository: jest.Mocked; +let apiKeyRepository: jest.Mocked; +let mcpServerApiKeyService: McpServerApiKeyService; + +describe('McpServerApiKeyService', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + beforeAll(() => { + userRepository = mockInstance(UserRepository); + apiKeyRepository = mockInstance(ApiKeyRepository); + mcpServerApiKeyService = new McpServerApiKeyService( + apiKeyRepository, + jwtService, + userRepository, + ); + }); + + describe('getAuthMiddleware', () => { + it('should return 401 if authorization header is missing', async () => { + // Arrange + const req = mockReqWith(undefined); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act + await middleware(req, res, next); + + // Assert + expect(res.status).toHaveBeenCalledWith(401); + expect(res.send).toHaveBeenCalledWith({ message: 'Unauthorized' }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should throw error if authorization header does not start with Bearer', async () => { + // Arrange + const req = mockReqWith('Basic sometoken'); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act & Assert + await expect(middleware(req, res, next)).rejects.toThrow( + 'Invalid authorization header format', + ); + expect(next).not.toHaveBeenCalled(); + }); + + it('should throw error if authorization header has invalid Bearer format', async () => { + // Arrange + const req = mockReqWith('Bearer'); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act & Assert + await expect(middleware(req, res, next)).rejects.toThrow( + 'Invalid authorization header format', + ); + expect(next).not.toHaveBeenCalled(); + }); + + it('should return 401 if API key is not found in database', async () => { + // Arrange + const apiKey = jwtService.sign({ + sub: randomUUID(), + iss: 'n8n', + aud: 'mcp-server-api', + jti: randomUUID(), + }); + + userRepository.findOne.mockResolvedValue(null); + + const req = mockReqWith(`Bearer ${apiKey}`); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act + await middleware(req, res, next); + + // Assert + expect(res.status).toHaveBeenCalledWith(401); + expect(res.send).toHaveBeenCalledWith({ message: 'Unauthorized' }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should return 401 if JWT verification fails (invalid signature)', async () => { + // Arrange + const userId = randomUUID(); + const mockUser = mockDeep(); + mockUser.id = userId; + + const wrongJwtService = new JwtService( + mock({ encryptionKey: 'wrong-key' }), + mock(), + ); + + const apiKey = wrongJwtService.sign({ + sub: userId, + iss: 'n8n', + aud: 'mcp-server-api', + jti: randomUUID(), + }); + + userRepository.findOne.mockResolvedValue(mockUser); + + const req = mockReqWith(`Bearer ${apiKey}`); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act + await middleware(req, res, next); + + // Assert + expect(res.status).toHaveBeenCalledWith(401); + expect(res.send).toHaveBeenCalledWith({ message: 'Unauthorized' }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should authenticate successfully with valid API key', async () => { + // Arrange + const userId = randomUUID(); + const mockUser = mockDeep(); + mockUser.id = userId; + + const apiKey = jwtService.sign({ + sub: userId, + iss: 'n8n', + aud: 'mcp-server-api', + jti: randomUUID(), + }); + + userRepository.findOne.mockResolvedValue(mockUser); + + const req = mockReqWith(`Bearer ${apiKey}`); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act + await middleware(req, res, next); + + // Assert + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + expect(res.send).not.toHaveBeenCalled(); + // @ts-ignore + expect(req.user).toBeDefined(); + // @ts-ignore + expect(req.user.id).toBe(userId); + }); + + it('should attach user with role information to request', async () => { + // Arrange + const userId = randomUUID(); + const mockUser = mockDeep(); + mockUser.id = userId; + + const apiKey = jwtService.sign({ + sub: userId, + iss: 'n8n', + aud: 'mcp-server-api', + jti: randomUUID(), + }); + + userRepository.findOne.mockResolvedValue(mockUser); + + const req = mockReqWith(`Bearer ${apiKey}`); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act + await middleware(req, res, next); + + // Assert + expect(next).toHaveBeenCalled(); + // @ts-ignore + expect(req.user).toBeDefined(); + // @ts-ignore + expect(req.user.role).toBeDefined(); + }); + + it('should handle Bearer token with exact case matching', async () => { + // Arrange + const userId = randomUUID(); + const mockUser = mockDeep(); + mockUser.id = userId; + + const apiKey = jwtService.sign({ + sub: userId, + iss: 'n8n', + aud: 'mcp-server-api', + jti: randomUUID(), + }); + + userRepository.findOne.mockResolvedValue(mockUser); + + const req = mockReqWith(`Bearer ${apiKey}`); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act + await middleware(req, res, next); + + // Assert + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + + it('should throw error with non-standard Bearer casing', async () => { + // Arrange + const userId = randomUUID(); + const mockUser = mockDeep(); + mockUser.id = userId; + + const apiKey = jwtService.sign({ + sub: userId, + iss: 'n8n', + aud: 'mcp-server-api', + jti: randomUUID(), + }); + + userRepository.findOne.mockResolvedValue(mockUser); + + const req = mockReqWith(`BEARER ${apiKey}`); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act & Assert + await expect(middleware(req, res, next)).rejects.toThrow( + 'Invalid authorization header format', + ); + expect(next).not.toHaveBeenCalled(); + }); + + it('should return 401 if user is not found for valid JWT', async () => { + // Arrange + const apiKey = jwtService.sign({ + sub: randomUUID(), + iss: 'n8n', + aud: 'mcp-server-api', + jti: randomUUID(), + }); + + userRepository.findOne.mockResolvedValue(null); + + const req = mockReqWith(`Bearer ${apiKey}`); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act + await middleware(req, res, next); + + // Assert + expect(res.status).toHaveBeenCalledWith(401); + expect(res.send).toHaveBeenCalledWith({ message: 'Unauthorized' }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should return 401 for malformed JWT', async () => { + // Arrange + userRepository.findOne.mockResolvedValue(null); + + const req = mockReqWith('Bearer malformed.jwt.token'); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act + await middleware(req, res, next); + + // Assert + expect(res.status).toHaveBeenCalledWith(401); + expect(res.send).toHaveBeenCalledWith({ message: 'Unauthorized' }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should handle Bearer token with extra whitespace', async () => { + // Arrange + const userId = randomUUID(); + const mockUser = mockDeep(); + mockUser.id = userId; + + const apiKey = jwtService.sign({ + sub: userId, + iss: 'n8n', + aud: 'mcp-server-api', + jti: randomUUID(), + }); + + userRepository.findOne.mockResolvedValue(mockUser); + + const req = mockReqWith(`Bearer ${apiKey}`); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act + await middleware(req, res, next); + + // Assert + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + + it('should return 401 if API key exists but user is deleted', async () => { + // Arrange + const apiKey = jwtService.sign({ + sub: randomUUID(), + iss: 'n8n', + aud: 'mcp-server-api', + jti: randomUUID(), + }); + + userRepository.findOne.mockResolvedValue(null); + + const req = mockReqWith(`Bearer ${apiKey}`); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act + await middleware(req, res, next); + + // Assert + expect(res.status).toHaveBeenCalledWith(401); + expect(res.send).toHaveBeenCalledWith({ message: 'Unauthorized' }); + expect(next).not.toHaveBeenCalled(); + }); + + it('should throw error with empty Bearer token', async () => { + // Arrange + const req = mockReqWith('Bearer '); + const res = mockDeep(); + res.status.mockReturnThis(); + res.send.mockReturnThis(); + const next = jest.fn() as NextFunction; + + const middleware = mcpServerApiKeyService.getAuthMiddleware(); + + // Act & Assert + await expect(middleware(req, res, next)).rejects.toThrow( + 'Invalid authorization header format', + ); + expect(next).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/cli/src/modules/mcp/__tests__/mcp.controller.test.ts b/packages/cli/src/modules/mcp/__tests__/mcp.controller.test.ts index 9dba5720445..dfa15c15b5b 100644 --- a/packages/cli/src/modules/mcp/__tests__/mcp.controller.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/mcp.controller.test.ts @@ -1,7 +1,19 @@ import { Logger } from '@n8n/backend-common'; -import type { AuthenticatedRequest } from '@n8n/db'; +import { type AuthenticatedRequest } from '@n8n/db'; import { Container } from '@n8n/di'; -import { mock } from 'jest-mock-extended'; +import { mock, mockDeep } from 'jest-mock-extended'; + +// eslint-disable-next-line import-x/order +import { McpServerApiKeyService } from '../mcp-api-key.service'; + +const mockAuthMiddleware = jest.fn().mockImplementation(async (_req, _res, next) => { + next(); +}); +const mcpServerApiKeyService = mockDeep(); +mcpServerApiKeyService.getAuthMiddleware.mockReturnValue(mockAuthMiddleware); + +// We need to mock the service before importing the controller as it's used in the middleware +Container.set(McpServerApiKeyService, mcpServerApiKeyService); import { McpController, type FlushableResponse } from '../mcp.controller'; import { McpService } from '../mcp.service'; @@ -33,9 +45,11 @@ describe('McpController', () => { beforeEach(() => { jest.clearAllMocks(); + Container.set(Logger, logger); Container.set(McpService, mcpService); Container.set(McpSettingsService, mcpSettingsService); + controller = Container.get(McpController); }); diff --git a/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.api.test.ts b/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.api.test.ts new file mode 100644 index 00000000000..7d989c70904 --- /dev/null +++ b/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.api.test.ts @@ -0,0 +1,186 @@ +import { testDb } from '@n8n/backend-test-utils'; +import { ApiKeyRepository, type User } from '@n8n/db'; +import { Container } from '@n8n/di'; + +import { createMember, createOwner, createUser } from '@test-integration/db/users'; +import { setupTestServer } from '@test-integration/utils'; + +const testServer = setupTestServer({ endpointGroups: ['mcp'] }); + +let owner: User; +let member: User; + +beforeAll(async () => { + owner = await createOwner(); + member = await createMember(); +}); + +afterEach(async () => { + await testDb.truncate(['ApiKey']); +}); + +describe('GET /mcp/api-key', () => { + test('should create and return new API key if user does not have one', async () => { + const response = await testServer.authAgentFor(owner).get('/mcp/api-key'); + + expect(response.statusCode).toBe(200); + + const { + data: { id, apiKey, userId }, + } = response.body; + + expect(id).toBeDefined(); + expect(apiKey).toBeDefined(); + expect(apiKey.length).toBeGreaterThanOrEqual(32); + expect(userId).toBe(owner.id); + }); + + test('should return existing API key (redacted) if user already has one', async () => { + const firstResponse = await testServer.authAgentFor(owner).get('/mcp/api-key'); + + const firstApiKey = firstResponse.body.data.apiKey; + + const secondResponse = await testServer.authAgentFor(owner).get('/mcp/api-key'); + const secondApiKey = secondResponse.body.data.apiKey; + + expect(secondResponse.statusCode).toBe(200); + expect(secondApiKey.slice(-4)).toBe(firstApiKey.slice(-4)); + }); + + test('should return different API keys for different users', async () => { + const ownerResponse = await testServer.authAgentFor(owner).get('/mcp/api-key'); + const memberResponse = await testServer.authAgentFor(member).get('/mcp/api-key'); + + expect(ownerResponse.statusCode).toBe(200); + expect(memberResponse.statusCode).toBe(200); + + expect(ownerResponse.body.data.apiKey).not.toBe(memberResponse.body.data.apiKey); + expect(ownerResponse.body.data.userId).toBe(owner.id); + expect(memberResponse.body.data.userId).toBe(member.id); + }); + + test('should require authentication', async () => { + const response = await testServer.authlessAgent.get('/mcp/api-key'); + + expect(response.statusCode).toBe(401); + }); +}); + +describe('POST /mcp/api-key/rotate', () => { + test('should rotate existing API key and return new one', async () => { + const initialResponse = await testServer.authAgentFor(owner).get('/mcp/api-key'); + const oldApiKey = initialResponse.body.data.apiKey; + + const rotateResponse = await testServer.authAgentFor(owner).post('/mcp/api-key/rotate'); + + expect(rotateResponse.statusCode).toBe(200); + + const { + data: { id, apiKey, userId }, + } = rotateResponse.body; + + expect(id).toBeDefined(); + expect(apiKey).toBeDefined(); + expect(apiKey).not.toBe(oldApiKey); + expect(userId).toBe(owner.id); + + // Verify old API key is no longer valid + const currentApiKeys = await Container.get(ApiKeyRepository).find({ + where: { userId: owner.id }, + }); + + expect(currentApiKeys.length).toBe(1); + expect(currentApiKeys[0].apiKey).toBe(apiKey); + expect(currentApiKeys[0].apiKey).not.toBe(oldApiKey); + }); + + test('should allow multiple rotations in sequence', async () => { + // Create initial API key + await testServer.authAgentFor(owner).get('/mcp/api-key'); + + // First rotation + const firstRotation = await testServer.authAgentFor(owner).post('/mcp/api-key/rotate'); + const firstApiKey = firstRotation.body.data.apiKey; + + // Second rotation + const secondRotation = await testServer.authAgentFor(owner).post('/mcp/api-key/rotate'); + const secondApiKey = secondRotation.body.data.apiKey; + + // Third rotation + const thirdRotation = await testServer.authAgentFor(owner).post('/mcp/api-key/rotate'); + const thirdApiKey = thirdRotation.body.data.apiKey; + + expect(firstRotation.statusCode).toBe(200); + expect(secondRotation.statusCode).toBe(200); + expect(thirdRotation.statusCode).toBe(200); + + expect(firstApiKey).not.toBe(secondApiKey); + expect(secondApiKey).not.toBe(thirdApiKey); + expect(firstApiKey).not.toBe(thirdApiKey); + }); + + test('should require authentication', async () => { + const response = await testServer.authlessAgent.post('/mcp/api-key/rotate'); + + expect(response.statusCode).toBe(401); + }); + + test('should require mcpApiKey:rotate scope', async () => { + // Create initial API key + await testServer.authAgentFor(owner).get('/mcp/api-key'); + + const response = await testServer.authAgentFor(owner).post('/mcp/api-key/rotate'); + + expect(response.statusCode).toBe(200); + }); + + test('should maintain user association after rotation', async () => { + // Create initial API key + await testServer.authAgentFor(owner).get('/mcp/api-key'); + + // Rotate + const response = await testServer.authAgentFor(owner).post('/mcp/api-key/rotate'); + + expect(response.statusCode).toBe(200); + expect(response.body.data.userId).toBe(owner.id); + + // Verify in database + const apiKeyRepo = Container.get(ApiKeyRepository); + const storedApiKey = await apiKeyRepo.findOne({ + where: { userId: owner.id }, + }); + + expect(storedApiKey).toBeDefined(); + expect(storedApiKey?.apiKey).toBe(response.body.data.apiKey); + }); +}); + +describe('MCP API Key Security', () => { + test('should generate unique API keys', async () => { + const keys = new Set(); + + for (let i = 0; i < 5; i++) { + const user = await createUser({ role: { slug: 'global:member' } }); + const response = await testServer.authAgentFor(user).get('/mcp/api-key'); + keys.add(response.body.data.apiKey); + } + + expect(keys.size).toBe(5); + }); +}); + +describe('MCP API Key Edge Cases', () => { + test('should handle concurrent get requests without creating duplicates', async () => { + const requests = Array(3) + .fill(null) + .map(() => testServer.authAgentFor(owner).get('/mcp/api-key')); + + await Promise.all(requests); + + const apiKeyRepo = Container.get(ApiKeyRepository); + const storedApiKey = await apiKeyRepo.find({ + where: { userId: owner.id }, + }); + expect(storedApiKey.length).toBe(1); + }); +}); diff --git a/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.test.ts b/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.test.ts index 80b9b458910..89d558133a1 100644 --- a/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/mcp.settings.controller.test.ts @@ -1,20 +1,21 @@ import { Logger, ModuleRegistry } from '@n8n/backend-common'; -import { GLOBAL_ADMIN_ROLE, GLOBAL_OWNER_ROLE, type AuthenticatedRequest } from '@n8n/db'; +import { type ApiKey, type AuthenticatedRequest } from '@n8n/db'; import { Container } from '@n8n/di'; import { mock, mockDeep } from 'jest-mock-extended'; -import { ForbiddenError } from '../../../errors/response-errors/forbidden.error'; import { UpdateMcpSettingsDto } from '../dto/update-mcp-settings.dto'; +import { McpServerApiKeyService } from '../mcp-api-key.service'; import { McpSettingsController } from '../mcp.settings.controller'; import { McpSettingsService } from '../mcp.settings.service'; -const createReq = (body: unknown, roleSlug: string): AuthenticatedRequest => - ({ body, user: { role: { slug: roleSlug } } }) as unknown as AuthenticatedRequest; +const createReq = (body: unknown): AuthenticatedRequest => + ({ body }) as unknown as AuthenticatedRequest; describe('McpSettingsController', () => { const logger = mock(); const moduleRegistry = mockDeep(); const mcpSettingsService = mock(); + const mcpServerApiKeyService = mockDeep(); let controller: McpSettingsController; @@ -23,43 +24,16 @@ describe('McpSettingsController', () => { Container.set(Logger, logger); Container.set(McpSettingsService, mcpSettingsService); Container.set(ModuleRegistry, moduleRegistry); + Container.set(McpServerApiKeyService, mcpServerApiKeyService); controller = Container.get(McpSettingsController); }); - test('gets settings correctly', async () => { - mcpSettingsService.getEnabled.mockResolvedValue(true); - await expect(controller.getSettings()).resolves.toEqual({ mcpAccessEnabled: true }); - mcpSettingsService.getEnabled.mockResolvedValue(false); - await expect(controller.getSettings()).resolves.toEqual({ mcpAccessEnabled: false }); - }); - describe('updateSettings', () => { - test('prevents non-admins from updating MCP access', async () => { - const req = createReq({ mcpAccessEnabled: false }, 'member'); - const dto = new UpdateMcpSettingsDto({ mcpAccessEnabled: false }); - const res = new Response(); - await expect(controller.updateSettings(req, res, dto)).rejects.toBeInstanceOf(ForbiddenError); - }); - - test('disables MCP access correctly for instance owners', async () => { - const req = createReq({ mcpAccessEnabled: false }, GLOBAL_OWNER_ROLE.slug); + test('disables MCP access correctly', async () => { + const req = createReq({ mcpAccessEnabled: false }); const dto = new UpdateMcpSettingsDto({ mcpAccessEnabled: false }); mcpSettingsService.setEnabled.mockResolvedValue(undefined); - moduleRegistry.refreshModuleSettings.mockResolvedValue({ mcpAccessEnabled: false }); - - const res = new Response(); - const result = await controller.updateSettings(req, res, dto); - - expect(mcpSettingsService.setEnabled).toHaveBeenCalledWith(false); - expect(moduleRegistry.refreshModuleSettings).toHaveBeenCalledWith('mcp'); - expect(result).toEqual({ mcpAccessEnabled: false }); - }); - - test('disables MCP access correctly for admins', async () => { - const req = createReq({ mcpAccessEnabled: false }, GLOBAL_ADMIN_ROLE.slug); - const dto = new UpdateMcpSettingsDto({ mcpAccessEnabled: false }); - mcpSettingsService.setEnabled.mockResolvedValue(undefined); - moduleRegistry.refreshModuleSettings.mockResolvedValue({ mcpAccessEnabled: false }); + moduleRegistry.refreshModuleSettings.mockResolvedValue(null); const res = new Response(); const result = await controller.updateSettings(req, res, dto); @@ -70,10 +44,10 @@ describe('McpSettingsController', () => { }); test('enables MCP access correctly', async () => { - const req = createReq({ mcpAccessEnabled: true }, GLOBAL_OWNER_ROLE.slug); + const req = createReq({ mcpAccessEnabled: true }); const dto = new UpdateMcpSettingsDto({ mcpAccessEnabled: true }); mcpSettingsService.setEnabled.mockResolvedValue(undefined); - moduleRegistry.refreshModuleSettings.mockResolvedValue({ mcpAccessEnabled: true }); + moduleRegistry.refreshModuleSettings.mockResolvedValue(null); const res = new Response(); const result = await controller.updateSettings(req, res, dto); @@ -83,9 +57,69 @@ describe('McpSettingsController', () => { expect(result).toEqual({ mcpAccessEnabled: true }); }); + test('handles module registry refresh failure gracefully', async () => { + const req = createReq({ mcpAccessEnabled: true }); + const dto = new UpdateMcpSettingsDto({ mcpAccessEnabled: true }); + const error = new Error('Registry sync failed'); + + mcpSettingsService.setEnabled.mockResolvedValue(undefined); + moduleRegistry.refreshModuleSettings.mockRejectedValue(error); + + const res = new Response(); + const result = await controller.updateSettings(req, res, dto); + + expect(mcpSettingsService.setEnabled).toHaveBeenCalledWith(true); + expect(moduleRegistry.refreshModuleSettings).toHaveBeenCalledWith('mcp'); + expect(logger.warn).toHaveBeenCalledWith('Failed to sync MCP settings to module registry', { + cause: 'Registry sync failed', + }); + expect(result).toEqual({ mcpAccessEnabled: true }); + }); + test('requires boolean mcpAccessEnabled value', () => { expect(() => new UpdateMcpSettingsDto({} as never)).toThrow(); expect(() => new UpdateMcpSettingsDto({ mcpAccessEnabled: 'yes' } as never)).toThrow(); }); }); + + describe('getApiKeyForMcpServer', () => { + const mockUser = { id: 'user123', role: { slug: 'member' } }; + const mockApiKey = { + id: 'api-key-123', + key: 'mcp-key-abc123', + userId: 'user123', + createdAt: new Date(), + } as unknown as ApiKey; + + test('returns API key from getOrCreateApiKey', async () => { + const req = { user: mockUser } as AuthenticatedRequest; + mcpServerApiKeyService.getOrCreateApiKey.mockResolvedValue(mockApiKey); + + const result = await controller.getApiKeyForMcpServer(req); + + expect(mcpServerApiKeyService.getOrCreateApiKey).toHaveBeenCalledWith(mockUser); + expect(result).toEqual(mockApiKey); + }); + }); + + describe('rotateApiKeyForMcpServer', () => { + const mockUser = { id: 'user123', role: { slug: 'member' } }; + const mockApiKey = { + id: 'api-key-123', + key: 'mcp-key-abc123', + userId: 'user123', + createdAt: new Date(), + } as unknown as ApiKey; + + test('successfully rotates API key', async () => { + const req = { user: mockUser } as AuthenticatedRequest; + + mcpServerApiKeyService.rotateMcpServerApiKey.mockResolvedValue(mockApiKey); + + const result = await controller.rotateApiKeyForMcpServer(req); + + expect(mcpServerApiKeyService.rotateMcpServerApiKey).toHaveBeenCalledWith(mockUser); + expect(result).toEqual(mockApiKey); + }); + }); }); diff --git a/packages/cli/src/modules/mcp/__tests__/mcp.settings.service.test.ts b/packages/cli/src/modules/mcp/__tests__/mcp.settings.service.test.ts index 96ffeab1201..dddf750ed6e 100644 --- a/packages/cli/src/modules/mcp/__tests__/mcp.settings.service.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/mcp.settings.service.test.ts @@ -1,6 +1,8 @@ import type { Settings, SettingsRepository } from '@n8n/db'; import { mock } from 'jest-mock-extended'; +import type { CacheService } from '@/services/cache/cache.service'; + import { McpSettingsService } from '../mcp.settings.service'; describe('McpSettingsService', () => { @@ -8,13 +10,15 @@ describe('McpSettingsService', () => { let findByKey: jest.Mock, [string]>; let upsert: jest.Mock; let settingsRepository: SettingsRepository; + const cacheService = mock(); beforeEach(() => { jest.clearAllMocks(); findByKey = jest.fn, [string]>(); upsert = jest.fn(); settingsRepository = { findByKey, upsert } as unknown as SettingsRepository; - service = new McpSettingsService(settingsRepository); + + service = new McpSettingsService(settingsRepository, cacheService); }); describe('getEnabled', () => { diff --git a/packages/cli/src/modules/mcp/mcp-api-key.service.ts b/packages/cli/src/modules/mcp/mcp-api-key.service.ts new file mode 100644 index 00000000000..b3001b4ae69 --- /dev/null +++ b/packages/cli/src/modules/mcp/mcp-api-key.service.ts @@ -0,0 +1,177 @@ +import { ApiKey, ApiKeyRepository, AuthenticatedRequest, User, UserRepository } from '@n8n/db'; +import { Service } from '@n8n/di'; +import { EntityManager } from '@n8n/typeorm'; +import { randomUUID } from 'crypto'; +import { NextFunction, Response, Request } from 'express'; +import { ApiKeyAudience } from 'n8n-workflow'; + +import { AuthError } from '@/errors/response-errors/auth.error'; +import { JwtService } from '@/services/jwt.service'; + +const API_KEY_AUDIENCE: ApiKeyAudience = 'mcp-server-api'; +const API_KEY_ISSUER = 'n8n'; +const REDACT_API_KEY_REVEAL_COUNT = 4; +const REDACT_API_KEY_MAX_LENGTH = 10; +const API_KEY_LABEL = 'MCP Server API Key'; +const REDACT_API_KEY_MIN_HIDDEN_CHARS = 6; + +/** + * Service for managing MCP server API keys, including creation, retrieval, deletion, and authentication middleware. + */ +@Service() +export class McpServerApiKeyService { + constructor( + private readonly apiKeyRepository: ApiKeyRepository, + private readonly jwtService: JwtService, + private readonly userRepository: UserRepository, + ) {} + + async createMcpServerApiKey(user: User, trx?: EntityManager) { + const manager = trx ?? this.apiKeyRepository.manager; + + const apiKey = this.jwtService.sign({ + sub: user.id, + iss: API_KEY_ISSUER, + aud: API_KEY_AUDIENCE, + jti: randomUUID(), + }); + + const apiKeyEntity = this.apiKeyRepository.create({ + userId: user.id, + apiKey, + audience: API_KEY_AUDIENCE, + scopes: [], + label: API_KEY_LABEL, + }); + + await manager.insert(ApiKey, apiKeyEntity); + + return await manager.findOneByOrFail(ApiKey, { apiKey }); + } + + async findServerApiKeyForUser(user: User, { redact = true } = {}) { + const apiKey = await this.apiKeyRepository.findOne({ + where: { + userId: user.id, + audience: API_KEY_AUDIENCE, + }, + }); + + if (apiKey && redact) { + apiKey.apiKey = this.redactApiKey(apiKey.apiKey); + } + + return apiKey; + } + + private async getUserForApiKey(apiKey: string) { + return await this.userRepository.findOne({ + where: { + apiKeys: { + apiKey, + audience: API_KEY_AUDIENCE, + }, + }, + relations: ['role'], + }); + } + + async deleteAllMcpApiKeysForUser(user: User, trx?: EntityManager) { + const manager = trx ?? this.apiKeyRepository.manager; + + await manager.delete(ApiKey, { + userId: user.id, + audience: API_KEY_AUDIENCE, + }); + } + + private redactApiKey(apiKey: string) { + if (REDACT_API_KEY_REVEAL_COUNT >= apiKey.length - REDACT_API_KEY_MIN_HIDDEN_CHARS) { + return '*'.repeat(apiKey.length); + } + + const visiblePart = apiKey.slice(-REDACT_API_KEY_REVEAL_COUNT); + const redactedPart = '*'.repeat( + Math.max(0, REDACT_API_KEY_MAX_LENGTH - REDACT_API_KEY_REVEAL_COUNT), + ); + + return redactedPart + visiblePart; + } + + private extractAPIKeyFromHeader(headerValue: string) { + if (!headerValue.startsWith('Bearer')) { + throw new AuthError('Invalid authorization header format'); + } + const apiKeyMatch = headerValue.match(/^Bearer\s+(.+)$/i); + if (apiKeyMatch) { + return apiKeyMatch[1]; + } + throw new AuthError('Invalid authorization header format'); + } + + getAuthMiddleware() { + return async (req: Request, res: Response, next: NextFunction) => { + const authorizationHeader = req.header('authorization'); + + if (!authorizationHeader) { + this.responseWithUnauthorized(res); + return; + } + + const apiKey = this.extractAPIKeyFromHeader(authorizationHeader); + + if (!apiKey) { + this.responseWithUnauthorized(res); + return; + } + + const user = await this.getUserForApiKey(apiKey); + + if (!user) { + this.responseWithUnauthorized(res); + return; + } + + try { + this.jwtService.verify(apiKey, { + issuer: API_KEY_ISSUER, + audience: API_KEY_AUDIENCE, + }); + } catch (e) { + this.responseWithUnauthorized(res); + return; + } + + (req as AuthenticatedRequest).user = user; + + next(); + }; + } + + private responseWithUnauthorized(res: Response) { + res.status(401).send({ message: 'Unauthorized' }); + } + + async getOrCreateApiKey(user: User) { + const apiKey = await this.apiKeyRepository.findOne({ + where: { + userId: user.id, + audience: API_KEY_AUDIENCE, + }, + }); + + if (apiKey) { + apiKey.apiKey = this.redactApiKey(apiKey.apiKey); + return apiKey; + } + + return await this.createMcpServerApiKey(user); + } + + async rotateMcpServerApiKey(user: User) { + return await this.apiKeyRepository.manager.transaction(async (trx) => { + await this.deleteAllMcpApiKeysForUser(user, trx); + return await this.createMcpServerApiKey(user, trx); + }); + } +} diff --git a/packages/cli/src/modules/mcp/mcp.controller.ts b/packages/cli/src/modules/mcp/mcp.controller.ts index de232e8b9a8..cb854413189 100644 --- a/packages/cli/src/modules/mcp/mcp.controller.ts +++ b/packages/cli/src/modules/mcp/mcp.controller.ts @@ -1,15 +1,19 @@ import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; import { AuthenticatedRequest } from '@n8n/db'; import { Post, RootLevelController } from '@n8n/decorators'; +import { Container } from '@n8n/di'; import type { Response } from 'express'; import { ErrorReporter } from 'n8n-core'; +import { McpServerApiKeyService } from './mcp-api-key.service'; import { McpService } from './mcp.service'; import { McpSettingsService } from './mcp.settings.service'; export type FlushableResponse = Response & { flush: () => void }; -@RootLevelController('/mcp-server') +const getAuthMiddleware = () => Container.get(McpServerApiKeyService).getAuthMiddleware(); + +@RootLevelController('/mcp-access') export class McpController { constructor( private readonly errorReporter: ErrorReporter, @@ -17,7 +21,12 @@ export class McpController { private readonly mcpSettingsService: McpSettingsService, ) {} - @Post('/http', { rateLimit: { limit: 100 }, apiKeyAuth: true, usesTemplates: true }) + @Post('/http', { + rateLimit: { limit: 100 }, + middlewares: [getAuthMiddleware()], + skipAuth: true, + usesTemplates: true, + }) async build(req: AuthenticatedRequest, res: FlushableResponse) { // Deny if MCP access is disabled const enabled = await this.mcpSettingsService.getEnabled(); diff --git a/packages/cli/src/modules/mcp/mcp.settings.controller.ts b/packages/cli/src/modules/mcp/mcp.settings.controller.ts index ceec5bf4bfa..b467d184a26 100644 --- a/packages/cli/src/modules/mcp/mcp.settings.controller.ts +++ b/packages/cli/src/modules/mcp/mcp.settings.controller.ts @@ -1,10 +1,10 @@ import { ModuleRegistry, Logger } from '@n8n/backend-common'; -import { GLOBAL_ADMIN_ROLE, GLOBAL_OWNER_ROLE, type AuthenticatedRequest } from '@n8n/db'; -import { Body, Get, Patch, RestController } from '@n8n/decorators'; +import { type AuthenticatedRequest } from '@n8n/db'; +import { Body, Post, Get, Patch, RestController, GlobalScope } from '@n8n/decorators'; import { UpdateMcpSettingsDto } from './dto/update-mcp-settings.dto'; +import { McpServerApiKeyService } from './mcp-api-key.service'; import { McpSettingsService } from './mcp.settings.service'; -import { ForbiddenError } from '../../errors/response-errors/forbidden.error'; @RestController('/mcp') export class McpSettingsController { @@ -12,19 +12,16 @@ export class McpSettingsController { private readonly mcpSettingsService: McpSettingsService, private readonly logger: Logger, private readonly moduleRegistry: ModuleRegistry, + private readonly mcpServerApiKeyService: McpServerApiKeyService, ) {} - @Get('/settings') - async getSettings() { - const mcpAccessEnabled = await this.mcpSettingsService.getEnabled(); - return { mcpAccessEnabled }; - } - + @GlobalScope('mcp:manage') @Patch('/settings') - async updateSettings(req: AuthenticatedRequest, _res: Response, @Body dto: UpdateMcpSettingsDto) { - if (![GLOBAL_OWNER_ROLE.slug, GLOBAL_ADMIN_ROLE.slug].includes(req.user.role?.slug)) { - throw new ForbiddenError('Only admin users can update MCP settings'); - } + async updateSettings( + _req: AuthenticatedRequest, + _res: Response, + @Body dto: UpdateMcpSettingsDto, + ) { const enabled = dto.mcpAccessEnabled; await this.mcpSettingsService.setEnabled(enabled); try { @@ -36,4 +33,16 @@ export class McpSettingsController { } return { mcpAccessEnabled: enabled }; } + + @GlobalScope('mcpApiKey:create') + @Get('/api-key') + async getApiKeyForMcpServer(req: AuthenticatedRequest) { + return await this.mcpServerApiKeyService.getOrCreateApiKey(req.user); + } + + @GlobalScope('mcpApiKey:rotate') + @Post('/api-key/rotate') + async rotateApiKeyForMcpServer(req: AuthenticatedRequest) { + return await this.mcpServerApiKeyService.rotateMcpServerApiKey(req.user); + } } diff --git a/packages/cli/src/modules/mcp/mcp.settings.service.ts b/packages/cli/src/modules/mcp/mcp.settings.service.ts index 7017caf520c..814ec702e95 100644 --- a/packages/cli/src/modules/mcp/mcp.settings.service.ts +++ b/packages/cli/src/modules/mcp/mcp.settings.service.ts @@ -1,21 +1,39 @@ import { SettingsRepository } from '@n8n/db'; import { Service } from '@n8n/di'; +import { CacheService } from '@/services/cache/cache.service'; + const KEY = 'mcp.access.enabled'; @Service() export class McpSettingsService { - constructor(private readonly settingsRepository: SettingsRepository) {} + constructor( + private readonly settingsRepository: SettingsRepository, + private readonly cacheService: CacheService, + ) {} async getEnabled(): Promise { + const isMcpAccessEnabled = await this.cacheService.get(KEY); + + if (isMcpAccessEnabled !== undefined) { + return isMcpAccessEnabled === 'true'; + } + const row = await this.settingsRepository.findByKey(KEY); - // Disabled by default - if (!row) return false; - return row.value === 'true'; + + const enabled = row?.value === 'true'; + + await this.cacheService.set(KEY, enabled.toString()); + + return enabled; } async setEnabled(enabled: boolean): Promise { - const value = enabled ? 'true' : 'false'; - await this.settingsRepository.upsert({ key: KEY, value, loadOnStartup: true }, ['key']); + await this.settingsRepository.upsert( + { key: KEY, value: enabled.toString(), loadOnStartup: true }, + ['key'], + ); + + await this.cacheService.set(KEY, enabled.toString()); } } diff --git a/packages/cli/src/services/public-api-key.service.ts b/packages/cli/src/services/public-api-key.service.ts index 3e13961dd4e..b4e51ead929 100644 --- a/packages/cli/src/services/public-api-key.service.ts +++ b/packages/cli/src/services/public-api-key.service.ts @@ -10,11 +10,11 @@ import type { NextFunction, Request, Response } from 'express'; import { TokenExpiredError } from 'jsonwebtoken'; import type { OpenAPIV3 } from 'openapi-types'; -import { EventService } from '@/events/event.service'; - import { JwtService } from './jwt.service'; import { LastActiveAtService } from './last-active-at.service'; +import { EventService } from '@/events/event.service'; + const API_KEY_AUDIENCE = 'public-api'; const API_KEY_ISSUER = 'n8n'; const REDACT_API_KEY_REVEAL_COUNT = 4; @@ -47,6 +47,7 @@ export class PublicApiKeyService { apiKey, label, scopes, + audience: API_KEY_AUDIENCE, }), ); @@ -58,7 +59,10 @@ export class PublicApiKeyService { * @param user - The user for whom to retrieve and redact API keys. */ async getRedactedApiKeysForUser(user: User) { - const apiKeys = await this.apiKeyRepository.findBy({ userId: user.id }); + const apiKeys = await this.apiKeyRepository.findBy({ + userId: user.id, + audience: API_KEY_AUDIENCE, + }); return apiKeys.map((apiKeyRecord) => ({ ...apiKeyRecord, apiKey: this.redactApiKey(apiKeyRecord.apiKey), @@ -83,6 +87,7 @@ export class PublicApiKeyService { where: { apiKeys: { apiKey, + audience: API_KEY_AUDIENCE, }, }, relations: ['role'], @@ -172,7 +177,7 @@ export class PublicApiKeyService { async apiKeyHasValidScopes(apiKey: string, endpointScope: ApiKeyScope) { const apiKeyData = await this.apiKeyRepository.findOne({ - where: { apiKey }, + where: { apiKey, audience: API_KEY_AUDIENCE }, select: { scopes: true }, }); if (!apiKeyData) return false; @@ -205,7 +210,7 @@ export class PublicApiKeyService { const ownerOnlyScopes = getOwnerOnlyApiKeyScopes(); const userApiKeys = await manager.find(ApiKey, { - where: { userId: user.id }, + where: { userId: user.id, audience: API_KEY_AUDIENCE }, }); const keysWithOwnerScopes = userApiKeys.filter((apiKey) => diff --git a/packages/cli/test/integration/api-keys.api.test.ts b/packages/cli/test/integration/api-keys.api.test.ts index f80ea618c1c..5187c60bedc 100644 --- a/packages/cli/test/integration/api-keys.api.test.ts +++ b/packages/cli/test/integration/api-keys.api.test.ts @@ -85,6 +85,7 @@ describe('Owner shell', () => { createdAt: expect.any(Date), updatedAt: expect.any(Date), scopes: ['workflow:create'], + audience: 'public-api', }); expect(newApiKey.expiresAt).toBeNull(); @@ -124,6 +125,7 @@ describe('Owner shell', () => { createdAt: expect.any(Date), updatedAt: expect.any(Date), scopes: ['workflow:create'], + audience: 'public-api', }); expect(newApiKey.expiresAt).toBe(expiresAt); @@ -155,6 +157,7 @@ describe('Owner shell', () => { createdAt: expect.any(Date), updatedAt: expect.any(Date), scopes: ['user:create'], + audience: 'public-api', }); expect(newApiKey.expiresAt).toBe(expiresAt); @@ -186,6 +189,7 @@ describe('Owner shell', () => { createdAt: expect.any(Date), updatedAt: expect.any(Date), scopes: ['user:create'], + audience: 'public-api', }); }); @@ -214,6 +218,7 @@ describe('Owner shell', () => { createdAt: expect.any(Date), updatedAt: expect.any(Date), scopes: ['user:create', 'workflow:create'], + audience: 'public-api', }); }); @@ -269,6 +274,7 @@ describe('Owner shell', () => { updatedAt: expect.any(String), expiresAt: expirationDateInTheFuture, scopes: ['workflow:create'], + audience: 'public-api', }); expect(retrieveAllApiKeysResponse.body.data[0]).toEqual({ @@ -280,6 +286,7 @@ describe('Owner shell', () => { updatedAt: expect.any(String), expiresAt: null, scopes: ['workflow:create'], + audience: 'public-api', }); }); @@ -344,6 +351,7 @@ describe('Member', () => { createdAt: expect.any(Date), updatedAt: expect.any(Date), scopes: ['workflow:create'], + audience: 'public-api', }); expect(newApiKeyResponse.body.data.expiresAt).toBeNull(); @@ -375,6 +383,7 @@ describe('Member', () => { createdAt: expect.any(Date), updatedAt: expect.any(Date), scopes: ['workflow:create'], + audience: 'public-api', }); expect(newApiKey.expiresAt).toBe(expiresAt); @@ -407,6 +416,7 @@ describe('Member', () => { createdAt: expect.any(Date), updatedAt: expect.any(Date), scopes: ['workflow:create'], + audience: 'public-api', }); expect(newApiKey.expiresAt).toBe(expiresAt); @@ -457,6 +467,7 @@ describe('Member', () => { updatedAt: expect.any(String), expiresAt: expirationDateInTheFuture, scopes: ['workflow:create'], + audience: 'public-api', }); expect(retrieveAllApiKeysResponse.body.data[0]).toEqual({ @@ -468,6 +479,7 @@ describe('Member', () => { updatedAt: expect.any(String), expiresAt: null, scopes: ['workflow:create'], + audience: 'public-api', }); }); diff --git a/packages/cli/test/integration/shared/types.ts b/packages/cli/test/integration/shared/types.ts index 3ea2862ef84..42b1e0eedbd 100644 --- a/packages/cli/test/integration/shared/types.ts +++ b/packages/cli/test/integration/shared/types.ts @@ -46,7 +46,8 @@ type EndpointGroup = | 'data-store' | 'module-settings' | 'data-table' - | 'third-party-licenses'; + | 'third-party-licenses' + | 'mcp'; type ModuleName = 'insights' | 'external-secrets' | 'community-packages' | 'data-table'; diff --git a/packages/cli/test/integration/shared/utils/test-server.ts b/packages/cli/test/integration/shared/utils/test-server.ts index 006d69b748a..d69397dbdf9 100644 --- a/packages/cli/test/integration/shared/utils/test-server.ts +++ b/packages/cli/test/integration/shared/utils/test-server.ts @@ -310,6 +310,10 @@ export const setupTestServer = ({ await import('@/modules/data-table/data-table.module'); break; + case 'mcp': + await import('@/modules/mcp/mcp.module'); + break; + case 'module-settings': await import('@/controllers/module-settings.controller'); break; diff --git a/packages/frontend/editor-ui/src/stores/rbac.store.ts b/packages/frontend/editor-ui/src/stores/rbac.store.ts index f77732d9f75..a5adfb4b197 100644 --- a/packages/frontend/editor-ui/src/stores/rbac.store.ts +++ b/packages/frontend/editor-ui/src/stores/rbac.store.ts @@ -42,6 +42,8 @@ export const useRBACStore = defineStore(STORES.RBAC, () => { execution: {}, workflowTags: {}, role: {}, + mcp: {}, + mcpApiKey: {}, }); function addGlobalRole(role: Role) { diff --git a/packages/workflow/src/interfaces.ts b/packages/workflow/src/interfaces.ts index 8557522439f..7e5c427bd13 100644 --- a/packages/workflow/src/interfaces.ts +++ b/packages/workflow/src/interfaces.ts @@ -3193,3 +3193,5 @@ export interface StructuredChunk { timestamp: number; }; } + +export type ApiKeyAudience = 'public-api' | 'mcp-server-api';