diff --git a/api/config/custom-environment-variables.cjs b/api/config/custom-environment-variables.cjs index 03619174..00592245 100644 --- a/api/config/custom-environment-variables.cjs +++ b/api/config/custom-environment-variables.cjs @@ -267,6 +267,10 @@ module.exports = { attempts: 'AUTHRATELIMIT_ATTEMPTS', duration: 'AUTHRATELIMIT_DURATION' }, + mailsRateLimit: { + points: 'MAILSRATELIMIT_POINTS', + duration: 'MAILSRATELIMIT_DURATION' + }, oauth: { providers: jsonEnv('OAUTH_PROVIDERS'), github: { diff --git a/api/config/default.cjs b/api/config/default.cjs index 8601d530..3e98ebd6 100644 --- a/api/config/default.cjs +++ b/api/config/default.cjs @@ -185,6 +185,12 @@ module.exports = { attempts: 5, duration: 60 }, + // per-recipient mail rate limit, prevents a bug or runaway webhook from spamming a user's mailbox + // points = max number of emails per recipient address within the rolling window of `duration` seconds + mailsRateLimit: { + points: 500, + duration: 86400 + }, // Example of full oauth provider configuration // oauth: {providers: [{ // id: 'github', diff --git a/api/config/test.cjs b/api/config/test.cjs index 5d75f9c7..4111655e 100644 --- a/api/config/test.cjs +++ b/api/config/test.cjs @@ -71,6 +71,11 @@ module.exports = { attempts: 100, duration: 60 }, + mailsRateLimit: { + // high enough that no existing test trips it; rate-limit test pre-fills the bucket directly + points: 100, + duration: 5 + }, anonymousAction: { // short notBefore keeps tests fast — in prod this is 8s to trip bots expiresIn: '1d', diff --git a/api/config/type/schema.json b/api/config/type/schema.json index a07d459e..d51fe86b 100644 --- a/api/config/type/schema.json +++ b/api/config/type/schema.json @@ -40,6 +40,7 @@ "saml2", "webhooks", "authRateLimit", + "mailsRateLimit", "plannedDeletionDelay", "cleanup", "avatars", @@ -482,6 +483,21 @@ } } }, + "mailsRateLimit": { + "type": "object", + "required": [ + "points", + "duration" + ], + "properties": { + "points": { + "type": "integer" + }, + "duration": { + "type": "integer" + } + } + }, "plannedDeletionDelay": { "type": "integer" }, diff --git a/api/src/mails/service.ts b/api/src/mails/service.ts index 4d50a041..5481a5f2 100644 --- a/api/src/mails/service.ts +++ b/api/src/mails/service.ts @@ -7,6 +7,8 @@ import { flatten } from 'flat' import EventEmitter from 'node:events' import mailsTransport from './transport.ts' import { getSiteByUrl, getSiteByHost } from '#services' +import { internalError } from '@data-fair/lib-node/observer.js' +import { mailLimiter } from '../utils/limiter.ts' export const events = new EventEmitter() @@ -86,6 +88,22 @@ export const sendMailI18n = async (key: string, messages: any, to: string, param } export const sendMail = async (to: string, params: SendMailParams, attachments?: { filename: string, path: string }[]) => { + // per-recipient rate limiting — protects users from a bug or runaway webhook spamming their mailbox + const recipients = to.split(',').map(r => r.trim().toLowerCase()).filter(r => r.length > 0) + const allowed: string[] = [] + for (const recipient of recipients) { + if (await mailLimiter()(recipient)) { + allowed.push(recipient) + } else { + internalError('mail-rate-limited', `recipient ${recipient} exceeded mail rate limit, dropping (subject: ${params.subject})`) + } + } + if (allowed.length === 0) { + internalError('mail-rate-limited', `all recipients dropped, mail not sent (subject: ${params.subject}, original to: ${to})`) + return + } + to = allowed.join(', ') + let site = params.link ? (await getSiteByUrl(params.link)) : undefined if (!site && params.host) { site = await getSiteByHost(params.host, params.path ?? '') diff --git a/api/src/test-env.ts b/api/src/test-env.ts index 8bf84255..1180c0bd 100644 --- a/api/src/test-env.ts +++ b/api/src/test-env.ts @@ -134,7 +134,9 @@ router.post('/clear-site-cache', async (req, res) => { router.get('/config', (req, res) => { res.json({ publicUrl: config.publicUrl, - secretKeys: config.secretKeys + secretKeys: config.secretKeys, + mongo: { url: config.mongo.url }, + mailsRateLimit: config.mailsRateLimit }) }) diff --git a/api/src/utils/limiter.ts b/api/src/utils/limiter.ts index 5f97f609..c06d58c1 100644 --- a/api/src/utils/limiter.ts +++ b/api/src/utils/limiter.ts @@ -4,18 +4,15 @@ import mongo from '#mongo' // protect authentication routes with rate limiting to prevent brute force attacks let _limiter: (key: string) => Promise | undefined -const limiterOptions = { - keyPrefix: 'sd-rate-limiter-auth', - points: config.authRateLimit.attempts, - duration: config.authRateLimit.duration -} export default () => { if (!_limiter) { const rateLimiterMongo = new RateLimiterMongo({ storeClient: mongo.client, dbName: mongo.db.databaseName, - ...limiterOptions + keyPrefix: 'sd-rate-limiter-auth', + points: config.authRateLimit.attempts, + duration: config.authRateLimit.duration }) _limiter = async (key: string) => { try { @@ -29,3 +26,28 @@ export default () => { } return _limiter } + +// per-recipient mail rate limit, prevents a bug or runaway webhook from spamming a user's mailbox +let _mailLimiter: (key: string) => Promise | undefined + +export const mailLimiter = () => { + if (!_mailLimiter) { + const rateLimiterMongo = new RateLimiterMongo({ + storeClient: mongo.client, + dbName: mongo.db.databaseName, + keyPrefix: 'sd-rate-limiter-mail', + points: config.mailsRateLimit.points, + duration: config.mailsRateLimit.duration + }) + _mailLimiter = async (key: string) => { + try { + await rateLimiterMongo.consume(key, 1) + } catch (err) { + if (err instanceof RateLimiterRes) return false + throw err + } + return true + } + } + return _mailLimiter +} diff --git a/tests/features/mails-rate-limit.api.spec.ts b/tests/features/mails-rate-limit.api.spec.ts new file mode 100644 index 00000000..d72cd2b5 --- /dev/null +++ b/tests/features/mails-rate-limit.api.spec.ts @@ -0,0 +1,99 @@ +import { strict as assert } from 'node:assert' +import { test } from '@playwright/test' +import { axios, testEnvAx, maildevAx, deleteAllEmails, getServerConfig } from '../support/axios.ts' +import { MongoClient } from 'mongodb' +import { RateLimiterMongo } from 'rate-limiter-flexible' + +// A unique recipient per test process avoids cross-test interference on the same bucket +const uniqueEmail = (label: string) => `rate-limit-${label}-${Date.now()}-${Math.floor(Math.random() * 1e6)}@test.com` + +let mongoClient: MongoClient +let rateLimiter: RateLimiterMongo + +test.describe('mails rate limiting', () => { + test.beforeAll(async () => { + const cfg = await getServerConfig() + mongoClient = await MongoClient.connect(cfg.mongo.url) + rateLimiter = new RateLimiterMongo({ + storeClient: mongoClient, + dbName: mongoClient.db().databaseName, + keyPrefix: 'sd-rate-limiter-mail', + points: cfg.mailsRateLimit.points, + duration: cfg.mailsRateLimit.duration + }) + // give the limiter time to ensure its underlying collection exists + await rateLimiter.get('warmup') + }) + + test.afterAll(async () => { + if (mongoClient) await mongoClient.close() + }) + + test.beforeEach(async () => { + await deleteAllEmails() + await testEnvAx.delete('/') + await testEnvAx.post('/seed') + }) + + test('Delivers email to a fresh recipient (sanity)', async () => { + const ax = await axios() + const to = uniqueEmail('fresh') + const res = await ax.post('/api/mails', { + to: [to], + subject: 'rate-limit-sanity', + text: 'hello' + }, { params: { key: 'testkey' } }) + assert.equal(res.status, 200) + await new Promise(resolve => setTimeout(resolve, 100)) + const emails: any[] = (await maildevAx.get('/email')).data + const email = emails.find((m: any) => m.subject === 'rate-limit-sanity') + assert.ok(email, 'expected a delivered email for fresh recipient') + assert.ok(email.to.some((t: any) => t.address === to), 'expected the fresh recipient to be in the to list') + }) + + test('Drops email to a recipient whose bucket is already full', async () => { + const ax = await axios() + const to = uniqueEmail('victim') + + const cfg = await getServerConfig() + // pre-fill the rate-limit bucket up to the limit so the next consume() rejects + await rateLimiter.set(to.toLowerCase(), cfg.mailsRateLimit.points, cfg.mailsRateLimit.duration * 1000) + + const res = await ax.post('/api/mails', { + to: [to], + subject: 'rate-limit-victim', + text: 'should be dropped' + }, { params: { key: 'testkey' } }) + assert.equal(res.status, 200) + + await new Promise(resolve => setTimeout(resolve, 100)) + const emails: any[] = (await maildevAx.get('/email')).data + const email = emails.find((m: any) => m.subject === 'rate-limit-victim') + assert.ok(!email, 'expected no email delivered to a rate-limited recipient') + }) + + test('Multi-recipient send filters out only the rate-limited address', async () => { + const ax = await axios() + const victim = uniqueEmail('multi-victim') + const fresh = uniqueEmail('multi-fresh') + + const cfg = await getServerConfig() + await rateLimiter.set(victim.toLowerCase(), cfg.mailsRateLimit.points, cfg.mailsRateLimit.duration * 1000) + + // a single `to` string with a comma exercises sendMail's multi-recipient split path + const res = await ax.post('/api/mails', { + to: [`${victim}, ${fresh}`], + subject: 'rate-limit-mixed', + text: 'partial delivery' + }, { params: { key: 'testkey' } }) + assert.equal(res.status, 200) + + await new Promise(resolve => setTimeout(resolve, 100)) + const emails: any[] = (await maildevAx.get('/email')).data + const email = emails.find((m: any) => m.subject === 'rate-limit-mixed') + assert.ok(email, 'expected the email to be delivered with the non-limited recipient') + const addresses = email.to.map((t: any) => t.address) + assert.ok(addresses.includes(fresh), 'expected the fresh recipient to receive the email') + assert.ok(!addresses.includes(victim), 'expected the rate-limited recipient to be filtered out') + }) +})