oficina / docs Public

Merge remote-tracking branch 'origin/main' into chore/door-copy

# Conflicts:
#	internal/docs/render.go

Paul Campbell pushed by paul@keymail.dev 2 weeks ago a244d547774e9dce03b76923060df94f2a4ae576 parent b241b77parent cc1bd3c

6 files changed, +88 −8

internal/docs/home_router_integration_test.go +1 −1
internal/docs/home_routes.go +20 −4
internal/docs/home_signout_chain_test.go +62 −0
internal/docs/native_stream_test.go +3 −1
internal/docs/team_router_test.go +1 −1
internal/docs/team_stream_test.go +1 −1

Intent

full ledger: chore/door-copy

diff --git a/internal/docs/home_router_integration_test.go b/internal/docs/home_router_integration_test.go

index 3d8fb31..6f83458 100644

--- a/internal/docs/home_router_integration_test.go

+++ b/internal/docs/home_router_integration_test.go

@@ -279,7 +279,7 @@ func TestHomeRouterExistingAccountRoundTripPreservesOwnedAndSharedContent(t *tes

if rec := b.do("GET", resource, ""); rec.Code != 200 {

t.Fatal("linked local account cannot read original document", rec.Code)

}

- routerRedirect(t, b.do("POST", "/signout", ""), "/signed-out")

+ signoutChained(t, b.do("POST", "/signout", ""))

signedOut := b.do("GET", "/signed-out", "")

if signedOut.Code != http.StatusOK || signedOut.Header().Get("Location") != "" || !strings.Contains(signedOut.Body.String(), `href="/signin"`) {

t.Fatal("signout automatically resumed Home authentication")

diff --git a/internal/docs/home_routes.go b/internal/docs/home_routes.go

index 188a490..496c626 100644

--- a/internal/docs/home_routes.go

+++ b/internal/docs/home_routes.go

@@ -21,6 +21,7 @@ type homeRoutes struct {

navigation *localLinkNavigation

handlers *handlers

formPolicy string

+ issuer string

}

func newHomeRoutes(settings *homeSettings, origin string, db *sql.DB, auth *rauth.Auth, h *handlers) (*homeRoutes, error) {

@@ -37,7 +38,7 @@ func newHomeRoutes(settings *homeSettings, origin string, db *sql.DB, auth *raut

return nil, errHomeConfig

}

b.returnPath = navigation.returnPath

- return &homeRoutes{bridge: b, navigation: navigation, handlers: h, formPolicy: homeFormPolicy(settings.issuer)}, nil

+ return &homeRoutes{bridge: b, navigation: navigation, handlers: h, formPolicy: homeFormPolicy(settings.issuer), issuer: settings.issuer}, nil

}

func cookieCount(r *http.Request, name string) int {

@@ -174,13 +175,28 @@ func (h *handlers) homeLinkLabel(r *http.Request) string {

return rastrillo.T(r, "docs.home.menu_action")

}

-// Docs sign-out stops here even while Home retains its own session. Returning

-// directly to /signin would immediately start another Home exchange.

+// signout ends the Docs session and clears the hand-off cookies, then hands

+// the same POST on to {issuer}/signout, which ends the id.* session too and

+// returns the browser to this origin's /signed-out (spec shared-chrome §9.5;

+// connect.Routes.Signout does the same for every other app). Ending only the

+// Docs session left "Sign in" on the signed-out page passing silently

+// through the live 30-day id.* session.

+//

+// - 307, not 303, so the hop stays a POST. id.* ignores a GET sign-out,

+// because any link or <img> can trigger one.

+// - The browser sends "Origin: null" on the redirected POST, so id.* admits

+// it by Referer. strict-origin-when-cross-origin makes the Referer

+// exactly this origin, which id.* checks against HOME_CONSUMERS.

+// - homeFormPolicy already names the issuer in form-action, which browsers

+// enforce on the redirect target of a form submission.

+//

+// /signed-out itself still never starts another exchange.

func (h *homeRoutes) signout(w http.ResponseWriter, r *http.Request) {

h.bridge.sessions.SignOut(w, r)

for _, name := range []string{homeInitCookie, homeProofCookie, localLinkNavigationCookie, homeReturnCookie, "__Host-oficina-assertion-pending", "__Host-oficina-assertion-inbox"} {

localNavigationCookie(w, name, "", -1)

}

w.Header().Set("Cache-Control", "no-store")

- http.Redirect(w, r, "/signed-out", http.StatusSeeOther)

+ w.Header().Set("Referrer-Policy", "strict-origin-when-cross-origin")

+ http.Redirect(w, r, h.issuer+"/signout", http.StatusTemporaryRedirect)

}

diff --git a/internal/docs/home_signout_chain_test.go b/internal/docs/home_signout_chain_test.go

new file mode 100644

index 0000000..7485f0e

--- /dev/null

+++ b/internal/docs/home_signout_chain_test.go

@@ -0,0 +1,62 @@

+package docs

+import (

+ "net/http"

+ "net/http/httptest"

+ "testing"

+// TestHomeSignoutChainsToIssuer: in Home mode, Docs' sign-out ends the Docs

+// session and then hands the same POST on to {issuer}/signout, so the id.*

+// session ends too and the next "Sign in" asks again (spec shared-chrome

+// §9.5; the same chain connect.Routes.Signout gives every other app).

+// Ending only the Docs session left "Sign in" on the signed-out page

+// passing silently through the live id.* session.

+//

+// 307, not 303, so the hop stays a POST: id.* ignores a GET sign-out. The

+// browser sends "Origin: null" on the redirected POST, so id.* admits it by

+// Referer, which strict-origin-when-cross-origin makes exactly this origin.

+func TestHomeSignoutChainsToIssuer(t *testing.T) {

+ f := newRouterHomeFixture(t)

+ b := f.browser()

+ b.localSignin(t, "alice@example.com")

+ cookie := b.cookies[f.sessions.CookieName()]

+ if cookie == nil {

+ t.Fatal("local sign-in set no session")

+ }

+ rec := b.do("POST", "/signout", "")

+ if rec.Code != http.StatusTemporaryRedirect || rec.Header().Get("Location") != bridgeIssuer+"/signout" {

+ t.Fatalf("POST /signout: status %d Location %q, want 307 to %s/signout", rec.Code, rec.Header().Get("Location"), bridgeIssuer)

+ }

+ if got := rec.Header().Get("Referrer-Policy"); got != "strict-origin-when-cross-origin" {

+ t.Fatalf("Referrer-Policy %q: id.* admits the null-Origin hop by Referer", got)

+ }

+ if got := rec.Header().Get("Cache-Control"); got != "no-store" {

+ t.Fatalf("Cache-Control %q, want no-store", got)

+ }

+ if b.cookies[f.sessions.CookieName()] != nil {

+ t.Fatal("sign-out kept the Docs session cookie")

+ }

+ old := httptest.NewRequest("GET", bridgeOrigin+"/", nil)

+ old.AddCookie(cookie)

+ oldRec := httptest.NewRecorder()

+ f.app.Handler().ServeHTTP(oldRec, old)

+ if oldRec.Code != http.StatusSeeOther {

+ t.Fatalf("revoked session still served / (status %d)", oldRec.Code)

+ }

+ // id.* returns the browser to /signed-out, which must still render and

+ // must not start another exchange by itself.

+ signedOut := b.do("GET", "/signed-out", "")

+ if signedOut.Code != http.StatusOK || signedOut.Header().Get("Location") != "" {

+ t.Fatalf("GET /signed-out: status %d Location %q", signedOut.Code, signedOut.Header().Get("Location"))

+ }

+// signoutChained asserts the Home-mode sign-out response: a 307 to the

+// issuer's /signout. The browser then returns to /signed-out.

+func signoutChained(t *testing.T, rec *httptest.ResponseRecorder) {

+ t.Helper()

+ if rec.Code != http.StatusTemporaryRedirect || rec.Header().Get("Location") != bridgeIssuer+"/signout" {

+ t.Fatalf("POST /signout: status %d Location %q, want 307 to %s/signout", rec.Code, rec.Header().Get("Location"), bridgeIssuer)

+ }

diff --git a/internal/docs/native_stream_test.go b/internal/docs/native_stream_test.go

index d65e355..b8cfad6 100644

--- a/internal/docs/native_stream_test.go

+++ b/internal/docs/native_stream_test.go

@@ -62,7 +62,9 @@ func assertNativeStreamRevoked(t *testing.T, app *App, address string, authentic

}

signedOut := httptest.NewRecorder()

app.Handler().ServeHTTP(signedOut, signout)

- if signedOut.Code != http.StatusSeeOther {

+ // 303 to /signed-out standalone; 307 on to the issuer's /signout in

+ // Home mode (homeRoutes.signout). Both have ended the Docs session.

+ if signedOut.Code != http.StatusSeeOther && signedOut.Code != http.StatusTemporaryRedirect {

t.Fatalf("signout failed: %d", signedOut.Code)

}

closed := make(chan error, 1)

diff --git a/internal/docs/team_router_test.go b/internal/docs/team_router_test.go

index 851939e..89d88ab 100644

--- a/internal/docs/team_router_test.go

+++ b/internal/docs/team_router_test.go

@@ -243,7 +243,7 @@ func TestTeamRouterLocalLinkMembershipRemovalAndIndependentGrant(t *testing.T) {

if code := b.update(f.shared, validUpdate(t)); code != 204 {

t.Fatal("writer grant depended on roster availability", code)

}

- routerRedirect(t, b.do("POST", "/signout", ""), "/signed-out")

+ signoutChained(t, b.do("POST", "/signout", ""))

nonce := b.start(t, "/auth/start?return_to=%2F", "")

authTime := time.Now().Add(-2 * time.Hour).Truncate(time.Second)

routerRedirect(t, b.exchange(t, nonce, teamSubject('a'), authTime), "/")

diff --git a/internal/docs/team_stream_test.go b/internal/docs/team_stream_test.go

index 13bfea1..86d678a 100644

--- a/internal/docs/team_stream_test.go

+++ b/internal/docs/team_stream_test.go

@@ -139,7 +139,7 @@ func TestTeamStreamGuestGrantAndActualSessionRevocation(t *testing.T) {

if reason == "grant" {

routerRedirect(t, owner.do("POST", "/d/"+f.shared.Sqid+"/share/revoke", url.Values{"address": {"alice@example.com"}}.Encode()), "/d/"+f.shared.Sqid+"/share")

} else {

- routerRedirect(t, guest.do("POST", "/signout", ""), "/signed-out")

+ signoutChained(t, guest.do("POST", "/signout", ""))

}

done := make(chan struct{})

go func() { io.Copy(io.Discard, res.Body); close(done) }()