From 55c3150c1126c277a3bfff5ff2fce07b4a667969 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour <4711238+mutdmour@users.noreply.github.com> Date: Thu, 20 Nov 2025 17:48:49 +0100 Subject: [PATCH] feat: Add support for global credentials (#21700) Co-authored-by: Claude --- .../dto/credentials/create-credential.dto.ts | 1 + .../credentials-get-many-request.dto.ts | 6 + .../@n8n/backend-test-utils/src/random.ts | 5 +- .../db/src/entities/credentials-entity.ts | 6 + packages/@n8n/db/src/entities/types-db.ts | 1 + ...-IsGlobalGlobalColumnToCredentialsTable.ts | 21 + .../@n8n/db/src/migrations/mysqldb/index.ts | 2 + .../db/src/migrations/postgresdb/index.ts | 2 + .../@n8n/db/src/migrations/sqlite/index.ts | 2 + .../repositories/credentials.repository.ts | 21 +- .../scope-information.test.ts.snap | 1 + packages/@n8n/permissions/src/constants.ee.ts | 2 +- .../src/roles/scopes/global-scopes.ee.ts | 1 + packages/cli/build.log | 4 + .../__tests__/credentials.controller.test.ts | 199 ++- .../__tests__/credentials.service.test.ts | 1077 ++++++++++++++++- .../credentials/credentials-finder.service.ts | 126 +- .../src/credentials/credentials.controller.ts | 15 +- .../src/credentials/credentials.service.ts | 334 +++-- .../__tests__/credentials.repository.test.ts | 293 ++++- .../source-control-export.service.test.ts | 158 +++ .../source-control-import.service.ee.test.ts | 76 ++ .../source-control-status.service.test.ts | 85 ++ .../source-control-export.service.ee.ts | 3 +- .../source-control-import.service.ee.ts | 6 +- .../source-control-status.service.ee.ts | 5 +- .../types/exportable-credential.ts | 5 + .../credentials-permission-checker.test.ts | 63 + .../credentials-permission-checker.ts | 24 +- .../chat-hub-credentials.service.test.ts | 244 ++++ .../chat-hub/chat-hub-credentials.service.ts | 2 + .../mcp/__tests__/webhook-utils.test.ts | 1 + .../__tests__/check-access.test.ts | 137 +++ .../cli/src/permissions.ee/check-access.ts | 20 +- packages/cli/src/requests.ts | 1 + .../credentials-finder.service.test.ts | 448 ++++++- packages/cli/src/services/role.service.ts | 17 +- .../__tests__/workflows.controller.test.ts | 131 +- .../cli/src/workflows/workflows.controller.ts | 4 +- .../cli/test/integration/ai/ai.api.test.ts | 1 + .../credentials/credentials.api.test.ts | 120 ++ .../source-control-export.service.test.ts | 87 ++ .../source-control-import.service.test.ts | 70 ++ .../source-control.service.test.ts | 195 +++ .../integration/services/role.service.test.ts | 280 +++++ .../frontend/@n8n/i18n/src/locales/en.json | 3 + packages/frontend/editor-ui/src/Interface.ts | 1 + .../components/ProjectCardBadge.test.ts | 94 ++ .../projects/components/ProjectCardBadge.vue | 29 + .../components/ProjectSharing.test.ts | 170 +++ .../projects/components/ProjectSharing.vue | 54 +- .../__tests__/credentials.store.test.ts | 331 +++++ .../components/CredentialCard.test.ts | 64 + .../credentials/components/CredentialCard.vue | 1 + .../CredentialEdit/CredentialEdit.vue | 13 + .../CredentialSharing.ee.test.ts | 314 +++++ .../CredentialEdit/CredentialSharing.ee.vue | 13 +- .../features/credentials/credentials.api.ts | 2 + .../features/credentials/credentials.store.ts | 4 + .../features/credentials/credentials.types.ts | 1 + .../credentials/views/CredentialsView.vue | 7 + .../playwright/tests/ui/17-sharing.spec.ts | 8 +- .../ui/credential-api-operations.spec.ts | 2 + .../tests/ui/global-credentials.spec.ts | 161 +++ packages/workflow/src/interfaces.ts | 1 + 65 files changed, 5388 insertions(+), 187 deletions(-) create mode 100644 packages/@n8n/db/src/migrations/common/1762771954619-IsGlobalGlobalColumnToCredentialsTable.ts create mode 100644 packages/cli/build.log create mode 100644 packages/cli/src/modules/chat-hub/__tests__/chat-hub-credentials.service.test.ts create mode 100644 packages/frontend/editor-ui/src/features/credentials/__tests__/credentials.store.test.ts create mode 100644 packages/frontend/editor-ui/src/features/credentials/components/CredentialEdit/CredentialSharing.ee.test.ts create mode 100644 packages/testing/playwright/tests/ui/global-credentials.spec.ts diff --git a/packages/@n8n/api-types/src/dto/credentials/create-credential.dto.ts b/packages/@n8n/api-types/src/dto/credentials/create-credential.dto.ts index 7b8690a0a11..f86b69d54db 100644 --- a/packages/@n8n/api-types/src/dto/credentials/create-credential.dto.ts +++ b/packages/@n8n/api-types/src/dto/credentials/create-credential.dto.ts @@ -7,4 +7,5 @@ export class CreateCredentialDto extends Z.class({ data: z.record(z.string(), z.unknown()), projectId: z.string().optional(), uiContext: z.string().optional(), + isGlobal: z.boolean().optional(), }) {} diff --git a/packages/@n8n/api-types/src/dto/credentials/credentials-get-many-request.dto.ts b/packages/@n8n/api-types/src/dto/credentials/credentials-get-many-request.dto.ts index cfc033fda6f..d51cc6cd402 100644 --- a/packages/@n8n/api-types/src/dto/credentials/credentials-get-many-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/credentials/credentials-get-many-request.dto.ts @@ -21,4 +21,10 @@ export class CredentialsGetManyRequestQuery extends Z.class({ includeData: booleanFromString.optional(), onlySharedWithMe: booleanFromString.optional(), + + /** + * Includes global credentials (credentials available to all users). + * Defaults to false. + */ + includeGlobal: booleanFromString.optional().default('false'), }) {} diff --git a/packages/@n8n/backend-test-utils/src/random.ts b/packages/@n8n/backend-test-utils/src/random.ts index e75a1d0d9b7..9e073614188 100644 --- a/packages/@n8n/backend-test-utils/src/random.ts +++ b/packages/@n8n/backend-test-utils/src/random.ts @@ -8,6 +8,7 @@ export type CredentialPayload = { type: string; data: ICredentialDataDecryptedObject; isManaged?: boolean; + isGlobal?: boolean; }; export const randomApiKey = () => `n8n_api_${randomString(40)}`; @@ -44,11 +45,13 @@ export const randomEmail = () => `${randomName()}@${randomName()}.${randomTopLev export const randomCredentialPayload = ({ isManaged = false, -}: { isManaged?: boolean } = {}): CredentialPayload => ({ + isGlobal, +}: { isManaged?: boolean; isGlobal?: boolean } = {}): CredentialPayload => ({ name: randomName(), type: randomName(), data: { accessToken: randomString(6, 16) }, isManaged, + isGlobal, }); export const randomCredentialPayloadWithOauthTokenData = ({ diff --git a/packages/@n8n/db/src/entities/credentials-entity.ts b/packages/@n8n/db/src/entities/credentials-entity.ts index fa427093923..1831525c0ed 100644 --- a/packages/@n8n/db/src/entities/credentials-entity.ts +++ b/packages/@n8n/db/src/entities/credentials-entity.ts @@ -36,6 +36,12 @@ export class CredentialsEntity extends WithTimestampsAndStringId implements ICre @Column({ default: false }) isManaged: boolean; + /** + * Whether the credential is available for use by all users. + */ + @Column({ default: false }) + isGlobal: boolean; + toJSON() { const { shared, ...rest } = this; return rest; diff --git a/packages/@n8n/db/src/entities/types-db.ts b/packages/@n8n/db/src/entities/types-db.ts index 82f3dba355e..97da356819d 100644 --- a/packages/@n8n/db/src/entities/types-db.ts +++ b/packages/@n8n/db/src/entities/types-db.ts @@ -87,6 +87,7 @@ export interface ICredentialsDb extends ICredentialsBase, ICredentialsEncrypted id: string; name: string; shared?: SharedCredentials[]; + isGlobal?: boolean; } export interface IExecutionResponse extends IExecutionBase { diff --git a/packages/@n8n/db/src/migrations/common/1762771954619-IsGlobalGlobalColumnToCredentialsTable.ts b/packages/@n8n/db/src/migrations/common/1762771954619-IsGlobalGlobalColumnToCredentialsTable.ts new file mode 100644 index 00000000000..d33efcef151 --- /dev/null +++ b/packages/@n8n/db/src/migrations/common/1762771954619-IsGlobalGlobalColumnToCredentialsTable.ts @@ -0,0 +1,21 @@ +import type { MigrationContext, ReversibleMigration } from '../migration-types'; + +export class AddIsGlobalColumnToCredentialsTable1762771954619 implements ReversibleMigration { + async up({ escape, runQuery, isSqlite }: MigrationContext) { + const tableName = escape.tableName('credentials_entity'); + const columnName = escape.columnName('isGlobal'); + + const defaultValue = isSqlite ? 0 : 'FALSE'; + + await runQuery( + `ALTER TABLE ${tableName} ADD COLUMN ${columnName} BOOLEAN NOT NULL DEFAULT ${defaultValue}`, + ); + } + + async down({ escape, runQuery }: MigrationContext) { + const tableName = escape.tableName('credentials_entity'); + const columnName = escape.columnName('isGlobal'); + + await runQuery(`ALTER TABLE ${tableName} DROP COLUMN ${columnName}`); + } +} diff --git a/packages/@n8n/db/src/migrations/mysqldb/index.ts b/packages/@n8n/db/src/migrations/mysqldb/index.ts index 78388f1f7af..333d7816a18 100644 --- a/packages/@n8n/db/src/migrations/mysqldb/index.ts +++ b/packages/@n8n/db/src/migrations/mysqldb/index.ts @@ -113,6 +113,7 @@ import { CreateWorkflowDependencyTable1760314000000 } from '../common/1760314000 import { AddAttachmentsToChatHubMessages1761773155024 } from '../common/1761773155024-AddAttachmentsToChatHubMessages'; import { AddWorkflowDescriptionColumn1762177736257 } from '../common/1762177736257-AddWorkflowDescriptionColumn'; import { BackfillMissingWorkflowHistoryRecords1762763704614 } from '../common/1762763704614-BackfillMissingWorkflowHistoryRecords'; +import { AddIsGlobalColumnToCredentialsTable1762771954619 } from '../common/1762771954619-IsGlobalGlobalColumnToCredentialsTable'; import { AddWorkflowHistoryAutoSaveFields1762847206508 } from '../common/1762847206508-AddWorkflowHistoryAutoSaveFields'; import { AddActiveVersionIdColumn1763047800000 } from '../common/1763047800000-AddActiveVersionIdColumn'; import { ChangeOAuthStateColumnToUnboundedVarchar1763572724000 } from '../common/1763572724000-ChangeOAuthStateColumnToUnboundedVarchar'; @@ -232,6 +233,7 @@ export const mysqlMigrations: Migration[] = [ AddWorkflowDescriptionColumn1762177736257, CreateOAuthEntities1760116750277, BackfillMissingWorkflowHistoryRecords1762763704614, + AddIsGlobalColumnToCredentialsTable1762771954619, AddWorkflowHistoryAutoSaveFields1762847206508, AddToolsColumnToChatHubTables1761830340990, ChangeOAuthStateColumnToUnboundedVarchar1763572724000, diff --git a/packages/@n8n/db/src/migrations/postgresdb/index.ts b/packages/@n8n/db/src/migrations/postgresdb/index.ts index 90a69d8a4c2..476402e79bf 100644 --- a/packages/@n8n/db/src/migrations/postgresdb/index.ts +++ b/packages/@n8n/db/src/migrations/postgresdb/index.ts @@ -113,6 +113,7 @@ import { AddAttachmentsToChatHubMessages1761773155024 } from '../common/17617731 import { AddToolsColumnToChatHubTables1761830340990 } from '../common/1761830340990-AddToolsColumnToChatHubTables'; import { AddWorkflowDescriptionColumn1762177736257 } from '../common/1762177736257-AddWorkflowDescriptionColumn'; import { BackfillMissingWorkflowHistoryRecords1762763704614 } from '../common/1762763704614-BackfillMissingWorkflowHistoryRecords'; +import { AddIsGlobalColumnToCredentialsTable1762771954619 } from '../common/1762771954619-IsGlobalGlobalColumnToCredentialsTable'; import { AddWorkflowHistoryAutoSaveFields1762847206508 } from '../common/1762847206508-AddWorkflowHistoryAutoSaveFields'; import { AddActiveVersionIdColumn1763047800000 } from '../common/1763047800000-AddActiveVersionIdColumn'; import { ChangeOAuthStateColumnToUnboundedVarchar1763572724000 } from '../common/1763572724000-ChangeOAuthStateColumnToUnboundedVarchar'; @@ -232,6 +233,7 @@ export const postgresMigrations: Migration[] = [ CreateOAuthEntities1760116750277, BackfillMissingWorkflowHistoryRecords1762763704614, ChangeDefaultForIdInUserTable1762771264000, + AddIsGlobalColumnToCredentialsTable1762771954619, AddWorkflowHistoryAutoSaveFields1762847206508, AddToolsColumnToChatHubTables1761830340990, ChangeOAuthStateColumnToUnboundedVarchar1763572724000, diff --git a/packages/@n8n/db/src/migrations/sqlite/index.ts b/packages/@n8n/db/src/migrations/sqlite/index.ts index 27fde9d26d0..aff67ae6b44 100644 --- a/packages/@n8n/db/src/migrations/sqlite/index.ts +++ b/packages/@n8n/db/src/migrations/sqlite/index.ts @@ -109,6 +109,7 @@ import { AddAttachmentsToChatHubMessages1761773155024 } from '../common/17617731 import { AddToolsColumnToChatHubTables1761830340990 } from '../common/1761830340990-AddToolsColumnToChatHubTables'; import { AddWorkflowDescriptionColumn1762177736257 } from '../common/1762177736257-AddWorkflowDescriptionColumn'; import { BackfillMissingWorkflowHistoryRecords1762763704614 } from '../common/1762763704614-BackfillMissingWorkflowHistoryRecords'; +import { AddIsGlobalColumnToCredentialsTable1762771954619 } from '../common/1762771954619-IsGlobalGlobalColumnToCredentialsTable'; import { AddWorkflowHistoryAutoSaveFields1762847206508 } from '../common/1762847206508-AddWorkflowHistoryAutoSaveFields'; import { AddActiveVersionIdColumn1763047800000 } from '../common/1763047800000-AddActiveVersionIdColumn'; import { ChangeOAuthStateColumnToUnboundedVarchar1763572724000 } from '../common/1763572724000-ChangeOAuthStateColumnToUnboundedVarchar'; @@ -224,6 +225,7 @@ const sqliteMigrations: Migration[] = [ AddWorkflowDescriptionColumn1762177736257, CreateOAuthEntities1760116750277, BackfillMissingWorkflowHistoryRecords1762763704614, + AddIsGlobalColumnToCredentialsTable1762771954619, AddWorkflowHistoryAutoSaveFields1762847206508, AddToolsColumnToChatHubTables1761830340990, ChangeOAuthStateColumnToUnboundedVarchar1763572724000, diff --git a/packages/@n8n/db/src/repositories/credentials.repository.ts b/packages/@n8n/db/src/repositories/credentials.repository.ts index 0f4db3ea2e3..613cdc24acc 100644 --- a/packages/@n8n/db/src/repositories/credentials.repository.ts +++ b/packages/@n8n/db/src/repositories/credentials.repository.ts @@ -38,7 +38,15 @@ export class CredentialsRepository extends Repository { type Select = Array; const defaultRelations = ['shared', 'shared.project', 'shared.project.projectRelations']; - const defaultSelect: Select = ['id', 'name', 'type', 'isManaged', 'createdAt', 'updatedAt']; + const defaultSelect: Select = [ + 'id', + 'name', + 'type', + 'isManaged', + 'createdAt', + 'updatedAt', + 'isGlobal', + ]; if (!listQueryOptions) return { select: defaultSelect, relations: defaultRelations }; @@ -133,6 +141,17 @@ export class CredentialsRepository extends Repository { return await this.find(findManyOptions); } + /** + * Find all global credentials + */ + async findAllGlobalCredentials(includeData = false): Promise { + const findManyOptions = this.toFindManyOptions({ includeData }); + + findManyOptions.where = { ...findManyOptions.where, isGlobal: true }; + + return await this.find(findManyOptions); + } + /** * Find all credentials that are owned by a personal project. */ diff --git a/packages/@n8n/permissions/src/__tests__/__snapshots__/scope-information.test.ts.snap b/packages/@n8n/permissions/src/__tests__/__snapshots__/scope-information.test.ts.snap index 31bfd80d0de..921bdcfe598 100644 --- a/packages/@n8n/permissions/src/__tests__/__snapshots__/scope-information.test.ts.snap +++ b/packages/@n8n/permissions/src/__tests__/__snapshots__/scope-information.test.ts.snap @@ -21,6 +21,7 @@ exports[`Scope Information ensure scopes are defined correctly 1`] = ` "communityPackage:manage", "communityPackage:*", "credential:share", + "credential:shareGlobally", "credential:move", "credential:create", "credential:read", diff --git a/packages/@n8n/permissions/src/constants.ee.ts b/packages/@n8n/permissions/src/constants.ee.ts index f9e83b1ff4d..1ef6aa52e48 100644 --- a/packages/@n8n/permissions/src/constants.ee.ts +++ b/packages/@n8n/permissions/src/constants.ee.ts @@ -6,7 +6,7 @@ export const RESOURCES = { banner: ['dismiss'] as const, community: ['register'] as const, communityPackage: ['install', 'uninstall', 'update', 'list', 'manage'] as const, - credential: ['share', 'move', ...DEFAULT_OPERATIONS] as const, + credential: ['share', 'shareGlobally', 'move', ...DEFAULT_OPERATIONS] as const, externalSecretsProvider: ['sync', ...DEFAULT_OPERATIONS] as const, externalSecret: ['list', 'use'] as const, eventBusDestination: ['test', ...DEFAULT_OPERATIONS] as const, diff --git a/packages/@n8n/permissions/src/roles/scopes/global-scopes.ee.ts b/packages/@n8n/permissions/src/roles/scopes/global-scopes.ee.ts index 2f3720f3040..b47c44aab70 100644 --- a/packages/@n8n/permissions/src/roles/scopes/global-scopes.ee.ts +++ b/packages/@n8n/permissions/src/roles/scopes/global-scopes.ee.ts @@ -14,6 +14,7 @@ export const GLOBAL_OWNER_SCOPES: Scope[] = [ 'credential:delete', 'credential:list', 'credential:share', + 'credential:shareGlobally', 'credential:move', 'community:register', 'communityPackage:install', diff --git a/packages/cli/build.log b/packages/cli/build.log new file mode 100644 index 00000000000..271c7bec031 --- /dev/null +++ b/packages/cli/build.log @@ -0,0 +1,4 @@ + +> n8n@1.121.0 build /Users/mutasem/repos/n8n-worktree/global-creds/packages/cli +> tsc -p tsconfig.build.json && tsc-alias -p tsconfig.build.json && pnpm run build:data + diff --git a/packages/cli/src/credentials/__tests__/credentials.controller.test.ts b/packages/cli/src/credentials/__tests__/credentials.controller.test.ts index 6ac0518ae8d..c327d23affc 100644 --- a/packages/cli/src/credentials/__tests__/credentials.controller.test.ts +++ b/packages/cli/src/credentials/__tests__/credentials.controller.test.ts @@ -1,4 +1,5 @@ -import type { AuthenticatedRequest, SharedCredentialsRepository } from '@n8n/db'; +import type { AuthenticatedRequest, SharedCredentialsRepository, CredentialsEntity } from '@n8n/db'; +import { GLOBAL_OWNER_ROLE, GLOBAL_MEMBER_ROLE } from '@n8n/db'; import { mock } from 'jest-mock-extended'; import { createRawProjectData } from '@/__tests__/project.test-data'; @@ -7,11 +8,14 @@ import type { EventService } from '@/events/event.service'; import { createdCredentialsWithScopes, createNewCredentialsPayload } from './credentials.test-data'; import { CredentialsController } from '../credentials.controller'; import type { CredentialsService } from '../credentials.service'; +import type { CredentialsFinderService } from '../credentials-finder.service'; +import type { CredentialRequest } from '@/requests'; describe('CredentialsController', () => { const eventService = mock(); const credentialsService = mock(); const sharedCredentialsRepository = mock(); + const credentialsFinderService = mock(); const credentialsController = new CredentialsController( mock(), @@ -24,7 +28,7 @@ describe('CredentialsController', () => { sharedCredentialsRepository, mock(), eventService, - mock(), + credentialsFinderService, ); let req: AuthenticatedRequest; @@ -33,6 +37,10 @@ describe('CredentialsController', () => { req = { user: { id: '123' } } as AuthenticatedRequest; }); + beforeEach(() => { + jest.resetAllMocks(); + }); + describe('createCredentials', () => { it('should create new credentials and emit "credentials-created"', async () => { // Arrange @@ -86,4 +94,191 @@ describe('CredentialsController', () => { expect(newApiKey).toEqual(createdCredentials); }); }); + + describe('updateCredentials', () => { + const credentialId = 'cred-123'; + const existingCredential = mock({ + id: credentialId, + name: 'Test Credential', + type: 'apiKey', + isGlobal: false, + isManaged: false, + }); + + beforeEach(() => { + credentialsService.decrypt.mockReturnValue({ apiKey: 'test-key' }); + credentialsService.prepareUpdateData.mockResolvedValue({ + name: 'Updated Credential', + type: 'apiKey', + data: { apiKey: 'updated-key' }, + } as any); + credentialsService.createEncryptedData.mockReturnValue({ + name: 'Updated Credential', + type: 'apiKey', + data: 'encrypted-data', + id: 'cred-123', + createdAt: new Date(), + updatedAt: new Date(), + } as any); + credentialsService.getCredentialScopes.mockResolvedValue([ + 'credential:read', + 'credential:update', + ] as any); + }); + + it('should allow owner to set isGlobal to true', async () => { + // ARRANGE + const ownerReq = { + user: { id: 'owner-id', role: GLOBAL_OWNER_ROLE }, + params: { credentialId }, + body: { + name: 'Updated Credential', + type: 'apiKey', + data: { apiKey: 'updated-key' }, + isGlobal: true, + }, + } as unknown as CredentialRequest.Update; + + credentialsFinderService.findCredentialForUser.mockResolvedValue(existingCredential); + credentialsService.update.mockResolvedValue({ + ...existingCredential, + name: 'Updated Credential', + isGlobal: true, + }); + + // ACT + await credentialsController.updateCredentials(ownerReq); + + // ASSERT + expect(credentialsService.update).toHaveBeenCalledWith( + credentialId, + expect.objectContaining({ + isGlobal: true, + }), + ); + expect(eventService.emit).toHaveBeenCalledWith('credentials-updated', { + user: ownerReq.user, + credentialType: existingCredential.type, + credentialId: existingCredential.id, + }); + }); + + it('should allow owner to set isGlobal to false', async () => { + // ARRANGE + const globalCredential = mock({ + ...existingCredential, + isGlobal: true, + }); + const ownerReq = { + user: { id: 'owner-id', role: GLOBAL_OWNER_ROLE }, + params: { credentialId }, + body: { + name: 'Updated Credential', + type: 'apiKey', + data: { apiKey: 'updated-key' }, + isGlobal: false, + }, + } as unknown as CredentialRequest.Update; + + credentialsFinderService.findCredentialForUser.mockResolvedValue(globalCredential); + credentialsService.update.mockResolvedValue({ + ...globalCredential, + isGlobal: false, + }); + + // ACT + await credentialsController.updateCredentials(ownerReq); + + // ASSERT + expect(credentialsService.update).toHaveBeenCalledWith( + credentialId, + expect.objectContaining({ + isGlobal: false, + }), + ); + }); + + it('should prevent non-owner from changing isGlobal', async () => { + // ARRANGE + const memberReq = { + user: { id: 'member-id', role: GLOBAL_MEMBER_ROLE }, + params: { credentialId }, + body: { + name: 'Updated Credential', + type: 'apiKey', + data: { apiKey: 'updated-key' }, + isGlobal: true, + }, + } as unknown as CredentialRequest.Update; + + credentialsFinderService.findCredentialForUser.mockResolvedValue(existingCredential); + + // ACT + await expect(credentialsController.updateCredentials(memberReq)).rejects.toThrowError( + 'You do not have permission to change global sharing for credentials', + ); + + // ASSERT + expect(credentialsService.update).not.toHaveBeenCalled(); + }); + + it('should prevent non-owner from changing isGlobal to true', async () => { + // ARRANGE + const memberReq = { + user: { id: 'member-id', role: GLOBAL_MEMBER_ROLE }, + params: { credentialId }, + body: { + name: 'Updated Credential', + type: 'apiKey', + data: { apiKey: 'updated-key' }, + isGlobal: false, + }, + } as unknown as CredentialRequest.Update; + + credentialsFinderService.findCredentialForUser.mockResolvedValue({ + ...existingCredential, + isGlobal: true, + }); + + // ACT + await expect(credentialsController.updateCredentials(memberReq)).rejects.toThrowError( + 'You do not have permission to change global sharing for credentials', + ); + + // ASSERT + expect(credentialsService.update).not.toHaveBeenCalled(); + }); + + it('should update credential without changing isGlobal when not provided', async () => { + // ARRANGE + const ownerReq = { + user: { id: 'owner-id', role: GLOBAL_OWNER_ROLE }, + params: { credentialId }, + body: { + name: 'Updated Credential', + type: 'apiKey', + data: { apiKey: 'updated-key' }, + // isGlobal not provided + }, + } as unknown as CredentialRequest.Update; + + credentialsFinderService.findCredentialForUser.mockResolvedValue(existingCredential); + credentialsService.update.mockResolvedValue({ + ...existingCredential, + name: 'Updated Credential', + }); + + // ACT + await credentialsController.updateCredentials(ownerReq); + + // ASSERT + // Should not include isGlobal in update when not provided + expect(credentialsService.update).toHaveBeenCalledWith( + credentialId, + expect.not.objectContaining({ + isGlobal: expect.anything(), + }), + ); + }); + }); }); diff --git a/packages/cli/src/credentials/__tests__/credentials.service.test.ts b/packages/cli/src/credentials/__tests__/credentials.service.test.ts index 04fd20c26f5..5aa3ab46686 100644 --- a/packages/cli/src/credentials/__tests__/credentials.service.test.ts +++ b/packages/cli/src/credentials/__tests__/credentials.service.test.ts @@ -1,11 +1,26 @@ -import type { CredentialsEntity, CredentialsRepository } from '@n8n/db'; +import type { + CredentialsEntity, + CredentialsRepository, + SharedCredentialsRepository, + ProjectRepository, + UserRepository, + User, +} from '@n8n/db'; +import { GLOBAL_OWNER_ROLE, GLOBAL_MEMBER_ROLE } from '@n8n/db'; import { mock } from 'jest-mock-extended'; import { CREDENTIAL_ERRORS, CredentialDataError, Credentials, type ErrorReporter } from 'n8n-core'; import { CREDENTIAL_EMPTY_VALUE, type ICredentialType } from 'n8n-workflow'; +import type { Logger } from '@n8n/backend-common'; import { CREDENTIAL_BLANKING_VALUE } from '@/constants'; import type { CredentialTypes } from '@/credential-types'; import { CredentialsService } from '@/credentials/credentials.service'; +import type { CredentialsFinderService } from '@/credentials/credentials-finder.service'; +import type { OwnershipService } from '@/services/ownership.service'; +import type { ProjectService } from '@/services/project.service.ee'; +import type { RoleService } from '@/services/role.service'; +import type { CredentialsTester } from '@/services/credentials-tester.service'; +import type { ExternalHooks } from '@/external-hooks'; describe('CredentialsService', () => { const credType = mock({ @@ -29,20 +44,31 @@ describe('CredentialsService', () => { const errorReporter = mock(); const credentialTypes = mock(); const credentialsRepository = mock(); + const sharedCredentialsRepository = mock(); + const ownershipService = mock(); + const logger = mock(); + const credentialsTester = mock(); + const externalHooks = mock(); + const projectRepository = mock(); + const projectService = mock(); + const roleService = mock(); + const userRepository = mock(); + const credentialsFinderService = mock(); + const service = new CredentialsService( credentialsRepository, - mock(), - mock(), - mock(), + sharedCredentialsRepository, + ownershipService, + logger, errorReporter, - mock(), - mock(), + credentialsTester, + externalHooks, credentialTypes, - mock(), - mock(), - mock(), - mock(), - mock(), + projectRepository, + projectService, + roleService, + userRepository, + credentialsFinderService, ); beforeEach(() => jest.resetAllMocks()); @@ -421,4 +447,1033 @@ describe('CredentialsService', () => { }); }); }); + + describe('getMany', () => { + const ownerUser = mock({ id: 'owner-id', role: GLOBAL_OWNER_ROLE }); + const memberUser = mock({ id: 'member-id', role: GLOBAL_MEMBER_ROLE }); + + const regularCredential = { + id: 'cred-1', + name: 'Regular Credential', + type: 'apiKey', + isGlobal: false, + shared: [], + } as Partial as CredentialsEntity; + + const globalCredential = { + id: 'cred-2', + name: 'Global Credential', + type: 'apiKey', + isGlobal: true, + shared: [], + } as Partial as CredentialsEntity; + + beforeEach(() => { + // Mock ownershipService to return credentials as-is + ownershipService.addOwnedByAndSharedWith.mockImplementation((c: any) => c); + }); + + describe('returnAll = true (owner user)', () => { + describe('with personal project filter', () => { + 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]); + projectService.getProject.mockResolvedValue(personalProject); + + // ACT + const result = await service.getMany(ownerUser, { + listQueryOptions: { + filter: { projectId: 'personal-proj' }, + }, + }); + + // ASSERT + expect(projectService.getProject).toHaveBeenCalledWith('personal-proj'); + expect(credentialsRepository.findMany).toHaveBeenCalledWith( + expect.objectContaining({ + filter: expect.objectContaining({ + withRole: 'credential:owner', + projectId: 'personal-proj', + }), + }), + ); + expect(result).toHaveLength(1); + }); + + 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]); + projectService.getProject.mockResolvedValue(teamProject); + + // ACT + await service.getMany(ownerUser, { + listQueryOptions: { + filter: { projectId: 'team-proj' }, + }, + }); + + // ASSERT + expect(projectService.getProject).toHaveBeenCalledWith('team-proj'); + expect(credentialsRepository.findMany).toHaveBeenCalledWith( + expect.objectContaining({ + filter: expect.not.objectContaining({ + withRole: 'credential:owner', + }), + }), + ); + }); + + it('should handle getProject throwing an error', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + projectService.getProject.mockRejectedValue(new Error('Project not found')); + + // ACT + await service.getMany(ownerUser, { + listQueryOptions: { + filter: { projectId: 'nonexistent-proj' }, + }, + }); + + // ASSERT - Should continue without filtering by role + expect(credentialsRepository.findMany).toHaveBeenCalledWith( + expect.objectContaining({ + filter: expect.not.objectContaining({ + withRole: 'credential:owner', + }), + }), + ); + }); + }); + + describe('with includeScopes', () => { + 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]); + projectService.getProjectRelationsForUser.mockResolvedValue(projectRelations); + roleService.addScopes.mockImplementation( + (c) => ({ ...c, scopes: ['credential:read', 'credential:update'] }) as any, + ); + + // ACT + const result = await service.getMany(ownerUser, { + includeScopes: true, + }); + + // ASSERT + expect(projectService.getProjectRelationsForUser).toHaveBeenCalledWith(ownerUser); + expect(roleService.addScopes).toHaveBeenCalledWith( + regularCredential, + ownerUser, + projectRelations, + ); + expect(result[0]).toHaveProperty('scopes'); + }); + + it('should not add scopes when includeScopes is false', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + + // ACT + const result = await service.getMany(ownerUser, { + includeScopes: false, + }); + + // ASSERT + expect(projectService.getProjectRelationsForUser).not.toHaveBeenCalled(); + expect(roleService.addScopes).not.toHaveBeenCalled(); + expect(result[0]).not.toHaveProperty('scopes'); + }); + }); + + describe('with includeData', () => { + beforeEach(() => { + projectService.getProjectRelationsForUser.mockResolvedValue([]); + jest.spyOn(Credentials.prototype, 'getData').mockReturnValue({ + apiKey: 'secret-key', + oauthTokenData: { token: 'secret-token' }, + }); + credentialTypes.getByName.mockReturnValue(credType); + }); + + it('should automatically set includeScopes to true when includeData is true', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + roleService.addScopes.mockImplementation( + (c) => ({ ...c, scopes: ['credential:update'] }) as any, + ); + + // ACT + await service.getMany(ownerUser, { + includeData: true, + }); + + // ASSERT + expect(projectService.getProjectRelationsForUser).toHaveBeenCalled(); + }); + + it('should include decrypted data when user has credential:update scope', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + roleService.addScopes.mockImplementation( + (c) => ({ ...c, scopes: ['credential:update'] }) as any, + ); + + // ACT + const result = await service.getMany(ownerUser, { + includeData: true, + }); + + // ASSERT + expect(result[0]).toHaveProperty('data'); + expect(result[0].data).toBeDefined(); + }); + + it('should not include decrypted data when user lacks credential:update scope', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + roleService.addScopes.mockImplementation( + (c) => ({ ...c, scopes: ['credential:read'] }) as any, + ); + + // ACT + const result = await service.getMany(ownerUser, { + includeData: true, + }); + + // ASSERT + expect(result[0]).toHaveProperty('data'); + expect(result[0].data).toBeUndefined(); + }); + + it('should replace oauthTokenData with true when present', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + roleService.addScopes.mockImplementation( + (c) => ({ ...c, scopes: ['credential:update'] }) as any, + ); + jest.spyOn(Credentials.prototype, 'getData').mockReturnValue({ + apiKey: 'secret-key', + oauthTokenData: { token: 'secret-token' }, + }); + + // ACT + const result = await service.getMany(ownerUser, { + includeData: true, + }); + + // ASSERT + expect(result[0].data).toBeDefined(); + expect((result[0].data as any)?.oauthTokenData).toBe(true); + }); + + it('should set includeData in listQueryOptions', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + roleService.addScopes.mockImplementation( + (c) => ({ ...c, scopes: ['credential:update'] }) as any, + ); + + // ACT + await service.getMany(ownerUser, { + includeData: true, + }); + + // ASSERT + expect(credentialsRepository.findMany).toHaveBeenCalledWith( + expect.objectContaining({ + includeData: true, + }), + ); + }); + }); + + describe('with shared project relations', () => { + const sharedRelation = { credentialsId: 'cred-1', projectId: 'proj-1' } as any; + + it('should fetch all relations when filtering by shared.projectId', async () => { + // ARRANGE + const credWithShared = { ...regularCredential, shared: [] } as any; + credentialsRepository.findMany.mockResolvedValue([credWithShared]); + sharedCredentialsRepository.getAllRelationsForCredentials.mockResolvedValue([ + sharedRelation, + ]); + + // ACT + const result = await service.getMany(ownerUser, { + listQueryOptions: { + filter: { shared: { projectId: 'proj-1' } }, + }, + }); + + // ASSERT + expect(sharedCredentialsRepository.getAllRelationsForCredentials).toHaveBeenCalledWith([ + 'cred-1', + ]); + expect(result[0].shared).toHaveLength(1); + }); + + it('should fetch all relations when onlySharedWithMe is true', async () => { + // ARRANGE + const credWithShared = { ...regularCredential, shared: [] } as any; + credentialsRepository.findMany.mockResolvedValue([credWithShared]); + sharedCredentialsRepository.getAllRelationsForCredentials.mockResolvedValue([ + sharedRelation, + ]); + + // ACT + const result = await service.getMany(ownerUser, { + onlySharedWithMe: true, + }); + + // ASSERT + expect(sharedCredentialsRepository.getAllRelationsForCredentials).toHaveBeenCalledWith([ + 'cred-1', + ]); + expect(result[0].shared).toHaveLength(1); + }); + + it('should not fetch all relations when shared.projectId is not present', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + + // ACT + await service.getMany(ownerUser, { + listQueryOptions: { + filter: { type: 'apiKey' }, + }, + }); + + // ASSERT + expect(sharedCredentialsRepository.getAllRelationsForCredentials).not.toHaveBeenCalled(); + }); + }); + + describe('with custom select (non-default select)', () => { + it('should skip addOwnedByAndSharedWith when select is custom', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + + // ACT + await service.getMany(ownerUser, { + listQueryOptions: { + select: { id: true, name: true }, + }, + }); + + // ASSERT + expect(ownershipService.addOwnedByAndSharedWith).not.toHaveBeenCalled(); + }); + + it('should skip fetching all relations when select is custom', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + + // ACT + await service.getMany(ownerUser, { + listQueryOptions: { + select: { id: true, name: true }, + filter: { shared: { projectId: 'proj-1' } }, + }, + }); + + // ASSERT + expect(sharedCredentialsRepository.getAllRelationsForCredentials).not.toHaveBeenCalled(); + }); + }); + }); + + describe('returnAll = false (member user)', () => { + beforeEach(() => { + credentialsFinderService.getCredentialIdsByUserAndRole.mockResolvedValue(['cred-1']); + }); + + it('should fetch credentials by user and role', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + + // ACT + await service.getMany(memberUser, {}); + + // ASSERT + expect(credentialsFinderService.getCredentialIdsByUserAndRole).toHaveBeenCalledWith( + [memberUser.id], + { scopes: ['credential:read'] }, + ); + expect(credentialsRepository.findMany).toHaveBeenCalledWith({}, ['cred-1']); + }); + + describe('with includeScopes', () => { + it('should add scopes to credentials when includeScopes is true', async () => { + // ARRANGE + const projectRelations = [{ projectId: 'proj-1', role: 'project:editor' }] as any; + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + projectService.getProjectRelationsForUser.mockResolvedValue(projectRelations); + roleService.addScopes.mockImplementation( + (c) => ({ ...c, scopes: ['credential:read'] }) as any, + ); + + // ACT + const result = await service.getMany(memberUser, { + includeScopes: true, + }); + + // ASSERT + expect(projectService.getProjectRelationsForUser).toHaveBeenCalledWith(memberUser); + expect(roleService.addScopes).toHaveBeenCalledWith( + regularCredential, + memberUser, + projectRelations, + ); + expect(result[0]).toHaveProperty('scopes'); + }); + }); + + describe('with includeData', () => { + beforeEach(() => { + projectService.getProjectRelationsForUser.mockResolvedValue([]); + jest.spyOn(Credentials.prototype, 'getData').mockReturnValue({ + apiKey: 'secret-key', + }); + credentialTypes.getByName.mockReturnValue(credType); + }); + + it('should include decrypted data when user has credential:update scope', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + roleService.addScopes.mockImplementation( + (c) => ({ ...c, scopes: ['credential:update'] }) as any, + ); + + // ACT + const result = await service.getMany(memberUser, { + includeData: true, + }); + + // ASSERT + expect(result[0]).toHaveProperty('data'); + expect(result[0].data).toBeDefined(); + }); + + it('should not include decrypted data when user lacks credential:update scope', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + roleService.addScopes.mockImplementation( + (c) => ({ ...c, scopes: ['credential:read'] }) as any, + ); + + // ACT + const result = await service.getMany(memberUser, { + includeData: true, + }); + + // ASSERT + expect(result[0]).toHaveProperty('data'); + expect(result[0].data).toBeUndefined(); + }); + }); + + describe('with shared project relations', () => { + const sharedRelation = { credentialsId: 'cred-1', projectId: 'proj-1' } as any; + + it('should fetch all relations when filtering by shared.projectId', async () => { + // ARRANGE + const credWithShared = { ...regularCredential, shared: [] } as any; + credentialsRepository.findMany.mockResolvedValue([credWithShared]); + sharedCredentialsRepository.getAllRelationsForCredentials.mockResolvedValue([ + sharedRelation, + ]); + + // ACT + const result = await service.getMany(memberUser, { + listQueryOptions: { + filter: { shared: { projectId: 'proj-1' } }, + }, + }); + + // ASSERT + expect(sharedCredentialsRepository.getAllRelationsForCredentials).toHaveBeenCalledWith([ + 'cred-1', + ]); + expect(result[0].shared).toHaveLength(1); + }); + + it('should fetch all relations when onlySharedWithMe is true', async () => { + // ARRANGE + const credWithShared = { ...regularCredential, shared: [] } as any; + credentialsRepository.findMany.mockResolvedValue([credWithShared]); + sharedCredentialsRepository.getAllRelationsForCredentials.mockResolvedValue([ + sharedRelation, + ]); + + // ACT + const result = await service.getMany(memberUser, { + onlySharedWithMe: true, + }); + + // ASSERT + expect(sharedCredentialsRepository.getAllRelationsForCredentials).toHaveBeenCalledWith([ + 'cred-1', + ]); + expect(result[0].shared).toHaveLength(1); + }); + }); + + describe('with custom select', () => { + it('should skip addOwnedByAndSharedWith when select is custom', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + + // ACT + await service.getMany(memberUser, { + listQueryOptions: { + select: { id: true, name: true }, + }, + }); + + // ASSERT + expect(ownershipService.addOwnedByAndSharedWith).not.toHaveBeenCalled(); + }); + }); + }); + + describe('with onlySharedWithMe', () => { + it('should filter by credential:user role when onlySharedWithMe is true', async () => { + // ARRANGE + const credWithShared = { ...regularCredential, shared: [] } as any; + const sharedRelation = { credentialsId: 'cred-1', projectId: 'proj-1' } as any; + credentialsFinderService.getCredentialIdsByUserAndRole.mockResolvedValue(['cred-1']); + credentialsRepository.findMany.mockResolvedValue([credWithShared]); + sharedCredentialsRepository.getAllRelationsForCredentials.mockResolvedValue([ + sharedRelation, + ]); + + // ACT + await service.getMany(memberUser, { + onlySharedWithMe: true, + }); + + // ASSERT + expect(credentialsRepository.findMany).toHaveBeenCalledWith( + expect.objectContaining({ + filter: expect.objectContaining({ + withRole: 'credential:user', + user: memberUser, + }), + }), + ['cred-1'], + ); + expect(sharedCredentialsRepository.getAllRelationsForCredentials).toHaveBeenCalledWith([ + 'cred-1', + ]); + }); + + it('should merge onlySharedWithMe with existing filters', async () => { + // ARRANGE + const credWithShared = { ...regularCredential, shared: [] } as any; + const sharedRelation = { credentialsId: 'cred-1', projectId: 'proj-1' } as any; + credentialsFinderService.getCredentialIdsByUserAndRole.mockResolvedValue(['cred-1']); + credentialsRepository.findMany.mockResolvedValue([credWithShared]); + sharedCredentialsRepository.getAllRelationsForCredentials.mockResolvedValue([ + sharedRelation, + ]); + + // ACT + await service.getMany(memberUser, { + listQueryOptions: { + filter: { type: 'apiKey' }, + }, + onlySharedWithMe: true, + }); + + // ASSERT + expect(credentialsRepository.findMany).toHaveBeenCalledWith( + expect.objectContaining({ + filter: expect.objectContaining({ + type: 'apiKey', + withRole: 'credential:user', + user: memberUser, + }), + }), + ['cred-1'], + ); + }); + }); + + describe('with includeGlobal = true', () => { + it('should include global credentials for owner users', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findAllGlobalCredentials.mockResolvedValue([globalCredential]); + + // ACT + const result = await service.getMany(ownerUser, { + includeGlobal: true, + }); + + // ASSERT + expect(credentialsRepository.findMany).toHaveBeenCalled(); + expect(credentialsRepository.findAllGlobalCredentials).toHaveBeenCalledWith(false); + expect(result).toHaveLength(2); + expect(result).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: 'cred-1' }), + expect.objectContaining({ id: 'cred-2' }), + ]), + ); + }); + + it('should include global credentials for member users', async () => { + // ARRANGE + credentialsFinderService.getCredentialIdsByUserAndRole.mockResolvedValue(['cred-1']); + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findAllGlobalCredentials.mockResolvedValue([globalCredential]); + + // ACT + const result = await service.getMany(memberUser, { + includeGlobal: true, + }); + + // ASSERT + expect(credentialsRepository.findMany).toHaveBeenCalled(); + expect(credentialsRepository.findAllGlobalCredentials).toHaveBeenCalledWith(false); + expect(result).toHaveLength(2); + expect(result).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: 'cred-1' }), + expect.objectContaining({ id: 'cred-2' }), + ]), + ); + }); + + it('should deduplicate credentials when user has project access to global credential', async () => { + // ARRANGE - User already has access to global credential through project + const sharedGlobalCred = { + id: 'cred-2', // Same ID as globalCredential + name: 'Global Credential', + type: 'apiKey', + isGlobal: true, + shared: [], + } as Partial as CredentialsEntity; + credentialsFinderService.getCredentialIdsByUserAndRole.mockResolvedValue([ + 'cred-1', + 'cred-2', + ]); + credentialsRepository.findMany.mockResolvedValue([regularCredential, sharedGlobalCred]); + credentialsRepository.findAllGlobalCredentials.mockResolvedValue([globalCredential]); + + // ACT + const result = await service.getMany(memberUser, { + includeGlobal: true, + }); + + // ASSERT + expect(result).toHaveLength(2); + // Should have exactly one instance of cred-2, not two + const credIds = result.map((c) => c.id); + expect(credIds.filter((id) => id === 'cred-2')).toHaveLength(1); + }); + + it('should include data for global credentials when includeData is true', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + credentialsRepository.findAllGlobalCredentials.mockResolvedValue([globalCredential]); + projectService.getProjectRelationsForUser.mockResolvedValue([]); + roleService.addScopes.mockImplementation( + (c) => ({ ...c, scopes: ['credential:read'] }) as any, + ); + + // ACT + await service.getMany(memberUser, { + includeGlobal: true, + includeData: true, + }); + + // ASSERT + expect(credentialsRepository.findAllGlobalCredentials).toHaveBeenCalledWith(true); + }); + }); + + describe('with includeGlobal = false', () => { + it('should exclude global credentials when includeGlobal is false', async () => { + // ARRANGE + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + + // ACT + const result = await service.getMany(ownerUser, { + includeGlobal: false, + }); + + // ASSERT + expect(credentialsRepository.findMany).toHaveBeenCalled(); + expect(credentialsRepository.findAllGlobalCredentials).not.toHaveBeenCalled(); + expect(result).toHaveLength(1); + expect(result[0].id).toBe('cred-1'); + }); + + it('should exclude global credentials when includeGlobal is undefined', async () => { + // ARRANGE + credentialsFinderService.getCredentialIdsByUserAndRole.mockResolvedValue(['cred-1']); + credentialsRepository.findMany.mockResolvedValue([regularCredential]); + + // ACT + const result = await service.getMany(memberUser, { + includeGlobal: false, + }); + + // ASSERT + expect(credentialsRepository.findAllGlobalCredentials).not.toHaveBeenCalled(); + expect(result).toHaveLength(1); + }); + }); + }); + + describe('getCredentialsAUserCanUseInAWorkflow', () => { + const user = mock({ id: 'user-1' }); + const regularCredential = { + id: 'cred-1', + name: 'Regular Credential', + type: 'apiKey', + isGlobal: false, + } as Partial as CredentialsEntity; + const globalCredential = { + id: 'cred-2', + name: 'Global Credential', + type: 'oauth2', + isGlobal: true, + } as Partial as CredentialsEntity; + + beforeEach(() => { + projectService.getProjectRelationsForUser.mockResolvedValue([]); + roleService.addScopes.mockImplementation( + (c) => ({ ...c, scopes: ['credential:read'] }) as any, + ); + }); + + it('should include global credentials for workflows', async () => { + // ARRANGE + credentialsFinderService.findCredentialsForUser.mockResolvedValue([ + regularCredential, + globalCredential, + ]); + credentialsRepository.findAllCredentialsForWorkflow.mockResolvedValue([regularCredential]); + + // ACT + const result = await service.getCredentialsAUserCanUseInAWorkflow(user, { + workflowId: 'workflow-1', + }); + + // ASSERT + expect(credentialsFinderService.findCredentialsForUser).toHaveBeenCalledWith(user, [ + 'credential:read', + ]); + expect(result).toHaveLength(2); + expect(result).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: 'cred-1', isGlobal: false }), + expect.objectContaining({ id: 'cred-2', isGlobal: true }), + ]), + ); + }); + + it('should include global credentials for projects', async () => { + // ARRANGE + credentialsFinderService.findCredentialsForUser.mockResolvedValue([ + regularCredential, + globalCredential, + ]); + credentialsRepository.findAllCredentialsForProject.mockResolvedValue([regularCredential]); + + // ACT + const result = await service.getCredentialsAUserCanUseInAWorkflow(user, { + projectId: 'project-1', + }); + + // ASSERT + expect(credentialsFinderService.findCredentialsForUser).toHaveBeenCalledWith(user, [ + 'credential:read', + ]); + expect(result).toHaveLength(2); + expect(result).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: 'cred-1' }), + expect.objectContaining({ id: 'cred-2' }), + ]), + ); + }); + + it('should not duplicate credentials when user has access to global credential through project', async () => { + // ARRANGE - Both regular and global credentials are in workflow's project + credentialsFinderService.findCredentialsForUser.mockResolvedValue([ + regularCredential, + globalCredential, + ]); + credentialsRepository.findAllCredentialsForWorkflow.mockResolvedValue([ + regularCredential, + globalCredential, + ]); + + // ACT + const result = await service.getCredentialsAUserCanUseInAWorkflow(user, { + workflowId: 'workflow-1', + }); + + // ASSERT + expect(result).toHaveLength(2); + const credIds = result.map((c) => c.id); + expect(credIds).toEqual(['cred-1', 'cred-2']); + // Verify no duplicates + expect(new Set(credIds).size).toBe(2); + }); + }); + + describe('findAllCredentialIdsForWorkflow', () => { + const workflowId = 'workflow-1'; + const ownerUser = mock({ id: 'owner-id', role: GLOBAL_OWNER_ROLE }); + const memberUser = mock({ id: 'member-id', role: GLOBAL_MEMBER_ROLE }); + + it('should return all personal credentials when owner has global read permissions', async () => { + // ARRANGE + const personalCred1 = mock({ id: 'cred-1' }); + const personalCred2 = mock({ id: 'cred-2' }); + userRepository.findPersonalOwnerForWorkflow.mockResolvedValue(ownerUser); + credentialsRepository.findAllPersonalCredentials.mockResolvedValue([ + personalCred1, + personalCred2, + ]); + + // ACT + const credentials = await service.findAllCredentialIdsForWorkflow(workflowId); + + // ASSERT + expect(userRepository.findPersonalOwnerForWorkflow).toHaveBeenCalledWith(workflowId); + expect(credentialsRepository.findAllPersonalCredentials).toHaveBeenCalled(); + expect(credentials).toHaveLength(2); + expect(credentials).toEqual([personalCred1, personalCred2]); + }); + + it('should return workflow credentials when owner lacks global read permissions', async () => { + // ARRANGE + const workflowCred1 = mock({ id: 'cred-1' }); + const workflowCred2 = mock({ id: 'cred-2' }); + userRepository.findPersonalOwnerForWorkflow.mockResolvedValue(memberUser); + credentialsRepository.findAllCredentialsForWorkflow.mockResolvedValue([ + workflowCred1, + workflowCred2, + ]); + + // ACT + const credentials = await service.findAllCredentialIdsForWorkflow(workflowId); + + // ASSERT + expect(userRepository.findPersonalOwnerForWorkflow).toHaveBeenCalledWith(workflowId); + expect(credentialsRepository.findAllCredentialsForWorkflow).toHaveBeenCalledWith(workflowId); + expect(credentials).toHaveLength(2); + expect(credentials).toEqual([workflowCred1, workflowCred2]); + }); + + it('should return workflow credentials when user is not found', async () => { + // ARRANGE + const workflowCred = mock({ id: 'cred-1' }); + userRepository.findPersonalOwnerForWorkflow.mockResolvedValue(null); + credentialsRepository.findAllCredentialsForWorkflow.mockResolvedValue([workflowCred]); + + // ACT + const credentials = await service.findAllCredentialIdsForWorkflow(workflowId); + + // ASSERT + expect(credentialsRepository.findAllCredentialsForWorkflow).toHaveBeenCalledWith(workflowId); + expect(credentials).toHaveLength(1); + }); + }); + + describe('findAllCredentialIdsForProject', () => { + const projectId = 'project-1'; + const ownerUser = mock({ id: 'owner-id', role: GLOBAL_OWNER_ROLE }); + const memberUser = mock({ id: 'member-id', role: GLOBAL_MEMBER_ROLE }); + + it('should return all personal credentials when project owner has global read permissions', async () => { + // ARRANGE + const personalCred1 = mock({ id: 'cred-1' }); + const personalCred2 = mock({ id: 'cred-2' }); + userRepository.findPersonalOwnerForProject.mockResolvedValue(ownerUser); + credentialsRepository.findAllPersonalCredentials.mockResolvedValue([ + personalCred1, + personalCred2, + ]); + + // ACT + const credentials = await service.findAllCredentialIdsForProject(projectId); + + // ASSERT + expect(userRepository.findPersonalOwnerForProject).toHaveBeenCalledWith(projectId); + expect(credentialsRepository.findAllPersonalCredentials).toHaveBeenCalled(); + expect(credentials).toHaveLength(2); + expect(credentials).toEqual([personalCred1, personalCred2]); + }); + + it('should return project credentials when owner lacks global read permissions', async () => { + // ARRANGE + const projectCred1 = mock({ id: 'cred-1' }); + const projectCred2 = mock({ id: 'cred-2' }); + userRepository.findPersonalOwnerForProject.mockResolvedValue(memberUser); + credentialsRepository.findAllCredentialsForProject.mockResolvedValue([ + projectCred1, + projectCred2, + ]); + + // ACT + const credentials = await service.findAllCredentialIdsForProject(projectId); + + // ASSERT + expect(userRepository.findPersonalOwnerForProject).toHaveBeenCalledWith(projectId); + expect(credentialsRepository.findAllCredentialsForProject).toHaveBeenCalledWith(projectId); + expect(credentials).toHaveLength(2); + expect(credentials).toEqual([projectCred1, projectCred2]); + }); + + it('should return project credentials when user is not found', async () => { + // ARRANGE + const projectCred = mock({ id: 'cred-1' }); + userRepository.findPersonalOwnerForProject.mockResolvedValue(null); + credentialsRepository.findAllCredentialsForProject.mockResolvedValue([projectCred]); + + // ACT + const credentials = await service.findAllCredentialIdsForProject(projectId); + + // ASSERT + expect(credentialsRepository.findAllCredentialsForProject).toHaveBeenCalledWith(projectId); + expect(credentials).toHaveLength(1); + }); + }); + + describe('createUnmanagedCredential', () => { + const ownerUser = mock({ id: 'owner-id', role: GLOBAL_OWNER_ROLE }); + const memberUser = mock({ id: 'member-id', role: GLOBAL_MEMBER_ROLE }); + + const credentialData = { + name: 'Test Credential', + type: 'apiKey', + data: { apiKey: 'test-key' }, + projectId: 'project-1', + }; + + beforeEach(() => { + // Mock the save chain + credentialsRepository.create.mockImplementation((data) => ({ ...data }) as any); + roleService.addScopes.mockImplementation( + (c) => + ({ + ...c, + scopes: ['credential:read', 'credential:update'], + }) as any, + ); + roleService.combineResourceScopes.mockReturnValue([ + 'credential:read', + 'credential:update', + ] as any); + sharedCredentialsRepository.findOne.mockResolvedValue({ role: 'credential:owner' } as any); + sharedCredentialsRepository.create.mockImplementation((data) => data as any); + sharedCredentialsRepository.find.mockResolvedValue([]); + externalHooks.run.mockResolvedValue(); + projectService.getProjectWithScope.mockResolvedValue({ + id: 'project-1', + } as any); + projectService.getProjectRelationsForUser.mockResolvedValue([]); + }); + + it('should allow owner to create global credential', async () => { + // ARRANGE + const payload = { ...credentialData, isGlobal: true }; + const savedEntities: any[] = []; + let credentialEntityInput: any; + credentialsRepository.create.mockImplementation((data) => { + credentialEntityInput = data; + return data as any; + }); + // @ts-expect-error - Mocking manager for testing + credentialsRepository.manager = { + transaction: jest.fn().mockImplementation(async (callback) => { + const mockManager = { + save: jest.fn().mockImplementation(async (entity) => { + savedEntities.push(entity); + return { ...entity, id: 'new-cred-id' }; + }), + }; + return await callback(mockManager); + }), + }; + + // ACT + await service.createUnmanagedCredential(payload, ownerUser); + + // ASSERT + expect(credentialEntityInput.isGlobal).toBe(true); + // First entity saved should be the credential with isGlobal + expect(savedEntities[0]).toMatchObject({ + isGlobal: true, + }); + }); + + it('should throw error when non-owner tries to create global credential', async () => { + // ARRANGE + const payload = { ...credentialData, isGlobal: true }; + + // ACT & ASSERT + await expect(service.createUnmanagedCredential(payload, memberUser)).rejects.toThrow( + 'You do not have permission to create globally shared credentials', + ); + }); + + it('should create non-global credential by default', async () => { + // ARRANGE + const payload = { ...credentialData }; // no isGlobal field + let savedCredential: any; + // @ts-expect-error - Mocking manager for testing + credentialsRepository.manager = { + transaction: jest.fn().mockImplementation(async (callback) => { + const mockManager = { + save: jest.fn().mockImplementation(async (entity) => { + savedCredential = entity; + return { ...entity, id: 'new-cred-id' }; + }), + }; + return await callback(mockManager); + }), + }; + + // ACT + await service.createUnmanagedCredential(payload, ownerUser); + + // ASSERT + expect(savedCredential.isGlobal).toBeUndefined(); + }); + + it('should allow member to create non-global credential', async () => { + // ARRANGE + const payload = { ...credentialData, isGlobal: false }; + let savedCredential: any; + // @ts-expect-error - Mocking manager for testing + credentialsRepository.manager = { + transaction: jest.fn().mockImplementation(async (callback) => { + const mockManager = { + save: jest.fn().mockImplementation(async (entity) => { + savedCredential = entity; + return { ...entity, id: 'new-cred-id' }; + }), + }; + return await callback(mockManager); + }), + }; + + // ACT + await service.createUnmanagedCredential(payload, memberUser); + + // ASSERT + expect(savedCredential.isGlobal).toBeUndefined(); + }); + }); }); diff --git a/packages/cli/src/credentials/credentials-finder.service.ts b/packages/cli/src/credentials/credentials-finder.service.ts index f0ad42ad938..b2f522b0a2b 100644 --- a/packages/cli/src/credentials/credentials-finder.service.ts +++ b/packages/cli/src/credentials/credentials-finder.service.ts @@ -1,5 +1,5 @@ -import type { CredentialsEntity, SharedCredentials, User } from '@n8n/db'; -import { CredentialsRepository, SharedCredentialsRepository } from '@n8n/db'; +import type { SharedCredentials, User } from '@n8n/db'; +import { CredentialsEntity, CredentialsRepository, SharedCredentialsRepository } from '@n8n/db'; import { Service } from '@n8n/di'; import { hasGlobalScope } from '@n8n/permissions'; import type { CredentialSharingRole, ProjectRole, Scope } from '@n8n/permissions'; @@ -18,6 +18,59 @@ export class CredentialsFinderService { private readonly roleService: RoleService, ) {} + /** + * Fetches global credentials from the database. + */ + private async fetchGlobalCredentials(trx?: EntityManager): Promise { + const em = trx ?? this.credentialsRepository.manager; + return await em.find(CredentialsEntity, { + where: { isGlobal: true }, + relations: { shared: true }, + }); + } + + /** + * Checks if the scopes allow read-only access to global credentials. + * Global credentials can be accessed with credential:read scope only. + */ + hasGlobalReadOnlyAccess(scopes: Scope[]): boolean { + return scopes.length === 1 && scopes[0] === 'credential:read'; + } + + /** + * Finds a global credential by ID if it exists. + */ + async findGlobalCredentialById( + credentialId: string, + relations?: { shared: { project: { projectRelations: { user: boolean } } } }, + ): Promise { + return await this.credentialsRepository.findOne({ + where: { + id: credentialId, + isGlobal: true, + }, + relations, + }); + } + + /** + * Merges global credentials with the provided credentials list, + * deduplicating based on credential ID. + */ + private mergeAndDeduplicateCredentials( + credentials: T[], + globalCredentials: CredentialsEntity[], + mapGlobalCredential: (cred: CredentialsEntity) => T | null, + ): T[] { + const credentialIds = new Set(credentials.map((c) => c.id)); + const newGlobalCreds = globalCredentials + .filter((gc) => !credentialIds.has(gc.id)) + .map(mapGlobalCredential) + .filter((mapped): mapped is T => mapped !== null); + + return [...credentials, ...newGlobalCreds]; + } + /** * Find all credentials that the user has access to taking the scopes into * account. @@ -26,7 +79,7 @@ export class CredentialsFinderService { * all scopes the user has for the credential using `RoleService.addScopes`. **/ async findCredentialsForUser(user: User, scopes: Scope[]) { - let where: FindOptionsWhere = {}; + let where: FindOptionsWhere = { isGlobal: false }; if (!hasGlobalScope(user, scopes, { mode: 'allOf' })) { const [projectRoles, credentialRoles] = await Promise.all([ @@ -47,11 +100,26 @@ export class CredentialsFinderService { }; } - return await this.credentialsRepository.find({ where, relations: { shared: true } }); + const credentials = await this.credentialsRepository.find({ + where, + relations: { shared: true }, + }); + + // Include global credentials only if the user has read-only access + if (this.hasGlobalReadOnlyAccess(scopes)) { + const globalCredentials = await this.fetchGlobalCredentials(); + return [...credentials, ...globalCredentials]; + } + + return credentials; } /** Get a credential if it has been shared with a user */ - async findCredentialForUser(credentialsId: string, user: User, scopes: Scope[]) { + async findCredentialForUser( + credentialsId: string, + user: User, + scopes: Scope[], + ): Promise { let where: FindOptionsWhere = { credentialsId }; if (!hasGlobalScope(user, scopes, { mode: 'allOf' })) { @@ -80,12 +148,28 @@ export class CredentialsFinderService { }, }, }); - if (!sharedCredential) return null; - return sharedCredential.credentials; + + if (sharedCredential) { + return sharedCredential.credentials; + } + + // Check for global credentials with read-only access + if (this.hasGlobalReadOnlyAccess(scopes)) { + return await this.findGlobalCredentialById(credentialsId, { + shared: { project: { projectRelations: { user: true } } }, + }); + } + + return null; } /** Get all credentials shared to a user */ - async findAllCredentialsForUser(user: User, scopes: Scope[], trx?: EntityManager) { + async findAllCredentialsForUser( + user: User, + scopes: Scope[], + trx?: EntityManager, + options?: { includeGlobalCredentials?: boolean }, + ) { let where: FindOptionsWhere = {}; if (!hasGlobalScope(user, scopes, { mode: 'allOf' })) { @@ -109,7 +193,31 @@ export class CredentialsFinderService { trx, ); - return sharedCredential.map((sc) => ({ ...sc.credentials, projectId: sc.projectId })); + let sharedCredentialsList = sharedCredential.map((sc) => ({ + ...sc.credentials, + projectId: sc.projectId, + })); + + // Include global credentials if flag is set + if (options?.includeGlobalCredentials) { + const globalCredentials = await this.fetchGlobalCredentials(trx); + sharedCredentialsList = this.mergeAndDeduplicateCredentials( + sharedCredentialsList, + globalCredentials, + (globalCred) => { + // For global credentials, use the owner's project ID + const ownerSharing = globalCred.shared?.find((s) => s.role === 'credential:owner'); + const projectId = ownerSharing?.projectId; + if (projectId) { + return { ...globalCred, projectId }; + } + // Skip credentials without a valid project ID + return null; + }, + ); + } + + return sharedCredentialsList; } async getCredentialIdsByUserAndRole( diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 709a8d85fc0..965966262d9 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -25,7 +25,7 @@ import { Param, Query, } from '@n8n/decorators'; -import { PROJECT_OWNER_ROLE_SLUG } from '@n8n/permissions'; +import { hasGlobalScope, PROJECT_OWNER_ROLE_SLUG } from '@n8n/permissions'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In } from '@n8n/typeorm'; import { deepCopy } from 'n8n-workflow'; @@ -74,6 +74,7 @@ export class CredentialsController { includeScopes: query.includeScopes, includeData: query.includeData, onlySharedWithMe: query.onlySharedWithMe, + includeGlobal: query.includeGlobal, }); credentials.forEach((c) => { // @ts-expect-error: This is to emulate the old behavior of removing the shared @@ -242,6 +243,18 @@ export class CredentialsController { data: preparedCredentialData.data as unknown as ICredentialDataDecryptedObject, }); + // Update isGlobal if provided in the payload and user has permission + const isGlobal = body.isGlobal; + if (isGlobal !== undefined && isGlobal !== credential.isGlobal) { + const canShareGlobally = hasGlobalScope(req.user, 'credential:shareGlobally'); + if (!canShareGlobally) { + throw new ForbiddenError( + 'You do not have permission to change global sharing for credentials', + ); + } + newCredentialData.isGlobal = isGlobal; + } + const responseData = await this.credentialsService.update(credentialId, newCredentialData); if (responseData === null) { diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index f52f2f6349b..4be07531fad 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -50,6 +50,7 @@ import { CredentialsTester } from '@/services/credentials-tester.service'; import { OwnershipService } from '@/services/ownership.service'; import { ProjectService } from '@/services/project.service.ee'; import { RoleService } from '@/services/role.service'; +import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; export type CredentialsGetSharedOptions = | { allowGlobalScope: true; globalScope: Scope } @@ -77,6 +78,40 @@ export class CredentialsService { private readonly credentialsFinderService: CredentialsFinderService, ) {} + private async addGlobalCredentials( + credentials: CredentialsEntity[], + includeData: boolean, + ): Promise { + const globalCredentials = + await this.credentialsRepository.findAllGlobalCredentials(includeData); + + // Merge and deduplicate based on credential ID + const credentialIds = new Set(credentials.map((c) => c.id)); + const newGlobalCreds = globalCredentials.filter((gc) => !credentialIds.has(gc.id)); + + return [...credentials, ...newGlobalCreds]; + } + + async getMany( + user: User, + options: { + listQueryOptions?: ListQuery.Options & { includeData?: boolean }; + includeScopes?: boolean; + includeData: true; + onlySharedWithMe?: boolean; + includeGlobal?: boolean; + }, + ): Promise>>; + async getMany( + user: User, + options?: { + listQueryOptions?: ListQuery.Options & { includeData?: boolean }; + includeScopes?: boolean; + includeData?: boolean; + onlySharedWithMe?: boolean; + includeGlobal?: boolean; + }, + ): Promise; async getMany( user: User, { @@ -84,20 +119,55 @@ export class CredentialsService { includeScopes = false, includeData = false, onlySharedWithMe = false, + includeGlobal = false, }: { listQueryOptions?: ListQuery.Options & { includeData?: boolean }; includeScopes?: boolean; includeData?: boolean; onlySharedWithMe?: boolean; + includeGlobal?: boolean; } = {}, - ) { + ): Promise> | CredentialsEntity[]> { const returnAll = hasGlobalScope(user, 'credential:list'); const isDefaultSelect = !listQueryOptions.select; - const projectId = - typeof listQueryOptions.filter?.projectId === 'string' - ? listQueryOptions.filter.projectId - : undefined; + this.applyOnlySharedWithMeFilter(listQueryOptions, onlySharedWithMe, user); + + // Auto-enable includeScopes when includeData is requested + if (includeData) { + includeScopes = true; + listQueryOptions.includeData = true; + } + + let credentials: CredentialsEntity[]; + + if (returnAll) { + credentials = await this.getManyForAdminUser(listQueryOptions, includeGlobal, includeData); + } else { + credentials = await this.getManyForMemberUser( + user, + listQueryOptions, + includeGlobal, + includeData, + ); + } + + return await this.enrichCredentials( + credentials, + user, + isDefaultSelect, + includeScopes, + includeData, + listQueryOptions, + onlySharedWithMe, + ); + } + + private applyOnlySharedWithMeFilter( + listQueryOptions: ListQuery.Options & { includeData?: boolean }, + onlySharedWithMe: boolean, + user: User, + ): void { if (onlySharedWithMe) { listQueryOptions.filter = { ...listQueryOptions.filter, @@ -105,125 +175,174 @@ export class CredentialsService { user, }; } + } - if (includeData) { - // We need the scopes to check if we're allowed to include the decrypted - // data. - // Only if the user has the `credential:update` scope the user is allowed - // to get the data. - includeScopes = true; - listQueryOptions.includeData = true; + private async getManyForAdminUser( + listQueryOptions: ListQuery.Options & { includeData?: boolean }, + includeGlobal: boolean, + includeData: boolean, + ): Promise { + await this.applyPersonalProjectFilter(listQueryOptions); + + let credentials = await this.credentialsRepository.findMany(listQueryOptions); + + if (includeGlobal) { + credentials = await this.addGlobalCredentials(credentials, includeData); } - if (returnAll) { - let project: Project | undefined; - - if (projectId) { - try { - project = await this.projectService.getProject(projectId); - } catch {} - } - - if (project?.type === 'personal') { - listQueryOptions.filter = { - ...listQueryOptions.filter, - withRole: 'credential:owner', - }; - } - - let credentials = await this.credentialsRepository.findMany(listQueryOptions); - - if (isDefaultSelect) { - // Since we're filtering using project ID as part of the relation, - // we end up filtering out all the other relations, meaning that if - // it's shared to a project, it won't be able to find the home project. - // To solve this, we have to get all the relation now, even though - // we're deleting them later. - if ( - (listQueryOptions.filter?.shared as { projectId?: string })?.projectId ?? - onlySharedWithMe - ) { - const relations = await this.sharedCredentialsRepository.getAllRelationsForCredentials( - credentials.map((c) => c.id), - ); - credentials.forEach((c) => { - c.shared = relations.filter((r) => r.credentialsId === c.id); - }); - } - credentials = credentials.map((c) => this.ownershipService.addOwnedByAndSharedWith(c)); - } - - if (includeScopes) { - const projectRelations = await this.projectService.getProjectRelationsForUser(user); - credentials = credentials.map((c) => this.roleService.addScopes(c, user, projectRelations)); - } - - if (includeData) { - credentials = credentials.map((c: CredentialsEntity & ScopesField) => { - const data = c.scopes.includes('credential:update') ? this.decrypt(c) : undefined; - // We never want to expose the oauthTokenData to the frontend, but it - // expects it to check if the credential is already connected. - if (data?.oauthTokenData) { - data.oauthTokenData = true; - } - - return { - ...c, - data, - } as unknown as CredentialsEntity; - }); - } - - return credentials; - } + return credentials; + } + private async getManyForMemberUser( + user: User, + listQueryOptions: ListQuery.Options & { includeData?: boolean }, + includeGlobal: boolean, + includeData: boolean, + ): Promise { const ids = await this.credentialsFinderService.getCredentialIdsByUserAndRole([user.id], { scopes: ['credential:read'], }); - let credentials = await this.credentialsRepository.findMany( - listQueryOptions, - ids, // only accessible credentials - ); + let credentials = await this.credentialsRepository.findMany(listQueryOptions, ids); + if (includeGlobal) { + credentials = await this.addGlobalCredentials(credentials, includeData); + } + + return credentials; + } + + private async applyPersonalProjectFilter( + listQueryOptions: ListQuery.Options & { includeData?: boolean }, + ): Promise { + const projectId = + typeof listQueryOptions.filter?.projectId === 'string' + ? listQueryOptions.filter.projectId + : undefined; + + if (!projectId) { + return; + } + + let project: Project | undefined; + try { + project = await this.projectService.getProject(projectId); + } catch {} + + if (project?.type === 'personal') { + listQueryOptions.filter = { + ...listQueryOptions.filter, + withRole: 'credential:owner', + }; + } + } + + private async enrichCredentials( + credentials: CredentialsEntity[], + user: User, + isDefaultSelect: boolean, + includeScopes: boolean, + includeData: true, + listQueryOptions: ListQuery.Options & { includeData?: boolean }, + onlySharedWithMe: boolean, + ): Promise>>; + private async enrichCredentials( + credentials: CredentialsEntity[], + user: User, + isDefaultSelect: boolean, + includeScopes: boolean, + includeData: boolean, + listQueryOptions: ListQuery.Options & { includeData?: boolean }, + onlySharedWithMe: boolean, + ): Promise; + private async enrichCredentials( + credentials: CredentialsEntity[], + user: User, + isDefaultSelect: boolean, + includeScopes: boolean, + includeData: boolean, + listQueryOptions: ListQuery.Options & { includeData?: boolean }, + onlySharedWithMe: boolean, + ): Promise> | CredentialsEntity[]> { if (isDefaultSelect) { // Since we're filtering using project ID as part of the relation, // we end up filtering out all the other relations, meaning that if // it's shared to a project, it won't be able to find the home project. // To solve this, we have to get all the relation now, even though // we're deleting them later. - if ( - (listQueryOptions.filter?.shared as { projectId?: string })?.projectId ?? - onlySharedWithMe - ) { - const relations = await this.sharedCredentialsRepository.getAllRelationsForCredentials( - credentials.map((c) => c.id), - ); - credentials.forEach((c) => { - c.shared = relations.filter((r) => r.credentialsId === c.id); - }); - } - - credentials = credentials.map((c) => this.ownershipService.addOwnedByAndSharedWith(c)); + credentials = await this.populateSharedRelations( + credentials, + listQueryOptions, + onlySharedWithMe, + ); } if (includeScopes) { - const projectRelations = await this.projectService.getProjectRelationsForUser(user); - credentials = credentials.map((c) => this.roleService.addScopes(c, user, projectRelations)); + credentials = await this.addScopesToCredentials(credentials, user); } if (includeData) { - credentials = credentials.map((c: CredentialsEntity & ScopesField) => { - return { - ...c, - data: c.scopes.includes('credential:update') ? this.decrypt(c) : undefined, - } as unknown as CredentialsEntity; - }); + return this.addDecryptedDataToCredentials(credentials); } return credentials; } + private async populateSharedRelations( + credentials: CredentialsEntity[], + listQueryOptions: ListQuery.Options & { includeData?: boolean }, + onlySharedWithMe: boolean, + ): Promise { + const needsRelations = + listQueryOptions.filter?.shared && + typeof listQueryOptions.filter.shared === 'object' && + 'projectId' in listQueryOptions.filter.shared + ? listQueryOptions.filter.shared.projectId + : onlySharedWithMe; + + if (needsRelations) { + const relations = await this.sharedCredentialsRepository.getAllRelationsForCredentials( + credentials.map((c) => c.id), + ); + credentials.forEach((c) => { + c.shared = relations.filter((r) => r.credentialsId === c.id); + }); + } + + return credentials.map((c) => this.ownershipService.addOwnedByAndSharedWith(c)); + } + + private async addScopesToCredentials( + credentials: CredentialsEntity[], + user: User, + ): Promise { + const projectRelations = await this.projectService.getProjectRelationsForUser(user); + return credentials.map((c) => this.roleService.addScopes(c, user, projectRelations)); + } + + private addDecryptedDataToCredentials( + credentials: CredentialsEntity[], + ): Array> { + return credentials.map( + ( + c: CredentialsEntity & ScopesField, + ): ICredentialsDecrypted => { + const data = c.scopes.includes('credential:update') ? this.decrypt(c) : undefined; + + // We never want to expose the oauthTokenData to the frontend, but it + // expects it to check if the credential is already connected. + if (data?.oauthTokenData) { + data.oauthTokenData = true; + } + + return { + ...c, + data, + }; + }, + ); + } + /** * @param user The user making the request * @param options.workflowId The workflow that is being edited @@ -237,7 +356,7 @@ export class CredentialsService { // necessary to get the scopes const projectRelations = await this.projectService.getProjectRelationsForUser(user); - // get all credentials the user has access to + // get all credentials the user has access to (including global credentials) const allCredentials = await this.credentialsFinderService.findCredentialsForUser(user, [ 'credential:read', ]); @@ -250,7 +369,9 @@ export class CredentialsService { // the intersection of both is all credentials the user can use in this // workflow or project - const intersection = allCredentials.filter((c) => allCredentialsForWorkflow.includes(c.id)); + const intersection = allCredentials.filter( + (c) => allCredentialsForWorkflow.includes(c.id) || c.isGlobal, + ); return intersection .map((c) => this.roleService.addScopes(c, user, projectRelations)) @@ -260,6 +381,7 @@ export class CredentialsService { type: c.type, scopes: c.scopes, isManaged: c.isManaged, + isGlobal: c.isGlobal, })); } @@ -757,6 +879,18 @@ export class CredentialsService { data: opts.data as ICredentialDataDecryptedObject, }); + // Set isGlobal if provided in the payload and user has permission + const isGlobal = opts.isGlobal; + if (isGlobal === true) { + const canShareGlobally = hasGlobalScope(user, 'credential:shareGlobally'); + if (!canShareGlobally) { + throw new ForbiddenError( + 'You do not have permission to create globally shared credentials', + ); + } + encryptedCredential.isGlobal = isGlobal; + } + const credentialEntity = this.credentialsRepository.create({ ...encryptedCredential, isManaged: opts.isManaged, 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 26e288a2639..71a93f8613c 100644 --- a/packages/cli/src/databases/repositories/__tests__/credentials.repository.test.ts +++ b/packages/cli/src/databases/repositories/__tests__/credentials.repository.test.ts @@ -7,42 +7,279 @@ import { mockEntityManager } from '@test/mocking'; const entityManager = mockEntityManager(CredentialsEntity); const repository = Container.get(CredentialsRepository); -describe('findMany', () => { - const credentialsId = 'cred_123'; - const credential = mock({ id: credentialsId }); - +describe('CredentialsRepository', () => { beforeEach(() => { jest.resetAllMocks(); }); - test('return `data` property if `includeData:true` and select is using the record syntax', async () => { - // ARRANGE - entityManager.find.mockResolvedValueOnce([credential]); + describe('findMany', () => { + const credentialsId = 'cred_123'; + const credential = mock({ id: credentialsId }); - // ACT - const credentials = await repository.findMany({ includeData: true, select: { id: true } }); + test('return `data` property if `includeData:true` and select is using the record syntax', async () => { + // ARRANGE + entityManager.find.mockResolvedValueOnce([credential]); - // ASSERT - expect(credentials).toHaveLength(1); - expect(credentials[0]).toHaveProperty('data'); - }); + // ACT + const credentials = await repository.findMany({ includeData: true, select: { id: true } }); - 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, - //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'); }); - // 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('findAllGlobalCredentials', () => { + test('should find all global credentials without data by default', async () => { + // ARRANGE + const globalCred1 = mock({ id: 'global1', isGlobal: true }); + const globalCred2 = mock({ id: 'global2', isGlobal: true }); + entityManager.find.mockResolvedValueOnce([globalCred1, globalCred2]); + + // ACT + const credentials = await repository.findAllGlobalCredentials(); + + // ASSERT + expect(entityManager.find).toHaveBeenCalledWith( + CredentialsEntity, + expect.objectContaining({ + where: expect.objectContaining({ isGlobal: true }), + select: expect.arrayContaining([ + 'id', + 'name', + 'type', + 'isManaged', + 'createdAt', + 'updatedAt', + 'isGlobal', + ]), + relations: ['shared', 'shared.project', 'shared.project.projectRelations'], + }), + ); + expect(entityManager.find).toHaveBeenCalledWith( + CredentialsEntity, + expect.not.objectContaining({ + select: expect.arrayContaining(['data']), + }), + ); + expect(credentials).toHaveLength(2); + expect(credentials).toEqual([globalCred1, globalCred2]); + }); + + test('should return empty array when no global credentials exist', async () => { + // ARRANGE + entityManager.find.mockResolvedValueOnce([]); + + // ACT + const credentials = await repository.findAllGlobalCredentials(); + + // ASSERT + expect(credentials).toHaveLength(0); + }); + + test('should include shared relations for global credentials', async () => { + // ARRANGE + const globalCred = mock({ + id: 'global1', + isGlobal: true, + shared: [mock()], + }); + entityManager.find.mockResolvedValueOnce([globalCred]); + + // ACT + const credentials = await repository.findAllGlobalCredentials(); + + // ASSERT + expect(entityManager.find).toHaveBeenCalledWith( + CredentialsEntity, + expect.objectContaining({ + relations: ['shared', 'shared.project', 'shared.project.projectRelations'], + }), + ); + expect(credentials[0].shared).toBeDefined(); + }); + + test('should include data when includeData is true', async () => { + // ARRANGE + const globalCred = mock({ + id: 'global1', + isGlobal: true, + data: 'encrypted-data', + }); + entityManager.find.mockResolvedValueOnce([globalCred]); + + // ACT + const credentials = await repository.findAllGlobalCredentials(true); + + // ASSERT + expect(entityManager.find).toHaveBeenCalledWith( + CredentialsEntity, + expect.objectContaining({ + where: expect.objectContaining({ isGlobal: true }), + select: expect.arrayContaining([ + 'id', + 'name', + 'type', + 'isManaged', + 'createdAt', + 'updatedAt', + 'isGlobal', + 'data', + ]), + }), + ); + expect(credentials).toHaveLength(1); + expect(credentials[0]).toHaveProperty('data'); + }); + + test('should not include data when includeData is false', async () => { + // ARRANGE + const globalCred = mock({ + id: 'global1', + isGlobal: true, + }); + entityManager.find.mockResolvedValueOnce([globalCred]); + + // ACT + const credentials = await repository.findAllGlobalCredentials(false); + + // ASSERT + expect(entityManager.find).toHaveBeenCalledWith( + CredentialsEntity, + expect.not.objectContaining({ + select: expect.arrayContaining(['data']), + }), + ); + expect(credentials).toHaveLength(1); + }); + }); + + describe('findAllPersonalCredentials', () => { + test('should find all credentials owned by personal projects', async () => { + // ARRANGE + const personalCred1 = mock({ id: 'cred1' }); + const personalCred2 = mock({ id: 'cred2' }); + entityManager.findBy.mockResolvedValueOnce([personalCred1, personalCred2]); + + // ACT + const credentials = await repository.findAllPersonalCredentials(); + + // ASSERT + expect(entityManager.findBy).toHaveBeenCalledWith(CredentialsEntity, { + shared: { project: { type: 'personal' } }, + }); + expect(credentials).toHaveLength(2); + expect(credentials).toEqual([personalCred1, personalCred2]); + }); + + test('should return empty array when no personal credentials exist', async () => { + // ARRANGE + entityManager.findBy.mockResolvedValueOnce([]); + + // ACT + const credentials = await repository.findAllPersonalCredentials(); + + // ASSERT + expect(credentials).toHaveLength(0); + }); + }); + + describe('findAllCredentialsForWorkflow', () => { + test('should find all credentials accessible to a workflow', async () => { + // ARRANGE + const workflowId = 'workflow123'; + const cred1 = mock({ id: 'cred1' }); + const cred2 = mock({ id: 'cred2' }); + entityManager.findBy.mockResolvedValueOnce([cred1, cred2]); + + // ACT + const credentials = await repository.findAllCredentialsForWorkflow(workflowId); + + // ASSERT + expect(entityManager.findBy).toHaveBeenCalledWith(CredentialsEntity, { + shared: { project: { sharedWorkflows: { workflowId } } }, + }); + expect(credentials).toHaveLength(2); + expect(credentials).toEqual([cred1, cred2]); + }); + + test('should return empty array when workflow has no accessible credentials', async () => { + // ARRANGE + const workflowId = 'workflow123'; + entityManager.findBy.mockResolvedValueOnce([]); + + // ACT + const credentials = await repository.findAllCredentialsForWorkflow(workflowId); + + // ASSERT + expect(credentials).toHaveLength(0); + }); + }); + + describe('findAllCredentialsForProject', () => { + test('should find all credentials in a project', async () => { + // ARRANGE + const projectId = 'project123'; + const cred1 = mock({ id: 'cred1' }); + const cred2 = mock({ id: 'cred2' }); + entityManager.findBy.mockResolvedValueOnce([cred1, cred2]); + + // ACT + const credentials = await repository.findAllCredentialsForProject(projectId); + + // ASSERT + expect(entityManager.findBy).toHaveBeenCalledWith(CredentialsEntity, { + shared: { projectId }, + }); + expect(credentials).toHaveLength(2); + expect(credentials).toEqual([cred1, cred2]); + }); + + test('should return empty array when project has no credentials', async () => { + // ARRANGE + const projectId = 'project123'; + entityManager.findBy.mockResolvedValueOnce([]); + + // ACT + const credentials = await repository.findAllCredentialsForProject(projectId); + + // ASSERT + expect(credentials).toHaveLength(0); + }); }); }); diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control-export.service.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control-export.service.test.ts index 5775f51552e..9adc7953ae2 100644 --- a/packages/cli/src/environments.ee/source-control/__tests__/source-control-export.service.test.ts +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control-export.service.test.ts @@ -185,6 +185,164 @@ describe('SourceControlExportService', () => { expect(result.missingIds).toHaveLength(1); expect(result.missingIds?.[0]).toBe('cred1'); }); + + it('should export global credentials with isGlobal flag set to true', async () => { + // Arrange + const mockGlobalCredential = mock({ + id: 'global-cred1', + name: 'Global Test Credential', + type: 'oauth2', + data: cipher.encrypt(credentialData), + isGlobal: true, + }); + + sharedCredentialsRepository.findByCredentialIds.mockResolvedValue([ + mock({ + credentials: mockGlobalCredential, + project: mock({ + type: 'team', + id: 'team1', + name: 'Test Team', + }), + }), + ]); + + // Act + const result = await service.exportCredentialsToWorkFolder([ + mock({ id: 'global-cred1' }), + ]); + + // Assert + expect(result.count).toBe(1); + expect(result.files).toHaveLength(1); + + const dataCaptor = captor(); + expect(fsWriteFile).toHaveBeenCalledWith( + '/mock/n8n/git/credential_stubs/global-cred1.json', + dataCaptor, + ); + + const exportedData = JSON.parse(dataCaptor.value); + expect(exportedData).toEqual({ + id: 'global-cred1', + name: 'Global Test Credential', + type: 'oauth2', + data: { + authUrl: '', + accessTokenUrl: '', + clientId: '', + clientSecret: '', + }, + ownedBy: { + type: 'team', + teamId: 'team1', + teamName: 'Test Team', + }, + isGlobal: true, + }); + }); + + it('should export non-global credentials with isGlobal flag set to false', async () => { + // Arrange + const mockNonGlobalCredential = mock({ + id: 'non-global-cred1', + name: 'Non-Global Test Credential', + type: 'oauth2', + data: cipher.encrypt(credentialData), + isGlobal: false, + }); + + sharedCredentialsRepository.findByCredentialIds.mockResolvedValue([ + mock({ + credentials: mockNonGlobalCredential, + project: mock({ + type: 'personal', + projectRelations: [ + { + role: PROJECT_OWNER_ROLE, + user: mock({ email: 'user@example.com' }), + }, + ], + }), + }), + ]); + + // Act + const result = await service.exportCredentialsToWorkFolder([ + mock({ id: 'non-global-cred1' }), + ]); + + // Assert + expect(result.count).toBe(1); + + const dataCaptor = captor(); + expect(fsWriteFile).toHaveBeenCalledWith( + '/mock/n8n/git/credential_stubs/non-global-cred1.json', + dataCaptor, + ); + + const exportedData = JSON.parse(dataCaptor.value); + expect(exportedData).toEqual({ + id: 'non-global-cred1', + name: 'Non-Global Test Credential', + type: 'oauth2', + data: { + authUrl: '', + accessTokenUrl: '', + clientId: '', + clientSecret: '', + }, + ownedBy: { + type: 'personal', + personalEmail: 'user@example.com', + }, + isGlobal: false, + }); + }); + + it('should default isGlobal to false when not specified', async () => { + // Arrange + const mockCredentialWithoutIsGlobal = mock({ + id: 'cred-no-flag', + name: 'Credential Without Flag', + type: 'oauth2', + data: cipher.encrypt(credentialData), + isGlobal: undefined, // explicitly undefined to test the default + }); + + sharedCredentialsRepository.findByCredentialIds.mockResolvedValue([ + mock({ + credentials: mockCredentialWithoutIsGlobal, + project: mock({ + type: 'personal', + projectRelations: [ + { + role: PROJECT_OWNER_ROLE, + user: mock({ email: 'user@example.com' }), + }, + ], + }), + }), + ]); + + // Act + const result = await service.exportCredentialsToWorkFolder([ + mock({ id: 'cred-no-flag' }), + ]); + + // Assert + expect(result.count).toBe(1); + + const dataCaptor = captor(); + expect(fsWriteFile).toHaveBeenCalledWith( + '/mock/n8n/git/credential_stubs/cred-no-flag.json', + dataCaptor, + ); + + const exportedData = JSON.parse(dataCaptor.value); + // When isGlobal is undefined, the service defaults it to false via destructuring + expect(exportedData.isGlobal).toBe(false); + }); }); describe('exportTagsToWorkFolder', () => { diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts index 986d8b796e7..945b6d38f27 100644 --- a/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts @@ -522,6 +522,82 @@ describe('SourceControlImportService', () => { expect(result).toHaveLength(0); }); + + it('should parse global credentials with isGlobal flag set to true', async () => { + globMock.mockResolvedValue(['/mock/global-credential.json']); + + const mockGlobalCredentialData = { + id: 'global-cred1', + name: 'Global Test Credential', + type: 'oauth2', + isGlobal: true, + }; + + fsReadFile.mockResolvedValue(JSON.stringify(mockGlobalCredentialData)); + + const result = await service.getRemoteCredentialsFromFiles(globalAdminContext); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual( + expect.objectContaining({ + id: 'global-cred1', + name: 'Global Test Credential', + type: 'oauth2', + isGlobal: true, + }), + ); + }); + + it('should parse non-global credentials with isGlobal flag set to false', async () => { + globMock.mockResolvedValue(['/mock/non-global-credential.json']); + + const mockNonGlobalCredentialData = { + id: 'non-global-cred1', + name: 'Non-Global Test Credential', + type: 'oauth2', + isGlobal: false, + }; + + fsReadFile.mockResolvedValue(JSON.stringify(mockNonGlobalCredentialData)); + + const result = await service.getRemoteCredentialsFromFiles(globalAdminContext); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual( + expect.objectContaining({ + id: 'non-global-cred1', + name: 'Non-Global Test Credential', + type: 'oauth2', + isGlobal: false, + }), + ); + }); + + it('should default isGlobal to false when not specified in credential file', async () => { + globMock.mockResolvedValue(['/mock/credential-no-flag.json']); + + const mockCredentialDataWithoutFlag = { + id: 'cred-no-flag', + name: 'Credential Without Flag', + type: 'oauth2', + // isGlobal not specified + }; + + fsReadFile.mockResolvedValue(JSON.stringify(mockCredentialDataWithoutFlag)); + + const result = await service.getRemoteCredentialsFromFiles(globalAdminContext); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual( + expect.objectContaining({ + id: 'cred-no-flag', + name: 'Credential Without Flag', + type: 'oauth2', + }), + ); + // isGlobal should default to false (undefined will be treated as false by the service) + expect(result[0].isGlobal).toBeFalsy(); + }); }); describe('getRemoteVariablesFromFile', () => { diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control-status.service.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control-status.service.test.ts index 9fa17549140..2e8b02c8973 100644 --- a/packages/cli/src/environments.ee/source-control/__tests__/source-control-status.service.test.ts +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control-status.service.test.ts @@ -1053,6 +1053,91 @@ describe('getStatus', () => { }); }); + describe('isGlobal changes', () => { + it('should detect when isGlobal changes from false to true', async () => { + const local = createCredential({ isGlobal: false }); + const remote = createCredential({ isGlobal: true }); + + sourceControlImportService.getRemoteCredentialsFromFiles.mockResolvedValue([remote]); + sourceControlImportService.getLocalCredentialsFromDb.mockResolvedValue([local]); + + const result = await sourceControlStatusService.getStatus(user, { + direction: 'push', + verbose: true, + preferLocalVersion: false, + }); + + if (Array.isArray(result)) fail('Expected result to be an object.'); + expect(result.credModifiedInEither).toHaveLength(1); + }); + + it('should detect when isGlobal changes from true to false', async () => { + const local = createCredential({ isGlobal: true }); + const remote = createCredential({ isGlobal: false }); + + sourceControlImportService.getRemoteCredentialsFromFiles.mockResolvedValue([remote]); + sourceControlImportService.getLocalCredentialsFromDb.mockResolvedValue([local]); + + const result = await sourceControlStatusService.getStatus(user, { + direction: 'push', + verbose: true, + preferLocalVersion: false, + }); + + if (Array.isArray(result)) fail('Expected result to be an object.'); + expect(result.credModifiedInEither).toHaveLength(1); + }); + + it('should detect when isGlobal changes from undefined to true', async () => { + const local = createCredential({ isGlobal: undefined }); + const remote = createCredential({ isGlobal: true }); + + sourceControlImportService.getRemoteCredentialsFromFiles.mockResolvedValue([remote]); + sourceControlImportService.getLocalCredentialsFromDb.mockResolvedValue([local]); + + const result = await sourceControlStatusService.getStatus(user, { + direction: 'push', + verbose: true, + preferLocalVersion: false, + }); + + if (Array.isArray(result)) fail('Expected result to be an object.'); + expect(result.credModifiedInEither).toHaveLength(1); + }); + + it('should not detect changes when isGlobal is the same (both true)', async () => { + const credential = createCredential({ isGlobal: true }); + + sourceControlImportService.getRemoteCredentialsFromFiles.mockResolvedValue([credential]); + sourceControlImportService.getLocalCredentialsFromDb.mockResolvedValue([credential]); + + const result = await sourceControlStatusService.getStatus(user, { + direction: 'push', + verbose: true, + preferLocalVersion: false, + }); + + if (Array.isArray(result)) fail('Expected result to be an object.'); + expect(result.credModifiedInEither).toHaveLength(0); + }); + + it('should not detect changes when isGlobal is the same (both false/undefined)', async () => { + const credential = createCredential({ isGlobal: false }); + + sourceControlImportService.getRemoteCredentialsFromFiles.mockResolvedValue([credential]); + sourceControlImportService.getLocalCredentialsFromDb.mockResolvedValue([credential]); + + const result = await sourceControlStatusService.getStatus(user, { + direction: 'push', + verbose: true, + preferLocalVersion: false, + }); + + if (Array.isArray(result)) fail('Expected result to be an object.'); + expect(result.credModifiedInEither).toHaveLength(0); + }); + }); + it('should not detect as modified when everything is the same', async () => { const credential = createCredential({ ownedBy: { type: 'team', projectId: 'team1', projectName: 'Team 1' } as any, diff --git a/packages/cli/src/environments.ee/source-control/source-control-export.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control-export.service.ee.ts index e64740ecc15..98e140115e6 100644 --- a/packages/cli/src/environments.ee/source-control/source-control-export.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control-export.service.ee.ts @@ -418,7 +418,7 @@ export class SourceControlExportService { } await Promise.all( credentialsToBeExported.map(async (sharing) => { - const { name, type, data, id } = sharing.credentials; + const { name, type, data, id, isGlobal = false } = sharing.credentials; const credentials = new Credentials({ id, name }, type, data); let owner: RemoteResourceOwner | null = null; @@ -455,6 +455,7 @@ export class SourceControlExportService { type, data: this.replaceCredentialData(rest), ownedBy: owner, + isGlobal, }; const filePath = this.getCredentialsPath(id); diff --git a/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts index 11d8df771c0..82afa4023cb 100644 --- a/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts @@ -388,6 +388,7 @@ export class SourceControlImportService { id: true, name: true, type: true, + isGlobal: true, shared: { project: { id: true, @@ -418,6 +419,7 @@ export class SourceControlImportService { type: local.type, filename: getCredentialExportPath(local.id, this.credentialExportFolder), ownedBy: remoteOwnerProject ? getOwnerFromProject(remoteOwnerProject) : undefined, + isGlobal: local.isGlobal, }; }) as StatusExportableCredential[]; } @@ -787,7 +789,7 @@ export class SourceControlImportService { (e) => e.id === credential.id && e.type === credential.type, ); - const { name, type, data, id } = credential; + const { name, type, data, id, isGlobal = false } = credential; const newCredentialObject = new Credentials({ id, name }, type); if (existingCredential?.data) { newCredentialObject.data = existingCredential.data; @@ -801,7 +803,7 @@ export class SourceControlImportService { } this.logger.debug(`Updating credential id ${newCredentialObject.id as string}`); - await this.credentialsRepository.upsert(newCredentialObject, ['id']); + await this.credentialsRepository.upsert({ ...newCredentialObject, isGlobal }, ['id']); const localOwner = existingSharedCredentials.find( (c) => c.credentialsId === credential.id && c.role === 'credential:owner', diff --git a/packages/cli/src/environments.ee/source-control/source-control-status.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control-status.service.ee.ts index b0b4bfab179..b82393be638 100644 --- a/packages/cli/src/environments.ee/source-control/source-control-status.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control-status.service.ee.ts @@ -307,13 +307,14 @@ export class SourceControlStatusService { const credModifiedInEither: StatusExportableCredential[] = []; credLocalIds.forEach((local) => { - // Compare name, type and owner since those are the synced properties for credentials + // Compare name, type, owner and isGlobal since those are the synced properties for credentials const mismatchingCreds = credRemoteIds.find((remote) => { return ( remote.id === local.id && (remote.name !== local.name || remote.type !== local.type || - hasOwnerChanged(remote.ownedBy, local.ownedBy)) + hasOwnerChanged(remote.ownedBy, local.ownedBy) || + (remote.isGlobal ?? false) !== (local.isGlobal ?? false)) ); }); diff --git a/packages/cli/src/environments.ee/source-control/types/exportable-credential.ts b/packages/cli/src/environments.ee/source-control/types/exportable-credential.ts index 2ffb04d6120..b5baca2a80f 100644 --- a/packages/cli/src/environments.ee/source-control/types/exportable-credential.ts +++ b/packages/cli/src/environments.ee/source-control/types/exportable-credential.ts @@ -13,6 +13,11 @@ export interface ExportableCredential { * Ownership is mirrored at target instance if user is also present there. */ ownedBy: RemoteResourceOwner | null; + + /** + * Whether this credential is globally accessible across all projects. + */ + isGlobal?: boolean; } export type StatusExportableCredential = ExportableCredential & { diff --git a/packages/cli/src/executions/pre-execution-checks/__tests__/credentials-permission-checker.test.ts b/packages/cli/src/executions/pre-execution-checks/__tests__/credentials-permission-checker.test.ts index bb2f1b2a2fa..14144704a86 100644 --- a/packages/cli/src/executions/pre-execution-checks/__tests__/credentials-permission-checker.test.ts +++ b/packages/cli/src/executions/pre-execution-checks/__tests__/credentials-permission-checker.test.ts @@ -2,6 +2,8 @@ import { type Project, type User, type SharedCredentialsRepository, + type CredentialsRepository, + type CredentialsEntity, GLOBAL_OWNER_ROLE, } from '@n8n/db'; import { mock } from 'jest-mock-extended'; @@ -14,10 +16,12 @@ import { CredentialsPermissionChecker } from '../credentials-permission-checker' describe('CredentialsPermissionChecker', () => { const sharedCredentialsRepository = mock(); + const credentialsRepository = mock(); const ownershipService = mock(); const projectService = mock(); const permissionChecker = new CredentialsPermissionChecker( sharedCredentialsRepository, + credentialsRepository, ownershipService, projectService, ); @@ -63,6 +67,7 @@ describe('CredentialsPermissionChecker', () => { it('should throw if a credential is not accessible', async () => { ownershipService.getPersonalProjectOwnerCached.mockResolvedValueOnce(null); sharedCredentialsRepository.getFilteredAccessibleCredentials.mockResolvedValueOnce([]); + credentialsRepository.find.mockResolvedValueOnce([]); await expect(permissionChecker.check(workflowId, [node])).rejects.toThrow( 'Node "Test Node" does not have access to the credential', @@ -87,6 +92,7 @@ describe('CredentialsPermissionChecker', () => { sharedCredentialsRepository.getFilteredAccessibleCredentials.mockResolvedValueOnce([ credentialId, ]); + credentialsRepository.find.mockResolvedValueOnce([]); await expect(permissionChecker.check(workflowId, [node])).resolves.not.toThrow(); @@ -106,4 +112,61 @@ describe('CredentialsPermissionChecker', () => { expect(projectService.findProjectsWorkflowIsIn).not.toHaveBeenCalled(); expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).not.toHaveBeenCalled(); }); + + it('should allow global credentials for any project', async () => { + ownershipService.getPersonalProjectOwnerCached.mockResolvedValueOnce(null); + sharedCredentialsRepository.getFilteredAccessibleCredentials.mockResolvedValueOnce([]); + const globalCredential = mock({ + id: credentialId, + isGlobal: true, + }); + credentialsRepository.find.mockResolvedValueOnce([globalCredential]); + + await expect(permissionChecker.check(workflowId, [node])).resolves.not.toThrow(); + + expect(projectService.findProjectsWorkflowIsIn).toHaveBeenCalledWith(workflowId); + expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).toHaveBeenCalledWith( + [personalProject.id], + [credentialId], + ); + expect(credentialsRepository.find).toHaveBeenCalledWith({ + select: ['id'], + where: { + isGlobal: true, + }, + }); + }); + + it('should allow global credentials for team projects', async () => { + const teamProject = mock({ + id: 'team-project', + name: 'Team Project', + type: 'team', + }); + // Reset and set up new mocks for this test + jest.resetAllMocks(); + ownershipService.getWorkflowProjectCached.mockResolvedValue(teamProject); + projectService.findProjectsWorkflowIsIn.mockResolvedValue([teamProject.id]); + ownershipService.getPersonalProjectOwnerCached.mockResolvedValue(null); + sharedCredentialsRepository.getFilteredAccessibleCredentials.mockResolvedValue([]); + const globalCredential = mock({ + id: credentialId, + isGlobal: true, + }); + credentialsRepository.find.mockResolvedValue([globalCredential]); + + await expect(permissionChecker.check(workflowId, [node])).resolves.not.toThrow(); + + expect(projectService.findProjectsWorkflowIsIn).toHaveBeenCalledWith(workflowId); + expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).toHaveBeenCalledWith( + [teamProject.id], + [credentialId], + ); + expect(credentialsRepository.find).toHaveBeenCalledWith({ + select: ['id'], + where: { + isGlobal: true, + }, + }); + }); }); diff --git a/packages/cli/src/executions/pre-execution-checks/credentials-permission-checker.ts b/packages/cli/src/executions/pre-execution-checks/credentials-permission-checker.ts index 5d067a88008..a6ebf583eb5 100644 --- a/packages/cli/src/executions/pre-execution-checks/credentials-permission-checker.ts +++ b/packages/cli/src/executions/pre-execution-checks/credentials-permission-checker.ts @@ -1,5 +1,5 @@ import type { Project } from '@n8n/db'; -import { SharedCredentialsRepository } from '@n8n/db'; +import { CredentialsRepository, SharedCredentialsRepository } from '@n8n/db'; import { Service } from '@n8n/di'; import { hasGlobalScope } from '@n8n/permissions'; import type { INode } from 'n8n-workflow'; @@ -34,6 +34,7 @@ class InaccessibleCredentialError extends UserError { export class CredentialsPermissionChecker { constructor( private readonly sharedCredentialsRepository: SharedCredentialsRepository, + private readonly credentialsRepository: CredentialsRepository, private readonly ownershipService: OwnershipService, private readonly projectService: ProjectService, ) {} @@ -67,14 +68,33 @@ export class CredentialsPermissionChecker { workflowCredIds, ); + const accessibleSet = await this.addGlobalCredentialsToAccessibleSet(accessible); + for (const credentialsId of workflowCredIds) { - if (!accessible.includes(credentialsId)) { + if (!accessibleSet.has(credentialsId)) { const nodeToFlag = credIdsToNodes[credentialsId][0]; throw new InaccessibleCredentialError(nodeToFlag, homeProject); } } } + /** + * Adds global credentials (isGlobal: true) to the set of accessible credentials. + */ + private async addGlobalCredentialsToAccessibleSet( + accessibleCredentialIds: string[], + ): Promise> { + const accessibleSet = new Set(accessibleCredentialIds); + const globalCredentials = await this.credentialsRepository.find({ + where: { isGlobal: true }, + select: ['id'], + }); + for (const globalCred of globalCredentials) { + accessibleSet.add(globalCred.id); + } + return accessibleSet; + } + private mapCredIdsToNodes(nodes: INode[]) { return nodes.reduce<{ [credentialId: string]: INode[] }>((map, node) => { if (node.disabled || !node.credentials) return map; diff --git a/packages/cli/src/modules/chat-hub/__tests__/chat-hub-credentials.service.test.ts b/packages/cli/src/modules/chat-hub/__tests__/chat-hub-credentials.service.test.ts new file mode 100644 index 00000000000..80c620dc569 --- /dev/null +++ b/packages/cli/src/modules/chat-hub/__tests__/chat-hub-credentials.service.test.ts @@ -0,0 +1,244 @@ +import type { User } from '@n8n/db'; +import { mock } from 'jest-mock-extended'; +import type { EntityManager } from '@n8n/typeorm'; +import type { INodeCredentials } from 'n8n-workflow'; + +import { + ChatHubCredentialsService, + type CredentialWithProjectId, +} from '../chat-hub-credentials.service'; +import type { CredentialsFinderService } from '@/credentials/credentials-finder.service'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; +import type { ChatHubLLMProvider } from '@n8n/api-types'; + +describe('ChatHubCredentialsService', () => { + const credentialsFinderService = mock(); + const service = new ChatHubCredentialsService(credentialsFinderService); + + const mockUser = mock({ id: 'user-123' }); + const mockTrx = mock(); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + describe('ensureCredentials', () => { + it('should return credential when user has access and credential is found', async () => { + const mockCredential = mock({ + id: 'cred-123', + name: 'OpenAI Credentials', + type: 'openAiApi', + projectId: 'project-456', + }); + + const credentials: INodeCredentials = { + openAiApi: { id: 'cred-123', name: 'OpenAI Credentials' }, + }; + + credentialsFinderService.findAllCredentialsForUser.mockResolvedValue([mockCredential]); + + const result = await service.ensureCredentials( + mockUser, + 'openai' as ChatHubLLMProvider, + credentials, + mockTrx, + ); + + expect(result).toEqual(mockCredential); + expect(credentialsFinderService.findAllCredentialsForUser).toHaveBeenCalledWith( + mockUser, + ['credential:read'], + mockTrx, + { includeGlobalCredentials: true }, + ); + }); + + it('should include global credentials when fetching credentials', async () => { + const mockGlobalCredential = mock({ + id: 'global-cred-123', + name: 'Global OpenAI Credentials', + type: 'openAiApi', + isGlobal: true, + projectId: 'project-global', + }); + + const credentials: INodeCredentials = { + openAiApi: { id: 'global-cred-123', name: 'Global OpenAI Credentials' }, + }; + + credentialsFinderService.findAllCredentialsForUser.mockResolvedValue([mockGlobalCredential]); + + const result = await service.ensureCredentials( + mockUser, + 'openai' as ChatHubLLMProvider, + credentials, + mockTrx, + ); + + expect(result).toEqual(mockGlobalCredential); + expect(credentialsFinderService.findAllCredentialsForUser).toHaveBeenCalledWith( + mockUser, + ['credential:read'], + mockTrx, + { includeGlobalCredentials: true }, + ); + }); + + it('should throw BadRequestError when no credentials are provided', async () => { + const credentials: INodeCredentials = {}; + + credentialsFinderService.findAllCredentialsForUser.mockResolvedValue([]); + + await expect( + service.ensureCredentials(mockUser, 'openai' as ChatHubLLMProvider, credentials, mockTrx), + ).rejects.toThrow(BadRequestError); + await expect( + service.ensureCredentials(mockUser, 'openai' as ChatHubLLMProvider, credentials, mockTrx), + ).rejects.toThrow('No credentials provided for the selected model provider'); + }); + + it('should throw ForbiddenError when user does not have access to the credential', async () => { + const mockCredential = mock({ + id: 'other-cred-456', + name: 'Other Credentials', + type: 'openAiApi', + projectId: 'project-other', + }); + + const credentials: INodeCredentials = { + openAiApi: { id: 'cred-123', name: 'OpenAI Credentials' }, + }; + + credentialsFinderService.findAllCredentialsForUser.mockResolvedValue([mockCredential]); + + await expect( + service.ensureCredentials(mockUser, 'openai' as ChatHubLLMProvider, credentials, mockTrx), + ).rejects.toThrow(ForbiddenError); + await expect( + service.ensureCredentials(mockUser, 'openai' as ChatHubLLMProvider, credentials, mockTrx), + ).rejects.toThrow("You don't have access to the provided credentials"); + }); + + it('should handle n8n provider by returning null credential ID', async () => { + const credentials: INodeCredentials = {}; + + credentialsFinderService.findAllCredentialsForUser.mockResolvedValue([]); + + await expect( + service.ensureCredentials(mockUser, 'n8n' as ChatHubLLMProvider, credentials, mockTrx), + ).rejects.toThrow(BadRequestError); + }); + + it('should handle custom-agent provider by returning null credential ID', async () => { + const credentials: INodeCredentials = {}; + + credentialsFinderService.findAllCredentialsForUser.mockResolvedValue([]); + + await expect( + service.ensureCredentials( + mockUser, + 'custom-agent' as ChatHubLLMProvider, + credentials, + mockTrx, + ), + ).rejects.toThrow(BadRequestError); + }); + + it('should return first credential when credential is shared through multiple projects', async () => { + const mockCredential = mock({ + id: 'cred-123', + name: 'Shared Credentials', + type: 'openAiApi', + projectId: 'project-1', + }); + + const credentials: INodeCredentials = { + openAiApi: { id: 'cred-123', name: 'Shared Credentials' }, + }; + + credentialsFinderService.findAllCredentialsForUser.mockResolvedValue([mockCredential]); + + const result = await service.ensureCredentials( + mockUser, + 'openai' as ChatHubLLMProvider, + credentials, + mockTrx, + ); + + expect(result).toEqual(mockCredential); + expect(result).toHaveProperty('projectId'); + }); + }); + + describe('ensureCredentialById', () => { + it('should return credential when user has access to the credential', async () => { + const mockCredential = mock({ + id: 'cred-123', + name: 'OpenAI Credentials', + type: 'openAiApi', + projectId: 'project-456', + }); + + credentialsFinderService.findAllCredentialsForUser.mockResolvedValue([mockCredential]); + + const result = await service.ensureCredentialById(mockUser, 'cred-123', mockTrx); + + expect(result).toEqual(mockCredential); + expect(credentialsFinderService.findAllCredentialsForUser).toHaveBeenCalledWith( + mockUser, + ['credential:read'], + mockTrx, + { includeGlobalCredentials: true }, + ); + }); + + it('should include global credentials when fetching by ID', async () => { + const mockGlobalCredential = mock({ + id: 'global-cred-123', + name: 'Global OpenAI Credentials', + type: 'openAiApi', + isGlobal: true, + projectId: 'project-global', + }); + + credentialsFinderService.findAllCredentialsForUser.mockResolvedValue([mockGlobalCredential]); + + const result = await service.ensureCredentialById(mockUser, 'global-cred-123', mockTrx); + + expect(result).toEqual(mockGlobalCredential); + expect(credentialsFinderService.findAllCredentialsForUser).toHaveBeenCalledWith( + mockUser, + ['credential:read'], + mockTrx, + { includeGlobalCredentials: true }, + ); + }); + + it('should throw ForbiddenError when user does not have access to the credential', async () => { + const mockCredential = mock({ + id: 'other-cred-456', + name: 'Other Credentials', + type: 'openAiApi', + projectId: 'project-other', + }); + + credentialsFinderService.findAllCredentialsForUser.mockResolvedValue([mockCredential]); + + await expect(service.ensureCredentialById(mockUser, 'cred-123', mockTrx)).rejects.toThrow( + ForbiddenError, + ); + await expect(service.ensureCredentialById(mockUser, 'cred-123', mockTrx)).rejects.toThrow( + "You don't have access to the provided credentials", + ); + }); + + it('should throw ForbiddenError when credential is not found', async () => { + credentialsFinderService.findAllCredentialsForUser.mockResolvedValue([]); + + await expect(service.ensureCredentialById(mockUser, 'cred-123', mockTrx)).rejects.toThrow( + ForbiddenError, + ); + }); + }); +}); diff --git a/packages/cli/src/modules/chat-hub/chat-hub-credentials.service.ts b/packages/cli/src/modules/chat-hub/chat-hub-credentials.service.ts index de2fa79d77d..90a4bb86279 100644 --- a/packages/cli/src/modules/chat-hub/chat-hub-credentials.service.ts +++ b/packages/cli/src/modules/chat-hub/chat-hub-credentials.service.ts @@ -28,6 +28,7 @@ export class ChatHubCredentialsService { user, ['credential:read'], trx, + { includeGlobalCredentials: true }, ); const credentialId = this.pickCredentialId(provider, credentials); @@ -52,6 +53,7 @@ export class ChatHubCredentialsService { user, ['credential:read'], trx, + { includeGlobalCredentials: true }, ); const credential = allCredentials.find((c) => c.id === credentialId); diff --git a/packages/cli/src/modules/mcp/__tests__/webhook-utils.test.ts b/packages/cli/src/modules/mcp/__tests__/webhook-utils.test.ts index 45923f63b52..0a2067359b7 100644 --- a/packages/cli/src/modules/mcp/__tests__/webhook-utils.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/webhook-utils.test.ts @@ -24,6 +24,7 @@ const mockCredentialsService = ( type: 'mock', shared: [] as SharedCredentials[], isManaged: false, + isGlobal: false, id, // Methods present on entities via WithTimestampsAndStringId mixin generateId() {}, diff --git a/packages/cli/src/permissions.ee/__tests__/check-access.test.ts b/packages/cli/src/permissions.ee/__tests__/check-access.test.ts index c2533ccb052..ce07b91d703 100644 --- a/packages/cli/src/permissions.ee/__tests__/check-access.test.ts +++ b/packages/cli/src/permissions.ee/__tests__/check-access.test.ts @@ -3,12 +3,14 @@ import { ProjectRepository, SharedCredentialsRepository, SharedWorkflowRepository, + CredentialsRepository, type User, } from '@n8n/db'; import { Container } from '@n8n/di'; import { type Scope } from '@n8n/permissions'; import { mock } from 'jest-mock-extended'; +import { CredentialsFinderService } from '@/credentials/credentials-finder.service'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { RoleService } from '@/services/role.service'; @@ -17,12 +19,18 @@ import { userHasScopes } from '../check-access'; describe('userHasScopes', () => { let findByWorkflowMock: jest.Mock; let findByCredentialMock: jest.Mock; + let findByGlobalCredentialMock: jest.Mock; + let findGlobalCredentialByIdMock: jest.Mock; + let hasGlobalReadOnlyAccessMock: jest.Mock; let roleServiceMock: jest.Mock; let mockQueryBuilder: any; beforeAll(() => { findByWorkflowMock = jest.fn(); findByCredentialMock = jest.fn(); + findByGlobalCredentialMock = jest.fn(); + findGlobalCredentialByIdMock = jest.fn(); + hasGlobalReadOnlyAccessMock = jest.fn(); roleServiceMock = jest.fn(); Container.set( @@ -39,6 +47,21 @@ describe('userHasScopes', () => { }), ); + Container.set( + CredentialsRepository, + mock({ + findBy: findByGlobalCredentialMock, + }), + ); + + Container.set( + CredentialsFinderService, + mock({ + findGlobalCredentialById: findGlobalCredentialByIdMock, + hasGlobalReadOnlyAccess: hasGlobalReadOnlyAccessMock, + }), + ); + mockQueryBuilder = { innerJoin: jest.fn().mockReturnThis(), where: jest.fn().mockReturnThis(), @@ -68,6 +91,9 @@ describe('userHasScopes', () => { jest.clearAllMocks(); findByWorkflowMock.mockReset(); findByCredentialMock.mockReset(); + findByGlobalCredentialMock.mockReset(); + findGlobalCredentialByIdMock.mockReset(); + hasGlobalReadOnlyAccessMock.mockReset(); roleServiceMock.mockReset(); // Default mock responses @@ -235,6 +261,8 @@ describe('userHasScopes', () => { role: 'workflow:owner', // Wrong namespace role }, ]); + hasGlobalReadOnlyAccessMock.mockReturnValue(true); + findGlobalCredentialByIdMock.mockResolvedValue(null); const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; const scopes = ['credential:read'] as Scope[]; @@ -295,6 +323,8 @@ describe('userHasScopes', () => { role: 'credential:owner', }, ]); + hasGlobalReadOnlyAccessMock.mockReturnValue(true); + findGlobalCredentialByIdMock.mockResolvedValue(null); const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; const scopes = ['credential:read'] as Scope[]; @@ -395,6 +425,8 @@ describe('userHasScopes', () => { .mockResolvedValueOnce([ { credentialsId: credentialId2, projectId: 'projectId', role: 'credential:viewer' }, ]); + hasGlobalReadOnlyAccessMock.mockReturnValue(true); + findGlobalCredentialByIdMock.mockResolvedValue(null); const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; const scopes = ['credential:read'] as Scope[]; @@ -409,4 +441,109 @@ describe('userHasScopes', () => { expect(roleServiceMock).toHaveBeenCalledTimes(2); }); }); + + describe('global credentials', () => { + it('should grant access to global credential when user lacks project access for credential:read', async () => { + const credentialId = 'global-cred-123'; + roleServiceMock.mockResolvedValue(['credential:owner']); + mockQueryBuilder.getRawMany.mockResolvedValue([]); // No project access + + // Credential exists but user doesn't have access through projects + findByCredentialMock.mockResolvedValue([ + { + credentialsId: credentialId, + projectId: 'otherProjectId', + role: 'credential:owner', + }, + ]); + + // But it's a global credential + hasGlobalReadOnlyAccessMock.mockReturnValue(true); + findGlobalCredentialByIdMock.mockResolvedValue({ id: credentialId, isGlobal: true }); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['credential:read'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { credentialId }); + + expect(hasGlobalReadOnlyAccessMock).toHaveBeenCalledWith(scopes); + expect(findGlobalCredentialByIdMock).toHaveBeenCalledWith(credentialId); + expect(result).toBe(true); + }); + + it('should not grant access to non-global credential when user lacks project access', async () => { + const credentialId = 'regular-cred-123'; + roleServiceMock.mockResolvedValue(['credential:owner']); + mockQueryBuilder.getRawMany.mockResolvedValue([]); // No project access + + findByCredentialMock.mockResolvedValue([ + { + credentialsId: credentialId, + projectId: 'otherProjectId', + role: 'credential:owner', + }, + ]); + + // Not a global credential + hasGlobalReadOnlyAccessMock.mockReturnValue(true); + findGlobalCredentialByIdMock.mockResolvedValue(null); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['credential:read'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { credentialId }); + + expect(result).toBe(false); + }); + + it('should not check global credentials if does not have global read only access', async () => { + const credentialId = 'global-cred-123'; + roleServiceMock.mockResolvedValue(['credential:owner']); + mockQueryBuilder.getRawMany.mockResolvedValue([]); // No project access + + findByCredentialMock.mockResolvedValue([ + { + credentialsId: credentialId, + projectId: 'otherProjectId', + role: 'credential:owner', + }, + ]); + + hasGlobalReadOnlyAccessMock.mockReturnValue(false); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['credential:update'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { credentialId }); + + // Should not check global credentials for non-read scopes + expect(hasGlobalReadOnlyAccessMock).toHaveBeenCalledWith(scopes); + expect(findGlobalCredentialByIdMock).not.toHaveBeenCalled(); + expect(result).toBe(false); + }); + + it('should not check global credentials when user has valid project access', async () => { + const credentialId = 'cred-123'; + roleServiceMock.mockResolvedValue(['credential:owner']); + mockQueryBuilder.getRawMany.mockResolvedValue([{ id: 'projectId' }]); + + findByCredentialMock.mockResolvedValue([ + { + credentialsId: credentialId, + projectId: 'projectId', + role: 'credential:owner', + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['credential:read'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { credentialId }); + + // Should not check global credentials when normal access works + expect(hasGlobalReadOnlyAccessMock).not.toHaveBeenCalled(); + expect(findGlobalCredentialByIdMock).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + }); }); diff --git a/packages/cli/src/permissions.ee/check-access.ts b/packages/cli/src/permissions.ee/check-access.ts index 585487c386c..0974b232f15 100644 --- a/packages/cli/src/permissions.ee/check-access.ts +++ b/packages/cli/src/permissions.ee/check-access.ts @@ -4,6 +4,7 @@ import { Container } from '@n8n/di'; import { hasGlobalScope, type Scope } from '@n8n/permissions'; import { UnexpectedError } from 'n8n-workflow'; +import { CredentialsFinderService } from '@/credentials/credentials-finder.service'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { RoleService } from '@/services/role.service'; @@ -60,9 +61,26 @@ export async function userHasScopes( const validRoles = await roleService.rolesWithScope('credential', scopes); - return credentials.some( + const hasValidRoles = credentials.some( (c) => userProjectIds.includes(c.projectId) && validRoles.includes(c.role), ); + + if (hasValidRoles) { + return true; + } + + // Check for global credentials with read-only access + const credentialsFinderService = Container.get(CredentialsFinderService); + if (credentialsFinderService.hasGlobalReadOnlyAccess(scopes)) { + const globalCredential = + await credentialsFinderService.findGlobalCredentialById(credentialId); + + if (globalCredential) { + return true; + } + } + + return false; } if (workflowId) { diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index db3fa8746ef..e3b3a91fc60 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -72,6 +72,7 @@ export declare namespace CredentialRequest { data: ICredentialDataDecryptedObject; projectId?: string; isManaged?: boolean; + isGlobal?: boolean; }>; type Get = AuthenticatedRequest<{ credentialId: string }, {}, {}, Record>; 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 a4ba74fad3a..1016058fa80 100644 --- a/packages/cli/src/services/__tests__/credentials-finder.service.test.ts +++ b/packages/cli/src/services/__tests__/credentials-finder.service.test.ts @@ -4,8 +4,9 @@ import { type SharedCredentials, CredentialsRepository, SharedCredentialsRepository, + CredentialsEntity, } from '@n8n/db'; -import type { CredentialsEntity, User } from '@n8n/db'; +import type { User } from '@n8n/db'; import { Container } from '@n8n/di'; import { PROJECT_ADMIN_ROLE_SLUG, @@ -35,6 +36,13 @@ describe('CredentialsFinderService', () => { beforeEach(() => { jest.clearAllMocks(); + // Setup manager mock for global credentials fetching + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + // @ts-ignore + credentialsRepository.manager = { + find: jest.fn().mockResolvedValue([]), + } as any; + // Default mock implementation for all tests roleService.rolesWithScope.mockImplementation(async (namespace) => { if (namespace === 'project') { @@ -119,8 +127,9 @@ describe('CredentialsFinderService', () => { expect(credential).toEqual(sharedCredential.credentials); }); - test('should return null when no shared credential is found', async () => { + test('should return null when no shared credential is found and not global', async () => { sharedCredentialsRepository.findOne.mockResolvedValueOnce(null); + credentialsRepository.findOne.mockResolvedValueOnce(null); const credential = await credentialsFinderService.findCredentialForUser( credentialsId, member, @@ -148,6 +157,64 @@ describe('CredentialsFinderService', () => { }, }, }); + expect(credentialsRepository.findOne).toHaveBeenCalledWith({ + where: { + id: credentialsId, + isGlobal: true, + }, + relations: { + shared: { project: { projectRelations: { user: true } } }, + }, + }); + expect(credential).toEqual(null); + }); + + test('should return global credential when not shared but is global for credential:read scope', async () => { + const globalCredential = mock({ id: credentialsId, isGlobal: true }); + sharedCredentialsRepository.findOne.mockResolvedValueOnce(null); + credentialsRepository.findOne.mockResolvedValueOnce(globalCredential); + + const credential = await credentialsFinderService.findCredentialForUser( + credentialsId, + member, + ['credential:read' as const], + ); + + expect(credentialsRepository.findOne).toHaveBeenCalledWith({ + where: { + id: credentialsId, + isGlobal: true, + }, + relations: { + shared: { project: { projectRelations: { user: true } } }, + }, + }); + expect(credential).toEqual(globalCredential); + }); + + test('should not fallback to global credential for write scopes', async () => { + sharedCredentialsRepository.findOne.mockResolvedValueOnce(null); + + const credential = await credentialsFinderService.findCredentialForUser( + credentialsId, + member, + ['credential:update' as const], + ); + + expect(credentialsRepository.findOne).not.toHaveBeenCalled(); + expect(credential).toEqual(null); + }); + + test('should not fallback to global credential for multiple scopes', async () => { + sharedCredentialsRepository.findOne.mockResolvedValueOnce(null); + + const credential = await credentialsFinderService.findCredentialForUser( + credentialsId, + member, + ['credential:read' as const, 'credential:update' as const], + ); + + expect(credentialsRepository.findOne).not.toHaveBeenCalled(); expect(credential).toEqual(null); }); @@ -215,17 +282,20 @@ describe('CredentialsFinderService', () => { test('should allow global owner access to all credentials without role filtering', async () => { credentialsRepository.find.mockResolvedValueOnce(credentials); - const result = await credentialsFinderService.findCredentialsForUser(owner, [ 'credential:read' as const, ]); expect(credentialsRepository.find).toHaveBeenCalledWith({ - where: {}, + where: { isGlobal: false }, + relations: { shared: true }, + }); + expect(credentialsRepository.manager.find).toHaveBeenCalledWith(CredentialsEntity, { + where: { isGlobal: true }, relations: { shared: true }, }); expect(roleService.rolesWithScope).not.toHaveBeenCalled(); - expect(result).toEqual(credentials); + expect(result).toEqual([...credentials]); }); test('should filter credentials by roles for regular members', async () => { @@ -239,6 +309,7 @@ describe('CredentialsFinderService', () => { expect(roleService.rolesWithScope).toHaveBeenCalledWith('credential', ['credential:update']); expect(credentialsRepository.find).toHaveBeenCalledWith({ where: { + isGlobal: false, shared: { role: In(['credential:owner', 'credential:user']), project: { @@ -256,9 +327,89 @@ describe('CredentialsFinderService', () => { }, relations: { shared: true }, }); + // Should NOT fetch global credentials for update scope + expect(credentialsRepository.manager.find).not.toHaveBeenCalled(); expect(result).toEqual(credentials); }); + test('should include global credentials when user has read-only access', async () => { + const mockGlobalCredentials = [ + mock({ id: 'global1', isGlobal: true }), + mock({ id: 'global2', isGlobal: true }), + ]; + credentialsRepository.find.mockResolvedValueOnce(credentials); + (credentialsRepository.manager.find as jest.Mock).mockResolvedValueOnce( + mockGlobalCredentials, + ); + + const result = await credentialsFinderService.findCredentialsForUser(member, [ + 'credential:read' as const, + ]); + + // Should include both non-global (cred1, cred2) and global (global1, global2) + expect(result).toHaveLength(4); + expect(result.map((c) => c.id)).toEqual(['cred1', 'cred2', 'global1', 'global2']); + }); + + test('should not include global credentials when user has write access', async () => { + const mockGlobalCredentials = [ + mock({ id: 'global1', isGlobal: true }), + mock({ id: 'global2', isGlobal: true }), + ]; + credentialsRepository.find.mockResolvedValueOnce(credentials); + (credentialsRepository.manager.find as jest.Mock).mockResolvedValueOnce( + mockGlobalCredentials, + ); + + const result = await credentialsFinderService.findCredentialsForUser(member, [ + 'credential:update' as const, + ]); + + // Should only include non-global credentials (cred1, cred2) + expect(result).toHaveLength(2); + expect(result.map((c) => c.id)).toEqual(['cred1', 'cred2']); + // Should not call fetchGlobalCredentials when not read-only + expect(credentialsRepository.manager.find).not.toHaveBeenCalled(); + }); + + test('should not include global credentials when user has multiple scopes', async () => { + const mockGlobalCredentials = [ + mock({ id: 'global1', isGlobal: true }), + mock({ id: 'global2', isGlobal: true }), + ]; + credentialsRepository.find.mockResolvedValueOnce(credentials); + (credentialsRepository.manager.find as jest.Mock).mockResolvedValueOnce( + mockGlobalCredentials, + ); + + const result = await credentialsFinderService.findCredentialsForUser(member, [ + 'credential:read' as const, + 'credential:list' as const, + ]); + + // Should only include non-global credentials (cred1, cred2) + expect(result).toHaveLength(2); + expect(result.map((c) => c.id)).toEqual(['cred1', 'cred2']); + // Should not call fetchGlobalCredentials when multiple scopes + expect(credentialsRepository.manager.find).not.toHaveBeenCalled(); + }); + + test('should handle empty global credentials list with read-only access', async () => { + credentialsRepository.find.mockResolvedValueOnce(credentials); + (credentialsRepository.manager.find as jest.Mock).mockResolvedValueOnce([]); + + const result = await credentialsFinderService.findCredentialsForUser(member, [ + 'credential:read' as const, + ]); + + expect(result).toEqual(credentials); + // Should call fetchGlobalCredentials when read-only + expect(credentialsRepository.manager.find).toHaveBeenCalledWith(CredentialsEntity, { + where: { isGlobal: true }, + relations: { shared: true }, + }); + }); + test('should handle custom roles in filtering', async () => { roleService.rolesWithScope.mockImplementation(async (namespace) => { if (namespace === 'project') return ['custom:project-lead-456']; @@ -275,6 +426,7 @@ describe('CredentialsFinderService', () => { expect(credentialsRepository.find).toHaveBeenCalledWith({ where: { + isGlobal: false, shared: { role: In(['custom:cred-admin-789']), project: { @@ -394,6 +546,167 @@ describe('CredentialsFinderService', () => { mockTrx, ); }); + + test('should include global credentials when includeGlobalCredentials flag is true', async () => { + const globalCredential = mock({ + id: 'global1', + isGlobal: true, + shared: [ + mock({ + credentialsId: 'global1', + role: 'credential:owner', + projectId: 'proj-owner', + }), + ], + }); + + sharedCredentialsRepository.findCredentialsWithOptions.mockResolvedValueOnce( + sharedCredentials, + ); + credentialsRepository.manager.find = jest.fn().mockResolvedValueOnce([globalCredential]); + + const result = await credentialsFinderService.findAllCredentialsForUser( + member, + ['credential:read' as const], + undefined, + { includeGlobalCredentials: true }, + ); + + expect(credentialsRepository.manager.find).toHaveBeenCalledWith(CredentialsEntity, { + where: { isGlobal: true }, + relations: { shared: true }, + }); + expect(result).toHaveLength(3); + expect(result[2]).toEqual({ ...globalCredential, projectId: 'proj-owner' }); + }); + + test('should not include global credentials when includeGlobalCredentials flag is false', async () => { + sharedCredentialsRepository.findCredentialsWithOptions.mockResolvedValueOnce( + sharedCredentials, + ); + + const result = await credentialsFinderService.findAllCredentialsForUser( + member, + ['credential:read' as const], + undefined, + { includeGlobalCredentials: false }, + ); + + expect(credentialsRepository.manager.find).not.toHaveBeenCalled(); + expect(result).toHaveLength(2); + }); + + test('should not include global credentials when no options provided', async () => { + sharedCredentialsRepository.findCredentialsWithOptions.mockResolvedValueOnce( + sharedCredentials, + ); + + const result = await credentialsFinderService.findAllCredentialsForUser(member, [ + 'credential:read' as const, + ]); + + expect(credentialsRepository.manager.find).not.toHaveBeenCalled(); + expect(result).toHaveLength(2); + }); + + test('should skip global credentials without valid projectId', async () => { + const globalCredentialWithoutProject = mock({ + id: 'global-no-proj', + isGlobal: true, + shared: [ + mock({ + credentialsId: 'global-no-proj', + role: 'credential:user', + projectId: undefined as any, + }), + ], + }); + + const globalCredentialWithProject = mock({ + id: 'global-with-proj', + isGlobal: true, + shared: [ + mock({ + credentialsId: 'global-with-proj', + role: 'credential:owner', + projectId: 'proj-owner', + }), + ], + }); + + sharedCredentialsRepository.findCredentialsWithOptions.mockResolvedValueOnce([]); + credentialsRepository.manager.find = jest + .fn() + .mockResolvedValueOnce([globalCredentialWithoutProject, globalCredentialWithProject]); + + const result = await credentialsFinderService.findAllCredentialsForUser( + member, + ['credential:read' as const], + undefined, + { includeGlobalCredentials: true }, + ); + + // Should only include the credential with valid projectId + expect(result).toHaveLength(1); + expect(result[0].id).toEqual('global-with-proj'); + }); + + test('should deduplicate global credentials with shared credentials', async () => { + const sharedGlobalCred = mock({ + credentials: mock({ id: 'cred1' }), + projectId: 'proj1', + credentialsId: 'cred1', + role: 'credential:owner', + }); + + const globalCredential = mock({ + id: 'cred1', // Same ID as shared credential + isGlobal: true, + shared: [ + mock({ + credentialsId: 'cred1', + role: 'credential:owner', + projectId: 'proj-owner', + }), + ], + }); + + sharedCredentialsRepository.findCredentialsWithOptions.mockResolvedValueOnce([ + sharedGlobalCred, + ]); + credentialsRepository.manager.find = jest.fn().mockResolvedValueOnce([globalCredential]); + + const result = await credentialsFinderService.findAllCredentialsForUser( + member, + ['credential:read' as const], + undefined, + { includeGlobalCredentials: true }, + ); + + // Should not duplicate cred1 + expect(result).toHaveLength(1); + expect(result[0].id).toEqual('cred1'); + }); + + test('should use transaction manager for fetching global credentials', async () => { + const mockTrx = mock(); + const mockFind = jest.fn().mockResolvedValueOnce([]); + mockTrx.find = mockFind; + + sharedCredentialsRepository.findCredentialsWithOptions.mockResolvedValueOnce([]); + + await credentialsFinderService.findAllCredentialsForUser( + member, + ['credential:read' as const], + mockTrx, + { includeGlobalCredentials: true }, + ); + + expect(mockFind).toHaveBeenCalledWith(CredentialsEntity, { + where: { isGlobal: true }, + relations: { shared: true }, + }); + }); }); describe('getCredentialIdsByUserAndRole', () => { @@ -518,6 +831,7 @@ describe('CredentialsFinderService', () => { expect(credentialsRepository.find).toHaveBeenCalledWith({ where: { + isGlobal: false, shared: { role: In([]), project: { @@ -569,6 +883,7 @@ describe('CredentialsFinderService', () => { expect(credentialsRepository.find).toHaveBeenCalledWith({ where: { + isGlobal: false, shared: { role: In(['project:admin']), // Uses what RoleService returned for credential namespace project: { @@ -584,4 +899,127 @@ describe('CredentialsFinderService', () => { expect(result).toEqual(isolationResult); }); }); + + describe('hasGlobalReadOnlyAccess', () => { + test('should return true for single credential:read scope', () => { + const result = credentialsFinderService.hasGlobalReadOnlyAccess(['credential:read']); + + expect(result).toBe(true); + }); + + test('should return false for multiple scopes including credential:read', () => { + const result = credentialsFinderService.hasGlobalReadOnlyAccess([ + 'credential:read', + 'credential:update', + ]); + + expect(result).toBe(false); + }); + + test('should return false for single non-read scope', () => { + const result = credentialsFinderService.hasGlobalReadOnlyAccess(['credential:update']); + + expect(result).toBe(false); + }); + + test('should return false for empty scopes array', () => { + const result = credentialsFinderService.hasGlobalReadOnlyAccess([]); + + expect(result).toBe(false); + }); + + test('should return false for credential:delete scope', () => { + const result = credentialsFinderService.hasGlobalReadOnlyAccess(['credential:delete']); + + expect(result).toBe(false); + }); + + test('should return false for credential:shareGlobally scope', () => { + const result = credentialsFinderService.hasGlobalReadOnlyAccess(['credential:shareGlobally']); + + expect(result).toBe(false); + }); + }); + + describe('findGlobalCredentialById', () => { + const credentialId = 'cred-123'; + const mockGlobalCredential = mock({ + id: credentialId, + name: 'Global Test Credential', + type: 'testApi', + isGlobal: true, + }); + + test('should find global credential by ID without relations', async () => { + credentialsRepository.findOne.mockResolvedValueOnce(mockGlobalCredential); + + const result = await credentialsFinderService.findGlobalCredentialById(credentialId); + + expect(credentialsRepository.findOne).toHaveBeenCalledWith({ + where: { + id: credentialId, + isGlobal: true, + }, + relations: undefined, + }); + expect(result).toEqual(mockGlobalCredential); + }); + + test('should find global credential by ID with relations', async () => { + const relations = { shared: { project: { projectRelations: { user: true } } } }; + credentialsRepository.findOne.mockResolvedValueOnce(mockGlobalCredential); + + const result = await credentialsFinderService.findGlobalCredentialById( + credentialId, + relations, + ); + + expect(credentialsRepository.findOne).toHaveBeenCalledWith({ + where: { + id: credentialId, + isGlobal: true, + }, + relations, + }); + expect(result).toEqual(mockGlobalCredential); + }); + + test('should return null when global credential not found', async () => { + credentialsRepository.findOne.mockResolvedValueOnce(null); + + const result = await credentialsFinderService.findGlobalCredentialById('non-existent-id'); + + expect(credentialsRepository.findOne).toHaveBeenCalledWith({ + where: { + id: 'non-existent-id', + isGlobal: true, + }, + relations: undefined, + }); + expect(result).toBeNull(); + }); + + test('should only query for global credentials', async () => { + credentialsRepository.findOne.mockResolvedValueOnce(null); + + await credentialsFinderService.findGlobalCredentialById(credentialId); + + expect(credentialsRepository.findOne).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ + isGlobal: true, + }), + }), + ); + }); + + test('should handle repository errors', async () => { + const error = new Error('Database connection failed'); + credentialsRepository.findOne.mockRejectedValueOnce(error); + + await expect(credentialsFinderService.findGlobalCredentialById(credentialId)).rejects.toThrow( + 'Database connection failed', + ); + }); + }); }); diff --git a/packages/cli/src/services/role.service.ts b/packages/cli/src/services/role.service.ts index f634259a43c..88d19b3484b 100644 --- a/packages/cli/src/services/role.service.ts +++ b/packages/cli/src/services/role.service.ts @@ -246,12 +246,17 @@ export class RoleService { throw new UnexpectedError('Cannot detect if entity is a workflow or credential.'); } - entity.scopes = this.combineResourceScopes( - 'active' in entity ? 'workflow' : 'credential', - user, - shared, - userProjectRelations, - ); + const entityType = 'active' in entity ? 'workflow' : 'credential'; + entity.scopes = this.combineResourceScopes(entityType, user, shared, userProjectRelations); + + if ( + entityType === 'credential' && + 'isGlobal' in entity && + entity.isGlobal && + !entity.scopes.includes('credential:read') + ) { + entity.scopes.push('credential:read'); + } return entity; } diff --git a/packages/cli/src/workflows/__tests__/workflows.controller.test.ts b/packages/cli/src/workflows/__tests__/workflows.controller.test.ts index 3f501a8c911..0beaffb2164 100644 --- a/packages/cli/src/workflows/__tests__/workflows.controller.test.ts +++ b/packages/cli/src/workflows/__tests__/workflows.controller.test.ts @@ -1,5 +1,6 @@ import type { ImportWorkflowFromUrlDto } from '@n8n/api-types'; -import type { AuthenticatedRequest, IExecutionResponse } from '@n8n/db'; +import type { AuthenticatedRequest, IExecutionResponse, CredentialsEntity, User } from '@n8n/db'; +import { WorkflowEntity } from '@n8n/db'; import axios from 'axios'; import type { Response } from 'express'; import { mock } from 'jest-mock-extended'; @@ -8,6 +9,10 @@ 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 type { ExecutionService } from '@/executions/execution.service'; +import type { CredentialsService } from '@/credentials/credentials.service'; +import type { EnterpriseWorkflowService } from '@/workflows/workflow.service.ee'; +import type { License } from '@/license'; +import type { WorkflowRequest } from '../workflow.request'; import type { ProjectService } from '@/services/project.service.ee'; import { WorkflowsController } from '../workflows.controller'; @@ -171,4 +176,128 @@ describe('WorkflowsController', () => { expect(executionService.getLastSuccessfulExecution).toHaveBeenCalledWith(workflowId); }); }); + + describe('create', () => { + describe('credential retrieval for workflow creation', () => { + it('should include global credentials when checking credential permissions', async () => { + /** + * Arrange + */ + const mockUser = mock({ id: 'user-123' }); + const mockRequest = mock({ + user: mockUser, + body: { + name: 'Test Workflow', + nodes: [], + connections: {}, + }, + }); + + const mockGlobalCredential = mock({ + id: 'global-cred-123', + name: 'Global Credential', + type: 'httpBasicAuth', + isGlobal: true, + }); + + const mockPersonalCredential = mock({ + id: 'personal-cred-456', + name: 'Personal Credential', + type: 'httpBasicAuth', + isGlobal: false, + }); + + const credentialsService = mock(); + const enterpriseWorkflowService = mock(); + const license = mock(); + + credentialsService.getMany.mockResolvedValue([ + mockGlobalCredential, + mockPersonalCredential, + ]); + license.isSharingEnabled.mockReturnValue(true); + + // Stop execution after credential validation + enterpriseWorkflowService.validateCredentialPermissionsToUser.mockImplementation(() => { + throw new BadRequestError('Stopping execution for test'); + }); + + controller.credentialsService = credentialsService; + controller.enterpriseWorkflowService = enterpriseWorkflowService; + controller.license = license; + controller.externalHooks = mock(); + controller.externalHooks.run = jest.fn().mockResolvedValue(undefined); + controller.tagRepository = mock(); + controller.globalConfig = { tags: { disabled: true } }; + + /** + * Act & Assert + */ + await expect(controller.create(mockRequest)).rejects.toThrow(BadRequestError); + + /** + * Assert - Verify credentials were fetched with includeGlobal: true + */ + expect(credentialsService.getMany).toHaveBeenCalledWith(mockUser, { + includeGlobal: true, + }); + expect(enterpriseWorkflowService.validateCredentialPermissionsToUser).toHaveBeenCalledWith( + expect.any(WorkflowEntity), + [mockGlobalCredential, mockPersonalCredential], + ); + }); + + it('should throw BadRequestError when user lacks access to credentials in workflow', async () => { + /** + * Arrange + */ + const mockUser = mock({ id: 'user-123' }); + const mockRequest = mock({ + user: mockUser, + body: { + name: 'Test Workflow', + nodes: [], + connections: {}, + }, + }); + + const mockGlobalCredential = mock({ + id: 'global-cred-123', + name: 'Global Credential', + type: 'httpBasicAuth', + isGlobal: true, + }); + + const credentialsService = mock(); + const enterpriseWorkflowService = mock(); + const license = mock(); + + credentialsService.getMany.mockResolvedValue([mockGlobalCredential]); + license.isSharingEnabled.mockReturnValue(true); + enterpriseWorkflowService.validateCredentialPermissionsToUser.mockImplementation(() => { + throw new Error('User does not have access'); + }); + + controller.credentialsService = credentialsService; + controller.enterpriseWorkflowService = enterpriseWorkflowService; + controller.license = license; + controller.externalHooks = mock(); + controller.externalHooks.run = jest.fn().mockResolvedValue(undefined); + controller.tagRepository = mock(); + controller.globalConfig = { tags: { disabled: true } }; + + /** + * Act & Assert + */ + await expect(controller.create(mockRequest)).rejects.toThrow(BadRequestError); + await expect(controller.create(mockRequest)).rejects.toThrow( + 'The workflow you are trying to save contains credentials that are not shared with you', + ); + + expect(credentialsService.getMany).toHaveBeenCalledWith(mockUser, { + includeGlobal: true, + }); + }); + }); + }); }); diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index bd4095b5d42..630e0128d3e 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -120,7 +120,9 @@ export class WorkflowsController { // This is a new workflow, so we simply check if the user has access to // all used credentials - const allCredentials = await this.credentialsService.getMany(req.user); + const allCredentials = await this.credentialsService.getMany(req.user, { + includeGlobal: true, + }); try { this.enterpriseWorkflowService.validateCredentialPermissionsToUser( diff --git a/packages/cli/test/integration/ai/ai.api.test.ts b/packages/cli/test/integration/ai/ai.api.test.ts index a7009028a6d..77d1d8705dc 100644 --- a/packages/cli/test/integration/ai/ai.api.test.ts +++ b/packages/cli/test/integration/ai/ai.api.test.ts @@ -74,6 +74,7 @@ describe('POST /ai/free-credits', () => { 'credential:move', 'credential:read', 'credential:share', + 'credential:shareGlobally', 'credential:update', ].sort(), ); diff --git a/packages/cli/test/integration/credentials/credentials.api.test.ts b/packages/cli/test/integration/credentials/credentials.api.test.ts index eb5fff028ec..243adccdf28 100644 --- a/packages/cli/test/integration/credentials/credentials.api.test.ts +++ b/packages/cli/test/integration/credentials/credentials.api.test.ts @@ -216,6 +216,7 @@ describe('GET /credentials', () => { 'credential:move', 'credential:read', 'credential:share', + 'credential:shareGlobally', 'credential:update', ].sort(), ); @@ -230,6 +231,7 @@ describe('GET /credentials', () => { 'credential:move', 'credential:read', 'credential:share', + 'credential:shareGlobally', 'credential:update', ].sort(), ); @@ -356,6 +358,7 @@ describe('GET /credentials', () => { 'credential:read', 'credential:update', 'credential:share', + 'credential:shareGlobally', 'credential:delete', 'credential:create', 'credential:list', @@ -370,6 +373,7 @@ describe('GET /credentials', () => { 'credential:read', 'credential:update', 'credential:share', + 'credential:shareGlobally', 'credential:delete', 'credential:create', 'credential:list', @@ -387,6 +391,7 @@ describe('GET /credentials', () => { 'credential:read', 'credential:update', 'credential:share', + 'credential:shareGlobally', 'credential:delete', 'credential:create', 'credential:list', @@ -924,6 +929,57 @@ describe('POST /credentials', () => { message: "You don't have the permissions to save the credential in this project.", }); }); + + test('should fail when member tries to create credential with isGlobal=true', async () => { + const response = await authMemberAgent + .post('/credentials') + .send({ ...randomCredentialPayload(), isGlobal: true }); + + expect(response.statusCode).toBe(403); + expect(response.body.message).toBe( + 'You do not have permission to create globally shared credentials', + ); + }); + + test('should allow owner to create credential with isGlobal=true', async () => { + const response = await authOwnerAgent + .post('/credentials') + .send({ ...randomCredentialPayload(), isGlobal: true }); + + expect(response.statusCode).toBe(200); + + const credential = await Container.get(CredentialsRepository).findOneByOrFail({ + id: response.body.data.id, + }); + expect(credential.isGlobal).toBe(true); + }); + + test('should allow member to create credential with isGlobal=false', async () => { + const response = await authMemberAgent + .post('/credentials') + .send({ ...randomCredentialPayload(), isGlobal: false }); + + expect(response.statusCode).toBe(200); + + const credential = await Container.get(CredentialsRepository).findOneByOrFail({ + id: response.body.data.id, + }); + expect(credential.isGlobal).toBe(false); + }); + + test('should allow member to create credential without passing isGlobal', async () => { + const payload = randomCredentialPayload(); + delete payload.isGlobal; + + const response = await authMemberAgent.post('/credentials').send(payload); + + expect(response.statusCode).toBe(200); + + const credential = await Container.get(CredentialsRepository).findOneByOrFail({ + id: response.body.data.id, + }); + expect(credential.isGlobal).toBe(false); + }); }); describe('DELETE /credentials/:id', () => { @@ -1071,6 +1127,7 @@ describe('PATCH /credentials/:id', () => { 'credential:move', 'credential:read', 'credential:share', + 'credential:shareGlobally', 'credential:update', ].sort(), ); @@ -1296,6 +1353,69 @@ describe('PATCH /credentials/:id', () => { expect(response.statusCode).toBe(400); }); + + test('should fail when member tries to change isGlobal value', async () => { + const savedCredential = await saveCredential(randomCredentialPayload(), { + user: member, + role: 'credential:owner', + }); + + const response = await authMemberAgent + .patch(`/credentials/${savedCredential.id}`) + .send({ ...randomCredentialPayload(), isGlobal: true }); + + expect(response.statusCode).toBe(403); + expect(response.body.message).toBe( + 'You do not have permission to change global sharing for credentials', + ); + }); + + test('should allow owner to set isGlobal to true', async () => { + const savedCredential = await saveCredential(randomCredentialPayload(), { + user: owner, + role: 'credential:owner', + }); + + const response = await authOwnerAgent + .patch(`/credentials/${savedCredential.id}`) + .send({ ...randomCredentialPayload(), isGlobal: true }); + + expect(response.statusCode).toBe(200); + + const credential = await Container.get(CredentialsRepository).findOneByOrFail({ + id: savedCredential.id, + }); + expect(credential.isGlobal).toBe(true); + }); + + test('should allow member to update credential with same isGlobal value', async () => { + const savedCredential = await saveCredential(randomCredentialPayload({ isGlobal: false }), { + user: member, + role: 'credential:owner', + }); + + const response = await authMemberAgent + .patch(`/credentials/${savedCredential.id}`) + .send({ ...randomCredentialPayload(), isGlobal: false }); + + expect(response.statusCode).toBe(200); + }); + + test('should allow member to update credential without passing isGlobal', async () => { + const savedCredential = await saveCredential(randomCredentialPayload({ isGlobal: false }), { + user: member, + role: 'credential:owner', + }); + + const payload = randomCredentialPayload(); + delete payload.isGlobal; + + const response = await authMemberAgent + .patch(`/credentials/${savedCredential.id}`) + .send(payload); + + expect(response.statusCode).toBe(200); + }); }); describe('GET /credentials/new', () => { diff --git a/packages/cli/test/integration/environments/source-control-export.service.test.ts b/packages/cli/test/integration/environments/source-control-export.service.test.ts index 0b4ed2d46a0..b66f6fe4145 100644 --- a/packages/cli/test/integration/environments/source-control-export.service.test.ts +++ b/packages/cli/test/integration/environments/source-control-export.service.test.ts @@ -421,5 +421,92 @@ describe('SourceControlExportService Integration', () => { expect((exportedCredential.data.connection as any).port).toBe(5432); expect((exportedCredential.data.settings as any).ssl).toBe(true); }); + + it('should export global credentials with isGlobal flag set to true', async () => { + // Arrange + const credentialData = { + apiKey: 'global-api-key', + apiSecret: 'global-secret', + }; + + const credential = await createCredentials( + { + name: 'Global Test Credential', + type: 'globalCredentialType', + data: Container.get(Cipher).encrypt(credentialData), + isGlobal: true, + }, + personalProject, + ); + + const candidates = [{ id: credential.id }] as SourceControlledFile[]; + + // Act + const result = await exportService.exportCredentialsToWorkFolder(candidates); + + // Assert + expect(result.count).toBe(1); + expect(result.files).toHaveLength(1); + + // Verify file write was called + expect(mockFsWriteFile).toHaveBeenCalledTimes(1); + + const exportedCredential = getWrittenCredentialData(credential.id); + + // Verify isGlobal flag is exported + expect(exportedCredential).toMatchObject({ + id: credential.id, + name: 'Global Test Credential', + type: 'globalCredentialType', + isGlobal: true, + data: { + apiKey: '', + apiSecret: '', + }, + }); + + // Verify isGlobal is explicitly true + expect(exportedCredential.isGlobal).toBe(true); + }); + + it('should export non-global credentials with isGlobal flag set to false', async () => { + // Arrange + const credentialData = { + username: 'test-user', + password: 'test-password', + }; + + const credential = await createCredentials( + { + name: 'Non-Global Credential', + type: 'standardCredentialType', + data: Container.get(Cipher).encrypt(credentialData), + isGlobal: false, + }, + teamProject, + ); + + const candidates = [{ id: credential.id }] as SourceControlledFile[]; + + // Act + const result = await exportService.exportCredentialsToWorkFolder(candidates); + + // Assert + expect(result.count).toBe(1); + expect(mockFsWriteFile).toHaveBeenCalledTimes(1); + + const exportedCredential = getWrittenCredentialData(credential.id); + + // Verify isGlobal flag is false + expect(exportedCredential).toMatchObject({ + id: credential.id, + name: 'Non-Global Credential', + type: 'standardCredentialType', + isGlobal: false, + }); + + // Verify isGlobal is explicitly false + expect(exportedCredential.isGlobal).toBe(false); + }); }); }); diff --git a/packages/cli/test/integration/environments/source-control-import.service.test.ts b/packages/cli/test/integration/environments/source-control-import.service.test.ts index 9f2581cfc4d..634ed06cdd2 100644 --- a/packages/cli/test/integration/environments/source-control-import.service.test.ts +++ b/packages/cli/test/integration/environments/source-control-import.service.test.ts @@ -1390,5 +1390,75 @@ describe('SourceControlImportService', () => { }, ]); }); + + it('should import global credentials with isGlobal flag set to true', async () => { + const importingUser = await getGlobalOwner(); + + fsp.readFile = jest.fn().mockResolvedValue(Buffer.from('some-content')); + + const CREDENTIAL_ID = nanoid(); + + const stub: ExportableCredential = { + id: CREDENTIAL_ID, + name: 'Global Test Credential', + type: 'globalCredentialType', + data: {}, + ownedBy: null, + isGlobal: true, + }; + + jest.spyOn(utils, 'jsonParse').mockReturnValue(stub); + + cipher.encrypt.mockReturnValue('some-encrypted-data'); + + await service.importCredentialsFromWorkFolder( + [mock({ id: CREDENTIAL_ID })], + importingUser.id, + ); + + const importedCredential = await credentialsRepository.findOneBy({ + id: CREDENTIAL_ID, + }); + + expect(importedCredential).toBeTruthy(); + expect(importedCredential?.isGlobal).toBe(true); + expect(importedCredential?.name).toBe('Global Test Credential'); + expect(importedCredential?.type).toBe('globalCredentialType'); + }); + + it('should import non-global credentials with isGlobal flag set to false', async () => { + const importingUser = await getGlobalOwner(); + + fsp.readFile = jest.fn().mockResolvedValue(Buffer.from('some-content')); + + const CREDENTIAL_ID = nanoid(); + + const stub: ExportableCredential = { + id: CREDENTIAL_ID, + name: 'Standard Credential', + type: 'standardCredentialType', + data: {}, + ownedBy: null, + isGlobal: false, + }; + + jest.spyOn(utils, 'jsonParse').mockReturnValue(stub); + + cipher.encrypt.mockReturnValue('some-encrypted-data'); + + await service.importCredentialsFromWorkFolder( + [mock({ id: CREDENTIAL_ID })], + importingUser.id, + ); + + const importedCredential = await credentialsRepository.findOneBy({ + id: CREDENTIAL_ID, + }); + + expect(importedCredential).toBeTruthy(); + expect(importedCredential?.isGlobal).toBe(false); + expect(importedCredential?.name).toBe('Standard Credential'); + expect(importedCredential?.type).toBe('standardCredentialType'); + }); }); }); diff --git a/packages/cli/test/integration/environments/source-control.service.test.ts b/packages/cli/test/integration/environments/source-control.service.test.ts index 1af38df1bcf..c05f5da159f 100644 --- a/packages/cli/test/integration/environments/source-control.service.test.ts +++ b/packages/cli/test/integration/environments/source-control.service.test.ts @@ -89,6 +89,7 @@ function toExportableCredential( name: cred.name, type: cred.type, ownedBy: resourceOwner, + isGlobal: cred.isGlobal ?? false, }; } @@ -1161,4 +1162,198 @@ describe('SourceControlService', () => { }); }); }); + + describe('isGlobal flag modification detection', () => { + let testGlobalOwner: User; + let testProject: Project; + + beforeAll(async () => { + testGlobalOwner = await createUser({ role: GLOBAL_OWNER_ROLE }); + testProject = await createTeamProject('TestProjectForGlobal', testGlobalOwner); + }); + + afterEach(() => { + globMock.mockClear(); + fsReadFile.mockClear(); + }); + + const setupMocksForCredential = ( + credential: CredentialsEntity, + remoteCredential: ExportableCredential, + ) => { + const testGitFiles = { + [`${SOURCE_CONTROL_CREDENTIAL_EXPORT_FOLDER}/${credential.id}.json`]: remoteCredential, + [SOURCE_CONTROL_TAGS_EXPORT_FILE]: { tags: [], mappings: [] }, + [SOURCE_CONTROL_FOLDERS_EXPORT_FILE]: { folders: [] }, + }; + + globMock.mockImplementation(async (path, opts) => { + if (opts.cwd?.endsWith(SOURCE_CONTROL_WORKFLOW_EXPORT_FOLDER)) { + return []; + } else if (opts.cwd?.endsWith(SOURCE_CONTROL_CREDENTIAL_EXPORT_FOLDER)) { + return Object.keys(testGitFiles).filter((file) => + file.startsWith(SOURCE_CONTROL_CREDENTIAL_EXPORT_FOLDER), + ); + } else if (path === SOURCE_CONTROL_FOLDERS_EXPORT_FILE) { + return [SOURCE_CONTROL_FOLDERS_EXPORT_FILE]; + } else if (path === SOURCE_CONTROL_TAGS_EXPORT_FILE) { + return [SOURCE_CONTROL_TAGS_EXPORT_FILE]; + } + return []; + }); + + fsReadFile.mockImplementation(async (file) => { + const fileName = basename(file as string); + const fullPath = Object.keys(testGitFiles).find((key) => key.endsWith(fileName)); + if (fullPath) { + return Buffer.from(JSON.stringify(testGitFiles[fullPath])); + } + return Buffer.from('{}'); + }); + }; + + it('should detect credential as modified when isGlobal changes from false to true', async () => { + // Create a test credential with isGlobal: false + const credential = await createCredentials( + { + name: 'Test Credential isGlobal false->true', + type: 'testType', + data: cipher.encrypt({}), + isGlobal: false, + }, + testProject, + ); + + // Setup: Mock remote credential with isGlobal: true + const remoteCredential = toExportableCredential(credential, testProject); + remoteCredential.isGlobal = true; + setupMocksForCredential(credential, remoteCredential); + + // Act + const result = (await service.getStatus(testGlobalOwner, { + direction: 'push', + preferLocalVersion: true, + verbose: false, + })) as SourceControlledFile[]; + + // Assert + const modifiedCredentials = result.filter( + (r: SourceControlledFile) => r.type === 'credential' && r.status === 'modified', + ); + + expect(modifiedCredentials.some((c) => c.id === credential.id)).toBe(true); + }); + + it('should detect credential as modified when isGlobal changes from true to false', async () => { + const credential = await createCredentials( + { + name: 'Test Credential isGlobal true->false', + type: 'testType', + data: cipher.encrypt({}), + isGlobal: true, + }, + testProject, + ); + + const remoteCredential = toExportableCredential(credential, testProject); + remoteCredential.isGlobal = false; + setupMocksForCredential(credential, remoteCredential); + + const result = (await service.getStatus(testGlobalOwner, { + direction: 'push', + preferLocalVersion: true, + verbose: false, + })) as SourceControlledFile[]; + + const modifiedCredentials = result.filter( + (r: SourceControlledFile) => r.type === 'credential' && r.status === 'modified', + ); + + expect(modifiedCredentials.some((c) => c.id === credential.id)).toBe(true); + }); + + it('should NOT detect credential as modified when isGlobal is undefined vs false', async () => { + const credential = await createCredentials( + { + name: 'Test Credential isGlobal undefined vs false', + type: 'testType', + data: cipher.encrypt({}), + isGlobal: false, + }, + testProject, + ); + + const remoteCredential = toExportableCredential(credential, testProject); + delete remoteCredential.isGlobal; + setupMocksForCredential(credential, remoteCredential); + + const result = (await service.getStatus(testGlobalOwner, { + direction: 'push', + preferLocalVersion: true, + verbose: false, + })) as SourceControlledFile[]; + + const modifiedCredentials = result.filter( + (r: SourceControlledFile) => r.type === 'credential' && r.status === 'modified', + ); + + expect(modifiedCredentials.some((c) => c.id === credential.id)).toBe(false); + }); + + it('should detect credential as modified when isGlobal changes from undefined to true', async () => { + const credential = await createCredentials( + { + name: 'Test Credential isGlobal undefined->true', + type: 'testType', + data: cipher.encrypt({}), + isGlobal: false, + }, + testProject, + ); + + const remoteCredential = toExportableCredential(credential, testProject); + remoteCredential.isGlobal = true; + setupMocksForCredential(credential, remoteCredential); + + const result = (await service.getStatus(testGlobalOwner, { + direction: 'push', + preferLocalVersion: true, + verbose: false, + })) as SourceControlledFile[]; + + const modifiedCredentials = result.filter( + (r: SourceControlledFile) => r.type === 'credential' && r.status === 'modified', + ); + + expect(modifiedCredentials.some((c) => c.id === credential.id)).toBe(true); + }); + + it('should NOT detect credential as modified when isGlobal is the same', async () => { + const credential = await createCredentials( + { + name: 'Test Credential isGlobal same value', + type: 'testType', + data: cipher.encrypt({}), + isGlobal: true, + }, + testProject, + ); + + const remoteCredential = toExportableCredential(credential, testProject); + remoteCredential.isGlobal = true; + setupMocksForCredential(credential, remoteCredential); + + const result = (await service.getStatus(testGlobalOwner, { + direction: 'push', + preferLocalVersion: true, + verbose: false, + })) as SourceControlledFile[]; + + const modifiedCredentials = result.filter( + (r: SourceControlledFile) => r.type === 'credential' && r.status === 'modified', + ); + + expect(modifiedCredentials.some((c) => c.id === credential.id)).toBe(false); + }); + }); }); diff --git a/packages/cli/test/integration/services/role.service.test.ts b/packages/cli/test/integration/services/role.service.test.ts index 5896a45ca67..0fe02b5338c 100644 --- a/packages/cli/test/integration/services/role.service.test.ts +++ b/packages/cli/test/integration/services/role.service.test.ts @@ -1397,5 +1397,285 @@ describe('RoleService', () => { roleService.addScopes(mockEntity, user, userProjectRelations); }).toThrow('Cannot detect if entity is a workflow or credential.'); }); + + it('should add credential:read scope to global credentials', async () => { + // + // ARRANGE + // + const user = await createMember(); + const mockGlobalCredential = { + id: 'global-cred-1', + name: 'Global Test Credential', + type: 'testCredential', + isGlobal: true, + shared: [ + { + projectId: 'project-1', + role: 'credential:owner', + }, + ], + } as any; + const userProjectRelations = [] as any[]; + + // + // ACT + // + const result = roleService.addScopes(mockGlobalCredential, user, userProjectRelations); + + // + // ASSERT + // + expect(result).toHaveProperty('scopes'); + expect(result.scopes).toContain('credential:read'); + }); + + it('should add credential:read scope to global credentials even when not in initial scopes', async () => { + // + // ARRANGE + // + const user = await createMember(); + const mockGlobalCredential = { + id: 'global-cred-2', + name: 'Global Test Credential 2', + type: 'testCredential', + isGlobal: true, + shared: [], + } as any; + const userProjectRelations = [] as any[]; + + // + // ACT + // + const result = roleService.addScopes(mockGlobalCredential, user, userProjectRelations); + + // + // ASSERT + // + expect(result).toHaveProperty('scopes'); + expect(result.scopes).toContain('credential:read'); + }); + + it('should not duplicate credential:read scope if already present for global credentials', async () => { + // + // ARRANGE + // + const user = await createMember(); + const mockGlobalCredential = { + id: 'global-cred-3', + name: 'Global Test Credential 3', + type: 'testCredential', + isGlobal: true, + shared: [ + { + projectId: 'project-1', + role: 'credential:owner', + }, + ], + } as any; + const userProjectRelations = [] as any[]; + + // + // ACT + // + const result = roleService.addScopes(mockGlobalCredential, user, userProjectRelations); + + // + // ASSERT + // + const readScopeCount = result.scopes.filter((s: string) => s === 'credential:read').length; + expect(readScopeCount).toBe(1); + }); + + it('should not add credential:read scope to non-global credentials', async () => { + // + // ARRANGE + // + const user = await createMember(); + const mockNonGlobalCredential = { + id: 'non-global-cred-1', + name: 'Non-Global Test Credential', + type: 'testCredential', + isGlobal: false, + shared: [ + { + projectId: 'project-1', + role: 'credential:user', + }, + ], + } as any; + const userProjectRelations = [] as any[]; + + // + // ACT + // + const result = roleService.addScopes(mockNonGlobalCredential, user, userProjectRelations); + + // + // ASSERT + // + expect(result).toHaveProperty('scopes'); + // Should not contain credential:read because the user only has credential:user role + // and isGlobal is false + expect(result.scopes).not.toContain('credential:read'); + }); + + it('should not add credential:read scope to credentials without isGlobal property', async () => { + // + // ARRANGE + // + const user = await createMember(); + const mockCredentialWithoutIsGlobal = { + id: 'cred-no-flag', + name: 'Credential Without isGlobal', + type: 'testCredential', + // isGlobal not specified + shared: [ + { + projectId: 'project-1', + role: 'credential:user', + }, + ], + } as any; + const userProjectRelations = [] as any[]; + + // + // ACT + // + const result = roleService.addScopes( + mockCredentialWithoutIsGlobal, + user, + userProjectRelations, + ); + + // + // ASSERT + // + expect(result).toHaveProperty('scopes'); + // Should not contain credential:read because the user only has credential:user role + // and isGlobal is not specified + expect(result.scopes).not.toContain('credential:read'); + }); + + it('should not add credential:read scope to workflow entities even with isGlobal property', async () => { + // + // ARRANGE + // + // Note: While workflows typically don't have isGlobal, the implementation + // only adds credential:read scope for credential entities, not workflows + const user = await createMember(); + const mockWorkflowWithIsGlobal = { + id: 'workflow-1', + name: 'Test Workflow', + active: true, + isGlobal: true, + shared: [ + { + projectId: 'project-1', + role: 'workflow:editor', + }, + ], + } as any; + const userProjectRelations = [] as any[]; + + // + // ACT + // + const result = roleService.addScopes(mockWorkflowWithIsGlobal, user, userProjectRelations); + + // + // ASSERT + // + expect(result).toHaveProperty('scopes'); + // The implementation only adds credential:read for credential entities (entityType === 'credential') + // Workflows should not get credential:read scope even if they have isGlobal: true + expect(result.scopes).not.toContain('credential:read'); + }); + }); + + describe('rolesWithScope', () => { + it('should return built-in project roles with given scope', async () => { + // + // ACT + // + const roles = await roleService.rolesWithScope('project', ['project:read']); + + // + // ASSERT + // + expect(roles).toBeInstanceOf(Array); + expect(roles.length).toBeGreaterThan(0); + // Should include built-in project roles that have project:read + expect(roles).toContain('project:admin'); + expect(roles).toContain('project:editor'); + }); + + it('should return built-in credential roles with given scope', async () => { + // + // ACT + // + const roles = await roleService.rolesWithScope('credential', ['credential:read']); + + // + // ASSERT + // + expect(roles).toBeInstanceOf(Array); + expect(roles.length).toBeGreaterThan(0); + // Should include built-in credential roles that have credential:read + expect(roles).toContain('credential:owner'); + expect(roles).toContain('credential:user'); + }); + + it('should handle multiple scopes', async () => { + // + // ACT + // + const roles = await roleService.rolesWithScope('project', ['project:read', 'project:update']); + + // + // ASSERT + // + expect(roles).toBeInstanceOf(Array); + expect(roles.length).toBeGreaterThan(0); + // Project admin should have both scopes + expect(roles).toContain('project:admin'); + }); + + it('should handle single scope as string', async () => { + // + // ACT + // + const roles = await roleService.rolesWithScope('workflow', 'workflow:read'); + + // + // ASSERT + // + expect(roles).toBeInstanceOf(Array); + expect(roles.length).toBeGreaterThan(0); + }); + + it('should return empty array when no roles match the scopes', async () => { + // + // ACT + // + const roles = await roleService.rolesWithScope('project', ['nonexistent:scope' as any]); + + // + // ASSERT + // + expect(roles).toEqual([]); + }); + + it('should cache results for repeated calls', async () => { + // + // ACT + // + const roles1 = await roleService.rolesWithScope('project', ['project:read']); + const roles2 = await roleService.rolesWithScope('project', ['project:read']); + + // + // ASSERT + // + expect(roles1).toEqual(roles2); + }); }); }); diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 2ba9c5269be..e5cfeb709a7 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -3546,6 +3546,7 @@ "projects.settings.member.removed.title": "Member removed successfully", "projects.settings.member.remove.error.title": "An error occurred while removing member", "projects.settings.member.added.title": "Member added successfully", + "projects.sharing.allUsers": "All users and projects", "projects.sharing.noMatchingProjects": "There are no available projects", "projects.sharing.noMatchingUsers": "No matching users or projects", "projects.sharing.select.placeholder": "Select project or user", @@ -3578,6 +3579,8 @@ "projects.move.resource.success.message.workflow.withAllCredentials": "The workflow's credentials were shared with the project.", "projects.move.resource.success.message.workflow.withSomeCredentials": "Due to missing permissions not all the workflow's credentials were shared with the project.", "projects.move.resource.success.link": "View {targetProjectName}", + "projects.badge.global": "Global", + "projects.badge.tooltip.global": "This {resourceTypeLabel} was shared globally with all users and projects", "projects.badge.tooltip.sharedOwned": "This {resourceTypeLabel} is owned by you and shared with {count} users", "projects.badge.tooltip.sharedPersonal": "This {resourceTypeLabel} is owned by {name} and shared with {count} users", "projects.badge.tooltip.personal": "This {resourceTypeLabel} is owned by {name}", diff --git a/packages/frontend/editor-ui/src/Interface.ts b/packages/frontend/editor-ui/src/Interface.ts index 1fcb8042fcf..3b8ee80bafa 100644 --- a/packages/frontend/editor-ui/src/Interface.ts +++ b/packages/frontend/editor-ui/src/Interface.ts @@ -305,6 +305,7 @@ export type CredentialsResource = BaseResource & { sharedWithProjects?: ProjectSharingData[]; readOnly: boolean; needsSetup: boolean; + isGlobal?: boolean; }; // Base resource types that are always available diff --git a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectCardBadge.test.ts b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectCardBadge.test.ts index 11a9bac68d8..f0e9a004d31 100644 --- a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectCardBadge.test.ts +++ b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectCardBadge.test.ts @@ -91,4 +91,98 @@ describe('ProjectCardBadge', () => { }); expect(getByText(truncate(result, 20))).toBeVisible(); }); + + describe('global badge', () => { + it('should show global badge when global prop is true', () => { + const { getByTestId } = renderComponent({ + props: { + resource: { + homeProject: { + id: '1', + name: 'Test Project', + }, + } as WorkflowResource, + resourceType: ResourceType.Credential, + resourceTypeLabel: 'credential', + personalProject: { + id: '1', + } as Project, + global: true, + }, + }); + + const globalBadge = getByTestId('credential-global-badge'); + expect(globalBadge).toBeVisible(); + expect(globalBadge).toHaveTextContent('Global'); + }); + + it('should not show global badge when global prop is false', () => { + const { queryByTestId } = renderComponent({ + props: { + resource: { + homeProject: { + id: '1', + name: 'Test Project', + }, + } as WorkflowResource, + resourceType: ResourceType.Credential, + resourceTypeLabel: 'credential', + personalProject: { + id: '1', + } as Project, + global: false, + }, + }); + + expect(queryByTestId('credential-global-badge')).not.toBeInTheDocument(); + }); + + it('should not show global badge when global prop is undefined', () => { + const { queryByTestId } = renderComponent({ + props: { + resource: { + homeProject: { + id: '1', + name: 'Test Project', + }, + } as WorkflowResource, + resourceType: ResourceType.Credential, + resourceTypeLabel: 'credential', + personalProject: { + id: '1', + } as Project, + }, + }); + + expect(queryByTestId('credential-global-badge')).not.toBeInTheDocument(); + }); + + it('should show both project badge and global badge together', () => { + const { getByTestId, getByText } = renderComponent({ + props: { + resource: { + homeProject: { + id: '1', + name: 'Test Project', + }, + } as WorkflowResource, + resourceType: ResourceType.Credential, + resourceTypeLabel: 'credential', + personalProject: { + id: '2', + } as Project, + global: true, + }, + }); + + // Project badge should be visible + expect(getByTestId('card-badge')).toBeVisible(); + expect(getByText('Test Project')).toBeVisible(); + + // Global badge should also be visible + const globalBadge = getByTestId('credential-global-badge'); + expect(globalBadge).toBeVisible(); + expect(globalBadge).toHaveTextContent('Global'); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectCardBadge.vue b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectCardBadge.vue index 1a8159238fb..f0afeffecc8 100644 --- a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectCardBadge.vue +++ b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectCardBadge.vue @@ -15,6 +15,7 @@ type Props = { resourceTypeLabel: string; personalProject: Project | null; showBadgeBorder?: boolean; + global?: boolean; }; const enum ProjectState { @@ -176,6 +177,27 @@ const projectLocation = computed(() => { + + +
+ + {{ i18n.baseText('projects.badge.global') }} +
+ + { line-height: var(--line-height--md); } +.global-badge { + composes: count-badge; + display: flex; + align-items: center; + gap: var(--spacing--3xs); +} + .nowrap { white-space: nowrap !important; } diff --git a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectSharing.test.ts b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectSharing.test.ts index f9993f87e7b..a0068b1380f 100644 --- a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectSharing.test.ts +++ b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectSharing.test.ts @@ -5,6 +5,24 @@ import { getDropdownItems, getSelectedDropdownValue } from '@/__tests__/utils'; import { createProjectListItem, createProjectSharingData } from '../__tests__/utils'; import ProjectSharing from './ProjectSharing.vue'; import type { AllRolesMap } from '@n8n/permissions'; +import { useI18n } from '@n8n/i18n'; +import type * as I18nModule from '@n8n/i18n'; + +vi.mock('@n8n/i18n', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useI18n: vi.fn(), + }; +}); + +const mockBaseText = vi.fn((key: string) => { + const translations: Record = { + 'projects.sharing.allUsers': 'All users and projects', + 'auth.roles.owner': 'Owner', + }; + return translations[key] || key; +}); const renderComponent = createComponentRenderer(ProjectSharing); @@ -13,6 +31,11 @@ const teamProjects = Array.from({ length: 3 }, () => createProjectListItem('team const homeProject = createProjectSharingData(); describe('ProjectSharing', () => { + beforeEach(() => { + vi.mocked(useI18n).mockReturnValue({ + baseText: mockBaseText, + } as unknown as ReturnType); + }); it('should render empty select when projects is empty and no selected project existing', async () => { const { getByTestId, queryByTestId } = renderComponent({ props: { @@ -163,4 +186,151 @@ describe('ProjectSharing', () => { expect(queryByTestId('project-sharing-list-item')).not.toBeInTheDocument(); expect(getByTestId('project-sharing-owner')).toBeInTheDocument(); }); + + describe('global sharing', () => { + it('should show "All Users" option when canShareGlobally is true', async () => { + const { getByTestId } = renderComponent({ + props: { + projects: personalProjects, + modelValue: [], + canShareGlobally: true, + }, + }); + + const projectSelect = getByTestId('project-sharing-select'); + const dropdownItems = await getDropdownItems(projectSelect); + + // "All users and projects" should be the first option + expect(dropdownItems[0]).toHaveTextContent('All users and projects'); + // Total items should be projects + "All Users" + expect(dropdownItems).toHaveLength(personalProjects.length + 1); + }); + + it('should not show "All Users" option when canShareGlobally is false', async () => { + const { getByTestId } = renderComponent({ + props: { + projects: personalProjects, + modelValue: [], + canShareGlobally: false, + }, + }); + + const projectSelect = getByTestId('project-sharing-select'); + const dropdownItems = await getDropdownItems(projectSelect); + + // "All users and projects" should not be present + expect(dropdownItems[0]).not.toHaveTextContent('All users and projects'); + expect(dropdownItems).toHaveLength(personalProjects.length); + }); + + it('should not show "All Users" option when canShareGlobally is undefined', async () => { + const { getByTestId } = renderComponent({ + props: { + projects: personalProjects, + modelValue: [], + }, + }); + + const projectSelect = getByTestId('project-sharing-select'); + const dropdownItems = await getDropdownItems(projectSelect); + + // "All users and projects" should not be present + expect(dropdownItems[0]).not.toHaveTextContent('All users and projects'); + expect(dropdownItems).toHaveLength(personalProjects.length); + }); + + it('should emit update:shareWithAllUsers when "All Users" is selected', async () => { + const { getByTestId, emitted } = renderComponent({ + props: { + projects: personalProjects, + modelValue: [], + canShareGlobally: true, + }, + }); + + const projectSelect = getByTestId('project-sharing-select'); + const dropdownItems = await getDropdownItems(projectSelect); + + // Select "All Users" (first item) + await userEvent.click(dropdownItems[0]); + + expect(emitted()['update:shareWithAllUsers']).toBeTruthy(); + expect(emitted()['update:shareWithAllUsers']).toEqual([[true]]); + }); + + it('should show "All Users" in selected list when isSharedGlobally is true', () => { + const { getAllByTestId } = renderComponent({ + props: { + projects: personalProjects, + modelValue: [], + canShareGlobally: true, + isSharedGlobally: true, + }, + }); + + const listItems = getAllByTestId('project-sharing-list-item'); + // First item should be "All users and projects" + expect(listItems[0]).toHaveTextContent('All users and projects'); + }); + + it('should emit update:shareWithAllUsers with false when "All Users" is removed', async () => { + const { getAllByTestId, emitted } = renderComponent({ + props: { + projects: personalProjects, + modelValue: [], + canShareGlobally: true, + isSharedGlobally: true, + roles: [ + { + role: 'project:admin', + name: 'Admin', + }, + ] as unknown as AllRolesMap['workflow' | 'credential' | 'project'], + }, + }); + + const listItems = getAllByTestId('project-sharing-list-item'); + const removeButton = within(listItems[0]).getByTestId('project-sharing-remove'); + + await userEvent.click(removeButton); + + expect(emitted()['update:shareWithAllUsers']).toBeTruthy(); + expect(emitted()['update:shareWithAllUsers']).toEqual([[false]]); + }); + + it('should not show remove button for "All Users" when canShareGlobally is false', () => { + const { getAllByTestId } = renderComponent({ + props: { + projects: personalProjects, + modelValue: [], + canShareGlobally: false, + isSharedGlobally: true, + }, + }); + + const listItems = getAllByTestId('project-sharing-list-item'); + const removeButton = within(listItems[0]).queryByTestId('project-sharing-remove'); + + // Remove button should not exist for "All Users" when canShareGlobally is false + expect(removeButton).not.toBeInTheDocument(); + }); + + it('should not show "All Users" in dropdown when already globally shared', async () => { + const { getByTestId } = renderComponent({ + props: { + projects: personalProjects, + modelValue: [], + canShareGlobally: true, + isSharedGlobally: true, + }, + }); + + const projectSelect = getByTestId('project-sharing-select'); + const dropdownItems = await getDropdownItems(projectSelect); + + // "All users and projects" should not be in dropdown when already shared globally + expect(dropdownItems[0]).not.toHaveTextContent('All users and projects'); + expect(dropdownItems).toHaveLength(personalProjects.length); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectSharing.vue b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectSharing.vue index 8335bc34de8..53a871cc5bd 100644 --- a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectSharing.vue +++ b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectSharing.vue @@ -9,8 +9,19 @@ import { ProjectTypes, type ProjectListItem, type ProjectSharingData } from '../ import ProjectSharingInfo from './ProjectSharingInfo.vue'; import { N8nBadge, N8nButton, N8nIcon, N8nOption, N8nSelect, N8nText } from '@n8n/design-system'; + const locale = useI18n(); +const GLOBAL_GROUP: ProjectListItem = { + id: 'all_users', + name: locale.baseText('projects.sharing.allUsers'), + type: 'public', + icon: { type: 'icon', value: 'globe' }, + role: 'member', + createdAt: `${Date.now()}`, + updatedAt: `${Date.now()}`, +}; + type Props = { projects: ProjectListItem[]; homeProject?: ProjectSharingData; @@ -21,19 +32,32 @@ type Props = { emptyOptionsText?: string; size?: SelectSize; clearable?: boolean; + canShareGlobally?: boolean; + isSharedGlobally?: boolean; }; const props = defineProps(); const model = defineModel<(ProjectSharingData | null) | ProjectSharingData[]>({ required: true, }); + const emit = defineEmits<{ projectAdded: [value: ProjectSharingData]; projectRemoved: [value: ProjectSharingData]; clear: []; + 'update:shareWithAllUsers': [value: boolean]; }>(); const selectedProject = ref(Array.isArray(model.value) ? '' : (model.value?.id ?? '')); + +const selectedProjects = computed((): ProjectSharingData[] | null => { + if (!Array.isArray(model.value)) { + return null; + } + + return props.isSharedGlobally ? [GLOBAL_GROUP, ...model.value] : model.value; +}); + const filter = ref(''); const selectPlaceholder = computed( () => props.placeholder ?? locale.baseText('projects.sharing.select.placeholder'), @@ -50,13 +74,14 @@ const filteredProjects = computed(() => ), ); -const sortedProjects = computed(() => - orderBy( +const sortedProjects = computed((): ProjectListItem[] => [ + ...(props.canShareGlobally && !props.isSharedGlobally ? [GLOBAL_GROUP] : []), + ...orderBy( filteredProjects.value, ['type', (project) => project.name?.toLowerCase()], ['desc', 'asc'], ), -); +]); const projectIcon = computed(() => { const defaultIcon: IconOrEmoji = { type: 'icon', value: 'layers' }; @@ -76,6 +101,11 @@ const setFilter = (query: string) => { }; const onProjectSelected = (projectId: string) => { + if (projectId === GLOBAL_GROUP.id) { + emit('update:shareWithAllUsers', true); + return; + } + const project = props.projects.find((p) => p.id === projectId); if (!project) { @@ -95,6 +125,12 @@ const onRoleAction = (project: ProjectSharingData, role: string) => { return; } + if (project.id === GLOBAL_GROUP.id && role === 'remove') { + emit('update:shareWithAllUsers', false); + + return; + } + const index = model.value?.findIndex((p) => p.id === project.id) ?? -1; if (index === -1) { return; @@ -151,7 +187,7 @@ watch( -
    +
    • @@ -160,14 +196,18 @@ watch( >
    • ({ + useRootStore: vi.fn(() => mockRootStore), +})); + +vi.mock('@n8n/stores/useRootStore', () => ({ + useRootStore, +})); + +vi.mock('@/app/stores/nodeTypes.store', () => ({ + useNodeTypesStore: vi.fn(() => ({ + getNodeType: vi.fn(), + })), +})); + +vi.mock('@/app/stores/settings.store', () => ({ + useSettingsStore: vi.fn(() => ({ + isEnterpriseFeatureEnabled: { + sharing: true, + }, + })), +})); + +vi.mock('../credentials.api'); +vi.mock('../credentials.ee.api'); + +describe('credentials.store', () => { + beforeEach(() => { + vi.clearAllMocks(); + setActivePinia(createPinia()); + }); + + describe('fetchAllCredentials', () => { + it('should pass includeGlobal parameter to API when provided', async () => { + const { useCredentialsStore } = await import('../credentials.store'); + const store = useCredentialsStore(); + + const mockCredentials: ICredentialsResponse[] = [ + mock({ + id: 'cred-1', + name: 'Personal Credential', + type: 'httpBasicAuth', + isGlobal: false, + }), + mock({ + id: 'cred-2', + name: 'Global Credential', + type: 'httpBasicAuth', + isGlobal: true, + }), + ]; + + vi.spyOn(credentialsApi, 'getAllCredentials').mockResolvedValue(mockCredentials); + + await store.fetchAllCredentials(undefined, true, false, true); + + expect(credentialsApi.getAllCredentials).toHaveBeenCalledWith( + mockRootStore.restApiContext, + undefined, + true, + false, + true, + ); + }); + + it('should pass includeGlobal as true when not provided', async () => { + const { useCredentialsStore } = await import('../credentials.store'); + const store = useCredentialsStore(); + + const mockCredentials: ICredentialsResponse[] = [ + mock({ + id: 'cred-1', + name: 'Personal Credential', + type: 'httpBasicAuth', + isGlobal: false, + }), + ]; + + vi.spyOn(credentialsApi, 'getAllCredentials').mockResolvedValue(mockCredentials); + + await store.fetchAllCredentials(); + + expect(credentialsApi.getAllCredentials).toHaveBeenCalledWith( + mockRootStore.restApiContext, + undefined, + true, + false, + true, + ); + }); + + it('should set credentials in store including global credentials', async () => { + const { useCredentialsStore } = await import('../credentials.store'); + const store = useCredentialsStore(); + + const mockCredentials: ICredentialsResponse[] = [ + mock({ + id: 'cred-1', + name: 'Personal Credential', + type: 'httpBasicAuth', + isGlobal: false, + }), + mock({ + id: 'cred-2', + name: 'Global Credential', + type: 'httpBasicAuth', + isGlobal: true, + }), + ]; + + vi.spyOn(credentialsApi, 'getAllCredentials').mockResolvedValue(mockCredentials); + + await store.fetchAllCredentials(undefined, true, false, true); + + expect(store.allCredentials).toHaveLength(2); + expect(store.allCredentials.find((c) => c.id === 'cred-2')?.isGlobal).toBe(true); + }); + }); + + describe('createNewCredential', () => { + it('should pass isGlobal parameter to API when creating credential', async () => { + const { useCredentialsStore } = await import('../credentials.store'); + const store = useCredentialsStore(); + + const mockCredential = mock({ + id: 'new-cred-1', + name: 'New Global Credential', + type: 'httpBasicAuth', + isGlobal: true, + }); + + vi.spyOn(credentialsApi, 'createNewCredential').mockResolvedValue(mockCredential); + + await store.createNewCredential( + { + id: 'new-cred-1', + name: 'New Global Credential', + type: 'httpBasicAuth', + data: {}, + isGlobal: true, + }, + 'project-123', + ); + + expect(credentialsApi.createNewCredential).toHaveBeenCalledWith( + mockRootStore.restApiContext, + { + name: 'New Global Credential', + type: 'httpBasicAuth', + data: {}, + projectId: 'project-123', + uiContext: undefined, + isGlobal: true, + }, + ); + }); + + it('should create non-global credential when isGlobal is false', async () => { + const { useCredentialsStore } = await import('../credentials.store'); + const store = useCredentialsStore(); + + const mockCredential = mock({ + id: 'new-cred-2', + name: 'New Personal Credential', + type: 'httpBasicAuth', + isGlobal: false, + }); + + vi.spyOn(credentialsApi, 'createNewCredential').mockResolvedValue(mockCredential); + + await store.createNewCredential( + { + id: 'new-cred-2', + name: 'New Personal Credential', + type: 'httpBasicAuth', + data: {}, + isGlobal: false, + }, + 'project-123', + ); + + expect(credentialsApi.createNewCredential).toHaveBeenCalledWith( + mockRootStore.restApiContext, + { + name: 'New Personal Credential', + type: 'httpBasicAuth', + data: {}, + projectId: 'project-123', + uiContext: undefined, + isGlobal: false, + }, + ); + }); + + it('should create credential without isGlobal when not provided', async () => { + const { useCredentialsStore } = await import('../credentials.store'); + const store = useCredentialsStore(); + + const mockCredential = mock({ + id: 'new-cred-3', + name: 'New Credential', + type: 'httpBasicAuth', + }); + + vi.spyOn(credentialsApi, 'createNewCredential').mockResolvedValue(mockCredential); + + await store.createNewCredential( + { + id: 'new-cred-3', + name: 'New Credential', + type: 'httpBasicAuth', + data: {}, + }, + 'project-123', + ); + + expect(credentialsApi.createNewCredential).toHaveBeenCalledWith( + mockRootStore.restApiContext, + { + name: 'New Credential', + type: 'httpBasicAuth', + data: {}, + projectId: 'project-123', + uiContext: undefined, + isGlobal: undefined, + }, + ); + }); + }); + + describe('setCredentialSharedWith', () => { + it('should pass isGlobal parameter when setting credential sharing', async () => { + const { useCredentialsStore } = await import('../credentials.store'); + const credentialsEeApi = await import('../credentials.ee.api'); + const store = useCredentialsStore(); + + // Initialize the store with a credential + store.state.credentials = { + 'cred-1': mock({ + id: 'cred-1', + name: 'Test Credential', + type: 'httpBasicAuth', + sharedWithProjects: [], + }), + }; + + vi.spyOn(credentialsEeApi, 'setCredentialSharedWith').mockResolvedValue( + mock({ id: 'cred-1' }), + ); + + await store.setCredentialSharedWith({ + credentialId: 'cred-1', + sharedWithProjects: [ + { + id: 'project-1', + name: 'Project 1', + type: 'team', + icon: null, + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + }, + ], + isGlobal: true, + }); + + expect(credentialsEeApi.setCredentialSharedWith).toHaveBeenCalledWith( + mockRootStore.restApiContext, + 'cred-1', + { + shareWithIds: ['project-1'], + }, + ); + }); + + it('should update credential state with new sharing settings', async () => { + const { useCredentialsStore } = await import('../credentials.store'); + const credentialsEeApi = await import('../credentials.ee.api'); + const store = useCredentialsStore(); + + const initialCredential = mock({ + id: 'cred-1', + name: 'Test Credential', + type: 'httpBasicAuth', + sharedWithProjects: [], + }); + + store.state.credentials = { + 'cred-1': initialCredential, + }; + + vi.spyOn(credentialsEeApi, 'setCredentialSharedWith').mockResolvedValue( + mock({ id: 'cred-1' }), + ); + + const newSharing = [ + { + id: 'project-1', + name: 'Project 1', + type: 'team' as const, + icon: null, + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + }, + { + id: 'project-2', + name: 'Project 2', + type: 'team' as const, + icon: null, + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + }, + ]; + + await store.setCredentialSharedWith({ + credentialId: 'cred-1', + sharedWithProjects: newSharing, + }); + + expect(store.state.credentials['cred-1']?.sharedWithProjects).toEqual(newSharing); + }); + }); +}); diff --git a/packages/frontend/editor-ui/src/features/credentials/components/CredentialCard.test.ts b/packages/frontend/editor-ui/src/features/credentials/components/CredentialCard.test.ts index 3e2833a44aa..051275bdebb 100644 --- a/packages/frontend/editor-ui/src/features/credentials/components/CredentialCard.test.ts +++ b/packages/frontend/editor-ui/src/features/credentials/components/CredentialCard.test.ts @@ -93,4 +93,68 @@ describe('CredentialCard', () => { const heading = getByRole('heading'); expect(heading).toHaveTextContent('Read only'); }); + + describe('global credentials', () => { + it('should display global badge when credential has isGlobal true', () => { + const data = createCredential({ + isGlobal: true, + homeProject: { + name: 'Test Project', + }, + }); + + const { getByTestId } = renderComponent({ props: { data } }); + + const globalBadge = getByTestId('credential-global-badge'); + expect(globalBadge).toBeInTheDocument(); + expect(globalBadge).toHaveTextContent('Global'); + }); + + it('should not display global badge when credential has isGlobal false', () => { + const data = createCredential({ + isGlobal: false, + homeProject: { + name: 'Test Project', + }, + }); + + const { queryByTestId } = renderComponent({ props: { data } }); + + expect(queryByTestId('credential-global-badge')).not.toBeInTheDocument(); + }); + + it('should not display global badge when isGlobal is undefined', () => { + const data = createCredential({ + homeProject: { + name: 'Test Project', + }, + }); + + const { queryByTestId } = renderComponent({ props: { data } }); + + expect(queryByTestId('credential-global-badge')).not.toBeInTheDocument(); + }); + + it('should display both project badge and global badge for global credentials', () => { + const projectName = 'Test Project'; + const data = createCredential({ + isGlobal: true, + homeProject: { + name: projectName, + }, + }); + + const { getByTestId } = renderComponent({ props: { data } }); + + // Project badge should be present + const projectBadge = getByTestId('card-badge'); + expect(projectBadge).toBeInTheDocument(); + expect(projectBadge).toHaveTextContent(projectName); + + // Global badge should also be present + const globalBadge = getByTestId('credential-global-badge'); + expect(globalBadge).toBeInTheDocument(); + expect(globalBadge).toHaveTextContent('Global'); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/features/credentials/components/CredentialCard.vue b/packages/frontend/editor-ui/src/features/credentials/components/CredentialCard.vue index 009120e5583..8ff6706a80f 100644 --- a/packages/frontend/editor-ui/src/features/credentials/components/CredentialCard.vue +++ b/packages/frontend/editor-ui/src/features/credentials/components/CredentialCard.vue @@ -166,6 +166,7 @@ function moveResource() { :resource-type-label="resourceTypeLabel" :personal-project="projectsStore.personalProject" :show-badge-border="false" + :global="data.isGlobal" /> (); +const isSharedGlobally = ref(false); const activeNodeType = computed(() => { const activeNode = ndvStore.activeNode; @@ -541,6 +542,10 @@ async function loadCurrentCredential() { } credentialName.value = currentCredentials.name; + isSharedGlobally.value = + 'isGlobal' in currentCredentials && typeof currentCredentials.isGlobal === 'boolean' + ? currentCredentials.isGlobal + : false; } catch (error) { toast.showError( error, @@ -576,6 +581,11 @@ function onChangeSharedWith(sharedWithProjects: ProjectSharingData[]) { hasUnsavedChanges.value = true; } +function onShareWithAllUsersUpdate(shareWithAllUsers: boolean) { + isSharedGlobally.value = shareWithAllUsers; + hasUnsavedChanges.value = true; +} + function onDataChange({ name, value }: IUpdateInformation) { const currentValue = get(credentialData.value, name); if (currentValue === value) { @@ -703,6 +713,7 @@ async function saveCredential(): Promise { name: credentialName.value, type: credentialTypeName.value, data: data as unknown as ICredentialDataDecryptedObject, + isGlobal: isSharedGlobally.value, }; if ( @@ -1240,8 +1251,10 @@ const { width } = useElementSize(credNameRef); :credential-data="credentialData" :credential-id="credentialId" :credential-permissions="credentialPermissions" + :isSharedGlobally="isSharedGlobally" :modal-bus="modalBus" @update:model-value="onChangeSharedWith" + @update:share-with-all-users="onShareWithAllUsersUpdate" />
      diff --git a/packages/frontend/editor-ui/src/features/credentials/components/CredentialEdit/CredentialSharing.ee.test.ts b/packages/frontend/editor-ui/src/features/credentials/components/CredentialEdit/CredentialSharing.ee.test.ts new file mode 100644 index 00000000000..d12b74ab1e1 --- /dev/null +++ b/packages/frontend/editor-ui/src/features/credentials/components/CredentialEdit/CredentialSharing.ee.test.ts @@ -0,0 +1,314 @@ +import { createComponentRenderer } from '@/__tests__/render'; +import { createTestingPinia } from '@pinia/testing'; +import { setActivePinia } from 'pinia'; +import CredentialSharing from './CredentialSharing.ee.vue'; +import { useUsersStore } from '@/features/settings/users/users.store'; +import { useProjectsStore } from '@/features/collaboration/projects/projects.store'; +import { useSettingsStore } from '@/app/stores/settings.store'; +import { useRolesStore } from '@/app/stores/roles.store'; +import type { ICredentialsResponse } from '../../credentials.types'; +import { createEventBus } from '@n8n/utils/event-bus'; +import { getDropdownItems } from '@/__tests__/utils'; +import { useI18n } from '@n8n/i18n'; +import type * as I18nModule from '@n8n/i18n'; + +vi.mock('@n8n/i18n', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useI18n: vi.fn(), + }; +}); + +const mockBaseText = vi.fn((key: string, options?: { interpolate?: Record }) => { + const translations: Record = { + 'projects.sharing.allUsers': 'All users and projects', + 'credentialEdit.credentialSharing.info.owner': + 'Only users with credential sharing permission can change who this credential is shared with', + 'credentialEdit.credentialSharing.info.sharee.team': 'Shared by team project', + 'credentialEdit.credentialSharing.info.sharee.personal': 'Shared by personal project', + 'credentialEdit.credentialSharing.role.user': 'User', + 'auth.roles.owner': 'Owner', + 'contextual.credentials.sharing.unavailable.title': 'Upgrade to collaborate', + 'contextual.credentials.sharing.unavailable.description': + 'You can share credentials with others when you upgrade your plan.', + 'contextual.credentials.sharing.unavailable.button': 'View plans', + }; + + let text = translations[key] || key; + + // Handle interpolation + if (options?.interpolate) { + Object.entries(options.interpolate).forEach(([placeholder, value]) => { + text = text.replace(`{${placeholder}}`, value); + }); + } + + return text; +}); + +const renderComponent = createComponentRenderer(CredentialSharing); + +const createCredential = (overrides = {}): ICredentialsResponse => ({ + id: '1', + name: 'Test Credential', + type: 'testType', + isManaged: false, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + homeProject: { + id: 'project-1', + name: 'Test Project', + type: 'team', + icon: null, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }, + sharedWithProjects: [], + ...overrides, +}); + +describe('CredentialSharing.ee', () => { + let usersStore: ReturnType; + let projectsStore: ReturnType; + let settingsStore: ReturnType; + let rolesStore: ReturnType; + let isEnterpriseFeatureEnabledSpy: ReturnType; + + beforeEach(() => { + const pinia = createTestingPinia(); + setActivePinia(pinia); + + usersStore = useUsersStore(); + projectsStore = useProjectsStore(); + settingsStore = useSettingsStore(); + rolesStore = useRolesStore(); + + // Mock i18n + vi.mocked(useI18n).mockReturnValue({ + baseText: mockBaseText, + } as unknown as ReturnType); + + // Mock store methods + vi.spyOn(usersStore, 'fetchUsers').mockResolvedValue(); + vi.spyOn(projectsStore, 'getAllProjects').mockResolvedValue(); + vi.spyOn(rolesStore, 'processedCredentialRoles', 'get').mockReturnValue([ + { + slug: 'credential:user', + displayName: 'User', + description: null, + systemRole: false, + roleType: 'credential', + scopes: [], + licensed: true, + }, + ]); + isEnterpriseFeatureEnabledSpy = vi + .spyOn(settingsStore, 'isEnterpriseFeatureEnabled', 'get') + .mockReturnValue({ + sharing: true, + ldap: false, + saml: false, + oidc: false, + mfaEnforcement: false, + logStreaming: false, + advancedExecutionFilters: false, + variables: false, + sourceControl: false, + externalSecrets: false, + auditLogs: false, + debugInEditor: false, + binaryDataS3: false, + workerView: false, + advancedPermissions: false, + apiKeyScopes: false, + workflowDiffs: false, + provisioning: true, + showNonProdBanner: false, + projects: { + team: { + limit: -1, + }, + }, + customRoles: false, + }); + }); + + it('should render ProjectSharing component when sharing is enabled', () => { + const credential = createCredential(); + const { getByTestId } = renderComponent({ + props: { + credentialId: credential.id, + credentialData: {}, + credentialPermissions: { share: true }, + credential, + modalBus: createEventBus(), + }, + }); + + expect(getByTestId('project-sharing-select')).toBeInTheDocument(); + }); + + describe('canShareGlobally computed property', () => { + it('should pass canShareGlobally as true when user has credential:shareGlobally scope', async () => { + vi.spyOn(usersStore, 'currentUser', 'get').mockReturnValue({ + id: '1', + email: 'test@example.com', + firstName: 'Test', + lastName: 'User', + isDefaultUser: false, + isPendingUser: false, + mfaEnabled: false, + globalScopes: ['credential:shareGlobally'], + }); + + const credential = createCredential(); + const { getByTestId } = renderComponent({ + props: { + credentialId: credential.id, + credentialData: {}, + credentialPermissions: { share: true }, + credential, + modalBus: createEventBus(), + }, + }); + + // When canShareGlobally is true, "All users and projects" option should be available + const projectSharingSelect = getByTestId('project-sharing-select'); + expect(projectSharingSelect).toBeInTheDocument(); + + // Open dropdown and verify "All users and projects" option is present + const dropdownItems = await getDropdownItems(projectSharingSelect); + expect(dropdownItems[0]).toHaveTextContent('All users and projects'); + }); + + it('should pass canShareGlobally as false when user does not have credential:shareGlobally scope', async () => { + vi.spyOn(usersStore, 'currentUser', 'get').mockReturnValue({ + id: '1', + email: 'test@example.com', + firstName: 'Test', + lastName: 'User', + isDefaultUser: false, + isPendingUser: false, + mfaEnabled: false, + globalScopes: [], + }); + + const credential = createCredential(); + const { getByTestId } = renderComponent({ + props: { + credentialId: credential.id, + credentialData: {}, + credentialPermissions: { share: true }, + credential, + modalBus: createEventBus(), + }, + }); + + // When canShareGlobally is false, "All users and projects" option should NOT be available + const projectSharingSelect = getByTestId('project-sharing-select'); + expect(projectSharingSelect).toBeInTheDocument(); + + // Open dropdown and verify "All users and projects" option is NOT present + const dropdownItems = await getDropdownItems(projectSharingSelect); + // Should have no items or first item should not be "All users and projects" + const hasAllUsersOption = Array.from(dropdownItems).some((item) => + item.textContent?.includes('All users and projects'), + ); + expect(hasAllUsersOption).toBe(false); + }); + + it('should pass canShareGlobally as false when user is undefined', async () => { + vi.spyOn(usersStore, 'currentUser', 'get').mockReturnValue(null); + + const credential = createCredential(); + const { getByTestId } = renderComponent({ + props: { + credentialId: credential.id, + credentialData: {}, + credentialPermissions: { share: true }, + credential, + modalBus: createEventBus(), + }, + }); + + // When user is undefined, canShareGlobally should default to false + const projectSharingSelect = getByTestId('project-sharing-select'); + expect(projectSharingSelect).toBeInTheDocument(); + + // Open dropdown and verify "All users and projects" option is NOT present + const dropdownItems = await getDropdownItems(projectSharingSelect); + // Should have no items or no "All users and projects" option + const hasAllUsersOption = Array.from(dropdownItems).some((item) => + item.textContent?.includes('All users and projects'), + ); + expect(hasAllUsersOption).toBe(false); + }); + }); + + describe('projects filtering', () => { + it('should show upgrade action box when sharing is not enabled', () => { + isEnterpriseFeatureEnabledSpy.mockReturnValue({ + sharing: false, + ldap: false, + saml: false, + oidc: false, + mfaEnforcement: false, + logStreaming: false, + advancedExecutionFilters: false, + variables: false, + sourceControl: false, + externalSecrets: false, + auditLogs: false, + debugInEditor: false, + binaryDataS3: false, + workerView: false, + advancedPermissions: false, + apiKeyScopes: false, + workflowDiffs: false, + provisioning: true, + showNonProdBanner: false, + projects: { + team: { + limit: -1, + }, + }, + customRoles: false, + }); + + const credential = createCredential(); + const { getByText } = renderComponent({ + props: { + credentialId: credential.id, + credentialData: {}, + credentialPermissions: { share: true }, + credential, + modalBus: createEventBus(), + }, + }); + + // Should show upgrade message + expect(getByText(/upgrade to collaborate/i)).toBeInTheDocument(); + }); + }); + + describe('readonly state', () => { + it('should hide select and show info tip when user lacks share permission', () => { + const credential = createCredential(); + const { queryByTestId, getByText } = renderComponent({ + props: { + credentialId: credential.id, + credentialData: {}, + credentialPermissions: { share: false }, + credential, + modalBus: createEventBus(), + }, + }); + + // Select should not be visible when static prop is true + expect(queryByTestId('project-sharing-select')).not.toBeInTheDocument(); + // Info tip should be shown - since credential is team project, shows "Shared by team project" + expect(getByText(/shared by team project/i)).toBeInTheDocument(); + }); + }); +}); diff --git a/packages/frontend/editor-ui/src/features/credentials/components/CredentialEdit/CredentialSharing.ee.vue b/packages/frontend/editor-ui/src/features/credentials/components/CredentialEdit/CredentialSharing.ee.vue index 02f0e1a2b15..5a7da704042 100644 --- a/packages/frontend/editor-ui/src/features/credentials/components/CredentialEdit/CredentialSharing.ee.vue +++ b/packages/frontend/editor-ui/src/features/credentials/components/CredentialEdit/CredentialSharing.ee.vue @@ -19,6 +19,7 @@ import { splitName } from '@/features/collaboration/projects/projects.utils'; import type { EventBus } from '@n8n/utils/event-bus'; import type { ICredentialDataDecryptedObject } from 'n8n-workflow'; import { computed, onMounted, ref, watch } from 'vue'; +import { getResourcePermissions } from '@n8n/permissions'; import { N8nActionBox, N8nInfoTip } from '@n8n/design-system'; type Props = { @@ -27,12 +28,14 @@ type Props = { credentialPermissions: PermissionsRecord['credential']; credential?: ICredentialsResponse | ICredentialsDecryptedResponse | null; modalBus: EventBus; + isSharedGlobally?: boolean; }; -const props = withDefaults(defineProps(), { credential: null }); +const props = withDefaults(defineProps(), { credential: null, isSharedGlobally: false }); const emit = defineEmits<{ 'update:modelValue': [value: ProjectSharingData[]]; + 'update:shareWithAllUsers': [value: boolean]; }>(); const i18n = useI18n(); @@ -105,6 +108,11 @@ const sharingSelectPlaceholder = computed(() => : i18n.baseText('projects.sharing.select.placeholder.user'), ); +const canShareGlobally = computed(() => { + const permissions = getResourcePermissions(usersStore.currentUser?.globalScopes); + return permissions.credential?.shareGlobally ?? false; +}); + watch( sharedWithProjects, (changedSharedWithProjects) => { @@ -162,6 +170,9 @@ function goToUpgrade() { :readonly="!credentialPermissions.share" :static="!credentialPermissions.share" :placeholder="sharingSelectPlaceholder" + :can-share-globally="canShareGlobally" + :is-shared-globally="isSharedGlobally" + @update:share-with-all-users="emit('update:shareWithAllUsers', $event)" />
      diff --git a/packages/frontend/editor-ui/src/features/credentials/credentials.api.ts b/packages/frontend/editor-ui/src/features/credentials/credentials.api.ts index 74bd1c0fbd7..28430f6f323 100644 --- a/packages/frontend/editor-ui/src/features/credentials/credentials.api.ts +++ b/packages/frontend/editor-ui/src/features/credentials/credentials.api.ts @@ -28,12 +28,14 @@ export async function getAllCredentials( filter?: object, includeScopes?: boolean, onlySharedWithMe?: boolean, + includeGlobal?: boolean, ): Promise { return await makeRestApiRequest(context, 'GET', '/credentials', { ...(includeScopes ? { includeScopes } : {}), includeData: true, ...(filter ? { filter } : {}), ...(onlySharedWithMe ? { onlySharedWithMe } : {}), + ...(typeof includeGlobal === 'boolean' ? { includeGlobal } : {}), }); } diff --git a/packages/frontend/editor-ui/src/features/credentials/credentials.store.ts b/packages/frontend/editor-ui/src/features/credentials/credentials.store.ts index bd5111c8905..b2cc4236491 100644 --- a/packages/frontend/editor-ui/src/features/credentials/credentials.store.ts +++ b/packages/frontend/editor-ui/src/features/credentials/credentials.store.ts @@ -267,6 +267,7 @@ export const useCredentialsStore = defineStore(STORES.CREDENTIALS, () => { projectId?: string, includeScopes = true, onlySharedWithMe = false, + includeGlobal = true, ): Promise => { const filter = { projectId, @@ -277,6 +278,7 @@ export const useCredentialsStore = defineStore(STORES.CREDENTIALS, () => { isEmpty(filter) ? undefined : filter, includeScopes, onlySharedWithMe, + includeGlobal, ); setCredentials(credentials); return credentials; @@ -326,6 +328,7 @@ export const useCredentialsStore = defineStore(STORES.CREDENTIALS, () => { data: data.data ?? {}, projectId, uiContext, + isGlobal: data.isGlobal, }); if (data?.homeProject && !credential.homeProject) { @@ -400,6 +403,7 @@ export const useCredentialsStore = defineStore(STORES.CREDENTIALS, () => { const setCredentialSharedWith = async (payload: { sharedWithProjects: ProjectSharingData[]; credentialId: string; + isGlobal?: boolean; }): Promise => { if (useSettingsStore().isEnterpriseFeatureEnabled[EnterpriseEditionFeature.Sharing]) { await credentialsEeApi.setCredentialSharedWith( diff --git a/packages/frontend/editor-ui/src/features/credentials/credentials.types.ts b/packages/frontend/editor-ui/src/features/credentials/credentials.types.ts index 54e3f5bc2a5..b383e32dd08 100644 --- a/packages/frontend/editor-ui/src/features/credentials/credentials.types.ts +++ b/packages/frontend/editor-ui/src/features/credentials/credentials.types.ts @@ -14,6 +14,7 @@ export interface ICredentialsResponse extends ICredentialsEncrypted { scopes?: Scope[]; ownedBy?: Pick; isManaged: boolean; + isGlobal?: boolean; } export interface IUsedCredential { diff --git a/packages/frontend/editor-ui/src/features/credentials/views/CredentialsView.vue b/packages/frontend/editor-ui/src/features/credentials/views/CredentialsView.vue index 2511c2bed4e..f50b8ac8b64 100644 --- a/packages/frontend/editor-ui/src/features/credentials/views/CredentialsView.vue +++ b/packages/frontend/editor-ui/src/features/credentials/views/CredentialsView.vue @@ -89,6 +89,7 @@ const allCredentials = computed(() => sharedWithProjects: credential.sharedWithProjects, readOnly: !getResourcePermissions(credential.scopes).credential.update, needsSetup: needsSetup(credential.data), + isGlobal: credential.isGlobal, type: credential.type, })), ); @@ -192,11 +193,17 @@ const initialize = async () => { const isVarsEnabled = useSettingsStore().isEnterpriseFeatureEnabled[EnterpriseEditionFeature.Variables]; + const isPersonalView = + !overview.isSharedSubPage && + overview.isProjectsSubPage && + route?.params?.projectId === projectsStore.personalProject?.id; + const loadPromises = [ credentialsStore.fetchAllCredentials( route?.params?.projectId as string | undefined, true, overview.isSharedSubPage, + !isPersonalView, // don't include global credentials if personal ), credentialsStore.fetchCredentialTypes(false), externalSecretsStore.fetchAllSecrets(), diff --git a/packages/testing/playwright/tests/ui/17-sharing.spec.ts b/packages/testing/playwright/tests/ui/17-sharing.spec.ts index dc2cd68f7ee..e2acfa0c642 100644 --- a/packages/testing/playwright/tests/ui/17-sharing.spec.ts +++ b/packages/testing/playwright/tests/ui/17-sharing.spec.ts @@ -203,7 +203,7 @@ test.describe('@isolated', () => { await expect( n8n.credentials.credentialModal.getVisibleDropdown().getByTestId('project-sharing-info'), - ).toHaveCount(3); + ).toHaveCount(4); // Admin can share with self await expect( @@ -251,7 +251,7 @@ test.describe('@isolated', () => { await n8n.credentials.credentialModal.getUsersSelect().click(); const sharingDropdown = n8n.credentials.credentialModal.getVisibleDropdown(); - await expect(sharingDropdown.locator('li')).toHaveCount(4); + await expect(sharingDropdown.locator('li')).toHaveCount(5); await expect(sharingDropdown.getByText('Development')).toBeVisible(); await sharingDropdown.getByText('Development').click(); @@ -290,9 +290,9 @@ test.describe('@isolated', () => { await n8n.credentials.credentialModal.getUsersSelect().click(); const sharingDropdown2 = n8n.credentials.credentialModal.getVisibleDropdown(); - await expect(sharingDropdown2.locator('li')).toHaveCount(4); + await expect(sharingDropdown2.locator('li')).toHaveCount(5); - await sharingDropdown2.locator('li').first().click(); + await sharingDropdown2.locator('li').nth(1).click(); await n8n.credentials.credentialModal.saveSharing(); await n8n.credentials.credentialModal.close(); diff --git a/packages/testing/playwright/tests/ui/credential-api-operations.spec.ts b/packages/testing/playwright/tests/ui/credential-api-operations.spec.ts index 79eaaa3a0d1..6893e93932c 100644 --- a/packages/testing/playwright/tests/ui/credential-api-operations.spec.ts +++ b/packages/testing/playwright/tests/ui/credential-api-operations.spec.ts @@ -77,12 +77,14 @@ test.describe('Credential API Operations', () => { expect(foundCredentials).toHaveLength(2); const credentialsWithScopes = await api.credentials.getCredentials({ + includeGlobal: false, includeScopes: true, }); expect(credentialsWithScopes[0].scopes).toBeDefined(); expect(Array.isArray(credentialsWithScopes[0].scopes)).toBe(true); const credentialsWithData = await api.credentials.getCredentials({ + includeGlobal: false, includeData: true, }); const foundWithData = credentialsWithData.filter((c) => createdIds.includes(c.id)); diff --git a/packages/testing/playwright/tests/ui/global-credentials.spec.ts b/packages/testing/playwright/tests/ui/global-credentials.spec.ts new file mode 100644 index 00000000000..b216266a183 --- /dev/null +++ b/packages/testing/playwright/tests/ui/global-credentials.spec.ts @@ -0,0 +1,161 @@ +import { test, expect } from '../../fixtures/base'; + +test.describe('Global credentials @isolated', () => { + test.describe.configure({ mode: 'serial' }); + + test.beforeAll(async ({ api }) => { + await api.resetDatabase(); + await api.enableFeature('sharing'); + }); + + test('owner should create HTTP header credential and set to global', async ({ n8n }) => { + await n8n.api.signin('owner'); + + // Navigate to credentials page + await n8n.navigate.toCredentials(); + + // Create new credential + await n8n.credentials.addResource.credential(); + await n8n.credentials.selectCredentialType('Header Auth'); + + // Fill in credential fields + await n8n.credentials.credentialModal.fillField('name', 'Authorization'); + await n8n.credentials.credentialModal.fillField('value', 'Bearer test-token-123'); + + // Set credential name + await n8n.credentials.credentialModal.getCredentialName().click(); + await n8n.credentials.credentialModal.getNameInput().fill('Global HTTP Header Cred'); + + // Switch to Sharing tab + await n8n.credentials.credentialModal.changeTab('Sharing'); + + // Share with all users (set to global) + await n8n.credentials.credentialModal.getUsersSelect().click(); + await n8n.credentials.credentialModal.getVisibleDropdown().getByText('All users').click(); + + // Save the credential with sharing + await n8n.credentials.credentialModal.save(); + await n8n.credentials.credentialModal.close(); + + // Verify credential appears in list with global badge + await expect(n8n.credentials.cards.getCredential('Global HTTP Header Cred')).toBeVisible(); + await expect( + n8n.credentials.cards + .getCredential('Global HTTP Header Cred') + .getByTestId('credential-global-badge'), + ).toBeVisible(); + }); + + test('member should see global credential in credentials view', async ({ n8n }) => { + await n8n.api.signin('member', 0); + + // Navigate to credentials page + await n8n.navigate.toCredentials(); + + // Verify global credential is visible to member + await expect(n8n.credentials.cards.getCredential('Global HTTP Header Cred')).toBeVisible(); + + // Verify global badge is displayed + await expect( + n8n.credentials.cards + .getCredential('Global HTTP Header Cred') + .getByTestId('credential-global-badge'), + ).toBeVisible(); + }); + + test('member should execute workflow with HTTP node using global credential', async ({ + n8n, + baseURL, + }) => { + await n8n.api.signin('member', 0); + + // Create a new workflow + await n8n.navigate.toWorkflow('new'); + await n8n.canvas.setWorkflowName('Test Global Credential Workflow'); + + // Add manual trigger and HTTP Request node + await n8n.canvas.addNode('Manual Trigger'); + await n8n.canvas.addNode('HTTP Request'); + + await n8n.ndv.fillParameterInput('URL', `${baseURL}/rest/settings`); + await n8n.ndv.selectOptionInParameterDropdown('authentication', 'Generic Credential Type'); + await n8n.ndv.selectOptionInParameterDropdown('genericAuthType', 'Header Auth'); + + // Verify global credential is available in the credential select + const credentialSelect = n8n.ndv.getCredentialSelect(); + await credentialSelect.click(); + + // Check that global credential appears in dropdown + const dropdown = n8n.credentials.credentialModal.getVisibleDropdown(); + await expect(dropdown.getByText('Global HTTP Header Cred')).toBeVisible(); + + // Select the global credential + await dropdown.getByText('Global HTTP Header Cred').click(); + + // Verify credential is selected + await expect(credentialSelect).toHaveValue('Global HTTP Header Cred'); + + // Close NDV + await n8n.ndv.clickBackToCanvasButton(); + + // Save workflow + await n8n.canvas.saveWorkflow(); + + // Verify workflow saved successfully + await expect(n8n.canvas.getWorkflowNameInput()).toHaveValue('Test Global Credential Workflow'); + + await n8n.workflowComposer.executeWorkflowAndWaitForNotification( + 'Workflow executed successfully', + ); + }); + + test('owner should be able to remove global sharing', async ({ n8n }) => { + await n8n.api.signin('owner'); + + // Navigate to credentials page + await n8n.navigate.toCredentials(); + + // Open the global credential + await n8n.credentials.cards.getCredential('Global HTTP Header Cred').click(); + + // Switch to Sharing tab + await n8n.credentials.credentialModal.changeTab('Sharing'); + + // Verify "All users" is in the sharing list + await expect( + n8n.credentials.credentialModal + .getModal() + .getByTestId('project-sharing-list-item') + .filter({ hasText: 'All users' }), + ).toBeVisible(); + + // Remove global sharing by clicking the remove button + await n8n.credentials.credentialModal + .getModal() + .getByTestId('project-sharing-list-item') + .filter({ hasText: 'All users' }) + .getByTestId('project-sharing-remove') + .click(); + + // Save the changes + await n8n.credentials.credentialModal.save(); + await n8n.credentials.credentialModal.close(); + + // Verify global badge is no longer visible + await expect( + n8n.credentials.cards + .getCredential('Global HTTP Header Cred') + .getByTestId('credential-global-badge'), + ).not.toBeVisible(); + }); + + test('member should not see credential after global sharing removed', async ({ n8n }) => { + await n8n.api.signin('member', 0); + + // Navigate to credentials page + await n8n.navigate.toCredentials(); + + // Verify credential is no longer visible to member + await expect(n8n.credentials.cards.getCredential('Global HTTP Header Cred')).not.toBeVisible(); + }); +}); diff --git a/packages/workflow/src/interfaces.ts b/packages/workflow/src/interfaces.ts index b0e2213bba9..c9ace55d3f3 100644 --- a/packages/workflow/src/interfaces.ts +++ b/packages/workflow/src/interfaces.ts @@ -148,6 +148,7 @@ export interface ICredentialsDecrypted