diff --git a/packages/cli/src/modules/mcp/__tests__/mcp-oauth-service.test.ts b/packages/cli/src/modules/mcp/__tests__/mcp-oauth-service.test.ts index 05eca7a3b471..17c0ee18a692 100644 --- a/packages/cli/src/modules/mcp/__tests__/mcp-oauth-service.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/mcp-oauth-service.test.ts @@ -459,6 +459,52 @@ describe('McpOAuthService', () => { }); }); + describe('deleteClient', () => { + it('should delete client when user has consent', async () => { + const client = { + id: 'client-123', + name: 'Test Client', + } as OAuthClient; + + oauthClientRepository.findOne.mockResolvedValue(client); + userConsentRepository.findOneBy.mockResolvedValue({ + userId: 'user-456', + clientId: 'client-123', + } as any); + oauthClientRepository.delete.mockResolvedValue({} as any); + + await service.deleteClient('client-123', 'user-456'); + + expect(oauthClientRepository.delete).toHaveBeenCalledWith({ id: 'client-123' }); + }); + + it('should throw when client does not exist', async () => { + oauthClientRepository.findOne.mockResolvedValue(null); + + await expect(service.deleteClient('nonexistent', 'user-456')).rejects.toThrow( + 'OAuth client with ID nonexistent not found', + ); + + expect(oauthClientRepository.delete).not.toHaveBeenCalled(); + }); + + it('should throw when user has no consent for the client', async () => { + const client = { + id: 'client-123', + name: 'Test Client', + } as OAuthClient; + + oauthClientRepository.findOne.mockResolvedValue(client); + userConsentRepository.findOneBy.mockResolvedValue(null); + + await expect(service.deleteClient('client-123', 'other-user')).rejects.toThrow( + 'OAuth client with ID client-123 not found', + ); + + expect(oauthClientRepository.delete).not.toHaveBeenCalled(); + }); + }); + describe('revokeToken', () => { it('should revoke access token when type hint is access_token', async () => { const client = { diff --git a/packages/cli/src/modules/mcp/__tests__/mcp.oauth-clients.controller.api.test.ts b/packages/cli/src/modules/mcp/__tests__/mcp.oauth-clients.controller.api.test.ts new file mode 100644 index 000000000000..5af386af58f6 --- /dev/null +++ b/packages/cli/src/modules/mcp/__tests__/mcp.oauth-clients.controller.api.test.ts @@ -0,0 +1,89 @@ +import { testDb } from '@n8n/backend-test-utils'; +import type { User } from '@n8n/db'; +import { Container } from '@n8n/di'; + +import { createOwner, createMember } from '@test-integration/db/users'; +import { setupTestServer } from '@test-integration/utils'; + +import { OAuthClientRepository } from '../database/repositories/oauth-client.repository'; +import { UserConsentRepository } from '../database/repositories/oauth-user-consent.repository'; + +const testServer = setupTestServer({ endpointGroups: ['mcp'], modules: ['mcp'] }); + +let owner: User; +let member: User; +let oauthClientRepository: OAuthClientRepository; +let userConsentRepository: UserConsentRepository; + +beforeAll(async () => { + owner = await createOwner(); + member = await createMember(); + oauthClientRepository = Container.get(OAuthClientRepository); + userConsentRepository = Container.get(UserConsentRepository); +}); + +afterEach(async () => { + await testDb.truncate(['OAuthClient', 'UserConsent']); +}); + +describe('DELETE /rest/mcp/oauth-clients/:clientId', () => { + test('should allow a user to delete their own OAuth client', async () => { + const client = await oauthClientRepository.save({ + id: 'owner-client-id', + name: 'Owner Client', + redirectUris: ['https://example.com/callback'], + grantTypes: ['authorization_code'], + tokenEndpointAuthMethod: 'none', + }); + + await userConsentRepository.save({ + userId: owner.id, + clientId: client.id, + grantedAt: Date.now(), + }); + + const response = await testServer.authAgentFor(owner).delete(`/mcp/oauth-clients/${client.id}`); + + expect(response.statusCode).toBe(200); + expect(response.body.data).toMatchObject({ + success: true, + }); + + const deletedClient = await oauthClientRepository.findOneBy({ id: client.id }); + expect(deletedClient).toBeNull(); + }); + + test("should return 404 when a user tries to delete another user's OAuth client", async () => { + const client = await oauthClientRepository.save({ + id: 'owner-only-client', + name: 'Owner Only Client', + redirectUris: ['https://example.com/callback'], + grantTypes: ['authorization_code'], + tokenEndpointAuthMethod: 'none', + }); + + await userConsentRepository.save({ + userId: owner.id, + clientId: client.id, + grantedAt: Date.now(), + }); + + const response = await testServer + .authAgentFor(member) + .delete(`/mcp/oauth-clients/${client.id}`); + + expect(response.statusCode).toBe(404); + + // Verify the client was NOT deleted + const existingClient = await oauthClientRepository.findOneBy({ id: client.id }); + expect(existingClient).not.toBeNull(); + }); + + test('should return 404 when deleting a non-existent OAuth client', async () => { + const response = await testServer + .authAgentFor(owner) + .delete('/mcp/oauth-clients/non-existent-id'); + + expect(response.statusCode).toBe(404); + }); +}); diff --git a/packages/cli/src/modules/mcp/mcp-oauth-service.ts b/packages/cli/src/modules/mcp/mcp-oauth-service.ts index 13c76e58e107..4bbabf21116a 100644 --- a/packages/cli/src/modules/mcp/mcp-oauth-service.ts +++ b/packages/cli/src/modules/mcp/mcp-oauth-service.ts @@ -210,9 +210,10 @@ export class McpOAuthService implements OAuthServerProvider { } /** - * Delete an OAuth client and all related data + * Delete an OAuth client and all related data. + * Verifies that the requesting user has a consent relationship with the client. */ - async deleteClient(clientId: string): Promise { + async deleteClient(clientId: string, userId: string): Promise { // First check if the client exists const client = await this.oauthClientRepository.findOne({ where: { id: clientId }, @@ -222,6 +223,12 @@ export class McpOAuthService 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 }); + if (!consent) { + throw new Error(`OAuth client with ID ${clientId} not found`); + } + this.logger.info('Deleting OAuth client and related data', { clientId }); await this.oauthClientRepository.delete({ id: clientId }); diff --git a/packages/cli/src/modules/mcp/mcp.oauth-clients.controller.ts b/packages/cli/src/modules/mcp/mcp.oauth-clients.controller.ts index 453f8eabddd3..ac1908b83e44 100644 --- a/packages/cli/src/modules/mcp/mcp.oauth-clients.controller.ts +++ b/packages/cli/src/modules/mcp/mcp.oauth-clients.controller.ts @@ -68,7 +68,7 @@ export class McpOAuthClientsController { }); try { - await this.mcpOAuthService.deleteClient(clientId); + await this.mcpOAuthService.deleteClient(clientId, req.user.id); this.logger.info('OAuth client deleted successfully', { clientId,