feat(core): Allow user role modification (#7797)

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) ```
2023-11-24 11:40:08 +01:00
parent 87fa3c2985
commit 7a86d36068
7 changed files with 384 additions and 19 deletions
--- a/packages/cli/test/integration/shared/db/roles.ts
+++ b/packages/cli/test/integration/shared/db/roles.ts
@@ -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();
 }
--- a/packages/cli/test/integration/shared/db/users.ts
+++ b/packages/cli/test/integration/shared/db/users.ts
@@ -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`.
@@ -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)}`);
@@ -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' },
--- a/packages/cli/test/integration/users.api.test.ts
+++ b/packages/cli/test/integration/users.api.test.ts
@@ -19,10 +19,14 @@ import * as testDb from './shared/testDb';
 import * as utils from './shared/utils/';
 import { saveCredential } from './shared/db/credentials';
 import { getAllRoles } from './shared/db/roles';
-import { createMember, createOwner, createUser } from './shared/db/users';
+import { createAdmin, createMember, createOwner, createUser, getUserById } from './shared/db/users';
 import { createWorkflow } from './shared/db/workflows';
 import type { PublicUser } from '@/Interfaces';
 import { InternalHooks } from '@/InternalHooks';
+import { UsersController } from '@/controllers/users.controller';
+import { ExternalHooks } from '@/ExternalHooks';
+import { Logger } from '@/Logger';
+import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner';

 const { any } = expect;

@@ -31,19 +35,25 @@ let globalMemberRole: Role;
 let workflowOwnerRole: Role;

 let owner: User;
+let admin: User;
+let otherAdmin: User;
 let member: User;
+let otherMember: User;
+
 let authOwnerAgent: SuperAgentTest;
+let authAdminAgent: SuperAgentTest;
+let authMemberAgent: SuperAgentTest;
 let authlessAgent: SuperAgentTest;

+let usersCount: number;
+
 mockInstance(InternalHooks);
+mockInstance(ExternalHooks);
+mockInstance(Logger);
+mockInstance(ActiveWorkflowRunner);

 const testServer = utils.setupTestServer({ endpointGroups: ['users'] });

-type UserInvitationResponse = {
-	user: Pick<User, 'id' | 'email'> & { inviteAcceptUrl: string; emailSent: boolean };
-	error?: string;
-};
-
 beforeAll(async () => {
 	const [_, fetchedGlobalMemberRole, fetchedWorkflowOwnerRole, fetchedCredentialOwnerRole] =
 		await getAllRoles();
@@ -51,6 +61,8 @@ beforeAll(async () => {
 	credentialOwnerRole = fetchedCredentialOwnerRole;
 	globalMemberRole = fetchedGlobalMemberRole;
 	workflowOwnerRole = fetchedWorkflowOwnerRole;
+
+	usersCount = [owner, admin, otherAdmin, member, otherMember].length;
 });

 beforeEach(async () => {
@@ -58,7 +70,12 @@ beforeEach(async () => {
 	await testDb.truncate(['User', 'SharedCredentials', 'SharedWorkflow', 'Workflow', 'Credentials']);
 	owner = await createOwner();
 	member = await createMember();
+	otherMember = await createMember();
+	admin = await createAdmin();
+	otherAdmin = await createAdmin();
 	authOwnerAgent = testServer.authAgentFor(owner);
+	authAdminAgent = testServer.authAgentFor(admin);
+	authMemberAgent = testServer.authAgentFor(member);
 	authlessAgent = testServer.authlessAgent;
 });

@@ -80,7 +97,7 @@ describe('GET /users', () => {
 	test('should return all users', async () => {
 		const response = await authOwnerAgent.get('/users').expect(200);

-		expect(response.body.data).toHaveLength(2);
+		expect(response.body.data).toHaveLength(usersCount);

 		response.body.data.forEach(validatePublicUser);
 	});
@@ -172,7 +189,7 @@ describe('GET /users', () => {
 				.query('filter={ "isOwner": false }')
 				.expect(200);

-			expect(_response.body.data).toHaveLength(1);
+			expect(_response.body.data).toHaveLength(usersCount - 1);

 			const [_user] = _response.body.data;

@@ -185,7 +202,7 @@ describe('GET /users', () => {
 			const response = await authOwnerAgent.get('/users').query('select=["id"]').expect(200);

 			expect(response.body).toEqual({
-				data: [{ id: any(String) }, { id: any(String) }],
+				data: new Array(usersCount).fill({ id: any(String) }),
 			});
 		});

@@ -193,7 +210,7 @@ describe('GET /users', () => {
 			const response = await authOwnerAgent.get('/users').query('select=["email"]').expect(200);

 			expect(response.body).toEqual({
-				data: [{ email: any(String) }, { email: any(String) }],
+				data: new Array(usersCount).fill({ email: any(String) }),
 			});
 		});

@@ -201,7 +218,7 @@ describe('GET /users', () => {
 			const response = await authOwnerAgent.get('/users').query('select=["firstName"]').expect(200);

 			expect(response.body).toEqual({
-				data: [{ firstName: any(String) }, { firstName: any(String) }],
+				data: new Array(usersCount).fill({ firstName: any(String) }),
 			});
 		});

@@ -209,7 +226,7 @@ describe('GET /users', () => {
 			const response = await authOwnerAgent.get('/users').query('select=["lastName"]').expect(200);

 			expect(response.body).toEqual({
-				data: [{ lastName: any(String) }, { lastName: any(String) }],
+				data: new Array(usersCount).fill({ lastName: any(String) }),
 			});
 		});
 	});
@@ -379,3 +396,241 @@ describe('DELETE /users/:id', () => {
 		expect(deletedUser).toBeNull();
 	});
 });
+
+describe('PATCH /users/:id/role', () => {
+	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;
+
+	describe('unauthenticated user', () => {
+		test('should receive 401', async () => {
+			const response = await authlessAgent.patch(`/users/${member.id}/role`).send({
+				newRole: { scope: 'global', name: 'admin' },
+			});
+
+			expect(response.statusCode).toBe(401);
+		});
+	});
+
+	describe('member', () => {
+		test('should fail to demote owner to member', async () => {
+			const response = await authMemberAgent.patch(`/users/${owner.id}/role`).send({
+				newRole: { scope: 'global', name: 'member' },
+			});
+
+			expect(response.statusCode).toBe(403);
+			expect(response.body.message).toBe(NO_MEMBER);
+		});
+
+		test('should fail to demote owner to admin', async () => {
+			const response = await authMemberAgent.patch(`/users/${owner.id}/role`).send({
+				newRole: { scope: 'global', name: 'admin' },
+			});
+
+			expect(response.statusCode).toBe(403);
+			expect(response.body.message).toBe(NO_MEMBER);
+		});
+
+		test('should fail to demote admin to member', async () => {
+			const response = await authMemberAgent.patch(`/users/${admin.id}/role`).send({
+				newRole: { scope: 'global', name: 'member' },
+			});
+
+			expect(response.statusCode).toBe(403);
+			expect(response.body.message).toBe(NO_MEMBER);
+		});
+
+		test('should fail to promote other member to owner', async () => {
+			const response = await authMemberAgent.patch(`/users/${otherMember.id}/role`).send({
+				newRole: { scope: 'global', name: 'owner' },
+			});
+
+			expect(response.statusCode).toBe(403);
+			expect(response.body.message).toBe(NO_MEMBER);
+		});
+
+		test('should fail to promote other member to admin', async () => {
+			const response = await authMemberAgent.patch(`/users/${otherMember.id}/role`).send({
+				newRole: { scope: 'global', name: 'admin' },
+			});
+
+			expect(response.statusCode).toBe(403);
+			expect(response.body.message).toBe(NO_MEMBER);
+		});
+
+		test('should fail to promote self to admin', async () => {
+			const response = await authMemberAgent.patch(`/users/${member.id}/role`).send({
+				newRole: { scope: 'global', name: 'admin' },
+			});
+
+			expect(response.statusCode).toBe(403);
+			expect(response.body.message).toBe(NO_MEMBER);
+		});
+
+		test('should fail to promote self to owner', async () => {
+			const response = await authMemberAgent.patch(`/users/${member.id}/role`).send({
+				newRole: { scope: 'global', name: 'owner' },
+			});
+
+			expect(response.statusCode).toBe(403);
+			expect(response.body.message).toBe(NO_MEMBER);
+		});
+	});
+
+	describe('admin', () => {
+		test('should receive 400 on invalid payload', async () => {
+			const response = await authAdminAgent.patch(`/users/${member.id}/role`).send({});
+
+			expect(response.statusCode).toBe(400);
+			expect(response.body.message).toBe(MISSING_NEW_ROLE_KEY);
+
+			const _response = await authAdminAgent.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 () => {
+			const response = await authAdminAgent.patch('/users/99999/role').send({
+				newRole: { scope: 'global', name: 'member' },
+			});
+
+			expect(response.statusCode).toBe(404);
+			expect(response.body.message).toBe(NO_USER);
+		});
+
+		test('should fail to demote owner to admin', async () => {
+			const response = await authAdminAgent.patch(`/users/${owner.id}/role`).send({
+				newRole: { scope: 'global', name: 'admin' },
+			});
+
+			expect(response.statusCode).toBe(403);
+			expect(response.body.message).toBe(NO_ADMIN_ON_OWNER);
+		});
+
+		test('should fail to demote owner to member', async () => {
+			const response = await authAdminAgent.patch(`/users/${owner.id}/role`).send({
+				newRole: { scope: 'global', name: 'member' },
+			});
+
+			expect(response.statusCode).toBe(403);
+			expect(response.body.message).toBe(NO_ADMIN_ON_OWNER);
+		});
+
+		test('should fail to promote member to owner', async () => {
+			const response = await authAdminAgent.patch(`/users/${member.id}/role`).send({
+				newRole: { scope: 'global', name: 'owner' },
+			});
+
+			expect(response.statusCode).toBe(403);
+			expect(response.body.message).toBe(NO_ADMIN_TO_OWNER);
+		});
+
+		test('should fail to promote admin to owner', async () => {
+			const response = await authAdminAgent.patch(`/users/${member.id}/role`).send({
+				newRole: { scope: 'global', name: 'owner' },
+			});
+
+			expect(response.statusCode).toBe(403);
+			expect(response.body.message).toBe(NO_ADMIN_TO_OWNER);
+		});
+
+		test('should be able to demote admin to member', async () => {
+			const response = await authAdminAgent.patch(`/users/${otherAdmin.id}/role`).send({
+				newRole: { scope: 'global', name: 'member' },
+			});
+
+			expect(response.statusCode).toBe(200);
+			expect(response.body.data).toStrictEqual({ success: true });
+
+			const user = await getUserById(otherAdmin.id);
+
+			expect(user.globalRole.scope).toBe('global');
+			expect(user.globalRole.name).toBe('member');
+		});
+
+		test('should be able to demote self to member', async () => {
+			const response = await authAdminAgent.patch(`/users/${admin.id}/role`).send({
+				newRole: { scope: 'global', name: 'member' },
+			});
+
+			expect(response.statusCode).toBe(200);
+			expect(response.body.data).toStrictEqual({ success: true });
+
+			const user = await getUserById(admin.id);
+
+			expect(user.globalRole.scope).toBe('global');
+			expect(user.globalRole.name).toBe('member');
+		});
+
+		test('should be able to promote member to admin', async () => {
+			const response = await authAdminAgent.patch(`/users/${member.id}/role`).send({
+				newRole: { scope: 'global', name: 'admin' },
+			});
+
+			expect(response.statusCode).toBe(200);
+			expect(response.body.data).toStrictEqual({ success: true });
+
+			const user = await getUserById(admin.id);
+
+			expect(user.globalRole.scope).toBe('global');
+			expect(user.globalRole.name).toBe('admin');
+		});
+	});
+
+	describe('owner', () => {
+		test('should be able to promote member to admin', async () => {
+			const response = await authOwnerAgent.patch(`/users/${member.id}/role`).send({
+				newRole: { scope: 'global', name: 'admin' },
+			});
+
+			expect(response.statusCode).toBe(200);
+			expect(response.body.data).toStrictEqual({ success: true });
+
+			const user = await getUserById(admin.id);
+
+			expect(user.globalRole.scope).toBe('global');
+			expect(user.globalRole.name).toBe('admin');
+		});
+
+		test('should be able to demote admin to member', async () => {
+			const response = await authOwnerAgent.patch(`/users/${admin.id}/role`).send({
+				newRole: { scope: 'global', name: 'member' },
+			});
+
+			expect(response.statusCode).toBe(200);
+			expect(response.body.data).toStrictEqual({ success: true });
+
+			const user = await getUserById(admin.id);
+
+			expect(user.globalRole.scope).toBe('global');
+			expect(user.globalRole.name).toBe('member');
+		});
+
+		test('should fail to demote self to admin', async () => {
+			const response = await authOwnerAgent.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 authOwnerAgent.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);
+		});
+	});
+});