Skip to content

Commit

Permalink
feat: fixed outbound smtp edge case error issues, added email for rat…
Browse files Browse the repository at this point in the history
…e limiting, fixed ENOTFOUND issue with check SMTP job, added job to fix pending emails, sync locales, sync translations
  • Loading branch information
titanism committed Apr 30, 2024
1 parent e6bfbe8 commit 334ca70
Show file tree
Hide file tree
Showing 41 changed files with 281 additions and 79 deletions.
4 changes: 3 additions & 1 deletion app/controllers/web/my-account/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const changeModulusLength = require('./change-modulus-length');
const checkVerifiedEmail = require('./check-verified-email');
const updateAllowlistAndDenylist = require('./update-allowlist-and-denylist');
const updateTimezone = require('./update-timezone');
const uploadAliasMbox = require('./upload-alias-mbox');

module.exports = {
cancelEmailChange,
Expand Down Expand Up @@ -126,5 +127,6 @@ module.exports = {
changeModulusLength,
checkVerifiedEmail,
updateAllowlistAndDenylist,
updateTimezone
updateTimezone,
uploadAliasMbox
};
36 changes: 36 additions & 0 deletions app/controllers/web/my-account/upload-alias-mbox.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
* Copyright (c) Forward Email LLC
* SPDX-License-Identifier: BUSL-1.1
*/

async function uploadAliasMbox() {
throw new Error('Coming soon');
/*
const redirectTo = ctx.state.l(
`/my-account/domains/${ctx.state.domain.name}/aliases`
);
try {
// ensure the size is not more than 2 GB
// store the file to the server
//
// copy the file to the sqlite server in the background
// once it's done then fire a websocket request to parse payload
// which will then kick off the mbox import and email the user once done
//
} catch (err) {
if (err && err.isBoom) throw err;
if (isErrorConstructorName(err, 'ValidationError')) throw err;
ctx.logger.fatal(err);
ctx.flash('error', ctx.translate('UNKNOWN_ERROR'));
const redirectTo = ctx.state.l(
`/my-account/domains/${ctx.state.domain.name}/aliases`
);
if (ctx.accepts('html')) ctx.redirect(redirectTo);
else ctx.body = { redirectTo };
}
*/
}

module.exports = uploadAliasMbox;
1 change: 1 addition & 0 deletions app/models/emails.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ const Emails = new mongoose.Schema(
rejectedErrors: [mongoose.Schema.Types.Mixed]
},
{
versionKey: false,
writeConcern: {
w: 'majority'
}
Expand Down
3 changes: 2 additions & 1 deletion app/models/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ const Users = new mongoose.Schema({
unique: true,
validate: (value) => validator.isEmail(value)
},
scheduled_send_sent_at: Date
scheduled_send_sent_at: Date,
smtp_rate_limit_sent_at: Date
});

// Additional variable based properties to add to the schema
Expand Down
4 changes: 2 additions & 2 deletions config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,8 @@ const config = {
},
defaultModulusLength: 1024,
defaultStoragePath: env.SQLITE_STORAGE_PATH,
// 60 items (50 MB * 60 = 3000 MB = 3 GB)
smtpMaxQueue: 60,
// 100 items (50 MB * 100 = 5000 MB = 5 GB)
smtpMaxQueue: 100,
smtpQueueTimeout: ms('180s'),
smtpLimitMessages: env.NODE_ENV === 'test' ? 10 : 300,
smtpLimitAuth: env.NODE_ENV === 'test' ? Number.MAX_VALUE : 10,
Expand Down
2 changes: 2 additions & 0 deletions config/phrases.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ for (const key of Object.keys(statuses.message)) {
}

module.exports = {
SMTP_RATE_LIMIT_EXCEEDED:
'You have exceeded your daily SMTP outbound rate limit.',
UBUNTU_NOT_ALLOWED_EMAIL:
'You cannot use that email address as a forwarding recipient.',
UBUNTU_PERMISSIONS:
Expand Down
9 changes: 7 additions & 2 deletions helpers/smtp/on-connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ const punycode = require('node:punycode');
const isFQDN = require('is-fqdn');
const { boolean } = require('boolean');

const SMTPError = require('#helpers/smtp-error');
// const SMTPError = require('#helpers/smtp-error');
const ServerShutdownError = require('#helpers/server-shutdown-error');
const config = require('#config');
// const config = require('#config');
const env = require('#config/env');
const parseRootDomain = require('#helpers/parse-root-domain');
const refineAndLogError = require('#helpers/refine-and-log-error');
Expand Down Expand Up @@ -59,6 +59,10 @@ async function onConnect(session, fn) {
if (boolean(result)) {
session.allowlistValue = rootDomain || session.remoteAddress;
} else {
//
// NOTE: because there are too many false positives with actual users
// we're not going to do denylist/silent/backscatter lookup anymore
/*
//
// prevent connections from backscatter, silent ban, and denylist
//
Expand Down Expand Up @@ -89,6 +93,7 @@ async function onConnect(session, fn) {
{ ignoreHook: true }
);
}
*/
}

fn();
Expand Down
61 changes: 56 additions & 5 deletions helpers/smtp/on-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

const _ = require('lodash');
const bytes = require('bytes');
const dayjs = require('dayjs-with-plugins');
const getStream = require('get-stream');
const mongoose = require('mongoose');
const safeStringify = require('fast-safe-stringify');
Expand All @@ -18,16 +19,56 @@ const ServerShutdownError = require('#helpers/server-shutdown-error');
const Users = require('#models/users');
const config = require('#config');
const createSession = require('#helpers/create-session');
const emailHelper = require('#helpers/email');
const env = require('#config/env');
const i18n = require('#helpers/i18n');
const isValidPassword = require('#helpers/is-valid-password');
const logger = require('#helpers/logger');
const refineAndLogError = require('#helpers/refine-and-log-error');
const validateAlias = require('#helpers/validate-alias');
const validateDomain = require('#helpers/validate-domain');
const isValidPassword = require('#helpers/is-valid-password');
const { decrypt } = require('#helpers/encrypt-decrypt');

const MAX_BYTES = bytes(env.SMTP_MESSAGE_MAX_SIZE);

async function sendRateLimitEmail(user) {
// if the user received rate limit email in past 30d
if (
_.isDate(user.smtp_rate_limit_sent_at) &&
dayjs().isBefore(dayjs(user.smtp_rate_limit_sent_at).add(30, 'days'))
) {
logger.info('user was already rate limited');
return;
}

await emailHelper({
template: 'alert',
message: {
to: user[config.userFields.fullEmail],
bcc: config.email.message.from,
locale: user[config.lastLocaleField],
subject: i18n.translate(
'SMTP_RATE_LIMIT_EXCEEDED',
user[config.lastLocaleField]
)
},
locals: {
locale: user[config.lastLocaleField],
message: i18n.translate(
'SMTP_RATE_LIMIT_EXCEEDED',
user[config.lastLocaleField]
)
}
});

// otherwise send the user an email and update the user record
await Users.findByIdAndUpdate(user._id, {
$set: {
smtp_rate_limit_sent_at: new Date()
}
});
}

//
// NOTE: we can merge SMTP/FE codebase in future and simply check if auth disabled
// then this will act as a forwarding server only (MTA)
Expand Down Expand Up @@ -80,7 +121,7 @@ async function onData(stream, _session, fn) {
})
.populate(
'user',
`id ${config.userFields.isBanned} ${config.userFields.smtpLimit}`
`id ${config.userFields.isBanned} ${config.userFields.smtpLimit} smtp_rate_limit_sent_at ${config.userFields.fullEmail} ${config.lastLocaleField}`
)
.select('+tokens.hash +tokens.salt')
.lean()
Expand All @@ -106,7 +147,7 @@ async function onData(stream, _session, fn) {
})
.populate(
'members.user',
`id plan email ${config.userFields.isBanned} ${config.userFields.hasVerifiedEmail} ${config.userFields.planExpiresAt} ${config.userFields.smtpLimit} ${config.userFields.stripeSubscriptionID} ${config.userFields.paypalSubscriptionID}`
`id plan email ${config.userFields.isBanned} ${config.userFields.hasVerifiedEmail} ${config.userFields.planExpiresAt} ${config.userFields.smtpLimit} ${config.userFields.stripeSubscriptionID} ${config.userFields.paypalSubscriptionID} smtp_rate_limit_sent_at ${config.userFields.fullEmail} ${config.lastLocaleField}`
)
.select('+tokens +tokens.hash +tokens.salt')
.exec();
Expand Down Expand Up @@ -342,8 +383,13 @@ async function onData(stream, _session, fn) {
`${config.smtpLimitNamespace}:${domain.id}`
);
// return 550 error code
if (count >= max)
if (count >= max) {
// send one-time email alert to admin + user
sendRateLimitEmail(user)
.then()
.catch((err) => logger.fatal(err, { session }));
throw new SMTPError('Rate limit exceeded', { ignoreHook: true });
}
}

// rate limit to X emails per day by alias user id then denylist
Expand All @@ -352,8 +398,13 @@ async function onData(stream, _session, fn) {
`${config.smtpLimitNamespace}:${user.id}`
);
// return 550 error code
if (count >= max)
if (count >= max) {
// send one-time email alert to admin + user
sendRateLimitEmail(user)
.then()
.catch((err) => logger.fatal(err, { session }));
throw new SMTPError('Rate limit exceeded', { ignoreHook: true });
}
}
}

Expand Down
3 changes: 1 addition & 2 deletions jobs/check-smtp.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ const DNS_RETRY_CODES = new Set([
// 'ENODATA',
'ENOMEM',
'ENONAME',
// NOTE: ENOTFOUND indicates the domain doesn't exist
// (and we don't want to send emails to people that didn't even register it yet)
// NOTE: ENOTFOUND indicates the record doesn't exist
'ENOTFOUND',
'ENOTIMP',
'ENOTINITIALIZED',
Expand Down
87 changes: 52 additions & 35 deletions jobs/send-emails.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ const graceful = new Graceful({
});

const queue = new PQueue({
concurrency: config.concurrency * 16
concurrency: Math.round(config.smtpMaxQueue / 2)
// concurrency: config.concurrency * 30
// timeout: config.smtpQueueTimeout
});

Expand Down Expand Up @@ -74,12 +75,12 @@ async function sendEmails() {

if (queue.size >= config.smtpMaxQueue) {
logger.info(`queue has more than ${config.smtpMaxQueue} tasks`);
// wait 1 second
await delay(1000);
// queue more messages once finished processing
return sendEmails();
await delay(5000);
return;
}

// TODO: capacity/recipient issues should be hard 550 bounce for outbound

const now = new Date();
const limit = config.smtpMaxQueue - queue.size;

Expand Down Expand Up @@ -199,43 +200,59 @@ async function sendEmails() {
// return early if the job was already cancelled
if (isCancelled) break;
// TODO: implement queue on a per-target/provider basis (e.g. 10 at once to Cox addresses)
queue.add(() => processEmail({ email, resolver, client }), {
// TODO: if the email was admin owned domain then priority higher (see email pre-save hook)
// priority: email.priority || 0
});
queue.add(
async () => {
try {
await processEmail({ email, resolver, client });
} catch (err) {
logger.error(err, { email });
}
},
{
// TODO: if the email was admin owned domain then priority higher (see email pre-save hook)
// priority: email.priority || 0
}
);
}

// wait 1 second
await delay(1000);

// queue more messages once finished processing
return sendEmails();
await delay(5000);
}

(async () => {
await setupMongoose(logger);

try {
await sendEmails();
} catch (err) {
await logger.error(err);
(async function startRecursion() {
if (isCancelled) {
if (parentPort) parentPort.postMessage('done');
else process.exit(0);
return;
}

await emailHelper({
template: 'alert',
message: {
to: config.email.message.from,
subject: 'Send emails had an error'
},
locals: {
message: `<pre><code>${JSON.stringify(
parseErr(err),
null,
2
)}</code></pre>`
}
});
try {
await sendEmails();
} catch (err) {
logger.error(err);

emailHelper({
template: 'alert',
message: {
to: config.email.message.from,
subject: 'Send emails had an error'
},
locals: {
message: `<pre><code>${JSON.stringify(
parseErr(err),
null,
2
)}</code></pre>`
}
})
.then()
.catch((err) => logger.fatal(err));

await delay(5000);
}

if (parentPort) parentPort.postMessage('done');
else process.exit(0);
}
startRecursion();
})();
})();
Loading

0 comments on commit 334ca70

Please sign in to comment.