amadan
Sign-in: continue to Keymail instead of redirecting a form POST there
Signing in with a Keymail address could not leave the sign-in page on memoria.oficina.works (reported by Paul, 2026-09-29, with Safari's console error). Begin answers such an address with 303 to the Keymail server's /oauth/authorize, and the CSP's form-action 'self' is enforced across a form submission's whole redirect chain, so the browser refused it and the button span forever. memoria now wraps Begin in home/connect's OAuthContinuation — the helper extracted from Docs and already used by Sheets — inside the existing admission guard. It replaces exactly that 303, after validating it as an authorize URL for this origin and callback with PKCE S256, with a no-store, no-referrer continuation page whose meta refresh and link are navigations rather than form submissions. Everything else, including the magic-link redirect and gate 33's refusal, passes through untouched. The CSP is not widened. Also: the model layer now reads its TensorX credential from MEMORIA_TENSORX_KEY, the name it was already given on the platform, so switching it on is MEMORIA_AI=on and nothing else. Tested through rastrillo's real Begin and the real mounted route, with only the network faked; restoring the old wiring fails with the production symptom. Reviewed by Codex GPT-6-Astra (dispatched codex/gpt-6-astra; self-reported "GPT-6 (Codex)"): APPROVE, no must-fix — including a WebKit and Chromium run of the rendered page under the exact CSP, where both refused the old redirect and followed the continuation. Two test-strength polish items are a follow-up. Docs, Calendar and Calls do not wire OAuthContinuation either and very likely have the same bug; not touched here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Squashed from fix/signin-keymail into main.
8 files changed,
+288
−8
MODEL-FALLBACK-PLAN.md+1 −1internal/memoria/app.go+3 −2internal/memoria/auth.go+43 −0internal/memoria/config.go+7 −2internal/memoria/model_config_test.go+2 −2internal/memoria/render.go+2 −1internal/memoria/templates/signin_oauth.html+10 −0internal/memoriatest/signin_keymail_test.go+220 −0
CI passed — run details
CI log
=== vet === go vet ./... === fmt === === test === go test ./... ? amadan.net/oficina/memoria/cmd/memoria [no test files] ok amadan.net/oficina/memoria/internal/memoria 3.217s ok amadan.net/oficina/memoria/internal/memoria/chunk (cached) ok amadan.net/oficina/memoria/internal/memoria/fact (cached) ? amadan.net/oficina/memoria/internal/memoria/icons [no test files] ok amadan.net/oficina/memoria/internal/memoria/index (cached) ok amadan.net/oficina/memoria/internal/memoria/notion (cached) ok amadan.net/oficina/memoria/internal/memoria/provider (cached) ok amadan.net/oficina/memoria/internal/memoria/query (cached) ok amadan.net/oficina/memoria/internal/memoria/seal (cached) ok amadan.net/oficina/memoria/internal/memoria/sibling (cached) ok amadan.net/oficina/memoria/internal/memoriatest 1.567s ? amadan.net/oficina/memoria/scripts/writeestilo [no test files] ? 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/MODEL-FALLBACK-PLAN.md b/MODEL-FALLBACK-PLAN.md| index 77c964b..7287816 100644 |
| --- a/MODEL-FALLBACK-PLAN.md |
| +++ b/MODEL-FALLBACK-PLAN.md |
| @@ -97,7 +97,7 @@ The only code in memoria that dials a model. |
| - `MEMORIA_AI` — `on` or unset. Decision 4: opt-in per team, the owner's |
| choice. Anything other than `on` or empty is a startup error, so a typo |
| does not silently mean off. |
| -- `MEMORIA_TENSORX_API_KEY` — required when `MEMORIA_AI=on`, **refused at |
| +- `MEMORIA_TENSORX_KEY` — required when `MEMORIA_AI=on`, **refused at |
| startup** if AI is on and it is absent (loud), and ignored when AI is |
| off. A key present with AI off does not turn AI on. |
diff --git a/internal/memoria/app.go b/internal/memoria/app.go| index 30cad65..150b625 100644 |
| --- a/internal/memoria/app.go |
| +++ b/internal/memoria/app.go |
| @@ -176,8 +176,9 @@ func (a *app) MountAuth(mux *http.ServeMux, origin string) { |
| r := chi.NewRouter() |
| r.Use(a.homeMiddleware(origin)) |
| // Guarded, not bare: the classifier inside Begin dials the submitted |
| - // address's domain before Authorize runs. beginGuard says why. |
| - r.Post("/auth/begin", a.beginGuard(origin, a.auth.Begin)) |
| + // address's domain before Authorize runs. beginGuard says why. And |
| + // continued, not redirected: signinBegin says why. |
| + r.Post("/auth/begin", a.signinBegin(origin, a.auth.Begin)) |
| if a.home == nil { |
| r.Get("/auth/callback", a.auth.Callback) |
| r.Get("/auth/verify", a.auth.Verify) |
diff --git a/internal/memoria/auth.go b/internal/memoria/auth.go| index a9a6b41..d5de3e1 100644 |
| --- a/internal/memoria/auth.go |
| +++ b/internal/memoria/auth.go |
| @@ -7,6 +7,7 @@ import ( |
| "net/http" |
| "strings" |
| + "amadan.net/oficina/home/connect" |
| "github.com/carlosframework/rastrillo/auth" |
| "github.com/carlosframework/rastrillo/mail" |
| ) |
| @@ -211,3 +212,45 @@ func NewAuth(sqlDB *sql.DB, env AuthEnv, logger *slog.Logger, authorize func(str |
| } |
| return a, nil |
| } |
| + |
| +// signinBegin is POST /auth/begin's whole handler chain, in one place with |
| +// its own tests rather than composed inline in MountAuth. |
| +// |
| +// **The guard is what keeps a non-admitted address from being dialled** |
| +// (gate 33), and it runs before Begin either way. It is placed outermost |
| +// by preference, so a refused address never reaches the continuation code |
| +// at all — but the two orders are behaviourally identical, and a review of |
| +// the mutation that swaps them found nothing to tell them apart: the |
| +// continuation rewrites only an EXTERNAL 303, and the guard only ever |
| +// answers with a same-origin one. The order is a preference, not a guard. |
| +// |
| +// **Inside it, Begin is wrapped in Home's OAuthContinuation.** For a |
| +// Keymail address Begin answers 303 to the Keymail server's |
| +// /oauth/authorize — and CSP's form-action 'self' is enforced across a |
| +// form submission's WHOLE redirect chain, so the browser refused it and |
| +// sign-in with a Keymail address could not leave the sign-in page (seen |
| +// in Safari, 2026-09-29; the rastrillo docs describe the same refusal). |
| +// OAuthContinuation replaces exactly that 303 — after checking it is a |
| +// well-formed authorize URL for THIS origin and callback, with PKCE S256 — |
| +// with a no-store, no-referrer continuation page whose navigation is not a |
| +// form submission. Every other response, including the magic-link path's |
| +// same-origin redirect, passes through untouched. The CSP is not widened: |
| +// that would have to name every Keymail server, and the framework notes |
| +// that a federated address on an unlisted host would still be refused. |
| +func (a *app) signinBegin(origin string, begin http.HandlerFunc) http.HandlerFunc { |
| + return a.beginGuard(origin, connect.OAuthContinuation(origin, begin, a.renderOAuth)) |
| +} |
| + |
| +// renderOAuth renders the continuation page inside memoria's layout, footer |
| +// and AI state included — it is a page like any other. |
| +func (a *app) renderOAuth(w http.ResponseWriter, r *http.Request, destination string) { |
| + render(w, "signin_oauth", oauthPage{ |
| + page: page{Chunks: a.chunkCount(), ModelOn: a.modelOn()}, |
| + Data: connect.OAuthPage(destination), |
| + }) |
| +} |
| + |
| +type oauthPage struct { |
| + page |
| + Data connect.OAuthData |
| +} |
diff --git a/internal/memoria/config.go b/internal/memoria/config.go| index 9d174fd..7950722 100644 |
| --- a/internal/memoria/config.go |
| +++ b/internal/memoria/config.go |
| @@ -111,8 +111,13 @@ func LoadConfig(getenv func(string) string) (Config, error) { |
| case "": |
| case "on": |
| c.AI = true |
| - if c.TensorXKey = getenv("MEMORIA_TENSORX_API_KEY"); c.TensorXKey == "" { |
| - return c, fmt.Errorf("%w: MEMORIA_TENSORX_API_KEY (MEMORIA_AI=on names a provider but gives no credential for it)", ErrMissing) |
| + // MEMORIA_TENSORX_KEY, because that is the name the credential was |
| + // already given on the platform (set by hand for the 2026-09-05 |
| + // live-key checks). This code first read MEMORIA_TENSORX_API_KEY, |
| + // which would have meant a second copy of the key under a new name — |
| + // and a platform secret cannot be read back to make one. |
| + if c.TensorXKey = getenv("MEMORIA_TENSORX_KEY"); c.TensorXKey == "" { |
| + return c, fmt.Errorf("%w: MEMORIA_TENSORX_KEY (MEMORIA_AI=on names a provider but gives no credential for it)", ErrMissing) |
| } |
| default: |
| return c, fmt.Errorf("memoria: MEMORIA_AI must be \"on\" or unset; refusing to guess what %q means", ai) |
diff --git a/internal/memoria/model_config_test.go b/internal/memoria/model_config_test.go| index c169509..1139e84 100644 |
| --- a/internal/memoria/model_config_test.go |
| +++ b/internal/memoria/model_config_test.go |
| @@ -34,7 +34,7 @@ func TestTheModelSwitch(t *testing.T) { |
| // Presence of a credential is not consent; the switch is. |
| {"a key with AI unset is still off", "", "k", false, ""}, |
| {"on with a key is on", "on", "k", true, ""}, |
| - {"on without a key is refused", "on", "", false, "MEMORIA_TENSORX_API_KEY"}, |
| + {"on without a key is refused", "on", "", false, "MEMORIA_TENSORX_KEY"}, |
| // A typo must not be read as either answer. |
| {"'true' is refused, not on", "true", "k", false, "MEMORIA_AI"}, |
| {"'On' is refused, not on", "On", "k", false, "MEMORIA_AI"}, |
| @@ -42,7 +42,7 @@ func TestTheModelSwitch(t *testing.T) { |
| } { |
| t.Run(c.name, func(t *testing.T) { |
| env := baseEnv() |
| - env["MEMORIA_AI"], env["MEMORIA_TENSORX_API_KEY"] = c.ai, c.key |
| + env["MEMORIA_AI"], env["MEMORIA_TENSORX_KEY"] = c.ai, c.key |
| cfg, err := LoadConfig(func(k string) string { return env[k] }) |
| if c.wantErr != "" { |
| if err == nil || !strings.Contains(err.Error(), c.wantErr) { |
diff --git a/internal/memoria/render.go b/internal/memoria/render.go| index fb5a43b..7b9d4f1 100644 |
| --- a/internal/memoria/render.go |
| +++ b/internal/memoria/render.go |
| @@ -52,13 +52,14 @@ func funcs() template.FuncMap { |
| } |
| func init() { |
| - for _, name := range []string{"index", "document", "signin", "connectors", "notion", "home", "credits"} { |
| + for _, name := range []string{"index", "document", "signin", "signin_oauth", "connectors", "notion", "home", "credits"} { |
| pages[name] = template.Must(template.New("layout"). |
| Funcs(funcs()). |
| ParseFS(appFS, "templates/layout.html", "templates/results.html", "templates/"+name+".html")) |
| } |
| pages["credits"] = template.Must(pages["credits"].ParseFS(credits.Templates(), "templates/credits.html")) |
| pages["home"] = template.Must(pages["home"].ParseFS(connect.Templates(), "templates/home_link.html")) |
| + pages["signin_oauth"] = template.Must(pages["signin_oauth"].ParseFS(connect.Templates(), "templates/oauth.html")) |
| fragments = template.Must(template.New("fragments"). |
| Funcs(funcs()). |
| ParseFS(appFS, "templates/results.html")) |
diff --git a/internal/memoria/templates/signin_oauth.html b/internal/memoria/templates/signin_oauth.html| new file mode 100644 |
| index 0000000..7823ba5 |
| --- /dev/null |
| +++ b/internal/memoria/templates/signin_oauth.html |
| @@ -0,0 +1,10 @@ |
| +{{/* The Keymail continuation page (home/connect OAuthContinuation). The |
| + sign-in POST ends here, with a 200, instead of a 303 to Keymail: CSP's |
| + form-action 'self' applies to a form submission's WHOLE redirect chain, |
| + so the 303 was refused and sign-in with a Keymail address could not |
| + leave the page. This page's meta refresh — and its link, for anyone |
| + without it — is a navigation, not a form submission, so form-action |
| + does not apply. The markup is Home's shared template (Estilo |
| + homeconnect), rendered inside memoria's own layout as Sheets does. */}} |
| +{{define "head"}}{{template "home-oauth-head" .Data}}{{end}} |
| +{{define "content"}}{{template "home-oauth" .Data}}{{end}} |
diff --git a/internal/memoriatest/signin_keymail_test.go b/internal/memoriatest/signin_keymail_test.go| new file mode 100644 |
| index 0000000..658f96a |
| --- /dev/null |
| +++ b/internal/memoriatest/signin_keymail_test.go |
| @@ -0,0 +1,220 @@ |
| +package memoriatest |
| + |
| +import ( |
| + "context" |
| + "encoding/binary" |
| + "io" |
| + "log/slog" |
| + "net" |
| + "net/http" |
| + "net/http/httptest" |
| + "net/url" |
| + "path/filepath" |
| + "strings" |
| + stdsync "sync" |
| + "testing" |
| + |
| + "github.com/carlosframework/rastrillo/db" |
| + |
| + "amadan.net/oficina/memoria/internal/memoria" |
| +) |
| + |
| +// fakeKeymail answers the two probes signin's classifier makes of a |
| +// Keymail server — /.well-known/keymail and /api/federation/lookup — for |
| +// one host, and 404s everything else. It stands in for http.DefaultTransport, |
| +// which the classifier's client falls back to. |
| +type fakeKeymail struct { |
| + host string |
| + mu stdsync.Mutex |
| + hits []string |
| +} |
| + |
| +func (f *fakeKeymail) RoundTrip(r *http.Request) (*http.Response, error) { |
| + f.mu.Lock() |
| + f.hits = append(f.hits, r.URL.Host+r.URL.Path) |
| + f.mu.Unlock() |
| + body, code := `{}`, http.StatusNotFound |
| + if r.URL.Host == f.host { |
| + switch r.URL.Path { |
| + case "/.well-known/keymail": |
| + body, code = `{"version":"1"}`, http.StatusOK |
| + case "/api/federation/lookup": |
| + code = http.StatusOK |
| + } |
| + } |
| + return &http.Response{StatusCode: code, Body: io.NopCloser(strings.NewReader(body)), |
| + Header: http.Header{"Content-Type": {"application/json"}}, Request: r}, nil |
| +} |
| + |
| +// nxdomainResolver answers every DNS query NXDOMAIN — "this domain |
| +// publishes no _keymail delegation" — which signin's classifier reads as |
| +// "the domain itself is the Keymail candidate". Go's resolver speaks |
| +// length-prefixed DNS over any conn that is not a PacketConn, which a |
| +// net.Pipe is not. |
| +func nxdomainResolver() *net.Resolver { |
| + return &net.Resolver{PreferGo: true, Dial: func(context.Context, string, string) (net.Conn, error) { |
| + client, server := net.Pipe() |
| + go func() { |
| + defer server.Close() |
| + for { |
| + var n [2]byte |
| + if _, err := io.ReadFull(server, n[:]); err != nil { |
| + return |
| + } |
| + q := make([]byte, binary.BigEndian.Uint16(n[:])) |
| + if _, err := io.ReadFull(server, q); err != nil || len(q) < 12 { |
| + return |
| + } |
| + // Header plus the question, and nothing after it. |
| + i := 12 |
| + for i < len(q) && q[i] != 0 { |
| + i += int(q[i]) + 1 |
| + } |
| + i += 5 // the root label, QTYPE, QCLASS |
| + if i > len(q) { |
| + return |
| + } |
| + resp := append([]byte(nil), q[:i]...) |
| + rd := binary.BigEndian.Uint16(q[2:4]) & 0x0100 |
| + binary.BigEndian.PutUint16(resp[2:4], 0x8000|0x0400|rd|0x0080|3) // QR AA RD RA, NXDOMAIN |
| + binary.BigEndian.PutUint16(resp[4:6], 1) |
| + for _, off := range []int{6, 8, 10} { |
| + binary.BigEndian.PutUint16(resp[off:off+2], 0) |
| + } |
| + binary.BigEndian.PutUint16(n[:], uint16(len(resp))) |
| + if _, err := server.Write(append(n[:], resp...)); err != nil { |
| + return |
| + } |
| + } |
| + }() |
| + return client, nil |
| + }} |
| +} |
| + |
| +// keymailSigninApp is the real app with the real sign-in wired, whose |
| +// owner's domain resolves — through the fakes above — to a Keymail server. |
| +func keymailSigninApp(t *testing.T) (http.Handler, *fakeKeymail) { |
| + t.Helper() |
| + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) |
| + d, err := db.Open(filepath.Join(t.TempDir(), "app.db"), logger) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + t.Cleanup(func() { d.Close() }) |
| + mux, a, err := memoria.App(d, testOrigin, testDEK(t), logger) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + au, err := memoria.NewAuth(d.Writer(), memoria.AuthEnv{ |
| + Origin: testOrigin, InstanceKey: "signin-keymail-instance-key", |
| + Owners: []string{dialTestOwner}, Mailer: &stubMailer{}, |
| + }, logger, a.Admit) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + a.UseAuth(au) |
| + a.SetOwners([]string{dialTestOwner}) |
| + a.MountAuth(mux, testOrigin) |
| + |
| + km := &fakeKeymail{host: "example.test"} |
| + prevT, prevR := http.DefaultTransport, net.DefaultResolver |
| + http.DefaultTransport, net.DefaultResolver = km, nxdomainResolver() |
| + t.Cleanup(func() { http.DefaultTransport, net.DefaultResolver = prevT, prevR }) |
| + return mux, km |
| +} |
| + |
| +// **Signing in with a Keymail address must be able to leave the page.** |
| +// |
| +// Begin answers a Keymail address with 303 to the server's /oauth/authorize, |
| +// and memoria's CSP says form-action 'self' — which browsers enforce across |
| +// a form submission's WHOLE redirect chain. Safari refused the redirect and |
| +// the sign-in button span forever (2026-09-29, memoria.oficina.works). The |
| +// POST now ends in a 200 continuation page whose navigation is not a form |
| +// submission. |
| +// |
| +// Through rastrillo's REAL Begin and the REAL mounted route: the classifier |
| +// is only faked at the network, so what is asserted is what production |
| +// sends. |
| +func TestAKeymailSigninCanLeaveThePage(t *testing.T) { |
| + h, km := keymailSigninApp(t) |
| + rr := beginSignin(t, h, dialTestOwner) |
| + |
| + // Presence: the classifier really took the Keymail path. Without this, |
| + // a 200 below could be the magic-link page and prove nothing. |
| + if !contains(km.hits, "example.test/api/federation/lookup") { |
| + t.Fatalf("the classifier never reached the Keymail probes (hits %v); this proves nothing", km.hits) |
| + } |
| + |
| + if rr.Code == http.StatusSeeOther { |
| + t.Fatalf("POST /auth/begin still redirects (Location %q): form-action 'self' refuses this in the browser", rr.Header().Get("Location")) |
| + } |
| + if rr.Code != http.StatusOK { |
| + t.Fatalf("status %d, want 200", rr.Code) |
| + } |
| + body := rr.Body.String() |
| + |
| + // The continuation navigates to the authorize URL Begin chose, for THIS |
| + // origin and callback. |
| + i := strings.Index(body, `http-equiv="refresh"`) |
| + if i < 0 { |
| + t.Fatalf("no meta refresh in the continuation page:\n%s", body) |
| + } |
| + for _, want := range []string{ |
| + "https://example.test/oauth/authorize", |
| + "client_id=" + url.QueryEscape(testOrigin), |
| + "redirect_uri=" + url.QueryEscape(testOrigin+"/auth/callback"), |
| + "code_challenge_method=S256", |
| + } { |
| + if !strings.Contains(strings.ReplaceAll(body, "&", "&"), want) { |
| + t.Errorf("continuation page lacks %q", want) |
| + } |
| + } |
| + if got := rr.Header().Get("Cache-Control"); got != "no-store" { |
| + t.Errorf("Cache-Control = %q, want no-store: the page carries a one-time PKCE state", got) |
| + } |
| + if got := rr.Header().Get("Referrer-Policy"); got != "no-referrer" { |
| + t.Errorf("Referrer-Policy = %q, want no-referrer", got) |
| + } |
| + // Begin's pending cookie must survive the wrapping, or the callback |
| + // cannot match the flow it started. |
| + if !strings.Contains(strings.Join(rr.Header().Values("Set-Cookie"), "\n"), "rastrillo_pending") { |
| + t.Errorf("the pending cookie did not survive: %q", rr.Header().Values("Set-Cookie")) |
| + } |
| + // Rendered as a memoria page, footer and all. |
| + if !strings.Contains(body, "<footer") { |
| + t.Error("the continuation page is not inside memoria's layout") |
| + } |
| +} |
| + |
| +// And the paths that must NOT change: a non-owner still gets the guard's |
| +// same-origin redirect with no dial (gate 33's ground), and an address with |
| +// no Keymail still gets the magic link's same-origin redirect, untouched. |
| +func TestSigninPathsThatDoNotLeaveAreUnchanged(t *testing.T) { |
| + h, km := keymailSigninApp(t) |
| + |
| + rr := beginSignin(t, h, "stranger@elsewhere.test") |
| + if rr.Code != http.StatusSeeOther || rr.Header().Get("Location") != "/signin?sent=1" { |
| + t.Errorf("non-owner: %d %q, want 303 /signin?sent=1", rr.Code, rr.Header().Get("Location")) |
| + } |
| + if len(km.hits) != 0 { |
| + t.Errorf("a non-owner's sign-in reached the network: %v", km.hits) |
| + } |
| + |
| + // An owner whose domain has no Keymail (the fake 404s other hosts). |
| + km.host = "nowhere.invalid" |
| + rr = beginSignin(t, h, dialTestOwner) |
| + if rr.Code != http.StatusSeeOther || !strings.HasPrefix(rr.Header().Get("Location"), "/") { |
| + t.Errorf("magic-link path: %d %q, want a same-origin 303", rr.Code, rr.Header().Get("Location")) |
| + } |
| +} |
| + |
| +func contains(xs []string, want string) bool { |
| + for _, x := range xs { |
| + if x == want { |
| + return true |
| + } |
| + } |
| + return false |
| +} |
| + |
| +var _ = httptest.NewRecorder |