Skip to content

Commit

Permalink
fix: fixed email retention, improved locking and email processing, fi…
Browse files Browse the repository at this point in the history
…xed pagination api bug, cleanup denylist issues
  • Loading branch information
titanism committed Aug 25, 2023
1 parent cb766ef commit d1a645d
Show file tree
Hide file tree
Showing 12 changed files with 131 additions and 29 deletions.
5 changes: 5 additions & 0 deletions .env.defaults
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,11 @@ SMTP_EXCHANGE_DOMAINS=mx1.forwardemail.net,mx2.forwardemail.net
TRUTH_SOURCES=
MAX_RECIPIENTS=50

#####################
## email retention ##
#####################
EMAIL_RETENTION="30d"

###################
## log retention ##
###################
Expand Down
5 changes: 5 additions & 0 deletions .env.schema
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,11 @@ SMTP_EXCHANGE_DOMAINS=
TRUTH_SOURCES=
MAX_RECIPIENTS=

#####################
## email retention ##
#####################
EMAIL_RETENTION=

###################
## log retention ##
###################
Expand Down
25 changes: 10 additions & 15 deletions app/models/emails.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@ const scanner = new SpamScanner({
});

const Emails = new mongoose.Schema({
//
// NOTE: `mongoose-common-plugin` will automatically set `timestamps` for us
// <https://github.com/Automattic/mongoose/blob/b2af0fe4a74aa39eaf3088447b4bb8feeab49342/test/timestamps.test.js#L123-L137>
//
created_at: {
type: Date,
expires: config.emailRetention,
index: true
},
hard_bounces: [String], // 5xx bounces (an array for storage to prevent duplicates)
soft_bounces: [String], // 4xx bounces (an array for storage to prevent duplicates)
is_bounce: {
Expand Down Expand Up @@ -189,14 +198,6 @@ const Emails = new mongoose.Schema({
rejectedErrors: [mongoose.Schema.Types.Mixed]
});

Emails.virtual('is_being_locked')
.get(function () {
return this.__is_being_locked;
})
.set(function (isBeingLocked) {
this.__is_being_locked = boolean(isBeingLocked);
});

Emails.plugin(mongooseCommonPlugin, {
object: 'email',
locale: false,
Expand All @@ -207,12 +208,7 @@ Emails.plugin(mongooseCommonPlugin, {
'locked_by',
'locked_at',
'priority'
],
mongooseHidden: {
virtuals: {
is_being_locked: 'hide'
}
}
]
});

// when we query against `locked_at` we also need to query for `$exists: true` for hint to work
Expand Down Expand Up @@ -364,7 +360,6 @@ Emails.pre('save', function (next) {
// update `status` based off `accepted` and `rejectedErrors` array
//
Emails.pre('save', function (next) {
if (this.is_being_locked) return next();
try {
// if email is still in "pending" state then do not modify it
if (this.status === 'pending' && this.rejectedErrors.length === 0)
Expand Down
12 changes: 8 additions & 4 deletions app/views/api/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,16 @@ Note that this endpoint does not return an already created email's `message`, `h

To return those properties and their values, please use the [Retrieve email](#retrieve-email) endpoint with an email ID.

This endpoint will return at most `50` results at a time. If you want to query for multiple pages, then append `?page=NUMBER` where `NUMBER` is an integer, e.g. `?page=1`.

> `GET /v1/emails`
| Querystring Parameter | Required | Type | Description |
| --------------------- | -------- | ------------------------- | -------------------------------- |
| `q` | No | String (RegExp supported) | Search for emails by metadata |
| `domain` | No | String (RegExp supported) | Search for emails by domain name |
| Querystring Parameter | Required | Type | Description |
| --------------------- | -------- | ------------------------- | -------------------------------------------------------------------------------------------- |
| `q` | No | String (RegExp supported) | Search for emails by metadata |
| `domain` | No | String (RegExp supported) | Search for emails by domain name |
| `page` | No | Number | Page to return of results (default is `1`) |
| `limit | No | Number | Number of results per page to return (default is `50` – the max is `50` and minimum is `10`) |

> Example Request:
Expand Down
1 change: 1 addition & 0 deletions config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const config = {
: []
),

emailRetention: env.EMAIL_RETENTION,
logRetention: env.LOG_RETENTION,

// custom rate limiting lookup for allowing whitelisted customers
Expand Down
30 changes: 24 additions & 6 deletions helpers/process-email.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,13 @@ async function processEmail({ email, port = 25, resolver, client }) {

try {
// lock job
email.is_locked = true;
email.locked_by = IP_ADDRESS;
email.locked_at = new Date();
email.is_being_locked = true;
email = await email.save();
email.is_being_locked = false;
await Emails.findByIdAndUpdate(email._id, {
$set: {
is_locked: true,
locked_by: IP_ADDRESS,
locked_at: new Date()
}
});

// ensure user, domain, and alias still exist and are all enabled
let [user, domain, alias] = await Promise.all([
Expand Down Expand Up @@ -246,6 +247,10 @@ async function processEmail({ email, port = 25, resolver, client }) {
// helper boolean for setting "bounced" status (see email pre-save hook)
err.maxRetryDuration = true;

// lookup the email by id to get most recent data and version key (`__v`)
email = await Emails.findById(email._id);
if (!email) throw new Error('Email does not exist');

// NOTE: save() will automatically remove from `rejectedErrors` any already `accepted`
email.rejectedErrors.push(
...email.envelope.to.map((recipient) => {
Expand Down Expand Up @@ -328,6 +333,11 @@ async function processEmail({ email, port = 25, resolver, client }) {
domain.smtp_emails_blocked.length > 0
) {
const matches = [];

// lookup the email by id to get most recent data and version key (`__v`)
email = await Emails.findById(email._id);
if (!email) throw new Error('Email does not exist');

for (const recipient of email.envelope.to) {
if (domain.smtp_emails_blocked.includes(recipient)) {
const error = Boom.forbidden(
Expand Down Expand Up @@ -890,6 +900,10 @@ async function processEmail({ email, port = 25, resolver, client }) {
//
}

// lookup the email by id to get most recent data and version key (`__v`)
email = await Emails.findById(email._id);
if (!email) throw new Error('Email does not exist');

//
// update `accepted` array
//
Expand Down Expand Up @@ -1032,6 +1046,10 @@ async function processEmail({ email, port = 25, resolver, client }) {
err.isCodeBug = isCodeBug(err);
err.responseCode = getErrorCode(err);

// lookup the email by id to get most recent data and version key (`__v`)
email = await Emails.findById(email._id);
if (!email) throw new Error('Email does not exist');

// if we threw an error with Boom, then based off the type we need to update email status
if (
err.isBoom === true &&
Expand Down
4 changes: 4 additions & 0 deletions helpers/send-email.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ async function shouldThrowError(err, session) {
} else if (err.response.includes('AUP#1260'))
// IPv6 not supported with Spectrum
err.responseCode = 421;
else if (err.response.includes('JunkMail rejected'))
err.bounceInfo.category = 'blocklist';
else if (
err.response.includes(
'spectrum.net/support/internet/understanding-email-error-codes'
Expand All @@ -201,6 +203,8 @@ async function shouldThrowError(err, session) {
// <https://github.com/zone-eu/zone-mta/issues/331>
else if (err.response.includes('spamcop.net'))
err.bounceInfo.category = 'blocklist';
// generic detection of RBL blocklist
else if (err.response.includes('RBL')) err.bounceInfo.category = 'blocklist';
else if (
err.target === 'qq.com' &&
err.response.includes('550 Mail content denied')
Expand Down
59 changes: 59 additions & 0 deletions jobs/delete-emails.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// eslint-disable-next-line import/no-unassigned-import
require('#config/env');

const process = require('process');
const { parentPort } = require('worker_threads');

// eslint-disable-next-line import/no-unassigned-import
require('#config/mongoose');

const Graceful = require('@ladjs/graceful');
const ms = require('ms');
const parseErr = require('parse-err');

const mongoose = require('mongoose');
const emailHelper = require('#helpers/email');
const Emails = require('#models/emails');
const logger = require('#helpers/logger');
const config = require('#config');
const setupMongoose = require('#helpers/setup-mongoose');

const graceful = new Graceful({
mongooses: [mongoose],
logger
});

graceful.listen();

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

try {
await Emails.deleteMany(
{
created_at: { $lt: Date.now() - ms(config.emailRetention) }
},
{ writeConcern: { w: 0, j: false } }
);
} catch (err) {
await logger.error(err);
// send an email to admins of the error
await emailHelper({
template: 'alert',
message: {
to: config.email.message.from,
subject: 'Delete Emails Issue'
},
locals: {
message: `<pre><code>${JSON.stringify(
parseErr(err),
null,
2
)}</code></pre>`
}
});
}

if (parentPort) parentPort.postMessage('done');
else process.exit(0);
})();
5 changes: 5 additions & 0 deletions jobs/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ const jobs = [
interval: '1h',
timeout: 0
},
{
name: 'delete-emails',
interval: '1h',
timeout: 0
},
{
name: 'parse-logs',
interval: '5m',
Expand Down
10 changes: 7 additions & 3 deletions jobs/sync-paid-alias-allowlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,14 @@ graceful.listen();
}
});

// filter out specific emails that were marked spam
// remove all domains and emails that were denylisted
// (paying customers shouldn't get denylisted but we monitor this via email alerts)
const p = client.pipeline();
for (const v of list) {
if (isEmail(v)) set.delete(v);
p.del(`denylist:${v}`);
}

await p.exec();
}
}

Expand All @@ -297,7 +301,7 @@ graceful.listen();
locals: {
message: `<p class="text-center">The domain ${domain.name} (${
domain.id
}) had the following denylist results:</p><ul class="mb-0 text-left"><li>${list.join(
}) had the following silent ban results:</p><ul class="mb-0 text-left"><li>${list.join(
'</li><li>'
)}</li></ul>`
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@
"striptags": "3.2.0",
"superagent": "8.0.9",
"sweetalert2": "8",
"tangerine": "1.5.3",
"tangerine": "1.5.4",
"titleize": "2",
"twilio": "4.12.0",
"uncaught": "0.0.5",
Expand Down
2 changes: 2 additions & 0 deletions routes/api/v1/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const Router = require('@koa/router');
const bodyParser = require('koa-bodyparser');
const paginate = require('koa-ctx-paginate');

const api = require('#controllers/api');
const policies = require('#helpers/policies');
Expand Down Expand Up @@ -96,6 +97,7 @@ router
.get(
'/emails',
rateLimit(100, 'list emails'),
paginate.middleware(10, 50),
web.myAccount.retrieveDomains,
web.myAccount.listEmails,
api.v1.emails.list
Expand Down

0 comments on commit d1a645d

Please sign in to comment.