⚡ Refactor DELETE /users

packages/cli/src/PublicApi/helpers.ts

@@ -1,18 +1,21 @@
+/* eslint-disable import/no-cycle */
 import * as querystring from 'querystring';
 // eslint-disable-next-line import/no-extraneous-dependencies
 import { pick } from 'lodash';
 import express = require('express');
 import * as SwaggerParser from '@apidevtools/swagger-parser';
 import { In } from 'typeorm';
-// eslint-disable-next-line import/no-cycle
+import { validate as uuidValidate } from 'uuid';
+import { Workflow } from 'n8n-workflow';
+import { worker } from 'cluster';
 import { User } from '../databases/entities/User';
 import type { Role } from '../databases/entities/Role';
-// eslint-disable-next-line import/no-cycle
+import { ActiveWorkflowRunner, Db, InternalHooksManager, ITelemetryUserDeletionData } from '..';
-import { Db, InternalHooksManager } from '..';
-// eslint-disable-next-line import/no-cycle
 import { getInstanceBaseUrl } from '../UserManagement/UserManagementHelper';
-// eslint-disable-next-line import/no-cycle
 import * as UserManagementMailer from '../UserManagement/email';
+import { SharedWorkflow } from '../databases/entities/SharedWorkflow';
+import { SharedCredentials } from '../databases/entities/SharedCredentials';
+import { WorkflowEntity } from '../databases/entities/WorkflowEntity';
 
 interface IPaginationOffsetDecoded {
 	offset: number;
@@ -68,9 +71,9 @@ export const connectionName = (): string => {
 	return 'default';
 };
 
-export const clean = (users: User[], keepRole = false): Array<Partial<User>> => {
+export const clean = (users: User[], options?: { includeRole: boolean }): Array<Partial<User>> => {
 	return users.map((user) =>
-		pick(user, getSelectableProperties('user').concat(keepRole ? ['globalRole'] : [])),
+		pick(user, getSelectableProperties('user').concat(options?.includeRole ? ['globalRole'] : [])),
 	);
 };
 
@@ -226,3 +229,118 @@ export async function inviteUsers(
 		}
 	});
 }
+
+export async function getUserByIdentifier(
+	identifier: string,
+	options?: { includeRole: boolean },
+): Promise<User | undefined> {
+	return Db.collections.User?.findOneOrFail({
+		where: {
+			...(uuidValidate(identifier) && { id: identifier }),
+			...(!uuidValidate(identifier) && { email: identifier }),
+		},
+		relations: options?.includeRole ? ['globalRole'] : undefined,
+	});
+}
+
+export async function getUsers(data: {
+	includeRole?: boolean;
+	withIdentifiers: string[];
+}): Promise<User[] | undefined> {
+	return Db.collections.User?.find({
+		where: {
+			...(uuidValidate(data.withIdentifiers[0]) && { id: In(data.withIdentifiers) }),
+			...(!uuidValidate(data.withIdentifiers[0]) && { email: In(data.withIdentifiers) }),
+		},
+		relations: data?.includeRole ? ['globalRole'] : undefined,
+	});
+}
+
+export async function transferWorkflowsAndCredentials(data: {
+	fromUser: User;
+	toUser: User;
+}): Promise<void> {
+	return Db.transaction(async (transactionManager) => {
+		await transactionManager.update(SharedWorkflow, { user: data.fromUser }, { user: data.toUser });
+		await transactionManager.update(
+			SharedCredentials,
+			{ user: data.fromUser },
+			{ user: data.toUser },
+		);
+		await transactionManager.delete(User, { id: data.fromUser });
+	});
+}
+
+async function getSharedWorkflows(data: { fromUser: User }): Promise<SharedWorkflow[] | undefined> {
+	return Db.collections.SharedWorkflow?.find({
+		relations: ['workflow'],
+		where: { user: data.fromUser },
+	});
+}
+
+async function getSharedCredentials(data: {
+	fromUser: User;
+}): Promise<SharedCredentials[] | undefined> {
+	return Db.collections.SharedCredentials?.find({
+		relations: ['credentials'],
+		where: { user: data.fromUser },
+	});
+}
+
+export async function getSharedWorkflowsAndCredentials(data: { fromUser: User }): Promise<{
+	workflows: SharedWorkflow[] | undefined;
+	credentials: SharedCredentials[] | undefined;
+}> {
+	return {
+		workflows: await getSharedWorkflows(data),
+		credentials: await getSharedCredentials(data),
+	};
+}
+
+async function desactiveWorkflow(data: { workflow: WorkflowEntity }) {
+	if (data.workflow.active) {
+		const activeWorkflowRunner = ActiveWorkflowRunner.getInstance();
+		void activeWorkflowRunner.remove(data.workflow?.id.toString());
+	}
+	return data.workflow;
+}
+
+async function deleteWorkflowsAndCredentials(data: { fromUser: User }): Promise<void> {
+	const { credentials: sharedCredentials = [], workflows: sharedWorkflows = [] } =
+		await getSharedWorkflowsAndCredentials(data);
+	await Db.transaction(async (transactionManager) => {
+		const ownedWorkflows = await Promise.all(sharedWorkflows.map(desactiveWorkflow));
+		await transactionManager.remove(ownedWorkflows);
+		await transactionManager.remove(sharedCredentials.map(({ credentials }) => credentials));
+		await transactionManager.delete(User, { id: data.fromUser });
+	});
+}
+
+export async function sendUserDeleteTelemetry(data: {
+	apiKeyOwnerUser: User;
+	fromUser: User;
+	transferId: string | undefined;
+}): Promise<void> {
+	const telemetryData: ITelemetryUserDeletionData = {
+		user_id: data.apiKeyOwnerUser.id,
+		target_user_old_status: data.fromUser.isPending ? 'invited' : 'active',
+		target_user_id: data.fromUser.id,
+	};
+
+	telemetryData.migration_strategy = data.transferId ? 'transfer_data' : 'delete_data';
+
+	if (data.transferId) {
+		telemetryData.migration_user_id = data.transferId;
+	}
+
+	void InternalHooksManager.getInstance().onUserDeletion(data.apiKeyOwnerUser.id, telemetryData);
+}
+
+export async function deleteDataAndSendTelemetry(data: {
+	fromUser: User;
+	apiKeyOwnerUser: User;
+	transferId: string | undefined;
+}): Promise<void> {
+	await deleteWorkflowsAndCredentials(data);
+	await sendUserDeleteTelemetry(data);
+}

packages/cli/src/PublicApi/middlewares.ts

@@ -58,8 +58,40 @@ const validEmail = (
 	next();
 };
 
+const deletingOwnUser = (
+	req: UserRequest.Delete,
+	res: express.Response,
+	next: express.NextFunction,
+): any => {
+	if (req.user.id === req.params.identifier) {
+		return res.status(400).json({
+			message: `Cannot delete your own user`,
+		});
+	}
+	next();
+};
+
+const transferingToDeletedUser = (
+	req: UserRequest.Delete,
+	res: express.Response,
+	next: express.NextFunction,
+): any => {
+	if (req.query.transferId === req.params.identifier) {
+		return res.status(400).json({
+			message: `Request to delete a user failed because the user to delete and the transferee are the same user`,
+		});
+	}
+	next();
+};
+
 export const middlewares = {
 	createUsers: [instanceOwnerSetup, emailSetup, validEmail, authorize(['owner'])],
+	deleteUsers: [
+		instanceOwnerSetup,
+		deletingOwnUser,
+		transferingToDeletedUser,
+		authorize(['owner']),
+	],
 	getUsers: [instanceOwnerSetup, authorize(['owner'])],
 	getUser: [instanceOwnerSetup, authorize(['owner'])],
 };

packages/cli/src/PublicApi/v1/routes/Users/index.ts

@@ -12,25 +12,20 @@ import {
 	clean,
 	connectionName,
 	decodeCursor,
+	deleteDataAndSendTelemetry,
 	getGlobalMemberRole,
 	getNextCursor,
 	getSelectableProperties,
+	getUsers,
 	getUsersToSaveAndInvite,
 	inviteUsers,
 	saveUsersWithRole,
+	transferWorkflowsAndCredentials,
 } from '../../../helpers';
 
 import * as UserManagementMailer from '../../../../UserManagement/email/UserManagementMailer';
 
-import {
+import { Db, ResponseHelper } from '../../../..';
-	Db,
-	ResponseHelper,
-	InternalHooksManager,
-	ActiveWorkflowRunner,
-	ITelemetryUserDeletionData,
-} from '../../../..';
-import { SharedWorkflow } from '../../../../databases/entities/SharedWorkflow';
-import { SharedCredentials } from '../../../../databases/entities/SharedCredentials';
 
 export = {
 	createUsers: ResponseHelper.send(async (req: UserRequest.Invite, res: express.Response) => {
@@ -81,97 +76,40 @@ export = {
 	// eslint-disable-next-line consistent-return
 	deleteUser: async (req: UserRequest.Delete, res: express.Response): Promise<any> => {
 		const { identifier: idToDelete } = req.params;
-
+		const { transferId } = req.query;
+		const apiKeyUserOwner = req.user;
 		const includeRole = req.query?.includeRole?.toLowerCase() === 'true' || false;
 
-		if (req.user.id === idToDelete) {
+		const users = await getUsers({ withIdentifiers: [idToDelete, transferId ?? ''], includeRole });
-			return res.status(400).json({
-				message: `Cannot delete your own user`,
-			});
-		}
-
-		const { transferId } = req.query;
-
-		if (transferId === idToDelete) {
-			return res.status(400).json({
-				message: `Request to delete a user failed because the user to delete and the transferee are the same user`,
-			});
-		}
-
-		const users = await Db.collections.User?.find({
-			where: { id: In([transferId, idToDelete]) },
-			relations: includeRole ? ['globalRole'] : undefined,
-		});
 
 		if (!users?.length || (transferId && users.length !== 2)) {
-			return res.status(400).json({
+			throw new ResponseHelper.ResponseError(
-				message: `Request to delete a user failed because the ID of the user to delete and/or the ID of the transferee were not found in DB`,
+				'Request to delete a user failed because the ID of the user to delete and/or the ID of the transferee were not found in DB',
-			});
+				undefined,
+				400,
+			);
 		}
 
 		const userToDelete = users?.find((user) => user.id === req.params.identifier) as User;
 
 		if (transferId) {
-			const transferee = users?.find((user) => user.id === transferId);
+			const transferee = users?.find((user) => user.id === transferId) as User;
-			await Db.transaction(async (transactionManager) => {
+
-				await transactionManager.update(
+			await transferWorkflowsAndCredentials({
-					SharedWorkflow,
+				fromUser: userToDelete,
-					{ user: userToDelete },
+				toUser: transferee,
-					{ user: transferee },
-				);
-				await transactionManager.update(
-					SharedCredentials,
-					{ user: userToDelete },
-					{ user: transferee },
-				);
-				await transactionManager.delete(User, { id: userToDelete.id });
 			});
 
-			res.json(clean([userToDelete], true)[0]);
+			return clean([userToDelete]).pop();
 		}
 
-		const [ownedSharedWorkflows = [], ownedSharedCredentials = []] = await Promise.all([
+		await deleteDataAndSendTelemetry({
-			Db.collections.SharedWorkflow?.find({
+			fromUser: userToDelete,
-				relations: ['workflow'],
+			apiKeyOwnerUser: apiKeyUserOwner,
-				where: { user: userToDelete },
+			transferId,
-			}),
-			Db.collections.SharedCredentials?.find({
-				relations: ['credentials'],
-				where: { user: userToDelete },
-			}),
-		]);
-
-		await Db.transaction(async (transactionManager) => {
-			const ownedWorkflows = await Promise.all(
-				ownedSharedWorkflows.map(async ({ workflow }) => {
-					if (workflow.active) {
-						const activeWorkflowRunner = ActiveWorkflowRunner.getInstance();
-						// deactivate before deleting
-						void activeWorkflowRunner.remove(workflow.id.toString());
-					}
-					return workflow;
-				}),
-			);
-			await transactionManager.remove(ownedWorkflows);
-			await transactionManager.remove(ownedSharedCredentials.map(({ credentials }) => credentials));
-			await transactionManager.delete(User, { id: userToDelete.id });
 		});
 
-		const telemetryData: ITelemetryUserDeletionData = {
+		return clean([userToDelete], { includeRole }).pop();
-			user_id: req.user.id,
-			target_user_old_status: userToDelete.isPending ? 'invited' : 'active',
-			target_user_id: idToDelete,
-		};
-
-		telemetryData.migration_strategy = transferId ? 'transfer_data' : 'delete_data';
-
-		if (transferId) {
-			telemetryData.migration_user_id = transferId;
-		}
-
-		void InternalHooksManager.getInstance().onUserDeletion(req.user.id, telemetryData);
-
-		res.json(clean([userToDelete], true)[0]);
 	},
 	// eslint-disable-next-line consistent-return
 	getUser: async (req: UserRequest.Get, res: express.Response): Promise<any> => {