fix(core): Don't allow using credentials that are not part of the sam…

…e project (#9916)
n8n-io · Jul 3, 2024 · ab2a548 · ab2a548
1 parent 962f0d7
commit ab2a548
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 57 deletions.
diff --git a/packages/cli/src/workflows/workflow.service.ee.ts b/packages/cli/src/workflows/workflow.service.ee.ts
@@ -141,7 +141,10 @@ export class EnterpriseWorkflowService {
 			throw new NotFoundError('Workflow not found');
 		}
 
-		const allCredentials = await this.credentialsService.getMany(user);
+		const allCredentials = await this.credentialsService.getCredentialsAUserCanUseInAWorkflow(
+			user,
+			{ workflowId },
+		);
 
 		try {
 			return this.validateWorkflowCredentialUsage(workflow, previousVersion, allCredentials);
@@ -158,7 +161,7 @@ export class EnterpriseWorkflowService {
 	validateWorkflowCredentialUsage(
 		newWorkflowVersion: WorkflowEntity,
 		previousWorkflowVersion: WorkflowEntity,
-		credentialsUserHasAccessTo: CredentialsEntity[],
+		credentialsUserHasAccessTo: Array<{ id: string }>,
 	) {
 		/**
 		 * We only need to check nodes that use credentials the current user cannot access,

diff --git a/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts b/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts
@@ -829,64 +829,104 @@ describe('PATCH /workflows/:workflowId', () => {
 			expect(response.statusCode).toBe(200);
 		});
 
-		it('Should prevent member from adding node containing credential inaccessible to member', async () => {
-			const savedCredential = await saveCredential(randomCredentialPayload(), { user: owner });
-
-			const workflow = {
-				name: 'test',
-				active: false,
-				connections: {},
-				nodes: [
-					{
-						id: 'uuid-1234',
-						name: 'Start',
-						parameters: {},
-						position: [-20, 260],
-						type: 'n8n-nodes-base.start',
-						typeVersion: 1,
-						credentials: {
-							default: {
-								id: savedCredential.id,
-								name: savedCredential.name,
+		it.each([
+			[
+				'the owner and shared with the member',
+				'the owner',
+				async function creteWorkflow() {
+					const workflow = await createWorkflow({}, owner);
+					await shareWorkflowWithUsers(workflow, [member]);
+					return workflow;
+				},
+				async function createCredential() {
+					return await saveCredential(randomCredentialPayload(), { user: owner });
+				},
+			],
+			[
+				'team 1',
+				'the member',
+				async function creteWorkflow() {
+					const team = await createTeamProject('Team 1', member);
+					return await createWorkflow({}, team);
+				},
+				async function createCredential() {
+					return await saveCredential(randomCredentialPayload(), { user: member });
+				},
+			],
+			[
+				'team 1',
+				'team 2',
+				async function creteWorkflow() {
+					const team1 = await createTeamProject('Team 1', member);
+					return await createWorkflow({}, team1);
+				},
+				async function createCredential() {
+					const team2 = await createTeamProject('Team 2', member);
+					return await saveCredential(randomCredentialPayload(), { project: team2 });
+				},
+			],
+			[
+				'the member',
+				'the owner',
+				async function creteWorkflow() {
+					return await createWorkflow({}, member);
+				},
+				async function createCredential() {
+					return await saveCredential(randomCredentialPayload(), { user: owner });
+				},
+			],
+			[
+				'the member',
+				'team 2',
+				async function creteWorkflow() {
+					return await createWorkflow({}, member);
+				},
+				async function createCredential() {
+					const team2 = await createTeamProject('Team 2', member);
+					return await saveCredential(randomCredentialPayload(), { project: team2 });
+				},
+			],
+		])(
+			'Tamper proofing kicks in if the workflow is owned by %s, the credentials is owned by %s, and the member tries to use the credential in the workflow',
+			async (_workflowText, _credentialText, createWorkflow, createCredential) => {
+				//
+				// ARRANGE
+				//
+				const workflow = await createWorkflow();
+				const credential = await createCredential();
+
+				//
+				// ACT
+				//
+				const response = await authMemberAgent.patch(`/workflows/${workflow.id}`).send({
+					versionId: workflow.versionId,
+					nodes: [
+						{
+							id: 'uuid-12345',
+							name: 'Start',
+							parameters: {},
+							position: [-20, 260],
+							type: 'n8n-nodes-base.start',
+							typeVersion: 1,
+							credentials: {
+								default: {
+									id: credential.id,
+									name: credential.name,
+								},
 							},
 						},
-					},
-				],
-			};
-
-			const createResponse = await authOwnerAgent.post('/workflows').send(workflow);
-			const { id, versionId } = createResponse.body.data;
+					],
+				});
 
-			const response = await authMemberAgent.patch(`/workflows/${id}`).send({
-				versionId,
-				nodes: [
-					{
-						id: 'uuid-1234',
-						name: 'Start',
-						parameters: {},
-						position: [-20, 260],
-						type: 'n8n-nodes-base.start',
-						typeVersion: 1,
-						credentials: {},
-					},
-					{
-						id: 'uuid-12345',
-						name: 'Start',
-						parameters: {},
-						position: [-20, 260],
-						type: 'n8n-nodes-base.start',
-						typeVersion: 1,
-						credentials: {
-							default: {
-								id: savedCredential.id,
-								name: savedCredential.name,
-							},
-						},
-					},
-				],
-			});
-			expect(response.statusCode).toBe(403);
-		});
+				//
+				// ASSERT
+				//
+				expect(response.statusCode).toBe(400);
+				expect(response.body.message).toBe(
+					"You don't have access to the credentials in the 'Start' node. Ask the owner to share them with you.",
+				);
+			},
+		);
 
 		it('Should succeed but prevent modifying node attributes other than position, name and disabled', async () => {
 			const savedCredential = await saveCredential(randomCredentialPayload(), { user: member });