fix(core): Make password-reset urls valid only for single-use (#7622)
This commit is contained in:
committed by
GitHub
parent
b3470fd64d
commit
60314248f4
@@ -272,15 +272,7 @@ export class Server extends AbstractServer {
|
||||
),
|
||||
Container.get(MeController),
|
||||
new NodeTypesController(config, nodeTypes),
|
||||
new PasswordResetController(
|
||||
logger,
|
||||
externalHooks,
|
||||
internalHooks,
|
||||
mailer,
|
||||
userService,
|
||||
jwtService,
|
||||
mfaService,
|
||||
),
|
||||
Container.get(PasswordResetController),
|
||||
Container.get(TagsController),
|
||||
new TranslationController(config, this.credentialTypes),
|
||||
new UsersController(
|
||||
|
||||
@@ -44,6 +44,11 @@ export function issueJWT(user: User): JwtToken {
|
||||
};
|
||||
}
|
||||
|
||||
export const createPasswordSha = (user: User) =>
|
||||
createHash('sha256')
|
||||
.update(user.password.slice(user.password.length / 2))
|
||||
.digest('hex');
|
||||
|
||||
export async function resolveJwtContent(jwtPayload: JwtPayload): Promise<User> {
|
||||
const user = await Db.collections.User.findOne({
|
||||
where: { id: jwtPayload.id },
|
||||
@@ -52,9 +57,7 @@ export async function resolveJwtContent(jwtPayload: JwtPayload): Promise<User> {
|
||||
|
||||
let passwordHash = null;
|
||||
if (user?.password) {
|
||||
passwordHash = createHash('sha256')
|
||||
.update(user.password.slice(user.password.length / 2))
|
||||
.digest('hex');
|
||||
passwordHash = createPasswordSha(user);
|
||||
}
|
||||
|
||||
// currently only LDAP users during synchronization
|
||||
|
||||
@@ -1,5 +1,9 @@
|
||||
import { Response } from 'express';
|
||||
import { rateLimit } from 'express-rate-limit';
|
||||
import { Service } from 'typedi';
|
||||
import { IsNull, Not } from 'typeorm';
|
||||
import validator from 'validator';
|
||||
|
||||
import { Get, Post, RestController } from '@/decorators';
|
||||
import {
|
||||
BadRequestError,
|
||||
@@ -14,23 +18,17 @@ import {
|
||||
validatePassword,
|
||||
} from '@/UserManagement/UserManagementHelper';
|
||||
import { UserManagementMailer } from '@/UserManagement/email';
|
||||
|
||||
import { Response } from 'express';
|
||||
import { PasswordResetRequest } from '@/requests';
|
||||
import { IExternalHooksClass, IInternalHooksClass } from '@/Interfaces';
|
||||
import { issueCookie } from '@/auth/jwt';
|
||||
import { isLdapEnabled } from '@/Ldap/helpers';
|
||||
import { isSamlCurrentAuthenticationMethod } from '@/sso/ssoHelpers';
|
||||
import { UserService } from '@/services/user.service';
|
||||
import { License } from '@/License';
|
||||
import { Container } from 'typedi';
|
||||
import { RESPONSE_ERROR_MESSAGES, inTest } from '@/constants';
|
||||
import { TokenExpiredError } from 'jsonwebtoken';
|
||||
import type { JwtPayload } from '@/services/jwt.service';
|
||||
import { JwtService } from '@/services/jwt.service';
|
||||
import { MfaService } from '@/Mfa/mfa.service';
|
||||
import { Logger } from '@/Logger';
|
||||
import { rateLimit } from 'express-rate-limit';
|
||||
import { ExternalHooks } from '@/ExternalHooks';
|
||||
import { InternalHooks } from '@/InternalHooks';
|
||||
|
||||
const throttle = rateLimit({
|
||||
windowMs: 5 * 60 * 1000, // 5 minutes
|
||||
@@ -38,16 +36,17 @@ const throttle = rateLimit({
|
||||
message: { message: 'Too many requests' },
|
||||
});
|
||||
|
||||
@Service()
|
||||
@RestController()
|
||||
export class PasswordResetController {
|
||||
constructor(
|
||||
private readonly logger: Logger,
|
||||
private readonly externalHooks: IExternalHooksClass,
|
||||
private readonly internalHooks: IInternalHooksClass,
|
||||
private readonly externalHooks: ExternalHooks,
|
||||
private readonly internalHooks: InternalHooks,
|
||||
private readonly mailer: UserManagementMailer,
|
||||
private readonly userService: UserService,
|
||||
private readonly jwtService: JwtService,
|
||||
private readonly mfaService: MfaService,
|
||||
private readonly license: License,
|
||||
) {}
|
||||
|
||||
/**
|
||||
@@ -92,7 +91,7 @@ export class PasswordResetController {
|
||||
relations: ['authIdentities', 'globalRole'],
|
||||
});
|
||||
|
||||
if (!user?.isOwner && !Container.get(License).isWithinUsersLimit()) {
|
||||
if (!user?.isOwner && !this.license.isWithinUsersLimit()) {
|
||||
this.logger.debug(
|
||||
'Request to send password reset email failed because the user limit was reached',
|
||||
);
|
||||
@@ -123,29 +122,16 @@ export class PasswordResetController {
|
||||
throw new UnprocessableRequestError('forgotPassword.ldapUserPasswordResetUnavailable');
|
||||
}
|
||||
|
||||
const baseUrl = getInstanceBaseUrl();
|
||||
const url = this.userService.generatePasswordResetUrl(user);
|
||||
|
||||
const { id, firstName, lastName } = user;
|
||||
|
||||
const resetPasswordToken = this.jwtService.signData(
|
||||
{ sub: id },
|
||||
{
|
||||
expiresIn: '20m',
|
||||
},
|
||||
);
|
||||
|
||||
const url = this.userService.generatePasswordResetUrl(
|
||||
baseUrl,
|
||||
resetPasswordToken,
|
||||
user.mfaEnabled,
|
||||
);
|
||||
|
||||
try {
|
||||
await this.mailer.passwordReset({
|
||||
email,
|
||||
firstName,
|
||||
lastName,
|
||||
passwordResetUrl: url,
|
||||
domain: baseUrl,
|
||||
domain: getInstanceBaseUrl(),
|
||||
});
|
||||
} catch (error) {
|
||||
void this.internalHooks.onEmailFailed({
|
||||
@@ -173,9 +159,9 @@ export class PasswordResetController {
|
||||
*/
|
||||
@Get('/resolve-password-token')
|
||||
async resolvePasswordToken(req: PasswordResetRequest.Credentials) {
|
||||
const { token: resetPasswordToken } = req.query;
|
||||
const { token } = req.query;
|
||||
|
||||
if (!resetPasswordToken) {
|
||||
if (!token) {
|
||||
this.logger.debug(
|
||||
'Request to resolve password token failed because of missing password reset token',
|
||||
{
|
||||
@@ -185,32 +171,17 @@ export class PasswordResetController {
|
||||
throw new BadRequestError('');
|
||||
}
|
||||
|
||||
const decodedToken = this.verifyResetPasswordToken(resetPasswordToken);
|
||||
const user = await this.userService.resolvePasswordResetToken(token);
|
||||
if (!user) throw new NotFoundError('');
|
||||
|
||||
const user = await this.userService.findOne({
|
||||
where: { id: decodedToken.sub },
|
||||
relations: ['globalRole'],
|
||||
});
|
||||
|
||||
if (!user?.isOwner && !Container.get(License).isWithinUsersLimit()) {
|
||||
if (!user?.isOwner && !this.license.isWithinUsersLimit()) {
|
||||
this.logger.debug(
|
||||
'Request to resolve password token failed because the user limit was reached',
|
||||
{ userId: decodedToken.sub },
|
||||
{ userId: user.id },
|
||||
);
|
||||
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',
|
||||
{
|
||||
userId: decodedToken.sub,
|
||||
resetPasswordToken,
|
||||
},
|
||||
);
|
||||
throw new NotFoundError('');
|
||||
}
|
||||
|
||||
this.logger.info('Reset-password token resolved successfully', { userId: user.id });
|
||||
void this.internalHooks.onUserPasswordResetEmailClick({ user });
|
||||
}
|
||||
@@ -220,9 +191,9 @@ export class PasswordResetController {
|
||||
*/
|
||||
@Post('/change-password')
|
||||
async changePassword(req: PasswordResetRequest.NewPassword, res: Response) {
|
||||
const { token: resetPasswordToken, password, mfaToken } = req.body;
|
||||
const { token, password, mfaToken } = req.body;
|
||||
|
||||
if (!resetPasswordToken || !password) {
|
||||
if (!token || !password) {
|
||||
this.logger.debug(
|
||||
'Request to change password failed because of missing user ID or password or reset password token in payload',
|
||||
{
|
||||
@@ -234,22 +205,8 @@ export class PasswordResetController {
|
||||
|
||||
const validPassword = validatePassword(password);
|
||||
|
||||
const decodedToken = this.verifyResetPasswordToken(resetPasswordToken);
|
||||
|
||||
const user = await this.userService.findOne({
|
||||
where: { id: decodedToken.sub },
|
||||
relations: ['authIdentities', 'globalRole'],
|
||||
});
|
||||
|
||||
if (!user) {
|
||||
this.logger.debug(
|
||||
'Request to resolve password token failed because no user was found for the provided user ID',
|
||||
{
|
||||
resetPasswordToken,
|
||||
},
|
||||
);
|
||||
throw new NotFoundError('');
|
||||
}
|
||||
const user = await this.userService.resolvePasswordResetToken(token);
|
||||
if (!user) throw new NotFoundError('');
|
||||
|
||||
if (user.mfaEnabled) {
|
||||
if (!mfaToken) throw new BadRequestError('If MFA enabled, mfaToken is required.');
|
||||
@@ -285,23 +242,4 @@ 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('');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -411,23 +411,8 @@ export class UsersController {
|
||||
throw new NotFoundError('User not found');
|
||||
}
|
||||
|
||||
const resetPasswordToken = this.jwtService.signData(
|
||||
{ sub: user.id },
|
||||
{
|
||||
expiresIn: '1d',
|
||||
},
|
||||
);
|
||||
|
||||
const baseUrl = getInstanceBaseUrl();
|
||||
|
||||
const link = this.userService.generatePasswordResetUrl(
|
||||
baseUrl,
|
||||
resetPasswordToken,
|
||||
user.mfaEnabled,
|
||||
);
|
||||
return {
|
||||
link,
|
||||
};
|
||||
const link = this.userService.generatePasswordResetUrl(user);
|
||||
return { link };
|
||||
}
|
||||
|
||||
@Authorized(['global', 'owner'])
|
||||
|
||||
@@ -10,8 +10,8 @@ export class JwtService {
|
||||
return jwt.sign(payload, this.userManagementSecret, options);
|
||||
}
|
||||
|
||||
public verifyToken(token: string, options: jwt.VerifyOptions = {}) {
|
||||
return jwt.verify(token, this.userManagementSecret, options) as jwt.JwtPayload;
|
||||
public verifyToken<T = JwtPayload>(token: string, options: jwt.VerifyOptions = {}) {
|
||||
return jwt.verify(token, this.userManagementSecret, options) as T;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -7,10 +7,18 @@ import { UserRepository } from '@/databases/repositories';
|
||||
import { getInstanceBaseUrl } from '@/UserManagement/UserManagementHelper';
|
||||
import type { PublicUser } from '@/Interfaces';
|
||||
import type { PostHogClient } from '@/posthog';
|
||||
import { type JwtPayload, JwtService } from './jwt.service';
|
||||
import { TokenExpiredError } from 'jsonwebtoken';
|
||||
import { Logger } from '@/Logger';
|
||||
import { createPasswordSha } from '@/auth/jwt';
|
||||
|
||||
@Service()
|
||||
export class UserService {
|
||||
constructor(private readonly userRepository: UserRepository) {}
|
||||
constructor(
|
||||
private readonly logger: Logger,
|
||||
private readonly userRepository: UserRepository,
|
||||
private readonly jwtService: JwtService,
|
||||
) {}
|
||||
|
||||
async findOne(options: FindOneOptions<User>) {
|
||||
return this.userRepository.findOne({ relations: ['globalRole'], ...options });
|
||||
@@ -54,15 +62,57 @@ export class UserService {
|
||||
return this.userRepository.update(userId, { settings: { ...settings, ...newSettings } });
|
||||
}
|
||||
|
||||
generatePasswordResetUrl(instanceBaseUrl: string, token: string, mfaEnabled: boolean) {
|
||||
generatePasswordResetToken(user: User, expiresIn = '20m') {
|
||||
return this.jwtService.signData(
|
||||
{ sub: user.id, passwordSha: createPasswordSha(user) },
|
||||
{ expiresIn },
|
||||
);
|
||||
}
|
||||
|
||||
generatePasswordResetUrl(user: User) {
|
||||
const instanceBaseUrl = getInstanceBaseUrl();
|
||||
const url = new URL(`${instanceBaseUrl}/change-password`);
|
||||
|
||||
url.searchParams.append('token', token);
|
||||
url.searchParams.append('mfaEnabled', mfaEnabled.toString());
|
||||
url.searchParams.append('token', this.generatePasswordResetToken(user));
|
||||
url.searchParams.append('mfaEnabled', user.mfaEnabled.toString());
|
||||
|
||||
return url.toString();
|
||||
}
|
||||
|
||||
async resolvePasswordResetToken(token: string): Promise<User | undefined> {
|
||||
let decodedToken: JwtPayload & { passwordSha: string };
|
||||
try {
|
||||
decodedToken = this.jwtService.verifyToken(token);
|
||||
} catch (e) {
|
||||
if (e instanceof TokenExpiredError) {
|
||||
this.logger.debug('Reset password token expired', { token });
|
||||
} else {
|
||||
this.logger.debug('Error verifying token', { token });
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
const user = await this.userRepository.findOne({
|
||||
where: { id: decodedToken.sub },
|
||||
relations: ['authIdentities', 'globalRole'],
|
||||
});
|
||||
|
||||
if (!user) {
|
||||
this.logger.debug(
|
||||
'Request to resolve password token failed because no user was found for the provided user ID',
|
||||
{ userId: decodedToken.sub, token },
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
if (createPasswordSha(user) !== decodedToken.passwordSha) {
|
||||
this.logger.debug('Password updated since this token was generated');
|
||||
return;
|
||||
}
|
||||
|
||||
return user;
|
||||
}
|
||||
|
||||
async toPublic(user: User, options?: { withInviteUrl?: boolean; posthog?: PostHogClient }) {
|
||||
const { password, updatedAt, apiKey, authIdentities, ...rest } = user;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user