From 0bec8614bb3beb37e9e880901e9b48c18d64d5a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 1 Feb 2024 15:09:54 +0100 Subject: [PATCH 1/4] when sharing is enabled just check if the user has access to the credential and not that they are the owner --- .../cli/src/UserManagement/PermissionChecker.ts | 8 ++++++-- .../repositories/sharedCredentials.repository.ts | 14 +++++++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts index 7b6261c4d6d18..b15d9ee6ddbaa 100644 --- a/packages/cli/src/UserManagement/PermissionChecker.ts +++ b/packages/cli/src/UserManagement/PermissionChecker.ts @@ -39,12 +39,14 @@ export class PermissionChecker { if (user.hasGlobalScope('workflow:execute')) return; + const isSharingEnabled = this.license.isSharingEnabled(); + // allow if all creds used in this workflow are a subset of // all creds accessible to users who have access to this workflow let workflowUserIds = [userId]; - if (workflow.id && this.license.isSharingEnabled()) { + if (workflow.id && isSharingEnabled) { const workflowSharings = await this.sharedWorkflowRepository.find({ relations: ['workflow'], where: { workflowId: workflow.id }, @@ -54,7 +56,9 @@ export class PermissionChecker { } const credentialSharings = - await this.sharedCredentialsRepository.findOwnedSharings(workflowUserIds); + await this.sharedCredentialsRepository[ + isSharingEnabled ? 'findAccessibleSharings' : 'findOwnedSharings' + ](workflowUserIds); const accessibleCredIds = credentialSharings.map((s) => s.credentialsId); diff --git a/packages/cli/src/databases/repositories/sharedCredentials.repository.ts b/packages/cli/src/databases/repositories/sharedCredentials.repository.ts index 0a52f521538d5..a83aef45a5d07 100644 --- a/packages/cli/src/databases/repositories/sharedCredentials.repository.ts +++ b/packages/cli/src/databases/repositories/sharedCredentials.repository.ts @@ -50,12 +50,16 @@ export class SharedCredentialsRepository extends Repository { return sharings.map((s) => s.credentialsId); } + async findAccessibleSharings(userIds: string[]) { + return await this.findBy({ + userId: In(userIds), + }); + } + async findOwnedSharings(userIds: string[]) { - return await this.find({ - where: { - userId: In(userIds), - role: 'credential:owner', - }, + return await this.findBy({ + userId: In(userIds), + role: 'credential:owner', }); } From 0fc262a279891c06f6ab11befafc01c96d0d5d2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 1 Feb 2024 16:58:11 +0100 Subject: [PATCH 2/4] if it uses database, it's not a unit test --- .../cli/test/{unit => integration}/PermissionChecker.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename packages/cli/test/{unit => integration}/PermissionChecker.test.ts (99%) diff --git a/packages/cli/test/unit/PermissionChecker.test.ts b/packages/cli/test/integration/PermissionChecker.test.ts similarity index 99% rename from packages/cli/test/unit/PermissionChecker.test.ts rename to packages/cli/test/integration/PermissionChecker.test.ts index 5eb4b6e0ea7cd..b8035926e8df7 100644 --- a/packages/cli/test/unit/PermissionChecker.test.ts +++ b/packages/cli/test/integration/PermissionChecker.test.ts @@ -24,7 +24,7 @@ import { import { LicenseMocker } from '../integration/shared/license'; import * as testDb from '../integration/shared/testDb'; import type { SaveCredentialFunction } from '../integration/shared/types'; -import { mockNodeTypesData } from './Helpers'; +import { mockNodeTypesData } from '../unit/Helpers'; import { affixRoleToSaveCredential } from '../integration/shared/db/credentials'; import { createOwner, createUser } from '../integration/shared/db/users'; From de0423f9852a8d236b603da5beb7bf06a394d3c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 1 Feb 2024 18:06:35 +0100 Subject: [PATCH 3/4] add tests --- .../src/UserManagement/PermissionChecker.ts | 17 +-- .../src/credentials/credentials.service.ts | 2 +- .../sharedCredentials.repository.ts | 34 ++--- .../repositories/sharedWorkflow.repository.ts | 8 ++ .../cli/test/unit/PermissionChecker.test.ts | 135 ++++++++++++++++++ 5 files changed, 162 insertions(+), 34 deletions(-) create mode 100644 packages/cli/test/unit/PermissionChecker.test.ts diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts index b15d9ee6ddbaa..70c52a0300a7a 100644 --- a/packages/cli/src/UserManagement/PermissionChecker.ts +++ b/packages/cli/src/UserManagement/PermissionChecker.ts @@ -47,20 +47,13 @@ export class PermissionChecker { let workflowUserIds = [userId]; if (workflow.id && isSharingEnabled) { - const workflowSharings = await this.sharedWorkflowRepository.find({ - relations: ['workflow'], - where: { workflowId: workflow.id }, - select: ['userId'], - }); - workflowUserIds = workflowSharings.map((s) => s.userId); + workflowUserIds = await this.sharedWorkflowRepository.getSharedUserIds(workflow.id); } - const credentialSharings = - await this.sharedCredentialsRepository[ - isSharingEnabled ? 'findAccessibleSharings' : 'findOwnedSharings' - ](workflowUserIds); - - const accessibleCredIds = credentialSharings.map((s) => s.credentialsId); + const accessibleCredIds = await this.sharedCredentialsRepository.getCredentialIdsForUserAndRole( + workflowUserIds, + isSharingEnabled ? ['credential:owner', 'credential:user'] : ['credential:owner'], + ); const inaccessibleCredIds = workflowCredIds.filter((id) => !accessibleCredIds.includes(id)); diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 9141ce6e29b11..74172ec96c802 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -63,7 +63,7 @@ export class CredentialsService { : credentials; } - const ids = await this.sharedCredentialsRepository.getAccessibleCredentials(user.id); + const ids = await this.sharedCredentialsRepository.getAccessibleCredentialIds(user.id); const credentials = await this.credentialsRepository.findMany( options.listQueryOptions, diff --git a/packages/cli/src/databases/repositories/sharedCredentials.repository.ts b/packages/cli/src/databases/repositories/sharedCredentials.repository.ts index a83aef45a5d07..65f112631d381 100644 --- a/packages/cli/src/databases/repositories/sharedCredentials.repository.ts +++ b/packages/cli/src/databases/repositories/sharedCredentials.repository.ts @@ -1,7 +1,7 @@ import { Service } from 'typedi'; import type { EntityManager } from 'typeorm'; import { DataSource, In, Not, Repository } from 'typeorm'; -import { SharedCredentials } from '../entities/SharedCredentials'; +import { type CredentialSharingRole, SharedCredentials } from '../entities/SharedCredentials'; import type { User } from '../entities/User'; @Service() @@ -36,33 +36,25 @@ export class SharedCredentialsRepository extends Repository { return await this.update({ userId: Not(user.id), role: 'credential:owner' }, { user }); } - /** - * Get the IDs of all credentials owned by or shared with a user. - */ - async getAccessibleCredentials(userId: string) { + /** Get the IDs of all credentials owned by or shared with a user */ + async getAccessibleCredentialIds(userIds: string | string[]) { + return await this.getCredentialIdsForUserAndRole(userIds, [ + 'credential:owner', + 'credential:user', + ]); + } + + async getCredentialIdsForUserAndRole(userIds: string | string[], roles: CredentialSharingRole[]) { + if (!Array.isArray(userIds)) userIds = [userIds]; const sharings = await this.find({ where: { - userId, - role: In(['credential:owner', 'credential:user']), + userId: In(userIds), + role: In(roles), }, }); - return sharings.map((s) => s.credentialsId); } - async findAccessibleSharings(userIds: string[]) { - return await this.findBy({ - userId: In(userIds), - }); - } - - async findOwnedSharings(userIds: string[]) { - return await this.findBy({ - userId: In(userIds), - role: 'credential:owner', - }); - } - async deleteByIds(transaction: EntityManager, sharedCredentialsIds: string[], user?: User) { return await transaction.delete(SharedCredentials, { user, diff --git a/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts b/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts index e3d321cab4509..acd8d3d8cbf02 100644 --- a/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts +++ b/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts @@ -22,6 +22,14 @@ export class SharedWorkflowRepository extends Repository { return await this.exist({ where }); } + async getSharedUserIds(workflowId: string) { + const sharedWorkflows = await this.find({ + select: ['userId'], + where: { workflowId }, + }); + return sharedWorkflows.map((sharing) => sharing.userId); + } + async getSharedWorkflowIds(workflowIds: string[]) { const sharedWorkflows = await this.find({ select: ['workflowId'], diff --git a/packages/cli/test/unit/PermissionChecker.test.ts b/packages/cli/test/unit/PermissionChecker.test.ts new file mode 100644 index 0000000000000..f064a6ed03393 --- /dev/null +++ b/packages/cli/test/unit/PermissionChecker.test.ts @@ -0,0 +1,135 @@ +import { type INodeTypes, Workflow } from 'n8n-workflow'; +import { mock } from 'jest-mock-extended'; +import type { User } from '@db/entities/User'; +import type { UserRepository } from '@db/repositories/user.repository'; +import type { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository'; +import type { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; +import type { License } from '@/License'; +import { PermissionChecker } from '@/UserManagement/PermissionChecker'; + +describe('PermissionChecker', () => { + const user = mock(); + const userRepo = mock(); + const sharedCredentialsRepo = mock(); + const sharedWorkflowRepo = mock(); + const license = mock(); + const permissionChecker = new PermissionChecker( + userRepo, + sharedCredentialsRepo, + sharedWorkflowRepo, + mock(), + license, + ); + + const workflow = new Workflow({ + id: '1', + name: 'test', + active: false, + connections: {}, + nodeTypes: mock(), + nodes: [ + { + id: 'node-id', + name: 'HTTP Request', + type: 'n8n-nodes-base.httpRequest', + parameters: {}, + typeVersion: 1, + position: [0, 0], + credentials: { + oAuth2Api: { + id: 'cred-id', + name: 'Custom oAuth2', + }, + }, + }, + ], + }); + + beforeEach(() => jest.clearAllMocks()); + + describe('check', () => { + it('should throw if no user is found', async () => { + userRepo.findOneOrFail.mockRejectedValue(new Error('Fail')); + await expect(permissionChecker.check(workflow, '123')).rejects.toThrow(); + expect(license.isSharingEnabled).not.toHaveBeenCalled(); + expect(sharedCredentialsRepo.getCredentialIdsForUserAndRole).not.toHaveBeenCalled(); + }); + + it('should allow a user if they have a global `workflow:execute` scope', async () => { + userRepo.findOneOrFail.mockResolvedValue(user); + user.hasGlobalScope.calledWith('workflow:execute').mockReturnValue(true); + await expect(permissionChecker.check(workflow, user.id)).resolves.not.toThrow(); + expect(license.isSharingEnabled).not.toHaveBeenCalled(); + expect(sharedCredentialsRepo.getCredentialIdsForUserAndRole).not.toHaveBeenCalled(); + }); + + describe('When sharing is disabled', () => { + beforeEach(() => { + userRepo.findOneOrFail.mockResolvedValue(user); + user.hasGlobalScope.calledWith('workflow:execute').mockReturnValue(false); + license.isSharingEnabled.mockReturnValue(false); + }); + + it('should validate credential access using only owned credentials', async () => { + sharedCredentialsRepo.getCredentialIdsForUserAndRole.mockResolvedValue(['cred-id']); + + await expect(permissionChecker.check(workflow, user.id)).resolves.not.toThrow(); + + expect(sharedWorkflowRepo.getSharedUserIds).not.toBeCalled(); + expect(sharedCredentialsRepo.getCredentialIdsForUserAndRole).toBeCalledWith( + [user.id], + ['credential:owner'], + ); + }); + + it('should throw when the user does not have access to the credential', async () => { + sharedCredentialsRepo.getCredentialIdsForUserAndRole.mockResolvedValue(['cred-id2']); + + await expect(permissionChecker.check(workflow, user.id)).rejects.toThrow( + 'Node has no access to credential', + ); + + expect(sharedWorkflowRepo.getSharedUserIds).not.toBeCalled(); + expect(sharedCredentialsRepo.getCredentialIdsForUserAndRole).toBeCalledWith( + [user.id], + ['credential:owner'], + ); + }); + }); + + describe('When sharing is enabled', () => { + beforeEach(() => { + userRepo.findOneOrFail.mockResolvedValue(user); + user.hasGlobalScope.calledWith('workflow:execute').mockReturnValue(false); + license.isSharingEnabled.mockReturnValue(true); + sharedWorkflowRepo.getSharedUserIds.mockResolvedValue([user.id, 'another-user']); + }); + + it('should validate credential access using only owned credentials', async () => { + sharedCredentialsRepo.getCredentialIdsForUserAndRole.mockResolvedValue(['cred-id']); + + await expect(permissionChecker.check(workflow, user.id)).resolves.not.toThrow(); + + expect(sharedWorkflowRepo.getSharedUserIds).toBeCalledWith(workflow.id); + expect(sharedCredentialsRepo.getCredentialIdsForUserAndRole).toBeCalledWith( + [user.id, 'another-user'], + ['credential:owner', 'credential:user'], + ); + }); + + it('should throw when the user does not have access to the credential', async () => { + sharedCredentialsRepo.getCredentialIdsForUserAndRole.mockResolvedValue(['cred-id2']); + + await expect(permissionChecker.check(workflow, user.id)).rejects.toThrow( + 'Node has no access to credential', + ); + + expect(sharedWorkflowRepo.find).not.toBeCalled(); + expect(sharedCredentialsRepo.getCredentialIdsForUserAndRole).toBeCalledWith( + [user.id, 'another-user'], + ['credential:owner', 'credential:user'], + ); + }); + }); + }); +}); From 882431d60cf008c2c47530c8ceb0e14c1f6f81e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Fri, 2 Feb 2024 11:25:03 +0100 Subject: [PATCH 4/4] address feeback --- .../src/UserManagement/PermissionChecker.ts | 7 ++- .../src/credentials/credentials.service.ts | 2 +- .../sharedCredentials.repository.ts | 12 +++-- .../repositories/sharedWorkflow.repository.ts | 1 + .../cli/test/unit/PermissionChecker.test.ts | 46 ++++++++++--------- 5 files changed, 37 insertions(+), 31 deletions(-) diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts index 70c52a0300a7a..de1e60f610279 100644 --- a/packages/cli/src/UserManagement/PermissionChecker.ts +++ b/packages/cli/src/UserManagement/PermissionChecker.ts @@ -50,10 +50,9 @@ export class PermissionChecker { workflowUserIds = await this.sharedWorkflowRepository.getSharedUserIds(workflow.id); } - const accessibleCredIds = await this.sharedCredentialsRepository.getCredentialIdsForUserAndRole( - workflowUserIds, - isSharingEnabled ? ['credential:owner', 'credential:user'] : ['credential:owner'], - ); + const accessibleCredIds = isSharingEnabled + ? await this.sharedCredentialsRepository.getAccessibleCredentialIds(workflowUserIds) + : await this.sharedCredentialsRepository.getOwnedCredentialIds(workflowUserIds); const inaccessibleCredIds = workflowCredIds.filter((id) => !accessibleCredIds.includes(id)); diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 74172ec96c802..0d40482990a3b 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -63,7 +63,7 @@ export class CredentialsService { : credentials; } - const ids = await this.sharedCredentialsRepository.getAccessibleCredentialIds(user.id); + const ids = await this.sharedCredentialsRepository.getAccessibleCredentialIds([user.id]); const credentials = await this.credentialsRepository.findMany( options.listQueryOptions, diff --git a/packages/cli/src/databases/repositories/sharedCredentials.repository.ts b/packages/cli/src/databases/repositories/sharedCredentials.repository.ts index 65f112631d381..eb1d194dd2766 100644 --- a/packages/cli/src/databases/repositories/sharedCredentials.repository.ts +++ b/packages/cli/src/databases/repositories/sharedCredentials.repository.ts @@ -36,16 +36,20 @@ export class SharedCredentialsRepository extends Repository { return await this.update({ userId: Not(user.id), role: 'credential:owner' }, { user }); } + /** Get the IDs of all credentials owned by a user */ + async getOwnedCredentialIds(userIds: string[]) { + return await this.getCredentialIdsByUserAndRole(userIds, ['credential:owner']); + } + /** Get the IDs of all credentials owned by or shared with a user */ - async getAccessibleCredentialIds(userIds: string | string[]) { - return await this.getCredentialIdsForUserAndRole(userIds, [ + async getAccessibleCredentialIds(userIds: string[]) { + return await this.getCredentialIdsByUserAndRole(userIds, [ 'credential:owner', 'credential:user', ]); } - async getCredentialIdsForUserAndRole(userIds: string | string[], roles: CredentialSharingRole[]) { - if (!Array.isArray(userIds)) userIds = [userIds]; + private async getCredentialIdsByUserAndRole(userIds: string[], roles: CredentialSharingRole[]) { const sharings = await this.find({ where: { userId: In(userIds), diff --git a/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts b/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts index acd8d3d8cbf02..a457efb5a8b7b 100644 --- a/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts +++ b/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts @@ -22,6 +22,7 @@ export class SharedWorkflowRepository extends Repository { return await this.exist({ where }); } + /** Get the IDs of all users this workflow is shared with */ async getSharedUserIds(workflowId: string) { const sharedWorkflows = await this.find({ select: ['userId'], diff --git a/packages/cli/test/unit/PermissionChecker.test.ts b/packages/cli/test/unit/PermissionChecker.test.ts index f064a6ed03393..7e3336230c2ac 100644 --- a/packages/cli/test/unit/PermissionChecker.test.ts +++ b/packages/cli/test/unit/PermissionChecker.test.ts @@ -52,7 +52,9 @@ describe('PermissionChecker', () => { userRepo.findOneOrFail.mockRejectedValue(new Error('Fail')); await expect(permissionChecker.check(workflow, '123')).rejects.toThrow(); expect(license.isSharingEnabled).not.toHaveBeenCalled(); - expect(sharedCredentialsRepo.getCredentialIdsForUserAndRole).not.toHaveBeenCalled(); + expect(sharedWorkflowRepo.getSharedUserIds).not.toBeCalled(); + expect(sharedCredentialsRepo.getOwnedCredentialIds).not.toHaveBeenCalled(); + expect(sharedCredentialsRepo.getAccessibleCredentialIds).not.toHaveBeenCalled(); }); it('should allow a user if they have a global `workflow:execute` scope', async () => { @@ -60,7 +62,9 @@ describe('PermissionChecker', () => { user.hasGlobalScope.calledWith('workflow:execute').mockReturnValue(true); await expect(permissionChecker.check(workflow, user.id)).resolves.not.toThrow(); expect(license.isSharingEnabled).not.toHaveBeenCalled(); - expect(sharedCredentialsRepo.getCredentialIdsForUserAndRole).not.toHaveBeenCalled(); + expect(sharedWorkflowRepo.getSharedUserIds).not.toBeCalled(); + expect(sharedCredentialsRepo.getOwnedCredentialIds).not.toHaveBeenCalled(); + expect(sharedCredentialsRepo.getAccessibleCredentialIds).not.toHaveBeenCalled(); }); describe('When sharing is disabled', () => { @@ -71,29 +75,25 @@ describe('PermissionChecker', () => { }); it('should validate credential access using only owned credentials', async () => { - sharedCredentialsRepo.getCredentialIdsForUserAndRole.mockResolvedValue(['cred-id']); + sharedCredentialsRepo.getOwnedCredentialIds.mockResolvedValue(['cred-id']); await expect(permissionChecker.check(workflow, user.id)).resolves.not.toThrow(); expect(sharedWorkflowRepo.getSharedUserIds).not.toBeCalled(); - expect(sharedCredentialsRepo.getCredentialIdsForUserAndRole).toBeCalledWith( - [user.id], - ['credential:owner'], - ); + expect(sharedCredentialsRepo.getOwnedCredentialIds).toBeCalledWith([user.id]); + expect(sharedCredentialsRepo.getAccessibleCredentialIds).not.toHaveBeenCalled(); }); it('should throw when the user does not have access to the credential', async () => { - sharedCredentialsRepo.getCredentialIdsForUserAndRole.mockResolvedValue(['cred-id2']); + sharedCredentialsRepo.getOwnedCredentialIds.mockResolvedValue(['cred-id2']); await expect(permissionChecker.check(workflow, user.id)).rejects.toThrow( 'Node has no access to credential', ); expect(sharedWorkflowRepo.getSharedUserIds).not.toBeCalled(); - expect(sharedCredentialsRepo.getCredentialIdsForUserAndRole).toBeCalledWith( - [user.id], - ['credential:owner'], - ); + expect(sharedCredentialsRepo.getOwnedCredentialIds).toBeCalledWith([user.id]); + expect(sharedCredentialsRepo.getAccessibleCredentialIds).not.toHaveBeenCalled(); }); }); @@ -106,29 +106,31 @@ describe('PermissionChecker', () => { }); it('should validate credential access using only owned credentials', async () => { - sharedCredentialsRepo.getCredentialIdsForUserAndRole.mockResolvedValue(['cred-id']); + sharedCredentialsRepo.getAccessibleCredentialIds.mockResolvedValue(['cred-id']); await expect(permissionChecker.check(workflow, user.id)).resolves.not.toThrow(); expect(sharedWorkflowRepo.getSharedUserIds).toBeCalledWith(workflow.id); - expect(sharedCredentialsRepo.getCredentialIdsForUserAndRole).toBeCalledWith( - [user.id, 'another-user'], - ['credential:owner', 'credential:user'], - ); + expect(sharedCredentialsRepo.getAccessibleCredentialIds).toBeCalledWith([ + user.id, + 'another-user', + ]); + expect(sharedCredentialsRepo.getOwnedCredentialIds).not.toHaveBeenCalled(); }); it('should throw when the user does not have access to the credential', async () => { - sharedCredentialsRepo.getCredentialIdsForUserAndRole.mockResolvedValue(['cred-id2']); + sharedCredentialsRepo.getAccessibleCredentialIds.mockResolvedValue(['cred-id2']); await expect(permissionChecker.check(workflow, user.id)).rejects.toThrow( 'Node has no access to credential', ); expect(sharedWorkflowRepo.find).not.toBeCalled(); - expect(sharedCredentialsRepo.getCredentialIdsForUserAndRole).toBeCalledWith( - [user.id, 'another-user'], - ['credential:owner', 'credential:user'], - ); + expect(sharedCredentialsRepo.getAccessibleCredentialIds).toBeCalledWith([ + user.id, + 'another-user', + ]); + expect(sharedCredentialsRepo.getOwnedCredentialIds).not.toHaveBeenCalled(); }); }); });