amadan
Merge branch 'front-door' into main
4 files changed,
+62
−6
internal/home/directory.go+23 −1internal/home/directory_catalog_test.go+29 −2internal/home/exchange_test.go+9 −2internal/home/static/site/index.html+1 −1
CI passed — run details
diff --git a/internal/home/directory.go b/internal/home/directory.go| index b9b04a1..fefa080 100644 |
| --- a/internal/home/directory.go |
| +++ b/internal/home/directory.go |
| @@ -80,8 +80,29 @@ func (a *App) directoryRouter() http.Handler { |
| r.Get("/signin", func(w http.ResponseWriter, r *http.Request) { |
| http.Redirect(w, r, handoff.StartPath, http.StatusSeeOther) |
| }) |
| - r.With(a.sessions.Require).Get("/", func(w http.ResponseWriter, r *http.Request) { |
| + // One URL, two audiences. A visitor with no session gets the front |
| + // page — what Oficina is, the apps, and the way in. A signed-in |
| + // person gets their directory. Gating this behind sessions.Require |
| + // instead, as it was, sends every stranger who types oficina.works |
| + // to a sign-in form: a door with no building attached to it. |
| + // |
| + // The session is READ here, never required, so an expired cookie |
| + // degrades to the front page rather than to a redirect loop. |
| + r.Get("/", func(w http.ResponseWriter, r *http.Request) { |
| + // no-store covers BOTH branches, and it is set before either |
| + // runs. One URL now answers two different bodies keyed on a |
| + // cookie, so a shared cache that stored the anonymous 200 |
| + // would hand the front page to signed-in people. Vary: Cookie |
| + // says the same thing to a cache that honours it; no-store is |
| + // the half that does not depend on it honouring anything. |
| w.Header().Set("Cache-Control", "no-store") |
| + w.Header().Add("Vary", "Cookie") |
| + // Current reads what Middleware already resolved for this |
| + // request, rather than asking the database a second time. |
| + if _, ok := sessions.Current(r); !ok { |
| + a.handleIndex(w, r) |
| + return |
| + } |
| a.render(w, 200, "directory.html", map[string]any{"Apps": directoryCatalog(*a.runtime)}) |
| }) |
| r.Post("/signout", func(w http.ResponseWriter, r *http.Request) { |
| @@ -96,6 +117,7 @@ func (a *App) directoryRouter() http.Handler { |
| r.Get("/static/tokens.css", serveAsset(staticAsset("tokens.css"), "text/css; charset=utf-8")) |
| r.Get("/static/theme.css", serveAsset(theme.CSS(), "text/css; charset=utf-8")) |
| mountBrand(r) |
| + mountSite(r) |
| }) |
| return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { |
| if req.URL.Path == rosterPath { |
diff --git a/internal/home/directory_catalog_test.go b/internal/home/directory_catalog_test.go| index 7727bc8..54a0682 100644 |
| --- a/internal/home/directory_catalog_test.go |
| +++ b/internal/home/directory_catalog_test.go |
| @@ -1,16 +1,43 @@ |
| package home |
| import ( |
| + "net/http" |
| "strings" |
| "testing" |
| ) |
| func TestDirectoryCatalogRequiresSigninAndDoesNotInventLaunchDestinations(t *testing.T) { |
| f := newMountedChooser(t) |
| + // "/" answers the front page to a visitor with no session, and the |
| + // catalogue only to a session that exists. The property under test is |
| + // unchanged and is the second condition: the catalogue — team choosers, |
| + // per-app entry paths — must never reach an anonymous request. What |
| + // changed is the mechanism. This used to 303 to /auth/start, which sent |
| + // every stranger who typed oficina.works to a sign-in form; it now |
| + // renders the front page, so the assertion is on CONTENT rather than on |
| + // a status code, which is the thing that actually matters. |
| anonymous := f.request("GET", "/", "", "", nil) |
| - if anonymous.Code != 303 || !strings.HasPrefix(anonymous.Header().Get("Location"), "/auth/start") || strings.Contains(anonymous.Body.String(), "Choose a team") { |
| - t.Fatal("anonymous directory exposed catalog", anonymous.Code, anonymous.Header()) |
| + anonymousBody := anonymous.Body.String() |
| + if anonymous.Code != 200 || !strings.Contains(anonymousBody, "office suite") { |
| + t.Fatal("anonymous directory did not serve the front page", anonymous.Code, anonymous.Header()) |
| } |
| + for _, leak := range []string{"Choose a team", `href="/apps/`, docsEntryPath} { |
| + if strings.Contains(anonymousBody, leak) { |
| + t.Fatal("anonymous front page exposed catalogue", leak) |
| + } |
| + } |
| + noStore(t, anonymous) |
| + if len(anonymous.Result().Cookies()) != 0 { |
| + t.Fatal("front page set a cookie on an anonymous request") |
| + } |
| + // A cookie that does not resolve must degrade to the front page, not to |
| + // a redirect loop and not to a partial catalogue render. |
| + tampered := f.request("GET", "/", "", "", &http.Cookie{Name: f.p.directory.sessions.CookieName(), Value: "not-a-token"}) |
| + if tampered.Code != 200 || !strings.Contains(tampered.Body.String(), "office suite") || |
| + strings.Contains(tampered.Body.String(), "Choose a team") { |
| + t.Fatal("tampered cookie did not degrade to the front page", tampered.Code) |
| + } |
| + |
| cookie := f.session(t, chooserPerson) |
| page := f.request("GET", "/", "", "", cookie) |
| if page.Code != 200 { |
diff --git a/internal/home/exchange_test.go b/internal/home/exchange_test.go| index d87aee9..f20cf79 100644 |
| --- a/internal/home/exchange_test.go |
| +++ b/internal/home/exchange_test.go |
| @@ -164,8 +164,15 @@ func TestHomeDirectoryExchangeAndPrivacy(t *testing.T) { |
| if subject == "" || strings.Contains(subject, "@") || authTime != old.Format(time.RFC3339) { |
| t.Fatal("identity or freshness changed") |
| } |
| - if rec := exchangeRequest(p.directory, "GET", directoryOrigin+"/", "", nil, credentialSession); rec.Code != 303 { |
| - t.Fatal("credential cookie opened directory") |
| + // A cookie minted on the credential origin must not carry a directory |
| + // session. "/" no longer redirects an unauthenticated caller — it |
| + // renders the front page — so the check is that the credential cookie |
| + // gets that anonymous page and none of the signed-in directory, which |
| + // asserts the same separation against the response body instead of |
| + // against a redirect that is no longer how the route reports it. |
| + if rec := exchangeRequest(p.directory, "GET", directoryOrigin+"/", "", nil, credentialSession); rec.Code != 200 || |
| + strings.Contains(rec.Body.String(), "You are signed in.") || strings.Contains(rec.Body.String(), "Choose a team") { |
| + t.Fatal("credential cookie opened directory", rec.Code) |
| } |
| if rec := exchangeRequest(p.credential.App, "GET", Origin+"/authorize/resume", "", nil, intent, directorySession); rec.Code != 303 { |
| t.Fatal("directory cookie opened credentials") |
diff --git a/internal/home/static/site/index.html b/internal/home/static/site/index.html| index 0826a32..8eadc30 100644 |
| --- a/internal/home/static/site/index.html |
| +++ b/internal/home/static/site/index.html |
| @@ -237,7 +237,7 @@ |
| <li><a href="https://amadan.net/oficina/docs/-/releases/v0.1.0">Releases</a></li> |
| </ul> |
| </div> |
| - <p class="colophon">This page is one HTML file, one stylesheet and two fonts served |
| + <p class="colophon">This page is one HTML file, one stylesheet and two typefaces served |
| from our own machines. It runs no scripts, sets no cookies and calls nothing |
| third-party — which is why there is no banner asking your permission.</p> |
| </div> |