amadan
Sign-in page: say what happened (B5)
The sign-in page switched on ?outcome=sent|denied|expired, a word nothing writes. rastrillo's auth handlers and beginGuard redirect with ?sent=1 and ?err=rate|address|expired|keymail|1, so "Send me a link" came back to a blank form and every failure was silent. signinOutcome now reads that vocabulary, plus ?reauth=1. The "not on this instance" sentence is gone: beginGuard answers strangers with ?sent=1 on purpose, so nothing could reach it, and it would be an owners-list oracle if anything did. The regression test drives the real handlers (begin as owner and as stranger, the rate budget, a dotless owner address, a dead magic link, a keymail return without its cookie) and follows the Location, asserting both the query and the toned callout. Verified red against the old switch: all five route cases failed. keymail, err=1 and unknown codes are pinned by a unit test because no route test reaches them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 files changed,
+198
−7
internal/memoria/handlers.go+37 −7internal/memoria/signinoutcome_test.go+33 −0internal/memoriatest/signinoutcome_test.go+128 −0
CI passed — run details
CI log
=== vet === go vet ./... go: downloading amadan.net/oficina/estilo.git v0.0.0-20260911182719-0bcb9287988d === fmt === === test === go test ./... ? amadan.net/oficina/memoria/cmd/memoria [no test files] ok amadan.net/oficina/memoria/internal/memoria 0.726s ok amadan.net/oficina/memoria/internal/memoria/chunk 0.005s ? amadan.net/oficina/memoria/internal/memoria/icons [no test files] ok amadan.net/oficina/memoria/internal/memoria/index 0.031s ok amadan.net/oficina/memoria/internal/memoria/notion 0.018s ok amadan.net/oficina/memoria/internal/memoria/query 0.002s ok amadan.net/oficina/memoria/internal/memoria/seal 0.002s ok amadan.net/oficina/memoria/internal/memoria/sibling 3.019s ok amadan.net/oficina/memoria/internal/memoriatest 1.319s ? amadan.net/oficina/memoria/tools/bench [no test files] ? amadan.net/oficina/memoria/tools/drive [no test files] ? amadan.net/oficina/memoria/tools/seed [no test files] ? amadan.net/oficina/memoria/tools/vendorassets [no test files] === migration-check === go run github.com/carlosframework/rastrillo/cmd/rastrillo migration check rastrillo migration check: models and migrations agree
diff --git a/internal/memoria/handlers.go b/internal/memoria/handlers.go| index 21a8c94..8096f4f 100644 |
| --- a/internal/memoria/handlers.go |
| +++ b/internal/memoria/handlers.go |
| @@ -8,6 +8,7 @@ import ( |
| "log/slog" |
| "mime/multipart" |
| "net/http" |
| + "net/url" |
| "strconv" |
| "strings" |
| @@ -1094,15 +1095,44 @@ func (a *app) deleteDocument(w http.ResponseWriter, r *http.Request) { |
| func (a *app) localSignin(w http.ResponseWriter, r *http.Request) { |
| p := page{Chunks: a.chunkCount()} |
| p.ForceEmail = a.home != nil && r.URL.Query().Get("force") == "1" |
| - switch r.URL.Query().Get("outcome") { |
| - case "sent": |
| - p.Flash, p.FlashKind = "Check your email — the link signs you in.", "good" |
| - case "denied": |
| - p.Flash, p.FlashKind = "That address is not on this instance.", "danger" |
| + p.Flash, p.FlashKind = signinOutcome(r.URL.Query()) |
| + render(w, "signin", p) |
| +} |
| + |
| +// signinOutcome turns the query a sign-in redirect lands with into the |
| +// sentence the page says. The vocabulary is not ours: rastrillo's auth |
| +// handlers and beginGuard write it (?sent=1, ?err=rate|address|expired| |
| +// keymail|1, ?reauth=1), and this page must read exactly those words. |
| +// It read ?outcome= for months — a spelling nothing ever wrote — so every |
| +// sign-in, sent or failed, came back to a blank form. The regression test |
| +// follows real redirects rather than building the query by hand, because |
| +// a hand-built query is written from the same model as this switch. |
| +// |
| +// There is no "not on this instance" sentence, deliberately: beginGuard |
| +// answers a stranger with ?sent=1 so the page is not an oracle for who |
| +// the owners are. |
| +func signinOutcome(q url.Values) (msg, kind string) { |
| + if q.Get("sent") != "" { |
| + return "Check your email. We've sent you a link to sign in.", "good" |
| + } |
| + switch q.Get("err") { |
| + case "": |
| + case "rate": |
| + return "Too many sign-in attempts. Wait a few minutes, then try again.", "danger" |
| + case "address": |
| + return "That doesn't look like an email address. Check it and try again.", "danger" |
| case "expired": |
| - p.Flash, p.FlashKind = "That link had expired. Here is a fresh start.", "danger" |
| + // rastrillo says one thing for unknown, used and expired alike. |
| + return "That sign-in link has expired or was already used. Enter your email for a new one.", "danger" |
| + case "keymail": |
| + return "Keymail couldn't finish signing you in. Enter your email and we'll send you a link instead.", "danger" |
| + default: |
| + return "Something went wrong and no link was sent. Try again.", "danger" |
| } |
| - render(w, "signin", p) |
| + if q.Get("reauth") != "" { |
| + return "Sign in again to continue.", "warn" |
| + } |
| + return "", "" |
| } |
| // sniffBytes is how much of an upload looksLikeText inspects. Named |
diff --git a/internal/memoria/signinoutcome_test.go b/internal/memoria/signinoutcome_test.go| new file mode 100644 |
| index 0000000..bc02ecf |
| --- /dev/null |
| +++ b/internal/memoria/signinoutcome_test.go |
| @@ -0,0 +1,33 @@ |
| +package memoria |
| + |
| +import ( |
| + "net/url" |
| + "testing" |
| +) |
| + |
| +// The route-driven test (memoriatest.TestSigninPageSaysWhatHappened) |
| +// follows every ?err= a test can reach through the real handlers. Two it |
| +// cannot: ?err=keymail needs a keymail round trip to fail after the |
| +// pending cookie verified, and ?err=1 needs signin.Begin to fail for a |
| +// reason other than rate or address. Both are rastrillo's words |
| +// (auth/handlers.go, v0.23.0), read off the source, not guessed. A code |
| +// the page does not know still says something: silence is the bug. |
| +func TestSigninOutcomeCoversTheCodesNoRouteTestReaches(t *testing.T) { |
| + for _, c := range []struct { |
| + query, kind string |
| + }{ |
| + {"force=1&err=keymail", "danger"}, |
| + {"err=1", "danger"}, |
| + {"err=some-future-code", "danger"}, |
| + {"reauth=1", "warn"}, |
| + } { |
| + q, _ := url.ParseQuery(c.query) |
| + msg, kind := signinOutcome(q) |
| + if msg == "" || kind != c.kind { |
| + t.Errorf("?%s: got (%q, %q), want a %s sentence", c.query, msg, kind, c.kind) |
| + } |
| + } |
| + if msg, _ := signinOutcome(url.Values{}); msg != "" { |
| + t.Errorf("a plain visit says %q", msg) |
| + } |
| +} |
diff --git a/internal/memoriatest/signinoutcome_test.go b/internal/memoriatest/signinoutcome_test.go| new file mode 100644 |
| index 0000000..7d7977c |
| --- /dev/null |
| +++ b/internal/memoriatest/signinoutcome_test.go |
| @@ -0,0 +1,128 @@ |
| +package memoriatest |
| + |
| +import ( |
| + "io" |
| + "net/http" |
| + "net/http/httptest" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +// B5 (polish audit, 2026-09-23): the sign-in page said nothing after any |
| +// sign-in attempt. It switched on ?outcome=, a word nothing writes; |
| +// rastrillo's auth handlers and memoria's own beginGuard redirect with |
| +// ?sent=1 and ?err=rate|address|expired|keymail|1. So "Send me a link" |
| +// came back to the same blank form, and a rate limit, an unparseable |
| +// address and a dead link were all silent. |
| +// |
| +// Every case here drives the REAL flow and follows the Location it was |
| +// given — never a hand-built query. A hand-built query is written from |
| +// the same model as the switch that reads it, which is exactly how the |
| +// old ?outcome= test-free code agreed with itself for months. Each case |
| +// also asserts the Location's query, so if rastrillo renames a code the |
| +// failure names the new spelling rather than a missing sentence. |
| +// |
| +// Verified red 2026-09-23 against the old localSignin (switch on |
| +// "outcome"): every subtest failed with the blank form. |
| + |
| +// followToSignin performs the redirect a browser would and returns the |
| +// page body. It refuses anything that is not a redirect to /signin, so a |
| +// changed status cannot pass as "the page rendered without the flash". |
| +func followToSignin(t *testing.T, h http.Handler, rr *httptest.ResponseRecorder, wantQuery string) string { |
| + t.Helper() |
| + if rr.Code != http.StatusSeeOther && rr.Code != http.StatusFound { |
| + t.Fatalf("status = %d, want a redirect to the sign-in page; body %q", rr.Code, rr.Body.String()) |
| + } |
| + loc := rr.Header().Get("Location") |
| + if !strings.HasPrefix(loc, "/signin?") { |
| + t.Fatalf("Location = %q, want /signin?…", loc) |
| + } |
| + if got := strings.TrimPrefix(loc, "/signin?"); got != wantQuery { |
| + t.Fatalf("Location query = %q, want %q — rastrillo's vocabulary moved; "+ |
| + "signinOutcome must read the new word", got, wantQuery) |
| + } |
| + req := httptest.NewRequest(http.MethodGet, loc, nil) |
| + page := httptest.NewRecorder() |
| + h.ServeHTTP(page, req) |
| + if page.Code != http.StatusOK { |
| + t.Fatalf("GET %s = %d", loc, page.Code) |
| + } |
| + b, _ := io.ReadAll(page.Body) |
| + return string(b) |
| +} |
| + |
| +func assertFlash(t *testing.T, body, tone, sentence string) { |
| + t.Helper() |
| + if !strings.Contains(body, `rst-tone="`+tone+`"`) { |
| + t.Errorf("no %s-toned callout on the page the redirect landed on", tone) |
| + } |
| + if !strings.Contains(body, sentence) { |
| + t.Errorf("page does not say %q", sentence) |
| + } |
| +} |
| + |
| +func TestSigninPageSaysWhatHappened(t *testing.T) { |
| + t.Run("a sent link says check your email", func(t *testing.T) { |
| + // Both writers of ?sent=1: rastrillo after mailing an owner's |
| + // link, and beginGuard answering a stranger with the owner's |
| + // answer by design. |
| + for _, address := range []string{dialTestOwner, "stranger@attacker.example"} { |
| + h, _ := signinApp(t) |
| + rr := beginSignin(t, h, address) |
| + body := followToSignin(t, h, rr, "sent=1") |
| + assertFlash(t, body, "positive", "Check your email") |
| + } |
| + }) |
| + |
| + t.Run("an address rastrillo cannot parse says so", func(t *testing.T) { |
| + // Admitted by the owners list, refused by signin's parser: a |
| + // dotless domain. Only an owner reaches rastrillo at all. |
| + const owner = "paul@localhost" |
| + h, _ := signinAppWithOwners(t, []string{owner}) |
| + rr := beginSignin(t, h, owner) |
| + body := followToSignin(t, h, rr, "err=address") |
| + assertFlash(t, body, "negative", "look like an email address") |
| + }) |
| + |
| + t.Run("a rate-limited address says wait", func(t *testing.T) { |
| + // signin's per-address budget, spent on an owner whose links |
| + // are being sent (the stub mailer takes them). |
| + h, _ := signinApp(t) |
| + var rr *httptest.ResponseRecorder |
| + for i := 0; i < 20; i++ { |
| + rr = beginSignin(t, h, dialTestOwner) |
| + if strings.Contains(rr.Header().Get("Location"), "err=rate") { |
| + break |
| + } |
| + } |
| + body := followToSignin(t, h, rr, "err=rate") |
| + assertFlash(t, body, "negative", "Too many sign-in attempts") |
| + }) |
| + |
| + t.Run("a dead magic link says it expired", func(t *testing.T) { |
| + h, _ := signinApp(t) |
| + req := httptest.NewRequest(http.MethodGet, "/auth/verify?token=not-a-real-token", nil) |
| + rr := httptest.NewRecorder() |
| + h.ServeHTTP(rr, req) |
| + body := followToSignin(t, h, rr, "err=expired") |
| + assertFlash(t, body, "negative", "expired or was already used") |
| + }) |
| + |
| + t.Run("a keymail return with no pending cookie says it expired", func(t *testing.T) { |
| + h, _ := signinApp(t) |
| + req := httptest.NewRequest(http.MethodGet, "/auth/callback?code=x&state=y", nil) |
| + rr := httptest.NewRecorder() |
| + h.ServeHTTP(rr, req) |
| + body := followToSignin(t, h, rr, "err=expired") |
| + assertFlash(t, body, "negative", "expired or was already used") |
| + }) |
| + |
| + t.Run("a plain visit says nothing", func(t *testing.T) { |
| + h, _ := signinApp(t) |
| + rr := httptest.NewRecorder() |
| + h.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/signin", nil)) |
| + if strings.Contains(rr.Body.String(), "rst-callout") { |
| + t.Error("a sign-in page nobody submitted carries a callout") |
| + } |
| + }) |
| +} |