amadan

oficina / home Public

Make Home's sign out end the id.* sign-in session

POST /signout on the directory cleared only the app-list session, so the
30-day id.* session survived and Sign in re-entered silently (B2). The
directory now ends its session and 307s the POST to a new credential-role
POST /signout, which deletes the id.* session and returns to
/signed-out. It is checked by Origin, same-origin, or an exact directory
Referer when Origin is null (Chromium and WebKit null it on the
redirected POST). The directory CSP form-action admits the id origin.
Per-app sessions stay independent; in-app sign-out chaining is round 2.
Deploy order: oficina-id before oficina. Reviewed by Fable 5.1 on #71.
Merged without green hub CI (runner 429s) on Paul's instruction.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Squashed from fix/r1-signout-ends-id-session into main.
Paul Campbell pushed by claude 802518fd7cb0254a1a1a2ef301a0b53339c7c03e parent 3339ff6
5 files changed, +309 −2
  • internal/home/app.go +7 −0
  • internal/home/directory.go +5 −2
  • internal/home/security.go +6 −0
  • internal/home/signout.go +91 −0
  • internal/home/signout_test.go +200 −0

CI cancelled — superseded by a newer push — run details

diff --git a/internal/home/app.go b/internal/home/app.go
index 408a2f3..85bb3e9 100644
--- a/internal/home/app.go
+++ b/internal/home/app.go
@@ -174,6 +174,13 @@ func (a *App) Router() http.Handler {
w.WriteHeader(http.StatusOK)
})
+ if a.runtime != nil && a.runtime.role == "credential" {
+ // Outside csrf.Protect on purpose, with its own exact origin
+ // check: the directory's sign-out arrives same-site, not
+ // same-origin. See signoutPath.
+ r.With(a.credentialOnly).Post(signoutPath, a.handleCredentialSignout)
+ }
+
r.Group(func(r chi.Router) {
r.Use(a.credentialOnly)
// POST /signin is an unauthenticated endpoint that sends mail and makes
diff --git a/internal/home/directory.go b/internal/home/directory.go
index fefa080..fad22de 100644
--- a/internal/home/directory.go
+++ b/internal/home/directory.go
@@ -105,9 +105,12 @@ func (a *App) directoryRouter() http.Handler {
}
a.render(w, 200, "directory.html", map[string]any{"Apps": directoryCatalog(*a.runtime)})
})
- r.Post("/signout", func(w http.ResponseWriter, r *http.Request) {
+ // Sign out ends both sessions: this one here, then the id.*
+ // session by handing the POST on. See signoutPath.
+ r.Post(signoutPath, func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Cache-Control", "no-store")
a.sessions.SignOut(w, r)
- http.Redirect(w, r, "/signed-out", http.StatusSeeOther)
+ http.Redirect(w, r, a.origins.Credential+signoutPath, http.StatusTemporaryRedirect)
})
r.Get("/signed-out", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Cache-Control", "no-store")
diff --git a/internal/home/security.go b/internal/home/security.go
index caaf7e9..9165dce 100644
--- a/internal/home/security.go
+++ b/internal/home/security.go
@@ -13,6 +13,12 @@ func (a *App) SecurityPolicy() string {
for _, origin := range a.runtime.consumers {
origins[origin] = true
}
+ // form-action governs a form's redirects too, so the directory's
+ // sign-out, which 307s its POST on to the credential origin, is
+ // blocked in the browser without this. See signoutPath.
+ if a.runtime.role == "directory" {
+ origins[a.origins.Credential] = true
+ }
}
list := make([]string, 0, len(origins))
for origin := range origins {
diff --git a/internal/home/signout.go b/internal/home/signout.go
new file mode 100644
index 0000000..410b216
--- /dev/null
+++ b/internal/home/signout.go
@@ -0,0 +1,91 @@
+package home
+
+import (
+ "net/http"
+ "net/url"
+
+ "github.com/carlosframework/rastrillo/csrf"
+)
+
+// signoutPath is the same path on both origins. The directory's POST
+// /signout ends the directory session and answers 307 to the credential
+// origin's POST /signout, which ends the id.* session and lands the
+// browser on the directory's /signed-out.
+//
+// Both halves are needed. The directory session is only the app list;
+// the credential session is what /authorize reads, and it lives 30 days.
+// Ending just the first (as sign-out did until polish-audit B2) made
+// "You are signed out" a promise the next click broke: Sign in, from the
+// directory or from any app's "Sign in with Home", passed straight back
+// through /authorize with no prompt.
+//
+// 307 rather than 303 so the hop stays a POST. A GET sign-out is one any
+// page can trigger with a link or an <img>; a POST is not. See
+// handleCredentialSignout for how the credential side tells the
+// directory's hand-off from anyone else's POST.
+//
+// What this does NOT end: sessions an app instance minted from an
+// earlier assertion. Those are the instance's own sessions on its own
+// host, and the home keeps no list of where it has handed an assertion
+// (a hibernating fleet would not hear a revocation anyway). Each app's
+// own Sign out ends its session; what this guarantees is that no app can
+// get a NEW assertion for this browser without the email or passkey step.
+const signoutPath = "/signout"
+
+// handleCredentialSignout ends the credential-origin session.
+//
+// It sits outside csrf.Protect because the directory's hand-off is
+// same-site but not same-origin (Sec-Fetch-Site: same-site), which
+// Protect refuses. In its place is a narrower check, signoutFromHome.
+func (a *App) handleCredentialSignout(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Cache-Control", "no-store")
+ if !a.signoutFromHome(r) {
+ http.Error(w, "Sign-out was refused because the request did not come from Oficina.", http.StatusForbidden)
+ return
+ }
+ a.sessions.SignOut(w, r)
+ http.Redirect(w, r, a.origins.Directory+"/signed-out", http.StatusSeeOther)
+}
+
+// signoutFromHome admits a sign-out POST from this origin's own pages or
+// from the directory origin, and nothing else: not a team instance on the
+// same site, not any other origin, not a request with no evidence.
+//
+// Origin alone cannot carry the directory's hand-off. Chromium sends
+// "Origin: null" on a POST that a 307 has carried across origins
+// (verified in a real browser, polish-audit B2; the Fetch spec's tainted-
+// origin rule would keep the directory, but the browser does not), so the
+// directory's own hop arrives with a null Origin. Referer survives the
+// redirect, and under rastrillo's Referrer-Policy of
+// strict-origin-when-cross-origin it is exactly the directory origin. So:
+//
+// - Origin, when it names an origin, must be the directory's (or pass
+// the ordinary same-origin check for this one);
+// - a null or absent Origin falls back to Referer, whose origin must be
+// exactly the directory's, and then only on a request the browser
+// marks same-site (or does not mark at all).
+//
+// A page elsewhere cannot set Referer to the directory origin, and the
+// only directory route that re-sends a POST here is its own
+// CSRF-protected /signout. A browser that strips both headers is refused
+// and sees the refusal: the failure is loud, not a silent survival of the
+// session.
+func (a *App) signoutFromHome(r *http.Request) bool {
+ if csrf.SameOrigin(r, a.origins.Credential) {
+ return true
+ }
+ switch origin := r.Header.Get("Origin"); origin {
+ case a.origins.Directory:
+ return true
+ case "", "null":
+ default:
+ return false
+ }
+ switch r.Header.Get("Sec-Fetch-Site") {
+ case "", "same-site":
+ default:
+ return false
+ }
+ ref, err := url.Parse(r.Header.Get("Referer"))
+ return err == nil && ref.User == nil && ref.Scheme+"://"+ref.Host == a.origins.Directory
+}
diff --git a/internal/home/signout_test.go b/internal/home/signout_test.go
new file mode 100644
index 0000000..f9a0050
--- /dev/null
+++ b/internal/home/signout_test.go
@@ -0,0 +1,200 @@
+package home
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "strings"
+ "testing"
+
+ "amadan.net/oficina/home/internal/home/sealtest"
+)
+
+// signedInPair walks a full sign-in through both roles and returns the
+// credential-origin session and the directory session it produced.
+func signedInPair(t *testing.T, p exchangePair) (credential, directory *http.Cookie) {
+ t.Helper()
+ credential = signInFully(t, p.credential, sealtest.CanaryAddress)
+ pending, intent := p.start(t)
+ token, _ := p.mint(t, intent, credential)
+ return credential, p.finish(t, token, pending)
+}
+
+// resumeMints reports whether the credential origin would mint an
+// assertion for this session without asking anything: the silent
+// re-entry that made "Sign out" a lie (polish audit B2).
+func resumeMints(t *testing.T, p exchangePair, credential *http.Cookie) bool {
+ t.Helper()
+ _, intent := p.start(t)
+ rec := exchangeRequest(p.credential.App, "GET", Origin+"/authorize/resume", "", nil, intent, credential)
+ switch {
+ case rec.Code == 200 && strings.Contains(rec.Body.String(), `name="assertion"`):
+ return true
+ case rec.Code == 303 && strings.HasPrefix(rec.Header().Get("Location"), "/signin"):
+ return false
+ }
+ t.Fatalf("resume answered %d %q", rec.Code, rec.Header().Get("Location"))
+ return false
+}
+
+// signoutHop posts to the credential sign-out with the given evidence
+// headers; an empty value leaves that header out.
+func signoutHop(p exchangePair, origin, fetchSite, referer string, cookies ...*http.Cookie) *httptest.ResponseRecorder {
+ r := httptest.NewRequest("POST", Origin+"/signout", nil)
+ for k, v := range map[string]string{"Origin": origin, "Sec-Fetch-Site": fetchSite, "Referer": referer} {
+ if v != "" {
+ r.Header.Set(k, v)
+ }
+ }
+ for _, c := range cookies {
+ r.AddCookie(c)
+ }
+ rec := httptest.NewRecorder()
+ p.credential.Router().ServeHTTP(rec, r)
+ return rec
+}
+
+func clearsCookie(rec interface{ Result() *http.Response }, name string) bool {
+ for _, c := range rec.Result().Cookies() {
+ if c.Name == name && c.Value == "" && c.MaxAge < 0 {
+ return true
+ }
+ }
+ return false
+}
+
+// TestSignOutEndsTheCredentialSession is B2: signing out of the
+// directory must also end the id.* session, so the next Sign in asks for
+// the email (or passkey) step instead of bouncing straight back in.
+func TestSignOutEndsTheCredentialSession(t *testing.T) {
+ p := newExchangePair(t)
+ credential, directory := signedInPair(t, p)
+ if !resumeMints(t, p, credential) {
+ t.Fatal("precondition: a live credential session mints silently")
+ }
+
+ // The directory ends its own session, then hands the same POST on to
+ // the credential origin. 307, not 303: the method has to survive so
+ // the credential origin receives a POST it can origin-check, rather
+ // than a GET anyone could link to.
+ out := exchangeRequest(p.directory, "POST", directoryOrigin+"/signout", directoryOrigin, url.Values{}, directory)
+ if out.Code != http.StatusTemporaryRedirect || out.Header().Get("Location") != Origin+"/signout" {
+ t.Fatalf("directory sign-out: %d %q", out.Code, out.Header().Get("Location"))
+ }
+ if !clearsCookie(out, directory.Name) {
+ t.Fatal("directory sign-out left its cookie")
+ }
+ if page := exchangeRequest(p.directory, "GET", directoryOrigin+"/", "", nil, directory); strings.Contains(page.Body.String(), "You are signed in.") {
+ t.Fatal("directory session survived sign-out")
+ }
+
+ // The browser re-sends the POST to id.*. These are the headers
+ // Chromium really sends on that hop (captured live, polish-audit B2):
+ // the 307 nulls Origin, so the directory is named by Referer alone.
+ end := signoutHop(p, "null", "same-site", directoryOrigin+"/", credential)
+ if end.Code != http.StatusSeeOther || end.Header().Get("Location") != directoryOrigin+"/signed-out" {
+ t.Fatalf("credential sign-out: %d %q", end.Code, end.Header().Get("Location"))
+ }
+ if !clearsCookie(end, credential.Name) {
+ t.Fatal("credential sign-out left its cookie")
+ }
+ if end.Header().Get("Cache-Control") != "no-store" {
+ t.Fatal("sign-out response cacheable")
+ }
+
+ // The cookie is gone from the browser, but a copy of it must not work
+ // either: the row is revoked, not merely forgotten.
+ if resumeMints(t, p, credential) {
+ t.Fatal("credential session survived sign-out: Sign in re-enters without a prompt")
+ }
+ var rows int
+ if err := p.credential.db.Writer().QueryRow(`SELECT count(*) FROM sessions`).Scan(&rows); err != nil || rows != 0 {
+ t.Fatal("credential session row not revoked", rows, err)
+ }
+}
+
+// TestCredentialSignOutRefusesOtherOrigins: the credential sign-out is
+// outside csrf.Protect (the directory's hand-off arrives same-site, not
+// same-origin), so it carries its own exact origin check. Anything other
+// than the directory or the credential origin itself is refused and the
+// session survives, and a GET does nothing at all.
+func TestCredentialSignOutRefusesOtherOrigins(t *testing.T) {
+ p := newExchangePair(t)
+ credential, _ := signedInPair(t, p)
+ for _, c := range []struct{ origin, site, referer string }{
+ {"", "", ""},
+ {"null", "", ""},
+ {"null", "same-site", ""},
+ {"https://evil.invalid", "cross-site", directoryOrigin + "/"},
+ {"https://blue.docs.oficina.works", "same-site", ""},
+ {"https://blue.docs.oficina.works", "same-site", directoryOrigin + "/"},
+ {"http://oficina.works", "cross-site", ""},
+ {"https://oficina.works.evil.invalid", "cross-site", ""},
+ {"null", "same-site", "https://blue.docs.oficina.works/"},
+ {"null", "cross-site", directoryOrigin + "/"},
+ {"null", "same-site", "http://oficina.works/"},
+ {"null", "same-site", "https://oficina.works.evil.invalid/"},
+ {"null", "same-site", "https://user@oficina.works/"},
+ {"", "same-site", "https://evil.invalid/"},
+ } {
+ rec := signoutHop(p, c.origin, c.site, c.referer, credential)
+ if rec.Code != http.StatusForbidden || clearsCookie(rec, credential.Name) {
+ t.Fatalf("%+v: %d", c, rec.Code)
+ }
+ }
+ // Both shapes of the directory's own hand-off pass: Origin named (the
+ // Fetch spec's reading), or nulled with Referer (Chromium's).
+ for _, c := range []struct{ origin, site, referer string }{
+ {directoryOrigin, "same-site", ""},
+ {"null", "same-site", directoryOrigin + "/"},
+ {"", "", directoryOrigin + "/"},
+ } {
+ fresh, _ := signedInPair(t, p)
+ if rec := signoutHop(p, c.origin, c.site, c.referer, fresh); rec.Code != http.StatusSeeOther || resumeMints(t, p, fresh) {
+ t.Fatalf("directory hand-off %+v refused or left the session: %d", c, rec.Code)
+ }
+ }
+ if rec := exchangeRequest(p.credential.App, "GET", Origin+"/signout", "", nil, credential); rec.Code/100 == 3 || clearsCookie(rec, credential.Name) {
+ t.Fatal("GET signed out", rec.Code)
+ }
+ if !resumeMints(t, p, credential) {
+ t.Fatal("refused sign-out still ended the session")
+ }
+ // Its own origin may sign out too, for a future button on id.* pages.
+ if rec := exchangeRequest(p.credential.App, "POST", Origin+"/signout", Origin, url.Values{}, credential); rec.Code != http.StatusSeeOther {
+ t.Fatal("same-origin sign-out refused", rec.Code)
+ }
+ if resumeMints(t, p, credential) {
+ t.Fatal("same-origin sign-out left the session")
+ }
+ // Neither role answers the other's host: the credential sign-out is
+ // id.*-only, and the directory's is the directory's.
+ if rec := exchangeRequest(p.directory, "POST", Origin+"/signout", directoryOrigin, url.Values{}); rec.Code != 404 {
+ t.Fatal("directory served credential sign-out", rec.Code)
+ }
+ if rec := exchangeRequest(p.credential.App, "POST", directoryOrigin+"/signout", directoryOrigin, url.Values{}); rec.Code != 404 {
+ t.Fatal("credential served directory sign-out", rec.Code)
+ }
+}
+
+// TestDirectoryPolicyAdmitsTheSignOutHandOff: CSP form-action is enforced
+// across a form's redirects, so without the credential origin in the
+// directory's form-action the 307 is blocked in the browser and B2 comes
+// back with nothing on screen but a console error.
+func TestDirectoryPolicyAdmitsTheSignOutHandOff(t *testing.T) {
+ p := newExchangePair(t)
+ policy := p.directory.SecurityPolicy()
+ var formAction string
+ for _, d := range strings.Split(policy, ";") {
+ if d = strings.TrimSpace(d); strings.HasPrefix(d, "form-action ") {
+ formAction = d
+ }
+ }
+ found := false
+ for _, source := range strings.Fields(formAction) {
+ found = found || source == Origin
+ }
+ if !found {
+ t.Fatalf("directory form-action omits %s: %q", Origin, formAction)
+ }
+}