mirror of
https://github.com/n8n-io/n8n.git
synced 2026-07-29 03:55:15 +02:00
fix(core): Reconcile per-user connections when a private credential is moved (#33630)
Some checks are pending
CI: Master (Build, Test, Lint) / Build for Github Cache (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (22.22.3) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (24.16.0) (push) Waiting to run
CI: Master (Build, Test, Lint) / Lint (push) Waiting to run
CI: Master (Build, Test, Lint) / Performance (push) Waiting to run
CI: Master (Build, Test, Lint) / Notify Slack on failure (push) Blocked by required conditions
Some checks are pending
CI: Master (Build, Test, Lint) / Build for Github Cache (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (22.22.3) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (24.16.0) (push) Waiting to run
CI: Master (Build, Test, Lint) / Lint (push) Waiting to run
CI: Master (Build, Test, Lint) / Performance (push) Waiting to run
CI: Master (Build, Test, Lint) / Notify Slack on failure (push) Blocked by required conditions
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
1bb253e21d
commit
136e774877
|
|
@ -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<ExternalSecretsConfig>();
|
||||
const externalSecretsProviderAccessCheckService = mock<SecretsProviderAccessCheckService>();
|
||||
const licenseState = mock<LicenseState>();
|
||||
const connectionStatusProxy = mock<CredentialConnectionStatusProxy>();
|
||||
|
||||
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<SharedCredentials>({
|
||||
credentialsId: credentialId,
|
||||
projectId: 'shared-project-id',
|
||||
role: 'credential:user',
|
||||
});
|
||||
credentialsFinderService.findCredentialForUser.mockResolvedValue(
|
||||
mock<CredentialsEntity>({
|
||||
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', () => {
|
||||
|
|
|
|||
|
|
@ -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<void>;
|
||||
cleanupOrphanedEntriesForUsers(
|
||||
userIds: string[],
|
||||
em?: EntityManager,
|
||||
credentialId?: string,
|
||||
): Promise<void>;
|
||||
|
||||
/**
|
||||
* 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<void>;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -33,8 +33,21 @@ export class CredentialConnectionStatusProxy implements ICredentialConnectionSta
|
|||
await this.provider.deleteAllUserEntries(credentialId, em);
|
||||
}
|
||||
|
||||
async cleanupOrphanedEntriesForUsers(userIds: string[], em?: EntityManager): Promise<void> {
|
||||
async cleanupOrphanedEntriesForUsers(
|
||||
userIds: string[],
|
||||
em?: EntityManager,
|
||||
credentialId?: string,
|
||||
): Promise<void> {
|
||||
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<void> {
|
||||
if (!this.provider || projectIds.length === 0) return;
|
||||
await this.provider.cleanupOrphanedEntriesForProjects(credentialId, projectIds, em);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<UserRepository>();
|
||||
const sharedCredentialsRepository = mock<SharedCredentialsRepository>();
|
||||
const roleService = mock<RoleService>();
|
||||
const projectRelationRepository = mock<ProjectRelationRepository>();
|
||||
const em = mock<EntityManager>();
|
||||
|
||||
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<string, unknown> }];
|
||||
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);
|
||||
|
|
|
|||
|
|
@ -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<Set<string>> {
|
||||
|
|
@ -77,14 +85,20 @@ export class CredentialConnectionStatusService implements ICredentialConnectionS
|
|||
await manager.delete(DynamicCredentialUserEntry, { credentialId });
|
||||
}
|
||||
|
||||
async cleanupOrphanedEntriesForUsers(userIds: string[], em?: EntityManager): Promise<void> {
|
||||
async cleanupOrphanedEntriesForUsers(
|
||||
userIds: string[],
|
||||
em?: EntityManager,
|
||||
credentialId?: string,
|
||||
): Promise<void> {
|
||||
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<void> {
|
||||
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<void> {
|
||||
|
|
@ -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<string>();
|
||||
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[],
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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...",
|
||||
|
|
|
|||
|
|
@ -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<typeof useCredentialsStore>;
|
|||
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<typeof ProjectMoveResourceModal> => ({
|
||||
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({
|
||||
|
|
|
|||
|
|
@ -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 () => {
|
|||
</I18nT>
|
||||
</N8nCallout>
|
||||
</N8nText>
|
||||
<N8nCallout
|
||||
v-if="isResolvableCredential"
|
||||
theme="warning"
|
||||
:class="$style.textBlock"
|
||||
data-test-id="project-move-resource-modal-resolvable-warning"
|
||||
>
|
||||
{{ i18n.baseText('projects.move.resource.modal.message.resolvableConnections') }}
|
||||
</N8nCallout>
|
||||
</div>
|
||||
</template>
|
||||
<template #footer>
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user