From 51230e51514861e238b509775c399e8193aecaa6 Mon Sep 17 00:00:00 2001 From: Jennie Robinson Faber Date: Sat, 25 Apr 2026 19:34:16 +0100 Subject: [PATCH] refactor(launch): simplify launch-readiness fixes Follow-up to 208638e. Code review surfaced a few real issues; this commit addresses them. - login.post.js now uses the new sendMagicLink util instead of duplicating the jti/jwt/Resend/logActivity logic. Reduces 60 lines. - sendMagicLink accepts an optional pre-loaded Member doc, skipping the redundant findOne when the caller already has one. customer.post.js passes the just-created/upgraded member, dropping signup from 3 Mongo round-trips to 1 (lookup is gone; jti burn remains). - sendMagicLink now lowercases the email defensively so callers don't have to remember. - rateLimit.js: replaced an effectively-dead eviction line with a probabilistic sweep (~1% of calls scan and evict keys whose newest entry has aged out). Caps unbounded Map growth under random-key spraying. - reconcile-payments.post.js: 401/403/404 from Helcim now bails out immediately instead of burning all 3 retry attempts; dry-run summary filters via the same RECONCILABLE_STATUSES set as apply mode so counts match. - Deleted WHAT-comments and section banners per CLAUDE.md no-comment rule. Kept genuine WHY-comments (validateBeforeSave rationale, amount-IGNORED-for-tickets, sendConfirmation deliberately-omitted). Tests: 758/760 passing (unchanged). --- app/composables/useHelcimPay.js | 3 - server/api/auth/login.post.js | 60 +------------------ server/api/helcim/customer.post.js | 15 +---- server/api/helcim/initialize-payment.post.js | 4 -- .../api/internal/reconcile-payments.post.js | 30 ++++------ server/utils/magicLink.js | 4 +- server/utils/rateLimit.js | 15 ++++- 7 files changed, 33 insertions(+), 98 deletions(-) diff --git a/app/composables/useHelcimPay.js b/app/composables/useHelcimPay.js index e696d10..775e86d 100644 --- a/app/composables/useHelcimPay.js +++ b/app/composables/useHelcimPay.js @@ -12,9 +12,6 @@ export const useHelcimPay = () => { customerId, customerCode, amount, - // Marks this as a paid-tier signup so the server accepts the - // payment-bridge cookie set by /api/helcim/customer (the user - // hasn't clicked their email-verify magic link yet). metadata: { type: "membership_signup" }, }, }); diff --git a/server/api/auth/login.post.js b/server/api/auth/login.post.js index d710d28..5978ec7 100644 --- a/server/api/auth/login.post.js +++ b/server/api/auth/login.post.js @@ -1,74 +1,18 @@ // server/api/auth/login.post.js -import jwt from "jsonwebtoken"; -import { randomUUID } from "crypto"; -import { Resend } from "resend"; -import Member from "../../models/member.js"; import { connectDB } from "../../utils/mongoose.js"; import { validateBody } from "../../utils/validateBody.js"; import { emailSchema } from "../../utils/schemas.js"; - -const resend = new Resend(process.env.RESEND_API_KEY); +import { sendMagicLink } from "../../utils/magicLink.js"; export default defineEventHandler(async (event) => { await connectDB(); - const baseUrl = process.env.BASE_URL; - if (!baseUrl) { - throw createError({ - statusCode: 500, - statusMessage: "BASE_URL environment variable is not set", - }); - } - const { email } = await validateBody(event, emailSchema); const GENERIC_MESSAGE = "If this email is registered, we've sent a login link."; - const member = await Member.findOne({ email }); - - if (!member) { - return { - success: true, - message: GENERIC_MESSAGE, - }; - } - - const config = useRuntimeConfig(event); - const jti = randomUUID(); - - const token = jwt.sign( - { memberId: member._id, jti }, - config.jwtSecret, - { expiresIn: "15m" }, - ); - - // Store jti so we can burn it on first use - await Member.findByIdAndUpdate( - member._id, - { $set: { magicLinkJti: jti, magicLinkJtiUsed: false } }, - { runValidators: false } - ); - - // Token goes in the fragment — never sent to server, never logged - const magicLink = `${baseUrl}/verify#${token}`; - - const emailSubject = "Your Ghost Guild login link"; - const emailBody = `Hi,\n\nSign in to Ghost Guild:\n${magicLink}\n\nThis link expires in 15 minutes. If you didn't request it, ignore this email.`; - try { - await resend.emails.send({ - from: "Ghost Guild ", - to: email, - subject: emailSubject, - text: emailBody, - }); - - logActivity(member._id, 'email_sent', { - emailType: 'magic_link', - subject: emailSubject, - body: emailBody - }) - + await sendMagicLink(email); return { success: true, message: GENERIC_MESSAGE, diff --git a/server/api/helcim/customer.post.js b/server/api/helcim/customer.post.js index f41abb6..b267ebd 100644 --- a/server/api/helcim/customer.post.js +++ b/server/api/helcim/customer.post.js @@ -1,8 +1,3 @@ -// Public signup endpoint. Creates a Helcim customer + Member record and -// dispatches a magic link for email verification. The full session cookie -// is set when the user clicks the magic link (see /api/auth/verify); paid-tier -// signups also receive a short-lived payment-bridge cookie so they can complete -// Helcim checkout in the same tab without verifying email first. import { getRequestHeader, getRequestIP } from 'h3' import Member from '../../models/member.js' import { connectDB } from '../../utils/mongoose.js' @@ -13,14 +8,12 @@ import { rateLimit } from '../../utils/rateLimit.js' export default defineEventHandler(async (event) => { try { - // --- Origin check (CSRF defense in depth on top of SameSite=Lax) --- const origin = getRequestHeader(event, 'origin') const allowed = process.env.BASE_URL if (!origin || (allowed && origin !== allowed)) { throw createError({ statusCode: 403, statusMessage: 'Invalid origin' }) } - // --- Per-IP rate limit (5 / hour) --- const ip = getRequestIP(event, { xForwardedFor: true }) || 'unknown' if (!rateLimit(`signup:ip:${ip}`, { max: 5, windowMs: 3600_000 })) { throw createError({ statusCode: 429, statusMessage: 'Too many signup attempts' }) @@ -29,7 +22,6 @@ export default defineEventHandler(async (event) => { await connectDB() const body = await validateBody(event, helcimCustomerSchema) - // --- Per-email rate limit (3 / hour) --- if (!rateLimit(`signup:email:${body.email}`, { max: 3, windowMs: 3600_000 })) { throw createError({ statusCode: 429, @@ -88,13 +80,10 @@ export default defineEventHandler(async (event) => { }) } - // Issue a magic link instead of an immediate session — the auth-token - // cookie is set when the user clicks through, proving email ownership. - // Use the normalized email so guest upgrades (which may not project - // the email field back) still get a magic link. await sendMagicLink(normalizedEmail, { subject: 'Verify your Ghost Guild signup', - intro: 'Verify your email to finish your Ghost Guild signup:' + intro: 'Verify your email to finish your Ghost Guild signup:', + member }) // Paid-tier signups need to complete Helcim checkout in the same tab diff --git a/server/api/helcim/initialize-payment.post.js b/server/api/helcim/initialize-payment.post.js index 33adbec..71996b1 100644 --- a/server/api/helcim/initialize-payment.post.js +++ b/server/api/helcim/initialize-payment.post.js @@ -1,4 +1,3 @@ -// Initialize HelcimPay.js session import Member from '../../models/member.js' import Series from '../../models/series.js' import { loadPublicEvent } from '../../utils/loadEvent.js' @@ -14,9 +13,6 @@ export default defineEventHandler(async (event) => { const isEventTicket = metaType === 'event_ticket' const isSeriesTicket = metaType === 'series_ticket' const isTicket = isEventTicket || isSeriesTicket - // Membership signup uses a short-lived payment-bridge cookie (set by - // /api/helcim/customer) so the user can complete checkout before clicking - // their email-verification magic link. const isMembershipSignup = metaType === 'membership_signup' if (!isTicket) { diff --git a/server/api/internal/reconcile-payments.post.js b/server/api/internal/reconcile-payments.post.js index 6502ead..762ba40 100644 --- a/server/api/internal/reconcile-payments.post.js +++ b/server/api/internal/reconcile-payments.post.js @@ -1,30 +1,19 @@ /** - * Reconciliation cron route — invoked by `netlify/functions/reconcile-payments.mjs` - * on a daily schedule. Mirrors the loop in `scripts/reconcile-helcim-payments.mjs` - * but lives inside Nitro so it can use auto-imported utils + the runtime config. + * Reconciliation cron route — invoked by `netlify/functions/reconcile-payments.mjs`. * * Auth: shared-secret header `X-Reconcile-Token` matched against * `runtimeConfig.reconcileToken` (env: NUXT_RECONCILE_TOKEN). Machine-to-machine - * only — no user session involved. - * - * Behavior: - * - For every Member with a helcimCustomerId, list Helcim transactions and - * upsert Payment docs (idempotent via `helcimTransactionId` unique index). - * - Transient Helcim API errors are retried up to 3 times with exponential - * backoff (250ms / 500ms / 1000ms). On final failure the member is counted - * as `memberErrors` and the loop continues. - * - Never passes `sendConfirmation: true` — the cron back-fills history and - * must not re-send confirmation emails. - * - `?apply=false` switches to dry-run: counts what WOULD be created via - * Payment.findOne, no writes. - * - * Returns a JSON summary; logs `[reconcile] done ` to stdout. + * only — never passes `sendConfirmation: true` so back-fills don't re-send + * confirmation emails. */ import Member from '../../models/member.js' import Payment from '../../models/payment.js' import { listHelcimCustomerTransactions } from '../../utils/helcim.js' import { upsertPaymentFromHelcim } from '../../utils/payments.js' +// Same filter upsertPaymentFromHelcim applies — keep dry-run summary in sync. +const RECONCILABLE_STATUSES = new Set(['paid', 'refunded', 'failed']) + const RETRY_ATTEMPTS = 3 const BASE_DELAY_MS = 250 @@ -38,7 +27,12 @@ async function listTransactionsWithRetry(customerCode) { try { return await listHelcimCustomerTransactions(customerCode) } catch (err) { + // Permanent failures (auth, missing) — don't burn retries on broken config. + if (err?.statusCode === 401 || err?.statusCode === 403 || err?.statusCode === 404) { + throw err + } lastErr = err + // Backoff after attempts 1 and 2: 250ms, then 500ms (no sleep after attempt 3). if (attempt < RETRY_ATTEMPTS) { await sleep(BASE_DELAY_MS * 2 ** (attempt - 1)) } @@ -80,7 +74,7 @@ export default defineEventHandler(async (event) => { for (const tx of txs) { txExamined++ - if (tx.status === 'other') { + if (!RECONCILABLE_STATUSES.has(tx?.status)) { skipped++ continue } diff --git a/server/utils/magicLink.js b/server/utils/magicLink.js index fc617b4..9503539 100644 --- a/server/utils/magicLink.js +++ b/server/utils/magicLink.js @@ -15,6 +15,7 @@ const resend = new Resend(process.env.RESEND_API_KEY) * @param {object} [options] * @param {string} [options.subject] - Email subject (default: "Your Ghost Guild login link") * @param {string} [options.intro] - Optional one-line intro before the link. + * @param {object} [options.member] - Pre-loaded Member doc; skips the findOne lookup. * @returns {Promise<{ sent: boolean }>} - sent=false when no member exists for the email * (caller can decide whether to surface that; the auth/login endpoint hides it for * anti-enumeration, signup knows the member was just created). @@ -28,7 +29,8 @@ export async function sendMagicLink(email, options = {}) { }) } - const member = await Member.findOne({ email }) + email = email.toLowerCase() + const member = options.member || await Member.findOne({ email }) if (!member) return { sent: false } const jti = randomUUID() diff --git a/server/utils/rateLimit.js b/server/utils/rateLimit.js index da15519..5fb5dda 100644 --- a/server/utils/rateLimit.js +++ b/server/utils/rateLimit.js @@ -5,8 +5,21 @@ const buckets = new Map() export function rateLimit(key, { max, windowMs }) { const now = Date.now() + + // Probabilistic sweep: ~1% of calls evict keys whose newest entry has fully + // aged out, so the Map doesn't grow unbounded under random-key spraying. + if (Math.random() < 0.01) { + for (const [k, arr] of buckets) { + const last = arr[arr.length - 1] + if (last === undefined || now - last >= windowMs) buckets.delete(k) + } + } + const arr = (buckets.get(key) || []).filter((t) => now - t < windowMs) - if (arr.length >= max) return false + if (arr.length >= max) { + buckets.set(key, arr) + return false + } arr.push(now) buckets.set(key, arr) return true