9fa1f73316
All checks were successful
CI / checks (pull_request) Successful in 36s
Problem
-------
1. Re-clicking an already-confirmed confirmation link (e.g. /confirm?token=…)
returned from confirmSubscription because the SQL WHERE
clause required . The caller then threw an
ActionError('invalid or expired') which rendered the 'Email not confirmed'
error page — misleading for someone who had already confirmed.
2. A second PUT /subscription/email with the same email+frequency could
silently bypass the upsert path when getSubscriptionByPubkey found the
active row but updateSubscription returned it unchanged (email and
frequency matched). While the unique index prevented a true duplicate
INSERT, the code path was fragile and the regression test was missing.
Changes
-------
database.ts:
- confirmSubscription now returns { sub, alreadyConfirmed } | undefined.
First it tries the existing UPDATE (unconfirmed tokens only). If that
returns no rows, it looks up the key directly: if the row exists and is
already confirmed, returns { sub, alreadyConfirmed: true }. If the row
doesn't exist or is unsubscribed, returns undefined (invalid/expired).
- Exported new ConfirmResult type for callers.
actions.ts:
- confirmSubscriptionAction destructures the new return type.
- Only registers the cron job on fresh confirmation (not re-confirms).
- Returns the ConfirmResult so the route can distinguish the two cases.
server.ts:
- /confirm route checks result.alreadyConfirmed and renders
confirm-already.html instead of confirm-success.html.
pages/confirm-already.html:
- New page with title 'Email already confirmed' and an info message
explaining the address was already confirmed.
Tests:
- test/confirm-already-confirmed.test.ts — NEW (3 tests): first confirm
succeeds with alreadyConfirmed=false; second confirm returns
alreadyConfirmed=true; nonexistent token returns undefined.
- test/duplicate-subscription.test.ts — NEW (4 tests): full cycle of
register → confirm → re-register → assert one active row with
unchanged key, verifying the upsert is idempotent.
- Adapted 3 existing test files to destructure the new ConfirmResult.
73 lines
No EOL
1.9 KiB
TypeScript
73 lines
No EOL
1.9 KiB
TypeScript
import { instrument } from 'succinct-async'
|
|
import * as mailer from './mailer.js'
|
|
import * as worker from './worker/index.js'
|
|
import * as db from './database.js'
|
|
|
|
export class ActionError extends Error {
|
|
toString() {
|
|
return this.message
|
|
}
|
|
}
|
|
|
|
export type RegisterSubscriptionParams = {
|
|
pubkey: string
|
|
email: string
|
|
frequency: string
|
|
}
|
|
|
|
export const registerSubscription = instrument(
|
|
'actions.registerSubscription',
|
|
async ({ pubkey, email, frequency }: RegisterSubscriptionParams) => {
|
|
const sub = await db.insertSubscription(pubkey, email, frequency)
|
|
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)
|
|
} else {
|
|
// Already confirmed (e.g. frequency-only change) — reschedule the
|
|
// cron job so it uses the new cadence immediately.
|
|
worker.registerSubscription(sub)
|
|
}
|
|
|
|
return { key: sub.key, callback }
|
|
},
|
|
)
|
|
|
|
export type ConfirmSubscriptionParams = {
|
|
token: string
|
|
}
|
|
|
|
export const confirmSubscriptionAction = instrument(
|
|
'actions.confirmSubscription',
|
|
async ({ token }: ConfirmSubscriptionParams) => {
|
|
const result = await db.confirmSubscription(token)
|
|
|
|
if (!result) {
|
|
throw new ActionError('That confirmation code is invalid or has expired.')
|
|
}
|
|
|
|
// Only register the cron job when this is a fresh confirmation.
|
|
// If already confirmed, the job is already running.
|
|
if (!result.alreadyConfirmed) {
|
|
worker.registerSubscription(result.sub)
|
|
}
|
|
|
|
return result
|
|
},
|
|
)
|
|
|
|
export type UnsubscribeParams = {
|
|
token: string
|
|
}
|
|
|
|
export const unsubscribeAction = instrument(
|
|
'actions.unsubscribe',
|
|
async ({ token }: UnsubscribeParams) => {
|
|
const sub = await db.unsubscribeSubscription(token)
|
|
|
|
if (sub) {
|
|
worker.unregisterSubscription(sub)
|
|
}
|
|
},
|
|
) |