From 1b5f9b220f59cc58016ca8cd3a33ae9af44758fc Mon Sep 17 00:00:00 2001 From: Andreas Fitzek Date: Mon, 1 Sep 2025 17:40:49 +0200 Subject: [PATCH] chore(core): Introduce license feature flag for custom roles (#19038) --- .../@n8n/backend-common/src/license-state.ts | 6 +- packages/@n8n/constants/src/index.ts | 1 + .../cli/src/controllers/e2e.controller.ts | 1 + .../cli/src/controllers/role.controller.ts | 5 + packages/cli/src/services/role.service.ts | 7 +- .../controllers/role.controller.test.ts | 103 ++++++++++++++++++ .../project.service.integration.test.ts | 2 + .../services/project.service.test.ts | 13 ++- .../integration/services/role.service.test.ts | 84 ++++++++++---- 9 files changed, 190 insertions(+), 32 deletions(-) diff --git a/packages/@n8n/backend-common/src/license-state.ts b/packages/@n8n/backend-common/src/license-state.ts index 76a2ae904a..ca226c064c 100644 --- a/packages/@n8n/backend-common/src/license-state.ts +++ b/packages/@n8n/backend-common/src/license-state.ts @@ -1,5 +1,5 @@ import type { BooleanLicenseFeature } from '@n8n/constants'; -import { UNLIMITED_LICENSE_QUOTA } from '@n8n/constants'; +import { LICENSE_FEATURES, UNLIMITED_LICENSE_QUOTA } from '@n8n/constants'; import { Service } from '@n8n/di'; import { UnexpectedError } from 'n8n-workflow'; @@ -43,6 +43,10 @@ export class LicenseState { // booleans // -------------------- + isCustomRolesLicensed() { + return this.isLicensed(LICENSE_FEATURES.CUSTOM_ROLES); + } + isSharingLicensed() { return this.isLicensed('feat:sharing'); } diff --git a/packages/@n8n/constants/src/index.ts b/packages/@n8n/constants/src/index.ts index 1042c57523..00b10ae984 100644 --- a/packages/@n8n/constants/src/index.ts +++ b/packages/@n8n/constants/src/index.ts @@ -36,6 +36,7 @@ export const LICENSE_FEATURES = { INSIGHTS_VIEW_HOURLY_DATA: 'feat:insights:viewHourlyData', API_KEY_SCOPES: 'feat:apiKeyScopes', WORKFLOW_DIFFS: 'feat:workflowDiffs', + CUSTOM_ROLES: 'feat:customRoles', } as const; export const LICENSE_QUOTAS = { diff --git a/packages/cli/src/controllers/e2e.controller.ts b/packages/cli/src/controllers/e2e.controller.ts index 228ae221f3..22cd88ac09 100644 --- a/packages/cli/src/controllers/e2e.controller.ts +++ b/packages/cli/src/controllers/e2e.controller.ts @@ -115,6 +115,7 @@ export class E2EController { [LICENSE_FEATURES.OIDC]: false, [LICENSE_FEATURES.MFA_ENFORCEMENT]: false, [LICENSE_FEATURES.WORKFLOW_DIFFS]: false, + [LICENSE_FEATURES.CUSTOM_ROLES]: false, }; private static readonly numericFeaturesDefaults: Record = { diff --git a/packages/cli/src/controllers/role.controller.ts b/packages/cli/src/controllers/role.controller.ts index 7ccdb0e579..ae45857e9d 100644 --- a/packages/cli/src/controllers/role.controller.ts +++ b/packages/cli/src/controllers/role.controller.ts @@ -1,9 +1,11 @@ import { CreateRoleDto, UpdateRoleDto } from '@n8n/api-types'; +import { LICENSE_FEATURES } from '@n8n/constants'; import { Body, Delete, Get, GlobalScope, + Licensed, Param, Patch, Post, @@ -35,18 +37,21 @@ export class RoleController { @Patch('/:slug') @GlobalScope('role:manage') + @Licensed(LICENSE_FEATURES.CUSTOM_ROLES) async updateRole(@Param('slug') slug: string, @Body body: UpdateRoleDto): Promise { return await this.roleService.updateCustomRole(slug, body); } @Delete('/:slug') @GlobalScope('role:manage') + @Licensed(LICENSE_FEATURES.CUSTOM_ROLES) async deleteRole(@Param('slug') slug: string): Promise { return await this.roleService.removeCustomRole(slug); } @Post('/') @GlobalScope('role:manage') + @Licensed(LICENSE_FEATURES.CUSTOM_ROLES) async createRole(@Body body: CreateRoleDto): Promise { return await this.roleService.createCustomRole(body); } diff --git a/packages/cli/src/services/role.service.ts b/packages/cli/src/services/role.service.ts index 2fc448c7f5..74e7f9e296 100644 --- a/packages/cli/src/services/role.service.ts +++ b/packages/cli/src/services/role.service.ts @@ -1,4 +1,5 @@ import { CreateRoleDto, UpdateRoleDto } from '@n8n/api-types'; +import { LicenseState } from '@n8n/backend-common'; import { CredentialsEntity, SharedCredentials, @@ -28,12 +29,11 @@ import { UnexpectedError, UserError } from 'n8n-workflow'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; -import { License } from '@/license'; @Service() export class RoleService { constructor( - private readonly license: License, + private readonly license: LicenseState, private readonly roleRepository: RoleRepository, private readonly scopeRepository: ScopeRepository, ) {} @@ -238,8 +238,7 @@ export class RoleService { if (!isBuiltInRole(role)) { // This is a custom role, there for we need to check if // custom roles are licensed - // TODO: add license check for custom roles - return true; + return this.license.isCustomRolesLicensed(); } switch (role) { diff --git a/packages/cli/test/integration/controllers/role.controller.test.ts b/packages/cli/test/integration/controllers/role.controller.test.ts index 2ce2d3967e..4fbffc972c 100644 --- a/packages/cli/test/integration/controllers/role.controller.test.ts +++ b/packages/cli/test/integration/controllers/role.controller.test.ts @@ -25,6 +25,8 @@ describe('RoleController', () => { beforeEach(() => { jest.clearAllMocks(); + // Enable CUSTOM_ROLES license for all tests by default + testServer.license.enable('feat:customRoles'); }); afterEach(async () => { @@ -626,4 +628,105 @@ describe('RoleController', () => { expect(response.body).toEqual({ data: mockRole }); }); }); + + describe('License enforcement for @Licensed(LICENSE_FEATURES.CUSTOM_ROLES)', () => { + describe('POST /roles', () => { + it('should return 403 when CUSTOM_ROLES license is disabled', async () => { + // + // ARRANGE + // + testServer.license.disable('feat:customRoles'); + + const createRoleDto = { + displayName: 'Test Role', + roleType: 'project' as const, + scopes: ['workflow:read'], + }; + + // + // ACT & ASSERT + // + await ownerAgent.post('/roles').send(createRoleDto).expect(403); + + // Verify service method was not called due to license check + expect(roleService.createCustomRole).not.toHaveBeenCalled(); + }); + }); + + describe('PATCH /roles/:slug', () => { + it('should return 403 when CUSTOM_ROLES license is disabled', async () => { + // + // ARRANGE + // + testServer.license.disable('feat:customRoles'); + + const roleSlug = 'project:test-role'; + const updateRoleDto = { + displayName: 'Updated Role', + scopes: ['workflow:read', 'workflow:edit'], + }; + + // + // ACT & ASSERT + // + await ownerAgent.patch(`/roles/${roleSlug}`).send(updateRoleDto).expect(403); + + // Verify service method was not called due to license check + expect(roleService.updateCustomRole).not.toHaveBeenCalled(); + }); + }); + + describe('DELETE /roles/:slug', () => { + it('should return 403 when CUSTOM_ROLES license is disabled', async () => { + // + // ARRANGE + // + testServer.license.disable('feat:customRoles'); + + const roleSlug = 'project:test-role'; + + // + // ACT & ASSERT + // + await ownerAgent.delete(`/roles/${roleSlug}`).expect(403); + + // Verify service method was not called due to license check + expect(roleService.removeCustomRole).not.toHaveBeenCalled(); + }); + }); + + it('should allow non-licensed methods to work when CUSTOM_ROLES is disabled', async () => { + // + // ARRANGE + // + testServer.license.disable('feat:customRoles'); + + const mockRoles = [ + { + slug: 'project:admin', + displayName: 'Project Admin', + description: 'Project administrator', + systemRole: true, + roleType: 'project' as const, + scopes: ['project:manage'], + licensed: true, + }, + ]; + + roleService.getAllRoles.mockResolvedValue(mockRoles); + + // + // ACT & ASSERT + // + // GET /roles should work (no @Licensed decorator) + await ownerAgent.get('/roles').expect(200); + expect(roleService.getAllRoles).toHaveBeenCalledTimes(1); + + // GET /roles/:slug should work (no @Licensed decorator) + const mockRole = mockRoles[0]; + roleService.getRole.mockResolvedValue(mockRole); + await ownerAgent.get('/roles/project:admin').expect(200); + expect(roleService.getRole).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/packages/cli/test/integration/project.service.integration.test.ts b/packages/cli/test/integration/project.service.integration.test.ts index a12b48725c..fe83dd03d3 100644 --- a/packages/cli/test/integration/project.service.integration.test.ts +++ b/packages/cli/test/integration/project.service.integration.test.ts @@ -13,6 +13,7 @@ import { ProjectService } from '@/services/project.service.ee'; import { LicenseMocker } from '@test-integration/license'; import { createUser } from './shared/db/users'; +import { LicenseState } from '@n8n/backend-common'; describe('ProjectService', () => { let projectService: ProjectService; @@ -26,6 +27,7 @@ describe('ProjectService', () => { const license: LicenseMocker = new LicenseMocker(); license.mock(Container.get(License)); + license.mockLicenseState(Container.get(LicenseState)); license.enable('feat:projectRole:editor'); }); diff --git a/packages/cli/test/integration/services/project.service.test.ts b/packages/cli/test/integration/services/project.service.test.ts index 79d109fba8..3a1cdb3c1c 100644 --- a/packages/cli/test/integration/services/project.service.test.ts +++ b/packages/cli/test/integration/services/project.service.test.ts @@ -1,3 +1,4 @@ +import { LicenseState } from '@n8n/backend-common'; import { testDb } from '@n8n/backend-test-utils'; import { ProjectRelationRepository, ProjectRepository } from '@n8n/db'; import { Container } from '@n8n/di'; @@ -8,11 +9,11 @@ import { ProjectService } from '@/services/project.service.ee'; import { createRole } from '@test-integration/db/roles'; import { createMember } from '../shared/db/users'; +import { LicenseMocker } from '@test-integration/license'; let projectRepository: ProjectRepository; let projectService: ProjectService; let projectRelationRepository: ProjectRelationRepository; -let license: License; beforeAll(async () => { await testDb.init(); @@ -20,7 +21,12 @@ beforeAll(async () => { projectRepository = Container.get(ProjectRepository); projectService = Container.get(ProjectService); projectRelationRepository = Container.get(ProjectRelationRepository); - license = Container.get(License); + const license: LicenseMocker = new LicenseMocker(); + license.mock(Container.get(License)); + license.mockLicenseState(Container.get(LicenseState)); + license.enable('feat:projectRole:editor'); + license.enable('feat:projectRole:viewer'); + license.enable('feat:customRoles'); }); afterAll(async () => { @@ -287,7 +293,6 @@ describe('ProjectService', () => { type: 'team', }), ); - jest.spyOn(license, 'isProjectRoleEditorLicensed').mockReturnValue(true); // // ACT @@ -342,7 +347,6 @@ describe('ProjectService', () => { type: 'team', }), ); - jest.spyOn(license, 'isProjectRoleEditorLicensed').mockReturnValue(true); // // ACT @@ -371,7 +375,6 @@ describe('ProjectService', () => { type: 'team', }), ); - jest.spyOn(license, 'isProjectRoleEditorLicensed').mockReturnValue(true); // // ACT diff --git a/packages/cli/test/integration/services/role.service.test.ts b/packages/cli/test/integration/services/role.service.test.ts index c32baefc6b..4d7530599b 100644 --- a/packages/cli/test/integration/services/role.service.test.ts +++ b/packages/cli/test/integration/services/role.service.test.ts @@ -1,4 +1,5 @@ import type { CreateRoleDto, UpdateRoleDto } from '@n8n/api-types'; +import { LicenseState } from '@n8n/backend-common'; import { testDb } from '@n8n/backend-test-utils'; import { RoleRepository } from '@n8n/db'; import { Container } from '@n8n/di'; @@ -21,6 +22,7 @@ import { createMember } from '../shared/db/users'; let roleService: RoleService; let roleRepository: RoleRepository; let license: License; +let licenseState: LicenseState; const ALL_ROLES_SET = ALL_ROLES.global.concat( ALL_ROLES.project, @@ -34,6 +36,8 @@ beforeAll(async () => { roleService = Container.get(RoleService); roleRepository = Container.get(RoleRepository); license = Container.get(License); + licenseState = Container.get(LicenseState); + licenseState.setLicenseProvider(license); }); afterAll(async () => { @@ -504,30 +508,62 @@ describe('RoleService', () => { { role: 'project:editor', licenseMethod: 'isProjectRoleEditorLicensed' }, { role: 'project:viewer', licenseMethod: 'isProjectRoleViewerLicensed' }, { role: 'global:admin', licenseMethod: 'isAdvancedPermissionsLicensed' }, - ] as const)('should check license for built-in role $role', async ({ role, licenseMethod }) => { - // - // ARRANGE - // - const mockLicenseResult = Math.random() > 0.5; // Random boolean - jest.spyOn(license, licenseMethod).mockReturnValue(mockLicenseResult); + ] as const)( + 'should pass license check for built-in role $role', + async ({ role, licenseMethod }) => { + // + // ARRANGE + // + const mockLicenseResult = true; + jest.spyOn(licenseState, licenseMethod).mockReturnValue(mockLicenseResult); - // - // ACT - // - const result = roleService.isRoleLicensed(role); + // + // ACT + // + const result = roleService.isRoleLicensed(role); - // - // ASSERT - // - expect(result).toBe(mockLicenseResult); - expect(license[licenseMethod]).toHaveBeenCalledTimes(1); - }); + // + // ASSERT + // + expect(result).toBe(mockLicenseResult); + expect(licenseState[licenseMethod]).toHaveBeenCalledTimes(1); + }, + ); - it('should return true for custom roles', async () => { + it.each([ + { role: 'project:admin', licenseMethod: 'isProjectRoleAdminLicensed' }, + { role: 'project:editor', licenseMethod: 'isProjectRoleEditorLicensed' }, + { role: 'project:viewer', licenseMethod: 'isProjectRoleViewerLicensed' }, + { role: 'global:admin', licenseMethod: 'isAdvancedPermissionsLicensed' }, + ] as const)( + 'should fail license state check for built-in role $role', + async ({ role, licenseMethod }) => { + // + // ARRANGE + // + const mockLicenseResult = false; + jest.spyOn(licenseState, licenseMethod).mockReturnValue(mockLicenseResult); + + // + // ACT + // + const result = roleService.isRoleLicensed(role); + + // + // ASSERT + // + expect(result).toBe(mockLicenseResult); + expect(licenseState[licenseMethod]).toHaveBeenCalledTimes(1); + }, + ); + + it('should return true for custom roles if licensed', async () => { // // ARRANGE // const customRoleSlug = 'custom:test-role'; + const mockLicenseResult = true; // Random boolean + jest.spyOn(licenseState, 'isCustomRolesLicensed').mockReturnValue(mockLicenseResult); // // ACT @@ -537,24 +573,28 @@ describe('RoleService', () => { // // ASSERT // - expect(result).toBe(true); + expect(result).toBe(mockLicenseResult); + expect(licenseState.isCustomRolesLicensed).toHaveBeenCalledTimes(1); }); - it('should return true for unknown role types', async () => { + it('should return false for custom roles if not licensed', async () => { // // ARRANGE // - const unknownRole = 'unknown:role' as any; + const customRoleSlug = 'custom:test-role'; + const mockLicenseResult = false; // Random boolean + jest.spyOn(licenseState, 'isCustomRolesLicensed').mockReturnValue(mockLicenseResult); // // ACT // - const result = roleService.isRoleLicensed(unknownRole); + const result = roleService.isRoleLicensed(customRoleSlug as any); // // ASSERT // - expect(result).toBe(true); + expect(result).toBe(mockLicenseResult); + expect(licenseState.isCustomRolesLicensed).toHaveBeenCalledTimes(1); }); });