diff --git a/src/actions.ts b/src/actions.ts index dec2608..a604033 100644 --- a/src/actions.ts +++ b/src/actions.ts @@ -19,6 +19,11 @@ export type RegisterSubscriptionParams = { timezone?: 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, hour, minute, dayOfWeek, timezone }: RegisterSubscriptionParams) => { @@ -26,8 +31,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. diff --git a/src/alert.ts b/src/alert.ts index 542086b..2f3dadf 100644 --- a/src/alert.ts +++ b/src/alert.ts @@ -12,6 +12,7 @@ export type Subscription = { confirmed_at?: number unsubscribed_at?: number last_digest_at?: number + last_confirm_sent_at?: number } export const getSubscriptionError = (sub: Subscription) => { diff --git a/src/database.ts b/src/database.ts index 42e63ee..e7053c7 100644 --- a/src/database.ts +++ b/src/database.ts @@ -65,7 +65,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 ) ` ) @@ -81,6 +82,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)` ) @@ -171,9 +180,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 = ?, hour = ?, minute = ?, day_of_week = ?, timezone = ?, confirmed_at = NULL, unsubscribed_at = NULL + `UPDATE subscriptions SET email = ?, frequency = ?, hour = ?, minute = ?, day_of_week = ?, timezone = ?, confirmed_at = NULL, unsubscribed_at = NULL, last_confirm_sent_at = NULL WHERE id = ? RETURNING *`, [email, frequency, hr, mn, dow, tz, existing.id] ) @@ -181,6 +192,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( @@ -194,7 +213,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] ) diff --git a/test/confirm-email-cooldown.test.ts b/test/confirm-email-cooldown.test.ts new file mode 100644 index 0000000..731418d --- /dev/null +++ b/test/confirm-email-cooldown.test.ts @@ -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) + }) +}) \ No newline at end of file