From dc05720609e7fe51509aca4a3affe563afaa6408 Mon Sep 17 00:00:00 2001 From: Dmitrii Date: Wed, 8 Jul 2026 09:31:37 +0300 Subject: [PATCH] feat(core): Add name-and-type and type-only credential matching modes for package import (#33689) --- .../import-package-request.dto.test.ts | 16 +- .../packages/import-package-request.dto.ts | 5 +- packages/@n8n/cli/docs/commands/package.md | 2 +- .../@n8n/cli/src/commands/package/import.ts | 2 +- .../__tests__/credential-importer.test.ts | 20 +- .../__tests__/credential-matcher.test.ts | 489 +++++++++++++++++- .../credential/credential-matcher-factory.ts | 10 +- .../entities/credential/credential-matcher.ts | 71 ++- .../credential/credential-tier-selection.ts | 37 ++ .../credential/id-based-credential-matcher.ts | 52 +- .../name-and-type-credential-matcher.ts | 34 ++ .../type-only-credential-matcher.ts | 33 ++ .../n8n-packages/n8n-packages.types.ts | 2 +- .../spec/paths/n8n-packages.import.yml | 15 +- 14 files changed, 701 insertions(+), 87 deletions(-) create mode 100644 packages/cli/src/modules/n8n-packages/entities/credential/credential-tier-selection.ts create mode 100644 packages/cli/src/modules/n8n-packages/entities/credential/name-and-type-credential-matcher.ts create mode 100644 packages/cli/src/modules/n8n-packages/entities/credential/type-only-credential-matcher.ts diff --git a/packages/@n8n/api-types/src/dto/packages/__tests__/import-package-request.dto.test.ts b/packages/@n8n/api-types/src/dto/packages/__tests__/import-package-request.dto.test.ts index 69f47ae9160..750b80e665c 100644 --- a/packages/@n8n/api-types/src/dto/packages/__tests__/import-package-request.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/packages/__tests__/import-package-request.dto.test.ts @@ -76,10 +76,24 @@ describe('ImportPackageRequestDto', () => { } }); + it.each(['id-only', 'name-and-type', 'type-only'] as const)( + 'accepts %s as a credentialMatchingMode value', + (credentialMatchingMode) => { + const result = ImportPackageRequestDto.safeParse({ + credentialMatchingMode, + workflowConflictPolicy: 'fail', + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.credentialMatchingMode).toBe(credentialMatchingMode); + } + }, + ); + it('rejects unsupported credentialMatchingMode values', () => { expect( ImportPackageRequestDto.safeParse({ - credentialMatchingMode: 'name-and-type', + credentialMatchingMode: 'fuzzy-match', workflowConflictPolicy: 'fail', }).success, ).toBe(false); diff --git a/packages/@n8n/api-types/src/dto/packages/import-package-request.dto.ts b/packages/@n8n/api-types/src/dto/packages/import-package-request.dto.ts index cbb3f292e51..9a4a26c4978 100644 --- a/packages/@n8n/api-types/src/dto/packages/import-package-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/packages/import-package-request.dto.ts @@ -65,7 +65,10 @@ const credentialBindingsSchema = z export class ImportPackageRequestDto extends Z.class({ projectId: optionalFormId, folderId: optionalFormId, - credentialMatchingMode: z.enum(['id-only']).optional().default('id-only'), + credentialMatchingMode: z + .enum(['id-only', 'name-and-type', 'type-only']) + .optional() + .default('id-only'), credentialMissingMode: z.enum(['must-preexist', 'create-stub']).optional().default('create-stub'), credentialBindings: credentialBindingsSchema, workflowConflictPolicy: z.enum(['new-version', 'fail', 'skip']), diff --git a/packages/@n8n/cli/docs/commands/package.md b/packages/@n8n/cli/docs/commands/package.md index ac4f3f86d72..2ed527015bf 100644 --- a/packages/@n8n/cli/docs/commands/package.md +++ b/packages/@n8n/cli/docs/commands/package.md @@ -46,7 +46,7 @@ n8n-cli package import --file=export.n8np --conflict-policy=fail --credential-mi | `--project` | Target project ID. Defaults to your personal project. | | `--folder` | Target folder ID within the project. Defaults to the project root. | | `--workflow-id-policy` | Whether imported workflows keep their source ID (`source`) or receive a new one (`new`). | -| `--credential-matching-mode` | How credential references are matched on the target instance (`id-only`). | +| `--credential-matching-mode` | How credential references are matched on the target instance: `id-only` (default, match by id), `name-and-type` (match by exact name and type), or `type-only` (match by type). For `name-and-type` and `type-only`, candidates are ranked by scope — owned by the target project, then shared into it, then global — and ties within a scope use the most recently updated credential. | | `--credential-missing-mode` | What to do when a referenced credential cannot be resolved. `create-stub` (instance default) creates empty placeholder credentials in the target project; `must-preexist` requires every referenced credential to already exist. | Requires the API key to hold the `workflow:import` scope. When the import is diff --git a/packages/@n8n/cli/src/commands/package/import.ts b/packages/@n8n/cli/src/commands/package/import.ts index 2bebf124ab4..d9f9c24f794 100644 --- a/packages/@n8n/cli/src/commands/package/import.ts +++ b/packages/@n8n/cli/src/commands/package/import.ts @@ -36,7 +36,7 @@ export default class PackageImport extends BaseCommand { }), credentialMatchingMode: Flags.string({ description: 'How credential references are matched on the target instance', - options: ['id-only'], + options: ['id-only', 'name-and-type', 'type-only'], aliases: ['credential-matching-mode'], }), credentialMissingMode: Flags.string({ diff --git a/packages/cli/src/modules/n8n-packages/entities/credential/__tests__/credential-importer.test.ts b/packages/cli/src/modules/n8n-packages/entities/credential/__tests__/credential-importer.test.ts index 408af7a1954..89fd8e744c4 100644 --- a/packages/cli/src/modules/n8n-packages/entities/credential/__tests__/credential-importer.test.ts +++ b/packages/cli/src/modules/n8n-packages/entities/credential/__tests__/credential-importer.test.ts @@ -1,10 +1,9 @@ -import type { Project, SharedCredentialsRepository, User } from '@n8n/db'; +import type { Project, User } from '@n8n/db'; import { Container } from '@n8n/di'; import { mock } from 'vitest-mock-extended'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import type { CredentialTypes } from '@/credential-types'; -import type { CredentialsFinderService } from '@/credentials/credentials-finder.service'; import type { CredentialsService } from '@/credentials/credentials.service'; import type { ImportContext } from '../../../n8n-packages.types'; @@ -12,6 +11,8 @@ import { CredentialImporter } from '../credential-importer'; import { CredentialMatcherFactory } from '../credential-matcher-factory'; import type { CredentialBindingRequest, CredentialResolutionFailure } from '../credential.types'; import { IdBasedCredentialMatcher } from '../id-based-credential-matcher'; +import type { NameAndTypeCredentialMatcher } from '../name-and-type-credential-matcher'; +import type { TypeOnlyCredentialMatcher } from '../type-only-credential-matcher'; type UsableCredential = Awaited< ReturnType @@ -22,8 +23,6 @@ const usable = (id: string, type = 'githubApi'): UsableCredential => ({ id, type }) as UsableCredential; describe('CredentialImporter', () => { - const credentialsFinderService = mock(); - const sharedCredentialsRepository = mock(); const credentialsService = mock(); const credentialTypes = mock(); const targetProject = mock({ id: 'project-target' }); @@ -73,15 +72,14 @@ describe('CredentialImporter', () => { credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([]); Container.set( IdBasedCredentialMatcher, - new IdBasedCredentialMatcher( - credentialsFinderService, - sharedCredentialsRepository, - credentialTypes, - credentialsService, - ), + new IdBasedCredentialMatcher(credentialTypes, credentialsService), ); importer = new CredentialImporter( - new CredentialMatcherFactory(Container.get(IdBasedCredentialMatcher)), + new CredentialMatcherFactory( + Container.get(IdBasedCredentialMatcher), + mock(), + mock(), + ), credentialsService, ); }); diff --git a/packages/cli/src/modules/n8n-packages/entities/credential/__tests__/credential-matcher.test.ts b/packages/cli/src/modules/n8n-packages/entities/credential/__tests__/credential-matcher.test.ts index a1c65e33dba..03d56b87563 100644 --- a/packages/cli/src/modules/n8n-packages/entities/credential/__tests__/credential-matcher.test.ts +++ b/packages/cli/src/modules/n8n-packages/entities/credential/__tests__/credential-matcher.test.ts @@ -1,42 +1,56 @@ -import type { Project, SharedCredentialsRepository, User } from '@n8n/db'; +import type { Project, SlimProject, User } from '@n8n/db'; import { Container } from '@n8n/di'; import { mock } from 'vitest-mock-extended'; import type { CredentialTypes } from '@/credential-types'; -import type { CredentialsFinderService } from '@/credentials/credentials-finder.service'; import type { CredentialsService } from '@/credentials/credentials.service'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import type { CredentialMatcherContext } from '../credential-matcher'; +import type { CredentialMatcherContext, UsableCredential } from '../credential-matcher'; import { CredentialMatcherFactory } from '../credential-matcher-factory'; import { createFailure } from '../credential.types'; import { IdBasedCredentialMatcher } from '../id-based-credential-matcher'; - -type UsableCredential = Awaited< - ReturnType ->[number]; +import { NameAndTypeCredentialMatcher } from '../name-and-type-credential-matcher'; +import { TypeOnlyCredentialMatcher } from '../type-only-credential-matcher'; const usable = (id: string, type = 'githubApi'): UsableCredential => ({ id, type }) as UsableCredential; -const credentialsFinderService = mock(); -const sharedCredentialsRepository = mock(); +const usableWith = (overrides: Partial & { id: string }): UsableCredential => + ({ + type: 'githubApi', + name: overrides.id, + updatedAt: '2024-01-01T00:00:00.000Z', + isGlobal: false, + homeProject: null, + sharedWithProjects: [], + ...overrides, + }) as UsableCredential; + const credentialsService = mock(); const credentialTypes = mock(); const targetProject = mock({ id: 'project-target' }); +const targetProjectRef = mock({ id: targetProject.id }); const user = mock({ id: 'user-1' }); function createMatcherFactory(): CredentialMatcherFactory { Container.set( IdBasedCredentialMatcher, - new IdBasedCredentialMatcher( - credentialsFinderService, - sharedCredentialsRepository, - credentialTypes, - credentialsService, - ), + new IdBasedCredentialMatcher(credentialTypes, credentialsService), + ); + Container.set( + NameAndTypeCredentialMatcher, + new NameAndTypeCredentialMatcher(credentialTypes, credentialsService), + ); + Container.set( + TypeOnlyCredentialMatcher, + new TypeOnlyCredentialMatcher(credentialTypes, credentialsService), + ); + return new CredentialMatcherFactory( + Container.get(IdBasedCredentialMatcher), + Container.get(NameAndTypeCredentialMatcher), + Container.get(TypeOnlyCredentialMatcher), ); - return new CredentialMatcherFactory(Container.get(IdBasedCredentialMatcher)); } describe('IdBasedCredentialMatcher', () => { @@ -260,9 +274,450 @@ describe('IdBasedCredentialMatcher', () => { }); }); +describe('NameAndTypeCredentialMatcher', () => { + let context: CredentialMatcherContext; + let matcherFactory: CredentialMatcherFactory; + + const requirement = { + id: 'source-cred', + name: 'Production GitHub', + type: 'githubApi', + usedByWorkflows: ['wf-1'], + }; + + beforeEach(() => { + vi.clearAllMocks(); + credentialTypes.recognizes.mockReturnValue(true); + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([]); + context = { projectId: targetProject.id, user }; + matcherFactory = createMatcherFactory(); + }); + + it('matches a credential with the exact same name and type', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'target-cred', + name: 'Production GitHub', + homeProject: targetProjectRef, + }), + ]); + + const result = await matcherFactory.getMatcher('name-and-type').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'target-cred']])); + expect(result.failures).toEqual([]); + }); + + it('does not match when only the name is equal', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'target-cred', + name: 'Production GitHub', + type: 'slackApi', + homeProject: targetProjectRef, + }), + ]); + + const result = await matcherFactory.getMatcher('name-and-type').match([requirement], context); + + expect(result.successes).toEqual(new Map()); + expect(result.failures).toEqual([createFailure(requirement, 'not_found')]); + }); + + it('does not match when only the type is equal', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'target-cred', + name: 'A different name', + homeProject: targetProjectRef, + }), + ]); + + const result = await matcherFactory.getMatcher('name-and-type').match([requirement], context); + + expect(result.successes).toEqual(new Map()); + expect(result.failures).toEqual([createFailure(requirement, 'not_found')]); + }); + + it('prefers a project-owned match over a more recently updated global match', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'global-cred', + name: 'Production GitHub', + isGlobal: true, + updatedAt: '2024-06-01T00:00:00.000Z', + }), + usableWith({ + id: 'project-cred', + name: 'Production GitHub', + homeProject: targetProjectRef, + updatedAt: '2024-01-01T00:00:00.000Z', + }), + ]); + + const result = await matcherFactory.getMatcher('name-and-type').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'project-cred']])); + }); + + it('falls back to a global match when no project-owned candidate exists', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ id: 'global-cred', name: 'Production GitHub', isGlobal: true }), + ]); + + const result = await matcherFactory.getMatcher('name-and-type').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'global-cred']])); + }); + + it('falls back to a credential shared into the target project when none is owned', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'shared-in-cred', + name: 'Production GitHub', + sharedWithProjects: [targetProjectRef], + }), + usableWith({ + id: 'global-cred', + name: 'Production GitHub', + isGlobal: true, + updatedAt: '2024-06-01T00:00:00.000Z', + }), + ]); + + const result = await matcherFactory.getMatcher('name-and-type').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'shared-in-cred']])); + }); + + it('prefers an owned candidate over a more recently updated one merely shared into the project', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'owned-cred', + name: 'Production GitHub', + homeProject: targetProjectRef, + updatedAt: '2024-01-01T00:00:00.000Z', + }), + usableWith({ + id: 'shared-in-cred', + name: 'Production GitHub', + sharedWithProjects: [targetProjectRef], + updatedAt: '2024-06-01T00:00:00.000Z', + }), + ]); + + const result = await matcherFactory.getMatcher('name-and-type').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'owned-cred']])); + }); + + it('picks the most recently updated candidate within a tier', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'older-cred', + name: 'Production GitHub', + homeProject: targetProjectRef, + updatedAt: '2024-01-01T00:00:00.000Z', + }), + usableWith({ + id: 'newer-cred', + name: 'Production GitHub', + homeProject: targetProjectRef, + updatedAt: '2024-06-01T00:00:00.000Z', + }), + ]); + + const result = await matcherFactory.getMatcher('name-and-type').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'newer-cred']])); + }); + + it('keeps the more recently updated candidate when an older one is encountered after it', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'newer-cred', + name: 'Production GitHub', + homeProject: targetProjectRef, + updatedAt: '2024-06-01T00:00:00.000Z', + }), + usableWith({ + id: 'older-cred', + name: 'Production GitHub', + homeProject: targetProjectRef, + updatedAt: '2024-01-01T00:00:00.000Z', + }), + ]); + + const result = await matcherFactory.getMatcher('name-and-type').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'newer-cred']])); + }); + + it('breaks ties on identical updatedAt by the lexicographically smaller id', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'cred-b', + name: 'Production GitHub', + homeProject: targetProjectRef, + }), + usableWith({ + id: 'cred-a', + name: 'Production GitHub', + homeProject: targetProjectRef, + }), + ]); + + const result = await matcherFactory.getMatcher('name-and-type').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'cred-a']])); + }); + + it('keeps the tie-break winner when the smaller id is encountered first', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'cred-a', + name: 'Production GitHub', + homeProject: targetProjectRef, + }), + usableWith({ + id: 'cred-b', + name: 'Production GitHub', + homeProject: targetProjectRef, + }), + ]); + + const result = await matcherFactory.getMatcher('name-and-type').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'cred-a']])); + }); + + it('bypasses name/type matching for a reference with an explicit credentialBindings entry', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'name-type-match', + name: 'Production GitHub', + homeProject: targetProjectRef, + }), + usableWith({ + id: 'explicit-target', + name: 'Completely unrelated name', + type: 'slackApi', + homeProject: targetProjectRef, + }), + ]); + + const result = await matcherFactory.getMatcher('name-and-type').match([requirement], { + ...context, + credentialBindings: new Map([['source-cred', 'explicit-target']]), + }); + + // Bound explicitly to a credential of a different type than the reference requires, + // so it surfaces as a type_mismatch rather than silently matching by name/type instead. + expect(result.successes).toEqual(new Map()); + expect(result.failures).toEqual([ + { + ...createFailure(requirement, 'type_mismatch'), + targetId: 'explicit-target', + expectedType: 'githubApi', + actualType: 'slackApi', + }, + ]); + }); + + it('uses the explicit binding target even when another credential would win the name/type search', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'fuzzy-winner', + name: 'Production GitHub', // exact name+type match — would win an unbound search + homeProject: targetProjectRef, + }), + usableWith({ + id: 'explicit-target', + name: 'Totally different name', // would never surface via name/type search on its own + homeProject: targetProjectRef, + }), + ]); + + const result = await matcherFactory.getMatcher('name-and-type').match([requirement], { + ...context, + credentialBindings: new Map([['source-cred', 'explicit-target']]), + }); + + expect(result.successes).toEqual(new Map([['source-cred', 'explicit-target']])); + expect(result.failures).toEqual([]); + }); +}); + +describe('TypeOnlyCredentialMatcher', () => { + let context: CredentialMatcherContext; + let matcherFactory: CredentialMatcherFactory; + + const requirement = { + id: 'source-cred', + name: 'Irrelevant name', + type: 'githubApi', + usedByWorkflows: ['wf-1'], + }; + + beforeEach(() => { + vi.clearAllMocks(); + credentialTypes.recognizes.mockReturnValue(true); + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([]); + context = { projectId: targetProject.id, user }; + matcherFactory = createMatcherFactory(); + }); + + it('matches any credential of the same type regardless of name', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'target-cred', + name: 'A completely different name', + homeProject: targetProjectRef, + }), + ]); + + const result = await matcherFactory.getMatcher('type-only').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'target-cred']])); + expect(result.failures).toEqual([]); + }); + + it('reports not_found when no credential of the type exists', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ id: 'other-cred', type: 'slackApi', homeProject: targetProjectRef }), + ]); + + const result = await matcherFactory.getMatcher('type-only').match([requirement], context); + + expect(result.successes).toEqual(new Map()); + expect(result.failures).toEqual([createFailure(requirement, 'not_found')]); + }); + + it('prefers a project-owned match over a more recently updated global match', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ id: 'global-cred', isGlobal: true, updatedAt: '2024-06-01T00:00:00.000Z' }), + usableWith({ + id: 'project-cred', + homeProject: targetProjectRef, + updatedAt: '2024-01-01T00:00:00.000Z', + }), + ]); + + const result = await matcherFactory.getMatcher('type-only').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'project-cred']])); + }); + + it('falls back to a global match when no project-owned candidate exists', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ id: 'global-cred', isGlobal: true }), + ]); + + const result = await matcherFactory.getMatcher('type-only').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'global-cred']])); + }); + + it('falls back to a credential shared into the target project when none is owned', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ id: 'shared-in-cred', sharedWithProjects: [targetProjectRef] }), + usableWith({ id: 'global-cred', isGlobal: true, updatedAt: '2024-06-01T00:00:00.000Z' }), + ]); + + const result = await matcherFactory.getMatcher('type-only').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'shared-in-cred']])); + }); + + it('prefers an owned candidate over a more recently updated one merely shared into the project', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'owned-cred', + homeProject: targetProjectRef, + updatedAt: '2024-01-01T00:00:00.000Z', + }), + usableWith({ + id: 'shared-in-cred', + sharedWithProjects: [targetProjectRef], + updatedAt: '2024-06-01T00:00:00.000Z', + }), + ]); + + const result = await matcherFactory.getMatcher('type-only').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'owned-cred']])); + }); + + it('picks the most recently updated candidate within a tier', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'older-cred', + homeProject: targetProjectRef, + updatedAt: '2024-01-01T00:00:00.000Z', + }), + usableWith({ + id: 'newer-cred', + homeProject: targetProjectRef, + updatedAt: '2024-06-01T00:00:00.000Z', + }), + ]); + + const result = await matcherFactory.getMatcher('type-only').match([requirement], context); + + expect(result.successes).toEqual(new Map([['source-cred', 'newer-cred']])); + }); + + it('bypasses type matching for a reference with an explicit credentialBindings entry', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ id: 'type-match', homeProject: targetProjectRef }), + usableWith({ id: 'explicit-target', type: 'slackApi', homeProject: targetProjectRef }), + ]); + + const result = await matcherFactory.getMatcher('type-only').match([requirement], { + ...context, + credentialBindings: new Map([['source-cred', 'explicit-target']]), + }); + + // Bound explicitly to a credential of a different type than the reference requires, + // so it surfaces as a type_mismatch rather than silently matching by type instead. + expect(result.successes).toEqual(new Map()); + expect(result.failures).toEqual([ + { + ...createFailure(requirement, 'type_mismatch'), + targetId: 'explicit-target', + expectedType: 'githubApi', + actualType: 'slackApi', + }, + ]); + }); + + it('uses the explicit binding target even when another credential would win the type search', async () => { + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + usableWith({ + id: 'fuzzy-winner', + homeProject: targetProjectRef, // project-tier — would win an unbound type-only search + updatedAt: '2024-06-01T00:00:00.000Z', + }), + usableWith({ + id: 'explicit-target', + isGlobal: true, // global-tier — would lose to fuzzy-winner in an unbound search + updatedAt: '2020-01-01T00:00:00.000Z', + }), + ]); + + const result = await matcherFactory.getMatcher('type-only').match([requirement], { + ...context, + credentialBindings: new Map([['source-cred', 'explicit-target']]), + }); + + expect(result.successes).toEqual(new Map([['source-cred', 'explicit-target']])); + expect(result.failures).toEqual([]); + }); +}); + describe('CredentialMatcherFactory', () => { it('rejects unsupported credential matching modes', () => { const factory = createMatcherFactory(); - expect(() => factory.getMatcher('name-and-type' as 'id-only')).toThrow(BadRequestError); + expect(() => factory.getMatcher('fuzzy-match' as 'id-only')).toThrow(BadRequestError); }); }); diff --git a/packages/cli/src/modules/n8n-packages/entities/credential/credential-matcher-factory.ts b/packages/cli/src/modules/n8n-packages/entities/credential/credential-matcher-factory.ts index c47e59ccb2a..b52816af90e 100644 --- a/packages/cli/src/modules/n8n-packages/entities/credential/credential-matcher-factory.ts +++ b/packages/cli/src/modules/n8n-packages/entities/credential/credential-matcher-factory.ts @@ -4,16 +4,24 @@ import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import type { CredentialMatcher } from './credential-matcher'; import { IdBasedCredentialMatcher } from './id-based-credential-matcher'; +import { NameAndTypeCredentialMatcher } from './name-and-type-credential-matcher'; +import { TypeOnlyCredentialMatcher } from './type-only-credential-matcher'; import type { CredentialMatchingMode } from '../../n8n-packages.types'; @Service() export class CredentialMatcherFactory { private readonly matchers: Record; - constructor(idBasedMatcher: IdBasedCredentialMatcher) { + constructor( + idBasedMatcher: IdBasedCredentialMatcher, + nameAndTypeMatcher: NameAndTypeCredentialMatcher, + typeOnlyMatcher: TypeOnlyCredentialMatcher, + ) { /* eslint-disable @typescript-eslint/naming-convention -- API credential matching mode keys */ this.matchers = { 'id-only': idBasedMatcher, + 'name-and-type': nameAndTypeMatcher, + 'type-only': typeOnlyMatcher, }; /* eslint-enable @typescript-eslint/naming-convention */ } diff --git a/packages/cli/src/modules/n8n-packages/entities/credential/credential-matcher.ts b/packages/cli/src/modules/n8n-packages/entities/credential/credential-matcher.ts index 59e727efab7..73870bf79e9 100644 --- a/packages/cli/src/modules/n8n-packages/entities/credential/credential-matcher.ts +++ b/packages/cli/src/modules/n8n-packages/entities/credential/credential-matcher.ts @@ -1,7 +1,7 @@ -import type { SharedCredentialsRepository, User } from '@n8n/db'; +import type { User } from '@n8n/db'; import type { CredentialTypes } from '@/credential-types'; -import type { CredentialsFinderService } from '@/credentials/credentials-finder.service'; +import type { CredentialsService } from '@/credentials/credentials.service'; import { createFailure, @@ -17,6 +17,11 @@ export interface CredentialMatcherContext { credentialBindings?: ImportBindingMap; } +/** A credential the importing user can use in the target project, as returned by `CredentialsService`. */ +export type UsableCredential = Awaited< + ReturnType +>[number]; + /** * A target-project credential a matcher located for a package reference, before * type compatibility is enforced. {@link CredentialMatcher.match} compares @@ -32,9 +37,8 @@ export interface ResolvedCredentialMatch { export abstract class CredentialMatcher { constructor( - protected readonly credentialsFinderService: CredentialsFinderService, - protected readonly sharedCredentialsRepository: SharedCredentialsRepository, protected readonly credentialTypes: CredentialTypes, + protected readonly credentialsService: CredentialsService, ) {} async match( @@ -43,8 +47,19 @@ export abstract class CredentialMatcher { ): Promise { const orphanFailures = orphanBindingFailures(context.credentialBindings, requirements); const { known, unknownTypeFailures } = partitionByKnownType(requirements, this.credentialTypes); + const { bound, unbound } = partitionByExplicitBinding(known, context.credentialBindings); - const located = await this.resolve(known, context); + const usableCredentials = + known.length === 0 + ? [] + : await this.credentialsService.getCredentialsAUserCanUseInAWorkflow(context.user, { + projectId: context.projectId, + }); + const usableTypesById = new Map(usableCredentials.map((c) => [c.id, c.type])); + + const boundLocated = resolveExplicitBindings(bound, usableTypesById); + const resolvedLocated = this.resolve(unbound, usableCredentials, context); + const located = new Map([...boundLocated, ...resolvedLocated]); const successes: ImportBindingMap = new Map(); const notFoundFailures: CredentialResolutionFailure[] = []; @@ -75,14 +90,52 @@ export abstract class CredentialMatcher { } /** - * Locates the target-project credential each known reference points at, keyed by - * the reference's source id. Returns only references that resolve to a reachable, + * Locates the target-project credential each reference points at, keyed by the + * reference's source id. Returns only references that resolve to a reachable, * usable credential; type compatibility is enforced by {@link match}, not here. */ protected abstract resolve( - known: PackageCredentialRequirement[], + unbound: PackageCredentialRequirement[], + usableCredentials: UsableCredential[], context: CredentialMatcherContext, - ): Promise>; + ): Map; +} + +interface BoundReference { + reference: PackageCredentialRequirement; + targetId: string; +} + +function partitionByExplicitBinding( + known: PackageCredentialRequirement[], + bindings: ImportBindingMap | undefined, +): { bound: BoundReference[]; unbound: PackageCredentialRequirement[] } { + const bound: BoundReference[] = []; + const unbound: PackageCredentialRequirement[] = []; + + for (const reference of known) { + const targetId = bindings?.get(reference.id); + if (targetId === undefined) { + unbound.push(reference); + } else { + bound.push({ reference, targetId }); + } + } + + return { bound, unbound }; +} + +function resolveExplicitBindings( + bound: BoundReference[], + usableTypesById: Map, +): Map { + return new Map( + bound.flatMap(({ reference, targetId }) => { + const targetType = usableTypesById.get(targetId); + if (targetType === undefined) return []; + return [[reference.id, { targetId, targetType }] as const]; + }), + ); } function partitionByKnownType( diff --git a/packages/cli/src/modules/n8n-packages/entities/credential/credential-tier-selection.ts b/packages/cli/src/modules/n8n-packages/entities/credential/credential-tier-selection.ts new file mode 100644 index 00000000000..8f0ece6a740 --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/entities/credential/credential-tier-selection.ts @@ -0,0 +1,37 @@ +import type { ResolvedCredentialMatch, UsableCredential } from './credential-matcher'; +import type { PackageCredentialRequirement } from '../../spec/requirements.schema'; + +function selectBestCandidate( + candidates: UsableCredential[], + projectId: string, +): UsableCredential | undefined { + const owned = candidates.filter((c) => c.homeProject?.id === projectId); + const sharedIn = candidates.filter((c) => c.sharedWithProjects.some((p) => p.id === projectId)); + const global = candidates.filter((c) => c.isGlobal); + + const tier = owned.length > 0 ? owned : sharedIn.length > 0 ? sharedIn : global; + + return tier.reduce((best, candidate) => { + if (!best) return candidate; + if (candidate.updatedAt !== best.updatedAt) { + return candidate.updatedAt > best.updatedAt ? candidate : best; + } + return candidate.id < best.id ? candidate : best; + }, undefined); +} + +export function resolveByCandidateFilter( + known: PackageCredentialRequirement[], + usableCredentials: UsableCredential[], + projectId: string, + isCandidate: (candidate: UsableCredential, reference: PackageCredentialRequirement) => boolean, +): Map { + return new Map( + known.flatMap((reference) => { + const candidates = usableCredentials.filter((candidate) => isCandidate(candidate, reference)); + const best = selectBestCandidate(candidates, projectId); + if (best === undefined) return []; + return [[reference.id, { targetId: best.id, targetType: best.type }] as const]; + }), + ); +} diff --git a/packages/cli/src/modules/n8n-packages/entities/credential/id-based-credential-matcher.ts b/packages/cli/src/modules/n8n-packages/entities/credential/id-based-credential-matcher.ts index b46d117654a..aa30ca3cf51 100644 --- a/packages/cli/src/modules/n8n-packages/entities/credential/id-based-credential-matcher.ts +++ b/packages/cli/src/modules/n8n-packages/entities/credential/id-based-credential-matcher.ts @@ -1,63 +1,35 @@ -import { SharedCredentialsRepository } from '@n8n/db'; -import type { User } from '@n8n/db'; import { Service } from '@n8n/di'; import { CredentialTypes } from '@/credential-types'; -import { CredentialsFinderService } from '@/credentials/credentials-finder.service'; import { CredentialsService } from '@/credentials/credentials.service'; import { CredentialMatcher, - type CredentialMatcherContext, type ResolvedCredentialMatch, + type UsableCredential, } from './credential-matcher'; import type { PackageCredentialRequirement } from '../../spec/requirements.schema'; @Service() export class IdBasedCredentialMatcher extends CredentialMatcher { - constructor( - credentialsFinderService: CredentialsFinderService, - sharedCredentialsRepository: SharedCredentialsRepository, - credentialTypes: CredentialTypes, - private readonly credentialsService: CredentialsService, - ) { - super(credentialsFinderService, sharedCredentialsRepository, credentialTypes); + constructor(credentialTypes: CredentialTypes, credentialsService: CredentialsService) { + super(credentialTypes, credentialsService); } - protected async resolve( - known: PackageCredentialRequirement[], - context: CredentialMatcherContext, - ): Promise> { - if (known.length === 0) { - return new Map(); - } - - const bindings = context.credentialBindings; - const usableTypesById = await this.findUsableCredentialTypesById( - context.projectId, - context.user, + protected resolve( + unbound: PackageCredentialRequirement[], + usableCredentials: UsableCredential[], + ): Map { + const usableTypesById = new Map( + usableCredentials.map((credential) => [credential.id, credential.type]), ); return new Map( - known.flatMap((reference) => { - const targetId = bindings?.get(reference.id) ?? reference.id; - const targetType = usableTypesById.get(targetId); + unbound.flatMap((reference) => { + const targetType = usableTypesById.get(reference.id); if (targetType === undefined) return []; - return [[reference.id, { targetId, targetType }] as const]; + return [[reference.id, { targetId: reference.id, targetType }] as const]; }), ); } - - /** Maps each credential the user can use in the target project to its type. */ - private async findUsableCredentialTypesById( - projectId: string, - user: User, - ): Promise> { - const usableCredentials = await this.credentialsService.getCredentialsAUserCanUseInAWorkflow( - user, - { projectId }, - ); - - return new Map(usableCredentials.map((credential) => [credential.id, credential.type])); - } } diff --git a/packages/cli/src/modules/n8n-packages/entities/credential/name-and-type-credential-matcher.ts b/packages/cli/src/modules/n8n-packages/entities/credential/name-and-type-credential-matcher.ts new file mode 100644 index 00000000000..68b3c6aaed8 --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/entities/credential/name-and-type-credential-matcher.ts @@ -0,0 +1,34 @@ +import { Service } from '@n8n/di'; + +import { CredentialTypes } from '@/credential-types'; +import { CredentialsService } from '@/credentials/credentials.service'; + +import { + CredentialMatcher, + type CredentialMatcherContext, + type ResolvedCredentialMatch, + type UsableCredential, +} from './credential-matcher'; +import { resolveByCandidateFilter } from './credential-tier-selection'; +import type { PackageCredentialRequirement } from '../../spec/requirements.schema'; + +@Service() +export class NameAndTypeCredentialMatcher extends CredentialMatcher { + constructor(credentialTypes: CredentialTypes, credentialsService: CredentialsService) { + super(credentialTypes, credentialsService); + } + + protected resolve( + unbound: PackageCredentialRequirement[], + usableCredentials: UsableCredential[], + context: CredentialMatcherContext, + ): Map { + return resolveByCandidateFilter( + unbound, + usableCredentials, + context.projectId, + (candidate, reference) => + candidate.name === reference.name && candidate.type === reference.type, + ); + } +} diff --git a/packages/cli/src/modules/n8n-packages/entities/credential/type-only-credential-matcher.ts b/packages/cli/src/modules/n8n-packages/entities/credential/type-only-credential-matcher.ts new file mode 100644 index 00000000000..592a418e4ff --- /dev/null +++ b/packages/cli/src/modules/n8n-packages/entities/credential/type-only-credential-matcher.ts @@ -0,0 +1,33 @@ +import { Service } from '@n8n/di'; + +import { CredentialTypes } from '@/credential-types'; +import { CredentialsService } from '@/credentials/credentials.service'; + +import { + CredentialMatcher, + type CredentialMatcherContext, + type ResolvedCredentialMatch, + type UsableCredential, +} from './credential-matcher'; +import { resolveByCandidateFilter } from './credential-tier-selection'; +import type { PackageCredentialRequirement } from '../../spec/requirements.schema'; + +@Service() +export class TypeOnlyCredentialMatcher extends CredentialMatcher { + constructor(credentialTypes: CredentialTypes, credentialsService: CredentialsService) { + super(credentialTypes, credentialsService); + } + + protected resolve( + unbound: PackageCredentialRequirement[], + usableCredentials: UsableCredential[], + context: CredentialMatcherContext, + ): Map { + return resolveByCandidateFilter( + unbound, + usableCredentials, + context.projectId, + (candidate, reference) => candidate.type === reference.type, + ); + } +} diff --git a/packages/cli/src/modules/n8n-packages/n8n-packages.types.ts b/packages/cli/src/modules/n8n-packages/n8n-packages.types.ts index a66dec89b17..59a875716e7 100644 --- a/packages/cli/src/modules/n8n-packages/n8n-packages.types.ts +++ b/packages/cli/src/modules/n8n-packages/n8n-packages.types.ts @@ -9,7 +9,7 @@ export type { CredentialResolution } from './entities/credential/credential.type export { WorkflowPublishingPolicy } from './entities/workflow/workflow-publishing-policy.types'; export type { WorkflowPublishingOutcome } from './entities/workflow/workflow-publishing-policy.types'; -export type CredentialMatchingMode = 'id-only'; +export type CredentialMatchingMode = 'id-only' | 'name-and-type' | 'type-only'; export type CredentialMissingMode = 'must-preexist' | 'create-stub'; /* eslint-disable @typescript-eslint/naming-convention -- enum-like members for IDE documentation */ diff --git a/packages/cli/src/public-api/v1/handlers/n8n-packages/spec/paths/n8n-packages.import.yml b/packages/cli/src/public-api/v1/handlers/n8n-packages/spec/paths/n8n-packages.import.yml index da3c82ea3e4..4187c8f3c21 100644 --- a/packages/cli/src/public-api/v1/handlers/n8n-packages/spec/paths/n8n-packages.import.yml +++ b/packages/cli/src/public-api/v1/handlers/n8n-packages/spec/paths/n8n-packages.import.yml @@ -23,9 +23,8 @@ post: fresh local id; imported workflows arrive inactive unless `workflowPublishingPolicy` publishes them after import. - Credential references are resolved before any workflow is written. Only credentials owned by the - target project (with `credential:read`) or global credentials (with `credential:read`) - match. Credentials shared into the project as `credential:user` are not matched. + Credential references are resolved before any workflow is written. Credentials owned by, + shared with, or global to the target project (with `credential:read`) match. requestBody: required: true content: @@ -54,10 +53,18 @@ post: type: string enum: - id-only + - name-and-type + - type-only default: id-only description: > How credential references in `requirements.credentials` are matched on - the target instance. Only `id-only` is supported today. + the target instance. `id-only` (default) matches by id. `name-and-type` + matches credentials with the exact same name and type. `type-only` + matches any credential of the same type. For `name-and-type` and + `type-only`, candidates are ranked by scope — a credential owned by + the target project wins over one merely shared into it, which in + turn wins over a global credential; if several candidates remain + in the winning scope, the most recently updated one is chosen. credentialMissingMode: type: string enum: