From 9a65549575bb201c3f55888d71e04663f622eb5b Mon Sep 17 00:00:00 2001 From: Ali Elkhateeb Date: Mon, 20 Apr 2026 22:56:51 +0200 Subject: [PATCH] feat(API): Add missing credential endpoints (GET by ID and test) (#28519) --- packages/@n8n/api-types/src/index.ts | 6 +- ...chema.ts => credential-response.schema.ts} | 7 +- .../permissions/src/__tests__/types.test.ts | 1 + packages/@n8n/permissions/src/constants.ee.ts | 2 +- .../__tests__/credentials.service.test.ts | 98 +++++++++++++++++++ .../src/credentials/credentials.controller.ts | 42 ++------ .../src/credentials/credentials.service.ts | 79 +++++++++++++++ .../src/errors/credential-not-found.error.ts | 8 +- packages/cli/src/public-api/types.ts | 4 + .../__tests__/credentials.mapper.test.ts | 66 +++++++++++++ .../credentials/credentials.handler.ts | 84 ++++++++++++---- .../credentials/credentials.mapper.ts | 30 ++++++ .../credentials/credentials.service.ts | 15 +-- .../spec/paths/credentials.id.test.yml | 26 +++++ .../credentials/spec/paths/credentials.id.yml | 28 ++++++ .../spec/schemas/credentialTestResponse.yml | 12 +++ packages/cli/src/public-api/v1/openapi.yml | 2 + .../public-api/credentials.test.ts | 88 +++++++++++++++++ .../endpoints-with-scopes-enabled.test.ts | 71 ++++++++++++++ .../nodes/N8n/n8n-api-coverage.json | 9 +- 20 files changed, 600 insertions(+), 78 deletions(-) rename packages/@n8n/api-types/src/schemas/{credential-created.schema.ts => credential-response.schema.ts} (53%) create mode 100644 packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.mapper.test.ts create mode 100644 packages/cli/src/public-api/v1/handlers/credentials/credentials.mapper.ts create mode 100644 packages/cli/src/public-api/v1/handlers/credentials/spec/paths/credentials.id.test.yml create mode 100644 packages/cli/src/public-api/v1/handlers/credentials/spec/schemas/credentialTestResponse.yml diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index 138894b0116..e26fe5550a4 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -250,9 +250,9 @@ export { } from './schemas/community-package.schema'; export { - publicApiCreatedCredentialSchema, - type PublicApiCreatedCredential, -} from './schemas/credential-created.schema'; + publicApiCredentialResponseSchema, + type PublicApiCredentialResponse, +} from './schemas/credential-response.schema'; export { instanceAiEventTypeSchema, diff --git a/packages/@n8n/api-types/src/schemas/credential-created.schema.ts b/packages/@n8n/api-types/src/schemas/credential-response.schema.ts similarity index 53% rename from packages/@n8n/api-types/src/schemas/credential-created.schema.ts rename to packages/@n8n/api-types/src/schemas/credential-response.schema.ts index 0e79ebd6951..ae0010cce96 100644 --- a/packages/@n8n/api-types/src/schemas/credential-created.schema.ts +++ b/packages/@n8n/api-types/src/schemas/credential-response.schema.ts @@ -1,10 +1,9 @@ import { z } from 'zod'; /** - * Plain credential row after creation - * Used by the public API to validate results from `CredentialsService.createUnmanagedCredential`. + * Plain credential row in public API responses. */ -export const publicApiCreatedCredentialSchema = z.object({ +export const publicApiCredentialResponseSchema = z.object({ id: z.string(), name: z.string(), type: z.string(), @@ -17,4 +16,4 @@ export const publicApiCreatedCredentialSchema = z.object({ updatedAt: z.coerce.date(), }); -export type PublicApiCreatedCredential = z.infer; +export type PublicApiCredentialResponse = z.infer; diff --git a/packages/@n8n/permissions/src/__tests__/types.test.ts b/packages/@n8n/permissions/src/__tests__/types.test.ts index d98518e54f0..37982e2752e 100644 --- a/packages/@n8n/permissions/src/__tests__/types.test.ts +++ b/packages/@n8n/permissions/src/__tests__/types.test.ts @@ -6,6 +6,7 @@ describe('ApiKeyScope', () => { test('Valid scopes', () => { const validScopes: ApiKeyScope[] = [ 'credential:create', + 'credential:read', 'credential:delete', 'credential:move', 'execution:delete', diff --git a/packages/@n8n/permissions/src/constants.ee.ts b/packages/@n8n/permissions/src/constants.ee.ts index 41d7abe2901..8f369f10392 100644 --- a/packages/@n8n/permissions/src/constants.ee.ts +++ b/packages/@n8n/permissions/src/constants.ee.ts @@ -71,7 +71,7 @@ export const API_KEY_RESOURCES = { project: ['create', 'update', 'delete', 'list'] as const, user: ['read', 'list', 'create', 'changeRole', 'delete', 'enforceMfa'] as const, execution: ['delete', 'read', 'retry', 'list', 'get', 'stop'] as const, - credential: ['create', 'update', 'move', 'delete', 'list'] as const, + credential: ['create', 'read', 'update', 'move', 'delete', 'list'] as const, sourceControl: ['pull'] as const, workflowTags: ['update', 'list'] as const, executionTags: ['update', 'list'] as const, diff --git a/packages/cli/src/credentials/__tests__/credentials.service.test.ts b/packages/cli/src/credentials/__tests__/credentials.service.test.ts index eec7409864d..1449b4263af 100644 --- a/packages/cli/src/credentials/__tests__/credentials.service.test.ts +++ b/packages/cli/src/credentials/__tests__/credentials.service.test.ts @@ -28,6 +28,7 @@ import { CredentialsService } from '@/credentials/credentials.service'; import * as validation from '@/credentials/validation'; import type { CredentialsHelper } from '@/credentials-helper'; import type { ExternalHooks } from '@/external-hooks'; +import { CredentialNotFoundError } from '@/errors/credential-not-found.error'; import type { ExternalSecretsConfig } from '@/modules/external-secrets.ee/external-secrets.config'; import type { SecretsProviderAccessCheckService } from '@/modules/external-secrets.ee/secret-provider-access-check.service.ee'; import * as checkAccess from '@/permissions.ee/check-access'; @@ -793,6 +794,103 @@ describe('CredentialsService', () => { }); }); + describe('testById', () => { + it('throws CredentialNotFoundError when credential does not exist', async () => { + credentialsFinderService.findCredentialById.mockResolvedValue(null); + + await expect(service.testById(ownerUser.id, 'missing-credential')).rejects.toThrow( + CredentialNotFoundError, + ); + expect(credentialsTester.testCredentials).not.toHaveBeenCalled(); + }); + + it('decrypts stored credential and calls credentials tester', async () => { + const storedCredential = mock({ + id: 'credential-id', + name: 'Test Credential', + type: 'githubApi', + }); + const decryptedData = { accessToken: 'secret-token' } as ICredentialDataDecryptedObject; + const testResult = { status: 'OK', message: 'Credential tested successfully' } as const; + + credentialsFinderService.findCredentialById.mockResolvedValue(storedCredential); + credentialsTester.testCredentials.mockResolvedValue(testResult); + jest.spyOn(service, 'decrypt').mockReturnValue(decryptedData); + + const result = await service.testById(ownerUser.id, storedCredential.id); + + expect(credentialsFinderService.findCredentialById).toHaveBeenCalledWith(storedCredential.id); + expect(service.decrypt).toHaveBeenCalledWith(storedCredential, true); + expect(credentialsTester.testCredentials).toHaveBeenCalledWith( + ownerUser.id, + storedCredential.type, + { + id: storedCredential.id, + name: storedCredential.name, + type: storedCredential.type, + data: decryptedData, + }, + ); + expect(result).toEqual(testResult); + }); + }); + + describe('testWithCredentials', () => { + it('throws CredentialNotFoundError when user cannot access credential', async () => { + credentialsFinderService.findCredentialForUser.mockResolvedValue(null); + + await expect( + service.testWithCredentials(ownerUser, { + id: 'missing-credential', + name: 'Missing Credential', + type: 'githubApi', + data: {}, + }), + ).rejects.toThrow(CredentialNotFoundError); + expect(credentialsTester.testCredentials).not.toHaveBeenCalled(); + }); + + it('prepares merged credentials and runs test', async () => { + const storedCredential = mock({ + id: 'credential-id', + name: 'Stored Credential', + type: 'githubApi', + }); + const decryptedData = { accessToken: 'stored-token' } as ICredentialDataDecryptedObject; + const unredactedData = { accessToken: 'live-token' } as ICredentialDataDecryptedObject; + const testResult = { status: 'OK', message: 'Credential tested successfully' } as const; + + credentialsFinderService.findCredentialForUser.mockResolvedValue(storedCredential); + jest.spyOn(service, 'decrypt').mockReturnValue(decryptedData); + jest.spyOn(service, 'replaceCredentialContentsForSharee').mockResolvedValue(undefined); + jest.spyOn(service, 'getCredentialTypeProperties').mockReturnValue([]); + jest.spyOn(service, 'unredact').mockReturnValue(unredactedData); + credentialsTester.testCredentials.mockResolvedValue(testResult); + + const payload = { + id: 'credential-id', + name: 'Stored Credential', + type: 'githubApi', + data: { accessToken: '***' }, + }; + + const result = await service.testWithCredentials(ownerUser, payload); + + expect(credentialsFinderService.findCredentialForUser).toHaveBeenCalledWith( + payload.id, + ownerUser, + ['credential:read'], + ); + expect(service.decrypt).toHaveBeenCalledWith(storedCredential, true); + expect(service.unredact).toHaveBeenCalledWith(payload.data, decryptedData, []); + expect(credentialsTester.testCredentials).toHaveBeenCalledWith(ownerUser.id, payload.type, { + ...payload, + data: unredactedData, + }); + expect(result).toEqual(testResult); + }); + }); + describe('getMany', () => { const regularCredential = { id: 'cred-1', diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 59afe6e682a..f44d7687706 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -28,7 +28,6 @@ import { import { hasGlobalScope, PROJECT_OWNER_ROLE_SLUG } from '@n8n/permissions'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In } from '@n8n/typeorm'; -import { deepCopy } from 'n8n-workflow'; import type { ICredentialDataDecryptedObject } from 'n8n-workflow'; import { z } from 'zod'; @@ -37,6 +36,7 @@ import { CredentialsService } from './credentials.service'; import { EnterpriseCredentialsService } from './credentials.service.ee'; import { getExternalSecretExpressionPaths } from './external-secrets.utils'; +import { CredentialNotFoundError } from '@/errors/credential-not-found.error'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; @@ -140,41 +140,15 @@ export class CredentialsController { // TODO: Write at least test cases for the failure paths. @Post('/test') async testCredentials(req: CredentialRequest.Test) { - const { credentials } = req.body; + try { + return await this.credentialsService.testWithCredentials(req.user, req.body.credentials); + } catch (error) { + if (error instanceof CredentialNotFoundError) { + throw new ForbiddenError(); + } - const storedCredential = await this.credentialsFinderService.findCredentialForUser( - credentials.id, - req.user, - ['credential:read'], - ); - - if (!storedCredential) { - throw new ForbiddenError(); + throw error; } - - const mergedCredentials = deepCopy(credentials); - const decryptedData = this.credentialsService.decrypt(storedCredential, true); - - // When a sharee (or project viewer) opens a credential, the fields and the - // credential data are missing so the payload will be empty - // We need to replace the credential contents with the db version if that's the case - // So the credential can be tested properly - await this.credentialsService.replaceCredentialContentsForSharee( - req.user, - storedCredential, - decryptedData, - mergedCredentials, - ); - - if (mergedCredentials.data) { - mergedCredentials.data = this.credentialsService.unredact( - mergedCredentials.data, - decryptedData, - this.credentialsService.getCredentialTypeProperties(storedCredential.type), - ); - } - - return await this.credentialsService.test(req.user.id, mergedCredentials); } @Post('/') diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 7048553ddaf..d3cc69152f3 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -43,6 +43,7 @@ import { import { CredentialTypes } from '@/credential-types'; import { createCredentialsFromCredentialsEntity, CredentialsHelper } from '@/credentials-helper'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { CredentialNotFoundError } from '@/errors/credential-not-found.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { ExternalHooks } from '@/external-hooks'; @@ -819,6 +820,37 @@ export class CredentialsService { return await this.credentialsTester.testCredentials(userId, credentials.type, credentials); } + async testById(userId: User['id'], credentialId: string) { + const storedCredential = await this.credentialsFinderService.findCredentialById(credentialId); + + if (!storedCredential) { + throw new CredentialNotFoundError(credentialId); + } + + const credentials = await this.prepareCredentialsForTest({ storedCredential }); + return await this.test(userId, credentials); + } + + async testWithCredentials(user: User, credentials: ICredentialsDecrypted) { + const storedCredential = await this.credentialsFinderService.findCredentialForUser( + credentials.id, + user, + ['credential:read'], + ); + + if (!storedCredential) { + throw new CredentialNotFoundError(credentials.id); + } + + const mergedCredentials = await this.prepareCredentialsForTest({ + storedCredential, + user, + credentialsToTest: credentials, + }); + + return await this.test(user.id, mergedCredentials); + } + // Take data and replace all sensitive values with a sentinel value. // This will replace password fields and oauth data. redact(data: ICredentialDataDecryptedObject, credential: CredentialsEntity) { @@ -1286,4 +1318,51 @@ export class CredentialsService { return { ...credential, scopes }; } + + /** + * Build credentials payload ready to pass to credential testing. + * + * - If `credentialsToTest` is not provided, uses stored decrypted credential data. + * - If `credentialsToTest` is provided, normalizes it for testing: + * - fills payload data for sharees when needed + * - restores redacted values from stored decrypted data + */ + private async prepareCredentialsForTest({ + storedCredential, + user, + credentialsToTest, + }: { + storedCredential: CredentialsEntity; + user?: User; + credentialsToTest?: ICredentialsDecrypted; + }): Promise { + const decryptedData = this.decrypt(storedCredential, true); + const mergedCredentials: ICredentialsDecrypted = credentialsToTest + ? deepCopy(credentialsToTest) + : { + id: storedCredential.id, + name: storedCredential.name, + type: storedCredential.type, + data: decryptedData, + }; + + if (user && credentialsToTest) { + await this.replaceCredentialContentsForSharee( + user, + storedCredential, + decryptedData, + mergedCredentials, + ); + + if (mergedCredentials.data) { + mergedCredentials.data = this.unredact( + mergedCredentials.data, + decryptedData, + this.getCredentialTypeProperties(storedCredential.type), + ); + } + } + + return mergedCredentials; + } } diff --git a/packages/cli/src/errors/credential-not-found.error.ts b/packages/cli/src/errors/credential-not-found.error.ts index 28829a397d9..e13d730e6a7 100644 --- a/packages/cli/src/errors/credential-not-found.error.ts +++ b/packages/cli/src/errors/credential-not-found.error.ts @@ -1,7 +1,11 @@ import { UserError } from 'n8n-workflow'; export class CredentialNotFoundError extends UserError { - constructor(credentialId: string, credentialType: string) { - super(`Credential with ID "${credentialId}" does not exist for type "${credentialType}".`); + constructor(credentialId: string, credentialType?: string) { + super( + credentialType + ? `Credential with ID "${credentialId}" does not exist for type "${credentialType}".` + : `Credential with ID "${credentialId}" was not found.`, + ); } } diff --git a/packages/cli/src/public-api/types.ts b/packages/cli/src/public-api/types.ts index 20e24bdcdca..8fd17d21d94 100644 --- a/packages/cli/src/public-api/types.ts +++ b/packages/cli/src/public-api/types.ts @@ -171,6 +171,8 @@ export declare namespace CredentialRequest { { limit?: number; cursor?: string; offset?: number } >; + type Get = AuthenticatedRequest<{ id: string }>; + type Create = AuthenticatedRequest< {}, {}, @@ -192,6 +194,8 @@ export declare namespace CredentialRequest { {} >; + type Test = AuthenticatedRequest<{ id: string }, {}, {}, {}>; + type Delete = AuthenticatedRequest<{ id: string }, {}, {}, Record>; type Transfer = AuthenticatedRequest<{ id: string }, {}, { destinationProjectId: string }>; diff --git a/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.mapper.test.ts b/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.mapper.test.ts new file mode 100644 index 00000000000..20b4581700f --- /dev/null +++ b/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.mapper.test.ts @@ -0,0 +1,66 @@ +import { ZodError } from 'zod'; +import { UnexpectedError } from 'n8n-workflow'; + +import { toPublicApiCredentialResponse } from '../credentials.mapper'; + +type MapperInput = Parameters[0]; + +const makeCredential = (overrides: Partial = {}): MapperInput => ({ + id: 'cred-1', + name: 'GitHub', + type: 'githubApi', + isManaged: false, + isGlobal: false, + isResolvable: false, + createdAt: new Date('2026-01-01T10:00:00.000Z'), + updatedAt: new Date('2026-01-02T10:00:00.000Z'), + ...overrides, +}); + +describe('toPublicApiCredentialResponse', () => { + it('sets defaults for optional mapper fields', () => { + const response = toPublicApiCredentialResponse(makeCredential()); + + expect(response.resolvableAllowFallback).toBe(false); + expect(response.resolverId).toBeNull(); + expect(response.id).toBe('cred-1'); + expect(response.name).toBe('GitHub'); + expect(response.type).toBe('githubApi'); + expect(response.createdAt).toEqual(new Date('2026-01-01T10:00:00.000Z')); + expect(response.updatedAt).toEqual(new Date('2026-01-02T10:00:00.000Z')); + }); + + it('keeps provided optional mapper fields', () => { + const response = toPublicApiCredentialResponse( + makeCredential({ + resolvableAllowFallback: true, + resolverId: 'resolver-1', + }), + ); + + expect(response.resolvableAllowFallback).toBe(true); + expect(response.resolverId).toBe('resolver-1'); + }); + + it('throws UnexpectedError with parse cause for invalid payload', () => { + expect(() => + toPublicApiCredentialResponse( + makeCredential({ + createdAt: 'not-a-date' as unknown as Date, + }), + ), + ).toThrow(UnexpectedError); + + try { + toPublicApiCredentialResponse( + makeCredential({ + createdAt: 'not-a-date' as unknown as Date, + }), + ); + } catch (error) { + expect(error).toBeInstanceOf(UnexpectedError); + expect(error.message).toBe('Failed to parse credential response'); + expect(error.cause).toBeInstanceOf(ZodError); + } + }); +}); diff --git a/packages/cli/src/public-api/v1/handlers/credentials/credentials.handler.ts b/packages/cli/src/public-api/v1/handlers/credentials/credentials.handler.ts index 6d36c0c01c2..0978deb670c 100644 --- a/packages/cli/src/public-api/v1/handlers/credentials/credentials.handler.ts +++ b/packages/cli/src/public-api/v1/handlers/credentials/credentials.handler.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/no-unsafe-argument */ import { LicenseState } from '@n8n/backend-common'; +import type { PublicApiCredentialResponse } from '@n8n/api-types'; import type { CredentialsEntity } from '@n8n/db'; import { CredentialsRepository } from '@n8n/db'; import { Container } from '@n8n/di'; @@ -8,9 +9,11 @@ import type express from 'express'; import { z } from 'zod'; import { CredentialTypes } from '@/credential-types'; +import { CredentialsService } from '@/credentials/credentials.service'; import { EnterpriseCredentialsService } from '@/credentials/credentials.service.ee'; import { CredentialsHelper } from '@/credentials-helper'; -import { ResponseError } from '@/errors/response-errors/abstract/response.error'; +import { CredentialNotFoundError } from '@/errors/credential-not-found.error'; +import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { validCredentialsProperties, @@ -29,6 +32,7 @@ import { toJsonSchema, updateCredential, } from './credentials.service'; +import { toPublicApiCredentialResponse } from './credentials.mapper'; import type { CredentialTypeRequest, CredentialRequest } from '../../../types'; import { publicApiScope, @@ -37,6 +41,8 @@ import { validCursor, } from '../../shared/middlewares/global.middleware'; import { encodeNextCursor } from '../../shared/services/pagination.service'; +import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; export = { getCredentials: [ @@ -92,6 +98,48 @@ export = { }); }, ], + getCredential: [ + publicApiScope('credential:read'), + projectScope('credential:read', 'credential'), + async ( + req: CredentialRequest.Get, + res: express.Response, + ): Promise> => { + const { id: credentialId } = req.params; + + const credential = await getCredential(credentialId); + if (!credential) { + throw new NotFoundError('Credential not found'); + } + + return res.json(toPublicApiCredentialResponse(credential)); + }, + ], + testCredential: [ + publicApiScope('credential:read'), + projectScope('credential:read', 'credential'), + async ( + req: CredentialRequest.Test, + res: express.Response<{ status: 'OK' | 'Error'; message: string } | { message: string }>, + ): Promise< + express.Response<{ status: 'OK' | 'Error'; message: string } | { message: string }> + > => { + const { id: credentialId } = req.params; + try { + const credentialTestResult = await Container.get(CredentialsService).testById( + req.user.id, + credentialId, + ); + return res.json(credentialTestResult); + } catch (error) { + if (error instanceof CredentialNotFoundError) { + throw new NotFoundError(error.message); + } + + throw error; + } + }, + ], createCredential: [ validCredentialType, validCredentialsProperties, @@ -99,10 +147,9 @@ export = { async ( req: CredentialRequest.Create, res: express.Response, - ): Promise>> => { + ): Promise> => { const savedCredential = await saveCredential(req.body, req.user); - - return res.json(sanitizeCredentials(savedCredential)); + return res.json(savedCredential); }, ], updateCredential: [ @@ -113,42 +160,37 @@ export = { async ( req: CredentialRequest.Update, res: express.Response, - ): Promise>> => { + ): Promise> => { const { id: credentialId } = req.params; const existingCredential = await getCredential(credentialId); if (!existingCredential) { - return res.status(404).json({ message: 'Credential not found' }); + throw new NotFoundError('Credential not found'); } if (req.body.isGlobal !== undefined && req.body.isGlobal !== existingCredential.isGlobal) { if (!Container.get(LicenseState).isSharingLicensed()) { - return res.status(403).json({ message: 'You are not licensed for sharing credentials' }); + throw new ForbiddenError('You are not licensed for sharing credentials'); } const canShareGlobally = hasGlobalScope(req.user, 'credential:shareGlobally'); if (!canShareGlobally) { - return res.status(403).json({ - message: 'You do not have permission to change global sharing for credentials', - }); + throw new ForbiddenError( + 'You do not have permission to change global sharing for credentials', + ); } } try { const updatedCredential = await updateCredential(existingCredential, req.user, req.body); - return res.json(sanitizeCredentials(updatedCredential as CredentialsEntity)); + return res.json(toPublicApiCredentialResponse(updatedCredential)); } catch (error) { if (error instanceof CredentialsIsNotUpdatableError) { - return res.status(400).json({ message: error.message }); + throw new BadRequestError(error.message); } - if (error instanceof ResponseError) { - return res.status(error.httpStatusCode).json({ message: error.message }); - } - - const message = error instanceof Error ? error.message : 'Unknown error'; - return res.status(500).json({ message }); + throw error; } }, ], @@ -184,11 +226,11 @@ export = { credential = shared.credentials; } } else { - credential = (await getCredential(credentialId)) as CredentialsEntity; + credential = (await getCredential(credentialId)) ?? undefined; } if (!credential) { - return res.status(404).json({ message: 'Not Found' }); + throw new NotFoundError('Not Found'); } await removeCredential(req.user, credential); @@ -203,7 +245,7 @@ export = { try { Container.get(CredentialTypes).getByName(credentialTypeName); } catch (error) { - return res.status(404).json({ message: 'Not Found' }); + throw new NotFoundError('Not Found'); } const schema = Container.get(CredentialsHelper) diff --git a/packages/cli/src/public-api/v1/handlers/credentials/credentials.mapper.ts b/packages/cli/src/public-api/v1/handlers/credentials/credentials.mapper.ts new file mode 100644 index 00000000000..f5a4edc051f --- /dev/null +++ b/packages/cli/src/public-api/v1/handlers/credentials/credentials.mapper.ts @@ -0,0 +1,30 @@ +import { + publicApiCredentialResponseSchema, + type PublicApiCredentialResponse, +} from '@n8n/api-types'; +import type { ICredentialsDb } from '@n8n/db'; +import { UnexpectedError } from 'n8n-workflow'; + +export function toPublicApiCredentialResponse( + credential: Pick< + ICredentialsDb, + 'id' | 'name' | 'type' | 'isManaged' | 'isGlobal' | 'isResolvable' | 'createdAt' | 'updatedAt' + > & { + resolvableAllowFallback?: boolean; + resolverId?: string | null; + }, +): PublicApiCredentialResponse { + const parsed = publicApiCredentialResponseSchema.safeParse({ + ...credential, + resolvableAllowFallback: credential.resolvableAllowFallback ?? false, + resolverId: credential.resolverId ?? null, + }); + + if (!parsed.success) { + throw new UnexpectedError('Failed to parse credential response', { + cause: parsed.error, + }); + } + + return parsed.data; +} diff --git a/packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts b/packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts index aa9bf969f40..b6d0642eda8 100644 --- a/packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts +++ b/packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts @@ -1,4 +1,4 @@ -import { publicApiCreatedCredentialSchema } from '@n8n/api-types'; +import type { PublicApiCredentialResponse } from '@n8n/api-types'; import type { User, ICredentialsDb, SharedCredentials } from '@n8n/db'; import { CredentialsEntity, CredentialsRepository, SharedCredentialsRepository } from '@n8n/db'; import { Container } from '@n8n/di'; @@ -10,7 +10,6 @@ import { type IDataObject, type INodeProperties, type INodePropertyOptions, - UnexpectedError, } from 'n8n-workflow'; import { CredentialsService } from '@/credentials/credentials.service'; @@ -23,6 +22,7 @@ import { ExternalHooks } from '@/external-hooks'; import { ExternalSecretsConfig } from '@/modules/external-secrets.ee/external-secrets.config'; import { SecretsProviderAccessCheckService } from '@/modules/external-secrets.ee/secret-provider-access-check.service.ee'; +import { toPublicApiCredentialResponse } from './credentials.mapper'; import type { IDependency, IJsonSchema } from '../../../types'; export class CredentialsIsNotUpdatableError extends BaseError {} @@ -58,7 +58,7 @@ export function buildSharedForCredential( })); } -export async function getCredential(credentialId: string): Promise { +export async function getCredential(credentialId: string): Promise { return await Container.get(CredentialsRepository).findOne({ where: { id: credentialId }, relations: ['shared', 'shared.project'], @@ -89,7 +89,7 @@ export async function getSharedCredentials( export async function saveCredential( payload: { type: string; name: string; data: ICredentialDataDecryptedObject; projectId?: string }, user: User, -): Promise { +): Promise { const { scopes: _scopes, ...credential } = await Container.get( CredentialsService, ).createUnmanagedCredential({ ...payload, projectId: payload.projectId ?? undefined }, user); @@ -121,12 +121,7 @@ export async function saveCredential( updatedAt: credential.updatedAt, }; - const parsed = publicApiCreatedCredentialSchema.safeParse(credentialForApi); - if (!parsed.success) { - throw new UnexpectedError('Credential create response failed validation'); - } - - return Object.assign(new CredentialsEntity(), parsed.data, { shared: [] }); + return toPublicApiCredentialResponse(credentialForApi); } export async function updateCredential( diff --git a/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/credentials.id.test.yml b/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/credentials.id.test.yml new file mode 100644 index 00000000000..07706f0009b --- /dev/null +++ b/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/credentials.id.test.yml @@ -0,0 +1,26 @@ +post: + x-eov-operation-id: testCredential + x-eov-operation-handler: v1/handlers/credentials/credentials.handler + tags: + - Credential + summary: Test credential by ID + description: Tests a credential by ID using the stored credential data. + operationId: testCredential + parameters: + - name: id + in: path + description: The credential ID + required: true + schema: + type: string + responses: + '200': + description: Operation successful. + content: + application/json: + schema: + $ref: '../schemas/credentialTestResponse.yml' + '401': + $ref: '../../../../shared/spec/responses/unauthorized.yml' + '404': + $ref: '../../../../shared/spec/responses/notFound.yml' diff --git a/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/credentials.id.yml b/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/credentials.id.yml index 9e365dce94f..b8d68ee33b4 100644 --- a/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/credentials.id.yml +++ b/packages/cli/src/public-api/v1/handlers/credentials/spec/paths/credentials.id.yml @@ -1,3 +1,31 @@ +get: + x-eov-operation-id: getCredential + x-eov-operation-handler: v1/handlers/credentials/credentials.handler + tags: + - Credential + summary: Get credential by ID + description: Retrieves a credential by ID. Credential data (secrets) is not included. + operationId: getCredential + parameters: + - name: id + in: path + description: The credential ID + required: true + schema: + type: string + responses: + '200': + description: Operation successful. + content: + application/json: + schema: + $ref: '../schemas/create-credential-response.yml' + '401': + $ref: '../../../../shared/spec/responses/unauthorized.yml' + '403': + $ref: '../../../../shared/spec/responses/forbidden.yml' + '404': + $ref: '../../../../shared/spec/responses/notFound.yml' patch: x-eov-operation-id: updateCredential x-eov-operation-handler: v1/handlers/credentials/credentials.handler diff --git a/packages/cli/src/public-api/v1/handlers/credentials/spec/schemas/credentialTestResponse.yml b/packages/cli/src/public-api/v1/handlers/credentials/spec/schemas/credentialTestResponse.yml new file mode 100644 index 00000000000..89da11004fb --- /dev/null +++ b/packages/cli/src/public-api/v1/handlers/credentials/spec/schemas/credentialTestResponse.yml @@ -0,0 +1,12 @@ +type: object +required: + - status + - message +properties: + status: + type: string + enum: + - OK + - Error + message: + type: string diff --git a/packages/cli/src/public-api/v1/openapi.yml b/packages/cli/src/public-api/v1/openapi.yml index 79f9d9c3fd8..875dc5bf84c 100644 --- a/packages/cli/src/public-api/v1/openapi.yml +++ b/packages/cli/src/public-api/v1/openapi.yml @@ -50,6 +50,8 @@ paths: $ref: './handlers/credentials/spec/paths/credentials.yml' /credentials/{id}: $ref: './handlers/credentials/spec/paths/credentials.id.yml' + /credentials/{id}/test: + $ref: './handlers/credentials/spec/paths/credentials.id.test.yml' /credentials/schema/{credentialTypeName}: $ref: './handlers/credentials/spec/paths/credentials.schema.id.yml' /credentials/{id}/transfer: diff --git a/packages/cli/test/integration/public-api/credentials.test.ts b/packages/cli/test/integration/public-api/credentials.test.ts index 6707594622c..2bae60949f7 100644 --- a/packages/cli/test/integration/public-api/credentials.test.ts +++ b/packages/cli/test/integration/public-api/credentials.test.ts @@ -4,6 +4,7 @@ import { createTeamProject, randomName, testDb } from '@n8n/backend-test-utils'; import type { User } from '@n8n/db'; import { CredentialsRepository, SharedCredentialsRepository } from '@n8n/db'; import { Container } from '@n8n/di'; +import { mock } from 'jest-mock-extended'; import { CREDENTIAL_BLANKING_VALUE, type ICredentialDataDecryptedObject, @@ -11,6 +12,7 @@ import { } from 'n8n-workflow'; import { CredentialsService } from '@/credentials/credentials.service'; +import { CredentialsTester } from '@/services/credentials-tester.service'; import { affixRoleToSaveCredential, @@ -365,6 +367,92 @@ describe('GET /credentials', () => { }); }); +describe('GET /credentials/:id', () => { + test('should return owned credential for owner without credential data', async () => { + const savedCredential = await saveCredential(dbCredential(), { user: owner }); + + const response = await authOwnerAgent.get(`/credentials/${savedCredential.id}`); + + expect(response.statusCode).toBe(200); + expect(response.body).toMatchObject({ + id: savedCredential.id, + name: savedCredential.name, + type: savedCredential.type, + }); + expect(response.body).not.toHaveProperty('data'); + expect(response.body).not.toHaveProperty('shared'); + }); + + test('should return owned credential for member', async () => { + const memberWithReadScope = await createMemberWithApiKey({ scopes: ['credential:read'] }); + const authMemberWithReadScopeAgent = testServer.publicApiAgentFor(memberWithReadScope); + const savedCredential = await saveCredential(dbCredential(), { user: memberWithReadScope }); + + const response = await authMemberWithReadScopeAgent.get(`/credentials/${savedCredential.id}`); + + expect(response.statusCode).toBe(200); + expect(response.body).toMatchObject({ + id: savedCredential.id, + name: savedCredential.name, + type: savedCredential.type, + }); + expect(response.body).not.toHaveProperty('data'); + expect(response.body).not.toHaveProperty('shared'); + }); + + test('should not return non-owned credential for member', async () => { + const savedCredential = await saveCredential(dbCredential(), { user: owner }); + + const response = await authMemberAgent.get(`/credentials/${savedCredential.id}`); + + expect(response.statusCode).toBe(403); + }); + + test('should return 404 if credential does not exist', async () => { + const response = await authOwnerAgent.get('/credentials/123'); + + expect(response.statusCode).toBe(404); + }); +}); + +describe('POST /credentials/:id/test', () => { + const mockCredentialsTester = mock(); + Container.set(CredentialsTester, mockCredentialsTester); + + afterEach(() => { + mockCredentialsTester.testCredentials.mockClear(); + }); + + test('should test credential with stored data when body is empty', async () => { + mockCredentialsTester.testCredentials.mockResolvedValue({ + status: 'OK', + message: 'Credential tested successfully', + }); + + const credential = dbCredential(); + const savedCredential = await saveCredential(credential, { user: owner }); + + const response = await authOwnerAgent.post(`/credentials/${savedCredential.id}/test`); + + expect(response.statusCode).toBe(200); + expect(mockCredentialsTester.testCredentials).toHaveBeenCalledWith( + owner.id, + savedCredential.type, + expect.objectContaining({ + id: savedCredential.id, + type: savedCredential.type, + data: credential.data, + }), + ); + }); + + test('should return 404 if credential does not exist', async () => { + const response = await authOwnerAgent.post('/credentials/123/test'); + + expect(response.statusCode).toBe(404); + }); +}); + describe('DELETE /credentials/:id', () => { test('should delete owned cred for owner', async () => { const savedCredential = await saveCredential(dbCredential(), { user: owner }); diff --git a/packages/cli/test/integration/public-api/endpoints-with-scopes-enabled.test.ts b/packages/cli/test/integration/public-api/endpoints-with-scopes-enabled.test.ts index cf7528b2d31..015c9842f32 100644 --- a/packages/cli/test/integration/public-api/endpoints-with-scopes-enabled.test.ts +++ b/packages/cli/test/integration/public-api/endpoints-with-scopes-enabled.test.ts @@ -17,9 +17,12 @@ import { } from '@n8n/db'; import { Container } from '@n8n/di'; import { getOwnerOnlyApiKeyScopes } from '@n8n/permissions'; +import { mock } from 'jest-mock-extended'; import { randomString } from 'n8n-workflow'; import validator from 'validator'; +import { CredentialsTester } from '@/services/credentials-tester.service'; + import { affixRoleToSaveCredential, createCredentials } from '@test-integration/db/credentials'; import { createErrorExecution, createSuccessfulExecution } from '@test-integration/db/executions'; import { createTag } from '@test-integration/db/tags'; @@ -470,6 +473,74 @@ describe('Public API endpoints with API key scopes', () => { expect(sharedCredential.credentials.name).toBe(payload.name); }); }); + describe('GET /credentials/:id', () => { + test('should retrieve credential when API key has "credential:read" scope', async () => { + const owner = await createOwnerWithApiKey({ scopes: ['credential:read'] }); + const authOwnerAgent = testServer.publicApiAgentFor(owner); + + const savedCredential = await saveCredential(credentialPayload(), { user: owner }); + + const response = await authOwnerAgent.get(`/credentials/${savedCredential.id}`); + + expect(response.statusCode).toBe(200); + expect(response.body).toMatchObject({ + id: savedCredential.id, + name: savedCredential.name, + type: savedCredential.type, + }); + expect(response.body).not.toHaveProperty('data'); + expect(response.body).not.toHaveProperty('shared'); + }); + + test('should fail to retrieve credential when API key doesn\'t have "credential:read" scope', async () => { + const owner = await createOwnerWithApiKey({ scopes: ['tag:create'] }); + const authOwnerAgent = testServer.publicApiAgentFor(owner); + + const savedCredential = await saveCredential(credentialPayload(), { user: owner }); + + const response = await authOwnerAgent.get(`/credentials/${savedCredential.id}`); + + expect(response.statusCode).toBe(403); + }); + }); + describe('POST /credentials/:id/test', () => { + const mockCredentialsTester = mock(); + Container.set(CredentialsTester, mockCredentialsTester); + + beforeEach(() => { + mockCredentialsTester.testCredentials.mockResolvedValue({ + status: 'OK', + message: 'Connection successful!', + }); + }); + + afterEach(() => { + mockCredentialsTester.testCredentials.mockClear(); + }); + + test('should test credential when API key has "credential:read" scope', async () => { + const owner = await createOwnerWithApiKey({ scopes: ['credential:read'] }); + const authOwnerAgent = testServer.publicApiAgentFor(owner); + + const savedCredential = await saveCredential(credentialPayload(), { user: owner }); + + const response = await authOwnerAgent.post(`/credentials/${savedCredential.id}/test`); + + expect(response.statusCode).toBe(200); + }); + + test('should fail to test credential when API key doesn\'t have "credential:read" scope', async () => { + const owner = await createOwnerWithApiKey({ scopes: ['tag:create'] }); + const authOwnerAgent = testServer.publicApiAgentFor(owner); + + const savedCredential = await saveCredential(credentialPayload(), { user: owner }); + + const response = await authOwnerAgent.post(`/credentials/${savedCredential.id}/test`); + + expect(response.statusCode).toBe(403); + }); + }); + describe('DELETE /credentials/:id', () => { test('should delete credential when API key has "credential:delete" scope', async () => { const owner = await createOwnerWithApiKey({ scopes: ['credential:delete'] }); diff --git a/packages/nodes-base/nodes/N8n/n8n-api-coverage.json b/packages/nodes-base/nodes/N8n/n8n-api-coverage.json index 5fa64a56671..1351ce8d4da 100644 --- a/packages/nodes-base/nodes/N8n/n8n-api-coverage.json +++ b/packages/nodes-base/nodes/N8n/n8n-api-coverage.json @@ -8,6 +8,9 @@ "GET /credentials": { "status": "gap" }, + "GET /credentials/{id}": { + "status": "gap" + }, "POST /credentials": { "status": "covered", "nodeOperation": "credential:create" @@ -26,6 +29,9 @@ "PUT /credentials/{id}/transfer": { "status": "gap" }, + "POST /credentials/{id}/test": { + "status": "gap" + }, "GET /community-packages": { "status": "gap" }, @@ -193,9 +199,6 @@ "DELETE /data-tables/{dataTableId}/rows/delete": { "status": "gap" }, - "GET /insights/summary": { - "status": "gap" - }, "POST /projects": { "status": "gap" },