feat: RBAC (#8922)

Signed-off-by: Oleg Ivaniv <me@olegivaniv.com>
Co-authored-by: Val <68596159+valya@users.noreply.github.com>
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
Co-authored-by: Valya Bullions <valya@n8n.io>
Co-authored-by: Danny Martini <danny@n8n.io>
Co-authored-by: Danny Martini <despair.blue@gmail.com>
Co-authored-by: Iván Ovejero <ivov.src@gmail.com>
Co-authored-by: Omar Ajoue <krynble@gmail.com>
Co-authored-by: oleg <me@olegivaniv.com>
Co-authored-by: Michael Kret <michael.k@radency.com>
Co-authored-by: Michael Kret <88898367+michael-radency@users.noreply.github.com>
Co-authored-by: Elias Meire <elias@meire.dev>
Co-authored-by: Giulio Andreini <andreini@netseven.it>
Co-authored-by: Giulio Andreini <g.andreini@gmail.com>
Co-authored-by: Ayato Hayashi <go12limchangyong@gmail.com>

packages/cli/test/integration/users.api.test.ts

@@ -8,15 +8,25 @@ import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.
 import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
 import { ExecutionService } from '@/executions/execution.service';
 
-import { getCredentialById, saveCredential } from './shared/db/credentials';
+import {
+	getCredentialById,
+	saveCredential,
+	shareCredentialWithUsers,
+} from './shared/db/credentials';
 import { createAdmin, createMember, createOwner, getUserById } from './shared/db/users';
-import { createWorkflow, getWorkflowById } from './shared/db/workflows';
+import { createWorkflow, getWorkflowById, shareWorkflowWithUsers } from './shared/db/workflows';
 import { SUCCESS_RESPONSE_BODY } from './shared/constants';
 import { validateUser } from './shared/utils/users';
-import { randomName } from './shared/random';
+import { randomCredentialPayload } from './shared/random';
 import * as utils from './shared/utils/';
 import * as testDb from './shared/testDb';
 import { mockInstance } from '../shared/mocking';
+import { RESPONSE_ERROR_MESSAGES } from '@/constants';
+import { ProjectRepository } from '@/databases/repositories/project.repository';
+import { createTeamProject, getPersonalProject, linkUserToProject } from './shared/db/projects';
+import { ProjectRelationRepository } from '@/databases/repositories/projectRelation.repository';
+import { CacheService } from '@/services/cache/cache.service';
+import { v4 as uuid } from 'uuid';
 
 mockInstance(ExecutionService);
 
@@ -25,6 +35,12 @@ const testServer = utils.setupTestServer({
 	enabledFeatures: ['feat:advancedPermissions'],
 });
 
+let projectRepository: ProjectRepository;
+
+beforeAll(() => {
+	projectRepository = Container.get(ProjectRepository);
+});
+
 describe('GET /users', () => {
 	let owner: User;
 	let member: User;
@@ -229,110 +245,338 @@ describe('GET /users', () => {
 
 describe('DELETE /users/:id', () => {
 	let owner: User;
-	let member: User;
 	let ownerAgent: SuperAgentTest;
 
 	beforeAll(async () => {
 		await testDb.truncate(['User']);
 
 		owner = await createOwner();
-		member = await createMember();
 		ownerAgent = testServer.authAgentFor(owner);
 	});
 
 	test('should delete user and their resources', async () => {
-		const savedWorkflow = await createWorkflow({ name: randomName() }, member);
+		//
+		// ARRANGE
+		//
+		// @TODO: Include active workflow and check whether webhook has been removed
 
-		const savedCredential = await saveCredential(
-			{ name: randomName(), type: '', data: {} },
-			{ user: member, role: 'credential:owner' },
-		);
+		const member = await createMember();
+		const memberPersonalProject = await getPersonalProject(member);
 
-		const response = await ownerAgent.delete(`/users/${member.id}`);
+		// stays untouched
+		const teamProject = await createTeamProject();
+		// will be deleted
+		await linkUserToProject(member, teamProject, 'project:admin');
 
-		expect(response.statusCode).toBe(200);
-		expect(response.body).toEqual(SUCCESS_RESPONSE_BODY);
+		const [savedWorkflow, savedCredential, teamWorkflow, teamCredential] = await Promise.all([
+			// personal resource -> deleted
+			createWorkflow({}, member),
+			saveCredential(randomCredentialPayload(), {
+				user: member,
+				role: 'credential:owner',
+			}),
+			// resources in a team project -> untouched
+			createWorkflow({}, teamProject),
+			saveCredential(randomCredentialPayload(), {
+				project: teamProject,
+				role: 'credential:owner',
+			}),
+		]);
+
+		//
+		// ACT
+		//
+		await ownerAgent.delete(`/users/${member.id}`).expect(200, SUCCESS_RESPONSE_BODY);
+
+		//
+		// ASSERT
+		//
+		const userRepository = Container.get(UserRepository);
+		const projectRepository = Container.get(ProjectRepository);
+		const projectRelationRepository = Container.get(ProjectRelationRepository);
+		const sharedWorkflowRepository = Container.get(SharedWorkflowRepository);
+		const sharedCredentialsRepository = Container.get(SharedCredentialsRepository);
+
+		await Promise.all([
+			// user, their personal project and their relationship to the team project is gone
+			expect(userRepository.findOneBy({ id: member.id })).resolves.toBeNull(),
+			expect(projectRepository.findOneBy({ id: memberPersonalProject.id })).resolves.toBeNull(),
+			expect(
+				projectRelationRepository.findOneBy({ userId: member.id, projectId: teamProject.id }),
+			).resolves.toBeNull(),
+
+			// their personal workflows and and credentials are gone
+			expect(
+				sharedWorkflowRepository.findOneBy({
+					workflowId: savedWorkflow.id,
+					projectId: memberPersonalProject.id,
+				}),
+			).resolves.toBeNull(),
+			expect(
+				sharedCredentialsRepository.findOneBy({
+					credentialsId: savedCredential.id,
+					projectId: memberPersonalProject.id,
+				}),
+			).resolves.toBeNull(),
+
+			// team workflows and credentials are untouched
+			expect(
+				sharedWorkflowRepository.findOneBy({
+					workflowId: teamWorkflow.id,
+					projectId: teamProject.id,
+					role: 'workflow:owner',
+				}),
+			).resolves.not.toBeNull(),
+			expect(
+				sharedCredentialsRepository.findOneBy({
+					credentialsId: teamCredential.id,
+					projectId: teamProject.id,
+					role: 'credential:owner',
+				}),
+			).resolves.not.toBeNull(),
+		]);
 
 		const user = await Container.get(UserRepository).findOneBy({ id: member.id });
-
 		const sharedWorkflow = await Container.get(SharedWorkflowRepository).findOne({
-			relations: ['user'],
-			where: { userId: member.id, role: 'workflow:owner' },
+			where: { projectId: memberPersonalProject.id, role: 'workflow:owner' },
 		});
-
 		const sharedCredential = await Container.get(SharedCredentialsRepository).findOne({
-			relations: ['user'],
-			where: { userId: member.id, role: 'credential:owner' },
+			where: { projectId: memberPersonalProject.id, role: 'credential:owner' },
 		});
-
 		const workflow = await getWorkflowById(savedWorkflow.id);
-
 		const credential = await getCredentialById(savedCredential.id);
 
-		// @TODO: Include active workflow and check whether webhook has been removed
-
 		expect(user).toBeNull();
 		expect(sharedWorkflow).toBeNull();
 		expect(sharedCredential).toBeNull();
 		expect(workflow).toBeNull();
 		expect(credential).toBeNull();
-
-		// restore
-
-		member = await createMember();
 	});
 
-	test('should delete user and transfer their resources', async () => {
-		const [savedWorkflow, savedCredential] = await Promise.all([
-			await createWorkflow({ name: randomName() }, member),
-			await saveCredential(
-				{ name: randomName(), type: '', data: {} },
-				{
-					user: member,
+	test('should delete user and team relations and transfer their personal resources', async () => {
+		//
+		// ARRANGE
+		//
+		const [member, transferee, otherMember] = await Promise.all([
+			createMember(),
+			createMember(),
+			createMember(),
+		]);
+
+		// stays untouched
+		const teamProject = await createTeamProject();
+		await Promise.all([
+			// will be deleted
+			linkUserToProject(member, teamProject, 'project:admin'),
+
+			// stays untouched
+			linkUserToProject(transferee, teamProject, 'project:editor'),
+		]);
+
+		const [
+			ownedWorkflow,
+			ownedCredential,
+			teamWorkflow,
+			teamCredential,
+			sharedByOtherMemberWorkflow,
+			sharedByOtherMemberCredential,
+			sharedByTransfereeWorkflow,
+			sharedByTransfereeCredential,
+		] = await Promise.all([
+			// personal resource
+			// -> transferred to transferee's personal project
+			createWorkflow({}, member),
+			saveCredential(randomCredentialPayload(), {
+				user: member,
+				role: 'credential:owner',
+			}),
+
+			// resources in a team project
+			// -> untouched
+			createWorkflow({}, teamProject),
+			saveCredential(randomCredentialPayload(), {
+				project: teamProject,
+				role: 'credential:owner',
+			}),
+
+			// credential and workflow that are shared with the user to delete
+			// -> transferred to transferee's personal project
+			createWorkflow({}, otherMember),
+			saveCredential(randomCredentialPayload(), {
+				user: otherMember,
+				role: 'credential:owner',
+			}),
+
+			// credential and workflow that are shared with the user to delete but owned by the transferee
+			// -> not transferred but deleted
+			createWorkflow({}, transferee),
+			saveCredential(randomCredentialPayload(), {
+				user: transferee,
+				role: 'credential:owner',
+			}),
+		]);
+
+		await Promise.all([
+			shareWorkflowWithUsers(sharedByOtherMemberWorkflow, [member]),
+			shareCredentialWithUsers(sharedByOtherMemberCredential, [member]),
+
+			shareWorkflowWithUsers(sharedByTransfereeWorkflow, [member]),
+			shareCredentialWithUsers(sharedByTransfereeCredential, [member]),
+		]);
+
+		const [memberPersonalProject, transfereePersonalProject] = await Promise.all([
+			getPersonalProject(member),
+			getPersonalProject(transferee),
+		]);
+
+		const deleteSpy = jest.spyOn(Container.get(CacheService), 'deleteMany');
+
+		//
+		// ACT
+		//
+		await ownerAgent
+			.delete(`/users/${member.id}`)
+			.query({ transferId: transfereePersonalProject.id })
+			.expect(200);
+
+		//
+		// ASSERT
+		//
+
+		expect(deleteSpy).toBeCalledWith(
+			expect.arrayContaining([
+				`credential-can-use-secrets:${sharedByTransfereeCredential.id}`,
+				`credential-can-use-secrets:${ownedCredential.id}`,
+			]),
+		);
+		deleteSpy.mockClear();
+
+		const userRepository = Container.get(UserRepository);
+		const projectRepository = Container.get(ProjectRepository);
+		const projectRelationRepository = Container.get(ProjectRelationRepository);
+		const sharedWorkflowRepository = Container.get(SharedWorkflowRepository);
+		const sharedCredentialsRepository = Container.get(SharedCredentialsRepository);
+
+		await Promise.all([
+			// user, their personal project and their relationship to the team project is gone
+			expect(userRepository.findOneBy({ id: member.id })).resolves.toBeNull(),
+			expect(projectRepository.findOneBy({ id: memberPersonalProject.id })).resolves.toBeNull(),
+			expect(
+				projectRelationRepository.findOneBy({
+					projectId: teamProject.id,
+					userId: member.id,
+				}),
+			).resolves.toBeNull(),
+
+			// their owned workflow and credential are transferred to the transferee
+			expect(
+				sharedWorkflowRepository.findOneBy({
+					workflowId: ownedWorkflow.id,
+					projectId: transfereePersonalProject.id,
+					role: 'workflow:owner',
+				}),
+			).resolves.not.toBeNull,
+			expect(
+				sharedCredentialsRepository.findOneBy({
+					credentialsId: ownedCredential.id,
+					projectId: transfereePersonalProject.id,
 					role: 'credential:owner',
-				},
+				}),
+			).resolves.not.toBeNull(),
+
+			// the credential and workflow shared with them by another member is now shared with the transferee
+			expect(
+				sharedWorkflowRepository.findOneBy({
+					workflowId: sharedByOtherMemberWorkflow.id,
+					projectId: transfereePersonalProject.id,
+					role: 'workflow:editor',
+				}),
+			).resolves.not.toBeNull(),
+			expect(
+				sharedCredentialsRepository.findOneBy({
+					credentialsId: sharedByOtherMemberCredential.id,
+					projectId: transfereePersonalProject.id,
+					role: 'credential:user',
+				}),
 			),
+
+			// the transferee is still owner of the workflow and credential they shared with the user to delete
+			expect(
+				sharedWorkflowRepository.findOneBy({
+					workflowId: sharedByTransfereeWorkflow.id,
+					projectId: transfereePersonalProject.id,
+					role: 'workflow:owner',
+				}),
+			).resolves.not.toBeNull(),
+			expect(
+				sharedCredentialsRepository.findOneBy({
+					credentialsId: sharedByTransfereeCredential.id,
+					projectId: transfereePersonalProject.id,
+					role: 'credential:owner',
+				}),
+			).resolves.not.toBeNull(),
+
+			// the transferee's relationship to the team project is unchanged
+			expect(
+				projectRepository.findOneBy({
+					id: teamProject.id,
+					projectRelations: {
+						userId: transferee.id,
+						role: 'project:editor',
+					},
+				}),
+			).resolves.not.toBeNull(),
+
+			// the sharing of the team workflow is unchanged
+			expect(
+				sharedWorkflowRepository.findOneBy({
+					workflowId: teamWorkflow.id,
+					projectId: teamProject.id,
+					role: 'workflow:owner',
+				}),
+			).resolves.not.toBeNull(),
+
+			// the sharing of the team credential is unchanged
+			expect(
+				sharedCredentialsRepository.findOneBy({
+					credentialsId: teamCredential.id,
+					projectId: teamProject.id,
+					role: 'credential:owner',
+				}),
+			).resolves.not.toBeNull(),
 		]);
-
-		const response = await ownerAgent.delete(`/users/${member.id}`).query({
-			transferId: owner.id,
-		});
-
-		expect(response.statusCode).toBe(200);
-
-		const [user, sharedWorkflow, sharedCredential] = await Promise.all([
-			await Container.get(UserRepository).findOneBy({ id: member.id }),
-			await Container.get(SharedWorkflowRepository).findOneOrFail({
-				relations: ['workflow'],
-				where: { userId: owner.id },
-			}),
-			await Container.get(SharedCredentialsRepository).findOneOrFail({
-				relations: ['credentials'],
-				where: { userId: owner.id },
-			}),
-		]);
-
-		expect(user).toBeNull();
-		expect(sharedWorkflow.workflow.id).toBe(savedWorkflow.id);
-		expect(sharedCredential.credentials.id).toBe(savedCredential.id);
 	});
 
 	test('should fail to delete self', async () => {
-		const response = await ownerAgent.delete(`/users/${owner.id}`);
-
-		expect(response.statusCode).toBe(400);
+		await ownerAgent.delete(`/users/${owner.id}`).expect(400);
 
 		const user = await getUserById(owner.id);
 
 		expect(user).toBeDefined();
 	});
 
-	test('should fail to delete if user to delete is transferee', async () => {
-		const response = await ownerAgent.delete(`/users/${member.id}`).query({
-			transferId: member.id,
-		});
+	test('should fail to delete a user that does not exist', async () => {
+		await ownerAgent.delete(`/users/${uuid()}`).query({ transferId: '' }).expect(404);
+	});
 
-		expect(response.statusCode).toBe(400);
+	test('should fail to transfer to a project that does not exist', async () => {
+		const member = await createMember();
+
+		await ownerAgent.delete(`/users/${member.id}`).query({ transferId: 'foobar' }).expect(404);
+
+		const user = await Container.get(UserRepository).findOneBy({ id: member.id });
+
+		expect(user).toBeDefined();
+	});
+
+	test('should fail to delete if user to delete is transferee', async () => {
+		const member = await createMember();
+		const personalProject = await getPersonalProject(member);
+
+		await ownerAgent
+			.delete(`/users/${member.id}`)
+			.query({ transferId: personalProject.id })
+			.expect(400);
 
 		const user = await Container.get(UserRepository).findOneBy({ id: member.id });
 
@@ -355,8 +599,6 @@ describe('PATCH /users/:id/role', () => {
 	const { NO_ADMIN_ON_OWNER, NO_USER, NO_OWNER_ON_OWNER } =
 		UsersController.ERROR_MESSAGES.CHANGE_ROLE;
 
-	const UNAUTHORIZED = 'Unauthorized';
-
 	beforeAll(async () => {
 		await testDb.truncate(['User']);
 
@@ -400,66 +642,66 @@ describe('PATCH /users/:id/role', () => {
 
 	describe('member', () => {
 		test('should fail to demote owner to member', async () => {
-			const response = await memberAgent.patch(`/users/${owner.id}/role`).send({
-				newRoleName: 'global:member',
-			});
-
-			expect(response.statusCode).toBe(403);
-			expect(response.body.message).toBe(UNAUTHORIZED);
+			await memberAgent
+				.patch(`/users/${owner.id}/role`)
+				.send({
+					newRoleName: 'global:member',
+				})
+				.expect(403, { status: 'error', message: RESPONSE_ERROR_MESSAGES.MISSING_SCOPE });
 		});
 
 		test('should fail to demote owner to admin', async () => {
-			const response = await memberAgent.patch(`/users/${owner.id}/role`).send({
-				newRoleName: 'global:admin',
-			});
-
-			expect(response.statusCode).toBe(403);
-			expect(response.body.message).toBe(UNAUTHORIZED);
+			await memberAgent
+				.patch(`/users/${owner.id}/role`)
+				.send({
+					newRoleName: 'global:admin',
+				})
+				.expect(403, { status: 'error', message: RESPONSE_ERROR_MESSAGES.MISSING_SCOPE });
 		});
 
 		test('should fail to demote admin to member', async () => {
-			const response = await memberAgent.patch(`/users/${admin.id}/role`).send({
-				newRoleName: 'global:member',
-			});
-
-			expect(response.statusCode).toBe(403);
-			expect(response.body.message).toBe(UNAUTHORIZED);
+			await memberAgent
+				.patch(`/users/${admin.id}/role`)
+				.send({
+					newRoleName: 'global:member',
+				})
+				.expect(403, { status: 'error', message: RESPONSE_ERROR_MESSAGES.MISSING_SCOPE });
 		});
 
 		test('should fail to promote other member to owner', async () => {
-			const response = await memberAgent.patch(`/users/${otherMember.id}/role`).send({
-				newRoleName: 'global:owner',
-			});
-
-			expect(response.statusCode).toBe(403);
-			expect(response.body.message).toBe(UNAUTHORIZED);
+			await memberAgent
+				.patch(`/users/${otherMember.id}/role`)
+				.send({
+					newRoleName: 'global:owner',
+				})
+				.expect(403, { status: 'error', message: RESPONSE_ERROR_MESSAGES.MISSING_SCOPE });
 		});
 
 		test('should fail to promote other member to admin', async () => {
-			const response = await memberAgent.patch(`/users/${otherMember.id}/role`).send({
-				newRoleName: 'global:admin',
-			});
-
-			expect(response.statusCode).toBe(403);
-			expect(response.body.message).toBe(UNAUTHORIZED);
+			await memberAgent
+				.patch(`/users/${otherMember.id}/role`)
+				.send({
+					newRoleName: 'global:admin',
+				})
+				.expect(403, { status: 'error', message: RESPONSE_ERROR_MESSAGES.MISSING_SCOPE });
 		});
 
 		test('should fail to promote self to admin', async () => {
-			const response = await memberAgent.patch(`/users/${member.id}/role`).send({
-				newRoleName: 'global:admin',
-			});
-
-			expect(response.statusCode).toBe(403);
-			expect(response.body.message).toBe(UNAUTHORIZED);
+			await memberAgent
+				.patch(`/users/${member.id}/role`)
+				.send({
+					newRoleName: 'global:admin',
+				})
+				.expect(403, { status: 'error', message: RESPONSE_ERROR_MESSAGES.MISSING_SCOPE });
 		});
 
 		test('should fail to promote self to owner', async () => {
-			const response = await memberAgent.patch(`/users/${member.id}/role`).send({
-				newRoleName: 'global:owner',
-			});
-
-			expect(response.statusCode).toBe(403);
-			expect(response.body.message).toBe(UNAUTHORIZED);
+			await memberAgent
+				.patch(`/users/${member.id}/role`)
+				.send({
+					newRoleName: 'global:owner',
+				})
+				.expect(403, { status: 'error', message: RESPONSE_ERROR_MESSAGES.MISSING_SCOPE });
 		});
 	});
 
@@ -625,4 +867,40 @@ describe('PATCH /users/:id/role', () => {
 			adminAgent = testServer.authAgentFor(admin);
 		});
 	});
+
+	test("should clear credential external secrets usability cache when changing a user's role", async () => {
+		const user = await createAdmin();
+
+		const [project1, project2] = await Promise.all([
+			createTeamProject(undefined, user),
+			createTeamProject(),
+		]);
+
+		const [credential1, credential2, credential3] = await Promise.all([
+			saveCredential(randomCredentialPayload(), {
+				user,
+				role: 'credential:owner',
+			}),
+			saveCredential(randomCredentialPayload(), {
+				project: project1,
+				role: 'credential:owner',
+			}),
+			saveCredential(randomCredentialPayload(), {
+				project: project2,
+				role: 'credential:owner',
+			}),
+			linkUserToProject(user, project2, 'project:editor'),
+		]);
+
+		const deleteSpy = jest.spyOn(Container.get(CacheService), 'deleteMany');
+		const response = await ownerAgent.patch(`/users/${user.id}/role`).send({
+			newRoleName: 'global:member',
+		});
+
+		expect(deleteSpy).toBeCalledTimes(2);
+		deleteSpy.mockClear();
+
+		expect(response.statusCode).toBe(200);
+		expect(response.body.data).toStrictEqual({ success: true });
+	});
 });