Skip to content

Commit

Permalink
feat(core): Allow user role modification (#7797)
Browse files Browse the repository at this point in the history
https://linear.app/n8n/issue/PAY-985

```
PATCH /users/:id/role
  unauthenticated user
    ✓ should receive 401 (349 ms)
  member
    ✓ should fail to demote owner to member (349 ms)
    ✓ should fail to demote owner to admin (359 ms)
    ✓ should fail to demote admin to member (381 ms)
    ✓ should fail to promote other member to owner (353 ms)
    ✓ should fail to promote other member to admin (377 ms)
    ✓ should fail to promote self to admin (354 ms)
    ✓ should fail to promote self to owner (371 ms)
  admin
    ✓ should receive 400 on invalid payload (351 ms)
    ✓ should receive 404 on unknown target user (351 ms)
    ✓ should fail to demote owner to admin (349 ms)
    ✓ should fail to demote owner to member (347 ms)
    ✓ should fail to promote member to owner (384 ms)
    ✓ should fail to promote admin to owner (350 ms)
    ✓ should be able to demote admin to member (354 ms)
    ✓ should be able to demote self to member (350 ms)
    ✓ should be able to promote member to admin (349 ms)
  owner
    ✓ should be able to promote member to admin (349 ms)
    ✓ should be able to demote admin to member (349 ms)
    ✓ should fail to demote self to admin (348 ms)
    ✓ should fail to demote self to member (354 ms)
```
  • Loading branch information
ivov authored Nov 24, 2023
1 parent 87fa3c2 commit 7a86d36
Show file tree
Hide file tree
Showing 7 changed files with 384 additions and 19 deletions.
90 changes: 87 additions & 3 deletions packages/cli/src/controllers/users.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { User } from '@db/entities/User';
import { SharedCredentials } from '@db/entities/SharedCredentials';
import { SharedWorkflow } from '@db/entities/SharedWorkflow';
import { Authorized, Delete, Get, RestController, Patch } from '@/decorators';
import { BadRequestError, NotFoundError } from '@/ResponseHelper';
import { BadRequestError, NotFoundError, UnauthorizedError } from '@/ResponseHelper';
import { ListQuery, UserRequest, UserSettingsUpdatePayload } from '@/requests';
import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner';
import { IExternalHooksClass, IInternalHooksClass } from '@/Interfaces';
Expand All @@ -18,7 +18,7 @@ import { UserService } from '@/services/user.service';
import { listQueryMiddleware } from '@/middlewares';
import { Logger } from '@/Logger';

@Authorized(['global', 'owner'])
@Authorized()
@RestController('/users')
export class UsersController {
constructor(
Expand All @@ -32,6 +32,18 @@ export class UsersController {
private readonly userService: UserService,
) {}

static ERROR_MESSAGES = {
CHANGE_ROLE: {
NO_MEMBER: 'Member cannot change role for any user',
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_ADMIN_ON_OWNER: 'Admin cannot change role on global owner',
NO_OWNER_ON_OWNER: 'Owner cannot change role on global owner',
NO_ADMIN_TO_OWNER: 'Admin cannot promote user to global owner',
},
} as const;

private async toFindManyOptions(listQueryOptions?: ListQuery.Options) {
const findManyOptions: FindManyOptions<User> = {};

Expand Down Expand Up @@ -70,7 +82,7 @@ export class UsersController {
return findManyOptions;
}

removeSupplementaryFields(
private removeSupplementaryFields(
publicUsers: Array<Partial<PublicUser>>,
listQueryOptions: ListQuery.Options,
) {
Expand Down Expand Up @@ -152,6 +164,7 @@ export class UsersController {
/**
* Delete a user. Optionally, designate a transferee for their workflows and credentials.
*/
@Authorized(['global', 'owner'])
@Delete('/:id')
async deleteUser(req: UserRequest.Delete) {
const { id: idToDelete } = req.params;
Expand Down Expand Up @@ -306,4 +319,75 @@ export class UsersController {
await this.externalHooks.run('user.deleted', [await this.userService.toPublic(userToDelete)]);
return { success: true };
}

// @TODO: Add scope check `@RequireGlobalScope('user:changeRole')`
// once this has been merged: https://github.com/n8n-io/n8n/pull/7737
@Authorized('any')
@Patch('/:id/role')
async changeRole(req: UserRequest.ChangeRole) {
const {
NO_MEMBER,
MISSING_NEW_ROLE_KEY,
MISSING_NEW_ROLE_VALUE,
NO_ADMIN_ON_OWNER,
NO_ADMIN_TO_OWNER,
NO_USER,
NO_OWNER_ON_OWNER,
} = UsersController.ERROR_MESSAGES.CHANGE_ROLE;

if (req.user.globalRole.scope === 'global' && req.user.globalRole.name === 'member') {
throw new UnauthorizedError(NO_MEMBER);
}

const { newRole } = req.body;

if (!newRole) {
throw new BadRequestError(MISSING_NEW_ROLE_KEY);
}

if (!newRole.name || !newRole.scope) {
throw new BadRequestError(MISSING_NEW_ROLE_VALUE);
}

if (
req.user.globalRole.scope === 'global' &&
req.user.globalRole.name === 'admin' &&
newRole.scope === 'global' &&
newRole.name === 'owner'
) {
throw new UnauthorizedError(NO_ADMIN_TO_OWNER);
}

const targetUser = await this.userService.findOne({
where: { id: req.params.id },
});

if (targetUser === null) {
throw new NotFoundError(NO_USER);
}

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);
}

if (
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);
}

const roleToSet = await this.roleService.findCached(newRole.scope, newRole.name);

await this.userService.update(targetUser.id, { globalRole: roleToSet });

return { success: true };
}
}
2 changes: 1 addition & 1 deletion packages/cli/src/databases/entities/Role.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { SharedCredentials } from './SharedCredentials';
import { WithTimestamps } from './AbstractEntity';
import { idStringifier } from '../utils/transformers';

export type RoleNames = 'owner' | 'member' | 'user' | 'editor';
export type RoleNames = 'owner' | 'member' | 'user' | 'editor' | 'admin';
export type RoleScopes = 'global' | 'workflow' | 'credential';

@Entity()
Expand Down
9 changes: 8 additions & 1 deletion packages/cli/src/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import type {
SecretsProvider,
SecretsProviderState,
} from '@/Interfaces';
import type { Role } from '@db/entities/Role';
import type { Role, RoleNames, RoleScopes } from '@db/entities/Role';
import type { User } from '@db/entities/User';
import type { UserManagementMailer } from '@/UserManagement/email';
import type { Variables } from '@db/entities/Variables';
Expand Down Expand Up @@ -322,6 +322,13 @@ export declare namespace UserRequest {
{ transferId?: string; includeRole: boolean }
>;

export type ChangeRole = AuthenticatedRequest<
{ id: string },
{},
{ newRole?: { scope?: RoleScopes; name?: RoleNames } },
{}
>;

export type Get = AuthenticatedRequest<
{ id: string; email: string; identifier: string },
{},
Expand Down
7 changes: 6 additions & 1 deletion packages/cli/src/services/role.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class RoleService {
void this.cacheService.setMany(allRoles.map((r) => [r.cacheKey, r]));
}

private async findCached(scope: RoleScopes, name: RoleNames) {
async findCached(scope: RoleScopes, name: RoleNames) {
const cacheKey = `role:${scope}:${name}`;

const cachedRole = await this.cacheService.get(cacheKey);
Expand All @@ -50,6 +50,7 @@ export class RoleService {
private roles: Array<{ name: RoleNames; scope: RoleScopes }> = [
{ scope: 'global', name: 'owner' },
{ scope: 'global', name: 'member' },
{ scope: 'global', name: 'admin' },
{ scope: 'workflow', name: 'owner' },
{ scope: 'credential', name: 'owner' },
{ scope: 'credential', name: 'user' },
Expand All @@ -68,6 +69,10 @@ export class RoleService {
return this.findCached('global', 'member');
}

async findGlobalAdminRole() {
return this.findCached('global', 'admin');
}

async findWorkflowOwnerRole() {
return this.findCached('workflow', 'owner');
}
Expand Down
4 changes: 4 additions & 0 deletions packages/cli/test/integration/shared/db/roles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ export async function getGlobalMemberRole() {
return Container.get(RoleService).findGlobalMemberRole();
}

export async function getGlobalAdminRole() {
return Container.get(RoleService).findGlobalAdminRole();
}

export async function getWorkflowOwnerRole() {
return Container.get(RoleService).findWorkflowOwnerRole();
}
Expand Down
12 changes: 11 additions & 1 deletion packages/cli/test/integration/shared/db/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { TOTPService } from '@/Mfa/totp.service';
import { MfaService } from '@/Mfa/mfa.service';

import { randomApiKey, randomEmail, randomName, randomValidPassword } from '../random';
import { getGlobalMemberRole, getGlobalOwnerRole } from './roles';
import { getGlobalAdminRole, getGlobalMemberRole, getGlobalOwnerRole } from './roles';

/**
* Store a user in the DB, defaulting to a `member`.
Expand Down Expand Up @@ -76,6 +76,10 @@ export async function createMember() {
return createUser({ globalRole: await getGlobalMemberRole() });
}

export async function createAdmin() {
return createUser({ globalRole: await getGlobalAdminRole() });
}

export async function createUserShell(globalRole: Role): Promise<User> {
if (globalRole.scope !== 'global') {
throw new Error(`Invalid role received: ${JSON.stringify(globalRole)}`);
Expand Down Expand Up @@ -128,6 +132,12 @@ export const getAllUsers = async () =>
relations: ['globalRole', 'authIdentities'],
});

export const getUserById = async (id: string) =>
Container.get(UserRepository).findOneOrFail({
where: { id },
relations: ['globalRole', 'authIdentities'],
});

export const getLdapIdentities = async () =>
Container.get(AuthIdentityRepository).find({
where: { providerType: 'ldap' },
Expand Down
Loading

0 comments on commit 7a86d36

Please sign in to comment.