fix(core): Better input validation for the changeRole endpoint (#8189)

also refactored the code to
1. stop passing around `scope === 'global'`, since this code can be used
only for changing globalRole.
2. leak less details when input validation fails.

## Review / Merge checklist
- [x] PR title and summary are descriptive
- [x] Tests included
This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™ 2024-01-03 09:33:35 +01:00 committed by GitHub
parent 11cda41214
commit cfe9525dd4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 102 additions and 160 deletions

View file

@ -4,7 +4,7 @@ import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
import type { TagEntity } from '@db/entities/TagEntity'; import type { TagEntity } from '@db/entities/TagEntity';
import type { User } from '@db/entities/User'; import type { User } from '@db/entities/User';
import type { UserUpdatePayload } from '@/requests'; import type { UserRoleChangePayload, UserUpdatePayload } from '@/requests';
import { BadRequestError } from './errors/response-errors/bad-request.error'; import { BadRequestError } from './errors/response-errors/bad-request.error';
/** /**
@ -15,7 +15,13 @@ export function getSessionId(req: express.Request): string | undefined {
} }
export async function validateEntity( export async function validateEntity(
entity: WorkflowEntity | CredentialsEntity | TagEntity | User | UserUpdatePayload, entity:
| WorkflowEntity
| CredentialsEntity
| TagEntity
| User
| UserUpdatePayload
| UserRoleChangePayload,
): Promise<void> { ): Promise<void> {
const errors = await validate(entity); const errors = await validate(entity);

View file

@ -2,13 +2,27 @@ import { In } from 'typeorm';
import { User } from '@db/entities/User'; import { User } from '@db/entities/User';
import { SharedCredentials } from '@db/entities/SharedCredentials'; import { SharedCredentials } from '@db/entities/SharedCredentials';
import { SharedWorkflow } from '@db/entities/SharedWorkflow'; import { SharedWorkflow } from '@db/entities/SharedWorkflow';
import { RequireGlobalScope, Authorized, Delete, Get, RestController, Patch } from '@/decorators'; import {
import { ListQuery, UserRequest, UserSettingsUpdatePayload } from '@/requests'; RequireGlobalScope,
Authorized,
Delete,
Get,
RestController,
Patch,
Licensed,
} from '@/decorators';
import {
ListQuery,
UserRequest,
UserRoleChangePayload,
UserSettingsUpdatePayload,
} from '@/requests';
import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner';
import type { PublicUser, ITelemetryUserDeletionData } from '@/Interfaces'; import type { PublicUser, ITelemetryUserDeletionData } from '@/Interfaces';
import { AuthIdentity } from '@db/entities/AuthIdentity'; import { AuthIdentity } from '@db/entities/AuthIdentity';
import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository'; import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository';
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
import { UserRepository } from '@db/repositories/user.repository';
import { plainToInstance } from 'class-transformer'; import { plainToInstance } from 'class-transformer';
import { RoleService } from '@/services/role.service'; import { RoleService } from '@/services/role.service';
import { UserService } from '@/services/user.service'; import { UserService } from '@/services/user.service';
@ -17,10 +31,9 @@ import { Logger } from '@/Logger';
import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error'; import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { License } from '@/License';
import { ExternalHooks } from '@/ExternalHooks'; import { ExternalHooks } from '@/ExternalHooks';
import { InternalHooks } from '@/InternalHooks'; import { InternalHooks } from '@/InternalHooks';
import { UserRepository } from '@/databases/repositories/user.repository'; import { validateEntity } from '@/GenericHelpers';
@Authorized() @Authorized()
@RestController('/users') @RestController('/users')
@ -31,22 +44,17 @@ export class UsersController {
private readonly internalHooks: InternalHooks, private readonly internalHooks: InternalHooks,
private readonly sharedCredentialsRepository: SharedCredentialsRepository, private readonly sharedCredentialsRepository: SharedCredentialsRepository,
private readonly sharedWorkflowRepository: SharedWorkflowRepository, private readonly sharedWorkflowRepository: SharedWorkflowRepository,
private readonly userRepository: UserRepository,
private readonly activeWorkflowRunner: ActiveWorkflowRunner, private readonly activeWorkflowRunner: ActiveWorkflowRunner,
private readonly roleService: RoleService, private readonly roleService: RoleService,
private readonly userService: UserService, private readonly userService: UserService,
private readonly license: License,
private readonly userRepository: UserRepository,
) {} ) {}
static ERROR_MESSAGES = { static ERROR_MESSAGES = {
CHANGE_ROLE: { CHANGE_ROLE: {
MISSING_NEW_ROLE_KEY: 'Expected `newRole` to exist',
MISSING_NEW_ROLE_VALUE: 'Expected `newRole` to have `name` and `scope`',
NO_USER: 'Target user not found', NO_USER: 'Target user not found',
NO_ADMIN_ON_OWNER: 'Admin cannot change role on global owner', NO_ADMIN_ON_OWNER: 'Admin cannot change role on global owner',
NO_OWNER_ON_OWNER: 'Owner cannot change role on global owner', NO_OWNER_ON_OWNER: 'Owner cannot change role on global owner',
NO_USER_TO_OWNER: 'Cannot promote user to global owner',
NO_ADMIN_IF_UNLICENSED: 'Admin role is not available without a license',
}, },
} as const; } as const;
@ -298,74 +306,38 @@ export class UsersController {
@Patch('/:id/role') @Patch('/:id/role')
@RequireGlobalScope('user:changeRole') @RequireGlobalScope('user:changeRole')
async changeRole(req: UserRequest.ChangeRole) { @Licensed('feat:advancedPermissions')
const { async changeGlobalRole(req: UserRequest.ChangeRole) {
MISSING_NEW_ROLE_KEY, const { NO_ADMIN_ON_OWNER, NO_USER, NO_OWNER_ON_OWNER } =
MISSING_NEW_ROLE_VALUE, UsersController.ERROR_MESSAGES.CHANGE_ROLE;
NO_ADMIN_ON_OWNER,
NO_USER_TO_OWNER,
NO_USER,
NO_OWNER_ON_OWNER,
NO_ADMIN_IF_UNLICENSED,
} = UsersController.ERROR_MESSAGES.CHANGE_ROLE;
const { newRole } = req.body; const payload = plainToInstance(UserRoleChangePayload, req.body);
await validateEntity(payload);
if (!newRole) {
throw new BadRequestError(MISSING_NEW_ROLE_KEY);
}
if (!newRole.name || !newRole.scope) {
throw new BadRequestError(MISSING_NEW_ROLE_VALUE);
}
if (newRole.scope === 'global' && newRole.name === 'owner') {
throw new UnauthorizedError(NO_USER_TO_OWNER);
}
const targetUser = await this.userRepository.findOne({ const targetUser = await this.userRepository.findOne({
where: { id: req.params.id }, where: { id: req.params.id },
relations: ['globalRole'], relations: ['globalRole'],
}); });
if (targetUser === null) { if (targetUser === null) {
throw new NotFoundError(NO_USER); throw new NotFoundError(NO_USER);
} }
if ( if (req.user.globalRole.name === 'admin' && targetUser.globalRole.name === 'owner') {
newRole.scope === 'global' &&
newRole.name === 'admin' &&
!this.license.isAdvancedPermissionsLicensed()
) {
throw new UnauthorizedError(NO_ADMIN_IF_UNLICENSED);
}
if (
req.user.globalRole.scope === 'global' &&
req.user.globalRole.name === 'admin' &&
targetUser.globalRole.scope === 'global' &&
targetUser.globalRole.name === 'owner'
) {
throw new UnauthorizedError(NO_ADMIN_ON_OWNER); throw new UnauthorizedError(NO_ADMIN_ON_OWNER);
} }
if ( if (req.user.globalRole.name === 'owner' && targetUser.globalRole.name === 'owner') {
req.user.globalRole.scope === 'global' &&
req.user.globalRole.name === 'owner' &&
targetUser.globalRole.scope === 'global' &&
targetUser.globalRole.name === 'owner'
) {
throw new UnauthorizedError(NO_OWNER_ON_OWNER); throw new UnauthorizedError(NO_OWNER_ON_OWNER);
} }
const roleToSet = await this.roleService.findCached(newRole.scope, newRole.name); const roleToSet = await this.roleService.findCached('global', payload.newRoleName);
await this.userService.update(targetUser.id, { globalRole: roleToSet }); await this.userService.update(targetUser.id, { globalRoleId: roleToSet.id });
void this.internalHooks.onUserRoleChange({ void this.internalHooks.onUserRoleChange({
user: req.user, user: req.user,
target_user_id: targetUser.id, target_user_id: targetUser.id,
target_user_new_role: [newRole.scope, newRole.name].join(' '), target_user_new_role: ['global', payload.newRoleName].join(' '),
public_api: false, public_api: false,
}); });

View file

@ -16,7 +16,7 @@ import type {
IWorkflowSettings, IWorkflowSettings,
} from 'n8n-workflow'; } from 'n8n-workflow';
import { IsBoolean, IsEmail, IsOptional, IsString, Length } from 'class-validator'; import { IsBoolean, IsEmail, IsIn, IsOptional, IsString, Length } from 'class-validator';
import { NoXss } from '@db/utils/customValidators'; import { NoXss } from '@db/utils/customValidators';
import type { import type {
PublicUser, PublicUser,
@ -25,7 +25,7 @@ import type {
SecretsProvider, SecretsProvider,
SecretsProviderState, SecretsProviderState,
} from '@/Interfaces'; } from '@/Interfaces';
import type { Role, RoleNames, RoleScopes } from '@db/entities/Role'; import type { Role, RoleNames } from '@db/entities/Role';
import type { User } from '@db/entities/User'; import type { User } from '@db/entities/User';
import type { UserManagementMailer } from '@/UserManagement/email'; import type { UserManagementMailer } from '@/UserManagement/email';
import type { Variables } from '@db/entities/Variables'; import type { Variables } from '@db/entities/Variables';
@ -47,6 +47,7 @@ export class UserUpdatePayload implements Pick<User, 'email' | 'firstName' | 'la
@Length(1, 32, { message: 'Last name must be $constraint1 to $constraint2 characters long.' }) @Length(1, 32, { message: 'Last name must be $constraint1 to $constraint2 characters long.' })
lastName: string; lastName: string;
} }
export class UserSettingsUpdatePayload { export class UserSettingsUpdatePayload {
@IsBoolean({ message: 'userActivated should be a boolean' }) @IsBoolean({ message: 'userActivated should be a boolean' })
@IsOptional() @IsOptional()
@ -57,6 +58,11 @@ export class UserSettingsUpdatePayload {
allowSSOManualLogin?: boolean; allowSSOManualLogin?: boolean;
} }
export class UserRoleChangePayload {
@IsIn(['member', 'admin'])
newRoleName: Exclude<RoleNames, 'user' | 'editor' | 'owner'>;
}
export type AuthlessRequest< export type AuthlessRequest<
RouteParams = {}, RouteParams = {},
ResponseBody = {}, ResponseBody = {},
@ -332,12 +338,7 @@ export declare namespace UserRequest {
{ transferId?: string; includeRole: boolean } { transferId?: string; includeRole: boolean }
>; >;
export type ChangeRole = AuthenticatedRequest< export type ChangeRole = AuthenticatedRequest<{ id: string }, {}, UserRoleChangePayload, {}>;
{ id: string },
{},
{ newRole?: { scope?: RoleScopes; name?: RoleNames } },
{}
>;
export type Get = AuthenticatedRequest< export type Get = AuthenticatedRequest<
{ id: string; email: string; identifier: string }, { id: string; email: string; identifier: string },

View file

@ -361,15 +361,8 @@ describe('PATCH /users/:id/role', () => {
let memberAgent: SuperAgentTest; let memberAgent: SuperAgentTest;
let authlessAgent: SuperAgentTest; let authlessAgent: SuperAgentTest;
const { const { NO_ADMIN_ON_OWNER, NO_USER, NO_OWNER_ON_OWNER } =
MISSING_NEW_ROLE_KEY, UsersController.ERROR_MESSAGES.CHANGE_ROLE;
MISSING_NEW_ROLE_VALUE,
NO_ADMIN_ON_OWNER,
NO_USER_TO_OWNER,
NO_USER,
NO_OWNER_ON_OWNER,
NO_ADMIN_IF_UNLICENSED,
} = UsersController.ERROR_MESSAGES.CHANGE_ROLE;
const UNAUTHORIZED = 'Unauthorized'; const UNAUTHORIZED = 'Unauthorized';
@ -393,17 +386,31 @@ describe('PATCH /users/:id/role', () => {
describe('unauthenticated user', () => { describe('unauthenticated user', () => {
test('should receive 401', async () => { test('should receive 401', async () => {
const response = await authlessAgent.patch(`/users/${member.id}/role`).send({ const response = await authlessAgent.patch(`/users/${member.id}/role`).send({
newRole: { scope: 'global', name: 'admin' }, newRoleName: 'admin',
}); });
expect(response.statusCode).toBe(401); expect(response.statusCode).toBe(401);
}); });
}); });
describe('Invalid payload should return 400 when newRoleName', () => {
test.each([
['is missing', {}],
['is `owner`', { newRoleName: 'owner' }],
['is an array', { newRoleName: ['owner'] }],
])('%s', async (_, payload) => {
const response = await adminAgent.patch(`/users/${member.id}/role`).send(payload);
expect(response.statusCode).toBe(400);
expect(response.body.message).toBe(
'newRoleName must be one of the following values: member, admin',
);
});
});
describe('member', () => { describe('member', () => {
test('should fail to demote owner to member', async () => { test('should fail to demote owner to member', async () => {
const response = await memberAgent.patch(`/users/${owner.id}/role`).send({ const response = await memberAgent.patch(`/users/${owner.id}/role`).send({
newRole: { scope: 'global', name: 'member' }, newRoleName: 'member',
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
@ -412,7 +419,7 @@ describe('PATCH /users/:id/role', () => {
test('should fail to demote owner to admin', async () => { test('should fail to demote owner to admin', async () => {
const response = await memberAgent.patch(`/users/${owner.id}/role`).send({ const response = await memberAgent.patch(`/users/${owner.id}/role`).send({
newRole: { scope: 'global', name: 'admin' }, newRoleName: 'admin',
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
@ -421,7 +428,7 @@ describe('PATCH /users/:id/role', () => {
test('should fail to demote admin to member', async () => { test('should fail to demote admin to member', async () => {
const response = await memberAgent.patch(`/users/${admin.id}/role`).send({ const response = await memberAgent.patch(`/users/${admin.id}/role`).send({
newRole: { scope: 'global', name: 'member' }, newRoleName: 'member',
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
@ -430,7 +437,7 @@ describe('PATCH /users/:id/role', () => {
test('should fail to promote other member to owner', async () => { test('should fail to promote other member to owner', async () => {
const response = await memberAgent.patch(`/users/${otherMember.id}/role`).send({ const response = await memberAgent.patch(`/users/${otherMember.id}/role`).send({
newRole: { scope: 'global', name: 'owner' }, newRoleName: 'owner',
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
@ -439,7 +446,7 @@ describe('PATCH /users/:id/role', () => {
test('should fail to promote other member to admin', async () => { test('should fail to promote other member to admin', async () => {
const response = await memberAgent.patch(`/users/${otherMember.id}/role`).send({ const response = await memberAgent.patch(`/users/${otherMember.id}/role`).send({
newRole: { scope: 'global', name: 'admin' }, newRoleName: 'admin',
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
@ -448,7 +455,7 @@ describe('PATCH /users/:id/role', () => {
test('should fail to promote self to admin', async () => { test('should fail to promote self to admin', async () => {
const response = await memberAgent.patch(`/users/${member.id}/role`).send({ const response = await memberAgent.patch(`/users/${member.id}/role`).send({
newRole: { scope: 'global', name: 'admin' }, newRoleName: 'admin',
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
@ -457,7 +464,7 @@ describe('PATCH /users/:id/role', () => {
test('should fail to promote self to owner', async () => { test('should fail to promote self to owner', async () => {
const response = await memberAgent.patch(`/users/${member.id}/role`).send({ const response = await memberAgent.patch(`/users/${member.id}/role`).send({
newRole: { scope: 'global', name: 'owner' }, newRoleName: 'owner',
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
@ -466,25 +473,11 @@ describe('PATCH /users/:id/role', () => {
}); });
describe('admin', () => { describe('admin', () => {
test('should receive 400 on invalid payload', async () => {
const response = await adminAgent.patch(`/users/${member.id}/role`).send({});
expect(response.statusCode).toBe(400);
expect(response.body.message).toBe(MISSING_NEW_ROLE_KEY);
const _response = await adminAgent.patch(`/users/${member.id}/role`).send({
newRole: {},
});
expect(_response.statusCode).toBe(400);
expect(_response.body.message).toBe(MISSING_NEW_ROLE_VALUE);
});
test('should receive 404 on unknown target user', async () => { test('should receive 404 on unknown target user', async () => {
const response = await adminAgent const response = await adminAgent
.patch('/users/c2317ff3-7a9f-4fd4-ad2b-7331f6359260/role') .patch('/users/c2317ff3-7a9f-4fd4-ad2b-7331f6359260/role')
.send({ .send({
newRole: { scope: 'global', name: 'member' }, newRoleName: 'member',
}); });
expect(response.statusCode).toBe(404); expect(response.statusCode).toBe(404);
@ -493,7 +486,7 @@ describe('PATCH /users/:id/role', () => {
test('should fail to demote owner to admin', async () => { test('should fail to demote owner to admin', async () => {
const response = await adminAgent.patch(`/users/${owner.id}/role`).send({ const response = await adminAgent.patch(`/users/${owner.id}/role`).send({
newRole: { scope: 'global', name: 'admin' }, newRoleName: 'admin',
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
@ -502,45 +495,27 @@ describe('PATCH /users/:id/role', () => {
test('should fail to demote owner to member', async () => { test('should fail to demote owner to member', async () => {
const response = await adminAgent.patch(`/users/${owner.id}/role`).send({ const response = await adminAgent.patch(`/users/${owner.id}/role`).send({
newRole: { scope: 'global', name: 'member' }, newRoleName: 'member',
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_ADMIN_ON_OWNER); expect(response.body.message).toBe(NO_ADMIN_ON_OWNER);
}); });
test('should fail to promote member to owner', async () => {
const response = await adminAgent.patch(`/users/${member.id}/role`).send({
newRole: { scope: 'global', name: 'owner' },
});
expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_USER_TO_OWNER);
});
test('should fail to promote admin to owner', async () => {
const response = await adminAgent.patch(`/users/${member.id}/role`).send({
newRole: { scope: 'global', name: 'owner' },
});
expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_USER_TO_OWNER);
});
test('should fail to promote member to admin if not licensed', async () => { test('should fail to promote member to admin if not licensed', async () => {
testServer.license.disable('feat:advancedPermissions'); testServer.license.disable('feat:advancedPermissions');
const response = await adminAgent.patch(`/users/${member.id}/role`).send({ const response = await adminAgent.patch(`/users/${member.id}/role`).send({
newRole: { scope: 'global', name: 'admin' }, newRoleName: 'admin',
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_ADMIN_IF_UNLICENSED); expect(response.body.message).toBe('Plan lacks license for this feature');
}); });
test('should be able to demote admin to member', async () => { test('should be able to demote admin to member', async () => {
const response = await adminAgent.patch(`/users/${otherAdmin.id}/role`).send({ const response = await adminAgent.patch(`/users/${otherAdmin.id}/role`).send({
newRole: { scope: 'global', name: 'member' }, newRoleName: 'member',
}); });
expect(response.statusCode).toBe(200); expect(response.statusCode).toBe(200);
@ -559,7 +534,7 @@ describe('PATCH /users/:id/role', () => {
test('should be able to demote self to member', async () => { test('should be able to demote self to member', async () => {
const response = await adminAgent.patch(`/users/${admin.id}/role`).send({ const response = await adminAgent.patch(`/users/${admin.id}/role`).send({
newRole: { scope: 'global', name: 'member' }, newRoleName: 'member',
}); });
expect(response.statusCode).toBe(200); expect(response.statusCode).toBe(200);
@ -578,7 +553,7 @@ describe('PATCH /users/:id/role', () => {
test('should be able to promote member to admin if licensed', async () => { test('should be able to promote member to admin if licensed', async () => {
const response = await adminAgent.patch(`/users/${member.id}/role`).send({ const response = await adminAgent.patch(`/users/${member.id}/role`).send({
newRole: { scope: 'global', name: 'admin' }, newRoleName: 'admin',
}); });
expect(response.statusCode).toBe(200); expect(response.statusCode).toBe(200);
@ -599,7 +574,7 @@ describe('PATCH /users/:id/role', () => {
describe('owner', () => { describe('owner', () => {
test('should fail to demote self to admin', async () => { test('should fail to demote self to admin', async () => {
const response = await ownerAgent.patch(`/users/${owner.id}/role`).send({ const response = await ownerAgent.patch(`/users/${owner.id}/role`).send({
newRole: { scope: 'global', name: 'admin' }, newRoleName: 'admin',
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
@ -608,45 +583,27 @@ describe('PATCH /users/:id/role', () => {
test('should fail to demote self to member', async () => { test('should fail to demote self to member', async () => {
const response = await ownerAgent.patch(`/users/${owner.id}/role`).send({ const response = await ownerAgent.patch(`/users/${owner.id}/role`).send({
newRole: { scope: 'global', name: 'member' }, newRoleName: 'member',
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_OWNER_ON_OWNER); expect(response.body.message).toBe(NO_OWNER_ON_OWNER);
}); });
test('should fail to promote admin to owner', async () => {
const response = await ownerAgent.patch(`/users/${admin.id}/role`).send({
newRole: { scope: 'global', name: 'owner' },
});
expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_USER_TO_OWNER);
});
test('should fail to promote member to owner', async () => {
const response = await ownerAgent.patch(`/users/${member.id}/role`).send({
newRole: { scope: 'global', name: 'owner' },
});
expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_USER_TO_OWNER);
});
test('should fail to promote member to admin if not licensed', async () => { test('should fail to promote member to admin if not licensed', async () => {
testServer.license.disable('feat:advancedPermissions'); testServer.license.disable('feat:advancedPermissions');
const response = await ownerAgent.patch(`/users/${member.id}/role`).send({ const response = await ownerAgent.patch(`/users/${member.id}/role`).send({
newRole: { scope: 'global', name: 'admin' }, newRoleName: 'admin',
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_ADMIN_IF_UNLICENSED); expect(response.body.message).toBe('Plan lacks license for this feature');
}); });
test('should be able to promote member to admin if licensed', async () => { test('should be able to promote member to admin if licensed', async () => {
const response = await ownerAgent.patch(`/users/${member.id}/role`).send({ const response = await ownerAgent.patch(`/users/${member.id}/role`).send({
newRole: { scope: 'global', name: 'admin' }, newRoleName: 'admin',
}); });
expect(response.statusCode).toBe(200); expect(response.statusCode).toBe(200);
@ -665,7 +622,7 @@ describe('PATCH /users/:id/role', () => {
test('should be able to demote admin to member', async () => { test('should be able to demote admin to member', async () => {
const response = await ownerAgent.patch(`/users/${admin.id}/role`).send({ const response = await ownerAgent.patch(`/users/${admin.id}/role`).send({
newRole: { scope: 'global', name: 'member' }, newRoleName: 'member',
}); });
expect(response.statusCode).toBe(200); expect(response.statusCode).toBe(200);

View file

@ -7,7 +7,6 @@ import type {
} from '@/Interface'; } from '@/Interface';
import type { IDataObject } from 'n8n-workflow'; import type { IDataObject } from 'n8n-workflow';
import { makeRestApiRequest } from '@/utils/apiUtils'; import { makeRestApiRequest } from '@/utils/apiUtils';
import type { ScopeLevel } from '@n8n/permissions';
export async function loginCurrentUser( export async function loginCurrentUser(
context: IRestApiContext, context: IRestApiContext,
@ -146,9 +145,14 @@ export async function submitPersonalizationSurvey(
await makeRestApiRequest(context, 'POST', '/me/survey', params as unknown as IDataObject); await makeRestApiRequest(context, 'POST', '/me/survey', params as unknown as IDataObject);
} }
export async function updateRole( export interface UpdateGlobalRolePayload {
context: IRestApiContext, id: string;
{ id, role }: { id: string; role: { scope: ScopeLevel; name: IRole } }, newRoleName: Exclude<IRole, 'default' | 'owner'>;
): Promise<IUserResponse> { }
return makeRestApiRequest(context, 'PATCH', `/users/${id}/role`, { newRole: role });
export async function updateGlobalRole(
context: IRestApiContext,
{ id, newRoleName }: UpdateGlobalRolePayload,
): Promise<IUserResponse> {
return makeRestApiRequest(context, 'PATCH', `/users/${id}/role`, { newRoleName });
} }

View file

@ -1,3 +1,4 @@
import type { UpdateGlobalRolePayload } from '@/api/users';
import { import {
changePassword, changePassword,
deleteUser, deleteUser,
@ -15,7 +16,7 @@ import {
updateOtherUserSettings, updateOtherUserSettings,
validatePasswordToken, validatePasswordToken,
validateSignupToken, validateSignupToken,
updateRole, updateGlobalRole,
} from '@/api/users'; } from '@/api/users';
import { PERSONALIZATION_MODAL_KEY, STORES } from '@/constants'; import { PERSONALIZATION_MODAL_KEY, STORES } from '@/constants';
import type { import type {
@ -40,7 +41,7 @@ import { useCloudPlanStore } from './cloudPlan.store';
import { disableMfa, enableMfa, getMfaQR, verifyMfaToken } from '@/api/mfa'; import { disableMfa, enableMfa, getMfaQR, verifyMfaToken } from '@/api/mfa';
import { confirmEmail, getCloudUserInfo } from '@/api/cloudPlans'; import { confirmEmail, getCloudUserInfo } from '@/api/cloudPlans';
import { useRBACStore } from '@/stores/rbac.store'; import { useRBACStore } from '@/stores/rbac.store';
import type { Scope, ScopeLevel } from '@n8n/permissions'; import type { Scope } from '@n8n/permissions';
import { inviteUsers, acceptInvitation } from '@/api/invitation'; import { inviteUsers, acceptInvitation } from '@/api/invitation';
const isPendingUser = (user: IUserResponse | null) => !!user?.isPending; const isPendingUser = (user: IUserResponse | null) => !!user?.isPending;
@ -379,9 +380,9 @@ export const useUsersStore = defineStore(STORES.USERS, {
await confirmEmail(useRootStore().getRestApiContext); await confirmEmail(useRootStore().getRestApiContext);
}, },
async updateRole({ id, role }: { id: string; role: { scope: ScopeLevel; name: IRole } }) { async updateGlobalRole({ id, newRoleName }: UpdateGlobalRolePayload) {
const rootStore = useRootStore(); const rootStore = useRootStore();
await updateRole(rootStore.getRestApiContext, { id, role }); await updateGlobalRole(rootStore.getRestApiContext, { id, newRoleName });
await this.fetchUsers(); await this.fetchUsers();
}, },
}, },

View file

@ -99,6 +99,7 @@ import { useSSOStore } from '@/stores/sso.store';
import { hasPermission } from '@/rbac/permissions'; import { hasPermission } from '@/rbac/permissions';
import { ROLE } from '@/utils/userUtils'; import { ROLE } from '@/utils/userUtils';
import { useClipboard } from '@/composables/useClipboard'; import { useClipboard } from '@/composables/useClipboard';
import type { UpdateGlobalRolePayload } from '@/api/users';
export default defineComponent({ export default defineComponent({
name: 'SettingsUsersView', name: 'SettingsUsersView',
@ -280,8 +281,8 @@ export default defineComponent({
goToUpgradeAdvancedPermissions() { goToUpgradeAdvancedPermissions() {
void this.uiStore.goToUpgrade('settings-users', 'upgrade-advanced-permissions'); void this.uiStore.goToUpgrade('settings-users', 'upgrade-advanced-permissions');
}, },
async onRoleChange(user: IUser, name: IRole) { async onRoleChange(user: IUser, newRoleName: UpdateGlobalRolePayload['newRoleName']) {
await this.usersStore.updateRole({ id: user.id, role: { scope: 'global', name } }); await this.usersStore.updateGlobalRole({ id: user.id, newRoleName });
}, },
}, },
}); });