refactor(core): Consolidate CredentialsService.getMany() (no-changelog)

packages/cli/src/WorkflowHelpers.ts

@@ -23,12 +23,7 @@ import {
 } from 'n8n-workflow';
 import { v4 as uuid } from 'uuid';
 import * as Db from '@/Db';
-import type {
-	ICredentialsDb,
-	IExecutionDb,
-	IWorkflowErrorData,
-	IWorkflowExecutionDataProcess,
-} from '@/Interfaces';
+import type { IExecutionDb, IWorkflowErrorData, IWorkflowExecutionDataProcess } from '@/Interfaces';
 import { NodeTypes } from '@/NodeTypes';
 // eslint-disable-next-line import/no-cycle
 import { WorkflowRunner } from '@/WorkflowRunner';
@@ -45,6 +40,7 @@ import type { RoleNames } from '@db/entities/Role';
 import { RoleService } from './services/role.service';
 import { ExecutionRepository, RoleRepository } from './databases/repositories';
 import { VariablesService } from './environments/variables/variables.service';
+import type { Credentials } from './requests';
 
 const ERROR_TRIGGER_TYPE = config.getEnv('nodes.errorTriggerType');
 
@@ -543,7 +539,7 @@ export function getNodesWithInaccessibleCreds(workflow: WorkflowEntity, userCred
 export function validateWorkflowCredentialUsage(
 	newWorkflowVersion: WorkflowEntity,
 	previousWorkflowVersion: WorkflowEntity,
-	credentialsUserHasAccessTo: ICredentialsDb[],
+	credentialsUserHasAccessTo: Credentials.WithOwnedByAndSharedWith[],
 ) {
 	/**
 	 * We only need to check nodes that use credentials the current user cannot access,

packages/cli/src/credentials/credentials.controller.ee.ts

@@ -1,16 +1,16 @@
 import express from 'express';
 import type { INodeCredentialTestResult } from 'n8n-workflow';
-import { deepCopy, LoggerProxy } from 'n8n-workflow';
+import { deepCopy } from 'n8n-workflow';
 import * as Db from '@/Db';
 import * as ResponseHelper from '@/ResponseHelper';
-import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
 
 import type { CredentialRequest } from '@/requests';
 import { isSharingEnabled, rightDiff } from '@/UserManagement/UserManagementHelper';
 import { EECredentialsService as EECredentials } from './credentials.service.ee';
-import type { CredentialWithSharings } from './credentials.types';
+import { OwnershipService } from '@/services/ownership.service';
 import { Container } from 'typedi';
 import { InternalHooks } from '@/InternalHooks';
+import type { CredentialsEntity } from '@/databases/entities/CredentialsEntity';
 
 // eslint-disable-next-line @typescript-eslint/naming-convention
 export const EECredentialsController = express.Router();
@@ -25,27 +25,6 @@ EECredentialsController.use((req, res, next) => {
 	next();
 });
 
-/**
- * GET /credentials
- */
-EECredentialsController.get(
-	'/',
-	ResponseHelper.send(async (req: CredentialRequest.GetAll): Promise<CredentialWithSharings[]> => {
-		try {
-			const allCredentials = await EECredentials.getAll(req.user, {
-				relations: ['shared', 'shared.role', 'shared.user'],
-			});
-
-			return allCredentials.map((credential: CredentialsEntity & CredentialWithSharings) =>
-				EECredentials.addOwnerAndSharings(credential),
-			);
-		} catch (error) {
-			LoggerProxy.error('Request to list credentials failed', error as Error);
-			throw error;
-		}
-	}),
-);
-
 /**
  * GET /credentials/:id
  */
@@ -59,7 +38,7 @@ EECredentialsController.get(
 		let credential = (await EECredentials.get(
 			{ id: credentialId },
 			{ relations: ['shared', 'shared.role', 'shared.user'] },
-		)) as CredentialsEntity & CredentialWithSharings;
+		)) as CredentialsEntity;
 
 		if (!credential) {
 			throw new ResponseHelper.NotFoundError(
@@ -73,7 +52,7 @@ EECredentialsController.get(
 			throw new ResponseHelper.UnauthorizedError('Forbidden.');
 		}
 
-		credential = EECredentials.addOwnerAndSharings(credential);
+		credential = Container.get(OwnershipService).addOwnedByAndSharedWith(credential);
 
 		if (!includeDecryptedData || !userSharing || userSharing.role.name !== 'owner') {
 			const { data: _, ...rest } = credential;

packages/cli/src/credentials/credentials.controller.ts

@@ -35,8 +35,8 @@ credentialsController.use('/', EECredentialsController);
  */
 credentialsController.get(
 	'/',
-	ResponseHelper.send(async (req: CredentialRequest.GetAll): Promise<ICredentialsDb[]> => {
-		return CredentialsService.getAll(req.user, { roles: ['owner'] });
+	ResponseHelper.send(async (req: CredentialRequest.GetAll) => {
+		return CredentialsService.getMany(req.user);
 	}),
 );
 

packages/cli/src/credentials/credentials.service.ee.ts

@@ -6,7 +6,6 @@ import { SharedCredentials } from '@db/entities/SharedCredentials';
 import type { User } from '@db/entities/User';
 import { UserService } from '@/services/user.service';
 import { CredentialsService } from './credentials.service';
-import type { CredentialWithSharings } from './credentials.types';
 import { RoleService } from '@/services/role.service';
 import Container from 'typedi';
 
@@ -93,27 +92,4 @@ export class EECredentialsService extends CredentialsService {
 
 		return transaction.save(newSharedCredentials);
 	}
-
-	static addOwnerAndSharings(
-		credential: CredentialsEntity & CredentialWithSharings,
-	): CredentialsEntity & CredentialWithSharings {
-		credential.ownedBy = null;
-		credential.sharedWith = [];
-
-		credential.shared?.forEach(({ user, role }) => {
-			const { id, email, firstName, lastName } = user;
-
-			if (role.name === 'owner') {
-				credential.ownedBy = { id, email, firstName, lastName };
-				return;
-			}
-
-			credential.sharedWith?.push({ id, email, firstName, lastName });
-		});
-
-		// @ts-ignore
-		delete credential.shared;
-
-		return credential;
-	}
 }

packages/cli/src/credentials/credentials.service.ts

@@ -24,6 +24,7 @@ import type { User } from '@db/entities/User';
 import type { CredentialRequest } from '@/requests';
 import { CredentialTypes } from '@/CredentialTypes';
 import { RoleService } from '@/services/role.service';
+import { OwnershipService } from '@/services/ownership.service';
 
 export class CredentialsService {
 	static async get(
@@ -36,48 +37,58 @@ export class CredentialsService {
 		});
 	}
 
-	static async getAll(
-		user: User,
-		options?: { relations?: string[]; roles?: string[]; disableGlobalRole?: boolean },
-	): Promise<ICredentialsDb[]> {
-		const SELECT_FIELDS: Array<keyof ICredentialsDb> = [
-			'id',
-			'name',
-			'type',
-			'nodesAccess',
-			'createdAt',
-			'updatedAt',
-		];
-
-		// if instance owner, return all credentials
-
-		if (user.globalRole.name === 'owner' && options?.disableGlobalRole !== true) {
-			return Db.collections.Credentials.find({
-				select: SELECT_FIELDS,
-				relations: options?.relations,
-			});
+	static async getMany(user: User, options?: { disableGlobalRole: boolean }) {
+		type Select = Array<keyof ICredentialsDb>;
+
+		const select: Select = ['id', 'name', 'type', 'nodesAccess', 'createdAt', 'updatedAt'];
+
+		const relations = ['shared', 'shared.role', 'shared.user'];
+
+		const returnAll = user.globalRole.name === 'owner' && options?.disableGlobalRole !== true;
+
+		const addOwnedByAndSharedWith = (c: CredentialsEntity) =>
+			Container.get(OwnershipService).addOwnedByAndSharedWith(c);
+
+		if (returnAll) {
+			const credentials = await Db.collections.Credentials.find({ select, relations });
+
+			return credentials.map(addOwnedByAndSharedWith);
 		}
 
-		// if member, return credentials owned by or shared with member
-		const userSharings = await Db.collections.SharedCredentials.find({
-			where: {
-				userId: user.id,
-				...(options?.roles?.length ? { role: { name: In(options.roles) } } : {}),
-			},
-			relations: options?.roles?.length ? ['role'] : [],
+		const ids = await CredentialsService.getAccessibleCredentials(user.id);
+
+		const credentials = await Db.collections.Credentials.find({
+			select,
+			relations,
+			where: { id: In(ids) },
 		});
 
-		return Db.collections.Credentials.find({
-			select: SELECT_FIELDS,
-			relations: options?.relations,
+		return credentials.map(addOwnedByAndSharedWith);
+	}
+
+	/**
+	 * Get the IDs of all credentials owned by or shared with a user.
+	 */
+	private static async getAccessibleCredentials(userId: string) {
+		const sharings = await Db.collections.SharedCredentials.find({
+			relations: ['role'],
 			where: {
-				id: In(userSharings.map((x) => x.credentialsId)),
+				userId,
+				role: { name: In(['owner', 'user']), scope: 'credential' },
 			},
 		});
+
+		return sharings.map((s) => s.credentialsId);
 	}
 
-	static async getMany(filter: FindManyOptions<ICredentialsDb>): Promise<ICredentialsDb[]> {
-		return Db.collections.Credentials.find(filter);
+	static async getManyByIds(ids: string[], { withSharings } = { withSharings: false }) {
+		const options: FindManyOptions<CredentialsEntity> = { where: { id: In(ids) } };
+
+		if (withSharings) {
+			options.relations = ['shared', 'shared.user', 'shared.role'];
+		}
+
+		return Db.collections.Credentials.find(options);
 	}
 
 	/**

packages/cli/src/requests.ts

@@ -27,6 +27,7 @@ import type { User } from '@db/entities/User';
 import type { UserManagementMailer } from '@/UserManagement/email';
 import type { Variables } from '@db/entities/Variables';
 import type { WorkflowEntity } from './databases/entities/WorkflowEntity';
+import type { CredentialsEntity } from './databases/entities/CredentialsEntity';
 
 export class UserUpdatePayload implements Pick<User, 'email' | 'firstName' | 'lastName'> {
 	@IsEmail()
@@ -162,6 +163,16 @@ export namespace ListQuery {
 	}
 }
 
+export namespace Credentials {
+	type SlimUser = Pick<IUser, 'id' | 'email' | 'firstName' | 'lastName'>;
+
+	type OwnedByField = { ownedBy: SlimUser | null };
+
+	type SharedWithField = { sharedWith: SlimUser[] };
+
+	export type WithOwnedByAndSharedWith = CredentialsEntity & OwnedByField & SharedWithField;
+}
+
 export function hasSharing(
 	workflows: ListQuery.Workflow.Plain[] | ListQuery.Workflow.WithSharing[],
 ): workflows is ListQuery.Workflow.WithSharing[] {

packages/cli/src/services/ownership.service.ts

@@ -4,8 +4,9 @@ import { SharedWorkflowRepository } from '@/databases/repositories';
 import type { User } from '@/databases/entities/User';
 import { RoleService } from './role.service';
 import { UserService } from './user.service';
-import type { ListQuery } from '@/requests';
+import type { Credentials, ListQuery } from '@/requests';
 import type { Role } from '@/databases/entities/Role';
+import type { CredentialsEntity } from '@/databases/entities/CredentialsEntity';
 
 @Service()
 export class OwnershipService {
@@ -50,4 +51,25 @@ export class OwnershipService {
 			ownedBy: ownerId ? { id: ownerId } : null,
 		});
 	}
+
+	addOwnedByAndSharedWith(_credential: CredentialsEntity): Credentials.WithOwnedByAndSharedWith {
+		const { shared, ...rest } = _credential;
+
+		const credential = rest as Credentials.WithOwnedByAndSharedWith;
+
+		credential.ownedBy = null;
+		credential.sharedWith = [];
+
+		shared?.forEach(({ user, role }) => {
+			const { id, email, firstName, lastName } = user;
+
+			if (role.name === 'owner') {
+				credential.ownedBy = { id, email, firstName, lastName };
+			} else {
+				credential.sharedWith.push({ id, email, firstName, lastName });
+			}
+		});
+
+		return credential;
+	}
 }

packages/cli/src/workflows/workflows.controller.ee.ts

@@ -12,7 +12,7 @@ import { EEWorkflowsService as EEWorkflows } from './workflows.services.ee';
 import { ExternalHooks } from '@/ExternalHooks';
 import { SharedWorkflow } from '@db/entities/SharedWorkflow';
 import { LoggerProxy } from 'n8n-workflow';
-import { EECredentialsService as EECredentials } from '../credentials/credentials.service.ee';
+import { CredentialsService } from '../credentials/credentials.service';
 import type { IExecutionPushResponse } from '@/Interfaces';
 import * as GenericHelpers from '@/GenericHelpers';
 import { In } from 'typeorm';
@@ -151,7 +151,7 @@ EEWorkflowController.post(
 		// This is a new workflow, so we simply check if the user has access to
 		// all used workflows
 
-		const allCredentials = await EECredentials.getAll(req.user);
+		const allCredentials = await CredentialsService.getMany(req.user);
 
 		try {
 			EEWorkflows.validateCredentialPermissionsToUser(newWorkflow, allCredentials);

packages/cli/src/workflows/workflows.services.ee.ts

@@ -3,7 +3,6 @@ import { In, Not } from 'typeorm';
 import * as Db from '@/Db';
 import * as ResponseHelper from '@/ResponseHelper';
 import * as WorkflowHelpers from '@/WorkflowHelpers';
-import type { ICredentialsDb } from '@/Interfaces';
 import { SharedWorkflow } from '@db/entities/SharedWorkflow';
 import type { User } from '@db/entities/User';
 import { WorkflowEntity } from '@db/entities/WorkflowEntity';
@@ -13,10 +12,11 @@ import type {
 	CredentialUsedByWorkflow,
 	WorkflowWithSharingsAndCredentials,
 } from './workflows.types';
-import { EECredentialsService as EECredentials } from '@/credentials/credentials.service.ee';
+import { CredentialsService } from '@/credentials/credentials.service';
 import { NodeOperationError } from 'n8n-workflow';
 import { RoleService } from '@/services/role.service';
 import Container from 'typedi';
+import type { Credentials } from '@/requests';
 
 export class EEWorkflowsService extends WorkflowsService {
 	static async isOwned(
@@ -106,7 +106,9 @@ export class EEWorkflowsService extends WorkflowsService {
 		currentUser: User,
 	): Promise<void> {
 		workflow.usedCredentials = [];
-		const userCredentials = await EECredentials.getAll(currentUser, { disableGlobalRole: true });
+		const userCredentials = await CredentialsService.getMany(currentUser, {
+			disableGlobalRole: true,
+		});
 		const credentialIdsUsedByWorkflow = new Set<string>();
 		workflow.nodes.forEach((node) => {
 			if (!node.credentials) {
@@ -120,12 +122,10 @@ export class EEWorkflowsService extends WorkflowsService {
 				credentialIdsUsedByWorkflow.add(credential.id);
 			});
 		});
-		const workflowCredentials = await EECredentials.getMany({
-			where: {
-				id: In(Array.from(credentialIdsUsedByWorkflow)),
-			},
-			relations: ['shared', 'shared.user', 'shared.role'],
-		});
+		const workflowCredentials = await CredentialsService.getManyByIds(
+			Array.from(credentialIdsUsedByWorkflow),
+			{ withSharings: true },
+		);
 		const userCredentialIds = userCredentials.map((credential) => credential.id);
 		workflowCredentials.forEach((credential) => {
 			const credentialId = credential.id;
@@ -151,7 +151,7 @@ export class EEWorkflowsService extends WorkflowsService {
 
 	static validateCredentialPermissionsToUser(
 		workflow: WorkflowEntity,
-		allowedCredentials: ICredentialsDb[],
+		allowedCredentials: Credentials.WithOwnedByAndSharedWith[],
 	) {
 		workflow.nodes.forEach((node) => {
 			if (!node.credentials) {
@@ -175,7 +175,7 @@ export class EEWorkflowsService extends WorkflowsService {
 			throw new ResponseHelper.NotFoundError('Workflow not found');
 		}
 
-		const allCredentials = await EECredentials.getAll(user);
+		const allCredentials = await CredentialsService.getMany(user);
 
 		try {
 			return WorkflowHelpers.validateWorkflowCredentialUsage(