From a2c19e46974990c39007ffe64ab2ae908fe2f1b9 Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Mon, 20 Jul 2026 10:33:30 -0400 Subject: [PATCH] feat(editor): Add Mine/All tabs, search and filters to MCP connected clients (#33857) --- packages/@n8n/api-types/src/dto/index.ts | 2 + .../src/dto/oauth/oauth-client.dto.ts | 41 ++ packages/@n8n/api-types/src/index.ts | 9 + .../@n8n/api-types/src/schemas/mcp.schema.ts | 48 ++ .../src/configs/user-management.config.ts | 4 + packages/@n8n/config/test/config.test.ts | 1 + .../oauth-clients.controller.api.test.ts | 415 ++++++++++++++++++ .../__tests__/oauth-server.service.test.ts | 59 ++- .../oauth-user-consent.repository.ts | 93 +++- .../oauth-server/oauth-clients.controller.ts | 43 +- .../oauth-server/oauth-server.service.ts | 156 ++++++- .../__tests__/user-management-mailer.test.ts | 26 ++ .../email/templates/mcp-client-revoked.mjml | 19 + .../email/user-management-mailer.ts | 32 +- .../frontend/@n8n/i18n/src/locales/en.json | 23 +- .../ai/mcpAccess/SettingsMCPView.test.ts | 29 +- .../features/ai/mcpAccess/SettingsMCPView.vue | 78 +++- .../ai/mcpAccess/clients.utils.test.ts | 23 + .../features/ai/mcpAccess/clients.utils.ts | 53 ++- .../ai/mcpAccess/components/MCPEmptyState.vue | 78 ++-- .../components/OAuthClientDetailsModal.vue | 16 + .../RevokeOAuthClientConfirmModal.vue | 60 +++ .../components/tabs/OAuthClientOwnerCell.vue | 20 + .../tabs/OAuthClientsFilters.test.ts | 86 ++++ .../components/tabs/OAuthClientsFilters.vue | 201 +++++++++ .../components/tabs/OAuthClientsTable.test.ts | 166 ++++++- .../components/tabs/OAuthClientsTable.vue | 213 ++++++++- .../src/features/ai/mcpAccess/mcp.api.ts | 25 +- .../features/ai/mcpAccess/mcp.store.test.ts | 178 +++++++- .../src/features/ai/mcpAccess/mcp.store.ts | 92 +++- 30 files changed, 2156 insertions(+), 133 deletions(-) create mode 100644 packages/cli/src/user-management/email/templates/mcp-client-revoked.mjml create mode 100644 packages/frontend/editor-ui/src/features/ai/mcpAccess/clients.utils.test.ts create mode 100644 packages/frontend/editor-ui/src/features/ai/mcpAccess/components/RevokeOAuthClientConfirmModal.vue create mode 100644 packages/frontend/editor-ui/src/features/ai/mcpAccess/components/tabs/OAuthClientOwnerCell.vue create mode 100644 packages/frontend/editor-ui/src/features/ai/mcpAccess/components/tabs/OAuthClientsFilters.test.ts create mode 100644 packages/frontend/editor-ui/src/features/ai/mcpAccess/components/tabs/OAuthClientsFilters.vue diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index 161bd0be672..404c8a18e1a 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -240,7 +240,9 @@ export * from './evaluations'; export { OAuthClientResponseDto, + ListOAuthClientsQueryDto, ListOAuthClientsResponseDto, + DeleteOAuthClientQueryDto, DeleteOAuthClientResponseDto, InstanceMcpClientStatsResponseDto, } from './oauth/oauth-client.dto'; diff --git a/packages/@n8n/api-types/src/dto/oauth/oauth-client.dto.ts b/packages/@n8n/api-types/src/dto/oauth/oauth-client.dto.ts index a97de3de97f..e6a3b4085c3 100644 --- a/packages/@n8n/api-types/src/dto/oauth/oauth-client.dto.ts +++ b/packages/@n8n/api-types/src/dto/oauth/oauth-client.dto.ts @@ -1,6 +1,15 @@ import { z } from 'zod'; +import { MCP_CLIENT_CONNECTED_PERIODS, MCP_CLIENT_TYPE_FILTERS } from '../../schemas/mcp.schema'; import { Z } from '../../zod-class'; +import { paginationSchema } from '../pagination/pagination.dto'; + +const oauthClientOwnerShape = z.object({ + id: z.string(), + firstName: z.string().nullable(), + lastName: z.string().nullable(), + email: z.string(), +}); const oauthClientShape = { id: z.string(), @@ -14,6 +23,8 @@ const oauthClientShape = { grantedAt: z.number(), /** Scopes granted on the consent screen. */ scopes: z.array(z.string()), + /** Consent owner; present only when listing with ownership=all. */ + owner: oauthClientOwnerShape.optional(), }; /** @@ -21,14 +32,44 @@ const oauthClientShape = { */ export class OAuthClientResponseDto extends Z.class(oauthClientShape) {} +/** + * DTO for the OAuth clients list query + */ +export class ListOAuthClientsQueryDto extends Z.class({ + ...paginationSchema, + /** 'all' requires mcp:manage; defaults to 'mine'. */ + ownership: z.enum(['mine', 'all']).optional(), + /** Case-insensitive substring match against the client's name. */ + name: z.string().trim().min(1).max(100).optional(), + /** Narrow the `all` view to consents of this user. Ignored without mcp:manage. */ + ownerId: z.string().max(36).optional(), + /** Client type bucket, resolved from the client name via the shared brand matchers. */ + type: z.enum(MCP_CLIENT_TYPE_FILTERS).optional(), + /** Date bucket applied to the consent's grantedAt. */ + connected: z.enum(MCP_CLIENT_CONNECTED_PERIODS).optional(), +}) {} + /** * DTO for listing OAuth clients response */ export class ListOAuthClientsResponseDto extends Z.class({ data: z.array(z.object(oauthClientShape)), + /** Total rows matching the filters (across all pages) for the current ownership. */ count: z.number(), /** Tool names each grantable scope unlocks on this instance, for the client details view. */ scopeTools: z.record(z.array(z.string())).optional(), + /** Unfiltered per-ownership totals for the tab badges. `all` only for mcp:manage callers. */ + totals: z.object({ mine: z.number(), all: z.number().optional() }), + /** Distinct consent owners for the "Connected by" filter; only for mcp:manage callers. */ + owners: z.array(oauthClientOwnerShape).optional(), +}) {} + +/** + * DTO for the OAuth client delete query + */ +export class DeleteOAuthClientQueryDto extends Z.class({ + /** Consent owner whose grant to revoke; defaults to the caller. Other users require mcp:manage. */ + userId: z.string().max(36).optional(), }) {} /** diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index 58cd70a6958..c70ece80db3 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -513,7 +513,16 @@ export { MCP_APPS_VARIANT_CONTROL, MCP_APPS_VARIANT_ENABLED, MCP_INSTANCE_SCOPES, + MCP_CLIENT_BRAND_MATCHERS, + MCP_CLIENT_TYPE_FILTERS, + MCP_CLIENT_TYPE_FILTER_BUCKETS, + MCP_CLIENT_CONNECTED_PERIODS, + getMcpClientType, type McpScope, + type McpClientType, + type McpClientBrandName, + type McpClientTypeFilter, + type McpClientConnectedPeriod, } from './schemas/mcp.schema'; export { diff --git a/packages/@n8n/api-types/src/schemas/mcp.schema.ts b/packages/@n8n/api-types/src/schemas/mcp.schema.ts index b1f7f087330..d8b4c9f5776 100644 --- a/packages/@n8n/api-types/src/schemas/mcp.schema.ts +++ b/packages/@n8n/api-types/src/schemas/mcp.schema.ts @@ -29,3 +29,51 @@ export const MCP_INSTANCE_SCOPES = [ ] as const; export type McpScope = (typeof MCP_INSTANCE_SCOPES)[number]; + +export type McpClientType = 'cli' | 'ide' | 'editor' | 'assistant'; + +/** Known client brands, used by the FE to pick the logo shown next to a client. */ +export type McpClientBrandName = 'claude' | 'cursor' | 'vscode' | 'openai'; + +/** + * Client names are free-form (self-reported at OAuth registration), so known + * clients are recognized by name patterns. First match wins: more specific + * patterns (e.g. "Claude Code") must come before broader ones ("Claude"). + * Shared FE/BE so the type shown in the clients table and the server-side + * type filter cannot drift. + */ +export const MCP_CLIENT_BRAND_MATCHERS: ReadonlyArray<{ + pattern: RegExp; + brand: McpClientBrandName; + type: McpClientType; +}> = [ + { pattern: /claude[ -]?code/i, brand: 'claude', type: 'cli' }, + { pattern: /claude/i, brand: 'claude', type: 'assistant' }, + { pattern: /cursor/i, brand: 'cursor', type: 'ide' }, + { pattern: /(visual studio code|vs ?code)/i, brand: 'vscode', type: 'editor' }, + { pattern: /codex/i, brand: 'openai', type: 'cli' }, + { pattern: /chatgpt|openai/i, brand: 'openai', type: 'assistant' }, +]; + +export function getMcpClientType(clientName: string): McpClientType | null { + return MCP_CLIENT_BRAND_MATCHERS.find(({ pattern }) => pattern.test(clientName))?.type ?? null; +} + +/** + * Client-type buckets offered by the connected-clients filter (per design), + * coarser than the derived brand types shown in the table rows. + */ +export const MCP_CLIENT_TYPE_FILTERS = ['ide', 'cli', 'web'] as const; + +export type McpClientTypeFilter = (typeof MCP_CLIENT_TYPE_FILTERS)[number]; + +export const MCP_CLIENT_TYPE_FILTER_BUCKETS: Record = { + ide: ['ide', 'editor'], + cli: ['cli'], + web: ['assistant'], +}; + +/** Date buckets for the "Connected" filter, applied to the consent's grantedAt. */ +export const MCP_CLIENT_CONNECTED_PERIODS = ['last7', 'last30', 'older'] as const; + +export type McpClientConnectedPeriod = (typeof MCP_CLIENT_CONNECTED_PERIODS)[number]; diff --git a/packages/@n8n/config/src/configs/user-management.config.ts b/packages/@n8n/config/src/configs/user-management.config.ts index 20748e17697..534f50ac4c0 100644 --- a/packages/@n8n/config/src/configs/user-management.config.ts +++ b/packages/@n8n/config/src/configs/user-management.config.ts @@ -81,6 +81,10 @@ export class TemplateConfig { /** Overrides default HTML template for notifying a user that their public API key was revoked by an admin (use full path) */ @Env('N8N_UM_EMAIL_TEMPLATES_API_KEY_REVOKED') 'api-key-revoked': string = ''; + + /** Overrides default HTML template for notifying a user that their connected MCP client was revoked by an admin (use full path) */ + @Env('N8N_UM_EMAIL_TEMPLATES_MCP_CLIENT_REVOKED') + 'mcp-client-revoked': string = ''; } const emailModeSchema = z.enum(['', 'smtp']); diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index a057c580bfc..da38d14ecf6 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -170,6 +170,7 @@ describe('GlobalConfig', () => { 'workflow-shared': '', 'project-shared': '', 'api-key-revoked': '', + 'mcp-client-revoked': '', }, }, }, diff --git a/packages/cli/src/modules/oauth-server/__tests__/oauth-clients.controller.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/oauth-clients.controller.api.test.ts index 7074950352e..3ad7a1a0355 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/oauth-clients.controller.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/oauth-clients.controller.api.test.ts @@ -71,6 +71,279 @@ describe('GET /rest/mcp/oauth-clients', () => { scopes: ['workflow:read', 'execution:read'], }); }); + + test('should include the all-consents total for managers even on the mine view', async () => { + const client = await oauthClientRepository.save({ + id: 'totals-client', + name: 'Totals Client', + redirectUris: ['https://example.com/callback'], + grantTypes: ['authorization_code'], + tokenEndpointAuthMethod: 'none', + }); + + await userConsentRepository.save({ + userId: owner.id, + clientId: client.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + await userConsentRepository.save({ + userId: member.id, + clientId: client.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + + const ownerResponse = await testServer.authAgentFor(owner).get('/mcp/oauth-clients'); + + expect(ownerResponse.statusCode).toBe(200); + expect(ownerResponse.body.data.totals).toEqual({ mine: 1, all: 2 }); + expect(ownerResponse.body.data.data[0].owner).toBeUndefined(); + + const memberResponse = await testServer.authAgentFor(member).get('/mcp/oauth-clients'); + + expect(memberResponse.statusCode).toBe(200); + expect(memberResponse.body.data.totals).toEqual({ mine: 1 }); + }); +}); + +describe('GET /rest/mcp/oauth-clients?ownership=all', () => { + test("should return every user's consents with owner info for a manager", async () => { + const sharedClient = await oauthClientRepository.save({ + id: 'all-shared-client', + name: 'Shared Client', + redirectUris: ['https://example.com/callback'], + grantTypes: ['authorization_code'], + tokenEndpointAuthMethod: 'none', + }); + const memberClient = await oauthClientRepository.save({ + id: 'all-member-client', + name: 'Member Client', + redirectUris: ['https://example.com/callback'], + grantTypes: ['authorization_code'], + tokenEndpointAuthMethod: 'none', + }); + + await userConsentRepository.save({ + userId: owner.id, + clientId: sharedClient.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + await userConsentRepository.save({ + userId: member.id, + clientId: sharedClient.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + await userConsentRepository.save({ + userId: member.id, + clientId: memberClient.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + + const response = await testServer + .authAgentFor(owner) + .get('/mcp/oauth-clients') + .query({ ownership: 'all' }); + + expect(response.statusCode).toBe(200); + expect(response.body.data.data).toHaveLength(3); + expect(response.body.data.totals).toEqual({ mine: 1, all: 3 }); + + // a consent row is (client × owner): the shared client appears once per user + const sharedRows = response.body.data.data.filter( + (row: { id: string }) => row.id === sharedClient.id, + ); + expect(sharedRows).toHaveLength(2); + expect(sharedRows.map((row: { owner: { id: string } }) => row.owner.id).sort()).toEqual( + [owner.id, member.id].sort(), + ); + expect(sharedRows[0].owner).toMatchObject({ + id: expect.any(String), + email: expect.any(String), + }); + }); + + test('should return 403 for a member', async () => { + const response = await testServer + .authAgentFor(member) + .get('/mcp/oauth-clients') + .query({ ownership: 'all' }); + + expect(response.statusCode).toBe(403); + }); +}); + +describe('GET /rest/mcp/oauth-clients pagination and filters', () => { + const saveClient = async (id: string, name: string) => + await oauthClientRepository.save({ + id, + name, + redirectUris: ['https://example.com/callback'], + grantTypes: ['authorization_code'], + tokenEndpointAuthMethod: 'none', + }); + + test('should page through results with take/skip while reporting the filtered total', async () => { + const now = Date.now(); + for (let i = 0; i < 3; i++) { + const client = await saveClient(`page-client-${i}`, `Page Client ${i}`); + // distinct grantedAt so the DESC ordering is deterministic + await userConsentRepository.save({ + userId: owner.id, + clientId: client.id, + grantedAt: now - i * 1000, + scope: ['workflow:read'], + }); + } + + const firstPage = await testServer + .authAgentFor(owner) + .get('/mcp/oauth-clients') + .query({ take: 2, skip: 0 }); + + expect(firstPage.statusCode).toBe(200); + expect(firstPage.body.data.count).toBe(3); + expect(firstPage.body.data.data.map((row: { id: string }) => row.id)).toEqual([ + 'page-client-0', + 'page-client-1', + ]); + + const secondPage = await testServer + .authAgentFor(owner) + .get('/mcp/oauth-clients') + .query({ take: 2, skip: 2 }); + + expect(secondPage.body.data.count).toBe(3); + expect(secondPage.body.data.data.map((row: { id: string }) => row.id)).toEqual([ + 'page-client-2', + ]); + }); + + test('should filter by client name, case-insensitively', async () => { + const cursor = await saveClient('name-cursor', 'Cursor'); + const claude = await saveClient('name-claude', 'Claude Code'); + await userConsentRepository.save({ + userId: owner.id, + clientId: cursor.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + await userConsentRepository.save({ + userId: owner.id, + clientId: claude.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + + const response = await testServer + .authAgentFor(owner) + .get('/mcp/oauth-clients') + .query({ name: 'CURSOR' }); + + expect(response.statusCode).toBe(200); + expect(response.body.data.count).toBe(1); + expect(response.body.data.data[0].id).toBe('name-cursor'); + }); + + test('should filter by client type bucket derived from the name', async () => { + const ide = await saveClient('type-ide', 'Cursor'); + const editor = await saveClient('type-editor', 'VS Code'); + const cli = await saveClient('type-cli', 'Claude Code'); + const web = await saveClient('type-web', 'ChatGPT'); + for (const client of [ide, editor, cli, web]) { + await userConsentRepository.save({ + userId: owner.id, + clientId: client.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + } + + const ideResponse = await testServer + .authAgentFor(owner) + .get('/mcp/oauth-clients') + .query({ type: 'ide' }); + + // the IDE bucket covers both ide (Cursor) and editor (VS Code) brands + expect(ideResponse.body.data.data.map((row: { id: string }) => row.id).sort()).toEqual([ + 'type-editor', + 'type-ide', + ]); + + const webResponse = await testServer + .authAgentFor(owner) + .get('/mcp/oauth-clients') + .query({ type: 'web' }); + + expect(webResponse.body.data.data.map((row: { id: string }) => row.id)).toEqual(['type-web']); + }); + + test('should filter by the connected date bucket', async () => { + const now = Date.now(); + const day = 24 * 60 * 60 * 1000; + const recent = await saveClient('date-recent', 'Recent Client'); + const old = await saveClient('date-old', 'Old Client'); + await userConsentRepository.save({ + userId: owner.id, + clientId: recent.id, + grantedAt: now - 2 * day, + scope: ['workflow:read'], + }); + await userConsentRepository.save({ + userId: owner.id, + clientId: old.id, + grantedAt: now - 60 * day, + scope: ['workflow:read'], + }); + + const last7 = await testServer + .authAgentFor(owner) + .get('/mcp/oauth-clients') + .query({ connected: 'last7' }); + + expect(last7.body.data.data.map((row: { id: string }) => row.id)).toEqual(['date-recent']); + + const older = await testServer + .authAgentFor(owner) + .get('/mcp/oauth-clients') + .query({ connected: 'older' }); + + expect(older.body.data.data.map((row: { id: string }) => row.id)).toEqual(['date-old']); + }); + + test('should return the distinct unfiltered owners in the all view', async () => { + const client = await saveClient('owners-client', 'Owners Client'); + await userConsentRepository.save({ + userId: owner.id, + clientId: client.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + await userConsentRepository.save({ + userId: member.id, + clientId: client.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + + const response = await testServer + .authAgentFor(owner) + .get('/mcp/oauth-clients') + .query({ ownership: 'all', name: 'no such client' }); + + expect(response.statusCode).toBe(200); + // filters narrow the rows but not the owners offered by the filter dropdown + expect(response.body.data.count).toBe(0); + expect(response.body.data.owners.map((o: { id: string }) => o.id).sort()).toEqual( + [owner.id, member.id].sort(), + ); + + const mineResponse = await testServer.authAgentFor(owner).get('/mcp/oauth-clients'); + expect(mineResponse.body.data.owners).toBeUndefined(); + }); }); describe('GET /rest/mcp/oauth-clients/instance-stats', () => { @@ -220,4 +493,146 @@ describe('DELETE /rest/mcp/oauth-clients/:clientId', () => { expect(response.statusCode).toBe(404); }); + + test("should allow a manager to revoke another user's grant", async () => { + const client = await oauthClientRepository.save({ + id: 'admin-revoke-client', + name: 'Admin Revoke Client', + redirectUris: ['https://example.com/callback'], + grantTypes: ['authorization_code'], + tokenEndpointAuthMethod: 'none', + }); + + await userConsentRepository.save({ + userId: owner.id, + clientId: client.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + await userConsentRepository.save({ + userId: member.id, + clientId: client.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + + const response = await testServer + .authAgentFor(owner) + .delete(`/mcp/oauth-clients/${client.id}`) + .query({ userId: member.id }); + + expect(response.statusCode).toBe(200); + + // the member's consent is gone, the owner's grant and the client survive + expect( + await userConsentRepository.findOneBy({ userId: member.id, clientId: client.id }), + ).toBeNull(); + expect( + await userConsentRepository.findOneBy({ userId: owner.id, clientId: client.id }), + ).not.toBeNull(); + expect(await oauthClientRepository.findOneBy({ id: client.id })).not.toBeNull(); + }); + + test('should garbage-collect the client when a manager revokes its last grant', async () => { + const client = await oauthClientRepository.save({ + id: 'admin-gc-client', + name: 'Admin GC Client', + redirectUris: ['https://example.com/callback'], + grantTypes: ['authorization_code'], + tokenEndpointAuthMethod: 'none', + }); + + await userConsentRepository.save({ + userId: member.id, + clientId: client.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + + const response = await testServer + .authAgentFor(owner) + .delete(`/mcp/oauth-clients/${client.id}`) + .query({ userId: member.id }); + + expect(response.statusCode).toBe(200); + expect(await oauthClientRepository.findOneBy({ id: client.id })).toBeNull(); + }); + + test("should return 403 when a member passes another user's userId", async () => { + const client = await oauthClientRepository.save({ + id: 'member-forbidden-client', + name: 'Member Forbidden Client', + redirectUris: ['https://example.com/callback'], + grantTypes: ['authorization_code'], + tokenEndpointAuthMethod: 'none', + }); + + await userConsentRepository.save({ + userId: owner.id, + clientId: client.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + + const response = await testServer + .authAgentFor(member) + .delete(`/mcp/oauth-clients/${client.id}`) + .query({ userId: owner.id }); + + expect(response.statusCode).toBe(403); + expect( + await userConsentRepository.findOneBy({ userId: owner.id, clientId: client.id }), + ).not.toBeNull(); + }); + + test('should treat a member passing their own userId like the default delete', async () => { + const client = await oauthClientRepository.save({ + id: 'member-self-client', + name: 'Member Self Client', + redirectUris: ['https://example.com/callback'], + grantTypes: ['authorization_code'], + tokenEndpointAuthMethod: 'none', + }); + + await userConsentRepository.save({ + userId: member.id, + clientId: client.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + + const response = await testServer + .authAgentFor(member) + .delete(`/mcp/oauth-clients/${client.id}`) + .query({ userId: member.id }); + + expect(response.statusCode).toBe(200); + expect( + await userConsentRepository.findOneBy({ userId: member.id, clientId: client.id }), + ).toBeNull(); + }); + + test('should return 404 when the target user has no grant for the client', async () => { + const client = await oauthClientRepository.save({ + id: 'no-consent-client', + name: 'No Consent Client', + redirectUris: ['https://example.com/callback'], + grantTypes: ['authorization_code'], + tokenEndpointAuthMethod: 'none', + }); + + await userConsentRepository.save({ + userId: owner.id, + clientId: client.id, + grantedAt: Date.now(), + scope: ['workflow:read'], + }); + + const response = await testServer + .authAgentFor(owner) + .delete(`/mcp/oauth-clients/${client.id}`) + .query({ userId: member.id }); + + expect(response.statusCode).toBe(404); + }); }); diff --git a/packages/cli/src/modules/oauth-server/__tests__/oauth-server.service.test.ts b/packages/cli/src/modules/oauth-server/__tests__/oauth-server.service.test.ts index b05494174c0..fb2893f5abc 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/oauth-server.service.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/oauth-server.service.test.ts @@ -22,6 +22,7 @@ import type { McpConfig } from '@/modules/mcp/mcp.config'; import type { McpSettingsService } from '@/modules/mcp/mcp.settings.service'; import { ProtectedResourceRegistry } from '@/services/protected-resource.registry'; import type { UrlService } from '@/services/url.service'; +import { UserManagementMailer } from '@/user-management/email'; const SUPPORTED_SCOPES = ['tool:listWorkflows', 'tool:getWorkflowDetails']; const TEST_RESOURCE_URL = 'https://n8n.example.com/mcp-server/http'; @@ -33,6 +34,7 @@ let tokenService: Mocked; let authorizationCodeService: Mocked; let service: OAuthServerService; let userConsentRepository: Mocked; +let mailer: Mocked; let getAllowedRedirectUris: Mock<() => Promise>; describe('OAuthServerService', () => { @@ -43,6 +45,7 @@ describe('OAuthServerService', () => { tokenService = mockInstance(OAuthTokenService); authorizationCodeService = mockInstance(OAuthAuthorizationCodeService); userConsentRepository = mockInstance(UserConsentRepository); + mailer = mockInstance(UserManagementMailer); getAllowedRedirectUris = vi.fn<(...args: []) => Promise>().mockResolvedValue([]); const resourceRegistry = new ProtectedResourceRegistry(mock()); @@ -65,6 +68,7 @@ describe('OAuthServerService', () => { authorizationCodeService, userConsentRepository, resourceRegistry, + mailer, ); }); @@ -905,9 +909,10 @@ describe('OAuthServerService', () => { } as OAuthClient; oauthClientRepository.findOne.mockResolvedValue(client); - userConsentRepository.findOneBy.mockResolvedValue({ + userConsentRepository.findOne.mockResolvedValue({ userId: 'user-456', clientId: 'client-123', + user: { id: 'user-456', email: 'owner@n8n.io', firstName: 'Owner' }, } as any); const execute = mockGarbageCollect(0); @@ -932,9 +937,10 @@ describe('OAuthServerService', () => { } as OAuthClient; oauthClientRepository.findOne.mockResolvedValue(client); - userConsentRepository.findOneBy.mockResolvedValue({ + userConsentRepository.findOne.mockResolvedValue({ userId: 'user-456', clientId: 'client-123', + user: { id: 'user-456', email: 'owner@n8n.io', firstName: 'Owner' }, } as any); const execute = mockGarbageCollect(1); @@ -947,6 +953,51 @@ describe('OAuthServerService', () => { expect(execute).toHaveBeenCalled(); }); + it('should notify the grant owner by email when someone else revokes their client', async () => { + const client = { + id: 'client-123', + name: 'Test Client', + } as OAuthClient; + + oauthClientRepository.findOne.mockResolvedValue(client); + userConsentRepository.findOne.mockResolvedValue({ + userId: 'user-456', + clientId: 'client-123', + user: { id: 'user-456', email: 'owner@n8n.io', firstName: 'Owner' }, + } as any); + userConsentRepository.countBy.mockResolvedValue(1); + mailer.notifyMcpClientRevoked.mockResolvedValue({ emailSent: true }); + + const revoker = { id: 'admin-1', email: 'admin@n8n.io' } as any; + await service.deleteClient('client-123', 'user-456', revoker); + + expect(mailer.notifyMcpClientRevoked).toHaveBeenCalledWith({ + clientName: 'Test Client', + owner: { id: 'user-456', email: 'owner@n8n.io', firstName: 'Owner' }, + revoker, + }); + }); + + it('should not send an email when users revoke their own client', async () => { + const client = { + id: 'client-123', + name: 'Test Client', + } as OAuthClient; + + oauthClientRepository.findOne.mockResolvedValue(client); + userConsentRepository.findOne.mockResolvedValue({ + userId: 'user-456', + clientId: 'client-123', + user: { id: 'user-456', email: 'owner@n8n.io', firstName: 'Owner' }, + } as any); + userConsentRepository.countBy.mockResolvedValue(1); + + const revoker = { id: 'user-456', email: 'owner@n8n.io' } as any; + await service.deleteClient('client-123', 'user-456', revoker); + + expect(mailer.notifyMcpClientRevoked).not.toHaveBeenCalled(); + }); + it('should throw when client does not exist', async () => { oauthClientRepository.findOne.mockResolvedValue(null); @@ -964,7 +1015,7 @@ describe('OAuthServerService', () => { } as OAuthClient; oauthClientRepository.findOne.mockResolvedValue(client); - userConsentRepository.findOneBy.mockResolvedValue(null); + userConsentRepository.findOne.mockResolvedValue(null); await expect(service.deleteClient('client-123', 'other-user')).rejects.toThrow( 'OAuth client with ID client-123 not found', @@ -1125,6 +1176,7 @@ describe('OAuthServerService', () => { authorizationCodeService, userConsentRepository, multiRegistry, + mailer, ); expect( @@ -1169,6 +1221,7 @@ describe('OAuthServerService', () => { authorizationCodeService, userConsentRepository, configuredRegistry, + mailer, ); }; diff --git a/packages/cli/src/modules/oauth-server/database/repositories/oauth-user-consent.repository.ts b/packages/cli/src/modules/oauth-server/database/repositories/oauth-user-consent.repository.ts index a4d13c377cb..1cabc26c996 100644 --- a/packages/cli/src/modules/oauth-server/database/repositories/oauth-user-consent.repository.ts +++ b/packages/cli/src/modules/oauth-server/database/repositories/oauth-user-consent.repository.ts @@ -1,8 +1,42 @@ +import type { McpClientConnectedPeriod } from '@n8n/api-types'; import { Service } from '@n8n/di'; import { DataSource, Repository } from '@n8n/typeorm'; import { UserConsent } from '../entities/oauth-user-consent.entity'; +const DAY_IN_MS = 24 * 60 * 60 * 1000; + +type ConsentOwner = { + id: string; + firstName: string | null; + lastName: string | null; + email: string; +}; + +export type FindConnectedClientsOptions = { + /** Restrict to a single user's consents (the "mine" view). */ + userId?: string; + /** Join the owner so the caller can surface it (the "all" view). */ + withOwner?: boolean; + /** Case-insensitive substring match on the client name. */ + name?: string; + /** Restrict to a single owner (managers filtering the "all" view). */ + ownerId?: string; + /** + * Restrict to these client ids. Used for the name-pattern "type" filter, + * which is resolved to matching clients up front (bounded by the registered + * client cap) so paging stays in SQL. Must be non-empty when provided. + */ + clientIds?: string[]; + /** Date bucket applied to the consent's `grantedAt`. */ + connected?: McpClientConnectedPeriod; + /** Reference timestamp for the `connected` buckets. */ + now: number; + skip?: number; + /** When undefined, all matching rows are returned (no pagination). */ + take?: number; +}; + @Service() export class UserConsentRepository extends Repository { constructor(dataSource: DataSource) { @@ -10,13 +44,58 @@ export class UserConsentRepository extends Repository { } /** - * Find all consents for a user with client information + * Connected clients (a consent joined with its client) matching the given + * filters, newest grant first. Name/owner/date filters and pagination run + * in SQL; the returned `total` is the full match count ignoring skip/take. + * The `type` filter is a name-pattern match the DB can't express and is + * applied by the caller. */ - async findByUserWithClient(userId: string): Promise { - return await this.find({ - where: { userId }, - relations: ['client'], - order: { grantedAt: 'DESC' }, - }); + async findConnectedClients( + options: FindConnectedClientsOptions, + ): Promise<{ rows: UserConsent[]; total: number }> { + const qb = this.createQueryBuilder('consent').leftJoinAndSelect('consent.client', 'client'); + + if (options.withOwner) qb.leftJoinAndSelect('consent.user', 'user'); + if (options.userId) qb.andWhere('consent.userId = :userId', { userId: options.userId }); + if (options.ownerId) qb.andWhere('consent.userId = :ownerId', { ownerId: options.ownerId }); + if (options.clientIds) { + qb.andWhere('consent.clientId IN (:...clientIds)', { clientIds: options.clientIds }); + } + + if (options.name?.trim()) { + qb.andWhere('LOWER(client.name) LIKE :name', { + name: `%${options.name.trim().toLowerCase()}%`, + }); + } + + if (options.connected === 'last7') { + qb.andWhere('consent.grantedAt >= :bound', { bound: options.now - 7 * DAY_IN_MS }); + } else if (options.connected === 'last30') { + qb.andWhere('consent.grantedAt >= :bound', { bound: options.now - 30 * DAY_IN_MS }); + } else if (options.connected === 'older') { + qb.andWhere('consent.grantedAt < :bound', { bound: options.now - 30 * DAY_IN_MS }); + } + + qb.orderBy('consent.grantedAt', 'DESC'); + + if (options.take !== undefined) qb.skip(options.skip ?? 0).take(options.take); + + const [rows, total] = await qb.getManyAndCount(); + return { rows, total }; + } + + /** + * Distinct owners across every consent, for the "Connected by" filter. Not + * scoped by the current filters, so the dropdown always lists all owners. + */ + async findConsentOwners(): Promise { + return await this.createQueryBuilder('consent') + .innerJoin('consent.user', 'user') + .select('user.id', 'id') + .addSelect('user.firstName', 'firstName') + .addSelect('user.lastName', 'lastName') + .addSelect('user.email', 'email') + .distinct(true) + .getRawMany(); } } diff --git a/packages/cli/src/modules/oauth-server/oauth-clients.controller.ts b/packages/cli/src/modules/oauth-server/oauth-clients.controller.ts index b746b10be8a..42be5d206fa 100644 --- a/packages/cli/src/modules/oauth-server/oauth-clients.controller.ts +++ b/packages/cli/src/modules/oauth-server/oauth-clients.controller.ts @@ -1,14 +1,18 @@ import { + DeleteOAuthClientQueryDto, DeleteOAuthClientResponseDto, InstanceMcpClientStatsResponseDto, + ListOAuthClientsQueryDto, ListOAuthClientsResponseDto, OAuthClientResponseDto, } from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; import { AuthenticatedRequest } from '@n8n/db'; -import { Delete, Get, GlobalScope, Param, RestController } from '@n8n/decorators'; +import { Delete, Get, GlobalScope, Param, Query, RestController } from '@n8n/decorators'; +import { hasGlobalScope } from '@n8n/permissions'; import type { Response } from 'express'; +import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { OAuthServerService } from './oauth-server.service'; @@ -21,19 +25,27 @@ export class OAuthClientsController { ) {} /** - * Get all OAuth clients for the current user + * Get connected OAuth clients. Defaults to the current user's consents; + * `ownership=all` returns every user's consents and requires `mcp:manage`. */ @GlobalScope('mcp:oauth') @Get('/') async getAllClients( req: AuthenticatedRequest, _res: Response, + @Query query: ListOAuthClientsQueryDto, ): Promise { - this.logger.debug('Fetching all OAuth clients for user', { userId: req.user.id }); + this.logger.debug('Fetching OAuth clients for user', { + userId: req.user.id, + ownership: query.ownership ?? 'mine', + }); - const clients = await this.oauthServerService.getAllClients(req.user.id); + const { clients, count, totals, owners } = await this.oauthServerService.getAllClients( + req.user, + query, + ); - this.logger.debug(`Found ${clients.length} OAuth clients`); + this.logger.debug(`Found ${count} OAuth clients`); const clientDtos: OAuthClientResponseDto[] = clients.map((client) => ({ id: client.id, @@ -45,12 +57,15 @@ export class OAuthClientsController { updatedAt: client.updatedAt.toISOString(), grantedAt: client.grantedAt, scopes: client.scopes, + owner: client.owner, })); return { data: clientDtos, - count: clients.length, + count, scopeTools: this.oauthServerService.getInstanceScopeTools(), + totals, + owners, }; } @@ -66,8 +81,10 @@ export class OAuthClientsController { } /** - * Delete an OAuth client by ID - * This will cascade delete all related tokens, authorization codes, and user consents + * Revoke a user's grant for an OAuth client (tokens, authorization codes, + * consent). Defaults to the caller's own grant; revoking another user's + * grant requires `mcp:manage`. The client registration is garbage-collected + * once its last consent is gone. */ @GlobalScope('mcp:oauth') @Delete('/:clientId') @@ -75,19 +92,27 @@ export class OAuthClientsController { req: AuthenticatedRequest, _res: Response, @Param('clientId') clientId: string, + @Query query: DeleteOAuthClientQueryDto, ): Promise { + const targetUserId = query.userId ?? req.user.id; + if (targetUserId !== req.user.id && !hasGlobalScope(req.user, 'mcp:manage')) { + throw new ForbiddenError('You are not allowed to revoke connected clients of other users'); + } + this.logger.info('Deleting OAuth client', { clientId, userId: req.user.id, + targetUserId, userEmail: req.user.email, }); try { - await this.oauthServerService.deleteClient(clientId, req.user.id); + await this.oauthServerService.deleteClient(clientId, targetUserId, req.user); this.logger.info('OAuth client deleted successfully', { clientId, userId: req.user.id, + targetUserId, }); return { diff --git a/packages/cli/src/modules/oauth-server/oauth-server.service.ts b/packages/cli/src/modules/oauth-server/oauth-server.service.ts index 8c41158ad0e..9347cd834bc 100644 --- a/packages/cli/src/modules/oauth-server/oauth-server.service.ts +++ b/packages/cli/src/modules/oauth-server/oauth-server.service.ts @@ -13,9 +13,13 @@ import type { OAuthTokens, OAuthTokenRevocationRequest, } from '@modelcontextprotocol/sdk/shared/auth.js'; +import type { McpClientConnectedPeriod, McpClientTypeFilter } from '@n8n/api-types'; +import { getMcpClientType, MCP_CLIENT_TYPE_FILTER_BUCKETS } from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; import { GlobalConfig } from '@n8n/config'; +import type { User } from '@n8n/db'; import { Service } from '@n8n/di'; +import { hasGlobalScope } from '@n8n/permissions'; import type { Response } from 'express'; import { ProtectedResourceRegistry } from '@/services/protected-resource.registry'; @@ -27,10 +31,19 @@ import { OAuthAuthorizationCodeService } from './oauth-authorization-code.servic import { OAuthSessionService } from './oauth-session.service'; import { OAuthTokenService } from './oauth-token.service'; import { OAuthClientLimitReachedError } from './oauth.errors'; +import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; +import { UserManagementMailer } from '@/user-management/email'; /** Maximum number of redirect URIs per client */ const MAX_REDIRECT_URIS = 10; +export type ConnectedOAuthClientOwner = { + id: string; + firstName: string | null; + lastName: string | null; + email: string; +}; + /** A client the user has consented to, enriched with the grant details of the consent. */ export type ConnectedOAuthClient = Omit< OAuthClient, @@ -38,8 +51,38 @@ export type ConnectedOAuthClient = Omit< > & { grantedAt: number; scopes: string[]; + /** Consent owner; present only when listing across users (ownership=all). */ + owner?: ConnectedOAuthClientOwner; }; +/** Per-ownership consent totals for the connected-clients tab badges. */ +export type ConnectedOAuthClientTotals = { mine: number; all?: number }; + +export type ListConnectedClientsOptions = { + ownership?: 'mine' | 'all'; + skip?: number; + take?: number; + name?: string; + ownerId?: string; + type?: McpClientTypeFilter; + connected?: McpClientConnectedPeriod; +}; + +/** Whether a client's derived brand type falls in the requested filter bucket. */ +function matchesTypeFilter(name: string, type: McpClientTypeFilter): boolean { + const clientType = getMcpClientType(name); + return clientType !== null && MCP_CLIENT_TYPE_FILTER_BUCKETS[type].includes(clientType); +} + +/** Sort owners by display name so the "Connected by" dropdown reads naturally. */ +function sortOwners(owners: ConnectedOAuthClientOwner[]): ConnectedOAuthClientOwner[] { + return [...owners].sort((a, b) => { + const nameA = [a.firstName, a.lastName].filter(Boolean).join(' ') || a.email; + const nameB = [b.firstName, b.lastName].filter(Boolean).join(' ') || b.email; + return nameA.localeCompare(nameB); + }); +} + /** Maximum length for a single redirect URI */ const MAX_REDIRECT_URI_LENGTH = 2048; @@ -58,6 +101,7 @@ export class OAuthServerService implements OAuthServerProvider { private readonly authorizationCodeService: OAuthAuthorizationCodeService, private readonly userConsentRepository: UserConsentRepository, private readonly resourceRegistry: ProtectedResourceRegistry, + private readonly mailer: UserManagementMailer, ) {} get clientsStore(): OAuthRegisteredClientsStore { @@ -430,23 +474,94 @@ export class OAuthServerService implements OAuthServerProvider { } /** - * Get all OAuth clients the user has consented to (excluding sensitive - * data), together with the grant details of the consent itself. + * Get OAuth clients users have consented to (excluding sensitive data), + * together with the grant details of each consent. `ownership: 'all'` + * returns every user's consents with owner info and requires `mcp:manage`. + * + * Filters and pagination are applied in memory after loading the ownership's + * consents: the set is small (bounded by the instance client cap) and the + * type filter reuses the shared name-pattern matchers, which SQL can't + * express. `count` is the filtered total, `clients` the requested page. */ - async getAllClients(userId: string): Promise { - // Get all consents for the user with client information - const userConsents = await this.userConsentRepository.findByUserWithClient(userId); + async getAllClients( + user: User, + options: ListConnectedClientsOptions = {}, + ): Promise<{ + clients: ConnectedOAuthClient[]; + count: number; + totals: ConnectedOAuthClientTotals; + owners?: ConnectedOAuthClientOwner[]; + }> { + const canSeeAll = hasGlobalScope(user, 'mcp:manage'); + const listAll = options.ownership === 'all'; - // Extract and sanitize the client information - return userConsents.map((consent) => { + if (listAll && !canSeeAll) { + throw new ForbiddenError('You are not allowed to list connected clients of other users'); + } + + // The `type` filter is a name-pattern match SQL can't express. Resolve it + // to the matching client ids first — bounded by the registered client cap, + // not the (client × user) consent set — so filtering and paging stay in SQL. + let clientIds: string[] | undefined; + if (options.type) { + const registered = await this.oauthClientRepository.find({ + select: { id: true, name: true }, + }); + clientIds = registered + .filter((client) => matchesTypeFilter(client.name, options.type!)) + .map((client) => client.id); + } + + const { rows: consents, total } = + clientIds?.length === 0 + ? { rows: [], total: 0 } + : await this.userConsentRepository.findConnectedClients({ + userId: listAll ? undefined : user.id, + withOwner: listAll, + name: options.name, + ownerId: listAll ? options.ownerId : undefined, + clientIds, + connected: options.connected, + now: Date.now(), + skip: options.skip, + take: options.take, + }); + + const clients: ConnectedOAuthClient[] = consents.map((consent) => { const { clientSecret, clientSecretExpiresAt, ...sanitizedClient } = consent.client; return { ...sanitizedClient, // bigint columns come back as strings on Postgres grantedAt: Number(consent.grantedAt), scopes: consent.scope, + ...(listAll + ? { + owner: { + id: consent.user.id, + firstName: consent.user.firstName ?? null, + lastName: consent.user.lastName ?? null, + email: consent.user.email, + }, + } + : {}), }; }); + const count = total; + + // Owners and the tab totals reflect the unfiltered set, so they come from + // dedicated counts rather than the filtered page above. + const [consentOwners, mineCount, allCount] = await Promise.all([ + listAll ? this.userConsentRepository.findConsentOwners() : undefined, + this.userConsentRepository.countBy({ userId: user.id }), + canSeeAll ? this.userConsentRepository.count() : undefined, + ]); + const owners = consentOwners ? sortOwners(consentOwners) : undefined; + const totals: ConnectedOAuthClientTotals = { mine: mineCount }; + if (allCount !== undefined) { + totals.all = allCount; + } + + return { clients, count, totals, owners }; } /** Tool names each scope unlocks on this instance, for the clients list UI. */ @@ -455,12 +570,14 @@ export class OAuthServerService implements OAuthServerProvider { } /** - * Revoke the requesting user's grant for a client: their consent, tokens, - * and authorization codes. Other users' grants for the same client are + * Revoke a user's grant for a client: their consent, tokens, and + * authorization codes. Other users' grants for the same client are * untouched. The client registration itself is garbage-collected once the * last consent is gone, freeing a slot under the instance client cap. + * When a `revoker` other than the grant owner is given (admin revoke), + * the owner is notified by email. */ - async deleteClient(clientId: string, userId: string): Promise { + async deleteClient(clientId: string, userId: string, revoker?: User): Promise { // First check if the client exists const client = await this.oauthClientRepository.findOne({ where: { id: clientId }, @@ -470,8 +587,11 @@ export class OAuthServerService implements OAuthServerProvider { throw new Error(`OAuth client with ID ${clientId} not found`); } - // Verify the requesting user has a consent relationship with this client - const consent = await this.userConsentRepository.findOneBy({ clientId, userId }); + // Verify the target user has a consent relationship with this client + const consent = await this.userConsentRepository.findOne({ + where: { clientId, userId }, + relations: ['user'], + }); if (!consent) { throw new Error(`OAuth client with ID ${clientId} not found`); } @@ -507,6 +627,18 @@ export class OAuthServerService implements OAuthServerProvider { clientName: client.name, }); } + + if (revoker && revoker.id !== userId) { + this.mailer + .notifyMcpClientRevoked({ clientName: client.name, owner: consent.user, revoker }) + .catch((e) => { + this.logger.error('Failed to send MCP client revocation email', { + clientId, + ownerId: userId, + error: e instanceof Error ? e.message : String(e), + }); + }); + } } } diff --git a/packages/cli/src/user-management/email/__tests__/user-management-mailer.test.ts b/packages/cli/src/user-management/email/__tests__/user-management-mailer.test.ts index f74ea5dc90d..9efa7997b19 100644 --- a/packages/cli/src/user-management/email/__tests__/user-management-mailer.test.ts +++ b/packages/cli/src/user-management/email/__tests__/user-management-mailer.test.ts @@ -218,6 +218,32 @@ describe('UserManagementMailer', () => { expect(callBody).toContain('jan@acme.test'); }); + it('should send mcp client revoked notifications', async () => { + const revoker = mock({ + firstName: 'Jan', + lastName: 'Ostrówka', + email: 'jan@acme.test', + }); + + const result = await userManagementMailer.notifyMcpClientRevoked({ + clientName: 'Claude Code', + owner: { email: 'owner@example.com', firstName: 'Maria' }, + revoker, + }); + + expect(result.emailSent).toBe(true); + expect(nodeMailer.sendMail).toHaveBeenCalledWith({ + emailRecipients: 'owner@example.com', + subject: 'Your n8n MCP client access was revoked', + body: expect.stringContaining('href="https://n8n.url/settings/mcp"'), + }); + + const callBody = nodeMailer.sendMail.mock.calls[0][0].body as string; + expect(callBody).toContain('Claude Code'); + expect(callBody).toContain('Jan Ostrówka'); + expect(callBody).toMatch(/\d{1,2} [A-Z][a-z]{2} \d{4}/); + }); + it('should send project share notifications', async () => { const sharer = mock({ firstName: 'Sharer', email: 'sharer@user.com' }); const newSharees = [ diff --git a/packages/cli/src/user-management/email/templates/mcp-client-revoked.mjml b/packages/cli/src/user-management/email/templates/mcp-client-revoked.mjml new file mode 100644 index 00000000000..687bb15c892 --- /dev/null +++ b/packages/cli/src/user-management/email/templates/mcp-client-revoked.mjml @@ -0,0 +1,19 @@ + + + + + + + Your MCP client access was revoked + + Your connected MCP client "{{clientName}}" on {{domain}} was revoked by {{revokedBy}} on {{revokedAt}}. + + + The client has lost access to this instance and any sessions using it have stopped working. If you still need access, you can reconnect the client. + + Manage MCP access + + + + + diff --git a/packages/cli/src/user-management/email/user-management-mailer.ts b/packages/cli/src/user-management/email/user-management-mailer.ts index d0588be22f6..0cc33c00bc7 100644 --- a/packages/cli/src/user-management/email/user-management-mailer.ts +++ b/packages/cli/src/user-management/email/user-management-mailer.ts @@ -47,7 +47,8 @@ type TemplateName = | 'credentials-shared' | 'project-shared' | 'workflow-failure' - | 'api-key-revoked'; + | 'api-key-revoked' + | 'mcp-client-revoked'; @Service() export class UserManagementMailer { @@ -126,6 +127,35 @@ export class UserManagementMailer { }); } + async notifyMcpClientRevoked({ + clientName, + owner, + revoker, + }: { + clientName: string; + owner: { email: string; firstName?: string | null }; + revoker: User; + }): Promise { + if (!this.mailer) return { emailSent: false }; + + const baseUrl = this.urlService.getInstanceBaseUrl(); + const template = await this.getTemplate('mcp-client-revoked'); + + return await this.mailer.sendMail({ + emailRecipients: owner.email, + subject: 'Your n8n MCP client access was revoked', + body: template({ + ...this.basePayload, + email: owner.email, + firstName: owner.firstName ?? 'there', + clientName, + revokedBy: formatRevokedBy(revoker), + revokedAt: formatRevokedAt(new Date()), + mcpSettingsUrl: `${baseUrl}/settings/mcp`, + }), + }); + } + async workflowFailure(data: { email: string; firstName?: string; diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 8e56c09fbf4..34e83e612d0 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -3176,7 +3176,7 @@ "settings.mcp.header.toggle.disabled": "Disabled", "settings.mcp.actionBox.heading": "Connect AI assistants to build and run workflows", "settings.mcp.actionBox.button.label": "Enable MCP access", - "settings.mcp.emptyState.docs.part1": "Read our docs to", + "settings.mcp.emptyState.description": "Let MCP clients like Claude Code and Cursor build, run, and iterate on workflows in your instance.", "settings.mcp.tabs.workflows": "Workflows", "settings.mcp.tabs.oauth": "Connected clients", "settings.mcp.tabs.oauthSettings": "OAuth settings", @@ -3238,6 +3238,27 @@ "settings.mcp.oAuthClients.revoke.success.title": "Access revoked", "settings.mcp.oAuthClients.revoke.success.message": "Client {name} access has been revoked", "settings.mcp.oAuthClients.revoke.error": "Error revoking client access", + "settings.mcp.oAuthClients.revoke.title": "Revoke access for \"{name}\"?", + "settings.mcp.oAuthClients.revoke.description.own": "The client will lose access to this instance immediately and will need to reconnect. This can't be undone.", + "settings.mcp.oAuthClients.revoke.description.other": "The client {ownerName} connected will lose access to this instance immediately and will need to reconnect. This can't be undone.", + "settings.mcp.oAuthClients.revoke.button": "Revoke", + "settings.mcp.oAuthClients.tabs.mine": "Mine", + "settings.mcp.oAuthClients.tabs.all": "All", + "settings.mcp.oAuthClients.search.placeholder": "Search clients", + "settings.mcp.oAuthClients.search.noResults": "No clients match your search or filters", + "settings.mcp.oAuthClients.filters.clientType": "Client type", + "settings.mcp.oAuthClients.filters.clientType.all": "All types", + "settings.mcp.oAuthClients.filters.clientType.ide": "IDE", + "settings.mcp.oAuthClients.filters.clientType.cli": "CLI", + "settings.mcp.oAuthClients.filters.clientType.web": "Web", + "settings.mcp.oAuthClients.filters.connectedBy": "Connected by", + "settings.mcp.oAuthClients.filters.connectedBy.all": "All users", + "settings.mcp.oAuthClients.filters.connected": "Connected", + "settings.mcp.oAuthClients.filters.connected.allTime": "All time", + "settings.mcp.oAuthClients.filters.connected.lastXDays": "Last {count} days", + "settings.mcp.oAuthClients.filters.connected.older": "Older", + "settings.mcp.oAuthClients.table.connectedBy": "Connected by", + "settings.mcp.oAuthClients.details.connectedBy": "Connected by", "settings.mcp.refresh.tooltip": "Refresh list", "settings.mcp.connectWorkflows": "Enable workflows", "settings.mcp.connectWorkflows.modalTitle": "Enable workflow MCP access", diff --git a/packages/frontend/editor-ui/src/features/ai/mcpAccess/SettingsMCPView.test.ts b/packages/frontend/editor-ui/src/features/ai/mcpAccess/SettingsMCPView.test.ts index 237eddb1d24..20d1ca2b597 100644 --- a/packages/frontend/editor-ui/src/features/ai/mcpAccess/SettingsMCPView.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/mcpAccess/SettingsMCPView.test.ts @@ -83,7 +83,8 @@ const createComponent = createComponentRenderer(SettingsMCPView, { }, OAuthClientsTable: { inheritAttrs: true, - template: '
OAuth Clients Table
', + template: + "
OAuth Clients Table
", }, }, }, @@ -349,6 +350,32 @@ describe('SettingsMCPView', () => { }); }); + it('should confirm before revoking and pass the consent owner to the store', async () => { + const { getByTestId, container } = createComponent({ pinia }); + await nextTick(); + + await clickTab(container, 'tab-oauth'); + await waitFor(() => { + expect(getByTestId('mcp-oauth-clients-table')).toBeVisible(); + }); + + await userEvent.click(getByTestId('stub-revoke-client')); + + // nothing is revoked until the dialog is confirmed + await waitFor(() => { + expect( + within(document.body).getByText('Revoke access for "Claude Code"?'), + ).toBeInTheDocument(); + }); + expect(mcpStore.removeOAuthClient).not.toHaveBeenCalled(); + + await userEvent.click(within(document.body).getByRole('button', { name: 'Revoke' })); + + await waitFor(() => { + expect(mcpStore.removeOAuthClient).toHaveBeenCalledWith('client-1', 'user-2'); + }); + }); + it('should switch back to Workflows tab and show workflows table', async () => { const { getByTestId, queryByTestId, container } = createComponent({ pinia }); await nextTick(); diff --git a/packages/frontend/editor-ui/src/features/ai/mcpAccess/SettingsMCPView.vue b/packages/frontend/editor-ui/src/features/ai/mcpAccess/SettingsMCPView.vue index 893b8978ff1..3ba6c29ec04 100644 --- a/packages/frontend/editor-ui/src/features/ai/mcpAccess/SettingsMCPView.vue +++ b/packages/frontend/editor-ui/src/features/ai/mcpAccess/SettingsMCPView.vue @@ -6,6 +6,7 @@ import { useI18n } from '@n8n/i18n'; import { computed, onMounted, ref } from 'vue'; import { useMCPStore } from '@/features/ai/mcpAccess/mcp.store'; import { useUIStore } from '@/app/stores/ui.store'; +import { useUsersStore } from '@/features/settings/users/users.store'; import { hasPermission } from '@/app/utils/rbac/permissions'; import { LOADING_INDICATOR_TIMEOUT, @@ -16,6 +17,7 @@ import MCPEmptyState from '@/features/ai/mcpAccess/components/MCPEmptyState.vue' import MCpHeaderActions from '@/features/ai/mcpAccess/components/header/MCPHeaderActions.vue'; import WorkflowsTable from '@/features/ai/mcpAccess/components/tabs/WorkflowsTable.vue'; import OAuthClientsTable from '@/features/ai/mcpAccess/components/tabs/OAuthClientsTable.vue'; +import RevokeOAuthClientConfirmModal from '@/features/ai/mcpAccess/components/RevokeOAuthClientConfirmModal.vue'; import { N8nHeading, N8nNotice, @@ -29,6 +31,7 @@ import { } from '@n8n/design-system'; import type { TabOptions } from '@n8n/design-system'; import { useMcp } from '@/features/ai/mcpAccess/composables/useMcp'; +import type { OAuthClientFilters } from '@/features/ai/mcpAccess/clients.utils'; import type { OAuthClientResponseDto } from '@n8n/api-types'; import { useTelemetry } from '@/app/composables/useTelemetry'; import { WORKFLOW_DESCRIPTION_MODAL_KEY } from '@/app/constants'; @@ -45,6 +48,7 @@ const telemetry = useTelemetry(); const mcpStore = useMCPStore(); const uiStore = useUIStore(); +const usersStore = useUsersStore(); const { offerToExposeAllWorkflows } = useExposeAllWorkflowsToMcpOffer(); const mcpStatusLoading = ref(false); @@ -85,7 +89,8 @@ const workflowsTableState = ref({ const workflowsTableItemsPerPage = ref(workflowsTableState.value.itemsPerPage); const oAuthClientsLoading = ref(false); -const connectedOAuthClients = ref([]); +const revokeClient = ref(null); +const revoking = ref(false); const redirectUrisInput = ref(''); const redirectUrisError = ref(''); @@ -117,7 +122,7 @@ const onTabSelected = async (tab: MCPTabs) => { selectedTab.value = tab; if (tab === 'workflows' && availableWorkflows.value.length === 0) { await fetchAvailableWorkflows(); - } else if (tab === 'oauth' && connectedOAuthClients.value.length === 0) { + } else if (tab === 'oauth' && mcpStore.oauthClients.length === 0) { await fetchoAuthCLients(); telemetry.track('User clicked connected clients tab'); } else if (tab === 'settings' && mcpStore.allowedRedirectUris.length === 0) { @@ -282,8 +287,7 @@ const onWorkflowsTableUpdate = async (options: TableOptions) => { const fetchoAuthCLients = async () => { try { oAuthClientsLoading.value = true; - const clients = await mcpStore.getAllOAuthClients(); - connectedOAuthClients.value = clients ?? []; + await mcpStore.getAllOAuthClients(); } catch (error) { toast.showError(error, i18n.baseText('settings.mcp.error.fetching.oAuthClients')); } finally { @@ -293,10 +297,48 @@ const fetchoAuthCLients = async () => { } }; -const revokeClientAccess = async (client: OAuthClientResponseDto) => { +const onOwnershipChange = async (ownership: 'mine' | 'all') => { try { - await mcpStore.removeOAuthClient(client.id); - connectedOAuthClients.value = connectedOAuthClients.value.filter((c) => c.id !== client.id); + oAuthClientsLoading.value = true; + await mcpStore.setOAuthClientsOwnership(ownership); + if (ownership === 'all') { + telemetry.track('User viewed all MCP clients'); + } + } catch (error) { + toast.showError(error, i18n.baseText('settings.mcp.error.fetching.oAuthClients')); + } finally { + setTimeout(() => { + oAuthClientsLoading.value = false; + }, LOADING_INDICATOR_TIMEOUT); + } +}; + +const withClientsErrorToast = async (fn: () => Promise) => { + try { + await fn(); + } catch (error) { + toast.showError(error, i18n.baseText('settings.mcp.error.fetching.oAuthClients')); + } +}; + +const onClientsFiltersChange = async (filters: OAuthClientFilters) => + await withClientsErrorToast(() => mcpStore.setOAuthClientsFilters(filters)); + +const onClientsOptionsChange = async (options: { page: number; itemsPerPage: number }) => + await withClientsErrorToast(() => + mcpStore.setOAuthClientsPagination(options.page, options.itemsPerPage), + ); + +const onRevokeRequest = (client: OAuthClientResponseDto) => { + revokeClient.value = client; +}; + +const onRevokeConfirm = async () => { + const client = revokeClient.value; + if (!client) return; + try { + revoking.value = true; + await mcpStore.removeOAuthClient(client.id, client.owner?.id); toast.showMessage({ type: 'success', title: i18n.baseText('settings.mcp.oAuthClients.revoke.success.title'), @@ -306,6 +348,9 @@ const revokeClientAccess = async (client: OAuthClientResponseDto) => { }); } catch (error) { toast.showError(error, i18n.baseText('settings.mcp.oAuthClients.revoke.error')); + } finally { + revoking.value = false; + revokeClient.value = null; } }; @@ -502,10 +547,13 @@ onMounted(async () => {
{
+ + diff --git a/packages/frontend/editor-ui/src/features/ai/mcpAccess/clients.utils.test.ts b/packages/frontend/editor-ui/src/features/ai/mcpAccess/clients.utils.test.ts new file mode 100644 index 00000000000..058554b307e --- /dev/null +++ b/packages/frontend/editor-ui/src/features/ai/mcpAccess/clients.utils.test.ts @@ -0,0 +1,23 @@ +import { describe, it, expect } from 'vitest'; + +import { getClientBrand } from './clients.utils'; + +describe('getClientBrand', () => { + it.each([ + ['Claude Code', 'cli'], + ['Claude', 'assistant'], + ['Cursor', 'ide'], + ['Visual Studio Code', 'editor'], + ['Codex CLI', 'cli'], + ['ChatGPT', 'assistant'], + ['Some Unknown Client', null], + ])('derives the type of %s as %s', (name, type) => { + expect(getClientBrand(name).type).toBe(type); + }); + + it('resolves a logo for recognized brands and none for unknown clients', () => { + expect(getClientBrand('Claude Code').icon).not.toBeNull(); + expect(getClientBrand('Cursor').icon).not.toBeNull(); + expect(getClientBrand('Some Unknown Client').icon).toBeNull(); + }); +}); diff --git a/packages/frontend/editor-ui/src/features/ai/mcpAccess/clients.utils.ts b/packages/frontend/editor-ui/src/features/ai/mcpAccess/clients.utils.ts index 53891a0888b..2f641e3e20d 100644 --- a/packages/frontend/editor-ui/src/features/ai/mcpAccess/clients.utils.ts +++ b/packages/frontend/editor-ui/src/features/ai/mcpAccess/clients.utils.ts @@ -1,6 +1,12 @@ import type { Component } from 'vue'; -import { MCP_INSTANCE_SCOPES } from '@n8n/api-types'; +import { MCP_CLIENT_BRAND_MATCHERS, MCP_INSTANCE_SCOPES } from '@n8n/api-types'; +import type { + McpClientBrandName, + McpClientConnectedPeriod, + McpClientType, + McpClientTypeFilter, +} from '@n8n/api-types'; import type { BaseTextKey } from '@n8n/i18n'; import ClaudeIcon from './assets/client-icons/claude.svg?component'; @@ -8,26 +14,18 @@ import CursorIcon from './assets/client-icons/cursor.svg?component'; import OpenAiIcon from './assets/client-icons/openai.svg?component'; import VsCodeIcon from './assets/client-icons/vscode.svg?component'; -export type McpClientType = 'cli' | 'ide' | 'editor' | 'assistant'; - export interface McpClientBrand { icon: Component | null; type: McpClientType | null; } -/** - * Client names are free-form (self-reported at OAuth registration), so known - * clients are recognized by name patterns. First match wins: more specific - * patterns (e.g. "Claude Code") must come before broader ones ("Claude"). - */ -const BRAND_MATCHERS: Array<{ pattern: RegExp; brand: McpClientBrand }> = [ - { pattern: /claude[ -]?code/i, brand: { icon: ClaudeIcon, type: 'cli' } }, - { pattern: /claude/i, brand: { icon: ClaudeIcon, type: 'assistant' } }, - { pattern: /cursor/i, brand: { icon: CursorIcon, type: 'ide' } }, - { pattern: /(visual studio code|vs ?code)/i, brand: { icon: VsCodeIcon, type: 'editor' } }, - { pattern: /codex/i, brand: { icon: OpenAiIcon, type: 'cli' } }, - { pattern: /chatgpt|openai/i, brand: { icon: OpenAiIcon, type: 'assistant' } }, -]; +/** Logos for the brands recognized by the shared name-pattern matchers. */ +const BRAND_ICONS: Record = { + claude: ClaudeIcon, + cursor: CursorIcon, + vscode: VsCodeIcon, + openai: OpenAiIcon, +}; // Client names are bounded (a user's own registered clients), so memoizing the // regex scan by name is safe; the map would need eviction only if that set ever @@ -37,10 +35,10 @@ const brandCache = new Map(); export function getClientBrand(clientName: string): McpClientBrand { let brand = brandCache.get(clientName); if (!brand) { - brand = BRAND_MATCHERS.find(({ pattern }) => pattern.test(clientName))?.brand ?? { - icon: null, - type: null, - }; + const match = MCP_CLIENT_BRAND_MATCHERS.find(({ pattern }) => pattern.test(clientName)); + brand = match + ? { icon: BRAND_ICONS[match.brand], type: match.type } + : { icon: null, type: null }; brandCache.set(clientName, brand); } return brand; @@ -75,3 +73,18 @@ export function scopeLabel( export function isFullAccessGrant(scopes: string[]): boolean { return scopes.length > 0 && MCP_INSTANCE_SCOPES.every((scope) => scopes.includes(scope)); } + +/** UI state of the connected-clients search + filter popover; applied server-side. */ +export interface OAuthClientFilters { + search: string; + type: McpClientTypeFilter | null; + ownerId: string | null; + connected: McpClientConnectedPeriod | null; +} + +export const EMPTY_OAUTH_CLIENT_FILTERS: OAuthClientFilters = { + search: '', + type: null, + ownerId: null, + connected: null, +}; diff --git a/packages/frontend/editor-ui/src/features/ai/mcpAccess/components/MCPEmptyState.vue b/packages/frontend/editor-ui/src/features/ai/mcpAccess/components/MCPEmptyState.vue index e2d5cb843f8..c4220ca19b4 100644 --- a/packages/frontend/editor-ui/src/features/ai/mcpAccess/components/MCPEmptyState.vue +++ b/packages/frontend/editor-ui/src/features/ai/mcpAccess/components/MCPEmptyState.vue @@ -1,9 +1,14 @@