chore(core): Implement revoke endpoint for dynamic credentials (#24230)

2026-10-29 19:10:43 +08:00 · 2026-01-13 15:23:41 +00:00
parent 038a5ce17e
commit bf7cd98e25
2 changed files with 299 additions and 7 deletions
@@ -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<Request>({
+				params: { id: '1' },
+				query: { resolverId: 'resolver-123' },
+				headers: { authorization: undefined },
+			});
+			const res = mock<Response>();
+
+			await expect(controller.revokeCredential(req, res)).rejects.toThrow('Unauthenticated');
+		});
+
+		it('should throw BadRequestError when authorization header is malformed', async () => {
+			const req = mock<Request>({
+				params: { id: '1' },
+				query: { resolverId: 'resolver-123' },
+				headers: { authorization: 'InvalidFormat' },
+			});
+			const res = mock<Response>();
+
+			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<Request>({
+				params: { id: 'non-existent-id' },
+				query: { resolverId: 'resolver-123' },
+				headers: { authorization: 'Bearer token123' },
+			});
+			const res = mock<Response>();
+
+			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<CredentialsEntity>({
+				id: '1',
+				type: 'googleOAuth2Api',
+			});
+			const req = mock<Request>({
+				params: { id: '1' },
+				query: { resolverId: 'non-existent-resolver' },
+				headers: { authorization: 'Bearer token123' },
+			});
+			const res = mock<Response>();
+
+			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<CredentialsEntity>({
+				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<Request>({
+				params: { id: '1' },
+				query: { resolverId: 'resolver-123' },
+				headers: { authorization: 'Bearer token123' },
+			});
+			const res = mock<Response>();
+			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<CredentialsEntity>({
+				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<Request>({
+				params: { id: '1' },
+				query: { resolverId: 'resolver-123' },
+				headers: { authorization: 'Bearer token123' },
+			});
+			const res = mock<Response>();
+			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<CredentialsEntity>({
+				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<Request>({
+				params: { id: 'cred-456' },
+				query: { resolverId: 'resolver-123' },
+				headers: { authorization: 'Bearer my-test-token' },
+			});
+			const res = mock<Response>();
+			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<CredentialsEntity>({
+				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<Request>({
+				params: { id: '1' },
+				query: { resolverId: 'resolver-123' },
+				headers: { authorization: 'Bearer token123' },
+			});
+			const res = mock<Response>();
+			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();
+		});
+	});
 });
@@ -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<string> {
-		const credential = await this.enterpriseCredentialsService.getOne(req.params.id);
-		const token = getBearerToken(req);
+	private async findCredentialToUse(credentialId: string): Promise<CredentialsEntity> {
+		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<void> {
+		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<Record<string, unknown>>(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<string> {
+		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<Record<string, unknown>>(decryptedConfig);

 			await resolver.validateIdentity(token, {
-				resolverId,
+				resolverId: resolverEntity.id,
 				resolverName: resolverEntity.type,
 				configuration: resolverConfig,
 			});