amadan

oficina / home Public

← Discussions

Sign out ends the id.* session too (B2) open

On branch fix/r1-signout-ends-id-session

claude operated by paul@keymail.dev session 21a32889 · claude-code/2.1.280 · MacBookPro.localdomain

🤖 Polish round 1, bug B2: Sign out doesn't sign you out.

The bug

POST /signout on the directory (oficina.works) cleared only the directory session. The 30-day credential session on id.oficina.works survived, so Sign in on "You are signed out", or any app's "Sign in with Home", went back through /authorize in about 130 ms with no prompt.

The fix (commit 1937592)

  • Directory POST /signout: ends its own session, then answers 307 to id.*/signout. The 307 keeps the hop a POST, so sign-out is never a GET that someone could link to.
  • Credential POST /signout (new, credential role only, id host only): revokes the session row, clears the cookie, then 303s to oficina.works/signed-out. - It sits outside csrf.Protect, because the hop is same-site, not same-origin. It has its own exact check instead (signoutFromHome). A request passes if any one of these holds: - Origin is exactly the directory origin. - The request passes the ordinary same-origin check for id.. - Origin is null or absent, and Referer is exactly the directory origin on a request marked same-site (or not marked at all). - Why the Referer fallback is needed: in a real browser, Chromium and* WebKit send Origin: null on the redirected POST. I captured this live, and it is why a pure Origin check failed end to end. - It refuses team instances (same-site), every other origin, bad schemes, userinfo tricks, and requests with no evidence.
  • Directory CSP: form-action adds the credential origin. form-action is enforced across a form's redirects, so without it the browser blocks the 307.

Per-app sessions: independent, deliberately

Sessions that an instance minted from an earlier assertion are the instance's own. The home keeps no list of where it has sent an assertion, and a hibernating fleet would not hear a revocation anyway. This matches the spec's deferred row, "Suite-wide sign-out / session revocation".

What this branch guarantees: after Home's Sign out, no app can get a new assertion for this browser without the email or passkey step. Each app's own Sign out still ends only that app's session.

Follow-up, not in this branch: have home/connect's sign-out chain to id.*/signout. It needs the allowlist widened to registered consumer origins, and it is a cross-app change.

Tests (internal/home/signout_test.go)

  • TestSignOutEndsTheCredentialSession: full sign-in across both roles, then directory sign-out, then the id hop with the exact headers Chromium sends. After that, /authorize/resume redirects to /signin instead of minting, and the session row is gone.
  • TestCredentialSignOutRefusesOtherOrigins: 14 refused header shapes, 3 accepted hand-off shapes, and a check that GET does nothing. Neither role answers the other's host.
  • TestDirectoryPolicyAdmitsTheSignOutHandOff: checks that the directory's form-action includes the credential origin.

Proof the tests exercise the fix: with the non-test changes stashed, all three fail (303 "/signed-out", 404, form-action omits https://id.oficina.works). With the changes restored, all three pass.

Live, production-shaped run (both roles behind a TLS front plus a mail sink; polish-audit/home-run/signout-e2e*.mjs), in Chromium and WebKit:

POST https://home.test/signout    -> 307
POST https://id.home.test/signout -> 303  origin=null sec-fetch-site=same-site referer=https://home.test/
GET  https://home.test/signed-out -> 200  "You are signed out"; all cookies gone
[Sign in] -> /auth/start -> id/authorize -> id/authorize/resume -> id/signin 200: email form shown

Signing in again by magic link works afterwards, and there are no CSP console errors. I could not test Firefox, because its headless build fails to launch in this sandbox.

Gate: make test-vet test-fmt test-build test-go passes. -race passes on internal/home, operator, directorydata, handoff, roster and connect, with GOTOOLCHAIN=go1.26.5. internal/provisioner is linux-only and cannot build -race on darwin, so CI covers it.

Needs a whole-branch review by the highest-capability model before merge (AGENTS.md).

claude operated by paul@keymail.dev session 21a32889 · claude-code/2.1.280 · MacBookPro.localdomain

🤖 CI is red for an infrastructure reason, not the code. The runner gets HTTP 429 from the hub while fetching private modules (amadan.net/oficina/estilo, amadan.net/carlos/platform) at the vet step, or during sync. main @ 3339ff6 fails the same way. One re-run of #71 (8673aea) hit the same 429. I've stopped retrying so I don't add to the rate limiting. The local gate passes: see the opening post.

claude operated by paul@keymail.dev session 21a32889 · claude-code/2.1.280 · MacBookPro.localdomain

🤖 Whole-branch review — Claude Fable 5.1 (claude-fable-5-1), per AGENTS.md "Review". Read as one diff origin/main...origin/fix/r1-signout-ends-id-session (merge base 3339ff6, fix 1937592, head 8673aea empty CI re-run) in a scratch worktree.

VERDICT: APPROVE-WITH-NITS

Nothing blocking. The fix is at the root (the credential session is revoked in the DB, not just the cookie cleared), the tests exercise it, the gate is green locally, and the CSRF reasoning holds up under the attacks I could construct.

What I verified

  • Gate: GOTOOLCHAIN=go1.26.5 make test-vet test-fmt test-build test-go — all packages ok, exit 0.
  • Tests fail without the fix: with app.go, directory.go, security.go reverted to main and signout.go reduced to the signoutPath const, all three new tests fail exactly as the opening post says (303 "/signed-out", 404, form-action omits https://id.oficina.works). Restored: pass. TestHomeRoleAndMutationIsolation (directory /signout still 403s a POST with the credential Origin) and TestHomePolicyMatchesRegisteredConsumers (credential CSP unchanged) still pass.
  • Logout CSRF, cross-site: evil.example form POST → Sec-Fetch-Site: cross-site, Origin: https://evil.example → refused by both csrf.SameOrigin and the default: arm. Via an attacker-side 307 → Origin: null, but Sec-Fetch-Site is computed over the whole redirect chain (cross-site) and Referer is the initiating document (evil), so refused. Legacy browsers with no Sec-Fetch-Site send Origin on cross-origin POSTs → refused; nulled by redirect → Referer is still evil → refused. The id.* cookie is SameSite=Lax, host-only, so it would not even ride a cross-site POST. Low-impact class anyway, and closed.
  • Referer fallback spoofing: the fallback needs Referer origin == the directory origin exactly (scheme, host, no userinfo). Referer is set by the browser from the initiating document and is not rewritten by redirects, so an attacker needs a document on oficina.works to initiate the POST. That is XSS on the directory or a POST-preserving redirector on it; the only TemporaryRedirect in the repo is this new one, everything else is 303 (becomes GET, which the endpoint ignores). XSS on the directory could already POST the directory's own /signout same-origin, so no new exposure. A same-site team instance (<team>.<app>.oficina.works) arrives with its own Origin or its own Referer → refused (tested).
  • Widened form-action: directory CSP only; adds the credential origin next to 'self' and the registered consumer origins it already allowed. Credential-role CSP untouched. Only a form injected into directory HTML could use it, which is the XSS case above.
  • *What the 307 carries to id.:** the form body is empty (<form method="post" action="/signout"> has one unnamed submit button; rastrillo CSRF is origin-evidence, there is no token). The directory session cookie is host-only (no Domain), so it is not sent to id.oficina.works. Nothing secret crosses the origin.
  • Host/role separation: credential /signout is credentialOnly and only mounted when role == "credential"; serveKnownHost still runs first; tests assert 404 both ways.
  • Spec reading: your comment is right that the Fetch spec's tainted-origin rule would not taint this hop (request origin == pre-redirect URL origin), so the spec says Origin: https://oficina.works and Chromium/WebKit send null anyway. Both shapes are accepted; good.

Findings (none blocking)

  1. [low] internal/home/signout.go:41 — the refusal is a bare http.Error after the directory half has already run. A browser that strips the cross-origin Referer (Firefox with network.http.referer.XOriginPolicy=2, some privacy extensions) reaches id. with Origin: null and no Referer, gets a plain-text 403, and is left half signed out: directory session gone, 30-day id. session alive — B2 with an error page on top. Loud is right, but it should offer the way out: render a small page on id.* with a same-origin <form method="post" action="/signout"> ("Confirm sign-out"), which passes csrf.SameOrigin and needs no Referer. Fits the audit's "errors are bare Go text" finding too. Follow-up branch is fine.
  2. [info] Deploy order is real: oficina-id (id.oficina.works) before oficina (oficina.works). With the new directory and the old credential binary, Sign out clears the directory session and 307s to a route that does not exist yet: the user lands on id.'s bare 404 page not found and the id. session survives until the second deploy. Reverse order is harmless (new route, no caller). No code change is worth it for a one-time ordering; record it in the merge note and verify X-Carlos-Version on id.oficina.works before deploying the directory. Finding 1's confirm page would also turn a mis-ordered deploy into a one-click recovery rather than a 404.
  3. [nit] Firefox is unverified. Firefox also sends Origin: null on a cross-origin redirected POST, so it should take the Referer path like Chromium; worth one manual click after deploy since the headless run could not launch.
  4. [nit] signout_test.go accepted-shapes table could add {directoryOrigin, "same-site", directoryOrigin + "/"} (the exact Fetch-spec shape, both headers present). Not needed for correctness; the Origin arm already covers it.

Per-app sessions staying independent — acceptable for round 1

Agreed. This is the OIDC norm (RP sessions outlive the OP session unless front/back-channel logout is built), the spec defers "suite-wide sign-out / session revocation", and the home keeping no list of where assertions went is the thin-and-blind rule, not laziness. What this branch actually guarantees, and states plainly, is the thing that matters on a shared machine: after Home's Sign out, no app can mint a new assertion without the email or passkey step.

The gap that remains is the B2 shape at app level: sign out of Docs, click Sign in, and you are back in silently because id. is alive. That is a real follow-up (home/connect sign-out chaining to `id./signout`, consumer origins in the allowlist), and belongs with the R2 "sign-on unification (S8 + B2)" item. Cheap interim: each app's signed-out page says it signed you out of that app only, with a link to Home to sign out of Oficina.

Conventions

Comments and naming match AGENTS.md; no address, team name or content in any log or error string; migrations untouched; no script added. CI is red only for the hub's HTTP 429 on private modules (main fails identically), which AGENTS.md's "CI actually green" still requires before the squash — re-run once the hub stops rate-limiting.

claude operated by paul@keymail.dev session 21a32889 · claude-code/2.1.280 · MacBookPro.localdomain

Merging without a green hub CI run, on Paul's instruction of 2026-09-23 ("Can we merge and deploy where we're at, and continue?"). CI fails for infrastructure reasons only: the hub runner gets HTTP 429 fetching private modules, and its module cache can't be reset (main fails the same way). Fable 5.1 approved the branch, and the local gate passes (GOTOOLCHAIN=go1.26.5 make test-vet test-fmt test-build test-go).