fix(core): Fix PermissionChecker.check, and add additional unit tests (…

…#8528)
n8n-io · Feb 2, 2024 · 6385fb2 · 6385fb2
1 parent d0e94b0
commit 6385fb2
Show file tree

Hide file tree

Showing 6 changed files with 518 additions and 376 deletions.
diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts
@@ -39,24 +39,20 @@ export class PermissionChecker {
 
 		if (user.hasGlobalScope('workflow:execute')) return;
 
+		const isSharingEnabled = this.license.isSharingEnabled();
+
 		// allow if all creds used in this workflow are a subset of
 		// all creds accessible to users who have access to this workflow
 
 		let workflowUserIds = [userId];
 
-		if (workflow.id && this.license.isSharingEnabled()) {
-			const workflowSharings = await this.sharedWorkflowRepository.find({
-				relations: ['workflow'],
-				where: { workflowId: workflow.id },
-				select: ['userId'],
-			});
-			workflowUserIds = workflowSharings.map((s) => s.userId);
+		if (workflow.id && isSharingEnabled) {
+			workflowUserIds = await this.sharedWorkflowRepository.getSharedUserIds(workflow.id);
 		}
 
-		const credentialSharings =
-			await this.sharedCredentialsRepository.findOwnedSharings(workflowUserIds);
-
-		const accessibleCredIds = credentialSharings.map((s) => s.credentialsId);
+		const accessibleCredIds = isSharingEnabled
+			? await this.sharedCredentialsRepository.getAccessibleCredentialIds(workflowUserIds)
+			: await this.sharedCredentialsRepository.getOwnedCredentialIds(workflowUserIds);
 
 		const inaccessibleCredIds = workflowCredIds.filter((id) => !accessibleCredIds.includes(id));
 

diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts
@@ -63,7 +63,7 @@ export class CredentialsService {
 				: credentials;
 		}
 
-		const ids = await this.sharedCredentialsRepository.getAccessibleCredentials(user.id);
+		const ids = await this.sharedCredentialsRepository.getAccessibleCredentialIds([user.id]);
 
 		const credentials = await this.credentialsRepository.findMany(
 			options.listQueryOptions,

diff --git a/packages/cli/src/databases/repositories/sharedCredentials.repository.ts b/packages/cli/src/databases/repositories/sharedCredentials.repository.ts
@@ -1,7 +1,7 @@
 import { Service } from 'typedi';
 import type { EntityManager } from 'typeorm';
 import { DataSource, In, Not, Repository } from 'typeorm';
-import { SharedCredentials } from '../entities/SharedCredentials';
+import { type CredentialSharingRole, SharedCredentials } from '../entities/SharedCredentials';
 import type { User } from '../entities/User';
 
 @Service()
@@ -36,27 +36,27 @@ export class SharedCredentialsRepository extends Repository<SharedCredentials> {
 		return await this.update({ userId: Not(user.id), role: 'credential:owner' }, { user });
 	}
 
-	/**
-	 * Get the IDs of all credentials owned by or shared with a user.
-	 */
-	async getAccessibleCredentials(userId: string) {
-		const sharings = await this.find({
-			where: {
-				userId,
-				role: In(['credential:owner', 'credential:user']),
-			},
-		});
+	/** Get the IDs of all credentials owned by a user */
+	async getOwnedCredentialIds(userIds: string[]) {
+		return await this.getCredentialIdsByUserAndRole(userIds, ['credential:owner']);
+	}
 
-		return sharings.map((s) => s.credentialsId);
+	/** Get the IDs of all credentials owned by or shared with a user */
+	async getAccessibleCredentialIds(userIds: string[]) {
+		return await this.getCredentialIdsByUserAndRole(userIds, [
+			'credential:owner',
+			'credential:user',
+		]);
 	}
 
-	async findOwnedSharings(userIds: string[]) {
-		return await this.find({
+	private async getCredentialIdsByUserAndRole(userIds: string[], roles: CredentialSharingRole[]) {
+		const sharings = await this.find({
 			where: {
 				userId: In(userIds),
-				role: 'credential:owner',
+				role: In(roles),
 			},
 		});
+		return sharings.map((s) => s.credentialsId);
 	}
 
 	async deleteByIds(transaction: EntityManager, sharedCredentialsIds: string[], user?: User) {

diff --git a/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts b/packages/cli/src/databases/repositories/sharedWorkflow.repository.ts
@@ -22,6 +22,15 @@ export class SharedWorkflowRepository extends Repository<SharedWorkflow> {
 		return await this.exist({ where });
 	}
 
+	/** Get the IDs of all users this workflow is shared with */
+	async getSharedUserIds(workflowId: string) {
+		const sharedWorkflows = await this.find({
+			select: ['userId'],
+			where: { workflowId },
+		});
+		return sharedWorkflows.map((sharing) => sharing.userId);
+	}
+
 	async getSharedWorkflowIds(workflowIds: string[]) {
 		const sharedWorkflows = await this.find({
 			select: ['workflowId'],