From 136e774877b5923e8fe4580da8e1fc7e1af20a79 Mon Sep 17 00:00:00 2001 From: Guillaume Jacquart Date: Tue, 7 Jul 2026 09:40:49 +0200 Subject: [PATCH] fix(core): Reconcile per-user connections when a private credential is moved (#33630) Co-authored-by: Claude Opus 4.8 (1M context) --- .../__tests__/credentials.service.ee.test.ts | 36 ++++++ ...al-connection-status-provider.interface.ts | 18 ++- .../credential-connection-status-proxy.ts | 17 ++- .../src/credentials/credentials.controller.ts | 12 +- .../src/credentials/credentials.service.ee.ts | 14 ++- ...edential-connection-status.service.test.ts | 107 ++++++++++++++++-- .../credential-connection-status.service.ts | 56 +++++++-- .../credentials.resolvable.api.test.ts | 73 +++++++++++- .../frontend/@n8n/i18n/src/locales/en.json | 1 + .../ProjectMoveResourceModal.test.ts | 57 ++++++++++ .../components/ProjectMoveResourceModal.vue | 17 +++ 11 files changed, 376 insertions(+), 32 deletions(-) diff --git a/packages/cli/src/credentials/__tests__/credentials.service.ee.test.ts b/packages/cli/src/credentials/__tests__/credentials.service.ee.test.ts index 1acda669d77..ea2762d8057 100644 --- a/packages/cli/src/credentials/__tests__/credentials.service.ee.test.ts +++ b/packages/cli/src/credentials/__tests__/credentials.service.ee.test.ts @@ -8,6 +8,7 @@ import type { } from '@n8n/db'; import { mock } from 'vitest-mock-extended'; +import type { CredentialConnectionStatusProxy } from '@/credentials/credential-connection-status-proxy'; import type { CredentialsFinderService } from '@/credentials/credentials-finder.service'; import type { CredentialsService } from '@/credentials/credentials.service'; import { EnterpriseCredentialsService } from '@/credentials/credentials.service.ee'; @@ -27,6 +28,7 @@ describe('EnterpriseCredentialsService', () => { const externalSecretsConfig = mock(); const externalSecretsProviderAccessCheckService = mock(); const licenseState = mock(); + const connectionStatusProxy = mock(); const service = new EnterpriseCredentialsService( sharedCredentialsRepository, @@ -38,6 +40,7 @@ describe('EnterpriseCredentialsService', () => { externalSecretsConfig, externalSecretsProviderAccessCheckService, licenseState, + connectionStatusProxy, ); beforeEach(() => { @@ -192,6 +195,39 @@ describe('EnterpriseCredentialsService', () => { ).not.toHaveBeenCalled(); }); }); + + describe('per-user connection reconciliation', () => { + beforeEach(() => { + // keep the external-secrets branch out of the way + externalSecretsConfig.externalSecretsForProjects = false; + }); + + it('reconciles connections for every project that shared the credential', async () => { + const sharee = mock({ + credentialsId: credentialId, + projectId: 'shared-project-id', + role: 'credential:user', + }); + credentialsFinderService.findCredentialForUser.mockResolvedValue( + mock({ + id: credentialId, + name: 'Test Credential', + type: 'testApi', + data: 'encrypted-data', + shared: [ownerSharing, sharee], + }), + ); + const trx = mockTransactionManager(); + + await service.transferOne(user, credentialId, destinationProjectId); + + expect(connectionStatusProxy.cleanupOrphanedEntriesForProjects).toHaveBeenCalledWith( + credentialId, + [sourceProjectId, 'shared-project-id'], + trx, + ); + }); + }); }); describe('getOneForUser', () => { diff --git a/packages/cli/src/credentials/credential-connection-status-provider.interface.ts b/packages/cli/src/credentials/credential-connection-status-provider.interface.ts index a322270f7fc..f7980ecfeba 100644 --- a/packages/cli/src/credentials/credential-connection-status-provider.interface.ts +++ b/packages/cli/src/credentials/credential-connection-status-provider.interface.ts @@ -37,7 +37,21 @@ export interface ICredentialConnectionStatusProvider { /** * Re-evaluates access for the given users and deletes all their per-user * entries (across all resolvers) for any credential where they no longer - * hold `credential:update`. + * hold `credential:connect`. Pass `credentialId` to scope to one credential. */ - cleanupOrphanedEntriesForUsers(userIds: string[], em?: EntityManager): Promise; + cleanupOrphanedEntriesForUsers( + userIds: string[], + em?: EntityManager, + credentialId?: string, + ): Promise; + + /** + * Re-evaluates one credential's connections for members of the given + * projects, deleting those who no longer hold `credential:connect`. + */ + cleanupOrphanedEntriesForProjects( + credentialId: string, + projectIds: string[], + em?: EntityManager, + ): Promise; } diff --git a/packages/cli/src/credentials/credential-connection-status-proxy.ts b/packages/cli/src/credentials/credential-connection-status-proxy.ts index 300adbac615..7bd9ec3112c 100644 --- a/packages/cli/src/credentials/credential-connection-status-proxy.ts +++ b/packages/cli/src/credentials/credential-connection-status-proxy.ts @@ -33,8 +33,21 @@ export class CredentialConnectionStatusProxy implements ICredentialConnectionSta await this.provider.deleteAllUserEntries(credentialId, em); } - async cleanupOrphanedEntriesForUsers(userIds: string[], em?: EntityManager): Promise { + async cleanupOrphanedEntriesForUsers( + userIds: string[], + em?: EntityManager, + credentialId?: string, + ): Promise { if (!this.provider || userIds.length === 0) return; - await this.provider.cleanupOrphanedEntriesForUsers(userIds, em); + await this.provider.cleanupOrphanedEntriesForUsers(userIds, em, credentialId); + } + + async cleanupOrphanedEntriesForProjects( + credentialId: string, + projectIds: string[], + em?: EntityManager, + ): Promise { + if (!this.provider || projectIds.length === 0) return; + await this.provider.cleanupOrphanedEntriesForProjects(credentialId, projectIds, em); } } diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 4699eccd95b..0873d656271 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -425,12 +425,6 @@ export class CredentialsController { } } - const unsharedProjectMembers = - toUnshare.length > 0 - ? await this.projectRelationRepository.findBy({ projectId: In(toUnshare) }) - : []; - const affectedUserIds = [...new Set(unsharedProjectMembers.map((pr) => pr.userId))]; - let amountRemoved: number | null = null; let newShareeIds: string[] = []; @@ -449,7 +443,11 @@ export class CredentialsController { if (deleteResult.affected) { amountRemoved = deleteResult.affected; - await this.connectionStatusProxy.cleanupOrphanedEntriesForUsers(affectedUserIds, trx); + await this.connectionStatusProxy.cleanupOrphanedEntriesForProjects( + credentialId, + toUnshare, + trx, + ); } newShareeIds = toShare; diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts index 795f645a31e..f7b84b2f59c 100644 --- a/packages/cli/src/credentials/credentials.service.ee.ts +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -15,6 +15,7 @@ import { OwnershipService } from '@/services/ownership.service'; import { ProjectService } from '@/services/project.service.ee'; import { RoleService } from '@/services/role.service'; +import { CredentialConnectionStatusProxy } from './credential-connection-status-proxy'; import { CredentialsFinderService } from './credentials-finder.service'; import { CredentialsService } from './credentials.service'; import { validateAccessToReferencedSecretProviders } from './validation'; @@ -31,6 +32,7 @@ export class EnterpriseCredentialsService { private readonly externalSecretsConfig: ExternalSecretsConfig, private readonly externalSecretsProviderAccessCheckService: SecretsProviderAccessCheckService, private readonly licenseState: LicenseState, + private readonly connectionStatusProxy: CredentialConnectionStatusProxy, ) {} async shareWithProjects( @@ -223,8 +225,11 @@ export class EnterpriseCredentialsService { ); } + // 7. projects losing access — the move drops all their sharings + const affectedProjectIds = [...new Set(credential.shared.map((s) => s.projectId))]; + await this.sharedCredentialsRepository.manager.transaction(async (trx) => { - // 7. transfer the credential + // 8. transfer the credential // remove all sharings await trx.remove(credential.shared); @@ -236,6 +241,13 @@ export class EnterpriseCredentialsService { role: 'credential:owner', }), ); + + // 9. drop connections for members who lost access in the new project + await this.connectionStatusProxy.cleanupOrphanedEntriesForProjects( + credential.id, + affectedProjectIds, + trx, + ); }); } } diff --git a/packages/cli/src/modules/dynamic-credentials.ee/services/__tests__/credential-connection-status.service.test.ts b/packages/cli/src/modules/dynamic-credentials.ee/services/__tests__/credential-connection-status.service.test.ts index 82b953f9821..cfeb5828975 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/services/__tests__/credential-connection-status.service.test.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/services/__tests__/credential-connection-status.service.test.ts @@ -1,4 +1,9 @@ -import type { SharedCredentialsRepository, User, UserRepository } from '@n8n/db'; +import type { + ProjectRelationRepository, + SharedCredentialsRepository, + User, + UserRepository, +} from '@n8n/db'; import type { EntityManager } from '@n8n/typeorm'; import { mock } from 'vitest-mock-extended'; @@ -24,6 +29,7 @@ describe('CredentialConnectionStatusService', () => { const userRepository = mock(); const sharedCredentialsRepository = mock(); const roleService = mock(); + const projectRelationRepository = mock(); const em = mock(); const service = new CredentialConnectionStatusService( @@ -31,6 +37,7 @@ describe('CredentialConnectionStatusService', () => { userRepository, sharedCredentialsRepository, roleService, + projectRelationRepository, ); const CRED_ID = 'cred-1'; @@ -66,7 +73,7 @@ describe('CredentialConnectionStatusService', () => { it('marks pair as orphaned when the user has been deleted from the DB', async () => { // ARRANGE em.find.mockResolvedValueOnce([makeEntry(CRED_ID, 'deleted-user')]); - userRepository.find.mockResolvedValueOnce([]); // user no longer exists + em.find.mockResolvedValueOnce([]); // user no longer exists // ACT await service.cleanupOrphanedEntriesForUsers(['deleted-user'], em); @@ -78,11 +85,11 @@ describe('CredentialConnectionStatusService', () => { ); }); - it('retains pair for a global admin (has credential:update in global role scopes)', async () => { + it('retains pair for a global admin (has credential:connect in global role scopes)', async () => { // ARRANGE - const admin = makeUser('admin-1', ['credential:update']); + const admin = makeUser('admin-1', ['credential:connect']); em.find.mockResolvedValueOnce([makeEntry(CRED_ID, 'admin-1')]); - userRepository.find.mockResolvedValueOnce([admin]); + em.find.mockResolvedValueOnce([admin]); // Even if the project check returned nothing, admin is retained in-memory sharedCredentialsRepository.findPairsWithCredentialAccess.mockResolvedValueOnce([]); @@ -93,11 +100,40 @@ describe('CredentialConnectionStatusService', () => { expect(repository.deleteByPairs).not.toHaveBeenCalled(); }); + it('retains pair for a connect-only sharee (has credential:connect but not credential:update)', async () => { + // ARRANGE — a `credential:user` sharee can still connect their own account, + // so their per-user entry must survive even without edit rights. + const sharee = makeUser('sharee-1'); // no global scope + em.find.mockResolvedValueOnce([makeEntry(CRED_ID, 'sharee-1')]); + em.find.mockResolvedValueOnce([sharee]); + // Project path retained via credential:connect + sharedCredentialsRepository.findPairsWithCredentialAccess.mockResolvedValueOnce([ + { credentialId: CRED_ID, userId: 'sharee-1' }, + ]); + + // ACT + await service.cleanupOrphanedEntriesForUsers(['sharee-1'], em); + + // ASSERT — retention is evaluated against credential:connect, not credential:update + expect(roleService.rolesWithScope).toHaveBeenCalledWith( + 'credential', + 'credential:connect', + em, + ); + expect(sharedCredentialsRepository.findPairsWithCredentialAccess).toHaveBeenCalledWith( + [{ credentialId: CRED_ID, userId: 'sharee-1' }], + 'credential:connect', + VALID_CRED_ROLES, + em, + ); + expect(repository.deleteByPairs).not.toHaveBeenCalled(); + }); + it('retains pair for a member who still has project-level credential:update', async () => { // ARRANGE const member = makeUser('member-1'); // no global scope em.find.mockResolvedValueOnce([makeEntry(CRED_ID, 'member-1')]); - userRepository.find.mockResolvedValueOnce([member]); + em.find.mockResolvedValueOnce([member]); // DB check confirms the project path is still alive sharedCredentialsRepository.findPairsWithCredentialAccess.mockResolvedValueOnce([ { credentialId: CRED_ID, userId: 'member-1' }, @@ -114,7 +150,7 @@ describe('CredentialConnectionStatusService', () => { // ARRANGE const member = makeUser('member-1'); // no global scope em.find.mockResolvedValueOnce([makeEntry(CRED_ID, 'member-1')]); - userRepository.find.mockResolvedValueOnce([member]); + em.find.mockResolvedValueOnce([member]); // No retained pairs — member was unshared / removed / role downgraded sharedCredentialsRepository.findPairsWithCredentialAccess.mockResolvedValueOnce([]); @@ -136,7 +172,7 @@ describe('CredentialConnectionStatusService', () => { makeEntry(CRED_ID, 'member-1'), // resolver A makeEntry(CRED_ID, 'member-1'), // resolver B (duplicate pair) ]); - userRepository.find.mockResolvedValueOnce([member]); + em.find.mockResolvedValueOnce([member]); // One project path was removed, but the second survives sharedCredentialsRepository.findPairsWithCredentialAccess.mockResolvedValueOnce([ { credentialId: CRED_ID, userId: 'member-1' }, @@ -160,7 +196,7 @@ describe('CredentialConnectionStatusService', () => { makeEntry(CRED_A, 'user-lost'), makeEntry(CRED_B, 'user-active'), ]); - userRepository.find.mockResolvedValueOnce([lostMember, activeMember]); + em.find.mockResolvedValueOnce([lostMember, activeMember]); // Only user-active retains access sharedCredentialsRepository.findPairsWithCredentialAccess.mockResolvedValueOnce([ { credentialId: CRED_B, userId: 'user-active' }, @@ -177,11 +213,62 @@ describe('CredentialConnectionStatusService', () => { }); }); + describe('credentialId scoping', () => { + it('scopes the entry scan to the given credential when credentialId is provided', async () => { + em.find.mockResolvedValueOnce([]); + + await service.cleanupOrphanedEntriesForUsers(['user-1'], em, 'cred-9'); + + expect(em.find).toHaveBeenCalledWith( + DynamicCredentialUserEntry, + expect.objectContaining({ where: expect.objectContaining({ credentialId: 'cred-9' }) }), + ); + }); + + it('does not filter by credential when credentialId is omitted', async () => { + em.find.mockResolvedValueOnce([]); + + await service.cleanupOrphanedEntriesForUsers(['user-1'], em); + + const [, options] = em.find.mock.calls[0] as [unknown, { where: Record }]; + expect(options.where).not.toHaveProperty('credentialId'); + }); + }); + + describe('cleanupOrphanedEntriesForProjects', () => { + it('is a no-op when projectIds is empty', async () => { + await service.cleanupOrphanedEntriesForProjects('cred-1', [], em); + + expect(em.findBy).not.toHaveBeenCalled(); + expect(em.find).not.toHaveBeenCalled(); + }); + + it('resolves distinct project members and prunes the credential scoped to them', async () => { + em.findBy.mockResolvedValueOnce([ + { userId: 'user-a' }, + { userId: 'user-b' }, + { userId: 'user-a' }, // duplicate across projects + ] as never); + em.find.mockResolvedValueOnce([]); // no entries → early return + + await service.cleanupOrphanedEntriesForProjects('cred-1', ['proj-1', 'proj-2'], em); + + expect(em.findBy).toHaveBeenCalledWith( + projectRelationRepository.target, + expect.objectContaining({ projectId: expect.anything() }), + ); + expect(em.find).toHaveBeenCalledWith( + DynamicCredentialUserEntry, + expect.objectContaining({ where: expect.objectContaining({ credentialId: 'cred-1' }) }), + ); + }); + }); + describe('deleteOrphanedPairs — DB check scope', () => { it('skips the project DB check when every user has been deleted', async () => { // ARRANGE — no user record in DB → pairsToCheck is empty → DB query skipped em.find.mockResolvedValueOnce([makeEntry(CRED_ID, 'ghost-user')]); - userRepository.find.mockResolvedValueOnce([]); + em.find.mockResolvedValueOnce([]); // ACT await service.cleanupOrphanedEntriesForUsers(['ghost-user'], em); diff --git a/packages/cli/src/modules/dynamic-credentials.ee/services/credential-connection-status.service.ts b/packages/cli/src/modules/dynamic-credentials.ee/services/credential-connection-status.service.ts index a4acdb1862c..f9d43941ff9 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/services/credential-connection-status.service.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/services/credential-connection-status.service.ts @@ -1,7 +1,12 @@ -import { In, SharedCredentialsRepository, UserRepository, type User } from '@n8n/db'; +import { + In, + ProjectRelationRepository, + SharedCredentialsRepository, + UserRepository, + type User, +} from '@n8n/db'; import { Service } from '@n8n/di'; import { hasGlobalScope } from '@n8n/permissions'; -// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import type { EntityManager } from '@n8n/typeorm'; import type { ICredentialConnectionStatusProvider } from '@/credentials/credential-connection-status-provider.interface'; @@ -13,7 +18,9 @@ import { DynamicCredentialUserEntryRepository } from '../database/repositories/d type CredentialUserPair = { credentialId: string; userId: string }; -const CREDENTIAL_RETAIN_SCOPE = 'credential:update' as const; +// A per-user connection is retained while the user can still connect their own +// account — `credential:connect`, not `credential:update` (see oauth.service.ts). +const CREDENTIAL_RETAIN_SCOPE = 'credential:connect' as const; const keyOf = (pair: CredentialUserPair) => `${pair.credentialId}|${pair.userId}`; @@ -36,6 +43,7 @@ export class CredentialConnectionStatusService implements ICredentialConnectionS private readonly userRepository: UserRepository, private readonly sharedCredentialsRepository: SharedCredentialsRepository, private readonly roleService: RoleService, + private readonly projectRelationRepository: ProjectRelationRepository, ) {} async findConnectedCredentialIds(userId: string, credentialIds: string[]): Promise> { @@ -77,14 +85,20 @@ export class CredentialConnectionStatusService implements ICredentialConnectionS await manager.delete(DynamicCredentialUserEntry, { credentialId }); } - async cleanupOrphanedEntriesForUsers(userIds: string[], em?: EntityManager): Promise { + async cleanupOrphanedEntriesForUsers( + userIds: string[], + em?: EntityManager, + credentialId?: string, + ): Promise { if (userIds.length === 0) return; const manager = em ?? this.repository.manager; + // When a credentialId is given, scope the scan to that credential so a + // single-credential event never re-evaluates unrelated connections. const entries = await manager.find(DynamicCredentialUserEntry, { select: ['credentialId', 'userId'], - where: { userId: In(userIds) }, + where: { userId: In(userIds), ...(credentialId ? { credentialId } : {}) }, }); if (entries.length === 0) return; @@ -93,9 +107,30 @@ export class CredentialConnectionStatusService implements ICredentialConnectionS await this.deleteOrphanedPairs(pairs, manager); } + /** + * Re-evaluates the given credential's connections for members of the given + * projects, deleting those who no longer retain access. Used when an event + * changes who can see one credential (unshare, move to another project). + */ + async cleanupOrphanedEntriesForProjects( + credentialId: string, + projectIds: string[], + em?: EntityManager, + ): Promise { + if (projectIds.length === 0) return; + + const manager = em ?? this.repository.manager; + const members = await manager.findBy(this.projectRelationRepository.target, { + projectId: In(projectIds), + }); + const userIds = [...new Set(members.map((m) => m.userId))]; + + await this.cleanupOrphanedEntriesForUsers(userIds, em, credentialId); + } + /** * Deletes ALL per-user entries (across all resolvers) for each - * (credentialId, userId) pair whose user no longer holds `credential:update` + * (credentialId, userId) pair whose user no longer holds `credential:connect` * on the credential. */ private async deleteOrphanedPairs(pairs: CredentialUserPair[], em: EntityManager): Promise { @@ -103,7 +138,9 @@ export class CredentialConnectionStatusService implements ICredentialConnectionS const uniquePairs = [...new Map(pairs.map((p) => [keyOf(p), p])).values()]; - const users = await this.userRepository.find({ + // All reads use `em` so a caller's transaction never has to acquire a + // second pooled connection (which deadlocks at pool size 1). + const users = await em.find(this.userRepository.target, { where: { id: In(uniquePairs.map((p) => p.userId)) }, relations: { role: { scopes: true } }, }); @@ -113,10 +150,11 @@ export class CredentialConnectionStatusService implements ICredentialConnectionS let projectRetainedKeys = new Set(); if (pairsToCheck.length > 0) { - // Credential roles that carry credential:update — served from the role + // Credential roles that carry credential:connect — served from the role const validCredRoles = await this.roleService.rolesWithScope( 'credential', CREDENTIAL_RETAIN_SCOPE, + em, ); const projectRetained = await this.sharedCredentialsRepository.findPairsWithCredentialAccess( pairsToCheck, @@ -134,7 +172,7 @@ export class CredentialConnectionStatusService implements ICredentialConnectionS } /** - * A pair is orphaned unless the user retains `credential:update`. + * A pair is orphaned unless the user retains `credential:connect`. */ private selectOrphanedPairs( pairs: CredentialUserPair[], diff --git a/packages/cli/test/integration/credentials/credentials.resolvable.api.test.ts b/packages/cli/test/integration/credentials/credentials.resolvable.api.test.ts index 4824af70eba..e083b133c2f 100644 --- a/packages/cli/test/integration/credentials/credentials.resolvable.api.test.ts +++ b/packages/cli/test/integration/credentials/credentials.resolvable.api.test.ts @@ -25,7 +25,7 @@ import { saveCredential, shareCredentialWithProjects, } from '../shared/db/credentials'; -import { createMember } from '../shared/db/users'; +import { createAdmin, createMember } from '../shared/db/users'; import { setupTestServer } from '../shared/utils'; mockInstance(Telemetry); @@ -392,6 +392,77 @@ describe('PATCH /credentials/:id — isResolvable toggle cleanup', () => { }); }); +describe('PUT /credentials/:id/transfer — resolvable connection reconciliation', () => { + const entryRepository = () => Container.get(DynamicCredentialUserEntryRepository); + + test('removes the connection of a member who loses access in the destination project', async () => { + const resolvable = await saveResolvableCredential(); + // memberB is an editor of the source project and connected their account. + await seedUserEntry(resolvable.id, memberB.id); + + // Destination project that memberB is NOT part of. + const destinationProject = await createTeamProject(undefined, memberA); + + await testServer + .authAgentFor(memberA) + .put(`/credentials/${resolvable.id}/transfer`) + .send({ destinationProjectId: destinationProject.id }) + .expect(200); + + const remaining = await entryRepository().countBy({ + credentialId: resolvable.id, + userId: memberB.id, + }); + expect(remaining).toBe(0); + }); + + test('keeps the connection of a member who retains access via the destination project', async () => { + const resolvable = await saveResolvableCredential(); + await seedUserEntry(resolvable.id, memberB.id); + + // memberB is also a member of the destination project, so they keep + // credential:connect after the move. + const destinationProject = await createTeamProject(undefined, memberA); + await linkUserToProject(memberB, destinationProject, 'project:editor'); + + await testServer + .authAgentFor(memberA) + .put(`/credentials/${resolvable.id}/transfer`) + .send({ destinationProjectId: destinationProject.id }) + .expect(200); + + const remaining = await entryRepository().countBy({ + credentialId: resolvable.id, + userId: memberB.id, + }); + expect(remaining).toBe(1); + }); + + test('keeps the connection of a user who retains access via a global role', async () => { + const admin = await createAdmin(); + // Admin is a source-project member (so they are re-evaluated), but has no + // access in the destination project — global scope is what retains them. + await linkUserToProject(admin, teamProject, 'project:viewer'); + + const resolvable = await saveResolvableCredential(); + await seedUserEntry(resolvable.id, admin.id); + + const destinationProject = await createTeamProject(undefined, memberA); + + await testServer + .authAgentFor(memberA) + .put(`/credentials/${resolvable.id}/transfer`) + .send({ destinationProjectId: destinationProject.id }) + .expect(200); + + const remaining = await entryRepository().countBy({ + credentialId: resolvable.id, + userId: admin.id, + }); + expect(remaining).toBe(1); + }); +}); + describe('Sharing dynamic credentials', () => { test('PUT /credentials/:id/share — allows sharing a dynamic credential', async () => { const resolvable = await saveResolvableCredential(); diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 0e7975f7b74..86910e5b36e 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -5239,6 +5239,7 @@ "projects.move.resource.modal.message.unAccessibleCredentials.count": "{count} credential | {count} credentials", "projects.move.resource.modal.message.unAccessibleCredentials.note": "{credentials} can't be shared. Move or recreate them in the destination project to keep this workflow running.", "projects.move.resource.modal.message.unAccessibleCredentials.personalSpaceNote": "{credentials} can't be shared because your admin has disabled sharing from personal spaces. Move or recreate them in the project or user you're moving into, to keep this workflow running.", + "projects.move.resource.modal.message.resolvableConnections": "Each user connects their own account to this credential. Users who lose access after the move will lose their connection. Users who keep access (such as admins) aren't affected.", "projects.move.resource.modal.message.noProjects": "Currently there are not any projects or users available for you to move this {resourceTypeLabel} to.", "projects.move.resource.modal.button": "Move {resourceTypeLabel}", "projects.move.resource.modal.selectPlaceholder": "Select project or user...", diff --git a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectMoveResourceModal.test.ts b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectMoveResourceModal.test.ts index ea16f5881c5..e02e1b4e1cd 100644 --- a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectMoveResourceModal.test.ts +++ b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectMoveResourceModal.test.ts @@ -14,6 +14,12 @@ import { useCredentialsStore } from '@/features/credentials/credentials.store'; import type { ComponentProps } from 'vue-component-type-helpers'; import { ResourceType } from '../projects.utils'; import type { ProjectSharingData } from 'n8n-workflow'; +import type { ICredentialsResponse } from '@/features/credentials/credentials.types'; + +const isPrivateCredentialsEnabled = { value: false }; +vi.mock('@/features/resolvers/composables/usePrivateCredentials', () => ({ + usePrivateCredentials: () => ({ isEnabled: isPrivateCredentialsEnabled }), +})); const renderComponent = createComponentRenderer(ProjectMoveResourceModal, { pinia: createTestingPinia(), @@ -35,6 +41,7 @@ let credentialsStore: MockedStore; describe('ProjectMoveResourceModal', () => { beforeEach(() => { vi.clearAllMocks(); + isPrivateCredentialsEnabled.value = false; telemetry = useTelemetry(); projectsStore = mockedStore(useProjectsStore); workflowsListStore = mockedStore(useWorkflowsListStore); @@ -254,6 +261,56 @@ describe('ProjectMoveResourceModal', () => { ); }); + describe('resolvable credential warning', () => { + const makeCredential = (isResolvable: boolean): ICredentialsResponse => + ({ + id: '1', + name: 'My private credential', + type: 'oAuth2Api', + createdAt: '2021-01-01T00:00:00Z', + updatedAt: '2021-01-01T00:00:00Z', + isManaged: false, + isResolvable, + homeProject: { id: '2', name: 'My Project' } as ProjectSharingData, + }) as ICredentialsResponse; + + const props = (isResolvable: boolean): ComponentProps => ({ + modalName: PROJECT_MOVE_RESOURCE_MODAL, + data: { + resourceType: ResourceType.Credential, + resourceTypeLabel: 'credential', + resource: makeCredential(isResolvable), + }, + }); + + beforeEach(() => { + isPrivateCredentialsEnabled.value = true; + projectsStore.searchProjects.mockResolvedValue({ count: 1, data: [createProjectListItem()] }); + }); + + it('warns when moving a resolvable credential', async () => { + const { findByTestId } = renderComponent({ props: props(true) }); + + expect(await findByTestId('project-move-resource-modal-resolvable-warning')).toBeVisible(); + }); + + it('does not warn when moving a non-resolvable credential', async () => { + const { queryByTestId } = renderComponent({ props: props(false) }); + await vi.waitFor(() => expect(projectsStore.searchProjects).toHaveBeenCalled()); + + expect(queryByTestId('project-move-resource-modal-resolvable-warning')).toBeNull(); + }); + + it('does not warn when private credentials are disabled', async () => { + isPrivateCredentialsEnabled.value = false; + + const { queryByTestId } = renderComponent({ props: props(true) }); + await vi.waitFor(() => expect(projectsStore.searchProjects).toHaveBeenCalled()); + + expect(queryByTestId('project-move-resource-modal-resolvable-warning')).toBeNull(); + }); + }); + it('should prevent duplicate submissions when button clicked multiple times', async () => { const destinationProject = createProjectListItem(); projectsStore.searchProjects.mockResolvedValue({ diff --git a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectMoveResourceModal.vue b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectMoveResourceModal.vue index 57d920bf3a2..98240a2d65a 100644 --- a/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectMoveResourceModal.vue +++ b/packages/frontend/editor-ui/src/features/collaboration/projects/components/ProjectMoveResourceModal.vue @@ -12,6 +12,7 @@ import type { import type { IWorkflowDb } from '@/Interface'; import { getResourcePermissions } from '@n8n/permissions'; import { useCredentialsStore } from '@/features/credentials/credentials.store'; +import { usePrivateCredentials } from '@/features/resolvers/composables/usePrivateCredentials'; import { useProjectsStore } from '../projects.store'; import { useUIStore } from '@/app/stores/ui.store'; import { useWorkflowsListStore } from '@/app/stores/workflowsList.store'; @@ -55,6 +56,7 @@ const projectsStore = useProjectsStore(); const searchFn = useAvailableProjectSearch(); const workflowsListStore = useWorkflowsListStore(); const credentialsStore = useCredentialsStore(); +const privateCredentials = usePrivateCredentials(); const telemetry = useTelemetry(); const { showMoveToProjectToast } = useMoveResourceToProjectToast(); @@ -96,6 +98,13 @@ const projectFilterFn = (p: ProjectListItem): boolean => const isResourceInTeamProject = computed(() => isHomeProjectTeam(props.data.resource)); const isResourceWorkflow = computed(() => props.data.resourceType === ResourceType.Workflow); + +const isResolvableCredential = computed( + () => + privateCredentials.isEnabled.value && + props.data.resourceType === ResourceType.Credential && + (props.data.resource as ICredentialsResponse).isResolvable === true, +); const targetProjectName = computed(() => { return getTruncatedProjectName(selectedProject.value?.name); }); @@ -324,6 +333,14 @@ onMounted(async () => { + + {{ i18n.baseText('projects.move.resource.modal.message.resolvableConnections') }} +