feat: Disable manual role management when expression-based mapping is enabled (#28105)

This commit is contained in:
Stephen Wright 2026-04-09 08:29:32 +01:00 committed by GitHub
parent cc32c507c5
commit 26d578dfc8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 177 additions and 13 deletions

View File

@ -5,6 +5,7 @@ import { mock } from 'jest-mock-extended';
import type { EventService } from '@/events/event.service';
import type { Response } from 'express';
import { ProjectController } from '@/controllers/project.controller';
import type { ProvisioningService } from '@/modules/provisioning.ee/provisioning.service.ee';
import type { ProjectService } from '@/services/project.service.ee';
import type { UserManagementMailer } from '@/user-management/email';
@ -13,12 +14,14 @@ describe('ProjectController', () => {
const projectsService = mock<ProjectService>();
const projectRepository = mock<ProjectRepository>();
const userManagementMailer = mock<UserManagementMailer>();
const provisioningService = mock<ProvisioningService>();
const controller = new ProjectController(
projectsService as unknown as ProjectService,
projectRepository as unknown as ProjectRepository,
eventService as unknown as EventService,
userManagementMailer as unknown as UserManagementMailer,
provisioningService as unknown as ProvisioningService,
);
const makeRes = () => {
@ -118,6 +121,7 @@ describe('ProjectController', () => {
it('emits team-project-updated on changeProjectUserRole and returns 204', async () => {
// Arrange
const projectId = 'p2';
provisioningService.isProjectRoleManaged.mockResolvedValue(false);
(projectsService.getProjectRelations as jest.Mock).mockResolvedValue([
{ userId: 'u1', role: { slug: 'project:admin' } },
{ userId: 'u2', role: { slug: 'project:editor' } },

View File

@ -4,6 +4,7 @@ import type { Response } from 'express';
import type { EventService } from '@/events/event.service';
import type { JwtService } from '@/services/jwt.service';
import type { ProvisioningService } from '@/modules/provisioning.ee/provisioning.service.ee';
import type { UrlService } from '@/services/url.service';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
@ -14,6 +15,7 @@ describe('UsersController', () => {
const userRepository = mock<UserRepository>();
const jwtService = mock<JwtService>();
const urlService = mock<UrlService>();
const provisioningService = mock<ProvisioningService>();
const controller = new UsersController(
mock(),
@ -30,6 +32,7 @@ describe('UsersController', () => {
mock(),
jwtService,
urlService,
provisioningService,
);
beforeEach(() => {
@ -43,6 +46,7 @@ describe('UsersController', () => {
user: { id: '123' },
});
userRepository.findOne.mockResolvedValue(mock<User>({ id: '456' }));
provisioningService.isInstanceRoleManaged.mockResolvedValue(false);
await controller.changeGlobalRole(
request,

View File

@ -27,8 +27,10 @@ import { In, Not } from '@n8n/typeorm';
import { Response } from 'express';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { EventService } from '@/events/event.service';
import { ProvisioningService } from '@/modules/provisioning.ee/provisioning.service.ee';
import type { ProjectRequest } from '@/requests';
import {
ProjectService,
@ -44,6 +46,7 @@ export class ProjectController {
private readonly projectRepository: ProjectRepository,
private readonly eventService: EventService,
private readonly userManagementMailer: UserManagementMailer,
private readonly provisioningService: ProvisioningService,
) {}
@Get('/')
@ -293,6 +296,12 @@ export class ProjectController {
@Param('userId') userId: string,
@Body body: ChangeUserRoleInProject,
) {
if (await this.provisioningService.isProjectRoleManaged()) {
throw new ForbiddenError(
'Project roles are managed automatically and cannot be changed manually',
);
}
try {
await this.projectsService.changeUserRoleInProject(projectId, userId, body.role);
const relations = await this.projectsService.getProjectRelations(projectId);

View File

@ -42,6 +42,7 @@ import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { EventService } from '@/events/event.service';
import { ExternalHooks } from '@/external-hooks';
import { ProvisioningService } from '@/modules/provisioning.ee/provisioning.service.ee';
import { UserRequest } from '@/requests';
import { FolderService } from '@/services/folder.service';
import { UserService } from '@/services/user.service';
@ -66,6 +67,7 @@ export class UsersController {
private readonly folderService: FolderService,
private readonly jwtService: JwtService,
private readonly urlService: UrlService,
private readonly provisioningService: ProvisioningService,
) {}
static ERROR_MESSAGES = {
@ -342,6 +344,12 @@ export class UsersController {
@Body payload: RoleChangeRequestDto,
@Param('id') id: string,
) {
if (await this.provisioningService.isInstanceRoleManaged()) {
throw new ForbiddenError(
'Instance roles are managed automatically and cannot be changed manually',
);
}
const { NO_ADMIN_ON_OWNER, NO_USER, NO_OWNER_ON_OWNER } =
UsersController.ERROR_MESSAGES.CHANGE_ROLE;

View File

@ -411,6 +411,18 @@ export class ProvisioningService {
return provisioningConfig.scopesUseExpressionMapping;
}
async isInstanceRoleManaged(): Promise<boolean> {
return (
(await this.isInstanceRoleProvisioningEnabled()) || (await this.isExpressionMappingEnabled())
);
}
async isProjectRoleManaged(): Promise<boolean> {
return (
(await this.isProjectRolesProvisioningEnabled()) || (await this.isExpressionMappingEnabled())
);
}
private async buildRoleMappingConfig(): Promise<RoleMappingConfig> {
const dbRules = await this.roleMappingRuleRepository.find({
relations: ['role', 'projects'],

View File

@ -38,6 +38,7 @@ import { createMember, createOwner, createUser } from './shared/db/users';
import * as utils from './shared/utils/';
import { ActiveWorkflowManager } from '@/active-workflow-manager';
import { ProvisioningService } from '@/modules/provisioning.ee/provisioning.service.ee';
import { getWorkflowById } from '@/public-api/v1/handlers/workflows/workflows.service';
const testServer = utils.setupTestServer({
@ -218,6 +219,57 @@ describe('Project members endpoints', () => {
);
});
describe('PATCH /projects/:projectId/users/:userId when project roles are managed by provisioning', () => {
let provisioningService: ProvisioningService;
let savedConfig: Record<string, unknown>;
beforeEach(async () => {
provisioningService = Container.get(ProvisioningService);
await provisioningService.getConfig();
// @ts-expect-error - provisioningConfig is private
savedConfig = { ...provisioningService.provisioningConfig };
});
afterEach(() => {
// @ts-expect-error - provisioningConfig is private
provisioningService.provisioningConfig = { ...savedConfig };
delete process.env.N8N_ENV_FEAT_ROLE_MAPPING_STRATEGY;
});
test('should return 403 when SSO provider controls project roles', async () => {
// @ts-expect-error - provisioningConfig is private
provisioningService.provisioningConfig.scopesProvisionProjectRoles = true;
const owner = await createOwner();
const member = await createUser();
const project = await createTeamProject('Team Project', owner);
await linkUserToProject(member, project, 'project:viewer');
const ownerAgent = testServer.authAgentFor(owner);
await ownerAgent
.patch(`/projects/${project.id}/users/${member.id}`)
.send({ role: 'project:editor' })
.expect(403);
});
test('should return 403 when expression-based role mapping is active', async () => {
process.env.N8N_ENV_FEAT_ROLE_MAPPING_STRATEGY = 'true';
// @ts-expect-error - provisioningConfig is private
provisioningService.provisioningConfig.scopesUseExpressionMapping = true;
const owner = await createOwner();
const member = await createUser();
const project = await createTeamProject('Team Project', owner);
await linkUserToProject(member, project, 'project:viewer');
const ownerAgent = testServer.authAgentFor(owner);
await ownerAgent
.patch(`/projects/${project.id}/users/${member.id}`)
.send({ role: 'project:editor' })
.expect(403);
});
});
test('DELETE /projects/:projectId/users/:userId removes a member', async () => {
const owner = await createOwner();
const member = await createUser();

View File

@ -27,6 +27,7 @@ import { v4 as uuid } from 'uuid';
import { RESPONSE_ERROR_MESSAGES } from '@/constants';
import { UsersController } from '@/controllers/users.controller';
import { ExecutionService } from '@/executions/execution.service';
import { ProvisioningService } from '@/modules/provisioning.ee/provisioning.service.ee';
import { OwnershipService } from '@/services/ownership.service';
import { Telemetry } from '@/telemetry';
import { createFolder } from '@test-integration/db/folders';
@ -1712,4 +1713,43 @@ describe('PATCH /users/:id/role', () => {
expect(user.role.slug).toBe(customRole);
});
describe('when instance roles are managed by provisioning', () => {
let provisioningService: ProvisioningService;
let savedConfig: Record<string, unknown>;
beforeEach(async () => {
provisioningService = Container.get(ProvisioningService);
await provisioningService.getConfig();
// @ts-expect-error - provisioningConfig is private
savedConfig = { ...provisioningService.provisioningConfig };
});
afterEach(() => {
// @ts-expect-error - provisioningConfig is private
provisioningService.provisioningConfig = { ...savedConfig };
delete process.env.N8N_ENV_FEAT_ROLE_MAPPING_STRATEGY;
});
test('should return 403 when SSO provider controls instance roles', async () => {
// @ts-expect-error - provisioningConfig is private
provisioningService.provisioningConfig.scopesProvisionInstanceRole = true;
await ownerAgent
.patch(`/users/${member.id}/role`)
.send({ newRoleName: 'global:admin' })
.expect(403);
});
test('should return 403 when expression-based role mapping is active', async () => {
process.env.N8N_ENV_FEAT_ROLE_MAPPING_STRATEGY = 'true';
// @ts-expect-error - provisioningConfig is private
provisioningService.provisioningConfig.scopesUseExpressionMapping = true;
await ownerAgent
.patch(`/users/${member.id}/role`)
.send({ newRoleName: 'global:admin' })
.expect(403);
});
});
});

View File

@ -3126,7 +3126,9 @@
"settings.provisioningConfirmDialog.button.downloadProjectRolesCsv": "Existing project access settings csv",
"settings.provisioningConfirmDialog.button.downloadInstanceRolesCsv": "Existing instance role settings csv",
"settings.provisioningInstanceRolesHandledBySsoProvider.description": "User management and instance roles are controlled by your SSO provider. Contact your n8n instance owner or admin to make changes.",
"settings.provisioningInstanceRolesHandledByExpressionMapping.description": "Instance roles are managed automatically by expression-based role mapping rules. Manual role changes are disabled.",
"settings.provisioningProjectRolesHandledBySsoProvider.description": "User management and project roles are controlled by your SSO provider. Contact your n8n instance owner or admin to make changes.",
"settings.provisioningProjectRolesHandledByExpressionMapping.description": "Project roles are managed automatically by expression-based role mapping rules. Manual role changes are disabled.",
"settings.externalSecrets.title": "External Secrets",
"settings.externalSecrets.info": "Connect external secrets tools for centralized credentials management across environments, and to enhance system security.",
"settings.externalSecrets.info.link": "More info",

View File

@ -7,6 +7,7 @@ export interface ProvisioningConfig {
scopesProjectsRolesClaimName: string;
scopesProvisionInstanceRole: boolean;
scopesProvisionProjectRoles: boolean;
scopesUseExpressionMapping: boolean;
}
export const getProvisioningConfig = async (

View File

@ -116,14 +116,19 @@ const firstLicensedRole = computed(
() => rolesStore.processedProjectRoles.find((role) => role.licensed)?.slug,
);
const projectMembersActions = computed<Array<UserAction<ProjectMemberData>>>(() => [
{
label: i18n.baseText('projects.settings.table.row.removeUser'),
value: 'remove',
guard: (member) =>
member.id !== usersStore.currentUser?.id && member.role !== 'project:personalOwner',
},
]);
const projectMembersActions = computed<Array<UserAction<ProjectMemberData>>>(() => {
if (isProjectRoleProvisioningEnabled.value || isExpressionMappingEnabled.value) {
return [];
}
return [
{
label: i18n.baseText('projects.settings.table.row.removeUser'),
value: 'remove',
guard: (member) =>
member.id !== usersStore.currentUser?.id && member.role !== 'project:personalOwner',
},
];
});
const onAddMember = async (userId: string) => {
if (!projectsStore.currentProject) return;
@ -530,6 +535,10 @@ const isProjectRoleProvisioningEnabled = computed(
() => userRoleProvisioningStore.provisioningConfig?.scopesProvisionProjectRoles || false,
);
const isExpressionMappingEnabled = computed(
() => userRoleProvisioningStore.provisioningConfig?.scopesUseExpressionMapping || false,
);
onMounted(async () => {
documentTitle.set(i18n.baseText('projects.settings'));
@ -637,7 +646,7 @@ onMounted(async () => {
:remote-method="debouncedUserSearch"
:loading="isLoadingUsers"
@update:model-value="onAddMember"
:disabled="isProjectRoleProvisioningEnabled"
:disabled="isProjectRoleProvisioningEnabled || isExpressionMappingEnabled"
>
<template #prefix>
<N8nIcon icon="search" />
@ -657,7 +666,17 @@ onMounted(async () => {
</template>
</N8nInput>
</div>
<div v-if="isProjectRoleProvisioningEnabled" class="mb-m">
<div v-if="isExpressionMappingEnabled" class="mb-m">
<N8nAlert
type="info"
:title="
i18n.baseText(
'settings.provisioningProjectRolesHandledByExpressionMapping.description',
)
"
/>
</div>
<div v-else-if="isProjectRoleProvisioningEnabled" class="mb-m">
<N8nAlert
type="info"
:title="
@ -673,7 +692,7 @@ onMounted(async () => {
:current-user-id="usersStore.currentUser?.id"
:project-roles="rolesStore.processedProjectRoles"
:actions="projectMembersActions"
:can-edit-role="!isProjectRoleProvisioningEnabled"
:can-edit-role="!isProjectRoleProvisioningEnabled && !isExpressionMappingEnabled"
@update:options="onUpdateMembersTableOptions"
@update:role="onUpdateMemberRole"
@action="onMembersListAction"

View File

@ -29,6 +29,7 @@ describe('useUserRoleProvisioningForm', () => {
scopesProjectsRolesClaimName: 'n8n_projects',
scopesProvisionInstanceRole: false,
scopesProvisionProjectRoles: false,
scopesUseExpressionMapping: false,
};
return { ...defaultConfig, ...config };
};

View File

@ -76,6 +76,10 @@ const isInstanceRoleProvisioningEnabled = computed(
() => userRoleProvisioningStore.provisioningConfig?.scopesProvisionInstanceRole || false,
);
const isExpressionMappingEnabled = computed(
() => userRoleProvisioningStore.provisioningConfig?.scopesUseExpressionMapping || false,
);
const isSSOEnabled = computed(() => !!ssoStore.isSamlLoginEnabled || !!ssoStore.isOidcLoginEnabled);
onMounted(async () => {
@ -462,7 +466,15 @@ const onSearch = (value: string) => {
</template>
</I18nT>
</N8nNotice>
<div v-if="isInstanceRoleProvisioningEnabled" :class="$style.container">
<div v-if="isExpressionMappingEnabled" :class="$style.container">
<N8nAlert
type="info"
:title="
i18n.baseText('settings.provisioningInstanceRolesHandledByExpressionMapping.description')
"
/>
</div>
<div v-else-if="isInstanceRoleProvisioningEnabled" :class="$style.container">
<N8nAlert
type="info"
:title="i18n.baseText('settings.provisioningInstanceRolesHandledBySsoProvider.description')"
@ -507,7 +519,7 @@ const onSearch = (value: string) => {
<SettingsUsersTable
v-model:table-options="usersTableState"
data-test-id="settings-users-table"
:can-edit-role="!isInstanceRoleProvisioningEnabled"
:can-edit-role="!isInstanceRoleProvisioningEnabled && !isExpressionMappingEnabled"
:data="usersStore.usersList.state"
:loading="usersStore.usersList.isLoading"
:updating-role-user-id="updatingRoleUserId"