From b14373eee96f1a36b91f16a07da5bd1b48be13df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 25 Jul 2023 16:02:08 +0200 Subject: [PATCH 01/11] refactor: Set up ownership service --- .../cli/src/services/ownership.service.ts | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 packages/cli/src/services/ownership.service.ts diff --git a/packages/cli/src/services/ownership.service.ts b/packages/cli/src/services/ownership.service.ts new file mode 100644 index 0000000000000..c4bbda635420a --- /dev/null +++ b/packages/cli/src/services/ownership.service.ts @@ -0,0 +1,36 @@ +import Container, { Service } from 'typedi'; +import * as Db from '@/Db'; +import { CacheService } from './cache.service'; +import { RoleRepository } from '@/databases/repositories'; +import type { User } from '@/databases/entities/User'; + +@Service() +export class OwnershipService { + private cacheService = Container.get(CacheService); + + private roleRepository = Container.get(RoleRepository); + + /** + * Retrieve the user who owns the workflow. Note that workflow ownership **cannot** be changed. + * + * @caching + */ + async getWorkflowOwner(workflowId: string) { + const cachedOwner = await this.cacheService.get(`workflow-owner:${workflowId}`); + + if (cachedOwner) return cachedOwner; + + const workflowOwnerRole = await this.roleRepository.findWorkflowOwnerRole(); + + if (!workflowOwnerRole) throw new Error('Failed to find workflow owner role'); + + const sharedWorkflow = await Db.collections.SharedWorkflow.findOneOrFail({ + where: { workflowId, roleId: workflowOwnerRole.id }, + relations: ['user', 'user.globalRole'], + }); + + void this.cacheService.set(`workflow-owner:${workflowId}`, sharedWorkflow.user); + + return sharedWorkflow.user; + } +} From be9b470f51d4e163cb1c232606ebbc2c9df61969 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 25 Jul 2023 16:02:24 +0200 Subject: [PATCH 02/11] refactor: Specify cache keys and values --- packages/cli/src/services/cache.service.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/cli/src/services/cache.service.ts b/packages/cli/src/services/cache.service.ts index 64d203106c0a9..0b033b35bd59f 100644 --- a/packages/cli/src/services/cache.service.ts +++ b/packages/cli/src/services/cache.service.ts @@ -9,6 +9,10 @@ import { LoggerProxy, jsonStringify } from 'n8n-workflow'; @Service() export class CacheService { + /** + * Keys and values: + * - `'workflow-owner:${workflowId}'`: `User` + */ private cache: RedisCache | MemoryCache | undefined; async init() { From f9061f47a3cf07b639b23cd01c4d753d169cc836 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 25 Jul 2023 16:02:47 +0200 Subject: [PATCH 03/11] refactor: Replace util with service calls --- .../cli/src/UserManagement/PermissionChecker.ts | 6 ++++-- .../cli/src/UserManagement/UserManagementHelper.ts | 11 ----------- packages/cli/src/WaitTracker.ts | 5 +++-- packages/cli/src/WaitingWebhooks.ts | 8 ++++---- packages/cli/src/WebhookHelpers.ts | 4 ++-- packages/cli/src/WorkflowExecuteAdditionalData.ts | 14 +++++++++----- packages/cli/src/commands/worker.ts | 4 ++-- packages/cli/src/services/events.service.ts | 8 ++++---- 8 files changed, 28 insertions(+), 32 deletions(-) diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts index b8b851419aec0..16582e87fbc0e 100644 --- a/packages/cli/src/UserManagement/PermissionChecker.ts +++ b/packages/cli/src/UserManagement/PermissionChecker.ts @@ -9,9 +9,11 @@ import { In } from 'typeorm'; import * as Db from '@/Db'; import config from '@/config'; import type { SharedCredentials } from '@db/entities/SharedCredentials'; -import { getRoleId, getWorkflowOwner, isSharingEnabled } from './UserManagementHelper'; +import { getRoleId, isSharingEnabled } from './UserManagementHelper'; import { WorkflowsService } from '@/workflows/workflows.services'; import { UserService } from '@/user/user.service'; +import { OwnershipService } from '@/services/ownership.service'; +import Container from 'typedi'; export class PermissionChecker { /** @@ -101,7 +103,7 @@ export class PermissionChecker { policy = 'workflowsFromSameOwner'; } - const subworkflowOwner = await getWorkflowOwner(subworkflow.id); + const subworkflowOwner = await Container.get(OwnershipService).getWorkflowOwner(subworkflow.id); const errorToThrow = new SubworkflowOperationError( `Target workflow ID ${subworkflow.id ?? ''} may not be called`, diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index 6b23f1207ce6d..2610554fb27ef 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -16,17 +16,6 @@ import { License } from '@/License'; import { getWebhookBaseUrl } from '@/WebhookHelpers'; import type { PostHogClient } from '@/posthog'; -export async function getWorkflowOwner(workflowId: string): Promise { - const workflowOwnerRole = await Container.get(RoleRepository).findWorkflowOwnerRole(); - - const sharedWorkflow = await Db.collections.SharedWorkflow.findOneOrFail({ - where: { workflowId, roleId: workflowOwnerRole?.id ?? undefined }, - relations: ['user', 'user.globalRole'], - }); - - return sharedWorkflow.user; -} - export function isEmailSetUp(): boolean { const smtp = config.getEnv('userManagement.emails.mode') === 'smtp'; const host = !!config.getEnv('userManagement.emails.smtp.host'); diff --git a/packages/cli/src/WaitTracker.ts b/packages/cli/src/WaitTracker.ts index 8af5074828f7b..4a96084fad726 100644 --- a/packages/cli/src/WaitTracker.ts +++ b/packages/cli/src/WaitTracker.ts @@ -21,10 +21,10 @@ import type { IWorkflowExecutionDataProcess, } from '@/Interfaces'; import { WorkflowRunner } from '@/WorkflowRunner'; -import { getWorkflowOwner } from '@/UserManagement/UserManagementHelper'; import { recoverExecutionDataFromEventLogMessages } from './eventbus/MessageEventBus/recoverEvents'; import { ExecutionRepository } from '@db/repositories'; import type { ExecutionEntity } from '@db/entities/ExecutionEntity'; +import { OwnershipService } from './services/ownership.service'; @Service() export class WaitTracker { @@ -186,7 +186,8 @@ export class WaitTracker { if (!fullExecutionData.workflowData.id) { throw new Error('Only saved workflows can be resumed.'); } - const user = await getWorkflowOwner(fullExecutionData.workflowData.id); + const workflowId = fullExecutionData.workflowData.id; + const user = await Container.get(OwnershipService).getWorkflowOwner(workflowId); const data: IWorkflowExecutionDataProcess = { executionMode: fullExecutionData.mode, diff --git a/packages/cli/src/WaitingWebhooks.ts b/packages/cli/src/WaitingWebhooks.ts index 7d66fda525f34..8a53695527e31 100644 --- a/packages/cli/src/WaitingWebhooks.ts +++ b/packages/cli/src/WaitingWebhooks.ts @@ -3,7 +3,7 @@ /* eslint-disable no-param-reassign */ import type { INode, WebhookHttpMethod } from 'n8n-workflow'; import { NodeHelpers, Workflow, LoggerProxy as Logger } from 'n8n-workflow'; -import { Service } from 'typedi'; +import Container, { Service } from 'typedi'; import type express from 'express'; import * as ResponseHelper from '@/ResponseHelper'; @@ -11,8 +11,8 @@ import * as WebhookHelpers from '@/WebhookHelpers'; import { NodeTypes } from '@/NodeTypes'; import type { IExecutionResponse, IResponseCallbackData, IWorkflowDb } from '@/Interfaces'; import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; -import { getWorkflowOwner } from '@/UserManagement/UserManagementHelper'; import { ExecutionRepository } from '@db/repositories'; +import { OwnershipService } from './services/ownership.service'; @Service() export class WaitingWebhooks { @@ -83,7 +83,7 @@ export class WaitingWebhooks { const { workflowData } = fullExecutionData; const workflow = new Workflow({ - id: workflowData.id!.toString(), + id: workflowData.id!, name: workflowData.name, nodes: workflowData.nodes, connections: workflowData.connections, @@ -95,7 +95,7 @@ export class WaitingWebhooks { let workflowOwner; try { - workflowOwner = await getWorkflowOwner(workflowData.id!.toString()); + workflowOwner = await Container.get(OwnershipService).getWorkflowOwner(workflowData.id!); } catch (error) { throw new ResponseHelper.NotFoundError('Could not find workflow'); } diff --git a/packages/cli/src/WebhookHelpers.ts b/packages/cli/src/WebhookHelpers.ts index c286d12a9ecc6..91e166a122596 100644 --- a/packages/cli/src/WebhookHelpers.ts +++ b/packages/cli/src/WebhookHelpers.ts @@ -60,8 +60,8 @@ import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData' import { ActiveExecutions } from '@/ActiveExecutions'; import type { User } from '@db/entities/User'; import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; -import { getWorkflowOwner } from '@/UserManagement/UserManagementHelper'; import { EventsService } from '@/services/events.service'; +import { OwnershipService } from './services/ownership.service'; const pipeline = promisify(stream.pipeline); @@ -177,7 +177,7 @@ export async function executeWebhook( user = (workflowData as WorkflowEntity).shared[0].user; } else { try { - user = await getWorkflowOwner(workflowData.id); + user = await Container.get(OwnershipService).getWorkflowOwner(workflowData.id); } catch (error) { throw new ResponseHelper.NotFoundError('Cannot find workflow'); } diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 448a8dc3fafad..a89701cd67884 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -64,7 +64,6 @@ import { NodeTypes } from '@/NodeTypes'; import { Push } from '@/push'; import * as WebhookHelpers from '@/WebhookHelpers'; import * as WorkflowHelpers from '@/WorkflowHelpers'; -import { getWorkflowOwner } from '@/UserManagement/UserManagementHelper'; import { findSubworkflowStart, isWorkflowIdValid } from '@/utils'; import { PermissionChecker } from './UserManagement/PermissionChecker'; import { WorkflowsService } from './workflows/workflows.services'; @@ -72,6 +71,7 @@ import { InternalHooks } from '@/InternalHooks'; import type { ExecutionMetadata } from '@db/entities/ExecutionMetadata'; import { ExecutionRepository } from '@db/repositories'; import { EventsService } from '@/services/events.service'; +import { OwnershipService } from './services/ownership.service'; const ERROR_TRIGGER_TYPE = config.getEnv('nodes.errorTriggerType'); @@ -152,7 +152,9 @@ export function executeErrorWorkflow( // make sure there are no possible security gaps return; } - getWorkflowOwner(workflowId) + + Container.get(OwnershipService) + .getWorkflowOwner(workflowId) .then((user) => { void WorkflowHelpers.executeErrorWorkflow(errorWorkflow, workflowErrorData, user); }) @@ -175,9 +177,11 @@ export function executeErrorWorkflow( workflowData.nodes.some((node) => node.type === ERROR_TRIGGER_TYPE) ) { Logger.verbose('Start internal error workflow', { executionId, workflowId }); - void getWorkflowOwner(workflowId).then((user) => { - void WorkflowHelpers.executeErrorWorkflow(workflowId, workflowErrorData, user); - }); + void Container.get(OwnershipService) + .getWorkflowOwner(workflowId) + .then((user) => { + void WorkflowHelpers.executeErrorWorkflow(workflowId, workflowErrorData, user); + }); } } } diff --git a/packages/cli/src/commands/worker.ts b/packages/cli/src/commands/worker.ts index e50fa29118692..c5f51fed8f1d7 100644 --- a/packages/cli/src/commands/worker.ts +++ b/packages/cli/src/commands/worker.ts @@ -18,11 +18,11 @@ import { PermissionChecker } from '@/UserManagement/PermissionChecker'; import config from '@/config'; import type { Job, JobId, JobQueue, JobResponse, WebhookResponse } from '@/Queue'; import { Queue } from '@/Queue'; -import { getWorkflowOwner } from '@/UserManagement/UserManagementHelper'; import { generateFailedExecutionFromError } from '@/WorkflowHelpers'; import { N8N_VERSION } from '@/constants'; import { BaseCommand } from './BaseCommand'; import { ExecutionRepository } from '@db/repositories'; +import { OwnershipService } from '@/services/ownership.service'; export class Worker extends BaseCommand { static description = '\nStarts a n8n worker'; @@ -112,7 +112,7 @@ export class Worker extends BaseCommand { `Start job: ${job.id} (Workflow ID: ${workflowId} | Execution: ${executionId})`, ); - const workflowOwner = await getWorkflowOwner(workflowId); + const workflowOwner = await Container.get(OwnershipService).getWorkflowOwner(workflowId); let { staticData } = fullExecutionData.workflowData; if (loadStaticData) { diff --git a/packages/cli/src/services/events.service.ts b/packages/cli/src/services/events.service.ts index ec288fa23bb21..be99d842eecd1 100644 --- a/packages/cli/src/services/events.service.ts +++ b/packages/cli/src/services/events.service.ts @@ -1,11 +1,11 @@ import { EventEmitter } from 'events'; -import { Service } from 'typedi'; +import Container, { Service } from 'typedi'; import type { INode, IRun, IWorkflowBase } from 'n8n-workflow'; import { LoggerProxy } from 'n8n-workflow'; import { StatisticsNames } from '@db/entities/WorkflowStatistics'; import { WorkflowStatisticsRepository } from '@db/repositories'; -import { getWorkflowOwner } from '@/UserManagement/UserManagementHelper'; import { UserService } from '@/user/user.service'; +import { OwnershipService } from './ownership.service'; @Service() export class EventsService extends EventEmitter { @@ -41,7 +41,7 @@ export class EventsService extends EventEmitter { const upsertResult = await this.repository.upsertWorkflowStatistics(name, workflowId); if (name === 'production_success' && upsertResult === 'insert') { - const owner = await getWorkflowOwner(workflowId); + const owner = await Container.get(OwnershipService).getWorkflowOwner(workflowId); const metrics = { user_id: owner.id, workflow_id: workflowId, @@ -72,7 +72,7 @@ export class EventsService extends EventEmitter { if (insertResult === 'failed') return; // Compile the metrics since this was a new data loaded event - const owner = await getWorkflowOwner(workflowId); + const owner = await Container.get(OwnershipService).getWorkflowOwner(workflowId); let metrics = { user_id: owner.id, From 6a6021db8ed30a1e5885b6baa382f6c2c1e9afd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 25 Jul 2023 16:02:55 +0200 Subject: [PATCH 04/11] test: Mock service in tests --- packages/cli/test/unit/PermissionChecker.test.ts | 14 ++++++++------ .../cli/test/unit/services/events.service.test.ts | 6 ++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/cli/test/unit/PermissionChecker.test.ts b/packages/cli/test/unit/PermissionChecker.test.ts index be12573b1deae..826044946c5fa 100644 --- a/packages/cli/test/unit/PermissionChecker.test.ts +++ b/packages/cli/test/unit/PermissionChecker.test.ts @@ -23,6 +23,7 @@ import * as testDb from '../integration/shared/testDb'; import { mockNodeTypesData } from './Helpers'; import type { SaveCredentialFunction } from '../integration/shared/types'; import { mockInstance } from '../integration/shared/utils/'; +import { OwnershipService } from '@/services/ownership.service'; let mockNodeTypes: INodeTypes; let credentialOwnerRole: Role; @@ -221,6 +222,7 @@ describe('PermissionChecker.checkSubworkflowExecutePolicy', () => { const userId = uuid(); const fakeUser = new User(); fakeUser.id = userId; + const ownershipService = mockInstance(OwnershipService); const ownerMockRole = new Role(); ownerMockRole.name = 'owner'; @@ -234,7 +236,7 @@ describe('PermissionChecker.checkSubworkflowExecutePolicy', () => { test('sets default policy from environment when subworkflow has none', async () => { config.set('workflows.callerPolicyDefaultOption', 'none'); - jest.spyOn(UserManagementHelper, 'getWorkflowOwner').mockImplementation(async (workflowId) => { + jest.spyOn(ownershipService, 'getWorkflowOwner').mockImplementation(async (workflowId) => { return fakeUser; }); jest.spyOn(UserManagementHelper, 'isSharingEnabled').mockReturnValue(true); @@ -253,7 +255,7 @@ describe('PermissionChecker.checkSubworkflowExecutePolicy', () => { test('if sharing is disabled, ensures that workflows are owner by same user', async () => { jest - .spyOn(UserManagementHelper, 'getWorkflowOwner') + .spyOn(ownershipService, 'getWorkflowOwner') .mockImplementation(async (workflowId) => fakeUser); jest.spyOn(UserManagementHelper, 'isSharingEnabled').mockReturnValue(false); jest.spyOn(UserService, 'get').mockImplementation(async () => fakeUser); @@ -287,7 +289,7 @@ describe('PermissionChecker.checkSubworkflowExecutePolicy', () => { test('list of ids must include the parent workflow id', async () => { const invalidParentWorkflowId = uuid(); jest - .spyOn(UserManagementHelper, 'getWorkflowOwner') + .spyOn(ownershipService, 'getWorkflowOwner') .mockImplementation(async (workflowId) => fakeUser); jest.spyOn(UserManagementHelper, 'isSharingEnabled').mockReturnValue(true); jest.spyOn(UserService, 'get').mockImplementation(async () => fakeUser); @@ -313,7 +315,7 @@ describe('PermissionChecker.checkSubworkflowExecutePolicy', () => { test('sameOwner passes when both workflows are owned by the same user', async () => { jest - .spyOn(UserManagementHelper, 'getWorkflowOwner') + .spyOn(ownershipService, 'getWorkflowOwner') .mockImplementation(async (workflowId) => fakeUser); jest.spyOn(UserManagementHelper, 'isSharingEnabled').mockReturnValue(false); jest.spyOn(UserService, 'get').mockImplementation(async () => fakeUser); @@ -336,7 +338,7 @@ describe('PermissionChecker.checkSubworkflowExecutePolicy', () => { test('workflowsFromAList works when the list contains the parent id', async () => { const workflowId = uuid(); jest - .spyOn(UserManagementHelper, 'getWorkflowOwner') + .spyOn(ownershipService, 'getWorkflowOwner') .mockImplementation(async (workflowId) => fakeUser); jest.spyOn(UserManagementHelper, 'isSharingEnabled').mockReturnValue(true); jest.spyOn(UserService, 'get').mockImplementation(async () => fakeUser); @@ -362,7 +364,7 @@ describe('PermissionChecker.checkSubworkflowExecutePolicy', () => { test('should not throw when workflow policy is set to any', async () => { jest - .spyOn(UserManagementHelper, 'getWorkflowOwner') + .spyOn(ownershipService, 'getWorkflowOwner') .mockImplementation(async (workflowId) => fakeUser); jest.spyOn(UserManagementHelper, 'isSharingEnabled').mockReturnValue(true); jest.spyOn(UserService, 'get').mockImplementation(async () => fakeUser); diff --git a/packages/cli/test/unit/services/events.service.test.ts b/packages/cli/test/unit/services/events.service.test.ts index 027e180cebf7d..3186c8f7b2c16 100644 --- a/packages/cli/test/unit/services/events.service.test.ts +++ b/packages/cli/test/unit/services/events.service.test.ts @@ -15,13 +15,15 @@ import type { WorkflowStatistics } from '@db/entities/WorkflowStatistics'; import { WorkflowStatisticsRepository } from '@db/repositories'; import { EventsService } from '@/services/events.service'; import { UserService } from '@/user/user.service'; -import { getWorkflowOwner } from '@/UserManagement/UserManagementHelper'; +import { OwnershipService } from '@/services/ownership.service'; +import { mockInstance } from '../../integration/shared/utils'; jest.mock('@/UserManagement/UserManagementHelper', () => ({ getWorkflowOwner: jest.fn() })); describe('EventsService', () => { const dbType = config.getEnv('database.type'); const fakeUser = mock({ id: 'abcde-fghij' }); + const ownershipService = mockInstance(OwnershipService); const entityManager = mock(); const dataSource = mock({ @@ -36,7 +38,7 @@ describe('EventsService', () => { LoggerProxy.init(mock()); config.set('diagnostics.enabled', true); config.set('deployment.type', 'n8n-testing'); - mocked(getWorkflowOwner).mockResolvedValue(fakeUser); + mocked(ownershipService.getWorkflowOwner).mockResolvedValue(fakeUser); const updateUserSettingsMock = jest.spyOn(UserService, 'updateUserSettings').mockImplementation(); const eventsService = new EventsService(new WorkflowStatisticsRepository(dataSource)); From ae2beb526acdf68fb00c402930617a140fe5ec3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 25 Jul 2023 18:26:12 +0200 Subject: [PATCH 05/11] refactor: Use dependency injection --- packages/cli/src/services/ownership.service.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/services/ownership.service.ts b/packages/cli/src/services/ownership.service.ts index c4bbda635420a..3df4e46f541d4 100644 --- a/packages/cli/src/services/ownership.service.ts +++ b/packages/cli/src/services/ownership.service.ts @@ -1,14 +1,15 @@ -import Container, { Service } from 'typedi'; -import * as Db from '@/Db'; +import { Service } from 'typedi'; import { CacheService } from './cache.service'; -import { RoleRepository } from '@/databases/repositories'; +import { RoleRepository, SharedWorkflowRepository } from '@/databases/repositories'; import type { User } from '@/databases/entities/User'; @Service() export class OwnershipService { - private cacheService = Container.get(CacheService); - - private roleRepository = Container.get(RoleRepository); + constructor( + private cacheService: CacheService, + private roleRepository: RoleRepository, + private sharedWorkflowRepository: SharedWorkflowRepository, + ) {} /** * Retrieve the user who owns the workflow. Note that workflow ownership **cannot** be changed. @@ -24,7 +25,7 @@ export class OwnershipService { if (!workflowOwnerRole) throw new Error('Failed to find workflow owner role'); - const sharedWorkflow = await Db.collections.SharedWorkflow.findOneOrFail({ + const sharedWorkflow = await this.sharedWorkflowRepository.findOneOrFail({ where: { workflowId, roleId: workflowOwnerRole.id }, relations: ['user', 'user.globalRole'], }); From 0c6cd67f9157415691796f574e6e460c6cdae15b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 25 Jul 2023 18:26:21 +0200 Subject: [PATCH 06/11] test: Write tests --- .../unit/services/ownership.service.test.ts | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 packages/cli/test/unit/services/ownership.service.test.ts diff --git a/packages/cli/test/unit/services/ownership.service.test.ts b/packages/cli/test/unit/services/ownership.service.test.ts new file mode 100644 index 0000000000000..d8a706588563f --- /dev/null +++ b/packages/cli/test/unit/services/ownership.service.test.ts @@ -0,0 +1,66 @@ +import { OwnershipService } from '@/services/ownership.service'; +import { RoleRepository, SharedWorkflowRepository } from '@/databases/repositories'; +import { mockInstance } from '../../integration/shared/utils'; +import { Role } from '@/databases/entities/Role'; +import { randomInteger } from '../../integration/shared/random'; +import { SharedWorkflow } from '@/databases/entities/SharedWorkflow'; +import { CacheService } from '@/services/cache.service'; +import { User } from '@/databases/entities/User'; + +const wfOwnerRole = () => + Object.assign(new Role(), { + scope: 'workflow', + name: 'owner', + id: randomInteger(), + }); + +describe('OwnershipService', () => { + const cacheService = mockInstance(CacheService); + const roleRepository = mockInstance(RoleRepository); + const sharedWorkflowRepository = mockInstance(SharedWorkflowRepository); + + const ownershipService = new OwnershipService( + cacheService, + roleRepository, + sharedWorkflowRepository, + ); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('getWorkflowOwner()', () => { + test('should retrieve a workflow owner', async () => { + roleRepository.findWorkflowOwnerRole.mockResolvedValueOnce(wfOwnerRole()); + + const mockOwner = new User(); + const mockNonOwner = new User(); + + const sharedWorkflow = Object.assign(new SharedWorkflow(), { + role: new Role(), + user: mockOwner, + }); + + sharedWorkflowRepository.findOneOrFail.mockResolvedValueOnce(sharedWorkflow); + + const returnedOwner = await ownershipService.getWorkflowOwner('some-workflow-id'); + + expect(returnedOwner).toBe(mockOwner); + expect(returnedOwner).not.toBe(mockNonOwner); + }); + + test('should throw if no workflow owner role found', async () => { + roleRepository.findWorkflowOwnerRole.mockRejectedValueOnce(new Error()); + + await expect(ownershipService.getWorkflowOwner('some-workflow-id')).rejects.toThrow(); + }); + + test('should throw if no workflow owner found', async () => { + roleRepository.findWorkflowOwnerRole.mockResolvedValueOnce(wfOwnerRole()); + + sharedWorkflowRepository.findOneOrFail.mockRejectedValue(new Error()); + + await expect(ownershipService.getWorkflowOwner('some-workflow-id')).rejects.toThrow(); + }); + }); +}); From 207ac24591ecfb88bcf78b701dd13527d5a71a72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 26 Jul 2023 11:13:49 +0200 Subject: [PATCH 07/11] refactor: Apply feedback from Omar and Micha --- packages/cli/src/UserManagement/PermissionChecker.ts | 4 +++- packages/cli/src/WaitTracker.ts | 2 +- packages/cli/src/WaitingWebhooks.ts | 4 +++- packages/cli/src/WebhookHelpers.ts | 2 +- packages/cli/src/WorkflowExecuteAdditionalData.ts | 4 ++-- packages/cli/src/commands/worker.ts | 2 +- packages/cli/src/services/cache.service.ts | 2 +- packages/cli/src/services/events.service.ts | 4 ++-- packages/cli/src/services/ownership.service.ts | 8 +++----- packages/cli/test/unit/services/ownership.service.test.ts | 6 +++--- 10 files changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts index 16582e87fbc0e..8ea5080ea0224 100644 --- a/packages/cli/src/UserManagement/PermissionChecker.ts +++ b/packages/cli/src/UserManagement/PermissionChecker.ts @@ -103,7 +103,9 @@ export class PermissionChecker { policy = 'workflowsFromSameOwner'; } - const subworkflowOwner = await Container.get(OwnershipService).getWorkflowOwner(subworkflow.id); + const subworkflowOwner = await Container.get(OwnershipService).getWorkflowOwnerCached( + subworkflow.id, + ); const errorToThrow = new SubworkflowOperationError( `Target workflow ID ${subworkflow.id ?? ''} may not be called`, diff --git a/packages/cli/src/WaitTracker.ts b/packages/cli/src/WaitTracker.ts index 4a96084fad726..c1eab014daf66 100644 --- a/packages/cli/src/WaitTracker.ts +++ b/packages/cli/src/WaitTracker.ts @@ -187,7 +187,7 @@ export class WaitTracker { throw new Error('Only saved workflows can be resumed.'); } const workflowId = fullExecutionData.workflowData.id; - const user = await Container.get(OwnershipService).getWorkflowOwner(workflowId); + const user = await Container.get(OwnershipService).getWorkflowOwnerCached(workflowId); const data: IWorkflowExecutionDataProcess = { executionMode: fullExecutionData.mode, diff --git a/packages/cli/src/WaitingWebhooks.ts b/packages/cli/src/WaitingWebhooks.ts index 8a53695527e31..a62d552abb201 100644 --- a/packages/cli/src/WaitingWebhooks.ts +++ b/packages/cli/src/WaitingWebhooks.ts @@ -95,7 +95,9 @@ export class WaitingWebhooks { let workflowOwner; try { - workflowOwner = await Container.get(OwnershipService).getWorkflowOwner(workflowData.id!); + workflowOwner = await Container.get(OwnershipService).getWorkflowOwnerCached( + workflowData.id!, + ); } catch (error) { throw new ResponseHelper.NotFoundError('Could not find workflow'); } diff --git a/packages/cli/src/WebhookHelpers.ts b/packages/cli/src/WebhookHelpers.ts index 91e166a122596..b14be6af0405d 100644 --- a/packages/cli/src/WebhookHelpers.ts +++ b/packages/cli/src/WebhookHelpers.ts @@ -177,7 +177,7 @@ export async function executeWebhook( user = (workflowData as WorkflowEntity).shared[0].user; } else { try { - user = await Container.get(OwnershipService).getWorkflowOwner(workflowData.id); + user = await Container.get(OwnershipService).getWorkflowOwnerCached(workflowData.id); } catch (error) { throw new ResponseHelper.NotFoundError('Cannot find workflow'); } diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index a89701cd67884..758e35da4ab0e 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -154,7 +154,7 @@ export function executeErrorWorkflow( } Container.get(OwnershipService) - .getWorkflowOwner(workflowId) + .getWorkflowOwnerCached(workflowId) .then((user) => { void WorkflowHelpers.executeErrorWorkflow(errorWorkflow, workflowErrorData, user); }) @@ -178,7 +178,7 @@ export function executeErrorWorkflow( ) { Logger.verbose('Start internal error workflow', { executionId, workflowId }); void Container.get(OwnershipService) - .getWorkflowOwner(workflowId) + .getWorkflowOwnerCached(workflowId) .then((user) => { void WorkflowHelpers.executeErrorWorkflow(workflowId, workflowErrorData, user); }); diff --git a/packages/cli/src/commands/worker.ts b/packages/cli/src/commands/worker.ts index c5f51fed8f1d7..c48f1bd553763 100644 --- a/packages/cli/src/commands/worker.ts +++ b/packages/cli/src/commands/worker.ts @@ -112,7 +112,7 @@ export class Worker extends BaseCommand { `Start job: ${job.id} (Workflow ID: ${workflowId} | Execution: ${executionId})`, ); - const workflowOwner = await Container.get(OwnershipService).getWorkflowOwner(workflowId); + const workflowOwner = await Container.get(OwnershipService).getWorkflowOwnerCached(workflowId); let { staticData } = fullExecutionData.workflowData; if (loadStaticData) { diff --git a/packages/cli/src/services/cache.service.ts b/packages/cli/src/services/cache.service.ts index 0b033b35bd59f..d3f003fa8cf48 100644 --- a/packages/cli/src/services/cache.service.ts +++ b/packages/cli/src/services/cache.service.ts @@ -11,7 +11,7 @@ import { LoggerProxy, jsonStringify } from 'n8n-workflow'; export class CacheService { /** * Keys and values: - * - `'workflow-owner:${workflowId}'`: `User` + * - `'cache:workflow-owner:${workflowId}'`: `User` */ private cache: RedisCache | MemoryCache | undefined; diff --git a/packages/cli/src/services/events.service.ts b/packages/cli/src/services/events.service.ts index be99d842eecd1..381f7a0cd2f78 100644 --- a/packages/cli/src/services/events.service.ts +++ b/packages/cli/src/services/events.service.ts @@ -41,7 +41,7 @@ export class EventsService extends EventEmitter { const upsertResult = await this.repository.upsertWorkflowStatistics(name, workflowId); if (name === 'production_success' && upsertResult === 'insert') { - const owner = await Container.get(OwnershipService).getWorkflowOwner(workflowId); + const owner = await Container.get(OwnershipService).getWorkflowOwnerCached(workflowId); const metrics = { user_id: owner.id, workflow_id: workflowId, @@ -72,7 +72,7 @@ export class EventsService extends EventEmitter { if (insertResult === 'failed') return; // Compile the metrics since this was a new data loaded event - const owner = await Container.get(OwnershipService).getWorkflowOwner(workflowId); + const owner = await Container.get(OwnershipService).getWorkflowOwnerCached(workflowId); let metrics = { user_id: owner.id, diff --git a/packages/cli/src/services/ownership.service.ts b/packages/cli/src/services/ownership.service.ts index 3df4e46f541d4..d543d5b8bffa4 100644 --- a/packages/cli/src/services/ownership.service.ts +++ b/packages/cli/src/services/ownership.service.ts @@ -13,11 +13,9 @@ export class OwnershipService { /** * Retrieve the user who owns the workflow. Note that workflow ownership **cannot** be changed. - * - * @caching */ - async getWorkflowOwner(workflowId: string) { - const cachedOwner = await this.cacheService.get(`workflow-owner:${workflowId}`); + async getWorkflowOwnerCached(workflowId: string) { + const cachedOwner = await this.cacheService.get(`cache:workflow-owner:${workflowId}`); if (cachedOwner) return cachedOwner; @@ -30,7 +28,7 @@ export class OwnershipService { relations: ['user', 'user.globalRole'], }); - void this.cacheService.set(`workflow-owner:${workflowId}`, sharedWorkflow.user); + void this.cacheService.set(`cache:workflow-owner:${workflowId}`, sharedWorkflow.user); return sharedWorkflow.user; } diff --git a/packages/cli/test/unit/services/ownership.service.test.ts b/packages/cli/test/unit/services/ownership.service.test.ts index d8a706588563f..8dc7bbea1b5bf 100644 --- a/packages/cli/test/unit/services/ownership.service.test.ts +++ b/packages/cli/test/unit/services/ownership.service.test.ts @@ -43,7 +43,7 @@ describe('OwnershipService', () => { sharedWorkflowRepository.findOneOrFail.mockResolvedValueOnce(sharedWorkflow); - const returnedOwner = await ownershipService.getWorkflowOwner('some-workflow-id'); + const returnedOwner = await ownershipService.getWorkflowOwnerCached('some-workflow-id'); expect(returnedOwner).toBe(mockOwner); expect(returnedOwner).not.toBe(mockNonOwner); @@ -52,7 +52,7 @@ describe('OwnershipService', () => { test('should throw if no workflow owner role found', async () => { roleRepository.findWorkflowOwnerRole.mockRejectedValueOnce(new Error()); - await expect(ownershipService.getWorkflowOwner('some-workflow-id')).rejects.toThrow(); + await expect(ownershipService.getWorkflowOwnerCached('some-workflow-id')).rejects.toThrow(); }); test('should throw if no workflow owner found', async () => { @@ -60,7 +60,7 @@ describe('OwnershipService', () => { sharedWorkflowRepository.findOneOrFail.mockRejectedValue(new Error()); - await expect(ownershipService.getWorkflowOwner('some-workflow-id')).rejects.toThrow(); + await expect(ownershipService.getWorkflowOwnerCached('some-workflow-id')).rejects.toThrow(); }); }); }); From 5547314305b8138d88d632cf3fa788d168df8e1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 26 Jul 2023 11:54:40 +0200 Subject: [PATCH 08/11] test: Fix tests --- .../cli/test/unit/PermissionChecker.test.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/cli/test/unit/PermissionChecker.test.ts b/packages/cli/test/unit/PermissionChecker.test.ts index 826044946c5fa..c3d4341ea9df0 100644 --- a/packages/cli/test/unit/PermissionChecker.test.ts +++ b/packages/cli/test/unit/PermissionChecker.test.ts @@ -236,9 +236,11 @@ describe('PermissionChecker.checkSubworkflowExecutePolicy', () => { test('sets default policy from environment when subworkflow has none', async () => { config.set('workflows.callerPolicyDefaultOption', 'none'); - jest.spyOn(ownershipService, 'getWorkflowOwner').mockImplementation(async (workflowId) => { - return fakeUser; - }); + jest + .spyOn(ownershipService, 'getWorkflowOwnerCached') + .mockImplementation(async (workflowId) => { + return fakeUser; + }); jest.spyOn(UserManagementHelper, 'isSharingEnabled').mockReturnValue(true); const subworkflow = new Workflow({ @@ -255,7 +257,7 @@ describe('PermissionChecker.checkSubworkflowExecutePolicy', () => { test('if sharing is disabled, ensures that workflows are owner by same user', async () => { jest - .spyOn(ownershipService, 'getWorkflowOwner') + .spyOn(ownershipService, 'getWorkflowOwnerCached') .mockImplementation(async (workflowId) => fakeUser); jest.spyOn(UserManagementHelper, 'isSharingEnabled').mockReturnValue(false); jest.spyOn(UserService, 'get').mockImplementation(async () => fakeUser); @@ -289,7 +291,7 @@ describe('PermissionChecker.checkSubworkflowExecutePolicy', () => { test('list of ids must include the parent workflow id', async () => { const invalidParentWorkflowId = uuid(); jest - .spyOn(ownershipService, 'getWorkflowOwner') + .spyOn(ownershipService, 'getWorkflowOwnerCached') .mockImplementation(async (workflowId) => fakeUser); jest.spyOn(UserManagementHelper, 'isSharingEnabled').mockReturnValue(true); jest.spyOn(UserService, 'get').mockImplementation(async () => fakeUser); @@ -315,7 +317,7 @@ describe('PermissionChecker.checkSubworkflowExecutePolicy', () => { test('sameOwner passes when both workflows are owned by the same user', async () => { jest - .spyOn(ownershipService, 'getWorkflowOwner') + .spyOn(ownershipService, 'getWorkflowOwnerCached') .mockImplementation(async (workflowId) => fakeUser); jest.spyOn(UserManagementHelper, 'isSharingEnabled').mockReturnValue(false); jest.spyOn(UserService, 'get').mockImplementation(async () => fakeUser); @@ -338,7 +340,7 @@ describe('PermissionChecker.checkSubworkflowExecutePolicy', () => { test('workflowsFromAList works when the list contains the parent id', async () => { const workflowId = uuid(); jest - .spyOn(ownershipService, 'getWorkflowOwner') + .spyOn(ownershipService, 'getWorkflowOwnerCached') .mockImplementation(async (workflowId) => fakeUser); jest.spyOn(UserManagementHelper, 'isSharingEnabled').mockReturnValue(true); jest.spyOn(UserService, 'get').mockImplementation(async () => fakeUser); @@ -364,7 +366,7 @@ describe('PermissionChecker.checkSubworkflowExecutePolicy', () => { test('should not throw when workflow policy is set to any', async () => { jest - .spyOn(ownershipService, 'getWorkflowOwner') + .spyOn(ownershipService, 'getWorkflowOwnerCached') .mockImplementation(async (workflowId) => fakeUser); jest.spyOn(UserManagementHelper, 'isSharingEnabled').mockReturnValue(true); jest.spyOn(UserService, 'get').mockImplementation(async () => fakeUser); From 23b78debafdc32faef2b115f2d885086acc3888d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 26 Jul 2023 12:16:05 +0200 Subject: [PATCH 09/11] test: Fix missing spot --- packages/cli/test/unit/services/events.service.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/test/unit/services/events.service.test.ts b/packages/cli/test/unit/services/events.service.test.ts index 3186c8f7b2c16..128596c44cd90 100644 --- a/packages/cli/test/unit/services/events.service.test.ts +++ b/packages/cli/test/unit/services/events.service.test.ts @@ -38,7 +38,7 @@ describe('EventsService', () => { LoggerProxy.init(mock()); config.set('diagnostics.enabled', true); config.set('deployment.type', 'n8n-testing'); - mocked(ownershipService.getWorkflowOwner).mockResolvedValue(fakeUser); + mocked(ownershipService.getWorkflowOwnerCached).mockResolvedValue(fakeUser); const updateUserSettingsMock = jest.spyOn(UserService, 'updateUserSettings').mockImplementation(); const eventsService = new EventsService(new WorkflowStatisticsRepository(dataSource)); From 01ccb69f354eb1ccb744ea5391576c4df2cd01b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 26 Jul 2023 16:11:40 +0200 Subject: [PATCH 10/11] refactor: Return user entity from cache --- packages/cli/src/services/ownership.service.ts | 9 +++++---- .../cli/test/unit/services/ownership.service.test.ts | 4 +++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/services/ownership.service.ts b/packages/cli/src/services/ownership.service.ts index d543d5b8bffa4..ec01bcca01881 100644 --- a/packages/cli/src/services/ownership.service.ts +++ b/packages/cli/src/services/ownership.service.ts @@ -1,23 +1,24 @@ import { Service } from 'typedi'; import { CacheService } from './cache.service'; -import { RoleRepository, SharedWorkflowRepository } from '@/databases/repositories'; +import { RoleRepository, SharedWorkflowRepository, UserRepository } from '@/databases/repositories'; import type { User } from '@/databases/entities/User'; @Service() export class OwnershipService { constructor( private cacheService: CacheService, + private userRepository: UserRepository, private roleRepository: RoleRepository, private sharedWorkflowRepository: SharedWorkflowRepository, ) {} /** - * Retrieve the user who owns the workflow. Note that workflow ownership **cannot** be changed. + * Retrieve the user who owns the workflow. Note that workflow ownership is **immutable**. */ async getWorkflowOwnerCached(workflowId: string) { - const cachedOwner = await this.cacheService.get(`cache:workflow-owner:${workflowId}`); + const cachedValue = await this.cacheService.get(`cache:workflow-owner:${workflowId}`); - if (cachedOwner) return cachedOwner; + if (cachedValue) return this.userRepository.create(cachedValue); const workflowOwnerRole = await this.roleRepository.findWorkflowOwnerRole(); diff --git a/packages/cli/test/unit/services/ownership.service.test.ts b/packages/cli/test/unit/services/ownership.service.test.ts index 8dc7bbea1b5bf..9de9169123a5d 100644 --- a/packages/cli/test/unit/services/ownership.service.test.ts +++ b/packages/cli/test/unit/services/ownership.service.test.ts @@ -1,5 +1,5 @@ import { OwnershipService } from '@/services/ownership.service'; -import { RoleRepository, SharedWorkflowRepository } from '@/databases/repositories'; +import { RoleRepository, SharedWorkflowRepository, UserRepository } from '@/databases/repositories'; import { mockInstance } from '../../integration/shared/utils'; import { Role } from '@/databases/entities/Role'; import { randomInteger } from '../../integration/shared/random'; @@ -17,10 +17,12 @@ const wfOwnerRole = () => describe('OwnershipService', () => { const cacheService = mockInstance(CacheService); const roleRepository = mockInstance(RoleRepository); + const userRepository = mockInstance(UserRepository); const sharedWorkflowRepository = mockInstance(SharedWorkflowRepository); const ownershipService = new OwnershipService( cacheService, + userRepository, roleRepository, sharedWorkflowRepository, ); From fd55596c205e97e60332906681a40b835a20dac8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 26 Jul 2023 16:19:52 +0200 Subject: [PATCH 11/11] refactor: More dependency injection! --- packages/cli/src/WaitTracker.ts | 7 +++++-- packages/cli/src/WaitingWebhooks.ts | 12 +++++++----- packages/cli/src/services/events.service.ts | 7 +++++-- .../cli/test/unit/services/events.service.test.ts | 5 ++++- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/WaitTracker.ts b/packages/cli/src/WaitTracker.ts index c1eab014daf66..fa29ccb30fb83 100644 --- a/packages/cli/src/WaitTracker.ts +++ b/packages/cli/src/WaitTracker.ts @@ -37,7 +37,10 @@ export class WaitTracker { mainTimer: NodeJS.Timeout; - constructor(private executionRepository: ExecutionRepository) { + constructor( + private executionRepository: ExecutionRepository, + private ownershipService: OwnershipService, + ) { // Poll every 60 seconds a list of upcoming executions this.mainTimer = setInterval(() => { void this.getWaitingExecutions(); @@ -187,7 +190,7 @@ export class WaitTracker { throw new Error('Only saved workflows can be resumed.'); } const workflowId = fullExecutionData.workflowData.id; - const user = await Container.get(OwnershipService).getWorkflowOwnerCached(workflowId); + const user = await this.ownershipService.getWorkflowOwnerCached(workflowId); const data: IWorkflowExecutionDataProcess = { executionMode: fullExecutionData.mode, diff --git a/packages/cli/src/WaitingWebhooks.ts b/packages/cli/src/WaitingWebhooks.ts index a62d552abb201..92a318a7884b0 100644 --- a/packages/cli/src/WaitingWebhooks.ts +++ b/packages/cli/src/WaitingWebhooks.ts @@ -3,7 +3,7 @@ /* eslint-disable no-param-reassign */ import type { INode, WebhookHttpMethod } from 'n8n-workflow'; import { NodeHelpers, Workflow, LoggerProxy as Logger } from 'n8n-workflow'; -import Container, { Service } from 'typedi'; +import { Service } from 'typedi'; import type express from 'express'; import * as ResponseHelper from '@/ResponseHelper'; @@ -16,7 +16,11 @@ import { OwnershipService } from './services/ownership.service'; @Service() export class WaitingWebhooks { - constructor(private nodeTypes: NodeTypes, private executionRepository: ExecutionRepository) {} + constructor( + private nodeTypes: NodeTypes, + private executionRepository: ExecutionRepository, + private ownershipService: OwnershipService, + ) {} async executeWebhook( httpMethod: WebhookHttpMethod, @@ -95,9 +99,7 @@ export class WaitingWebhooks { let workflowOwner; try { - workflowOwner = await Container.get(OwnershipService).getWorkflowOwnerCached( - workflowData.id!, - ); + workflowOwner = await this.ownershipService.getWorkflowOwnerCached(workflowData.id!); } catch (error) { throw new ResponseHelper.NotFoundError('Could not find workflow'); } diff --git a/packages/cli/src/services/events.service.ts b/packages/cli/src/services/events.service.ts index 381f7a0cd2f78..0b800a0d4b318 100644 --- a/packages/cli/src/services/events.service.ts +++ b/packages/cli/src/services/events.service.ts @@ -9,7 +9,10 @@ import { OwnershipService } from './ownership.service'; @Service() export class EventsService extends EventEmitter { - constructor(private repository: WorkflowStatisticsRepository) { + constructor( + private repository: WorkflowStatisticsRepository, + private ownershipService: OwnershipService, + ) { super({ captureRejections: true }); if ('SKIP_STATISTICS_EVENTS' in process.env) return; @@ -72,7 +75,7 @@ export class EventsService extends EventEmitter { if (insertResult === 'failed') return; // Compile the metrics since this was a new data loaded event - const owner = await Container.get(OwnershipService).getWorkflowOwnerCached(workflowId); + const owner = await this.ownershipService.getWorkflowOwnerCached(workflowId); let metrics = { user_id: owner.id, diff --git a/packages/cli/test/unit/services/events.service.test.ts b/packages/cli/test/unit/services/events.service.test.ts index 128596c44cd90..cc640295575d8 100644 --- a/packages/cli/test/unit/services/events.service.test.ts +++ b/packages/cli/test/unit/services/events.service.test.ts @@ -41,7 +41,10 @@ describe('EventsService', () => { mocked(ownershipService.getWorkflowOwnerCached).mockResolvedValue(fakeUser); const updateUserSettingsMock = jest.spyOn(UserService, 'updateUserSettings').mockImplementation(); - const eventsService = new EventsService(new WorkflowStatisticsRepository(dataSource)); + const eventsService = new EventsService( + new WorkflowStatisticsRepository(dataSource), + ownershipService, + ); const onFirstProductionWorkflowSuccess = jest.fn(); const onFirstWorkflowDataLoad = jest.fn();