feat: Add user management invite links without SMTP set up (#5084)

* feat: update n8n-users-list to no longer use preset list of actions

* feat: prepared users settings for invite links feature

* refactor: Return invite link URLs when inviting users (#5079)

* refactor: Return invite link URLs when inviting users

* test: Refactor and add tests to mailer

* feat: Add FE inviteAcceptUrl integration (#5085)

* feat: update n8n-users-list to no longer use preset list of actions

* feat: prepared users settings for invite links feature

* feat: add integration with new inviteAcceptUrl changes

* feat: Add inviteAcceptUrl to user list for pending users

Co-authored-by: Alex Grozav <alex@grozav.com>

* fix conflicts

* fix lint issue

* test: Make sure inviteAcceptUrl is defined

* feat: update smtp setup suggestion

* feat: add invite link summary when inviting multiple users

* refactor: Add telemetry flag for when email is sent

* fix: add email_sent correctly to telemetry event

* feat: move SMTP info-tip to invite modal

Co-authored-by: Omar Ajoue <krynble@gmail.com>
This commit is contained in:
Alex Grozav
2023-01-05 17:10:08 +02:00
committed by GitHub
parent 11a46a4cbc
commit 2327563c44
22 changed files with 419 additions and 247 deletions

View File

@@ -436,6 +436,7 @@ export class InternalHooksClass implements IInternalHooksClass {
user: User;
target_user_id: string[];
public_api: boolean;
email_sent: boolean;
}): Promise<void> {
void Promise.all([
eventBus.sendAuditEvent({
@@ -449,6 +450,7 @@ export class InternalHooksClass implements IInternalHooksClass {
user_id: userInviteData.user.id,
target_user_id: userInviteData.target_user_id,
public_api: userInviteData.public_api,
email_sent: userInviteData.email_sent,
}),
]);
}

View File

@@ -23,6 +23,7 @@ export interface PublicUser {
passwordResetToken?: string;
createdAt: Date;
isPending: boolean;
inviteAcceptUrl?: string;
}
export interface N8nApp {

View File

@@ -99,6 +99,10 @@ export function getInstanceBaseUrl(): string {
return n8nBaseUrl.endsWith('/') ? n8nBaseUrl.slice(0, n8nBaseUrl.length - 1) : n8nBaseUrl;
}
export function generateUserInviteUrl(inviterId: string, inviteeId: string): string {
return `${getInstanceBaseUrl()}/signup?inviterId=${inviterId}&inviteeId=${inviteeId}`;
}
// TODO: Enforce at model level
export function validatePassword(password?: string): string {
if (!password) {
@@ -156,6 +160,13 @@ export function sanitizeUser(user: User, withoutKeys?: string[]): PublicUser {
return sanitizedUser;
}
export function addInviteLinktoUser(user: PublicUser, inviterId: string): PublicUser {
if (user.isPending) {
user.inviteAcceptUrl = generateUserInviteUrl(inviterId, user.id);
}
return user;
}
export async function getUserById(userId: string): Promise<User> {
const user = await Db.collections.User.findOneOrFail(userId, {
relations: ['globalRole'],

View File

@@ -1,4 +1,5 @@
export interface UserManagementMailerImplementation {
init: () => Promise<void>;
sendMail: (mailData: MailData) => Promise<SendEmailResult>;
verifyConnection: () => Promise<void>;
}
@@ -20,8 +21,7 @@ export type PasswordResetData = {
};
export type SendEmailResult = {
success: boolean;
error?: Error;
emailSent: boolean;
};
export type MailData = {

View File

@@ -5,9 +5,9 @@ import config from '@/config';
import { MailData, SendEmailResult, UserManagementMailerImplementation } from './Interfaces';
export class NodeMailer implements UserManagementMailerImplementation {
private transport: Transporter;
private transport?: Transporter;
constructor() {
async init(): Promise<void> {
this.transport = createTransport({
host: config.getEnv('userManagement.emails.smtp.host'),
port: config.getEnv('userManagement.emails.smtp.port'),
@@ -20,12 +20,15 @@ export class NodeMailer implements UserManagementMailerImplementation {
}
async verifyConnection(): Promise<void> {
if (!this.transport) {
await this.init();
}
const host = config.getEnv('userManagement.emails.smtp.host');
const user = config.getEnv('userManagement.emails.smtp.auth.user');
const pass = config.getEnv('userManagement.emails.smtp.auth.pass');
try {
await this.transport.verify();
await this.transport?.verify();
} catch (error) {
const message: string[] = [];
if (!host) message.push('SMTP host not defined (N8N_SMTP_HOST).');
@@ -36,6 +39,9 @@ export class NodeMailer implements UserManagementMailerImplementation {
}
async sendMail(mailData: MailData): Promise<SendEmailResult> {
if (!this.transport) {
await this.init();
}
let sender = config.getEnv('userManagement.emails.smtp.sender');
const user = config.getEnv('userManagement.emails.smtp.auth.user');
@@ -44,7 +50,7 @@ export class NodeMailer implements UserManagementMailerImplementation {
}
try {
await this.transport.sendMail({
await this.transport?.sendMail({
from: sender,
to: mailData.emailRecipients,
subject: mailData.subject,
@@ -57,12 +63,9 @@ export class NodeMailer implements UserManagementMailerImplementation {
} catch (error) {
ErrorReporter.error(error);
Logger.error('Failed to send email', { recipients: mailData.emailRecipients, error });
return {
success: false,
error,
};
throw error;
}
return { success: true };
return { emailSent: true };
}
}

View File

@@ -44,57 +44,52 @@ export class UserManagementMailer {
constructor() {
// Other implementations can be used in the future.
if (config.getEnv('userManagement.emails.mode') === 'smtp') {
if (
config.getEnv('userManagement.emails.mode') === 'smtp' &&
config.getEnv('userManagement.emails.smtp.host') !== ''
) {
this.mailer = new NodeMailer();
}
}
async verifyConnection(): Promise<void> {
if (!this.mailer) return Promise.reject();
if (!this.mailer) throw new Error('No mailer configured.');
return this.mailer.verifyConnection();
}
async invite(inviteEmailData: InviteEmailData): Promise<SendEmailResult> {
if (!this.mailer) return Promise.reject();
const template = await getTemplate('invite');
const result = await this.mailer.sendMail({
const result = await this.mailer?.sendMail({
emailRecipients: inviteEmailData.email,
subject: 'You have been invited to n8n',
body: template(inviteEmailData),
});
// If mailer does not exist it means mail has been disabled.
return result ?? { success: true };
// No error, just say no email was sent.
return result ?? { emailSent: false };
}
async passwordReset(passwordResetData: PasswordResetData): Promise<SendEmailResult> {
if (!this.mailer) return Promise.reject();
const template = await getTemplate('passwordReset');
const result = await this.mailer.sendMail({
const result = await this.mailer?.sendMail({
emailRecipients: passwordResetData.email,
subject: 'n8n password reset',
body: template(passwordResetData),
});
// If mailer does not exist it means mail has been disabled.
return result ?? { success: true };
// No error, just say no email was sent.
return result ?? { emailSent: false };
}
}
let mailerInstance: UserManagementMailer | undefined;
export async function getInstance(): Promise<UserManagementMailer> {
export function getInstance(): UserManagementMailer {
if (mailerInstance === undefined) {
mailerInstance = new UserManagementMailer();
try {
await mailerInstance.verifyConnection();
} catch (error) {
mailerInstance = undefined;
throw error;
}
}
return mailerInstance;
}

View File

@@ -76,7 +76,7 @@ export function passwordResetNamespace(this: N8nApp): void {
url.searchParams.append('token', resetPasswordToken);
try {
const mailer = await UserManagementMailer.getInstance();
const mailer = UserManagementMailer.getInstance();
await mailer.passwordReset({
email,
firstName,

View File

@@ -14,6 +14,8 @@ import { UserRequest } from '@/requests';
import * as UserManagementMailer from '../email/UserManagementMailer';
import { N8nApp, PublicUser } from '../Interfaces';
import {
addInviteLinktoUser,
generateUserInviteUrl,
getInstanceBaseUrl,
hashPassword,
isEmailSetUp,
@@ -34,25 +36,7 @@ export function usersNamespace(this: N8nApp): void {
this.app.post(
`/${this.restEndpoint}/users`,
ResponseHelper.send(async (req: UserRequest.Invite) => {
if (config.getEnv('userManagement.emails.mode') === '') {
Logger.debug(
'Request to send email invite(s) to user(s) failed because emailing was not set up',
);
throw new ResponseHelper.InternalServerError(
'Email sending must be set up in order to request a password reset email',
);
}
let mailer: UserManagementMailer.UserManagementMailer | undefined;
try {
mailer = await UserManagementMailer.getInstance();
} catch (error) {
if (error instanceof Error) {
throw new ResponseHelper.InternalServerError(
`There is a problem with your SMTP setup! ${error.message}`,
);
}
}
const mailer = UserManagementMailer.getInstance();
// TODO: this should be checked in the middleware rather than here
if (isUserManagementDisabled()) {
@@ -143,19 +127,13 @@ export function usersNamespace(this: N8nApp): void {
}),
);
});
void InternalHooksManager.getInstance().onUserInvite({
user: req.user,
target_user_id: Object.values(createUsers) as string[],
public_api: false,
});
} catch (error) {
ErrorReporter.error(error);
Logger.error('Failed to create user shells', { userShells: createUsers });
throw new ResponseHelper.InternalServerError('An error occurred during user creation');
}
Logger.info('Created user shell(s) successfully', { userId: req.user.id });
Logger.debug('Created user shell(s) successfully', { userId: req.user.id });
Logger.verbose(total > 1 ? `${total} user shells created` : '1 user shell created', {
userShells: createUsers,
});
@@ -168,39 +146,61 @@ export function usersNamespace(this: N8nApp): void {
const emailingResults = await Promise.all(
usersPendingSetup.map(async ([email, id]) => {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
const inviteAcceptUrl = `${baseUrl}/signup?inviterId=${req.user.id}&inviteeId=${id}`;
const result = await mailer?.invite({
email,
inviteAcceptUrl,
domain: baseUrl,
});
const resp: { user: { id: string | null; email: string }; error?: string } = {
if (!id) {
// This should never happen since those are removed from the list before reaching this point
throw new ResponseHelper.InternalServerError(
'User ID is missing for user with email address',
);
}
const inviteAcceptUrl = generateUserInviteUrl(req.user.id, id);
const resp: {
user: { id: string | null; email: string; inviteAcceptUrl: string; emailSent: boolean };
error?: string;
} = {
user: {
id,
email,
inviteAcceptUrl,
emailSent: false,
},
};
if (result?.success) {
void InternalHooksManager.getInstance().onUserTransactionalEmail({
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
user_id: id!,
message_type: 'New user invite',
public_api: false,
});
} else {
void InternalHooksManager.getInstance().onEmailFailed({
user: req.user,
message_type: 'New user invite',
public_api: false,
});
Logger.error('Failed to send email', {
userId: req.user.id,
try {
const result = await mailer.invite({
email,
inviteAcceptUrl,
domain: baseUrl,
email,
});
resp.error = 'Email could not be sent';
if (result.emailSent) {
resp.user.emailSent = true;
void InternalHooksManager.getInstance().onUserTransactionalEmail({
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
user_id: id,
message_type: 'New user invite',
public_api: false,
});
}
void InternalHooksManager.getInstance().onUserInvite({
user: req.user,
target_user_id: Object.values(createUsers) as string[],
public_api: false,
email_sent: result.emailSent,
});
} catch (error) {
if (error instanceof Error) {
void InternalHooksManager.getInstance().onEmailFailed({
user: req.user,
message_type: 'New user invite',
public_api: false,
});
Logger.error('Failed to send email', {
userId: req.user.id,
inviteAcceptUrl,
domain: baseUrl,
email,
});
resp.error = error.message;
}
}
return resp;
}),
@@ -361,10 +361,13 @@ export function usersNamespace(this: N8nApp): void {
this.app.get(
`/${this.restEndpoint}/users`,
ResponseHelper.send(async () => {
ResponseHelper.send(async (req: UserRequest.List) => {
const users = await Db.collections.User.find({ relations: ['globalRole'] });
return users.map((user): PublicUser => sanitizeUser(user, ['personalizationAnswers']));
return users.map(
(user): PublicUser =>
addInviteLinktoUser(sanitizeUser(user, ['personalizationAnswers']), req.user.id),
);
}),
);
@@ -563,22 +566,27 @@ export function usersNamespace(this: N8nApp): void {
const baseUrl = getInstanceBaseUrl();
const inviteAcceptUrl = `${baseUrl}/signup?inviterId=${req.user.id}&inviteeId=${reinvitee.id}`;
let mailer: UserManagementMailer.UserManagementMailer | undefined;
const mailer = UserManagementMailer.getInstance();
try {
mailer = await UserManagementMailer.getInstance();
} catch (error) {
if (error instanceof Error) {
throw new ResponseHelper.InternalServerError(error.message);
const result = await mailer.invite({
email: reinvitee.email,
inviteAcceptUrl,
domain: baseUrl,
});
if (result.emailSent) {
void InternalHooksManager.getInstance().onUserReinvite({
user: req.user,
target_user_id: reinvitee.id,
public_api: false,
});
void InternalHooksManager.getInstance().onUserTransactionalEmail({
user_id: reinvitee.id,
message_type: 'Resend invite',
public_api: false,
});
}
}
const result = await mailer?.invite({
email: reinvitee.email,
inviteAcceptUrl,
domain: baseUrl,
});
if (!result?.success) {
} catch (error) {
void InternalHooksManager.getInstance().onEmailFailed({
user: reinvitee,
message_type: 'Resend invite',
@@ -591,19 +599,6 @@ export function usersNamespace(this: N8nApp): void {
});
throw new ResponseHelper.InternalServerError(`Failed to send email to ${reinvitee.email}`);
}
void InternalHooksManager.getInstance().onUserReinvite({
user: reinvitee,
target_user_id: reinvitee.id,
public_api: false,
});
void InternalHooksManager.getInstance().onUserTransactionalEmail({
user_id: reinvitee.id,
message_type: 'Resend invite',
public_api: false,
});
return { success: true };
}),
);

View File

@@ -197,6 +197,8 @@ export declare namespace PasswordResetRequest {
// ----------------------------------
export declare namespace UserRequest {
export type List = AuthenticatedRequest;
export type Invite = AuthenticatedRequest<{}, {}, Array<{ email: string }>>;
export type ResolveSignUp = AuthlessRequest<

View File

@@ -65,7 +65,8 @@ beforeEach(async () => {
config.set('userManagement.disabled', false);
config.set('userManagement.isInstanceOwnerSetUp', true);
config.set('userManagement.emails.mode', '');
config.set('userManagement.emails.mode', 'smtp');
config.set('userManagement.emails.smtp.host', '');
});
afterAll(async () => {
@@ -432,26 +433,28 @@ test('POST /users/:id should fail with already accepted invite', async () => {
expect(storedMember.password).not.toBe(newMemberData.password);
});
test('POST /users should fail if emailing is not set up', async () => {
test('POST /users should succeed if emailing is not set up', async () => {
const owner = await testDb.createUser({ globalRole: globalOwnerRole });
const response = await authAgent(owner)
.post('/users')
.send([{ email: randomEmail() }]);
expect(response.statusCode).toBe(500);
expect(response.statusCode).toBe(200);
expect(response.body.data[0].user.inviteAcceptUrl).toBeDefined();
});
test('POST /users should fail if user management is disabled', async () => {
const owner = await testDb.createUser({ globalRole: globalOwnerRole });
config.set('userManagement.disabled', true);
config.set('userManagement.isInstanceOwnerSetUp', false);
const response = await authAgent(owner)
.post('/users')
.send([{ email: randomEmail() }]);
expect(response.statusCode).toBe(500);
expect(response.statusCode).toBe(400);
});
test('POST /users should email invites and create user shells but ignore existing', async () => {
@@ -567,16 +570,34 @@ test('POST /users/:id/reinvite should send reinvite, but fail if user already ac
expect(reinviteMemberResponse.statusCode).toBe(400);
});
test('UserManagementMailer expect NodeMailer.verifyConnection have been called', async () => {
jest.spyOn(NodeMailer.prototype, 'verifyConnection').mockImplementation(async () => {});
test('UserManagementMailer expect NodeMailer.verifyConnection not be called when SMTP not set up', async () => {
const mockVerifyConnection = jest.spyOn(NodeMailer.prototype, 'verifyConnection');
mockVerifyConnection.mockImplementation(async () => {});
// NodeMailer.verifyConnection called 1 time
const userManagementMailer = UserManagementMailer.getInstance();
// NodeMailer.verifyConnection called 2 time
(await userManagementMailer).verifyConnection();
// NodeMailer.verifyConnection gets called only explicitly
expect(async () => await userManagementMailer.verifyConnection()).rejects.toThrow();
expect(NodeMailer.prototype.verifyConnection).toHaveBeenCalledTimes(2);
expect(NodeMailer.prototype.verifyConnection).toHaveBeenCalledTimes(0);
// @ts-ignore
NodeMailer.prototype.verifyConnection.mockRestore();
mockVerifyConnection.mockRestore();
});
test('UserManagementMailer expect NodeMailer.verifyConnection to be called when SMTP set up', async () => {
const mockVerifyConnection = jest.spyOn(NodeMailer.prototype, 'verifyConnection');
mockVerifyConnection.mockImplementation(async () => {});
const mockInit = jest.spyOn(NodeMailer.prototype, 'init');
mockInit.mockImplementation(async () => {});
// host needs to be set, otherwise smtp is skipped
config.set('userManagement.emails.smtp.host', 'host');
config.set('userManagement.emails.mode', 'smtp');
const userManagementMailer = new UserManagementMailer.UserManagementMailer();
// NodeMailer.verifyConnection gets called only explicitly
expect(async () => await userManagementMailer.verifyConnection()).not.toThrow();
// expect(NodeMailer.prototype.verifyConnection).toHaveBeenCalledTimes(1);
mockVerifyConnection.mockRestore();
mockInit.mockRestore();
});