From f1a3791abc54e75683599f0d76d215dca24a3e76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 24 Jul 2024 12:51:01 +0200 Subject: [PATCH] refactor(core): Split out subworkflow policy check from permission checker (no-changelog) (#10168) --- .../src/UserManagement/PermissionChecker.ts | 107 +--------- .../cli/src/WorkflowExecuteAdditionalData.ts | 3 +- .../subworkflow-policy-checker.test.ts | 161 +++++++++++++++ .../subworkflow-policy-checker.service.ts | 110 +++++++++++ .../workflows/workflowExecution.service.ts | 6 +- .../integration/PermissionChecker.test.ts | 183 +----------------- .../unit/workflow-execution.service.test.ts | 3 +- 7 files changed, 284 insertions(+), 289 deletions(-) create mode 100644 packages/cli/src/subworkflows/__tests__/subworkflow-policy-checker.test.ts create mode 100644 packages/cli/src/subworkflows/subworkflow-policy-checker.service.ts diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts index 424227e60d..f8759a4163 100644 --- a/packages/cli/src/UserManagement/PermissionChecker.ts +++ b/packages/cli/src/UserManagement/PermissionChecker.ts @@ -1,22 +1,17 @@ import { Service } from 'typedi'; -import type { INode, Workflow } from 'n8n-workflow'; -import { CredentialAccessError, NodeOperationError, WorkflowOperationError } from 'n8n-workflow'; +import type { INode } from 'n8n-workflow'; +import { CredentialAccessError, NodeOperationError } from 'n8n-workflow'; -import config from '@/config'; -import { License } from '@/License'; import { OwnershipService } from '@/services/ownership.service'; import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository'; import { ProjectService } from '@/services/project.service'; -import { Logger } from '@/Logger'; @Service() export class PermissionChecker { constructor( private readonly sharedCredentialsRepository: SharedCredentialsRepository, private readonly ownershipService: OwnershipService, - private readonly license: License, private readonly projectService: ProjectService, - private readonly logger: Logger, ) {} /** @@ -50,104 +45,6 @@ export class PermissionChecker { } } - async checkSubworkflowExecutePolicy( - subworkflow: Workflow, - parentWorkflowId: string, - node?: INode, - ) { - /** - * Important considerations: both the current workflow and the parent can have empty IDs. - * This happens when a user is executing an unsaved workflow manually running a workflow - * loaded from a file or code, for instance. - * This is an important topic to keep in mind for all security checks - */ - if (!subworkflow.id) { - // It's a workflow from code and not loaded from DB - // No checks are necessary since it doesn't have any sort of settings - return; - } - - let policy = - subworkflow.settings?.callerPolicy ?? config.getEnv('workflows.callerPolicyDefaultOption'); - - const isSharingEnabled = this.license.isSharingEnabled(); - - if (!isSharingEnabled) { - // Community version allows only same owner workflows - policy = 'workflowsFromSameOwner'; - } - - const parentWorkflowOwner = - await this.ownershipService.getWorkflowProjectCached(parentWorkflowId); - - const subworkflowOwner = await this.ownershipService.getWorkflowProjectCached(subworkflow.id); - - const description = - subworkflowOwner.id === parentWorkflowOwner.id - ? 'Change the settings of the sub-workflow so it can be called by this one.' - : `An admin for the ${subworkflowOwner.name} project can make this change. You may need to tell them the ID of the sub-workflow, which is ${subworkflow.id}`; - - const errorToThrow = new WorkflowOperationError( - `Target workflow ID ${subworkflow.id} may not be called`, - node, - description, - ); - - if (policy === 'none') { - this.logger.warn('[PermissionChecker] Subworkflow execution denied', { - callerWorkflowId: parentWorkflowId, - subworkflowId: subworkflow.id, - reason: 'Subworkflow may not be called', - policy, - isSharingEnabled, - }); - throw errorToThrow; - } - - if (policy === 'workflowsFromAList') { - if (parentWorkflowId === undefined) { - this.logger.warn('[PermissionChecker] Subworkflow execution denied', { - reason: 'Subworkflow may be called only by workflows from an allowlist', - callerWorkflowId: parentWorkflowId, - subworkflowId: subworkflow.id, - policy, - isSharingEnabled, - }); - throw errorToThrow; - } - - const allowedCallerIds = subworkflow.settings.callerIds - ?.split(',') - .map((id) => id.trim()) - .filter((id) => id !== ''); - - if (!allowedCallerIds?.includes(parentWorkflowId)) { - this.logger.warn('[PermissionChecker] Subworkflow execution denied', { - reason: 'Subworkflow may be called only by workflows from an allowlist', - callerWorkflowId: parentWorkflowId, - subworkflowId: subworkflow.id, - allowlist: allowedCallerIds, - policy, - isSharingEnabled, - }); - throw errorToThrow; - } - } - - if (policy === 'workflowsFromSameOwner' && subworkflowOwner?.id !== parentWorkflowOwner.id) { - this.logger.warn('[PermissionChecker] Subworkflow execution denied', { - reason: 'Subworkflow may be called only by workflows owned by the same project', - callerWorkflowId: parentWorkflowId, - subworkflowId: subworkflow.id, - callerProjectId: parentWorkflowOwner.id, - subworkflowProjectId: subworkflowOwner.id, - policy, - isSharingEnabled, - }); - throw errorToThrow; - } - } - 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/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index d2f53ef1e6..e5a17de7ec 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -73,6 +73,7 @@ import { WorkflowExecutionService } from './workflows/workflowExecution.service' import { MessageEventBus } from '@/eventbus/MessageEventBus/MessageEventBus'; import { EventService } from './eventbus/event.service'; import { GlobalConfig } from '@n8n/config'; +import { SubworkflowPolicyChecker } from './subworkflows/subworkflow-policy-checker.service'; export function objectToError(errorObject: unknown, workflow: Workflow): Error { // TODO: Expand with other error types @@ -826,7 +827,7 @@ async function executeWorkflow( let data; try { await Container.get(PermissionChecker).check(workflowData.id, workflowData.nodes); - await Container.get(PermissionChecker).checkSubworkflowExecutePolicy( + await Container.get(SubworkflowPolicyChecker).check( workflow, options.parentWorkflowId, options.node, diff --git a/packages/cli/src/subworkflows/__tests__/subworkflow-policy-checker.test.ts b/packages/cli/src/subworkflows/__tests__/subworkflow-policy-checker.test.ts new file mode 100644 index 0000000000..bb66220120 --- /dev/null +++ b/packages/cli/src/subworkflows/__tests__/subworkflow-policy-checker.test.ts @@ -0,0 +1,161 @@ +import { v4 as uuid } from 'uuid'; +import type { Workflow } from 'n8n-workflow'; +import { SubworkflowOperationError } from 'n8n-workflow'; +import { Project } from '@/databases/entities/Project'; +import { OwnershipService } from '@/services/ownership.service'; +import { mockInstance } from '@test/mocking'; +import config from '@/config'; +import { mock } from 'jest-mock-extended'; +import { SubworkflowPolicyChecker } from '../subworkflow-policy-checker.service'; + +import type { WorkflowEntity } from '@/databases/entities/WorkflowEntity'; +import type { License } from '@/License'; + +const toTargetCallErrorMsg = (subworkflowId: string) => + `Target workflow ID ${subworkflowId} may not be called`; + +const ownershipService = mockInstance(OwnershipService); +const memberPersonalProject = mock(); + +describe('SubworkflowPolicyChecker', () => { + const license = mock(); + const checker = new SubworkflowPolicyChecker(mock(), license, ownershipService); + + beforeEach(() => { + license.isSharingEnabled.mockReturnValue(true); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('no caller policy', () => { + test('should fall back to N8N_WORKFLOW_CALLER_POLICY_DEFAULT_OPTION', async () => { + config.set('workflows.callerPolicyDefaultOption', 'none'); + + const parentWorkflow = mock(); + const subworkflow = mock(); // no caller policy + + ownershipService.getWorkflowProjectCached.mockResolvedValue(memberPersonalProject); + + const check = checker.check(subworkflow, parentWorkflow.id); + + await expect(check).rejects.toThrow(toTargetCallErrorMsg(subworkflow.id)); + + config.load(config.default); + }); + }); + + describe('overridden caller policy', () => { + test('if no sharing, should override policy to workflows-from-same-owner', async () => { + license.isSharingEnabled.mockReturnValue(false); + + const parentWorkflow = mock(); + const subworkflow = mock({ settings: { callerPolicy: 'any' } }); // should be overridden + + const firstProject = mock({ id: uuid() }); + const secondProject = mock({ id: uuid() }); + + ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(firstProject); // parent workflow + ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(secondProject); // subworkflow + + const check = checker.check(subworkflow, parentWorkflow.id); + + await expect(check).rejects.toThrow(toTargetCallErrorMsg(subworkflow.id)); + + try { + await checker.check(subworkflow, uuid()); + } catch (error) { + if (error instanceof SubworkflowOperationError) { + expect(error.description).toBe( + `An admin for the ${firstProject.name} project can make this change. You may need to tell them the ID of the sub-workflow, which is ${subworkflow.id}`, + ); + } + } + }); + }); + + describe('workflows-from-list caller policy', () => { + // xyz + test('should allow if caller list contains parent workflow ID', async () => { + const parentWorkflow = mock({ id: uuid() }); + + const subworkflow = mock({ + settings: { + callerPolicy: 'workflowsFromAList', + callerIds: `123,456,bcdef, ${parentWorkflow.id}`, + }, + }); + + const parentWorkflowProject = mock({ id: uuid() }); + const subworkflowOwner = mock({ id: uuid() }); + ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(parentWorkflowProject); // parent workflow + ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(subworkflowOwner); // subworkflow + + const check = checker.check(subworkflow, parentWorkflow.id); + + await expect(check).resolves.not.toThrow(); + }); + + test('should deny if caller list does not contain parent workflow ID', async () => { + const parentWorkflow = mock(); + + const subworkflow = mock({ + settings: { callerPolicy: 'workflowsFromAList', callerIds: 'xyz' }, + }); + + const check = checker.check(subworkflow, parentWorkflow.id); + + await expect(check).rejects.toThrow(); + }); + }); + + describe('any caller policy', () => { + test('should not throw', async () => { + const parentWorkflow = mock({ id: uuid() }); + const subworkflow = mock({ settings: { callerPolicy: 'any' } }); + + const parentWorkflowProject = mock({ id: uuid() }); + const subworkflowOwner = mock({ id: uuid() }); + ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(parentWorkflowProject); // parent workflow + ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(subworkflowOwner); // subworkflow + + ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(new Project()); + + const check = checker.check(subworkflow, parentWorkflow.id); + + await expect(check).resolves.not.toThrow(); + }); + }); + + describe('workflows-from-same-owner caller policy', () => { + test('should deny if the two workflows are owned by different users', async () => { + const parentWorkflowProject = mock({ id: uuid() }); + const subworkflowOwner = mock({ id: uuid() }); + + ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(parentWorkflowProject); // parent workflow + ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(subworkflowOwner); // subworkflow + + const subworkflow = mock({ settings: { callerPolicy: 'workflowsFromSameOwner' } }); + + const check = checker.check(subworkflow, uuid()); + + await expect(check).rejects.toThrow(toTargetCallErrorMsg(subworkflow.id)); + }); + + test('should allow if both workflows are owned by the same user', async () => { + const parentWorkflow = mock(); + + const bothWorkflowsProject = mock({ id: uuid() }); + + ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(bothWorkflowsProject); // parent workflow + ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(bothWorkflowsProject); // subworkflow + + const subworkflow = mock({ settings: { callerPolicy: 'workflowsFromSameOwner' } }); + + const check = checker.check(subworkflow, parentWorkflow.id); + + await expect(check).resolves.not.toThrow(); + }); + }); +}); diff --git a/packages/cli/src/subworkflows/subworkflow-policy-checker.service.ts b/packages/cli/src/subworkflows/subworkflow-policy-checker.service.ts new file mode 100644 index 0000000000..ee7bdcd899 --- /dev/null +++ b/packages/cli/src/subworkflows/subworkflow-policy-checker.service.ts @@ -0,0 +1,110 @@ +import { Service } from 'typedi'; +import { WorkflowOperationError } from 'n8n-workflow'; +import config from '@/config'; +import { Logger } from '@/Logger'; +import { License } from '@/License'; +import { OwnershipService } from '@/services/ownership.service'; +import type { Workflow, INode } from 'n8n-workflow'; + +@Service() +export class SubworkflowPolicyChecker { + constructor( + private readonly logger: Logger, + private readonly license: License, + private readonly ownershipService: OwnershipService, + ) {} + + async check(subworkflow: Workflow, parentWorkflowId: string, node?: INode) { + /** + * Important considerations: both the current workflow and the parent can have empty IDs. + * This happens when a user is executing an unsaved workflow manually running a workflow + * loaded from a file or code, for instance. + * This is an important topic to keep in mind for all security checks + */ + if (!subworkflow.id) { + // It's a workflow from code and not loaded from DB + // No checks are necessary since it doesn't have any sort of settings + return; + } + + let policy = + subworkflow.settings?.callerPolicy ?? config.getEnv('workflows.callerPolicyDefaultOption'); + + const isSharingEnabled = this.license.isSharingEnabled(); + + if (!isSharingEnabled) { + // Community version allows only same owner workflows + policy = 'workflowsFromSameOwner'; + } + + const parentWorkflowOwner = + await this.ownershipService.getWorkflowProjectCached(parentWorkflowId); + + const subworkflowOwner = await this.ownershipService.getWorkflowProjectCached(subworkflow.id); + + const description = + subworkflowOwner.id === parentWorkflowOwner.id + ? 'Change the settings of the sub-workflow so it can be called by this one.' + : `An admin for the ${subworkflowOwner.name} project can make this change. You may need to tell them the ID of the sub-workflow, which is ${subworkflow.id}`; + + const errorToThrow = new WorkflowOperationError( + `Target workflow ID ${subworkflow.id} may not be called`, + node, + description, + ); + + if (policy === 'none') { + this.logger.warn('[PermissionChecker] Subworkflow execution denied', { + callerWorkflowId: parentWorkflowId, + subworkflowId: subworkflow.id, + reason: 'Subworkflow may not be called', + policy, + isSharingEnabled, + }); + throw errorToThrow; + } + + if (policy === 'workflowsFromAList') { + if (parentWorkflowId === undefined) { + this.logger.warn('[PermissionChecker] Subworkflow execution denied', { + reason: 'Subworkflow may be called only by workflows from an allowlist', + callerWorkflowId: parentWorkflowId, + subworkflowId: subworkflow.id, + policy, + isSharingEnabled, + }); + throw errorToThrow; + } + + const allowedCallerIds = subworkflow.settings.callerIds + ?.split(',') + .map((id) => id.trim()) + .filter((id) => id !== ''); + + if (!allowedCallerIds?.includes(parentWorkflowId)) { + this.logger.warn('[PermissionChecker] Subworkflow execution denied', { + reason: 'Subworkflow may be called only by workflows from an allowlist', + callerWorkflowId: parentWorkflowId, + subworkflowId: subworkflow.id, + allowlist: allowedCallerIds, + policy, + isSharingEnabled, + }); + throw errorToThrow; + } + } + + if (policy === 'workflowsFromSameOwner' && subworkflowOwner?.id !== parentWorkflowOwner.id) { + this.logger.warn('[PermissionChecker] Subworkflow execution denied', { + reason: 'Subworkflow may be called only by workflows owned by the same project', + callerWorkflowId: parentWorkflowId, + subworkflowId: subworkflow.id, + callerProjectId: parentWorkflowOwner.id, + subworkflowProjectId: subworkflowOwner.id, + policy, + isSharingEnabled, + }); + throw errorToThrow; + } + } +} diff --git a/packages/cli/src/workflows/workflowExecution.service.ts b/packages/cli/src/workflows/workflowExecution.service.ts index 4816be6d26..8ebe7aed0c 100644 --- a/packages/cli/src/workflows/workflowExecution.service.ts +++ b/packages/cli/src/workflows/workflowExecution.service.ts @@ -32,9 +32,9 @@ import { WorkflowRunner } from '@/WorkflowRunner'; import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; import { TestWebhooks } from '@/TestWebhooks'; import { Logger } from '@/Logger'; -import { PermissionChecker } from '@/UserManagement/PermissionChecker'; import type { Project } from '@/databases/entities/Project'; import { GlobalConfig } from '@n8n/config'; +import { SubworkflowPolicyChecker } from '@/subworkflows/subworkflow-policy-checker.service'; @Service() export class WorkflowExecutionService { @@ -44,9 +44,9 @@ export class WorkflowExecutionService { private readonly workflowRepository: WorkflowRepository, private readonly nodeTypes: NodeTypes, private readonly testWebhooks: TestWebhooks, - private readonly permissionChecker: PermissionChecker, private readonly workflowRunner: WorkflowRunner, private readonly globalConfig: GlobalConfig, + private readonly subworkflowPolicyChecker: SubworkflowPolicyChecker, ) {} async runWorkflow( @@ -188,7 +188,7 @@ export class WorkflowExecutionService { const failedNode = workflowErrorData.execution?.lastNodeExecuted ? workflowInstance.getNode(workflowErrorData.execution?.lastNodeExecuted) : undefined; - await this.permissionChecker.checkSubworkflowExecutePolicy( + await this.subworkflowPolicyChecker.check( workflowInstance, workflowErrorData.workflow.id!, failedNode ?? undefined, diff --git a/packages/cli/test/integration/PermissionChecker.test.ts b/packages/cli/test/integration/PermissionChecker.test.ts index 5cdf74fdb8..d5262a50d0 100644 --- a/packages/cli/test/integration/PermissionChecker.test.ts +++ b/packages/cli/test/integration/PermissionChecker.test.ts @@ -1,22 +1,17 @@ import { v4 as uuid } from 'uuid'; import { Container } from 'typedi'; -import type { INode, WorkflowSettings } from 'n8n-workflow'; -import { SubworkflowOperationError, Workflow, randomInt } from 'n8n-workflow'; - -import config from '@/config'; +import type { INode } from 'n8n-workflow'; +import { randomInt } from 'n8n-workflow'; import type { User } from '@db/entities/User'; import { WorkflowRepository } from '@db/repositories/workflow.repository'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; -import { generateNanoId } from '@/databases/utils/generators'; -import { License } from '@/License'; import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials'; import { NodeTypes } from '@/NodeTypes'; import { OwnershipService } from '@/services/ownership.service'; import { PermissionChecker } from '@/UserManagement/PermissionChecker'; import { mockInstance } from '../shared/mocking'; -import { randomCredentialPayload as randomCred, randomName } from '../integration/shared/random'; -import { LicenseMocker } from '../integration/shared/license'; +import { randomCredentialPayload as randomCred } from '../integration/shared/random'; import * as testDb from '../integration/shared/testDb'; import type { SaveCredentialFunction } from '../integration/shared/types'; import { mockNodeTypesData } from '../unit/Helpers'; @@ -25,50 +20,9 @@ import { createOwner, createUser } from '../integration/shared/db/users'; import { SharedCredentialsRepository } from '@/databases/repositories/sharedCredentials.repository'; import { getPersonalProject } from './shared/db/projects'; import type { WorkflowEntity } from '@/databases/entities/WorkflowEntity'; -import { Project } from '@/databases/entities/Project'; +import type { Project } from '@/databases/entities/Project'; import { ProjectRepository } from '@/databases/repositories/project.repository'; -export const toTargetCallErrorMsg = (subworkflowId: string) => - `Target workflow ID ${subworkflowId} may not be called`; - -export function createParentWorkflow() { - return Container.get(WorkflowRepository).create({ - id: generateNanoId(), - name: randomName(), - active: false, - connections: {}, - nodes: [ - { - name: '', - typeVersion: 1, - type: 'n8n-nodes-base.executeWorkflow', - position: [0, 0], - parameters: {}, - }, - ], - }); -} - -export function createSubworkflow({ - policy, - callerIds, -}: { - policy?: WorkflowSettings.CallerPolicy; - callerIds?: string; -} = {}) { - return new Workflow({ - id: uuid(), - nodes: [], - connections: {}, - active: false, - nodeTypes: mockNodeTypes, - settings: { - ...(policy ? { callerPolicy: policy } : {}), - ...(callerIds ? { callerIds } : {}), - }, - }); -} - const ownershipService = mockInstance(OwnershipService); const createWorkflow = async (nodes: INode[], workflowOwner?: User): Promise => { @@ -289,132 +243,3 @@ describe('check()', () => { ).resolves.not.toThrow(); }); }); - -describe('checkSubworkflowExecutePolicy()', () => { - let license: LicenseMocker; - - beforeAll(() => { - license = new LicenseMocker(); - license.mock(Container.get(License)); - license.enable('feat:sharing'); - }); - - describe('no caller policy', () => { - test('should fall back to N8N_WORKFLOW_CALLER_POLICY_DEFAULT_OPTION', async () => { - config.set('workflows.callerPolicyDefaultOption', 'none'); - - const parentWorkflow = createParentWorkflow(); - const subworkflow = createSubworkflow(); // no caller policy - - ownershipService.getWorkflowProjectCached.mockResolvedValue(memberPersonalProject); - - const check = permissionChecker.checkSubworkflowExecutePolicy(subworkflow, parentWorkflow.id); - - await expect(check).rejects.toThrow(toTargetCallErrorMsg(subworkflow.id)); - - config.load(config.default); - }); - }); - - describe('overridden caller policy', () => { - test('if no sharing, should override policy to workflows-from-same-owner', async () => { - license.disable('feat:sharing'); - - const parentWorkflow = createParentWorkflow(); - const subworkflow = createSubworkflow({ policy: 'any' }); // should be overridden - - const firstProject = Container.get(ProjectRepository).create({ id: uuid() }); - const secondProject = Container.get(ProjectRepository).create({ id: uuid() }); - - ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(firstProject); // parent workflow - ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(secondProject); // subworkflow - - const check = permissionChecker.checkSubworkflowExecutePolicy(subworkflow, parentWorkflow.id); - - await expect(check).rejects.toThrow(toTargetCallErrorMsg(subworkflow.id)); - - try { - await permissionChecker.checkSubworkflowExecutePolicy(subworkflow, uuid()); - } catch (error) { - if (error instanceof SubworkflowOperationError) { - expect(error.description).toBe( - `An admin for the ${firstProject.name} project can make this change. You may need to tell them the ID of the sub-workflow, which is ${subworkflow.id}`, - ); - } - } - - license.enable('feat:sharing'); - }); - }); - - describe('workflows-from-list caller policy', () => { - test('should allow if caller list contains parent workflow ID', async () => { - const parentWorkflow = createParentWorkflow(); - - const subworkflow = createSubworkflow({ - policy: 'workflowsFromAList', - callerIds: `123,456,bcdef, ${parentWorkflow.id}`, - }); - - const check = permissionChecker.checkSubworkflowExecutePolicy(subworkflow, parentWorkflow.id); - - await expect(check).resolves.not.toThrow(); - }); - - test('should deny if caller list does not contain parent workflow ID', async () => { - const parentWorkflow = createParentWorkflow(); - - const subworkflow = createSubworkflow({ - policy: 'workflowsFromAList', - callerIds: 'xyz', - }); - - const check = permissionChecker.checkSubworkflowExecutePolicy(subworkflow, parentWorkflow.id); - - await expect(check).rejects.toThrow(); - }); - }); - - describe('any caller policy', () => { - test('should not throw', async () => { - const parentWorkflow = createParentWorkflow(); - const subworkflow = createSubworkflow({ policy: 'any' }); - ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(new Project()); - - const check = permissionChecker.checkSubworkflowExecutePolicy(subworkflow, parentWorkflow.id); - - await expect(check).resolves.not.toThrow(); - }); - }); - - describe('workflows-from-same-owner caller policy', () => { - test('should deny if the two workflows are owned by different users', async () => { - const parentWorkflowProject = Container.get(ProjectRepository).create({ id: uuid() }); - const subworkflowOwner = Container.get(ProjectRepository).create({ id: uuid() }); - - ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(parentWorkflowProject); // parent workflow - ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(subworkflowOwner); // subworkflow - - const subworkflow = createSubworkflow({ policy: 'workflowsFromSameOwner' }); - - const check = permissionChecker.checkSubworkflowExecutePolicy(subworkflow, uuid()); - - await expect(check).rejects.toThrow(toTargetCallErrorMsg(subworkflow.id)); - }); - - test('should allow if both workflows are owned by the same user', async () => { - const parentWorkflow = createParentWorkflow(); - - const bothWorkflowsProject = Container.get(ProjectRepository).create({ id: uuid() }); - - ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(bothWorkflowsProject); // parent workflow - ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(bothWorkflowsProject); // subworkflow - - const subworkflow = createSubworkflow({ policy: 'workflowsFromSameOwner' }); - - const check = permissionChecker.checkSubworkflowExecutePolicy(subworkflow, parentWorkflow.id); - - await expect(check).resolves.not.toThrow(); - }); - }); -}); diff --git a/packages/cli/test/unit/workflow-execution.service.test.ts b/packages/cli/test/unit/workflow-execution.service.test.ts index 567f6bf6e7..d06dde35e2 100644 --- a/packages/cli/test/unit/workflow-execution.service.test.ts +++ b/packages/cli/test/unit/workflow-execution.service.test.ts @@ -57,8 +57,9 @@ describe('WorkflowExecutionService', () => { mock(), mock(), mock(), - mock(), workflowRunner, + mock(), + mock(), ); describe('runWorkflow()', () => {