Commit graph

6 commits

Author SHA1 Message Date
Agent
9fa1f73316 fix: distinguish already-confirmed tokens from invalid ones; prevent duplicate active subscriptions
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.
2026-09-17 16:57:30 -04:00
694791bfdd Merge pull request 'Reschedule cron job when confirmed subscriber changes frequency' (#2) from mailship-e04-frequency-change-updates-db-but-never-re-63d into main
Reviewed-on: #2
Reviewed-by: matt <matt@lorentz.is>
2026-09-10 19:55:42 +00:00
Agent
4497a7dee9 fix: remove pre-eslint unused-variable errors across src/
Remove 21 unused imports/variables that caused eslint failures:
- actions.ts (2): Subscription type import, getCronExpression import
- alert.ts (4): CronExpressionParser, tryCatch, int, HOUR
- digest.ts (12): now, nth, nthEq, dateToSeconds, getIdFilters,
  getReplyFilters, Loader, AdapterContext, makeLoader, SocketAdapter,
  call, loadRelaySelections
- env.ts (1): netContext
- worker/email.ts (1): purgeJob variable (kept CronJob side-effect)

All unused symbols were pre-existing, not related to changed files.
tsc --noEmit passes; script/checks now returns 0 on the src check.
2026-09-10 11:58:05 -04:00
Agent
3e667fe3c6 fix: reschedule cron job when confirmed subscriber changes frequency
When a confirmed subscriber calls PUT /subscription/email with a changed
frequency, db.updateSubscription updates the row but the running CronJob
captured the original frequency in createJob and was never rescheduled.
A subscriber switching daily→weekly kept the daily cadence until restart.

Fix: in actions.ts:registerSubscription, call worker.registerSubscription(sub)
when the subscription is already confirmed, so addJob stops the old job and
creates a new one with the updated frequency.

Changes:
- src/actions.ts: add else branch calling worker.registerSubscription when
  sub.confirmed_at is set
- test/reschedule-on-frequency-change.test.js: new failing-before/passing-after
  test verifying the cron expression is updated after frequency change

Closes mailship-e04
2026-09-10 11:30:56 -04:00
mplorentz
5abec46cb7 Make subscription upsert idempotent via PUT
Switch the subscribe endpoint from POST to an idempotent PUT so clients can
always upsert without a stateful lookup first.

- Add GET /subscription/email?pubkey= lookup so clients can avoid re-POSTing.
- insertSubscription no longer clears confirmed_at unless the email address
  changes; a frequency change keeps the existing confirmation.
- registerSubscription only sends a confirmation email when the subscription
  is new, unconfirmed, or the email address changed.
- Update integration test and README for the PUT endpoint.
2026-09-03 11:47:21 -04:00
Agent
21e7058862 Initial mailship fork from anchor
Fork anchor, strip all push notification code (APNs, FCM, WebPush),
rename from anchor to mailship, add new database schema for
subscriptions + events tables, and add HTTP API for email
notification registration and NIP-9a relay push callbacks.

- POST /subscription/email — register for email digests
- DELETE /subscription/:key — unsubscribe
- POST /notify/:id — NIP-9a relay push callback
- GET /confirm?token=... — confirm email
- GET /unsubscribe?token=... — unsubscribe

Co-authored-by: mplorentz
2026-08-18 12:21:22 -04:00