diff --git a/packages/cli/src/workflows/__tests__/workflow.service.ee.test.ts b/packages/cli/src/workflows/__tests__/workflow.service.ee.test.ts index 0e30285241f..b0ee3c4a4c9 100644 --- a/packages/cli/src/workflows/__tests__/workflow.service.ee.test.ts +++ b/packages/cli/src/workflows/__tests__/workflow.service.ee.test.ts @@ -1,5 +1,6 @@ import type { CredentialsEntity, + FolderRepository, Project, SharedWorkflow, User, @@ -13,6 +14,9 @@ import { WorkflowActivationError } from 'n8n-workflow'; import { mock } from 'vitest-mock-extended'; import type { ActiveWorkflowManager } from '@/active-workflow-manager'; +import type { PolicyEnforcementService } from '@/policy/policy-enforcement.service'; +import type { ProjectService } from '@/services/project.service.ee'; +import type { WorkflowFinderService } from '@/workflows/workflow-finder.service'; import type { WorkflowMutationHooksProxy } from '@/workflows/workflow-mutation-hooks-proxy.service'; import { EnterpriseWorkflowService } from '@/workflows/workflow.service.ee'; @@ -22,6 +26,10 @@ describe('EnterpriseWorkflowService', () => { const activeWorkflowManager = mock(); const workflowPublishHistoryRepository = mock(); const workflowMutationHooks = mock(); + const workflowFinderService = mock(); + const projectService = mock(); + const folderRepository = mock(); + const policyEnforcementService = mock(); beforeEach(() => { vi.clearAllMocks(); @@ -32,14 +40,15 @@ describe('EnterpriseWorkflowService', () => { mock(), // credentialsRepository mock(), // credentialsService mock(), // ownershipService - mock(), // projectService + projectService, activeWorkflowManager, mock(), // credentialsFinderService mock(), // enterpriseCredentialsService - mock(), // workflowFinderService - mock(), // folderRepository + workflowFinderService, + folderRepository, workflowPublishHistoryRepository, workflowMutationHooks, + policyEnforcementService, ); }); @@ -353,6 +362,96 @@ describe('EnterpriseWorkflowService', () => { }); }); + describe('transferWorkflow()', () => { + const user = mock({ id: 'user-1' }); + const sourceProject = mock({ id: 'proj-source' }); + const destinationProject = mock({ id: 'proj-dest' }); + + const makeWorkflow = (overrides: Partial = {}) => + mock({ + id: 'wf-1', + name: 'My workflow', + nodes: [], + activeVersionId: null, + parentFolder: null, + shared: [mock({ role: 'workflow:owner', project: sourceProject })], + ...overrides, + }); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let transferOwnershipSpy: ReturnType>; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let shareCredentialsSpy: ReturnType>; + + beforeEach(() => { + workflowFinderService.findWorkflowForUser.mockResolvedValue(makeWorkflow()); + projectService.getProjectWithScope.mockResolvedValue(destinationProject); + policyEnforcementService.enforceWorkflowTransfer.mockResolvedValue(mock()); + // Ownership transfer and credential sharing are exercised by their own + // describe blocks below; stubbed here so these tests isolate the + // policy-enforcement wiring in `transferWorkflow` itself. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + transferOwnershipSpy = vi + .spyOn(service as any, 'transferWorkflowOwnership') + .mockResolvedValue(undefined); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + shareCredentialsSpy = vi + .spyOn(service as any, 'shareCredentialsWithProject') + .mockResolvedValue(undefined); + }); + + it('calls enforceWorkflowTransfer with the target project, not the source', async () => { + const workflow = makeWorkflow(); + workflowFinderService.findWorkflowForUser.mockResolvedValue(workflow); + + await service.transferWorkflow(user, 'wf-1', 'proj-dest'); + + expect(policyEnforcementService.enforceWorkflowTransfer).toHaveBeenCalledExactlyOnceWith({ + workflow, + targetProjectId: destinationProject.id, + }); + }); + + it('proceeds with the transfer unchanged when the policy check clears', async () => { + await service.transferWorkflow(user, 'wf-1', 'proj-dest'); + + expect(transferOwnershipSpy).toHaveBeenCalledTimes(1); + expect(shareCredentialsSpy).toHaveBeenCalledTimes(1); + expect(workflowRepository.update).toHaveBeenCalledWith( + { id: 'wf-1' }, + { parentFolder: null }, + ); + }); + + it('blocks the transfer and performs no mutation when the policy check throws', async () => { + const violation = new Error('blocked by policy'); + policyEnforcementService.enforceWorkflowTransfer.mockRejectedValue(violation); + + await expect(service.transferWorkflow(user, 'wf-1', 'proj-dest')).rejects.toThrow(violation); + + expect(activeWorkflowManager.remove).not.toHaveBeenCalled(); + expect(transferOwnershipSpy).not.toHaveBeenCalled(); + expect(shareCredentialsSpy).not.toHaveBeenCalled(); + expect(workflowRepository.update).not.toHaveBeenCalled(); + }); + + it('resolves the destination project before enforcing the policy check', async () => { + const callOrder: string[] = []; + projectService.getProjectWithScope.mockImplementation(async () => { + callOrder.push('getProjectWithScope'); + return destinationProject; + }); + policyEnforcementService.enforceWorkflowTransfer.mockImplementation(async () => { + callOrder.push('enforceWorkflowTransfer'); + return await mock(); + }); + + await service.transferWorkflow(user, 'wf-1', 'proj-dest'); + + expect(callOrder).toEqual(['getProjectWithScope', 'enforceWorkflowTransfer']); + }); + }); + describe('transferWorkflowOwnership', () => { const destinationProject = mock({ id: 'proj-dest' }); diff --git a/packages/cli/src/workflows/workflow.service.ee.ts b/packages/cli/src/workflows/workflow.service.ee.ts index 6947efcbed2..1dfe9b7479c 100644 --- a/packages/cli/src/workflows/workflow.service.ee.ts +++ b/packages/cli/src/workflows/workflow.service.ee.ts @@ -39,6 +39,7 @@ import { FolderNotFoundError } from '@/errors/folder-not-found.error'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { TransferWorkflowError } from '@/errors/response-errors/transfer-workflow.error'; +import { PolicyEnforcementService } from '@/policy/policy-enforcement.service'; import { OwnershipService } from '@/services/ownership.service'; import { ProjectService } from '@/services/project.service.ee'; @@ -62,6 +63,7 @@ export class EnterpriseWorkflowService { private readonly folderRepository: FolderRepository, private readonly workflowPublishHistoryRepository: WorkflowPublishHistoryRepository, private readonly workflowMutationHooks: WorkflowMutationHooksProxy, + private readonly policyEnforcementService: PolicyEnforcementService, ) {} async shareWithProjects( @@ -469,23 +471,29 @@ export class EnterpriseWorkflowService { } } + // 6. validate against the destination project's policy + await this.policyEnforcementService.enforceWorkflowTransfer({ + workflow, + targetProjectId: destinationProject.id, + }); + const wasActive = this.isActiveWorkflow(workflow); - // 6. deactivate workflow if necessary + // 7. deactivate workflow if necessary if (wasActive) { await this.activeWorkflowManager.remove(workflowId); } - // 7. transfer the workflow + // 8. transfer the workflow await this.transferWorkflowOwnership(user, [workflow], destinationProject); - // 8. share credentials into the destination project + // 9. share credentials into the destination project await this.shareCredentialsWithProject(user, shareCredentials, destinationProject.id); - // 9. Move workflow to the right folder if any + // 10. Move workflow to the right folder if any await this.workflowRepository.update({ id: workflow.id }, { parentFolder }); - // 10. try to activate it again if it was active + // 11. try to activate it again if it was active if (wasActive) { return await this.attemptWorkflowReactivation(workflowId, workflow.activeVersionId, user.id); } diff --git a/packages/cli/test/integration/workflows/workflow.service.ee.test.ts b/packages/cli/test/integration/workflows/workflow.service.ee.test.ts index 0a7b4e04e64..061c643d64f 100644 --- a/packages/cli/test/integration/workflows/workflow.service.ee.test.ts +++ b/packages/cli/test/integration/workflows/workflow.service.ee.test.ts @@ -40,6 +40,7 @@ describe('EnterpriseWorkflowService', () => { mock(), // folderRepository mock(), // workflowPublishHistoryRepository mock(), // workflowMutationHooks + mock(), // policyEnforcementService ); });