fix(core): Ensure member and admin cannot be promoted to owner (#7830)

https://linear.app/n8n/issue/PAY-985/add-user-role-modification-endpoint#comment-62355f6b
This commit is contained in:
Iván Ovejero 2023-11-27 17:35:58 +01:00 committed by GitHub
parent ac744d6702
commit 9b87a596ca
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 43 additions and 30 deletions

View file

@ -40,7 +40,7 @@ export class UsersController {
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_ADMIN_TO_OWNER: 'Admin cannot promote user to global owner', NO_USER_TO_OWNER: 'Cannot promote user to global owner',
}, },
} as const; } as const;
@ -330,7 +330,7 @@ export class UsersController {
MISSING_NEW_ROLE_KEY, MISSING_NEW_ROLE_KEY,
MISSING_NEW_ROLE_VALUE, MISSING_NEW_ROLE_VALUE,
NO_ADMIN_ON_OWNER, NO_ADMIN_ON_OWNER,
NO_ADMIN_TO_OWNER, NO_USER_TO_OWNER,
NO_USER, NO_USER,
NO_OWNER_ON_OWNER, NO_OWNER_ON_OWNER,
} = UsersController.ERROR_MESSAGES.CHANGE_ROLE; } = UsersController.ERROR_MESSAGES.CHANGE_ROLE;
@ -349,13 +349,8 @@ export class UsersController {
throw new BadRequestError(MISSING_NEW_ROLE_VALUE); throw new BadRequestError(MISSING_NEW_ROLE_VALUE);
} }
if ( if (newRole.scope === 'global' && newRole.name === 'owner') {
req.user.globalRole.scope === 'global' && throw new UnauthorizedError(NO_USER_TO_OWNER);
req.user.globalRole.name === 'admin' &&
newRole.scope === 'global' &&
newRole.name === 'owner'
) {
throw new UnauthorizedError(NO_ADMIN_TO_OWNER);
} }
const targetUser = await this.userService.findOne({ const targetUser = await this.userService.findOne({

View file

@ -359,7 +359,7 @@ describe('PATCH /users/:id/role', () => {
MISSING_NEW_ROLE_KEY, MISSING_NEW_ROLE_KEY,
MISSING_NEW_ROLE_VALUE, MISSING_NEW_ROLE_VALUE,
NO_ADMIN_ON_OWNER, NO_ADMIN_ON_OWNER,
NO_ADMIN_TO_OWNER, NO_USER_TO_OWNER,
NO_USER, NO_USER,
NO_OWNER_ON_OWNER, NO_OWNER_ON_OWNER,
} = UsersController.ERROR_MESSAGES.CHANGE_ROLE; } = UsersController.ERROR_MESSAGES.CHANGE_ROLE;
@ -506,7 +506,7 @@ describe('PATCH /users/:id/role', () => {
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_ADMIN_TO_OWNER); expect(response.body.message).toBe(NO_USER_TO_OWNER);
}); });
test('should fail to promote admin to owner', async () => { test('should fail to promote admin to owner', async () => {
@ -515,7 +515,7 @@ describe('PATCH /users/:id/role', () => {
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_ADMIN_TO_OWNER); expect(response.body.message).toBe(NO_USER_TO_OWNER);
}); });
test('should be able to demote admin to member', async () => { test('should be able to demote admin to member', async () => {
@ -577,6 +577,42 @@ describe('PATCH /users/:id/role', () => {
}); });
describe('owner', () => { describe('owner', () => {
test('should fail to demote self to admin', async () => {
const response = await ownerAgent.patch(`/users/${owner.id}/role`).send({
newRole: { scope: 'global', name: 'admin' },
});
expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_OWNER_ON_OWNER);
});
test('should fail to demote self to member', async () => {
const response = await ownerAgent.patch(`/users/${owner.id}/role`).send({
newRole: { scope: 'global', name: 'member' },
});
expect(response.statusCode).toBe(403);
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 be able to promote member to admin', async () => { test('should be able to promote member to admin', 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' }, newRole: { scope: 'global', name: 'admin' },
@ -614,23 +650,5 @@ describe('PATCH /users/:id/role', () => {
admin = await createAdmin(); admin = await createAdmin();
adminAgent = testServer.authAgentFor(admin); adminAgent = testServer.authAgentFor(admin);
}); });
test('should fail to demote self to admin', async () => {
const response = await ownerAgent.patch(`/users/${owner.id}/role`).send({
newRole: { scope: 'global', name: 'admin' },
});
expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_OWNER_ON_OWNER);
});
test('should fail to demote self to member', async () => {
const response = await ownerAgent.patch(`/users/${owner.id}/role`).send({
newRole: { scope: 'global', name: 'member' },
});
expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_OWNER_ON_OWNER);
});
}); });
}); });