From f5b1e2aef1af8db0a008bcd8a663448dcdcb78b1 Mon Sep 17 00:00:00 2001 From: Yuliia Pominchuk Date: Thu, 24 Sep 2026 10:38:12 +0200 Subject: [PATCH] Revert "feat(core): Attach the publishing user to triggered executions" This reverts commit 47761178a451553750ceaba52b0f41802508107c. --- .../workflow-publish-history.repository.ts | 28 ------ .../__tests__/active-workflow-manager.test.ts | 13 +-- .../workflow-execute-additional-data.test.ts | 52 +--------- packages/cli/src/active-workflow-manager.ts | 6 -- .../schedule-trigger-task-handler.test.ts | 1 - .../schedule-trigger-task-handler.ts | 7 -- .../webhooks/__tests__/live-webhooks.test.ts | 64 ------------ packages/cli/src/webhooks/live-webhooks.ts | 8 -- packages/cli/src/webhooks/webhook.service.ts | 4 - .../src/workflow-execute-additional-data.ts | 17 +--- .../workflow-publisher.service.test.ts | 99 ------------------- .../trigger-execution-context.factory.test.ts | 2 - .../trigger-execution-context.factory.ts | 7 -- .../workflows/workflow-publisher.service.ts | 70 ------------- ...orkflow-publish-history.repository.test.ts | 97 ------------------ 15 files changed, 4 insertions(+), 471 deletions(-) delete mode 100644 packages/cli/src/workflows/__tests__/workflow-publisher.service.test.ts delete mode 100644 packages/cli/src/workflows/workflow-publisher.service.ts diff --git a/packages/@n8n/db/src/repositories/workflow-publish-history.repository.ts b/packages/@n8n/db/src/repositories/workflow-publish-history.repository.ts index 6637ecbd610..6a62561bb2b 100644 --- a/packages/@n8n/db/src/repositories/workflow-publish-history.repository.ts +++ b/packages/@n8n/db/src/repositories/workflow-publish-history.repository.ts @@ -47,32 +47,4 @@ export class WorkflowPublishHistoryRepository extends Repository { - if (versionId) { - const forVersion = await this.findOne({ - select: ['userId'], - where: { workflowId, versionId, event: 'activated' }, - order: { createdAt: 'DESC' }, - }); - if (forVersion?.userId) return forVersion.userId; - } - - return await this.findActivatedByUserId(workflowId); - } } diff --git a/packages/cli/src/__tests__/active-workflow-manager.test.ts b/packages/cli/src/__tests__/active-workflow-manager.test.ts index 089e6f49e49..203819087e0 100644 --- a/packages/cli/src/__tests__/active-workflow-manager.test.ts +++ b/packages/cli/src/__tests__/active-workflow-manager.test.ts @@ -94,7 +94,6 @@ describe('ActiveWorkflowManager', () => { mock(), // workflowPushNotifier policyEnforcementService, ownershipService, - mock(), ); }); @@ -233,7 +232,6 @@ describe('ActiveWorkflowManager', () => { mock(), // pollTriggerJobRegistrar policyEnforcementService, ownershipService, - mock(), // workflowPublisherService ); const makeWorkflow = (overrides: Partial = {}) => @@ -446,7 +444,6 @@ describe('ActiveWorkflowManager', () => { workflowPushNotifier, policyEnforcementService, ownershipService, - mock(), // workflowPublisherService ); }); @@ -706,7 +703,6 @@ describe('ActiveWorkflowManager', () => { workflowPushNotifier, policyEnforcementService, ownershipService, - mock(), // workflowPublisherService ); }); @@ -893,8 +889,7 @@ describe('ActiveWorkflowManager', () => { mock(), // nodeTypes pollCursorService, mock({ scheduler: { pollTimeoutSeconds: 45, leaseDurationSeconds: 60 } }), - mock(), // engineV2ActiveTriggers, - mock(), // workflowPublisherService + mock(), // engineV2ActiveTriggers ); activeWorkflowManager = new ActiveWorkflowManager( @@ -918,7 +913,6 @@ describe('ActiveWorkflowManager', () => { mock(), // workflowPushNotifier policyEnforcementService, ownershipService, - mock(), // workflowPublisherService ); }); @@ -1342,7 +1336,7 @@ describe('ActiveWorkflowManager', () => { realScheduledTaskManager = new ScheduledTaskManager( mock({ isLeader: true }), mock({ scoped: vi.fn().mockReturnValue(mock()) }), - mock(), // workflowPublisherService + mock(), ); realActiveWorkflowTriggers = new ActiveWorkflowTriggers( mock({ scoped: vi.fn().mockReturnValue(mock()) }), @@ -1372,7 +1366,6 @@ describe('ActiveWorkflowManager', () => { mock(), // workflowPushNotifier policyEnforcementService, ownershipService, - mock(), // workflowPublisherService ); }); @@ -1561,7 +1554,6 @@ describe('ActiveWorkflowManager', () => { mock(), // workflowPushNotifier policyEnforcementService, ownershipService, - mock(), // workflowPublisherService ); beforeEach(() => vi.clearAllMocks()); @@ -1641,7 +1633,6 @@ describe('ActiveWorkflowManager', () => { mock(), // workflowPushNotifier policyEnforcementService, ownershipService, - mock(), // workflowPublisherService ); const makeWorkflow = () => { diff --git a/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts b/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts index a76a7aa8311..44881935153 100644 --- a/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts +++ b/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts @@ -313,17 +313,11 @@ describe('WorkflowExecuteAdditionalData', () => { it('checks credentials against the triggering user for an inline sub-workflow', async () => { await executeWorkflow( mock({ id: undefined, code: subWorkflowData() }), - // `rootExecutionMode` explicit: a deep mock auto-stubs every - // property, so leaving it out would hand `??` a truthy proxy. - mock({ - userId: 'user-1', - rootExecutionMode: undefined, - }), + mock({ userId: 'user-1' }), mock({ loadedWorkflowData: subWorkflowData(), doNotWaitToFinish: false, parentWorkflowId: 'parent-1', - executionMode: 'manual', }), ); @@ -334,50 +328,6 @@ describe('WorkflowExecuteAdditionalData', () => { expect(credentialsPermissionChecker.check).not.toHaveBeenCalled(); }); - // A sub-workflow's own WorkflowExecute runs as 'integrated', so the - // original mode has to come from the root. - it('checks against the triggering user through a nested inline sub-workflow', async () => { - await executeWorkflow( - mock({ id: undefined, code: subWorkflowData() }), - mock({ - userId: 'user-1', - rootExecutionMode: 'manual', - }), - mock({ - loadedWorkflowData: subWorkflowData(), - doNotWaitToFinish: false, - parentWorkflowId: 'parent-1', - executionMode: 'integrated', - }), - ); - - expect(credentialsPermissionChecker.checkForUser).toHaveBeenCalledTimes(1); - expect(credentialsPermissionChecker.check).not.toHaveBeenCalled(); - }); - - // A triggered run now carries the publishing user for attribution. That - // must not turn this project check into a user check: an ordinary project - // credential has to keep working on a schedule even if the publisher's own - // access to it has since changed. - it('keeps the project check for an inline sub-workflow in a triggered run', async () => { - await executeWorkflow( - mock({ id: undefined, code: subWorkflowData() }), - mock({ - userId: 'publisher-1', - rootExecutionMode: undefined, - }), - mock({ - loadedWorkflowData: subWorkflowData(), - doNotWaitToFinish: false, - parentWorkflowId: 'parent-1', - executionMode: 'trigger', - }), - ); - - expect(credentialsPermissionChecker.check).toHaveBeenCalled(); - expect(credentialsPermissionChecker.checkForUser).not.toHaveBeenCalled(); - }); - it('checks credentials against the project for a database sub-workflow', async () => { await executeWorkflow( mock({ id: 'db-id', code: undefined }), diff --git a/packages/cli/src/active-workflow-manager.ts b/packages/cli/src/active-workflow-manager.ts index d272a227f5d..523bb05500d 100644 --- a/packages/cli/src/active-workflow-manager.ts +++ b/packages/cli/src/active-workflow-manager.ts @@ -58,7 +58,6 @@ import { WebhookService } from '@/webhooks/webhook.service'; import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data'; import { TriggerExecutionContextFactory } from '@/workflows/triggers/trigger-execution-context.factory'; import { getErrorDescription, getErrorNodeId } from '@/workflows/utils'; -import { WorkflowPublisherService } from '@/workflows/workflow-publisher.service'; import { WorkflowPushNotifier } from '@/workflows/workflow-push-notifier.service'; import { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service'; import { formatWorkflow } from '@/workflows/workflow.formatter'; @@ -95,7 +94,6 @@ export class ActiveWorkflowManager { private readonly workflowPushNotifier: WorkflowPushNotifier, private readonly policyEnforcementService: PolicyEnforcementService, private readonly ownershipService: OwnershipService, - private readonly workflowPublisherService: WorkflowPublisherService, ) { this.logger = this.logger.scoped(['workflow-activation']); } @@ -635,13 +633,9 @@ export class ActiveWorkflowManager { ); } - // A triggered run carries no user of its own, so attribute it to whoever - // published the workflow. Flows on into `IWorkflowExecutionDataProcess` - // when a trigger or webhook fires. const additionalData = await WorkflowExecuteAdditionalData.getBase({ workflowId: workflow.id, workflowSettings: dbWorkflow.settings, - userId: await this.workflowPublisherService.findPublisherUserId(dbWorkflow.id), }); let triggerCount = 0; diff --git a/packages/cli/src/scheduling/schedule-trigger-node/__tests__/schedule-trigger-task-handler.test.ts b/packages/cli/src/scheduling/schedule-trigger-node/__tests__/schedule-trigger-task-handler.test.ts index bbabda2dffe..8164c38d136 100644 --- a/packages/cli/src/scheduling/schedule-trigger-node/__tests__/schedule-trigger-task-handler.test.ts +++ b/packages/cli/src/scheduling/schedule-trigger-node/__tests__/schedule-trigger-task-handler.test.ts @@ -41,7 +41,6 @@ describe('ScheduleTriggerTaskHandler', () => { triggerExecutionContextFactory, workflowExecutionService, ownershipService, - mock(), ); // The executor's dispatch-marker callback; cleared each test by vi.clearAllMocks(). diff --git a/packages/cli/src/scheduling/schedule-trigger-node/schedule-trigger-task-handler.ts b/packages/cli/src/scheduling/schedule-trigger-node/schedule-trigger-task-handler.ts index 1718a4755c1..4fff7e4aa9f 100644 --- a/packages/cli/src/scheduling/schedule-trigger-node/schedule-trigger-task-handler.ts +++ b/packages/cli/src/scheduling/schedule-trigger-node/schedule-trigger-task-handler.ts @@ -13,7 +13,6 @@ import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-da import { TriggerExecutionContextFactory } from '@/workflows/triggers/trigger-execution-context.factory'; import { getWorkflowProjectDetailsSafe } from '@/workflows/utils'; import { WorkflowExecutionService } from '@/workflows/workflow-execution.service'; -import { WorkflowPublisherService } from '@/workflows/workflow-publisher.service'; import { resolveTaskTriggerNode } from '../resolve-task-trigger-node'; import { @@ -43,7 +42,6 @@ export class ScheduleTriggerTaskHandler implements TaskHandler { private readonly triggerExecutionContextFactory: TriggerExecutionContextFactory, private readonly workflowExecutionService: WorkflowExecutionService, private readonly ownershipService: OwnershipService, - private readonly workflowPublisherService: WorkflowPublisherService, ) { this.logger = this.logger.scoped('scheduler'); } @@ -82,11 +80,6 @@ export class ScheduleTriggerTaskHandler implements TaskHandler { const additionalData = await WorkflowExecuteAdditionalData.getBase({ workflowId, workflowSettings: workflowData.settings, - // A schedule has nobody to be, so the run is attributed to the publisher. - userId: await this.workflowPublisherService.findPublisherUserId( - workflowId, - workflowData.activeVersionId, - ), }); try { diff --git a/packages/cli/src/webhooks/__tests__/live-webhooks.test.ts b/packages/cli/src/webhooks/__tests__/live-webhooks.test.ts index 25600c986cb..d92188ecb53 100644 --- a/packages/cli/src/webhooks/__tests__/live-webhooks.test.ts +++ b/packages/cli/src/webhooks/__tests__/live-webhooks.test.ts @@ -33,7 +33,6 @@ import type { WebhookService } from '@/webhooks/webhook.service'; import type { WebhookRequest } from '@/webhooks/webhook.types'; import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data'; import type { WorkflowPublishedDataService } from '@/workflows/workflow-published-data.service'; -import type { WorkflowPublisherService } from '@/workflows/workflow-publisher.service'; import type { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service'; vi.mock('@/webhooks/webhook-helpers'); @@ -53,7 +52,6 @@ describe('LiveWebhooks', () => { const expressionEngineConfig = mock({ allowWebhookIsolateSkip: true, }); - const workflowPublisherService = mock(); let liveWebhooks: LiveWebhooks; @@ -70,7 +68,6 @@ describe('LiveWebhooks', () => { workflowsConfig, workflowPublishedDataService, expressionEngineConfig, - workflowPublisherService, ); // Mock WorkflowExecuteAdditionalData.getBase to avoid DI issues @@ -204,67 +201,6 @@ describe('LiveWebhooks', () => { expect(webhookService.getWebhookMethods).not.toHaveBeenCalled(); }); - /** Minimal published workflow the webhook path can actually execute. */ - function publishedWorkflowEntity() { - const nodes: INode[] = [ - { - id: 'webhook-node', - name: NODE_NAME, - type: 'n8n-nodes-base.webhook', - typeVersion: 1, - position: [0, 0], - parameters: { path: WEBHOOK_PATH, httpMethod: 'GET' }, - }, - ]; - return mock({ - id: WORKFLOW_ID, - name: 'Test Workflow', - active: true, - activeVersionId: 'v1', - nodes, - connections: {}, - staticData: {}, - activeVersion: mock({ - versionId: 'v1', - workflowId: WORKFLOW_ID, - nodes, - connections: {}, - }), - shared: [{ role: 'workflow:owner', project: { id: 'project-1', projectRelations: [] } }], - }); - } - - // A production webhook is fired by a third party, so the run is attributed - // to whoever published the workflow. - it('passes the publishing user into the execution context', async () => { - workflowPublisherService.findPublisherUserId.mockResolvedValue('publisher-1'); - - const workflowEntity = publishedWorkflowEntity(); - const request = setupExecuteWebhookMocks(workflowEntity); - - await liveWebhooks.executeWebhook(request, mock()); - - // The version comes from the workflow the webhook path already loaded, - // so this costs no extra query. - expect(workflowPublisherService.findPublisherUserId).toHaveBeenCalledWith(WORKFLOW_ID, 'v1'); - expect(WorkflowExecuteAdditionalData.getBase).toHaveBeenCalledWith( - expect.objectContaining({ userId: 'publisher-1' }), - ); - }); - - it('leaves the context without a user when nobody published the workflow', async () => { - workflowPublisherService.findPublisherUserId.mockResolvedValue(undefined); - - const workflowEntity = publishedWorkflowEntity(); - const request = setupExecuteWebhookMocks(workflowEntity); - - await liveWebhooks.executeWebhook(request, mock()); - - expect(WorkflowExecuteAdditionalData.getBase).toHaveBeenCalledWith( - expect.objectContaining({ userId: undefined }), - ); - }); - it('should look up allowed methods and include them in the 404 when no webhook is registered', async () => { webhookService.findWebhook.mockResolvedValue(null); webhookService.getWebhookMethods.mockResolvedValue(['POST', 'PUT']); diff --git a/packages/cli/src/webhooks/live-webhooks.ts b/packages/cli/src/webhooks/live-webhooks.ts index 42fbaf5e36d..f0e112ad7c7 100644 --- a/packages/cli/src/webhooks/live-webhooks.ts +++ b/packages/cli/src/webhooks/live-webhooks.ts @@ -20,7 +20,6 @@ import * as WebhookHelpers from '@/webhooks/webhook-helpers'; import { WebhookService } from '@/webhooks/webhook.service'; import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data'; import { WorkflowPublishedDataService } from '@/workflows/workflow-published-data.service'; -import { WorkflowPublisherService } from '@/workflows/workflow-publisher.service'; import { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service'; import { authAllowlistedNodes } from './constants'; @@ -50,7 +49,6 @@ export class LiveWebhooks implements IWebhookManager { private readonly workflowsConfig: WorkflowsConfig, private readonly workflowPublishedDataService: WorkflowPublishedDataService, private readonly expressionEngineConfig: ExpressionEngineConfig, - private readonly workflowPublisherService: WorkflowPublisherService, ) {} async getWebhookMethods(path: string) { @@ -138,12 +136,6 @@ export class LiveWebhooks implements IWebhookManager { )?.projectId; const additionalData = await WorkflowExecuteAdditionalData.getBase({ projectId: ownerProjectId, - // A production webhook is fired by a third party, so the run is - // attributed to whoever published the workflow. - userId: await this.workflowPublisherService.findPublisherUserId( - webhook.workflowId, - workflowData.activeVersionId, - ), }); const startNode = workflow.getNode(webhook.node); diff --git a/packages/cli/src/webhooks/webhook.service.ts b/packages/cli/src/webhooks/webhook.service.ts index 300ccfd67e2..25f7d7e6027 100644 --- a/packages/cli/src/webhooks/webhook.service.ts +++ b/packages/cli/src/webhooks/webhook.service.ts @@ -472,10 +472,6 @@ export class WebhookService { workflowId, workflowExecuteAdditionalData: additionalData, webhookId, - // Who the run is attributed to: the test-webhook owner on a - // manual run, the workflow's publisher on a production one. - // `executeWebhook` copies this onto the execution. - userId: additionalData.userId, }); }); } diff --git a/packages/cli/src/workflow-execute-additional-data.ts b/packages/cli/src/workflow-execute-additional-data.ts index 5a942d1685f..f67780316c5 100644 --- a/packages/cli/src/workflow-execute-additional-data.ts +++ b/packages/cli/src/workflow-execute-additional-data.ts @@ -573,25 +573,10 @@ async function startExecution( let data; try { - // The original mode of the whole run tree: a sub-workflow's own - // `WorkflowExecute` always runs as 'integrated'. Same pattern as - // `credentials-helper`. - const rootExecutionMode = additionalData.rootExecutionMode ?? options.executionMode; - - if ( - isInlineSubworkflow && - additionalData.userId && - isManualOrChatExecution(rootExecutionMode) - ) { + if (isInlineSubworkflow && additionalData.userId) { // Inline sub-workflow triggered by a specific user: its credentials were // never vetted against that user (they live only in the parameter JSON), // so validate them against the user rather than the parent's project. - // - // Gated on the run being user-initiated, not merely on a user being - // present. Triggered runs now carry the publishing user for attribution, - // and that must not silently swap this project check for a user check — - // an ordinary project credential has to keep working on a schedule even - // if the publisher's own access to it has since changed. await Container.get(CredentialsPermissionChecker).checkForUser( additionalData.userId, workflowData.nodes, diff --git a/packages/cli/src/workflows/__tests__/workflow-publisher.service.test.ts b/packages/cli/src/workflows/__tests__/workflow-publisher.service.test.ts deleted file mode 100644 index 43c23f0bd73..00000000000 --- a/packages/cli/src/workflows/__tests__/workflow-publisher.service.test.ts +++ /dev/null @@ -1,99 +0,0 @@ -import type { Logger } from '@n8n/backend-common'; -import type { WorkflowEntity, WorkflowPublishHistoryRepository, WorkflowRepository } from '@n8n/db'; -import { mock } from 'vitest-mock-extended'; - -import * as credentialSharing from '@/constants/credential-sharing'; -import { WorkflowPublisherService } from '@/workflows/workflow-publisher.service'; - -describe('WorkflowPublisherService', () => { - const logger = mock(); - const workflowRepository = mock(); - const publishHistoryRepository = mock(); - - const service = new WorkflowPublisherService( - logger, - workflowRepository, - publishHistoryRepository, - ); - - const workflowId = 'workflow-1'; - - beforeEach(() => { - vi.resetAllMocks(); - vi.spyOn(credentialSharing, 'isCredSharingEnabled').mockReturnValue(true); - workflowRepository.findOne.mockResolvedValue( - mock({ id: workflowId, activeVersionId: 'version-1' }), - ); - }); - - it('attributes the run to whoever published the active version', async () => { - publishHistoryRepository.findPublisherUserId.mockResolvedValue('user-1'); - - await expect(service.findPublisherUserId(workflowId)).resolves.toBe('user-1'); - - expect(publishHistoryRepository.findPublisherUserId).toHaveBeenCalledWith( - workflowId, - 'version-1', - ); - }); - - // A deleted publisher nulls the column; a git-imported workflow never had one. - // Either way the run carries no identity and behaves as it does today. - it('returns nothing and logs when there is no publisher', async () => { - publishHistoryRepository.findPublisherUserId.mockResolvedValue(undefined); - - await expect(service.findPublisherUserId(workflowId)).resolves.toBeUndefined(); - - expect(logger.debug).toHaveBeenCalledWith( - 'Triggered execution has no publishing user to attribute it to', - { workflowId }, - ); - }); - - it('still asks when the workflow has no active version', async () => { - workflowRepository.findOne.mockResolvedValue(null); - publishHistoryRepository.findPublisherUserId.mockResolvedValue('user-1'); - - await expect(service.findPublisherUserId(workflowId)).resolves.toBe('user-1'); - - expect(publishHistoryRepository.findPublisherUserId).toHaveBeenCalledWith( - workflowId, - undefined, - ); - }); - - // The schedule, poll and webhook paths run this per execution and already - // hold the workflow, so they pass the version and save a query each time. - it('skips the workflow lookup when the caller supplies the version', async () => { - publishHistoryRepository.findPublisherUserId.mockResolvedValue('user-1'); - - await expect(service.findPublisherUserId(workflowId, 'version-9')).resolves.toBe('user-1'); - - expect(workflowRepository.findOne).not.toHaveBeenCalled(); - expect(publishHistoryRepository.findPublisherUserId).toHaveBeenCalledWith( - workflowId, - 'version-9', - ); - }); - - // `null` is a real value here — an unpublished workflow — and must not be - // mistaken for "the caller did not tell me". - it('treats a null version from the caller as given, not missing', async () => { - publishHistoryRepository.findPublisherUserId.mockResolvedValue(undefined); - - await service.findPublisherUserId(workflowId, null); - - expect(workflowRepository.findOne).not.toHaveBeenCalled(); - expect(publishHistoryRepository.findPublisherUserId).toHaveBeenCalledWith(workflowId, null); - }); - - // The single gate for this behaviour, so every call site stays unconditional. - it('reads nothing at all while the feature flag is off', async () => { - vi.spyOn(credentialSharing, 'isCredSharingEnabled').mockReturnValue(false); - - await expect(service.findPublisherUserId(workflowId)).resolves.toBeUndefined(); - - expect(workflowRepository.findOne).not.toHaveBeenCalled(); - expect(publishHistoryRepository.findPublisherUserId).not.toHaveBeenCalled(); - }); -}); diff --git a/packages/cli/src/workflows/triggers/__tests__/trigger-execution-context.factory.test.ts b/packages/cli/src/workflows/triggers/__tests__/trigger-execution-context.factory.test.ts index 00689f9bc20..e80cd683bbd 100644 --- a/packages/cli/src/workflows/triggers/__tests__/trigger-execution-context.factory.test.ts +++ b/packages/cli/src/workflows/triggers/__tests__/trigger-execution-context.factory.test.ts @@ -118,7 +118,6 @@ describe('TriggerExecutionContextFactory', () => { pollCursorService, mock({ scheduler: { pollTimeoutSeconds: 45, leaseDurationSeconds: 60 } }), engineV2ActiveTriggers, - mock(), ); }); @@ -1087,7 +1086,6 @@ describe('TriggerExecutionContextFactory', () => { pollCursorService, globalConfig, engineV2ActiveTriggers, - mock(), ); const getPollFunctions = budgetFactory.getExecutePollFunctions( mock({ id: 'wf-1', name: 'Test Workflow' }), diff --git a/packages/cli/src/workflows/triggers/trigger-execution-context.factory.ts b/packages/cli/src/workflows/triggers/trigger-execution-context.factory.ts index ea58017e420..d45efda82bf 100644 --- a/packages/cli/src/workflows/triggers/trigger-execution-context.factory.ts +++ b/packages/cli/src/workflows/triggers/trigger-execution-context.factory.ts @@ -47,7 +47,6 @@ import { PollCursorService } from '@/workflows/triggers/poll-cursor.service'; import { getWorkflowProjectDetailsSafe } from '@/workflows/utils'; import { WorkflowExecutionService } from '@/workflows/workflow-execution.service'; import { WorkflowPublishedDataService } from '@/workflows/workflow-published-data.service'; -import { WorkflowPublisherService } from '@/workflows/workflow-publisher.service'; import { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service'; export type TriggerFailureHandler = (opts: { @@ -86,7 +85,6 @@ export class TriggerExecutionContextFactory { private readonly pollCursorService: PollCursorService, private readonly globalConfig: GlobalConfig, private readonly engineV2ActiveTriggers: EngineV2ActiveTriggers, - private readonly workflowPublisherService: WorkflowPublisherService, ) { this.logger = this.logger.scoped(['workflow-activation']); } @@ -523,11 +521,6 @@ export class TriggerExecutionContextFactory { const additionalData = await WorkflowExecuteAdditionalData.getBase({ workflowId: workflowData.id, workflowSettings: workflowData.settings, - // A poll has nobody to be, so the run is attributed to the publisher. - userId: await this.workflowPublisherService.findPublisherUserId( - workflowData.id, - workflowData.activeVersionId, - ), }); const resolveWorkflowData = async () => diff --git a/packages/cli/src/workflows/workflow-publisher.service.ts b/packages/cli/src/workflows/workflow-publisher.service.ts deleted file mode 100644 index ea27645ec36..00000000000 --- a/packages/cli/src/workflows/workflow-publisher.service.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { Logger } from '@n8n/backend-common'; -import { WorkflowPublishHistoryRepository, WorkflowRepository } from '@n8n/db'; -import { Service } from '@n8n/di'; - -import { isCredSharingEnabled } from '@/constants/credential-sharing'; - -/** - * Resolves who a triggered execution should be attributed to. - * - * A schedule, poll or production webhook is started by the system, so nothing - * on the execution path carries a user. n8n already records who published every - * version, though — it just never reads it back. That record gives a triggered - * run the identity it lacks: the workflow is attributed to whoever published - * it, and stays attributed to them while they are away. - */ -@Service() -export class WorkflowPublisherService { - constructor( - private readonly logger: Logger, - private readonly workflowRepository: WorkflowRepository, - private readonly publishHistoryRepository: WorkflowPublishHistoryRepository, - ) {} - - /** - * The publisher of the workflow's active version, or `undefined` when there - * is none to attribute the run to. - * - * Left empty rather than substituted with a stand-in. A run with no identity - * behaves exactly as it does today, so callers need no fallback: this only - * ever adds attribution where there was none. - * - * Returns `undefined` while the feature flag is off, which is the single - * gate for this behaviour — call sites stay unconditional. - * - * @param activeVersionId - pass it when the caller already holds the - * workflow, which the schedule, poll and webhook paths all do. This runs per - * execution on those paths, so the lookup it saves is per execution too. - */ - async findPublisherUserId( - workflowId: string, - activeVersionId?: string | null, - ): Promise { - if (!isCredSharingEnabled()) return undefined; - - const versionId = - activeVersionId !== undefined - ? activeVersionId - : ( - await this.workflowRepository.findOne({ - where: { id: workflowId }, - select: ['id', 'activeVersionId'], - }) - )?.activeVersionId; - - const publisherUserId = await this.publishHistoryRepository.findPublisherUserId( - workflowId, - versionId, - ); - - if (!publisherUserId) { - // A deleted publisher, or a workflow that arrived without an activation - // record — imported from source control, say. - this.logger.debug('Triggered execution has no publishing user to attribute it to', { - workflowId, - }); - } - - return publisherUserId; - } -} diff --git a/packages/cli/test/integration/database/repositories/workflow-publish-history.repository.test.ts b/packages/cli/test/integration/database/repositories/workflow-publish-history.repository.test.ts index 1bc70cd1471..754e279464b 100644 --- a/packages/cli/test/integration/database/repositories/workflow-publish-history.repository.test.ts +++ b/packages/cli/test/integration/database/repositories/workflow-publish-history.repository.test.ts @@ -206,103 +206,6 @@ describe('WorkflowPublishHistoryRepository', () => { }); }); - describe('findPublisherUserId', () => { - // Republishing an older version means the newest activation is not - // necessarily the live one, so the version asked about wins. - it('prefers the publisher of the version it is asked about', async () => { - const repository = Container.get(WorkflowPublishHistoryRepository); - const older = await createUser(); - const newer = await createUser(); - const id1 = uuid(); - const id2 = uuid(); - const workflow = await createWorkflow(); - await createWorkflowHistory({ ...workflow, versionId: id1 }); - await createWorkflowHistory({ ...workflow, versionId: id2 }); - - await repository.addRecord({ - workflowId: workflow.id, - versionId: id1, - event: 'activated', - userId: older.id, - }); - - // Keep `createdAt` ordering deterministic, as the tests above do. - await new Promise((resolve) => setTimeout(resolve, 5)); - - await repository.addRecord({ - workflowId: workflow.id, - versionId: id2, - event: 'activated', - userId: newer.id, - }); - - await expect(repository.findPublisherUserId(workflow.id, id1)).resolves.toBe(older.id); - // and without a version, the most recent activation wins - await expect(repository.findPublisherUserId(workflow.id)).resolves.toBe(newer.id); - }); - - // Covers a published version whose history row has since been pruned. - it('falls back to the latest activation when the version has none', async () => { - const repository = Container.get(WorkflowPublishHistoryRepository); - const user = await createUser(); - const workflow = await createWorkflowWithHistory(); - - await repository.addRecord({ - workflowId: workflow.id, - versionId: workflow.versionId, - event: 'activated', - userId: user.id, - }); - - const result = await repository.findPublisherUserId(workflow.id, 'a-version-never-activated'); - - expect(result).toBe(user.id); - }); - - it('falls back to the latest activation when no version is given', async () => { - const repository = Container.get(WorkflowPublishHistoryRepository); - const user = await createUser(); - const workflow = await createWorkflowWithHistory(); - - await repository.addRecord({ - workflowId: workflow.id, - versionId: workflow.versionId, - event: 'activated', - userId: user.id, - }); - - const result = await repository.findPublisherUserId(workflow.id); - - expect(result).toBe(user.id); - }); - - // The FK nulls the column when the publisher is deleted. - it('returns undefined when the activation records no user', async () => { - const repository = Container.get(WorkflowPublishHistoryRepository); - const workflow = await createWorkflowWithHistory(); - - await repository.addRecord({ - workflowId: workflow.id, - versionId: workflow.versionId, - event: 'activated', - userId: null, - }); - - const result = await repository.findPublisherUserId(workflow.id, workflow.versionId); - - expect(result).toBeUndefined(); - }); - - it('returns undefined for a workflow that was never activated', async () => { - const repository = Container.get(WorkflowPublishHistoryRepository); - const workflow = await createWorkflowWithHistory(); - - const result = await repository.findPublisherUserId(workflow.id, workflow.versionId); - - expect(result).toBeUndefined(); - }); - }); - describe('Foreign key constraints', () => { it('should cascade delete when workflow is deleted', async () => { const repository = Container.get(WorkflowPublishHistoryRepository);