Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add logging to user management endpoints v2 #2836

Merged
merged 23 commits into from
Feb 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/cli/src/UserManagement/UserManagementHelper.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
/* eslint-disable @typescript-eslint/no-unused-vars */
/* eslint-disable @typescript-eslint/no-non-null-assertion */
/* eslint-disable import/no-cycle */
import express = require('express');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why require() instead of import?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When exporting a module using export =, TypeScript-specific import module = require("module") must be used to import the module. Source and Express typings export line

import { IsNull, Not } from 'typeorm';
import { Db, GenericHelpers, ResponseHelper } from '..';
import config = require('../../config');
import { MAX_PASSWORD_LENGTH, MIN_PASSWORD_LENGTH, User } from '../databases/entities/User';
import { AuthenticatedRequest } from '../requests';
import { PublicUser } from './Interfaces';

export const isEmailSetUp = Boolean(config.get('userManagement.emails.mode'));
Expand Down Expand Up @@ -53,3 +55,7 @@ export function sanitizeUser(user: User): PublicUser {
} = user;
return sanitizedUser;
}

export function isAuthenticatedRequest(request: express.Request): request is AuthenticatedRequest {
return request.user !== undefined;
}
5 changes: 5 additions & 0 deletions packages/cli/src/UserManagement/email/NodeMailer.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
import { createTransport, Transporter } from 'nodemailer';
import { LoggerProxy as Logger } from 'n8n-workflow';
import config = require('../../../config');
import { MailData, SendEmailResult, UserManagementMailerImplementation } from './Interfaces';

Expand Down Expand Up @@ -27,7 +28,11 @@ export class NodeMailer implements UserManagementMailerImplementation {
text: mailData.textOnly,
html: mailData.body,
});
Logger.verbose(
`Email sent successfully to the following recipients: ${mailData.emailRecipients.toString()}`,
);
} catch (error) {
Logger.error('Failed to send email', { recipients: mailData.emailRecipients, error });
return {
success: false,
error,
Expand Down
12 changes: 9 additions & 3 deletions packages/cli/src/UserManagement/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@ import * as passport from 'passport';
import { Strategy } from 'passport-jwt';
import { NextFunction, Request, Response } from 'express';
import * as jwt from 'jsonwebtoken';
import { LoggerProxy as Logger } from 'n8n-workflow';

import { JwtPayload, N8nApp } from '../Interfaces';
import { authenticationMethods } from './auth';
import config = require('../../../config');
import { User } from '../../databases/entities/User';
import { issueCookie, resolveJwtContent } from '../auth/jwt';
import { meNamespace } from './me';
import { usersNamespace } from './users';
import { passwordResetNamespace } from './passwordReset';
import { AuthenticatedRequest } from '../../requests';
import { ownerNamespace } from './owner';
import { isAuthenticatedRequest } from '../UserManagementHelper';

export function addRoutes(this: N8nApp, ignoredEndpoints: string[], restEndpoint: string): void {
this.app.use(cookieParser());
Expand All @@ -36,6 +37,7 @@ export function addRoutes(this: N8nApp, ignoredEndpoints: string[], restEndpoint
const user = await resolveJwtContent(jwtPayload);
return done(null, user);
} catch (error) {
Logger.debug('Failed to extract user from JWT payload', { jwtPayload });
return done(null, false, { message: 'User not found' });
}
}),
Expand Down Expand Up @@ -81,10 +83,10 @@ export function addRoutes(this: N8nApp, ignoredEndpoints: string[], restEndpoint
return passport.authenticate('jwt', { session: false })(req, res, next);
});

this.app.use((req: Request, res: Response, next: NextFunction) => {
this.app.use((req: Request | AuthenticatedRequest, res: Response, next: NextFunction) => {
// req.user is empty for public routes, so just proceed
// owner can do anything, so proceed as well
if (req.user === undefined || (req.user && (req.user as User).globalRole.name === 'owner')) {
if (!req.user || (isAuthenticatedRequest(req) && req.user.globalRole.name === 'owner')) {
next();
return;
}
Expand All @@ -101,6 +103,10 @@ export function addRoutes(this: N8nApp, ignoredEndpoints: string[], restEndpoint
(req.method === 'POST' &&
new RegExp(`/${restEndpoint}/users/[^/]/reinvite+`, 'gm').test(trimmedUrl))
) {
Logger.verbose('User attempted to access endpoint without authorization', {
endpoint: `${req.method} ${trimmedUrl}`,
userId: isAuthenticatedRequest(req) ? req.user.id : 'unknown',
});
res.status(403).json({ status: 'error', message: 'Unauthorized' });
return;
}
Expand Down
19 changes: 19 additions & 0 deletions packages/cli/src/UserManagement/routes/me.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { genSaltSync, hashSync } from 'bcryptjs';
import express = require('express');
import validator from 'validator';
import { LoggerProxy as Logger } from 'n8n-workflow';

import { Db, ResponseHelper } from '../..';
import { issueCookie } from '../auth/jwt';
Expand Down Expand Up @@ -32,10 +33,18 @@ export function meNamespace(this: N8nApp): void {
ResponseHelper.send(
async (req: MeRequest.Settings, res: express.Response): Promise<PublicUser> => {
if (!req.body.email) {
Logger.debug('Request to update user email failed because of missing email in payload', {
userId: req.user.id,
payload: req.body,
});
throw new ResponseHelper.ResponseError('Email is mandatory', undefined, 400);
}

if (!validator.isEmail(req.body.email)) {
Logger.debug('Request to update user email failed because of invalid email in payload', {
userId: req.user.id,
invalidEmail: req.body.email,
});
throw new ResponseHelper.ResponseError('Invalid email address', undefined, 400);
}

Expand All @@ -47,6 +56,8 @@ export function meNamespace(this: N8nApp): void {

const user = await Db.collections.User!.save(newUser);

Logger.info('User updated successfully', { userId: user.id });

await issueCookie(res, user);

return sanitizeUser(user);
Expand Down Expand Up @@ -80,6 +91,12 @@ export function meNamespace(this: N8nApp): void {
const { body: personalizationAnswers } = req;

if (!personalizationAnswers) {
Logger.debug(
'Request to store user personalization survey failed because of empty payload',
{
userId: req.user.id,
},
);
throw new ResponseHelper.ResponseError(
'Personalization answers are mandatory',
undefined,
Expand All @@ -92,6 +109,8 @@ export function meNamespace(this: N8nApp): void {
personalizationAnswers,
});

Logger.info('User survey updated successfully', { userId: req.user.id });

return { success: true };
}),
);
Expand Down
19 changes: 19 additions & 0 deletions packages/cli/src/UserManagement/routes/owner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { hashSync, genSaltSync } from 'bcryptjs';
import * as express from 'express';
import validator from 'validator';
import { LoggerProxy as Logger } from 'n8n-workflow';

import { Db, ResponseHelper } from '../..';
import config = require('../../../config');
Expand All @@ -25,16 +26,30 @@ export function ownerNamespace(this: N8nApp): void {
const { id: userId } = req.user;

if (config.get('userManagement.hasOwner')) {
Logger.debug(
'Request to claim instance ownership failed because instance owner already exists',
{
userId,
},
);
throw new ResponseHelper.ResponseError('Invalid request', undefined, 400);
}

if (!email || !validator.isEmail(email)) {
Logger.debug('Request to claim instance ownership failed because of invalid email', {
userId,
invalidEmail: email,
});
throw new ResponseHelper.ResponseError('Invalid email address', undefined, 400);
}

const validPassword = validatePassword(password);

if (!firstName || !lastName) {
Logger.debug(
'Request to claim instance ownership failed because of missing first name or last name in payload',
{ userId, payload: req.body },
);
throw new ResponseHelper.ResponseError(
'First and last names are mandatory',
undefined,
Expand Down Expand Up @@ -62,13 +77,17 @@ export function ownerNamespace(this: N8nApp): void {

const owner = await Db.collections.User!.save(newUser);

Logger.info('Owner updated successfully', { userId: req.user.id });

config.set('userManagement.hasOwner', true);

await Db.collections.Settings!.update(
{ key: 'userManagement.hasOwner' },
{ value: JSON.stringify(true) },
);

Logger.debug('Setting hasOwner updated successfully', { userId: req.user.id });

await issueCookie(res, owner);

return sanitizeUser(owner);
Expand Down
64 changes: 57 additions & 7 deletions packages/cli/src/UserManagement/routes/passwordReset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { URL } from 'url';
import { genSaltSync, hashSync } from 'bcryptjs';
import validator from 'validator';
import { IsNull, MoreThanOrEqual, Not } from 'typeorm';
import { LoggerProxy as Logger } from 'n8n-workflow';

import { Db, ResponseHelper } from '../..';
import { N8nApp } from '../Interfaces';
Expand All @@ -25,6 +26,7 @@ export function passwordResetNamespace(this: N8nApp): void {
`/${this.restEndpoint}/forgot-password`,
ResponseHelper.send(async (req: PasswordResetRequest.Email) => {
if (config.get('userManagement.emails.mode') === '') {
Logger.debug('Request to send password reset email failed because emailing was not set up');
throw new ResponseHelper.ResponseError(
'Email sending must be set up in order to request a password reset email',
undefined,
Expand All @@ -35,17 +37,29 @@ export function passwordResetNamespace(this: N8nApp): void {
const { email } = req.body;

if (!email) {
Logger.debug(
'Request to send password reset email failed because of missing email in payload',
{ payload: req.body },
);
throw new ResponseHelper.ResponseError('Email is mandatory', undefined, 400);
}

if (!validator.isEmail(email)) {
Logger.debug(
'Request to send password reset email failed because of invalid email in payload',
{ invalidEmail: email },
);
throw new ResponseHelper.ResponseError('Invalid email address', undefined, 400);
}

// User should just be able to reset password if one is already present
const user = await Db.collections.User!.findOne({ email, password: Not(IsNull()) });

if (!user || !user.password) {
Logger.debug(
'Request to send password reset email failed because no user was found for the provided email',
{ invalidEmail: email },
);
return;
}

Expand All @@ -62,13 +76,15 @@ export function passwordResetNamespace(this: N8nApp): void {
url.searchParams.append('userId', id);
url.searchParams.append('token', resetPasswordToken);

void UserManagementMailer.getInstance().passwordReset({
await UserManagementMailer.getInstance().passwordReset({
email,
firstName,
lastName,
passwordResetUrl: url.toString(),
domain: baseUrl,
});

Logger.info('Sent password reset email successfully', { userId: user.id, email });
}),
);

Expand All @@ -81,6 +97,12 @@ export function passwordResetNamespace(this: N8nApp): void {
const { token: resetPasswordToken, userId: id } = req.query;

if (!resetPasswordToken || !id) {
Logger.debug(
'Request to resolve password token failed because of missing password reset token or user ID in query string',
{
queryString: req.query,
},
);
throw new ResponseHelper.ResponseError('', undefined, 400);
}

Expand All @@ -94,8 +116,17 @@ export function passwordResetNamespace(this: N8nApp): void {
});

if (!user) {
Logger.debug(
'Request to resolve password token failed because no user was found for the provided user ID and reset password token',
{
userId: id,
resetPasswordToken,
},
);
throw new ResponseHelper.ResponseError('', undefined, 404);
}

Logger.info('Reset-password token resolved successfully', { userId: id });
}),
);

Expand All @@ -105,10 +136,20 @@ export function passwordResetNamespace(this: N8nApp): void {
this.app.post(
`/${this.restEndpoint}/change-password`,
ResponseHelper.send(async (req: PasswordResetRequest.NewPassword, res: express.Response) => {
const { token: resetPasswordToken, userId: id, password } = req.body;

if (!resetPasswordToken || !id || !password) {
throw new ResponseHelper.ResponseError('Parameter missing', undefined, 400);
const { token: resetPasswordToken, userId, password } = req.body;

if (!resetPasswordToken || !userId || !password) {
Logger.debug(
'Request to change password failed because of missing user ID or password or reset password token in payload',
{
payload: req.body,
},
);
throw new ResponseHelper.ResponseError(
'Missing user ID or password or reset password token',
undefined,
400,
);
}

const validPassword = validatePassword(password);
Expand All @@ -117,21 +158,30 @@ export function passwordResetNamespace(this: N8nApp): void {
const currentTimestamp = Math.floor(Date.now() / 1000);

const user = await Db.collections.User!.findOne({
id,
id: userId,
resetPasswordToken,
resetPasswordTokenExpiration: MoreThanOrEqual(currentTimestamp),
});

if (!user) {
Logger.debug(
'Request to resolve password token failed because no user was found for the provided user ID and reset password token',
{
userId,
resetPasswordToken,
},
);
throw new ResponseHelper.ResponseError('', undefined, 404);
}

await Db.collections.User!.update(id, {
await Db.collections.User!.update(userId, {
password: hashSync(validPassword, genSaltSync(10)),
resetPasswordToken: null,
resetPasswordTokenExpiration: null,
});

Logger.info('User password updated successfully', { userId });

await issueCookie(res, user);
}),
);
Expand Down
Loading