amadan

oficina / home Public

Sign-in through signin.Flow, with the link redeemed on POST

§13 step 2, Task 5. Six routes, a renderer, and the four properties that
were wrong in earlier drafts of the plan.

## The link is redeemed on POST, never on GET

GET /signin/verify renders a confirmation form and touches no state. It
does not look the token up either. Two things break if it does:

- Mail security scanners prefetch links. The scanner burns the
  single-use token and the person's own click fails. Routine, not
  hypothetical.
- It is login CSRF. Any page the victim loads can embed
  <img src="https://id.oficina.works/signin/verify?token=…"> and silently
  sign them into an account the attacker controls, after which
  everything they do happens in the attacker's session.

Not looking the token up is the second half: a lookup would render a
valid token differently from an invalid one, which is an oracle a
prefetch can probe.

Verified by breaking it. Making GET redeem fails BOTH
TestGetDoesNotRedeemTheLink and TestVerifyPageDoesNotDistinguishTokens.

GET /signin/callback may mint on a GET, and the difference is not
arbitrary: the keymail return is bound to the browser that began the
flow by the __Host- pending cookie plus state, so an attacker cannot
make a victim's browser complete a flow it never started. A magic link
has no such binding. The callback redirects and never renders — the
return host is address-chosen, and rendering an account-influenced
string on the credential origin is what §4 forbids.

## The per-IP rate limit does not collapse into a suite-wide outage

Over a Linux unix socket r.RemoteAddr is "@". SplitHostPort("@") fails,
and signin skips the per-IP budget only when the key is exactly "" — so
passing "@" through would give every request in the world one shared
budget of 20 per 15 minutes. Twenty POSTs would lock sign-in out for the
whole suite for a quarter of an hour, and §2.1 already names this
service the suite's single point of failure.

clientIP takes the last X-Forwarded-For element (the carlos edge sets it
and strips inbound forwarded headers on unverified hops, so it is
trustworthy here and unforgeable), falls back to RemoteAddr, and returns
"" rather than "@" when no IP is knowable — no per-IP budget is strictly
better than one budget shared by everybody, and the per-address budget
still applies. Five cases tested, including that two clients never share
a key.

## csrf.Protect is router-wide

Not just on the verify POST. POST /signin needs it too: it is an
unauthenticated endpoint that sends mail and makes outbound DNS and
HTTPS calls decided by the submitted address — a blind-SSRF surface by
design. csrf.Protect is pure origin evidence and never consults a
session, so it works before anybody is signed in.

This changed gate 9's route sweep, which was sending POSTs with no
origin evidence: csrf short-circuits before chi matches, so the handlers
never ran and the sweep was scanning nothing. It sends the app's origin
now — and sweeps the refused path separately, because a middleware that
logged what it rejected would leak the address in the form body.

## The session subject is the person ref

sessions.Session.Subject is stored in the clear. Putting an address
there would put every signed-in person's address at rest in a plaintext
column, which is exactly what rastrillo/auth does and why this app does
not use it. Asserted: the session row's subject must satisfy
ValidPersonRef.

## Gate 2, and proof it bites

TestFullSigninLeavesNoReadableAddress is §12 gate 2's sign-in half: a
full sign-in leaves no readable address anywhere, WAL included. It must
be extended, not replaced, when the directory and rekey halves land.
Storing the address unsealed fails it.

Also here: the classifier is bounded by a 5s timeout because it makes
outbound calls to whatever domain the person typed; the test app's
LookupTXT returns a real error rather than nil, nil, because an empty
TXT result makes signin fall through to a live HTTPS GET to the typed
domain — every test in the suite would touch the network; and classification
failing open is asserted rather than assumed.

DEPLOYMENT NOTE: Origin is the constant https://id.oficina.works, which
is both the CSRF origin and the cookie origin. §4's split between the
credential origin and the directory origin is not wired yet — the two
hostnames currently serve the same deployment — so sign-in will only
work on that hostname once deployed. That split is step 3's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E4uVkB6xnZJYuxTooPiUn4
Paul Campbell pushed by paul@keymail.dev 494db92c98167adfb6948b29e5765a6e8a7cceea parent e41d43c
11 files changed, +868 −6
  • internal/home/app.go +58 −6
  • internal/home/logs_test.go +23 −0
  • internal/home/render.go +93 −0
  • internal/home/signin.go +301 −0
  • internal/home/signin_test.go +337 −0
  • internal/home/templates/check_your_mail.html +6 −0
  • internal/home/templates/layout.html +14 −0
  • internal/home/templates/signin.html +10 −0
  • internal/home/templates/signin_confirm.html +9 −0
  • internal/home/templates/signin_failed.html +6 −0
  • internal/home/testapp_test.go +11 −0

Intent

full ledger: step2/seal-and-secrets

diff --git a/internal/home/app.go b/internal/home/app.go
index 02f852c..b1d5584 100644
--- a/internal/home/app.go
+++ b/internal/home/app.go
@@ -15,9 +15,13 @@ import (
"errors"
"log/slog"
"net/http"
+ "time"
+ "github.com/carlosframework/rastrillo/csrf"
"github.com/carlosframework/rastrillo/db"
+ "github.com/carlosframework/rastrillo/sessions"
"github.com/go-chi/chi/v5"
+ "github.com/keymaildev/signin"
)
// App holds the wiring. One per process.
@@ -26,9 +30,13 @@ import (
// is sealed on the way in, and the database handle because that is what
// it is sealed into. Nothing here reaches for a raw key.
type App struct {
- secrets Secrets
- db *db.DB
- links *linkStore
+ secrets Secrets
+ db *db.DB
+ links *linkStore
+ logger *slog.Logger
+ renderer *renderer
+ flow *signin.Flow
+ sessions *sessions.Sessions
}
// New builds an App on validated secrets, and refuses anything less.
@@ -59,16 +67,51 @@ func New(secrets Secrets, d *db.DB, logger *slog.Logger) (*App, error) {
case secrets.Mailer == nil:
return nil, errors.New("home: no mailer, so no sign-in link could be sent")
}
+ links := &linkStore{db: d, sealer: secrets.Sealer, logger: logger}
+
+ rend, err := newRenderer()
+ if err != nil {
+ return nil, err
+ }
+
+ // Exactly one Sessions per process, per the package's own
+ // instruction. TTL and SigninPath are named rather than defaulted so
+ // a reader of this file knows what they are without opening
+ // rastrillo.
+ sess, err := sessions.New(sessions.Config{
+ DB: d.Writer(),
+ Origin: Origin,
+ TTL: 30 * 24 * time.Hour,
+ SigninPath: "/signin",
+ Logger: logger,
+ })
+ if err != nil {
+ return nil, err
+ }
+
return &App{
- secrets: secrets,
- db: d,
- links: &linkStore{db: d, sealer: secrets.Sealer, logger: logger},
+ secrets: secrets,
+ db: d,
+ links: links,
+ logger: logger,
+ renderer: rend,
+ flow: newFlow(secrets, links),
+ sessions: sess,
}, nil
}
// Router returns the mounted handler.
func (a *App) Router() http.Handler {
r := chi.NewRouter()
+
+ // Router-wide, not just on the verify POST. POST /signin needs it
+ // too: it is an unauthenticated endpoint that sends mail and makes
+ // outbound DNS and HTTPS calls decided by the submitted address — a
+ // blind-SSRF surface by design — and csrf.Protect is pure origin
+ // evidence (Sec-Fetch-Site, then Origin, then Referer) that never
+ // consults a session, so it works before anybody is signed in.
+ r.Use(csrf.Protect(Origin))
+
r.Get("/", a.handleIndex)
// This /healthz is NOT what answers in a deployed instance:
// rastrillo.Serve registers its own GET /healthz on the outer mux
@@ -80,5 +123,14 @@ func (a *App) Router() http.Handler {
r.Get("/healthz", func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
})
+
+ r.Get("/signin", a.handleSigninForm)
+ r.Post("/signin", a.handleSigninBegin)
+ r.Get("/signin/sent", a.handleSigninSent)
+ // GET renders, POST redeems. See handleVerifyForm for why.
+ r.Get(linkPath, a.handleVerifyForm)
+ r.Post(linkPath, a.handleVerify)
+ r.Get(callbackPath, a.handleCallback)
+
return r
}
diff --git a/internal/home/logs_test.go b/internal/home/logs_test.go
index 5a1d493..73dbdea 100644
--- a/internal/home/logs_test.go
+++ b/internal/home/logs_test.go
@@ -123,6 +123,13 @@ func TestNoContentInLogs(t *testing.T) {
req := httptest.NewRequest(tg.method, tg.path+"?q="+url.QueryEscape(logtest.Canaries[0]), body)
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+ // csrf.Protect is mounted router-wide and short-circuits a
+ // state-changing request with no origin evidence — before chi
+ // matches, so the handler never runs and the gate scans
+ // nothing. Sending the app's own origin is what lets these
+ // routes dispatch at all. The rejected path is swept below,
+ // separately, because it is also a path that could log.
+ req.Header.Set("Origin", Origin)
// The headers most likely to be logged wholesale by a future
// request logger, each carrying content that must never appear.
req.Header.Set("Referer", "https://"+logtest.Canaries[1]+".example.invalid")
@@ -156,6 +163,22 @@ func TestNoContentInLogs(t *testing.T) {
}
}
+ // The CSRF rejection is its own code path, and it sees the same
+ // canary-bearing body, headers and cookie. A middleware that logged
+ // what it refused would leak exactly the address the form carried.
+ for _, tg := range targets {
+ if tg.method != http.MethodPost {
+ continue
+ }
+ body := strings.NewReader(url.Values{"address": {logtest.Canaries[0]}}.Encode())
+ req := httptest.NewRequest(tg.method, tg.path, body)
+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+ req.Header.Set("Referer", "https://"+logtest.Canaries[1]+".example.invalid")
+ req.Header.Set("User-Agent", logtest.Canaries[0])
+ // No Origin, and a Referer from somewhere else: refused.
+ router.ServeHTTP(httptest.NewRecorder(), req)
+ }
+
rec.AssertNoContent(t)
}
diff --git a/internal/home/render.go b/internal/home/render.go
new file mode 100644
index 0000000..7d275da
--- /dev/null
+++ b/internal/home/render.go
@@ -0,0 +1,93 @@
+package home
+
+import (
+ "bytes"
+ "embed"
+ "fmt"
+ "html/template"
+ "io/fs"
+ "net/http"
+ "path"
+
+ "github.com/carlosframework/rastrillo/ui"
+)
+
+//go:embed templates/*.html
+var templatesFS embed.FS
+
+// layoutFile is the app's own shell. It defines "layout" and executes
+// {{template "content" .}}, which is the contract rastrillo's shipped
+// shells use, so swapping in one of those later is a file copy rather
+// than a rewrite.
+const layoutFile = "templates/layout.html"
+
+// renderer holds one parsed tree per page.
+//
+// Parsed once, at construction, so a broken template is a boot failure
+// rather than a 500 on the one page nobody visits in testing. Minimal on
+// purpose: no asset pipeline, no theme, no vendored static files. The
+// scriptless sweep and the assets land with the task that adds them.
+type renderer struct {
+ pages map[string]*template.Template
+}
+
+func newRenderer() (*renderer, error) {
+ files, err := fs.Glob(templatesFS, "templates/*.html")
+ if err != nil {
+ return nil, err
+ }
+ r := &renderer{pages: make(map[string]*template.Template, len(files))}
+ for _, f := range files {
+ name := path.Base(f)
+ if f == layoutFile {
+ continue
+ }
+ // The layout first, then rastrillo's partials, then the page —
+ // so a page may use a partial, and a page redefining a block
+ // wins over the layout's default.
+ // ui.Funcs supplies the helpers rastrillo's partials call —
+ // "T" among them — so the tree must carry them before those
+ // partials are parsed, not after.
+ t, err := template.New(name).Funcs(ui.Funcs()).ParseFS(templatesFS, layoutFile)
+ if err != nil {
+ return nil, fmt.Errorf("parse %s: %w", layoutFile, err)
+ }
+ if t, err = t.ParseFS(ui.Templates(), "*.html"); err != nil {
+ return nil, fmt.Errorf("parse rastrillo partials: %w", err)
+ }
+ if t, err = t.ParseFS(templatesFS, f); err != nil {
+ return nil, fmt.Errorf("parse %s: %w", f, err)
+ }
+ r.pages[name] = t
+ }
+ if len(r.pages) == 0 {
+ return nil, fmt.Errorf("no page templates found")
+ }
+ return r, nil
+}
+
+// render writes one page.
+//
+// It renders to a buffer first. A template that fails halfway through
+// would otherwise have already written a 200 and half a page, and the
+// error handler could only append to it — so the status code would say
+// the opposite of what happened.
+func (a *App) render(w http.ResponseWriter, status int, page string, data any) {
+ t, ok := a.renderer.pages[page]
+ if !ok {
+ a.logger.Error("no such page template", "page", page)
+ http.Error(w, "internal error", http.StatusInternalServerError)
+ return
+ }
+ var buf bytes.Buffer
+ if err := t.ExecuteTemplate(&buf, "layout", data); err != nil {
+ a.logger.Error("render", "page", page, "err", err)
+ http.Error(w, "internal error", http.StatusInternalServerError)
+ return
+ }
+ w.Header().Set("Content-Type", "text/html; charset=utf-8")
+ w.WriteHeader(status)
+ if _, err := buf.WriteTo(w); err != nil {
+ a.logger.Warn("write response", "page", page, "err", err)
+ }
+}
diff --git a/internal/home/signin.go b/internal/home/signin.go
new file mode 100644
index 0000000..05483d5
--- /dev/null
+++ b/internal/home/signin.go
@@ -0,0 +1,301 @@
+package home
+
+import (
+ "errors"
+ "net"
+ "net/http"
+ "strings"
+ "time"
+
+ "github.com/carlosframework/rastrillo/sessions"
+ "github.com/keymaildev/signin"
+)
+
+// Origin is the credential origin: the host that renders sign-in, holds
+// the session cookie and is the WebAuthn RP. §4 splits it from the
+// directory origin deliberately, and both the CSRF check and the cookie
+// attributes are keyed on this exact string.
+const Origin = "https://id.oficina.works"
+
+// linkPath is where magic links land. It must match the route mounted
+// below; signin's own default is /auth/verify, which this app does not
+// serve.
+const linkPath = "/signin/verify"
+
+// callbackPath is where keymail returns. signin.Keymail defaults to
+// /auth/callback, so setting this is not optional — mounting the route
+// without it leaves the two disagreeing and every keymail sign-in
+// failing at the last hop.
+const callbackPath = "/signin/callback"
+
+// pendingCookie carries the sealed keymail pending blob between Begin
+// and the callback.
+//
+// Every attribute here is load-bearing, and they are the reason the
+// callback is allowed to mint a session on a GET where the magic link is
+// not. __Host- pins it to this exact origin with no Domain and Path=/,
+// so no sibling host can set or read it. HttpOnly keeps script out.
+// SameSite=Lax is what stops a cross-site request carrying it — Strict
+// would break the keymail return itself, which is a top-level navigation
+// from another origin. And it is cleared on read, so a blob is good for
+// exactly one callback.
+const pendingCookie = "__Host-oficina-signin"
+
+// pendingTTL bounds how long a half-finished keymail round trip stays
+// usable. signin's own PendingTTL defaults to 10 minutes; the cookie is
+// given the same life so the two expire together rather than leaving a
+// cookie that outlives the blob it carries.
+const pendingTTL = 10 * time.Minute
+
+// classifierTimeout bounds the classifier's outbound calls. It makes DNS
+// and HTTPS requests to whatever domain the person typed, so an
+// unbounded client is a way to hold a handler open on a host the
+// submitter chose.
+const classifierTimeout = 5 * time.Second
+
+// newFlow wires signin.Flow. Exactly one per process: the rate limiter's
+// state lives on the value, so one per request would hand every caller a
+// fresh empty budget and rate-limit nothing, silently.
+func newFlow(secrets Secrets, links signin.LinkStore) *signin.Flow {
+ http := &http.Client{Timeout: classifierTimeout}
+ return &signin.Flow{
+ Classifier: &signin.Classifier{HTTP: http, Scheme: "https"},
+ Keymail: func(server string) *signin.Keymail {
+ return &signin.Keymail{
+ Base: "https://" + server,
+ Origin: Origin,
+ RedirectPath: callbackPath,
+ HTTP: http,
+ }
+ },
+ Links: links,
+ Mailer: secrets.Mailer,
+ Key: secrets.SigninKey,
+ Origin: Origin,
+ LinkPath: linkPath,
+ Subject: "Sign in to Oficina",
+ }
+}
+
+// clientIP is the rate limiter's per-IP key, and getting it wrong is a
+// suite-wide outage rather than a missing nicety.
+//
+// Over a Linux unix socket r.RemoteAddr is "@". net.SplitHostPort("@")
+// fails, so a naive implementation passes "@" through — and signin skips
+// the per-IP budget only when the key is exactly "". Every request in
+// the world would then share the key "@" with a budget of 20 per 15
+// minutes: twenty POSTs would lock sign-in out for the whole suite for a
+// quarter of an hour, and §2.1 already names this service the suite's
+// single point of failure.
+//
+// The carlos edge sets X-Forwarded-For on every proxied request and
+// strips inbound Forwarded/X-Forwarded-* on unverified hops, so the last
+// element is the edge's own view of the client and a client cannot forge
+// it. Falling back to RemoteAddr covers a direct local connection.
+//
+// Returning "" is deliberate where no IP is knowable: no per-IP budget
+// is strictly better than one global budget shared by everyone, and the
+// per-address budget still applies.
+func clientIP(r *http.Request) string {
+ if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
+ parts := strings.Split(xff, ",")
+ if ip := strings.TrimSpace(parts[len(parts)-1]); ip != "" && ip != "@" {
+ return ip
+ }
+ }
+ if host, _, err := net.SplitHostPort(r.RemoteAddr); err == nil && host != "" && host != "@" {
+ return host
+ }
+ if r.RemoteAddr != "" && r.RemoteAddr != "@" {
+ return r.RemoteAddr
+ }
+ return ""
+}
+
+// handleSigninForm renders the address form.
+func (a *App) handleSigninForm(w http.ResponseWriter, r *http.Request) {
+ a.render(w, http.StatusOK, "signin.html", map[string]any{})
+}
+
+// handleSigninSent is the check-your-mail page. It is reached by
+// redirect and asserts nothing about the address, so it is the same page
+// whether or not anybody has an account here.
+func (a *App) handleSigninSent(w http.ResponseWriter, r *http.Request) {
+ a.render(w, http.StatusOK, "check_your_mail.html", map[string]any{})
+}
+
+// handleSigninBegin starts a sign-in.
+//
+// It branches on what Begin decided. A magic link has already been sent
+// by the time Begin returns; a keymail upgrade has sent nothing and
+// hands back a redirect and a pending blob instead. Both answers are
+// decided by the address's domain, which the submitter already knows, so
+// neither discloses whether that person has an account here.
+func (a *App) handleSigninBegin(w http.ResponseWriter, r *http.Request) {
+ if err := r.ParseForm(); err != nil {
+ a.render(w, http.StatusBadRequest, "signin.html",
+ map[string]any{"Error": "That did not look like a form."})
+ return
+ }
+ address := strings.TrimSpace(r.PostFormValue("address"))
+
+ // The force override carries a failed keymail round trip back to the
+ // magic link, so a failed approval is not a dead end.
+ var force signin.Method
+ if r.URL.Query().Get("force") == "1" {
+ force = signin.MethodMagicLink
+ }
+
+ next, err := a.flow.Begin(r.Context(), address, clientIP(r), force)
+ switch {
+ case errors.Is(err, signin.ErrBadAddress):
+ a.render(w, http.StatusBadRequest, "signin.html",
+ map[string]any{"Error": "That does not look like an email address."})
+ return
+ case errors.Is(err, signin.ErrRateLimited):
+ // 429 without saying which budget: which one was hit is itself
+ // information about whether that address has been tried.
+ a.render(w, http.StatusTooManyRequests, "signin.html",
+ map[string]any{"Error": "Too many attempts. Try again shortly."})
+ return
+ case err != nil:
+ a.logger.Error("signin begin", "err", err)
+ a.render(w, http.StatusInternalServerError, "signin.html",
+ map[string]any{"Error": "Something went wrong. Try again."})
+ return
+ }
+
+ if next.Kind == signin.NextKeymail {
+ a.setPending(w, next.Pending)
+ http.Redirect(w, r, next.Redirect, http.StatusSeeOther)
+ return
+ }
+ http.Redirect(w, r, "/signin/sent", http.StatusSeeOther)
+}
+
+// handleVerifyForm renders the confirmation page for a magic link. It
+// TOUCHES NO STATE, and that is the whole point.
+//
+// Redeeming on GET breaks twice. Mail security scanners prefetch links,
+// which burns the single-use token before the person clicks it — routine,
+// not hypothetical. And it is login CSRF: any page the victim loads can
+// embed <img src=".../signin/verify?token=…"> and silently sign them into
+// an account the attacker controls, after which everything they do
+// happens in the attacker's session.
+//
+// It must not look the token up either. A lookup would reintroduce the
+// oracle a prefetch could probe: a valid token would render differently
+// from an invalid one, which is exactly the difference this page exists
+// not to have.
+func (a *App) handleVerifyForm(w http.ResponseWriter, r *http.Request) {
+ a.render(w, http.StatusOK, "signin_confirm.html",
+ map[string]any{"Token": r.URL.Query().Get("token")})
+}
+
+// handleVerify redeems a magic link. Behind csrf.Protect, on POST.
+func (a *App) handleVerify(w http.ResponseWriter, r *http.Request) {
+ if err := r.ParseForm(); err != nil {
+ a.signinFailed(w)
+ return
+ }
+ id, err := a.flow.CompleteMagicLink(r.Context(), r.PostFormValue("token"))
+ if err != nil {
+ // Unknown, expired and already-used are one answer. TakeLink
+ // makes them one answer at the store; this keeps them one
+ // answer at the door.
+ a.signinFailed(w)
+ return
+ }
+ a.completeSignIn(w, r, id)
+}
+
+// handleCallback completes a keymail round trip.
+//
+// This one may mint on a GET where the magic link may not, and the
+// difference is not arbitrary: the return is bound to the browser that
+// began the flow, by the __Host- pending cookie plus state. An attacker
+// cannot make a victim's browser complete a flow it never started. A
+// magic link has no such binding — anyone holding the URL can replay it
+// — which is why that one needs the form.
+//
+// It redirects and never renders: the return host is chosen by the
+// address, and rendering an account-influenced string on the credential
+// origin is what §4 forbids.
+func (a *App) handleCallback(w http.ResponseWriter, r *http.Request) {
+ blob := a.takePending(w, r)
+ q := r.URL.Query()
+
+ id, err := a.flow.CompleteKeymail(r.Context(), blob, q.Get("code"), q.Get("state"))
+ if err != nil {
+ // Not a dead end: back to the form with the override set, so the
+ // next attempt takes the magic link instead.
+ http.Redirect(w, r, "/signin?force=1", http.StatusSeeOther)
+ return
+ }
+ a.completeSignIn(w, r, id)
+}
+
+// completeSignIn turns a verified identity into a session.
+//
+// The session's subject is the person ref, never the address.
+// sessions.Session.Subject is an opaque string the app chooses, and it
+// is stored in the clear — putting an address there would put every
+// signed-in person's address at rest in a plaintext column, which is
+// precisely what rastrillo/auth does and why this app does not use it.
+func (a *App) completeSignIn(w http.ResponseWriter, r *http.Request, id signin.Identity) {
+ ref, err := a.findOrMintPerson(r.Context(), id.Address)
+ if err != nil {
+ a.logger.Error("find or mint person", "err", err)
+ a.signinFailed(w)
+ return
+ }
+ if err := a.sessions.SignIn(w, r, sessions.Session{
+ Subject: ref,
+ Method: string(id.Method),
+ AuthTime: id.AuthTime,
+ At: id.At,
+ }); err != nil {
+ a.logger.Error("mint session", "person", ref, "err", err)
+ a.signinFailed(w)
+ return
+ }
+ http.Redirect(w, r, "/", http.StatusSeeOther)
+}
+
+// signinFailed is the one answer every failed redemption gets. One page,
+// one status, no detail: unknown, expired, already used and
+// wrong-purpose must be indistinguishable from outside.
+func (a *App) signinFailed(w http.ResponseWriter) {
+ a.render(w, http.StatusBadRequest, "signin_failed.html", map[string]any{})
+}
+
+func (a *App) setPending(w http.ResponseWriter, blob string) {
+ http.SetCookie(w, &http.Cookie{
+ Name: pendingCookie,
+ Value: blob,
+ Path: "/",
+ MaxAge: int(pendingTTL.Seconds()),
+ HttpOnly: true,
+ Secure: true,
+ SameSite: http.SameSiteLaxMode,
+ })
+}
+
+// takePending reads the pending blob and clears it in the same breath,
+// so one blob completes at most one callback.
+func (a *App) takePending(w http.ResponseWriter, r *http.Request) string {
+ http.SetCookie(w, &http.Cookie{
+ Name: pendingCookie,
+ Value: "",
+ Path: "/",
+ MaxAge: -1,
+ HttpOnly: true,
+ Secure: true,
+ SameSite: http.SameSiteLaxMode,
+ })
+ c, err := r.Cookie(pendingCookie)
+ if err != nil {
+ return ""
+ }
+ return c.Value
+}
diff --git a/internal/home/signin_test.go b/internal/home/signin_test.go
new file mode 100644
index 0000000..3292669
--- /dev/null
+++ b/internal/home/signin_test.go
@@ -0,0 +1,337 @@
+package home
+
+import (
+ "context"
+ "errors"
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "strings"
+ "sync"
+ "testing"
+
+ "github.com/keymaildev/signin"
+
+ "amadan.net/oficina/home/internal/home/sealtest"
+)
+
+// signIn posts the sign-in form.
+//
+// It sends an Origin header because csrf.Protect is mounted router-wide
+// and refuses a POST without origin evidence — before chi matches, so
+// without it every test here would fail for the wrong reason.
+func signIn(t *testing.T, app *testApp, addr string) *httptest.ResponseRecorder {
+ t.Helper()
+ form := url.Values{"address": {addr}}
+ req := httptest.NewRequest(http.MethodPost, "/signin", strings.NewReader(form.Encode()))
+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+ req.Header.Set("Origin", Origin)
+ w := httptest.NewRecorder()
+ app.Router().ServeHTTP(w, req)
+ return w
+}
+
+// capturedLink returns the token from the most recent magic link.
+func capturedLink(t *testing.T, app *testApp) string {
+ t.Helper()
+ sent, ok := app.mailer.last()
+ if !ok {
+ t.Fatal("no mail was sent")
+ }
+ i := strings.Index(sent.Body, Origin+linkPath)
+ if i < 0 {
+ t.Fatalf("no sign-in link in the mail body: %q", sent.Body)
+ }
+ rest := sent.Body[i:]
+ if j := strings.IndexAny(rest, " \r\n\t\""); j >= 0 {
+ rest = rest[:j]
+ }
+ u, err := url.Parse(rest)
+ if err != nil {
+ t.Fatalf("parse link %q: %v", rest, err)
+ }
+ token := u.Query().Get("token")
+ if token == "" {
+ t.Fatalf("link %q carries no token", rest)
+ }
+ return token
+}
+
+// redeem posts the confirmation form.
+func redeem(t *testing.T, app *testApp, token string) *httptest.ResponseRecorder {
+ t.Helper()
+ form := url.Values{"token": {token}}
+ req := httptest.NewRequest(http.MethodPost, linkPath, strings.NewReader(form.Encode()))
+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+ req.Header.Set("Origin", Origin)
+ w := httptest.NewRecorder()
+ app.Router().ServeHTTP(w, req)
+ return w
+}
+
+func sessionCookie(rec *httptest.ResponseRecorder) *http.Cookie {
+ for _, c := range (&http.Response{Header: rec.Header()}).Cookies() {
+ if strings.Contains(c.Name, "session") && c.Value != "" {
+ return c
+ }
+ }
+ return nil
+}
+
+// TestFullSigninLeavesNoReadableAddress is §12 gate 2: a full sign-in
+// leaves no readable address anywhere, WAL included. The directory and
+// rekey halves arrive with those steps; this asserts the sign-in half
+// and must be EXTENDED, not replaced, then.
+func TestFullSigninLeavesNoReadableAddress(t *testing.T) {
+ app := newTestApp(t)
+
+ if rec := signIn(t, app, sealtest.CanaryAddress); rec.Code != http.StatusSeeOther {
+ t.Fatalf("POST /signin = %d, want 303", rec.Code)
+ }
+ if rec := redeem(t, app, capturedLink(t, app)); rec.Code != http.StatusSeeOther {
+ t.Fatalf("redeem = %d, want 303", rec.Code)
+ }
+
+ sealtest.AssertNoPlaintext(t, app.dbPath)
+}
+
+// A sign-in that completes must actually mint a session, or the gate
+// above would pass on an app that stores nothing because it does
+// nothing.
+func TestSigninMintsASessionOnThePersonRef(t *testing.T) {
+ app := newTestApp(t)
+ signIn(t, app, "person@example.invalid")
+ rec := redeem(t, app, capturedLink(t, app))
+
+ if sessionCookie(rec) == nil {
+ t.Fatal("redemption set no session cookie")
+ }
+
+ var subject string
+ if err := app.db.Writer().QueryRow("SELECT subject FROM sessions").Scan(&subject); err != nil {
+ t.Fatalf("no session row: %v", err)
+ }
+ if !ValidPersonRef(subject) {
+ t.Fatalf("session subject is %q, which is not a person ref", subject)
+ }
+}
+
+// Classification fails open: a resolver failure must still send a magic
+// link, never a dead end.
+func TestFailedClassificationStillSendsAMagicLink(t *testing.T) {
+ app := newTestApp(t)
+ app.flow.Classifier.LookupTXT = func(context.Context, string) ([]string, error) {
+ return nil, errors.New("servfail")
+ }
+ next, err := app.flow.Begin(context.Background(), "someone@example.invalid", "", signin.Method(""))
+ if err != nil {
+ t.Fatalf("Begin: %v", err)
+ }
+ if next.Kind != signin.NextMagicLink {
+ t.Fatalf("Kind = %v, want NextMagicLink: classification must fail open", next.Kind)
+ }
+}
+
+// A returning person gets the SAME person ref. Without the blind index
+// every sign-in minted a new one, so nobody could ever return.
+func TestSecondSigninFindsTheSamePerson(t *testing.T) {
+ app := newTestApp(t)
+
+ signIn(t, app, "person@example.invalid")
+ redeem(t, app, capturedLink(t, app))
+ signIn(t, app, "Person@Example.Invalid")
+ redeem(t, app, capturedLink(t, app))
+
+ var n int
+ if err := app.db.Writer().QueryRow("SELECT count(*) FROM people").Scan(&n); err != nil {
+ t.Fatal(err)
+ }
+ if n != 1 {
+ t.Fatalf("%d person rows for one address, want 1", n)
+ }
+}
+
+// A GET does not redeem. A mail scanner's prefetch must leave the link
+// usable, and must not sign anybody in — the second half is login CSRF,
+// where any page the victim loads could embed the link as an image and
+// silently put them in the attacker's session.
+func TestGetDoesNotRedeemTheLink(t *testing.T) {
+ app := newTestApp(t)
+ signIn(t, app, "person@example.invalid")
+ token := capturedLink(t, app)
+
+ req := httptest.NewRequest(http.MethodGet, linkPath+"?token="+url.QueryEscape(token), nil)
+ w := httptest.NewRecorder()
+ app.Router().ServeHTTP(w, req)
+
+ if w.Code != http.StatusOK {
+ t.Fatalf("GET %s = %d, want 200", linkPath, w.Code)
+ }
+ if c := sessionCookie(w); c != nil {
+ t.Fatal("a GET minted a session; that is login CSRF")
+ }
+ var n int
+ if err := app.db.Writer().QueryRow("SELECT count(*) FROM sessions").Scan(&n); err != nil {
+ t.Fatal(err)
+ }
+ if n != 0 {
+ t.Fatalf("a GET created %d sessions", n)
+ }
+
+ // And the link still works afterwards, which is the prefetch half.
+ if rec := redeem(t, app, token); rec.Code != http.StatusSeeOther {
+ t.Fatalf("the link was burned by a GET: redeem = %d", rec.Code)
+ }
+}
+
+// The confirmation page must not look the token up. If it did, a valid
+// token would render differently from an invalid one, and a prefetch
+// could probe validity — the oracle the POST-redeems design closes.
+func TestVerifyPageDoesNotDistinguishTokens(t *testing.T) {
+ app := newTestApp(t)
+ signIn(t, app, "person@example.invalid")
+ real := capturedLink(t, app)
+
+ get := func(token string) *httptest.ResponseRecorder {
+ req := httptest.NewRequest(http.MethodGet, linkPath+"?token="+url.QueryEscape(token), nil)
+ w := httptest.NewRecorder()
+ app.Router().ServeHTTP(w, req)
+ return w
+ }
+
+ good, bad := get(real), get("not-a-real-token-at-all")
+ if good.Code != bad.Code {
+ t.Errorf("status differs: %d vs %d", good.Code, bad.Code)
+ }
+ // The bodies differ only where the token itself is echoed into the
+ // hidden field, so compare with each one's own token removed.
+ g := strings.ReplaceAll(good.Body.String(), real, "")
+ b := strings.ReplaceAll(bad.Body.String(), "not-a-real-token-at-all", "")
+ if g != b {
+ t.Error("the confirmation page renders differently for a valid token")
+ }
+}
+
+// Unknown, expired and already-used must be one answer, byte for byte.
+func TestFailedRedemptionsAreIndistinguishable(t *testing.T) {
+ app := newTestApp(t)
+ signIn(t, app, "person@example.invalid")
+ used := capturedLink(t, app)
+ if rec := redeem(t, app, used); rec.Code != http.StatusSeeOther {
+ t.Fatalf("setup redemption failed: %d", rec.Code)
+ }
+
+ unknown := redeem(t, app, "no-such-token")
+ again := redeem(t, app, used)
+ empty := redeem(t, app, "")
+
+ for _, tc := range []struct {
+ name string
+ rec *httptest.ResponseRecorder
+ }{{"already used", again}, {"empty", empty}} {
+ if tc.rec.Code != unknown.Code {
+ t.Errorf("%s: status %d, unknown token gave %d", tc.name, tc.rec.Code, unknown.Code)
+ }
+ if tc.rec.Body.String() != unknown.Body.String() {
+ t.Errorf("%s: body differs from an unknown token's", tc.name)
+ }
+ }
+ if unknown.Code == http.StatusSeeOther {
+ t.Fatal("an unknown token was accepted")
+ }
+}
+
+// Two clicks at the same moment mint one session, not two.
+func TestConcurrentRedemptionMintsOneSession(t *testing.T) {
+ app := newTestApp(t)
+ signIn(t, app, "person@example.invalid")
+ token := capturedLink(t, app)
+
+ const racers = 12
+ var (
+ wg sync.WaitGroup
+ mu sync.Mutex
+ winners int
+ )
+ start := make(chan struct{})
+ for range racers {
+ wg.Add(1)
+ go func() {
+ defer wg.Done()
+ <-start
+ rec := redeem(t, app, token)
+ mu.Lock()
+ defer mu.Unlock()
+ if rec.Code == http.StatusSeeOther {
+ winners++
+ }
+ }()
+ }
+ close(start)
+ wg.Wait()
+
+ if winners != 1 {
+ t.Errorf("%d of %d redemptions succeeded, want exactly 1", winners, racers)
+ }
+ var n int
+ if err := app.db.Writer().QueryRow("SELECT count(*) FROM sessions").Scan(&n); err != nil {
+ t.Fatal(err)
+ }
+ if n != 1 {
+ t.Errorf("%d sessions minted from one link, want 1", n)
+ }
+}
+
+// csrf.Protect is mounted router-wide, so the mail-sending endpoint is
+// covered too — it makes outbound DNS and HTTPS calls decided by the
+// submitted address, which is a blind-SSRF surface by design.
+func TestSigninPostRequiresOriginEvidence(t *testing.T) {
+ app := newTestApp(t)
+ for _, path := range []string{"/signin", linkPath} {
+ form := url.Values{"address": {"person@example.invalid"}}
+ req := httptest.NewRequest(http.MethodPost, path, strings.NewReader(form.Encode()))
+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+ w := httptest.NewRecorder()
+ app.Router().ServeHTTP(w, req)
+ if w.Code < 400 {
+ t.Errorf("POST %s with no origin evidence = %d, want a refusal", path, w.Code)
+ }
+ }
+ if _, sent := app.mailer.last(); sent {
+ t.Error("a CSRF-refused POST still sent mail")
+ }
+}
+
+// The per-IP limiter key is the difference between a rate limit and a
+// suite-wide outage. Over a unix socket RemoteAddr is "@", and signin
+// skips the per-IP budget only for the empty string — so "@" would make
+// every request in the world share one budget of 20 per 15 minutes.
+func TestClientIPNeverCollapsesToOneBucket(t *testing.T) {
+ req := func(remote, xff string) *http.Request {
+ r := httptest.NewRequest(http.MethodPost, "/signin", nil)
+ r.RemoteAddr = remote
+ if xff != "" {
+ r.Header.Set("X-Forwarded-For", xff)
+ }
+ return r
+ }
+
+ if got := clientIP(req("@", "")); got != "" {
+ t.Errorf("clientIP over a unix socket = %q, want \"\" — %q would be one budget for everybody", got, got)
+ }
+ if got := clientIP(req("@", "203.0.113.7")); got != "203.0.113.7" {
+ t.Errorf("clientIP with X-Forwarded-For = %q, want 203.0.113.7", got)
+ }
+ // The last element is the edge's own view; earlier ones are
+ // whatever the client claimed.
+ if got := clientIP(req("@", "10.0.0.1, 203.0.113.7")); got != "203.0.113.7" {
+ t.Errorf("clientIP took %q rather than the last element", got)
+ }
+ if got := clientIP(req("192.0.2.4:5555", "")); got != "192.0.2.4" {
+ t.Errorf("clientIP from RemoteAddr = %q, want 192.0.2.4", got)
+ }
+ // Two different clients must not share a key.
+ if clientIP(req("@", "203.0.113.7")) == clientIP(req("@", "203.0.113.8")) {
+ t.Error("two clients share one limiter key")
+ }
+}
diff --git a/internal/home/templates/check_your_mail.html b/internal/home/templates/check_your_mail.html
new file mode 100644
index 0000000..8d38667
--- /dev/null
+++ b/internal/home/templates/check_your_mail.html
@@ -0,0 +1,6 @@
+{{define "title"}}Check your mail · Oficina{{end}}
+{{define "content"}}
+<h1>Check your mail</h1>
+<p>If that address can sign in here, a link is on its way. It works once, and expires in fifteen minutes.</p>
+<p><a href="/signin">Use a different address</a></p>
+{{end}}
diff --git a/internal/home/templates/layout.html b/internal/home/templates/layout.html
new file mode 100644
index 0000000..9998b43
--- /dev/null
+++ b/internal/home/templates/layout.html
@@ -0,0 +1,14 @@
+{{define "layout"}}<!doctype html>
+<html lang="{{block "lang" .}}en{{end}}">
+<head>
+<meta charset="utf-8">
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<title>{{block "title" .}}Oficina{{end}}</title>
+</head>
+<body>
+<main>
+{{template "content" .}}
+</main>
+</body>
+</html>
+{{end}}
diff --git a/internal/home/templates/signin.html b/internal/home/templates/signin.html
new file mode 100644
index 0000000..c6392bc
--- /dev/null
+++ b/internal/home/templates/signin.html
@@ -0,0 +1,10 @@
+{{define "title"}}Sign in · Oficina{{end}}
+{{define "content"}}
+<h1>Sign in</h1>
+{{if .Error}}<p role="alert">{{.Error}}</p>{{end}}
+<form method="post" action="/signin">
+ <label for="address">Email address</label>
+ <input id="address" name="address" type="email" autocomplete="email" required autofocus>
+ <button type="submit">Continue</button>
+</form>
+{{end}}
diff --git a/internal/home/templates/signin_confirm.html b/internal/home/templates/signin_confirm.html
new file mode 100644
index 0000000..1259ff4
--- /dev/null
+++ b/internal/home/templates/signin_confirm.html
@@ -0,0 +1,9 @@
+{{define "title"}}Finish signing in · Oficina{{end}}
+{{define "content"}}
+<h1>Finish signing in</h1>
+<p>Press the button to sign in on this device.</p>
+<form method="post" action="/signin/verify">
+ <input type="hidden" name="token" value="{{.Token}}">
+ <button type="submit">Sign in</button>
+</form>
+{{end}}
diff --git a/internal/home/templates/signin_failed.html b/internal/home/templates/signin_failed.html
new file mode 100644
index 0000000..74929f8
--- /dev/null
+++ b/internal/home/templates/signin_failed.html
@@ -0,0 +1,6 @@
+{{define "title"}}That link did not work · Oficina{{end}}
+{{define "content"}}
+<h1>That link did not work</h1>
+<p>Sign-in links work once and expire after fifteen minutes. Ask for a new one.</p>
+<p><a href="/signin">Start again</a></p>
+{{end}}
diff --git a/internal/home/testapp_test.go b/internal/home/testapp_test.go
index 864ca94..0b3da0b 100644
--- a/internal/home/testapp_test.go
+++ b/internal/home/testapp_test.go
@@ -2,6 +2,7 @@ package home
import (
"context"
+ "errors"
"path/filepath"
"sync"
"testing"
@@ -113,5 +114,15 @@ func newTestApp(t *testing.T) *testApp {
if err != nil {
t.Fatalf("New: %v", err)
}
+ // The classifier must never touch the network from a test. An empty
+ // TXT result is NOT enough: signin falls through to delegate, which
+ // returns the domain itself, and domainInfo then makes a live HTTPS
+ // GET to https://<domain>/.well-known/keymail. Every signIn in the
+ // suite's tests would reach out. A real error stops it dead, and
+ // classification fails open, so the magic-link path is what runs.
+ app.flow.Classifier.LookupTXT = func(context.Context, string) ([]string, error) {
+ return nil, errors.New("no resolver in tests")
+ }
+
return &testApp{App: app, mailer: mailer, db: d, dbPath: dbPath}
}