diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 5c33d42..0f263a6 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -1444,6 +1444,93 @@ paths: application/json: schema: { $ref: '#/components/schemas/Error' } + /api/v1/account/email/start: + post: + tags: [account] + operationId: emailOtpStart + summary: Mint and deliver an email one-time code for the caller (web onboarding, spec §B2). + description: > + Generates a one-time code bound to the authenticated principal and the + supplied address, persists only its hash, and delivers it out of band. The + code is never returned in the response. A re-request supersedes the prior + unconsumed code. + x-felis-face: [external] + x-felis-tier: app + security: [{ accessJWT: [] }] + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [email] + properties: + email: { type: string, format: email } + responses: + '202': + description: Code minted and dispatched (or logged server-side when no mailer is wired). + content: + application/json: + schema: + type: object + required: [sent, expires_at] + properties: + sent: { type: boolean, const: true } + expires_at: { type: string, format: date-time } + '400': + description: Missing or malformed email address. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '401': + $ref: '#/components/responses/Unauthorized' + + /api/v1/account/email/verify: + post: + tags: [account] + operationId: emailOtpVerify + summary: Redeem an email one-time code and mark the caller's email verified (spec §B2). + description: > + Consumes a previously delivered code for the authenticated principal. On + success the user's email is written and email_verified is set true. Too many + incorrect attempts lock the code (429); an unknown, expired, consumed, or + mismatched code is a 400. + x-felis-face: [external] + x-felis-tier: app + security: [{ accessJWT: [] }] + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [code] + properties: + code: { type: string } + responses: + '200': + description: Email verified. + content: + application/json: + schema: + type: object + required: [verified, email] + properties: + verified: { type: boolean, const: true } + email: { type: string, format: email } + '400': + description: Invalid or expired code. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '401': + $ref: '#/components/responses/Unauthorized' + '429': + description: Too many incorrect attempts; the code is locked. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + /api/v1/me/submissions: post: tags: [submissions] diff --git a/internal/api/api.go b/internal/api/api.go index 5b13242..0727aa7 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -66,6 +66,13 @@ type API struct { // runs, distinct from Builder which an admin drives directly. Submissions SubmissionService + // Mailer delivers player email one-time codes (spec §B2 onboarding). It is + // optional: when nil the email-OTP start route mints and persists the code but + // logs it server-side instead of mailing it (a KNOWN-LIMITATION — the demo has no + // SMTP), so the verify flow is still exercised end-to-end. Production wires a real + // sender. The code is never returned to the client on either path. + Mailer OTPMailer + // RootDomain is injected from config (spec §2). It is the only place the // deployment zone enters the API; hostnames are validated against it and // never hardcoded. @@ -228,6 +235,12 @@ func (a *API) externalAPIRoutes() []apiRoute { // authenticated operation. {Method: "POST", Pattern: "/api/v1/account/link/start", h: a.handleLinkStart}, {Method: "POST", Pattern: "/api/v1/account/link/verify", h: a.handleLinkVerify}, + // Email verification (spec §B2 onboarding), web side: /start mints+delivers a + // one-time code for the caller's chosen address, /verify redeems it and flips + // email_verified. App-tier like the link routes — proving control of your own + // email is an ordinary authenticated operation, scoped to the principal. + {Method: "POST", Pattern: "/api/v1/account/email/start", h: a.handleEmailOTPStart}, + {Method: "POST", Pattern: "/api/v1/account/email/verify", h: a.handleEmailOTPVerify}, // Modpack submission (user-directed lane over §16), user side: a user files an upload for review // and lists their own. App-tier — the submitter and the "my uploads" scope are // both taken from the principal, never the body, so an ordinary authenticated diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 56d82a9..b6fcec7 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -48,6 +48,24 @@ type fakeRepo struct { staff map[string]*StaffUser // username -> staff login row sessions map[string]*fakeSession // token_hash -> session settings map[string][]byte // key -> jsonb value + // player email OTPs (spec §B2). Keyed by row id; the verify path scans for the + // newest live (user, purpose) just as the PG query does. + otps map[string]*fakeEmailOTP +} + +// fakeEmailOTP mirrors an email_otps row: only the code hash is held (never the +// digits), attempts caps brute force, consumed marks single-use, and createdAt +// orders the newest-live lookup. +type fakeEmailOTP struct { + id string + userID string + email string + codeHash string + purpose string + attempts int + expiresAt time.Time + consumed bool + createdAt time.Time } // fakeSession mirrors a sessions row: its owner, its expiry, and whether it has @@ -83,6 +101,7 @@ func newFakeRepo() *fakeRepo { staff: map[string]*StaffUser{}, sessions: map[string]*fakeSession{}, settings: map[string][]byte{}, + otps: map[string]*fakeEmailOTP{}, } } @@ -122,6 +141,56 @@ func (f *fakeRepo) VerifyLinkCode(_ context.Context, userID, code string, now ti delete(f.linkCodes, code) return rec.mcUUID, nil } + +// CreateEmailOTP / VerifyEmailOTP mirror PGRepo's contract so the hermetic tests +// exercise the same semantics the integration impl honors: a fresh code supersedes +// the prior live one for (user, purpose), expiry and the attempt cap are checked +// before the hash compare, a wrong guess costs an attempt without consuming the +// code, and a match consumes it and flips the user row verified. +func (f *fakeRepo) CreateEmailOTP(_ context.Context, id, userID, email, codeHash, purpose string, expiresAt time.Time) error { + for k, o := range f.otps { // supersede any prior live code (DELETE ... consumed_at IS NULL) + if o.userID == userID && o.purpose == purpose && !o.consumed { + delete(f.otps, k) + } + } + f.otps[id] = &fakeEmailOTP{ + id: id, userID: userID, email: email, codeHash: codeHash, purpose: purpose, + expiresAt: expiresAt, createdAt: expiresAt, // createdAt proxy: constant TTL ⇒ later expiry == later creation + } + return nil +} +func (f *fakeRepo) VerifyEmailOTP(_ context.Context, userID, purpose, codeHash string, now time.Time) (string, error) { + var live *fakeEmailOTP + for _, o := range f.otps { // newest live (user, purpose) + if o.userID != userID || o.purpose != purpose || o.consumed { + continue + } + if live == nil || o.createdAt.After(live.createdAt) { + live = o + } + } + if live == nil { + return "", ErrOTPInvalid + } + if !live.expiresAt.After(now) { + return "", ErrOTPInvalid + } + if live.attempts >= otpMaxAttempts { + return "", ErrOTPLocked + } + if live.codeHash != codeHash { + live.attempts++ // a typo costs an attempt but does not consume the code + return "", ErrOTPInvalid + } + live.consumed = true + for _, u := range f.staff { // flip the user row verified (UPDATE users ...) + if u.ID == userID { + u.Email = live.email + u.EmailVerified = true + } + } + return live.email, nil +} func (f *fakeRepo) UserInAllowlist(_ context.Context, n, u string) (bool, error) { return f.allowlist[n][u], nil } diff --git a/internal/api/errors.go b/internal/api/errors.go index aeaf974..bafae19 100644 --- a/internal/api/errors.go +++ b/internal/api/errors.go @@ -25,6 +25,17 @@ var ( // actually up", not a server bug. Handlers map it to 503, not 500, so the // caller is told to wake/retry rather than shown an opaque internal error. ErrConsoleUnavailable = errors.New("server console is unavailable") + // ErrOTPInvalid means an email one-time code is unknown, expired, already + // consumed, or did not match (spec §B2 onboarding). Like ErrLinkCodeInvalid it + // is a client error — the verify endpoint exists; the code is bad — so handlers + // map it to 400, not 404. A wrong-but-not-yet-locked guess collapses to it too, + // so the response never distinguishes "no such code" from "wrong digits". + ErrOTPInvalid = errors.New("email code invalid or expired") + // ErrOTPLocked means the live email code has exhausted its attempt budget: too + // many wrong guesses (spec §B2). It is distinct from ErrOTPInvalid so handlers + // can answer 429 (back off / request a new code) rather than inviting another + // guess against a code that will never accept one. + ErrOTPLocked = errors.New("email code locked: too many attempts") ) // apiError is a handler-level error carrying an HTTP status and a stable, diff --git a/internal/api/handlers_email_otp.go b/internal/api/handlers_email_otp.go new file mode 100644 index 0000000..0218360 --- /dev/null +++ b/internal/api/handlers_email_otp.go @@ -0,0 +1,202 @@ +package api + +import ( + "context" + "crypto/rand" + "encoding/hex" + "errors" + "fmt" + "log" + "math/big" + "net/http" + "strings" + "time" +) + +// Player email verification (spec §B2 onboarding). Forced web onboarding proves a +// player controls an email before it is bound to their account: they request a +// one-time code, the platform mails it, and they type it back. Only a matching, +// unexpired, unconsumed code flips users.email_verified true. The two halves are +// app-tier external routes — verifying your OWN email is an ordinary authenticated +// operation, scoped entirely to the principal (the body never names a user). +// +// The code is a short numeric secret, so two independent defenses bound brute +// force: a short TTL (otpTTL) and a per-code attempt cap (otpMaxAttempts) checked +// inside VerifyEmailOTP. Only the sha-256 of the code is ever stored; the digits +// live only in the email. + +const ( + // otpTTL bounds how long a freshly mailed code is accepted. Long enough to + // switch to an inbox and back, short enough that a leaked code is useless soon. + otpTTL = 10 * time.Minute + // otpMaxAttempts caps wrong guesses against one code before it locks (429). With + // a 6-digit code (1e6 keyspace) five tries is a ~5e-6 chance of a blind hit; the + // cap is enforced in VerifyEmailOTP (Repo), not here, so the fake and PG agree. + otpMaxAttempts = 5 + // otpPurposeOnboard scopes a code to the onboarding email-proof flow. The column + // exists so later flows (e.g. email change) can mint codes that never collide + // with an onboarding code for the same user. + otpPurposeOnboard = "onboard_email" + // otpCodeDigits is the code length; otpCodeBound is its exclusive upper bound, so + // a value in [0, otpCodeBound) zero-pads to exactly otpCodeDigits digits. + otpCodeDigits = 6 + otpCodeBound = 1_000_000 +) + +// OTPMailer delivers a one-time code to an email address. It is a seam, not a +// dependency: the demo ships without SMTP, so a nil Mailer logs the code +// server-side instead of mailing it (a KNOWN-LIMITATION, never a code returned to +// the client). Production wires a real sender. +type OTPMailer interface { + SendOTP(ctx context.Context, email, code string) error +} + +// newEmailOTP returns a cryptographically random otpCodeDigits-digit numeric code. +// crypto/rand.Int over a 10^digits bound is uniform with no modulo bias; the value +// is zero-padded so every code is exactly otpCodeDigits long. +func newEmailOTP() (string, error) { + n, err := rand.Int(rand.Reader, big.NewInt(otpCodeBound)) + if err != nil { + return "", err + } + return fmt.Sprintf("%0*d", otpCodeDigits, n.Int64()), nil +} + +// newOTPID returns an opaque random row id (128 bits, hex) for an email_otps row. +func newOTPID() (string, error) { + var b [16]byte + if _, err := rand.Read(b[:]); err != nil { + return "", err + } + return hex.EncodeToString(b[:]), nil +} + +// otpCodeHash maps a code to its storage key (sha-256 hex), reusing the session +// helper so the raw digits are never written to the database. +func otpCodeHash(code string) string { return hashCookie(code) } + +// looksLikeEmail is a deliberately small sanity check, not RFC 5322: it rejects the +// obvious garbage (empty, no/multiple '@', '@' at an edge, whitespace, no dot in the +// domain) so a code is never minted against an un-mailable string. Real validation +// is delivery itself — a wrong-but-plausible address simply never yields a code. +func looksLikeEmail(s string) bool { + if len(s) < 3 || len(s) > 254 || strings.ContainsAny(s, " \t\r\n") { + return false + } + at := strings.IndexByte(s, '@') + if at <= 0 || at != strings.LastIndexByte(s, '@') || at == len(s)-1 { + return false + } + domain := s[at+1:] + dot := strings.IndexByte(domain, '.') + return dot > 0 && dot < len(domain)-1 +} + +// emailOTPStartRequest is the start-onboarding-verification body: the address the +// player wants to prove control of. +type emailOTPStartRequest struct { + Email string `json:"email"` +} + +// handleEmailOTPStart mints and delivers a one-time code for the caller's chosen +// email (spec §B2, external app face). The code is bound to the principal's user_id +// and the onboarding purpose; a re-request supersedes the prior code. The response +// NEVER carries the code — it is delivered out of band — only that it was sent and +// when it expires. +func (a *API) handleEmailOTPStart(w http.ResponseWriter, r *http.Request) { + p := principalFromContext(r.Context()) + var req emailOTPStartRequest + if err := decodeJSON(w, r, &req); err != nil { + writeError(w, r, err) + return + } + email := strings.TrimSpace(req.Email) + if !looksLikeEmail(email) { + writeError(w, r, newError(http.StatusBadRequest, "bad_request", "a valid email is required")) + return + } + code, err := newEmailOTP() + if err != nil { + writeError(w, r, err) + return + } + id, err := newOTPID() + if err != nil { + writeError(w, r, err) + return + } + expiresAt := a.now().Add(otpTTL) + if err := a.Repo.CreateEmailOTP(r.Context(), id, p.UserID, email, otpCodeHash(code), otpPurposeOnboard, expiresAt); err != nil { + writeError(w, r, err) + return + } + if err := a.deliverOTP(r.Context(), email, code); err != nil { + writeError(w, r, err) + return + } + a.audit(r, auditActor(p), "account.email.otp_sent", "") + writeJSON(w, http.StatusAccepted, map[string]any{ + "sent": true, + "expires_at": expiresAt.UTC(), + }) +} + +// emailOTPVerifyRequest is the verify body: the code the player read from the email. +type emailOTPVerifyRequest struct { + Code string `json:"code"` +} + +// handleEmailOTPVerify redeems a code for the caller (spec §B2, external app face). +// Outcomes mirror the link-verify shape: an invalid/expired/mismatched code → 400 +// invalid_code, a locked code (too many wrong guesses) → 429 otp_locked, and on +// success the user's email is written and email_verified flips true. The verified +// address is echoed so the panel can render it. +func (a *API) handleEmailOTPVerify(w http.ResponseWriter, r *http.Request) { + p := principalFromContext(r.Context()) + var req emailOTPVerifyRequest + if err := decodeJSON(w, r, &req); err != nil { + writeError(w, r, err) + return + } + code := strings.TrimSpace(req.Code) + if code == "" { + writeError(w, r, newError(http.StatusBadRequest, "bad_request", "code is required")) + return + } + email, err := a.Repo.VerifyEmailOTP(r.Context(), p.UserID, otpPurposeOnboard, otpCodeHash(code), a.now()) + switch { + case errors.Is(err, ErrOTPLocked): + writeError(w, r, newError(http.StatusTooManyRequests, "otp_locked", + "too many incorrect attempts; request a new code")) + return + case errors.Is(err, ErrOTPInvalid): + writeError(w, r, newError(http.StatusBadRequest, "invalid_code", "email code is invalid or expired")) + return + case err != nil: + writeError(w, r, err) + return + } + a.audit(r, auditActor(p), "account.email.verified", "") + writeJSON(w, http.StatusOK, map[string]any{"verified": true, "email": email}) +} + +// deliverOTP hands the code to the configured Mailer, or — when none is wired (the +// demo) — logs it server-side as a KNOWN-LIMITATION. The code is logged ONLY in the +// no-mailer fallback and ONLY to the server log; it is never put in an HTTP response. +func (a *API) deliverOTP(ctx context.Context, email, code string) error { + if a.Mailer == nil { + log.Printf("email-otp: no Mailer configured; code for %s is %s (KNOWN-LIMITATION: demo has no SMTP)", email, code) + return nil + } + return a.Mailer.SendOTP(ctx, email, code) +} + +// auditActor picks the most identifying actor string for a principal: the audited +// Access email when present, else the stable user id. A player mid-onboarding may +// not have a verified email yet, so the id keeps the audit row attributable. +func auditActor(p *Principal) string { + if p.Email != "" { + return p.Email + } + return p.UserID +} diff --git a/internal/api/handlers_email_otp_test.go b/internal/api/handlers_email_otp_test.go new file mode 100644 index 0000000..c56e7fb --- /dev/null +++ b/internal/api/handlers_email_otp_test.go @@ -0,0 +1,334 @@ +package api + +import ( + "context" + "errors" + "net/http" + "testing" + "time" +) + +// captureMailer is the OTPMailer seam under test: it records the last code so a +// test can read the digits that production would only ever email out of band. +type captureMailer struct { + email, code string + calls int + err error +} + +func (m *captureMailer) SendOTP(_ context.Context, email, code string) error { + m.calls++ + if m.err != nil { + return m.err + } + m.email, m.code = email, code + return nil +} + +// TestEmailOTPVertical walks the whole §B2 email-proof slice across the external +// face: a player asks for a code, the platform delivers it (here, into the test +// mailer), the player types it back, and only then does the user row flip verified. +// It proves the digits never ride the HTTP response and that both halves audit. +func TestEmailOTPVertical(t *testing.T) { + const email = "player@example.net" + user := &Principal{UserID: "u1", Email: "u1@example.net", Role: "user"} + + repo := newFakeRepo() + // Seed the staff/user row the verify path flips, keyed (in the fake) by username + // but matched by ID — exactly how PGRepo updates users by id. + repo.staff["player"] = &StaffUser{ID: "u1", Email: "old@example.net"} + + mailer := &captureMailer{} + api := newTestAPI(repo, newFakeCluster()) + api.External = staticExternal{p: user} + api.Mailer = mailer + eh := api.ExternalHandler() + + // 1) start mints + delivers a code. The response says "sent" and when it expires + // — but NEVER the code itself. + w := do(eh, "POST", "/api/v1/account/email/start", `{"email":"`+email+`"}`, nil) + if w.Code != http.StatusAccepted { + t.Fatalf("start: code = %d, want 202 (%s)", w.Code, w.Body.String()) + } + b := acctBody(t, w) + if b["sent"] != true { + t.Errorf("start body sent = %v, want true", b["sent"]) + } + if _, leaked := b["code"]; leaked { + t.Error("start response must NEVER carry the code") + } + if s, _ := b["expires_at"].(string); s == "" { + t.Error("start must report expires_at") + } + if mailer.calls != 1 || mailer.email != email { + t.Fatalf("mailer not invoked for %s: calls=%d email=%q", email, mailer.calls, mailer.email) + } + code := mailer.code + if len(code) != otpCodeDigits { + t.Fatalf("delivered code %q: len = %d, want %d", code, len(code), otpCodeDigits) + } + // Only the hash is persisted — the plaintext must not be findable in the store. + for _, o := range repo.otps { + if o.codeHash == code { + t.Error("store holds the plaintext code, not its hash") + } + } + + // 2) the player submits the code. The email is written and verified flips true. + w = do(eh, "POST", "/api/v1/account/email/verify", `{"code":"`+code+`"}`, nil) + if w.Code != http.StatusOK { + t.Fatalf("verify: code = %d, want 200 (%s)", w.Code, w.Body.String()) + } + if vb := acctBody(t, w); vb["verified"] != true || vb["email"] != email { + t.Fatalf("verify body = %v, want verified:true email:%s", vb, email) + } + if su := repo.staff["player"]; !su.EmailVerified || su.Email != email { + t.Fatalf("user row not flipped: verified=%v email=%q", su.EmailVerified, su.Email) + } + + // Both halves are audited by the principal's Access email. + if n := len(repo.audits); n != 2 { + t.Fatalf("want 2 audits (otp_sent, verified), got %d: %+v", n, repo.audits) + } + if repo.audits[0].Action != "account.email.otp_sent" || repo.audits[0].Actor != "u1@example.net" { + t.Errorf("first audit = %+v, want account.email.otp_sent by u1@example.net", repo.audits[0]) + } + if repo.audits[1].Action != "account.email.verified" || repo.audits[1].Actor != "u1@example.net" { + t.Errorf("second audit = %+v, want account.email.verified by u1@example.net", repo.audits[1]) + } + + // 3) the code is single-use: re-submitting the consumed code now fails. + if w := do(eh, "POST", "/api/v1/account/email/verify", `{"code":"`+code+`"}`, nil); w.Code != http.StatusBadRequest || decodeErr(t, w) != "invalid_code" { + t.Fatalf("replay of consumed code: code = %d body %s, want 400 invalid_code", w.Code, w.Body.String()) + } +} + +// TestEmailOTPStartValidation covers the mint-side input gate and the no-mailer +// fallback (the demo path): a malformed address never mints, and a nil Mailer still +// persists a code (logged server-side) so the verify flow stays exercisable. +func TestEmailOTPStartValidation(t *testing.T) { + user := &Principal{UserID: "u1", Email: "u1@example.net", Role: "user"} + mk := func(repo *fakeRepo) http.Handler { + api := newTestAPI(repo, newFakeCluster()) + api.External = staticExternal{p: user} + return api.ExternalHandler() // no Mailer wired → demo fallback + } + + bad := map[string]string{ + "missing email": `{}`, + "empty email": `{"email":""}`, + "whitespace email": `{"email":" "}`, + "no at-sign": `{"email":"notanemail"}`, + "at-sign at edge": `{"email":"@example.net"}`, + "no dot in domain": `{"email":"a@bcd"}`, + "two at-signs": `{"email":"a@b@example.net"}`, + "unknown field": `{"email":"a@example.net","x":1}`, + "trailing at": `{"email":"player@"}`, + } + for name, body := range bad { + t.Run(name, func(t *testing.T) { + repo := newFakeRepo() + w := do(mk(repo), "POST", "/api/v1/account/email/start", body, nil) + if w.Code != http.StatusBadRequest { + t.Fatalf("code = %d, want 400 (%s)", w.Code, w.Body.String()) + } + if len(repo.otps) != 0 { + t.Errorf("a rejected start must not mint a code, got %d", len(repo.otps)) + } + }) + } + + t.Run("no mailer still persists a code (demo fallback)", func(t *testing.T) { + repo := newFakeRepo() + w := do(mk(repo), "POST", "/api/v1/account/email/start", `{"email":"player@example.net"}`, nil) + if w.Code != http.StatusAccepted { + t.Fatalf("code = %d, want 202 (%s)", w.Code, w.Body.String()) + } + if len(repo.otps) != 1 { + t.Fatalf("want exactly 1 persisted code, got %d", len(repo.otps)) + } + }) +} + +// TestEmailOTPVerifyRejections is the redeem-side failure matrix. The expired and +// locked cases plant rows directly: the test clock is frozen, so an already-expired +// or already-exhausted row is the only way to reach those branches deterministically. +func TestEmailOTPVerifyRejections(t *testing.T) { + user := &Principal{UserID: "u1", Email: "u1@example.net", Role: "user"} + mk := func(repo *fakeRepo) http.Handler { + api := newTestAPI(repo, newFakeCluster()) + api.External = staticExternal{p: user} + return api.ExternalHandler() + } + // live seeds an unconsumed onboarding code for u1 with the given hash/expiry. + live := func(repo *fakeRepo, id, codeHash string, expiresAt time.Time, attempts int) { + repo.otps[id] = &fakeEmailOTP{ + id: id, userID: "u1", email: "player@example.net", codeHash: codeHash, + purpose: otpPurposeOnboard, attempts: attempts, + expiresAt: expiresAt, createdAt: expiresAt, + } + } + + t.Run("empty code -> 400 bad_request", func(t *testing.T) { + w := do(mk(newFakeRepo()), "POST", "/api/v1/account/email/verify", `{"code":""}`, nil) + if w.Code != http.StatusBadRequest || decodeErr(t, w) != "bad_request" { + t.Fatalf("code = %d body %s", w.Code, w.Body.String()) + } + }) + t.Run("whitespace code -> 400 bad_request", func(t *testing.T) { + w := do(mk(newFakeRepo()), "POST", "/api/v1/account/email/verify", `{"code":" "}`, nil) + if w.Code != http.StatusBadRequest || decodeErr(t, w) != "bad_request" { + t.Fatalf("code = %d body %s", w.Code, w.Body.String()) + } + }) + t.Run("unknown field -> 400", func(t *testing.T) { + w := do(mk(newFakeRepo()), "POST", "/api/v1/account/email/verify", `{"token":"123456"}`, nil) + if w.Code != http.StatusBadRequest { + t.Fatalf("strict decode must reject unknown field, code = %d", w.Code) + } + }) + t.Run("no live code -> 400 invalid_code", func(t *testing.T) { + w := do(mk(newFakeRepo()), "POST", "/api/v1/account/email/verify", `{"code":"000000"}`, nil) + if w.Code != http.StatusBadRequest || decodeErr(t, w) != "invalid_code" { + t.Fatalf("code = %d body %s", w.Code, w.Body.String()) + } + }) + t.Run("expired code -> 400 invalid_code, not consumed", func(t *testing.T) { + repo := newFakeRepo() + // One second before the frozen test clock (time.Unix(1_700_000_000, 0)). + live(repo, "ex", otpCodeHash("424242"), time.Unix(1_699_999_999, 0), 0) + w := do(mk(repo), "POST", "/api/v1/account/email/verify", `{"code":"424242"}`, nil) + if w.Code != http.StatusBadRequest || decodeErr(t, w) != "invalid_code" { + t.Fatalf("code = %d body %s", w.Code, w.Body.String()) + } + if repo.otps["ex"].consumed { + t.Error("an expired code must not be consumed") + } + }) + t.Run("wrong code -> 400 invalid_code, attempt charged, not consumed", func(t *testing.T) { + repo := newFakeRepo() + live(repo, "wr", otpCodeHash("123456"), time.Unix(1_700_000_600, 0), 0) + w := do(mk(repo), "POST", "/api/v1/account/email/verify", `{"code":"654321"}`, nil) + if w.Code != http.StatusBadRequest || decodeErr(t, w) != "invalid_code" { + t.Fatalf("code = %d body %s", w.Code, w.Body.String()) + } + if repo.otps["wr"].attempts != 1 { + t.Errorf("a wrong guess must cost one attempt, got %d", repo.otps["wr"].attempts) + } + if repo.otps["wr"].consumed { + t.Error("a wrong guess must not consume the code") + } + }) + t.Run("exhausted code -> 429 otp_locked", func(t *testing.T) { + repo := newFakeRepo() + // Attempt budget already spent: even the correct code must be refused. + live(repo, "lk", otpCodeHash("123456"), time.Unix(1_700_000_600, 0), otpMaxAttempts) + w := do(mk(repo), "POST", "/api/v1/account/email/verify", `{"code":"123456"}`, nil) + if w.Code != http.StatusTooManyRequests || decodeErr(t, w) != "otp_locked" { + t.Fatalf("code = %d body %s, want 429 otp_locked", w.Code, w.Body.String()) + } + }) +} + +// TestEmailOTPBruteForceLockout drives the lockout end-to-end through the handler: +// wrong guesses are charged one at a time until the budget is spent, after which +// even the correct code is refused with 429 — the brute-force ceiling in action. +func TestEmailOTPBruteForceLockout(t *testing.T) { + user := &Principal{UserID: "u1", Email: "u1@example.net", Role: "user"} + repo := newFakeRepo() + mailer := &captureMailer{} + api := newTestAPI(repo, newFakeCluster()) + api.External = staticExternal{p: user} + api.Mailer = mailer + eh := api.ExternalHandler() + + if w := do(eh, "POST", "/api/v1/account/email/start", `{"email":"player@example.net"}`, nil); w.Code != http.StatusAccepted { + t.Fatalf("start: code = %d (%s)", w.Code, w.Body.String()) + } + good := mailer.code + + // Exhaust the budget with wrong guesses; each is a plain invalid_code. + for i := 0; i < otpMaxAttempts; i++ { + w := do(eh, "POST", "/api/v1/account/email/verify", `{"code":"000000"}`, nil) + // "000000" could, with 1-in-a-million odds, equal the real code; guard that. + if good == "000000" { + t.Skip("astronomically unlucky code collision; rerun") + } + if w.Code != http.StatusBadRequest || decodeErr(t, w) != "invalid_code" { + t.Fatalf("guess %d: code = %d body %s, want 400 invalid_code", i, w.Code, w.Body.String()) + } + } + // Budget spent: the CORRECT code is now locked out, not accepted. + w := do(eh, "POST", "/api/v1/account/email/verify", `{"code":"`+good+`"}`, nil) + if w.Code != http.StatusTooManyRequests || decodeErr(t, w) != "otp_locked" { + t.Fatalf("post-lockout correct code: code = %d body %s, want 429 otp_locked", w.Code, w.Body.String()) + } +} + +// TestEmailOTPSupersede proves a re-request invalidates the prior code: only the +// newest live code for (user, purpose) can be redeemed, so an intercepted-then- +// reissued code cannot be used after the player asks again. +func TestEmailOTPSupersede(t *testing.T) { + user := &Principal{UserID: "u1", Email: "u1@example.net", Role: "user"} + repo := newFakeRepo() + mailer := &captureMailer{} + api := newTestAPI(repo, newFakeCluster()) + api.External = staticExternal{p: user} + api.Mailer = mailer + eh := api.ExternalHandler() + + do(eh, "POST", "/api/v1/account/email/start", `{"email":"player@example.net"}`, nil) + first := mailer.code + do(eh, "POST", "/api/v1/account/email/start", `{"email":"player@example.net"}`, nil) + second := mailer.code + + if first == second { + t.Skip("rng produced identical codes; rerun") + } + // Only the second code survives. + if w := do(eh, "POST", "/api/v1/account/email/verify", `{"code":"`+first+`"}`, nil); w.Code != http.StatusBadRequest || decodeErr(t, w) != "invalid_code" { + t.Fatalf("superseded code: code = %d body %s, want 400 invalid_code", w.Code, w.Body.String()) + } + if w := do(eh, "POST", "/api/v1/account/email/verify", `{"code":"`+second+`"}`, nil); w.Code != http.StatusOK { + t.Fatalf("current code: code = %d body %s, want 200", w.Code, w.Body.String()) + } +} + +// TestEmailOTPFaceSeparation enforces that both halves are web-only: they require a +// logged-in principal the internal (service-token) face never carries, so crossing +// the face boundary must 404, not silently work. +func TestEmailOTPFaceSeparation(t *testing.T) { + user := &Principal{UserID: "u1", Email: "u1@example.net", Role: "user"} + api := newTestAPI(newFakeRepo(), newFakeCluster()) + api.External = staticExternal{p: user} + ih := api.InternalHandler() + + if w := do(ih, "POST", "/api/v1/account/email/start", `{"email":"a@example.net"}`, nil); w.Code != http.StatusNotFound { + t.Errorf("start on internal face: code = %d, want 404", w.Code) + } + if w := do(ih, "POST", "/api/v1/account/email/verify", `{"code":"123456"}`, nil); w.Code != http.StatusNotFound { + t.Errorf("verify on internal face: code = %d, want 404", w.Code) + } +} + +// TestEmailOTPMailerError pins the delivery-failure path: a Mailer that errors +// surfaces as a 5xx (the code was persisted but never delivered), and the failure +// is NOT audited as a successful send. +func TestEmailOTPMailerError(t *testing.T) { + user := &Principal{UserID: "u1", Email: "u1@example.net", Role: "user"} + repo := newFakeRepo() + api := newTestAPI(repo, newFakeCluster()) + api.External = staticExternal{p: user} + api.Mailer = &captureMailer{err: errors.New("smtp down")} + eh := api.ExternalHandler() + + w := do(eh, "POST", "/api/v1/account/email/start", `{"email":"player@example.net"}`, nil) + if w.Code < 500 { + t.Fatalf("mailer error: code = %d, want 5xx (%s)", w.Code, w.Body.String()) + } + for _, a := range repo.audits { + if a.Action == "account.email.otp_sent" { + t.Error("a failed delivery must not be audited as otp_sent") + } + } +} diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index 85bffdd..ea7ae0f 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -364,6 +364,98 @@ func (p *PGRepo) Audit(ctx context.Context, e AuditEntry) error { return err } +// ---- player email verification (spec §B2 onboarding) ---- + +// CreateEmailOTP supersedes any prior live code for (user, purpose) and inserts the +// fresh one, in one transaction (spec §B2). The supersede DELETE means a re-request +// invalidates the earlier mail, so only the most recent code can ever verify — a +// player who requested twice cannot be confused into typing the stale digits. Only +// the hash is stored; the digits live only in the email. +func (p *PGRepo) CreateEmailOTP(ctx context.Context, id, userID, email, codeHash, purpose string, expiresAt time.Time) error { + tx, err := p.db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer tx.Rollback() //nolint:errcheck // no-op after commit + + if _, err := tx.ExecContext(ctx, + `DELETE FROM email_otps WHERE user_id = $1 AND purpose = $2 AND consumed_at IS NULL`, + userID, purpose); err != nil { + return fmt.Errorf("supersede prior otp: %w", err) + } + if _, err := tx.ExecContext(ctx, + `INSERT INTO email_otps (id, user_id, email, code_hash, purpose, expires_at) + VALUES ($1, $2, $3, $4, $5, $6)`, + id, userID, email, codeHash, purpose, expiresAt); err != nil { + return fmt.Errorf("insert otp: %w", err) + } + return tx.Commit() +} + +// VerifyEmailOTP redeems the newest live code for (user, purpose) in one +// transaction (spec §B2). The row is taken FOR UPDATE so a concurrent verify of the +// same code cannot double-spend it. The branch order is deliberate: expiry and the +// attempt cap are checked before the hash compare, so an expired or locked code is +// never silently accepted, and a hash mismatch costs an attempt (UPDATE attempts+1) +// without consuming the code — a typo must not burn a still-valid code. On a match +// the code is consumed and the user row is flipped verified, returning the proven +// address. ErrOTPInvalid / ErrOTPLocked are the only domain errors. +func (p *PGRepo) VerifyEmailOTP(ctx context.Context, userID, purpose, codeHash string, now time.Time) (string, error) { + tx, err := p.db.BeginTx(ctx, nil) + if err != nil { + return "", err + } + defer tx.Rollback() //nolint:errcheck // no-op after commit + + var ( + id string + email string + storedHash string + attempts int + expiresAt time.Time + ) + switch err := tx.QueryRowContext(ctx, + `SELECT id, email, code_hash, attempts, expires_at FROM email_otps + WHERE user_id = $1 AND purpose = $2 AND consumed_at IS NULL + ORDER BY created_at DESC LIMIT 1 FOR UPDATE`, + userID, purpose).Scan(&id, &email, &storedHash, &attempts, &expiresAt); { + case errors.Is(err, sql.ErrNoRows): + return "", ErrOTPInvalid + case err != nil: + return "", err + } + + if !expiresAt.After(now) { + return "", ErrOTPInvalid + } + if attempts >= otpMaxAttempts { + return "", ErrOTPLocked + } + if storedHash != codeHash { + if _, err := tx.ExecContext(ctx, + `UPDATE email_otps SET attempts = attempts + 1 WHERE id = $1`, id); err != nil { + return "", fmt.Errorf("record otp attempt: %w", err) + } + if err := tx.Commit(); err != nil { + return "", err + } + return "", ErrOTPInvalid + } + + if _, err := tx.ExecContext(ctx, + `UPDATE email_otps SET consumed_at = $2 WHERE id = $1`, id, now); err != nil { + return "", fmt.Errorf("consume otp: %w", err) + } + if _, err := tx.ExecContext(ctx, + `UPDATE users SET email = $2, email_verified = true WHERE id = $1`, userID, email); err != nil { + return "", fmt.Errorf("mark email verified: %w", err) + } + if err := tx.Commit(); err != nil { + return "", err + } + return email, nil +} + // ---- local-password auth (spec §B) ---- // UserByUsername loads a staff login projection by username, or ErrNotFound. A @@ -372,11 +464,11 @@ func (p *PGRepo) Audit(ctx context.Context, e AuditEntry) error { // enumerate which usernames carry a password. func (p *PGRepo) UserByUsername(ctx context.Context, username string) (*StaffUser, error) { const q = `SELECT id, username, COALESCE(email, ''), role::text, - COALESCE(password_hash, ''), must_change_password + COALESCE(password_hash, ''), must_change_password, email_verified FROM users WHERE username = $1` var u StaffUser switch err := p.db.QueryRowContext(ctx, q, username).Scan( - &u.ID, &u.Username, &u.Email, &u.Role, &u.PasswordHash, &u.MustChangePassword); { + &u.ID, &u.Username, &u.Email, &u.Role, &u.PasswordHash, &u.MustChangePassword, &u.EmailVerified); { case errors.Is(err, sql.ErrNoRows): return nil, ErrNotFound case err != nil: @@ -406,11 +498,11 @@ func (p *PGRepo) AdminExists(ctx context.Context) (bool, error) { // session yields a user id, not a username. func (p *PGRepo) UserByID(ctx context.Context, id string) (*StaffUser, error) { const q = `SELECT id, username, COALESCE(email, ''), role::text, - COALESCE(password_hash, ''), must_change_password + COALESCE(password_hash, ''), must_change_password, email_verified FROM users WHERE id = $1` var u StaffUser switch err := p.db.QueryRowContext(ctx, q, id).Scan( - &u.ID, &u.Username, &u.Email, &u.Role, &u.PasswordHash, &u.MustChangePassword); { + &u.ID, &u.Username, &u.Email, &u.Role, &u.PasswordHash, &u.MustChangePassword, &u.EmailVerified); { case errors.Is(err, sql.ErrNoRows): return nil, ErrNotFound case err != nil: diff --git a/internal/api/repo.go b/internal/api/repo.go index c5032b1..8f2edbc 100644 --- a/internal/api/repo.go +++ b/internal/api/repo.go @@ -83,6 +83,10 @@ type StaffUser struct { Role string PasswordHash string MustChangePassword bool + // EmailVerified mirrors users.email_verified (spec §B2): the address was proven + // via an email OTP, not merely asserted. Players carry it through onboarding; + // staff rows seeded by break-glass leave it false until a code is redeemed. + EmailVerified bool } // SessionedUser is the projection resolved from a live session cookie: the @@ -172,6 +176,23 @@ type Repo interface { // Audit appends one audit row. Audit(ctx context.Context, e AuditEntry) error + // ---- player email verification (spec §B2 onboarding) ---- + + // CreateEmailOTP persists a freshly minted one-time code for (userID, purpose): + // only its sha-256 (codeHash), never the digits. It supersedes any prior live + // (unconsumed) code for the same (userID, purpose) so a user has at most one + // outstanding code per purpose — a re-request invalidates the earlier mail. + // expiresAt is the API clock + TTL so expiry is driven by one authoritative clock. + CreateEmailOTP(ctx context.Context, id, userID, email, codeHash, purpose string, expiresAt time.Time) error + // VerifyEmailOTP redeems the newest live code for (userID, purpose) against + // codeHash, atomically (spec §B2). No live code, an expired one, or a consumed + // one → ErrOTPInvalid; an exhausted attempt budget → ErrOTPLocked; a hash + // mismatch increments attempts and returns ErrOTPInvalid WITHOUT consuming the + // code (so a typo does not burn it). On a match the code is consumed and the + // user row is flipped to email=, email_verified=true; the + // proven email is returned. now is the API clock so expiry is testable. + VerifyEmailOTP(ctx context.Context, userID, purpose, codeHash string, now time.Time) (email string, err error) + // ---- local-password auth (spec §B) ---- // UserByUsername loads the login projection of a staff account by its unique diff --git a/internal/store/migrations/0004_player_email_otp.sql b/internal/store/migrations/0004_player_email_otp.sql new file mode 100644 index 0000000..2150bb5 --- /dev/null +++ b/internal/store/migrations/0004_player_email_otp.sql @@ -0,0 +1,34 @@ +-- Phase B2 player onboarding: verified email via one-time code (OTP). +-- The forced web onboarding flow (link + email-OTP + passkey) needs to prove a +-- player controls an email address before it is bound to their account. A player +-- requests a code, the platform mails it, and the player types it back; only a +-- matching, unexpired, unconsumed code flips users.email_verified true and writes +-- the verified address onto the users row. + +-- email_verified records that the address on the users row was proven via OTP, not +-- merely asserted. It defaults false so every existing (and link-only) row reads +-- unverified until a code is redeemed; the verify path is the only writer. +ALTER TABLE users + ADD COLUMN email_verified boolean NOT NULL DEFAULT false; + +-- One row per outstanding (and historical) email code. Only the sha-256 of the +-- code is stored, never the digits the player typed, so a database read cannot +-- replay a live code (same principle as sessions.token_hash). A code is single-use: +-- consumed_at is stamped the moment it is redeemed, and attempts caps brute force +-- against the short numeric keyspace independently of expiry. +CREATE TABLE email_otps ( + id text PRIMARY KEY, -- opaque row id (random hex) + user_id text NOT NULL REFERENCES users(id), + email text NOT NULL, -- the address this code proves + code_hash text NOT NULL, -- sha-256(code); never the code + purpose text NOT NULL, -- e.g. 'onboard_email' + attempts int NOT NULL DEFAULT 0, -- wrong-guess counter, capped + expires_at timestamptz NOT NULL, -- short TTL set by the API clock + consumed_at timestamptz, -- non-NULL once redeemed (single-use) + created_at timestamptz NOT NULL DEFAULT now() +); + +-- The verify path looks up the newest live code for a (user, purpose), so index +-- that lookup. A user has at most one live code per purpose at a time (the mint +-- path supersedes the prior one), keeping this small. +CREATE INDEX email_otps_user_purpose_idx ON email_otps (user_id, purpose);