From 37d57c06bf25e21774b62d06c89bfa0bd923299c Mon Sep 17 00:00:00 2001 From: Feranmi Oresajo Date: Fri, 25 Sep 2026 21:32:39 +0100 Subject: [PATCH 1/5] feat(auth): OAuth sign-in (Google/GitHub) and TOTP MFA for human accounts Migration 000032, hand-rolled OAuth code flow with DB-stored single-use state, RFC 6238 TOTP with secretbox-sealed secrets, hashed single-use backup codes, opaque single-use MFA challenge tokens, dedicated MFA verify rate limit. Ilana: DEC-228..231, RSK-046/047. --- .ilana/architecture.md | 11 +- .ilana/decisions.md | 4 + .ilana/ledger.md | 3 + .ilana/milestones.md | 7 + .ilana/risks.md | 2 + .ilana/state.json | 8 +- cmd/mailx/abuseconfig.go | 2 + cmd/mailx/authconfig.go | 63 ++++ cmd/mailx/serve.go | 5 + internal/api/abuse.go | 38 +++ internal/api/humanauth_handler.go | 3 + internal/api/humanauth_mfa_handler.go | 178 ++++++++++ internal/api/humanauth_mfa_handler_test.go | 53 +++ internal/api/openapi.go | 91 ++++- internal/api/routes.go | 1 + internal/database/human_mfa.go | 263 +++++++++++++++ internal/database/humans.go | 10 +- .../migrations/000032_oauth_mfa.down.sql | 12 + .../migrations/000032_oauth_mfa.up.sql | 61 ++++ internal/humanauth/mfa.go | 259 ++++++++++++++ internal/humanauth/mfa_test.go | 200 +++++++++++ internal/humanauth/oauth.go | 319 ++++++++++++++++++ internal/humanauth/oauth_test.go | 182 ++++++++++ internal/humanauth/service.go | 19 ++ internal/humanauth/totp.go | 75 ++++ internal/ratelimit/policy.go | 8 + 26 files changed, 1867 insertions(+), 10 deletions(-) create mode 100644 cmd/mailx/authconfig.go create mode 100644 internal/api/humanauth_mfa_handler.go create mode 100644 internal/api/humanauth_mfa_handler_test.go create mode 100644 internal/database/human_mfa.go create mode 100644 internal/database/migrations/000032_oauth_mfa.down.sql create mode 100644 internal/database/migrations/000032_oauth_mfa.up.sql create mode 100644 internal/humanauth/mfa.go create mode 100644 internal/humanauth/mfa_test.go create mode 100644 internal/humanauth/oauth.go create mode 100644 internal/humanauth/oauth_test.go create mode 100644 internal/humanauth/totp.go diff --git a/.ilana/architecture.md b/.ilana/architecture.md index f0521ae..9294ada 100644 --- a/.ilana/architecture.md +++ b/.ilana/architecture.md @@ -492,7 +492,16 @@ Redis queue polling (200ms..2s), not blocking primitives (multi-condition wake-u - New env var: `MAILX_JWT_SECRET` (same convention as `MAILX_API_KEY_PEPPER`; unset falls back to an ephemeral per-process secret with a startup warning — sessions do not survive a restart in that mode). - Password reset (DEC-213): migration 000029 adds `password_reset_tokens` (human_id FK CASCADE, `token_hash` unique-indexed, `expires_at`, `used_at` nullable, partial index on unresolved tokens per human). `Service.ForgotPassword`/`ResetPassword`: 5-minute single-use tokens (`PasswordResetTokenTTL`), same anti-enumeration response shape as `Login`, one collapsed `ErrPasswordResetTokenInvalid` for any unknown/expired/used/race-lost token. `database.ResetPassword` is one transaction: consume-token (RowsAffected-checked), update `password_hash`, revoke every refresh token the account has. Email delivery reuses `api.SubmissionAcceptor` (the v0.42 accept-a-message primitive) via a new `humanauth.Mailer` interface, configured by `MAILX_SYSTEM_TENANT_ID`/`MAILX_SYSTEM_FROM_ADDRESS`/`MAILX_DASHBOARD_BASE_URL`; unset means no mailer (token still created, logged not sent) rather than a startup failure. New routes `POST /v1/auth/forgot-password`/`reset-password`, public, behind their own `passwordResetIPLimitMiddleware` bucket (`Policy.PasswordResetIPRate`/`PasswordResetIPBurst`, default 1/60 rps burst 3 — much tighter than login's, since forgot-password sends a real email per call). - Organization invitations (DEC-216): migration 000030 adds `org_invitations` (tenant_id FK CASCADE, invited_by FK CASCADE, `normalized_email`/`raw_email`, `token_hash` unique-indexed, `expires_at`, `accepted_at` nullable, partial index on `(tenant_id, normalized_email) WHERE accepted_at IS NULL`) plus plain nullable URL columns `tenants.logo_url` and `humans.avatar_url` (no upload pipeline — operator decision). `Service.InviteToOrganization(ctx, inviterHumanID, tenantID, email)`: owner-only (`database.IsTenantOwner` re-checked server-side, `ErrNotOrgOwner` otherwise — no broader RBAC), 5-hour single-use tokens (`OrgInvitationTTL`), same `humanauth.Mailer`/`WithDashboardBaseURL` delivery wiring as password reset, degrades the same way when no mailer is configured. `Service.AcceptOrgInvitation(ctx, rawToken, existingHumanID, signupName, signupPassword)` is a single combined accept path (operator decision, not separate signup-then-join calls): with an existing session, the invitation's email must match that account's own (`ErrOrgInvitationEmailMismatch` otherwise) and `database.AcceptOrgInvitationForExistingHuman` atomically consumes the token and inserts `tenant_members` (`ON CONFLICT DO NOTHING`); with no session, `database.AcceptOrgInvitationWithSignup` atomically consumes the token, creates the human account (ALWAYS using the invitation's own stored email, never client-supplied), and inserts membership, all in one transaction. Both failure paths collapse to `ErrOrgInvitationInvalid`. New routes: `POST /v1/orgs/{id}/invites` (owner-only, behind `humanAuthMiddleware` then a new per-human `orgInviteLimitMiddleware` — `Policy.OrgInviteRate`/`OrgInviteBurst`, default 1/30 rps burst 10, keyed by the inviting human's ID rather than IP since the caller is already authenticated) and `POST /v1/orgs/invites/accept` (deliberately NOT behind `humanAuthMiddleware` — the invitee may have no account yet; reads an optional bearer token directly). -- Known limitation / explicitly deferred (not built): Paystack/billing, a `plan` field, email verification, OAuth/social login, MFA, and any RBAC beyond owner/member. The frontend's `{name, slug}` org-create contract is accepted; `slug` is currently a no-op input (tenants has no slug column yet). Avatar/logo are URL-only fields with no upload/hosting pipeline. +- Known limitation / explicitly deferred (not built): email verification and any RBAC beyond owner/member. The frontend's `{name, slug}` org-create contract is accepted; `slug` is currently a no-op input (tenants has no slug column yet). Avatar/logo are URL-only fields with no upload/hosting pipeline. + +## OAuth sign-in & TOTP MFA (v0.47 phase 3b; design decisions DEC-228..231) + +- Migration 000032: `humans.mfa_enabled` + sealed `mfa_secret_*`/`mfa_pending_secret_*` (secretbox, AD `mfa:`) + `mfa_last_used_step` (TOTP replay guard); `mfa_backup_codes` (SHA-256 hashed, single-use), `mfa_challenges` (hashed opaque token, 5-min TTL, single-use, burned after 5 wrong codes), `human_oauth_identities` (PK provider+provider_user_id -> human_id), `oauth_states` (hashed, 10-min TTL, consumed by DELETE). +- Routes (under the humanAuth block): `GET /v1/auth/oauth/{provider}/start|callback` (authIP bucket; 404 `oauth_provider_not_configured` per unconfigured provider), `POST /v1/auth/mfa/verify` (public, new `MFAVerifyIPRate` bucket 1/12s burst 5), `POST /v1/auth/mfa/enroll|confirm|disable` (human JWT; confirm on the MFA bucket, disable re-checks password on the authIP bucket). +- Invariant: a correct first factor on an MFA account (password Login OR OAuth callback) returns `*humanauth.MFARequiredError` (HTTP 200 `{mfa_required, mfa_token, expires_at}`), never a session. Every full session goes through `Service.completeLogin` (TouchLoginAndCreateRefreshToken). +- Invariant: OAuth links to an existing account by email only for a provider-VERIFIED email (Google `email_verified`, GitHub primary+verified from /user/emails). OAuth-created humans get an unusable password hash (password login impossible until a password reset). +- Config: `MAILX_GOOGLE_OAUTH_CLIENT_ID/SECRET`, `MAILX_GITHUB_OAUTH_CLIENT_ID/SECRET`, `MAILX_OAUTH_REDIRECT_BASE_URL` (each provider independent; half-config is a startup error), `MAILX_MFA_MASTER_KEY` (base64 32 bytes, must differ from DKIM/webhook keys; unset = enrollment 404, TOTP verify 503, backup codes still work), `MAILX_LIMIT_MFA_VERIFY_IP_RPS/BURST`. Wiring: `cmd/mailx/authconfig.go`. +- Limitations: RSK-046 (OAuth state not bound to the browser), RSK-047 (no per-account MFA lockout across challenges). Recovery/regeneration of backup codes requires disable + re-enroll. ## Billing & Plans (v0.47 phase 2; design decisions DEC-221..225) diff --git a/.ilana/decisions.md b/.ilana/decisions.md index 4940a85..058b996 100644 --- a/.ilana/decisions.md +++ b/.ilana/decisions.md @@ -232,3 +232,7 @@ Process decisions above (DEC-001..DEC-007) belong to the v0.23 FLEET run and sta - DEC-225 [v0.47 phase 2 billing]: Paystack integration is Initialize Transaction + webhook only (no Paystack Subscriptions/plan codes). `POST /v1/billing/webhook` is public and authenticated solely by `x-paystack-signature` = hex HMAC-SHA512(secret key, raw body), verified with `hmac.Equal` before any parsing; failure is 401. A verified `charge.success` is applied only if status=success, currency=USD, amount >= plan price, metadata names a tenant and a paid plan; it sets plan/active/period_end = now+30d. Replays are neutralized by `billing_payments.reference` PRIMARY KEY inserted in the same transaction as the plan update (duplicate -> rollback, 200 `already_applied`). Unknown events and signed-but-unusable payloads get 200 (no Paystack retry); only our storage errors are 5xx. Checkout/subscription take the org as `tenant_id` (body/query) because a human may own several orgs; checkout is owner-only, subscription any member (non-member gets 404, no enumeration). - DEC-226 [org invitations, CI caught what Greptile's trial limit ended before finding]: Greptile's free-trial credit limit was exhausted after PR #23 merged (no more automated review available going forward - manual review is now this project's primary line of defense, alongside CI). CI's own `-race` run on PR #24 caught a genuine, deeper bug in DEC-219/220's own fix: `TestConcurrentResendsNeverInvalidateBothLinks` failed intermittently with 2 survivors instead of 1, reproducing locally at roughly 50% under `go test -count=25`. Root cause: the `(created_at, id)` tuple comparison in `SupersedeOtherPendingOrgInvitations` assumes a smaller tuple means "already committed, therefore visible to a later query" - true only if inserts for the same address are serialized, which they were not. PostgreSQL's `now()` is fixed at a transaction's BEGIN, not its commit, so two genuinely concurrent autocommit INSERTs can commit in a DIFFERENT order than their `created_at` values suggest; a row with a small timestamp can still become visible to a later query AFTER that query's snapshot was already taken, meaning nothing ever supersedes it. Fixed properly this time with two changes together (verified each is independently necessary): (1) `CreateOrgInvitation` now holds `pg_advisory_xact_lock(hashtextextended(tenant_id||':'||normalized_email, 0))` for the duration of its own transaction, fully serializing concurrent inserts for the same address (the lock is scoped to a short transaction that commits well before the slower `SendSystemEmail` network call that follows - never held across it, preserving DEC-218's fix). (2) `created_at` is now set explicitly via `clock_timestamp()` in the INSERT rather than left to the column's `now()` default - critical, because a transaction that waited on the advisory lock would otherwise still capture an EARLIER timestamp (from its own BEGIN) than one that acquired the lock and committed first, silently reintroducing the exact ordering violation the lock exists to prevent. The advisory lock alone was proven insufficient by testing: re-ran the stress test 25x after adding only the lock and it still failed roughly half the time; only after also switching to `clock_timestamp()` did 40/40 stress runs (and the full `-race` suite) pass cleanly. This is the fifth fix-on-a-fix in this feature's review history (DEC-217→218→219→220→226), and the first one Greptile never got to review - a reminder that "manual review = read the diff and reason about it" is not equivalent to "manual review = actually stress-test the concurrency claim under `-count=N` before trusting it," especially now that automated review isn't available as a backstop. Verified: gofmt/go vet/go build clean, full `go test ./...` and `go test -race ./...` clean, `TestConcurrentResendsNeverInvalidateBothLinks` run 40x consecutively with `-race` with zero failures, Docker rebuild+boot smoke clean. - DEC-227 [billing plans, CodeRabbit review pass]: Greptile's trial is exhausted (see DEC-226); CodeRabbit is now active on this repo and reviewed PR #24, catching 3 real findings, 2 of them data-integrity-critical. (1) **Data loss on billing enablement or plan lapse** (the most severe): with plan enforcement on, a NULL `tenants.retention_days` falls back to the tenant's PLAN window (Free = 7 days) instead of the flat `DefaultRetentionDays` (90). An operator turning on `MAILX_PAYSTACK_SECRET_KEY` for the first time on an existing deployment would silently shrink every pre-existing tenant's retention window, and the next hourly `retention-purge` run would irreversibly hard-delete any terminal message between 7 and 90 days old; the same happens to a Plus/Pro tenant the instant `plan-lapse` downgrades it (a renewal running even one hour late triggers it). Fixed two ways: migration 000031 now backfills `retention_days = 90` for every tenant that has no explicit value BEFORE the plan columns' semantics can apply (pins pre-existing tenants at today's effective window; a brand-new tenant created after billing is enabled has no messages yet, so its plan's window applying from day one is correct, not a regression) - and `DowngradeLapsedPlans` now pins `retention_days` to the lapsing plan's own window (`COALESCE(retention_days, CASE plan WHEN 'plus' THEN 30 WHEN 'pro' THEN 90 END)`) before clearing `plan`, so a lapse only ever changes billing state, never retention behavior. (2) **Renewal loses remaining paid days** (major): `ApplyPlanPayment` overwrote `plan_current_period_end` to `now + 30 days` regardless of any remaining time on the current period, so an owner renewing 5 days early paid for 30 days but only received 25. Fixed by changing the parameter from an absolute `periodEnd` to a `period time.Duration`, and extending from the LATER of `now()` and the existing `plan_current_period_end` when the tenant is renewing the SAME plan while still active; a plan CHANGE (upgrade/downgrade) or a renewal after the plan had already lapsed still starts a fresh period from now, since carrying over time priced under a different plan has no well-defined meaning. (3) **Signed-but-unusable webhook payloads answered 400 instead of 200** (minor): Paystack can send non-object `metadata` (e.g. `0` or `""`) for a transaction MailX's own checkout never created (a payment page or another integration on the same Paystack account); `Metadata`'s plain-struct JSON decoding failed on those, and `handleWebhook`'s contract requires unrecognized/unusable-but-validly-signed payloads to be acknowledged 200 (Paystack retries forever on anything else). Fixed with a custom `Metadata.UnmarshalJSON` that treats a decode failure as an empty (not erroring) `Metadata` - the handler already ignores an empty `TenantID`. All three fixes covered by new/updated tests (`TestParseEventToleratesNonObjectMetadata`; `TestApplyPlanPaymentAndReplay` extended with same-plan-extends and different-plan-fresh-period cases; `TestDowngradeLapsedPlans` extended to assert the pinned retention window). Verified: gofmt/go vet/go build clean, full `go test ./...` and `go test -race ./...` clean, migration 000031 round-trip re-validated with the new backfill statement, Docker rebuild+boot smoke clean. +- DEC-228 [v0.47 phase 3b OAuth]: OAuth 2.0 Authorization Code client hand-rolled on net/http (`internal/humanauth/oauth.go`, ~250 lines), not `golang.org/x/oauth2`, matching the minimal-dependency stance (hand-rolled HS256 JWT, DEC-206). Endpoints must be absolute https URLs (validated at NewService); response bodies capped at 1 MiB; tokens are never logged or stored. State is stored in PostgreSQL (`oauth_states`, SHA-256 hash, 10-min TTL, single-use via guarded DELETE), not Redis: every other single-use auth token here is a hashed DB row, Redis is an optional dependency for the limiter only, and DB storage keeps state testable and durable across restarts. Hash lookup means the raw state is never compared directly (no timing oracle). +- DEC-229 [v0.47 phase 3b TOTP]: RFC 6238 TOTP hand-rolled (`internal/humanauth/totp.go`: HMAC-SHA1, 30s, 6 digits, ±1 step, constant-time compare), verified against all RFC 6238 Appendix B SHA-1 vectors; no library added. Secrets (20 random bytes) are sealed with the existing `internal/secretbox` under its own `MAILX_MFA_MASTER_KEY` (per secretbox's one-key-per-purpose rule). An accepted step is recorded in `mfa_last_used_step` and must strictly increase, so a code cannot be replayed. Backup codes: 10 x 64-bit random, SHA-256 hashed via `hashRawToken` (same as refresh/reset tokens), consumed with a RowsAffected-checked UPDATE. +- DEC-230 [v0.47 phase 3b MFA challenge]: The login intermediate state is an opaque 256-bit random token stored hashed in `mfa_challenges` (5-min TTL, single-use, burned after 5 wrong codes) - deliberately NOT a JWT, so it can never pass `VerifyAccessToken`. Login/OAuth return it as a typed `*MFARequiredError` (existing callers that treat any error as failure stay fail-closed); HTTP returns 200 `{mfa_required:true, mfa_token, expires_at}`. `CompleteMFAChallenge` consumes the challenge and the factor (TOTP step or backup code) in one transaction; any guard failure rolls back both. Session minting reuses Login's `completeLogin` path. +- DEC-231 [v0.47 phase 3b account linking]: An OAuth login with no existing identity link is LINKED to an existing human with the same normalized email (same lower+trim normalization as CreateHuman) rather than creating a duplicate - only when the provider reports the email verified. New OAuth humans get a non-bcrypt placeholder hash so password login is impossible until reset. OAuth for an MFA-enabled account still requires the MFA challenge. diff --git a/.ilana/ledger.md b/.ilana/ledger.md index d54b519..7f05b02 100644 --- a/.ilana/ledger.md +++ b/.ilana/ledger.md @@ -326,3 +326,6 @@ Greptile's free trial hit its credit limit after PR #23, so PR #24 (billing) got ## 2026-09-25 | billing plans: CodeRabbit review pass (first reviewer since Greptile's trial ran out) | GATE PASS Greptile's free-trial credit limit was hit right after PR #23 merged - it can no longer review this repo. CodeRabbit turned out to be configured on the repo too and reviewed PR #24 (billing) in Greptile's place, catching 3 real findings the earlier manual review had missed, 2 of them serious data-integrity bugs: enabling billing on an existing deployment (or a paid plan simply lapsing) could silently shrink a tenant's retention window from 90 days down to Free's 7, and the very next hourly retention-purge run would irreversibly hard-delete anything in that gap - fixed by backfilling retention_days for pre-existing tenants in the migration itself and by pinning the lapsing plan's window explicitly before a lapse clears the plan column, so billing state and retention behavior are now fully decoupled. Also fixed: a plan renewal was overwriting the remaining paid period instead of extending it (an owner renewing early lost days they'd already paid for), and a webhook whose metadata wasn't a JSON object (a real Paystack scenario for transactions MailX didn't create) was answered 400 instead of the required 200, which would make Paystack retry forever. This confirms the plan from the last session: without Greptile, review discipline has to come from us directly, and it's working - CodeRabbit filled the gap this time, but the org-invitation race from the previous entry proves CI's own -race run is just as important a backstop as any external reviewer. Verified: gofmt/go vet/go build clean, full go test ./... and go test -race ./... clean, migration round-trip re-validated, Docker rebuild+boot smoke clean. Ilana updated: decisions.md (DEC-227), state.json (DEC/RSK counters). + +## 2026-09-25 | v0.47 phase 3b: OAuth + TOTP MFA | GATE PASS (security review pending) +Built OAuth (Google/GitHub, hand-rolled) and TOTP MFA (hand-rolled RFC 6238, secretbox-sealed secrets) per DEC-228..231; migration 000032. Evidence: gofmt/vet clean, full `go test -race ./...` clean against real Postgres+Redis (humanauth ran, not skipped), migration round-trip, Docker boot smoke in both configurations. Open risks RSK-046/047. diff --git a/.ilana/milestones.md b/.ilana/milestones.md index 0edda0f..29585e5 100644 --- a/.ilana/milestones.md +++ b/.ilana/milestones.md @@ -106,3 +106,10 @@ explicitly deferred to a later phase — v0.47 as a whole is NOT complete. - Routes (only when `MAILX_PAYSTACK_SECRET_KEY` set): POST /v1/billing/checkout, GET /v1/billing/subscription, POST /v1/billing/webhook; `plan-lapse` hourly component. - Enforcement wired into admitSend (daily volume), domain create, invite send + both accept transactions (member cap, row-locked), broadcast create, webhook create, retention purge default. All inert when billing is not configured (DEC-221..225). - Tests: `internal/billing/billing_test.go`, `internal/database/plans_test.go` (incl. concurrent-accept race), `internal/api/billing_handler_test.go`, `internal/humanauth` member-cap test. Full `go test -race ./...` clean against real Postgres+Redis (0 skips in database/api/humanauth); migration down/up round-trip on a disposable DB; Docker rebuild+boot clean. + +## v0.47 phase 3b — OAuth (Google/GitHub) + TOTP MFA — COMPLETE (pending independent security review) + +- Migration 000032 (`oauth_states`, `human_oauth_identities`, `mfa_challenges`, `mfa_backup_codes`, `humans.mfa_*`); round-trip down/up validated on a disposable database. +- `internal/humanauth`: `oauth.go` (StartOAuth/CompleteOAuth), `totp.go`, `mfa.go` (Enroll/Confirm/Disable/VerifyMFA); Login returns `*MFARequiredError` for MFA accounts. Routes and env vars: architecture.md "OAuth sign-in & TOTP MFA". DEC-228..231, RSK-046/047. +- Tests: RFC 6238 vectors; full MFA flow on real Postgres (enroll, confirm, MFA-gated login, challenge not an access token, single-use, replay blocked, burned after 5 wrong codes, backup code once, expiry, disable needs password); OAuth against an httptest TLS fake Google (new account, link-not-duplicate, missing/unknown/reused/expired state, provider errors, unverified email, not configured, non-https rejected, MFA not bypassed); API test for the MFA IP bucket (429) and 404 unconfigured provider. Full `go test -race ./...` clean on Postgres+Redis; Docker boot smoke with nothing configured and with Google+GitHub+MFA configured. +- Not built: GitHub-provider fake test (only Google is exercised end-to-end), backup-code regeneration endpoint, per-account MFA lockout, browser-bound OAuth state. diff --git a/.ilana/risks.md b/.ilana/risks.md index cfe0b13..9fc73b8 100644 --- a/.ilana/risks.md +++ b/.ilana/risks.md @@ -54,3 +54,5 @@ - RSK-043 [low, v0.47 phase 1 PR review]: `internal/broadcast/expander.go`'s GDPR-erasure disk cleanup (see DEC-210's `ErrRecipientErased` path) retries 3 times on failure, but a disk error that survives all 3 (a genuine, non-transient FS problem, not just a momentary blip) still leaves the raw message file orphaned on disk with no database row left to ever find or retry it - the `broadcast_recipients` row that would normally drive a retry is already gone (that's what triggered the cleanup in the first place). Logged at Error level and counted via `BroadcastExpansionBatch("erasure_cleanup","error")` so an operator can alert on it, but not auto-healing. A fully durable fix needs a reconciliation sweep (e.g. periodically diff on-disk message directories against `messages` rows and delete orphans), which is a separate, larger change not undertaken here. Revisit if this metric/log ever fires in practice. - RSK-044 [high, v0.47 phase 2]: paid plans do NOT auto-renew. MVP bills by one-off Paystack Initialize Transaction; the hourly `plan-lapse` component downgrades any paid tenant whose `plan_current_period_end` has passed to `free`/`lapsed`, and the owner must check out again every 30 days. No renewal reminder email exists. Fix: Paystack Subscriptions (needs operator-side plan codes) or charge_authorization with the saved authorization. - RSK-045 [medium, v0.47 phase 2]: plan-enforcement gaps. (1) Broadcast expansion creates messages without passing admitSend, so broadcasts do not count toward the daily send cap. (2) The daily cap and the domain cap are checks, not reservations; concurrent requests can overshoot slightly. (3) The system tenant (`MAILX_SYSTEM_TENANT_ID`) is subject to its own plan once billing is enabled; operators must set it to pro (`UPDATE tenants SET plan='pro' ...`; there is no CLI for plan changes yet, and plan-lapse would downgrade it if a period end is set) or password-reset/invite mail stops at 500/day. (4) Paystack USD charging requires the Paystack account to be approved for USD; not verifiable from the repo. +- RSK-046 [medium, v0.47 phase 3b]: OAuth `state` is single-use and server-stored but not bound to the initiating browser (no cookie), because start/callback are JSON API endpoints. A login-CSRF (attacker gets a victim to complete the attacker's own callback URL, logging the victim into the attacker's account) is not prevented. Mitigation: bind state to an HttpOnly cookie or have the dashboard verify it initiated the flow. +- RSK-047 [medium, v0.47 phase 3b]: MFA brute-force limits are per-IP (5 burst, 1/12s) and per-challenge (5 wrong codes). An attacker who already has the password and many IPs can open new challenges (each login is also authIP-limited) and keep guessing; there is no per-account lockout or alert. Losing `MAILX_MFA_MASTER_KEY` makes TOTP unverifiable (backup codes still work). diff --git a/.ilana/state.json b/.ilana/state.json index 70c417c..5910a8e 100644 --- a/.ilana/state.json +++ b/.ilana/state.json @@ -17,8 +17,8 @@ "TC": 0, "DEF": 27, "CR": 24, - "RSK": 45, - "DEC": 227, + "RSK": 47, + "DEC": 231, "MET": 72, "ETH": 1 }, @@ -35,9 +35,9 @@ }, "mailx": { "last_completed_milestone": "v0.47 phase 2 (billing & plans MVP)", - "ilana_current_through": "v0.47 phase 2", + "ilana_current_through": "v0.47 phase 3b", "current_milestone": "v0.47 Human Accounts & Organizations", - "current_milestone_status": "v0.47 phase 1 (human auth, orgs, password reset, invitations) and phase 2 (Free/Plus/Pro plans + Paystack one-off checkout, enforcement gated on MAILX_PAYSTACK_SECRET_KEY) complete; auto-renewal (RSK-044), OAuth, MFA deferred", + "current_milestone_status": "v0.47 phase 1 + password reset + invitations, phase 2 billing, and phase 3b OAuth (Google/GitHub) + TOTP MFA complete; email verification and RBAC still deferred", "next_milestone": "v0.47 phase 3 (plan auto-renewal, dashboard backend) or v0.48 (TBD)" } } diff --git a/cmd/mailx/abuseconfig.go b/cmd/mailx/abuseconfig.go index 3ed81e0..092451b 100644 --- a/cmd/mailx/abuseconfig.go +++ b/cmd/mailx/abuseconfig.go @@ -93,6 +93,8 @@ func loadAbusePolicy(get func(string) string) (policy ratelimit.Policy, enabled n("MAILX_LIMIT_AUTH_IP_BURST", &policy.AuthIPBurst) f("MAILX_LIMIT_PASSWORD_RESET_IP_RPS", &policy.PasswordResetIPRate) n("MAILX_LIMIT_PASSWORD_RESET_IP_BURST", &policy.PasswordResetIPBurst) + f("MAILX_LIMIT_MFA_VERIFY_IP_RPS", &policy.MFAVerifyIPRate) + n("MAILX_LIMIT_MFA_VERIFY_IP_BURST", &policy.MFAVerifyIPBurst) f("MAILX_LIMIT_ORG_INVITE_RPS", &policy.OrgInviteRate) n("MAILX_LIMIT_ORG_INVITE_BURST", &policy.OrgInviteBurst) n("MAILX_RETRY_JITTER_PERCENT", &policy.RetryJitterPercent) diff --git a/cmd/mailx/authconfig.go b/cmd/mailx/authconfig.go new file mode 100644 index 0000000..f2741a2 --- /dev/null +++ b/cmd/mailx/authconfig.go @@ -0,0 +1,63 @@ +package main + +import ( + "fmt" + "os" + + "github.com/Ferousco-dev/mailx/internal/humanauth" + "github.com/Ferousco-dev/mailx/internal/secretbox" +) + +// buildOAuthMFAOptions wires OAuth providers and TOTP MFA from the +// environment, each independently and following billingconfig.go's +// graceful-degradation pattern: an unset variable disables only that feature. +// +// - Google: MAILX_GOOGLE_OAUTH_CLIENT_ID + MAILX_GOOGLE_OAUTH_CLIENT_SECRET +// - GitHub: MAILX_GITHUB_OAUTH_CLIENT_ID + MAILX_GITHUB_OAUTH_CLIENT_SECRET +// - both need MAILX_OAUTH_REDIRECT_BASE_URL (redirect_uri = +// /v1/auth/oauth//callback) +// - MFA: MAILX_MFA_MASTER_KEY (base64 32 bytes; seals TOTP secrets and must +// differ from the DKIM/webhook master keys) +// +// Half-configured providers (id without secret, or no redirect base) are a +// startup error rather than silently disabled, so a typo is noticed. +func buildOAuthMFAOptions(log func(msg string, args ...any)) ([]humanauth.Option, error) { + var opts []humanauth.Option + base := os.Getenv("MAILX_OAUTH_REDIRECT_BASE_URL") + for _, p := range []struct { + name, idEnv, secretEnv string + build func(id, secret, base string) *humanauth.OAuthProvider + }{ + {"google", "MAILX_GOOGLE_OAUTH_CLIENT_ID", "MAILX_GOOGLE_OAUTH_CLIENT_SECRET", humanauth.GoogleProvider}, + {"github", "MAILX_GITHUB_OAUTH_CLIENT_ID", "MAILX_GITHUB_OAUTH_CLIENT_SECRET", humanauth.GitHubProvider}, + } { + id, secret := os.Getenv(p.idEnv), os.Getenv(p.secretEnv) + if id == "" && secret == "" { + continue + } + if id == "" || secret == "" || base == "" { + return nil, fmt.Errorf("%s OAuth needs %s, %s and MAILX_OAUTH_REDIRECT_BASE_URL together", p.name, p.idEnv, p.secretEnv) + } + opts = append(opts, humanauth.WithOAuthProvider(p.build(id, secret, base))) + log("oauth_provider_enabled", "provider", p.name) + } + if enc := os.Getenv("MAILX_MFA_MASTER_KEY"); enc != "" { + key, err := secretbox.DecodeKey(enc, "MAILX_MFA_MASTER_KEY") + if err != nil { + return nil, err + } + for _, other := range []string{"MAILX_DKIM_MASTER_KEY", "MAILX_WEBHOOK_MASTER_KEY"} { + if o, derr := secretbox.DecodeKey(os.Getenv(other), other); derr == nil && string(o) == string(key) { + return nil, fmt.Errorf("MAILX_MFA_MASTER_KEY must differ from %s", other) + } + } + box, err := secretbox.New(key) + if err != nil { + return nil, err + } + opts = append(opts, humanauth.WithMFABox(box)) + } else { + log("mfa_disabled", "hint", "MAILX_MFA_MASTER_KEY not set: MFA enrollment is unavailable") + } + return opts, nil +} diff --git a/cmd/mailx/serve.go b/cmd/mailx/serve.go index 85f5742..d12e566 100644 --- a/cmd/mailx/serve.go +++ b/cmd/mailx/serve.go @@ -125,6 +125,11 @@ func runFull() error { } else { o.log.Warn("system_mailer_disabled", "hint", "MAILX_SYSTEM_TENANT_ID/MAILX_SYSTEM_FROM_ADDRESS not set: password reset tokens will be created but no email will be sent") } + oauthMFAOpts, err := buildOAuthMFAOptions(o.log.Info) + if err != nil { + return err + } + humanAuthOpts = append(humanAuthOpts, oauthMFAOpts...) humanAuthSvc, err := humanauth.NewService(db, jwtSecret(), humanAuthOpts...) if err != nil { return err diff --git a/internal/api/abuse.go b/internal/api/abuse.go index 3b3c9e4..6003cab 100644 --- a/internal/api/abuse.go +++ b/internal/api/abuse.go @@ -422,3 +422,41 @@ func (h *emailHandler) releaseClaim(tenantID string, c *database.IdempotencyComp h.abuse.log().Warn("idempotency_claim_release_failed") } } + +// mfaVerifyIPLimitMiddleware protects /v1/auth/mfa/verify and +// /v1/auth/mfa/confirm with their own tight per-IP bucket +// (Policy.MFAVerifyIPRate): a 6-digit code is brute-forceable, so neither the +// login bucket nor the general tenant buckets are tight enough. +func mfaVerifyIPLimitMiddleware(a *AbuseControls) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + if a == nil || a.Limiter == nil { + return next + } + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ip := authClientIP(r, a.TrustedProxyCIDRs) + p := a.Policy + dec, err := a.Limiter.Allow(r.Context(), + ratelimit.Bucket{Key: "auth:mfa:ip:" + ip, Rate: p.MFAVerifyIPRate, Burst: p.MFAVerifyIPBurst, Cost: 1}, + ) + switch { + case err != nil: + a.Metrics.AbuseDecision("mfa_verify", "unavailable") + a.log().Error("rate_limiter_unavailable", "route_class", "mfa_verify") + e := newError(ErrTemporarilyUnavailable, "rate_limiter_unavailable", "request limiting is temporarily unavailable; retry later") + e.RetryAfter = unavailableRetryAfter + writeError(w, r, e) + case dec.Impossible: + a.Metrics.AbuseDecision("mfa_verify", "impossible") + writeError(w, r, newError(ErrInternal, "internal_error", "request limit is misconfigured")) + case !dec.Allowed: + a.Metrics.AbuseDecision("mfa_verify", "limited") + e := newError(ErrRateLimited, "mfa_rate_limited", "too many MFA attempts from this address; retry after the interval in Retry-After") + e.RetryAfter = ratelimit.RetryAfterSeconds(dec.RetryAfter) + writeError(w, r, e) + default: + a.Metrics.AbuseDecision("mfa_verify", "allowed") + next.ServeHTTP(w, r) + } + }) + } +} diff --git a/internal/api/humanauth_handler.go b/internal/api/humanauth_handler.go index 33f77da..12e15aa 100644 --- a/internal/api/humanauth_handler.go +++ b/internal/api/humanauth_handler.go @@ -94,6 +94,9 @@ func (h *humanAuthHandler) handleLogin(w http.ResponseWriter, r *http.Request) { return } session, err := h.svc.Login(r.Context(), req.Email, req.Password) + if writeMFARequired(w, err) { + return + } if err != nil { // Login intentionally returns the SAME error for "no such email" // and "wrong password" — see humanauth.Service.Login's doc. diff --git a/internal/api/humanauth_mfa_handler.go b/internal/api/humanauth_mfa_handler.go new file mode 100644 index 0000000..db9acb9 --- /dev/null +++ b/internal/api/humanauth_mfa_handler.go @@ -0,0 +1,178 @@ +package api + +import ( + "encoding/json" + "errors" + "net/http" + "time" + + "github.com/Ferousco-dev/mailx/internal/humanauth" +) + +// OAuth sign-in and TOTP MFA endpoints (v0.47 phase 3b, DEC-228..DEC-231). + +func registerMFAAndOAuthRoutes(mux *http.ServeMux, ha *humanAuthHandler, authenticated func(http.Handler) http.Handler, abuse *AbuseControls) { + mux.Handle("GET /v1/auth/oauth/{provider}/start", chain(http.HandlerFunc(ha.handleOAuthStart), authIPLimitMiddleware(abuse))) + mux.Handle("GET /v1/auth/oauth/{provider}/callback", chain(http.HandlerFunc(ha.handleOAuthCallback), authIPLimitMiddleware(abuse))) + mux.Handle("POST /v1/auth/mfa/verify", chain(http.HandlerFunc(ha.handleMFAVerify), mfaVerifyIPLimitMiddleware(abuse))) + mux.Handle("POST /v1/auth/mfa/enroll", authenticated(http.HandlerFunc(ha.handleMFAEnroll))) + mux.Handle("POST /v1/auth/mfa/confirm", authenticated(chain(http.HandlerFunc(ha.handleMFAConfirm), mfaVerifyIPLimitMiddleware(abuse)))) + mux.Handle("POST /v1/auth/mfa/disable", authenticated(chain(http.HandlerFunc(ha.handleMFADisable), authIPLimitMiddleware(abuse)))) +} + +// writeMFARequired writes the intermediate MFA state if err is +// *humanauth.MFARequiredError, reporting whether it did. +func writeMFARequired(w http.ResponseWriter, err error) bool { + var mfa *humanauth.MFARequiredError + if !errors.As(err, &mfa) { + return false + } + writeJSON(w, http.StatusOK, map[string]any{ + "mfa_required": true, + "mfa_token": mfa.ChallengeToken, + "expires_at": mfa.ExpiresAt.UTC().Format(time.RFC3339), + }) + return true +} + +func decodeHumanJSON(w http.ResponseWriter, r *http.Request, dst any) bool { + if !acceptsJSONContentType(r.Header.Get("Content-Type")) { + writeError(w, r, newError(ErrUnsupportedMediaType, "unsupported_media_type", "Content-Type must be application/json")) + return false + } + if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, maxBodyBytes)).Decode(dst); err != nil { + writeError(w, r, newError(ErrInvalidRequest, "invalid_json", "request body is not valid JSON")) + return false + } + return true +} + +func (h *humanAuthHandler) handleOAuthStart(w http.ResponseWriter, r *http.Request) { + u, err := h.svc.StartOAuth(r.Context(), r.PathValue("provider")) + if errors.Is(err, humanauth.ErrOAuthProviderNotConfigured) { + writeError(w, r, newError(ErrNotFoundType, "oauth_provider_not_configured", "this sign-in provider is not configured on this server")) + return + } + if err != nil { + writeError(w, r, newError(ErrInternal, "oauth_start_failed", "could not start sign-in")) + return + } + writeJSON(w, http.StatusOK, map[string]string{"authorization_url": u}) +} + +func (h *humanAuthHandler) handleOAuthCallback(w http.ResponseWriter, r *http.Request) { + q := r.URL.Query() + code := q.Get("code") + if q.Get("error") != "" { + code = "" // consent denied: still consume the state, then report a provider error + } + session, err := h.svc.CompleteOAuth(r.Context(), r.PathValue("provider"), code, q.Get("state")) + if writeMFARequired(w, err) { + return + } + switch { + case err == nil: + writeJSON(w, http.StatusOK, sessionResponseFrom(session)) + case errors.Is(err, humanauth.ErrOAuthProviderNotConfigured): + writeError(w, r, newError(ErrNotFoundType, "oauth_provider_not_configured", "this sign-in provider is not configured on this server")) + case errors.Is(err, humanauth.ErrOAuthStateInvalid): + writeError(w, r, newError(ErrAuthentication, "invalid_oauth_state", "sign-in state is missing, expired, or already used; start again")) + case errors.Is(err, humanauth.ErrOAuthProvider): + writeError(w, r, newError(ErrAuthentication, "oauth_provider_error", "the sign-in provider did not complete authentication")) + default: + writeError(w, r, newError(ErrInternal, "oauth_callback_failed", "could not complete sign-in")) + } +} + +type mfaCodeRequest struct { + MFAToken string `json:"mfa_token"` + Code string `json:"code"` + Password string `json:"password"` +} + +func (h *humanAuthHandler) handleMFAVerify(w http.ResponseWriter, r *http.Request) { + var req mfaCodeRequest + if !decodeHumanJSON(w, r, &req) { + return + } + if req.MFAToken == "" || req.Code == "" { + writeError(w, r, newError(ErrInvalidRequest, "invalid_json", "mfa_token and code are required")) + return + } + session, err := h.svc.VerifyMFA(r.Context(), req.MFAToken, req.Code) + switch { + case err == nil: + writeJSON(w, http.StatusOK, sessionResponseFrom(session)) + case errors.Is(err, humanauth.ErrMFAChallengeInvalid): + writeError(w, r, newError(ErrAuthentication, "invalid_mfa", "MFA challenge is invalid or expired, or the code is incorrect")) + case errors.Is(err, humanauth.ErrMFANotConfigured): + writeError(w, r, newError(ErrTemporarilyUnavailable, "mfa_not_configured", "authenticator codes cannot be checked on this server right now; use a backup code")) + default: + writeError(w, r, newError(ErrInternal, "mfa_verify_failed", "could not verify MFA")) + } +} + +func (h *humanAuthHandler) handleMFAEnroll(w http.ResponseWriter, r *http.Request) { + humanID, ok := humanIDFromContext(r.Context()) + if !ok { + writeError(w, r, newError(ErrAuthentication, "invalid_access_token", "missing human session")) + return + } + enr, err := h.svc.EnrollMFA(r.Context(), humanID) + switch { + case err == nil: + writeJSON(w, http.StatusOK, map[string]any{"secret": enr.Secret, "otpauth_uri": enr.OTPAuthURI, "backup_codes": enr.BackupCodes}) + case errors.Is(err, humanauth.ErrMFANotConfigured): + writeError(w, r, newError(ErrNotFoundType, "mfa_not_configured", "MFA is not configured on this server")) + case errors.Is(err, humanauth.ErrMFAAlreadyEnabled): + writeError(w, r, newError(ErrConflictType, "mfa_already_enabled", "MFA is already enabled; disable it first")) + default: + writeError(w, r, newError(ErrInternal, "mfa_enroll_failed", "could not start MFA enrollment")) + } +} + +func (h *humanAuthHandler) handleMFAConfirm(w http.ResponseWriter, r *http.Request) { + humanID, ok := humanIDFromContext(r.Context()) + if !ok { + writeError(w, r, newError(ErrAuthentication, "invalid_access_token", "missing human session")) + return + } + var req mfaCodeRequest + if !decodeHumanJSON(w, r, &req) { + return + } + err := h.svc.ConfirmMFA(r.Context(), humanID, req.Code) + switch { + case err == nil: + writeJSON(w, http.StatusOK, map[string]bool{"mfa_enabled": true}) + case errors.Is(err, humanauth.ErrMFANotConfigured): + writeError(w, r, newError(ErrNotFoundType, "mfa_not_configured", "MFA is not configured on this server")) + case errors.Is(err, humanauth.ErrMFACodeInvalid): + writeError(w, r, newError(ErrValidation, "invalid_mfa_code", "the code is incorrect")) + case errors.Is(err, humanauth.ErrMFANotPending), errors.Is(err, humanauth.ErrMFAAlreadyEnabled): + writeError(w, r, newError(ErrConflictType, "mfa_not_pending", "no pending MFA enrollment; call /v1/auth/mfa/enroll first")) + default: + writeError(w, r, newError(ErrInternal, "mfa_confirm_failed", "could not confirm MFA")) + } +} + +func (h *humanAuthHandler) handleMFADisable(w http.ResponseWriter, r *http.Request) { + humanID, ok := humanIDFromContext(r.Context()) + if !ok { + writeError(w, r, newError(ErrAuthentication, "invalid_access_token", "missing human session")) + return + } + var req mfaCodeRequest + if !decodeHumanJSON(w, r, &req) { + return + } + err := h.svc.DisableMFA(r.Context(), humanID, req.Password) + switch { + case err == nil: + writeJSON(w, http.StatusOK, map[string]bool{"mfa_enabled": false}) + case errors.Is(err, humanauth.ErrInvalidCredentials): + writeError(w, r, newError(ErrAuthentication, "invalid_credentials", "password is incorrect")) + default: + writeError(w, r, newError(ErrInternal, "mfa_disable_failed", "could not disable MFA")) + } +} diff --git a/internal/api/humanauth_mfa_handler_test.go b/internal/api/humanauth_mfa_handler_test.go new file mode 100644 index 0000000..ab3bace --- /dev/null +++ b/internal/api/humanauth_mfa_handler_test.go @@ -0,0 +1,53 @@ +package api + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/Ferousco-dev/mailx/internal/auth" + "github.com/Ferousco-dev/mailx/internal/humanauth" + "github.com/Ferousco-dev/mailx/internal/storage" +) + +// MFA verify has its own tight per-IP bucket: after MFAVerifyIPBurst wrong +// attempts the next is 429, and OAuth for an unconfigured provider is a clear 404. +func TestMFAVerifyRateLimitedAndOAuthNotConfigured(t *testing.T) { + rig := newAbuseRig(t, testPolicy()) + svc, err := humanauth.NewService(rig.db, []byte("test-secret-at-least-32-bytes-long!!")) + if err != nil { + t.Fatal(err) + } + store, err := storage.NewFileStore(t.TempDir()) + if err != nil { + t.Fatal(err) + } + p := testPolicy() + ac := &AbuseControls{Limiter: rig.limit, Policy: p} + mux := newMux(newEmailHandler(rig.db, store), auth.NewService(rig.db, nil), func() error { return nil }, routeServices{abuse: ac, humanAuth: svc}) + + do := func(method, path, body string) int { + req := httptest.NewRequest(method, path, strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = "203.0.113.9:1234" + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + return w.Code + } + for i := 0; i < p.MFAVerifyIPBurst; i++ { + if c := do("POST", "/v1/auth/mfa/verify", `{"mfa_token":"nope","code":"123456"}`); c != http.StatusUnauthorized { + t.Fatalf("attempt %d: got %d want 401", i, c) + } + } + if c := do("POST", "/v1/auth/mfa/verify", `{"mfa_token":"nope","code":"123456"}`); c != http.StatusTooManyRequests { + t.Fatalf("over burst: got %d want 429", c) + } + if c := do("GET", "/v1/auth/oauth/google/start", ""); c != http.StatusNotFound { + t.Fatalf("unconfigured provider: got %d want 404", c) + } + // Enroll requires a session. + if c := do("POST", "/v1/auth/mfa/enroll", `{}`); c != http.StatusUnauthorized { + t.Fatalf("enroll without session: got %d want 401", c) + } +} diff --git a/internal/api/openapi.go b/internal/api/openapi.go index e1f3a59..21ce5d1 100644 --- a/internal/api/openapi.go +++ b/internal/api/openapi.go @@ -66,11 +66,91 @@ const openAPISpec = `{ "security": [], "requestBody": {"required": true, "content": {"application/json": {"schema": {"$ref": "#/components/schemas/LoginRequest"}}}}, "responses": { - "200": {"description": "OK", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/Session"}}}}, + "200": {"description": "OK: either a Session, or (account has MFA enabled) an MFAChallenge to complete via /auth/mfa/verify", "content": {"application/json": {"schema": {"oneOf": [{"$ref": "#/components/schemas/Session"}, {"$ref": "#/components/schemas/MFAChallenge"}]}}}}, "401": {"description": "Invalid credentials", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}} } } }, + "/auth/oauth/{provider}/start": { + "get": { + "summary": "Start Google/GitHub sign-in", + "description": "Public. provider is google or github. Returns the provider consent URL carrying a single-use, 10-minute state value. 404 oauth_provider_not_configured when that provider's MAILX_*_OAUTH_CLIENT_ID/SECRET are unset. IP rate-limited like login.", + "security": [], + "parameters": [{"name": "provider", "in": "path", "required": true, "schema": {"type": "string", "enum": ["google", "github"]}}], + "responses": { + "200": {"description": "OK", "content": {"application/json": {"schema": {"type": "object", "properties": {"authorization_url": {"type": "string"}}}}}}, + "404": {"description": "Provider not configured", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}} + } + } + }, + "/auth/oauth/{provider}/callback": { + "get": { + "summary": "Complete Google/GitHub sign-in", + "description": "Public; the provider redirects here (redirect_uri = MAILX_OAUTH_REDIRECT_BASE_URL + this path). Validates and consumes state, exchanges code, and requires a provider-verified email. Links to an existing account with the same (case-insensitive) email, otherwise creates one with no usable password. Returns a Session, or an MFAChallenge when the account has MFA enabled.", + "security": [], + "parameters": [ + {"name": "provider", "in": "path", "required": true, "schema": {"type": "string", "enum": ["google", "github"]}}, + {"name": "code", "in": "query", "schema": {"type": "string"}}, + {"name": "state", "in": "query", "required": true, "schema": {"type": "string"}} + ], + "responses": { + "200": {"description": "OK", "content": {"application/json": {"schema": {"oneOf": [{"$ref": "#/components/schemas/Session"}, {"$ref": "#/components/schemas/MFAChallenge"}]}}}}, + "401": {"description": "invalid_oauth_state or oauth_provider_error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}}, + "404": {"description": "Provider not configured", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}} + } + } + }, + "/auth/mfa/enroll": { + "post": { + "summary": "Start TOTP MFA enrollment", + "description": "Requires a human access token. Returns a new TOTP secret, an otpauth:// URI for client-side QR rendering, and 10 single-use backup codes, shown once. Not active until /auth/mfa/confirm. 404 mfa_not_configured without MAILX_MFA_MASTER_KEY; 409 mfa_already_enabled.", + "security": [{"HumanAuth": []}], + "responses": { + "200": {"description": "OK", "content": {"application/json": {"schema": {"type": "object", "properties": {"secret": {"type": "string"}, "otpauth_uri": {"type": "string"}, "backup_codes": {"type": "array", "items": {"type": "string"}}}}}}}, + "401": {"description": "Missing or invalid access token", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}}, + "409": {"description": "MFA already enabled", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}} + } + } + }, + "/auth/mfa/confirm": { + "post": { + "summary": "Activate MFA with a code from the pending secret", + "description": "Requires a human access token and body {code}. Rate-limited by the dedicated MFA per-IP bucket.", + "security": [{"HumanAuth": []}], + "requestBody": {"required": true, "content": {"application/json": {"schema": {"type": "object", "required": ["code"], "properties": {"code": {"type": "string"}}}}}}, + "responses": { + "200": {"description": "MFA enabled", "content": {"application/json": {"schema": {"type": "object", "properties": {"mfa_enabled": {"type": "boolean"}}}}}}, + "409": {"description": "No pending enrollment", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}}, + "422": {"description": "Incorrect code", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}}, + "429": {"description": "Rate limited", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}} + } + } + }, + "/auth/mfa/disable": { + "post": { + "summary": "Disable MFA (password re-confirmation required)", + "description": "Requires a human access token and body {password}. Removes the secret, backup codes, and pending challenges.", + "security": [{"HumanAuth": []}], + "requestBody": {"required": true, "content": {"application/json": {"schema": {"type": "object", "required": ["password"], "properties": {"password": {"type": "string"}}}}}}, + "responses": { + "200": {"description": "MFA disabled", "content": {"application/json": {"schema": {"type": "object", "properties": {"mfa_enabled": {"type": "boolean"}}}}}}, + "401": {"description": "Wrong password or invalid access token", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}} + } + } + }, + "/auth/mfa/verify": { + "post": { + "summary": "Complete an MFA login", + "description": "Public. Body {mfa_token, code}: the token from an MFAChallenge plus a 6-digit TOTP code (±30s skew, each code accepted once) or a backup code (single-use). The challenge lives 5 minutes, is single-use, and is burned after 5 wrong codes; it is never accepted as an access token. Dedicated tight per-IP rate limit.", + "security": [], + "requestBody": {"required": true, "content": {"application/json": {"schema": {"type": "object", "required": ["mfa_token", "code"], "properties": {"mfa_token": {"type": "string"}, "code": {"type": "string"}}}}}}, + "responses": { + "200": {"description": "OK", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/Session"}}}}, + "401": {"description": "invalid_mfa (challenge invalid/expired/used or code incorrect)", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}}, + "429": {"description": "Rate limited", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}} + } + } + }, "/auth/refresh": { "post": { "summary": "Rotate a refresh token for a new access/refresh pair", @@ -2865,6 +2945,15 @@ const openAPISpec = `{ } }, "schemas": { + "MFAChallenge": { + "type": "object", + "required": ["mfa_required", "mfa_token", "expires_at"], + "properties": { + "mfa_required": {"type": "boolean", "enum": [true]}, + "mfa_token": {"type": "string", "description": "Opaque, single-use; only valid for POST /auth/mfa/verify"}, + "expires_at": {"type": "string", "format": "date-time"} + } + }, "SignUpRequest": { "type": "object", "required": ["name", "email", "password"], diff --git a/internal/api/routes.go b/internal/api/routes.go index d7a6a2b..6aa8b3a 100644 --- a/internal/api/routes.go +++ b/internal/api/routes.go @@ -186,6 +186,7 @@ func newMux(h *emailHandler, authSvc authService, readiness func() error, extras mux.Handle("POST /v1/auth/forgot-password", chain(http.HandlerFunc(ha.handleForgotPassword), passwordResetIPLimitMiddleware(abuse))) mux.Handle("POST /v1/auth/reset-password", chain(http.HandlerFunc(ha.handleResetPassword), passwordResetIPLimitMiddleware(abuse))) orgsAuthenticated := humanAuthMiddleware(extras[0].humanAuth) + registerMFAAndOAuthRoutes(mux, ha, orgsAuthenticated, abuse) mux.Handle("POST /v1/orgs", orgsAuthenticated(http.HandlerFunc(ha.handleCreateOrg))) mux.Handle("GET /v1/orgs", orgsAuthenticated(http.HandlerFunc(ha.handleListOrgs))) // Owner-only, JWT-authenticated, then its own tighter per-human diff --git a/internal/database/human_mfa.go b/internal/database/human_mfa.go new file mode 100644 index 0000000..f206843 --- /dev/null +++ b/internal/database/human_mfa.go @@ -0,0 +1,263 @@ +package database + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/jackc/pgx/v5" +) + +// Storage for TOTP MFA and OAuth sign-in (migration 000032). Every +// single-use artifact here (challenge, backup code, OAuth state) is consumed +// with a guarded UPDATE/DELETE whose RowsAffected is checked, the same +// race-safety pattern as password_reset_tokens and org_invitations. + +// ErrMFAConsumed means a challenge/backup code/TOTP step was already used, +// expired, or lost a concurrent consume race. Callers never distinguish. +var ErrMFAConsumed = errors.New("database: mfa artifact already used or expired") + +// MFASecrets is a human's sealed TOTP material (never plaintext). +type MFASecrets struct { + Enabled bool + SecretCiphertext []byte + SecretNonce []byte + PendingCiphertext []byte + PendingNonce []byte +} + +// GetMFASecrets loads the sealed TOTP material for a human. +func (db *DB) GetMFASecrets(ctx context.Context, humanID string) (MFASecrets, error) { + var m MFASecrets + err := db.pool.QueryRow(ctx, ` + SELECT mfa_enabled, mfa_secret_ciphertext, mfa_secret_nonce, mfa_pending_secret_ciphertext, mfa_pending_secret_nonce + FROM humans WHERE id = $1`, humanID, + ).Scan(&m.Enabled, &m.SecretCiphertext, &m.SecretNonce, &m.PendingCiphertext, &m.PendingNonce) + if err != nil { + return MFASecrets{}, normalizeErr(err) + } + return m, nil +} + +// SetPendingMFA stores a freshly enrolled (unconfirmed) sealed secret and +// replaces the human's backup codes, but only while MFA is NOT enabled +// (re-enrolling an active factor requires disabling it first). Returns +// ErrConflict when MFA is already enabled. +func (db *DB) SetPendingMFA(ctx context.Context, humanID string, ciphertext, nonce []byte, backupCodeHashes []string) error { + tx, err := db.pool.Begin(ctx) + if err != nil { + return normalizeErr(err) + } + defer func() { _ = tx.Rollback(ctx) }() + tag, err := tx.Exec(ctx, `UPDATE humans SET mfa_pending_secret_ciphertext = $2, mfa_pending_secret_nonce = $3, updated_at = now() + WHERE id = $1 AND NOT mfa_enabled`, humanID, ciphertext, nonce) + if err != nil { + return normalizeErr(err) + } + if tag.RowsAffected() == 0 { + return ErrConflict + } + if _, err := tx.Exec(ctx, `DELETE FROM mfa_backup_codes WHERE human_id = $1`, humanID); err != nil { + return normalizeErr(err) + } + for _, h := range backupCodeHashes { + id, err := newID() + if err != nil { + return err + } + if _, err := tx.Exec(ctx, `INSERT INTO mfa_backup_codes (id, human_id, code_hash) VALUES ($1, $2, $3)`, id, humanID, h); err != nil { + return normalizeErr(err) + } + } + return normalizeErr(tx.Commit(ctx)) +} + +// ActivateMFA promotes the pending secret to active and enables MFA, recording +// step as the last used TOTP step (the confirming code cannot be replayed). +// The pending ciphertext is matched so a concurrent re-enroll cannot activate a +// secret the caller did not verify. Returns ErrMFAConsumed otherwise. +func (db *DB) ActivateMFA(ctx context.Context, humanID string, pendingCiphertext []byte, step int64) error { + tag, err := db.pool.Exec(ctx, ` + UPDATE humans SET mfa_enabled = true, + mfa_secret_ciphertext = mfa_pending_secret_ciphertext, mfa_secret_nonce = mfa_pending_secret_nonce, + mfa_pending_secret_ciphertext = NULL, mfa_pending_secret_nonce = NULL, + mfa_last_used_step = $3, updated_at = now() + WHERE id = $1 AND NOT mfa_enabled AND mfa_pending_secret_ciphertext = $2`, humanID, pendingCiphertext, step) + if err != nil { + return normalizeErr(err) + } + if tag.RowsAffected() == 0 { + return ErrMFAConsumed + } + return nil +} + +// DisableMFA clears all MFA state for a human: secrets, backup codes, and +// outstanding challenges. +func (db *DB) DisableMFA(ctx context.Context, humanID string) error { + tx, err := db.pool.Begin(ctx) + if err != nil { + return normalizeErr(err) + } + defer func() { _ = tx.Rollback(ctx) }() + stmts := []string{ + `UPDATE humans SET mfa_enabled = false, mfa_secret_ciphertext = NULL, mfa_secret_nonce = NULL, + mfa_pending_secret_ciphertext = NULL, mfa_pending_secret_nonce = NULL, mfa_last_used_step = NULL, updated_at = now() + WHERE id = $1`, + `DELETE FROM mfa_backup_codes WHERE human_id = $1`, + `DELETE FROM mfa_challenges WHERE human_id = $1`, + } + for _, q := range stmts { + if _, err := tx.Exec(ctx, q, humanID); err != nil { + return normalizeErr(err) + } + } + return normalizeErr(tx.Commit(ctx)) +} + +// MFAChallenge is one issued login challenge row. +type MFAChallenge struct { + ID string + HumanID string + ExpiresAt time.Time + UsedAt *time.Time + FailedAttempts int +} + +// CreateMFAChallenge inserts a challenge for humanID. +func (db *DB) CreateMFAChallenge(ctx context.Context, humanID, tokenHash string, expiresAt time.Time) error { + id, err := newID() + if err != nil { + return err + } + _, err = db.pool.Exec(ctx, `INSERT INTO mfa_challenges (id, human_id, token_hash, expires_at) VALUES ($1, $2, $3, $4)`, + id, humanID, tokenHash, expiresAt) + return normalizeErr(err) +} + +// GetMFAChallengeByHash loads a challenge. ErrNotFound if absent. +func (db *DB) GetMFAChallengeByHash(ctx context.Context, tokenHash string) (MFAChallenge, error) { + var c MFAChallenge + err := db.pool.QueryRow(ctx, `SELECT id, human_id, expires_at, used_at, failed_attempts FROM mfa_challenges WHERE token_hash = $1`, tokenHash). + Scan(&c.ID, &c.HumanID, &c.ExpiresAt, &c.UsedAt, &c.FailedAttempts) + if err != nil { + return MFAChallenge{}, normalizeErr(err) + } + return c, nil +} + +// RecordMFAChallengeFailure counts a wrong code and burns the challenge once +// failed_attempts reaches maxAttempts. +func (db *DB) RecordMFAChallengeFailure(ctx context.Context, challengeID string, maxAttempts int, now time.Time) error { + _, err := db.pool.Exec(ctx, ` + UPDATE mfa_challenges SET failed_attempts = failed_attempts + 1, + used_at = CASE WHEN failed_attempts + 1 >= $2 THEN COALESCE(used_at, $3) ELSE used_at END + WHERE id = $1`, challengeID, maxAttempts, now) + return normalizeErr(err) +} + +// CompleteMFAChallenge atomically consumes the challenge AND the second factor +// that satisfied it: either a TOTP step (must be newer than the last accepted +// step, so a code cannot be replayed) or a backup code hash (must be unused). +// Exactly one of totpStep (non-nil) or backupCodeHash (non-empty) is used. +// Any guard failing rolls everything back and returns ErrMFAConsumed. +func (db *DB) CompleteMFAChallenge(ctx context.Context, challengeID, humanID string, now time.Time, totpStep *int64, backupCodeHash string) error { + tx, err := db.pool.Begin(ctx) + if err != nil { + return normalizeErr(err) + } + defer func() { _ = tx.Rollback(ctx) }() + tag, err := tx.Exec(ctx, `UPDATE mfa_challenges SET used_at = $3 WHERE id = $1 AND human_id = $2 AND used_at IS NULL AND expires_at > $3`, + challengeID, humanID, now) + if err != nil { + return normalizeErr(err) + } + if tag.RowsAffected() == 0 { + return ErrMFAConsumed + } + if totpStep != nil { + tag, err = tx.Exec(ctx, `UPDATE humans SET mfa_last_used_step = $2 WHERE id = $1 AND mfa_enabled AND (mfa_last_used_step IS NULL OR mfa_last_used_step < $2)`, + humanID, *totpStep) + } else { + tag, err = tx.Exec(ctx, `UPDATE mfa_backup_codes SET used_at = $3 WHERE human_id = $1 AND code_hash = $2 AND used_at IS NULL`, + humanID, backupCodeHash, now) + } + if err != nil { + return normalizeErr(err) + } + if tag.RowsAffected() == 0 { + return ErrMFAConsumed + } + return normalizeErr(tx.Commit(ctx)) +} + +// CreateOAuthState stores a hashed OAuth state value. +func (db *DB) CreateOAuthState(ctx context.Context, stateHash, provider string, expiresAt time.Time) error { + _, err := db.pool.Exec(ctx, `INSERT INTO oauth_states (state_hash, provider, expires_at) VALUES ($1, $2, $3)`, stateHash, provider, expiresAt) + return normalizeErr(err) +} + +// ConsumeOAuthState deletes the state row (single use) and reports whether it +// existed for this provider and was unexpired. Expired rows are also swept. +func (db *DB) ConsumeOAuthState(ctx context.Context, stateHash, provider string, now time.Time) (bool, error) { + tag, err := db.pool.Exec(ctx, `DELETE FROM oauth_states WHERE state_hash = $1 AND provider = $2 AND expires_at > $3`, stateHash, provider, now) + if err != nil { + return false, normalizeErr(err) + } + if _, err := db.pool.Exec(ctx, `DELETE FROM oauth_states WHERE expires_at <= $1`, now); err != nil { + return false, normalizeErr(err) + } + return tag.RowsAffected() == 1, nil +} + +// unusablePasswordHash is stored for accounts created via OAuth. It is not a +// valid bcrypt hash, so bcrypt.CompareHashAndPassword always fails and no +// password login is possible until the human sets one via password reset. +const unusablePasswordHash = "!oauth-no-password" + +// ResolveOAuthHuman finds or creates the human for a provider identity, in one +// transaction: (1) an existing identity link wins; (2) otherwise a human with +// the same (case-insensitive, same normalization as CreateHuman) email is +// LINKED, not duplicated; (3) otherwise a new human is created with an +// unusable password. Callers must only pass a provider-verified email. +func (db *DB) ResolveOAuthHuman(ctx context.Context, provider, providerUserID, email, name string, avatarURL *string) (Human, bool, error) { + tx, err := db.pool.Begin(ctx) + if err != nil { + return Human{}, false, normalizeErr(err) + } + defer func() { _ = tx.Rollback(ctx) }() + + var humanID string + created := false + err = tx.QueryRow(ctx, `SELECT human_id FROM human_oauth_identities WHERE provider = $1 AND provider_user_id = $2`, provider, providerUserID).Scan(&humanID) + switch { + case err == nil: + case errors.Is(err, pgx.ErrNoRows): + err = tx.QueryRow(ctx, `SELECT id FROM humans WHERE normalized_email = $1`, normalizeEmail(email)).Scan(&humanID) + if errors.Is(err, pgx.ErrNoRows) { + id, idErr := newID() + if idErr != nil { + return Human{}, false, idErr + } + if _, err := tx.Exec(ctx, `INSERT INTO humans (id, name, normalized_email, email, password_hash, avatar_url) VALUES ($1, $2, $3, $4, $5, $6)`, + id, name, normalizeEmail(email), email, unusablePasswordHash, avatarURL); err != nil { + return Human{}, false, normalizeErr(err) + } + humanID, created = id, true + } else if err != nil { + return Human{}, false, normalizeErr(err) + } + if _, err := tx.Exec(ctx, `INSERT INTO human_oauth_identities (provider, provider_user_id, human_id, email) VALUES ($1, $2, $3, $4)`, + provider, providerUserID, humanID, email); err != nil { + return Human{}, false, normalizeErr(err) + } + default: + return Human{}, false, normalizeErr(err) + } + if err := tx.Commit(ctx); err != nil { + return Human{}, false, fmt.Errorf("database: commit oauth resolve: %w", normalizeErr(err)) + } + h, err := db.GetHuman(ctx, humanID) + return h, created, err +} diff --git a/internal/database/humans.go b/internal/database/humans.go index efd1891..77874c0 100644 --- a/internal/database/humans.go +++ b/internal/database/humans.go @@ -22,6 +22,8 @@ type Human struct { // AvatarURL is nil until the human sets one; a plain URL, no upload // pipeline (see migration 000030's doc). Used by org-invite emails. AvatarURL *string + // MFAEnabled is true once TOTP MFA is confirmed (migration 000032). + MFAEnabled bool } // RefreshToken is one issued refresh token row (see migration 000025). @@ -71,10 +73,10 @@ func (db *DB) CreateHuman(ctx context.Context, name, email, passwordHash string) func (db *DB) GetHumanByEmail(ctx context.Context, email string) (Human, error) { var h Human err := db.pool.QueryRow(ctx, ` - SELECT id, name, email, password_hash, role, created_at, updated_at, last_login_at, avatar_url + SELECT id, name, email, password_hash, role, created_at, updated_at, last_login_at, avatar_url, mfa_enabled FROM humans WHERE normalized_email = $1`, normalizeEmail(email), - ).Scan(&h.ID, &h.Name, &h.Email, &h.PasswordHash, &h.Role, &h.CreatedAt, &h.UpdatedAt, &h.LastLoginAt, &h.AvatarURL) + ).Scan(&h.ID, &h.Name, &h.Email, &h.PasswordHash, &h.Role, &h.CreatedAt, &h.UpdatedAt, &h.LastLoginAt, &h.AvatarURL, &h.MFAEnabled) if err != nil { return Human{}, normalizeErr(err) } @@ -85,9 +87,9 @@ func (db *DB) GetHumanByEmail(ctx context.Context, email string) (Human, error) func (db *DB) GetHuman(ctx context.Context, id string) (Human, error) { var h Human err := db.pool.QueryRow(ctx, ` - SELECT id, name, email, password_hash, role, created_at, updated_at, last_login_at, avatar_url + SELECT id, name, email, password_hash, role, created_at, updated_at, last_login_at, avatar_url, mfa_enabled FROM humans WHERE id = $1`, id, - ).Scan(&h.ID, &h.Name, &h.Email, &h.PasswordHash, &h.Role, &h.CreatedAt, &h.UpdatedAt, &h.LastLoginAt, &h.AvatarURL) + ).Scan(&h.ID, &h.Name, &h.Email, &h.PasswordHash, &h.Role, &h.CreatedAt, &h.UpdatedAt, &h.LastLoginAt, &h.AvatarURL, &h.MFAEnabled) if err != nil { return Human{}, normalizeErr(err) } diff --git a/internal/database/migrations/000032_oauth_mfa.down.sql b/internal/database/migrations/000032_oauth_mfa.down.sql new file mode 100644 index 0000000..3826fe5 --- /dev/null +++ b/internal/database/migrations/000032_oauth_mfa.down.sql @@ -0,0 +1,12 @@ +DROP TABLE oauth_states; +DROP TABLE human_oauth_identities; +DROP TABLE mfa_challenges; +DROP TABLE mfa_backup_codes; +ALTER TABLE humans + DROP CONSTRAINT humans_mfa_enabled_has_secret, + DROP COLUMN mfa_last_used_step, + DROP COLUMN mfa_pending_secret_nonce, + DROP COLUMN mfa_pending_secret_ciphertext, + DROP COLUMN mfa_secret_nonce, + DROP COLUMN mfa_secret_ciphertext, + DROP COLUMN mfa_enabled; diff --git a/internal/database/migrations/000032_oauth_mfa.up.sql b/internal/database/migrations/000032_oauth_mfa.up.sql new file mode 100644 index 0000000..202f363 --- /dev/null +++ b/internal/database/migrations/000032_oauth_mfa.up.sql @@ -0,0 +1,61 @@ +-- v0.47 phase 3b: OAuth (Google/GitHub) sign-in and TOTP MFA for human +-- accounts. See .ilana/decisions.md DEC-228..DEC-231. + +-- TOTP secrets are sealed with internal/secretbox under MAILX_MFA_MASTER_KEY +-- (associated data "mfa:"); plaintext secrets are never stored. +-- The pending pair holds an enrolled-but-unconfirmed secret; confirm moves it +-- into the active pair and sets mfa_enabled. mfa_last_used_step blocks replay +-- of an already-accepted TOTP code within its validity window. +ALTER TABLE humans + ADD COLUMN mfa_enabled BOOLEAN NOT NULL DEFAULT false, + ADD COLUMN mfa_secret_ciphertext BYTEA, + ADD COLUMN mfa_secret_nonce BYTEA, + ADD COLUMN mfa_pending_secret_ciphertext BYTEA, + ADD COLUMN mfa_pending_secret_nonce BYTEA, + ADD COLUMN mfa_last_used_step BIGINT, + ADD CONSTRAINT humans_mfa_enabled_has_secret CHECK (NOT mfa_enabled OR (mfa_secret_ciphertext IS NOT NULL AND mfa_secret_nonce IS NOT NULL)); + +-- Single-use recovery codes, SHA-256 hashed like every other token here. +CREATE TABLE mfa_backup_codes ( + id TEXT PRIMARY KEY, + human_id TEXT NOT NULL REFERENCES humans(id) ON DELETE CASCADE, + code_hash TEXT NOT NULL UNIQUE, + used_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); +CREATE INDEX mfa_backup_codes_human_idx ON mfa_backup_codes (human_id); + +-- Short-lived, single-use login challenge issued after a correct first +-- factor on an MFA-enabled account. Opaque random token (hash stored), NOT a +-- JWT, so it can never verify as an access token. +CREATE TABLE mfa_challenges ( + id TEXT PRIMARY KEY, + human_id TEXT NOT NULL REFERENCES humans(id) ON DELETE CASCADE, + token_hash TEXT NOT NULL UNIQUE, + expires_at TIMESTAMPTZ NOT NULL, + used_at TIMESTAMPTZ, + failed_attempts INT NOT NULL DEFAULT 0, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); +CREATE INDEX mfa_challenges_human_idx ON mfa_challenges (human_id); + +-- Links a provider account to a human. One provider account maps to exactly +-- one human; a human may link several providers. +CREATE TABLE human_oauth_identities ( + provider TEXT NOT NULL CHECK (provider IN ('google','github')), + provider_user_id TEXT NOT NULL CHECK (length(provider_user_id) BETWEEN 1 AND 255), + human_id TEXT NOT NULL REFERENCES humans(id) ON DELETE CASCADE, + email TEXT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + PRIMARY KEY (provider, provider_user_id) +); +CREATE INDEX human_oauth_identities_human_idx ON human_oauth_identities (human_id); + +-- OAuth CSRF state: hash of a random value, single-use (DELETE ... RETURNING), +-- 10-minute TTL. +CREATE TABLE oauth_states ( + state_hash TEXT PRIMARY KEY, + provider TEXT NOT NULL, + expires_at TIMESTAMPTZ NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); diff --git a/internal/humanauth/mfa.go b/internal/humanauth/mfa.go new file mode 100644 index 0000000..dfd9d62 --- /dev/null +++ b/internal/humanauth/mfa.go @@ -0,0 +1,259 @@ +package humanauth + +import ( + "context" + "crypto/rand" + "encoding/hex" + "errors" + "fmt" + "strings" + "time" + + "github.com/Ferousco-dev/mailx/internal/database" + "github.com/Ferousco-dev/mailx/internal/secretbox" + "golang.org/x/crypto/bcrypt" +) + +// TOTP MFA for human accounts (DEC-229/DEC-230). + +const ( + // MFAChallengeTTL bounds how long a password-verified login may wait + // for its second factor. + MFAChallengeTTL = 5 * time.Minute + // MFAChallengeMaxAttempts burns a challenge after this many wrong codes, + // independent of the per-IP rate limit. + MFAChallengeMaxAttempts = 5 + backupCodeCount = 10 + totpSecretBytes = 20 + totpIssuer = "MailX" +) + +var ( + // ErrMFANotConfigured means MAILX_MFA_MASTER_KEY is unset, so TOTP + // secrets cannot be sealed or opened. + ErrMFANotConfigured = errors.New("humanauth: mfa is not configured on this server") + // ErrMFAAlreadyEnabled is returned by EnrollMFA when MFA is active. + ErrMFAAlreadyEnabled = errors.New("humanauth: mfa is already enabled; disable it first") + // ErrMFANotPending is returned by ConfirmMFA without a pending enrollment. + ErrMFANotPending = errors.New("humanauth: no pending mfa enrollment") + // ErrMFACodeInvalid is returned for a wrong TOTP code on confirm. + ErrMFACodeInvalid = errors.New("humanauth: invalid mfa code") + // ErrMFAChallengeInvalid covers every failure of VerifyMFA (unknown, + // expired, used, burned challenge, or wrong/replayed code) - never + // distinguished to the caller. + ErrMFAChallengeInvalid = errors.New("humanauth: mfa challenge invalid, expired, or code incorrect") +) + +// MFARequiredError is returned by Login (and the OAuth callback) instead of a +// session when the account has MFA enabled. ChallengeToken is an opaque, +// single-use, MFAChallengeTTL-lived value that is ONLY accepted by VerifyMFA: +// it is not a JWT and never verifies as an access token. +type MFARequiredError struct { + ChallengeToken string + ExpiresAt time.Time +} + +func (e *MFARequiredError) Error() string { return "humanauth: mfa required" } + +// WithMFABox enables TOTP enrollment/verification with a secretbox keyed by +// MAILX_MFA_MASTER_KEY. +func WithMFABox(b *secretbox.Box) Option { return func(s *Service) { s.mfaBox = b } } + +// MFAConfigured reports whether TOTP secrets can be sealed/opened. +func (s *Service) MFAConfigured() bool { return s.mfaBox != nil } + +func mfaAD(humanID string) []byte { return []byte("mfa:" + humanID) } + +// normalizeBackupCode lowercases and strips separators/spaces. +func normalizeBackupCode(c string) string { + return strings.Map(func(r rune) rune { + if r == '-' || r == ' ' { + return -1 + } + return r + }, strings.ToLower(strings.TrimSpace(c))) +} + +func generateBackupCode() (string, error) { + b := make([]byte, 8) // 64 bits + if _, err := rand.Read(b); err != nil { + return "", err + } + h := hex.EncodeToString(b) + return h[0:4] + "-" + h[4:8] + "-" + h[8:12] + "-" + h[12:16], nil +} + +// MFAEnrollment is returned once by EnrollMFA. Secret and BackupCodes are +// shown to the human exactly once and never retrievable again. +type MFAEnrollment struct { + Secret string // base32 + OTPAuthURI string + BackupCodes []string +} + +// EnrollMFA generates a new pending TOTP secret and fresh backup codes. MFA is +// not active until ConfirmMFA succeeds. +func (s *Service) EnrollMFA(ctx context.Context, humanID string) (MFAEnrollment, error) { + if s.mfaBox == nil { + return MFAEnrollment{}, ErrMFANotConfigured + } + h, err := s.db.GetHuman(ctx, humanID) + if err != nil { + return MFAEnrollment{}, fmt.Errorf("humanauth: get human: %w", err) + } + if h.MFAEnabled { + return MFAEnrollment{}, ErrMFAAlreadyEnabled + } + key := make([]byte, totpSecretBytes) + if _, err := rand.Read(key); err != nil { + return MFAEnrollment{}, fmt.Errorf("humanauth: generate totp secret: %w", err) + } + ct, nonce, err := s.mfaBox.Encrypt(key, mfaAD(humanID)) + if err != nil { + return MFAEnrollment{}, err + } + codes := make([]string, backupCodeCount) + hashes := make([]string, backupCodeCount) + for i := range codes { + c, err := generateBackupCode() + if err != nil { + return MFAEnrollment{}, err + } + codes[i], hashes[i] = c, hashRawToken(normalizeBackupCode(c)) + } + if err := s.db.SetPendingMFA(ctx, humanID, ct, nonce, hashes); err != nil { + if errors.Is(err, database.ErrConflict) { + return MFAEnrollment{}, ErrMFAAlreadyEnabled + } + return MFAEnrollment{}, fmt.Errorf("humanauth: store pending mfa: %w", err) + } + return MFAEnrollment{Secret: totpB32.EncodeToString(key), OTPAuthURI: otpauthURI(totpIssuer, h.Email, key), BackupCodes: codes}, nil +} + +// ConfirmMFA activates the pending secret if code is valid for it. +func (s *Service) ConfirmMFA(ctx context.Context, humanID, code string) error { + if s.mfaBox == nil { + return ErrMFANotConfigured + } + m, err := s.db.GetMFASecrets(ctx, humanID) + if err != nil { + return fmt.Errorf("humanauth: get mfa: %w", err) + } + if m.Enabled { + return ErrMFAAlreadyEnabled + } + if m.PendingCiphertext == nil { + return ErrMFANotPending + } + key, err := s.mfaBox.Decrypt(m.PendingCiphertext, m.PendingNonce, mfaAD(humanID)) + if err != nil { + return fmt.Errorf("humanauth: open pending mfa secret: %w", err) + } + step, ok := verifyTOTP(key, strings.TrimSpace(code), s.now()) + if !ok { + return ErrMFACodeInvalid + } + if err := s.db.ActivateMFA(ctx, humanID, m.PendingCiphertext, step); err != nil { + if errors.Is(err, database.ErrMFAConsumed) { + return ErrMFANotPending + } + return fmt.Errorf("humanauth: activate mfa: %w", err) + } + return nil +} + +// DisableMFA removes MFA after re-confirming the account password. A wrong +// password returns ErrInvalidCredentials. Accounts created via OAuth have no +// usable password and must set one (password reset) first. +func (s *Service) DisableMFA(ctx context.Context, humanID, password string) error { + h, err := s.db.GetHuman(ctx, humanID) + if err != nil { + return fmt.Errorf("humanauth: get human: %w", err) + } + if err := bcrypt.CompareHashAndPassword([]byte(h.PasswordHash), []byte(password)); err != nil { + return ErrInvalidCredentials + } + if err := s.db.DisableMFA(ctx, humanID); err != nil { + return fmt.Errorf("humanauth: disable mfa: %w", err) + } + return nil +} + +// newMFAChallenge issues the intermediate login state for an MFA account. +func (s *Service) newMFAChallenge(ctx context.Context, h database.Human) error { + raw, err := generateRawToken() + if err != nil { + return err + } + exp := s.now().Add(MFAChallengeTTL) + if err := s.db.CreateMFAChallenge(ctx, h.ID, hashRawToken(raw), exp); err != nil { + return fmt.Errorf("humanauth: create mfa challenge: %w", err) + } + return &MFARequiredError{ChallengeToken: raw, ExpiresAt: exp} +} + +// VerifyMFA completes an MFA login: challengeToken from MFARequiredError plus +// either a 6-digit TOTP code or a backup code. On success the challenge and the +// factor are consumed atomically and a full session is minted through the same +// path as a password-only Login. +func (s *Service) VerifyMFA(ctx context.Context, challengeToken, code string) (Session, error) { + c, err := s.db.GetMFAChallengeByHash(ctx, hashRawToken(challengeToken)) + if err != nil { + if errors.Is(err, database.ErrNotFound) { + return Session{}, ErrMFAChallengeInvalid + } + return Session{}, fmt.Errorf("humanauth: get mfa challenge: %w", err) + } + now := s.now() + if c.UsedAt != nil || !c.ExpiresAt.After(now) { + return Session{}, ErrMFAChallengeInvalid + } + fail := func() (Session, error) { + if err := s.db.RecordMFAChallengeFailure(ctx, c.ID, MFAChallengeMaxAttempts, now); err != nil { + return Session{}, fmt.Errorf("humanauth: record mfa failure: %w", err) + } + return Session{}, ErrMFAChallengeInvalid + } + code = strings.TrimSpace(code) + var step *int64 + backupHash := "" + if len(code) == totpDigits { + if s.mfaBox == nil { + return Session{}, ErrMFANotConfigured + } + m, err := s.db.GetMFASecrets(ctx, c.HumanID) + if err != nil { + return Session{}, fmt.Errorf("humanauth: get mfa: %w", err) + } + if !m.Enabled || m.SecretCiphertext == nil { + return Session{}, ErrMFAChallengeInvalid + } + key, err := s.mfaBox.Decrypt(m.SecretCiphertext, m.SecretNonce, mfaAD(c.HumanID)) + if err != nil { + return Session{}, fmt.Errorf("humanauth: open mfa secret: %w", err) + } + st, ok := verifyTOTP(key, code, now) + if !ok { + return fail() + } + step = &st + } else { + n := normalizeBackupCode(code) + if len(n) != 16 { + return fail() + } + backupHash = hashRawToken(n) + } + if err := s.db.CompleteMFAChallenge(ctx, c.ID, c.HumanID, now, step, backupHash); err != nil { + if errors.Is(err, database.ErrMFAConsumed) { + // Replayed TOTP step, unknown/used backup code, or a lost race. + return fail() + } + return Session{}, fmt.Errorf("humanauth: complete mfa challenge: %w", err) + } + h, err := s.db.GetHuman(ctx, c.HumanID) + if err != nil { + return Session{}, fmt.Errorf("humanauth: get human: %w", err) + } + return s.completeLogin(ctx, h) +} diff --git a/internal/humanauth/mfa_test.go b/internal/humanauth/mfa_test.go new file mode 100644 index 0000000..bc23414 --- /dev/null +++ b/internal/humanauth/mfa_test.go @@ -0,0 +1,200 @@ +package humanauth + +import ( + "context" + "crypto/rand" + "encoding/base32" + "errors" + "strings" + "sync" + "testing" + "time" + + "github.com/Ferousco-dev/mailx/internal/secretbox" +) + +// RFC 6238 Appendix B SHA-1 vectors (8 digits; the 6-digit value is the +// low-order 6 digits because both are code mod 10^n). +func TestTOTPRFC6238Vectors(t *testing.T) { + key := []byte("12345678901234567890") + for _, v := range []struct { + unix int64 + want string + }{ + {59, "94287082"}, {1111111109, "07081804"}, {1111111111, "14050471"}, + {1234567890, "89005924"}, {2000000000, "69279037"}, {20000000000, "65353130"}, + } { + step := uint64(v.unix / 30) + if got := hotp(key, step, 8); got != v.want { + t.Errorf("T=%d: got %s want %s", v.unix, got, v.want) + } + if got := hotp(key, step, 6); got != v.want[2:] { + t.Errorf("T=%d 6-digit: got %s want %s", v.unix, got, v.want[2:]) + } + } + now := time.Unix(1111111111, 0) + for _, d := range []int64{-30, 0, 30} { + if _, ok := verifyTOTP(key, hotp(key, uint64((now.Unix()+d)/30), 6), now); !ok { + t.Errorf("skew %d rejected", d) + } + } + if _, ok := verifyTOTP(key, hotp(key, uint64((now.Unix()+90)/30), 6), now); ok { + t.Error("code 3 steps away accepted") + } +} + +type mfaFixture struct { + svc *Service + clock *time.Time + mu *sync.Mutex +} + +func (f mfaFixture) advance(d time.Duration) { f.mu.Lock(); *f.clock = f.clock.Add(d); f.mu.Unlock() } + +func newMFAService(t *testing.T) mfaFixture { + t.Helper() + db := newTestDB(t) + key := make([]byte, 32) + _, _ = rand.Read(key) + box, err := secretbox.New(key) + if err != nil { + t.Fatal(err) + } + clock := time.Now().UTC().Truncate(time.Second) + mu := &sync.Mutex{} + svc, err := NewService(db, testSecret(), WithMFABox(box), WithNow(func() time.Time { mu.Lock(); defer mu.Unlock(); return clock })) + if err != nil { + t.Fatal(err) + } + return mfaFixture{svc: svc, clock: &clock, mu: mu} +} + +func codeAt(t *testing.T, secret string, now time.Time) string { + t.Helper() + key, err := base32.StdEncoding.WithPadding(base32.NoPadding).DecodeString(secret) + if err != nil { + t.Fatal(err) + } + return hotp(key, uint64(totpStep(now)), 6) +} + +func loginChallenge(t *testing.T, svc *Service, email string) string { + t.Helper() + _, err := svc.Login(context.Background(), email, "password123") + var mfa *MFARequiredError + if !errors.As(err, &mfa) || mfa.ChallengeToken == "" { + t.Fatalf("expected MFA challenge, got %v", err) + } + return mfa.ChallengeToken +} + +func TestMFAFullFlow(t *testing.T) { + f := newMFAService(t) + svc, ctx := f.svc, context.Background() + sess, err := svc.SignUp(ctx, "Ada", "ada@example.com", "password123") + if err != nil { + t.Fatal(err) + } + id := sess.Human.ID + + enr, err := svc.EnrollMFA(ctx, id) + if err != nil { + t.Fatal(err) + } + if len(enr.BackupCodes) != 10 || !strings.HasPrefix(enr.OTPAuthURI, "otpauth://totp/MailX:") || !strings.Contains(enr.OTPAuthURI, "secret="+enr.Secret) { + t.Fatalf("bad enrollment: %+v", enr) + } + // Not active before confirm: login still yields a full session. + if _, err := svc.Login(ctx, "ada@example.com", "password123"); err != nil { + t.Fatalf("login before confirm: %v", err) + } + if err := svc.ConfirmMFA(ctx, id, "000000"); !errors.Is(err, ErrMFACodeInvalid) && codeAt(t, enr.Secret, *f.clock) != "000000" { + t.Fatalf("wrong confirm code: %v", err) + } + if err := svc.ConfirmMFA(ctx, id, codeAt(t, enr.Secret, *f.clock)); err != nil { + t.Fatalf("confirm: %v", err) + } + if _, err := svc.EnrollMFA(ctx, id); !errors.Is(err, ErrMFAAlreadyEnabled) { + t.Fatalf("re-enroll while enabled: %v", err) + } + + // Login now requires MFA; the challenge is not an access token. + ch := loginChallenge(t, svc, "ada@example.com") + if _, err := svc.VerifyAccessToken(ch); err == nil { + t.Fatal("challenge token verified as an access token") + } + // The confirming code's step is burned: advance one step first. + f.advance(30 * time.Second) + s2, err := svc.VerifyMFA(ctx, ch, codeAt(t, enr.Secret, *f.clock)) + if err != nil || s2.AccessToken == "" || s2.RefreshToken == "" { + t.Fatalf("verify: %v", err) + } + // Challenge cannot be reused after success. + if _, err := svc.VerifyMFA(ctx, ch, codeAt(t, enr.Secret, *f.clock)); !errors.Is(err, ErrMFAChallengeInvalid) { + t.Fatalf("reused challenge: %v", err) + } + // Same TOTP code cannot be replayed on a fresh challenge. + ch = loginChallenge(t, svc, "ada@example.com") + if _, err := svc.VerifyMFA(ctx, ch, codeAt(t, enr.Secret, *f.clock)); !errors.Is(err, ErrMFAChallengeInvalid) { + t.Fatalf("replayed TOTP code: %v", err) + } + + // Wrong codes burn the challenge after MFAChallengeMaxAttempts. + ch = loginChallenge(t, svc, "ada@example.com") + f.advance(30 * time.Second) + good := codeAt(t, enr.Secret, *f.clock) + bad := "000000" + if good == bad { + bad = "111111" + } + for i := 0; i < MFAChallengeMaxAttempts; i++ { + if _, err := svc.VerifyMFA(ctx, ch, bad); !errors.Is(err, ErrMFAChallengeInvalid) { + t.Fatalf("bad code %d: %v", i, err) + } + } + if _, err := svc.VerifyMFA(ctx, ch, good); !errors.Is(err, ErrMFAChallengeInvalid) { + t.Fatalf("burned challenge accepted a good code: %v", err) + } + + // Backup code works exactly once (any case / separators). + ch = loginChallenge(t, svc, "ada@example.com") + if _, err := svc.VerifyMFA(ctx, ch, strings.ToUpper(enr.BackupCodes[0])); err != nil { + t.Fatalf("backup code: %v", err) + } + ch = loginChallenge(t, svc, "ada@example.com") + if _, err := svc.VerifyMFA(ctx, ch, enr.BackupCodes[0]); !errors.Is(err, ErrMFAChallengeInvalid) { + t.Fatalf("backup code reused: %v", err) + } + + // Challenge expires. + ch = loginChallenge(t, svc, "ada@example.com") + f.advance(MFAChallengeTTL + time.Second) + if _, err := svc.VerifyMFA(ctx, ch, codeAt(t, enr.Secret, *f.clock)); !errors.Is(err, ErrMFAChallengeInvalid) { + t.Fatalf("expired challenge: %v", err) + } + + // Disable requires the password. + if err := svc.DisableMFA(ctx, id, "wrong-password"); !errors.Is(err, ErrInvalidCredentials) { + t.Fatalf("disable wrong password: %v", err) + } + if err := svc.DisableMFA(ctx, id, "password123"); err != nil { + t.Fatal(err) + } + if _, err := svc.Login(ctx, "ada@example.com", "password123"); err != nil { + t.Fatalf("login after disable: %v", err) + } +} + +func TestMFANotConfigured(t *testing.T) { + svc, err := NewService(newTestDB(t), testSecret()) + if err != nil { + t.Fatal(err) + } + sess, err := svc.SignUp(context.Background(), "B", "b@example.com", "password123") + if err != nil { + t.Fatal(err) + } + if _, err := svc.EnrollMFA(context.Background(), sess.Human.ID); !errors.Is(err, ErrMFANotConfigured) { + t.Fatalf("got %v", err) + } +} diff --git a/internal/humanauth/oauth.go b/internal/humanauth/oauth.go new file mode 100644 index 0000000..0b92be9 --- /dev/null +++ b/internal/humanauth/oauth.go @@ -0,0 +1,319 @@ +package humanauth + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strconv" + "strings" + "time" +) + +// OAuth 2.0 Authorization Code sign-in with Google and GitHub, hand-rolled on +// net/http (DEC-228). No token is ever logged or stored: the provider access +// token is used once to fetch the verified email and then discarded. + +const ( + // OAuthStateTTL bounds a started-but-unfinished OAuth flow. + OAuthStateTTL = 10 * time.Minute + // maxOAuthResponseBytes caps provider response bodies. + maxOAuthResponseBytes = 1 << 20 +) + +var ( + // ErrOAuthProviderNotConfigured is returned for an unknown or unconfigured provider. + ErrOAuthProviderNotConfigured = errors.New("humanauth: oauth provider is not configured") + // ErrOAuthStateInvalid covers missing, unknown, expired, reused, or + // wrong-provider state values. + ErrOAuthStateInvalid = errors.New("humanauth: oauth state invalid or expired") + // ErrOAuthProvider covers any provider-side failure (denied consent, bad + // code, unreachable endpoint, no verified email). + ErrOAuthProvider = errors.New("humanauth: oauth provider error") +) + +// OAuthProvider is one configured provider. Endpoint URLs must be https. +type OAuthProvider struct { + Name string // "google" | "github" + ClientID string + ClientSecret string + AuthURL string + TokenURL string + UserInfoURL string + EmailsURL string // github only + Scopes []string + RedirectURL string + HTTPClient *http.Client +} + +// GoogleProvider returns Google's production endpoints. +func GoogleProvider(clientID, clientSecret, redirectBaseURL string) *OAuthProvider { + return &OAuthProvider{ + Name: "google", ClientID: clientID, ClientSecret: clientSecret, + AuthURL: "https://accounts.google.com/o/oauth2/v2/auth", + TokenURL: "https://oauth2.googleapis.com/token", + UserInfoURL: "https://openidconnect.googleapis.com/v1/userinfo", + Scopes: []string{"openid", "email", "profile"}, + RedirectURL: oauthRedirectURL(redirectBaseURL, "google"), + } +} + +// GitHubProvider returns GitHub's production endpoints. +func GitHubProvider(clientID, clientSecret, redirectBaseURL string) *OAuthProvider { + return &OAuthProvider{ + Name: "github", ClientID: clientID, ClientSecret: clientSecret, + AuthURL: "https://github.com/login/oauth/authorize", + TokenURL: "https://github.com/login/oauth/access_token", + UserInfoURL: "https://api.github.com/user", + EmailsURL: "https://api.github.com/user/emails", + Scopes: []string{"read:user", "user:email"}, + RedirectURL: oauthRedirectURL(redirectBaseURL, "github"), + } +} + +func oauthRedirectURL(base, provider string) string { + return strings.TrimRight(base, "/") + "/v1/auth/oauth/" + provider + "/callback" +} + +func (p *OAuthProvider) validate() error { + if p.Name != "google" && p.Name != "github" { + return fmt.Errorf("humanauth: unknown oauth provider %q", p.Name) + } + if p.ClientID == "" || p.ClientSecret == "" { + return fmt.Errorf("humanauth: oauth provider %s needs a client id and secret", p.Name) + } + urls := []string{p.AuthURL, p.TokenURL, p.UserInfoURL} + if p.Name == "github" { + urls = append(urls, p.EmailsURL) + } + for _, u := range urls { + pu, err := url.Parse(u) + if err != nil || pu.Scheme != "https" || pu.Host == "" { + return fmt.Errorf("humanauth: oauth provider %s endpoints must be absolute https URLs", p.Name) + } + } + ru, err := url.Parse(p.RedirectURL) + if err != nil || (ru.Scheme != "https" && ru.Scheme != "http") || ru.Host == "" { + return fmt.Errorf("humanauth: oauth redirect base URL must be an absolute http(s) URL") + } + return nil +} + +// WithOAuthProvider enables sign-in with p. Invalid config is a startup error +// surfaced by NewService. +func WithOAuthProvider(p *OAuthProvider) Option { + return func(s *Service) { + if s.oauth == nil { + s.oauth = map[string]*OAuthProvider{} + } + s.oauth[p.Name] = p + } +} + +// OAuthConfigured reports whether provider is enabled. +func (s *Service) OAuthConfigured(provider string) bool { return s.oauth[provider] != nil } + +// StartOAuth creates a single-use state and returns the provider consent URL. +func (s *Service) StartOAuth(ctx context.Context, provider string) (string, error) { + p := s.oauth[provider] + if p == nil { + return "", ErrOAuthProviderNotConfigured + } + state, err := generateRawToken() + if err != nil { + return "", err + } + if err := s.db.CreateOAuthState(ctx, hashRawToken(state), provider, s.now().Add(OAuthStateTTL)); err != nil { + return "", fmt.Errorf("humanauth: store oauth state: %w", err) + } + q := url.Values{} + q.Set("client_id", p.ClientID) + q.Set("redirect_uri", p.RedirectURL) + q.Set("response_type", "code") + q.Set("scope", strings.Join(p.Scopes, " ")) + q.Set("state", state) + if provider == "google" { + q.Set("prompt", "select_account") + } + return p.AuthURL + "?" + q.Encode(), nil +} + +type oauthUser struct { + ID string + Email string + Name string + AvatarURL string +} + +// CompleteOAuth validates and consumes state, exchanges code, fetches the +// provider-verified email, resolves (link or create) the human, and returns a +// session - or *MFARequiredError when the account has MFA enabled, so OAuth +// never bypasses the second factor. +func (s *Service) CompleteOAuth(ctx context.Context, provider, code, state string) (Session, error) { + p := s.oauth[provider] + if p == nil { + return Session{}, ErrOAuthProviderNotConfigured + } + if state == "" { + return Session{}, ErrOAuthStateInvalid + } + // The state is looked up by its SHA-256 hash, so the comparison never + // touches the raw value; DELETE ... RETURNING-style consume makes it + // single-use even under concurrent callbacks. + ok, err := s.db.ConsumeOAuthState(ctx, hashRawToken(state), provider, s.now()) + if err != nil { + return Session{}, fmt.Errorf("humanauth: consume oauth state: %w", err) + } + if !ok { + return Session{}, ErrOAuthStateInvalid + } + if code == "" { + return Session{}, ErrOAuthProvider + } + token, err := p.exchange(ctx, code) + if err != nil { + return Session{}, err + } + u, err := p.fetchUser(ctx, token) + if err != nil { + return Session{}, err + } + name := strings.TrimSpace(u.Name) + if name == "" { + name = strings.SplitN(u.Email, "@", 2)[0] + } + if r := []rune(name); len(r) > 200 { + name = string(r[:200]) + } + var avatar *string + if strings.HasPrefix(u.AvatarURL, "https://") { + avatar = &u.AvatarURL + } + h, _, err := s.db.ResolveOAuthHuman(ctx, provider, u.ID, u.Email, name, avatar) + if err != nil { + return Session{}, fmt.Errorf("humanauth: resolve oauth human: %w", err) + } + if h.MFAEnabled { + return Session{}, s.newMFAChallenge(ctx, h) + } + return s.completeLogin(ctx, h) +} + +func (p *OAuthProvider) client() *http.Client { + if p.HTTPClient != nil { + return p.HTTPClient + } + return &http.Client{Timeout: 10 * time.Second} +} + +func (p *OAuthProvider) doJSON(req *http.Request, out any) error { + req.Header.Set("Accept", "application/json") + resp, err := p.client().Do(req) + if err != nil { + return fmt.Errorf("%w: %s request failed", ErrOAuthProvider, p.Name) + } + defer func() { _ = resp.Body.Close() }() + body, err := io.ReadAll(io.LimitReader(resp.Body, maxOAuthResponseBytes)) + if err != nil { + return fmt.Errorf("%w: %s read failed", ErrOAuthProvider, p.Name) + } + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("%w: %s returned HTTP %d", ErrOAuthProvider, p.Name, resp.StatusCode) + } + if err := json.Unmarshal(body, out); err != nil { + return fmt.Errorf("%w: %s returned malformed JSON", ErrOAuthProvider, p.Name) + } + return nil +} + +func (p *OAuthProvider) exchange(ctx context.Context, code string) (string, error) { + form := url.Values{} + form.Set("grant_type", "authorization_code") + form.Set("code", code) + form.Set("redirect_uri", p.RedirectURL) + form.Set("client_id", p.ClientID) + form.Set("client_secret", p.ClientSecret) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, p.TokenURL, strings.NewReader(form.Encode())) + if err != nil { + return "", err + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + var tok struct { + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + Error string `json:"error"` + } + if err := p.doJSON(req, &tok); err != nil { + return "", err + } + // GitHub reports exchange failures as HTTP 200 with an "error" field. + if tok.Error != "" || tok.AccessToken == "" { + return "", fmt.Errorf("%w: %s token exchange rejected", ErrOAuthProvider, p.Name) + } + return tok.AccessToken, nil +} + +func (p *OAuthProvider) authedGET(ctx context.Context, u, token string, out any) error { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil) + if err != nil { + return err + } + req.Header.Set("Authorization", "Bearer "+token) + return p.doJSON(req, out) +} + +func (p *OAuthProvider) fetchUser(ctx context.Context, token string) (oauthUser, error) { + switch p.Name { + case "google": + var g struct { + Sub string `json:"sub"` + Email string `json:"email"` + EmailVerified bool `json:"email_verified"` + Name string `json:"name"` + Picture string `json:"picture"` + } + if err := p.authedGET(ctx, p.UserInfoURL, token, &g); err != nil { + return oauthUser{}, err + } + // Linking by email is only safe for a provider-VERIFIED address. + if g.Sub == "" || g.Email == "" || !g.EmailVerified { + return oauthUser{}, fmt.Errorf("%w: google account has no verified email", ErrOAuthProvider) + } + return oauthUser{ID: g.Sub, Email: g.Email, Name: g.Name, AvatarURL: g.Picture}, nil + default: // github + var gh struct { + ID int64 `json:"id"` + Login string `json:"login"` + Name string `json:"name"` + AvatarURL string `json:"avatar_url"` + } + if err := p.authedGET(ctx, p.UserInfoURL, token, &gh); err != nil { + return oauthUser{}, err + } + var emails []struct { + Email string `json:"email"` + Primary bool `json:"primary"` + Verified bool `json:"verified"` + } + if err := p.authedGET(ctx, p.EmailsURL, token, &emails); err != nil { + return oauthUser{}, err + } + email := "" + for _, e := range emails { + if e.Primary && e.Verified { + email = e.Email + } + } + if gh.ID == 0 || email == "" { + return oauthUser{}, fmt.Errorf("%w: github account has no verified primary email", ErrOAuthProvider) + } + name := gh.Name + if name == "" { + name = gh.Login + } + return oauthUser{ID: strconv.FormatInt(gh.ID, 10), Email: email, Name: name, AvatarURL: gh.AvatarURL}, nil + } +} diff --git a/internal/humanauth/oauth_test.go b/internal/humanauth/oauth_test.go new file mode 100644 index 0000000..21a7744 --- /dev/null +++ b/internal/humanauth/oauth_test.go @@ -0,0 +1,182 @@ +package humanauth + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "net/url" + "testing" + "time" +) + +// fakeGoogle is an httptest TLS server standing in for Google's token and +// userinfo endpoints (mirrors internal/billing's fake-Paystack approach). +type fakeGoogle struct { + srv *httptest.Server + sub, email string + emailVerified bool + tokenStatus int +} + +func newFakeGoogle(t *testing.T) *fakeGoogle { + f := &fakeGoogle{sub: "g-123", email: "Grace@Example.com", emailVerified: true, tokenStatus: http.StatusOK} + mux := http.NewServeMux() + mux.HandleFunc("POST /token", func(w http.ResponseWriter, r *http.Request) { + _ = r.ParseForm() + if r.Form.Get("code") != "good-code" || r.Form.Get("client_secret") != "shh" || r.Form.Get("grant_type") != "authorization_code" { + w.WriteHeader(http.StatusBadRequest) + return + } + w.WriteHeader(f.tokenStatus) + _ = json.NewEncoder(w).Encode(map[string]string{"access_token": "at-1", "token_type": "Bearer"}) + }) + mux.HandleFunc("GET /userinfo", func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer at-1" { + w.WriteHeader(http.StatusUnauthorized) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{"sub": f.sub, "email": f.email, "email_verified": f.emailVerified, "name": "Grace Hopper", "picture": "https://img.example/g.png"}) + }) + f.srv = httptest.NewTLSServer(mux) + t.Cleanup(f.srv.Close) + return f +} + +func (f *fakeGoogle) provider() *OAuthProvider { + p := GoogleProvider("cid", "shh", "https://api.mailx.test") + p.AuthURL, p.TokenURL, p.UserInfoURL = f.srv.URL+"/auth", f.srv.URL+"/token", f.srv.URL+"/userinfo" + p.HTTPClient = f.srv.Client() + return p +} + +func startState(t *testing.T, svc *Service) string { + t.Helper() + u, err := svc.StartOAuth(context.Background(), "google") + if err != nil { + t.Fatal(err) + } + pu, _ := url.Parse(u) + q := pu.Query() + if q.Get("redirect_uri") != "https://api.mailx.test/v1/auth/oauth/google/callback" || q.Get("client_id") != "cid" || q.Get("state") == "" { + t.Fatalf("bad authorization url %s", u) + } + return q.Get("state") +} + +func TestOAuthNewAccountLinkAndState(t *testing.T) { + fg := newFakeGoogle(t) + db := newTestDB(t) + svc, err := NewService(db, testSecret(), WithOAuthProvider(fg.provider())) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + + // New account created. + s1, err := svc.CompleteOAuth(ctx, "google", "good-code", startState(t, svc)) + if err != nil || s1.AccessToken == "" || s1.Human.Email != "Grace@Example.com" { + t.Fatalf("new account: %v %+v", err, s1.Human) + } + // OAuth-created account has no usable password. + if _, err := svc.Login(ctx, "grace@example.com", "!oauth-no-password"); !errors.Is(err, ErrInvalidCredentials) { + t.Fatalf("oauth account password login: %v", err) + } + // Same identity again -> same human. + s2, err := svc.CompleteOAuth(ctx, "google", "good-code", startState(t, svc)) + if err != nil || s2.Human.ID != s1.Human.ID { + t.Fatalf("repeat login: %v", err) + } + + // Existing password account is LINKED (case-insensitive), not duplicated. + pw, err := svc.SignUp(ctx, "Alan", "alan@example.com", "password123") + if err != nil { + t.Fatal(err) + } + fg.sub, fg.email = "g-456", "ALAN@example.com" + s3, err := svc.CompleteOAuth(ctx, "google", "good-code", startState(t, svc)) + if err != nil || s3.Human.ID != pw.Human.ID { + t.Fatalf("link: %v got %s want %s", err, s3.Human.ID, pw.Human.ID) + } + + // State: missing, unknown, reused, wrong provider. + if _, err := svc.CompleteOAuth(ctx, "google", "good-code", ""); !errors.Is(err, ErrOAuthStateInvalid) { + t.Fatalf("missing state: %v", err) + } + if _, err := svc.CompleteOAuth(ctx, "google", "good-code", "deadbeef"); !errors.Is(err, ErrOAuthStateInvalid) { + t.Fatalf("unknown state: %v", err) + } + st := startState(t, svc) + if _, err := svc.CompleteOAuth(ctx, "google", "good-code", st); err != nil { + t.Fatal(err) + } + if _, err := svc.CompleteOAuth(ctx, "google", "good-code", st); !errors.Is(err, ErrOAuthStateInvalid) { + t.Fatalf("reused state: %v", err) + } + // Expired state. + st = startState(t, svc) + svc.now = func() time.Time { return time.Now().UTC().Add(OAuthStateTTL + time.Minute) } + if _, err := svc.CompleteOAuth(ctx, "google", "good-code", st); !errors.Is(err, ErrOAuthStateInvalid) { + t.Fatalf("expired state: %v", err) + } +} + +func TestOAuthProviderErrorsAndNotConfigured(t *testing.T) { + fg := newFakeGoogle(t) + svc, err := NewService(newTestDB(t), testSecret(), WithOAuthProvider(fg.provider())) + if err != nil { + t.Fatal(err) + } + ctx := context.Background() + if _, err := svc.CompleteOAuth(ctx, "google", "bad-code", startState(t, svc)); !errors.Is(err, ErrOAuthProvider) { + t.Fatalf("bad code: %v", err) + } + if _, err := svc.CompleteOAuth(ctx, "google", "", startState(t, svc)); !errors.Is(err, ErrOAuthProvider) { + t.Fatalf("denied consent: %v", err) + } + fg.tokenStatus = http.StatusInternalServerError + if _, err := svc.CompleteOAuth(ctx, "google", "good-code", startState(t, svc)); !errors.Is(err, ErrOAuthProvider) { + t.Fatalf("provider 500: %v", err) + } + fg.tokenStatus, fg.emailVerified = http.StatusOK, false + if _, err := svc.CompleteOAuth(ctx, "google", "good-code", startState(t, svc)); !errors.Is(err, ErrOAuthProvider) { + t.Fatalf("unverified email: %v", err) + } + if _, err := svc.StartOAuth(ctx, "github"); !errors.Is(err, ErrOAuthProviderNotConfigured) { + t.Fatalf("github not configured: %v", err) + } + if _, err := svc.CompleteOAuth(ctx, "github", "x", "y"); !errors.Is(err, ErrOAuthProviderNotConfigured) { + t.Fatalf("github callback not configured: %v", err) + } + // Non-https endpoints are rejected at construction. + p := fg.provider() + p.TokenURL = "http://evil.example/token" + if _, err := NewService(newTestDB(t), testSecret(), WithOAuthProvider(p)); err == nil { + t.Fatal("http token endpoint accepted") + } +} + +// OAuth must never bypass MFA on an MFA-enabled (linked) account. +func TestOAuthRespectsMFA(t *testing.T) { + f := newMFAService(t) + fg := newFakeGoogle(t) + WithOAuthProvider(fg.provider())(f.svc) + ctx := context.Background() + sess, err := f.svc.SignUp(ctx, "Grace", "grace@example.com", "password123") + if err != nil { + t.Fatal(err) + } + enr, err := f.svc.EnrollMFA(ctx, sess.Human.ID) + if err != nil { + t.Fatal(err) + } + if err := f.svc.ConfirmMFA(ctx, sess.Human.ID, codeAt(t, enr.Secret, *f.clock)); err != nil { + t.Fatal(err) + } + _, err = f.svc.CompleteOAuth(ctx, "google", "good-code", startState(t, f.svc)) + var mfa *MFARequiredError + if !errors.As(err, &mfa) { + t.Fatalf("oauth on MFA account returned %v, want MFARequiredError", err) + } +} diff --git a/internal/humanauth/service.go b/internal/humanauth/service.go index 793ce14..d80f01f 100644 --- a/internal/humanauth/service.go +++ b/internal/humanauth/service.go @@ -12,6 +12,7 @@ import ( "time" "github.com/Ferousco-dev/mailx/internal/database" + "github.com/Ferousco-dev/mailx/internal/secretbox" "golang.org/x/crypto/bcrypt" ) @@ -110,6 +111,8 @@ type Service struct { now func() time.Time mailer Mailer dashboardBaseURL string + mfaBox *secretbox.Box + oauth map[string]*OAuthProvider } // NewService constructs a Service. jwtSecret must be non-empty — callers @@ -127,6 +130,11 @@ func NewService(db *database.DB, jwtSecret []byte, opts ...Option) (*Service, er for _, opt := range opts { opt(s) } + for _, p := range s.oauth { + if err := p.validate(); err != nil { + return nil, err + } + } return s, nil } @@ -227,6 +235,17 @@ func (s *Service) Login(ctx context.Context, email, password string) (Session, e if err := bcrypt.CompareHashAndPassword([]byte(h.PasswordHash), []byte(password)); err != nil { return Session{}, ErrInvalidCredentials } + if h.MFAEnabled { + // Correct first factor on an MFA account: no session, only a + // challenge usable solely by VerifyMFA (DEC-230). + return Session{}, s.newMFAChallenge(ctx, h) + } + return s.completeLogin(ctx, h) +} + +// completeLogin mints the session for a fully authenticated human (password +// login, MFA verify, OAuth callback). +func (s *Service) completeLogin(ctx context.Context, h database.Human) (Session, error) { // Deliberately NOT mintSession here: recording the login and issuing // its refresh token must succeed or fail TOGETHER (see // TouchLoginAndCreateRefreshToken's doc) - a plain mintSession call diff --git a/internal/humanauth/totp.go b/internal/humanauth/totp.go new file mode 100644 index 0000000..74257ae --- /dev/null +++ b/internal/humanauth/totp.go @@ -0,0 +1,75 @@ +package humanauth + +import ( + "crypto/hmac" + "crypto/sha1" //nolint:gosec // RFC 6238 default algorithm; HMAC-SHA1 is not broken as a MAC. + "crypto/subtle" + "encoding/base32" + "encoding/binary" + "fmt" + "net/url" + "time" +) + +// Hand-rolled RFC 4226 (HOTP) / RFC 6238 (TOTP) — see DEC-229. Parameters are +// the authenticator-app defaults: HMAC-SHA1, 30-second step, 6 digits, and a +// ±1 step verification window for clock skew. +const ( + totpPeriod = 30 + totpDigits = 6 + totpSkew = 1 +) + +var totpB32 = base32.StdEncoding.WithPadding(base32.NoPadding) + +// hotp computes RFC 4226 section 5.3: HMAC-SHA1 over the 8-byte big-endian +// counter, dynamic truncation, modulo 10^digits. +func hotp(key []byte, counter uint64, digits int) string { + var msg [8]byte + binary.BigEndian.PutUint64(msg[:], counter) + mac := hmac.New(sha1.New, key) + mac.Write(msg[:]) + sum := mac.Sum(nil) + off := sum[len(sum)-1] & 0x0f + code := binary.BigEndian.Uint32(sum[off:off+4]) & 0x7fffffff + mod := uint32(1) + for i := 0; i < digits; i++ { + mod *= 10 + } + return fmt.Sprintf("%0*d", digits, code%mod) +} + +func totpStep(t time.Time) int64 { return t.Unix() / totpPeriod } + +// verifyTOTP returns the matched step for code within ±totpSkew of now. Every +// candidate is compared in constant time and all candidates are checked. +func verifyTOTP(key []byte, code string, now time.Time) (int64, bool) { + if len(code) != totpDigits { + return 0, false + } + cur := totpStep(now) + var matched int64 + ok := false + for d := int64(-totpSkew); d <= totpSkew; d++ { + s := cur + d + if s < 0 { + continue + } + if subtle.ConstantTimeCompare([]byte(hotp(key, uint64(s), totpDigits)), []byte(code)) == 1 && !ok { + matched, ok = s, true + } + } + return matched, ok +} + +// otpauthURI builds the Key URI Format understood by authenticator apps. +func otpauthURI(issuer, account string, key []byte) string { + label := url.PathEscape(issuer + ":" + account) + q := url.Values{} + q.Set("secret", totpB32.EncodeToString(key)) + q.Set("issuer", issuer) + q.Set("algorithm", "SHA1") + q.Set("digits", fmt.Sprint(totpDigits)) + q.Set("period", fmt.Sprint(totpPeriod)) + return "otpauth://totp/" + label + "?" + q.Encode() +} diff --git a/internal/ratelimit/policy.go b/internal/ratelimit/policy.go index 350e74b..26d4bfc 100644 --- a/internal/ratelimit/policy.go +++ b/internal/ratelimit/policy.go @@ -75,6 +75,12 @@ type Policy struct { // email-bombing concern PasswordResetIPRate exists for. OrgInviteRate float64 OrgInviteBurst int + // MFAVerifyIPRate/MFAVerifyIPBurst bound POST /v1/auth/mfa/verify and + // /v1/auth/mfa/confirm per client IP: their own, tight bucket because a + // 6-digit TOTP code is brute-forceable. A second, per-challenge cap + // (humanauth.MFAChallengeMaxAttempts) applies regardless of IP. + MFAVerifyIPRate float64 + MFAVerifyIPBurst int } const ( @@ -99,6 +105,7 @@ func DefaultPolicy() Policy { AuthIPRate: 1, AuthIPBurst: 10, PasswordResetIPRate: 1.0 / 60, PasswordResetIPBurst: 3, OrgInviteRate: 1.0 / 30, OrgInviteBurst: 10, + MFAVerifyIPRate: 1.0 / 12, MFAVerifyIPBurst: 5, } } @@ -127,6 +134,7 @@ func (p Policy) Validate() error { count("max recipients per message", p.MaxRecipientsPerMessage, 1000), rate("auth IP rate", p.AuthIPRate), count("auth IP burst", p.AuthIPBurst, maxBurst), rate("password reset IP rate", p.PasswordResetIPRate), count("password reset IP burst", p.PasswordResetIPBurst, maxBurst), + rate("mfa verify IP rate", p.MFAVerifyIPRate), count("mfa verify IP burst", p.MFAVerifyIPBurst, maxBurst), rate("org invite rate", p.OrgInviteRate), count("org invite burst", p.OrgInviteBurst, maxBurst), } { if e != nil { From fde1ab205ce44a4703439cc961fee19f249ea9bf Mon Sep 17 00:00:00 2001 From: Feranmi Oresajo Date: Fri, 25 Sep 2026 21:35:01 +0100 Subject: [PATCH 2/5] feat(billing): opt-in plan auto-renewal with reminders (v0.47 phase 3c, RSK-044) --- cmd/mailx/billingconfig.go | 34 +- cmd/mailx/serve.go | 14 +- internal/api/billing_handler.go | 92 +++- internal/api/billing_renewal.go | 227 +++++++++ internal/api/billing_renewal_test.go | 457 ++++++++++++++++++ internal/api/openapi.go | 16 +- internal/api/routes.go | 1 + internal/billing/paystack.go | 8 + internal/billing/renewal.go | 165 +++++++ .../000032_plan_auto_renew.down.sql | 9 + .../migrations/000032_plan_auto_renew.up.sql | 36 ++ internal/database/plans.go | 5 +- internal/database/renewal.go | 314 ++++++++++++ internal/database/renewal_test.go | 160 ++++++ 14 files changed, 1525 insertions(+), 13 deletions(-) create mode 100644 internal/api/billing_renewal.go create mode 100644 internal/api/billing_renewal_test.go create mode 100644 internal/billing/renewal.go create mode 100644 internal/database/migrations/000032_plan_auto_renew.down.sql create mode 100644 internal/database/migrations/000032_plan_auto_renew.up.sql create mode 100644 internal/database/renewal.go create mode 100644 internal/database/renewal_test.go diff --git a/cmd/mailx/billingconfig.go b/cmd/mailx/billingconfig.go index 9601f81..dcfb22b 100644 --- a/cmd/mailx/billingconfig.go +++ b/cmd/mailx/billingconfig.go @@ -8,6 +8,7 @@ import ( "github.com/Ferousco-dev/mailx/internal/api" "github.com/Ferousco-dev/mailx/internal/billing" "github.com/Ferousco-dev/mailx/internal/database" + "github.com/Ferousco-dev/mailx/internal/secretbox" ) // buildBillingConfig wires Paystack billing from MAILX_PAYSTACK_SECRET_KEY. @@ -29,8 +30,24 @@ func buildBillingConfig(db *database.DB) (*api.BillingConfig, error) { if err != nil { return nil, err } + cfg := &api.BillingConfig{Paystack: ps, CallbackURL: os.Getenv("MAILX_PAYSTACK_CALLBACK_URL")} + // MAILX_BILLING_MASTER_KEY (base64, 32 bytes; its own key, never shared + // with the DKIM/webhook keys) encrypts saved card authorizations. Unset: + // auto-renewal is unavailable (reminders still go out); set but invalid: + // startup fails rather than silently disabling a money path. + if enc := os.Getenv("MAILX_BILLING_MASTER_KEY"); enc != "" { + key, err := secretbox.DecodeKey(enc, "MAILX_BILLING_MASTER_KEY") + if err != nil { + return nil, err + } + box, err := secretbox.New(key) + if err != nil { + return nil, err + } + cfg.AuthBox = box + } db.EnablePlanEnforcement() - return &api.BillingConfig{Paystack: ps, CallbackURL: os.Getenv("MAILX_PAYSTACK_CALLBACK_URL")}, nil + return cfg, nil } // enablePlanEnforcementFromEnv is the admin-CLI equivalent (no routes): it @@ -41,13 +58,15 @@ func enablePlanEnforcementFromEnv(db *database.DB) { } } -// planLapseInterval is how often lapsed paid plans are downgraded. +// planLapseInterval is how often the billing pass (reminders, auto-renewal +// charges, then lapsing) runs. const planLapseInterval = time.Hour -// runPlanLapse downgrades paid tenants whose period has ended. MVP LIMITATION -// (RSK-044): Plus/Pro do NOT auto-renew. No recurring charge is attempted; -// the owner must run checkout again each 30-day cycle or drop to Free. -func runPlanLapse(ctx context.Context, db *database.DB, o obs) error { +// runPlanLapse runs the hourly billing pass: renewer (reminders before every +// period end; opt-in auto-renewal charges, DEC-228..231) then lapse of any +// paid tenant whose period has ended (retention pinned, DEC-227). renewer may +// be nil (no system mailer): then nothing is reminded or charged. +func runPlanLapse(ctx context.Context, db *database.DB, renewer *api.Renewer, o obs) error { ticker := time.NewTicker(planLapseInterval) defer ticker.Stop() for { @@ -55,6 +74,9 @@ func runPlanLapse(ctx context.Context, db *database.DB, o obs) error { case <-ctx.Done(): return nil case <-ticker.C: + if renewer != nil { + renewer.RunOnce(ctx, time.Now().UTC()) + } n, err := db.DowngradeLapsedPlans(ctx, time.Now().UTC()) if err != nil { o.log.Warn("plan_lapse_failed", "error", err.Error()) diff --git a/cmd/mailx/serve.go b/cmd/mailx/serve.go index d12e566..eb22cef 100644 --- a/cmd/mailx/serve.go +++ b/cmd/mailx/serve.go @@ -120,7 +120,8 @@ func runFull() error { return err } humanAuthOpts := []humanauth.Option{humanauth.WithDashboardBaseURL(dashboardBaseURL())} - if mailer := buildSystemMailer(api.NewSubmissionAcceptor(db, store, dkimSvc, abuse.apiControls(o), ident.Name())); mailer != nil { + mailer := buildSystemMailer(api.NewSubmissionAcceptor(db, store, dkimSvc, abuse.apiControls(o), ident.Name())) + if mailer != nil { humanAuthOpts = append(humanAuthOpts, humanauth.WithMailer(mailer)) } else { o.log.Warn("system_mailer_disabled", "hint", "MAILX_SYSTEM_TENANT_ID/MAILX_SYSTEM_FROM_ADDRESS not set: password reset tokens will be created but no email will be sent") @@ -179,7 +180,16 @@ func runFull() error { o.logged("retention-purge", func(ctx context.Context) error { return runRetentionPurge(ctx, db, store, o) }), } if billingCfg != nil { - components = append(components, o.logged("plan-lapse", func(ctx context.Context) error { return runPlanLapse(ctx, db, o) })) + var renewer *api.Renewer + if mailer != nil { // never pass a typed-nil *systemMailer as humanauth.Mailer + renewer = api.NewRenewer(db, billingCfg, mailer, o.log) + } else { + o.log.Warn("billing_renewal_disabled", "hint", "no system mailer: plan reminders and auto-renewal charges are off (a charge always requires a prior reminder)") + } + if billingCfg.AuthBox == nil { + o.log.Warn("billing_auto_renew_unavailable", "hint", "MAILX_BILLING_MASTER_KEY not set: cards are not saved and auto-renewal cannot be enabled; reminders still sent") + } + components = append(components, o.logged("plan-lapse", func(ctx context.Context) error { return runPlanLapse(ctx, db, renewer, o) })) } if addr := observabilityAddr(); addr != "" { op := observability.NewServer(addr, observability.OperatorMux(o.metrics, ready)) diff --git a/internal/api/billing_handler.go b/internal/api/billing_handler.go index bd32396..d2097cd 100644 --- a/internal/api/billing_handler.go +++ b/internal/api/billing_handler.go @@ -11,6 +11,7 @@ import ( "github.com/Ferousco-dev/mailx/internal/billing" "github.com/Ferousco-dev/mailx/internal/database" + "github.com/Ferousco-dev/mailx/internal/secretbox" ) // BillingConfig enables /v1/billing/* (v0.47 phase 2, Paystack). Nil — the @@ -21,6 +22,10 @@ import ( type BillingConfig struct { Paystack *billing.Paystack CallbackURL string + // AuthBox encrypts saved Paystack card authorizations at rest + // (MAILX_BILLING_MASTER_KEY). Nil disables auto-renewal entirely: no + // authorization is stored and auto_renew cannot be turned on (DEC-229). + AuthBox *secretbox.Box } // planPeriod is how long one successful charge keeps a paid plan active. @@ -123,6 +128,8 @@ type subscriptionResponse struct { Plan string `json:"plan"` Status string `json:"status"` CurrentPeriodEnd *string `json:"current_period_end"` + AutoRenew bool `json:"auto_renew"` + CardOnFile bool `json:"card_on_file"` } // handleSubscription returns an org's plan to any of its members. @@ -152,7 +159,12 @@ func (h *billingHandler) handleSubscription(w http.ResponseWriter, r *http.Reque writeError(w, r, newError(ErrInternal, "internal_error", "failed to load plan")) return } - resp := subscriptionResponse{TenantID: tenantID, Plan: tp.Plan, Status: tp.Status} + rs, err := h.db.GetRenewalSettings(r.Context(), tenantID) + if err != nil { + writeError(w, r, newError(ErrInternal, "internal_error", "failed to load plan")) + return + } + resp := subscriptionResponse{TenantID: tenantID, Plan: tp.Plan, Status: tp.Status, AutoRenew: rs.AutoRenew, CardOnFile: rs.CardOnFile} if tp.CurrentPeriodEnd != nil { s := tp.CurrentPeriodEnd.UTC().Format(time.RFC3339) resp.CurrentPeriodEnd = &s @@ -215,7 +227,83 @@ func (h *billingHandler) handleWebhook(w http.ResponseWriter, r *http.Request) { h.log.Error("billing_webhook_apply_failed", "error", err.Error()) writeError(w, r, newError(ErrInternal, "internal_error", "failed to apply payment")) default: - h.log.Info("billing_plan_applied", "tenant_id", d.Metadata.TenantID, "plan", plan.ID) + h.log.Info("billing_plan_applied", "tenant_id", d.Metadata.TenantID, "plan", plan.ID, "reference", d.Reference) + h.saveAuthorization(r, ev) ack("applied") } } + +// saveAuthorization stores a reusable card authorization from a verified, +// just-applied charge.success, encrypted and bound to the tenant id. It is +// only a capability: no charge is ever made unless the owner has turned +// auto_renew on (DEC-229). Failures are logged, never surfaced to Paystack +// (the payment itself is already applied; a retry would only be a replay). +// The authorization code itself is never logged. +func (h *billingHandler) saveAuthorization(r *http.Request, ev billing.Event) { + d := ev.Data + if h.cfg.AuthBox == nil || !d.Authorization.Reusable || d.Authorization.AuthorizationCode == "" || d.Customer.Email == "" { + return + } + ct, nonce, err := h.cfg.AuthBox.Encrypt([]byte(d.Authorization.AuthorizationCode), []byte(d.Metadata.TenantID)) + if err == nil { + err = h.db.SaveRenewalAuthorization(r.Context(), d.Metadata.TenantID, ct, nonce, d.Customer.Email) + } + if err != nil { + h.log.Error("billing_authorization_save_failed", "tenant_id", d.Metadata.TenantID, "error", err.Error()) + return + } + h.log.Info("billing_authorization_saved", "tenant_id", d.Metadata.TenantID) +} + +type autoRenewRequest struct { + TenantID string `json:"tenant_id"` + AutoRenew *bool `json:"auto_renew"` +} + +// handleAutoRenew lets an org OWNER opt in to (or out of) automatic renewal. +// It never charges and accepts no amount: renewal charges are made later by +// the Renewer at the server-side plan price, only after a reminder (DEC-230). +func (h *billingHandler) handleAutoRenew(w http.ResponseWriter, r *http.Request) { + humanID, ok := humanIDFromContext(r.Context()) + if !ok { + writeError(w, r, newError(ErrAuthentication, "invalid_access_token", "missing or invalid access token")) + return + } + if !acceptsJSONContentType(r.Header.Get("Content-Type")) { + writeError(w, r, newError(ErrUnsupportedMediaType, "unsupported_media_type", "Content-Type must be application/json")) + return + } + var req autoRenewRequest + if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, maxBodyBytes)).Decode(&req); err != nil { + writeError(w, r, newError(ErrInvalidRequest, "invalid_json", "request body is not valid JSON")) + return + } + if req.TenantID == "" { + writeError(w, r, newError(ErrValidation, "invalid_tenant", "tenant_id is required")) + return + } + if req.AutoRenew == nil { + writeError(w, r, newError(ErrValidation, "invalid_auto_renew", "auto_renew (boolean) is required")) + return + } + isOwner, err := h.db.IsTenantOwner(r.Context(), req.TenantID, humanID) + if err != nil { + writeError(w, r, newError(ErrInternal, "internal_error", "failed to check organization ownership")) + return + } + if !isOwner { + writeError(w, r, newError(ErrForbidden, "not_org_owner", "only an organization owner can change automatic renewal")) + return + } + if *req.AutoRenew && h.cfg.AuthBox == nil { + writeError(w, r, newError(ErrTemporarilyUnavailable, "auto_renew_unavailable", "automatic renewal is not available on this deployment")) + return + } + rs, err := h.db.SetAutoRenew(r.Context(), req.TenantID, *req.AutoRenew) + if err != nil { + writeError(w, r, newError(ErrInternal, "internal_error", "failed to update automatic renewal")) + return + } + h.log.Info("billing_auto_renew_set", "tenant_id", req.TenantID, "auto_renew", rs.AutoRenew, "by_human_id", humanID) + writeJSON(w, http.StatusOK, map[string]any{"tenant_id": req.TenantID, "auto_renew": rs.AutoRenew, "card_on_file": rs.CardOnFile}) +} diff --git a/internal/api/billing_renewal.go b/internal/api/billing_renewal.go new file mode 100644 index 0000000..b2e8674 --- /dev/null +++ b/internal/api/billing_renewal.go @@ -0,0 +1,227 @@ +package api + +import ( + "context" + "errors" + "fmt" + "log/slog" + "strings" + "time" + + "github.com/Ferousco-dev/mailx/internal/billing" + "github.com/Ferousco-dev/mailx/internal/database" + "github.com/Ferousco-dev/mailx/internal/humanauth" + "github.com/Ferousco-dev/mailx/internal/observability" +) + +// Renewer runs one hourly pass of plan reminders and opt-in auto-renewal +// charges (v0.47 phase 3c, DEC-228..231). Order per pass: reminders, then +// reconcile unknown-outcome charges, then new charges. Lapsing stays in +// database.DowngradeLapsedPlans (called by cmd/mailx after RunOnce). +// +// Every charge is MailX-initiated (Paystack charge_authorization with the +// saved, encrypted authorization) - never Paystack's own subscription engine. +type Renewer struct { + db *database.DB + cfg *BillingConfig + mailer humanauth.Mailer + log *slog.Logger +} + +// NewRenewer builds a Renewer. mailer nil means no reminder can be sent, and +// therefore (by design) no auto-renewal charge is ever made either: a charge +// requires a sent "charge is coming" reminder. +func NewRenewer(db *database.DB, cfg *BillingConfig, mailer humanauth.Mailer, log *slog.Logger) *Renewer { + if log == nil { + log = observability.Discard() + } + return &Renewer{db: db, cfg: cfg, mailer: mailer, log: log} +} + +// RunOnce performs one pass at now. Errors for individual tenants are logged +// and do not stop the pass. +func (r *Renewer) RunOnce(ctx context.Context, now time.Time) { + if r.mailer == nil { + return + } + r.sendReminders(ctx, now) + if r.cfg.AuthBox == nil { + return + } + r.reconcilePending(ctx, now) + ids, err := r.db.RenewalCandidates(ctx, now) + if err != nil { + r.log.Warn("billing_renewal_candidates_failed", "error", err.Error()) + return + } + for _, id := range ids { + r.renewOne(ctx, id, now) + } +} + +func usd(cents int64) string { return fmt.Sprintf("$%d.%02d", cents/100, cents%100) } + +func (r *Renewer) email(ctx context.Context, tenantID, subject, body string) { + owners, err := r.db.TenantOwnerEmails(ctx, tenantID) + if err != nil { + r.log.Warn("billing_email_owners_failed", "tenant_id", tenantID, "error", err.Error()) + return + } + for _, to := range owners { + if err := r.mailer.SendSystemEmail(ctx, to, subject, body, ""); err != nil { + r.log.Warn("billing_email_failed", "tenant_id", tenantID, "subject", subject, "error", err.Error()) + } + } +} + +func (r *Renewer) sendReminders(ctx context.Context, now time.Time) { + claimed, err := r.db.ClaimPlanReminders(ctx, now, billing.ReminderLead) + if err != nil { + r.log.Warn("billing_reminder_claim_failed", "error", err.Error()) + return + } + for _, rem := range claimed { + plan := billing.PlanFor(rem.Plan) + end := rem.PeriodEnd.UTC().Format("2006-01-02 15:04 MST") + var subject, body string + if rem.AutoRenew { + subject = fmt.Sprintf("Your MailX %s plan will renew automatically", plan.ID) + body = fmt.Sprintf("Your organization %q is on the MailX %s plan, which ends on %s.\n\n"+ + "Automatic renewal is ON: we will charge %s (USD) to your saved card, no earlier than %s, to renew for another 30 days.\n\n"+ + "To avoid this charge, turn off automatic renewal in your billing settings before then.\n", + rem.TenantName, plan.ID, end, usd(plan.PriceUSDCents), + rem.PeriodEnd.Add(-billing.RenewalChargeWindow).UTC().Format("2006-01-02 15:04 MST")) + } else { + subject = fmt.Sprintf("Your MailX %s plan ends soon", plan.ID) + body = fmt.Sprintf("Your organization %q is on the MailX %s plan, which ends on %s.\n\n"+ + "Automatic renewal is OFF, so you will not be charged. To keep the plan, renew it (%s for 30 days) from your billing settings before it ends; otherwise the organization moves to the Free plan.\n", + rem.TenantName, plan.ID, end, usd(plan.PriceUSDCents)) + } + owners, err := r.db.TenantOwnerEmails(ctx, rem.TenantID) + sent := 0 + if err == nil { + for _, to := range owners { + if serr := r.mailer.SendSystemEmail(ctx, to, subject, body, ""); serr == nil { + sent++ + } else { + err = serr + } + } + } + if sent == 0 { + // Nobody was told: release so the next pass retries. Without a + // sent auto-renew reminder no charge can be claimed. + reason := "no owner" + if err != nil { + reason = err.Error() + } + r.log.Warn("billing_reminder_send_failed", "tenant_id", rem.TenantID, "error", reason) + if rerr := r.db.ReleasePlanReminder(ctx, rem.TenantID, rem.PeriodEnd); rerr != nil { + r.log.Warn("billing_reminder_release_failed", "tenant_id", rem.TenantID, "error", rerr.Error()) + } + continue + } + r.log.Info("billing_reminder_sent", "tenant_id", rem.TenantID, "plan", rem.Plan, "auto_renew", rem.AutoRenew, "period_end", rem.PeriodEnd.UTC().Format(time.RFC3339)) + } +} + +func (r *Renewer) renewOne(ctx context.Context, tenantID string, now time.Time) { + c, err := r.db.ClaimRenewalAttempt(ctx, tenantID, now) + if err != nil { + r.log.Warn("billing_renewal_claim_failed", "tenant_id", tenantID, "error", err.Error()) + return + } + if c == nil { + return + } + logAttempt := []any{"tenant_id", c.TenantID, "reference", c.Reference, "attempt", c.Attempt, "plan", c.Plan, "amount", c.Amount} + authCode, err := r.cfg.AuthBox.Decrypt(c.AuthCiphertext, c.AuthNonce, []byte(c.TenantID)) + if err != nil { + // Nothing was sent to Paystack: this attempt definitively failed. + r.log.Error("billing_renewal_auth_decrypt_failed", logAttempt...) + r.settle(ctx, c, billing.ChargeResult{Outcome: billing.ChargeFailed, Reason: "saved card could not be read; please renew manually"}, now) + return + } + r.log.Info("billing_renewal_attempt", logAttempt...) + res, err := r.cfg.Paystack.ChargeAuthorization(ctx, c.Email, string(authCode), billing.PlanFor(c.Plan), c.Reference, + billing.Metadata{TenantID: c.TenantID, Plan: c.Plan}) + if err != nil { + r.log.Warn("billing_renewal_charge_error", append(logAttempt, "error", err.Error())...) + } + r.settle(ctx, c, res, now) +} + +func (r *Renewer) reconcilePending(ctx context.Context, now time.Time) { + pending, err := r.db.PendingRenewalAttempts(ctx, now.Add(-billing.RenewalReconcileAfter)) + if err != nil { + r.log.Warn("billing_renewal_pending_failed", "error", err.Error()) + return + } + for i := range pending { + c := &pending[i] + res, err := r.cfg.Paystack.VerifyTransaction(ctx, c.Reference) + if err != nil { + r.log.Warn("billing_renewal_verify_error", "tenant_id", c.TenantID, "reference", c.Reference, "error", err.Error()) + } + r.log.Info("billing_renewal_reconciled", "tenant_id", c.TenantID, "reference", c.Reference, "outcome", int(res.Outcome)) + r.settle(ctx, c, res, now) + } +} + +// settle applies a charge result to a pending attempt. Unknown outcomes stay +// pending (blocking any further attempt for the period) until verified. +func (r *Renewer) settle(ctx context.Context, c *database.RenewalClaim, res billing.ChargeResult, now time.Time) { + logAttempt := []any{"tenant_id", c.TenantID, "reference", c.Reference, "attempt", c.Attempt, "plan", c.Plan, "amount", c.Amount} + plan := billing.PlanFor(c.Plan) + switch res.Outcome { + case billing.ChargeUnknown: + r.log.Warn("billing_renewal_outcome_unknown", logAttempt...) + return + case billing.ChargeSucceeded: + if res.Amount != c.Amount || !strings.EqualFold(res.Currency, billing.Currency) { + // Money moved but not what we asked for: never apply, never retry. + // Stays pending (blocks further charges) and is logged every pass + // for manual resolution by the operator. + r.log.Error("billing_renewal_amount_mismatch", append(logAttempt, "paid_amount", res.Amount, "paid_currency", res.Currency)...) + return + } + err := r.db.ApplyPlanPayment(ctx, database.Payment{ + Reference: c.Reference, TenantID: c.TenantID, Plan: c.Plan, Amount: res.Amount, Currency: billing.Currency, + }, planPeriod) + if err != nil && !errors.Is(err, database.ErrPaymentAlreadyApplied) { + // Leave pending: reconcile re-verifies and re-applies (idempotent). + r.log.Error("billing_renewal_apply_failed", append(logAttempt, "error", err.Error())...) + return + } + if ok, ferr := r.db.FinishRenewalAttempt(ctx, c.Reference, true, "", now); ferr != nil || !ok { + r.log.Warn("billing_renewal_finish_failed", logAttempt...) + } + r.log.Info("billing_renewal_succeeded", append(logAttempt, "already_applied", err != nil)...) + tp, _ := r.db.GetTenantPlan(ctx, c.TenantID) + until := "the next 30 days" + if tp.CurrentPeriodEnd != nil { + until = tp.CurrentPeriodEnd.UTC().Format("2006-01-02 15:04 MST") + } + r.email(ctx, c.TenantID, fmt.Sprintf("Your MailX %s plan was renewed", plan.ID), + fmt.Sprintf("We charged %s (USD) to your saved card to renew the MailX %s plan for %q. Your plan is now active until %s.\n\nPayment reference: %s\n", + usd(c.Amount), plan.ID, c.TenantName, until, c.Reference)) + case billing.ChargeFailed: + reason := strings.TrimSpace(res.Reason) + if reason == "" { + reason = "the payment was declined" + } + if _, err := r.db.FinishRenewalAttempt(ctx, c.Reference, false, reason, now); err != nil { + r.log.Error("billing_renewal_finish_failed", append(logAttempt, "error", err.Error())...) + return + } + r.log.Warn("billing_renewal_failed", append(logAttempt, "reason", reason)...) + next := "If time remains before your plan ends, we will try again in about 12 hours." + if c.Attempt >= billing.RenewalMaxAttempts { + next = fmt.Sprintf("This was the final automatic attempt. Your plan ends on %s and the organization will move to the Free plan unless you renew manually from your billing settings.", + c.PeriodEnd.UTC().Format("2006-01-02 15:04 MST")) + } + r.email(ctx, c.TenantID, fmt.Sprintf("Automatic renewal of your MailX %s plan failed", plan.ID), + fmt.Sprintf("We could not charge your saved card %s (USD) to renew the MailX %s plan for %q (attempt %d of %d).\n\nReason from the payment provider: %s\n\n%s\n", + usd(c.Amount), plan.ID, c.TenantName, c.Attempt, billing.RenewalMaxAttempts, reason, next)) + } +} diff --git a/internal/api/billing_renewal_test.go b/internal/api/billing_renewal_test.go new file mode 100644 index 0000000..bc04139 --- /dev/null +++ b/internal/api/billing_renewal_test.go @@ -0,0 +1,457 @@ +package api + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + "github.com/Ferousco-dev/mailx/internal/auth" + "github.com/Ferousco-dev/mailx/internal/billing" + "github.com/Ferousco-dev/mailx/internal/database" + "github.com/Ferousco-dev/mailx/internal/humanauth" + "github.com/Ferousco-dev/mailx/internal/secretbox" + "github.com/Ferousco-dev/mailx/internal/storage" +) + +// fakeChargeServer imitates Paystack's charge_authorization and verify +// endpoints. mode: "success", "decline", "error500" (charge taken but the +// response is a 5xx - the ambiguous case). +type fakeChargeServer struct { + mu sync.Mutex + mode string + delay time.Duration + charges []map[string]any + charged map[string]bool // references Paystack actually charged + refs map[string]int // every reference ever submitted (dup detection) +} + +func (f *fakeChargeServer) server(t *testing.T) *httptest.Server { + f.charged, f.refs = map[string]bool{}, map[string]int{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer "+testPaystackSecret { + w.WriteHeader(http.StatusUnauthorized) + return + } + if f.delay > 0 { + time.Sleep(f.delay) + } + f.mu.Lock() + defer f.mu.Unlock() + switch { + case r.URL.Path == "/transaction/charge_authorization": + var body map[string]any + _ = json.NewDecoder(r.Body).Decode(&body) + ref, _ := body["reference"].(string) + f.refs[ref]++ + if f.refs[ref] > 1 { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"status":false,"message":"Duplicate Transaction Reference"}`)) + return + } + f.charges = append(f.charges, body) + amt := int64(body["amount"].(float64)) + switch f.mode { + case "decline": + _, _ = w.Write([]byte(`{"status":true,"message":"Charge attempted","data":{"status":"failed","reference":"` + ref + `","amount":` + itoa(amt) + `,"currency":"USD","gateway_response":"Insufficient Funds"}}`)) + case "error500": + f.charged[ref] = true + w.WriteHeader(http.StatusBadGateway) + default: + f.charged[ref] = true + _, _ = w.Write([]byte(`{"status":true,"message":"Charge attempted","data":{"status":"success","reference":"` + ref + `","amount":` + itoa(amt) + `,"currency":"USD","gateway_response":"Approved"}}`)) + } + case strings.HasPrefix(r.URL.Path, "/transaction/verify/"): + ref := strings.TrimPrefix(r.URL.Path, "/transaction/verify/") + if !f.charged[ref] { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"status":false,"message":"Transaction reference not found"}`)) + return + } + _, _ = w.Write([]byte(`{"status":true,"message":"Verification successful","data":{"status":"success","reference":"` + ref + `","amount":600,"currency":"USD","gateway_response":"Approved"}}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(srv.Close) + return srv +} + +func itoa(n int64) string { b, _ := json.Marshal(n); return string(b) } + +func (f *fakeChargeServer) count() int { f.mu.Lock(); defer f.mu.Unlock(); return len(f.charges) } + +type sentMail struct{ to, subject, body string } + +type captureMailer struct { + mu sync.Mutex + sent []sentMail +} + +func (m *captureMailer) SendSystemEmail(_ context.Context, to, subject, text, _ string) error { + m.mu.Lock() + defer m.mu.Unlock() + m.sent = append(m.sent, sentMail{to, subject, text}) + return nil +} + +func (m *captureMailer) matching(sub string) []sentMail { + m.mu.Lock() + defer m.mu.Unlock() + var out []sentMail + for _, s := range m.sent { + if strings.Contains(s.subject, sub) { + out = append(out, s) + } + } + return out +} + +type renewalFixture struct { + db *database.DB + svc *humanauth.Service + cfg *BillingConfig + mux http.Handler + fc *fakeChargeServer + mailer *captureMailer + renewer *Renewer + orgID string + owner humanauth.Session + end time.Time // current period end after the initial checkout +} + +const testAuthCode = "AUTH_supersecret_code" + +// newRenewalFixture: an owner with a Plus org paid via checkout (period E), +// with a reusable card saved through the real webhook path. +func newRenewalFixture(t *testing.T, withBox bool) *renewalFixture { + t.Helper() + db := newTestDB(t) + db.EnablePlanEnforcement() + store, err := storage.NewFileStore(t.TempDir()) + if err != nil { + t.Fatal(err) + } + svc, err := humanauth.NewService(db, []byte("test-secret-at-least-32-bytes-long!!")) + if err != nil { + t.Fatal(err) + } + fc := &fakeChargeServer{} + ps, err := billing.NewPaystack(testPaystackSecret, fc.server(t).URL) + if err != nil { + t.Fatal(err) + } + cfg := &BillingConfig{Paystack: ps} + if withBox { + box, err := secretbox.New([]byte("0123456789abcdef0123456789abcdef")) + if err != nil { + t.Fatal(err) + } + cfg.AuthBox = box + } + mux := newMux(newEmailHandler(db, store), auth.NewService(db, nil), func() error { return nil }, + routeServices{humanAuth: svc, billing: cfg}) + mailer := &captureMailer{} + f := &renewalFixture{db: db, svc: svc, cfg: cfg, mux: mux, fc: fc, mailer: mailer, renewer: NewRenewer(db, cfg, mailer, nil)} + ctx := context.Background() + f.owner, err = svc.SignUp(ctx, "Ada", "ada@example.com", "hunter22hunter") + if err != nil { + t.Fatal(err) + } + org, err := svc.CreateOrganization(ctx, f.owner.Human.ID, "Acme", "") + if err != nil { + t.Fatal(err) + } + f.orgID = org.ID + raw := []byte(`{"event":"charge.success","data":{"reference":"ref_checkout_1","status":"success","amount":600,"currency":"USD", + "metadata":{"tenant_id":"` + org.ID + `","plan":"plus"},"customer":{"customer_code":"CUS_1","email":"ada@example.com"}, + "authorization":{"authorization_code":"` + testAuthCode + `","reusable":true}}}`) + if rec := f.webhook(t, raw); rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), "applied") { + t.Fatalf("checkout webhook: %d %s", rec.Code, rec.Body) + } + f.end = f.periodEnd(t) + return f +} + +func (f *renewalFixture) webhook(t *testing.T, raw []byte) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest("POST", "/v1/billing/webhook", strings.NewReader(string(raw))) + req.Header.Set("x-paystack-signature", f.cfg.Paystack.Sign(raw)) + rec := httptest.NewRecorder() + f.mux.ServeHTTP(rec, req) + return rec +} + +func (f *renewalFixture) patch(t *testing.T, token string, body string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest("PATCH", "/v1/billing/auto-renew", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + rec := httptest.NewRecorder() + f.mux.ServeHTTP(rec, req) + return rec +} + +func (f *renewalFixture) periodEnd(t *testing.T) time.Time { + t.Helper() + tp, err := f.db.GetTenantPlan(context.Background(), f.orgID) + if err != nil || tp.CurrentPeriodEnd == nil { + t.Fatalf("plan: %+v %v", tp, err) + } + return *tp.CurrentPeriodEnd +} + +func (f *renewalFixture) enable(t *testing.T) { + t.Helper() + if _, err := f.db.SetAutoRenew(context.Background(), f.orgID, true); err != nil { + t.Fatal(err) + } +} + +func TestAutoRenewToggleOwnerOnlyAndNeverCharges(t *testing.T) { + f := newRenewalFixture(t, true) + other, err := f.svc.SignUp(context.Background(), "Eve", "eve@example.com", "hunter22hunter") + if err != nil { + t.Fatal(err) + } + on := `{"tenant_id":"` + f.orgID + `","auto_renew":true,"amount":1,"plan":"pro"}` + if rec := f.patch(t, "", on); rec.Code != http.StatusUnauthorized { + t.Fatalf("no token: %d", rec.Code) + } + if rec := f.patch(t, other.AccessToken, on); rec.Code != http.StatusForbidden { + t.Fatalf("non-owner: %d %s", rec.Code, rec.Body) + } + if rec := f.patch(t, f.owner.AccessToken, `{"tenant_id":"`+f.orgID+`"}`); rec.Code != http.StatusUnprocessableEntity && rec.Code != http.StatusBadRequest { + t.Fatalf("missing auto_renew: %d", rec.Code) + } + rs, _ := f.db.GetRenewalSettings(context.Background(), f.orgID) + if rs.AutoRenew || !rs.CardOnFile { + t.Fatalf("default must be off with card saved from checkout: %+v", rs) + } + rec := f.patch(t, f.owner.AccessToken, on) + if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `"auto_renew":true`) || !strings.Contains(rec.Body.String(), `"card_on_file":true`) { + t.Fatalf("owner enable: %d %s", rec.Code, rec.Body) + } + // The toggle never charges, and client-supplied amount/plan are ignored. + if f.fc.count() != 0 { + t.Fatalf("toggle triggered %d charges", f.fc.count()) + } + if tp, _ := f.db.GetTenantPlan(context.Background(), f.orgID); tp.Plan != "plus" { + t.Fatalf("toggle changed plan: %+v", tp) + } + + // Without MAILX_BILLING_MASTER_KEY auto-renew cannot be enabled, and no + // card is ever stored. + g := newRenewalFixture(t, false) + if rec := g.patch(t, g.owner.AccessToken, `{"tenant_id":"`+g.orgID+`","auto_renew":true}`); rec.Code != http.StatusServiceUnavailable { + t.Fatalf("no box: %d %s", rec.Code, rec.Body) + } + if rs, _ := g.db.GetRenewalSettings(context.Background(), g.orgID); rs.CardOnFile || rs.AutoRenew { + t.Fatalf("no box must store nothing: %+v", rs) + } +} + +func TestRenewalReminderExactlyOncePerPeriod(t *testing.T) { + for _, autoRenew := range []bool{false, true} { + f := newRenewalFixture(t, true) + if autoRenew { + f.enable(t) + } + ctx := context.Background() + f.renewer.RunOnce(ctx, f.end.Add(-80*time.Hour)) + if n := len(f.mailer.sent); n != 0 { + t.Fatalf("reminder too early: %d", n) + } + for _, h := range []int{71, 70, 60, 50} { + f.renewer.RunOnce(ctx, f.end.Add(-time.Duration(h)*time.Hour)) + } + want := "ends soon" + if autoRenew { + want = "will renew automatically" + } + got := f.mailer.matching("plan") + reminders := f.mailer.matching(want) + if len(reminders) != 1 || reminders[0].to != "ada@example.com" { + t.Fatalf("auto_renew=%v: want exactly 1 %q reminder, got %+v", autoRenew, want, got) + } + if autoRenew && !strings.Contains(reminders[0].body, "$6.00") { + t.Fatalf("auto-renew reminder must state the amount: %s", reminders[0].body) + } + if !autoRenew && f.fc.count() != 0 { + t.Fatalf("auto-renew off must never charge: %d", f.fc.count()) + } + } +} + +func TestAutoRenewSuccessExtendsFromPeriodEnd(t *testing.T) { + f := newRenewalFixture(t, true) + f.enable(t) + ctx := context.Background() + E := f.end + f.renewer.RunOnce(ctx, E.Add(-71*time.Hour)) // reminder + f.renewer.RunOnce(ctx, E.Add(-49*time.Hour)) // outside charge window + if f.fc.count() != 0 { + t.Fatal("charged before the charge window") + } + f.renewer.RunOnce(ctx, E.Add(-47*time.Hour)) + if f.fc.count() != 1 { + t.Fatalf("charges = %d, want 1", f.fc.count()) + } + c := f.fc.charges[0] + if c["amount"] != float64(600) || c["currency"] != "USD" || c["authorization_code"] != testAuthCode || c["email"] != "ada@example.com" || + !strings.HasPrefix(c["reference"].(string), "mailx-renew-"+f.orgID+"-") { + t.Fatalf("unexpected charge body %+v", c) + } + // Same-plan renewal extends from E (DEC-227), not from "now". + if got := f.periodEnd(t); !got.Equal(E.Add(planPeriod)) { + t.Fatalf("period end %v, want %v", got, E.Add(planPeriod)) + } + if len(f.mailer.matching("was renewed")) != 1 { + t.Fatalf("missing success email: %+v", f.mailer.sent) + } + // Paystack's own charge.success webhook for the same reference is a replay. + ref := c["reference"].(string) + raw := []byte(`{"event":"charge.success","data":{"reference":"` + ref + `","status":"success","amount":600,"currency":"USD","metadata":{"tenant_id":"` + f.orgID + `","plan":"plus"}}}`) + if rec := f.webhook(t, raw); !strings.Contains(rec.Body.String(), "already_applied") { + t.Fatalf("renewal webhook must be a replay: %s", rec.Body) + } + for _, h := range []int{46, 30, 20, 1} { + f.renewer.RunOnce(ctx, E.Add(-time.Duration(h)*time.Hour)) + } + if f.fc.count() != 1 || !f.periodEnd(t).Equal(E.Add(planPeriod)) { + t.Fatalf("extra charges/extensions: charges=%d end=%v", f.fc.count(), f.periodEnd(t)) + } +} + +func TestAutoRenewDeclinesAreBoundedThenLapseWithRetentionPinned(t *testing.T) { + f := newRenewalFixture(t, true) + f.fc.mode = "decline" + f.enable(t) + ctx := context.Background() + E := f.end + for _, h := range []int{71, 47, 46, 40, 35, 30, 23, 12, 2} { + f.renewer.RunOnce(ctx, E.Add(-time.Duration(h)*time.Hour)) + } + if f.fc.count() != billing.RenewalMaxAttempts { + t.Fatalf("charge attempts = %d, want %d", f.fc.count(), billing.RenewalMaxAttempts) + } + fails := f.mailer.matching("failed") + if len(fails) != billing.RenewalMaxAttempts || !strings.Contains(fails[0].body, "Insufficient Funds") || + !strings.Contains(fails[len(fails)-1].body, "final automatic attempt") { + t.Fatalf("failure emails: %+v", fails) + } + for _, m := range f.mailer.sent { + if strings.Contains(m.body, testAuthCode) { + t.Fatal("authorization code leaked into an email") + } + } + if !f.periodEnd(t).Equal(E) { + t.Fatal("declined renewal must not extend the period") + } + if n, err := f.db.DowngradeLapsedPlans(ctx, E.Add(time.Minute)); err != nil || n != 1 { + t.Fatalf("lapse: %d %v", n, err) + } + tn, err := f.db.GetTenant(ctx, f.orgID) + if err != nil { + t.Fatal(err) + } + if tp, _ := f.db.GetTenantPlan(ctx, f.orgID); tp.Plan != "free" || tp.Status != "lapsed" { + t.Fatalf("not lapsed: %+v", tp) + } + if tn.RetentionDays == nil || *tn.RetentionDays != 30 { + t.Fatalf("retention must be pinned to plus's 30 days on lapse, got %v", tn.RetentionDays) + } + f.renewer.RunOnce(ctx, E.Add(2*time.Hour)) + if f.fc.count() != billing.RenewalMaxAttempts { + t.Fatal("lapsed tenant was charged") + } +} + +func TestAutoRenewRequiresNoticeAfterLateOptIn(t *testing.T) { + f := newRenewalFixture(t, true) + ctx := context.Background() + E := f.end + f.renewer.RunOnce(ctx, E.Add(-71*time.Hour)) // "ends soon" (auto-renew off) + f.enable(t) // late opt-in resets the reminder + f.renewer.RunOnce(ctx, E.Add(-30*time.Hour)) // "will renew" reminder; too soon to charge + f.renewer.RunOnce(ctx, E.Add(-10*time.Hour)) + if f.fc.count() != 0 { + t.Fatalf("charged with < %v notice", billing.RenewalMinNotice) + } + if len(f.mailer.matching("will renew automatically")) != 1 { + t.Fatalf("late opt-in reminder missing: %+v", f.mailer.sent) + } + f.renewer.RunOnce(ctx, E.Add(-5*time.Hour)) + if f.fc.count() != 1 { + t.Fatalf("charges = %d, want 1 after notice elapsed", f.fc.count()) + } +} + +func TestAutoRenewUnknownOutcomeIsReconciledNeverRecharged(t *testing.T) { + f := newRenewalFixture(t, true) + f.fc.mode = "error500" // Paystack took the money, but we got a 502 + f.enable(t) + ctx := context.Background() + E := f.end + f.renewer.RunOnce(ctx, E.Add(-71*time.Hour)) + f.renewer.RunOnce(ctx, E.Add(-47*time.Hour)) + f.renewer.RunOnce(ctx, E.Add(-47*time.Hour+30*time.Minute)) // too fresh to verify, and pending blocks + if f.fc.count() != 1 || !f.periodEnd(t).Equal(E) { + t.Fatalf("after ambiguous charge: charges=%d end=%v", f.fc.count(), f.periodEnd(t)) + } + f.renewer.RunOnce(ctx, E.Add(-35*time.Hour)) // verify -> success -> apply once + if f.fc.count() != 1 { + t.Fatalf("ambiguous charge was retried: %d", f.fc.count()) + } + if got := f.periodEnd(t); !got.Equal(E.Add(planPeriod)) { + t.Fatalf("reconciled renewal: end %v want %v", got, E.Add(planPeriod)) + } + f.renewer.RunOnce(ctx, E.Add(-20*time.Hour)) + if f.fc.count() != 1 || !f.periodEnd(t).Equal(E.Add(planPeriod)) { + t.Fatal("reconciled payment applied twice or re-charged") + } +} + +// N concurrent renewal passes (e.g. several replicas' tickers firing at once) +// for the same tenant and instant: exactly one charge, one extension. +func TestAutoRenewConcurrentPassesChargeOnce(t *testing.T) { + f := newRenewalFixture(t, true) + f.enable(t) + f.fc.delay = 30 * time.Millisecond + ctx := context.Background() + E := f.end + f.renewer.RunOnce(ctx, E.Add(-71*time.Hour)) + const racers = 12 + var wg sync.WaitGroup + start := make(chan struct{}) + for i := 0; i < racers; i++ { + wg.Add(1) + go func() { + defer wg.Done() + <-start + NewRenewer(f.db, f.cfg, f.mailer, nil).RunOnce(ctx, E.Add(-47*time.Hour)) + }() + } + close(start) + wg.Wait() + if f.fc.count() != 1 { + t.Fatalf("charges = %d, want exactly 1", f.fc.count()) + } + for ref, n := range f.fc.refs { + if n != 1 { + t.Fatalf("reference %s submitted %d times", ref, n) + } + } + if got := f.periodEnd(t); !got.Equal(E.Add(planPeriod)) { + t.Fatalf("period end %v, want exactly one extension to %v", got, E.Add(planPeriod)) + } + if len(f.mailer.matching("was renewed")) != 1 { + t.Fatalf("success emails: %d", len(f.mailer.matching("was renewed"))) + } +} diff --git a/internal/api/openapi.go b/internal/api/openapi.go index 0887eb6..efd969b 100644 --- a/internal/api/openapi.go +++ b/internal/api/openapi.go @@ -355,12 +355,26 @@ const openAPISpec = `{ {"name": "tenant_id", "in": "query", "required": true, "schema": {"type": "string"}, "description": "Organization (tenant) ID."} ], "responses": { - "200": {"description": "OK", "content": {"application/json": {"schema": {"type": "object", "properties": {"tenant_id": {"type": "string"}, "plan": {"type": "string", "enum": ["free", "plus", "pro"]}, "status": {"type": "string", "enum": ["active", "lapsed"]}, "current_period_end": {"type": "string", "format": "date-time", "nullable": true}}}}}}, + "200": {"description": "OK", "content": {"application/json": {"schema": {"type": "object", "properties": {"tenant_id": {"type": "string"}, "plan": {"type": "string", "enum": ["free", "plus", "pro"]}, "status": {"type": "string", "enum": ["active", "lapsed"]}, "current_period_end": {"type": "string", "format": "date-time", "nullable": true}, "auto_renew": {"type": "boolean"}, "card_on_file": {"type": "boolean"}}}}}}, "401": {"description": "Missing or invalid access token", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}}, "404": {"description": "Organization not found or caller is not a member", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}} } } }, + "/billing/auto-renew": { + "patch": { + "summary": "Turn automatic plan renewal on or off", + "description": "MailX Cloud only. Owner of tenant_id only (HumanAuth). Off by default. Never charges and takes no amount: when on, and a reusable card was saved from a previous successful checkout, MailX charges the server-side plan price itself shortly before the period ends, only after emailing owners a reminder at least 24 hours in advance. Owners are reminded before every period end whether or not auto-renew is on. 503 auto_renew_unavailable when the deployment has no MAILX_BILLING_MASTER_KEY.", + "security": [{"HumanAuth": []}], + "requestBody": {"required": true, "content": {"application/json": {"schema": {"type": "object", "required": ["tenant_id", "auto_renew"], "properties": {"tenant_id": {"type": "string"}, "auto_renew": {"type": "boolean"}}}}}}, + "responses": { + "200": {"description": "OK", "content": {"application/json": {"schema": {"type": "object", "properties": {"tenant_id": {"type": "string"}, "auto_renew": {"type": "boolean"}, "card_on_file": {"type": "boolean"}}}}}}, + "401": {"description": "Missing or invalid access token", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}}, + "403": {"description": "Caller is not an owner of the organization", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}}, + "503": {"description": "Automatic renewal not available on this deployment", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/APIError"}}}} + } + } + }, "/billing/webhook": { "post": { "summary": "Paystack webhook receiver", diff --git a/internal/api/routes.go b/internal/api/routes.go index 57f9b6a..0650cee 100644 --- a/internal/api/routes.go +++ b/internal/api/routes.go @@ -228,6 +228,7 @@ func newMux(h *emailHandler, authSvc authService, readiness func() error, extras billingAuthenticated := humanAuthMiddleware(extras[0].humanAuth) mux.Handle("POST /v1/billing/checkout", billingAuthenticated(http.HandlerFunc(bh.handleCheckout))) mux.Handle("GET /v1/billing/subscription", billingAuthenticated(http.HandlerFunc(bh.handleSubscription))) + mux.Handle("PATCH /v1/billing/auto-renew", billingAuthenticated(http.HandlerFunc(bh.handleAutoRenew))) } } if len(extras) > 0 && extras[0].feedback != nil { diff --git a/internal/billing/paystack.go b/internal/billing/paystack.go index 0a613dc..e622361 100644 --- a/internal/billing/paystack.go +++ b/internal/billing/paystack.go @@ -162,7 +162,15 @@ type Event struct { Metadata Metadata `json:"metadata"` Customer struct { CustomerCode string `json:"customer_code"` + Email string `json:"email"` } `json:"customer"` + // Authorization is the card token Paystack returns on a successful + // charge. AuthorizationCode is SENSITIVE (it can be charged again): + // never log it; store it only encrypted (DEC-229). + Authorization struct { + AuthorizationCode string `json:"authorization_code"` + Reusable bool `json:"reusable"` + } `json:"authorization"` } `json:"data"` } diff --git a/internal/billing/renewal.go b/internal/billing/renewal.go new file mode 100644 index 0000000..376a04c --- /dev/null +++ b/internal/billing/renewal.go @@ -0,0 +1,165 @@ +package billing + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "time" +) + +// Auto-renewal policy (DEC-230). Timeline for a period ending at E: +// reminder at E-72h; charge attempts no earlier than E-48h, at most +// RenewalMaxAttempts, spaced >= RenewalRetrySpacing (so E-48h, E-36h, E-24h +// on an hourly ticker), all before E. No charge is ever made unless an +// auto-renew reminder for THIS period was sent >= RenewalMinNotice earlier. +// If every attempt fails, the tenant lapses at E through the unchanged +// DowngradeLapsedPlans path (retention pinned, DEC-227). +const ( + ReminderLead = 72 * time.Hour + RenewalChargeWindow = 48 * time.Hour + RenewalRetrySpacing = 12 * time.Hour + RenewalMaxAttempts = 3 + RenewalMinNotice = 24 * time.Hour + RenewalReconcileAfter = time.Hour // a pending attempt older than this is verified with Paystack +) + +// ChargeOutcome classifies a server-initiated charge (DEC-228). +type ChargeOutcome int + +const ( + // ChargeUnknown: MailX cannot tell whether money moved (network error, + // timeout, 5xx, undecodable body, or a non-final Paystack status). The + // attempt MUST stay pending and be reconciled with VerifyTransaction; + // it must never be retried under a new reference. + ChargeUnknown ChargeOutcome = iota + // ChargeSucceeded: Paystack reports status "success" for the reference. + ChargeSucceeded + // ChargeFailed: Paystack definitively did not take money (declined, + // rejected request, abandoned, reversed, or reference never created). + ChargeFailed +) + +// ChargeResult is what a charge or verify call established. Reason is +// Paystack's human-readable gateway response/message (e.g. "Insufficient +// Funds"); it never contains card data. Amount/Currency are what Paystack +// reports, for the caller to check before applying. +type ChargeResult struct { + Outcome ChargeOutcome + Reason string + Amount int64 + Currency string +} + +type txData struct { + Status string `json:"status"` + Reference string `json:"reference"` + Amount int64 `json:"amount"` + Currency string `json:"currency"` + GatewayResponse string `json:"gateway_response"` +} + +type txEnvelope struct { + Status bool `json:"status"` + Message string `json:"message"` + Data txData `json:"data"` +} + +func classify(d txData) ChargeOutcome { + switch strings.ToLower(d.Status) { + case "success": + return ChargeSucceeded + case "failed", "abandoned", "reversed": + return ChargeFailed + default: // "pending", "ongoing", "processing", "queued", "send_otp", ... + return ChargeUnknown + } +} + +// ChargeAuthorization calls POST /transaction/charge_authorization for plan's +// server-side price, under the caller-chosen reference (Paystack rejects a +// duplicate reference, a second line of defense against double charging). +// The authorization code is never logged or included in any error. +func (p *Paystack) ChargeAuthorization(ctx context.Context, email, authorizationCode string, plan Plan, reference string, meta Metadata) (ChargeResult, error) { + if plan.PriceUSDCents <= 0 { + return ChargeResult{Outcome: ChargeFailed, Reason: "plan is not purchasable"}, fmt.Errorf("billing: plan %q is not purchasable", plan.ID) + } + raw, err := json.Marshal(map[string]any{ + "email": email, + "amount": plan.PriceUSDCents, + "currency": Currency, + "authorization_code": authorizationCode, + "reference": reference, + "metadata": meta, + }) + if err != nil { + return ChargeResult{Outcome: ChargeFailed, Reason: "could not build request"}, err + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, p.baseURL+"/transaction/charge_authorization", bytes.NewReader(raw)) + if err != nil { + return ChargeResult{Outcome: ChargeFailed, Reason: "could not build request"}, err + } + req.Header.Set("Content-Type", "application/json") + return p.doTx(req, "charge_authorization") +} + +// VerifyTransaction calls GET /transaction/verify/:reference, used to settle a +// charge whose outcome was ChargeUnknown. A reference Paystack does not know +// (HTTP 400/404 with status false) is ChargeFailed: no money moved. +func (p *Paystack) VerifyTransaction(ctx context.Context, reference string) (ChargeResult, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, p.baseURL+"/transaction/verify/"+url.PathEscape(reference), nil) + if err != nil { + return ChargeResult{Outcome: ChargeUnknown}, err + } + return p.doTx(req, "verify") +} + +func (p *Paystack) doTx(req *http.Request, op string) (ChargeResult, error) { + req.Header.Set("Authorization", "Bearer "+p.secretKey) + resp, err := p.http.Do(req) + if err != nil { + // Includes timeouts: the request may have reached Paystack. + return ChargeResult{Outcome: ChargeUnknown}, fmt.Errorf("billing: paystack %s: %w", op, redactURLErr(err)) + } + defer resp.Body.Close() + var env txEnvelope + decErr := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&env) + switch { + case resp.StatusCode >= 500 || resp.StatusCode == http.StatusTooManyRequests: + return ChargeResult{Outcome: ChargeUnknown}, fmt.Errorf("billing: paystack %s: HTTP %d", op, resp.StatusCode) + case decErr != nil: + return ChargeResult{Outcome: ChargeUnknown}, fmt.Errorf("billing: paystack %s: decode (HTTP %d): %w", op, resp.StatusCode, decErr) + case resp.StatusCode >= 400 || !env.Status: + // Paystack rejected the request itself (bad/revoked authorization, + // unknown reference, validation error): no transaction was charged. + return ChargeResult{Outcome: ChargeFailed, Reason: env.Message}, nil + } + r := ChargeResult{Outcome: classify(env.Data), Reason: env.Data.GatewayResponse, Amount: env.Data.Amount, Currency: env.Data.Currency} + if r.Reason == "" { + r.Reason = env.Message + } + return r, nil +} + +// redactURLErr keeps transport errors from echoing request URLs (verify URLs +// carry only the reference, but keep errors uniform and short). +func redactURLErr(err error) error { + var ue *url.Error + if errors.As(err, &ue) { + return fmt.Errorf("%s: %w", ue.Op, ue.Err) + } + return err +} + +// RenewalReference is the deterministic Paystack reference for one renewal +// attempt. It is unique per (tenant, period end, attempt) and has a prefix +// Paystack-generated checkout references never carry. Only characters +// Paystack accepts in a reference are used (alphanumerics and '-'). +func RenewalReference(tenantID string, periodEndUnixMicro int64, attempt int) string { + return fmt.Sprintf("mailx-renew-%s-%d-%d", tenantID, periodEndUnixMicro, attempt) +} diff --git a/internal/database/migrations/000032_plan_auto_renew.down.sql b/internal/database/migrations/000032_plan_auto_renew.down.sql new file mode 100644 index 0000000..a3bc2dd --- /dev/null +++ b/internal/database/migrations/000032_plan_auto_renew.down.sql @@ -0,0 +1,9 @@ +DROP TABLE IF EXISTS billing_renewal_attempts; +ALTER TABLE tenants + DROP COLUMN IF EXISTS plan_reminder_auto_renew, + DROP COLUMN IF EXISTS plan_reminder_sent_at, + DROP COLUMN IF EXISTS plan_reminder_period_end, + DROP COLUMN IF EXISTS paystack_auth_email, + DROP COLUMN IF EXISTS paystack_auth_nonce, + DROP COLUMN IF EXISTS paystack_auth_ciphertext, + DROP COLUMN IF EXISTS auto_renew; diff --git a/internal/database/migrations/000032_plan_auto_renew.up.sql b/internal/database/migrations/000032_plan_auto_renew.up.sql new file mode 100644 index 0000000..8fe6b17 --- /dev/null +++ b/internal/database/migrations/000032_plan_auto_renew.up.sql @@ -0,0 +1,36 @@ +-- v0.47 phase 3c: opt-in plan auto-renewal (RSK-044). +-- auto_renew is OFF by default and only an org owner can turn it on. +-- The Paystack authorization code (a reusable card token) is stored ONLY +-- encrypted (secretbox, AES-256-GCM under MAILX_BILLING_MASTER_KEY, bound to +-- the tenant id as associated data); plaintext never touches the database. +-- plan_reminder_period_end/_sent_at/_auto_renew record the one reminder sent +-- for the current period (once per period; also the "owner was warned of +-- the charge" precondition for any auto-renewal charge). +ALTER TABLE tenants + ADD COLUMN auto_renew BOOLEAN NOT NULL DEFAULT false, + ADD COLUMN paystack_auth_ciphertext BYTEA, + ADD COLUMN paystack_auth_nonce BYTEA, + ADD COLUMN paystack_auth_email TEXT, + ADD COLUMN plan_reminder_period_end TIMESTAMPTZ, + ADD COLUMN plan_reminder_sent_at TIMESTAMPTZ, + ADD COLUMN plan_reminder_auto_renew BOOLEAN NOT NULL DEFAULT false; + +-- One row per MailX-initiated renewal charge attempt (audit log + the claim +-- that guarantees at most one in-flight charge per tenant period). +-- reference is deterministic per (tenant, period_end, attempt) and is sent to +-- Paystack as the transaction reference, so it is also the billing_payments +-- key: the webhook and the ticker applying the same charge collapse to one. +CREATE TABLE billing_renewal_attempts ( + tenant_id TEXT NOT NULL REFERENCES tenants(id) ON DELETE CASCADE, + period_end TIMESTAMPTZ NOT NULL, + attempt SMALLINT NOT NULL CHECK (attempt BETWEEN 1 AND 3), + reference TEXT NOT NULL UNIQUE, + plan TEXT NOT NULL CHECK (plan IN ('plus', 'pro')), + amount BIGINT NOT NULL, + status TEXT NOT NULL DEFAULT 'pending' CHECK (status IN ('pending', 'succeeded', 'failed')), + failure_reason TEXT, + created_at TIMESTAMPTZ NOT NULL, + finished_at TIMESTAMPTZ, + PRIMARY KEY (tenant_id, period_end, attempt) +); +CREATE INDEX idx_billing_renewal_attempts_pending ON billing_renewal_attempts (created_at) WHERE status = 'pending'; diff --git a/internal/database/plans.go b/internal/database/plans.go index 1ebc16a..905dcd0 100644 --- a/internal/database/plans.go +++ b/internal/database/plans.go @@ -262,8 +262,9 @@ func (db *DB) ApplyPlanPayment(ctx context.Context, p Payment, period time.Durat } // DowngradeLapsedPlans moves every paid tenant whose period ended before now -// back to free with status 'lapsed', returning how many changed. MVP: no -// automatic renewal charge is attempted (RSK-044). +// back to free with status 'lapsed', returning how many changed. Opt-in +// auto-renewal (api.Renewer) runs before this each pass; a tenant whose +// renewal did not succeed by period end lapses here like any other. // // Before clearing plan, it PINS retention_days explicitly to the lapsing // plan's own window (COALESCE: only when the tenant has no existing diff --git a/internal/database/renewal.go b/internal/database/renewal.go new file mode 100644 index 0000000..358db03 --- /dev/null +++ b/internal/database/renewal.go @@ -0,0 +1,314 @@ +package database + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/Ferousco-dev/mailx/internal/billing" +) + +// RenewalSettings is an org's auto-renewal state as shown to its owner. +type RenewalSettings struct { + AutoRenew bool + CardOnFile bool +} + +// SetAutoRenew sets tenants.auto_renew. When the value actually changes, the +// current period's reminder marker is cleared so the owner gets a fresh +// reminder that matches the new setting (a "charge is coming" notice is a +// precondition for any charge, DEC-230). ErrNotFound if the tenant is absent. +func (db *DB) SetAutoRenew(ctx context.Context, tenantID string, on bool) (RenewalSettings, error) { + var s RenewalSettings + err := db.pool.QueryRow(ctx, ` + UPDATE tenants SET + plan_reminder_period_end = CASE WHEN auto_renew <> $2 THEN NULL ELSE plan_reminder_period_end END, + plan_reminder_sent_at = CASE WHEN auto_renew <> $2 THEN NULL ELSE plan_reminder_sent_at END, + plan_reminder_auto_renew = CASE WHEN auto_renew <> $2 THEN false ELSE plan_reminder_auto_renew END, + auto_renew = $2 + WHERE id = $1 + RETURNING auto_renew, paystack_auth_ciphertext IS NOT NULL`, tenantID, on).Scan(&s.AutoRenew, &s.CardOnFile) + if err != nil { + return RenewalSettings{}, normalizeErr(err) + } + return s, nil +} + +// GetRenewalSettings returns tenantID's auto-renewal state. +func (db *DB) GetRenewalSettings(ctx context.Context, tenantID string) (RenewalSettings, error) { + var s RenewalSettings + err := db.pool.QueryRow(ctx, + `SELECT auto_renew, paystack_auth_ciphertext IS NOT NULL FROM tenants WHERE id = $1`, tenantID, + ).Scan(&s.AutoRenew, &s.CardOnFile) + if err != nil { + return RenewalSettings{}, normalizeErr(err) + } + return s, nil +} + +// SaveRenewalAuthorization stores the ENCRYPTED Paystack authorization (the +// caller seals it with secretbox, associated data = tenant id) and the email +// Paystack's charge_authorization requires alongside it. Plaintext +// authorization codes never reach this package. +func (db *DB) SaveRenewalAuthorization(ctx context.Context, tenantID string, ciphertext, nonce []byte, email string) error { + if len(ciphertext) == 0 || len(nonce) == 0 || email == "" { + return errors.New("database: incomplete renewal authorization") + } + tag, err := db.pool.Exec(ctx, ` + UPDATE tenants SET paystack_auth_ciphertext = $2, paystack_auth_nonce = $3, paystack_auth_email = $4 + WHERE id = $1`, tenantID, ciphertext, nonce, email) + if err != nil { + return fmt.Errorf("database: save renewal authorization: %w", normalizeErr(err)) + } + if tag.RowsAffected() == 0 { + return ErrNotFound + } + return nil +} + +// PlanReminder is one claimed pre-period-end reminder to send. +type PlanReminder struct { + TenantID string + TenantName string + Plan string + PeriodEnd time.Time + // AutoRenew is true when the reminder announces a charge (auto_renew on + // AND a card on file); false means "renew manually or lapse". + AutoRenew bool +} + +// ClaimPlanReminders atomically marks, and returns, every active paid tenant +// whose period ends within (now, now+lead] and has not yet been reminded for +// that exact period end. The single UPDATE ... RETURNING makes the claim +// exactly-once across concurrent tickers. A caller that fails to send must +// call ReleasePlanReminder so the next tick retries. +func (db *DB) ClaimPlanReminders(ctx context.Context, now time.Time, lead time.Duration) ([]PlanReminder, error) { + rows, err := db.pool.Query(ctx, ` + UPDATE tenants SET plan_reminder_period_end = plan_current_period_end, + plan_reminder_sent_at = $1, + plan_reminder_auto_renew = (auto_renew AND paystack_auth_ciphertext IS NOT NULL) + WHERE plan <> 'free' AND plan_status = 'active' + AND plan_current_period_end > $1 AND plan_current_period_end <= $1 + make_interval(secs => $2) + AND plan_reminder_period_end IS DISTINCT FROM plan_current_period_end + RETURNING id, name, plan, plan_current_period_end, plan_reminder_auto_renew`, now, lead.Seconds()) + if err != nil { + return nil, fmt.Errorf("database: claim plan reminders: %w", normalizeErr(err)) + } + defer rows.Close() + var out []PlanReminder + for rows.Next() { + var r PlanReminder + if err := rows.Scan(&r.TenantID, &r.TenantName, &r.Plan, &r.PeriodEnd, &r.AutoRenew); err != nil { + return nil, fmt.Errorf("database: scan plan reminder: %w", err) + } + out = append(out, r) + } + return out, rows.Err() +} + +// ReleasePlanReminder undoes a claim whose email could not be sent (only if +// the marker still points at that period). +func (db *DB) ReleasePlanReminder(ctx context.Context, tenantID string, periodEnd time.Time) error { + _, err := db.pool.Exec(ctx, ` + UPDATE tenants SET plan_reminder_period_end = NULL, plan_reminder_sent_at = NULL, plan_reminder_auto_renew = false + WHERE id = $1 AND plan_reminder_period_end = $2`, tenantID, periodEnd) + if err != nil { + return fmt.Errorf("database: release plan reminder: %w", normalizeErr(err)) + } + return nil +} + +// TenantOwnerEmails returns the email address of every owner of tenantID. +func (db *DB) TenantOwnerEmails(ctx context.Context, tenantID string) ([]string, error) { + rows, err := db.pool.Query(ctx, ` + SELECT h.email FROM tenant_members m JOIN humans h ON h.id = m.human_id + WHERE m.tenant_id = $1 AND m.role = 'owner' ORDER BY m.created_at`, tenantID) + if err != nil { + return nil, fmt.Errorf("database: tenant owner emails: %w", normalizeErr(err)) + } + defer rows.Close() + var out []string + for rows.Next() { + var e string + if err := rows.Scan(&e); err != nil { + return nil, err + } + out = append(out, e) + } + return out, rows.Err() +} + +// RenewalCandidates lists tenants that MAY be due an auto-renewal charge at +// now. It is only a prefilter: ClaimRenewalAttempt re-checks everything +// under the tenant row lock. +func (db *DB) RenewalCandidates(ctx context.Context, now time.Time) ([]string, error) { + rows, err := db.pool.Query(ctx, ` + SELECT id FROM tenants + WHERE auto_renew AND plan <> 'free' AND plan_status = 'active' + AND paystack_auth_ciphertext IS NOT NULL + AND plan_current_period_end > $1 AND plan_current_period_end <= $1 + make_interval(secs => $2)`, + now, billing.RenewalChargeWindow.Seconds()) + if err != nil { + return nil, fmt.Errorf("database: renewal candidates: %w", normalizeErr(err)) + } + defer rows.Close() + var out []string + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + return nil, err + } + out = append(out, id) + } + return out, rows.Err() +} + +// RenewalClaim is one claimed (status 'pending', committed) charge attempt. +// Exactly one caller ever receives a given claim; only that caller may send +// the charge to Paystack. +type RenewalClaim struct { + TenantID string + TenantName string + Plan string + Amount int64 + PeriodEnd time.Time + Attempt int + Reference string + Email string + AuthCiphertext []byte + AuthNonce []byte +} + +// ClaimRenewalAttempt decides, under SELECT ... FOR UPDATE on the tenant row +// (the lockMemberCap pattern), whether a charge attempt is due and, if so, +// records it as 'pending' before returning. It returns (nil, nil) when no +// charge may be made now. Conditions (all required, DEC-230): +// - paid plan, status active, auto_renew on, card on file; +// - now < period end <= now + RenewalChargeWindow; +// - an auto-renew reminder for THIS period end was sent >= RenewalMinNotice ago; +// - no pending or succeeded attempt for this period (an unknown outcome +// blocks all further attempts until reconciled - never re-charge blind); +// - fewer than RenewalMaxAttempts attempts, the last >= RenewalRetrySpacing ago. +// +// The amount is the SERVER-SIDE price of the tenant's current plan. +func (db *DB) ClaimRenewalAttempt(ctx context.Context, tenantID string, now time.Time) (*RenewalClaim, error) { + tx, err := db.pool.Begin(ctx) + if err != nil { + return nil, fmt.Errorf("database: begin renewal claim: %w", normalizeErr(err)) + } + defer func() { _ = tx.Rollback(ctx) }() + + var ( + c RenewalClaim + status string + autoRenew, remindedAutoRenew bool + periodEnd, remindedEnd, remindedAt *time.Time + email *string + ) + err = tx.QueryRow(ctx, ` + SELECT name, plan, plan_status, plan_current_period_end, auto_renew, + paystack_auth_ciphertext, paystack_auth_nonce, paystack_auth_email, + plan_reminder_period_end, plan_reminder_sent_at, plan_reminder_auto_renew + FROM tenants WHERE id = $1 FOR UPDATE`, tenantID).Scan( + &c.TenantName, &c.Plan, &status, &periodEnd, &autoRenew, + &c.AuthCiphertext, &c.AuthNonce, &email, + &remindedEnd, &remindedAt, &remindedAutoRenew) + if err != nil { + return nil, fmt.Errorf("database: lock tenant for renewal: %w", normalizeErr(err)) + } + switch { + case !billing.IsPaid(c.Plan), status != "active", !autoRenew, + len(c.AuthCiphertext) == 0, len(c.AuthNonce) == 0, email == nil || *email == "", + periodEnd == nil, !now.Before(*periodEnd), periodEnd.Sub(now) > billing.RenewalChargeWindow, + remindedEnd == nil, !remindedEnd.Equal(*periodEnd), !remindedAutoRenew, + remindedAt == nil, now.Sub(*remindedAt) < billing.RenewalMinNotice: + return nil, nil + } + + rows, err := tx.Query(ctx, ` + SELECT status, created_at FROM billing_renewal_attempts + WHERE tenant_id = $1 AND period_end = $2 ORDER BY attempt`, tenantID, *periodEnd) + if err != nil { + return nil, fmt.Errorf("database: list renewal attempts: %w", normalizeErr(err)) + } + n := 0 + var last time.Time + blocked := false + for rows.Next() { + var st string + if err := rows.Scan(&st, &last); err != nil { + rows.Close() + return nil, err + } + n++ + if st != "failed" { + blocked = true + } + } + rows.Close() + if err := rows.Err(); err != nil { + return nil, err + } + if blocked || n >= billing.RenewalMaxAttempts || (n > 0 && now.Sub(last) < billing.RenewalRetrySpacing) { + return nil, nil + } + + c.TenantID = tenantID + c.PeriodEnd = *periodEnd + c.Attempt = n + 1 + c.Amount = billing.PlanFor(c.Plan).PriceUSDCents + c.Email = *email + c.Reference = billing.RenewalReference(tenantID, periodEnd.UnixMicro(), c.Attempt) + if _, err := tx.Exec(ctx, ` + INSERT INTO billing_renewal_attempts (tenant_id, period_end, attempt, reference, plan, amount, status, created_at) + VALUES ($1, $2, $3, $4, $5, $6, 'pending', $7)`, + tenantID, *periodEnd, c.Attempt, c.Reference, c.Plan, c.Amount, now); err != nil { + return nil, fmt.Errorf("database: record renewal attempt: %w", normalizeErr(err)) + } + if err := tx.Commit(ctx); err != nil { + return nil, fmt.Errorf("database: commit renewal claim: %w", normalizeErr(err)) + } + return &c, nil +} + +// FinishRenewalAttempt settles a PENDING attempt as 'succeeded' or 'failed'. +// It never rewrites an already-settled attempt (returns false then). +func (db *DB) FinishRenewalAttempt(ctx context.Context, reference string, succeeded bool, reason string, now time.Time) (bool, error) { + status := "failed" + if succeeded { + status = "succeeded" + } + if len(reason) > 500 { + reason = reason[:500] + } + tag, err := db.pool.Exec(ctx, ` + UPDATE billing_renewal_attempts SET status = $2, failure_reason = NULLIF($3, ''), finished_at = $4 + WHERE reference = $1 AND status = 'pending'`, reference, status, reason, now) + if err != nil { + return false, fmt.Errorf("database: finish renewal attempt: %w", normalizeErr(err)) + } + return tag.RowsAffected() == 1, nil +} + +// PendingRenewalAttempts returns pending attempts created at or before +// olderThan (outcome unknown; to be verified with Paystack). +func (db *DB) PendingRenewalAttempts(ctx context.Context, olderThan time.Time) ([]RenewalClaim, error) { + rows, err := db.pool.Query(ctx, ` + SELECT a.tenant_id, t.name, a.plan, a.amount, a.period_end, a.attempt, a.reference + FROM billing_renewal_attempts a JOIN tenants t ON t.id = a.tenant_id + WHERE a.status = 'pending' AND a.created_at <= $1 ORDER BY a.created_at`, olderThan) + if err != nil { + return nil, fmt.Errorf("database: pending renewal attempts: %w", normalizeErr(err)) + } + defer rows.Close() + var out []RenewalClaim + for rows.Next() { + var c RenewalClaim + if err := rows.Scan(&c.TenantID, &c.TenantName, &c.Plan, &c.Amount, &c.PeriodEnd, &c.Attempt, &c.Reference); err != nil { + return nil, err + } + out = append(out, c) + } + return out, rows.Err() +} diff --git a/internal/database/renewal_test.go b/internal/database/renewal_test.go new file mode 100644 index 0000000..7d25427 --- /dev/null +++ b/internal/database/renewal_test.go @@ -0,0 +1,160 @@ +package database + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/Ferousco-dev/mailx/internal/billing" +) + +// Racing claims for one tenant period: exactly one wins, exactly one attempt +// row exists, and applying its payment from several goroutines (ticker vs +// webhook) records exactly one billing_payments row and one extension. +func TestClaimRenewalAttemptConcurrentExactlyOnce(t *testing.T) { + db := newTestDB(t) + ctx := context.Background() + tn := newTestTenant(t, db) + if err := db.ApplyPlanPayment(ctx, Payment{Reference: "ref_init", TenantID: tn.ID, Plan: "plus", Amount: 600, Currency: "USD"}, 30*24*time.Hour); err != nil { + t.Fatal(err) + } + if err := db.SaveRenewalAuthorization(ctx, tn.ID, []byte("ct"), []byte("nonce"), "a@example.com"); err != nil { + t.Fatal(err) + } + if _, err := db.SetAutoRenew(ctx, tn.ID, true); err != nil { + t.Fatal(err) + } + tp, _ := db.GetTenantPlan(ctx, tn.ID) + E := *tp.CurrentPeriodEnd + if rems, err := db.ClaimPlanReminders(ctx, E.Add(-71*time.Hour), billing.ReminderLead); err != nil || len(rems) != 1 || !rems[0].AutoRenew { + t.Fatalf("reminder claim: %+v %v", rems, err) + } + now := E.Add(-47 * time.Hour) + + const racers = 16 + var wg sync.WaitGroup + var mu sync.Mutex + var claims []*RenewalClaim + start := make(chan struct{}) + for i := 0; i < racers; i++ { + wg.Add(1) + go func() { + defer wg.Done() + <-start + c, err := db.ClaimRenewalAttempt(ctx, tn.ID, now) + if err != nil { + t.Error(err) + return + } + if c != nil { + mu.Lock() + claims = append(claims, c) + mu.Unlock() + } + }() + } + close(start) + wg.Wait() + if len(claims) != 1 { + t.Fatalf("winning claims = %d, want 1", len(claims)) + } + c := claims[0] + if c.Amount != 600 || c.Attempt != 1 || c.Plan != "plus" { + t.Fatalf("claim %+v", c) + } + + // Ticker and webhook both apply the same renewal reference concurrently. + var applied, replays int + for i := 0; i < racers; i++ { + wg.Add(1) + go func() { + defer wg.Done() + err := db.ApplyPlanPayment(ctx, Payment{Reference: c.Reference, TenantID: tn.ID, Plan: "plus", Amount: 600, Currency: "USD"}, 30*24*time.Hour) + mu.Lock() + defer mu.Unlock() + switch err { + case nil: + applied++ + case ErrPaymentAlreadyApplied: + replays++ + default: + t.Error(err) + } + }() + } + wg.Wait() + if applied != 1 || replays != racers-1 { + t.Fatalf("applied=%d replays=%d", applied, replays) + } + var payments, attempts int + _ = db.pool.QueryRow(ctx, `SELECT count(*) FROM billing_payments WHERE tenant_id = $1`, tn.ID).Scan(&payments) + _ = db.pool.QueryRow(ctx, `SELECT count(*) FROM billing_renewal_attempts WHERE tenant_id = $1`, tn.ID).Scan(&attempts) + if payments != 2 || attempts != 1 { // ref_init + one renewal + t.Fatalf("billing_payments=%d attempts=%d, want 2 and 1", payments, attempts) + } + tp, _ = db.GetTenantPlan(ctx, tn.ID) + if !tp.CurrentPeriodEnd.Equal(E.Add(30 * 24 * time.Hour)) { + t.Fatalf("period end %v, want one extension from %v", tp.CurrentPeriodEnd, E) + } + // Pending blocks, settled-once semantics. + if ok, _ := db.FinishRenewalAttempt(ctx, c.Reference, true, "", now); !ok { + t.Fatal("finish pending") + } + if ok, _ := db.FinishRenewalAttempt(ctx, c.Reference, false, "x", now); ok { + t.Fatal("a settled attempt must never be rewritten") + } +} + +func TestClaimRenewalAttemptPreconditions(t *testing.T) { + db := newTestDB(t) + ctx := context.Background() + tn := newTestTenant(t, db) + if err := db.ApplyPlanPayment(ctx, Payment{Reference: "ref_p", TenantID: tn.ID, Plan: "pro", Amount: 2400, Currency: "USD"}, 30*24*time.Hour); err != nil { + t.Fatal(err) + } + tp, _ := db.GetTenantPlan(ctx, tn.ID) + E := *tp.CurrentPeriodEnd + now := E.Add(-47 * time.Hour) + claim := func() *RenewalClaim { + c, err := db.ClaimRenewalAttempt(ctx, tn.ID, now) + if err != nil { + t.Fatal(err) + } + return c + } + if claim() != nil { + t.Fatal("claimed with auto_renew off and no card") + } + if err := db.SaveRenewalAuthorization(ctx, tn.ID, []byte("ct"), []byte("n"), "a@example.com"); err != nil { + t.Fatal(err) + } + if _, err := db.SetAutoRenew(ctx, tn.ID, true); err != nil { + t.Fatal(err) + } + if claim() != nil { + t.Fatal("claimed without any reminder") + } + if _, err := db.ClaimPlanReminders(ctx, E.Add(-71*time.Hour), billing.ReminderLead); err != nil { + t.Fatal(err) + } + if _, err := db.SetAutoRenew(ctx, tn.ID, false); err != nil { + t.Fatal(err) + } + if claim() != nil { + t.Fatal("claimed after opt-out") + } + if _, err := db.SetAutoRenew(ctx, tn.ID, true); err != nil { // reminder reset by toggle + t.Fatal(err) + } + if claim() != nil { + t.Fatal("claimed although the opt-in reset the reminder") + } + if _, err := db.ClaimPlanReminders(ctx, E.Add(-71*time.Hour), billing.ReminderLead); err != nil { + t.Fatal(err) + } + c := claim() + if c == nil || c.Amount != 2400 { + t.Fatalf("expected a pro claim at server price, got %+v", c) + } +} From 9bd7bb4a8b286486c7daefac88180af5f6194135 Mon Sep 17 00:00:00 2001 From: Feranmi Oresajo Date: Fri, 25 Sep 2026 21:44:00 +0100 Subject: [PATCH 3/5] docs: mark OAuth/MFA independent security review complete --- .ilana/ledger.md | 3 +++ .ilana/milestones.md | 2 +- .ilana/state.json | 4 ++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.ilana/ledger.md b/.ilana/ledger.md index 2967dc0..6ec5df6 100644 --- a/.ilana/ledger.md +++ b/.ilana/ledger.md @@ -332,3 +332,6 @@ Built the human-JWT dashboard surface: /v1/me (GET/PATCH), /v1/orgs/{id} (GET/PA ## 2026-09-25 | v0.47 phase 3b: OAuth + TOTP MFA | GATE PASS (security review pending) Built OAuth (Google/GitHub, hand-rolled) and TOTP MFA (hand-rolled RFC 6238, secretbox-sealed secrets); migration 000032. Built concurrently with phase 3a from the same base commit, so its DEC-228..231 collided with dashboard's own DEC-228..230 - renumbered to DEC-231..234 (both in decisions.md and the source comments) when merging the two branches; no RSK collision (dashboard added none). Evidence: gofmt/vet clean, full `go test -race ./...` clean against real Postgres+Redis (humanauth ran, not skipped), migration round-trip, Docker boot smoke in both configurations. Open risks RSK-046/047. + +## 2026-09-25 | v0.47 phase 3b: OAuth + MFA independent security review | GATE PASS +Merged the OAuth/MFA agent's work and did the promised careful manual security review before pushing (not just running tests): OAuth state is single-use, hashed, time-boxed, provider-scoped, consumed atomically before token exchange; both providers correctly require a PROVIDER-VERIFIED email before linking/creating an account; account linking is one atomic transaction keyed on (provider, provider_user_id) with a stable email fallback and a non-bcrypt placeholder hash for OAuth-only accounts. TOTP is a correct RFC 6238 implementation (checked the dynamic-truncation math and constant-time comparison by hand, not just trusting the passing test vectors). The MFA challenge token is opaque and genuinely cannot pass as an access token; CompleteMFAChallenge consumes the challenge and the second factor (TOTP step or backup code) in one transaction with RowsAffected-checked guards, so a replayed TOTP step or reused backup code cannot slip through a partial failure. Assessed RSK-046 (OAuth state not bound to the browser, a real login-CSRF gap) and deliberately left it as a documented risk rather than bolting on a cookie mechanism inconsistent with this app's bearer-token-only architecture - a proper fix needs frontend coordination that doesn't exist yet. Also resolved a real DEC-number collision from two agents building concurrently off the same base commit (both used DEC-228..231 independently) by renumbering the OAuth/MFA branch's entries to DEC-231..234, in both decisions.md and the source comments that reference them. Verified independently: gofmt/go vet/go build clean, full go test ./... and go test -race ./... clean, migration 000032 round-trip re-validated (then rolled back so the real migrator applies it cleanly on boot), Docker boot smoke clean both unconfigured and with Google+GitHub+MFA all configured, OpenAPI tests pass. Ilana updated: milestone status marked as independently security-reviewed (was "pending review" in the agent's own report). diff --git a/.ilana/milestones.md b/.ilana/milestones.md index ed458ad..bbe886b 100644 --- a/.ilana/milestones.md +++ b/.ilana/milestones.md @@ -112,7 +112,7 @@ explicitly deferred to a later phase — v0.47 as a whole is NOT complete. - Tests: `internal/database/dashboard_test.go` (rules, concurrent mutual-removal race, invite list/revoke idempotency, profile/org update), `internal/api/dashboard_handler_test.go` (every endpoint: happy path, non-member 404, non-owner 403, validation). - Deferred: leave org / transfer ownership / role changes, email change, org slug column. Plan auto-renewal and OAuth/MFA are separate parallel work, not part of 3a. -## v0.47 phase 3b — OAuth (Google/GitHub) + TOTP MFA — COMPLETE (pending independent security review) +## v0.47 phase 3b — OAuth (Google/GitHub) + TOTP MFA — COMPLETE (independently security-reviewed) - Migration 000032 (`oauth_states`, `human_oauth_identities`, `mfa_challenges`, `mfa_backup_codes`, `humans.mfa_*`); round-trip down/up validated on a disposable database. - `internal/humanauth`: `oauth.go` (StartOAuth/CompleteOAuth), `totp.go`, `mfa.go` (Enroll/Confirm/Disable/VerifyMFA); Login returns `*MFARequiredError` for MFA accounts. Routes and env vars: architecture.md "OAuth sign-in & TOTP MFA". DEC-231..234 (renumbered from the agent's own DEC-228..231 - collided with phase 3a's DEC-228..230, built concurrently from the same base commit), RSK-046/047. diff --git a/.ilana/state.json b/.ilana/state.json index 26bebe8..d544807 100644 --- a/.ilana/state.json +++ b/.ilana/state.json @@ -34,10 +34,10 @@ "G8": 1 }, "mailx": { - "last_completed_milestone": "v0.47 phase 3b (OAuth Google/GitHub + TOTP MFA, pending independent security review)", + "last_completed_milestone": "v0.47 phase 3b (OAuth Google/GitHub + TOTP MFA, independently security-reviewed)", "ilana_current_through": "v0.47 phase 3b", "current_milestone": "v0.47 Human Accounts & Organizations", - "current_milestone_status": "v0.47 phase 1 (human auth, orgs, password reset, invitations), phase 2 (billing), phase 3a (dashboard backend), and phase 3b (OAuth + TOTP MFA, DEC-231..234, pending security review) complete; plan auto-renewal (RSK-044) still running as a parallel agent; leave-org/transfer-ownership, email change, org slug, email verification deferred", + "current_milestone_status": "v0.47 phase 1 (human auth, orgs, password reset, invitations), phase 2 (billing), phase 3a (dashboard backend), and phase 3b (OAuth + TOTP MFA, DEC-231..234, independently security-reviewed) complete; plan auto-renewal (RSK-044) still running as a parallel agent; leave-org/transfer-ownership, email change, org slug, email verification deferred", "next_milestone": "v0.47 phase 3c (plan auto-renewal) or v0.48 (TBD)" } } From 7fe1b0237c5e9675cd5ea5965ab21a16f9ba3ed4 Mon Sep 17 00:00:00 2001 From: Feranmi Oresajo Date: Fri, 25 Sep 2026 21:38:41 +0100 Subject: [PATCH 4/5] docs(ilana): record plan auto-renewal (DEC-231..234, RSK-044 closed, RSK-046) --- .ilana/architecture.md | 7 +++++-- .ilana/decisions.md | 4 ++++ .ilana/ledger.md | 3 +++ .ilana/milestones.md | 5 +++++ .ilana/risks.md | 3 ++- .ilana/state.json | 12 ++++++------ cmd/mailx/billingconfig.go | 2 +- internal/api/billing_handler.go | 6 +++--- internal/api/billing_renewal.go | 2 +- internal/billing/paystack.go | 2 +- internal/billing/renewal.go | 4 ++-- ...enew.down.sql => 000033_plan_auto_renew.down.sql} | 0 ...to_renew.up.sql => 000033_plan_auto_renew.up.sql} | 0 internal/database/renewal.go | 4 ++-- 14 files changed, 35 insertions(+), 19 deletions(-) rename internal/database/migrations/{000032_plan_auto_renew.down.sql => 000033_plan_auto_renew.down.sql} (100%) rename internal/database/migrations/{000032_plan_auto_renew.up.sql => 000033_plan_auto_renew.up.sql} (100%) diff --git a/.ilana/architecture.md b/.ilana/architecture.md index f358d70..fbb0dbc 100644 --- a/.ilana/architecture.md +++ b/.ilana/architecture.md @@ -503,13 +503,16 @@ Redis queue polling (200ms..2s), not blocking primitives (multi-condition wake-u - Config: `MAILX_GOOGLE_OAUTH_CLIENT_ID/SECRET`, `MAILX_GITHUB_OAUTH_CLIENT_ID/SECRET`, `MAILX_OAUTH_REDIRECT_BASE_URL` (each provider independent; half-config is a startup error), `MAILX_MFA_MASTER_KEY` (base64 32 bytes, must differ from DKIM/webhook keys; unset = enrollment 404, TOTP verify 503, backup codes still work), `MAILX_LIMIT_MFA_VERIFY_IP_RPS/BURST`. Wiring: `cmd/mailx/authconfig.go`. - Limitations: RSK-046 (OAuth state not bound to the browser), RSK-047 (no per-account MFA lockout across challenges). Recovery/regeneration of backup codes requires disable + re-enroll. -## Billing & Plans (v0.47 phase 2; design decisions DEC-221..225) +## Billing & Plans (v0.47 phase 2 + phase 3c auto-renewal; design decisions DEC-221..225, DEC-235..238) - Plans (`internal/billing/plans.go`, `billing.Plans`/`PlanFor`, unknown -> free; `billing.Unlimited` = 0, the ratelimit.Policy "zero = off" convention): free / plus ($6) / pro ($24), limits in milestones.md. Stored on `tenants.plan` (CHECK free|plus|pro, default free), `plan_status` (active|lapsed), `plan_current_period_end`, `paystack_customer_code` (migration 000031). - Single enforcement switch: `database.DB.EnablePlanEnforcement()`, on iff `MAILX_PAYSTACK_SECRET_KEY` is set. Off = self-hosted = unlimited, billing routes unregistered. Plan-limit refusals wrap `database.ErrPlanLimit`; API maps them to 429 `daily_send_limit_reached` (volume) or 403 `plan_limit_reached` (caps/features). - Enforcement points: `admitSend` (daily UTC-day message count), `domainHandler.handleCreate` (non-deleted domains), `humanauth.InviteToOrganization` (early) + `AcceptOrgInvitationForExistingHuman`/`WithSignup` (authoritative, tenant row `FOR UPDATE`), `broadcastHandler.handleCreate`, `webhookHandler.handleCreate`, retention purge default. - Paystack: `POST /v1/billing/checkout` (owner-only, body `{tenant_id, plan}`, Initialize Transaction in USD cents with metadata `{tenant_id, plan}`, returns `authorization_url`); `GET /v1/billing/subscription?tenant_id=` (any member); `POST /v1/billing/webhook` (public, x-paystack-signature HMAC-SHA512 verified, charge.success applies plan for 30 days, `billing_payments.reference` PK makes it idempotent). `plan-lapse` hourly component downgrades expired paid plans to free/lapsed; no auto-renewal (RSK-044). -- Config: `MAILX_PAYSTACK_SECRET_KEY` (enables billing + enforcement), `MAILX_PAYSTACK_PUBLIC_KEY` (frontend only, unused server-side), optional `MAILX_PAYSTACK_CALLBACK_URL`, optional `MAILX_PAYSTACK_BASE_URL` (tests/tooling). +- Config: `MAILX_PAYSTACK_SECRET_KEY` (enables billing + enforcement), `MAILX_PAYSTACK_PUBLIC_KEY` (frontend only, unused server-side), optional `MAILX_PAYSTACK_CALLBACK_URL`, optional `MAILX_PAYSTACK_BASE_URL` (tests/tooling). `MAILX_BILLING_MASTER_KEY` (base64 32 bytes, own key): encrypts saved card authorizations; unset = auto-renew unavailable (PATCH on -> 503), invalid = startup fails. +- Auto-renewal (phase 3c, `internal/api/billing_renewal.go` `Renewer`, `internal/database/renewal.go`, migration 000033): opt-in per org (`tenants.auto_renew` default false), toggled by `PATCH /v1/billing/auto-renew` `{tenant_id, auto_renew}` (owner-only, human JWT, never charges, no amount accepted). A verified, applied `charge.success` with `authorization.reusable` stores the authorization code secretbox-encrypted (AD = tenant id) + payer email; plaintext is never stored or logged. Charges are MailX-initiated `POST /transaction/charge_authorization` at `billing.PlanFor(plan)` price (no Paystack Plans/Subscriptions, DEC-235). +- Hourly `plan-lapse` pass = `Renewer.RunOnce` (reminders -> reconcile pending -> new charges) then `DowngradeLapsedPlans` (unchanged, retention pinned). Timeline (DEC-237): reminder to all owners once per period at E-72h (auto-renew off: "renew manually"; on: "will charge $X"); charges only in (E-48h, E), only if an auto-renew reminder for this exact period was sent >= 24h earlier, max 3 attempts spaced >= 12h; success/failure emailed with Paystack's gateway reason. No system mailer => no reminders and therefore no charges. +- No double charge (DEC-238): `ClaimRenewalAttempt` takes tenant row `FOR UPDATE`, refuses if any pending/succeeded attempt exists for the period, inserts `billing_renewal_attempts` (PK tenant+period_end+attempt) as `pending` and commits BEFORE calling Paystack. Reference `mailx-renew---` is deterministic and is the `billing_payments` key, so ticker and webhook applying the same charge collapse via `ApplyPlanPayment`'s replay protection. Unknown outcomes (network/5xx/non-final) stay pending, block further attempts, and are resolved by `GET /transaction/verify/:ref` after 1h; never retried under a new reference. ## Dashboard backend (v0.47 phase 3a; design decisions DEC-228..230) diff --git a/.ilana/decisions.md b/.ilana/decisions.md index f36807c..d7b8b7c 100644 --- a/.ilana/decisions.md +++ b/.ilana/decisions.md @@ -239,3 +239,7 @@ Process decisions above (DEC-001..DEC-007) belong to the v0.23 FLEET run and sta - DEC-232 [v0.47 phase 3b TOTP] (renumbered from DEC-229): RFC 6238 TOTP hand-rolled (`internal/humanauth/totp.go`: HMAC-SHA1, 30s, 6 digits, ±1 step, constant-time compare), verified against all RFC 6238 Appendix B SHA-1 vectors; no library added. Secrets (20 random bytes) are sealed with the existing `internal/secretbox` under its own `MAILX_MFA_MASTER_KEY` (per secretbox's one-key-per-purpose rule). An accepted step is recorded in `mfa_last_used_step` and must strictly increase, so a code cannot be replayed. Backup codes: 10 x 64-bit random, SHA-256 hashed via `hashRawToken` (same as refresh/reset tokens), consumed with a RowsAffected-checked UPDATE. - DEC-233 [v0.47 phase 3b MFA challenge] (renumbered from DEC-230): The login intermediate state is an opaque 256-bit random token stored hashed in `mfa_challenges` (5-min TTL, single-use, burned after 5 wrong codes) - deliberately NOT a JWT, so it can never pass `VerifyAccessToken`. Login/OAuth return it as a typed `*MFARequiredError` (existing callers that treat any error as failure stay fail-closed); HTTP returns 200 `{mfa_required:true, mfa_token, expires_at}`. `CompleteMFAChallenge` consumes the challenge and the factor (TOTP step or backup code) in one transaction; any guard failure rolls back both. Session minting reuses Login's `completeLogin` path. - DEC-234 [v0.47 phase 3b account linking] (renumbered from DEC-231): An OAuth login with no existing identity link is LINKED to an existing human with the same normalized email (same lower+trim normalization as CreateHuman) rather than creating a duplicate - only when the provider reports the email verified. New OAuth humans get a non-bcrypt placeholder hash so password login is impossible until reset. OAuth for an MFA-enabled account still requires the MFA challenge. +- DEC-235 [v0.47 phase 3c auto-renewal]: renewal charges are MailX-initiated Paystack `charge_authorization` calls with a saved reusable authorization, NOT Paystack Subscriptions/Plans. Why: MailX decides exactly when (and whether) money moves, logs every attempt in its own table, and can require a prior reminder; Paystack's subscription engine would charge on its own schedule and need extra lifecycle webhooks. Consequence: no Paystack Plan objects are created (the task's "create Plans idempotently" step is unnecessary under this design), and no new webhook event types are handled; renewal charges produce ordinary `charge.success` events that the existing handler treats as replays or first applies. +- DEC-236 [v0.47 phase 3c]: auto-renew is opt-in per org (default false, owner-only toggle); the card authorization is saved from any verified, applied `charge.success` with `reusable=true` (so an owner can opt in after paying), encrypted with secretbox under its own `MAILX_BILLING_MASTER_KEY` with the tenant id as associated data. Saving is a capability only; charging requires `auto_renew`. Without the key nothing is saved and enabling is 503. +- DEC-237 [v0.47 phase 3c]: timing. Reminder at E-72h, exactly once per period (atomic `UPDATE ... RETURNING` claim on `plan_reminder_period_end`; released if no owner email could be sent). Charges only within 48h before E, only after a "will charge" reminder for that exact period at least 24h old (toggling auto_renew clears the marker, so a late opt-in gets a fresh reminder and may lapse if < 24h remain), max 3 attempts >= 12h apart, all before E. After failures the tenant lapses at E through the unchanged `DowngradeLapsedPlans` (DEC-227 retention pin). Owners are emailed on every success and failure. +- DEC-238 [v0.47 phase 3c]: at most one in-flight charge per tenant period. Claim under tenant `FOR UPDATE`, committed as `pending` before the network call; a pending attempt (outcome unknown) blocks all further attempts until `verify` settles it (success -> `ApplyPlanPayment`, idempotent by reference; failed/abandoned/reversed/"reference not found" -> failed). A success whose amount/currency differs from the claim is never applied and stays pending (logged `billing_renewal_amount_mismatch` every pass) for manual resolution. Verified by `TestClaimRenewalAttemptConcurrentExactlyOnce` (16 racers; fails when `FOR UPDATE` is removed) and `TestAutoRenewConcurrentPassesChargeOnce` (12 concurrent passes -> 1 Paystack charge, 1 extension). diff --git a/.ilana/ledger.md b/.ilana/ledger.md index 2967dc0..b921052 100644 --- a/.ilana/ledger.md +++ b/.ilana/ledger.md @@ -332,3 +332,6 @@ Built the human-JWT dashboard surface: /v1/me (GET/PATCH), /v1/orgs/{id} (GET/PA ## 2026-09-25 | v0.47 phase 3b: OAuth + TOTP MFA | GATE PASS (security review pending) Built OAuth (Google/GitHub, hand-rolled) and TOTP MFA (hand-rolled RFC 6238, secretbox-sealed secrets); migration 000032. Built concurrently with phase 3a from the same base commit, so its DEC-228..231 collided with dashboard's own DEC-228..230 - renumbered to DEC-231..234 (both in decisions.md and the source comments) when merging the two branches; no RSK collision (dashboard added none). Evidence: gofmt/vet clean, full `go test -race ./...` clean against real Postgres+Redis (humanauth ran, not skipped), migration round-trip, Docker boot smoke in both configurations. Open risks RSK-046/047. + +## 2026-09-25 | v0.47 phase 3c: plan auto-renewal + reminders (RSK-044) | GATE PASS +Opt-in, owner-controlled auto-renewal via MailX-initiated Paystack charge_authorization (DEC-235), encrypted saved authorization (DEC-236), reminder before every period end + bounded retries (DEC-237), claim-before-charge idempotency with verify-based reconciliation (DEC-238). Evidence: gofmt/vet/build clean; full `go test ./...` and `-race` clean against real Postgres+Redis (0 skips); lock-removal mutation made the concurrency test fail; 000033 round-trip; Docker boot smoke. Not verified against live Paystack (RSK-048). DEC/RSK/migration numbers renumbered (231..234 -> 235..238, RSK-046 -> RSK-048, 000032 -> 000033) after rebasing onto the concurrently merged OAuth/MFA work. diff --git a/.ilana/milestones.md b/.ilana/milestones.md index ed458ad..83c42e1 100644 --- a/.ilana/milestones.md +++ b/.ilana/milestones.md @@ -118,3 +118,8 @@ explicitly deferred to a later phase — v0.47 as a whole is NOT complete. - `internal/humanauth`: `oauth.go` (StartOAuth/CompleteOAuth), `totp.go`, `mfa.go` (Enroll/Confirm/Disable/VerifyMFA); Login returns `*MFARequiredError` for MFA accounts. Routes and env vars: architecture.md "OAuth sign-in & TOTP MFA". DEC-231..234 (renumbered from the agent's own DEC-228..231 - collided with phase 3a's DEC-228..230, built concurrently from the same base commit), RSK-046/047. - Tests: RFC 6238 vectors; full MFA flow on real Postgres (enroll, confirm, MFA-gated login, challenge not an access token, single-use, replay blocked, burned after 5 wrong codes, backup code once, expiry, disable needs password); OAuth against an httptest TLS fake Google (new account, link-not-duplicate, missing/unknown/reused/expired state, provider errors, unverified email, not configured, non-https rejected, MFA not bypassed); API test for the MFA IP bucket (429) and 404 unconfigured provider. Full `go test -race ./...` clean on Postgres+Redis; Docker boot smoke with nothing configured and with Google+GitHub+MFA configured. - Not built: GitHub-provider fake test (only Google is exercised end-to-end), backup-code regeneration endpoint, per-account MFA lockout, browser-bound OAuth state (RSK-046). + +## v0.47 phase 3c — Plan auto-renewal + reminders (RSK-044) — COMPLETE (scope per DEC-235..238; residuals RSK-048) +- Migration 000033 (`tenants.auto_renew`, encrypted `paystack_auth_*`, `plan_reminder_*`; `billing_renewal_attempts`). `PATCH /v1/billing/auto-renew`; subscription response gains `auto_renew`, `card_on_file`. +- `billing.Paystack.ChargeAuthorization`/`VerifyTransaction`; `api.Renewer` run by the hourly `plan-lapse` component before lapsing. New env `MAILX_BILLING_MASTER_KEY`. +- Tests: `internal/api/billing_renewal_test.go` (toggle owner-only/never charges/no key -> 503; reminder exactly once for on/off; success extends from period end + webhook replay; declines bounded to 3 then lapse with retention pinned; late opt-in notice; unknown outcome reconciled never re-charged; 12 concurrent passes -> 1 charge), `internal/database/renewal_test.go` (16-racer claim, exact billing_payments/attempt counts; preconditions). Full `go test -race ./...` clean vs real Postgres+Redis; 000033 down/up round-trip; Docker boot smoke with/without Paystack, bad key fails startup. diff --git a/.ilana/risks.md b/.ilana/risks.md index 9fc73b8..c88257e 100644 --- a/.ilana/risks.md +++ b/.ilana/risks.md @@ -52,7 +52,8 @@ - RSK-041 [low, v0.46 PR review]: `internal/database.findRecipientsByAddress` (GDPR export/erasure subject matching) streams and Go-decodes EVERY recipient row for the tenant to find matches for one address — on a tenant with a large recipient history this is a full scan that could exceed the operator CLI's fixed context deadline. Left as-is deliberately for this PR-review-fix pass: it is an operator-only CLI path (`mailx gdpr-export`/`gdpr-delete`), not a hot request path, and a real fix (an indexed normalized-address column on `recipients`, populated at insert time) touches `messages.go`'s INSERT path — a bigger, separate change not undertaken here given the session's cost ceiling. Revisit if GDPR requests need to run routinely against large tenants, or if the CLI's context deadline is ever hit in practice. - RSK-042 [low, v0.47 phase 1 PR review, decided with the human operator]: `POST /v1/auth/signup` returns a distinct `email_taken` (409) for an already-registered address, while `POST /v1/auth/login` deliberately returns the same generic error for both "wrong password" and "unknown email" (anti-enumeration). This is a real, acknowledged inconsistency — an attacker can use signup instead of login to probe which addresses have accounts. Not fixed: the correct fix is to make signup never synchronously confirm existence (respond identically either way, and notify the real account owner by email that someone tried to sign up with their address) — MailX has no human-facing email-sending capability yet to deliver that notification, and changing signup's response shape would break the already-established `Mailx_fe` mock contract (`{name,email,password} -> session`) this phase was built to match. Mitigated, not eliminated, by the IP-keyed rate limit added in the same PR review pass (`authIPLimitMiddleware`, `MAILX_LIMIT_AUTH_IP_RPS`/`_BURST`), which bounds how fast an attacker can enumerate via signup. Revisit once MailX has an email-verification/notification flow for human accounts (a natural fit for a later auth phase, alongside password reset). - RSK-043 [low, v0.47 phase 1 PR review]: `internal/broadcast/expander.go`'s GDPR-erasure disk cleanup (see DEC-210's `ErrRecipientErased` path) retries 3 times on failure, but a disk error that survives all 3 (a genuine, non-transient FS problem, not just a momentary blip) still leaves the raw message file orphaned on disk with no database row left to ever find or retry it - the `broadcast_recipients` row that would normally drive a retry is already gone (that's what triggered the cleanup in the first place). Logged at Error level and counted via `BroadcastExpansionBatch("erasure_cleanup","error")` so an operator can alert on it, but not auto-healing. A fully durable fix needs a reconciliation sweep (e.g. periodically diff on-disk message directories against `messages` rows and delete orphans), which is a separate, larger change not undertaken here. Revisit if this metric/log ever fires in practice. -- RSK-044 [high, v0.47 phase 2]: paid plans do NOT auto-renew. MVP bills by one-off Paystack Initialize Transaction; the hourly `plan-lapse` component downgrades any paid tenant whose `plan_current_period_end` has passed to `free`/`lapsed`, and the owner must check out again every 30 days. No renewal reminder email exists. Fix: Paystack Subscriptions (needs operator-side plan codes) or charge_authorization with the saved authorization. +- RSK-044 [CLOSED in v0.47 phase 3c, was high]: paid plans did not auto-renew and no reminder existed. Now: reminder before every period end for all paid orgs, opt-in auto-renewal via MailX-initiated charge_authorization (DEC-235..238). Residuals tracked in RSK-048. - RSK-045 [medium, v0.47 phase 2]: plan-enforcement gaps. (1) Broadcast expansion creates messages without passing admitSend, so broadcasts do not count toward the daily send cap. (2) The daily cap and the domain cap are checks, not reservations; concurrent requests can overshoot slightly. (3) The system tenant (`MAILX_SYSTEM_TENANT_ID`) is subject to its own plan once billing is enabled; operators must set it to pro (`UPDATE tenants SET plan='pro' ...`; there is no CLI for plan changes yet, and plan-lapse would downgrade it if a period end is set) or password-reset/invite mail stops at 500/day. (4) Paystack USD charging requires the Paystack account to be approved for USD; not verifiable from the repo. - RSK-046 [medium, v0.47 phase 3b]: OAuth `state` is single-use and server-stored but not bound to the initiating browser (no cookie), because start/callback are JSON API endpoints. A login-CSRF (attacker gets a victim to complete the attacker's own callback URL, logging the victim into the attacker's account) is not prevented. Mitigation: bind state to an HttpOnly cookie or have the dashboard verify it initiated the flow. - RSK-047 [medium, v0.47 phase 3b]: MFA brute-force limits are per-IP (5 burst, 1/12s) and per-challenge (5 wrong codes). An attacker who already has the password and many IPs can open new challenges (each login is also authIP-limited) and keep guessing; there is no per-account lockout or alert. Losing `MAILX_MFA_MASTER_KEY` makes TOTP unverifiable (backup codes still work). +- RSK-048 [medium, v0.47 phase 3c]: auto-renewal residuals. (1) "Transaction reference not found" from verify is treated as no charge; if Paystack were eventually consistent past the 1h reconcile delay this could permit a retry (not observed; unverified against live Paystack). (2) Charge-endpoint request/response shapes were implemented from Paystack's documented API and tested only against a fake server; needs a live test-mode run before launch. (3) An unknown-outcome attempt still pending at period end does not delay the lapse; a later verified success re-activates with a fresh 30-day period. (4) Amount-mismatch successes need manual operator resolution. (5) Renewal requires the system mailer; without it nothing is reminded or charged (safe but silent beyond a startup WARN). (6) No endpoint to delete a saved card (disable auto_renew stops charging but keeps the encrypted authorization). diff --git a/.ilana/state.json b/.ilana/state.json index 26bebe8..e80f871 100644 --- a/.ilana/state.json +++ b/.ilana/state.json @@ -17,8 +17,8 @@ "TC": 0, "DEF": 27, "CR": 24, - "RSK": 47, - "DEC": 234, + "RSK": 48, + "DEC": 238, "MET": 72, "ETH": 1 }, @@ -34,10 +34,10 @@ "G8": 1 }, "mailx": { - "last_completed_milestone": "v0.47 phase 3b (OAuth Google/GitHub + TOTP MFA, pending independent security review)", - "ilana_current_through": "v0.47 phase 3b", + "last_completed_milestone": "v0.47 phase 3c (plan auto-renewal + reminders)", + "ilana_current_through": "v0.47 phase 3c", "current_milestone": "v0.47 Human Accounts & Organizations", - "current_milestone_status": "v0.47 phase 1 (human auth, orgs, password reset, invitations), phase 2 (billing), phase 3a (dashboard backend), and phase 3b (OAuth + TOTP MFA, DEC-231..234, pending security review) complete; plan auto-renewal (RSK-044) still running as a parallel agent; leave-org/transfer-ownership, email change, org slug, email verification deferred", - "next_milestone": "v0.47 phase 3c (plan auto-renewal) or v0.48 (TBD)" + "current_milestone_status": "v0.47 phase 1 (human auth, orgs, password reset, invitations), phase 2 (billing), phase 3a (dashboard backend), phase 3b (OAuth + TOTP MFA, DEC-231..234, pending security review) and phase 3c (opt-in plan auto-renewal + reminders, DEC-235..238, RSK-044 closed, residuals RSK-048; pending live Paystack test-mode verification and independent review) complete; leave-org/transfer-ownership, email change, org slug, email verification deferred", + "next_milestone": "v0.48 (TBD); live Paystack test-mode verification of auto-renewal before launch (RSK-048)" } } diff --git a/cmd/mailx/billingconfig.go b/cmd/mailx/billingconfig.go index dcfb22b..9bf8db2 100644 --- a/cmd/mailx/billingconfig.go +++ b/cmd/mailx/billingconfig.go @@ -63,7 +63,7 @@ func enablePlanEnforcementFromEnv(db *database.DB) { const planLapseInterval = time.Hour // runPlanLapse runs the hourly billing pass: renewer (reminders before every -// period end; opt-in auto-renewal charges, DEC-228..231) then lapse of any +// period end; opt-in auto-renewal charges, DEC-235..238) then lapse of any // paid tenant whose period has ended (retention pinned, DEC-227). renewer may // be nil (no system mailer): then nothing is reminded or charged. func runPlanLapse(ctx context.Context, db *database.DB, renewer *api.Renewer, o obs) error { diff --git a/internal/api/billing_handler.go b/internal/api/billing_handler.go index d2097cd..65f9520 100644 --- a/internal/api/billing_handler.go +++ b/internal/api/billing_handler.go @@ -24,7 +24,7 @@ type BillingConfig struct { CallbackURL string // AuthBox encrypts saved Paystack card authorizations at rest // (MAILX_BILLING_MASTER_KEY). Nil disables auto-renewal entirely: no - // authorization is stored and auto_renew cannot be turned on (DEC-229). + // authorization is stored and auto_renew cannot be turned on (DEC-236). AuthBox *secretbox.Box } @@ -236,7 +236,7 @@ func (h *billingHandler) handleWebhook(w http.ResponseWriter, r *http.Request) { // saveAuthorization stores a reusable card authorization from a verified, // just-applied charge.success, encrypted and bound to the tenant id. It is // only a capability: no charge is ever made unless the owner has turned -// auto_renew on (DEC-229). Failures are logged, never surfaced to Paystack +// auto_renew on (DEC-236). Failures are logged, never surfaced to Paystack // (the payment itself is already applied; a retry would only be a replay). // The authorization code itself is never logged. func (h *billingHandler) saveAuthorization(r *http.Request, ev billing.Event) { @@ -262,7 +262,7 @@ type autoRenewRequest struct { // handleAutoRenew lets an org OWNER opt in to (or out of) automatic renewal. // It never charges and accepts no amount: renewal charges are made later by -// the Renewer at the server-side plan price, only after a reminder (DEC-230). +// the Renewer at the server-side plan price, only after a reminder (DEC-237). func (h *billingHandler) handleAutoRenew(w http.ResponseWriter, r *http.Request) { humanID, ok := humanIDFromContext(r.Context()) if !ok { diff --git a/internal/api/billing_renewal.go b/internal/api/billing_renewal.go index b2e8674..2c74a46 100644 --- a/internal/api/billing_renewal.go +++ b/internal/api/billing_renewal.go @@ -15,7 +15,7 @@ import ( ) // Renewer runs one hourly pass of plan reminders and opt-in auto-renewal -// charges (v0.47 phase 3c, DEC-228..231). Order per pass: reminders, then +// charges (v0.47 phase 3c, DEC-235..238). Order per pass: reminders, then // reconcile unknown-outcome charges, then new charges. Lapsing stays in // database.DowngradeLapsedPlans (called by cmd/mailx after RunOnce). // diff --git a/internal/billing/paystack.go b/internal/billing/paystack.go index e622361..08dbbf3 100644 --- a/internal/billing/paystack.go +++ b/internal/billing/paystack.go @@ -166,7 +166,7 @@ type Event struct { } `json:"customer"` // Authorization is the card token Paystack returns on a successful // charge. AuthorizationCode is SENSITIVE (it can be charged again): - // never log it; store it only encrypted (DEC-229). + // never log it; store it only encrypted (DEC-236). Authorization struct { AuthorizationCode string `json:"authorization_code"` Reusable bool `json:"reusable"` diff --git a/internal/billing/renewal.go b/internal/billing/renewal.go index 376a04c..dd2168f 100644 --- a/internal/billing/renewal.go +++ b/internal/billing/renewal.go @@ -13,7 +13,7 @@ import ( "time" ) -// Auto-renewal policy (DEC-230). Timeline for a period ending at E: +// Auto-renewal policy (DEC-237). Timeline for a period ending at E: // reminder at E-72h; charge attempts no earlier than E-48h, at most // RenewalMaxAttempts, spaced >= RenewalRetrySpacing (so E-48h, E-36h, E-24h // on an hourly ticker), all before E. No charge is ever made unless an @@ -29,7 +29,7 @@ const ( RenewalReconcileAfter = time.Hour // a pending attempt older than this is verified with Paystack ) -// ChargeOutcome classifies a server-initiated charge (DEC-228). +// ChargeOutcome classifies a server-initiated charge (DEC-238). type ChargeOutcome int const ( diff --git a/internal/database/migrations/000032_plan_auto_renew.down.sql b/internal/database/migrations/000033_plan_auto_renew.down.sql similarity index 100% rename from internal/database/migrations/000032_plan_auto_renew.down.sql rename to internal/database/migrations/000033_plan_auto_renew.down.sql diff --git a/internal/database/migrations/000032_plan_auto_renew.up.sql b/internal/database/migrations/000033_plan_auto_renew.up.sql similarity index 100% rename from internal/database/migrations/000032_plan_auto_renew.up.sql rename to internal/database/migrations/000033_plan_auto_renew.up.sql diff --git a/internal/database/renewal.go b/internal/database/renewal.go index 358db03..87990e3 100644 --- a/internal/database/renewal.go +++ b/internal/database/renewal.go @@ -18,7 +18,7 @@ type RenewalSettings struct { // SetAutoRenew sets tenants.auto_renew. When the value actually changes, the // current period's reminder marker is cleared so the owner gets a fresh // reminder that matches the new setting (a "charge is coming" notice is a -// precondition for any charge, DEC-230). ErrNotFound if the tenant is absent. +// precondition for any charge, DEC-237). ErrNotFound if the tenant is absent. func (db *DB) SetAutoRenew(ctx context.Context, tenantID string, on bool) (RenewalSettings, error) { var s RenewalSettings err := db.pool.QueryRow(ctx, ` @@ -183,7 +183,7 @@ type RenewalClaim struct { // ClaimRenewalAttempt decides, under SELECT ... FOR UPDATE on the tenant row // (the lockMemberCap pattern), whether a charge attempt is due and, if so, // records it as 'pending' before returning. It returns (nil, nil) when no -// charge may be made now. Conditions (all required, DEC-230): +// charge may be made now. Conditions (all required, DEC-237): // - paid plan, status active, auto_renew on, card on file; // - now < period end <= now + RenewalChargeWindow; // - an auto-renew reminder for THIS period end was sent >= RenewalMinNotice ago; From 2e498e548b0a2e10ab1dbc9f7c6cb1409a99bd24 Mon Sep 17 00:00:00 2001 From: Feranmi Oresajo Date: Fri, 25 Sep 2026 22:12:42 +0100 Subject: [PATCH 5/5] fix: OAuth account-takeover and verify-4xx double-charge risk - ResolveOAuthHuman no longer auto-links an OAuth login to an existing password-based account; only accounts that have never had a real password are safe to auto-link. Prevents an attacker from pre-registering a victim's email to capture their later OAuth login. - Paystack verify-call 4xx is no longer treated the same as charge_authorization's: only an "unknown reference" response proves no charge happened. Any other verify failure (e.g. a rotated secret key) now leaves the attempt pending instead of unblocking a second charge under a fresh reference. --- .ilana/decisions.md | 1 + .ilana/ledger.md | 3 ++ .ilana/state.json | 2 +- internal/api/billing_renewal_test.go | 40 +++++++++++++++++++++++++++ internal/api/humanauth_mfa_handler.go | 2 ++ internal/billing/renewal.go | 14 ++++++++++ internal/database/human_mfa.go | 23 +++++++++++++-- internal/humanauth/oauth.go | 12 ++++++++ internal/humanauth/oauth_test.go | 32 ++++++++++++++++----- 9 files changed, 118 insertions(+), 11 deletions(-) diff --git a/.ilana/decisions.md b/.ilana/decisions.md index d7b8b7c..41085e4 100644 --- a/.ilana/decisions.md +++ b/.ilana/decisions.md @@ -243,3 +243,4 @@ Process decisions above (DEC-001..DEC-007) belong to the v0.23 FLEET run and sta - DEC-236 [v0.47 phase 3c]: auto-renew is opt-in per org (default false, owner-only toggle); the card authorization is saved from any verified, applied `charge.success` with `reusable=true` (so an owner can opt in after paying), encrypted with secretbox under its own `MAILX_BILLING_MASTER_KEY` with the tenant id as associated data. Saving is a capability only; charging requires `auto_renew`. Without the key nothing is saved and enabling is 503. - DEC-237 [v0.47 phase 3c]: timing. Reminder at E-72h, exactly once per period (atomic `UPDATE ... RETURNING` claim on `plan_reminder_period_end`; released if no owner email could be sent). Charges only within 48h before E, only after a "will charge" reminder for that exact period at least 24h old (toggling auto_renew clears the marker, so a late opt-in gets a fresh reminder and may lapse if < 24h remain), max 3 attempts >= 12h apart, all before E. After failures the tenant lapses at E through the unchanged `DowngradeLapsedPlans` (DEC-227 retention pin). Owners are emailed on every success and failure. - DEC-238 [v0.47 phase 3c]: at most one in-flight charge per tenant period. Claim under tenant `FOR UPDATE`, committed as `pending` before the network call; a pending attempt (outcome unknown) blocks all further attempts until `verify` settles it (success -> `ApplyPlanPayment`, idempotent by reference; failed/abandoned/reversed/"reference not found" -> failed). A success whose amount/currency differs from the claim is never applied and stays pending (logged `billing_renewal_amount_mismatch` every pass) for manual resolution. Verified by `TestClaimRenewalAttemptConcurrentExactlyOnce` (16 racers; fails when `FOR UPDATE` is removed) and `TestAutoRenewConcurrentPassesChargeOnce` (12 concurrent passes -> 1 Paystack charge, 1 extension). +- DEC-239 [OAuth/MFA + auto-renewal, CodeRabbit review pass]: fixed 2 findings on PR #26, one a genuine account-takeover vulnerability. (1) **OAuth email-linking account takeover** (CWE-287, major): `ResolveOAuthHuman` linked a provider-verified OAuth identity to ANY existing human with the same normalized email, including one created via ordinary password signup. An attacker could pre-register a victim's real email with a password the attacker controls; when the victim later signs in via Google/GitHub with that same (genuinely theirs) verified email, the OAuth login would silently attach to the attacker's account - the attacker keeps password access to whatever the victim then does under that account. Fixed by adding `database.ErrOAuthAccountRequiresPasswordLogin`: `ResolveOAuthHuman` now only auto-links when the existing account's `password_hash` is still `unusablePasswordHash` (i.e. it was itself only ever reached through OAuth, so every existing "owner" already proved control via provider verification, not a password an attacker could have set unilaterally). A password-holding account is never touched; the caller gets a clear, distinct error instead (`oauth_account_requires_password_login`, 409) telling them to log in with the password first. Deliberately did NOT build an authenticated "link accounts" flow in this pass - refusing silently-unsafe linking is the security-correct minimum; an explicit opt-in linking flow (log in with password, then link a provider) is a reasonable follow-up, not required to close this hole. (2) **Paystack verify-call 4xx misclassification** (major, financial correctness): `Paystack.doTx` mapped ANY 4xx response to `ChargeFailed` ("no money moved"), a rule that is only true for `charge_authorization` (a rejected charge request never creates a charge) - NOT for `VerifyTransaction`, which `reconcilePending` only ever calls for an attempt whose outcome is ALREADY ambiguous, meaning the charge may well have already succeeded. A verify call failing with 401/403 (rotated/misconfigured secret key) or any other non-"unknown reference" 4xx says nothing about whether money moved, but was being classified as `ChargeFailed` - which unblocks `ClaimRenewalAttempt` for a fresh attempt under a NEW reference, double-charging the card. Fixed by splitting verify's 4xx handling: only the specific "Paystack has no record of this reference" response (400/404 with `status:false`) is `ChargeFailed`; every other verify 4xx is `ChargeUnknown` (stays pending, blocks further attempts, gets reconciled again later) - `charge_authorization`'s 4xx handling is unchanged. Both fixes covered by new regression tests: `TestOAuthNewAccountLinkAndState` updated (the old test literally asserted the vulnerable auto-link behavior - now asserts it's refused, and separately that a password-less OAuth-only account still links correctly across a changed provider ID), `TestOAuthRespectsMFA` updated to use an OAuth-created account (a password account can no longer reach the MFA-required path via OAuth at all, by design), and a new `TestAutoRenewVerify401NeverDoubleCharges` (ambiguous charge stays pending through a verify-layer auth failure, no second charge fires). Verified: gofmt/go vet/go build clean, full `go test ./...` and `go test -race ./...` clean, Docker rebuild+boot smoke clean. diff --git a/.ilana/ledger.md b/.ilana/ledger.md index a46e296..4204712 100644 --- a/.ilana/ledger.md +++ b/.ilana/ledger.md @@ -338,3 +338,6 @@ Merged the OAuth/MFA agent's work and did the promised careful manual security r ## 2026-09-25 | v0.47 phase 3c: plan auto-renewal + reminders (RSK-044) | GATE PASS Opt-in, owner-controlled auto-renewal via MailX-initiated Paystack charge_authorization (DEC-235), encrypted saved authorization (DEC-236), reminder before every period end + bounded retries (DEC-237), claim-before-charge idempotency with verify-based reconciliation (DEC-238). Evidence: gofmt/vet/build clean; full `go test ./...` and `-race` clean against real Postgres+Redis (0 skips); lock-removal mutation made the concurrency test fail; 000033 round-trip; Docker boot smoke. Not verified against live Paystack (RSK-048). DEC/RSK/migration numbers renumbered (231..234 -> 235..238, RSK-046 -> RSK-048, 000032 -> 000033) after rebasing onto the concurrently merged OAuth/MFA work. + +## 2026-09-25 | OAuth/MFA + auto-renewal: CodeRabbit review pass (PR #26) | GATE PASS +CodeRabbit reviewed the combined OAuth/MFA + auto-renewal PR and caught 2 real findings, one of them a genuine account-takeover vulnerability that my own earlier manual security review missed: ResolveOAuthHuman auto-linked a provider-verified OAuth login to ANY existing account with the same email, including a password-based one - meaning an attacker could pre-register a victim's real email with an attacker-controlled password, then have the victim's own later, genuinely-theirs OAuth login silently attach to the attacker's account. Fixed by only auto-linking when the existing account has never had a real password (still carries the OAuth-only placeholder hash); a password-holding account now gets a clear "log in with your password first" error instead of being silently linked. Also fixed a real financial-correctness bug in the auto-renewal reconciliation path: a verify-call failure (e.g. 401 from a rotated secret key) was being misclassified the same way as "Paystack has no record of this charge," which would have unblocked a second charge attempt under a fresh reference and double-charged a card whose original charge may have actually succeeded - now only the specific "unknown reference" response counts as proof no charge happened; any other verify failure keeps the attempt pending for later reconciliation. Both fixes came with an existing test that had to change (one literally asserted the vulnerable auto-link behavior as correct) plus new regression coverage. This is a useful reminder even after a careful manual review: account-linking logic needs to be reasoned about from the attacker's setup-in-advance perspective (what if they got there first?), not just the happy-path perspective the code was reviewed from. Verified: gofmt/go vet/go build clean, full go test ./... and go test -race ./... clean, Docker rebuild+boot smoke clean. Ilana updated: decisions.md (DEC-239), state.json (DEC counter). diff --git a/.ilana/state.json b/.ilana/state.json index 6b23b44..eeb840c 100644 --- a/.ilana/state.json +++ b/.ilana/state.json @@ -18,7 +18,7 @@ "DEF": 27, "CR": 24, "RSK": 48, - "DEC": 238, + "DEC": 239, "MET": 72, "ETH": 1 }, diff --git a/internal/api/billing_renewal_test.go b/internal/api/billing_renewal_test.go index bc04139..8b20008 100644 --- a/internal/api/billing_renewal_test.go +++ b/internal/api/billing_renewal_test.go @@ -67,6 +67,14 @@ func (f *fakeChargeServer) server(t *testing.T) *httptest.Server { } case strings.HasPrefix(r.URL.Path, "/transaction/verify/"): ref := strings.TrimPrefix(r.URL.Path, "/transaction/verify/") + if f.mode == "verify401" { + // A rotated/misconfigured secret key, or any transient + // auth/proxy failure on the VERIFY call itself - this says + // nothing about whether the underlying charge went through. + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"status":false,"message":"Invalid key"}`)) + return + } if !f.charged[ref] { w.WriteHeader(http.StatusBadRequest) _, _ = w.Write([]byte(`{"status":false,"message":"Transaction reference not found"}`)) @@ -418,6 +426,38 @@ func TestAutoRenewUnknownOutcomeIsReconciledNeverRecharged(t *testing.T) { } } +// TestAutoRenewVerify401NeverDoubleCharges is a regression test for +// CodeRabbit's finding (PR #26): a charge whose outcome was ambiguous (here, +// a 502 from charge_authorization) must stay pending - never re-charged under +// a fresh reference - when the LATER verify call itself fails with a +// transient error (401 from a rotated key, here) rather than an authoritative +// "Paystack has no record of this reference" response. Misclassifying that +// verify failure as "definitely not charged" would let a fresh attempt fire +// and double-charge the card if the original charge actually succeeded. +func TestAutoRenewVerify401NeverDoubleCharges(t *testing.T) { + f := newRenewalFixture(t, true) + f.fc.mode = "error500" // ambiguous: charge_authorization "took the money" but answered 502 + f.enable(t) + ctx := context.Background() + E := f.end + f.renewer.RunOnce(ctx, E.Add(-71*time.Hour)) + f.renewer.RunOnce(ctx, E.Add(-47*time.Hour)) // fires the ambiguous charge, attempt stays pending + if f.fc.count() != 1 { + t.Fatalf("expected exactly one charge attempt, got %d", f.fc.count()) + } + f.fc.mode = "verify401" // verify itself now fails with an auth error + f.renewer.RunOnce(ctx, E.Add(-35*time.Hour)) + if f.fc.count() != 1 { + t.Fatalf("a failed verify call caused a second charge attempt: %d", f.fc.count()) + } + // The period must NOT have been extended (the ambiguous charge was never + // applied) and must NOT be lapsed either - it stays pending for a human + // to resolve, exactly as the amount-mismatch case does. + if got := f.periodEnd(t); !got.Equal(E) { + t.Fatalf("period end changed to %v while the charge outcome was still unresolved", got) + } +} + // N concurrent renewal passes (e.g. several replicas' tickers firing at once) // for the same tenant and instant: exactly one charge, one extension. func TestAutoRenewConcurrentPassesChargeOnce(t *testing.T) { diff --git a/internal/api/humanauth_mfa_handler.go b/internal/api/humanauth_mfa_handler.go index 52a7322..09a318e 100644 --- a/internal/api/humanauth_mfa_handler.go +++ b/internal/api/humanauth_mfa_handler.go @@ -79,6 +79,8 @@ func (h *humanAuthHandler) handleOAuthCallback(w http.ResponseWriter, r *http.Re writeError(w, r, newError(ErrAuthentication, "invalid_oauth_state", "sign-in state is missing, expired, or already used; start again")) case errors.Is(err, humanauth.ErrOAuthProvider): writeError(w, r, newError(ErrAuthentication, "oauth_provider_error", "the sign-in provider did not complete authentication")) + case errors.Is(err, humanauth.ErrOAuthAccountRequiresPasswordLogin): + writeError(w, r, newError(ErrConflictType, "oauth_account_requires_password_login", "an account with this email already has a password; log in with it first to link this sign-in method")) default: writeError(w, r, newError(ErrInternal, "oauth_callback_failed", "could not complete sign-in")) } diff --git a/internal/billing/renewal.go b/internal/billing/renewal.go index dd2168f..59df62a 100644 --- a/internal/billing/renewal.go +++ b/internal/billing/renewal.go @@ -134,6 +134,20 @@ func (p *Paystack) doTx(req *http.Request, op string) (ChargeResult, error) { return ChargeResult{Outcome: ChargeUnknown}, fmt.Errorf("billing: paystack %s: HTTP %d", op, resp.StatusCode) case decErr != nil: return ChargeResult{Outcome: ChargeUnknown}, fmt.Errorf("billing: paystack %s: decode (HTTP %d): %w", op, resp.StatusCode, decErr) + // verify's 4xx does NOT mean "no transaction was charged" the way + // charge_authorization's does: reconcilePending only ever calls verify + // for an attempt whose outcome is ALREADY unknown, meaning the charge + // may well have gone through - a 401/403 from a rotated secret key, or + // any other non-"unknown reference" 4xx, says nothing about whether + // money moved. Only the specific "Paystack has no record of this + // reference" response (400/404 with status:false) actually proves no + // charge exists. Misclassifying any other verify 4xx as ChargeFailed + // would unblock ClaimRenewalAttempt for a fresh attempt/reference and + // double-charge the card (CodeRabbit, PR #26). + case op == "verify" && (resp.StatusCode == http.StatusBadRequest || resp.StatusCode == http.StatusNotFound) && !env.Status: + return ChargeResult{Outcome: ChargeFailed, Reason: env.Message}, nil + case op == "verify" && (resp.StatusCode >= 400 || !env.Status): + return ChargeResult{Outcome: ChargeUnknown}, fmt.Errorf("billing: paystack verify: HTTP %d: %s", resp.StatusCode, env.Message) case resp.StatusCode >= 400 || !env.Status: // Paystack rejected the request itself (bad/revoked authorization, // unknown reference, validation error): no transaction was charged. diff --git a/internal/database/human_mfa.go b/internal/database/human_mfa.go index f206843..5c068e9 100644 --- a/internal/database/human_mfa.go +++ b/internal/database/human_mfa.go @@ -216,11 +216,24 @@ func (db *DB) ConsumeOAuthState(ctx context.Context, stateHash, provider string, // password login is possible until the human sets one via password reset. const unusablePasswordHash = "!oauth-no-password" +// ErrOAuthAccountRequiresPasswordLogin means an existing account with the +// OAuth email was created with a real (non-OAuth) password. Auto-linking to +// it would let an attacker pre-register a victim's email with a password +// they control, then have the victim's own provider-verified OAuth login +// silently attach to the attacker's account - a real account-takeover path +// (CodeRabbit, CWE-287, PR #26). Only an account that has NEVER had a real +// password (unusablePasswordHash - meaning every existing link to it, if +// any, is itself a provider-verified OAuth identity) is safe to auto-link. +// A password-holding account must be linked deliberately: log in with the +// password first, then link, or reset the password. +var ErrOAuthAccountRequiresPasswordLogin = errors.New("database: an account with this email already has a password; log in with it to link this sign-in method") + // ResolveOAuthHuman finds or creates the human for a provider identity, in one // transaction: (1) an existing identity link wins; (2) otherwise a human with // the same (case-insensitive, same normalization as CreateHuman) email is -// LINKED, not duplicated; (3) otherwise a new human is created with an -// unusable password. Callers must only pass a provider-verified email. +// LINKED, not duplicated - but ONLY if that account has no real password (see +// ErrOAuthAccountRequiresPasswordLogin); (3) otherwise a new human is created +// with an unusable password. Callers must only pass a provider-verified email. func (db *DB) ResolveOAuthHuman(ctx context.Context, provider, providerUserID, email, name string, avatarURL *string) (Human, bool, error) { tx, err := db.pool.Begin(ctx) if err != nil { @@ -234,7 +247,11 @@ func (db *DB) ResolveOAuthHuman(ctx context.Context, provider, providerUserID, e switch { case err == nil: case errors.Is(err, pgx.ErrNoRows): - err = tx.QueryRow(ctx, `SELECT id FROM humans WHERE normalized_email = $1`, normalizeEmail(email)).Scan(&humanID) + var existingPasswordHash string + err = tx.QueryRow(ctx, `SELECT id, password_hash FROM humans WHERE normalized_email = $1`, normalizeEmail(email)).Scan(&humanID, &existingPasswordHash) + if err == nil && existingPasswordHash != unusablePasswordHash { + return Human{}, false, ErrOAuthAccountRequiresPasswordLogin + } if errors.Is(err, pgx.ErrNoRows) { id, idErr := newID() if idErr != nil { diff --git a/internal/humanauth/oauth.go b/internal/humanauth/oauth.go index 21aa6e8..9d8cffb 100644 --- a/internal/humanauth/oauth.go +++ b/internal/humanauth/oauth.go @@ -11,6 +11,8 @@ import ( "strconv" "strings" "time" + + "github.com/Ferousco-dev/mailx/internal/database" ) // OAuth 2.0 Authorization Code sign-in with Google and GitHub, hand-rolled on @@ -33,6 +35,13 @@ var ( // ErrOAuthProvider covers any provider-side failure (denied consent, bad // code, unreachable endpoint, no verified email). ErrOAuthProvider = errors.New("humanauth: oauth provider error") + // ErrOAuthAccountRequiresPasswordLogin means an account with this email + // already has a real password and cannot be auto-linked - see + // database.ErrOAuthAccountRequiresPasswordLogin's doc (CWE-287 fix, + // CodeRabbit, PR #26): silently linking here would let an attacker who + // pre-registered a victim's email with a password they control capture + // the victim's own subsequent OAuth login onto the attacker's account. + ErrOAuthAccountRequiresPasswordLogin = errors.New("humanauth: an account with this email already has a password; log in with it first to link this sign-in method") ) // OAuthProvider is one configured provider. Endpoint URLs must be https. @@ -193,6 +202,9 @@ func (s *Service) CompleteOAuth(ctx context.Context, provider, code, state strin avatar = &u.AvatarURL } h, _, err := s.db.ResolveOAuthHuman(ctx, provider, u.ID, u.Email, name, avatar) + if errors.Is(err, database.ErrOAuthAccountRequiresPasswordLogin) { + return Session{}, ErrOAuthAccountRequiresPasswordLogin + } if err != nil { return Session{}, fmt.Errorf("humanauth: resolve oauth human: %w", err) } diff --git a/internal/humanauth/oauth_test.go b/internal/humanauth/oauth_test.go index 21a7744..dbeaf9f 100644 --- a/internal/humanauth/oauth_test.go +++ b/internal/humanauth/oauth_test.go @@ -89,15 +89,30 @@ func TestOAuthNewAccountLinkAndState(t *testing.T) { t.Fatalf("repeat login: %v", err) } - // Existing password account is LINKED (case-insensitive), not duplicated. - pw, err := svc.SignUp(ctx, "Alan", "alan@example.com", "password123") - if err != nil { + // An existing PASSWORD account is NEVER auto-linked (CodeRabbit CWE-287, + // PR #26): silently linking here would let an attacker who pre-registers + // a victim's email with a password they control capture the victim's own + // later OAuth login onto the attacker's account. + if _, err := svc.SignUp(ctx, "Alan", "alan@example.com", "password123"); err != nil { t.Fatal(err) } fg.sub, fg.email = "g-456", "ALAN@example.com" - s3, err := svc.CompleteOAuth(ctx, "google", "good-code", startState(t, svc)) - if err != nil || s3.Human.ID != pw.Human.ID { - t.Fatalf("link: %v got %s want %s", err, s3.Human.ID, pw.Human.ID) + if _, err := svc.CompleteOAuth(ctx, "google", "good-code", startState(t, svc)); !errors.Is(err, ErrOAuthAccountRequiresPasswordLogin) { + t.Fatalf("expected ErrOAuthAccountRequiresPasswordLogin, got %v", err) + } + // The password account must be entirely untouched by the refused attempt. + if _, err := svc.Login(ctx, "alan@example.com", "password123"); err != nil { + t.Fatalf("password account should be unaffected by the refused link: %v", err) + } + + // An account that has NEVER had a real password (only ever reached via + // OAuth, still carrying unusablePasswordHash) IS safely auto-linkable by + // email - a different provider_user_id for the same OAuth-only account + // (e.g. Google issuing a new internal id) must still link, not refuse. + fg.sub, fg.email = "g-789", "Grace@Example.com" + s4, err := svc.CompleteOAuth(ctx, "google", "good-code", startState(t, svc)) + if err != nil || s4.Human.ID != s1.Human.ID { + t.Fatalf("re-link of a password-less account: %v got %s want %s", err, s4.Human.ID, s1.Human.ID) } // State: missing, unknown, reused, wrong provider. @@ -163,7 +178,10 @@ func TestOAuthRespectsMFA(t *testing.T) { fg := newFakeGoogle(t) WithOAuthProvider(fg.provider())(f.svc) ctx := context.Background() - sess, err := f.svc.SignUp(ctx, "Grace", "grace@example.com", "password123") + // The account must be OAuth-created (no real password - see + // ErrOAuthAccountRequiresPasswordLogin) for a SECOND OAuth login on the + // SAME identity to reach the MFA check at all. + sess, err := f.svc.CompleteOAuth(ctx, "google", "good-code", startState(t, f.svc)) if err != nil { t.Fatal(err) }