diff --git a/.env.example b/.env.example index ba53ac6..20b3c1d 100644 --- a/.env.example +++ b/.env.example @@ -45,11 +45,17 @@ MESHTENDER_TLS_CERT=./certs/dev.pem MESHTENDER_TLS_KEY=./certs/dev-key.pem # Outbound mail (Resend) — OPTIONAL, and used only for account recovery: verifying -# an address a user added, and sending a password-reset link. Leave the API key -# blank in dev: the server then LOGS each message (recipient, subject, full body) -# instead of sending it, so the whole recovery flow is walkable from the terminal -# with no provider, no verified domain, and no network. Recovery-by-email is hidden -# in the UI while it's unset, rather than offering mail that would never arrive. +# an address a user added, and sending a password-reset link. +# +# Two switches, deliberately separate: +# MAIL_FROM turns the email FEATURE on (the account page's Email card, the +# "Forgot password?" link). Unset ⇒ none of it is shown, so we never +# offer recovery mail we couldn't deliver. +# RESEND_API_KEY turns real DELIVERY on. Without it — the dev default below — the +# server LOGS each message (recipient, subject, full body) instead of +# sending it, so the whole flow is walkable from the terminal with no +# provider, no verified domain, and no network. +MESHTENDER_MAIL_FROM=MeshTender # # Before setting a key in production: # 1. Add the sending domain in Resend and publish its SPF + DKIM records. @@ -59,5 +65,4 @@ MESHTENDER_TLS_KEY=./certs/dev-key.pem # Free tier: 3,000 emails/month with a 100/day cap — the reset flow is rate limited # per IP and per address partly to stay inside that. # MESHTENDER_RESEND_API_KEY= -# MESHTENDER_MAIL_FROM=MeshTender # MESHTENDER_MAIL_REPLY_TO= diff --git a/cmd/meshtender/main.go b/cmd/meshtender/main.go index be075d2..287ced1 100644 --- a/cmd/meshtender/main.go +++ b/cmd/meshtender/main.go @@ -87,16 +87,18 @@ func run(logger *slog.Logger) error { } logger.Info("server identity ready", "pubkey", idSvc.PublicKeyHex()) - // Outbound mail is optional. Without a provider the server logs messages - // instead of sending them, so account recovery is fully walkable in dev, and - // cfg.MailEnabled keeps the UI from offering mail that would never arrive. + // Delivery is keyed on the API key, not on cfg.MailEnabled: MailEnabled is the + // feature switch (is the email UI offered at all), while the key decides whether + // messages actually leave the process. Setting only MESHTENDER_MAIL_FROM is the + // dev configuration — the recovery flow is live and its links land in this log. var sender mail.Sender - if cfg.MailEnabled { + if cfg.ResendAPIKey != "" { sender = mail.NewResend(cfg.ResendAPIKey, cfg.MailFrom, cfg.MailReplyTo) logger.Info("mail provider ready", "from", cfg.MailFrom) } else { sender = &mail.LogSender{Logger: logger} - logger.Warn("no mail provider configured — recovery messages will be logged, not sent") + logger.Warn("no mail provider configured — messages will be logged, not sent", + "email_features_offered", cfg.MailEnabled) } authSvc, err := auth.New(st, st.Pool(), auth.Config{ @@ -148,6 +150,9 @@ func run(logger *slog.Logger) error { runJanitor(bgCtx, janitorInterval, logger, janitorSweep{"auth codes", st.PruneAuthCodes}, janitorSweep{"share links", st.PruneInvites}, + // Verification and reset links are deleted only when someone actually + // clicks one, so abandoned ones would otherwise pile up forever. + janitorSweep{"email tokens", st.PruneEmailTokens}, // A violation that stops recurring eventually ages out, so a fixed // problem leaves the admin page on its own. janitorSweep{"csp reports", func(ctx context.Context) (int64, error) { diff --git a/internal/auth/account.go b/internal/auth/account.go index bd99259..1c30e92 100644 --- a/internal/auth/account.go +++ b/internal/auth/account.go @@ -88,6 +88,15 @@ func (s *Handlers) renderAccount(w http.ResponseWriter, r *http.Request, uid int "Platforms": store.UserLinkPlatforms(), "PlatformsJS": web.LinkPlatformsJS(store.UserLinkPlatforms()), "HasPassword": u.PasswordHash != nil, + // Email state, rendered honestly rather than as a single "has email" flag: + // what an address does for this account depends on whether it's confirmed AND + // whether there's a password to reset (see store.User.CanResetPassword). + "Email": u.Email, // nil when unset + "EmailVerified": u.EmailVerified(), + "CanResetByMail": u.CanResetPassword(), + // False when no mail provider is configured, which hides the whole card: + // offering recovery we can't deliver is worse than not offering it. + "MailEnabled": s.Auth.MailEnabled(), // Stated in the change-password form and used as minlength, so the client // hint can't drift from the server's floor. "MinPasswordLen": MinPasswordLen, @@ -97,6 +106,8 @@ func (s *Handlers) renderAccount(w http.ResponseWriter, r *http.Request, uid int "OK": r.URL.Query().Get("ok"), "PKMsg": r.URL.Query().Get("pk"), "PKErr": r.URL.Query().Get("pkerr"), + "EmMsg": r.URL.Query().Get(flashEmail), + "EmErr": r.URL.Query().Get(flashEmailErr), } for k, v := range overrides { data[k] = v diff --git a/internal/auth/email.go b/internal/auth/email.go new file mode 100644 index 0000000..7ac0285 --- /dev/null +++ b/internal/auth/email.go @@ -0,0 +1,172 @@ +package auth + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "log/slog" + "net/http" + "net/mail" + "strings" + "time" + + mailer "github.com/jleight/meshtender/internal/mail" + "github.com/jleight/meshtender/internal/store" +) + +// ErrTooManyEmails reports that an account has asked for more messages of one kind +// than its budget allows. Surfaced to the user as "try again later" — the cap exists +// so nobody can use us to flood a mailbox, and so a metered daily send quota can't +// be drained by one account. +var ErrTooManyEmails = errors.New("auth: too many email requests") + +// Per-account send budgets, counted over sendBudgetWindow. Both are generous for a +// real person (a resend after a typo, a second attempt when the first mail is slow) +// and useless for anyone trying to bury an inbox. +const ( + sendBudgetWindow = time.Hour + maxVerifySends = 5 + maxResetSends = 3 +) + +// mailSendTimeout bounds the provider call a handler waits on. +const mailSendTimeout = 15 * time.Second + +// MaxEmailLen bounds a stored address. RFC 5321 caps a path at 256 octets; this is +// the same order and keeps an absurd paste out of the column and the mail body. +const MaxEmailLen = 254 + +// NormalizeEmailInput trims a submitted address. Case is preserved as typed — the +// user sees their own capitalization back — while all comparison is +// case-insensitive (store.NormalizeEmail). +func NormalizeEmailInput(s string) string { return strings.TrimSpace(s) } + +// ValidEmail reports whether addr is a single, plausible address. It leans on +// net/mail's parser rather than a hand-rolled regex, then rejects the display-name +// form ("Someone ") that ParseAddress also accepts: we store a bare address, +// and letting a display name through would put attacker-chosen text into a +// To: header. +func ValidEmail(addr string) bool { + if addr == "" || len(addr) > MaxEmailLen { + return false + } + parsed, err := mail.ParseAddress(addr) + if err != nil { + return false + } + return parsed.Name == "" && parsed.Address == addr +} + +// SetAccountEmail stores a new address for the user and mails a confirmation link. +// The address is unverified until that link is clicked, so nothing can be sent to it +// beyond this one message. +func (s *Service) SetAccountEmail(ctx context.Context, r *http.Request, userID int64, addr string) error { + if err := s.store.SetEmail(ctx, userID, addr); err != nil { + return err + } + return s.SendEmailVerification(ctx, r, userID) +} + +// SendEmailVerification mails a fresh confirmation link for the user's current +// address. Used both when an address is first set and when the user asks for a +// resend. A verified address is a no-op — there is nothing to confirm. +func (s *Service) SendEmailVerification(ctx context.Context, r *http.Request, userID int64) error { + u, err := s.store.GetUserByID(ctx, userID) + if err != nil { + return err + } + if u.Email == nil || *u.Email == "" || u.EmailVerifiedAt != nil { + return nil + } + addr := *u.Email + + n, err := s.store.CountRecentEmailTokens(ctx, userID, store.PurposeVerifyEmail, sendBudgetWindow) + if err != nil { + return err + } + if n >= maxVerifySends { + return ErrTooManyEmails + } + + token, err := s.store.CreateEmailToken(ctx, userID, store.PurposeVerifyEmail, addr, store.VerifyTokenTTL) + if err != nil { + return err + } + link := s.authOrigin(r) + "/verify-email/" + token + return s.send(ctx, mailer.Message{ + To: addr, + Subject: "Confirm your email for MeshTender", + Kind: mailer.KindVerifyEmail, + // Keyed on the token, so a double-submitted form can't mail twice. + IdempotencyKey: hashForIdempotency(token), + Text: fmt.Sprintf(`Hi %s, + +Confirm this address so it can be used to recover your MeshTender account: + +%s + +The link works once and expires in %s. + +If you didn't add this address to a MeshTender account, ignore this message — +nothing will change, and the address won't be used for anything. +`, atName(u), link, humanDuration(store.VerifyTokenTTL)), + }) +} + +// send delivers a message on a context detached from the request's. +// +// The token is already committed by the time we get here, so a visitor who closes +// the tab mid-send must not abort delivery — that would leave a live token whose +// link nobody ever received, and the user would have to start over for no visible +// reason. The timeout keeps the detached call bounded. +func (s *Service) send(ctx context.Context, m mailer.Message) error { + sendCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), mailSendTimeout) + defer cancel() + if err := s.mail.Send(sendCtx, m); err != nil { + // Logged here rather than only bubbling up, because the caller deliberately + // tells the user as little as possible (the reset flow must not confirm + // whether an address exists), and a quota wall is an operational problem the + // operator needs to see. + level := slog.LevelError + if errors.Is(err, mailer.ErrQuota) { + level = slog.LevelWarn + } + slog.Log(sendCtx, level, "mail send failed", "kind", string(m.Kind), "err", err) + return err + } + return nil +} + +// hashForIdempotency derives the provider's de-duplication key from a token. The +// token itself is already in the body we hand the provider, so this isn't about +// secrecy — it's about not also placing a live credential in a header, which +// providers log and retain on different terms than message bodies. Truncated +// because collisions only matter across one account's in-flight sends. +func hashForIdempotency(token string) string { + sum := sha256.Sum256([]byte("meshtender-idempotency:" + token)) + return hex.EncodeToString(sum[:16]) +} + +// atName renders a user as "@username" for message bodies. Deliberately the +// username, not the display name: it's what they type to sign in, and the reset mail +// has to name accounts unambiguously when several share one address. +func atName(u *store.User) string { return "@" + u.Username } + +// humanDuration renders a TTL the way copy should read it ("24 hours", "45 +// minutes"), so the stated shelf life can't drift from the constant that enforces it. +func humanDuration(d time.Duration) string { + if d >= time.Hour { + h := int(d.Hours()) + if h == 1 { + return "1 hour" + } + return fmt.Sprintf("%d hours", h) + } + m := int(d.Minutes()) + if m == 1 { + return "1 minute" + } + return fmt.Sprintf("%d minutes", m) +} diff --git a/internal/auth/email_handlers.go b/internal/auth/email_handlers.go new file mode 100644 index 0000000..ecc5d4d --- /dev/null +++ b/internal/auth/email_handlers.go @@ -0,0 +1,122 @@ +package auth + +import ( + "errors" + "net/http" + + "github.com/go-chi/chi/v5" + + "github.com/jleight/meshtender/internal/store" + "github.com/jleight/meshtender/internal/web" +) + +// Flash keys "em" (success) and "emerr" (failure) render inside the account page's +// Email card rather than the top banner, so a message about an address stays next to +// the controls that produced it — the same split the Passkeys card uses. +const ( + flashEmail = "em" + flashEmailErr = "emerr" +) + +// handleSetEmail stores or clears the account's email address. +// +// Setting an address always mails a confirmation link; until that link is clicked +// the address is unverified and can't be used for recovery. Clearing is immediate +// and takes any outstanding links with it. +func (s *Handlers) handleSetEmail(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + uid := s.Auth.CurrentUserID(ctx) + + if r.FormValue("remove") == "1" { + if err := s.Store.ClearEmail(ctx, uid); err != nil { + accountRedirect(w, r, flashEmailErr, "Could not remove your email address.") + return + } + accountRedirect(w, r, flashEmail, "Email address removed.") + return + } + + addr := NormalizeEmailInput(r.FormValue("email")) + if !ValidEmail(addr) { + accountRedirect(w, r, flashEmailErr, "Enter a single email address, like you@example.com.") + return + } + if err := s.Auth.SetAccountEmail(ctx, r, uid, addr); err != nil { + // The address is stored either way — only the confirmation mail failed — so + // the message points at the resend button rather than implying nothing + // happened. + if errors.Is(err, ErrTooManyEmails) { + accountRedirect(w, r, flashEmailErr, "Too many confirmation emails requested. Try again in an hour.") + return + } + accountRedirect(w, r, flashEmailErr, "Saved your address, but the confirmation email could not be sent. Try resending it.") + return + } + accountRedirect(w, r, flashEmail, "Check "+addr+" for a link to confirm the address.") +} + +// handleResendEmailVerification mails a fresh confirmation link for an address the +// user has already saved. +func (s *Handlers) handleResendEmailVerification(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + uid := s.Auth.CurrentUserID(ctx) + + if err := s.Auth.SendEmailVerification(ctx, r, uid); err != nil { + if errors.Is(err, ErrTooManyEmails) { + accountRedirect(w, r, flashEmailErr, "Too many confirmation emails requested. Try again in an hour.") + return + } + accountRedirect(w, r, flashEmailErr, "Could not send the confirmation email. Try again shortly.") + return + } + accountRedirect(w, r, flashEmail, "Confirmation email sent.") +} + +// handleVerifyEmail redeems a confirmation link. +// +// Registered without an authentication requirement on purpose: the link is opened +// from a mailbox, which is often a different browser (or a phone) from the one that +// added the address. The token is the proof, and it names exactly one account — so +// requiring a session would only break the common case. +func (s *Handlers) handleVerifyEmail(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + token := chi.URLParam(r, "token") + + tok, ok, err := s.Store.ConsumeEmailToken(ctx, store.PurposeVerifyEmail, token) + if err != nil { + s.ServerError(w, r, "could not verify email", err) + return + } + if !ok { + s.verifyResult(w, r, flashEmailErr, "That confirmation link is invalid or has expired. Sign in and send a new one.") + return + } + // Re-checked against the address the token was issued for, so a link minted for + // one address can't confirm a different one saved afterwards. + updated, err := s.Store.MarkEmailVerified(ctx, tok.UserID, tok.Email) + if err != nil { + s.ServerError(w, r, "could not verify email", err) + return + } + if !updated { + s.verifyResult(w, r, flashEmailErr, "That link was for a different address than the one on the account now. Send a new confirmation email.") + return + } + s.verifyResult(w, r, flashEmail, "Email confirmed. You can now reset your password by email if you ever need to.") +} + +// verifyResult lands the visitor somewhere sensible after a verification attempt: +// the account page when this browser is signed in (they can see the result in +// context), otherwise the sign-in page, since the link may well have been opened on +// a device with no session. +func (s *Handlers) verifyResult(w http.ResponseWriter, r *http.Request, key, msg string) { + if s.Auth.CurrentUserID(r.Context()) != 0 { + accountRedirect(w, r, key, msg) + return + } + loginKey := "ok" + if key == flashEmailErr { + loginKey = "error" + } + web.RedirectFlash(w, r, "/login", loginKey, msg) +} diff --git a/internal/auth/templates/account.html b/internal/auth/templates/account.html index 69b7f90..9d2d3d9 100644 --- a/internal/auth/templates/account.html +++ b/internal/auth/templates/account.html @@ -53,6 +53,78 @@ +{{/* Email card. Hidden entirely when no mail provider is configured: an address we + can't send to would be a promise we can't keep. What the card SAYS depends on + three things — whether an address is set, whether it's confirmed, and whether + there's a password to reset — because a passkey-only account's address genuinely + can't recover it, and implying otherwise is the one thing worse than silence. + + Placed here, with the account-identity fields, because that's where people look + for it — but it sits directly above the two PUBLIC cards, so the header carries + "(private)" the way Links carries "(public)". Position reads louder than body + copy, and an address nobody meant to publish is the one mistake worth this much + redundancy. */}} +{{if .MailEnabled}} +
+

Email (private)

+
+

+ Optional. Used to recover your account if you forget your password. It's never shown + on your public profile, and we only email you about your account — never marketing. +

+ {{if .EmErr}}
{{.EmErr}}
{{end}} + {{if .EmMsg}}
{{.EmMsg}}
{{end}} + + {{if .Email}} +
+
+ {{template "icon-mail" ""}} +
+
{{.Email}}
+
+ {{if .EmailVerified}} + {{if .CanResetByMail}} + confirmed You can reset your password by email. + {{else}} + confirmed + This account has no password, so there's nothing to reset — sign-in is by passkey only. + To keep access if you lose your device, add a passkey on a second device in the Passkeys section below. + {{end}} + {{else}} + unconfirmed + Check your inbox for the confirmation link. Until it's confirmed, this address can't recover your account. + {{end}} +
+
+ {{if not .EmailVerified}} +
+ +
+ {{end}} +
+ + +
+
+
+ {{end}} + +
+
+
+ + +
+
+ +
+
+ We'll send a confirmation link. A new address has to be confirmed before it can recover your account. +
+
+
+{{end}} +

Time zone

@@ -249,7 +321,10 @@ {{if .HasPassword}} -
+ {{/* The confirm text names the recovery consequence, not just the sign-in one: + email reset only ever sets a password on an account that HAS one, so + removing it silently turns a working recovery path into a dead end. */}} + Only available if you have at least one passkey. diff --git a/internal/auth/web.go b/internal/auth/web.go index a2e0e32..6374162 100644 --- a/internal/auth/web.go +++ b/internal/auth/web.go @@ -83,6 +83,8 @@ func (s *Handlers) Routes() chi.Router { r.Post("/account/profile-fields", s.handleSetProfileFields) r.Post("/account/links", s.handleSetUserLinks) r.Post("/account/password", s.handleChangePassword) + r.Post("/account/email", s.handleSetEmail) + r.Post("/account/email/verify", s.handleResendEmailVerification) r.Post("/account/passkeys/rename", s.handleRenamePasskey) r.Post("/account/passkeys/delete", s.handleDeletePasskey) }) @@ -100,6 +102,12 @@ func (s *Handlers) Routes() chi.Router { func (s *Handlers) credentialRoutes(r chi.Router) { r.Get("/login", s.pageLogin) r.Get("/signup", s.pageSignup) + // Confirming an email address is deliberately session-free: the link is opened + // from a mailbox, frequently on a different device than the one that added the + // address. The single-use token names the account, so a session would add nothing + // but a failure mode. It sits here (inside the session-loading group but outside + // RequireSSO) so the page chrome still renders signed-in-aware. + r.Get("/verify-email/{token}", s.handleVerifyEmail) // Throttle credential submission per client IP to blunt password guessing // and signup spam. Allows a burst (e.g. fat-fingered retries), then ~1 try // every 6s; bcrypt's cost is the second line of defense. diff --git a/internal/config/config.go b/internal/config/config.go index 517d198..d299d5b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -124,7 +124,12 @@ func Load() (*Config, error) { if c.ResendAPIKey != "" && c.MailFrom == "" { return nil, fmt.Errorf("MESHTENDER_MAIL_FROM is required when MESHTENDER_RESEND_API_KEY is set") } - c.MailEnabled = c.ResendAPIKey != "" && c.MailFrom != "" + // MailFrom is the switch for the FEATURE; the API key is the switch for real + // DELIVERY. Keeping them separate is what makes the flow walkable in dev: set only + // MailFrom and the recovery UI is live while messages go to the log. Gating the + // feature on the API key instead would hide every email control locally, which + // would make the logging sender useless for the case it exists to serve. + c.MailEnabled = c.MailFrom != "" // MeshTender runs across three hosts (auth + app + root). Require the two that // have no sane default (PrimaryHost falls back to RPID above). diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 11d8685..4d9929d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -26,9 +26,8 @@ func validEnv(t *testing.T) { t.Setenv("MESHTENDER_MAIL_REPLY_TO", "") } -// TestLoadMailDisabledByDefault: no API key means no delivery path, and the flag -// that gates the recovery-by-email UI must say so rather than the UI offering mail -// that would never arrive. +// TestLoadMailDisabledByDefault: with nothing configured there's no from-address, so +// the recovery-by-email UI stays hidden rather than offering mail that can't be sent. func TestLoadMailDisabledByDefault(t *testing.T) { validEnv(t) c, err := Load() @@ -36,7 +35,26 @@ func TestLoadMailDisabledByDefault(t *testing.T) { t.Fatalf("Load: %v", err) } if c.MailEnabled { - t.Fatal("MailEnabled = true with no API key configured") + t.Fatal("MailEnabled = true with nothing configured") + } +} + +// TestLoadMailEnabledWithoutAPIKey is the dev path, and the reason the feature switch +// and the delivery switch are separate: MAIL_FROM alone turns the recovery UI on +// while messages go to the log. If this required an API key, none of the email UI +// would be reachable locally and the logging sender would be useless. +func TestLoadMailEnabledWithoutAPIKey(t *testing.T) { + validEnv(t) + t.Setenv("MESHTENDER_MAIL_FROM", "MeshTender ") + c, err := Load() + if err != nil { + t.Fatalf("Load: %v", err) + } + if !c.MailEnabled { + t.Error("MailEnabled = false with a from-address set") + } + if c.ResendAPIKey != "" { + t.Error("an API key appeared from nowhere") } } @@ -51,7 +69,7 @@ func TestLoadMailKeyWithoutFromFailsFast(t *testing.T) { } } -// TestLoadMailEnabled: both halves present ⇒ mail is on. +// TestLoadMailEnabled: both halves present ⇒ mail is on and delivers for real. func TestLoadMailEnabled(t *testing.T) { validEnv(t) t.Setenv("MESHTENDER_RESEND_API_KEY", "re_live_key") diff --git a/internal/core/email_recovery_test.go b/internal/core/email_recovery_test.go new file mode 100644 index 0000000..c2f5094 --- /dev/null +++ b/internal/core/email_recovery_test.go @@ -0,0 +1,445 @@ +package core + +import ( + "context" + "io" + "net/http" + "net/http/cookiejar" + "net/http/httptest" + "net/url" + "regexp" + "strings" + "sync" + "testing" + + "golang.org/x/crypto/bcrypt" + + mailer "github.com/jleight/meshtender/internal/mail" +) + +// Black-box coverage for the optional account email: setting it, confirming it, and +// what the UI promises about it. The reset flow itself is covered separately. + +// fakeSender captures outbound mail instead of delivering it. +type fakeSender struct { + mu sync.Mutex + sent []mailer.Message +} + +func (f *fakeSender) Send(_ context.Context, m mailer.Message) error { + f.mu.Lock() + defer f.mu.Unlock() + f.sent = append(f.sent, m) + return nil +} + +func (f *fakeSender) count() int { + f.mu.Lock() + defer f.mu.Unlock() + return len(f.sent) +} + +// last returns the most recent message, failing the test if nothing was sent. +func (f *fakeSender) last(t *testing.T) mailer.Message { + t.Helper() + f.mu.Lock() + defer f.mu.Unlock() + if len(f.sent) == 0 { + t.Fatal("no mail was sent") + } + return f.sent[len(f.sent)-1] +} + +func (f *fakeSender) reset() { + f.mu.Lock() + defer f.mu.Unlock() + f.sent = nil +} + +// tokenPathRe pulls a token out of a link in a message body, so tests follow the +// same URL a recipient would click rather than reaching into the store. +var tokenPathRe = regexp.MustCompile(`https?://[^/\s]+(/(?:verify-email|reset)/[A-Za-z0-9_-]+)`) + +// linkPath extracts the first recovery link's path from a message body. +func linkPath(t *testing.T, m mailer.Message) string { + t.Helper() + match := tokenPathRe.FindStringSubmatch(m.Text) + if match == nil { + t.Fatalf("no recovery link in message body:\n%s", m.Text) + } + return match[1] +} + +// setEmail posts an address to the account page with the given SSO session. +func setEmail(t *testing.T, ts *httptest.Server, h hostEnv, sso *http.Cookie, addr string) *http.Response { + t.Helper() + return post(t, ts, h.auth, "/account/email", url.Values{"email": {addr}}, sso) +} + +// TestSetEmailStartsUnverifiedAndMailsLink: saving an address must not trust it. +// Confirmation is what stops a typo from pointing recovery at a stranger's mailbox. +func TestSetEmailStartsUnverifiedAndMailsLink(t *testing.T) { + t.Parallel() + st, ctx, ts, h, sender := splitServerMail(t) + sso := authSSO(t, ts, h, "emailuser") + + resp := setEmail(t, ts, h, sso, "user@example.test") + resp.Body.Close() + if resp.StatusCode != http.StatusSeeOther { + t.Fatalf("set email = %d, want 303", resp.StatusCode) + } + + u, err := st.GetUserByUsername(ctx, "emailuser") + if err != nil { + t.Fatalf("get user: %v", err) + } + if u.Email == nil || *u.Email != "user@example.test" { + t.Fatalf("stored email = %v", u.Email) + } + if u.EmailVerified() { + t.Error("address is verified straight from the form, with nothing proving the user owns it") + } + + msg := sender.last(t) + if msg.Kind != mailer.KindVerifyEmail { + t.Errorf("Kind = %q, want %q", msg.Kind, mailer.KindVerifyEmail) + } + if msg.To != "user@example.test" { + t.Errorf("To = %q", msg.To) + } + if msg.IdempotencyKey == "" { + t.Error("no idempotency key, so a resubmitted form can mail the link twice") + } + // The raw token must not be reused as the provider-facing key. + if strings.Contains(msg.Text, msg.IdempotencyKey) { + t.Error("idempotency key is the token itself; it should be derived") + } +} + +// TestVerifyEmailLinkConfirmsAddress walks the confirmation the way a recipient +// does — following the link from the message body, with no session, since the +// mailbox is usually open in another browser or on a phone. +func TestVerifyEmailLinkConfirmsAddress(t *testing.T) { + t.Parallel() + st, ctx, ts, h, sender := splitServerMail(t) + sso := authSSO(t, ts, h, "verifyuser") + setEmail(t, ts, h, sso, "verify@example.test").Body.Close() + + path := linkPath(t, sender.last(t)) + // No cookie: a link opened in a different browser must still work. + resp := do(t, ts, h.auth, path) + resp.Body.Close() + if resp.StatusCode != http.StatusSeeOther { + t.Fatalf("verify = %d, want 303", resp.StatusCode) + } + + u, err := st.GetUserByUsername(ctx, "verifyuser") + if err != nil { + t.Fatalf("get user: %v", err) + } + if !u.EmailVerified() { + t.Fatal("address still unverified after following the confirmation link") + } + if !u.CanResetPassword() { + t.Error("a password account with a confirmed address still can't be reset by email") + } + + // Single-use: the same link must not confirm anything a second time. + again := do(t, ts, h.auth, path) + again.Body.Close() + loc, _ := url.Parse(again.Header.Get("Location")) + if loc.Query().Get("emerr") == "" && loc.Query().Get("error") == "" { + t.Error("a spent confirmation link reported success on replay") + } +} + +// TestVerifyEmailRejectsGarbageToken: an unknown token is an invalid link, never a +// server error and never a silent success. +func TestVerifyEmailRejectsGarbageToken(t *testing.T) { + t.Parallel() + _, _, ts, h, _ := splitServerMail(t) + + resp := do(t, ts, h.auth, "/verify-email/not-a-real-token") + resp.Body.Close() + if resp.StatusCode != http.StatusSeeOther { + t.Fatalf("verify with garbage token = %d, want 303", resp.StatusCode) + } + loc, _ := url.Parse(resp.Header.Get("Location")) + if loc.Query().Get("emerr") == "" && loc.Query().Get("error") == "" { + t.Error("a bogus token produced no error message") + } +} + +// TestStaleVerifyLinkCannotConfirmNewAddress closes the swap window: ask for a link +// for one address, change the address, then click the old link. Without the +// address re-check, the stale mail would confirm an address its recipient never +// approved. +func TestStaleVerifyLinkCannotConfirmNewAddress(t *testing.T) { + t.Parallel() + st, ctx, ts, h, sender := splitServerMail(t) + sso := authSSO(t, ts, h, "swapuser") + + setEmail(t, ts, h, sso, "first@example.test").Body.Close() + stale := linkPath(t, sender.last(t)) + setEmail(t, ts, h, sso, "second@example.test").Body.Close() + + resp := do(t, ts, h.auth, stale) + resp.Body.Close() + + u, err := st.GetUserByUsername(ctx, "swapuser") + if err != nil { + t.Fatalf("get user: %v", err) + } + if u.EmailVerified() { + t.Fatal("a link issued for the previous address confirmed the current one") + } +} + +// TestSetEmailRejectsBadAddresses: the stored value ends up in a To: header, so a +// display-name form or a bare word must not get through. +func TestSetEmailRejectsBadAddresses(t *testing.T) { + t.Parallel() + st, ctx, ts, h, sender := splitServerMail(t) + sso := authSSO(t, ts, h, "badaddr") + + for _, bad := range []string{ + "", + "not-an-address", + "Someone ", // display-name form + "a@example.test, b@example.test", // two addresses + "a@example.test\nBcc: victim@example.test", // header injection attempt + } { + resp := setEmail(t, ts, h, sso, bad) + resp.Body.Close() + loc, _ := url.Parse(resp.Header.Get("Location")) + if loc.Query().Get("emerr") == "" { + t.Errorf("address %q was accepted", bad) + } + } + u, err := st.GetUserByUsername(ctx, "badaddr") + if err != nil { + t.Fatalf("get user: %v", err) + } + if u.Email != nil { + t.Errorf("stored email = %v after only invalid submissions", u.Email) + } + if sender.count() != 0 { + t.Errorf("%d messages sent for invalid addresses", sender.count()) + } +} + +// TestRemoveEmailIsImmediateAndKillsLinks: "optional" has to mean removable, and +// removal must invalidate links already sitting in a mailbox the user may no longer +// control. +func TestRemoveEmailIsImmediateAndKillsLinks(t *testing.T) { + t.Parallel() + st, ctx, ts, h, sender := splitServerMail(t) + sso := authSSO(t, ts, h, "removeuser") + setEmail(t, ts, h, sso, "gone@example.test").Body.Close() + stale := linkPath(t, sender.last(t)) + + resp := post(t, ts, h.auth, "/account/email", url.Values{"remove": {"1"}}, sso) + resp.Body.Close() + + u, err := st.GetUserByUsername(ctx, "removeuser") + if err != nil { + t.Fatalf("get user: %v", err) + } + if u.Email != nil { + t.Errorf("Email = %v after removal, want nil", u.Email) + } + // The outstanding confirmation link must be dead. + do(t, ts, h.auth, stale).Body.Close() + after, err := st.GetUserByUsername(ctx, "removeuser") + if err != nil { + t.Fatalf("get user: %v", err) + } + if after.Email != nil || after.EmailVerified() { + t.Error("a link from before removal resurrected the address") + } +} + +// TestVerificationResendIsCapped: the per-account budget is what stops us being used +// to bury someone's inbox (and stops one account draining a metered daily quota). +func TestVerificationResendIsCapped(t *testing.T) { + t.Parallel() + _, _, ts, h, sender := splitServerMail(t) + sso := authSSO(t, ts, h, "resenduser") + setEmail(t, ts, h, sso, "resend@example.test").Body.Close() + sender.reset() + + var capped bool + // The first send already spent one token, so the budget runs out inside this loop. + for range 10 { + resp := post(t, ts, h.auth, "/account/email/verify", url.Values{}, sso) + resp.Body.Close() + loc, _ := url.Parse(resp.Header.Get("Location")) + if strings.Contains(strings.ToLower(loc.Query().Get("emerr")), "too many") { + capped = true + break + } + } + if !capped { + t.Fatal("resend was never capped; an account can mail its address without limit") + } + if sender.count() >= 10 { + t.Errorf("%d messages sent before the cap bit", sender.count()) + } +} + +// TestAccountPageStatesAreHonest is the anti-false-promise test. What the Email card +// claims must match what the account can actually do — in particular, a passkey-only +// account must be told plainly that its address cannot recover it, rather than being +// shown reassuring copy that only applies to password holders. +func TestAccountPageStatesAreHonest(t *testing.T) { + t.Parallel() + st, ctx, ts, h, sender := splitServerMail(t) + + t.Run("password account with a confirmed address is told reset works", func(t *testing.T) { + sso := authSSO(t, ts, h, "honest-pw") + setEmail(t, ts, h, sso, "honest-pw@example.test").Body.Close() + do(t, ts, h.auth, linkPath(t, sender.last(t))).Body.Close() + + page := authPageHTML(t, ts, h, "/account", sso) + if !strings.Contains(page, "reset your password by email") { + t.Error("a resettable account isn't told it can reset by email") + } + }) + + t.Run("passkey-only account is told its address cannot recover it", func(t *testing.T) { + sso := authSSO(t, ts, h, "honest-pk") + setEmail(t, ts, h, sso, "honest-pk@example.test").Body.Close() + do(t, ts, h.auth, linkPath(t, sender.last(t))).Body.Close() + + u, err := st.GetUserByUsername(ctx, "honest-pk") + if err != nil { + t.Fatalf("get user: %v", err) + } + // Drop the password directly: the UI route requires a passkey first, and this + // test is about the resulting state, not that guard. + if err := st.ClearPassword(ctx, u.ID); err != nil { + t.Fatalf("clear password: %v", err) + } + + page := authPageHTML(t, ts, h, "/account", sso) + if !strings.Contains(page, "no password, so there's nothing to reset") { + t.Error("a passkey-only account isn't told its address can't recover it") + } + if strings.Contains(page, "You can reset your password by email") { + t.Error("a passkey-only account is promised email reset it will never get") + } + }) +} + +// TestEmailNeverAppearsOnPublicProfile: the address is recovery data, not profile +// data. Leaking it on /u/{username} would publish something users gave us for one +// narrow purpose. +func TestEmailNeverAppearsOnPublicProfile(t *testing.T) { + t.Parallel() + _, _, ts, h, sender := splitServerMail(t) + const addr = "private@example.test" + sso := authSSO(t, ts, h, "publicuser") + setEmail(t, ts, h, sso, addr).Body.Close() + do(t, ts, h.auth, linkPath(t, sender.last(t))).Body.Close() + + for _, page := range []struct{ host, path string }{ + {h.root, "/u/publicuser"}, + {h.app, "/u/publicuser"}, + } { + resp := do(t, ts, page.host, page.path) + body := readAll(t, resp) + if strings.Contains(body, addr) { + t.Errorf("%s%s leaks the account's email address", page.host, page.path) + } + } +} + +// TestRecoveryEmailChecklistStepTargetsPasswordUsers: the nudge is only meaningful +// for accounts that can actually use it. Asking a passkey-only user for an address we +// would never send a reset to is asking for data we can't use. +func TestRecoveryEmailChecklistStepTargetsPasswordUsers(t *testing.T) { + t.Parallel() + st, ctx, ts, h, _ := splitServerMail(t) + + t.Run("password user with no address sees the step", func(t *testing.T) { + jar := newJar(t) + u := seedSession(t, ts, st, ctx, jar, "checklist-pw") + if err := st.SetPassword(ctx, u.ID, testBcryptHash(t)); err != nil { + t.Fatalf("set password: %v", err) + } + page := dashboardHTML(t, ts, h.app, jar.Cookies(mustURL(t, ts.URL))) + if !strings.Contains(page, "Add a recovery email") { + t.Error("password account isn't nudged to add a recovery email") + } + if !strings.Contains(page, "Add email") { + t.Error("the step renders no CTA, so it reads as already satisfied") + } + }) + + t.Run("passkey-only user is not asked", func(t *testing.T) { + jar := newJar(t) + seedSession(t, ts, st, ctx, jar, "checklist-pk") // no password + page := dashboardHTML(t, ts, h.app, jar.Cookies(mustURL(t, ts.URL))) + if strings.Contains(page, "Add a recovery email") { + t.Error("a passkey-only account is asked for an address that could never recover it") + } + }) + + t.Run("step is satisfied once the address is confirmed", func(t *testing.T) { + jar := newJar(t) + u := seedSession(t, ts, st, ctx, jar, "checklist-done") + if err := st.SetPassword(ctx, u.ID, testBcryptHash(t)); err != nil { + t.Fatalf("set password: %v", err) + } + if err := st.SetEmail(ctx, u.ID, "done@example.test"); err != nil { + t.Fatalf("set email: %v", err) + } + if ok, err := st.MarkEmailVerified(ctx, u.ID, "done@example.test"); err != nil || !ok { + t.Fatalf("mark verified: ok=%v err=%v", ok, err) + } + page := dashboardHTML(t, ts, h.app, jar.Cookies(mustURL(t, ts.URL))) + if strings.Contains(page, "Add email") { + t.Error("the step still shows its CTA for someone with a confirmed address") + } + }) + +} + +// authPageHTML fetches an auth-host page with the given session and returns its body. +func authPageHTML(t *testing.T, ts *httptest.Server, h hostEnv, path string, sso *http.Cookie) string { + t.Helper() + resp := do(t, ts, h.auth, path, sso) + return readAll(t, resp) +} + +// readAll drains and closes a response body. +func readAll(t *testing.T, resp *http.Response) string { + t.Helper() + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read body: %v", err) + } + return string(body) +} + +// newJar returns a cookie jar for driving app-host sessions. +func newJar(t *testing.T) *cookiejar.Jar { + t.Helper() + jar, err := cookiejar.New(nil) + if err != nil { + t.Fatalf("cookiejar: %v", err) + } + return jar +} + +// testBcryptHash is a stored password hash for fixtures that only need "this account +// has a password", without going through the sign-up form. +func testBcryptHash(t *testing.T) string { + t.Helper() + h, err := bcrypt.GenerateFromPassword([]byte(testPassword), bcrypt.MinCost) + if err != nil { + t.Fatalf("bcrypt: %v", err) + } + return string(h) +} diff --git a/internal/core/handoff_test.go b/internal/core/handoff_test.go index a30024f..6053d79 100644 --- a/internal/core/handoff_test.go +++ b/internal/core/handoff_test.go @@ -40,15 +40,27 @@ func TestFixturePasswordMeetsFloor(t *testing.T) { } func splitServer(t *testing.T) (*store.Store, context.Context, *httptest.Server, hostEnv) { + t.Helper() + st, ctx, ts, h, _ := splitServerMail(t) + return st, ctx, ts, h +} + +// splitServerMail is splitServer plus the captured outbound mail. Every test server +// gets a capturing sender (never a real provider, and never the log sender — which +// would print recovery links into test output), with mail reported as configured so +// the recovery UI is exercised rather than hidden. +func splitServerMail(t *testing.T) (*store.Store, context.Context, *httptest.Server, hostEnv, *fakeSender) { t.Helper() st, ctx := coreStore(t) masterKey := testMasterKey idSvc, _ := identity.LoadOrCreate(ctx, st, masterKey) + sender := &fakeSender{} authSvc, err := auth.New(st, st.Pool(), auth.Config{ RPID: "localhost", RPDisplayName: "test", RPOrigins: []string{"http://auth.localhost", "http://app.localhost"}, AppHost: testAppHost, AuthHost: testAuthHost, RootHost: testRootHost, + Mail: sender, MailEnabled: true, }) if err != nil { t.Fatalf("auth: %v", err) @@ -61,7 +73,7 @@ func splitServer(t *testing.T) (*store.Store, context.Context, *httptest.Server, t.Cleanup(ts.Close) port := mustURL(t, ts.URL).Port() hp := func(h string) string { return h + ":" + port } - return st, ctx, ts, hostEnv{auth: hp(testAuthHost), app: hp(testAppHost), root: hp(testRootHost), www: hp(testWWWHost)} + return st, ctx, ts, hostEnv{auth: hp(testAuthHost), app: hp(testAppHost), root: hp(testRootHost), www: hp(testWWWHost)}, sender } // seedSession creates a user and establishes an authenticated app-host session diff --git a/internal/core/web.go b/internal/core/web.go index 1703b92..e45c0c1 100644 --- a/internal/core/web.go +++ b/internal/core/web.go @@ -419,6 +419,20 @@ func (s *Handlers) pageDashboard(w http.ResponseWriter, r *http.Request) { Done: len(orgs) > 0, }, } + // Only password holders are nudged, and only while mail is configured. A + // passkey-only account genuinely gains no recovery from an address (reset sets a + // password on accounts that have one), so asking would be asking for data we + // can't use — the right advice for them is a second passkey, which the step + // above already covers. + if user.PasswordHash != nil && s.Auth.MailEnabled() { + steps = append(steps, onboardingStep{ + Title: "Add a recovery email", + Desc: "Optional, but without one a forgotten password can't be reset.", + Action: "Add email", + Href: s.Origin(r, s.Cfg.AuthHost) + "/account", + Done: user.EmailVerified(), + }) + } // People listed publicly (org admins, public repeater owners/stewards) should // give visitors a way to reach them. Only nudge those users, and only until // they set a primary contact link. diff --git a/internal/store/email.go b/internal/store/email.go new file mode 100644 index 0000000..6945abd --- /dev/null +++ b/internal/store/email.go @@ -0,0 +1,255 @@ +package store + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "strings" + "time" + + "github.com/jackc/pgx/v5" +) + +// TokenPurpose distinguishes the two emailed-token flows. It's filtered on at +// redemption, so a verification link can never be replayed as a password reset. +type TokenPurpose string + +const ( + // PurposeVerifyEmail proves an address belongs to the account that set it. + PurposeVerifyEmail TokenPurpose = "verify" + // PurposeResetPassword authorizes setting a new password on one account. + PurposeResetPassword TokenPurpose = "reset" +) + +// VerifyTokenTTL is how long an address-confirmation link stays usable. Generous: +// people check mail hours later, and the worst case of a stale link is a resend. +const VerifyTokenTTL = 24 * time.Hour + +// ResetTokenTTL is how long a password-reset link stays usable. Deliberately much +// shorter than VerifyTokenTTL — this one is a live credential for taking over an +// account, and the legitimate user acts on it within minutes of asking. +const ResetTokenTTL = 45 * time.Minute + +// hashToken is the one-way transform applied before a token touches the database. +// Only the hash is stored, so a leaked email_tokens table yields nothing +// replayable. Hex of SHA-256: tokens are 256 bits of randomness from randomToken, +// so there is no dictionary to attack and no salt/stretching needed. +func hashToken(raw string) string { + sum := sha256.Sum256([]byte(raw)) + return hex.EncodeToString(sum[:]) +} + +// NormalizeEmail trims an address and lowercases it for comparison. +// +// The local part is technically case-sensitive per RFC 5321, but no mail provider +// in practice treats it that way, and someone typing "Me@Example.com" at the reset +// form means the address they registered. Deliberately NOT doing Gmail-style dot or +// +tag stripping: that's provider-specific, wrong elsewhere, and surprising. +func NormalizeEmail(addr string) string { + return strings.ToLower(strings.TrimSpace(addr)) +} + +// SetEmail stores the account's address and marks it unverified, since the new +// address hasn't been proven yet. Passing an empty address clears both columns. +// Format validation is the caller's responsibility (see auth's mail.ParseAddress +// check). +func (s *Store) SetEmail(ctx context.Context, userID int64, addr string) error { + addr = strings.TrimSpace(addr) + if addr == "" { + return s.ClearEmail(ctx, userID) + } + _, err := s.pool.Exec(ctx, + `UPDATE users SET email = $2, email_verified_at = NULL WHERE id = $1`, + userID, addr) + if err != nil { + return fmt.Errorf("set email: %w", err) + } + return nil +} + +// ClearEmail removes the account's address. Outstanding tokens for it go too: +// leaving a live reset link pointing at an address the user just disowned would +// keep a mailbox they may no longer control able to take the account over. +func (s *Store) ClearEmail(ctx context.Context, userID int64) error { + err := s.inTx(ctx, func(tx pgx.Tx) error { + if _, err := tx.Exec(ctx, + `UPDATE users SET email = NULL, email_verified_at = NULL WHERE id = $1`, + userID); err != nil { + return fmt.Errorf("clear email: %w", err) + } + if _, err := tx.Exec(ctx, `DELETE FROM email_tokens WHERE user_id = $1`, userID); err != nil { + return fmt.Errorf("clear email tokens: %w", err) + } + return nil + }) + return err +} + +// MarkEmailVerified confirms the account's address, but only if it still matches +// addr — the address the redeemed token was issued for. Someone who requests a link +// for one address and then changes it to another must not have the old link confirm +// the new one. Reports whether the row was updated. +func (s *Store) MarkEmailVerified(ctx context.Context, userID int64, addr string) (bool, error) { + tag, err := s.pool.Exec(ctx, + `UPDATE users SET email_verified_at = now() + WHERE id = $1 AND email IS NOT NULL AND lower(email) = lower($2)`, + userID, addr) + if err != nil { + return false, fmt.Errorf("mark email verified: %w", err) + } + return tag.RowsAffected() > 0, nil +} + +// CreateEmailToken mints a single-use token for userID and returns it in the clear +// — the only time it exists outside the recipient's mailbox, since the database +// keeps just its hash. addr records which address a verify token was issued for and +// should be empty for a reset token. +func (s *Store) CreateEmailToken(ctx context.Context, userID int64, purpose TokenPurpose, addr string, ttl time.Duration) (string, error) { + raw, err := randomToken() + if err != nil { + return "", err + } + var addrArg *string + if addr != "" { + addrArg = &addr + } + _, err = s.pool.Exec(ctx, + `INSERT INTO email_tokens (user_id, purpose, token_hash, email, expires_at) + VALUES ($1, $2, $3, $4, now() + $5)`, + userID, string(purpose), hashToken(raw), addrArg, ttl) + if err != nil { + return "", fmt.Errorf("create email token: %w", err) + } + return raw, nil +} + +// EmailToken is a redeemed or inspected token. +type EmailToken struct { + UserID int64 + // Email is the address a verify token was issued for, "" for a reset token. + Email string + ExpiresAt time.Time +} + +// PeekEmailToken looks a token up without consuming it, so a GET can render a form +// (and name the account it belongs to) while leaving the token spendable for the +// POST that follows. ok is false when the token is unknown, expired, or of another +// purpose — all of which the caller should treat as "invalid link", not an error. +func (s *Store) PeekEmailToken(ctx context.Context, purpose TokenPurpose, raw string) (EmailToken, bool, error) { + var t EmailToken + var addr *string + err := s.pool.QueryRow(ctx, + `SELECT user_id, email, expires_at FROM email_tokens + WHERE token_hash = $1 AND purpose = $2 AND expires_at > now()`, + hashToken(raw), string(purpose)).Scan(&t.UserID, &addr, &t.ExpiresAt) + if errors.Is(err, pgx.ErrNoRows) { + return EmailToken{}, false, nil + } + if err != nil { + return EmailToken{}, false, fmt.Errorf("peek email token: %w", err) + } + if addr != nil { + t.Email = *addr + } + return t, true, nil +} + +// ConsumeEmailToken atomically redeems a token, deleting it so it can be spent +// exactly once (the DELETE … RETURNING pattern the login handoff uses). ok is false +// when the token is unknown, expired, already spent, or of another purpose — an +// auth failure for the caller to report generically, not an error. +func (s *Store) ConsumeEmailToken(ctx context.Context, purpose TokenPurpose, raw string) (EmailToken, bool, error) { + var t EmailToken + var addr *string + err := s.pool.QueryRow(ctx, + `DELETE FROM email_tokens + WHERE token_hash = $1 AND purpose = $2 AND expires_at > now() + RETURNING user_id, email, expires_at`, + hashToken(raw), string(purpose)).Scan(&t.UserID, &addr, &t.ExpiresAt) + if errors.Is(err, pgx.ErrNoRows) { + return EmailToken{}, false, nil + } + if err != nil { + return EmailToken{}, false, fmt.Errorf("consume email token: %w", err) + } + if addr != nil { + t.Email = *addr + } + return t, true, nil +} + +// DeleteEmailTokens drops every outstanding token of one purpose for a user. Called +// after a successful reset so the other links from the same request (or from an +// earlier one) die with the password they were minted to change. +func (s *Store) DeleteEmailTokens(ctx context.Context, userID int64, purpose TokenPurpose) error { + _, err := s.pool.Exec(ctx, + `DELETE FROM email_tokens WHERE user_id = $1 AND purpose = $2`, userID, string(purpose)) + if err != nil { + return fmt.Errorf("delete email tokens: %w", err) + } + return nil +} + +// CountRecentEmailTokens reports how many tokens of one purpose were minted for a +// user within window. It backs the per-account cap: without it, anyone who knows an +// address could have us mail its owner repeatedly, and a metered daily send quota +// would be trivial to exhaust. +func (s *Store) CountRecentEmailTokens(ctx context.Context, userID int64, purpose TokenPurpose, window time.Duration) (int, error) { + var n int + err := s.pool.QueryRow(ctx, + `SELECT count(*) FROM email_tokens + WHERE user_id = $1 AND purpose = $2 AND created_at > now() - $3::interval`, + userID, string(purpose), window).Scan(&n) + if err != nil { + return 0, fmt.Errorf("count recent email tokens: %w", err) + } + return n, nil +} + +// FindUsersByVerifiedEmail returns every account holding addr as a VERIFIED +// address, matched case-insensitively. +// +// It returns a slice rather than one user because the address column is +// deliberately not unique (see migration 0043): one person may run a personal and +// an ops account on the same mailbox. The caller mails one message listing each +// match with its own link — a token names exactly one account, so which account to +// reset is never ambiguous. An unknown address yields an empty slice, not +// ErrNotFound: "no accounts here" is an ordinary outcome the caller must not +// distinguish to the visitor. +func (s *Store) FindUsersByVerifiedEmail(ctx context.Context, addr string) ([]*User, error) { + rows, err := s.pool.Query(ctx, + `SELECT `+userCols+` FROM users + WHERE email IS NOT NULL AND email_verified_at IS NOT NULL AND lower(email) = lower($1) + ORDER BY username`, + NormalizeEmail(addr)) + if err != nil { + return nil, fmt.Errorf("find users by email: %w", err) + } + defer rows.Close() + var out []*User + for rows.Next() { + u, err := scanUser(rows) + if err != nil { + return nil, fmt.Errorf("scan user by email: %w", err) + } + out = append(out, u) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("find users by email: %w", err) + } + return out, nil +} + +// PruneEmailTokens deletes tokens whose expiry has passed, returning how many went. +// Redemption only removes tokens that are actually spent, so without this the +// abandoned ones accumulate forever — each holding a user_id and, for verify +// tokens, an address. Index-assisted by email_tokens_expiry_idx. +func (s *Store) PruneEmailTokens(ctx context.Context) (int64, error) { + tag, err := s.pool.Exec(ctx, `DELETE FROM email_tokens WHERE expires_at < now()`) + if err != nil { + return 0, fmt.Errorf("prune email tokens: %w", err) + } + return tag.RowsAffected(), nil +} diff --git a/internal/store/email_test.go b/internal/store/email_test.go new file mode 100644 index 0000000..f47b0e3 --- /dev/null +++ b/internal/store/email_test.go @@ -0,0 +1,505 @@ +package store + +import ( + "context" + "testing" + "time" +) + +// emailTestStore returns a Store on a fresh, throwaway database (see internal/testdb). +func emailTestStore(t *testing.T) (*Store, context.Context) { + t.Helper() + return orgTestStore(t) +} + +// mkUser creates a user, optionally with a password hash and a verified address. +func mkUser(t *testing.T, st *Store, ctx context.Context, name, addr string, verified, withPassword bool) *User { + t.Helper() + u, err := st.CreateUser(ctx, name, "") + if err != nil { + t.Fatalf("create user %q: %v", name, err) + } + if withPassword { + if err := st.SetPassword(ctx, u.ID, "$2a$10$notarealhashjustplaceholdervalue000000000000000000000"); err != nil { + t.Fatalf("set password: %v", err) + } + } + if addr != "" { + if err := st.SetEmail(ctx, u.ID, addr); err != nil { + t.Fatalf("set email: %v", err) + } + if verified { + ok, err := st.MarkEmailVerified(ctx, u.ID, addr) + if err != nil || !ok { + t.Fatalf("mark verified: ok=%v err=%v", ok, err) + } + } + } + reloaded, err := st.GetUserByID(ctx, u.ID) + if err != nil { + t.Fatalf("reload user: %v", err) + } + return reloaded +} + +// expireToken backdates a token so it reads as just-lapsed, without waiting out the +// real TTL. +func expireToken(t *testing.T, st *Store, ctx context.Context, raw string) { + t.Helper() + if _, err := st.pool.Exec(ctx, + `UPDATE email_tokens SET expires_at = now() - interval '1 minute' WHERE token_hash = $1`, + hashToken(raw)); err != nil { + t.Fatalf("expire token: %v", err) + } +} + +// TestSetEmailStartsUnverified: a newly-set address is never trusted on the word of +// whoever typed it. If it were, a typo would silently point recovery at a stranger's +// mailbox — or nobody's. +func TestSetEmailStartsUnverified(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", false, true) + + if u.Email == nil || *u.Email != "alice@example.test" { + t.Fatalf("Email = %v, want alice@example.test", u.Email) + } + if u.EmailVerified() { + t.Error("EmailVerified() = true for a freshly set address") + } + if u.CanResetPassword() { + t.Error("CanResetPassword() = true on an unverified address") + } +} + +// TestChangingEmailRevokesVerification: an address that was verified, then replaced, +// must go back to unverified — otherwise typing a new address would inherit the old +// one's trust and immediately be able to receive reset links. +func TestChangingEmailRevokesVerification(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", true, true) + if !u.CanResetPassword() { + t.Fatal("precondition: verified address with a password should be resettable") + } + + if err := st.SetEmail(ctx, u.ID, "attacker@example.test"); err != nil { + t.Fatalf("set email: %v", err) + } + after, err := st.GetUserByID(ctx, u.ID) + if err != nil { + t.Fatalf("get user: %v", err) + } + if after.EmailVerified() { + t.Error("EmailVerified() = true after the address changed") + } +} + +// TestMarkEmailVerifiedRequiresMatchingAddress closes the swap window: request a +// link for address A, change the address to B, then click the old link. The stale +// link must not confirm B. +func TestMarkEmailVerifiedRequiresMatchingAddress(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", false, true) + + if err := st.SetEmail(ctx, u.ID, "second@example.test"); err != nil { + t.Fatalf("set email: %v", err) + } + // The old link was issued for the first address. + ok, err := st.MarkEmailVerified(ctx, u.ID, "alice@example.test") + if err != nil { + t.Fatalf("mark verified: %v", err) + } + if ok { + t.Error("a link issued for the previous address verified the current one") + } + after, err := st.GetUserByID(ctx, u.ID) + if err != nil { + t.Fatalf("get user: %v", err) + } + if after.EmailVerified() { + t.Error("address ended up verified") + } +} + +// TestMarkEmailVerifiedIsCaseInsensitive: mail providers don't distinguish case, so +// a link issued for "Alice@Example.test" must confirm the stored "alice@example.test". +func TestMarkEmailVerifiedIsCaseInsensitive(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", false, true) + + ok, err := st.MarkEmailVerified(ctx, u.ID, "Alice@Example.TEST") + if err != nil { + t.Fatalf("mark verified: %v", err) + } + if !ok { + t.Fatal("case-differing address failed to verify") + } +} + +// TestConsumeEmailTokenSingleUse: a reset link is a credential, so redeeming it must +// spend it. A replayable link means a leaked mailbox archive stays dangerous forever. +func TestConsumeEmailTokenSingleUse(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", true, true) + + raw, err := st.CreateEmailToken(ctx, u.ID, PurposeResetPassword, "", ResetTokenTTL) + if err != nil { + t.Fatalf("create token: %v", err) + } + tok, ok, err := st.ConsumeEmailToken(ctx, PurposeResetPassword, raw) + if err != nil || !ok { + t.Fatalf("first consume: ok=%v err=%v", ok, err) + } + if tok.UserID != u.ID { + t.Errorf("UserID = %d, want %d", tok.UserID, u.ID) + } + if _, ok, err := st.ConsumeEmailToken(ctx, PurposeResetPassword, raw); err != nil || ok { + t.Errorf("second consume: ok=%v err=%v, want ok=false", ok, err) + } +} + +// TestPeekEmailTokenDoesNotConsume: the GET that renders the reset form must leave +// the token spendable for the POST that follows, or every link would die on being +// opened (and mail clients that prefetch links would burn them sight unseen). +func TestPeekEmailTokenDoesNotConsume(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", true, true) + + raw, err := st.CreateEmailToken(ctx, u.ID, PurposeResetPassword, "", ResetTokenTTL) + if err != nil { + t.Fatalf("create token: %v", err) + } + for i := range 2 { + tok, ok, err := st.PeekEmailToken(ctx, PurposeResetPassword, raw) + if err != nil || !ok { + t.Fatalf("peek %d: ok=%v err=%v", i, ok, err) + } + if tok.UserID != u.ID { + t.Errorf("peek %d: UserID = %d, want %d", i, tok.UserID, u.ID) + } + } + if _, ok, _ := st.ConsumeEmailToken(ctx, PurposeResetPassword, raw); !ok { + t.Error("token was not spendable after being peeked") + } +} + +// TestEmailTokenPurposeIsolation: a verification link must not be redeemable as a +// password reset. Without the purpose filter, the low-value token (mailed to an +// unverified, possibly mistyped address) would authorize taking over the account. +func TestEmailTokenPurposeIsolation(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", false, true) + + raw, err := st.CreateEmailToken(ctx, u.ID, PurposeVerifyEmail, "alice@example.test", VerifyTokenTTL) + if err != nil { + t.Fatalf("create token: %v", err) + } + if _, ok, err := st.ConsumeEmailToken(ctx, PurposeResetPassword, raw); err != nil || ok { + t.Errorf("verify token redeemed as a reset: ok=%v err=%v", ok, err) + } + // Still good for what it was minted for. + if _, ok, err := st.ConsumeEmailToken(ctx, PurposeVerifyEmail, raw); err != nil || !ok { + t.Errorf("verify token failed for its own purpose: ok=%v err=%v", ok, err) + } +} + +// TestConsumeEmailTokenRejectsExpired: the short reset TTL is only real if expiry is +// enforced at redemption, not merely displayed. +func TestConsumeEmailTokenRejectsExpired(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", true, true) + + raw, err := st.CreateEmailToken(ctx, u.ID, PurposeResetPassword, "", ResetTokenTTL) + if err != nil { + t.Fatalf("create token: %v", err) + } + expireToken(t, st, ctx, raw) + + if _, ok, err := st.PeekEmailToken(ctx, PurposeResetPassword, raw); err != nil || ok { + t.Errorf("peek of expired token: ok=%v err=%v", ok, err) + } + if _, ok, err := st.ConsumeEmailToken(ctx, PurposeResetPassword, raw); err != nil || ok { + t.Errorf("consume of expired token: ok=%v err=%v", ok, err) + } +} + +// TestEmailTokensStoreNoPlaintext: the point of hashing. If the raw token were +// stored, a leaked table would be a pile of live account-takeover links. +func TestEmailTokensStoreNoPlaintext(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", true, true) + + raw, err := st.CreateEmailToken(ctx, u.ID, PurposeResetPassword, "", ResetTokenTTL) + if err != nil { + t.Fatalf("create token: %v", err) + } + var n int + if err := st.pool.QueryRow(ctx, + `SELECT count(*) FROM email_tokens WHERE token_hash = $1`, raw).Scan(&n); err != nil { + t.Fatalf("query: %v", err) + } + if n != 0 { + t.Error("the raw token is stored in token_hash") + } + if err := st.pool.QueryRow(ctx, + `SELECT count(*) FROM email_tokens WHERE token_hash = $1`, hashToken(raw)).Scan(&n); err != nil { + t.Fatalf("query: %v", err) + } + if n != 1 { + t.Errorf("hashed token rows = %d, want 1", n) + } +} + +// TestFindUsersByVerifiedEmailFanout: the address column is deliberately not unique +// (one person, a personal plus an ops account), so a lookup must return every match. +// Dropping any of them would make one account silently unrecoverable. +func TestFindUsersByVerifiedEmailFanout(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + const addr = "jon@example.test" + mkUser(t, st, ctx, "jleight", addr, true, true) + mkUser(t, st, ctx, "meshtender-ops", addr, true, true) + mkUser(t, st, ctx, "someone-else", "other@example.test", true, true) + + found, err := st.FindUsersByVerifiedEmail(ctx, addr) + if err != nil { + t.Fatalf("find: %v", err) + } + if len(found) != 2 { + t.Fatalf("found %d accounts, want 2", len(found)) + } + if found[0].Username != "jleight" || found[1].Username != "meshtender-ops" { + t.Errorf("usernames = %q/%q", found[0].Username, found[1].Username) + } +} + +// TestFindUsersByVerifiedEmailIgnoresUnverified: an address nobody has proven must +// never receive recovery mail — that's what makes a typo (or an address someone +// else owns) harmless. +func TestFindUsersByVerifiedEmailIgnoresUnverified(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + mkUser(t, st, ctx, "alice", "shared@example.test", false, true) + mkUser(t, st, ctx, "bob", "shared@example.test", true, true) + + found, err := st.FindUsersByVerifiedEmail(ctx, "shared@example.test") + if err != nil { + t.Fatalf("find: %v", err) + } + if len(found) != 1 || found[0].Username != "bob" { + t.Fatalf("found %d accounts (%v), want only bob", len(found), usernames(found)) + } +} + +// TestFindUsersByVerifiedEmailCaseInsensitive: people type their address however +// they remember it. A case-sensitive lookup would tell a legitimate user "no +// account here" and leave them stuck. +func TestFindUsersByVerifiedEmailCaseInsensitive(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + mkUser(t, st, ctx, "alice", "Alice@Example.test", true, true) + + for _, probe := range []string{"alice@example.test", "ALICE@EXAMPLE.TEST", " Alice@Example.test "} { + found, err := st.FindUsersByVerifiedEmail(ctx, probe) + if err != nil { + t.Fatalf("find %q: %v", probe, err) + } + if len(found) != 1 { + t.Errorf("find %q returned %d accounts, want 1", probe, len(found)) + } + } +} + +// TestFindUsersByVerifiedEmailUnknownIsEmpty: an unknown address is an ordinary +// outcome, not an error — the caller must not be able to tell it apart from a hit +// when reporting to the visitor. +func TestFindUsersByVerifiedEmailUnknownIsEmpty(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + + found, err := st.FindUsersByVerifiedEmail(ctx, "nobody@example.test") + if err != nil { + t.Fatalf("find: %v", err) + } + if len(found) != 0 { + t.Errorf("found %d accounts, want 0", len(found)) + } +} + +// TestCanResetPasswordRequiresPassword is the regression test for the rule that +// keeps a passkey-only account's security floor off the mailbox: email recovery only +// ever sets a password on an account that already has one. Relaxing this quietly +// would demote the strongest accounts to whoever controls the inbox. +func TestCanResetPasswordRequiresPassword(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + passkeyOnly := mkUser(t, st, ctx, "passkey-only", "pk@example.test", true, false) + withPassword := mkUser(t, st, ctx, "has-password", "pw@example.test", true, true) + + if passkeyOnly.CanResetPassword() { + t.Error("a passkey-only account reports itself resettable by email") + } + if !withPassword.CanResetPassword() { + t.Error("a password account with a verified address is not resettable") + } +} + +// TestClearEmailDropsOutstandingTokens: disowning an address must kill links already +// sent to it, or a mailbox the user no longer controls keeps a live way in. +func TestClearEmailDropsOutstandingTokens(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", true, true) + + raw, err := st.CreateEmailToken(ctx, u.ID, PurposeResetPassword, "", ResetTokenTTL) + if err != nil { + t.Fatalf("create token: %v", err) + } + if err := st.ClearEmail(ctx, u.ID); err != nil { + t.Fatalf("clear email: %v", err) + } + if _, ok, err := st.ConsumeEmailToken(ctx, PurposeResetPassword, raw); err != nil || ok { + t.Errorf("token still live after the address was cleared: ok=%v err=%v", ok, err) + } + after, err := st.GetUserByID(ctx, u.ID) + if err != nil { + t.Fatalf("get user: %v", err) + } + if after.Email != nil { + t.Errorf("Email = %v after clear, want nil", after.Email) + } +} + +// TestDeleteEmailTokensScopedToPurpose: a completed reset invalidates the other +// reset links from that request, but must not sweep away an unrelated pending +// address verification. +func TestDeleteEmailTokensScopedToPurpose(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", true, true) + + reset, err := st.CreateEmailToken(ctx, u.ID, PurposeResetPassword, "", ResetTokenTTL) + if err != nil { + t.Fatalf("create reset token: %v", err) + } + verify, err := st.CreateEmailToken(ctx, u.ID, PurposeVerifyEmail, "alice@example.test", VerifyTokenTTL) + if err != nil { + t.Fatalf("create verify token: %v", err) + } + + if err := st.DeleteEmailTokens(ctx, u.ID, PurposeResetPassword); err != nil { + t.Fatalf("delete: %v", err) + } + if _, ok, _ := st.ConsumeEmailToken(ctx, PurposeResetPassword, reset); ok { + t.Error("reset token survived") + } + if _, ok, _ := st.ConsumeEmailToken(ctx, PurposeVerifyEmail, verify); !ok { + t.Error("verification token was swept away with the reset tokens") + } +} + +// TestCountRecentEmailTokens backs the per-account cap that keeps us from being used +// to flood someone's inbox. Only tokens inside the window count. +func TestCountRecentEmailTokens(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", true, true) + + for range 3 { + if _, err := st.CreateEmailToken(ctx, u.ID, PurposeResetPassword, "", ResetTokenTTL); err != nil { + t.Fatalf("create token: %v", err) + } + } + // A verify token must not count against the reset budget. + if _, err := st.CreateEmailToken(ctx, u.ID, PurposeVerifyEmail, "alice@example.test", VerifyTokenTTL); err != nil { + t.Fatalf("create verify token: %v", err) + } + + n, err := st.CountRecentEmailTokens(ctx, u.ID, PurposeResetPassword, time.Hour) + if err != nil { + t.Fatalf("count: %v", err) + } + if n != 3 { + t.Errorf("count = %d, want 3", n) + } + + // Backdate them out of the window; the budget refills. + if _, err := st.pool.Exec(ctx, + `UPDATE email_tokens SET created_at = now() - interval '2 hours' WHERE user_id = $1`, u.ID); err != nil { + t.Fatalf("backdate: %v", err) + } + n, err = st.CountRecentEmailTokens(ctx, u.ID, PurposeResetPassword, time.Hour) + if err != nil { + t.Fatalf("count: %v", err) + } + if n != 0 { + t.Errorf("count after backdating = %d, want 0", n) + } +} + +// TestPruneEmailTokens: redemption only deletes tokens someone actually clicked, so +// the janitor is the only thing keeping abandoned ones from accumulating forever. +func TestPruneEmailTokens(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", true, true) + + stale, err := st.CreateEmailToken(ctx, u.ID, PurposeResetPassword, "", ResetTokenTTL) + if err != nil { + t.Fatalf("create token: %v", err) + } + live, err := st.CreateEmailToken(ctx, u.ID, PurposeResetPassword, "", ResetTokenTTL) + if err != nil { + t.Fatalf("create token: %v", err) + } + expireToken(t, st, ctx, stale) + + n, err := st.PruneEmailTokens(ctx) + if err != nil { + t.Fatalf("prune: %v", err) + } + if n != 1 { + t.Errorf("pruned %d, want 1", n) + } + if _, ok, _ := st.ConsumeEmailToken(ctx, PurposeResetPassword, live); !ok { + t.Error("prune took an unexpired token") + } +} + +// TestUserDeletionCascadesEmailTokens: the tokens reference users, so removing an +// account (the --reset sweep today, account deletion later) must not leave orphans. +func TestUserDeletionCascadesEmailTokens(t *testing.T) { + t.Parallel() + st, ctx := emailTestStore(t) + u := mkUser(t, st, ctx, "alice", "alice@example.test", true, true) + if _, err := st.CreateEmailToken(ctx, u.ID, PurposeResetPassword, "", ResetTokenTTL); err != nil { + t.Fatalf("create token: %v", err) + } + + if _, err := st.pool.Exec(ctx, `DELETE FROM users WHERE id = $1`, u.ID); err != nil { + t.Fatalf("delete user: %v", err) + } + var n int + if err := st.pool.QueryRow(ctx, `SELECT count(*) FROM email_tokens WHERE user_id = $1`, u.ID).Scan(&n); err != nil { + t.Fatalf("count: %v", err) + } + if n != 0 { + t.Errorf("%d orphaned tokens remain", n) + } +} + +func usernames(us []*User) []string { + out := make([]string, 0, len(us)) + for _, u := range us { + out = append(out, u.Username) + } + return out +} diff --git a/internal/store/migrations/0043_account_email.sql b/internal/store/migrations/0043_account_email.sql new file mode 100644 index 0000000..8995c70 --- /dev/null +++ b/internal/store/migrations/0043_account_email.sql @@ -0,0 +1,65 @@ +-- +goose Up +-- Optional account email, plus the single-use emailed tokens that make account +-- recovery possible. Before this, a user who signed up with a password and forgot +-- it was locked out permanently (audit finding B1). +-- +-- The address is OPTIONAL and stays that way: signup never asks for one, nothing +-- is gated on having one, and clearing it is one click. Recovery is its only use +-- today; security notifications (a passkey was added, an invite was accepted) are +-- expected later, which is why nothing here is named "recovery_email". +ALTER TABLE users ADD COLUMN email TEXT; + +-- NULL means the address is unconfirmed — set but not yet proven to belong to the +-- account. Only a verified address can receive recovery mail, so a typo can't +-- silently redirect someone's reset link (or mail a stranger). +ALTER TABLE users ADD COLUMN email_verified_at TIMESTAMPTZ; + +-- Deliberately NOT unique. Two reasons: +-- 1. A unique constraint turns account creation into an email-enumeration +-- oracle ("that address is already in use" tells an attacker who's here). +-- 2. One person legitimately runs several accounts on one address — a personal +-- one plus a club/ops one. The reset flow handles the fan-out by listing each +-- account with its own link, so ambiguity is presentational, never a question +-- of which account a token belongs to (a token names exactly one user). +-- +-- Partial + functional: the reset lookup is always a case-insensitive match against +-- a VERIFIED address, so the index covers exactly that and stays off the rows +-- (unverified, or no address at all) it would never serve. +CREATE INDEX users_email_lower_verified_idx ON users (lower(email)) + WHERE email IS NOT NULL AND email_verified_at IS NOT NULL; + +-- One table for both emailed-token flows. They're the same primitive — a +-- single-use token bound to a user with a TTL — so they share one consume path and +-- one prune sweep rather than duplicating both. +CREATE TABLE email_tokens ( + id BIGSERIAL PRIMARY KEY, + user_id BIGINT NOT NULL REFERENCES users (id) ON DELETE CASCADE, + -- 'verify' proves an address belongs to the account; 'reset' authorizes setting + -- a new password. Filtered on at consume time, so a verification link can never + -- be replayed as a password reset. + purpose TEXT NOT NULL CHECK (purpose IN ('verify', 'reset')), + -- The SHA-256 of the token, never the token itself. Unlike repeater_invites + -- (which stores its token raw), these land in a mailbox and authorize taking + -- over an account, so a database leak must not hand over live credentials. + token_hash TEXT NOT NULL UNIQUE, + -- The address a 'verify' token was issued for, re-checked on redemption so a + -- link minted for one address can't confirm a different one the user typed + -- afterwards. NULL for 'reset' (which targets the account, not an address). + email TEXT, + expires_at TIMESTAMPTZ NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +-- The janitor sweep deletes by expiry. Redemption deletes the row it consumes, so +-- without this the abandoned ones would accumulate forever. +CREATE INDEX email_tokens_expiry_idx ON email_tokens (expires_at); + +-- Per-account throttling counts a user's recent tokens of one purpose, so nobody +-- can be used to flood someone else's inbox (or burn a metered daily send quota). +CREATE INDEX email_tokens_user_purpose_idx ON email_tokens (user_id, purpose, created_at DESC); + +-- +goose Down +DROP TABLE email_tokens; +DROP INDEX users_email_lower_verified_idx; +ALTER TABLE users DROP COLUMN email_verified_at; +ALTER TABLE users DROP COLUMN email; diff --git a/internal/store/users.go b/internal/store/users.go index adaacdd..53bd15f 100644 --- a/internal/store/users.go +++ b/internal/store/users.go @@ -33,6 +33,13 @@ type User struct { // Timezone is the user's preferred IANA zone name (e.g. "America/New_York") // for date/time display, or "" when unset (the browser auto-detects). Timezone string + // Email is the account's optional email address, nil when unset. It is never + // public — it exists for account recovery (and, later, security notices) and + // must not be rendered on any page but the owner's own account settings. + // EmailVerifiedAt is nil until the address has been confirmed; only a verified + // address receives mail. + Email *string + EmailVerifiedAt *time.Time // Instance-level capability flags. CapManageUsers bool CapManageCatalog bool @@ -50,17 +57,36 @@ func (u *User) Name() string { return u.Username } -const userCols = `id, username, display_name, password_hash, bio, location, callsign, timezone, cap_manage_users, cap_manage_catalog, last_login_at, created_at` +const userCols = `id, username, display_name, password_hash, bio, location, callsign, timezone, email, email_verified_at, cap_manage_users, cap_manage_catalog, last_login_at, created_at` func scanUser(row pgx.Row) (*User, error) { var u User if err := row.Scan(&u.ID, &u.Username, &u.DisplayName, &u.PasswordHash, - &u.Bio, &u.Location, &u.Callsign, &u.Timezone, &u.CapManageUsers, &u.CapManageCatalog, &u.LastLoginAt, &u.CreatedAt); err != nil { + &u.Bio, &u.Location, &u.Callsign, &u.Timezone, &u.Email, &u.EmailVerifiedAt, + &u.CapManageUsers, &u.CapManageCatalog, &u.LastLoginAt, &u.CreatedAt); err != nil { return nil, err } return &u, nil } +// EmailVerified reports whether the account has a confirmed address — the +// precondition for sending it anything. +func (u *User) EmailVerified() bool { + return u.Email != nil && *u.Email != "" && u.EmailVerifiedAt != nil +} + +// CanResetPassword reports whether this account can be recovered by email. +// +// Both halves are required, and the password half is the load-bearing one: reset +// only ever SETS a password on an account that already has one. A passkey-only +// account is deliberately not recoverable this way — allowing it would silently +// demote a phishing-resistant credential to "whoever controls the mailbox", which +// is the opposite of why someone chose a passkey. Such users are pointed at adding +// a second passkey instead. +func (u *User) CanResetPassword() bool { + return u.PasswordHash != nil && u.EmailVerified() +} + // TouchLastLogin stamps the user's most recent sign-in time. Best-effort // telemetry — callers should not fail a login if this errors. func (s *Store) TouchLastLogin(ctx context.Context, userID int64) error {