Skip to content

Commit

Permalink
Add logging to user management endpoints v2 (#2836)
Browse files Browse the repository at this point in the history
* ⚡ Initialize logger in tests

* ⚡ Add logs to mailer

* ⚡ Add logs to middleware

* ⚡ Add logs to me endpoints

* ⚡ Add logs to owner endpoints

* ⚡ Add logs to pass flow endpoints

* ⚡ Add logs to users endpoints

* 📘 Improve typings

* ⚡ Merge two logs into one

* ⚡ Adjust log type

* ⚡ Add password reset email log

* ✏️ Reword log message

* ⚡ Adjust log meta object

* ⚡ Add total to log

* ✏️ Add detail to log message

* ✏️ Reword log message

* ✏️ Reword log message

* 🐛 Make total users to set up accurate

* ✏️ Reword `Logger.debug()` messages

* ✏️ Phrasing change for consistency
  • Loading branch information
ivov authored Feb 22, 2022
1 parent 5ea87a3 commit 533759d
Show file tree
Hide file tree
Showing 15 changed files with 433 additions and 137 deletions.
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');
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

0 comments on commit 533759d

Please sign in to comment.