amadan

oficina / home Public

Render validated Docs team selection as scriptless continuation

Paul Campbell pushed by paul@keymail.dev 670db3122c3257350983d2c1542338ff33c11347 parent 3192862
7 files changed, +172 −9
  • internal/home/directorydata/store.go +5 −2
  • internal/home/directorydata/store_test.go +2 −2
  • internal/home/docs_chooser.go +12 −1
  • internal/home/docs_chooser_test.go +2 −4
  • internal/home/docs_continue_test.go +144 −0
  • internal/home/templates/docs_continue.html +6 −0
  • internal/home/templates/layout.html +1 −0

CI passed — run details

Intent

full ledger: feat/chooser-continuation

diff --git a/internal/home/directorydata/store.go b/internal/home/directorydata/store.go
index f5092a0..c7bb31b 100644
--- a/internal/home/directorydata/store.go
+++ b/internal/home/directorydata/store.go
@@ -145,7 +145,7 @@ func (s *Store) Choices(ctx context.Context, subject, app string) ([]Choice, err
continue
}
route := Route{TeamID: c.ID, TeamSQID: c.SQID, Label: c.Label, App: c.App, Origin: c.Origin, ConsumerID: c.ConsumerID}
- if !validOrigin(c.Origin) || !validConsumer(c.ConsumerID) || !s.validateRoute(route) {
+ if !ValidOrigin(c.Origin) || !validConsumer(c.ConsumerID) || !s.validateRoute(route) {
return nil, ErrUnavailable
}
if err = s.openTeam(&c.Team, sealed); err != nil {
@@ -216,7 +216,10 @@ func validName(s string) bool {
}
return true
}
-func validOrigin(s string) bool {
+
+// ValidOrigin accepts only canonical HTTPS DNS origins without ports or URL components.
+// It checks syntax only; callers must also apply their provisioning policy.
+func ValidOrigin(s string) bool {
u, err := url.Parse(s)
if err != nil || u.Scheme != "https" || u.Host == "" || u.User != nil || u.Path != "" || u.RawQuery != "" || u.Fragment != "" || u.ForceQuery || u.Opaque != "" || u.Port() != "" || u.Host != strings.ToLower(u.Host) {
return false
diff --git a/internal/home/directorydata/store_test.go b/internal/home/directorydata/store_test.go
index b512f0d..c84b7bb 100644
--- a/internal/home/directorydata/store_test.go
+++ b/internal/home/directorydata/store_test.go
@@ -221,11 +221,11 @@ func TestUnsafeStoredRouteRefusesEntireList(t *testing.T) {
func TestInputAndErrorPrivacy(t *testing.T) {
s, _, _ := fixture(t)
for _, origin := range []string{"http://alpha.example", "https://user@example.com", "https://example.com/", "https://example.com?", "https://EXAMPLE.com", "https://example.com:443", "https://example.com#x", "https://example.com.", "https://localhost", "https://a..example"} {
- if validOrigin(origin) {
+ if ValidOrigin(origin) {
t.Errorf("accepted %q", origin)
}
}
- if !validOrigin("https://alpha.docs.oficina.works") {
+ if !ValidOrigin("https://alpha.docs.oficina.works") {
t.Fatal("valid origin refused")
}
for _, cfg := range []Config{{}, {Apps: []string{"docs"}}, {Apps: []string{"docs", "docs"}, ValidateRoute: func(Route) bool { return true }}, {Apps: []string{"bad/app"}, ValidateRoute: func(Route) bool { return true }}} {
diff --git a/internal/home/docs_chooser.go b/internal/home/docs_chooser.go
index fca4bb0..b664a5f 100644
--- a/internal/home/docs_chooser.go
+++ b/internal/home/docs_chooser.go
@@ -107,7 +107,7 @@ func (c *docsChooser) choose(w http.ResponseWriter, r *http.Request) {
}
for _, choice := range choices {
if choice.SQID == form.Get("team") {
- c.redirect(w, r, choice)
+ c.continueToDocs(w, choice)
return
}
}
@@ -133,3 +133,14 @@ func (c *docsChooser) redirect(w http.ResponseWriter, r *http.Request, choice di
// policy. Generic entry carries only the app root, never a document path.
http.Redirect(w, r, choice.Origin+handoff.StartPath+"?"+url.Values{"return_to": {"/"}}.Encode(), http.StatusSeeOther)
}
+
+func (c *docsChooser) continueToDocs(w http.ResponseWriter, choice directorydata.Choice) {
+ if !directorydata.ValidOrigin(choice.Origin) {
+ c.unavailable(w)
+ return
+ }
+ destination := choice.Origin + handoff.StartPath + "?" + url.Values{"return_to": {"/"}}.Encode()
+ w.Header().Set("Cache-Control", "no-store")
+ w.Header().Set("Referrer-Policy", "no-referrer")
+ c.app.render(w, http.StatusOK, "docs_continue.html", struct{ Destination string }{destination})
+}
diff --git a/internal/home/docs_chooser_test.go b/internal/home/docs_chooser_test.go
index eb76b4a..5c06fa2 100644
--- a/internal/home/docs_chooser_test.go
+++ b/internal/home/docs_chooser_test.go
@@ -144,9 +144,7 @@ func TestDocsChooserZeroOneManyAndRemovedChoice(t *testing.T) {
}
r = f.request("POST", docsChoicePath, directoryOrigin, "team=B2", cookie)
noStore(t, r)
- if r.Code != 303 || r.Header().Get("Location") != "https://beta.docs.oficina.works/auth/start?return_to=%2F" {
- t.Fatal(r.Code, r.Header())
- }
+ assertDocsContinuation(t, r, "https://beta.docs.oficina.works/auth/start?return_to=%2F")
if _, err := f.p.directory.db.Writer().Exec(`DELETE FROM memberships WHERE team_id=? AND person_ref=?`, a, chooserPerson); err != nil {
t.Fatal(err)
}
@@ -208,7 +206,7 @@ func TestDocsChooserRefusesHostileInputAndCrossOriginSelection(t *testing.T) {
t.Fatal(body, r.Code)
}
}
- for _, origin := range []string{"https://attacker.example", Origin} {
+ for _, origin := range []string{"", "https://attacker.example", Origin} {
r := f.request("POST", docsChoicePath, origin, "team=A1", cookie)
noStore(t, r)
if r.Code != 403 {
diff --git a/internal/home/docs_continue_test.go b/internal/home/docs_continue_test.go
new file mode 100644
index 0000000..8b7b237
--- /dev/null
+++ b/internal/home/docs_continue_test.go
@@ -0,0 +1,144 @@
+package home
+
+import (
+ "bytes"
+ "context"
+ "encoding/xml"
+ "io"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+
+ "amadan.net/oficina/home/internal/home/directorydata"
+)
+
+func continuationTargets(t *testing.T, body string) (refresh, links []string) {
+ t.Helper()
+ d := xml.NewDecoder(strings.NewReader(body))
+ d.Strict = false
+ d.AutoClose = xml.HTMLAutoClose
+ d.Entity = xml.HTMLEntity
+ for {
+ token, err := d.Token()
+ if err == io.EOF {
+ break
+ }
+ if err != nil {
+ t.Fatal(err)
+ }
+ el, ok := token.(xml.StartElement)
+ if !ok {
+ continue
+ }
+ a := map[string]string{}
+ for _, v := range el.Attr {
+ a[v.Name.Local] = v.Value
+ }
+ if el.Name.Local == "meta" && strings.EqualFold(a["http-equiv"], "refresh") {
+ refresh = append(refresh, a["content"])
+ }
+ if el.Name.Local == "a" {
+ if a["referrerpolicy"] != "no-referrer" {
+ t.Fatal("anchor sends referrer")
+ }
+ links = append(links, a["href"])
+ }
+ }
+ return
+}
+func assertDocsContinuation(t *testing.T, r *httptest.ResponseRecorder, want string) {
+ t.Helper()
+ noStore(t, r)
+ if r.Code != 200 || r.Header().Get("Location") != "" || r.Header().Get("Refresh") != "" || r.Header().Get("Content-Type") != "text/html; charset=utf-8" || r.Header().Get("Referrer-Policy") != "no-referrer" {
+ t.Fatal(r.Code, r.Header())
+ }
+ refresh, links := continuationTargets(t, r.Body.String())
+ if len(refresh) != 1 || refresh[0] != "0;url="+want || len(links) != 1 || links[0] != want || strings.Contains(r.Body.String(), "ZgotmplZ") || strings.Contains(r.Body.String(), "<script") {
+ t.Fatal(refresh, links, r.Body.String())
+ }
+}
+func TestDocsContinueCanonicalResponseBoundary(t *testing.T) {
+ f := newChooserFixture(t)
+ c, err := f.p.directory.newDocsChooser(f.store)
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, origin := range []string{"https://alpha.docs.oficina.works", "https://approved.fixture.example"} {
+ r := httptest.NewRecorder()
+ c.continueToDocs(r, directorydata.Choice{Origin: origin})
+ assertDocsContinuation(t, r, origin+"/auth/start?return_to=%2F")
+ }
+ for _, origin := range []string{"", "http://alpha.example", "https://Alpha.example", "https://alpha.example/", "https://alpha.example:443", "https://user@alpha.example", "https://alpha.example?x=1", "https://alpha.example#f", "https://alpha.example?", "https://alpha.example\n", " https://alpha.example", "https://alpha.example\";url=https://bad.example", "javascript:alert(1)", "//alpha.example", "https://[::1]", "https://alpha.example/%22"} {
+ r := httptest.NewRecorder()
+ c.continueToDocs(r, directorydata.Choice{Origin: origin})
+ if r.Code != 503 || strings.Contains(r.Body.String(), "http-equiv") || r.Header().Get("Location") != "" {
+ t.Fatal(origin, r.Code, r.Body.String())
+ }
+ }
+}
+func TestDocsContinueRendererEscapesIdenticalTargets(t *testing.T) {
+ f := newChooserFixture(t)
+ // Exercise real template contexts independently of the fixed runtime URL.
+ want := "https://alpha.docs.oficina.works/auth/start?return_to=%2F&probe=%22%3C"
+ r := httptest.NewRecorder()
+ f.p.directory.render(r, 200, "docs_continue.html", struct{ Destination string }{want})
+ refresh, links := continuationTargets(t, r.Body.String())
+ if len(refresh) != 1 || refresh[0] != "0;url="+want || len(links) != 1 || links[0] != want || !strings.Contains(r.Body.String(), "&amp;probe=") {
+ t.Fatal(refresh, links, r.Body.String())
+ }
+ for name, page := range f.p.directory.renderer.pages {
+ if name == "docs_continue.html" {
+ continue
+ }
+ var b bytes.Buffer
+ _ = page.ExecuteTemplate(&b, "layout", map[string]any{})
+ if strings.Contains(strings.ToLower(b.String()), `http-equiv="refresh"`) {
+ t.Fatalf("refresh leaked to %s", name)
+ }
+ }
+}
+func TestDocsContinueRequeriesCurrentOrigin(t *testing.T) {
+ f := newChooserFixture(t)
+ cookie := f.session(t, chooserPerson)
+ id := f.team(t, "A1", "alpha", "Alpha", chooserPerson)
+ f.team(t, "B2", "beta", "Beta", chooserPerson)
+ if r := f.request("GET", docsEntryPath, "", "", cookie); r.Code != 200 {
+ t.Fatal(r.Code)
+ }
+ if _, err := f.p.directory.db.Writer().Exec(`UPDATE teams SET label='changed' WHERE id=?`, id); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := f.p.directory.db.Writer().Exec(`UPDATE team_instances SET origin='https://changed.docs.oficina.works' WHERE team_id=?`, id); err != nil {
+ t.Fatal(err)
+ }
+ assertDocsContinuation(t, f.request("POST", docsChoicePath, directoryOrigin, "team=A1", cookie), "https://changed.docs.oficina.works/auth/start?return_to=%2F")
+}
+func TestDocsContinueExactHostAndExpiredSession(t *testing.T) {
+ f := newChooserFixture(t)
+ cookie := f.session(t, chooserPerson)
+ f.team(t, "A1", "alpha", "Alpha", chooserPerson)
+ for _, method := range []string{"GET", "POST"} {
+ path := docsEntryPath
+ if method == "POST" {
+ path = docsChoicePath
+ }
+ r := httptest.NewRequest(method, directoryOrigin+path, strings.NewReader("team=A1"))
+ r.Host = "wrong.example"
+ r.AddCookie(cookie)
+ r.Header.Set("Origin", directoryOrigin)
+ r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+ w := httptest.NewRecorder()
+ f.handler.ServeHTTP(w, r)
+ if w.Code != 404 {
+ t.Fatal(method, w.Code)
+ }
+ }
+ if _, err := f.p.directory.db.Writer().ExecContext(context.Background(), `UPDATE sessions SET expires_at='2000-01-01T00:00:00Z'`); err != nil {
+ t.Fatal(err)
+ }
+ r := f.request(http.MethodPost, docsChoicePath, directoryOrigin, "team=A1", cookie)
+ if r.Code != 303 || r.Header().Get("Location") != "/auth/start?return_to=%2Fapps%2Fdocs" {
+ t.Fatal(r.Code, r.Header())
+ }
+}
diff --git a/internal/home/templates/docs_continue.html b/internal/home/templates/docs_continue.html
new file mode 100644
index 0000000..ff03281
--- /dev/null
+++ b/internal/home/templates/docs_continue.html
@@ -0,0 +1,6 @@
+{{define "title"}}Continue to Docs{{end}}
+{{define "head"}}<meta http-equiv="refresh" content="0;url={{.Destination}}">{{end}}
+{{define "content"}}
+<h1>Continue to Docs</h1>
+<p><a href="{{.Destination}}" referrerpolicy="no-referrer">Continue to Docs</a></p>
+{{end}}
diff --git a/internal/home/templates/layout.html b/internal/home/templates/layout.html
index 9340be0..6054667 100644
--- a/internal/home/templates/layout.html
+++ b/internal/home/templates/layout.html
@@ -6,6 +6,7 @@
<title>{{block "title" .}}Oficina{{end}}</title>
<link rel="stylesheet" href="/static/tokens.css">
<link rel="stylesheet" href="/static/theme.css">
+{{block "head" .}}{{end}}
</head>
<body{{block "bodyattrs" .}}{{end}}>
<main rst-page>