[permissions] permissions and workflows (#12436)
In this PR - Determine object record permissions on workflows objects (workflow, workflowVersion, workflowRun) base on settings permissions @Weiko - Add Workflow permission guards on workflow resolvers @thomtrp . **Any method within a resolver that has the SettingsPermission Guard is only callable by a apiKey or a user that has the permission** (so not by external parties). - Add checks bypass in workflow services since 1) for actions gated by settings permissions, the gate should be done at resolver level, so it will have been done before the call to the service 2) some service methods may be called by workflowTriggerController which is callable by external parties without permissions (ex: workflowCommonWorkspaceService.getWorkflowVersionOrFail). This is something we may want to change in the future (still to discuss), by removing the guard at resolver-level and relying on shouldBypassPermissionChecks at getRepository and made in a way that we only bypass for external parties. - Add checks bypass for actions performed by workflows since they should not be restricted in our current vision - Add tests
This commit is contained in:
+8
-5
@@ -11,7 +11,7 @@ import { RecordInputTransformerService } from 'src/engine/core-modules/record-tr
|
||||
import { FieldActorSource } from 'src/engine/metadata-modules/field-metadata/composite-types/actor.composite-type';
|
||||
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
|
||||
import { ScopedWorkspaceContextFactory } from 'src/engine/twenty-orm/factories/scoped-workspace-context.factory';
|
||||
import { TwentyORMManager } from 'src/engine/twenty-orm/twenty-orm.manager';
|
||||
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
|
||||
import { WorkspaceEventEmitter } from 'src/engine/workspace-event-emitter/workspace-event-emitter';
|
||||
import { WorkflowCommonWorkspaceService } from 'src/modules/workflow/common/workspace-services/workflow-common.workspace-service';
|
||||
import {
|
||||
@@ -31,7 +31,7 @@ import { WorkflowCreateRecordActionInput } from 'src/modules/workflow/workflow-e
|
||||
@Injectable()
|
||||
export class CreateRecordWorkflowAction implements WorkflowExecutor {
|
||||
constructor(
|
||||
private readonly twentyORMManager: TwentyORMManager,
|
||||
private readonly twentyORMGlobalManager: TwentyORMGlobalManager,
|
||||
@InjectRepository(ObjectMetadataEntity, 'core')
|
||||
private readonly objectMetadataRepository: Repository<ObjectMetadataEntity>,
|
||||
private readonly workspaceEventEmitter: WorkspaceEventEmitter,
|
||||
@@ -76,9 +76,12 @@ export class CreateRecordWorkflowAction implements WorkflowExecutor {
|
||||
context,
|
||||
) as WorkflowCreateRecordActionInput;
|
||||
|
||||
const repository = await this.twentyORMManager.getRepository(
|
||||
workflowActionInput.objectName,
|
||||
);
|
||||
const repository =
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
|
||||
workspaceId,
|
||||
workflowActionInput.objectName,
|
||||
{ shouldBypassPermissionChecks: true },
|
||||
);
|
||||
|
||||
const objectMetadata = await this.objectMetadataRepository.findOne({
|
||||
where: {
|
||||
|
||||
+9
-6
@@ -10,7 +10,7 @@ import { WorkflowExecutor } from 'src/modules/workflow/workflow-executor/interfa
|
||||
import { DatabaseEventAction } from 'src/engine/api/graphql/graphql-query-runner/enums/database-event-action';
|
||||
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
|
||||
import { ScopedWorkspaceContextFactory } from 'src/engine/twenty-orm/factories/scoped-workspace-context.factory';
|
||||
import { TwentyORMManager } from 'src/engine/twenty-orm/twenty-orm.manager';
|
||||
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
|
||||
import { WorkspaceEventEmitter } from 'src/engine/workspace-event-emitter/workspace-event-emitter';
|
||||
import {
|
||||
WorkflowStepExecutorException,
|
||||
@@ -29,7 +29,7 @@ import { WorkflowDeleteRecordActionInput } from 'src/modules/workflow/workflow-e
|
||||
@Injectable()
|
||||
export class DeleteRecordWorkflowAction implements WorkflowExecutor {
|
||||
constructor(
|
||||
private readonly twentyORMManager: TwentyORMManager,
|
||||
private readonly twentyORMGlobalManager: TwentyORMGlobalManager,
|
||||
@InjectRepository(ObjectMetadataEntity, 'core')
|
||||
private readonly objectMetadataRepository: Repository<ObjectMetadataEntity>,
|
||||
private readonly workspaceEventEmitter: WorkspaceEventEmitter,
|
||||
@@ -72,10 +72,6 @@ export class DeleteRecordWorkflowAction implements WorkflowExecutor {
|
||||
);
|
||||
}
|
||||
|
||||
const repository = await this.twentyORMManager.getRepository(
|
||||
workflowActionInput.objectName,
|
||||
);
|
||||
|
||||
const workspaceId = this.scopedWorkspaceContextFactory.create().workspaceId;
|
||||
|
||||
if (!workspaceId) {
|
||||
@@ -85,6 +81,13 @@ export class DeleteRecordWorkflowAction implements WorkflowExecutor {
|
||||
);
|
||||
}
|
||||
|
||||
const repository =
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
|
||||
workspaceId,
|
||||
workflowActionInput.objectName,
|
||||
{ shouldBypassPermissionChecks: true },
|
||||
);
|
||||
|
||||
const objectMetadata = await this.objectMetadataRepository.findOne({
|
||||
where: {
|
||||
nameSingular: workflowActionInput.objectName,
|
||||
|
||||
+9
-6
@@ -16,7 +16,7 @@ import { ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/typ
|
||||
import { ObjectMetadataMaps } from 'src/engine/metadata-modules/types/object-metadata-maps';
|
||||
import { ScopedWorkspaceContextFactory } from 'src/engine/twenty-orm/factories/scoped-workspace-context.factory';
|
||||
import { WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository';
|
||||
import { TwentyORMManager } from 'src/engine/twenty-orm/twenty-orm.manager';
|
||||
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
|
||||
import { formatResult } from 'src/engine/twenty-orm/utils/format-result.util';
|
||||
import { WorkflowCommonWorkspaceService } from 'src/modules/workflow/common/workspace-services/workflow-common.workspace-service';
|
||||
import {
|
||||
@@ -36,7 +36,7 @@ import { WorkflowFindRecordsActionInput } from 'src/modules/workflow/workflow-ex
|
||||
@Injectable()
|
||||
export class FindRecordsWorkflowAction implements WorkflowExecutor {
|
||||
constructor(
|
||||
private readonly twentyORMManager: TwentyORMManager,
|
||||
private readonly twentyORMGlobalManager: TwentyORMGlobalManager,
|
||||
private readonly scopedWorkspaceContextFactory: ScopedWorkspaceContextFactory,
|
||||
private readonly workflowCommonWorkspaceService: WorkflowCommonWorkspaceService,
|
||||
) {}
|
||||
@@ -67,10 +67,6 @@ export class FindRecordsWorkflowAction implements WorkflowExecutor {
|
||||
context,
|
||||
) as WorkflowFindRecordsActionInput;
|
||||
|
||||
const repository = await this.twentyORMManager.getRepository(
|
||||
workflowActionInput.objectName,
|
||||
);
|
||||
|
||||
const workspaceId = this.scopedWorkspaceContextFactory.create().workspaceId;
|
||||
|
||||
if (!workspaceId) {
|
||||
@@ -80,6 +76,13 @@ export class FindRecordsWorkflowAction implements WorkflowExecutor {
|
||||
);
|
||||
}
|
||||
|
||||
const repository =
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
|
||||
workspaceId,
|
||||
workflowActionInput.objectName,
|
||||
{ shouldBypassPermissionChecks: true },
|
||||
);
|
||||
|
||||
const { objectMetadataItemWithFieldsMaps, objectMetadataMaps } =
|
||||
await this.workflowCommonWorkspaceService.getObjectMetadataItemWithFieldsMaps(
|
||||
workflowActionInput.objectName,
|
||||
|
||||
+9
-6
@@ -12,7 +12,7 @@ import { objectRecordChangedValues } from 'src/engine/core-modules/event-emitter
|
||||
import { RecordInputTransformerService } from 'src/engine/core-modules/record-transformer/services/record-input-transformer.service';
|
||||
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
|
||||
import { ScopedWorkspaceContextFactory } from 'src/engine/twenty-orm/factories/scoped-workspace-context.factory';
|
||||
import { TwentyORMManager } from 'src/engine/twenty-orm/twenty-orm.manager';
|
||||
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
|
||||
import { formatData } from 'src/engine/twenty-orm/utils/format-data.util';
|
||||
import { WorkspaceEventEmitter } from 'src/engine/workspace-event-emitter/workspace-event-emitter';
|
||||
import { WorkflowCommonWorkspaceService } from 'src/modules/workflow/common/workspace-services/workflow-common.workspace-service';
|
||||
@@ -33,7 +33,7 @@ import { WorkflowUpdateRecordActionInput } from 'src/modules/workflow/workflow-e
|
||||
@Injectable()
|
||||
export class UpdateRecordWorkflowAction implements WorkflowExecutor {
|
||||
constructor(
|
||||
private readonly twentyORMManager: TwentyORMManager,
|
||||
private readonly twentyORMGlobalManager: TwentyORMGlobalManager,
|
||||
private readonly scopedWorkspaceContextFactory: ScopedWorkspaceContextFactory,
|
||||
@InjectRepository(ObjectMetadataEntity, 'core')
|
||||
private readonly objectMetadataRepository: Repository<ObjectMetadataEntity>,
|
||||
@@ -79,10 +79,6 @@ export class UpdateRecordWorkflowAction implements WorkflowExecutor {
|
||||
);
|
||||
}
|
||||
|
||||
const repository = await this.twentyORMManager.getRepository(
|
||||
workflowActionInput.objectName,
|
||||
);
|
||||
|
||||
const workspaceId = this.scopedWorkspaceContextFactory.create().workspaceId;
|
||||
|
||||
if (!workspaceId) {
|
||||
@@ -92,6 +88,13 @@ export class UpdateRecordWorkflowAction implements WorkflowExecutor {
|
||||
);
|
||||
}
|
||||
|
||||
const repository =
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
|
||||
workspaceId,
|
||||
workflowActionInput.objectName,
|
||||
{ shouldBypassPermissionChecks: true },
|
||||
);
|
||||
|
||||
const objectMetadata = await this.objectMetadataRepository.findOne({
|
||||
where: {
|
||||
nameSingular: workflowActionInput.objectName,
|
||||
|
||||
+7
-25
@@ -18,7 +18,6 @@ describe('WorkflowExecutorWorkspaceService', () => {
|
||||
let service: WorkflowExecutorWorkspaceService;
|
||||
let workflowExecutorFactory: WorkflowExecutorFactory;
|
||||
let workspaceEventEmitter: WorkspaceEventEmitter;
|
||||
let scopedWorkspaceContextFactory: ScopedWorkspaceContextFactory;
|
||||
let workflowRunWorkspaceService: WorkflowRunWorkspaceService;
|
||||
|
||||
const mockWorkflowExecutor = {
|
||||
@@ -86,9 +85,6 @@ describe('WorkflowExecutorWorkspaceService', () => {
|
||||
workspaceEventEmitter = module.get<WorkspaceEventEmitter>(
|
||||
WorkspaceEventEmitter,
|
||||
);
|
||||
scopedWorkspaceContextFactory = module.get<ScopedWorkspaceContextFactory>(
|
||||
ScopedWorkspaceContextFactory,
|
||||
);
|
||||
workflowRunWorkspaceService = module.get<WorkflowRunWorkspaceService>(
|
||||
WorkflowRunWorkspaceService,
|
||||
);
|
||||
@@ -185,6 +181,7 @@ describe('WorkflowExecutorWorkspaceService', () => {
|
||||
data: 'some-data',
|
||||
'step-1': { stepOutput: 'success' },
|
||||
},
|
||||
workspaceId: 'workspace-id',
|
||||
});
|
||||
expect(result).toEqual({ result: { success: true } });
|
||||
|
||||
@@ -221,6 +218,7 @@ describe('WorkflowExecutorWorkspaceService', () => {
|
||||
},
|
||||
},
|
||||
context: mockContext,
|
||||
workspaceId: 'workspace-id',
|
||||
});
|
||||
});
|
||||
|
||||
@@ -248,6 +246,7 @@ describe('WorkflowExecutorWorkspaceService', () => {
|
||||
output: mockPendingEvent,
|
||||
},
|
||||
context: mockContext,
|
||||
workspaceId: 'workspace-id',
|
||||
});
|
||||
|
||||
// No recursive call to execute should happen
|
||||
@@ -304,6 +303,7 @@ describe('WorkflowExecutorWorkspaceService', () => {
|
||||
},
|
||||
},
|
||||
context: mockContext,
|
||||
workspaceId: 'workspace-id',
|
||||
});
|
||||
expect(result).toEqual({ result: { success: true } });
|
||||
|
||||
@@ -387,6 +387,7 @@ describe('WorkflowExecutorWorkspaceService', () => {
|
||||
output: errorOutput,
|
||||
},
|
||||
context: mockContext,
|
||||
workspaceId: 'workspace-id',
|
||||
});
|
||||
expect(result).toEqual(errorOutput);
|
||||
});
|
||||
@@ -414,6 +415,7 @@ describe('WorkflowExecutorWorkspaceService', () => {
|
||||
},
|
||||
},
|
||||
context: mockContext,
|
||||
workspaceId: 'workspace-id',
|
||||
});
|
||||
expect(result).toEqual({
|
||||
error: BILLING_WORKFLOW_EXECUTION_ERROR_MESSAGE,
|
||||
@@ -423,9 +425,8 @@ describe('WorkflowExecutorWorkspaceService', () => {
|
||||
|
||||
describe('sendWorkflowNodeRunEvent', () => {
|
||||
it('should emit a billing event', () => {
|
||||
service['sendWorkflowNodeRunEvent']();
|
||||
service['sendWorkflowNodeRunEvent']('workspace-id');
|
||||
|
||||
expect(scopedWorkspaceContextFactory.create).toHaveBeenCalled();
|
||||
expect(workspaceEventEmitter.emitCustomBatchEvent).toHaveBeenCalledWith(
|
||||
BILLING_FEATURE_USED,
|
||||
[
|
||||
@@ -437,24 +438,5 @@ describe('WorkflowExecutorWorkspaceService', () => {
|
||||
'workspace-id',
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle missing workspace ID', () => {
|
||||
mockScopedWorkspaceContextFactory.create.mockReturnValueOnce({
|
||||
workspaceId: null,
|
||||
});
|
||||
|
||||
service['sendWorkflowNodeRunEvent']();
|
||||
|
||||
expect(workspaceEventEmitter.emitCustomBatchEvent).toHaveBeenCalledWith(
|
||||
BILLING_FEATURE_USED,
|
||||
[
|
||||
{
|
||||
eventName: BillingMeterEventName.WORKFLOW_NODE_RUN,
|
||||
value: 1,
|
||||
},
|
||||
],
|
||||
'',
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
+21
-10
@@ -21,6 +21,10 @@ import { WorkflowExecutorFactory } from 'src/modules/workflow/workflow-executor/
|
||||
import { WorkflowExecutorInput } from 'src/modules/workflow/workflow-executor/types/workflow-executor-input';
|
||||
import { WorkflowExecutorOutput } from 'src/modules/workflow/workflow-executor/types/workflow-executor-output.type';
|
||||
import { WorkflowRunWorkspaceService } from 'src/modules/workflow/workflow-runner/workflow-run/workflow-run.workspace-service';
|
||||
import {
|
||||
WorkflowTriggerException,
|
||||
WorkflowTriggerExceptionCode,
|
||||
} from 'src/modules/workflow/workflow-trigger/exceptions/workflow-trigger.exception';
|
||||
|
||||
const MAX_RETRIES_ON_FAILURE = 3;
|
||||
|
||||
@@ -58,15 +62,25 @@ export class WorkflowExecutorWorkspaceService implements WorkflowExecutor {
|
||||
|
||||
let actionOutput: WorkflowExecutorOutput;
|
||||
|
||||
const { workspaceId } = this.scopedWorkspaceContextFactory.create();
|
||||
|
||||
if (!workspaceId) {
|
||||
throw new WorkflowTriggerException(
|
||||
'No workspace id found',
|
||||
WorkflowTriggerExceptionCode.INTERNAL_ERROR,
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
this.billingService.isBillingEnabled() &&
|
||||
!(await this.canBillWorkflowNodeExecution())
|
||||
!(await this.canBillWorkflowNodeExecution(workspaceId))
|
||||
) {
|
||||
const billingOutput = {
|
||||
error: BILLING_WORKFLOW_EXECUTION_ERROR_MESSAGE,
|
||||
};
|
||||
|
||||
await this.workflowRunWorkspaceService.saveWorkflowRunState({
|
||||
workspaceId,
|
||||
workflowRunId,
|
||||
stepOutput: {
|
||||
id: step.id,
|
||||
@@ -93,7 +107,7 @@ export class WorkflowExecutorWorkspaceService implements WorkflowExecutor {
|
||||
}
|
||||
|
||||
if (!actionOutput.error) {
|
||||
this.sendWorkflowNodeRunEvent();
|
||||
this.sendWorkflowNodeRunEvent(workspaceId);
|
||||
}
|
||||
|
||||
const stepOutput: StepOutput = {
|
||||
@@ -106,6 +120,7 @@ export class WorkflowExecutorWorkspaceService implements WorkflowExecutor {
|
||||
workflowRunId,
|
||||
stepOutput,
|
||||
context,
|
||||
workspaceId,
|
||||
});
|
||||
|
||||
return actionOutput;
|
||||
@@ -127,6 +142,7 @@ export class WorkflowExecutorWorkspaceService implements WorkflowExecutor {
|
||||
workflowRunId,
|
||||
stepOutput,
|
||||
context: updatedContext,
|
||||
workspaceId,
|
||||
});
|
||||
|
||||
if (!isDefined(step.nextStepIds?.[0])) {
|
||||
@@ -159,15 +175,13 @@ export class WorkflowExecutorWorkspaceService implements WorkflowExecutor {
|
||||
workflowRunId,
|
||||
stepOutput,
|
||||
context,
|
||||
workspaceId,
|
||||
});
|
||||
|
||||
return actionOutput;
|
||||
}
|
||||
|
||||
private sendWorkflowNodeRunEvent() {
|
||||
const workspaceId =
|
||||
this.scopedWorkspaceContextFactory.create().workspaceId ?? '';
|
||||
|
||||
private sendWorkflowNodeRunEvent(workspaceId: string) {
|
||||
this.workspaceEventEmitter.emitCustomBatchEvent<BillingUsageEvent>(
|
||||
BILLING_FEATURE_USED,
|
||||
[
|
||||
@@ -180,10 +194,7 @@ export class WorkflowExecutorWorkspaceService implements WorkflowExecutor {
|
||||
);
|
||||
}
|
||||
|
||||
private async canBillWorkflowNodeExecution() {
|
||||
const workspaceId =
|
||||
this.scopedWorkspaceContextFactory.create().workspaceId ?? '';
|
||||
|
||||
private async canBillWorkflowNodeExecution(workspaceId: string) {
|
||||
return this.billingService.canBillMeteredProduct(
|
||||
workspaceId,
|
||||
BillingProductKey.WORKFLOW_NODE_EXECUTION,
|
||||
|
||||
Reference in New Issue
Block a user