mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-19 16:55:02 +02:00
Revert "feat(core): Attach the publishing user to triggered executions"
This reverts commit 47761178a4.
This commit is contained in:
parent
47761178a4
commit
f5b1e2aef1
|
|
@ -47,32 +47,4 @@ export class WorkflowPublishHistoryRepository extends Repository<WorkflowPublish
|
|||
});
|
||||
return record?.userId ?? undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Who published the workflow's currently active version, so a triggered run
|
||||
* can be attributed to them.
|
||||
*
|
||||
* Prefers the activation of `versionId`: republishing an older version means
|
||||
* the most recent activation is not necessarily the live one. Falls back to
|
||||
* the latest activation of any version, which covers a published version
|
||||
* whose history row has since been pruned.
|
||||
*
|
||||
* Returns `undefined` when the publisher was deleted (the FK nulls the
|
||||
* column) or the workflow never recorded an activation.
|
||||
*/
|
||||
async findPublisherUserId(
|
||||
workflowId: string,
|
||||
versionId?: string | null,
|
||||
): Promise<string | undefined> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<WorkflowEntity> = {}) =>
|
||||
|
|
@ -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<GlobalConfig>({ 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<InstanceSettings>({ isLeader: true }),
|
||||
mock<Logger>({ scoped: vi.fn().mockReturnValue(mock<Logger>()) }),
|
||||
mock(), // workflowPublisherService
|
||||
mock(),
|
||||
);
|
||||
realActiveWorkflowTriggers = new ActiveWorkflowTriggers(
|
||||
mock<Logger>({ scoped: vi.fn().mockReturnValue(mock<Logger>()) }),
|
||||
|
|
@ -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 = () => {
|
||||
|
|
|
|||
|
|
@ -313,17 +313,11 @@ describe('WorkflowExecuteAdditionalData', () => {
|
|||
it('checks credentials against the triggering user for an inline sub-workflow', async () => {
|
||||
await executeWorkflow(
|
||||
mock<IExecuteWorkflowInfo>({ id: undefined, code: subWorkflowData() }),
|
||||
// `rootExecutionMode` explicit: a deep mock auto-stubs every
|
||||
// property, so leaving it out would hand `??` a truthy proxy.
|
||||
mock<IWorkflowExecuteAdditionalData>({
|
||||
userId: 'user-1',
|
||||
rootExecutionMode: undefined,
|
||||
}),
|
||||
mock<IWorkflowExecuteAdditionalData>({ userId: 'user-1' }),
|
||||
mock<ExecuteWorkflowOptions>({
|
||||
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<IExecuteWorkflowInfo>({ id: undefined, code: subWorkflowData() }),
|
||||
mock<IWorkflowExecuteAdditionalData>({
|
||||
userId: 'user-1',
|
||||
rootExecutionMode: 'manual',
|
||||
}),
|
||||
mock<ExecuteWorkflowOptions>({
|
||||
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<IExecuteWorkflowInfo>({ id: undefined, code: subWorkflowData() }),
|
||||
mock<IWorkflowExecuteAdditionalData>({
|
||||
userId: 'publisher-1',
|
||||
rootExecutionMode: undefined,
|
||||
}),
|
||||
mock<ExecuteWorkflowOptions>({
|
||||
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<IExecuteWorkflowInfo>({ id: 'db-id', code: undefined }),
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -41,7 +41,6 @@ describe('ScheduleTriggerTaskHandler', () => {
|
|||
triggerExecutionContextFactory,
|
||||
workflowExecutionService,
|
||||
ownershipService,
|
||||
mock(),
|
||||
);
|
||||
|
||||
// The executor's dispatch-marker callback; cleared each test by vi.clearAllMocks().
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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<ExpressionEngineConfig>({
|
||||
allowWebhookIsolateSkip: true,
|
||||
});
|
||||
const workflowPublisherService = mock<WorkflowPublisherService>();
|
||||
|
||||
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<WorkflowEntity>({
|
||||
id: WORKFLOW_ID,
|
||||
name: 'Test Workflow',
|
||||
active: true,
|
||||
activeVersionId: 'v1',
|
||||
nodes,
|
||||
connections: {},
|
||||
staticData: {},
|
||||
activeVersion: mock<WorkflowHistory>({
|
||||
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<Response>());
|
||||
|
||||
// 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<Response>());
|
||||
|
||||
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']);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
});
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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<Logger>();
|
||||
const workflowRepository = mock<WorkflowRepository>();
|
||||
const publishHistoryRepository = mock<WorkflowPublishHistoryRepository>();
|
||||
|
||||
const service = new WorkflowPublisherService(
|
||||
logger,
|
||||
workflowRepository,
|
||||
publishHistoryRepository,
|
||||
);
|
||||
|
||||
const workflowId = 'workflow-1';
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.spyOn(credentialSharing, 'isCredSharingEnabled').mockReturnValue(true);
|
||||
workflowRepository.findOne.mockResolvedValue(
|
||||
mock<WorkflowEntity>({ 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();
|
||||
});
|
||||
});
|
||||
|
|
@ -118,7 +118,6 @@ describe('TriggerExecutionContextFactory', () => {
|
|||
pollCursorService,
|
||||
mock<GlobalConfig>({ scheduler: { pollTimeoutSeconds: 45, leaseDurationSeconds: 60 } }),
|
||||
engineV2ActiveTriggers,
|
||||
mock(),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -1087,7 +1086,6 @@ describe('TriggerExecutionContextFactory', () => {
|
|||
pollCursorService,
|
||||
globalConfig,
|
||||
engineV2ActiveTriggers,
|
||||
mock(),
|
||||
);
|
||||
const getPollFunctions = budgetFactory.getExecutePollFunctions(
|
||||
mock<IWorkflowBase>({ id: 'wf-1', name: 'Test Workflow' }),
|
||||
|
|
|
|||
|
|
@ -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 () =>
|
||||
|
|
|
|||
|
|
@ -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<string | undefined> {
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user