From 9b87a596ca4aec462faedcca1ba4655b168bc3bc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= <ivov.src@gmail.com>
Date: Mon, 27 Nov 2023 17:35:58 +0100
Subject: [PATCH] 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
---
 .../cli/src/controllers/users.controller.ts   | 13 ++--
 .../cli/test/integration/users.api.test.ts    | 60 ++++++++++++-------
 2 files changed, 43 insertions(+), 30 deletions(-)

diff --git a/packages/cli/src/controllers/users.controller.ts b/packages/cli/src/controllers/users.controller.ts
index 46f098736c..3e239c7adc 100644
--- a/packages/cli/src/controllers/users.controller.ts
+++ b/packages/cli/src/controllers/users.controller.ts
@@ -40,7 +40,7 @@ export class UsersController {
 			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',
+			NO_USER_TO_OWNER: 'Cannot promote user to global owner',
 		},
 	} as const;
 
@@ -330,7 +330,7 @@ export class UsersController {
 			MISSING_NEW_ROLE_KEY,
 			MISSING_NEW_ROLE_VALUE,
 			NO_ADMIN_ON_OWNER,
-			NO_ADMIN_TO_OWNER,
+			NO_USER_TO_OWNER,
 			NO_USER,
 			NO_OWNER_ON_OWNER,
 		} = UsersController.ERROR_MESSAGES.CHANGE_ROLE;
@@ -349,13 +349,8 @@ export class UsersController {
 			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);
+		if (newRole.scope === 'global' && newRole.name === 'owner') {
+			throw new UnauthorizedError(NO_USER_TO_OWNER);
 		}
 
 		const targetUser = await this.userService.findOne({
diff --git a/packages/cli/test/integration/users.api.test.ts b/packages/cli/test/integration/users.api.test.ts
index c1d031b635..ff15cb0655 100644
--- a/packages/cli/test/integration/users.api.test.ts
+++ b/packages/cli/test/integration/users.api.test.ts
@@ -359,7 +359,7 @@ describe('PATCH /users/:id/role', () => {
 		MISSING_NEW_ROLE_KEY,
 		MISSING_NEW_ROLE_VALUE,
 		NO_ADMIN_ON_OWNER,
-		NO_ADMIN_TO_OWNER,
+		NO_USER_TO_OWNER,
 		NO_USER,
 		NO_OWNER_ON_OWNER,
 	} = UsersController.ERROR_MESSAGES.CHANGE_ROLE;
@@ -506,7 +506,7 @@ describe('PATCH /users/:id/role', () => {
 			});
 
 			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 () => {
@@ -515,7 +515,7 @@ describe('PATCH /users/:id/role', () => {
 			});
 
 			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 () => {
@@ -577,6 +577,42 @@ describe('PATCH /users/:id/role', () => {
 	});
 
 	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 () => {
 			const response = await ownerAgent.patch(`/users/${member.id}/role`).send({
 				newRole: { scope: 'global', name: 'admin' },
@@ -614,23 +650,5 @@ describe('PATCH /users/:id/role', () => {
 			admin = await createAdmin();
 			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);
-		});
 	});
 });