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 19 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 @@ -46,3 +48,7 @@ export function sanitizeUser(user: User): PublicUser {
const { password, resetPasswordToken, createdAt, updatedAt, ...sanitizedUser } = user;
return sanitizedUser;
}

export function isAuthenticatedRequest(request: express.Request): request is AuthenticatedRequest {
return request.user !== undefined;
}
3 changes: 3 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,9 @@ export class NodeMailer implements UserManagementMailerImplementation {
text: mailData.textOnly,
html: mailData.body,
});
Logger.verbose('Email sent successfully 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
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 @@ -78,10 +80,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 @@ -98,6 +100,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
14 changes: 14 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('Email not found in payload', {
ivov marked this conversation as resolved.
Show resolved Hide resolved
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 +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,7 @@ export function meNamespace(this: N8nApp): void {
const { body: personalizationAnswers } = req;

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

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

return { success: true };
}),
);
Expand Down
8 changes: 8 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,19 @@ 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 });
ivov marked this conversation as resolved.
Show resolved Hide resolved
throw new ResponseHelper.ResponseError('Invalid email address', undefined, 400);
}

const validPassword = validatePassword(password);

if (!firstName || !lastName) {
Logger.debug('Missing firstName or lastName in payload', { userId, payload: req.body });
ivov marked this conversation as resolved.
Show resolved Hide resolved
throw new ResponseHelper.ResponseError(
'First and last names are mandatory',
undefined,
Expand Down Expand Up @@ -62,13 +66,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
26 changes: 25 additions & 1 deletion 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, Not } from 'typeorm';
import { LoggerProxy as Logger } from 'n8n-workflow';

import { Db, ResponseHelper } from '../..';
import { N8nApp } from '../Interfaces';
Expand Down Expand Up @@ -35,17 +36,20 @@ export function passwordResetNamespace(this: N8nApp): void {
const { email } = req.body;

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

if (!validator.isEmail(email)) {
Logger.debug('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) {
Logger.debug('User not found for email', { invalidEmail: email });
return;
}

Expand All @@ -60,13 +64,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 @@ -79,14 +85,23 @@ 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', {
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: id,
resetPasswordToken,
});
throw new ResponseHelper.ResponseError('', undefined, 404);
}

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

Expand All @@ -99,6 +114,9 @@ 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', {
payload: req.body,
});
throw new ResponseHelper.ResponseError('Parameter missing', undefined, 400);
}

Expand All @@ -107,6 +125,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,
resetPasswordToken,
});
throw new ResponseHelper.ResponseError('', undefined, 404);
}

Expand All @@ -115,6 +137,8 @@ export function passwordResetNamespace(this: N8nApp): void {
resetPasswordToken: null,
});

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

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