fix(core): Use JWT as reset password token (#6714)

* use jwt to reset password

* increase expiration time to 1d

* drop user id query string

* refactor

* use service instead of package in tests

* sqlite migration

* postgres migration

* mysql migration

* remove unused properties

* remove userId from FE

* fix test for users.api

* move migration to the common folder

* move type assertion to the jwt.service

* Add jwt secret as a readonly property

* use signData instead of sign in user.controller

* remove base class

* remove base class

* add tests

packages/cli/src/Server.ts

@@ -169,6 +169,7 @@ import { SourceControlService } from '@/environments/sourceControl/sourceControl
 import { SourceControlController } from '@/environments/sourceControl/sourceControl.controller.ee';
 import { ExecutionRepository } from '@db/repositories';
 import type { ExecutionEntity } from '@db/entities/ExecutionEntity';
+import { JwtService } from './services/jwt.service';
 
 const exec = promisify(callbackExec);
 
@@ -466,6 +467,7 @@ export class Server extends AbstractServer {
 		const internalHooks = Container.get(InternalHooks);
 		const mailer = Container.get(UserManagementMailer);
 		const postHog = this.postHog;
+		const jwtService = Container.get(JwtService);
 
 		const controllers: object[] = [
 			new EventBusController(),
@@ -480,6 +482,7 @@ export class Server extends AbstractServer {
 				mailer,
 				repositories,
 				logger,
+				jwtService,
 			}),
 			new TagsController({ config, repositories, externalHooks }),
 			new TranslationController(config, this.credentialTypes),
@@ -492,6 +495,7 @@ export class Server extends AbstractServer {
 				activeWorkflowRunner,
 				logger,
 				postHog,
+				jwtService,
 			}),
 
 			Container.get(SamlController),

packages/cli/src/UserManagement/UserManagementHelper.ts

@@ -111,15 +111,7 @@ export function validatePassword(password?: string): string {
  * Remove sensitive properties from the user to return to the client.
  */
 export function sanitizeUser(user: User, withoutKeys?: string[]): PublicUser {
-	const {
-		password,
-		resetPasswordToken,
-		resetPasswordTokenExpiration,
-		updatedAt,
-		apiKey,
-		authIdentities,
-		...rest
-	} = user;
+	const { password, updatedAt, apiKey, authIdentities, ...rest } = user;
 	if (withoutKeys) {
 		withoutKeys.forEach((key) => {
 			// @ts-ignore

packages/cli/src/auth/jwt.ts

@@ -38,7 +38,6 @@ export function issueJWT(user: User): JwtToken {
 
 	const signedToken = jwt.sign(payload, config.getEnv('userManagement.jwtSecret'), {
 		expiresIn: expiresIn / 1000 /* in seconds */,
-		algorithm: 'HS256',
 	});
 
 	return {

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

@@ -1,4 +1,4 @@
-import { IsNull, MoreThanOrEqual, Not } from 'typeorm';
+import { IsNull, Not } from 'typeorm';
 import validator from 'validator';
 import { Get, Post, RestController } from '@/decorators';
 import {
@@ -28,6 +28,8 @@ import { UserService } from '@/user/user.service';
 import { License } from '@/License';
 import { Container } from 'typedi';
 import { RESPONSE_ERROR_MESSAGES } from '@/constants';
+import { TokenExpiredError } from 'jsonwebtoken';
+import type { JwtService, JwtPayload } from '@/services/jwt.service';
 
 @RestController()
 export class PasswordResetController {
@@ -43,27 +45,32 @@ export class PasswordResetController {
 
 	private readonly userRepository: UserRepository;
 
+	private readonly jwtService: JwtService;
+
 	constructor({
 		config,
 		logger,
 		externalHooks,
 		internalHooks,
 		mailer,
 		repositories,
+		jwtService,
 	}: {
 		config: Config;
 		logger: ILogger;
 		externalHooks: IExternalHooksClass;
 		internalHooks: IInternalHooksClass;
 		mailer: UserManagementMailer;
 		repositories: Pick<IDatabaseCollections, 'User'>;
+		jwtService: JwtService;
 	}) {
 		this.config = config;
 		this.logger = logger;
 		this.externalHooks = externalHooks;
 		this.internalHooks = internalHooks;
 		this.mailer = mailer;
 		this.userRepository = repositories.User;
+		this.jwtService = jwtService;
 	}
 
 	/**
@@ -139,7 +146,15 @@ export class PasswordResetController {
 
 		const baseUrl = getInstanceBaseUrl();
 		const { id, firstName, lastName } = user;
-		const url = await UserService.generatePasswordResetUrl(user);
+
+		const resetPasswordToken = this.jwtService.signData(
+			{ sub: id },
+			{
+				expiresIn: '1d',
+			},
+		);
+
+		const url = await UserService.generatePasswordResetUrl(baseUrl, resetPasswordToken);
 
 		try {
 			await this.mailer.passwordReset({
@@ -175,59 +190,58 @@ export class PasswordResetController {
 	 */
 	@Get('/resolve-password-token')
 	async resolvePasswordToken(req: PasswordResetRequest.Credentials) {
-		const { token: resetPasswordToken, userId: id } = req.query;
+		const { token: resetPasswordToken } = req.query;
 
-		if (!resetPasswordToken || !id) {
+		if (!resetPasswordToken) {
 			this.logger.debug(
-				'Request to resolve password token failed because of missing password reset token or user ID in query string',
+				'Request to resolve password token failed because of missing password reset token',
 				{
 					queryString: req.query,
 				},
 			);
 			throw new BadRequestError('');
 		}
 
-		// Timestamp is saved in seconds
-		const currentTimestamp = Math.floor(Date.now() / 1000);
+		const decodedToken = this.verifyResetPasswordToken(resetPasswordToken);
 
 		const user = await this.userRepository.findOne({
 			where: {
-				id,
-				resetPasswordToken,
-				resetPasswordTokenExpiration: MoreThanOrEqual(currentTimestamp),
+				id: decodedToken.sub,
 			},
 			relations: ['globalRole'],
 		});
+
 		if (!user?.isOwner && !Container.get(License).isWithinUsersLimit()) {
 			this.logger.debug(
 				'Request to resolve password token failed because the user limit was reached',
-				{ userId: id },
+				{ userId: decodedToken.sub },
 			);
 			throw new UnauthorizedError(RESPONSE_ERROR_MESSAGES.USERS_QUOTA_REACHED);
 		}
+
 		if (!user) {
 			this.logger.debug(
-				'Request to resolve password token failed because no user was found for the provided user ID and reset password token',
+				'Request to resolve password token failed because no user was found for the provided user ID',
 				{
-					userId: id,
+					userId: decodedToken.sub,
 					resetPasswordToken,
 				},
 			);
 			throw new NotFoundError('');
 		}
 
-		this.logger.info('Reset-password token resolved successfully', { userId: id });
+		this.logger.info('Reset-password token resolved successfully', { userId: user.id });
 		void this.internalHooks.onUserPasswordResetEmailClick({ user });
 	}
 
 	/**
-	 * Verify password reset token and user ID and update password.
+	 * Verify password reset token and update password.
 	 */
 	@Post('/change-password')
 	async changePassword(req: PasswordResetRequest.NewPassword, res: Response) {
-		const { token: resetPasswordToken, userId, password } = req.body;
+		const { token: resetPasswordToken, password } = req.body;
 
-		if (!resetPasswordToken || !userId || !password) {
+		if (!resetPasswordToken || !password) {
 			this.logger.debug(
 				'Request to change password failed because of missing user ID or password or reset password token in payload',
 				{
@@ -239,23 +253,17 @@ export class PasswordResetController {
 
 		const validPassword = validatePassword(password);
 
-		// Timestamp is saved in seconds
-		const currentTimestamp = Math.floor(Date.now() / 1000);
+		const decodedToken = this.verifyResetPasswordToken(resetPasswordToken);
 
 		const user = await this.userRepository.findOne({
-			where: {
-				id: userId,
-				resetPasswordToken,
-				resetPasswordTokenExpiration: MoreThanOrEqual(currentTimestamp),
-			},
+			where: { id: decodedToken.sub },
 			relations: ['authIdentities'],
 		});
 
 		if (!user) {
 			this.logger.debug(
-				'Request to resolve password token failed because no user was found for the provided user ID and reset password token',
+				'Request to resolve password token failed because no user was found for the provided user ID',
 				{
-					userId,
 					resetPasswordToken,
 				},
 			);
@@ -264,13 +272,11 @@ export class PasswordResetController {
 
 		const passwordHash = await hashPassword(validPassword);
 
-		await this.userRepository.update(userId, {
+		await this.userRepository.update(user.id, {
 			password: passwordHash,
-			resetPasswordToken: null,
-			resetPasswordTokenExpiration: null,
 		});
 
-		this.logger.info('User password updated successfully', { userId });
+		this.logger.info('User password updated successfully', { userId: user.id });
 
 		await issueCookie(res, user);
 
@@ -290,4 +296,23 @@ export class PasswordResetController {
 
 		await this.externalHooks.run('user.password.update', [user.email, passwordHash]);
 	}
+
+	private verifyResetPasswordToken(resetPasswordToken: string) {
+		let decodedToken: JwtPayload;
+		try {
+			decodedToken = this.jwtService.verifyToken(resetPasswordToken);
+			return decodedToken;
+		} catch (e) {
+			if (e instanceof TokenExpiredError) {
+				this.logger.debug('Reset password token expired', {
+					resetPasswordToken,
+				});
+				throw new NotFoundError('');
+			}
+			this.logger.debug('Error verifying token', {
+				resetPasswordToken,
+			});
+			throw new BadRequestError('');
+		}
+	}
 }

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

@@ -49,6 +49,7 @@ import { plainToInstance } from 'class-transformer';
 import { License } from '@/License';
 import { Container } from 'typedi';
 import { RESPONSE_ERROR_MESSAGES } from '@/constants';
+import type { JwtService } from '@/services/jwt.service';
 
 @Authorized(['global', 'owner'])
 @RestController('/users')
@@ -73,6 +74,8 @@ export class UsersController {
 
 	private mailer: UserManagementMailer;
 
+	private jwtService: JwtService;
+
 	private postHog?: PostHogClient;
 
 	constructor({
@@ -83,6 +86,7 @@ export class UsersController {
 		repositories,
 		activeWorkflowRunner,
 		mailer,
+		jwtService,
 		postHog,
 	}: {
 		config: Config;
@@ -95,6 +99,7 @@ export class UsersController {
 		>;
 		activeWorkflowRunner: ActiveWorkflowRunner;
 		mailer: UserManagementMailer;
+		jwtService: JwtService;
 		postHog?: PostHogClient;
 	}) {
 		this.config = config;
@@ -107,6 +112,7 @@ export class UsersController {
 		this.sharedWorkflowRepository = repositories.SharedWorkflow;
 		this.activeWorkflowRunner = activeWorkflowRunner;
 		this.mailer = mailer;
+		this.jwtService = jwtService;
 		this.postHog = postHog;
 	}
 
@@ -382,7 +388,17 @@ export class UsersController {
 		if (!user) {
 			throw new NotFoundError('User not found');
 		}
-		const link = await UserService.generatePasswordResetUrl(user);
+
+		const resetPasswordToken = this.jwtService.signData(
+			{ sub: user.id },
+			{
+				expiresIn: '1d',
+			},
+		);
+
+		const baseUrl = getInstanceBaseUrl();
+
+		const link = await UserService.generatePasswordResetUrl(baseUrl, resetPasswordToken);
 		return {
 			link,
 		};

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

@@ -55,13 +55,6 @@ export class User extends AbstractEntity implements IUser {
 	@IsString({ message: 'Password must be of type string.' })
 	password: string;
 
-	@Column({ type: String, nullable: true })
-	resetPasswordToken?: string | null;
-
-	// Expiration timestamp saved in seconds
-	@Column({ type: Number, nullable: true })
-	resetPasswordTokenExpiration?: number | null;
-
 	@Column({
 		type: jsonColumnType,
 		nullable: true,

packages/cli/src/databases/migrations/common/1690000000030-RemoveResetPasswordColumns.ts

@@ -0,0 +1,29 @@
+import type { MigrationContext, ReversibleMigration } from '@db/types';
+import { TableColumn } from 'typeorm';
+
+export class RemoveResetPasswordColumns1690000000030 implements ReversibleMigration {
+	async up({ queryRunner, tablePrefix }: MigrationContext) {
+		await queryRunner.dropColumn(`${tablePrefix}user`, 'resetPasswordToken');
+		await queryRunner.dropColumn(`${tablePrefix}user`, 'resetPasswordTokenExpiration');
+	}
+
+	async down({ queryRunner, tablePrefix }: MigrationContext) {
+		await queryRunner.addColumn(
+			`${tablePrefix}user`,
+			new TableColumn({
+				name: 'resetPasswordToken',
+				type: 'varchar',
+				isNullable: true,
+			}),
+		);
+
+		await queryRunner.addColumn(
+			`${tablePrefix}user`,
+			new TableColumn({
+				name: 'resetPasswordTokenExpiration',
+				type: 'int',
+				isNullable: true,
+			}),
+		);
+	}
+}

packages/cli/src/databases/migrations/mysqldb/index.ts

@@ -42,6 +42,7 @@ import { MigrateIntegerKeysToString1690000000001 } from './1690000000001-Migrate
 import { SeparateExecutionData1690000000030 } from './1690000000030-SeparateExecutionData';
 import { FixExecutionDataType1690000000031 } from './1690000000031-FixExecutionDataType';
 import { RemoveSkipOwnerSetup1681134145997 } from './1681134145997-RemoveSkipOwnerSetup';
+import { RemoveResetPasswordColumns1690000000030 } from '../common/1690000000030-RemoveResetPasswordColumns';
 
 export const mysqlMigrations: Migration[] = [
 	InitialMigration1588157391238,
@@ -87,4 +88,5 @@ export const mysqlMigrations: Migration[] = [
 	SeparateExecutionData1690000000030,
 	FixExecutionDataType1690000000031,
 	RemoveSkipOwnerSetup1681134145997,
+	RemoveResetPasswordColumns1690000000030,
 ];

packages/cli/src/databases/migrations/postgresdb/index.ts

@@ -39,6 +39,7 @@ import { AddUserActivatedProperty1681134145996 } from './1681134145996-AddUserAc
 import { MigrateIntegerKeysToString1690000000000 } from './1690000000000-MigrateIntegerKeysToString';
 import { SeparateExecutionData1690000000020 } from './1690000000020-SeparateExecutionData';
 import { RemoveSkipOwnerSetup1681134145997 } from './1681134145997-RemoveSkipOwnerSetup';
+import { RemoveResetPasswordColumns1690000000030 } from '../common/1690000000030-RemoveResetPasswordColumns';
 
 export const postgresMigrations: Migration[] = [
 	InitialMigration1587669153312,
@@ -81,4 +82,5 @@ export const postgresMigrations: Migration[] = [
 	MigrateIntegerKeysToString1690000000000,
 	SeparateExecutionData1690000000020,
 	RemoveSkipOwnerSetup1681134145997,
+	RemoveResetPasswordColumns1690000000030,
 ];