refactor(core): Improve UX on permission errors (no-changelog) (#13795)

Co-authored-by: Iván Ovejero <ivov.src@gmail.com>
This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™
2025-03-11 17:52:33 +01:00
committed by GitHub
parent a12935d724
commit ca9e62bdc0
17 changed files with 298 additions and 134 deletions

View File

@@ -17,12 +17,14 @@ import { ExecutionRepository } from '@/databases/repositories/execution.reposito
import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository';
import { VariablesService } from '@/environments.ee/variables/variables.service.ee'; import { VariablesService } from '@/environments.ee/variables/variables.service.ee';
import { EventService } from '@/events/event.service'; import { EventService } from '@/events/event.service';
import {
CredentialsPermissionChecker,
SubworkflowPolicyChecker,
} from '@/executions/pre-execution-checks';
import { ExternalHooks } from '@/external-hooks'; import { ExternalHooks } from '@/external-hooks';
import { SecretsHelper } from '@/secrets-helpers.ee'; import { SecretsHelper } from '@/secrets-helpers.ee';
import { WorkflowStatisticsService } from '@/services/workflow-statistics.service'; import { WorkflowStatisticsService } from '@/services/workflow-statistics.service';
import { SubworkflowPolicyChecker } from '@/subworkflows/subworkflow-policy-checker.service';
import { Telemetry } from '@/telemetry'; import { Telemetry } from '@/telemetry';
import { PermissionChecker } from '@/user-management/permission-checker';
import { executeWorkflow, getBase, getRunData } from '@/workflow-execute-additional-data'; import { executeWorkflow, getBase, getRunData } from '@/workflow-execute-additional-data';
import { mockInstance } from '@test/mocking'; import { mockInstance } from '@test/mocking';
@@ -91,7 +93,7 @@ describe('WorkflowExecuteAdditionalData', () => {
mockInstance(Telemetry); mockInstance(Telemetry);
const workflowRepository = mockInstance(WorkflowRepository); const workflowRepository = mockInstance(WorkflowRepository);
const activeExecutions = mockInstance(ActiveExecutions); const activeExecutions = mockInstance(ActiveExecutions);
mockInstance(PermissionChecker); mockInstance(CredentialsPermissionChecker);
mockInstance(SubworkflowPolicyChecker); mockInstance(SubworkflowPolicyChecker);
mockInstance(WorkflowStatisticsService); mockInstance(WorkflowStatisticsService);

View File

@@ -21,8 +21,8 @@ import config from '@/config';
import type { ExecutionEntity } from '@/databases/entities/execution-entity'; import type { ExecutionEntity } from '@/databases/entities/execution-entity';
import type { User } from '@/databases/entities/user'; import type { User } from '@/databases/entities/user';
import { ExecutionNotFoundError } from '@/errors/execution-not-found-error'; import { ExecutionNotFoundError } from '@/errors/execution-not-found-error';
import { CredentialsPermissionChecker } from '@/executions/pre-execution-checks';
import { Telemetry } from '@/telemetry'; import { Telemetry } from '@/telemetry';
import { PermissionChecker } from '@/user-management/permission-checker';
import { WorkflowRunner } from '@/workflow-runner'; import { WorkflowRunner } from '@/workflow-runner';
import { mockInstance } from '@test/mocking'; import { mockInstance } from '@test/mocking';
import { createExecution } from '@test-integration/db/executions'; import { createExecution } from '@test-integration/db/executions';
@@ -131,7 +131,7 @@ describe('run', () => {
const activeExecutions = Container.get(ActiveExecutions); const activeExecutions = Container.get(ActiveExecutions);
jest.spyOn(activeExecutions, 'add').mockResolvedValue('1'); jest.spyOn(activeExecutions, 'add').mockResolvedValue('1');
jest.spyOn(activeExecutions, 'attachWorkflowExecution').mockReturnValueOnce(); jest.spyOn(activeExecutions, 'attachWorkflowExecution').mockReturnValueOnce();
const permissionChecker = Container.get(PermissionChecker); const permissionChecker = Container.get(CredentialsPermissionChecker);
jest.spyOn(permissionChecker, 'check').mockResolvedValueOnce(); jest.spyOn(permissionChecker, 'check').mockResolvedValueOnce();
jest.spyOn(WorkflowExecute.prototype, 'processRunExecutionData').mockReturnValueOnce( jest.spyOn(WorkflowExecute.prototype, 'processRunExecutionData').mockReturnValueOnce(
@@ -171,7 +171,7 @@ describe('run', () => {
const activeExecutions = Container.get(ActiveExecutions); const activeExecutions = Container.get(ActiveExecutions);
jest.spyOn(activeExecutions, 'add').mockResolvedValue('1'); jest.spyOn(activeExecutions, 'add').mockResolvedValue('1');
jest.spyOn(activeExecutions, 'attachWorkflowExecution').mockReturnValueOnce(); jest.spyOn(activeExecutions, 'attachWorkflowExecution').mockReturnValueOnce();
const permissionChecker = Container.get(PermissionChecker); const permissionChecker = Container.get(CredentialsPermissionChecker);
jest.spyOn(permissionChecker, 'check').mockResolvedValueOnce(); jest.spyOn(permissionChecker, 'check').mockResolvedValueOnce();
jest.spyOn(WorkflowExecute.prototype, 'processRunExecutionData').mockReturnValueOnce( jest.spyOn(WorkflowExecute.prototype, 'processRunExecutionData').mockReturnValueOnce(

View File

@@ -0,0 +1,47 @@
import { mock } from 'jest-mock-extended';
import { NodeOperationError } from 'n8n-workflow';
import type { INode, WorkflowExecuteMode } from 'n8n-workflow';
import { ExecutionDataService } from '../execution-data.service';
describe('ExecutionDataService', () => {
const service = new ExecutionDataService();
describe('generateFailedExecutionFromError', () => {
const mode: WorkflowExecuteMode = 'manual';
const node = mock<INode>({ name: 'Test Node' });
const error = new NodeOperationError(node, 'Test error message');
it('should generate a failed execution with error details', () => {
const startTime = Date.now();
const result = service.generateFailedExecutionFromError(mode, error, node, startTime);
expect(result.mode).toBe(mode);
expect(result.status).toBe('error');
expect(result.startedAt).toBeInstanceOf(Date);
expect(result.stoppedAt).toBeInstanceOf(Date);
expect(result.data.resultData.error?.message).toBe(error.message);
const taskData = result.data.resultData.runData[node.name][0];
expect(taskData.error?.message).toBe(error.message);
expect(taskData.startTime).toBe(startTime);
expect(taskData.executionStatus).toBe('error');
expect(result.data.resultData.lastNodeExecuted).toBe(node.name);
expect(result.data.executionData?.nodeExecutionStack[0].node).toEqual(node);
});
it('should generate a failed execution without node details if node is undefined', () => {
const result = service.generateFailedExecutionFromError(mode, error, undefined);
expect(result.mode).toBe(mode);
expect(result.status).toBe('error');
expect(result.startedAt).toBeInstanceOf(Date);
expect(result.stoppedAt).toBeInstanceOf(Date);
expect(result.data.resultData.error?.message).toBe(error.message);
expect(result.data.resultData.runData).toEqual({});
expect(result.data.resultData.lastNodeExecuted).toBeUndefined();
expect(result.data.executionData).toBeUndefined();
});
});
});

View File

@@ -0,0 +1,62 @@
import { Service } from '@n8n/di';
import type { ExecutionError, INode, IRun, WorkflowExecuteMode } from 'n8n-workflow';
@Service()
export class ExecutionDataService {
generateFailedExecutionFromError(
mode: WorkflowExecuteMode,
error: ExecutionError,
node: INode | undefined,
startTime = Date.now(),
): IRun {
const executionError = {
...error,
message: error.message,
stack: error.stack,
};
const returnData: IRun = {
data: {
resultData: {
error: executionError,
runData: {},
},
},
finished: false,
mode,
startedAt: new Date(),
stoppedAt: new Date(),
status: 'error',
};
if (node) {
returnData.data.startData = {
destinationNode: node.name,
runNodeFilter: [node.name],
};
returnData.data.resultData.lastNodeExecuted = node.name;
returnData.data.resultData.runData[node.name] = [
{
startTime,
executionTime: 0,
executionStatus: 'error',
error: executionError,
source: [],
},
];
returnData.data.executionData = {
contextData: {},
metadata: {},
waitingExecution: {},
waitingExecutionSource: {},
nodeExecutionStack: [
{
node,
data: {},
source: null,
},
],
};
}
return returnData;
}
}

View File

@@ -0,0 +1,108 @@
import { mock } from 'jest-mock-extended';
import type { INode } from 'n8n-workflow';
import type { Project } from '@/databases/entities/project';
import type { User } from '@/databases/entities/user';
import type { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository';
import type { OwnershipService } from '@/services/ownership.service';
import type { ProjectService } from '@/services/project.service.ee';
import { CredentialsPermissionChecker } from '../credentials-permission-checker';
describe('CredentialsPermissionChecker', () => {
const sharedCredentialsRepository = mock<SharedCredentialsRepository>();
const ownershipService = mock<OwnershipService>();
const projectService = mock<ProjectService>();
const permissionChecker = new CredentialsPermissionChecker(
sharedCredentialsRepository,
ownershipService,
projectService,
);
const workflowId = 'workflow123';
const credentialId = 'cred123';
const personalProject = mock<Project>({
id: 'personal-project',
name: 'Personal Project',
type: 'personal',
});
const node = mock<INode>({
name: 'Test Node',
credentials: {
someCredential: {
id: credentialId,
name: 'Test Credential',
},
},
disabled: false,
});
beforeEach(async () => {
jest.resetAllMocks();
node.credentials!.someCredential.id = credentialId;
ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(personalProject);
projectService.findProjectsWorkflowIsIn.mockResolvedValueOnce([personalProject.id]);
});
it('should throw if a node has a credential without an id', async () => {
node.credentials!.someCredential.id = null;
await expect(permissionChecker.check(workflowId, [node])).rejects.toThrow(
'Node "Test Node" uses invalid credential',
);
expect(projectService.findProjectsWorkflowIsIn).toHaveBeenCalledWith(workflowId);
expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).not.toHaveBeenCalled();
});
it('should throw if a credential is not accessible', async () => {
ownershipService.getPersonalProjectOwnerCached.mockResolvedValueOnce(null);
sharedCredentialsRepository.getFilteredAccessibleCredentials.mockResolvedValueOnce([]);
await expect(permissionChecker.check(workflowId, [node])).rejects.toThrow(
'Node "Test Node" does not have access to the credential',
);
expect(projectService.findProjectsWorkflowIsIn).toHaveBeenCalledWith(workflowId);
expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).toHaveBeenCalledWith(
[personalProject.id],
[credentialId],
);
});
it('should not throw an error if the workflow has no credentials', async () => {
await expect(permissionChecker.check(workflowId, [])).resolves.not.toThrow();
expect(projectService.findProjectsWorkflowIsIn).toHaveBeenCalledWith(workflowId);
expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).not.toHaveBeenCalled();
});
it('should not throw an error if all credentials are accessible', async () => {
ownershipService.getPersonalProjectOwnerCached.mockResolvedValueOnce(null);
sharedCredentialsRepository.getFilteredAccessibleCredentials.mockResolvedValueOnce([
credentialId,
]);
await expect(permissionChecker.check(workflowId, [node])).resolves.not.toThrow();
expect(projectService.findProjectsWorkflowIsIn).toHaveBeenCalledWith(workflowId);
expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).toHaveBeenCalledWith(
[personalProject.id],
[credentialId],
);
});
it('should skip credential checks if the home project owner has global scope', async () => {
const projectOwner = mock<User>({
hasGlobalScope: (scope) => scope === 'credential:list',
});
ownershipService.getPersonalProjectOwnerCached.mockResolvedValueOnce(projectOwner);
await expect(permissionChecker.check(workflowId, [node])).resolves.not.toThrow();
expect(projectService.findProjectsWorkflowIsIn).not.toHaveBeenCalled();
expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).not.toHaveBeenCalled();
});
});

View File

@@ -15,7 +15,7 @@ import { OwnershipService } from '@/services/ownership.service';
import type { UrlService } from '@/services/url.service'; import type { UrlService } from '@/services/url.service';
import { mockInstance } from '@test/mocking'; import { mockInstance } from '@test/mocking';
import { SubworkflowPolicyChecker } from '../subworkflow-policy-checker.service'; import { SubworkflowPolicyChecker } from '../subworkflow-policy-checker';
describe('SubworkflowPolicyChecker', () => { describe('SubworkflowPolicyChecker', () => {
const ownershipService = mockInstance(OwnershipService); const ownershipService = mockInstance(OwnershipService);

View File

@@ -1,13 +1,36 @@
import { Service } from '@n8n/di'; import { Service } from '@n8n/di';
import type { INode } from 'n8n-workflow'; import type { INode } from 'n8n-workflow';
import { CredentialAccessError, NodeOperationError } from 'n8n-workflow'; import { UserError } from 'n8n-workflow';
import type { Project } from '@/databases/entities/project';
import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository';
import { OwnershipService } from '@/services/ownership.service'; import { OwnershipService } from '@/services/ownership.service';
import { ProjectService } from '@/services/project.service.ee'; import { ProjectService } from '@/services/project.service.ee';
class InvalidCredentialError extends UserError {
override description = 'Please recreate the credential.';
constructor(readonly node: INode) {
super(`Node "${node.name}" uses invalid credential`);
}
}
class InaccessibleCredentialError extends UserError {
override description =
this.project.type === 'personal'
? 'Please recreate the credential or ask its owner to share it with you.'
: `Please make sure that the credential is shared with the project "${this.project.name}"`;
constructor(
readonly node: INode,
private readonly project: Project,
) {
super(`Node "${node.name}" does not have access to the credential`);
}
}
@Service() @Service()
export class PermissionChecker { export class CredentialsPermissionChecker {
constructor( constructor(
private readonly sharedCredentialsRepository: SharedCredentialsRepository, private readonly sharedCredentialsRepository: SharedCredentialsRepository,
private readonly ownershipService: OwnershipService, private readonly ownershipService: OwnershipService,
@@ -42,7 +65,7 @@ export class PermissionChecker {
for (const credentialsId of workflowCredIds) { for (const credentialsId of workflowCredIds) {
if (!accessible.includes(credentialsId)) { if (!accessible.includes(credentialsId)) {
const nodeToFlag = credIdsToNodes[credentialsId][0]; const nodeToFlag = credIdsToNodes[credentialsId][0];
throw new CredentialAccessError(nodeToFlag, credentialsId, workflowId); throw new InaccessibleCredentialError(nodeToFlag, homeProject);
} }
} }
} }
@@ -52,12 +75,7 @@ export class PermissionChecker {
if (node.disabled || !node.credentials) return map; if (node.disabled || !node.credentials) return map;
Object.values(node.credentials).forEach((cred) => { Object.values(node.credentials).forEach((cred) => {
if (!cred.id) { if (!cred.id) throw new InvalidCredentialError(node);
throw new NodeOperationError(node, 'Node uses invalid credential', {
description: 'Please recreate the credential.',
level: 'warning',
});
}
map[cred.id] = map[cred.id] ? [...map[cred.id], node] : [node]; map[cred.id] = map[cred.id] ? [...map[cred.id], node] : [node];
}); });

View File

@@ -0,0 +1,2 @@
export { CredentialsPermissionChecker } from './credentials-permission-checker';
export { SubworkflowPolicyChecker } from './subworkflow-policy-checker';

View File

@@ -14,7 +14,6 @@ import type {
INode, INode,
INodeExecutionData, INodeExecutionData,
INodeParameters, INodeParameters,
IRun,
IRunExecutionData, IRunExecutionData,
IWorkflowBase, IWorkflowBase,
IWorkflowExecuteAdditionalData, IWorkflowExecuteAdditionalData,
@@ -38,14 +37,17 @@ import { WorkflowRepository } from '@/databases/repositories/workflow.repository
import { EventService } from '@/events/event.service'; import { EventService } from '@/events/event.service';
import type { AiEventMap, AiEventPayload } from '@/events/maps/ai.event-map'; import type { AiEventMap, AiEventPayload } from '@/events/maps/ai.event-map';
import { getLifecycleHooksForSubExecutions } from '@/execution-lifecycle/execution-lifecycle-hooks'; import { getLifecycleHooksForSubExecutions } from '@/execution-lifecycle/execution-lifecycle-hooks';
import { ExecutionDataService } from '@/executions/execution-data.service';
import {
CredentialsPermissionChecker,
SubworkflowPolicyChecker,
} from '@/executions/pre-execution-checks';
import type { UpdateExecutionPayload } from '@/interfaces'; import type { UpdateExecutionPayload } from '@/interfaces';
import { NodeTypes } from '@/node-types'; import { NodeTypes } from '@/node-types';
import { Push } from '@/push'; import { Push } from '@/push';
import { SecretsHelper } from '@/secrets-helpers.ee'; import { SecretsHelper } from '@/secrets-helpers.ee';
import { UrlService } from '@/services/url.service'; import { UrlService } from '@/services/url.service';
import { SubworkflowPolicyChecker } from '@/subworkflows/subworkflow-policy-checker.service';
import { TaskRequester } from '@/task-runners/task-managers/task-requester'; import { TaskRequester } from '@/task-runners/task-managers/task-requester';
import { PermissionChecker } from '@/user-management/permission-checker';
import { findSubworkflowStart } from '@/utils'; import { findSubworkflowStart } from '@/utils';
import { objectToError } from '@/utils/object-to-error'; import { objectToError } from '@/utils/object-to-error';
import * as WorkflowHelpers from '@/workflow-helpers'; import * as WorkflowHelpers from '@/workflow-helpers';
@@ -204,9 +206,11 @@ async function startExecution(
*/ */
await executionRepository.setRunning(executionId); await executionRepository.setRunning(executionId);
const startTime = Date.now();
let data; let data;
try { try {
await Container.get(PermissionChecker).check(workflowData.id, workflowData.nodes); await Container.get(CredentialsPermissionChecker).check(workflowData.id, workflowData.nodes);
await Container.get(SubworkflowPolicyChecker).check( await Container.get(SubworkflowPolicyChecker).check(
workflow, workflow,
options.parentWorkflowId, options.parentWorkflowId,
@@ -239,7 +243,7 @@ async function startExecution(
// If no timeout was given from the parent, then we use our timeout. // If no timeout was given from the parent, then we use our timeout.
subworkflowTimeout = Math.min( subworkflowTimeout = Math.min(
additionalData.executionTimeoutTimestamp || Number.MAX_SAFE_INTEGER, additionalData.executionTimeoutTimestamp || Number.MAX_SAFE_INTEGER,
Date.now() + workflowSettings.executionTimeout * 1000, startTime + workflowSettings.executionTimeout * 1000,
); );
} }
@@ -257,20 +261,14 @@ async function startExecution(
activeExecutions.attachWorkflowExecution(executionId, execution); activeExecutions.attachWorkflowExecution(executionId, execution);
data = await execution; data = await execution;
} catch (error) { } catch (error) {
const executionError = error ? (error as ExecutionError) : undefined; const executionError = error as ExecutionError;
const fullRunData: IRun = { const fullRunData = Container.get(ExecutionDataService).generateFailedExecutionFromError(
data: { runData.executionMode,
resultData: { executionError,
error: executionError, 'node' in executionError ? executionError.node : undefined,
runData: {}, startTime,
}, );
},
finished: false,
mode: 'integrated',
startedAt: new Date(),
stoppedAt: new Date(),
status: 'error',
};
// When failing, we might not have finished the execution // When failing, we might not have finished the execution
// Therefore, database might not contain finished errors. // Therefore, database might not contain finished errors.
// Force an update to db as there should be no harm doing this // Force an update to db as there should be no harm doing this

View File

@@ -1,14 +1,9 @@
import { Container } from '@n8n/di'; import { Container } from '@n8n/di';
import type { import type {
IDataObject, IDataObject,
INode,
INodeCredentialsDetails, INodeCredentialsDetails,
IRun, IRun,
ITaskData, ITaskData,
NodeApiError,
WorkflowExecuteMode,
WorkflowOperationError,
NodeOperationError,
IWorkflowBase, IWorkflowBase,
} from 'n8n-workflow'; } from 'n8n-workflow';
import { v4 as uuid } from 'uuid'; import { v4 as uuid } from 'uuid';
@@ -16,53 +11,6 @@ import { v4 as uuid } from 'uuid';
import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository';
import { VariablesService } from '@/environments.ee/variables/variables.service.ee'; import { VariablesService } from '@/environments.ee/variables/variables.service.ee';
export function generateFailedExecutionFromError(
mode: WorkflowExecuteMode,
error: NodeApiError | NodeOperationError | WorkflowOperationError,
node: INode,
): IRun {
return {
data: {
startData: {
destinationNode: node.name,
runNodeFilter: [node.name],
},
resultData: {
error,
runData: {
[node.name]: [
{
startTime: 0,
executionTime: 0,
error,
source: [],
},
],
},
lastNodeExecuted: node.name,
},
executionData: {
contextData: {},
metadata: {},
nodeExecutionStack: [
{
node,
data: {},
source: null,
},
],
waitingExecution: {},
waitingExecutionSource: {},
},
},
finished: false,
mode,
startedAt: new Date(),
stoppedAt: new Date(),
status: 'error',
};
}
/** /**
* Returns the data of the last executed node * Returns the data of the last executed node
*/ */

View File

@@ -21,22 +21,21 @@ import { ActiveExecutions } from '@/active-executions';
import config from '@/config'; import config from '@/config';
import { ExecutionRepository } from '@/databases/repositories/execution.repository'; import { ExecutionRepository } from '@/databases/repositories/execution.repository';
import { ExecutionNotFoundError } from '@/errors/execution-not-found-error'; import { ExecutionNotFoundError } from '@/errors/execution-not-found-error';
import { MaxStalledCountError } from '@/errors/max-stalled-count.error';
import { import {
getLifecycleHooksForRegularMain, getLifecycleHooksForRegularMain,
getLifecycleHooksForScalingWorker, getLifecycleHooksForScalingWorker,
getLifecycleHooksForScalingMain, getLifecycleHooksForScalingMain,
} from '@/execution-lifecycle/execution-lifecycle-hooks'; } from '@/execution-lifecycle/execution-lifecycle-hooks';
import { ExecutionDataService } from '@/executions/execution-data.service';
import { CredentialsPermissionChecker } from '@/executions/pre-execution-checks';
import { ManualExecutionService } from '@/manual-execution.service'; import { ManualExecutionService } from '@/manual-execution.service';
import { NodeTypes } from '@/node-types'; import { NodeTypes } from '@/node-types';
import type { ScalingService } from '@/scaling/scaling.service'; import type { ScalingService } from '@/scaling/scaling.service';
import type { Job, JobData } from '@/scaling/scaling.types'; import type { Job, JobData } from '@/scaling/scaling.types';
import { PermissionChecker } from '@/user-management/permission-checker';
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data'; import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
import { generateFailedExecutionFromError } from '@/workflow-helpers';
import { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service'; import { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service';
import { MaxStalledCountError } from './errors/max-stalled-count.error';
@Service() @Service()
export class WorkflowRunner { export class WorkflowRunner {
private scalingService: ScalingService; private scalingService: ScalingService;
@@ -50,9 +49,10 @@ export class WorkflowRunner {
private readonly executionRepository: ExecutionRepository, private readonly executionRepository: ExecutionRepository,
private readonly workflowStaticDataService: WorkflowStaticDataService, private readonly workflowStaticDataService: WorkflowStaticDataService,
private readonly nodeTypes: NodeTypes, private readonly nodeTypes: NodeTypes,
private readonly permissionChecker: PermissionChecker, private readonly credentialsPermissionChecker: CredentialsPermissionChecker,
private readonly instanceSettings: InstanceSettings, private readonly instanceSettings: InstanceSettings,
private readonly manualExecutionService: ManualExecutionService, private readonly manualExecutionService: ManualExecutionService,
private readonly executionDataService: ExecutionDataService,
) {} ) {}
/** The process did error */ /** The process did error */
@@ -134,10 +134,14 @@ export class WorkflowRunner {
const { id: workflowId, nodes } = data.workflowData; const { id: workflowId, nodes } = data.workflowData;
try { try {
await this.permissionChecker.check(workflowId, nodes); await this.credentialsPermissionChecker.check(workflowId, nodes);
} catch (error) { } catch (error) {
// Create a failed execution with the data for the node, save it and abort execution // Create a failed execution with the data for the node, save it and abort execution
const runData = generateFailedExecutionFromError(data.executionMode, error, error.node); const runData = this.executionDataService.generateFailedExecutionFromError(
data.executionMode,
error,
error.node,
);
const lifecycleHooks = getLifecycleHooksForRegularMain(data, executionId); const lifecycleHooks = getLifecycleHooksForRegularMain(data, executionId);
await lifecycleHooks.runHook('workflowExecuteBefore', [undefined, data.executionData]); await lifecycleHooks.runHook('workflowExecuteBefore', [undefined, data.executionData]);
await lifecycleHooks.runHook('workflowExecuteAfter', [runData]); await lifecycleHooks.runHook('workflowExecuteAfter', [runData]);

View File

@@ -65,6 +65,7 @@ describe('WorkflowExecutionService', () => {
workflowRunner, workflowRunner,
mock(), mock(),
mock(), mock(),
mock(),
); );
const additionalData = mock<IWorkflowExecuteAdditionalData>({}); const additionalData = mock<IWorkflowExecuteAdditionalData>({});

View File

@@ -21,12 +21,12 @@ import type { Project } from '@/databases/entities/project';
import type { User } from '@/databases/entities/user'; import type { User } from '@/databases/entities/user';
import { ExecutionRepository } from '@/databases/repositories/execution.repository'; import { ExecutionRepository } from '@/databases/repositories/execution.repository';
import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository';
import { ExecutionDataService } from '@/executions/execution-data.service';
import { SubworkflowPolicyChecker } from '@/executions/pre-execution-checks';
import type { CreateExecutionPayload, IWorkflowErrorData } from '@/interfaces'; import type { CreateExecutionPayload, IWorkflowErrorData } from '@/interfaces';
import { NodeTypes } from '@/node-types'; import { NodeTypes } from '@/node-types';
import { SubworkflowPolicyChecker } from '@/subworkflows/subworkflow-policy-checker.service';
import { TestWebhooks } from '@/webhooks/test-webhooks'; import { TestWebhooks } from '@/webhooks/test-webhooks';
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data'; import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
import * as WorkflowHelpers from '@/workflow-helpers';
import { WorkflowRunner } from '@/workflow-runner'; import { WorkflowRunner } from '@/workflow-runner';
import type { WorkflowRequest } from '@/workflows/workflow.request'; import type { WorkflowRequest } from '@/workflows/workflow.request';
@@ -42,6 +42,7 @@ export class WorkflowExecutionService {
private readonly workflowRunner: WorkflowRunner, private readonly workflowRunner: WorkflowRunner,
private readonly globalConfig: GlobalConfig, private readonly globalConfig: GlobalConfig,
private readonly subworkflowPolicyChecker: SubworkflowPolicyChecker, private readonly subworkflowPolicyChecker: SubworkflowPolicyChecker,
private readonly executionDataService: ExecutionDataService,
) {} ) {}
async runWorkflow( async runWorkflow(
@@ -273,7 +274,7 @@ export class WorkflowExecutionService {
); );
// Create a fake execution and save it to DB. // Create a fake execution and save it to DB.
const fakeExecution = WorkflowHelpers.generateFailedExecutionFromError( const fakeExecution = this.executionDataService.generateFailedExecutionFromError(
'error', 'error',
errorWorkflowPermissionError, errorWorkflowPermissionError,
initialNode, initialNode,

View File

@@ -9,20 +9,19 @@ import { ProjectRepository } from '@/databases/repositories/project.repository';
import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository';
import { SharedWorkflowRepository } from '@/databases/repositories/shared-workflow.repository'; import { SharedWorkflowRepository } from '@/databases/repositories/shared-workflow.repository';
import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository';
import { CredentialsPermissionChecker } from '@/executions/pre-execution-checks';
import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
import { NodeTypes } from '@/node-types'; import { NodeTypes } from '@/node-types';
import { OwnershipService } from '@/services/ownership.service'; import { OwnershipService } from '@/services/ownership.service';
import { PermissionChecker } from '@/user-management/permission-checker'; import { mockInstance } from '@test/mocking';
import { affixRoleToSaveCredential } from '@test-integration/db/credentials';
import { getPersonalProject } from '@test-integration/db/projects';
import { createOwner, createUser } from '@test-integration/db/users';
import { randomCredentialPayload as randomCred } from '@test-integration/random';
import * as testDb from '@test-integration/test-db';
import type { SaveCredentialFunction } from '@test-integration/types';
import { mockNodeTypesData } from '@test-integration/utils/node-types-data'; import { mockNodeTypesData } from '@test-integration/utils/node-types-data';
import { affixRoleToSaveCredential } from './shared/db/credentials';
import { getPersonalProject } from './shared/db/projects';
import { createOwner, createUser } from './shared/db/users';
import { randomCredentialPayload as randomCred } from './shared/random';
import * as testDb from './shared/test-db';
import type { SaveCredentialFunction } from './shared/types';
import { mockInstance } from '../shared/mocking';
const ownershipService = mockInstance(OwnershipService); const ownershipService = mockInstance(OwnershipService);
const createWorkflow = async (nodes: INode[], workflowOwner?: User): Promise<IWorkflowBase> => { const createWorkflow = async (nodes: INode[], workflowOwner?: User): Promise<IWorkflowBase> => {
@@ -62,14 +61,14 @@ mockInstance(LoadNodesAndCredentials, {
loadedNodes: mockNodeTypesData(['start', 'actionNetwork']), loadedNodes: mockNodeTypesData(['start', 'actionNetwork']),
}); });
let permissionChecker: PermissionChecker; let permissionChecker: CredentialsPermissionChecker;
beforeAll(async () => { beforeAll(async () => {
await testDb.init(); await testDb.init();
saveCredential = affixRoleToSaveCredential('credential:owner'); saveCredential = affixRoleToSaveCredential('credential:owner');
permissionChecker = Container.get(PermissionChecker); permissionChecker = Container.get(CredentialsPermissionChecker);
[owner, member] = await Promise.all([createOwner(), createUser()]); [owner, member] = await Promise.all([createOwner(), createUser()]);
ownerPersonalProject = await Container.get(ProjectRepository).getPersonalProjectForUserOrFail( ownerPersonalProject = await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(

View File

@@ -1,25 +0,0 @@
import { ExecutionBaseError } from './abstract/execution-base.error';
import type { INode } from '../Interfaces';
export class CredentialAccessError extends ExecutionBaseError {
override readonly description =
'Please recreate the credential or ask its owner to share it with you.';
override readonly level = 'warning';
constructor(
readonly node: INode,
credentialId: string,
workflowId: string,
) {
super('Node has no access to credential', {
tags: {
nodeType: node.type,
},
extra: {
credentialId,
workflowId,
},
});
}
}

View File

@@ -5,7 +5,6 @@ export { UnexpectedError, type UnexpectedErrorOptions } from './base/unexpected.
export { UserError, type UserErrorOptions } from './base/user.error'; export { UserError, type UserErrorOptions } from './base/user.error';
export { ApplicationError } from './application.error'; export { ApplicationError } from './application.error';
export { ExpressionError } from './expression.error'; export { ExpressionError } from './expression.error';
export { CredentialAccessError } from './credential-access-error';
export { ExecutionCancelledError } from './execution-cancelled.error'; export { ExecutionCancelledError } from './execution-cancelled.error';
export { NodeApiError } from './node-api.error'; export { NodeApiError } from './node-api.error';
export { NodeOperationError } from './node-operation.error'; export { NodeOperationError } from './node-operation.error';