🐛 SMTP fixes (#2937)

* 🔥 Remove `UM_` from SMTP env vars * 🔥 Remove SMTP host default value * ⚡ Update sender value * ⚡ Update invite template * ⚡ Update password reset template * ⚡ Update `N8N_EMAIL_MODE` default value * 🔥 Remove `EMAIL` from all SMTP vars * ✨ Implement `verifyConnection()` * 🚚 Reposition comment * ✏️ Fix typo * ✏️ Minor env var documentation improvements * 🎨 Fix spacing * 🎨 Fix spacing * 🗃️ Remove SMTP settings cache * ⚡ Adjust log message * ⚡ Update error message * ✏️ Fix template typo * ✏️ Adjust wording * ⚡ Interpolate email into success toast * ✏️ Adjust base message in `verifyConnection()` * ⚡ Verify connection on password reset * ⚡ Bring up POST /users SMTP check * 🐛 remove cookie if cookie is not valid * ⚡ verify connection on instantiation Co-authored-by: Ben Hesseldieck <b.hesseldieck@gmail.com>
n8n-io · Mar 7, 2022 · 49f82dc · 49f82dc
1 parent ea40412
commit 49f82dc
Show file tree

Hide file tree

Showing 12 changed files with 114 additions and 83 deletions.
diff --git a/packages/cli/commands/start.ts b/packages/cli/commands/start.ts
@@ -11,7 +11,6 @@ import { BinaryDataManager, IBinaryDataConfig, TUNNEL_SUBDOMAIN_ENV, UserSetting
 import { Command, flags } from '@oclif/command';
 // eslint-disable-next-line import/no-extraneous-dependencies
 import * as Redis from 'ioredis';
-import { AES, enc } from 'crypto-js';
 
 import { IDataObject, LoggerProxy } from 'n8n-workflow';
 import { createHash } from 'crypto';
@@ -219,38 +218,6 @@ export class Start extends Command {
 					throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY);
 				}
 
-				if (config.get('userManagement.emails.mode') === 'smtp') {
-					const { auth, ...rest } = config.get('userManagement.emails.smtp');
-
-					const encryptedAuth = {
-						user: auth.user,
-						pass: AES.encrypt(auth.pass, encryptionKey).toString(),
-					};
-
-					await Db.collections.Settings!.save({
-						key: 'userManagement.emails.smtp',
-						value: JSON.stringify({ ...rest, auth: encryptedAuth }),
-						loadOnStartup: false,
-					});
-				} else {
-					// If we don't have SMTP settings, try loading from db.
-					const smtpSetting = await Db.collections.Settings!.findOne({
-						key: 'userManagement.emails.smtp',
-					});
-
-					if (smtpSetting) {
-						const { auth, ...rest } = JSON.parse(smtpSetting.value) as SmtpConfig;
-
-						const decryptedAuth = {
-							user: auth.user,
-							pass: AES.decrypt(auth.pass, encryptionKey).toString(enc.Utf8),
-						};
-
-						config.set('userManagement.emails.mode', 'smtp');
-						config.set('userManagement.emails.smtp', { ...rest, auth: decryptedAuth });
-					}
-				}
-
 				// Load settings from database and set them to config.
 				const databaseSettings = await Db.collections.Settings!.find({ loadOnStartup: true });
 				databaseSettings.forEach((setting) => {

diff --git a/packages/cli/config/index.ts b/packages/cli/config/index.ts
@@ -598,47 +598,47 @@ const config = convict({
 			mode: {
 				doc: 'How to send emails',
 				format: ['', 'smtp'],
-				default: '',
-				env: 'N8N_UM_EMAIL_MODE',
+				default: 'smtp',
+				env: 'N8N_MODE',
 			},
 			smtp: {
 				host: {
 					doc: 'SMTP server host',
-					format: String,
-					default: 'smtp.gmail.com',
-					env: 'N8N_UM_EMAIL_SMTP_HOST',
+					format: String, // e.g. 'smtp.gmail.com'
+					default: '',
+					env: 'N8N_SMTP_HOST',
 				},
 				port: {
-					doc: 'SMTP Server port',
+					doc: 'SMTP server port',
 					format: Number,
 					default: 465,
-					env: 'N8N_UM_EMAIL_SMTP_PORT',
+					env: 'N8N_SMTP_PORT',
 				},
 				secure: {
-					doc: 'Whether or not to use SSL',
+					doc: 'Whether or not to use SSL for SMTP',
 					format: Boolean,
 					default: true,
-					env: 'N8N_UM_EMAIL_SMTP_SSL',
+					env: 'N8N_SMTP_SSL',
 				},
 				auth: {
 					user: {
-						doc: 'SMTP Login username',
-						format: String,
-						default: 'youremail@gmail.com',
-						env: 'N8N_UM_EMAIL_SMTP_USER',
+						doc: 'SMTP login username',
+						format: String, // e.g.'you@gmail.com'
+						default: '',
+						env: 'N8N_SMTP_USER',
 					},
 					pass: {
-						doc: 'SMTP Login password',
+						doc: 'SMTP login password',
 						format: String,
-						default: 'my-super-password',
-						env: 'N8N_UM_EMAIL_SMTP_PASS',
+						default: '',
+						env: 'N8N_SMTP_PASS',
 					},
 				},
 				sender: {
 					doc: 'How to display sender name',
 					format: String,
-					default: '"n8n rocks" <n8n@n8n.io>',
-					env: 'N8N_UM_EMAIL_SMTP_SENDER',
+					default: '',
+					env: 'N8N_SMTP_SENDER',
 				},
 			},
 			templates: {

diff --git a/packages/cli/src/UserManagement/email/Interfaces.ts b/packages/cli/src/UserManagement/email/Interfaces.ts
@@ -1,5 +1,6 @@
 export interface UserManagementMailerImplementation {
 	sendMail: (mailData: MailData) => Promise<SendEmailResult>;
+	verifyConnection: () => Promise<void>;
 }
 
 export type InviteEmailData = {

diff --git a/packages/cli/src/UserManagement/email/NodeMailer.ts b/packages/cli/src/UserManagement/email/NodeMailer.ts
@@ -19,10 +19,37 @@ export class NodeMailer implements UserManagementMailerImplementation {
 		});
 	}
 
+	async verifyConnection(): Promise<void> {
+		const host = config.get('userManagement.emails.smtp.host') as string;
+		const user = config.get('userManagement.emails.smtp.auth.user') as string;
+		const pass = config.get('userManagement.emails.smtp.auth.pass') as string;
+
+		return new Promise((resolve, reject) => {
+			this.transport.verify((error: Error) => {
+				if (!error) resolve();
+
+				const message = [];
+
+				if (!host) message.push('SMTP host not defined (N8N_SMTP_HOST).');
+				if (!user) message.push('SMTP user not defined (N8N_SMTP_USER).');
+				if (!pass) message.push('SMTP pass not defined (N8N_SMTP_PASS).');
+
+				reject(new Error(message.join(' ')));
+			});
+		});
+	}
+
 	async sendMail(mailData: MailData): Promise<SendEmailResult> {
+		let sender = config.get('userManagement.emails.smtp.sender');
+		const user = config.get('userManagement.emails.smtp.auth.user') as string;
+
+		if (!sender && user.includes('@')) {
+			sender = user;
+		}
+
 		try {
 			await this.transport.sendMail({
-				from: config.get('userManagement.emails.smtp.sender'),
+				from: sender,
 				to: mailData.emailRecipients,
 				subject: mailData.subject,
 				text: mailData.textOnly,

diff --git a/packages/cli/src/UserManagement/email/UserManagementMailer.ts b/packages/cli/src/UserManagement/email/UserManagementMailer.ts
@@ -50,6 +50,12 @@ export class UserManagementMailer {
 		}
 	}
 
+	async verifyConnection(): Promise<void> {
+		if (!this.mailer) return Promise.reject();
+
+		return this.mailer.verifyConnection();
+	}
+
 	async invite(inviteEmailData: InviteEmailData): Promise<SendEmailResult> {
 		let template = await getTemplate('invite', 'invite.html');
 		template = replaceStrings(template, inviteEmailData);
@@ -83,9 +89,10 @@ export class UserManagementMailer {
 
 let mailerInstance: UserManagementMailer | undefined;
 
-export function getInstance(): UserManagementMailer {
+export async function getInstance(): Promise<UserManagementMailer> {
 	if (mailerInstance === undefined) {
 		mailerInstance = new UserManagementMailer();
+		await mailerInstance.verifyConnection();
 	}
 	return mailerInstance;
 }
diff --git a/packages/cli/src/UserManagement/email/templates/invite.html b/packages/cli/src/UserManagement/email/templates/invite.html
@@ -1,6 +1,4 @@
 <p>Hi there,</p>
-<p>You have been invited to join n8n {{domain}}.</p>
-<p>Please click on the following link, or paste it into your browser to complete the process.</p>
+<p>You have been invited to join n8n ({{ domain }}).</p>
+<p>To accept, click the following link:</p>
 <p><a href="{{ inviteAcceptUrl }}" target="_blank">{{ inviteAcceptUrl }}</a></p>
-<br>
-<p>Thanks!</p>
diff --git a/packages/cli/src/UserManagement/email/templates/passwordReset.html b/packages/cli/src/UserManagement/email/templates/passwordReset.html
@@ -1,9 +1,5 @@
 <p>Hi {{firstName}},</p>
-<p>You are receiving this because you (or someone else) requested a password reset for your account {{email}} on n8n {{domain}}.</p>
-<p>Please click on the following link, or paste it into your browser to complete the process:</p>
-<a href="{{ passwordResetUrl }}">{{ passwordResetUrl }}</a> (link expires in 2 hours)
-<br><br>
-<p>If you received this in error, you can safely ignore it.</p>
-<p>Contact your n8n instance owner if you did not request to reset your password.</p>
-<br>
-<p>Thanks!</p>
+<p>Somebody asked to reset your password on n8n ({{ domain }}).</p>
+<p>If it was not you, you can safely ignore this email.</p>
+<p>Click the following link to choose a new password. It is valid for 2 hours.</p>
+<a href="{{ passwordResetUrl }}">{{ passwordResetUrl }}</a>
diff --git a/packages/cli/src/UserManagement/routes/auth.ts b/packages/cli/src/UserManagement/routes/auth.ts
@@ -71,7 +71,7 @@ export function authenticationMethods(this: N8nApp): void {
 					user = await resolveJwt(cookieContents);
 					return sanitizeUser(user);
 				} catch (error) {
-					throw new Error('Invalid login information');
+					res.clearCookie(AUTH_COOKIE_NAME);
 				}
 			}
 

diff --git a/packages/cli/src/UserManagement/routes/passwordReset.ts b/packages/cli/src/UserManagement/routes/passwordReset.ts
@@ -78,13 +78,24 @@ export function passwordResetNamespace(this: N8nApp): void {
 			url.searchParams.append('userId', id);
 			url.searchParams.append('token', resetPasswordToken);
 
-			await UserManagementMailer.getInstance().passwordReset({
-				email,
-				firstName,
-				lastName,
-				passwordResetUrl: url.toString(),
-				domain: baseUrl,
-			});
+			try {
+				const mailer = await UserManagementMailer.getInstance();
+				await mailer.passwordReset({
+					email,
+					firstName,
+					lastName,
+					passwordResetUrl: url.toString(),
+					domain: baseUrl,
+				});
+			} catch (error) {
+				if (error instanceof Error) {
+					throw new ResponseHelper.ResponseError(
+						`Please contact your administrator: ${error.message}`,
+						undefined,
+						500,
+					);
+				}
+			}
 
 			Logger.info('Sent password reset email successfully', { userId: user.id, email });
 		}),

diff --git a/packages/cli/src/UserManagement/routes/users.ts b/packages/cli/src/UserManagement/routes/users.ts
@@ -14,7 +14,7 @@ import { getInstanceBaseUrl, sanitizeUser, validatePassword } from '../UserManag
 import { User } from '../../databases/entities/User';
 import { SharedWorkflow } from '../../databases/entities/SharedWorkflow';
 import { SharedCredentials } from '../../databases/entities/SharedCredentials';
-import { getInstance } from '../email/UserManagementMailer';
+import * as UserManagementMailer from '../email/UserManagementMailer';
 
 import config = require('../../../config');
 import { issueCookie } from '../auth/jwt';
@@ -37,6 +37,19 @@ export function usersNamespace(this: N8nApp): void {
 				);
 			}
 
+			let mailer: UserManagementMailer.UserManagementMailer | undefined;
+			try {
+				mailer = await UserManagementMailer.getInstance();
+			} catch (error) {
+				if (error instanceof Error) {
+					throw new ResponseHelper.ResponseError(
+						`There is a problem with your SMTP setup: ${error.message}`,
+						undefined,
+						500,
+					);
+				}
+			}
+
 			if (!config.get('userManagement.isInstanceOwnerSetUp')) {
 				Logger.debug(
 					'Request to send email invite(s) to user(s) failed because emailing was not set up',
@@ -131,7 +144,7 @@ export function usersNamespace(this: N8nApp): void {
 				throw new ResponseHelper.ResponseError('An error occurred during user creation');
 			}
 
-			Logger.info('Created user shells successfully', { userId: req.user.id });
+			Logger.info('Created user shell(s) successfully', { userId: req.user.id });
 			Logger.verbose(total > 1 ? `${total} user shells created` : `1 user shell created`, {
 				userShells: createUsers,
 			});
@@ -141,13 +154,12 @@ export function usersNamespace(this: N8nApp): void {
 			const usersPendingSetup = Object.entries(createUsers).filter(([email, id]) => id && email);
 
 			// send invite email to new or not yet setup users
-			const mailer = getInstance();
 
 			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({
+					const result = await mailer?.invite({
 						email,
 						inviteAcceptUrl,
 						domain: baseUrl,
@@ -158,7 +170,7 @@ export function usersNamespace(this: N8nApp): void {
 							email,
 						},
 					};
-					if (!result.success) {
+					if (!result?.success) {
 						Logger.error('Failed to send email', {
 							userId: req.user.id,
 							inviteAcceptUrl,
@@ -455,13 +467,22 @@ export function usersNamespace(this: N8nApp): void {
 			const baseUrl = getInstanceBaseUrl();
 			const inviteAcceptUrl = `${baseUrl}/signup?inviterId=${req.user.id}&inviteeId=${reinvitee.id}`;
 
-			const result = await getInstance().invite({
+			let mailer: UserManagementMailer.UserManagementMailer | undefined;
+			try {
+				mailer = await UserManagementMailer.getInstance();
+			} catch (error) {
+				if (error instanceof Error) {
+					throw new ResponseHelper.ResponseError(error.message, undefined, 500);
+				}
+			}
+
+			const result = await mailer?.invite({
 				email: reinvitee.email,
 				inviteAcceptUrl,
 				domain: baseUrl,
 			});
 
-			if (!result.success) {
+			if (!result?.success) {
 				Logger.error('Failed to send email', {
 					email: reinvitee.email,
 					inviteAcceptUrl,

diff --git a/packages/editor-ui/src/plugins/i18n/locales/en.json b/packages/editor-ui/src/plugins/i18n/locales/en.json
@@ -1097,7 +1097,6 @@
 	},
 	"BASIC_INFORMATION": "Basic Information",
 	"CHANGE_PASSWORD": "Change Password",
-	"CHECK_INBOX_AND_SPAM": "Please check your inbox (and perhaps your spam folder)",
 	"CONFIRM_DATA_HANDLING_AFTER_DELETION": "What should we do with their data?",
 	"CONFIRM_USER_DELETION": "Are you sure you want to delete this invited user?",
 	"CURRENT_PASSWORD": "Current password",
@@ -1115,6 +1114,7 @@
 	"FINISH_ACCOUNT_SETUP": "Finish account setup",
 	"FIRST_NAME": "First name",
 	"FORGOT_MY_PASSWORD": "Forgot my password",
+	"FORGOT_PASSWORD_SUCCESS_MESSAGE": "We’ve emailed {email} (if there’s a matching account)",
 	"GET_RECOVERY_LINK": "Email me a recovery link",
 	"GO_BACK": "Go back",
 	"INVALID_EMAIL_ERROR": "{email} is not a valid email",

diff --git a/packages/editor-ui/src/views/ForgotMyPasswordView.vue b/packages/editor-ui/src/views/ForgotMyPasswordView.vue
@@ -72,15 +72,18 @@ export default mixins(
 		},
 	},
 	methods: {
-		async onSubmit(values: {email: string}) {
+		async onSubmit(values: { email: string }) {
 			try {
 				this.loading = true;
 				await this.$store.dispatch('users/sendForgotPasswordEmail', values);
 
 				this.$showMessage({
 					type: 'success',
 					title: this.$locale.baseText('RECOVERY_EMAIL_SENT'),
-					message: this.$locale.baseText('EMAIL_SENT_IF_EXISTS', {interpolate: {email: values.email}}),
+					message: this.$locale.baseText(
+						'FORGOT_PASSWORD_SUCCESS_MESSAGE',
+						{ interpolate: { email: values.email }},
+					),
 				});
 			} catch (error) {
 				this.$showMessage({