From 7d3f388d8fc111db141dda03eccf815a5c841398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Fri, 15 Dec 2023 17:55:58 +0100 Subject: [PATCH 1/4] refactor: Add telemetry for RBAC --- packages/cli/src/services/user.service.ts | 19 +++++++++++++++++++ packages/cli/src/telemetry/index.ts | 2 ++ packages/editor-ui/src/Interface.ts | 1 + .../src/components/InviteUsersModal.vue | 2 +- 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/services/user.service.ts b/packages/cli/src/services/user.service.ts index 41037f26c27c1..02fab23e4b9e5 100644 --- a/packages/cli/src/services/user.service.ts +++ b/packages/cli/src/services/user.service.ts @@ -17,6 +17,7 @@ import { RoleService } from '@/services/role.service'; import { ApplicationError, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; import type { UserRequest } from '@/requests'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; +import { RoleRepository } from '@/databases/repositories/role.repository'; @Service() export class UserService { @@ -307,4 +308,22 @@ export class UserService { return { usersInvited, usersCreated: toCreateUsers.map(({ email }) => email) }; } + + /** + * Counts the number of users in each role, e.g. `{ admin: 2, member: 6, owner: 1 }` + */ + async countUsersByRole() { + const result: Array<{ role_name: string; count: number }> = await Container.get(RoleRepository) + .createQueryBuilder('role') + .select('role.name') + .addSelect('COUNT(user.id)', 'count') + .innerJoin('user', 'user', 'role.id = user.globalRoleId') + .groupBy('role.name') + .getRawMany(); + + return result.reduce>((acc, item) => { + acc[item.role_name] = item.count; + return acc; + }, {}); + } } diff --git a/packages/cli/src/telemetry/index.ts b/packages/cli/src/telemetry/index.ts index bbd8cfcbeb8e8..2836d119f6257 100644 --- a/packages/cli/src/telemetry/index.ts +++ b/packages/cli/src/telemetry/index.ts @@ -10,6 +10,7 @@ import { LicenseService } from '@/license/License.service'; import { N8N_VERSION } from '@/constants'; import { SourceControlPreferencesService } from '../environments/sourceControl/sourceControlPreferences.service.ee'; import { InstanceSettings } from 'n8n-core'; +import { UserService } from '@/services/user.service'; type ExecutionTrackDataKey = 'manual_error' | 'manual_success' | 'prod_error' | 'prod_success'; @@ -108,6 +109,7 @@ export class Telemetry { plan_name_current: this.license.getPlanName(), quota: this.license.getTriggerLimit(), usage: await LicenseService.getActiveTriggerCount(), + role_count: await Container.get(UserService).countUsersByRole(), source_control_set_up: Container.get(SourceControlPreferencesService).isSourceControlSetup(), branchName: sourceControlPreferences.branchName, read_only_instance: sourceControlPreferences.branchReadOnly, diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index 37beb90ebfe33..cfbe7ae5200a6 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -1766,6 +1766,7 @@ export interface ExternalSecretsProviderWithProperties extends ExternalSecretsPr } export type CloudUpdateLinkSourceType = + | 'advanced-permissions' | 'canvas-nav' | 'custom-data-filter' | 'workflow_sharing' diff --git a/packages/editor-ui/src/components/InviteUsersModal.vue b/packages/editor-ui/src/components/InviteUsersModal.vue index 3145a91d1bdda..d8844697e949e 100644 --- a/packages/editor-ui/src/components/InviteUsersModal.vue +++ b/packages/editor-ui/src/components/InviteUsersModal.vue @@ -333,7 +333,7 @@ export default defineComponent({ } }, goToUpgradeAdvancedPermissions() { - void this.uiStore.goToUpgrade('settings-users', 'upgrade-advanced-permissions'); + void this.uiStore.goToUpgrade('advanced-permissions', 'upgrade-advanced-permissions'); }, }, }); From c3c02469f2ff44eb30bef80b54ab2345faa0084f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 18 Dec 2023 17:41:34 +0100 Subject: [PATCH 2/4] Move to `RoleService` and add tests --- packages/cli/src/services/role.service.ts | 18 +++++++ packages/cli/src/services/user.service.ts | 18 ------- packages/cli/src/telemetry/index.ts | 4 +- .../cli/test/integration/role.service.test.ts | 48 +++++++++++++++++++ 4 files changed, 68 insertions(+), 20 deletions(-) create mode 100644 packages/cli/test/integration/role.service.test.ts diff --git a/packages/cli/src/services/role.service.ts b/packages/cli/src/services/role.service.ts index 5b81b8b99ffc3..3d2b68bfb8cc2 100644 --- a/packages/cli/src/services/role.service.ts +++ b/packages/cli/src/services/role.service.ts @@ -100,4 +100,22 @@ export class RoleService { }) .then((shared) => shared?.role); } + + /** + * Counts the number of users in each role, e.g. `{ admin: 2, member: 6, owner: 1 }` + */ + async countUsersByRole() { + const result: Array<{ role_name: string; count: number }> = await this.roleRepository + .createQueryBuilder('role') + .select('role.name') + .addSelect('COUNT(user.id)', 'count') + .innerJoin('user', 'user', 'role.id = user.globalRoleId') + .groupBy('role.name') + .getRawMany(); + + return result.reduce>((acc, item) => { + acc[item.role_name] = item.count; + return acc; + }, {}); + } } diff --git a/packages/cli/src/services/user.service.ts b/packages/cli/src/services/user.service.ts index 02fab23e4b9e5..9cbf4b9f7b7ed 100644 --- a/packages/cli/src/services/user.service.ts +++ b/packages/cli/src/services/user.service.ts @@ -308,22 +308,4 @@ export class UserService { return { usersInvited, usersCreated: toCreateUsers.map(({ email }) => email) }; } - - /** - * Counts the number of users in each role, e.g. `{ admin: 2, member: 6, owner: 1 }` - */ - async countUsersByRole() { - const result: Array<{ role_name: string; count: number }> = await Container.get(RoleRepository) - .createQueryBuilder('role') - .select('role.name') - .addSelect('COUNT(user.id)', 'count') - .innerJoin('user', 'user', 'role.id = user.globalRoleId') - .groupBy('role.name') - .getRawMany(); - - return result.reduce>((acc, item) => { - acc[item.role_name] = item.count; - return acc; - }, {}); - } } diff --git a/packages/cli/src/telemetry/index.ts b/packages/cli/src/telemetry/index.ts index 2836d119f6257..caa240d9054d7 100644 --- a/packages/cli/src/telemetry/index.ts +++ b/packages/cli/src/telemetry/index.ts @@ -10,7 +10,7 @@ import { LicenseService } from '@/license/License.service'; import { N8N_VERSION } from '@/constants'; import { SourceControlPreferencesService } from '../environments/sourceControl/sourceControlPreferences.service.ee'; import { InstanceSettings } from 'n8n-core'; -import { UserService } from '@/services/user.service'; +import { RoleService } from '@/services/role.service'; type ExecutionTrackDataKey = 'manual_error' | 'manual_success' | 'prod_error' | 'prod_success'; @@ -109,7 +109,7 @@ export class Telemetry { plan_name_current: this.license.getPlanName(), quota: this.license.getTriggerLimit(), usage: await LicenseService.getActiveTriggerCount(), - role_count: await Container.get(UserService).countUsersByRole(), + role_count: await Container.get(RoleService).countUsersByRole(), source_control_set_up: Container.get(SourceControlPreferencesService).isSourceControlSetup(), branchName: sourceControlPreferences.branchName, read_only_instance: sourceControlPreferences.branchReadOnly, diff --git a/packages/cli/test/integration/role.service.test.ts b/packages/cli/test/integration/role.service.test.ts new file mode 100644 index 0000000000000..e473a8fb6b1b1 --- /dev/null +++ b/packages/cli/test/integration/role.service.test.ts @@ -0,0 +1,48 @@ +import { RoleService } from '@/services/role.service'; +import { createAdmin, createMember, createOwner } from './shared/db/users'; +import * as testDb from './shared/testDb'; +import { mock } from 'jest-mock-extended'; +import { RoleRepository } from '@/databases/repositories/role.repository'; +import Container from 'typedi'; +import { SharedWorkflowRepository } from '@/databases/repositories/sharedWorkflow.repository'; + +describe('RoleService', () => { + let roleService: RoleService; + + beforeAll(async () => { + await testDb.init(); + + roleService = new RoleService( + Container.get(RoleRepository), + Container.get(SharedWorkflowRepository), + mock(), + ); + + testDb.truncate(['User']); + }); + + afterEach(async () => { + await testDb.truncate(['SharedWorkflow']); + }); + + afterAll(async () => { + await testDb.terminate(); + }); + + describe('countUsersByRole()', () => { + test('should return the number of users in each role', async () => { + await Promise.all([ + createOwner(), + createAdmin(), + createAdmin(), + createMember(), + createMember(), + createMember(), + ]); + + const usersByRole = await roleService.countUsersByRole(); + + expect(usersByRole).toStrictEqual({ admin: 2, member: 3, owner: 1 }); + }); + }); +}); From 3c5538a4d8ed561df7c7b951aea6c8e4309d64a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 18 Dec 2023 18:30:03 +0100 Subject: [PATCH 3/4] Fix lint --- packages/cli/src/services/user.service.ts | 1 - packages/cli/test/integration/role.service.test.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/cli/src/services/user.service.ts b/packages/cli/src/services/user.service.ts index 9cbf4b9f7b7ed..41037f26c27c1 100644 --- a/packages/cli/src/services/user.service.ts +++ b/packages/cli/src/services/user.service.ts @@ -17,7 +17,6 @@ import { RoleService } from '@/services/role.service'; import { ApplicationError, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; import type { UserRequest } from '@/requests'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; -import { RoleRepository } from '@/databases/repositories/role.repository'; @Service() export class UserService { diff --git a/packages/cli/test/integration/role.service.test.ts b/packages/cli/test/integration/role.service.test.ts index e473a8fb6b1b1..27534bd54a399 100644 --- a/packages/cli/test/integration/role.service.test.ts +++ b/packages/cli/test/integration/role.service.test.ts @@ -18,7 +18,7 @@ describe('RoleService', () => { mock(), ); - testDb.truncate(['User']); + await testDb.truncate(['User']); }); afterEach(async () => { From f02a38788c999edcd8bc02a9e16e768f93a4b1f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 19 Dec 2023 16:24:56 +0100 Subject: [PATCH 4/4] Address feedback --- .../databases/repositories/role.repository.ts | 19 +++++++++++++++++++ packages/cli/src/services/role.service.ts | 18 ------------------ packages/cli/src/telemetry/index.ts | 4 ++-- ...ervice.test.ts => role.repository.test.ts} | 19 ++++--------------- 4 files changed, 25 insertions(+), 35 deletions(-) rename packages/cli/test/integration/{role.service.test.ts => role.repository.test.ts} (58%) diff --git a/packages/cli/src/databases/repositories/role.repository.ts b/packages/cli/src/databases/repositories/role.repository.ts index a78efadf10d72..8ecee595c1564 100644 --- a/packages/cli/src/databases/repositories/role.repository.ts +++ b/packages/cli/src/databases/repositories/role.repository.ts @@ -12,4 +12,23 @@ export class RoleRepository extends Repository { async findRole(scope: RoleScopes, name: RoleNames) { return this.findOne({ where: { scope, name } }); } + + /** + * Counts the number of users in each role, e.g. `{ admin: 2, member: 6, owner: 1 }` + */ + async countUsersByRole() { + type Row = { role_name: string; count: number }; + + const rows: Row[] = await this.createQueryBuilder('role') + .select('role.name') + .addSelect('COUNT(user.id)', 'count') + .innerJoin('user', 'user', 'role.id = user.globalRoleId') + .groupBy('role.name') + .getRawMany(); + + return rows.reduce>((acc, item) => { + acc[item.role_name] = item.count; + return acc; + }, {}); + } } diff --git a/packages/cli/src/services/role.service.ts b/packages/cli/src/services/role.service.ts index 3d2b68bfb8cc2..5b81b8b99ffc3 100644 --- a/packages/cli/src/services/role.service.ts +++ b/packages/cli/src/services/role.service.ts @@ -100,22 +100,4 @@ export class RoleService { }) .then((shared) => shared?.role); } - - /** - * Counts the number of users in each role, e.g. `{ admin: 2, member: 6, owner: 1 }` - */ - async countUsersByRole() { - const result: Array<{ role_name: string; count: number }> = await this.roleRepository - .createQueryBuilder('role') - .select('role.name') - .addSelect('COUNT(user.id)', 'count') - .innerJoin('user', 'user', 'role.id = user.globalRoleId') - .groupBy('role.name') - .getRawMany(); - - return result.reduce>((acc, item) => { - acc[item.role_name] = item.count; - return acc; - }, {}); - } } diff --git a/packages/cli/src/telemetry/index.ts b/packages/cli/src/telemetry/index.ts index caa240d9054d7..c4fb66865d0b0 100644 --- a/packages/cli/src/telemetry/index.ts +++ b/packages/cli/src/telemetry/index.ts @@ -10,7 +10,7 @@ import { LicenseService } from '@/license/License.service'; import { N8N_VERSION } from '@/constants'; import { SourceControlPreferencesService } from '../environments/sourceControl/sourceControlPreferences.service.ee'; import { InstanceSettings } from 'n8n-core'; -import { RoleService } from '@/services/role.service'; +import { RoleRepository } from '@/databases/repositories/role.repository'; type ExecutionTrackDataKey = 'manual_error' | 'manual_success' | 'prod_error' | 'prod_success'; @@ -109,7 +109,7 @@ export class Telemetry { plan_name_current: this.license.getPlanName(), quota: this.license.getTriggerLimit(), usage: await LicenseService.getActiveTriggerCount(), - role_count: await Container.get(RoleService).countUsersByRole(), + role_count: await Container.get(RoleRepository).countUsersByRole(), source_control_set_up: Container.get(SourceControlPreferencesService).isSourceControlSetup(), branchName: sourceControlPreferences.branchName, read_only_instance: sourceControlPreferences.branchReadOnly, diff --git a/packages/cli/test/integration/role.service.test.ts b/packages/cli/test/integration/role.repository.test.ts similarity index 58% rename from packages/cli/test/integration/role.service.test.ts rename to packages/cli/test/integration/role.repository.test.ts index 27534bd54a399..04e645928d9bb 100644 --- a/packages/cli/test/integration/role.service.test.ts +++ b/packages/cli/test/integration/role.repository.test.ts @@ -1,30 +1,19 @@ -import { RoleService } from '@/services/role.service'; import { createAdmin, createMember, createOwner } from './shared/db/users'; import * as testDb from './shared/testDb'; -import { mock } from 'jest-mock-extended'; import { RoleRepository } from '@/databases/repositories/role.repository'; import Container from 'typedi'; -import { SharedWorkflowRepository } from '@/databases/repositories/sharedWorkflow.repository'; -describe('RoleService', () => { - let roleService: RoleService; +describe('RoleRepository', () => { + let roleRepository: RoleRepository; beforeAll(async () => { await testDb.init(); - roleService = new RoleService( - Container.get(RoleRepository), - Container.get(SharedWorkflowRepository), - mock(), - ); + roleRepository = Container.get(RoleRepository); await testDb.truncate(['User']); }); - afterEach(async () => { - await testDb.truncate(['SharedWorkflow']); - }); - afterAll(async () => { await testDb.terminate(); }); @@ -40,7 +29,7 @@ describe('RoleService', () => { createMember(), ]); - const usersByRole = await roleService.countUsersByRole(); + const usersByRole = await roleRepository.countUsersByRole(); expect(usersByRole).toStrictEqual({ admin: 2, member: 3, owner: 1 }); });