From bf7cd98e253c8d5382c31c9001df54b83c0edb84 Mon Sep 17 00:00:00 2001 From: Andreas Fitzek Date: Tue, 13 Jan 2026 16:23:41 +0100 Subject: [PATCH] chore(core): Implement revoke endpoint for dynamic credentials (#24230) --- .../dynamic-credentials.controller.test.ts | 252 ++++++++++++++++++ .../dynamic-credentials.controller.ts | 54 +++- 2 files changed, 299 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials.controller.test.ts b/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials.controller.test.ts index 4ad3a4aa6c2..0ea85ff863d 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials.controller.test.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials.controller.test.ts @@ -306,4 +306,256 @@ describe('DynamicCredentialsController', () => { expect(cipher.decrypt).not.toHaveBeenCalled(); }); }); + + describe('revokeCredential', () => { + const mockResolverEntity: DynamicCredentialResolver = { + id: 'resolver-123', + name: 'Test Resolver', + type: 'oauth2-introspection-identifier', + config: 'encrypted-config', + createdAt: new Date(), + updatedAt: new Date(), + generateId: jest.fn(), + setUpdateDate: jest.fn(), + }; + + it('should throw UnauthenticatedError when authorization header is missing', async () => { + const req = mock({ + params: { id: '1' }, + query: { resolverId: 'resolver-123' }, + headers: { authorization: undefined }, + }); + const res = mock(); + + await expect(controller.revokeCredential(req, res)).rejects.toThrow('Unauthenticated'); + }); + + it('should throw BadRequestError when authorization header is malformed', async () => { + const req = mock({ + params: { id: '1' }, + query: { resolverId: 'resolver-123' }, + headers: { authorization: 'InvalidFormat' }, + }); + const res = mock(); + + await expect(controller.revokeCredential(req, res)).rejects.toThrow( + 'Authorization header is malformed', + ); + }); + + it('should throw NotFoundError when credential is not found', async () => { + const req = mock({ + params: { id: 'non-existent-id' }, + query: { resolverId: 'resolver-123' }, + headers: { authorization: 'Bearer token123' }, + }); + const res = mock(); + + enterpriseCredentialsService.getOne.mockResolvedValue(null); + + await expect(controller.revokeCredential(req, res)).rejects.toThrow('Credential not found'); + expect(enterpriseCredentialsService.getOne).toHaveBeenCalledWith('non-existent-id'); + }); + + it('should throw NotFoundError when resolver is not found', async () => { + const mockCredential = mock({ + id: '1', + type: 'googleOAuth2Api', + }); + const req = mock({ + params: { id: '1' }, + query: { resolverId: 'non-existent-resolver' }, + headers: { authorization: 'Bearer token123' }, + }); + const res = mock(); + + enterpriseCredentialsService.getOne.mockResolvedValue(mockCredential); + resolverRepository.findOneBy.mockResolvedValue(null); + + await expect(controller.revokeCredential(req, res)).rejects.toThrow('Resolver not found'); + expect(resolverRepository.findOneBy).toHaveBeenCalledWith({ + id: 'non-existent-resolver', + }); + }); + + it('should successfully revoke credential and return 204 when deleteSecret exists', async () => { + const mockCredential = mock({ + id: '1', + type: 'googleOAuth2Api', + }); + const mockResolver = { + metadata: { + name: 'oauth2-introspection-identifier', + description: 'OAuth2 Introspection Identifier', + }, + getSecret: jest.fn(), + setSecret: jest.fn(), + validateOptions: jest.fn(), + deleteSecret: jest.fn().mockResolvedValue(undefined), + }; + const req = mock({ + params: { id: '1' }, + query: { resolverId: 'resolver-123' }, + headers: { authorization: 'Bearer token123' }, + }); + const res = mock(); + res.status.mockReturnThis(); + + enterpriseCredentialsService.getOne.mockResolvedValue(mockCredential); + resolverRepository.findOneBy.mockResolvedValue(mockResolverEntity); + resolverRegistry.getResolverByTypename.mockReturnValue(mockResolver); + cipher.decrypt.mockReturnValue('{"introspectionUrl":"https://example.com/introspect"}'); + + await controller.revokeCredential(req, res); + + expect(mockResolver.deleteSecret).toHaveBeenCalledTimes(1); + expect(mockResolver.deleteSecret).toHaveBeenCalledWith( + '1', + { identity: 'token123', version: 1 }, + { + configuration: { introspectionUrl: 'https://example.com/introspect' }, + resolverId: 'resolver-123', + resolverName: 'oauth2-introspection-identifier', + }, + ); + expect(cipher.decrypt).toHaveBeenCalledWith('encrypted-config'); + expect(res.status).toHaveBeenCalledWith(204); + expect(res.send).toHaveBeenCalled(); + }); + + it('should return 204 when resolver lacks deleteSecret method', async () => { + const mockCredential = mock({ + id: '1', + type: 'googleOAuth2Api', + }); + const mockResolver = { + metadata: { + name: 'oauth2-introspection-identifier', + description: 'OAuth2 Introspection Identifier', + }, + getSecret: jest.fn(), + setSecret: jest.fn(), + validateOptions: jest.fn(), + // No deleteSecret method + }; + const req = mock({ + params: { id: '1' }, + query: { resolverId: 'resolver-123' }, + headers: { authorization: 'Bearer token123' }, + }); + const res = mock(); + res.status.mockReturnThis(); + + enterpriseCredentialsService.getOne.mockResolvedValue(mockCredential); + resolverRepository.findOneBy.mockResolvedValue(mockResolverEntity); + resolverRegistry.getResolverByTypename.mockReturnValue(mockResolver); + + await controller.revokeCredential(req, res); + + expect(cipher.decrypt).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(204); + expect(res.send).toHaveBeenCalled(); + }); + + it('should pass correct parameters to deleteSecret', async () => { + // Arrange + const mockCredential = mock({ + id: 'cred-456', + type: 'googleOAuth2Api', + }); + const mockResolver = { + metadata: { + name: 'oauth2-introspection-identifier', + description: 'OAuth2 Introspection Identifier', + }, + getSecret: jest.fn(), + setSecret: jest.fn(), + validateOptions: jest.fn(), + deleteSecret: jest.fn().mockResolvedValue(undefined), + }; + const req = mock({ + params: { id: 'cred-456' }, + query: { resolverId: 'resolver-123' }, + headers: { authorization: 'Bearer my-test-token' }, + }); + const res = mock(); + res.status.mockReturnThis(); + + enterpriseCredentialsService.getOne.mockResolvedValue(mockCredential); + resolverRepository.findOneBy.mockResolvedValue(mockResolverEntity); + resolverRegistry.getResolverByTypename.mockReturnValue(mockResolver); + cipher.decrypt.mockReturnValue('{"key":"value","url":"https://test.com"}'); + + // Act + await controller.revokeCredential(req, res); + + // Assert - PRIMARY FOCUS: Explicit parameter contract verification + expect(mockResolver.deleteSecret).toHaveBeenCalledWith( + 'cred-456', + { + identity: 'my-test-token', + version: 1, + }, + { + configuration: { key: 'value', url: 'https://test.com' }, + resolverId: 'resolver-123', + resolverName: 'oauth2-introspection-identifier', + }, + ); + }); + + it('should work with OAuth1 credential type', async () => { + // This test verifies that the revoke endpoint is OAuth-version agnostic + // Unlike authorize (which calls different OAuth1/OAuth2 services), + // revoke treats OAuth1 and OAuth2 the same way + + // Arrange + const mockCredential = mock({ + id: '1', + type: 'twitterOAuth1Api', + }); + const mockResolver = { + metadata: { + name: 'oauth2-introspection-identifier', + description: 'OAuth2 Introspection Identifier', + }, + getSecret: jest.fn(), + setSecret: jest.fn(), + validateOptions: jest.fn(), + deleteSecret: jest.fn().mockResolvedValue(undefined), + }; + const req = mock({ + params: { id: '1' }, + query: { resolverId: 'resolver-123' }, + headers: { authorization: 'Bearer token123' }, + }); + const res = mock(); + res.status.mockReturnThis(); + + enterpriseCredentialsService.getOne.mockResolvedValue(mockCredential); + resolverRepository.findOneBy.mockResolvedValue(mockResolverEntity); + resolverRegistry.getResolverByTypename.mockReturnValue(mockResolver); + cipher.decrypt.mockReturnValue('{"introspectionUrl":"https://example.com/introspect"}'); + + // Act + await controller.revokeCredential(req, res); + + // Assert + // 1. deleteSecret should be called (OAuth1 works the same as OAuth2 for revocation) + expect(mockResolver.deleteSecret).toHaveBeenCalledTimes(1); + expect(mockResolver.deleteSecret).toHaveBeenCalledWith( + '1', + { identity: 'token123', version: 1 }, + { + configuration: { introspectionUrl: 'https://example.com/introspect' }, + resolverId: 'resolver-123', + resolverName: 'oauth2-introspection-identifier', + }, + ); + + // 2. Returns 204 status + expect(res.status).toHaveBeenCalledWith(204); + expect(res.send).toHaveBeenCalled(); + }); + }); }); diff --git a/packages/cli/src/modules/dynamic-credentials.ee/dynamic-credentials.controller.ts b/packages/cli/src/modules/dynamic-credentials.ee/dynamic-credentials.controller.ts index 241c49424db..082b7337edd 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/dynamic-credentials.controller.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/dynamic-credentials.controller.ts @@ -1,4 +1,4 @@ -import { Post, RestController } from '@n8n/decorators'; +import { Delete, Post, RestController } from '@n8n/decorators'; import { Request, Response } from 'express'; import { EnterpriseCredentialsService } from '@/credentials/credentials.service.ee'; @@ -22,10 +22,8 @@ export class DynamicCredentialsController { private readonly cipher: Cipher, ) {} - @Post('/:id/authorize', { skipAuth: true }) - async authorizeCredential(req: Request, _res: Response): Promise { - const credential = await this.enterpriseCredentialsService.getOne(req.params.id); - const token = getBearerToken(req); + private async findCredentialToUse(credentialId: string): Promise { + const credential = await this.enterpriseCredentialsService.getOne(credentialId); if (!credential) { throw new NotFoundError('Credential not found'); @@ -34,8 +32,10 @@ export class DynamicCredentialsController { if (!credential.type.includes('OAuth2') && !credential.type.includes('OAuth1')) { throw new BadRequestError('Credential type not supported'); } + return credential; + } - const resolverId = req.query.resolverId as string | undefined; + private async getResolverInstance(resolverId: string | undefined) { if (!resolverId) { throw new BadRequestError('Missing resolverId query parameter'); } @@ -54,6 +54,46 @@ export class DynamicCredentialsController { if (!resolver) { throw new NotFoundError('Resolver type not found'); } + return { resolver, resolverEntity }; + } + + @Delete('/:id/revoke', { skipAuth: true }) + async revokeCredential(req: Request, res: Response): Promise { + const token = getBearerToken(req); + const credential = await this.findCredentialToUse(req.params.id); + + const resolverId = req.query.resolverId as string | undefined; + const { resolver, resolverEntity } = await this.getResolverInstance(resolverId); + + if (resolver.deleteSecret) { + // Decrypt and parse resolver configuration + const decryptedConfig = this.cipher.decrypt(resolverEntity.config); + const resolverConfig = jsonParse>(decryptedConfig); + + await resolver.deleteSecret( + credential.id, + { + identity: token, + version: 1, + }, + { + configuration: resolverConfig, + resolverId: resolverEntity.id, + resolverName: resolverEntity.type, + }, + ); + } + + res.status(204).send(); // 204 No Content indicates successful deletion + } + + @Post('/:id/authorize', { skipAuth: true }) + async authorizeCredential(req: Request, _res: Response): Promise { + const token = getBearerToken(req); + const credential = await this.findCredentialToUse(req.params.id); + + const resolverId = req.query.resolverId as string | undefined; + const { resolver, resolverEntity } = await this.getResolverInstance(resolverId); if (resolver.validateIdentity) { // Decrypt and parse resolver configuration @@ -61,7 +101,7 @@ export class DynamicCredentialsController { const resolverConfig = jsonParse>(decryptedConfig); await resolver.validateIdentity(token, { - resolverId, + resolverId: resolverEntity.id, resolverName: resolverEntity.type, configuration: resolverConfig, });