From 7c4ff273d4a7a7bde3022516da6209aae7b1f9bc Mon Sep 17 00:00:00 2001 From: ricardo Date: Sun, 24 Mar 2024 12:39:33 -0400 Subject: [PATCH 01/13] fix: Introduce `HooksService` --- packages/cli/src/services/hooks.service.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 packages/cli/src/services/hooks.service.ts diff --git a/packages/cli/src/services/hooks.service.ts b/packages/cli/src/services/hooks.service.ts new file mode 100644 index 0000000000000..181ec997cf879 --- /dev/null +++ b/packages/cli/src/services/hooks.service.ts @@ -0,0 +1,20 @@ +import { Service } from 'typedi'; +import { UserService } from '@/services/user.service'; +import type { AssignableRole, User } from '@/databases/entities/User'; + +/** + * Exposes function to be used by the cloud BE hooks. + * DO NOT DELETE any of the methods without making sure this is not used in cloud BE hooks. + */ +@Service() +export class HooksService { + constructor(private userService: UserService) {} + + /** + * Invites users to the instance. + * This method is used in the cloud BE hooks, to invite members during sign-up + */ + async inviteUsers(owner: User, attributes: Array<{ email: string; role: AssignableRole }>) { + return await this.userService.inviteUsers(owner, attributes); + } +} From 656002d697b6477036aaf8ade46944e453287dae Mon Sep 17 00:00:00 2001 From: ricardo Date: Sun, 24 Mar 2024 12:50:10 -0400 Subject: [PATCH 02/13] fix description --- packages/cli/src/services/hooks.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/services/hooks.service.ts b/packages/cli/src/services/hooks.service.ts index 181ec997cf879..44d8aaac1c467 100644 --- a/packages/cli/src/services/hooks.service.ts +++ b/packages/cli/src/services/hooks.service.ts @@ -3,7 +3,7 @@ import { UserService } from '@/services/user.service'; import type { AssignableRole, User } from '@/databases/entities/User'; /** - * Exposes function to be used by the cloud BE hooks. + * Exposes functionality to be used by the cloud BE hooks. * DO NOT DELETE any of the methods without making sure this is not used in cloud BE hooks. */ @Service() From b64f340d861c3c871d51471f4cf75893c3a9486c Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Wed, 27 Mar 2024 18:39:25 -0400 Subject: [PATCH 03/13] add test to make sure downstream service is called. --- .../test/unit/services/hooks.service.test.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 packages/cli/test/unit/services/hooks.service.test.ts diff --git a/packages/cli/test/unit/services/hooks.service.test.ts b/packages/cli/test/unit/services/hooks.service.test.ts new file mode 100644 index 0000000000000..5f3debe25ebcc --- /dev/null +++ b/packages/cli/test/unit/services/hooks.service.test.ts @@ -0,0 +1,33 @@ +import { UserService } from '@/services/user.service'; +import { HooksService } from '@/services/hooks.service'; +import { User } from '@/databases/entities/User'; +import { mockInstance } from '../../shared/mocking'; +import { v4 as uuid } from 'uuid'; + +describe('HooksService', () => { + const userService = mockInstance(UserService); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('hooksService.inviteUsers should call userService.inviteUsers', async () => { + const hooksService = new HooksService(userService); + const usersToInvite: Parameters[1] = [ + { email: 'test@n8n.io', role: 'global:member' }, + ]; + + const mockUser = Object.assign(new User(), { + id: uuid(), + password: 'passwordHash', + mfaEnabled: false, + mfaSecret: 'test', + mfaRecoveryCodes: ['test'], + updatedAt: new Date(), + authIdentities: [], + }); + + await hooksService.inviteUsers(mockUser, usersToInvite); + expect(userService.inviteUsers).toHaveBeenCalledWith(mockUser, usersToInvite); + }); +}); From d0d8d44fbc8939adcfafaab2757f15af87b4c76f Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Wed, 27 Mar 2024 18:40:08 -0400 Subject: [PATCH 04/13] improve function documentation --- packages/cli/src/services/hooks.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/services/hooks.service.ts b/packages/cli/src/services/hooks.service.ts index 44d8aaac1c467..e7ed32d19ea7e 100644 --- a/packages/cli/src/services/hooks.service.ts +++ b/packages/cli/src/services/hooks.service.ts @@ -4,7 +4,7 @@ import type { AssignableRole, User } from '@/databases/entities/User'; /** * Exposes functionality to be used by the cloud BE hooks. - * DO NOT DELETE any of the methods without making sure this is not used in cloud BE hooks. + * DO NOT DELETE or RENAME any of the methods without making sure this is not used in cloud BE hooks. */ @Service() export class HooksService { From d793d48e71b7c7de81ccb58eb2c4f5d8f70af7bf Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Wed, 5 Jun 2024 21:01:21 -0400 Subject: [PATCH 05/13] Add more methods used in the hooks to the service --- packages/cli/src/services/hooks.service.ts | 33 +++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/services/hooks.service.ts b/packages/cli/src/services/hooks.service.ts index e7ed32d19ea7e..be7629f7335c5 100644 --- a/packages/cli/src/services/hooks.service.ts +++ b/packages/cli/src/services/hooks.service.ts @@ -1,6 +1,12 @@ import { Service } from 'typedi'; import { UserService } from '@/services/user.service'; import type { AssignableRole, User } from '@/databases/entities/User'; +import { AuthService } from '@/auth/auth.service'; +import type { Response } from 'express'; +import { UserRepository } from '@/databases/repositories/user.repository'; +import { SettingsRepository } from '@/databases/repositories/settings.repository'; +import type { Settings } from '@oclif/core'; +import type { QueryDeepPartialEntity } from '@n8n/typeorm/query-builder/QueryPartialEntity'; /** * Exposes functionality to be used by the cloud BE hooks. @@ -8,7 +14,12 @@ import type { AssignableRole, User } from '@/databases/entities/User'; */ @Service() export class HooksService { - constructor(private userService: UserService) {} + constructor( + private userService: UserService, + private authService: AuthService, + private userRepository: UserRepository, + private settingsRepository: SettingsRepository, + ) {} /** * Invites users to the instance. @@ -17,4 +28,24 @@ export class HooksService { async inviteUsers(owner: User, attributes: Array<{ email: string; role: AssignableRole }>) { return await this.userService.inviteUsers(owner, attributes); } + + issueCookie(res: Response, user: User) { + return this.authService.issueCookie(res, user); + } + + async findInstanceOwner() { + return await this.userRepository.findOne({ where: { role: 'global:owner' } }); + } + + async findUserById(id: string) { + return await this.userRepository.findOne({ where: { id } }); + } + + async saveUser(user: User) { + return await this.userRepository.save(user); + } + + async updateSettings(key: keyof Settings, value: QueryDeepPartialEntity) { + return await this.settingsRepository.update({ key }, { value: JSON.stringify(value) }); + } } From d7b201c7c0d50b95a7df3a47d73a3f6c99e85b2e Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Thu, 6 Jun 2024 16:23:07 -0400 Subject: [PATCH 06/13] Add DB access in BE hooks to hooks service --- packages/cli/src/services/hooks.service.ts | 64 ++++++++++--- .../test/unit/services/hooks.service.test.ts | 90 ++++++++++++++++--- 2 files changed, 130 insertions(+), 24 deletions(-) diff --git a/packages/cli/src/services/hooks.service.ts b/packages/cli/src/services/hooks.service.ts index be7629f7335c5..afe688d97b3cc 100644 --- a/packages/cli/src/services/hooks.service.ts +++ b/packages/cli/src/services/hooks.service.ts @@ -5,8 +5,13 @@ import { AuthService } from '@/auth/auth.service'; import type { Response } from 'express'; import { UserRepository } from '@/databases/repositories/user.repository'; import { SettingsRepository } from '@/databases/repositories/settings.repository'; -import type { Settings } from '@oclif/core'; import type { QueryDeepPartialEntity } from '@n8n/typeorm/query-builder/QueryPartialEntity'; +import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; +import type { FindManyOptions, FindOneOptions, FindOptionsWhere } from '@n8n/typeorm'; +import type { WorkflowEntity } from '@/databases/entities/WorkflowEntity'; +import type { CredentialsEntity } from '@/databases/entities/CredentialsEntity'; +import type { Settings } from '@/databases/entities/Settings'; /** * Exposes functionality to be used by the cloud BE hooks. @@ -19,33 +24,70 @@ export class HooksService { private authService: AuthService, private userRepository: UserRepository, private settingsRepository: SettingsRepository, + private workflowRepository: WorkflowRepository, + private credentialsRepository: CredentialsRepository, ) {} /** - * Invites users to the instance. - * This method is used in the cloud BE hooks, to invite members during sign-up + * Invite users to instance during signup */ async inviteUsers(owner: User, attributes: Array<{ email: string; role: AssignableRole }>) { return await this.userService.inviteUsers(owner, attributes); } + /** + * Set the n8n-auth cookie in the response to auto-login + * the user after instance is provisioned + */ issueCookie(res: Response, user: User) { return this.authService.issueCookie(res, user); } - async findInstanceOwner() { - return await this.userRepository.findOne({ where: { role: 'global:owner' } }); - } - - async findUserById(id: string) { - return await this.userRepository.findOne({ where: { id } }); + /** + * Find user in the instance + * 1. To know whether the instance owner is already setup + * 2. To know when to update the user's profile also in cloud + */ + async findOneUser(filter: FindOneOptions) { + return await this.userRepository.findOne(filter); } + /** + * Save instance owner with the cloud signup data + */ async saveUser(user: User) { return await this.userRepository.save(user); } - async updateSettings(key: keyof Settings, value: QueryDeepPartialEntity) { - return await this.settingsRepository.update({ key }, { value: JSON.stringify(value) }); + /** + * Update instance's settings + * 1. To keep the state when users are invited to the instance + */ + async updateSettings(filter: FindOptionsWhere, set: QueryDeepPartialEntity) { + return await this.settingsRepository.update(filter, set); + } + + /** + * Count the number of workflows + * 1. To enforce the active workflow limits in cloud + */ + async workflowCount(filter: FindManyOptions) { + return await this.workflowRepository.count(filter); + } + + /** + * Count the number of credentials + * 1. To enforce the max credential limits in cloud + */ + async credentialCount(filter: FindManyOptions) { + return await this.credentialsRepository.count(filter); + } + + /** + * Count the number of occurrences of a specific key + * 1. To know when to stop attempting to invite users + */ + async settingsCount(filter: FindManyOptions) { + return await this.settingsRepository.count(filter); } } diff --git a/packages/cli/test/unit/services/hooks.service.test.ts b/packages/cli/test/unit/services/hooks.service.test.ts index 5f3debe25ebcc..6b43771072097 100644 --- a/packages/cli/test/unit/services/hooks.service.test.ts +++ b/packages/cli/test/unit/services/hooks.service.test.ts @@ -3,31 +3,95 @@ import { HooksService } from '@/services/hooks.service'; import { User } from '@/databases/entities/User'; import { mockInstance } from '../../shared/mocking'; import { v4 as uuid } from 'uuid'; +import { AuthService } from '@/auth/auth.service'; +import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; +import { SettingsRepository } from '@/databases/repositories/settings.repository'; +import { UserRepository } from '@/databases/repositories/user.repository'; +import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import type { Response } from 'express'; + +let hooksService: HooksService; + +const mockedUser = Object.assign(new User(), { + id: uuid(), + password: 'passwordHash', + mfaEnabled: false, + mfaSecret: 'test', + mfaRecoveryCodes: ['test'], + updatedAt: new Date(), +}); describe('HooksService', () => { const userService = mockInstance(UserService); + const authService = mockInstance(AuthService); + const userRepository = mockInstance(UserRepository); + const settingsRepository = mockInstance(SettingsRepository); + const workflowRepository = mockInstance(WorkflowRepository); + const credentialsRepository = mockInstance(CredentialsRepository); + hooksService = new HooksService( + userService, + authService, + userRepository, + settingsRepository, + workflowRepository, + credentialsRepository, + ); beforeEach(() => { jest.clearAllMocks(); }); it('hooksService.inviteUsers should call userService.inviteUsers', async () => { - const hooksService = new HooksService(userService); const usersToInvite: Parameters[1] = [ { email: 'test@n8n.io', role: 'global:member' }, ]; - const mockUser = Object.assign(new User(), { - id: uuid(), - password: 'passwordHash', - mfaEnabled: false, - mfaSecret: 'test', - mfaRecoveryCodes: ['test'], - updatedAt: new Date(), - authIdentities: [], - }); - - await hooksService.inviteUsers(mockUser, usersToInvite); - expect(userService.inviteUsers).toHaveBeenCalledWith(mockUser, usersToInvite); + await hooksService.inviteUsers(mockedUser, usersToInvite); + expect(userService.inviteUsers).toHaveBeenCalledWith(mockedUser, usersToInvite); + }); + + it('hooksService.issueCookie should call authService.issueCookie', async () => { + const res = { + cookie: jest.fn(), + } as unknown as Response; + + hooksService.issueCookie(res, mockedUser); + expect(authService.issueCookie).toHaveBeenCalledWith(res, mockedUser); + }); + + it('hooksService.findOneUser should call userRepository.findOne', async () => { + const filter = { where: { id: '1' } }; + await hooksService.findOneUser(filter); + expect(userRepository.findOne).toHaveBeenCalledWith(filter); + }); + + it('hooksService.saveUser should call userRepository.save', async () => { + await hooksService.saveUser(mockedUser); + expect(userRepository.save).toHaveBeenCalledWith(mockedUser); + }); + + it('hooksService.updateSettings should call settingRepository.update', async () => { + const filter = { key: 'test' }; + const set = { value: 'true' }; + await hooksService.updateSettings(filter, set); + expect(settingsRepository.update).toHaveBeenCalledWith(filter, set); + }); + + it('hooksService.workflowCount should call workflowRepository.count', async () => { + const filter = { where: { active: true } }; + await hooksService.workflowCount(filter); + expect(workflowRepository.count).toHaveBeenCalledWith(filter); + }); + + it('hooksService.credentialCount should call credentialRepository.count', async () => { + const filter = { where: {} }; + await hooksService.credentialCount(filter); + expect(credentialsRepository.count).toHaveBeenCalledWith(filter); + }); + + it('hooksService.settingsCount should call settingsRepository.count', async () => { + const filter = { where: { key: 'test' } }; + await hooksService.settingsCount(filter); + expect(settingsRepository.count).toHaveBeenCalledWith(filter); }); }); From 87e2e6291f1fe272dc83aa089a9c855fd7ea817c Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Sun, 9 Jun 2024 11:08:10 -0400 Subject: [PATCH 07/13] Make all properties read only Co-authored-by: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> --- packages/cli/src/services/hooks.service.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/services/hooks.service.ts b/packages/cli/src/services/hooks.service.ts index afe688d97b3cc..df62221fc5469 100644 --- a/packages/cli/src/services/hooks.service.ts +++ b/packages/cli/src/services/hooks.service.ts @@ -20,12 +20,12 @@ import type { Settings } from '@/databases/entities/Settings'; @Service() export class HooksService { constructor( - private userService: UserService, - private authService: AuthService, - private userRepository: UserRepository, - private settingsRepository: SettingsRepository, - private workflowRepository: WorkflowRepository, - private credentialsRepository: CredentialsRepository, + private readonly userService: UserService, + private readonly authService: AuthService, + private readonly userRepository: UserRepository, + private readonly settingsRepository: SettingsRepository, + private readonly workflowRepository: WorkflowRepository, + private readonly credentialsRepository: CredentialsRepository, ) {} /** From 88622c1a80f56e2e7dc94da9f6905f5b1fad0ecf Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Sun, 9 Jun 2024 21:46:31 -0400 Subject: [PATCH 08/13] renames and add authMiddleware method --- packages/cli/src/services/hooks.service.ts | 15 +++++++++--- .../test/unit/services/hooks.service.test.ts | 23 +++++++++++++++---- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/services/hooks.service.ts b/packages/cli/src/services/hooks.service.ts index df62221fc5469..3f653c78c2dc8 100644 --- a/packages/cli/src/services/hooks.service.ts +++ b/packages/cli/src/services/hooks.service.ts @@ -2,7 +2,7 @@ import { Service } from 'typedi'; import { UserService } from '@/services/user.service'; import type { AssignableRole, User } from '@/databases/entities/User'; import { AuthService } from '@/auth/auth.service'; -import type { Response } from 'express'; +import type { NextFunction, Response } from 'express'; import { UserRepository } from '@/databases/repositories/user.repository'; import { SettingsRepository } from '@/databases/repositories/settings.repository'; import type { QueryDeepPartialEntity } from '@n8n/typeorm/query-builder/QueryPartialEntity'; @@ -12,6 +12,7 @@ import type { FindManyOptions, FindOneOptions, FindOptionsWhere } from '@n8n/typ import type { WorkflowEntity } from '@/databases/entities/WorkflowEntity'; import type { CredentialsEntity } from '@/databases/entities/CredentialsEntity'; import type { Settings } from '@/databases/entities/Settings'; +import type { AuthenticatedRequest } from '@/requests'; /** * Exposes functionality to be used by the cloud BE hooks. @@ -71,7 +72,7 @@ export class HooksService { * Count the number of workflows * 1. To enforce the active workflow limits in cloud */ - async workflowCount(filter: FindManyOptions) { + async workflowsCount(filter: FindManyOptions) { return await this.workflowRepository.count(filter); } @@ -79,7 +80,7 @@ export class HooksService { * Count the number of credentials * 1. To enforce the max credential limits in cloud */ - async credentialCount(filter: FindManyOptions) { + async credentialsCount(filter: FindManyOptions) { return await this.credentialsRepository.count(filter); } @@ -90,4 +91,12 @@ export class HooksService { async settingsCount(filter: FindManyOptions) { return await this.settingsRepository.count(filter); } + + /** + * Add auth middleware to routes injected via the hooks + * 1. To authenticate the /proxy routes in the hooks + */ + async authMiddleware(req: AuthenticatedRequest, res: Response, next: NextFunction) { + return await this.authService.authMiddleware(req, res, next); + } } diff --git a/packages/cli/test/unit/services/hooks.service.test.ts b/packages/cli/test/unit/services/hooks.service.test.ts index 6b43771072097..5aee7a0fd2814 100644 --- a/packages/cli/test/unit/services/hooks.service.test.ts +++ b/packages/cli/test/unit/services/hooks.service.test.ts @@ -77,15 +77,15 @@ describe('HooksService', () => { expect(settingsRepository.update).toHaveBeenCalledWith(filter, set); }); - it('hooksService.workflowCount should call workflowRepository.count', async () => { + it('hooksService.workflowsCount should call workflowRepository.count', async () => { const filter = { where: { active: true } }; - await hooksService.workflowCount(filter); + await hooksService.workflowsCount(filter); expect(workflowRepository.count).toHaveBeenCalledWith(filter); }); - it('hooksService.credentialCount should call credentialRepository.count', async () => { + it('hooksService.credentialsCount should call credentialRepository.count', async () => { const filter = { where: {} }; - await hooksService.credentialCount(filter); + await hooksService.credentialsCount(filter); expect(credentialsRepository.count).toHaveBeenCalledWith(filter); }); @@ -94,4 +94,19 @@ describe('HooksService', () => { await hooksService.settingsCount(filter); expect(settingsRepository.count).toHaveBeenCalledWith(filter); }); + + it('hooksService.authMiddleware should call authService.authMiddleware', async () => { + const res = { + cookie: jest.fn(), + } as unknown as Response; + + const req = { + cookie: jest.fn(), + } as unknown as Response; + + const next = jest.fn(); + + await hooksService.authMiddleware(req, res, next); + expect(authService.authMiddleware).toHaveBeenCalledWith(req, res); + }); }); From e78467e4dcf544d5d7878c69f8baf8c19b0e3059 Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Mon, 10 Jun 2024 09:19:58 -0400 Subject: [PATCH 09/13] expose db collections --- packages/cli/src/services/hooks.service.ts | 13 +++++++++++++ .../cli/test/unit/services/hooks.service.test.ts | 10 +++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/services/hooks.service.ts b/packages/cli/src/services/hooks.service.ts index 3f653c78c2dc8..68c8bfce4fb46 100644 --- a/packages/cli/src/services/hooks.service.ts +++ b/packages/cli/src/services/hooks.service.ts @@ -99,4 +99,17 @@ export class HooksService { async authMiddleware(req: AuthenticatedRequest, res: Response, next: NextFunction) { return await this.authService.authMiddleware(req, res, next); } + + /** + * Return repositories to be used in the hooks + * 1. Some self-hosted users rely in the repositories to interact with the DB directly + */ + dbCollections() { + return { + User: this.userRepository, + Settings: this.settingsRepository, + Credentials: this.credentialsRepository, + Workflow: this.workflowRepository, + }; + } } diff --git a/packages/cli/test/unit/services/hooks.service.test.ts b/packages/cli/test/unit/services/hooks.service.test.ts index 5aee7a0fd2814..9c72092feaecd 100644 --- a/packages/cli/test/unit/services/hooks.service.test.ts +++ b/packages/cli/test/unit/services/hooks.service.test.ts @@ -107,6 +107,14 @@ describe('HooksService', () => { const next = jest.fn(); await hooksService.authMiddleware(req, res, next); - expect(authService.authMiddleware).toHaveBeenCalledWith(req, res); + expect(authService.authMiddleware).toHaveBeenCalledWith(req, res, next); + }); + + it('hooksService.dbCollections should return valid repositories', async () => { + const collections = hooksService.dbCollections(); + expect(collections).toHaveProperty('User'); + expect(collections).toHaveProperty('Settings'); + expect(collections).toHaveProperty('Credentials'); + expect(collections).toHaveProperty('Workflow'); }); }); From e00a64f0807d34c325cc082ae9518e4fa776546c Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Mon, 10 Jun 2024 09:24:16 -0400 Subject: [PATCH 10/13] add comments to tests --- .../test/unit/services/hooks.service.test.ts | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/cli/test/unit/services/hooks.service.test.ts b/packages/cli/test/unit/services/hooks.service.test.ts index 9c72092feaecd..adb3500d1c013 100644 --- a/packages/cli/test/unit/services/hooks.service.test.ts +++ b/packages/cli/test/unit/services/hooks.service.test.ts @@ -42,60 +42,98 @@ describe('HooksService', () => { }); it('hooksService.inviteUsers should call userService.inviteUsers', async () => { + // ARRANGE const usersToInvite: Parameters[1] = [ { email: 'test@n8n.io', role: 'global:member' }, ]; + // ACT await hooksService.inviteUsers(mockedUser, usersToInvite); + + // ASSERT expect(userService.inviteUsers).toHaveBeenCalledWith(mockedUser, usersToInvite); }); it('hooksService.issueCookie should call authService.issueCookie', async () => { + // ARRANGE const res = { cookie: jest.fn(), } as unknown as Response; + // ACT hooksService.issueCookie(res, mockedUser); + + // ASSERT expect(authService.issueCookie).toHaveBeenCalledWith(res, mockedUser); }); it('hooksService.findOneUser should call userRepository.findOne', async () => { + // ARRANGE const filter = { where: { id: '1' } }; + + // ACT await hooksService.findOneUser(filter); + + // ASSERT expect(userRepository.findOne).toHaveBeenCalledWith(filter); }); it('hooksService.saveUser should call userRepository.save', async () => { + // ACT await hooksService.saveUser(mockedUser); + + // ASSERT + expect(userRepository.save).toHaveBeenCalledWith(mockedUser); }); it('hooksService.updateSettings should call settingRepository.update', async () => { + // ARRANGE const filter = { key: 'test' }; const set = { value: 'true' }; + + // ACT await hooksService.updateSettings(filter, set); + + // ASSERT expect(settingsRepository.update).toHaveBeenCalledWith(filter, set); }); it('hooksService.workflowsCount should call workflowRepository.count', async () => { + // ARRANGE const filter = { where: { active: true } }; + + // ACT await hooksService.workflowsCount(filter); + + // ASSERT expect(workflowRepository.count).toHaveBeenCalledWith(filter); }); it('hooksService.credentialsCount should call credentialRepository.count', async () => { + // ARRANGE const filter = { where: {} }; + + // ACT await hooksService.credentialsCount(filter); + + // ASSERT expect(credentialsRepository.count).toHaveBeenCalledWith(filter); }); it('hooksService.settingsCount should call settingsRepository.count', async () => { + // ARRANGE const filter = { where: { key: 'test' } }; + + // ACT await hooksService.settingsCount(filter); + + // ASSERT expect(settingsRepository.count).toHaveBeenCalledWith(filter); }); it('hooksService.authMiddleware should call authService.authMiddleware', async () => { + // ARRANGE const res = { cookie: jest.fn(), } as unknown as Response; @@ -106,12 +144,18 @@ describe('HooksService', () => { const next = jest.fn(); + // ACT await hooksService.authMiddleware(req, res, next); + + // ASSERT expect(authService.authMiddleware).toHaveBeenCalledWith(req, res, next); }); it('hooksService.dbCollections should return valid repositories', async () => { + // ACT const collections = hooksService.dbCollections(); + + // ASSERT expect(collections).toHaveProperty('User'); expect(collections).toHaveProperty('Settings'); expect(collections).toHaveProperty('Credentials'); From 1e3db1e980ae77400cda67f4bfb5fe1b29101ada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Mon, 10 Jun 2024 16:25:34 +0200 Subject: [PATCH 11/13] fix typing errors --- packages/cli/src/Interfaces.ts | 7 +- .../cli/src/databases/entities/AuthUser.ts | 14 +--- packages/cli/src/services/hooks.service.ts | 27 ++++---- packages/cli/src/services/user.service.ts | 10 +-- .../test/unit/services/hooks.service.test.ts | 64 +++++++------------ 5 files changed, 53 insertions(+), 69 deletions(-) diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 9a6cffd782a22..d3c0fc4e28a78 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -35,7 +35,7 @@ import type PCancelable from 'p-cancelable'; import type { AuthProviderType } from '@db/entities/AuthIdentity'; import type { SharedCredentials } from '@db/entities/SharedCredentials'; import type { TagEntity } from '@db/entities/TagEntity'; -import type { GlobalRole, User } from '@db/entities/User'; +import type { AssignableRole, GlobalRole, User } from '@db/entities/User'; import type { CredentialsRepository } from '@db/repositories/credentials.repository'; import type { SettingsRepository } from '@db/repositories/settings.repository'; import type { UserRepository } from '@db/repositories/user.repository'; @@ -634,6 +634,11 @@ export interface PublicUser { featureFlags?: FeatureFlags; } +export interface Invitation { + email: string; + role: AssignableRole; +} + export interface N8nApp { app: Application; restEndpoint: string; diff --git a/packages/cli/src/databases/entities/AuthUser.ts b/packages/cli/src/databases/entities/AuthUser.ts index f9f8d89f8e9e3..dbfb8f83fd297 100644 --- a/packages/cli/src/databases/entities/AuthUser.ts +++ b/packages/cli/src/databases/entities/AuthUser.ts @@ -1,16 +1,8 @@ -import { Column, Entity, PrimaryColumn } from '@n8n/typeorm'; +import { Column, Entity } from '@n8n/typeorm'; +import { User } from './User'; @Entity({ name: 'user' }) -export class AuthUser { - @PrimaryColumn({ type: 'uuid', update: false }) - id: string; - - @Column({ type: String, update: false }) - email: string; - - @Column({ type: Boolean, default: false }) - mfaEnabled: boolean; - +export class AuthUser extends User { @Column({ type: String, nullable: true }) mfaSecret?: string | null; diff --git a/packages/cli/src/services/hooks.service.ts b/packages/cli/src/services/hooks.service.ts index 68c8bfce4fb46..4b01e72f0308d 100644 --- a/packages/cli/src/services/hooks.service.ts +++ b/packages/cli/src/services/hooks.service.ts @@ -1,18 +1,21 @@ import { Service } from 'typedi'; -import { UserService } from '@/services/user.service'; -import type { AssignableRole, User } from '@/databases/entities/User'; -import { AuthService } from '@/auth/auth.service'; import type { NextFunction, Response } from 'express'; -import { UserRepository } from '@/databases/repositories/user.repository'; -import { SettingsRepository } from '@/databases/repositories/settings.repository'; import type { QueryDeepPartialEntity } from '@n8n/typeorm/query-builder/QueryPartialEntity'; -import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; -import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; import type { FindManyOptions, FindOneOptions, FindOptionsWhere } from '@n8n/typeorm'; -import type { WorkflowEntity } from '@/databases/entities/WorkflowEntity'; -import type { CredentialsEntity } from '@/databases/entities/CredentialsEntity'; -import type { Settings } from '@/databases/entities/Settings'; + +import { AuthService } from '@/auth/auth.service'; +import type { AuthUser } from '@db/entities/AuthUser'; +import type { User } from '@db/entities/User'; +import { UserRepository } from '@db/repositories/user.repository'; +import { SettingsRepository } from '@db/repositories/settings.repository'; +import { WorkflowRepository } from '@db/repositories/workflow.repository'; +import { CredentialsRepository } from '@db/repositories/credentials.repository'; +import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; +import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; +import type { Settings } from '@db/entities/Settings'; +import { UserService } from '@/services/user.service'; import type { AuthenticatedRequest } from '@/requests'; +import type { Invitation } from '@/Interfaces'; /** * Exposes functionality to be used by the cloud BE hooks. @@ -32,7 +35,7 @@ export class HooksService { /** * Invite users to instance during signup */ - async inviteUsers(owner: User, attributes: Array<{ email: string; role: AssignableRole }>) { + async inviteUsers(owner: AuthUser, attributes: Invitation[]) { return await this.userService.inviteUsers(owner, attributes); } @@ -40,7 +43,7 @@ export class HooksService { * Set the n8n-auth cookie in the response to auto-login * the user after instance is provisioned */ - issueCookie(res: Response, user: User) { + issueCookie(res: Response, user: AuthUser) { return this.authService.issueCookie(res, user); } diff --git a/packages/cli/src/services/user.service.ts b/packages/cli/src/services/user.service.ts index 24343c8957a2a..e9681d68da818 100644 --- a/packages/cli/src/services/user.service.ts +++ b/packages/cli/src/services/user.service.ts @@ -4,7 +4,7 @@ import { ApplicationError, ErrorReporterProxy as ErrorReporter } from 'n8n-workf import type { User, AssignableRole } from '@db/entities/User'; import { UserRepository } from '@db/repositories/user.repository'; -import type { PublicUser } from '@/Interfaces'; +import type { Invitation, PublicUser } from '@/Interfaces'; import type { PostHogClient } from '@/posthog'; import { Logger } from '@/Logger'; import { UserManagementMailer } from '@/UserManagement/email'; @@ -178,14 +178,14 @@ export class UserService { ); } - async inviteUsers(owner: User, attributes: Array<{ email: string; role: AssignableRole }>) { - const emails = attributes.map(({ email }) => email); + async inviteUsers(owner: User, invitations: Invitation[]) { + const emails = invitations.map(({ email }) => email); const existingUsers = await this.userRepository.findManyByEmail(emails); const existUsersEmails = existingUsers.map((user) => user.email); - const toCreateUsers = attributes.filter(({ email }) => !existUsersEmails.includes(email)); + const toCreateUsers = invitations.filter(({ email }) => !existUsersEmails.includes(email)); const pendingUsersToInvite = existingUsers.filter((email) => email.isPending); @@ -222,7 +222,7 @@ export class UserService { const usersInvited = await this.sendEmails( owner, Object.fromEntries(createdUsers), - attributes[0].role, // same role for all invited users + invitations[0].role, // same role for all invited users ); return { usersInvited, usersCreated: toCreateUsers.map(({ email }) => email) }; diff --git a/packages/cli/test/unit/services/hooks.service.test.ts b/packages/cli/test/unit/services/hooks.service.test.ts index adb3500d1c013..eba2b67235dbb 100644 --- a/packages/cli/test/unit/services/hooks.service.test.ts +++ b/packages/cli/test/unit/services/hooks.service.test.ts @@ -1,34 +1,26 @@ -import { UserService } from '@/services/user.service'; -import { HooksService } from '@/services/hooks.service'; -import { User } from '@/databases/entities/User'; -import { mockInstance } from '../../shared/mocking'; -import { v4 as uuid } from 'uuid'; -import { AuthService } from '@/auth/auth.service'; -import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; -import { SettingsRepository } from '@/databases/repositories/settings.repository'; -import { UserRepository } from '@/databases/repositories/user.repository'; -import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import type { Response } from 'express'; - -let hooksService: HooksService; - -const mockedUser = Object.assign(new User(), { - id: uuid(), - password: 'passwordHash', - mfaEnabled: false, - mfaSecret: 'test', - mfaRecoveryCodes: ['test'], - updatedAt: new Date(), -}); +import { mock } from 'jest-mock-extended'; + +import type { AuthUser } from '@db/entities/AuthUser'; +import type { CredentialsRepository } from '@db/repositories/credentials.repository'; +import type { SettingsRepository } from '@db/repositories/settings.repository'; +import type { UserRepository } from '@db/repositories/user.repository'; +import type { WorkflowRepository } from '@db/repositories/workflow.repository'; +import type { AuthService } from '@/auth/auth.service'; +import type { UserService } from '@/services/user.service'; +import { HooksService } from '@/services/hooks.service'; +import type { Invitation } from '@/Interfaces'; +import type { AuthenticatedRequest } from '@/requests'; describe('HooksService', () => { - const userService = mockInstance(UserService); - const authService = mockInstance(AuthService); - const userRepository = mockInstance(UserRepository); - const settingsRepository = mockInstance(SettingsRepository); - const workflowRepository = mockInstance(WorkflowRepository); - const credentialsRepository = mockInstance(CredentialsRepository); - hooksService = new HooksService( + const mockedUser = mock(); + const userService = mock(); + const authService = mock(); + const userRepository = mock(); + const settingsRepository = mock(); + const workflowRepository = mock(); + const credentialsRepository = mock(); + const hooksService = new HooksService( userService, authService, userRepository, @@ -43,9 +35,7 @@ describe('HooksService', () => { it('hooksService.inviteUsers should call userService.inviteUsers', async () => { // ARRANGE - const usersToInvite: Parameters[1] = [ - { email: 'test@n8n.io', role: 'global:member' }, - ]; + const usersToInvite: Invitation[] = [{ email: 'test@n8n.io', role: 'global:member' }]; // ACT await hooksService.inviteUsers(mockedUser, usersToInvite); @@ -56,9 +46,7 @@ describe('HooksService', () => { it('hooksService.issueCookie should call authService.issueCookie', async () => { // ARRANGE - const res = { - cookie: jest.fn(), - } as unknown as Response; + const res = mock(); // ACT hooksService.issueCookie(res, mockedUser); @@ -134,13 +122,9 @@ describe('HooksService', () => { it('hooksService.authMiddleware should call authService.authMiddleware', async () => { // ARRANGE - const res = { - cookie: jest.fn(), - } as unknown as Response; + const res = mock(); - const req = { - cookie: jest.fn(), - } as unknown as Response; + const req = mock(); const next = jest.fn(); From 51c11f71ab891b4a3c2dd6c5c4d1163e57342c4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Mon, 10 Jun 2024 16:56:34 +0200 Subject: [PATCH 12/13] revert the accidental change --- packages/cli/src/services/hooks.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/services/hooks.service.ts b/packages/cli/src/services/hooks.service.ts index 4b01e72f0308d..811ea4a4a2284 100644 --- a/packages/cli/src/services/hooks.service.ts +++ b/packages/cli/src/services/hooks.service.ts @@ -35,7 +35,7 @@ export class HooksService { /** * Invite users to instance during signup */ - async inviteUsers(owner: AuthUser, attributes: Invitation[]) { + async inviteUsers(owner: User, attributes: Invitation[]) { return await this.userService.inviteUsers(owner, attributes); } From a4bbdbaa17b0f55f6163da283e52889d00a5a061 Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Mon, 10 Jun 2024 11:04:24 -0400 Subject: [PATCH 13/13] Use the Auth repo when looking for user --- packages/cli/src/services/hooks.service.ts | 6 ++++-- packages/cli/test/unit/services/hooks.service.test.ts | 7 +++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/services/hooks.service.ts b/packages/cli/src/services/hooks.service.ts index 811ea4a4a2284..72d4b7f2612e5 100644 --- a/packages/cli/src/services/hooks.service.ts +++ b/packages/cli/src/services/hooks.service.ts @@ -12,6 +12,7 @@ import { WorkflowRepository } from '@db/repositories/workflow.repository'; import { CredentialsRepository } from '@db/repositories/credentials.repository'; import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; +import { AuthUserRepository } from '@db/repositories/authUser.repository'; import type { Settings } from '@db/entities/Settings'; import { UserService } from '@/services/user.service'; import type { AuthenticatedRequest } from '@/requests'; @@ -30,6 +31,7 @@ export class HooksService { private readonly settingsRepository: SettingsRepository, private readonly workflowRepository: WorkflowRepository, private readonly credentialsRepository: CredentialsRepository, + private readonly authUserRepository: AuthUserRepository, ) {} /** @@ -52,8 +54,8 @@ export class HooksService { * 1. To know whether the instance owner is already setup * 2. To know when to update the user's profile also in cloud */ - async findOneUser(filter: FindOneOptions) { - return await this.userRepository.findOne(filter); + async findOneUser(filter: FindOneOptions) { + return await this.authUserRepository.findOne(filter); } /** diff --git a/packages/cli/test/unit/services/hooks.service.test.ts b/packages/cli/test/unit/services/hooks.service.test.ts index eba2b67235dbb..406ede0374d4d 100644 --- a/packages/cli/test/unit/services/hooks.service.test.ts +++ b/packages/cli/test/unit/services/hooks.service.test.ts @@ -11,6 +11,7 @@ import type { UserService } from '@/services/user.service'; import { HooksService } from '@/services/hooks.service'; import type { Invitation } from '@/Interfaces'; import type { AuthenticatedRequest } from '@/requests'; +import type { AuthUserRepository } from '@/databases/repositories/authUser.repository'; describe('HooksService', () => { const mockedUser = mock(); @@ -20,6 +21,7 @@ describe('HooksService', () => { const settingsRepository = mock(); const workflowRepository = mock(); const credentialsRepository = mock(); + const authUserRepository = mock(); const hooksService = new HooksService( userService, authService, @@ -27,6 +29,7 @@ describe('HooksService', () => { settingsRepository, workflowRepository, credentialsRepository, + authUserRepository, ); beforeEach(() => { @@ -55,7 +58,7 @@ describe('HooksService', () => { expect(authService.issueCookie).toHaveBeenCalledWith(res, mockedUser); }); - it('hooksService.findOneUser should call userRepository.findOne', async () => { + it('hooksService.findOneUser should call authUserRepository.findOne', async () => { // ARRANGE const filter = { where: { id: '1' } }; @@ -63,7 +66,7 @@ describe('HooksService', () => { await hooksService.findOneUser(filter); // ASSERT - expect(userRepository.findOne).toHaveBeenCalledWith(filter); + expect(authUserRepository.findOne).toHaveBeenCalledWith(filter); }); it('hooksService.saveUser should call userRepository.save', async () => {