fix(core): User update endpoint should only allow updating email, fir…

…stName, and lastName (n8n-io#5526)

packages/cli/package.json

@@ -125,6 +125,7 @@
     "callsites": "^3.1.0",
     "change-case": "^4.1.1",
     "class-validator": "^0.14.0",
+    "class-transformer": "^0.5.1",
     "client-oauth2": "^4.2.5",
     "compression": "^1.7.4",
     "connect-history-api-fallback": "^1.6.0",

packages/cli/src/GenericHelpers.ts

@@ -22,6 +22,7 @@ import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
 import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
 import type { TagEntity } from '@db/entities/TagEntity';
 import type { User } from '@db/entities/User';
+import type { UserUpdatePayload } from '@/requests';
 
 /**
  * Returns the base URL n8n is reachable from
@@ -99,7 +100,7 @@ export async function generateUniqueName(
 }
 
 export async function validateEntity(
-	entity: WorkflowEntity | CredentialsEntity | TagEntity | User,
+	entity: WorkflowEntity | CredentialsEntity | TagEntity | User | UserUpdatePayload,
 ): Promise<void> {
 	const errors = await validate(entity);
 

packages/cli/src/controllers/me.controller.ts

@@ -1,4 +1,5 @@
 import validator from 'validator';
+import { plainToInstance } from 'class-transformer';
 import { Delete, Get, Patch, Post, RestController } from '@/decorators';
 import {
 	compareHash,
@@ -7,13 +8,13 @@ import {
 	validatePassword,
 } from '@/UserManagement/UserManagementHelper';
 import { BadRequestError } from '@/ResponseHelper';
-import { User } from '@db/entities/User';
+import type { User } from '@db/entities/User';
 import { validateEntity } from '@/GenericHelpers';
 import { issueCookie } from '@/auth/jwt';
 import { Response } from 'express';
 import type { Repository } from 'typeorm';
 import type { ILogger } from 'n8n-workflow';
-import { AuthenticatedRequest, MeRequest } from '@/requests';
+import { AuthenticatedRequest, MeRequest, UserUpdatePayload } from '@/requests';
 import type {
 	PublicUser,
 	IDatabaseCollections,
@@ -61,38 +62,40 @@ export class MeController {
 	 * Update the logged-in user's settings, except password.
 	 */
 	@Patch('/')
-	async updateCurrentUser(req: MeRequest.Settings, res: Response): Promise<PublicUser> {
-		const { email } = req.body;
+	async updateCurrentUser(req: MeRequest.UserUpdate, res: Response): Promise<PublicUser> {
+		const { id: userId, email: currentEmail } = req.user;
+		const payload = plainToInstance(UserUpdatePayload, req.body);
+
+		const { email } = payload;
 		if (!email) {
 			this.logger.debug('Request to update user email failed because of missing email in payload', {
-				userId: req.user.id,
-				payload: req.body,
+				userId,
+				payload,
 			});
 			throw new BadRequestError('Email is mandatory');
 		}
 
 		if (!validator.isEmail(email)) {
 			this.logger.debug('Request to update user email failed because of invalid email in payload', {
-				userId: req.user.id,
+				userId,
 				invalidEmail: email,
 			});
 			throw new BadRequestError('Invalid email address');
 		}
 
-		const { email: currentEmail } = req.user;
-		const newUser = new User();
-
-		Object.assign(newUser, req.user, req.body);
+		await validateEntity(payload);
 
-		await validateEntity(newUser);
-
-		const user = await this.userRepository.save(newUser);
+		await this.userRepository.update(userId, payload);
+		const user = await this.userRepository.findOneOrFail({
+			where: { id: userId },
+			relations: { globalRole: true },
+		});
 
-		this.logger.info('User updated successfully', { userId: user.id });
+		this.logger.info('User updated successfully', { userId });
 
 		await issueCookie(res, user);
 
-		const updatedKeys = Object.keys(req.body);
+		const updatedKeys = Object.keys(payload);
 		void this.internalHooks.onUserUpdate({
 			user,
 			fields_changed: updatedKeys,

packages/cli/src/databases/entities/User.ts

@@ -111,6 +111,9 @@ export class User extends AbstractEntity implements IUser {
 	@AfterLoad()
 	@AfterUpdate()
 	computeIsPending(): void {
-		this.isPending = this.password === null;
+		this.isPending =
+			this.globalRole?.name === 'owner' && this.globalRole.scope === 'global'
+				? false
+				: this.password === null;
 	}
 }

packages/cli/src/requests.ts

@@ -10,11 +10,28 @@ import type {
 	IWorkflowSettings,
 } from 'n8n-workflow';
 
+import { IsEmail, IsString, Length } from 'class-validator';
+import { NoXss } from '@db/utils/customValidators';
 import type { PublicUser, IExecutionDeleteFilter, IWorkflowDb } from '@/Interfaces';
 import type { Role } from '@db/entities/Role';
 import type { User } from '@db/entities/User';
 import type * as UserManagementMailer from '@/UserManagement/email/UserManagementMailer';
 
+export class UserUpdatePayload implements Pick<User, 'email' | 'firstName' | 'lastName'> {
+	@IsEmail()
+	email: string;
+
+	@NoXss()
+	@IsString({ message: 'First name must be of type string.' })
+	@Length(1, 32, { message: 'First name must be $constraint1 to $constraint2 characters long.' })
+	firstName: string;
+
+	@NoXss()
+	@IsString({ message: 'Last name must be of type string.' })
+	@Length(1, 32, { message: 'Last name must be $constraint1 to $constraint2 characters long.' })
+	lastName: string;
+}
+
 export type AuthlessRequest<
 	RouteParams = {},
 	ResponseBody = {},
@@ -144,11 +161,7 @@ export declare namespace ExecutionRequest {
 // ----------------------------------
 
 export declare namespace MeRequest {
-	export type Settings = AuthenticatedRequest<
-		{},
-		{},
-		Pick<PublicUser, 'email' | 'firstName' | 'lastName'>
-	>;
+	export type UserUpdate = AuthenticatedRequest<{}, {}, UserUpdatePayload>;
 	export type Password = AuthenticatedRequest<
 		{},
 		{},