Skip to content

Commit

Permalink
fix(core): Make password-reset urls valid only for single-use (n8n-io…
Browse files Browse the repository at this point in the history 
…#7622)
  • Loading branch information
@netroy
netroy authored Nov 7, 2023
1 parent b3470fd commit 6031424
Show file tree
Hide file tree
Showing 13 changed files with 206 additions and 168 deletions.
10 changes: 1 addition & 9 deletions packages/cli/src/Server.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
9 changes: 6 additions & 3 deletions packages/cli/src/auth/jwt.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand All @@ -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
Expand Down
110 changes: 24 additions & 86 deletions packages/cli/src/controllers/passwordReset.controller.ts
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -14,40 +18,35 @@ 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
limit: 5, // Limit each IP to 5 requests per `window` (here, per 5 minutes).
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,
) {}

/**
Expand Down Expand Up @@ -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',
);
Expand Down Expand Up @@ -123,29 +122,16 @@ export class PasswordResetController {
throw new UnprocessableRequestError('forgotPassword.ldapUserPasswordResetUnavailable');
}

const baseUrl = getInstanceBaseUrl();
const { id, firstName, lastName } = user;

const resetPasswordToken = this.jwtService.signData(
{ sub: id },
{
expiresIn: '20m',
},
);

const url = this.userService.generatePasswordResetUrl(
baseUrl,
resetPasswordToken,
user.mfaEnabled,
);
const url = this.userService.generatePasswordResetUrl(user);

const { id, firstName, lastName } = user;
try {
await this.mailer.passwordReset({
email,
firstName,
lastName,
passwordResetUrl: url,
domain: baseUrl,
domain: getInstanceBaseUrl(),
});
} catch (error) {
void this.internalHooks.onEmailFailed({
Expand Down Expand Up @@ -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',
{
Expand All @@ -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 });
}
Expand All @@ -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',
{
Expand All @@ -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.');
Expand Down Expand Up @@ -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('');
}
}
}
19 changes: 2 additions & 17 deletions packages/cli/src/controllers/users.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/services/jwt.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
58 changes: 54 additions & 4 deletions packages/cli/src/services/user.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down Expand Up @@ -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;

Expand Down
Loading

0 comments on commit 6031424

Please sign in to comment.