Rate-limit confirmation emails and restore daily/weekly digest cron
- Add last_confirm_sent_at to subscriptions, with a migration for existing databases. registerSubscription now sends at most one confirmation email per 10 minutes per subscription, so a page-refresh or client retry storm can't spam an inbox. The cooldown resets when the address changes or a tombstoned row is reactivated (fresh key), so genuinely new addresses always get an email immediately. - Restore getCronExpression to the documented daily (17:00 UTC) and weekly (Monday 17:00 UTC) schedules with the 6-field cron syntax the cron package expects, fixing the pre-existing reschedule tests that the 'run every minute' testing hack had broken. - Add confirm-email-cooldown.test.ts covering first send, refresh storms, frequency changes, email changes, reactivation, and the 10-minute constant.
This commit is contained in:
parent
81f5602ce4
commit
051c1e88fb
4 changed files with 151 additions and 10 deletions
|
|
@ -15,6 +15,11 @@ export type RegisterSubscriptionParams = {
|
|||
frequency: string
|
||||
}
|
||||
|
||||
// Floor on how often a confirmation email can be sent for the same subscription.
|
||||
// The client is expected to only PUT on an explicit save (GET-first on boot),
|
||||
// but a retry loop or refresh storm shouldn't be able to spam an inbox either.
|
||||
export const CONFIRM_EMAIL_COOLDOWN_SECONDS = 10 * 60
|
||||
|
||||
export const registerSubscription = instrument(
|
||||
'actions.registerSubscription',
|
||||
async ({ pubkey, email, frequency }: RegisterSubscriptionParams) => {
|
||||
|
|
@ -22,8 +27,18 @@ export const registerSubscription = instrument(
|
|||
const callback = `${process.env.BASE_URL}/notify/${sub.id}`
|
||||
|
||||
if (!sub.confirmed_at) {
|
||||
// New or email-changed subscription — send a confirmation email.
|
||||
await mailer.sendConfirm(sub)
|
||||
// New or email-changed subscription — send a confirmation email, but never
|
||||
// more than once per cooldown window. The email-change path resets
|
||||
// last_confirm_sent_at, so a genuinely new address always gets an email
|
||||
// right away; this only throttles repeated sends of the same address.
|
||||
const lastSent = sub.last_confirm_sent_at
|
||||
const cooldownElapsed =
|
||||
!lastSent || Math.floor(Date.now() / 1000) - lastSent >= CONFIRM_EMAIL_COOLDOWN_SECONDS
|
||||
|
||||
if (cooldownElapsed) {
|
||||
await mailer.sendConfirm(sub)
|
||||
await db.markConfirmSent(sub.id)
|
||||
}
|
||||
} else {
|
||||
// Already confirmed (e.g. frequency-only change) — reschedule the
|
||||
// cron job so it uses the new cadence immediately.
|
||||
|
|
|
|||
14
src/alert.ts
14
src/alert.ts
|
|
@ -8,6 +8,7 @@ export type Subscription = {
|
|||
confirmed_at?: number
|
||||
unsubscribed_at?: number
|
||||
last_digest_at?: number
|
||||
last_confirm_sent_at?: number
|
||||
}
|
||||
|
||||
export const getSubscriptionError = (sub: Subscription) => {
|
||||
|
|
@ -18,11 +19,14 @@ export const getSubscriptionError = (sub: Subscription) => {
|
|||
if (!['daily', 'weekly'].includes(sub.frequency)) {
|
||||
return 'Frequency must be "daily" or "weekly"'
|
||||
}
|
||||
|
||||
// Daily: fire at 17:00 UTC. Weekly: fire Monday at 17:00 UTC.
|
||||
// Validation: just ensure frequency is valid, cron is generated internally.
|
||||
}
|
||||
|
||||
export const getCronExpression = (frequency: string, hour = 17, minute = 0) => {
|
||||
return `* * * * *`
|
||||
}
|
||||
// Daily: fire at 17:00 UTC. Weekly: fire Monday (day-of-week 1) at 17:00 UTC.
|
||||
// The `cron` package uses 6-field syntax (leading field is seconds).
|
||||
if (frequency === 'weekly') {
|
||||
return `0 ${minute} ${hour} * * 1`
|
||||
}
|
||||
|
||||
return `0 ${minute} ${hour} * * *`
|
||||
}
|
||||
|
|
|
|||
|
|
@ -61,7 +61,8 @@ export const migrate = () =>
|
|||
created_at INTEGER NOT NULL,
|
||||
confirmed_at INTEGER,
|
||||
unsubscribed_at INTEGER,
|
||||
last_digest_at INTEGER
|
||||
last_digest_at INTEGER,
|
||||
last_confirm_sent_at INTEGER
|
||||
)
|
||||
`
|
||||
)
|
||||
|
|
@ -77,6 +78,14 @@ export const migrate = () =>
|
|||
)
|
||||
`
|
||||
)
|
||||
// Add last_confirm_sent_at for existing databases created before the
|
||||
// confirmation-email cooldown migration.
|
||||
const columns = await all(
|
||||
`SELECT name FROM pragma_table_info('subscriptions')`
|
||||
)
|
||||
if (!columns.some((c: any) => c.name === 'last_confirm_sent_at')) {
|
||||
await run(`ALTER TABLE subscriptions ADD COLUMN last_confirm_sent_at INTEGER`)
|
||||
}
|
||||
await run(
|
||||
`CREATE INDEX IF NOT EXISTS idx_events_subscription_received ON events (subscription_id, received_at)`
|
||||
)
|
||||
|
|
@ -142,9 +151,11 @@ export const updateSubscription = instrument(
|
|||
)
|
||||
}
|
||||
|
||||
// Address changed: it's a new inbox to verify, so reset the confirm-email
|
||||
// cooldown or the new address would inherit the old one's lockout.
|
||||
return parseSubscription(
|
||||
await get(
|
||||
`UPDATE subscriptions SET email = ?, frequency = ?, confirmed_at = NULL, unsubscribed_at = NULL
|
||||
`UPDATE subscriptions SET email = ?, frequency = ?, confirmed_at = NULL, unsubscribed_at = NULL, last_confirm_sent_at = NULL
|
||||
WHERE id = ? RETURNING *`,
|
||||
[email, frequency, existing.id]
|
||||
)
|
||||
|
|
@ -152,6 +163,14 @@ export const updateSubscription = instrument(
|
|||
}
|
||||
)
|
||||
|
||||
// Record that a confirmation email was sent, for the send-cooldown.
|
||||
export const markConfirmSent = instrument(
|
||||
'database.markConfirmSent',
|
||||
async (id: string) => {
|
||||
await run(`UPDATE subscriptions SET last_confirm_sent_at = ? WHERE id = ?`, [now(), id])
|
||||
}
|
||||
)
|
||||
|
||||
const getMostRecentTombstonedSubscription = async (pubkey: string, email: string) =>
|
||||
parseSubscription(
|
||||
await get(
|
||||
|
|
@ -165,7 +184,9 @@ const getMostRecentTombstonedSubscription = async (pubkey: string, email: string
|
|||
const reactivateSubscription = async (tombstoned: Subscription, frequency: string) =>
|
||||
parseSubscription(
|
||||
await get(
|
||||
`UPDATE subscriptions SET key = ?, frequency = ?, unsubscribed_at = NULL
|
||||
// A fresh key invalidates any previously emailed confirm link, so a new
|
||||
// confirmation email must be allowed immediately (reset the cooldown).
|
||||
`UPDATE subscriptions SET key = ?, frequency = ?, unsubscribed_at = NULL, last_confirm_sent_at = NULL
|
||||
WHERE id = ? RETURNING *`,
|
||||
[crypto.randomBytes(32).toString('hex'), frequency, tombstoned.id]
|
||||
)
|
||||
|
|
|
|||
101
test/confirm-email-cooldown.test.ts
Normal file
101
test/confirm-email-cooldown.test.ts
Normal file
|
|
@ -0,0 +1,101 @@
|
|||
import { describe, it, expect, beforeAll, vi, afterEach, beforeEach } from 'vitest'
|
||||
import * as db from '../src/database.js'
|
||||
import { registerSubscription, CONFIRM_EMAIL_COOLDOWN_SECONDS } from '../src/actions.js'
|
||||
import * as mailer from '../src/mailer.js'
|
||||
|
||||
// Guard: at most one confirmation email per subscription per cooldown window,
|
||||
// no matter how many PUTs arrive (e.g. a page-refresh storm while unconfirmed).
|
||||
|
||||
vi.mock('../src/mailer.js', async (importOriginal) => {
|
||||
const actual: any = await importOriginal()
|
||||
return { ...actual, sendConfirm: vi.fn(async () => {}) }
|
||||
})
|
||||
|
||||
const unique = (label: string) => `${label}-${Date.now()}-${Math.random().toString(36).slice(2)}`
|
||||
const pubkey = unique('cooldown')
|
||||
const email = `${unique('cooldown')}@example.com`
|
||||
|
||||
const confirmCalls = () => vi.mocked(mailer.sendConfirm).mock.calls.length
|
||||
|
||||
describe('Confirmation email cooldown', () => {
|
||||
beforeAll(async () => {
|
||||
await db.migrate()
|
||||
})
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(mailer.sendConfirm).mockClear()
|
||||
})
|
||||
|
||||
it('sends a confirmation email on the first register', async () => {
|
||||
const res = await registerSubscription({ pubkey, email, frequency: 'daily' })
|
||||
expect(res.key).toBeTruthy()
|
||||
expect(confirmCalls()).toBe(1)
|
||||
})
|
||||
|
||||
it('does NOT re-send a confirmation email on re-register (refresh/retry storm)', async () => {
|
||||
// Simulate several PUTs arriving in quick succession (e.g. page reloads).
|
||||
for (let i = 0; i < 5; i++) {
|
||||
await registerSubscription({ pubkey, email, frequency: 'daily' })
|
||||
}
|
||||
expect(confirmCalls()).toBe(0)
|
||||
})
|
||||
|
||||
it('a new frequency (setting change) does not re-send', async () => {
|
||||
await registerSubscription({ pubkey, email, frequency: 'weekly' })
|
||||
expect(confirmCalls()).toBe(0)
|
||||
})
|
||||
|
||||
it('changing the email address sends immediately (cooldown reset)', async () => {
|
||||
const newEmail = `${unique('cooldown')}@example.com`
|
||||
await registerSubscription({ pubkey, email: newEmail, frequency: 'daily' })
|
||||
expect(confirmCalls()).toBe(1)
|
||||
})
|
||||
|
||||
it('a fresh unconfirmed subscription is still throttled after confirm-sent marker', async () => {
|
||||
// New pubkey: first PUT sends one email; immediate re-PUT is suppressed.
|
||||
const pk2 = unique('cooldown2')
|
||||
const em2 = `${unique('cooldown2')}@example.com`
|
||||
await registerSubscription({ pubkey: pk2, email: em2, frequency: 'daily' })
|
||||
await registerSubscription({ pubkey: pk2, email: em2, frequency: 'daily' })
|
||||
expect(confirmCalls()).toBe(1)
|
||||
})
|
||||
|
||||
it('cooldown window constant is 10 minutes', () => {
|
||||
expect(CONFIRM_EMAIL_COOLDOWN_SECONDS).toBe(600)
|
||||
})
|
||||
})
|
||||
|
||||
describe('Cooldown reset on subscription reactivation (same email, off/on cycle)', () => {
|
||||
const k = unique('cooldown-reactivate')
|
||||
const e = `${unique('cooldown-reactivate')}@example.com`
|
||||
|
||||
beforeAll(async () => {
|
||||
await db.migrate()
|
||||
})
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(mailer.sendConfirm).mockClear()
|
||||
})
|
||||
|
||||
it('register, confirm, unsubscribe, re-register same email → fresh key emails immediately', async () => {
|
||||
const r1 = await registerSubscription({ pubkey: k, email: e, frequency: 'daily' })
|
||||
const active = await db.getSubscriptionByPubkey(k)
|
||||
|
||||
// Confirm first so reactivation is a "keep confirmed" path — but that also
|
||||
// means no confirm email on re-register (already confirmed). Verify the row
|
||||
// is reactivated with a fresh key, not a duplicate.
|
||||
const confirmed = await db.confirmSubscription(active!.key)
|
||||
expect(confirmed).toBeTruthy()
|
||||
|
||||
await db.unsubscribeSubscription(active!.key)
|
||||
|
||||
vi.mocked(mailer.sendConfirm).mockClear()
|
||||
const r2 = await registerSubscription({ pubkey: k, email: e, frequency: 'daily' })
|
||||
|
||||
expect(r2.key).not.toBe(r1.key) // fresh key issued
|
||||
expect(confirmCalls()).toBe(0) // still confirmed → no new email
|
||||
|
||||
const rows = await db.getAllSubscriptionsByPubkey(k)
|
||||
expect(rows).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
Loading…
Reference in a new issue