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 #2821

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9e09621
:shirt: Fix build
ivov Feb 14, 2022
2fecf12
:zap: Add logging
ivov Feb 14, 2022
c374832
:twisted_rightwards_arrows: Merge parent branch
ivov Feb 14, 2022
4a0c890
:pencil2: Fix typo
ivov Feb 15, 2022
033ca8b
:bug: Fix import
ivov Feb 15, 2022
9e1d587
:fire: Remove unneeded logs
ivov Feb 15, 2022
ea99936
:zap: Adjust log levels
ivov Feb 15, 2022
efef70c
:twisted_rightwards_arrows: Merge parent branch
ivov Feb 15, 2022
528a140
:zap: Simplify log message
ivov Feb 15, 2022
b3bc597
:shirt: Remove lint exception
ivov Feb 15, 2022
b87fdfa
:zap: Simplify log message
ivov Feb 15, 2022
9188738
:rewind: Restore error codes
ivov Feb 15, 2022
4d09435
:zap: Adjust log levels
ivov Feb 15, 2022
d831071
:fire: Remove unneeded meta property
ivov Feb 15, 2022
0d5bef0
:pencil2: Improve wording
ivov Feb 15, 2022
7875dba
:zap: Add double log
ivov Feb 15, 2022
bdd8ca2
:zap: Improve log call readability
ivov Feb 15, 2022
9e8ccb6
:zap: Adjust log levels
ivov Feb 15, 2022
7c0110e
:fire: Remove unneeded optional access operator
ivov Feb 15, 2022
dbdd54a
:zap: Adjust meta objects
ivov Feb 15, 2022
b9e21f7
:zap: Adjust invite email validation log
ivov Feb 15, 2022
cdf8105
:zap: Add missing log
ivov Feb 15, 2022
694a502
:zap: Add missing `userId`
ivov Feb 15, 2022
b6354b4
:zap: Add duplicate log
ivov Feb 15, 2022
8e4eb41
:fire: Remove endpoints references
ivov Feb 15, 2022
3f41dd6
:zap: Improve naming
ivov Feb 15, 2022
70f3a66
:zap: Centralize logger initialization
ivov Feb 15, 2022
efc6d2b
:shirt: Fix build
ivov Feb 15, 2022
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
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,7 @@
/* 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 +29,10 @@ export class NodeMailer implements UserManagementMailerImplementation {
text: mailData.textOnly,
html: mailData.body,
});
Logger.info('Email sent successfully');
Logger.verbose('Email sent to', { recipients: mailData.emailRecipients });
ivov marked this conversation as resolved.
Show resolved Hide resolved
} catch (error) {
Logger.error('Failed to send email', { recipients: mailData.emailRecipients, error });
return {
success: false,
error,
Expand Down
15 changes: 12 additions & 3 deletions packages/cli/src/UserManagement/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,22 @@ 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';

function isAuthenticatedRequest(request: Request): request is AuthenticatedRequest {
return request.user !== undefined;
}

export function addRoutes(this: N8nApp, ignoredEndpoints: string[], restEndpoint: string): void {
this.app.use(cookieParser());

Expand All @@ -36,6 +40,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 @@ -77,10 +82,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')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't !req.user and isAuthenticatedRequest(req) doing the same thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not exactly. The type guard specifies not only that req.user is not undefined but also that req.user is of type User, which allows for role access without the assertion. Without the type guard, in the second half we know that req.user is not undefined but not what type it is.

next();
return;
}
Expand All @@ -97,6 +102,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
13 changes: 13 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,15 @@ export function meNamespace(this: N8nApp): void {
ResponseHelper.send(
async (req: MeRequest.Settings, res: express.Response): Promise<PublicUser> => {
if (!req.body.email) {
Logger.debug('Email not found 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('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 +53,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 All @@ -65,6 +73,8 @@ export function meNamespace(this: N8nApp): void {

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

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

await issueCookie(res, user);

return { success: true };
Expand All @@ -80,6 +90,7 @@ export function meNamespace(this: N8nApp): void {
const { body: personalizationAnswers } = req;

if (!personalizationAnswers) {
Logger.debug('Empty survey in payload', { userId: req.user.id });
throw new ResponseHelper.ResponseError(
'Personalization answers are mandatory',
undefined,
Expand All @@ -92,6 +103,8 @@ export function meNamespace(this: N8nApp): void {
personalizationAnswers,
});

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

return { success: true };
}),
);
Expand Down
9 changes: 9 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,14 +26,17 @@ export function ownerNamespace(this: N8nApp): void {
const { id: userId } = req.user;

if (config.get('userManagement.hasOwner')) {
Logger.debug('Attempted to create owner when owner already exists', { userId });
throw new ResponseHelper.ResponseError('Invalid request', undefined, 400);
}

if (!email || !validator.isEmail(email)) {
Logger.debug('Invalid email in payload', { userId, invalidEmail: email });
throw new ResponseHelper.ResponseError('Invalid email address', undefined, 400);
}

if (!password) {
Logger.debug('Empty password in payload', { userId });
throw new ResponseHelper.ResponseError(
'Password does not comply to security standards',
undefined,
Expand All @@ -41,6 +45,7 @@ export function ownerNamespace(this: N8nApp): void {
}

if (!firstName || !lastName) {
Logger.debug('Missing firstName or lastName in payload', { userId, payload: req.body });
throw new ResponseHelper.ResponseError(
'First and last names are mandatory',
undefined,
Expand Down Expand Up @@ -68,13 +73,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.info('Setting hasOwner updated successfully', { userId: req.user.id });
ivov marked this conversation as resolved.
Show resolved Hide resolved

await issueCookie(res, owner);

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

import { Db, ResponseHelper } from '../..';
import { N8nApp } from '../Interfaces';
Expand All @@ -25,16 +26,19 @@ export function passwordResetNamespace(this: N8nApp): void {
const { email } = req.body;

if (!email) {
Logger.debug('Missing email in payload', { userId: req.user.id, payload: req.body });
throw new ResponseHelper.ResponseError('Email is mandatory', undefined, 400);
}

if (!validator.isEmail(email)) {
Logger.debug('Invalid email in payload', { userId: req.user.id, invalidEmail: email });
throw new ResponseHelper.ResponseError('Invalid email address', undefined, 400);
}

const user = await Db.collections.User!.findOne({ email });

if (!user) {
Logger.debug('User not found for email', { userId: req.user.id, invalidEmail: email });
throw new ResponseHelper.ResponseError('', undefined, 404);
}

Expand All @@ -49,13 +53,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: req.user.id });
ivov marked this conversation as resolved.
Show resolved Hide resolved
}),
);

Expand All @@ -68,14 +74,24 @@ export function passwordResetNamespace(this: N8nApp): void {
const { token: resetPasswordToken, userId: id } = req.query;

if (!resetPasswordToken || !id) {
Logger.debug('Missing password reset token or user ID in query string', {
userId: req.user.id,
queryString: req.query,
});
throw new ResponseHelper.ResponseError('', undefined, 400);
}

const user = await Db.collections.User!.findOne({ resetPasswordToken, id });

if (!user) {
Logger.debug('User not found for user ID and reset password token', {
userId: req.user.id,
queryString: req.query,
});
throw new ResponseHelper.ResponseError('', undefined, 404);
}

Logger.info('Password token resolved successfully', { userId: id });
ivov marked this conversation as resolved.
Show resolved Hide resolved
}),
);

Expand All @@ -88,6 +104,10 @@ export function passwordResetNamespace(this: N8nApp): void {
const { token: resetPasswordToken, userId, password } = req.body;

if (!resetPasswordToken || !userId || !password) {
Logger.debug('User ID or password or reset password token missing from payload', {
userId: req.user.id,
ivov marked this conversation as resolved.
Show resolved Hide resolved
payload: req.body,
});
throw new ResponseHelper.ResponseError('Parameter missing', undefined, 400);
}

Expand All @@ -96,6 +116,10 @@ export function passwordResetNamespace(this: N8nApp): void {
const user = await Db.collections.User!.findOne({ id: userId, resetPasswordToken });

if (!user) {
Logger.debug('User not found for user ID and reset password token', {
userId: req.user.id,
ivov marked this conversation as resolved.
Show resolved Hide resolved
queryString: req.query,
ivov marked this conversation as resolved.
Show resolved Hide resolved
});
throw new ResponseHelper.ResponseError('', undefined, 404);
}

Expand All @@ -104,7 +128,9 @@ export function passwordResetNamespace(this: N8nApp): void {
resetPasswordToken: null,
});

await issueCookie(res, user);
Logger.info('User password updated successfully', { userId });

await issueCookie(res, req.user);
ivov marked this conversation as resolved.
Show resolved Hide resolved
}),
);
}
Loading