From 7060434f1f01aafdfeb153fa837c8aecf1904ce6 Mon Sep 17 00:00:00 2001 From: Ali Elkhateeb Date: Mon, 17 Aug 2026 13:14:21 +0000 Subject: [PATCH] refactor(API): Route public API credentials through the shared credentials service (#36048) --- packages/@n8n/db/src/index.ts | 2 + .../__tests__/credentials.repository.test.ts | 31 + .../repositories/credentials.repository.ts | 71 +- .../db/src/repositories/folder.repository.ts | 10 +- .../src/repositories/workflow.repository.ts | 12 +- .../utils/__tests__/list-query-sort.test.ts | 14 + packages/@n8n/db/src/utils/list-query-sort.ts | 14 + packages/cli/eslint.config.mjs | 2 - .../__tests__/credentials.controller.test.ts | 31 +- .../__tests__/credentials.service.test.ts | 208 ++++- .../credentials/credentials-finder.service.ts | 8 +- .../src/credentials/credentials.controller.ts | 14 - .../src/credentials/credentials.service.ee.ts | 2 +- .../src/credentials/credentials.service.ts | 286 ++++--- .../__tests__/credentials.repository.test.ts | 52 -- .../agents-builder-settings.service.test.ts | 6 +- .../builder-model-live-lookup.service.test.ts | 14 +- .../agents-builder-settings.service.ts | 4 +- .../builder-model-live-lookup.service.ts | 2 +- .../data-table/data-table.repository.ts | 11 +- .../dynamic-credentials.controller.ts | 2 +- .../insights-by-period.repository.ts | 9 +- .../src/oauth/__tests__/oauth.service.test.ts | 4 +- packages/cli/src/oauth/oauth.service.ts | 9 +- .../__tests__/credentials.service.test.ts | 760 ------------------ .../__tests__/credentials.utils.test.ts | 433 ++++++++++ .../credentials/credentials.handler.ts | 220 +++-- .../credentials/credentials.middleware.ts | 8 +- .../credentials/credentials.service.ts | 494 ------------ .../handlers/credentials/credentials.utils.ts | 249 ++++++ .../credentials-finder.service.test.ts | 11 +- .../credentials.repository.test.ts | 9 +- 32 files changed, 1330 insertions(+), 1672 deletions(-) create mode 100644 packages/@n8n/db/src/utils/__tests__/list-query-sort.test.ts create mode 100644 packages/@n8n/db/src/utils/list-query-sort.ts delete mode 100644 packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.service.test.ts create mode 100644 packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.utils.test.ts delete mode 100644 packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts create mode 100644 packages/cli/src/public-api/v1/handlers/credentials/credentials.utils.ts diff --git a/packages/@n8n/db/src/index.ts b/packages/@n8n/db/src/index.ts index 41d2a7ad8a3..586407cec7d 100644 --- a/packages/@n8n/db/src/index.ts +++ b/packages/@n8n/db/src/index.ts @@ -16,6 +16,8 @@ export { generateHostInstanceId } from './utils/generators'; export { isStringArray } from './utils/is-string-array'; export { isUniqueConstraintError } from './utils/is-unique-constraint-error'; export { isValidEmail } from './utils/is-valid-email'; +export { parseListQuerySortBy } from './utils/list-query-sort'; +export type { ListQuerySort, ListQuerySortDirection } from './utils/list-query-sort'; export { separate } from './utils/separate'; export { sql } from './utils/sql'; export { idStringifier, lowerCaser, objectRetriever, sqlite } from './utils/transformers'; diff --git a/packages/@n8n/db/src/repositories/__tests__/credentials.repository.test.ts b/packages/@n8n/db/src/repositories/__tests__/credentials.repository.test.ts index 83ebd2cde91..5ee9cf99a70 100644 --- a/packages/@n8n/db/src/repositories/__tests__/credentials.repository.test.ts +++ b/packages/@n8n/db/src/repositories/__tests__/credentials.repository.test.ts @@ -156,6 +156,37 @@ describe('CredentialsRepository', () => { expect(callArg).toBeDefined(); expect(callArg!.where).toEqual(expect.objectContaining({ id: In(['id1', 'id2']) })); }); + + it('should apply sortBy as TypeORM order', async () => { + entityManager.findAndCount.mockResolvedValueOnce([[], 0]); + + await credentialsRepository.findManyAndCount({ + take: 10, + skip: 0, + sortBy: 'createdAt:desc', + }); + + const callArg = entityManager.findAndCount.mock.calls[0]?.[1]; + expect(callArg?.order).toEqual({ createdAt: 'DESC' }); + }); + + it('should default sort direction to ASC when omitted', async () => { + entityManager.findAndCount.mockResolvedValueOnce([[], 0]); + + await credentialsRepository.findManyAndCount({ sortBy: 'name' }); + + const callArg = entityManager.findAndCount.mock.calls[0]?.[1]; + expect(callArg?.order).toEqual({ name: 'ASC' }); + }); + + it('should ignore unknown sortBy columns', async () => { + entityManager.findAndCount.mockResolvedValueOnce([[], 0]); + + await credentialsRepository.findManyAndCount({ sortBy: 'data:desc' }); + + const callArg = entityManager.findAndCount.mock.calls[0]?.[1]; + expect(callArg?.order).toBeUndefined(); + }); }); describe('findAllGlobalCredentials', () => { diff --git a/packages/@n8n/db/src/repositories/credentials.repository.ts b/packages/@n8n/db/src/repositories/credentials.repository.ts index a1c83f14bca..a203e7f1c4e 100644 --- a/packages/@n8n/db/src/repositories/credentials.repository.ts +++ b/packages/@n8n/db/src/repositories/credentials.repository.ts @@ -14,6 +14,14 @@ import { SharedCredentialsRepository } from './shared-credentials.repository'; import type { ICredentialsDb, ListQuery } from '../entities/types-db'; import type { OperationContext } from '../services/transaction'; import { TransactionRunner } from '../services/transaction'; +import { parseListQuerySortBy } from '../utils/list-query-sort'; + +const SORTABLE_COLUMNS = new Set(['id', 'name', 'createdAt', 'updatedAt']); + +type CredentialsListQueryOptions = ListQuery.Options & { + includeData?: boolean; + user?: User; +}; @Service() export class CredentialsRepository extends BaseRepository { @@ -116,31 +124,8 @@ export class CredentialsRepository extends BaseRepository { } } - async findMany( - listQueryOptions?: ListQuery.Options & { - includeData?: boolean; - user?: User; - /** When provided, sets sort order for the query. */ - order?: FindManyOptions['order']; - }, - credentialIds?: string[], - ) { - const findManyOptions = this.toFindManyOptions(listQueryOptions); - - if (credentialIds) { - findManyOptions.where = { ...findManyOptions.where, id: In(credentialIds) }; - } - - return await this.find(this.onlyProjectCredentials(findManyOptions)); - } - async findManyAndCount( - listQueryOptions?: ListQuery.Options & { - includeData?: boolean; - user?: User; - /** When provided, sets sort order for the query. */ - order?: FindManyOptions['order']; - }, + listQueryOptions?: CredentialsListQueryOptions, credentialIds?: string[], ): Promise<[CredentialsEntity[], number]> { const findManyOptions = this.toFindManyOptions(listQueryOptions); @@ -159,12 +144,7 @@ export class CredentialsRepository extends BaseRepository { return findManyOptions; } - private toFindManyOptions( - listQueryOptions?: ListQuery.Options & { - includeData?: boolean; - order?: FindManyOptions['order']; - }, - ) { + private toFindManyOptions(listQueryOptions?: CredentialsListQueryOptions) { const findManyOptions: FindManyOptions = {}; type Select = Array; @@ -189,7 +169,7 @@ export class CredentialsRepository extends BaseRepository { } as FindManyOptions; } - const { filter, select, take, skip, order } = listQueryOptions; + const { filter, select, take, skip, sortBy } = listQueryOptions; if (typeof filter?.name === 'string' && filter?.name !== '') { filter.name = Like(`%${filter.name}%`); @@ -215,8 +195,11 @@ export class CredentialsRepository extends BaseRepository { findManyOptions.relations = defaultRelations; } - if (order !== undefined) { - findManyOptions.order = order; + if (sortBy) { + const { column, direction } = parseListQuerySortBy(sortBy); + if (SORTABLE_COLUMNS.has(column)) { + findManyOptions.order = { [column]: direction }; + } } if (listQueryOptions.includeData) { @@ -230,9 +213,7 @@ export class CredentialsRepository extends BaseRepository { return findManyOptions; } - private handleSharedFilters( - listQueryOptions?: ListQuery.Options & { includeData?: boolean }, - ): void { + private handleSharedFilters(listQueryOptions?: CredentialsListQueryOptions): void { if (!listQueryOptions?.filter) return; const { filter } = listQueryOptions; @@ -418,9 +399,7 @@ export class CredentialsRepository extends BaseRepository { personalProjectOwnerId?: string; onlySharedWithMe?: boolean; }, - options: ListQuery.Options & { - includeData?: boolean; - order?: FindManyOptions['order']; + options: CredentialsListQueryOptions & { filters?: { dependency?: CredentialDependencyFilter; }; @@ -458,9 +437,7 @@ export class CredentialsRepository extends BaseRepository { personalProjectOwnerId?: string; onlySharedWithMe?: boolean; }, - options: ListQuery.Options & { - includeData?: boolean; - order?: FindManyOptions['order']; + options: CredentialsListQueryOptions & { filters?: { dependency?: CredentialDependencyFilter; }; @@ -549,11 +526,11 @@ export class CredentialsRepository extends BaseRepository { .leftJoinAndSelect('project.projectRelations', 'projectRelations'); } - // Apply sorting - if (options.order) { - Object.entries(options.order).forEach(([key, direction]) => { - qb.addOrderBy(`credential.${key}`, direction as 'ASC' | 'DESC'); - }); + if (options.sortBy) { + const { column, direction } = parseListQuerySortBy(options.sortBy); + if (SORTABLE_COLUMNS.has(column)) { + qb.addOrderBy(`credential.${column}`, direction); + } } // Apply pagination diff --git a/packages/@n8n/db/src/repositories/folder.repository.ts b/packages/@n8n/db/src/repositories/folder.repository.ts index 7b2a3966d05..0c046adace0 100644 --- a/packages/@n8n/db/src/repositories/folder.repository.ts +++ b/packages/@n8n/db/src/repositories/folder.repository.ts @@ -5,6 +5,7 @@ import { PROJECT_ROOT } from 'n8n-workflow'; import { Folder, FolderTagMapping, TagEntity } from '../entities'; import type { FolderWithWorkflowAndSubFolderCountAndPath, ListQuery } from '../entities/types-db'; +import { parseListQuerySortBy } from '../utils/list-query-sort'; @Service() export class FolderRepository extends Repository { @@ -228,13 +229,8 @@ export class FolderRepository extends Repository { return; } - const [field, order] = this.parseSortingParams(sortBy); - this.applySortingByField(query, field, order); - } - - private parseSortingParams(sortBy: string): [string, 'DESC' | 'ASC'] { - const [field, order] = sortBy.split(':'); - return [field, order?.toLowerCase() === 'desc' ? 'DESC' : 'ASC']; + const { column, direction } = parseListQuerySortBy(sortBy); + this.applySortingByField(query, column, direction); } private applySortingByField( diff --git a/packages/@n8n/db/src/repositories/workflow.repository.ts b/packages/@n8n/db/src/repositories/workflow.repository.ts index 06ea4913e76..ec731ae4e18 100644 --- a/packages/@n8n/db/src/repositories/workflow.repository.ts +++ b/packages/@n8n/db/src/repositories/workflow.repository.ts @@ -34,6 +34,7 @@ import type { import { type OperationContext, TransactionRunner } from '../services/transaction'; import { applyWorkflowBooleanSettingFilter } from '../utils/apply-workflow-boolean-setting-filter'; import { isStringArray } from '../utils/is-string-array'; +import { parseListQuerySortBy } from '../utils/list-query-sort'; import { TimedQuery } from '../utils/timed-query'; type ResourceType = 'folder' | 'workflow'; @@ -369,7 +370,7 @@ export class WorkflowRepository extends BaseRepository { // For union, we need to have the same columns, so add NULL as description for folders const columnNames = [...Object.keys(workflowQueryParameters.select ?? {}), 'resource']; - const [sortByColumn, sortByDirection] = this.parseSortingParams( + const { column: sortByColumn, direction: sortByDirection } = parseListQuerySortBy( options.sortBy ?? 'updatedAt:asc', ); @@ -735,7 +736,7 @@ export class WorkflowRepository extends BaseRepository { // For union, we need to have the same columns, so add NULL as description for folders const columnNames = [...Object.keys(workflowQueryParameters.select ?? {}), 'resource']; - const [sortByColumn, sortByDirection] = this.parseSortingParams( + const { column: sortByColumn, direction: sortByDirection } = parseListQuerySortBy( options.sortBy ?? 'updatedAt:asc', ); @@ -1438,15 +1439,10 @@ export class WorkflowRepository extends BaseRepository { return; } - const [column, direction] = this.parseSortingParams(sortBy); + const { column, direction } = parseListQuerySortBy(sortBy); this.applySortingByColumn(qb, column, direction); } - private parseSortingParams(sortBy: string): [string, 'ASC' | 'DESC'] { - const [column, order] = sortBy.split(':'); - return [column, order.toUpperCase() as 'ASC' | 'DESC']; - } - private applySortingByColumn( qb: SelectQueryBuilder, column: string, diff --git a/packages/@n8n/db/src/utils/__tests__/list-query-sort.test.ts b/packages/@n8n/db/src/utils/__tests__/list-query-sort.test.ts new file mode 100644 index 00000000000..12347708309 --- /dev/null +++ b/packages/@n8n/db/src/utils/__tests__/list-query-sort.test.ts @@ -0,0 +1,14 @@ +import { parseListQuerySortBy } from '../list-query-sort'; + +describe('parseListQuerySortBy', () => { + it.each([ + ['createdAt:desc', { column: 'createdAt', direction: 'DESC' }], + ['createdAt:DESC', { column: 'createdAt', direction: 'DESC' }], + ['name:asc', { column: 'name', direction: 'ASC' }], + ['name:ASC', { column: 'name', direction: 'ASC' }], + ['updatedAt', { column: 'updatedAt', direction: 'ASC' }], + ['name:foo', { column: 'name', direction: 'ASC' }], + ] as const)('parses %s', (sortBy, expected) => { + expect(parseListQuerySortBy(sortBy)).toEqual(expected); + }); +}); diff --git a/packages/@n8n/db/src/utils/list-query-sort.ts b/packages/@n8n/db/src/utils/list-query-sort.ts new file mode 100644 index 00000000000..4a383b290f5 --- /dev/null +++ b/packages/@n8n/db/src/utils/list-query-sort.ts @@ -0,0 +1,14 @@ +export type ListQuerySortDirection = 'ASC' | 'DESC'; + +export type ListQuerySort = { + column: string; + direction: ListQuerySortDirection; +}; + +export function parseListQuerySortBy(sortBy: string): ListQuerySort { + const [column, order] = sortBy.split(':'); + return { + column, + direction: order?.toLowerCase() === 'desc' ? 'DESC' : 'ASC', + }; +} diff --git a/packages/cli/eslint.config.mjs b/packages/cli/eslint.config.mjs index b06940d464f..a496a064e88 100644 --- a/packages/cli/eslint.config.mjs +++ b/packages/cli/eslint.config.mjs @@ -117,8 +117,6 @@ export default defineConfig( // migration to the `@PublicApiController` + service pattern (API-70). NEVER add to this // list — a new violation must fail CI. Entries are removed as each file migrates. files: [ - './src/public-api/v1/handlers/credentials/credentials.handler.ts', - './src/public-api/v1/handlers/credentials/credentials.service.ts', './src/public-api/v1/handlers/data-tables/data-tables.handler.ts', './src/public-api/v1/handlers/data-tables/data-tables.service.ts', './src/public-api/v1/handlers/discover/discover.handler.ts', diff --git a/packages/cli/src/credentials/__tests__/credentials.controller.test.ts b/packages/cli/src/credentials/__tests__/credentials.controller.test.ts index 05eb26cbb1a..59fcbfd6746 100644 --- a/packages/cli/src/credentials/__tests__/credentials.controller.test.ts +++ b/packages/cli/src/credentials/__tests__/credentials.controller.test.ts @@ -65,6 +65,7 @@ describe('CredentialsController', () => { mock(), // instanceCredentialAssignmentRepository mock(), // instanceCredentialUseRegistry mock(), // dbLockService + mock(), // eventService ); // Spy on methods that need to be mocked in tests @@ -1034,14 +1035,14 @@ describe('CredentialsController', () => { describe('deleteCredentials', () => { const credentialId = 'cred-del-1'; - it('should emit "private-credential-deleted" when deleting a resolvable credential', async () => { + it('should delete an accessible credential via CredentialsService', async () => { const privateCredential = mock({ id: credentialId, type: 'gmailOAuth2', isResolvable: true, }); credentialsFinderService.findCredentialForUser.mockResolvedValue(privateCredential); - vi.spyOn(credentialsService, 'delete').mockResolvedValue(undefined); + const deleteSpy = vi.spyOn(credentialsService, 'delete').mockResolvedValue(undefined); const deleteReq = { user: { id: 'u1' }, @@ -1050,32 +1051,10 @@ describe('CredentialsController', () => { await credentialsController.deleteCredentials(deleteReq); - expect(emitSpy).toHaveBeenCalledWith('private-credential-deleted', { - user: deleteReq.user, - credentialType: privateCredential.type, - credentialId: privateCredential.id, + expect(deleteSpy).toHaveBeenCalledWith(deleteReq.user, credentialId, { + includeInstanceCredentials: true, }); }); - - it('should not emit "private-credential-deleted" when deleting a static credential', async () => { - const staticCredential = mock({ - id: credentialId, - type: 'gmailOAuth2', - isResolvable: false, - }); - credentialsFinderService.findCredentialForUser.mockResolvedValue(staticCredential); - vi.spyOn(credentialsService, 'delete').mockResolvedValue(undefined); - - const deleteReq = { - user: { id: 'u1' }, - params: { credentialId }, - } as unknown as CredentialRequest.Delete; - - await credentialsController.deleteCredentials(deleteReq); - - const emittedEventNames = emitSpy.mock.calls.map((call) => call[0]); - expect(emittedEventNames).not.toContain('private-credential-deleted'); - }); }); describe('disconnectOauthToken', () => { diff --git a/packages/cli/src/credentials/__tests__/credentials.service.test.ts b/packages/cli/src/credentials/__tests__/credentials.service.test.ts index 521223a42d6..648cf760e64 100644 --- a/packages/cli/src/credentials/__tests__/credentials.service.test.ts +++ b/packages/cli/src/credentials/__tests__/credentials.service.test.ts @@ -36,6 +36,7 @@ import * as validation from '@/credentials/validation'; import type { CredentialsHelper } from '@/credentials-helper'; import { CredentialNotFoundError } from '@/errors/credential-not-found.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; +import type { EventService } from '@/events/event.service'; import type { ExternalHooks } from '@/external-hooks'; 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'; @@ -94,6 +95,7 @@ describe('CredentialsService', () => { const instanceCredentialAssignmentRepository = mock(); const instanceCredentialUseRegistry = mock(); const dbLockService = mock(); + const eventService = mock(); const service = new CredentialsService( credentialsRepository, @@ -117,6 +119,7 @@ describe('CredentialsService', () => { instanceCredentialAssignmentRepository, instanceCredentialUseRegistry, dbLockService, + eventService, ); beforeEach(() => { @@ -1002,7 +1005,7 @@ describe('CredentialsService', () => { describe('testById', () => { it('throws CredentialNotFoundError when credential does not exist', async () => { - credentialsFinderService.findCredentialById.mockResolvedValue(null); + credentialsFinderService.findById.mockResolvedValue(null); await expect(service.testById(ownerUser.id, 'missing-credential')).rejects.toThrow( CredentialNotFoundError, @@ -1011,7 +1014,7 @@ describe('CredentialsService', () => { }); it('does not expose instance credentials through public API testing', async () => { - credentialsFinderService.findCredentialById.mockResolvedValue( + credentialsFinderService.findById.mockResolvedValue( mock({ id: 'instance-credential', usageScope: 'instance', @@ -1034,13 +1037,13 @@ describe('CredentialsService', () => { const decryptedData = { accessToken: 'secret-token' } as ICredentialDataDecryptedObject; const testResult = { status: 'OK', message: 'Credential tested successfully' } as const; - credentialsFinderService.findCredentialById.mockResolvedValue(storedCredential); + credentialsFinderService.findById.mockResolvedValue(storedCredential); credentialsTester.testCredentials.mockResolvedValue(testResult); vi.spyOn(service, 'decrypt').mockResolvedValue(decryptedData); const result = await service.testById(ownerUser.id, storedCredential.id); - expect(credentialsFinderService.findCredentialById).toHaveBeenCalledWith(storedCredential.id); + expect(credentialsFinderService.findById).toHaveBeenCalledWith(storedCredential.id); expect(service.decrypt).toHaveBeenCalledWith(storedCredential, true); expect(credentialsTester.testCredentials).toHaveBeenCalledWith( ownerUser.id, @@ -1130,11 +1133,13 @@ describe('CredentialsService', () => { {}, ); expect(credentialsRepository.remove).not.toHaveBeenCalled(); + expect(eventService.emit).not.toHaveBeenCalled(); }); it('deletes instance credentials when management access is explicitly requested', async () => { const credential = mock({ id: 'instance-credential', + type: 'openAiApi', usageScope: 'instance', isResolvable: false, }); @@ -1155,6 +1160,11 @@ describe('CredentialsService', () => { credential.id, ); expect(externalHooks.run).toHaveBeenCalledWith('credentials.delete', [credential.id]); + expect(eventService.emit).toHaveBeenCalledWith('credentials-deleted', { + user: ownerUser, + credentialType: credential.type, + credentialId: credential.id, + }); }); it('does not delete an instance credential bound to a feature', async () => { @@ -1173,6 +1183,73 @@ describe('CredentialsService', () => { service.delete(ownerUser, credential.id, { includeInstanceCredentials: true }), ).rejects.toThrow('instance-ai:model'); expect(externalHooks.run).toHaveBeenCalledWith('credentials.delete', [credential.id]); + expect(eventService.emit).not.toHaveBeenCalled(); + }); + + it('does not emit credentials-deleted when instance credential was already removed', async () => { + const credential = mock({ + id: 'instance-credential', + type: 'openAiApi', + usageScope: 'instance', + isResolvable: false, + }); + credentialsFinderService.findCredentialForUser.mockResolvedValue(credential); + credentialsRepository.deleteInstanceCredentialIfUnassigned.mockResolvedValue({ + status: 'notFound', + }); + + await service.delete(ownerUser, credential.id, { includeInstanceCredentials: true }); + + expect(eventService.emit).not.toHaveBeenCalled(); + }); + + it('emits credentials-deleted and private-credential-deleted for resolvable project credentials', async () => { + const credential = mock({ + id: 'project-credential', + type: 'gmailOAuth2', + usageScope: 'project', + isResolvable: true, + }); + credentialsFinderService.findCredentialForUser.mockResolvedValue(credential); + sharedCredentialsRepository.findCredentialOwningProject.mockResolvedValue( + mock({ id: 'project-1' }), + ); + vi.spyOn(service, 'ensureCanManageEndUserCredential').mockResolvedValue(undefined); + credentialsRepository.remove.mockResolvedValue(credential); + + await service.delete(ownerUser, credential.id); + + expect(eventService.emit).toHaveBeenCalledWith('credentials-deleted', { + user: ownerUser, + credentialType: credential.type, + credentialId: credential.id, + }); + expect(eventService.emit).toHaveBeenCalledWith('private-credential-deleted', { + user: ownerUser, + credentialType: credential.type, + credentialId: credential.id, + }); + }); + + it('should not emit "private-credential-deleted" when deleting a static credential', async () => { + const credential = mock({ + id: 'project-credential', + type: 'gmailOAuth2', + usageScope: 'project', + isResolvable: false, + }); + credentialsFinderService.findCredentialForUser.mockResolvedValue(credential); + credentialsRepository.remove.mockResolvedValue(credential); + + await service.delete(ownerUser, credential.id); + + expect(eventService.emit).toHaveBeenCalledWith('credentials-deleted', { + user: ownerUser, + credentialType: credential.type, + credentialId: credential.id, + }); + const emittedEventNames = eventService.emit.mock.calls.map((call) => call[0]); + expect(emittedEventNames).not.toContain('private-credential-deleted'); }); }); @@ -1609,6 +1686,24 @@ describe('CredentialsService', () => { }); }); + describe('getManyAndCount', () => { + it('returns credentials with total count using the same path as getMany', async () => { + const credentials = [mock({ id: 'cred-1', shared: [] })]; + credentialsRepository.findManyAndCount.mockResolvedValue([credentials, 1]); + ownershipService.addOwnedByAndSharedWith.mockImplementation((c: any) => c); + + const result = await service.getManyAndCount(ownerUser, { + listQueryOptions: { take: 50, skip: 10 }, + }); + + expect(credentialsRepository.findManyAndCount).toHaveBeenCalledWith({ + take: 50, + skip: 10, + }); + expect(result).toEqual({ credentials, count: 1 }); + }); + }); + describe('getMany', () => { const regularCredential = { id: 'cred-1', @@ -1636,7 +1731,7 @@ describe('CredentialsService', () => { it('should filter by credential:owner role when projectId is for a personal project', async () => { // ARRANGE const personalProject = { id: 'personal-proj', type: 'personal' } as any; - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); projectService.getProject.mockResolvedValue(personalProject); // ACT @@ -1648,7 +1743,7 @@ describe('CredentialsService', () => { // ASSERT expect(projectService.getProject).toHaveBeenCalledWith('personal-proj'); - expect(credentialsRepository.findMany).toHaveBeenCalledWith( + expect(credentialsRepository.findManyAndCount).toHaveBeenCalledWith( expect.objectContaining({ filter: expect.objectContaining({ withRole: 'credential:owner', @@ -1662,7 +1757,7 @@ describe('CredentialsService', () => { it('should not filter by role when projectId is for a team project', async () => { // ARRANGE const teamProject = { id: 'team-proj', type: 'team' } as any; - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); projectService.getProject.mockResolvedValue(teamProject); // ACT @@ -1674,7 +1769,7 @@ describe('CredentialsService', () => { // ASSERT expect(projectService.getProject).toHaveBeenCalledWith('team-proj'); - expect(credentialsRepository.findMany).toHaveBeenCalledWith( + expect(credentialsRepository.findManyAndCount).toHaveBeenCalledWith( expect.objectContaining({ filter: expect.not.objectContaining({ withRole: 'credential:owner', @@ -1685,7 +1780,7 @@ describe('CredentialsService', () => { it('should handle getProject throwing an error', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); projectService.getProject.mockRejectedValue(new Error('Project not found')); // ACT @@ -1696,7 +1791,7 @@ describe('CredentialsService', () => { }); // ASSERT - Should continue without filtering by role - expect(credentialsRepository.findMany).toHaveBeenCalledWith( + expect(credentialsRepository.findManyAndCount).toHaveBeenCalledWith( expect.objectContaining({ filter: expect.not.objectContaining({ withRole: 'credential:owner', @@ -1710,7 +1805,7 @@ describe('CredentialsService', () => { it('should add scopes to credentials when includeScopes is true', async () => { // ARRANGE const projectRelations = [{ projectId: 'proj-1', role: 'project:owner' }] as any; - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); projectService.getProjectRelationsForUser.mockResolvedValue(projectRelations); roleService.addScopes.mockImplementation( (c) => ({ ...c, scopes: ['credential:read', 'credential:update'] }) as any, @@ -1733,7 +1828,7 @@ describe('CredentialsService', () => { it('should not add scopes when includeScopes is false', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); // ACT const result = await service.getMany(ownerUser, { @@ -1759,7 +1854,7 @@ describe('CredentialsService', () => { it('should automatically set includeScopes to true when includeData is true', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); roleService.addScopes.mockImplementation( (c) => ({ ...c, scopes: ['credential:update'] }) as any, ); @@ -1775,7 +1870,7 @@ describe('CredentialsService', () => { it('should include decrypted data when user has credential:update scope', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); roleService.addScopes.mockImplementation( (c) => ({ ...c, scopes: ['credential:update'] }) as any, ); @@ -1792,7 +1887,7 @@ describe('CredentialsService', () => { it('should not include decrypted data when user lacks credential:update scope', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); roleService.addScopes.mockImplementation( (c) => ({ ...c, scopes: ['credential:read'] }) as any, ); @@ -1809,7 +1904,7 @@ describe('CredentialsService', () => { it('should replace oauthTokenData with true when present', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); roleService.addScopes.mockImplementation( (c) => ({ ...c, scopes: ['credential:update'] }) as any, ); @@ -1830,7 +1925,7 @@ describe('CredentialsService', () => { it('should set includeData in listQueryOptions', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); roleService.addScopes.mockImplementation( (c) => ({ ...c, scopes: ['credential:update'] }) as any, ); @@ -1841,7 +1936,7 @@ describe('CredentialsService', () => { }); // ASSERT - expect(credentialsRepository.findMany).toHaveBeenCalledWith( + expect(credentialsRepository.findManyAndCount).toHaveBeenCalledWith( expect.objectContaining({ includeData: true, }), @@ -1855,7 +1950,7 @@ describe('CredentialsService', () => { it('should fetch all relations when filtering by shared.projectId', async () => { // ARRANGE const credWithShared = { ...regularCredential, shared: [] } as any; - credentialsRepository.findMany.mockResolvedValue([credWithShared]); + credentialsRepository.findManyAndCount.mockResolvedValue([[credWithShared], 1]); sharedCredentialsRepository.getAllRelationsForCredentials.mockResolvedValue([ sharedRelation, ]); @@ -1899,7 +1994,7 @@ describe('CredentialsService', () => { it('should not fetch all relations when shared.projectId is not present', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); // ACT await service.getMany(ownerUser, { @@ -1916,7 +2011,7 @@ describe('CredentialsService', () => { describe('with custom select (non-default select)', () => { it('should skip addOwnedByAndSharedWith when select is custom', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); // ACT await service.getMany(ownerUser, { @@ -1931,7 +2026,7 @@ describe('CredentialsService', () => { it('should skip fetching all relations when select is custom', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); // ACT await service.getMany(ownerUser, { @@ -2201,7 +2296,7 @@ describe('CredentialsService', () => { describe('with includeGlobal = true', () => { it('should include global credentials for owner users', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); credentialsRepository.findAllGlobalCredentials.mockResolvedValue([globalCredential]); // ACT @@ -2210,7 +2305,7 @@ describe('CredentialsService', () => { }); // ASSERT - expect(credentialsRepository.findMany).toHaveBeenCalled(); + expect(credentialsRepository.findManyAndCount).toHaveBeenCalled(); expect(credentialsRepository.findAllGlobalCredentials).toHaveBeenCalledWith({ includeData: false, filters: { dependency: undefined }, @@ -2306,7 +2401,7 @@ describe('CredentialsService', () => { it('should forward the credential type filter to the global credentials lookup (owner user)', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([]); + credentialsRepository.findManyAndCount.mockResolvedValue([[], 0]); credentialsRepository.findAllGlobalCredentials.mockResolvedValue([]); // ACT @@ -2347,7 +2442,7 @@ describe('CredentialsService', () => { it('should not pass a type filter when the listQueryOptions filter has no type', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([]); + credentialsRepository.findManyAndCount.mockResolvedValue([[], 0]); credentialsRepository.findAllGlobalCredentials.mockResolvedValue([]); // ACT @@ -2363,7 +2458,7 @@ describe('CredentialsService', () => { describe('with includeGlobal = false', () => { it('should exclude global credentials when includeGlobal is false', async () => { // ARRANGE - credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findManyAndCount.mockResolvedValue([[regularCredential], 1]); // ACT const result = await service.getMany(ownerUser, { @@ -2371,7 +2466,7 @@ describe('CredentialsService', () => { }); // ASSERT - expect(credentialsRepository.findMany).toHaveBeenCalled(); + expect(credentialsRepository.findManyAndCount).toHaveBeenCalled(); expect(credentialsRepository.findAllGlobalCredentials).not.toHaveBeenCalled(); expect(result).toHaveLength(1); expect(result[0].id).toBe('cred-1'); @@ -3380,6 +3475,63 @@ describe('CredentialsService', () => { }); }); + describe('dataMerge', () => { + beforeEach(() => { + credentialsHelper.getCredentialsProperties.mockReturnValue([]); + credentialsRepository.create.mockImplementation((data) => ({ ...data }) as never); + vi.spyOn(checkAccess, 'userHasScopes').mockResolvedValue(true); + }); + + it('keeps incoming data as-is when dataMerge is replace', async () => { + vi.spyOn(service, 'decrypt').mockResolvedValue({ + apiKey: 'stored-secret', + oauthTokenData: { access_token: 'token' }, + }); + const unredactSpy = vi.spyOn(service, 'unredact'); + const existingCredential = mockExistingCredential({ + name: 'Test Credential', + type: 'apiKey', + data: {}, + }); + + const prepared = await service.prepareUpdateData( + ownerUser, + { name: 'Test Credential', type: 'apiKey', data: { apiKey: 'new-secret' } }, + existingCredential, + { dataMerge: 'replace' }, + ); + + expect(unredactSpy).not.toHaveBeenCalled(); + expect(prepared.data).toEqual({ apiKey: 'new-secret' }); + }); + + it('merges stored data then unredacts when dataMerge is partial', async () => { + vi.spyOn(service, 'decrypt').mockResolvedValue({ + apiKey: 'stored-secret', + other: 'keep-me', + }); + const unredactSpy = vi.spyOn(service, 'unredact').mockImplementation((data) => data); + const existingCredential = mockExistingCredential({ + name: 'Test Credential', + type: 'apiKey', + data: {}, + }); + + await service.prepareUpdateData( + ownerUser, + { name: 'Test Credential', type: 'apiKey', data: { apiKey: '***' } }, + existingCredential, + { dataMerge: 'partial' }, + ); + + expect(unredactSpy).toHaveBeenCalledWith( + { apiKey: '***', other: 'keep-me' }, + { apiKey: 'stored-secret', other: 'keep-me' }, + [], + ); + }); + }); + describe('assigned instance credentials', () => { const existingCredential = mockExistingCredential({ id: 'instance-credential-id', diff --git a/packages/cli/src/credentials/credentials-finder.service.ts b/packages/cli/src/credentials/credentials-finder.service.ts index d55b47bca10..a895bb13188 100644 --- a/packages/cli/src/credentials/credentials-finder.service.ts +++ b/packages/cli/src/credentials/credentials-finder.service.ts @@ -52,15 +52,19 @@ export class CredentialsFinderService { }); } - async findCredentialById( + async findById( credentialId: string, - options: { includeInstanceCredentials?: boolean } = {}, + options: { + includeInstanceCredentials?: boolean; + includeSharedProject?: boolean; + } = {}, ): Promise { return await this.credentialsRepository.findOne({ where: { id: credentialId, usageScope: options.includeInstanceCredentials ? In(['project', 'instance']) : 'project', }, + relations: options.includeSharedProject ? { shared: { project: true } } : undefined, }); } diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index c7b5777473a..c002d34a316 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -462,20 +462,6 @@ export class CredentialsController { includeInstanceCredentials: true, }); - this.eventService.emit('credentials-deleted', { - user: req.user, - credentialType: credential.type, - credentialId: credential.id, - }); - - if (credential.isResolvable) { - this.eventService.emit('private-credential-deleted', { - user: req.user, - credentialType: credential.type, - credentialId: credential.id, - }); - } - return true; } diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts index d708e930487..837da8dbf8f 100644 --- a/packages/cli/src/credentials/credentials.service.ee.ts +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -97,7 +97,7 @@ export class EnterpriseCredentialsService { } async getOne(credentialId: string) { - return await this.credentialsFinderService.findCredentialById(credentialId); + return await this.credentialsFinderService.findById(credentialId); } async getOneForUser(user: User, credentialId: string, includeDecryptedData: boolean) { diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 7c0f9b279a3..4bcdc77ac95 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -56,14 +56,15 @@ 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'; +import { EventService } from '@/events/event.service'; import { ExternalHooks } from '@/external-hooks'; import { validateEntity } from '@/generic-helpers'; +import { getChangedSharedFields } from '@/modules/dynamic-credentials.ee/services/shared-fields'; import { ExternalSecretsConfig } from '@/modules/external-secrets.ee/external-secrets.config'; import { SecretsProviderAccessCheckService } from '@/modules/external-secrets.ee/secret-provider-access-check.service.ee'; import { DCR_MANAGED_CREDENTIAL_FIELDS } from '@/oauth/dcr-managed-fields'; import { validateOAuthUrl } from '@/oauth/validate-oauth-url'; import { userHasScopes } from '@/permissions.ee/check-access'; -import { getChangedSharedFields } from '@/modules/dynamic-credentials.ee/services/shared-fields'; import type { CredentialRequest, ListQuery } from '@/requests'; import { CredentialsTester } from '@/services/credentials-tester.service'; import { OwnershipService } from '@/services/ownership.service'; @@ -97,6 +98,24 @@ type PrepareUpdateDataOptions = { */ clearOauthTokenData?: boolean; operationContext?: OperationContext; + /** + * How to combine incoming `data` with the stored decrypted blob: + * - `unredact` (default): replace `***` redacted values from stored data + * - `replace`: use incoming data as-is + * - `partial`: merge stored + incoming, then unredact + */ + dataMerge?: 'unredact' | 'replace' | 'partial'; +}; + +type GetManyOptions = { + listQueryOptions?: ListQuery.Options; + includeScopes?: boolean; + includeData?: boolean; + onlySharedWithMe?: boolean; + includeGlobal?: boolean; + filters?: { + externalSecretsStore?: string; + }; }; type UpdateOptions = { @@ -142,6 +161,11 @@ type GetManyCredentialsOptions = { }; }; +type CredentialsWithCount = { + credentials: T[]; + count: number; +}; + type WorkflowCredentialResult = { id: string; name: string; @@ -190,6 +214,7 @@ export class CredentialsService { private readonly instanceCredentialAssignmentRepository: InstanceCredentialAssignmentRepository, private readonly instanceCredentialUseRegistry: InstanceCredentialUseRegistry, private readonly dbLockService: DbLockService, + private readonly eventService: EventService, ) {} async countConnectedUsers(credentialId: string): Promise { @@ -256,31 +281,23 @@ export class CredentialsService { async getMany( user: User, - options: { - listQueryOptions?: ListQuery.Options; - includeScopes?: boolean; - includeData: true; - onlySharedWithMe?: boolean; - includeGlobal?: boolean; - filters?: { - externalSecretsStore?: string; - }; - }, + options: GetManyOptions & { includeData: true }, ): Promise>>; + async getMany(user: User, options?: GetManyOptions): Promise; async getMany( user: User, - options?: { - listQueryOptions?: ListQuery.Options; - includeScopes?: boolean; - includeData?: boolean; - onlySharedWithMe?: boolean; - includeGlobal?: boolean; - filters?: { - externalSecretsStore?: string; - }; - }, - ): Promise; - async getMany( + options: GetManyOptions = {}, + ): Promise> | CredentialsEntity[]> { + const { credentials } = await this.getManyAndCount(user, options); + return credentials; + } + + async getManyAndCount( + user: User, + options: GetManyOptions & { includeData: true }, + ): Promise>>; + async getManyAndCount(user: User, options?: GetManyOptions): Promise; + async getManyAndCount( user: User, { listQueryOptions = {}, @@ -289,17 +306,10 @@ export class CredentialsService { onlySharedWithMe = false, includeGlobal = false, filters = {}, - }: { - listQueryOptions?: ListQuery.Options; - includeScopes?: boolean; - includeData?: boolean; - onlySharedWithMe?: boolean; - includeGlobal?: boolean; - filters?: { - externalSecretsStore?: string; - }; - } = {}, - ): Promise> | CredentialsEntity[]> { + }: GetManyOptions = {}, + ): Promise< + CredentialsWithCount | CredentialsEntity> + > { const { externalSecretsStore } = filters; const returnAll = hasGlobalScope(user, 'credential:list'); const isDefaultSelect = !listQueryOptions.select; @@ -310,7 +320,7 @@ export class CredentialsService { : undefined; if (externalSecretsStore && !dependencyFilter) { - return []; + return { credentials: [], count: 0 }; } // Auto-enable includeScopes when includeData is requested @@ -318,27 +328,19 @@ export class CredentialsService { includeScopes = true; } - let credentials: CredentialsEntity[]; + const fetchOptions: GetManyCredentialsOptions = { + listQueryOptions, + includeGlobal, + includeData, + onlySharedWithMe, + filters: { dependency: dependencyFilter }, + }; - if (returnAll) { - credentials = await this.getManyForAdminUser(user, { - listQueryOptions, - includeGlobal, - includeData, - onlySharedWithMe, - filters: { dependency: dependencyFilter }, - }); - } else { - credentials = await this.getManyForMemberUser(user, { - listQueryOptions, - includeGlobal, - includeData, - onlySharedWithMe, - filters: { dependency: dependencyFilter }, - }); - } + const { credentials, count } = returnAll + ? await this.getManyForAdminUser(user, fetchOptions) + : await this.getManyForMemberUser(user, fetchOptions); - return await this.enrichCredentials( + const enriched = await this.enrichCredentials( credentials, user, isDefaultSelect, @@ -347,6 +349,8 @@ export class CredentialsService { listQueryOptions, onlySharedWithMe, ); + + return { credentials: enriched, count }; } private async getManyForAdminUser( @@ -358,7 +362,7 @@ export class CredentialsService { onlySharedWithMe, filters, }: GetManyCredentialsOptions, - ): Promise { + ): Promise { const { dependency: dependencyFilter } = filters ?? {}; const typeFilter = this.extractTypeFilter(listQueryOptions); @@ -367,47 +371,50 @@ export class CredentialsService { const sharingOptions = { ...(onlySharedWithMe ? { onlySharedWithMe: true } : {}), }; - const { credentials } = await this.credentialsRepository.getManyAndCountWithSharingSubquery( - user, - sharingOptions, - { + const { credentials, count } = + await this.credentialsRepository.getManyAndCountWithSharingSubquery(user, sharingOptions, { ...listQueryOptions, ...(includeData ? { includeData: true } : {}), filters: { dependency: dependencyFilter, }, - }, - ); + }); if (includeGlobal) { - return await this.addGlobalCredentials( - credentials, - includeData, - dependencyFilter, - typeFilter, - ); + return { + credentials: await this.addGlobalCredentials( + credentials, + includeData, + dependencyFilter, + typeFilter, + ), + count, + }; } - return credentials; + return { credentials, count }; } await this.applyPersonalProjectFilter(listQueryOptions); - let credentials = await this.credentialsRepository.findMany({ + const [credentials, count] = await this.credentialsRepository.findManyAndCount({ ...listQueryOptions, ...(includeData ? { includeData: true } : {}), }); if (includeGlobal) { - credentials = await this.addGlobalCredentials( - credentials, - includeData, - dependencyFilter, - typeFilter, - ); + return { + credentials: await this.addGlobalCredentials( + credentials, + includeData, + dependencyFilter, + typeFilter, + ), + count, + }; } - return credentials; + return { credentials, count }; } private async getManyForMemberUser( @@ -419,7 +426,7 @@ export class CredentialsService { onlySharedWithMe, filters, }: GetManyCredentialsOptions, - ): Promise { + ): Promise { const { dependency: dependencyFilter } = filters ?? {}; const typeFilter = this.extractTypeFilter(listQueryOptions); @@ -432,7 +439,7 @@ export class CredentialsService { id: listQueryOptions.filter.projectId as string, }); if (!project) { - return []; + return { credentials: [], count: 0 }; } isPersonalProject = project.type === 'personal'; personalProjectOwnerId = project.creatorId; @@ -451,7 +458,7 @@ export class CredentialsService { if (isPersonalProject && personalProjectOwnerId) { // Prevent users from accessing another user's personal project credentials if (personalProjectOwnerId !== user.id && !hasGlobalScope(user, 'credential:read')) { - return []; + return { credentials: [], count: 0 }; } sharingOptions.isPersonalProject = true; sharingOptions.personalProjectOwnerId = personalProjectOwnerId; @@ -468,29 +475,28 @@ export class CredentialsService { sharingOptions.credentialRoles = credentialRoles; } - // Use the new subquery-based repository method - const { credentials } = await this.credentialsRepository.getManyAndCountWithSharingSubquery( - user, - sharingOptions, - { + const { credentials, count } = + await this.credentialsRepository.getManyAndCountWithSharingSubquery(user, sharingOptions, { ...listQueryOptions, ...(includeData ? { includeData: true } : {}), filters: { dependency: dependencyFilter, }, - }, - ); + }); if (includeGlobal) { - return await this.addGlobalCredentials( - credentials, - includeData, - dependencyFilter, - typeFilter, - ); + return { + credentials: await this.addGlobalCredentials( + credentials, + includeData, + dependencyFilter, + typeFilter, + ), + count, + }; } - return credentials; + return { credentials, count }; } private async applyPersonalProjectFilter(listQueryOptions: ListQuery.Options): Promise { @@ -516,24 +522,6 @@ export class CredentialsService { } } - private async enrichCredentials( - credentials: CredentialsEntity[], - user: User, - isDefaultSelect: boolean, - includeScopes: boolean, - includeData: true, - listQueryOptions: ListQuery.Options, - onlySharedWithMe: boolean, - ): Promise>>; - private async enrichCredentials( - credentials: CredentialsEntity[], - user: User, - isDefaultSelect: boolean, - includeScopes: boolean, - includeData: boolean, - listQueryOptions: ListQuery.Options, - onlySharedWithMe: boolean, - ): Promise; private async enrichCredentials( credentials: CredentialsEntity[], user: User, @@ -756,6 +744,24 @@ export class CredentialsService { }); } + private applyDataMerge( + incomingData: ICredentialDataDecryptedObject, + decryptedData: ICredentialDataDecryptedObject, + credentialType: string, + dataMerge: NonNullable, + ): ICredentialDataDecryptedObject { + if (dataMerge === 'replace') { + return incomingData; + } + const dataToUnredact = + dataMerge === 'partial' ? { ...decryptedData, ...incomingData } : incomingData; + return this.unredact( + dataToUnredact, + decryptedData, + this.getCredentialTypeProperties(credentialType), + ); + } + async prepareUpdateData( user: User, data: CredentialRequest.CredentialProperties, @@ -763,13 +769,15 @@ export class CredentialsService { options?: PrepareUpdateDataOptions, ): Promise { const decryptedData = await this.decrypt(existingCredential, true); + const dataMerge = options?.dataMerge ?? 'unredact'; const mergedData = deepCopy(data); if (mergedData.data) { - mergedData.data = this.unredact( + mergedData.data = this.applyDataMerge( mergedData.data, decryptedData, - this.getCredentialTypeProperties(existingCredential.type), + existingCredential.type, + dataMerge, ); } if (existingCredential.usageScope === 'instance') { @@ -810,15 +818,13 @@ export class CredentialsService { await validateEntity(updateData); - // Do not overwrite the oauth data else data like the access or refresh token would get lost - // every time anybody changes anything on the credentials even if it is just the name. - // Exception: when toggling to private (Static→Private), the shared token must be cleared. - if (decryptedData.oauthTokenData && !options?.clearOauthTokenData) { - // @ts-expect-error data is typed as encrypted string - updateData.data.oauthTokenData = decryptedData.oauthTokenData; - } - - if (!options?.clearOauthTokenData) { + // Keep oauth / DCR fields unless the caller replaces the blob or clears the token + // (e.g. Static→Private toggle). + if (dataMerge !== 'replace' && !options?.clearOauthTokenData) { + if (decryptedData.oauthTokenData) { + // @ts-expect-error data is typed as encrypted string + updateData.data.oauthTokenData = decryptedData.oauthTokenData; + } this.restoreHiddenDcrFields( existingCredential.type, updateData.data as unknown as ICredentialDataDecryptedObject, @@ -826,10 +832,12 @@ export class CredentialsService { ); } - this.validateOAuthCredentialUrls( - updateData.type, - updateData.data as unknown as ICredentialDataDecryptedObject, - ); + if (updateData.data) { + this.validateOAuthCredentialUrls( + updateData.type, + updateData.data as unknown as ICredentialDataDecryptedObject, + ); + } return updateData; } @@ -1162,6 +1170,10 @@ export class CredentialsService { return result; } + async findCredentialOwningProject(credentialId: string) { + return await this.sharedCredentialsRepository.findCredentialOwningProject(credentialId); + } + /** * Deletes a credential. * @@ -1200,10 +1212,30 @@ export class CredentialsService { `This credential is assigned to credential use "${result.credentialUseIds.join(', ')}" and cannot be deleted`, ); } + if (result.status === 'deleted') { + this.emitCredentialDeleted(user, credential); + } return; } await this.credentialsRepository.remove(credential); + this.emitCredentialDeleted(user, credential); + } + + private emitCredentialDeleted(user: User, credential: CredentialsEntity) { + this.eventService.emit('credentials-deleted', { + user, + credentialType: credential.type, + credentialId: credential.id, + }); + + if (credential.isResolvable) { + this.eventService.emit('private-credential-deleted', { + user, + credentialType: credential.type, + credentialId: credential.id, + }); + } } async test(userId: User['id'], credentials: ICredentialsDecrypted) { @@ -1211,7 +1243,7 @@ export class CredentialsService { } async testById(userId: User['id'], credentialId: string) { - const storedCredential = await this.credentialsFinderService.findCredentialById(credentialId); + const storedCredential = await this.credentialsFinderService.findById(credentialId); // Dynamic-credential flows only; admins test instance credentials via testWithCredentials if (!storedCredential || storedCredential.usageScope !== 'project') { diff --git a/packages/cli/src/databases/repositories/__tests__/credentials.repository.test.ts b/packages/cli/src/databases/repositories/__tests__/credentials.repository.test.ts index 8e5eaa57867..7ec4a971431 100644 --- a/packages/cli/src/databases/repositories/__tests__/credentials.repository.test.ts +++ b/packages/cli/src/databases/repositories/__tests__/credentials.repository.test.ts @@ -12,58 +12,6 @@ describe('CredentialsRepository', () => { vi.resetAllMocks(); }); - describe('findMany', () => { - const credentialsId = 'cred_123'; - const credential = mock({ id: credentialsId }); - - test('return `data` property if `includeData:true` and select is using the record syntax', async () => { - // ARRANGE - entityManager.find.mockResolvedValueOnce([credential]); - - // ACT - const credentials = await repository.findMany({ includeData: true, select: { id: true } }); - - // ASSERT - expect(credentials).toHaveLength(1); - expect(credentials[0]).toHaveProperty('data'); - }); - - test('return `data` property if `includeData:true` and select is using the array syntax', async () => { - // ARRANGE - entityManager.find.mockResolvedValueOnce([credential]); - - // ACT - const credentials = await repository.findMany({ - includeData: true, - //TODO: fix this - // The function's type does not support this but this is what it - // actually gets from the service because the middlewares are typed - // loosely. - select: ['id'] as never, - }); - - // ASSERT - expect(credentials).toHaveLength(1); - expect(credentials[0]).toHaveProperty('data'); - }); - - test('should include isGlobal in default select', async () => { - // ARRANGE - entityManager.find.mockResolvedValueOnce([credential]); - - // ACT - await repository.findMany(); - - // ASSERT - expect(entityManager.find).toHaveBeenCalledWith( - CredentialsEntity, - expect.objectContaining({ - select: expect.arrayContaining(['isGlobal']), - }), - ); - }); - }); - describe('findStartingWith', () => { it('only searches project credential names', async () => { entityManager.find.mockResolvedValueOnce([]); diff --git a/packages/cli/src/modules/agents/builder/__tests__/agents-builder-settings.service.test.ts b/packages/cli/src/modules/agents/builder/__tests__/agents-builder-settings.service.test.ts index 8d32bbf27b7..27b078b1a76 100644 --- a/packages/cli/src/modules/agents/builder/__tests__/agents-builder-settings.service.test.ts +++ b/packages/cli/src/modules/agents/builder/__tests__/agents-builder-settings.service.test.ts @@ -93,7 +93,7 @@ describe('AgentsBuilderSettingsService', () => { id: 'cred-1', type: opts.credentialType ?? 'anthropicApi', }); - credentialsFinderService.findCredentialById.mockResolvedValue(credential); + credentialsFinderService.findById.mockResolvedValue(credential); credentialsService.decrypt.mockReturnValue({ apiKey: opts.apiKey ?? 'sk-test', ...(opts.url ? { url: opts.url } : {}), @@ -272,7 +272,7 @@ describe('AgentsBuilderSettingsService', () => { modelName: 'anthropic.claude-3-5-sonnet-20240620-v1:0', }); aiService.isProxyEnabled.mockReturnValue(false); - credentialsFinderService.findCredentialById.mockResolvedValue( + credentialsFinderService.findById.mockResolvedValue( mock({ id: 'cred-1', type: 'aws' }), ); credentialsService.decrypt.mockReturnValue({ @@ -320,7 +320,7 @@ describe('AgentsBuilderSettingsService', () => { credentialId: 'cred-deleted', modelName: 'claude-3-5-sonnet', }); - credentialsFinderService.findCredentialById.mockResolvedValue(null); + credentialsFinderService.findById.mockResolvedValue(null); aiService.isProxyEnabled.mockReturnValue(false); process.env.N8N_AI_ANTHROPIC_KEY = 'sk-env'; diff --git a/packages/cli/src/modules/agents/builder/__tests__/builder-model-live-lookup.service.test.ts b/packages/cli/src/modules/agents/builder/__tests__/builder-model-live-lookup.service.test.ts index 689cc1fbff2..c97d29d9016 100644 --- a/packages/cli/src/modules/agents/builder/__tests__/builder-model-live-lookup.service.test.ts +++ b/packages/cli/src/modules/agents/builder/__tests__/builder-model-live-lookup.service.test.ts @@ -53,7 +53,7 @@ describe('BuilderModelLiveLookupService', () => { credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue( usable('cred-1', 'openAiApi'), ); - credentialsFinderService.findCredentialById.mockResolvedValue(mock()); + credentialsFinderService.findById.mockResolvedValue(mock()); credentialsService.decrypt.mockResolvedValue({ apiKey: 'sk-key', url: 'https://openai-compatible.example/v1', @@ -89,7 +89,7 @@ describe('BuilderModelLiveLookupService', () => { credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue( usable('cred-1', 'openAiApi'), ); - credentialsFinderService.findCredentialById.mockResolvedValue(mock()); + credentialsFinderService.findById.mockResolvedValue(mock()); credentialsService.decrypt.mockResolvedValue({ apiKey: 'sk-key', url: 'https://openai-compatible.example/v1', @@ -115,7 +115,7 @@ describe('BuilderModelLiveLookupService', () => { credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue( usable('cred-1', 'openAiApi'), ); - credentialsFinderService.findCredentialById.mockResolvedValue(mock()); + credentialsFinderService.findById.mockResolvedValue(mock()); credentialsService.decrypt.mockResolvedValue({ apiKey: 'sk-key', url: 'https://api.openai.com/v1', @@ -136,7 +136,7 @@ describe('BuilderModelLiveLookupService', () => { credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue( usable('cred-1', 'openAiApi'), ); - credentialsFinderService.findCredentialById.mockResolvedValue(mock()); + credentialsFinderService.findById.mockResolvedValue(mock()); credentialsService.decrypt.mockResolvedValue({ apiKey: 'sk-key', url: 'https://openai-compatible.example/v1', @@ -154,7 +154,7 @@ describe('BuilderModelLiveLookupService', () => { credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue( usable('cred-1', 'openAiApi'), ); - credentialsFinderService.findCredentialById.mockResolvedValue(mock()); + credentialsFinderService.findById.mockResolvedValue(mock()); credentialsService.decrypt.mockResolvedValue({ apiKey: 'sk-key', url: 'https://openai-compatible.example/v1', @@ -218,7 +218,7 @@ describe('BuilderModelLiveLookupService', () => { credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue( usable('cred-1', 'anthropicApi'), ); - credentialsFinderService.findCredentialById.mockResolvedValue(mock()); + credentialsFinderService.findById.mockResolvedValue(mock()); credentialsService.decrypt.mockResolvedValue({ apiKey: 'sk-key', url: 'https://proxy.local' }); listModelsForProvider.mockResolvedValue([ { id: 'claude-sonnet-4-6', name: 'Claude Sonnet 4.6' }, @@ -242,7 +242,7 @@ describe('BuilderModelLiveLookupService', () => { credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue( usable('cred-1', 'anthropicApi'), ); - credentialsFinderService.findCredentialById.mockResolvedValue(mock()); + credentialsFinderService.findById.mockResolvedValue(mock()); credentialsService.decrypt.mockResolvedValue({ apiKey: 'sk-key' }); // An empty list from a chat provider is far more likely a broken request // or drifted response shape than a real zero-model account — callers must diff --git a/packages/cli/src/modules/agents/builder/agents-builder-settings.service.ts b/packages/cli/src/modules/agents/builder/agents-builder-settings.service.ts index 98a7b0bcd8a..e7ac72877ca 100644 --- a/packages/cli/src/modules/agents/builder/agents-builder-settings.service.ts +++ b/packages/cli/src/modules/agents/builder/agents-builder-settings.service.ts @@ -207,9 +207,7 @@ export class AgentsBuilderSettingsService { return null; } - const credential = await this.credentialsFinderService.findCredentialById( - settings.credentialId, - ); + const credential = await this.credentialsFinderService.findById(settings.credentialId); if (!credential) return null; const data = await this.credentialsService.decrypt(credential, true); diff --git a/packages/cli/src/modules/agents/builder/builder-model-live-lookup.service.ts b/packages/cli/src/modules/agents/builder/builder-model-live-lookup.service.ts index 6085a0716ea..9825b0a5cbc 100644 --- a/packages/cli/src/modules/agents/builder/builder-model-live-lookup.service.ts +++ b/packages/cli/src/modules/agents/builder/builder-model-live-lookup.service.ts @@ -77,7 +77,7 @@ export class BuilderModelLiveLookupService { throw new Error(`Credential ${credentialId} not found or not accessible`); } - const credential = await this.credentialsFinderService.findCredentialById(credentialId); + const credential = await this.credentialsFinderService.findById(credentialId); if (!credential) { throw new Error(`Credential ${credentialId} not found or not accessible`); } diff --git a/packages/cli/src/modules/data-table/data-table.repository.ts b/packages/cli/src/modules/data-table/data-table.repository.ts index 3ea6e111db9..66d790e74f9 100644 --- a/packages/cli/src/modules/data-table/data-table.repository.ts +++ b/packages/cli/src/modules/data-table/data-table.repository.ts @@ -5,7 +5,7 @@ import { } from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; import { GlobalConfig } from '@n8n/config'; -import { Project, withTransaction } from '@n8n/db'; +import { parseListQuerySortBy, Project, withTransaction } from '@n8n/db'; import { Service } from '@n8n/di'; import { DataSource, EntityManager, In, Repository, SelectQueryBuilder } from '@n8n/typeorm'; import { @@ -262,13 +262,8 @@ export class DataTableRepository extends Repository { return; } - const [field, order] = this.parseSortingParams(sortBy); - this.applySortingByField(query, field, order); - } - - private parseSortingParams(sortBy: string): [string, 'DESC' | 'ASC'] { - const [field, order] = sortBy.split(':'); - return [field, order?.toLowerCase() === 'desc' ? 'DESC' : 'ASC']; + const { column, direction } = parseListQuerySortBy(sortBy); + this.applySortingByField(query, column, direction); } private applySortingByField( 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 8ef2942c2dc..cda5225c13f 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 @@ -310,7 +310,7 @@ export class DynamicCredentialsController { res: Response, @Param('credentialId') credentialId: string, ): Promise { - const credential = await this.credentialsFinderService.findCredentialById(credentialId); + const credential = await this.credentialsFinderService.findById(credentialId); if (!credential) { throw new NotFoundError('Credential not found'); diff --git a/packages/cli/src/modules/insights/database/repositories/insights-by-period.repository.ts b/packages/cli/src/modules/insights/database/repositories/insights-by-period.repository.ts index d27493825c5..004e97c71ed 100644 --- a/packages/cli/src/modules/insights/database/repositories/insights-by-period.repository.ts +++ b/packages/cli/src/modules/insights/database/repositories/insights-by-period.repository.ts @@ -1,6 +1,6 @@ import { isValidTimeZone } from '@n8n/api-types'; import { GlobalConfig } from '@n8n/config'; -import { sql } from '@n8n/db'; +import { parseListQuerySortBy, sql } from '@n8n/db'; import { Container, Service } from '@n8n/di'; import type { SelectQueryBuilder } from '@n8n/typeorm'; import { DataSource, LessThanOrEqual, Repository } from '@n8n/typeorm'; @@ -347,11 +347,6 @@ export class InsightsByPeriodRepository extends Repository { return summaryParser.parse(rawRows); } - private parseSortingParams(sortBy: string): [string, 'ASC' | 'DESC'] { - const [column, order] = sortBy.split(':'); - return [column, order.toUpperCase() as 'ASC' | 'DESC']; - } - private async countInsightsByWorkflowGroups( rawRowsQuery: SelectQueryBuilder, ): Promise { @@ -382,7 +377,7 @@ export class InsightsByPeriodRepository extends Repository { endDate: Date; timeZone?: string; }) { - const [sortField, sortOrder] = this.parseSortingParams(sortBy); + const { column: sortField, direction: sortOrder } = parseListQuerySortBy(sortBy); const sumOfExecutions = sql`SUM(CASE WHEN insights.type IN (${TypeToNumber.success.toString()}, ${TypeToNumber.failure.toString()}) THEN value ELSE 0 END)`; const cte = getDateRangesCommonTableExpressionQuery({ dbType, startDate, endDate, timeZone }); diff --git a/packages/cli/src/oauth/__tests__/oauth.service.test.ts b/packages/cli/src/oauth/__tests__/oauth.service.test.ts index 8c5bd874aad..8b68d67a948 100644 --- a/packages/cli/src/oauth/__tests__/oauth.service.test.ts +++ b/packages/cli/src/oauth/__tests__/oauth.service.test.ts @@ -304,7 +304,7 @@ describe('OauthService', () => { user: mock({ id: '123' }), }); - credentialsFinderService.findCredentialById.mockResolvedValue( + credentialsFinderService.findById.mockResolvedValue( mock({ id: 'credential-id', isResolvable: false }), ); credentialsFinderService.findCredentialForUser.mockResolvedValue(mockCredential); @@ -327,7 +327,7 @@ describe('OauthService', () => { user: mock({ id: '123' }), }); - credentialsFinderService.findCredentialById.mockResolvedValue(mockCredential); + credentialsFinderService.findById.mockResolvedValue(mockCredential); credentialsFinderService.findCredentialForUser.mockResolvedValue(mockCredential); const result = await service.getCredentialForAuthFlow(req); diff --git a/packages/cli/src/oauth/oauth.service.ts b/packages/cli/src/oauth/oauth.service.ts index b231cc4b1fa..ec424853a03 100644 --- a/packages/cli/src/oauth/oauth.service.ts +++ b/packages/cli/src/oauth/oauth.service.ts @@ -284,12 +284,9 @@ export class OauthService { // Private credentials are connected per-user, so executing users can authorize // their own account without edit rights. Shared/static credentials store the // token on the shared credential itself, so connecting them still requires edit. - const existingCredential = await this.credentialsFinderService.findCredentialById( - credentialId, - { - includeInstanceCredentials: true, - }, - ); + const existingCredential = await this.credentialsFinderService.findById(credentialId, { + includeInstanceCredentials: true, + }); const requiredScope = existingCredential?.isResolvable ? 'credential:connect' : 'credential:update'; diff --git a/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.service.test.ts b/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.service.test.ts deleted file mode 100644 index 49f34868fc9..00000000000 --- a/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.service.test.ts +++ /dev/null @@ -1,760 +0,0 @@ -import type { CredentialsEntity, Project, SharedCredentials, User } from '@n8n/db'; -import { - CredentialsRepository, - GLOBAL_OWNER_ROLE, - GLOBAL_MEMBER_ROLE, - SharedCredentialsRepository, -} from '@n8n/db'; -import { Container } from '@n8n/di'; -import { mock } from 'vitest-mock-extended'; -import { validate, type Schema } from 'jsonschema'; -import { Cipher, CipherAes256GCM, CipherAes256CBC, EncryptionKeyProxy } from 'n8n-core'; -import type { InstanceSettings } from 'n8n-core'; -import type { GenericValue, IDataObject, INodeProperties } from 'n8n-workflow'; - -import { - buildSharedForCredential, - saveCredential, - toJsonSchema, - updateCredential, -} from '../credentials.service'; - -import { CredentialsService } from '@/credentials/credentials.service'; -import { EventService } from '@/events/event.service'; -import { ExternalSecretsConfig } from '@/modules/external-secrets.ee/external-secrets.config'; -import { SecretsProviderAccessCheckService } from '@/modules/external-secrets.ee/secret-provider-access-check.service.ee'; -import * as checkAccess from '@/permissions.ee/check-access'; -import type { IDependency } from '@/public-api/types'; - -// Set up real Cipher with mocked InstanceSettings for encryption -const cipher = new Cipher( - mock({ encryptionKey: 'test-encryption-key' }), - new CipherAes256GCM(), - new CipherAes256CBC(), - new EncryptionKeyProxy(), -); -Container.set(Cipher, cipher); - -describe('CredentialsService', () => { - let mockExternalSecretsConfig: ExternalSecretsConfig; - const canAccessProviderFromProjectMock = vi.fn(); - const mockSecretsProviderAccessCheckService = mock({ - isProviderAvailableInProject: canAccessProviderFromProjectMock, - }); - beforeEach(() => { - mockExternalSecretsConfig = new ExternalSecretsConfig(); - Container.set(ExternalSecretsConfig, mockExternalSecretsConfig); - Container.set(SecretsProviderAccessCheckService, mockSecretsProviderAccessCheckService); - - canAccessProviderFromProjectMock.mockResolvedValue(true); - }); - describe('buildSharedForCredential', () => { - it('returns one shared entry when credential is shared with one project', () => { - const createdAt = new Date('2024-01-01T00:00:00.000Z'); - const updatedAt = new Date('2024-01-02T00:00:00.000Z'); - const credential = { - shared: [ - { - role: 'credential:owner', - createdAt, - updatedAt, - project: { id: 'proj-1', name: 'My Project' }, - }, - ], - } as unknown as CredentialsEntity; - expect(buildSharedForCredential(credential)).toEqual([ - { - id: 'proj-1', - name: 'My Project', - role: 'credential:owner', - createdAt, - updatedAt, - }, - ]); - }); - - it('returns multiple shared entries and skips shared entries without project', () => { - const createdAt1 = new Date('2024-01-01T00:00:00.000Z'); - const updatedAt1 = new Date('2024-01-02T00:00:00.000Z'); - const createdAt2 = new Date('2024-02-01T00:00:00.000Z'); - const updatedAt2 = new Date('2024-02-02T00:00:00.000Z'); - const credential = { - shared: [ - { - role: 'credential:owner', - createdAt: createdAt1, - updatedAt: updatedAt1, - project: { id: 'proj-1', name: 'Project One' }, - }, - { role: 'credential:user', createdAt: createdAt2, updatedAt: updatedAt2, project: null }, - { - role: 'credential:user', - createdAt: createdAt2, - updatedAt: updatedAt2, - project: { id: 'proj-2', name: 'Project Two' }, - }, - ], - } as unknown as CredentialsEntity; - expect(buildSharedForCredential(credential)).toEqual([ - { - id: 'proj-1', - name: 'Project One', - role: 'credential:owner', - createdAt: createdAt1, - updatedAt: updatedAt1, - }, - { - id: 'proj-2', - name: 'Project Two', - role: 'credential:user', - createdAt: createdAt2, - updatedAt: updatedAt2, - }, - ]); - }); - }); - - describe('saveCredential', () => { - it('forces public API credentials to project usage scope', async () => { - const credentialsService = mock(); - const sharedCredentialsRepository = mock(); - const eventService = mock(); - Container.set(CredentialsService, credentialsService); - Container.set(SharedCredentialsRepository, sharedCredentialsRepository); - Container.set(EventService, eventService); - credentialsService.createUnmanagedCredential.mockResolvedValue({ - id: 'credential-id', - name: 'Credential', - type: 'testApi', - isManaged: false, - isGlobal: false, - isResolvable: false, - resolvableAllowFallback: false, - resolverId: null, - createdAt: new Date(), - updatedAt: new Date(), - scopes: [], - } as never); - sharedCredentialsRepository.findCredentialOwningProject.mockResolvedValue(undefined); - const payload = { - type: 'testApi', - name: 'Credential', - data: { apiKey: 'secret' }, - usageScope: 'instance', - }; - - await saveCredential(payload, mock()); - - expect(credentialsService.createUnmanagedCredential).toHaveBeenCalledWith( - { - type: payload.type, - name: payload.name, - data: payload.data, - projectId: undefined, - isResolvable: undefined, - usageScope: 'project', - }, - expect.anything(), - ); - }); - }); - - describe('toJsonSchema', () => { - it('should create separate conditionals for different values of the same dependant field', () => { - // This test simulates the JWT auth credential scenario where - // multiple properties depend on the same field (keyType) but with different values - const properties: INodeProperties[] = [ - { - name: 'keyType', - type: 'options', - options: [ - { value: 'passphrase', name: 'Passphrase' }, - { value: 'pemKey', name: 'PEM Key' }, - ], - displayName: 'Key Type', - default: 'passphrase', - }, - { - name: 'secret', - type: 'string', - required: true, - displayName: 'Secret', - default: '', - displayOptions: { - show: { - keyType: ['passphrase'], - }, - }, - }, - { - name: 'privateKey', - type: 'string', - required: true, - displayName: 'Private Key', - default: '', - displayOptions: { - show: { - keyType: ['pemKey'], - }, - }, - }, - { - name: 'publicKey', - type: 'string', - required: true, - displayName: 'Public Key', - default: '', - displayOptions: { - show: { - keyType: ['pemKey'], - }, - }, - }, - ]; - - const schema = toJsonSchema(properties); - - const props = schema.properties as IDataObject; - expect(props).toBeDefined(); - expect(props.keyType).toEqual({ - type: 'string', - enum: ['passphrase', 'pemKey'], - }); - - // All conditional fields should not be globally required - expect(schema.required).not.toContain('secret'); - expect(schema.required).not.toContain('privateKey'); - expect(schema.required).not.toContain('publicKey'); - - // Should have 2 separate conditionals (one for each keyType value) - const allOf = schema.allOf as GenericValue[] | IDataObject[]; - expect(Array.isArray(allOf)).toBe(true); - expect(allOf?.length).toBe(2); - - // Find conditional for passphrase - const passphraseCondition = allOf?.find( - (cond) => (cond as any).if?.properties?.keyType?.enum?.[0] === 'passphrase', - ) as IDependency; - expect(passphraseCondition).toBeDefined(); - expect(passphraseCondition.then?.allOf).toHaveLength(1); - expect(passphraseCondition.then?.allOf[0].required).toContain('secret'); - expect(passphraseCondition.then?.allOf[0].required).not.toContain('privateKey'); - expect(passphraseCondition.then?.allOf[0].required).not.toContain('publicKey'); - - // Find conditional for pemKey - const pemKeyCondition = allOf?.find( - (cond) => (cond as any).if?.properties?.keyType?.enum?.[0] === 'pemKey', - ) as IDependency; - expect(pemKeyCondition).toBeDefined(); - expect(pemKeyCondition.then?.allOf).toHaveLength(2); - expect( - pemKeyCondition.then?.allOf.some((req: any) => req.required?.includes('privateKey')), - ).toBe(true); - expect( - pemKeyCondition.then?.allOf.some((req: any) => req.required?.includes('publicKey')), - ).toBe(true); - expect(pemKeyCondition.then?.allOf.some((req: any) => req.required?.includes('secret'))).toBe( - false, - ); - }); - - it('should handle properties with no displayOptions as globally required', () => { - const properties: INodeProperties[] = [ - { name: 'apiKey', type: 'string', required: true, displayName: 'API Key', default: '' }, - { name: 'domain', type: 'string', required: true, displayName: 'Domain', default: '' }, - { - name: 'optionalField', - type: 'string', - required: false, - displayName: 'Optional', - default: '', - }, - ]; - - const schema = toJsonSchema(properties); - - expect(schema.required).toEqual(expect.arrayContaining(['apiKey', 'domain'])); - expect(schema.required).not.toContain('optionalField'); - expect(schema.allOf).toBeUndefined(); - }); - - it('should handle mix of required and conditional properties', () => { - const properties: INodeProperties[] = [ - { name: 'apiKey', type: 'string', required: true, displayName: 'API Key', default: '' }, - { - name: 'authType', - type: 'options', - required: true, - options: [ - { value: 'basic', name: 'Basic' }, - { value: 'oauth2', name: 'OAuth2' }, - ], - displayName: 'Auth Type', - default: 'basic', - }, - { - name: 'username', - type: 'string', - required: true, - displayName: 'Username', - default: '', - displayOptions: { - show: { - authType: ['basic'], - }, - }, - }, - { - name: 'password', - type: 'string', - required: true, - displayName: 'Password', - default: '', - displayOptions: { - show: { - authType: ['basic'], - }, - }, - }, - { - name: 'clientId', - type: 'string', - required: true, - displayName: 'Client ID', - default: '', - displayOptions: { - show: { - authType: ['oauth2'], - }, - }, - }, - ]; - - const schema = toJsonSchema(properties); - - // apiKey and authType should be globally required - expect(schema.required).toEqual(expect.arrayContaining(['apiKey', 'authType'])); - // Conditional fields should not be globally required - expect(schema.required).not.toContain('username'); - expect(schema.required).not.toContain('password'); - expect(schema.required).not.toContain('clientId'); - - // Should have 2 conditionals - const allOf = schema.allOf as GenericValue[] | IDataObject[]; - expect(allOf?.length).toBe(2); - }); - - it('should handle properties with multiple options depending on same field', () => { - const properties: INodeProperties[] = [ - { - name: 'operation', - type: 'options', - options: [ - { value: 'create', name: 'Create' }, - { value: 'update', name: 'Update' }, - { value: 'delete', name: 'Delete' }, - ], - displayName: 'Operation', - default: 'create', - }, - { - name: 'createField', - type: 'string', - required: true, - displayName: 'Create Field', - default: '', - displayOptions: { - show: { - operation: ['create'], - }, - }, - }, - { - name: 'updateField', - type: 'string', - required: true, - displayName: 'Update Field', - default: '', - displayOptions: { - show: { - operation: ['update'], - }, - }, - }, - { - name: 'deleteField', - type: 'string', - required: true, - displayName: 'Delete Field', - default: '', - displayOptions: { - show: { - operation: ['delete'], - }, - }, - }, - ]; - - const schema = toJsonSchema(properties); - - // Should have 3 separate conditionals (one for each operation) - const allOf = schema.allOf as GenericValue[] | IDataObject[]; - expect(allOf?.length).toBe(3); - - // Verify each conditional is correct - const createCondition = allOf?.find( - (cond) => (cond as any).if?.properties?.operation?.enum?.[0] === 'create', - ) as IDependency; - expect(createCondition?.then?.allOf[0].required).toContain('createField'); - - const updateCondition = allOf?.find( - (cond) => (cond as any).if?.properties?.operation?.enum?.[0] === 'update', - ) as IDependency; - expect(updateCondition?.then?.allOf[0].required).toContain('updateField'); - - const deleteCondition = allOf?.find( - (cond) => (cond as any).if?.properties?.operation?.enum?.[0] === 'delete', - ) as IDependency; - expect(deleteCondition?.then?.allOf[0].required).toContain('deleteField'); - }); - - it('should add "false" displayOptions.show dependant value as allof condition', () => { - const properties: INodeProperties[] = [ - { name: 'field1', type: 'string', required: true, displayName: 'Field 1', default: '' }, - { - name: 'field2', - type: 'options', - required: true, - options: [ - { value: 'opt1', name: 'opt1' }, - { value: 'opt2', name: 'opt2' }, - ], - displayName: 'Field 2', - default: 'opt1', - }, - { - name: 'field3', - type: 'string', - required: true, - displayName: 'Field 3', - default: '', - displayOptions: { - show: { - field2: [false], // boolean false as dependant value - }, - }, - }, - ]; - - const schema = toJsonSchema(properties); - - // Cast properties as IDataObject - const props = schema.properties as IDataObject; - - expect(props).toBeDefined(); - expect(props.field1).toEqual({ type: 'string' }); - expect(props.field2).toEqual({ - type: 'string', - enum: ['opt1', 'opt2'], - }); - expect(props.field3).toEqual({ type: 'string' }); - - // field1 and field2 required globally, field3 required conditionally - expect(schema.required).toEqual(expect.arrayContaining(['field1', 'field2'])); - expect(schema.required).not.toContain('field3'); - - const allOf = schema.allOf as GenericValue[] | IDataObject[]; - expect(Array.isArray(allOf)).toBe(true); - expect(allOf?.length).toBeGreaterThan(0); - - const condition = allOf?.find((cond) => (cond as any).if?.properties?.field2) as IDependency; - expect(condition).toBeDefined(); - expect((condition.if?.properties as any).field2).toEqual({ - enum: [false], // boolean false as dependant value - }); - - // then block requires field3 when field2 === false - expect(condition.then?.allOf.some((req: any) => req.required?.includes('field3'))).toBe(true); - // no else block: hidden fields are optional, not forbidden - expect((condition as any).else).toBeUndefined(); - }); - - it('should not forbid conditional fields belonging to inactive conditions', () => { - // Mirrors the mongoDb scenario: two fields depend on the same options field - // but with different values. A payload that includes both the active - // and the inactive conditional field must validate. - const properties: INodeProperties[] = [ - { - name: 'configurationType', - type: 'options', - options: [ - { value: 'connectionString', name: 'Connection String' }, - { value: 'values', name: 'Values' }, - ], - displayName: 'Configuration Type', - default: 'values', - }, - { - name: 'connectionString', - type: 'string', - required: true, - displayName: 'Connection String', - default: '', - displayOptions: { show: { configurationType: ['connectionString'] } }, - }, - { - name: 'host', - type: 'string', - required: true, - displayName: 'Host', - default: '', - displayOptions: { show: { configurationType: ['values'] } }, - }, - ]; - - const schema = toJsonSchema(properties); - - // No `not: { required }` anywhere in the generated schema - expect(JSON.stringify(schema)).not.toContain('"not"'); - - // A payload using connectionString but also carrying the inactive `host` field - // must be accepted. - const connectionStringPayload = { - configurationType: 'connectionString', - connectionString: 'mongodb://localhost:27017/mydb', - host: 'localhost', - }; - expect(validate(connectionStringPayload, schema as unknown as Schema).valid).toBe(true); - - // And the reverse direction. - const valuesPayload = { - configurationType: 'values', - host: 'localhost', - connectionString: 'mongodb://localhost:27017/mydb', - }; - expect(validate(valuesPayload, schema as unknown as Schema).valid).toBe(true); - - // Required fields are still enforced via the `then` block. - const missingRequired = { configurationType: 'values' }; - expect(validate(missingRequired, schema as unknown as Schema).valid).toBe(false); - }); - }); - - describe('updateCredential', () => { - let credentialsRepository: CredentialsRepository; - let ownerUser: User; - let memberUser: User; - - const credentialsService = new CredentialsService( - mock(), // credentialsRepository - mock(), - mock(), // sharedCredentialsRepository - mock(), // ownershipService - mock(), // logger - mock(), // errorReporter - mock(), // credentialsTester - mock(), // externalHooks - mock(), // credentialTypes - mock(), // projectRepository - mock(), // projectService - mock(), // roleService - mock(), // userRepository - mock(), // credentialsFinderService - mock(), // credentialsHelper - mock(), // externalSecretsConfig - mock(), // externalSecretsProviderAccessCheckService - mock(), // connectionStatusProxy - mock(), // instanceCredentialAssignmentRepository - mock(), // instanceCredentialUseRegistry - mock(), // dbLockService - ); - - beforeEach(() => { - credentialsRepository = mock(); - - vi.spyOn(credentialsService, 'decrypt'); - - vi.spyOn(Container, 'get').mockImplementation((serviceClass) => { - if (serviceClass === CredentialsService) { - return credentialsService; - } - if (serviceClass === CredentialsRepository) { - return credentialsRepository; - } - if (serviceClass === Cipher) { - return cipher; - } - if (serviceClass === SecretsProviderAccessCheckService) { - return mockSecretsProviderAccessCheckService; - } - if (serviceClass === ExternalSecretsConfig) { - return mockExternalSecretsConfig; - } - return mock(); - }); - - //vi.clearAllMocks(); - - ownerUser = { id: 'user-with-permission', role: GLOBAL_OWNER_ROLE } as User; - memberUser = { id: 'user-without-permission', role: GLOBAL_MEMBER_ROLE } as User; - }); - - describe('external secrets', () => { - const owningProjectData: Partial = { - id: 'nUnAvqXSO4nw522z', - name: 'Test Project', - type: 'team', - icon: { type: 'icon', value: 'layers' }, - }; - const owningProject = { - role: 'credential:owner', - project: owningProjectData as Project, - } as SharedCredentials; - it('should throw error when user without permission tries to add external secret expression', async () => { - vi.spyOn(checkAccess, 'userHasScopes').mockResolvedValue(false); - const existingCredential = mock({ - id: 'cred-id', - name: 'Test Credential', - type: 'testApi', - isManaged: false, - shared: [owningProject], - }); - - credentialsRepository.findOne = vi.fn().mockResolvedValue(existingCredential); - credentialsRepository.update = vi.fn().mockResolvedValue(undefined); - // mock credential that doesn't have secret expression yet - vi.mocked(credentialsService.decrypt).mockResolvedValue({ apiKey: 'regular-secret' }); - - await expect( - updateCredential(existingCredential, memberUser, { - data: { apiKey: '{{ $secrets.vault.myKey }}' }, - }), - ).rejects.toThrow('Lacking permissions to reference external secrets in credentials'); - }); - - it('should throw error when user without permission tries to modify existing external secret expression', async () => { - vi.spyOn(checkAccess, 'userHasScopes').mockResolvedValue(false); - const existingCredential = mock({ - id: 'cred-id', - name: 'Test Credential', - type: 'testApi', - isManaged: false, - shared: [owningProject], - }); - - credentialsRepository.findOne = vi.fn().mockResolvedValue(existingCredential); - credentialsRepository.update = vi.fn().mockResolvedValue(undefined); - - // Mock credential that already has secret expression - vi.mocked(credentialsService.decrypt).mockResolvedValue({ - apiKey: '{{ $secrets.vault.oldKey }}', - }); - - await expect( - updateCredential(existingCredential, memberUser, { - data: { apiKey: '{{ $secrets.vault.newKey }}' }, - }), - ).rejects.toThrow('Lacking permissions to reference external secrets in credentials'); - }); - - it('should throw error when external secret store referenced in expression is not shared with current project', async () => { - vi.spyOn(checkAccess, 'userHasScopes').mockResolvedValue(true); - const existingCredential = mock({ - id: 'UdGtZBYb2TLDgSHy', - name: 'Test Credential', - type: 'testApi', - isManaged: false, - shared: [owningProject], - }); - const secretProviderKey = 'vault'; - const secretExpression = `={{ $secrets.${secretProviderKey}.myKey }}`; - - credentialsRepository.findOne = vi.fn().mockResolvedValue(existingCredential); - credentialsRepository.update = vi.fn().mockResolvedValue(undefined); - vi.mocked(credentialsService.decrypt).mockResolvedValue({ - apiKey: 'currentPlainTextValue', - }); - vi.mocked( - mockSecretsProviderAccessCheckService.isProviderAvailableInProject, - ).mockResolvedValue(false); - mockExternalSecretsConfig.externalSecretsForProjects = true; - - await expect( - updateCredential(existingCredential, ownerUser, { - data: { apiKey: secretExpression }, - }), - ).rejects.toThrow( - 'The secret provider "vault" used in "apiKey" does not exist in this project', - ); - }); - - it('should allow updates when no external secret expression is being changed', async () => { - const existingCredential = mock({ - id: 'cred-id', - name: 'Test Credential', - type: 'testApi', - isManaged: false, - shared: [owningProject], - }); - - credentialsRepository.findOne = vi.fn().mockResolvedValue(existingCredential); - credentialsRepository.update = vi.fn().mockResolvedValue(undefined); - - // Mock credential that has existing secret expression - vi.mocked(credentialsService.decrypt).mockResolvedValue({ - apiKey: '{{ $secrets.vault.myKey }}', - }); - - credentialsRepository.update = vi.fn().mockResolvedValue(undefined); - - await updateCredential(existingCredential, memberUser, { - name: 'Updated Name', - }); - }); - - it('should allow updates of non-secret data without specific external-secret permission', async () => { - const existingCredential = mock({ - id: 'cred-id', - name: 'Test Credential', - type: 'testApi', - isManaged: false, - shared: [owningProject], - }); - - credentialsRepository.findOne = vi.fn().mockResolvedValue(existingCredential); - credentialsRepository.update = vi.fn().mockResolvedValue(undefined); - - // Mock credential not using any external secret expressions - vi.mocked(credentialsService.decrypt).mockResolvedValue({ apiKey: 'regular-key' }); - - credentialsRepository.update = vi.fn().mockResolvedValue(undefined); - - await updateCredential(existingCredential, memberUser, { - data: { apiKey: 'another-regular-key' }, - }); - }); - - it('should allow user with permission to add external secret expression', async () => { - vi.spyOn(checkAccess, 'userHasScopes').mockResolvedValue(true); - const existingCredential = mock({ - id: 'cred-id', - name: 'Test Credential', - type: 'testApi', - isManaged: false, - shared: [owningProject], - }); - credentialsRepository.findOne = vi.fn().mockResolvedValue(existingCredential); - credentialsRepository.update = vi.fn().mockResolvedValue(undefined); - vi.mocked(credentialsService.decrypt).mockResolvedValue({ apiKey: 'regular-key' }); - vi.mocked( - mockSecretsProviderAccessCheckService.isProviderAvailableInProject, - ).mockResolvedValue(true); - credentialsRepository.update = vi.fn().mockResolvedValue(undefined); - - await updateCredential(existingCredential, ownerUser, { - data: { apiKey: '{{ $secrets.vault.myKey }}' }, - }); - }); - }); - }); -}); diff --git a/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.utils.test.ts b/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.utils.test.ts new file mode 100644 index 00000000000..05cca815ca4 --- /dev/null +++ b/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.utils.test.ts @@ -0,0 +1,433 @@ +import type { CredentialsEntity } from '@n8n/db'; +import { validate, type Schema } from 'jsonschema'; +import type { GenericValue, IDataObject, INodeProperties } from 'n8n-workflow'; + +import type { IDependency } from '@/public-api/types'; + +import { buildSharedForCredential, toJsonSchema } from '../credentials.utils'; + +describe('credentials.utils', () => { + describe('buildSharedForCredential', () => { + it('returns one shared entry when credential is shared with one project', () => { + const createdAt = new Date('2024-01-01T00:00:00.000Z'); + const updatedAt = new Date('2024-01-02T00:00:00.000Z'); + const credential = { + shared: [ + { + role: 'credential:owner', + createdAt, + updatedAt, + project: { id: 'proj-1', name: 'My Project' }, + }, + ], + } as unknown as CredentialsEntity; + expect(buildSharedForCredential(credential)).toEqual([ + { + id: 'proj-1', + name: 'My Project', + role: 'credential:owner', + createdAt, + updatedAt, + }, + ]); + }); + + it('returns multiple shared entries and skips shared entries without project', () => { + const createdAt1 = new Date('2024-01-01T00:00:00.000Z'); + const updatedAt1 = new Date('2024-01-02T00:00:00.000Z'); + const createdAt2 = new Date('2024-02-01T00:00:00.000Z'); + const updatedAt2 = new Date('2024-02-02T00:00:00.000Z'); + const credential = { + shared: [ + { + role: 'credential:owner', + createdAt: createdAt1, + updatedAt: updatedAt1, + project: { id: 'proj-1', name: 'Project One' }, + }, + { role: 'credential:user', createdAt: createdAt2, updatedAt: updatedAt2, project: null }, + { + role: 'credential:user', + createdAt: createdAt2, + updatedAt: updatedAt2, + project: { id: 'proj-2', name: 'Project Two' }, + }, + ], + } as unknown as CredentialsEntity; + expect(buildSharedForCredential(credential)).toEqual([ + { + id: 'proj-1', + name: 'Project One', + role: 'credential:owner', + createdAt: createdAt1, + updatedAt: updatedAt1, + }, + { + id: 'proj-2', + name: 'Project Two', + role: 'credential:user', + createdAt: createdAt2, + updatedAt: updatedAt2, + }, + ]); + }); + }); + + describe('toJsonSchema', () => { + it('should create separate conditionals for different values of the same dependant field', () => { + const properties: INodeProperties[] = [ + { + name: 'keyType', + type: 'options', + options: [ + { value: 'passphrase', name: 'Passphrase' }, + { value: 'pemKey', name: 'PEM Key' }, + ], + displayName: 'Key Type', + default: 'passphrase', + }, + { + name: 'secret', + type: 'string', + required: true, + displayName: 'Secret', + default: '', + displayOptions: { + show: { + keyType: ['passphrase'], + }, + }, + }, + { + name: 'privateKey', + type: 'string', + required: true, + displayName: 'Private Key', + default: '', + displayOptions: { + show: { + keyType: ['pemKey'], + }, + }, + }, + { + name: 'publicKey', + type: 'string', + required: true, + displayName: 'Public Key', + default: '', + displayOptions: { + show: { + keyType: ['pemKey'], + }, + }, + }, + ]; + + const schema = toJsonSchema(properties); + + const props = schema.properties as IDataObject; + expect(props).toBeDefined(); + expect(props.keyType).toEqual({ + type: 'string', + enum: ['passphrase', 'pemKey'], + }); + + expect(schema.required).not.toContain('secret'); + expect(schema.required).not.toContain('privateKey'); + expect(schema.required).not.toContain('publicKey'); + + const allOf = schema.allOf as GenericValue[] | IDataObject[]; + expect(Array.isArray(allOf)).toBe(true); + expect(allOf?.length).toBe(2); + + const passphraseCondition = allOf?.find( + (cond) => (cond as any).if?.properties?.keyType?.enum?.[0] === 'passphrase', + ) as IDependency; + expect(passphraseCondition).toBeDefined(); + expect(passphraseCondition.then?.allOf).toHaveLength(1); + expect(passphraseCondition.then?.allOf[0].required).toContain('secret'); + expect(passphraseCondition.then?.allOf[0].required).not.toContain('privateKey'); + expect(passphraseCondition.then?.allOf[0].required).not.toContain('publicKey'); + + const pemKeyCondition = allOf?.find( + (cond) => (cond as any).if?.properties?.keyType?.enum?.[0] === 'pemKey', + ) as IDependency; + expect(pemKeyCondition).toBeDefined(); + expect(pemKeyCondition.then?.allOf).toHaveLength(2); + expect( + pemKeyCondition.then?.allOf.some((req: any) => req.required?.includes('privateKey')), + ).toBe(true); + expect( + pemKeyCondition.then?.allOf.some((req: any) => req.required?.includes('publicKey')), + ).toBe(true); + expect(pemKeyCondition.then?.allOf.some((req: any) => req.required?.includes('secret'))).toBe( + false, + ); + }); + + it('should handle properties with no displayOptions as globally required', () => { + const properties: INodeProperties[] = [ + { name: 'apiKey', type: 'string', required: true, displayName: 'API Key', default: '' }, + { name: 'domain', type: 'string', required: true, displayName: 'Domain', default: '' }, + { + name: 'optionalField', + type: 'string', + required: false, + displayName: 'Optional', + default: '', + }, + ]; + + const schema = toJsonSchema(properties); + + expect(schema.required).toEqual(expect.arrayContaining(['apiKey', 'domain'])); + expect(schema.required).not.toContain('optionalField'); + expect(schema.allOf).toBeUndefined(); + }); + + it('should handle mix of required and conditional properties', () => { + const properties: INodeProperties[] = [ + { name: 'apiKey', type: 'string', required: true, displayName: 'API Key', default: '' }, + { + name: 'authType', + type: 'options', + required: true, + options: [ + { value: 'basic', name: 'Basic' }, + { value: 'oauth2', name: 'OAuth2' }, + ], + displayName: 'Auth Type', + default: 'basic', + }, + { + name: 'username', + type: 'string', + required: true, + displayName: 'Username', + default: '', + displayOptions: { + show: { + authType: ['basic'], + }, + }, + }, + { + name: 'password', + type: 'string', + required: true, + displayName: 'Password', + default: '', + displayOptions: { + show: { + authType: ['basic'], + }, + }, + }, + { + name: 'clientId', + type: 'string', + required: true, + displayName: 'Client ID', + default: '', + displayOptions: { + show: { + authType: ['oauth2'], + }, + }, + }, + ]; + + const schema = toJsonSchema(properties); + + expect(schema.required).toEqual(expect.arrayContaining(['apiKey', 'authType'])); + expect(schema.required).not.toContain('username'); + expect(schema.required).not.toContain('password'); + expect(schema.required).not.toContain('clientId'); + + const allOf = schema.allOf as GenericValue[] | IDataObject[]; + expect(allOf?.length).toBe(2); + }); + + it('should handle properties with multiple options depending on same field', () => { + const properties: INodeProperties[] = [ + { + name: 'operation', + type: 'options', + options: [ + { value: 'create', name: 'Create' }, + { value: 'update', name: 'Update' }, + { value: 'delete', name: 'Delete' }, + ], + displayName: 'Operation', + default: 'create', + }, + { + name: 'createField', + type: 'string', + required: true, + displayName: 'Create Field', + default: '', + displayOptions: { + show: { + operation: ['create'], + }, + }, + }, + { + name: 'updateField', + type: 'string', + required: true, + displayName: 'Update Field', + default: '', + displayOptions: { + show: { + operation: ['update'], + }, + }, + }, + { + name: 'deleteField', + type: 'string', + required: true, + displayName: 'Delete Field', + default: '', + displayOptions: { + show: { + operation: ['delete'], + }, + }, + }, + ]; + + const schema = toJsonSchema(properties); + + const allOf = schema.allOf as GenericValue[] | IDataObject[]; + expect(allOf?.length).toBe(3); + + const createCondition = allOf?.find( + (cond) => (cond as any).if?.properties?.operation?.enum?.[0] === 'create', + ) as IDependency; + expect(createCondition?.then?.allOf[0].required).toContain('createField'); + + const updateCondition = allOf?.find( + (cond) => (cond as any).if?.properties?.operation?.enum?.[0] === 'update', + ) as IDependency; + expect(updateCondition?.then?.allOf[0].required).toContain('updateField'); + + const deleteCondition = allOf?.find( + (cond) => (cond as any).if?.properties?.operation?.enum?.[0] === 'delete', + ) as IDependency; + expect(deleteCondition?.then?.allOf[0].required).toContain('deleteField'); + }); + + it('should add "false" displayOptions.show dependant value as allof condition', () => { + const properties: INodeProperties[] = [ + { name: 'field1', type: 'string', required: true, displayName: 'Field 1', default: '' }, + { + name: 'field2', + type: 'options', + required: true, + options: [ + { value: 'opt1', name: 'opt1' }, + { value: 'opt2', name: 'opt2' }, + ], + displayName: 'Field 2', + default: 'opt1', + }, + { + name: 'field3', + type: 'string', + required: true, + displayName: 'Field 3', + default: '', + displayOptions: { + show: { + field2: [false], + }, + }, + }, + ]; + + const schema = toJsonSchema(properties); + + const props = schema.properties as IDataObject; + + expect(props).toBeDefined(); + expect(props.field1).toEqual({ type: 'string' }); + expect(props.field2).toEqual({ + type: 'string', + enum: ['opt1', 'opt2'], + }); + expect(props.field3).toEqual({ type: 'string' }); + + expect(schema.required).toEqual(expect.arrayContaining(['field1', 'field2'])); + expect(schema.required).not.toContain('field3'); + + const allOf = schema.allOf as GenericValue[] | IDataObject[]; + expect(Array.isArray(allOf)).toBe(true); + expect(allOf?.length).toBeGreaterThan(0); + + const condition = allOf?.find((cond) => (cond as any).if?.properties?.field2) as IDependency; + expect(condition).toBeDefined(); + expect((condition.if?.properties as any).field2).toEqual({ + enum: [false], + }); + + expect(condition.then?.allOf.some((req: any) => req.required?.includes('field3'))).toBe(true); + expect((condition as any).else).toBeUndefined(); + }); + + it('should not forbid conditional fields belonging to inactive conditions', () => { + const properties: INodeProperties[] = [ + { + name: 'configurationType', + type: 'options', + options: [ + { value: 'connectionString', name: 'Connection String' }, + { value: 'values', name: 'Values' }, + ], + displayName: 'Configuration Type', + default: 'values', + }, + { + name: 'connectionString', + type: 'string', + required: true, + displayName: 'Connection String', + default: '', + displayOptions: { show: { configurationType: ['connectionString'] } }, + }, + { + name: 'host', + type: 'string', + required: true, + displayName: 'Host', + default: '', + displayOptions: { show: { configurationType: ['values'] } }, + }, + ]; + + const schema = toJsonSchema(properties); + + expect(JSON.stringify(schema)).not.toContain('"not"'); + + const connectionStringPayload = { + configurationType: 'connectionString', + connectionString: 'mongodb://localhost:27017/mydb', + host: 'localhost', + }; + expect(validate(connectionStringPayload, schema as unknown as Schema).valid).toBe(true); + + const valuesPayload = { + configurationType: 'values', + host: 'localhost', + connectionString: 'mongodb://localhost:27017/mydb', + }; + expect(validate(valuesPayload, schema as unknown as Schema).valid).toBe(true); + + const missingRequired = { configurationType: 'values' }; + expect(validate(missingRequired, schema as unknown as Schema).valid).toBe(false); + }); + }); +}); 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 cf2d5a777e6..2bfc1ff353e 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,11 +1,12 @@ import { LicenseState } from '@n8n/backend-common'; -import type { CredentialsEntity } from '@n8n/db'; -import { CredentialsRepository, SharedCredentialsRepository } from '@n8n/db'; +import type { CredentialsEntity, ICredentialsDb, User } from '@n8n/db'; import { Container } from '@n8n/di'; import { hasGlobalScope } from '@n8n/permissions'; +import type { ICredentialDataDecryptedObject } from 'n8n-workflow'; import { z } from 'zod'; import { CredentialTypes } from '@/credential-types'; +import { CredentialsFinderService } from '@/credentials/credentials-finder.service'; import { CredentialsService } from '@/credentials/credentials.service'; import { EnterpriseCredentialsService } from '@/credentials/credentials.service.ee'; import { CredentialsHelper } from '@/credentials-helper'; @@ -13,6 +14,7 @@ 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'; +import { EventService } from '@/events/event.service'; import { toPublicApiCredentialResponse } from './credentials.mapper'; import { @@ -21,17 +23,7 @@ import { validCredentialTypeForUpdate, validCredentialsPropertiesForUpdate, } from './credentials.middleware'; -import { - buildSharedForCredential, - CredentialsIsNotUpdatableError, - getCredential, - getSharedCredentials, - removeCredential, - sanitizeCredentials, - saveCredential, - toJsonSchema, - updateCredential, -} from './credentials.service'; +import { buildSharedForCredential, sanitizeCredentials, toJsonSchema } from './credentials.utils'; import type { CredentialTypeRequest, CredentialRequest } from '../../../types'; import type { PublicAPIEndpoint } from '../../shared/handler.types'; import { @@ -42,6 +34,77 @@ import { } from '../../shared/middlewares/global.middleware'; import { encodeNextCursor } from '../../shared/services/pagination.service'; +async function buildUpdatePayload({ + credentialsService, + user, + existingCredential, + body, + clearOauthTokenData, +}: { + credentialsService: CredentialsService; + user: User; + existingCredential: CredentialsEntity; + body: { + name?: string; + type?: string; + data?: ICredentialDataDecryptedObject; + isGlobal?: boolean; + isResolvable?: boolean; + isPartialData?: boolean; + }; + clearOauthTokenData: boolean; +}): Promise<{ + updatePayload: ICredentialsDb; + decryptedDataForDeps?: ICredentialDataDecryptedObject; +}> { + let updatePayload: ICredentialsDb; + let decryptedDataForDeps: ICredentialDataDecryptedObject | undefined; + + if (body.data) { + const preparedCredentialData = await credentialsService.prepareUpdateData( + user, + { + name: body.name ?? existingCredential.name, + type: body.type ?? existingCredential.type, + data: body.data, + }, + existingCredential, + { + dataMerge: body.isPartialData ? 'partial' : 'replace', + clearOauthTokenData, + }, + ); + + decryptedDataForDeps = preparedCredentialData.data as unknown as ICredentialDataDecryptedObject; + + updatePayload = await credentialsService.createEncryptedData({ + id: existingCredential.id, + name: preparedCredentialData.name, + type: preparedCredentialData.type, + data: decryptedDataForDeps, + }); + } else { + updatePayload = { + id: existingCredential.id, + name: body.name ?? existingCredential.name, + type: body.type ?? existingCredential.type, + data: existingCredential.data, + createdAt: existingCredential.createdAt, + updatedAt: existingCredential.updatedAt, + }; + } + + if (body.isGlobal !== undefined) { + updatePayload.isGlobal = body.isGlobal; + } + if (body.isResolvable !== undefined) { + updatePayload.isResolvable = body.isResolvable; + } + updatePayload.updatedAt = new Date(); + + return { updatePayload, decryptedDataForDeps }; +} + type CredentialsHandlers = { getCredentials: PublicAPIEndpoint; getCredential: PublicAPIEndpoint; @@ -61,15 +124,16 @@ const credentialsHandlers: CredentialsHandlers = { const offset = Number(req.query.offset) || 0; const limit = Math.min(Number(req.query.limit) || 100, 250); - const repo = Container.get(CredentialsRepository); - const [credentials, count] = await repo.findAndCount({ - take: limit, - skip: offset, - select: ['id', 'name', 'type', 'createdAt', 'updatedAt'], - relations: ['shared', 'shared.project'], - order: { createdAt: 'DESC' }, - where: { usageScope: 'project' }, - }); + const { credentials, count } = await Container.get(CredentialsService).getManyAndCount( + req.user, + { + listQueryOptions: { + take: limit, + skip: offset, + sortBy: 'createdAt:desc', + }, + }, + ); const data = credentials.map((credential: CredentialsEntity) => { const shared = buildSharedForCredential(credential); @@ -99,7 +163,9 @@ const credentialsHandlers: CredentialsHandlers = { async (req, res) => { const { id: credentialId } = req.params; - const credential = await getCredential(credentialId); + const credential = await Container.get(CredentialsFinderService).findById(credentialId, { + includeSharedProject: true, + }); if (!credential) { throw new NotFoundError('Credential not found'); } @@ -132,8 +198,46 @@ const credentialsHandlers: CredentialsHandlers = { validCredentialsProperties, publicApiScope('credential:create'), async (req, res) => { - const savedCredential = await saveCredential(req.body, req.user); - return res.json(savedCredential); + const credentialsService = Container.get(CredentialsService); + const { scopes: _scopes, ...credential } = await credentialsService.createUnmanagedCredential( + { + type: req.body.type, + name: req.body.name, + data: req.body.data, + projectId: req.body.projectId, + isResolvable: req.body.isResolvable, + usageScope: 'project', + }, + req.user, + ); + + const project = await credentialsService.findCredentialOwningProject(credential.id); + + Container.get(EventService).emit('credentials-created', { + user: req.user, + credentialType: credential.type, + credentialId: credential.id, + publicApi: true, + projectId: project?.id, + projectType: project?.type, + isDynamic: credential.isResolvable ?? false, + jweEnabled: req.body.data.jweEnabled === true, + }); + + return res.json( + toPublicApiCredentialResponse({ + id: credential.id, + name: credential.name, + type: credential.type, + isManaged: credential.isManaged, + isGlobal: credential.isGlobal, + isResolvable: credential.isResolvable, + resolvableAllowFallback: credential.resolvableAllowFallback, + resolverId: credential.resolverId, + createdAt: credential.createdAt, + updatedAt: credential.updatedAt, + }), + ); }, ], updateCredential: [ @@ -143,12 +247,20 @@ const credentialsHandlers: CredentialsHandlers = { projectScope('credential:update', 'credential'), async (req, res) => { const { id: credentialId } = req.params; + const credentialsService = Container.get(CredentialsService); - const existingCredential = await getCredential(credentialId); + const existingCredential = await Container.get(CredentialsFinderService).findById( + credentialId, + { includeSharedProject: true }, + ); if (!existingCredential) { throw new NotFoundError('Credential not found'); } + if (existingCredential.isManaged) { + throw new BadRequestError('Managed credentials cannot be updated.'); + } + if (req.body.isGlobal !== undefined && req.body.isGlobal !== existingCredential.isGlobal) { if (!Container.get(LicenseState).isSharingLicensed()) { throw new ForbiddenError('You are not licensed for sharing credentials'); @@ -180,17 +292,30 @@ const credentialsHandlers: CredentialsHandlers = { ); } - try { - const updatedCredential = await updateCredential(existingCredential, req.user, req.body); + const isChangingAuthType = + req.body.type !== undefined && req.body.type !== existingCredential.type; + const isTogglingToPrivate = + Boolean(req.body.isResolvable) && !existingCredential.isResolvable; - return res.json(toPublicApiCredentialResponse(updatedCredential)); - } catch (error) { - if (error instanceof CredentialsIsNotUpdatableError) { - throw new BadRequestError(error.message); - } + const { updatePayload, decryptedDataForDeps } = await buildUpdatePayload({ + credentialsService, + user: req.user, + existingCredential, + body: req.body, + clearOauthTokenData: isTogglingToPrivate || isChangingAuthType, + }); - throw error; + const updatedCredential = await credentialsService.update( + credentialId, + updatePayload, + decryptedDataForDeps, + ); + + if (!updatedCredential) { + throw new NotFoundError('Credential not found'); } + + return res.json(toPublicApiCredentialResponse(updatedCredential)); }, ], transferCredential: [ @@ -213,33 +338,18 @@ const credentialsHandlers: CredentialsHandlers = { projectScope('credential:delete', 'credential'), async (req, res) => { const { id: credentialId } = req.params; - let credential: CredentialsEntity | undefined; - if (!hasGlobalScope(req.user, ['credential:read'])) { - const shared = await getSharedCredentials(req.user.id, credentialId); - - if (shared?.role === 'credential:owner') { - credential = shared.credentials; - } - } else { - credential = (await getCredential(credentialId)) ?? undefined; - } + const credential = await Container.get(CredentialsFinderService).findCredentialForUser( + credentialId, + req.user, + ['credential:delete'], + ); if (!credential) { throw new NotFoundError('Not Found'); } - if (credential.isResolvable) { - const owningProject = await Container.get( - SharedCredentialsRepository, - ).findCredentialOwningProject(credentialId); - await Container.get(CredentialsService).ensureCanManageEndUserCredential( - req.user, - owningProject?.id, - ); - } - - await removeCredential(req.user, credential); + await Container.get(CredentialsService).delete(req.user, credentialId); return res.json(sanitizeCredentials(credential)); }, ], diff --git a/packages/cli/src/public-api/v1/handlers/credentials/credentials.middleware.ts b/packages/cli/src/public-api/v1/handlers/credentials/credentials.middleware.ts index 6873b5a9226..2315b3805c9 100644 --- a/packages/cli/src/public-api/v1/handlers/credentials/credentials.middleware.ts +++ b/packages/cli/src/public-api/v1/handlers/credentials/credentials.middleware.ts @@ -6,9 +6,10 @@ import { validate } from 'jsonschema'; import type { IDataObject } from 'n8n-workflow'; import { CredentialTypes } from '@/credential-types'; +import { CredentialsFinderService } from '@/credentials/credentials-finder.service'; import { CredentialsHelper } from '@/credentials-helper'; -import { getCredential, toJsonSchema } from './credentials.service'; +import { toJsonSchema } from './credentials.utils'; import type { CredentialRequest } from '../../../types'; /** @@ -109,7 +110,8 @@ export const validCredentialsPropertiesForUpdate = async ( if (data !== undefined) { // Fetch existing credential to get type if not provided if (type === undefined) { - const existingCredential = await getCredential(credentialId); + const existingCredential = + await Container.get(CredentialsFinderService).findById(credentialId); if (!existingCredential) { return res.status(404).json({ message: 'Credential not found' }); } @@ -127,7 +129,7 @@ export const validCredentialsPropertiesForUpdate = async ( // If type is provided but data is not, check if type is changing if (type !== undefined && data === undefined) { - const existingCredential = await getCredential(credentialId); + const existingCredential = await Container.get(CredentialsFinderService).findById(credentialId); if (!existingCredential) { return res.status(404).json({ message: 'Credential not found' }); } 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 deleted file mode 100644 index 4d0bc370b21..00000000000 --- a/packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts +++ /dev/null @@ -1,494 +0,0 @@ -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'; -import { Credentials } from 'n8n-core'; -import { - BaseError, - type DisplayCondition, - type ICredentialDataDecryptedObject, - type IDataObject, - type INodeProperties, - type INodePropertyOptions, -} from 'n8n-workflow'; - -import { CredentialsService } from '@/credentials/credentials.service'; -import { - validateAccessToReferencedSecretProviders, - validateExternalSecretsPermissions, -} from '@/credentials/validation'; -import { EventService } from '@/events/event.service'; -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 {} - -function isNodePropertyOptions(options: unknown): options is INodePropertyOptions[] { - return ( - Array.isArray(options) && - options.every( - (item) => typeof item === 'object' && item !== null && 'value' in item && 'name' in item, - ) - ); -} - -/** - * Shared entry for credential list: project id/name plus sharing role and timestamps. - * Derived from credential.shared (SharedCredentials + Project), limited to these fields. - */ -export type CredentialListSharedItem = { - id: string; - name: string; - role: string; - createdAt: Date; - updatedAt: Date; -}; - -/** - * Build the shared array for a credential list item from credential.shared. - * Each entry has id, name from the project and role, createdAt, updatedAt from the shared relation. - */ -export function buildSharedForCredential( - credential: CredentialsEntity, -): CredentialListSharedItem[] { - const shared = credential.shared; - return shared - .filter((sh) => typeof sh.project?.id === 'string') - .map((sh) => ({ - id: sh.project.id, - name: sh.project.name, - role: sh.role, - createdAt: sh.createdAt, - updatedAt: sh.updatedAt, - })); -} - -export async function getCredential(credentialId: string): Promise { - return await Container.get(CredentialsRepository).findOne({ - where: { id: credentialId, usageScope: 'project' }, - relations: ['shared', 'shared.project'], - }); -} - -function isProjectScopedExternalSecretsEnabled() { - return Container.get(ExternalSecretsConfig).externalSecretsForProjects; -} - -export async function getSharedCredentials( - userId: string, - credentialId: string, -): Promise { - return await Container.get(SharedCredentialsRepository).findOne({ - where: { - project: { projectRelations: { userId } }, - credentialsId: credentialId, - }, - relations: ['credentials'], - }); -} - -/** - * Creates a credential via the internal CredentialsService, which handles project - * resolution, validation, and encryption. - */ -export async function saveCredential( - payload: { - type: string; - name: string; - data: ICredentialDataDecryptedObject; - projectId?: string; - isResolvable?: boolean; - }, - user: User, -): Promise { - const { scopes: _scopes, ...credential } = await Container.get( - CredentialsService, - ).createUnmanagedCredential( - { - type: payload.type, - name: payload.name, - data: payload.data, - projectId: payload.projectId, - isResolvable: payload.isResolvable, - usageScope: 'project', - }, - user, - ); - - const project = await Container.get(SharedCredentialsRepository).findCredentialOwningProject( - credential.id, - ); - - Container.get(EventService).emit('credentials-created', { - user, - credentialType: credential.type, - credentialId: credential.id, - publicApi: true, - projectId: project?.id, - projectType: project?.type, - isDynamic: credential.isResolvable ?? false, - jweEnabled: payload.data.jweEnabled === true, - }); - - const credentialForApi = { - id: credential.id, - name: credential.name, - type: credential.type, - isManaged: credential.isManaged, - isGlobal: credential.isGlobal, - isResolvable: credential.isResolvable, - resolvableAllowFallback: credential.resolvableAllowFallback, - resolverId: credential.resolverId, - createdAt: credential.createdAt, - updatedAt: credential.updatedAt, - }; - - return toPublicApiCredentialResponse(credentialForApi); -} - -export async function updateCredential( - existingCredential: ICredentialsDb, - user: User, - updateData: { - type?: string; - name?: string; - data?: ICredentialDataDecryptedObject; - isGlobal?: boolean; - isResolvable?: boolean; - isPartialData?: boolean; - }, -): Promise { - if (existingCredential.isManaged) { - throw new CredentialsIsNotUpdatableError('Managed credentials cannot be updated.'); - } - - const credentialId = existingCredential.id; - - // Merge the update data with existing credential - const credentialData: Partial = {}; - - if (updateData.name !== undefined) { - credentialData.name = updateData.name; - } - - if (updateData.type !== undefined) { - credentialData.type = updateData.type; - } - - // If data is provided, encrypt it - if (updateData.data !== undefined) { - const credentialsService = Container.get(CredentialsService); - - // Decrypt existing data to access oauthTokenData - const decryptedData = await credentialsService.decrypt( - existingCredential as CredentialsEntity, - true, - ); - - // eslint-disable-next-line @typescript-eslint/no-non-null-asserted-optional-chain -- credential will always have an owner - const projectOwningCredential = existingCredential.shared?.find( - (shared) => shared.role === 'credential:owner', - )!; - - await validateExternalSecretsPermissions({ - user, - projectId: projectOwningCredential.project.id, - dataToSave: updateData.data, - decryptedExistingData: decryptedData, - }); - - if (isProjectScopedExternalSecretsEnabled() && decryptedData) { - await validateAccessToReferencedSecretProviders( - projectOwningCredential.project.id, - updateData.data, - Container.get(SecretsProviderAccessCheckService), - 'update', - ); - } - - let dataToEncrypt: ICredentialDataDecryptedObject; - - // If isPartialData is true, merge with existing decrypted data and unredact - if (updateData.isPartialData === true) { - // First merge existing decrypted data with new data - // This ensures all existing fields are preserved unless explicitly overridden - const mergedData = { - ...decryptedData, - ...updateData.data, - }; - - // Then unredact any redacted values (e.g., replace "***" with original values) - dataToEncrypt = credentialsService.unredact(mergedData, decryptedData); - } else { - // isPartialData is false or undefined (default): replace entire data object - dataToEncrypt = updateData.data; - } - - const newCredential = new CredentialsEntity(); - Object.assign(newCredential, { - id: credentialId, - name: updateData.name ?? existingCredential.name, - type: updateData.type ?? existingCredential.type, - data: dataToEncrypt, - }); - - const encryptedData = await encryptCredential(newCredential); - Object.assign(credentialData, encryptedData); - } - - if (updateData.isResolvable !== undefined) { - credentialData.isResolvable = updateData.isResolvable; - } - - if (updateData.isGlobal !== undefined) { - credentialData.isGlobal = updateData.isGlobal; - } - - credentialData.updatedAt = new Date(); - - await Container.get(CredentialsRepository).update(credentialId, credentialData); - - // credential exists since we just updated it - return (await getCredential(credentialId))!; -} - -export async function removeCredential( - user: User, - credentials: CredentialsEntity, -): Promise { - await Container.get(ExternalHooks).run('credentials.delete', [credentials.id]); - Container.get(EventService).emit('credentials-deleted', { - user, - credentialType: credentials.type, - credentialId: credentials.id, - }); - return await Container.get(CredentialsRepository).remove(credentials); -} - -export async function encryptCredential(credential: CredentialsEntity): Promise { - // Encrypt the data - const coreCredential = new Credentials({ id: null, name: credential.name }, credential.type); - - // @ts-expect-error entity data typed as string - await coreCredential.setData(credential.data); - - return coreCredential.getDataToSave() as ICredentialsDb; -} - -export function sanitizeCredentials(credentials: CredentialsEntity): Partial; -export function sanitizeCredentials( - credentials: CredentialsEntity[], -): Array>; - -export function sanitizeCredentials( - credentials: CredentialsEntity | CredentialsEntity[], -): Partial | Array> { - const argIsArray = Array.isArray(credentials); - const credentialsList = argIsArray ? credentials : [credentials]; - - const sanitizedCredentials = credentialsList.map((credential) => { - const { data, shared, ...rest } = credential; - return rest; - }); - - return argIsArray ? sanitizedCredentials : sanitizedCredentials[0]; -} - -/** - * toJsonSchema - * Take an array of credentials parameter and map it - * to a JSON Schema (see https://json-schema.org/). With - * the JSON Schema definition we can validate the credential's shape - * @param properties - Credentials properties - */ -export function toJsonSchema(properties: INodeProperties[]): IDataObject { - const jsonSchema: IJsonSchema = { - additionalProperties: false, - type: 'object', - properties: {}, - allOf: [], - required: [], - }; - - const optionsValues: { [key: string]: string[] } = {}; - const resolveProperties: string[] = []; - - // get all possible values of properties type "options" - // so we can later resolve the displayOptions dependencies - properties - .filter((property) => property.type === 'options') - .forEach((property) => { - Object.assign(optionsValues, { - [property.name]: isNodePropertyOptions(property.options) - ? property.options.map((option) => option.value) - : undefined, - }); - }); - - let requiredFields: string[] = []; - - const propertyRequiredDependencies: { [key: string]: IDependency } = {}; - - // add all credential's properties to the properties - // object in the JSON Schema definition. This allows us - // to later validate that only this properties are set in - // the credentials sent in the API call. - // eslint-disable-next-line complexity - properties.forEach((property) => { - if (property.required) { - requiredFields.push(property.name); - } - if (property.type === 'options') { - // if the property is type options, - // include all possible values in the enum property. - Object.assign(jsonSchema.properties, { - [property.name]: { - type: 'string', - enum: isNodePropertyOptions(property.options) - ? property.options.map((data) => data.value) - : undefined, - }, - }); - } else { - Object.assign(jsonSchema.properties, { - [property.name]: { - type: property.type, - }, - }); - } - - // if the credential property has a dependency - // then add a JSON Schema condition that satisfy each property value - // e.x: If A has value X then required B, else required C - // see https://json-schema.org/understanding-json-schema/reference/conditionals.html#if-then-else - if (property.displayOptions?.show) { - const dependantName = Object.keys(property.displayOptions?.show)[0] || ''; - const displayOptionsValues = property.displayOptions.show[dependantName]; - let dependantValue: DisplayCondition | string | number | boolean = ''; - - if ( - displayOptionsValues && - Array.isArray(displayOptionsValues) && - displayOptionsValues[0] !== undefined && - displayOptionsValues[0] !== null - ) { - dependantValue = displayOptionsValues[0]; - } - - // Create a unique key for each dependant name and value combination - // so that if multiple properties depend on the same property but different values - // they get their own if-then-else block - const dependencyKey = `${dependantName}:${JSON.stringify(dependantValue)}`; - - if (!resolveProperties.includes(dependencyKey)) { - let conditionalValue; - if (typeof dependantValue === 'object' && dependantValue._cnd) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const [key, targetValue] = Object.entries(dependantValue._cnd)[0]; - - if (key === 'eq') { - conditionalValue = { - const: [targetValue], - }; - } else if (key === 'not') { - conditionalValue = { - not: { - const: [targetValue], - }, - }; - } else if (key === 'gt') { - conditionalValue = { - type: 'number', - exclusiveMinimum: [targetValue], - }; - } else if (key === 'gte') { - conditionalValue = { - type: 'number', - minimum: [targetValue], - }; - } else if (key === 'lt') { - conditionalValue = { - type: 'number', - exclusiveMaximum: [targetValue], - }; - } else if (key === 'lte') { - conditionalValue = { - type: 'number', - maximum: [targetValue], - }; - } else if (key === 'startsWith') { - conditionalValue = { - type: 'string', - pattern: `^${targetValue}`, - }; - } else if (key === 'endsWith') { - conditionalValue = { - type: 'string', - pattern: `${targetValue}$`, - }; - } else if (key === 'includes') { - conditionalValue = { - type: 'string', - pattern: `${targetValue}`, - }; - } else if (key === 'regex') { - conditionalValue = { - type: 'string', - pattern: `${targetValue}`, - }; - } else { - conditionalValue = { - enum: [dependantValue], - }; - } - } else { - conditionalValue = { - enum: [dependantValue], - }; - } - propertyRequiredDependencies[dependencyKey] = { - if: { - properties: { - [dependantName]: conditionalValue, - }, - // Require the controlling field in the `if` so the condition only - // matches when it is actually present and equal. Without this, an - // absent controlling field makes `properties` vacuously true and the - // `then` block would fire unexpectedly. - required: [dependantName], - }, - then: { - allOf: [], - }, - }; - resolveProperties.push(dependencyKey); - } - - // Only enforce a field as required when the credential actually marks it `required`. - if (property.required) { - propertyRequiredDependencies[dependencyKey].then?.allOf.push({ - required: [property.name], - }); - } - // Requiredness is now conditional, so drop it from the global required list. - requiredFields = requiredFields.filter((field) => field !== property.name); - } - }); - Object.assign(jsonSchema, { required: requiredFields }); - - // Drop conditionals that ended up with no required fields, so credentials whose - // conditional fields are all optional produce no `allOf` constraints. - jsonSchema.allOf = Object.values(propertyRequiredDependencies).filter( - (dependency) => (dependency.then?.allOf.length ?? 0) > 0, - ); - - if (!jsonSchema.allOf.length) { - delete jsonSchema.allOf; - } - - return jsonSchema as unknown as IDataObject; -} diff --git a/packages/cli/src/public-api/v1/handlers/credentials/credentials.utils.ts b/packages/cli/src/public-api/v1/handlers/credentials/credentials.utils.ts new file mode 100644 index 00000000000..91544dec41b --- /dev/null +++ b/packages/cli/src/public-api/v1/handlers/credentials/credentials.utils.ts @@ -0,0 +1,249 @@ +import type { CredentialsEntity } from '@n8n/db'; +import { + type DisplayCondition, + type IDataObject, + type INodeProperties, + type INodePropertyOptions, +} from 'n8n-workflow'; + +import type { IDependency, IJsonSchema } from '../../../types'; + +function isNodePropertyOptions(options: unknown): options is INodePropertyOptions[] { + return ( + Array.isArray(options) && + options.every( + (item) => typeof item === 'object' && item !== null && 'value' in item && 'name' in item, + ) + ); +} + +/** + * Shared entry for credential list: project id/name plus sharing role and timestamps. + * Derived from credential.shared (SharedCredentials + Project), limited to these fields. + */ +export type CredentialListSharedItem = { + id: string; + name: string; + role: string; + createdAt: Date; + updatedAt: Date; +}; + +/** + * Build the shared array for a credential list item from credential.shared. + * Each entry has id, name from the project and role, createdAt, updatedAt from the shared relation. + */ +export function buildSharedForCredential( + credential: CredentialsEntity, +): CredentialListSharedItem[] { + const shared = credential.shared; + return shared + .filter((sh) => typeof sh.project?.id === 'string') + .map((sh) => ({ + id: sh.project.id, + name: sh.project.name, + role: sh.role, + createdAt: sh.createdAt, + updatedAt: sh.updatedAt, + })); +} + +export function sanitizeCredentials(credential: CredentialsEntity): Partial { + const { data, shared, ...rest } = credential; + return rest; +} + +/** + * toJsonSchema + * Take an array of credentials parameter and map it + * to a JSON Schema (see https://json-schema.org/). With + * the JSON Schema definition we can validate the credential's shape + * @param properties - Credentials properties + */ +export function toJsonSchema(properties: INodeProperties[]): IDataObject { + const jsonSchema: IJsonSchema = { + additionalProperties: false, + type: 'object', + properties: {}, + allOf: [], + required: [], + }; + + const optionsValues: { [key: string]: string[] } = {}; + const resolveProperties: string[] = []; + + // get all possible values of properties type "options" + // so we can later resolve the displayOptions dependencies + properties + .filter((property) => property.type === 'options') + .forEach((property) => { + Object.assign(optionsValues, { + [property.name]: isNodePropertyOptions(property.options) + ? property.options.map((option) => option.value) + : undefined, + }); + }); + + let requiredFields: string[] = []; + + const propertyRequiredDependencies: { [key: string]: IDependency } = {}; + + // add all credential's properties to the properties + // object in the JSON Schema definition. This allows us + // to later validate that only this properties are set in + // the credentials sent in the API call. + // eslint-disable-next-line complexity + properties.forEach((property) => { + if (property.required) { + requiredFields.push(property.name); + } + if (property.type === 'options') { + // if the property is type options, + // include all possible values in the enum property. + Object.assign(jsonSchema.properties, { + [property.name]: { + type: 'string', + enum: isNodePropertyOptions(property.options) + ? property.options.map((data) => data.value) + : undefined, + }, + }); + } else { + Object.assign(jsonSchema.properties, { + [property.name]: { + type: property.type, + }, + }); + } + + // if the credential property has a dependency + // then add a JSON Schema condition that satisfy each property value + // e.x: If A has value X then required B, else required C + // see https://json-schema.org/understanding-json-schema/reference/conditionals.html#if-then-else + if (property.displayOptions?.show) { + const dependantName = Object.keys(property.displayOptions?.show)[0] || ''; + const displayOptionsValues = property.displayOptions.show[dependantName]; + let dependantValue: DisplayCondition | string | number | boolean = ''; + + if ( + displayOptionsValues && + Array.isArray(displayOptionsValues) && + displayOptionsValues[0] !== undefined && + displayOptionsValues[0] !== null + ) { + dependantValue = displayOptionsValues[0]; + } + + // Create a unique key for each dependant name and value combination + // so that if multiple properties depend on the same property but different values + // they get their own if-then-else block + const dependencyKey = `${dependantName}:${JSON.stringify(dependantValue)}`; + + if (!resolveProperties.includes(dependencyKey)) { + let conditionalValue; + if (typeof dependantValue === 'object' && dependantValue._cnd) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const [key, targetValue] = Object.entries(dependantValue._cnd)[0]; + + if (key === 'eq') { + conditionalValue = { + const: [targetValue], + }; + } else if (key === 'not') { + conditionalValue = { + not: { + const: [targetValue], + }, + }; + } else if (key === 'gt') { + conditionalValue = { + type: 'number', + exclusiveMinimum: [targetValue], + }; + } else if (key === 'gte') { + conditionalValue = { + type: 'number', + minimum: [targetValue], + }; + } else if (key === 'lt') { + conditionalValue = { + type: 'number', + exclusiveMaximum: [targetValue], + }; + } else if (key === 'lte') { + conditionalValue = { + type: 'number', + maximum: [targetValue], + }; + } else if (key === 'startsWith') { + conditionalValue = { + type: 'string', + pattern: `^${targetValue}`, + }; + } else if (key === 'endsWith') { + conditionalValue = { + type: 'string', + pattern: `${targetValue}$`, + }; + } else if (key === 'includes') { + conditionalValue = { + type: 'string', + pattern: `${targetValue}`, + }; + } else if (key === 'regex') { + conditionalValue = { + type: 'string', + pattern: `${targetValue}`, + }; + } else { + conditionalValue = { + enum: [dependantValue], + }; + } + } else { + conditionalValue = { + enum: [dependantValue], + }; + } + propertyRequiredDependencies[dependencyKey] = { + if: { + properties: { + [dependantName]: conditionalValue, + }, + // Require the controlling field in the `if` so the condition only + // matches when it is actually present and equal. Without this, an + // absent controlling field makes `properties` vacuously true and the + // `then` block would fire unexpectedly. + required: [dependantName], + }, + then: { + allOf: [], + }, + }; + resolveProperties.push(dependencyKey); + } + + // Only enforce a field as required when the credential actually marks it `required`. + if (property.required) { + propertyRequiredDependencies[dependencyKey].then?.allOf.push({ + required: [property.name], + }); + } + // Requiredness is now conditional, so drop it from the global required list. + requiredFields = requiredFields.filter((field) => field !== property.name); + } + }); + Object.assign(jsonSchema, { required: requiredFields }); + + // Drop conditionals that ended up with no required fields, so credentials whose + // conditional fields are all optional produce no `allOf` constraints. + jsonSchema.allOf = Object.values(propertyRequiredDependencies).filter( + (dependency) => (dependency.then?.allOf.length ?? 0) > 0, + ); + + if (!jsonSchema.allOf.length) { + delete jsonSchema.allOf; + } + + return jsonSchema as unknown as IDataObject; +} diff --git a/packages/cli/src/services/__tests__/credentials-finder.service.test.ts b/packages/cli/src/services/__tests__/credentials-finder.service.test.ts index 4723f0bebdb..be9b7abb031 100644 --- a/packages/cli/src/services/__tests__/credentials-finder.service.test.ts +++ b/packages/cli/src/services/__tests__/credentials-finder.service.test.ts @@ -61,22 +61,24 @@ describe('CredentialsFinderService', () => { }); }); - describe('findCredentialById', () => { + describe('findById', () => { it('queries project credentials by default', async () => { credentialsRepository.findOne.mockResolvedValueOnce(null); - await credentialsFinderService.findCredentialById('credential-id'); + await credentialsFinderService.findById('credential-id'); expect(credentialsRepository.findOne).toHaveBeenCalledWith({ where: { id: 'credential-id', usageScope: 'project' }, + relations: undefined, }); }); - it('includes instance credentials only when explicitly requested', async () => { + it('includes instance credentials and shared project when requested', async () => { credentialsRepository.findOne.mockResolvedValueOnce(null); - await credentialsFinderService.findCredentialById('credential-id', { + await credentialsFinderService.findById('credential-id', { includeInstanceCredentials: true, + includeSharedProject: true, }); expect(credentialsRepository.findOne).toHaveBeenCalledWith({ @@ -84,6 +86,7 @@ describe('CredentialsFinderService', () => { id: 'credential-id', usageScope: In(['project', 'instance']), }, + relations: { shared: { project: true } }, }); }); }); diff --git a/packages/cli/test/integration/database/repositories/credentials.repository.test.ts b/packages/cli/test/integration/database/repositories/credentials.repository.test.ts index 686ab249593..8e03b23e4bc 100644 --- a/packages/cli/test/integration/database/repositories/credentials.repository.test.ts +++ b/packages/cli/test/integration/database/repositories/credentials.repository.test.ts @@ -1,4 +1,5 @@ import { testDb } from '@n8n/backend-test-utils'; +import type { ListQuery } from '@n8n/db'; import { CredentialsRepository, SharedCredentialsRepository } from '@n8n/db'; import { Container } from '@n8n/di'; import type { Scope } from '@n8n/permissions'; @@ -546,18 +547,18 @@ describe('CredentialsRepository', () => { // Both approaches need an explicit order: without one, Postgres is free to // return the rows in any order, and the two queries use different plans. - const oldOptions = { + const oldOptions: ListQuery.Options = { filter: { projectId: teamProject.id, name: 'Test' }, take: 2, skip: 0, - order: { id: 'ASC' as const }, + sortBy: 'id:asc', }; - const newOptions = { + const newOptions: ListQuery.Options = { filter: { name: 'Test' }, take: 2, skip: 0, - order: { id: 'ASC' as const }, + sortBy: 'id:asc', }; // ACT - Old Approach