From 5719e51eb3dce09b2a9bfeddde7e8202c0a606cc Mon Sep 17 00:00:00 2001 From: bigbes Date: Sun, 9 Aug 2026 00:17:57 +0300 Subject: [PATCH] login: ecore's cookie decoder, which validates the name ours did not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit authz/identity.go was the fifth copy of a decode the instance has one of, and it was one of the two that validated nothing: whatever name a sealed payload carried went on to the GraphQL viewer field, the chrome's nav and every log line the request produced. login.ValidName refuses it, and there is no spelling of the API that turns the check off. Gone with it: CookieName, UsernameFromRequest, Middleware, ForContext and the ctxKey, plus identity_test.go, whose round-trip, tilde-stripping and garbage-cookie cases are ecore's tests now. The Middleware becomes login.Optional() — this service refuses nobody, git.sr.ht decides what an anonymous viewer may see — and the default validator is kept rather than core.ValidOwner, which is lowercase-only and would log a real account out of compare alone. The one behaviour change a viewer could notice: a cookie whose name carries a '/', a control byte or a non-ASCII letter now reads as anonymous instead of as that name. --- README.md | 6 ++- authz/authz_test.go | 9 ++++ authz/doc.go | 13 +++--- authz/identity.go | 63 -------------------------- authz/identity_test.go | 98 ----------------------------------------- cmd/comparesrht/main.go | 8 +++- docs/inline-comments.md | 5 ++- web/handlers.go | 5 ++- web/server.go | 20 ++++++--- web/web_test.go | 14 +++--- 10 files changed, 53 insertions(+), 188 deletions(-) delete mode 100644 authz/identity.go delete mode 100644 authz/identity_test.go diff --git a/README.md b/README.md index 17141be84d6261c8aeedeae6ae5644a6d940f204..90f78026dbf043185a6e58d2966ecf1bfcba726c 100644 --- a/README.md +++ b/README.md @@ -43,8 +43,10 @@ authorizer spares git.sr.ht a GraphQL call on every page load. sentinel errors. No external dependencies. - `gitx/` — bare-repo access over go-git: refs, ref-to-ref diffs, single-commit diffs, and commit logs, all bounded by context timeouts and output-size caps. -- `authz/` — cookie→identity and the git.sr.ht GraphQL authorizer with a short - TTL cache. +- `authz/` — the git.sr.ht GraphQL authorizer with a short TTL cache. Identity + is not here any more: the unified-login cookie is decoded by [sr-ht-ecore]'s + `login`, the instance's one copy of that decode, which — unlike the local one + it replaced — refuses a name that could not be a username. - `web/` — chi router, handlers, Go templates, embedded static assets. Most of the machinery around them is not here but in [sr-ht-ecore], wired up in `web/server.go`: `chrome` for the page frame, `pages` for template discovery diff --git a/authz/authz_test.go b/authz/authz_test.go index 9cf5c09c1bb7e50488014eac2d8862a4be946761..86e28dbd8d72dfa5767a512f4452910dd1e7402c 100644 --- a/authz/authz_test.go +++ b/authz/authz_test.go @@ -6,6 +6,7 @@ import ( "errors" "net/http" "net/http/httptest" + "os" "strings" "sync/atomic" "testing" @@ -18,6 +19,14 @@ import ( "sourcecraft.dev/bigbes/sr-ht-compare/core" ) +// TestMain installs ecore's fixed test keyset into core-go's process-global +// crypto, so the Internal authorization this package seals for git.sr.ht can be +// opened again by the stub receiver below. +func TestMain(m *testing.M) { + ecoretest.InitCrypto() + os.Exit(m.Run()) +} + // authNameFromRequest asserts the incoming request bears an "Internal " // Authorization header that decrypts to InternalAuth JSON, and returns the Name // field (empty for anonymous). This mirrors what git.sr.ht's receiver does. diff --git a/authz/doc.go b/authz/doc.go index 74a9a416e96b71915308789770fa41ac65dbc48d..c5fe930416ad6d2321ec0b5adfb1e580aced0f26 100644 --- a/authz/doc.go +++ b/authz/doc.go @@ -1,9 +1,10 @@ -// Package authz answers two orthogonal questions for compare.sr.ht: who is -// making a request, and what may they see. Identity is derived purely from the -// SourceHut unified-login cookie (sr.ht.unified-login.v1), a Fernet token -// encrypted with the instance [sr.ht] network-key; UsernameFromRequest decrypts -// it and yields a bare username, or "" for an anonymous viewer. It never -// rejects a request — an unreadable or absent cookie simply means anonymous. +// Package authz answers one question for compare.sr.ht: what may a given viewer +// see. Who the viewer is, is not this package's question any more — the +// unified-login cookie is decoded by sr-ht-ecore/login, which every custom +// service on the instance shares, and a handler reads the answer with +// login.FromContext. The copy that used to live here decoded the same cookie +// and did not validate the name it found, which is the whole reason that decode +// is one package now. // // Authorization is delegated entirely to git.sr.ht over its internal GraphQL // API: compare.sr.ht owns no user or repository data of its own, so there is no diff --git a/authz/identity.go b/authz/identity.go deleted file mode 100644 index ca6bd4781a077bea334a927a0ecd3b5153757eca..0000000000000000000000000000000000000000 --- a/authz/identity.go +++ /dev/null @@ -1,63 +0,0 @@ -package authz - -import ( - "context" - "encoding/json" - "net/http" - "strings" - - "sourcecraft.dev/bigbes/sr-ht-core/crypto" -) - -// CookieName is the SourceHut unified-login session cookie. Its value is a -// Fernet token encrypted with the instance [sr.ht] network-key. -const CookieName = "sr.ht.unified-login.v1" - -// UsernameFromRequest extracts the authenticated username from the unified-login -// cookie, or returns "" for an anonymous viewer. Any failure — missing cookie, -// undecryptable token, malformed JSON — is treated as anonymous rather than an -// error: this service never rejects a request on identity grounds, it only lets -// git.sr.ht decide what an anonymous viewer may see. -func UsernameFromRequest(r *http.Request) string { - c, err := r.Cookie(CookieName) - if err != nil { - return "" - } - // InitCrypto must have run (server.New does it); the network-key here is - // the same Fernet key meta.sr.ht used to seal the cookie. - payload := crypto.DecryptWithoutExpiration([]byte(c.Value)) - if payload == nil { - return "" - } - var claims struct { - Name string `json:"name"` - } - if err := json.Unmarshal(payload, &claims); err != nil { - return "" - } - // Cookies carry the bare username; strip a leading "~" defensively in case - // a caller stored the canonical "~user" form. - return strings.TrimPrefix(claims.Name, "~") -} - -type ctxKey int - -const usernameKey ctxKey = iota - -// Middleware stores the cookie-derived username in the request context. It -// never writes a 401: an anonymous viewer flows through with an empty username -// and git.sr.ht enforces visibility downstream. -func Middleware() func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx := context.WithValue(r.Context(), usernameKey, UsernameFromRequest(r)) - next.ServeHTTP(w, r.WithContext(ctx)) - }) - } -} - -// ForContext returns the username stored by Middleware, or "" if absent. -func ForContext(ctx context.Context) string { - username, _ := ctx.Value(usernameKey).(string) - return username -} diff --git a/authz/identity_test.go b/authz/identity_test.go deleted file mode 100644 index 6d6e8314ea4a04fdbb35ce50fa44dfa4bdd9152a..0000000000000000000000000000000000000000 --- a/authz/identity_test.go +++ /dev/null @@ -1,98 +0,0 @@ -package authz - -import ( - "encoding/json" - "net/http" - "net/http/httptest" - "os" - "testing" - - "sourcecraft.dev/bigbes/sr-ht-core/crypto" - "sourcecraft.dev/bigbes/sr-ht-ecore/ecoretest" -) - -// TestMain installs ecore's fixed test keyset into core-go's process-global -// crypto, so Encrypt/Decrypt work offline. The keys are constants and the call -// is idempotent, which is what lets this package and web/ both initialise -// without the second rotating what the first sealed with. -func TestMain(m *testing.M) { - ecoretest.InitCrypto() - os.Exit(m.Run()) -} - -// sealCookie builds a valid unified-login cookie value carrying the given name. -func sealCookie(t *testing.T, name string) string { - t.Helper() - payload, err := json.Marshal(map[string]string{"name": name}) - if err != nil { - t.Fatalf("marshal claims: %v", err) - } - return string(crypto.Encrypt(payload)) -} - -func TestUsernameFromRequest_RoundTrip(t *testing.T) { - r := httptest.NewRequest(http.MethodGet, "/", nil) - r.AddCookie(&http.Cookie{Name: CookieName, Value: sealCookie(t, "bigbes")}) - if got := UsernameFromRequest(r); got != "bigbes" { - t.Fatalf("username = %q, want %q", got, "bigbes") - } -} - -func TestUsernameFromRequest_StripsTilde(t *testing.T) { - r := httptest.NewRequest(http.MethodGet, "/", nil) - r.AddCookie(&http.Cookie{Name: CookieName, Value: sealCookie(t, "~bigbes")}) - if got := UsernameFromRequest(r); got != "bigbes" { - t.Fatalf("username = %q, want %q", got, "bigbes") - } -} - -func TestUsernameFromRequest_GarbageCookie(t *testing.T) { - r := httptest.NewRequest(http.MethodGet, "/", nil) - r.AddCookie(&http.Cookie{Name: CookieName, Value: "not-a-valid-fernet-token"}) - if got := UsernameFromRequest(r); got != "" { - t.Fatalf("username = %q, want empty", got) - } -} - -func TestUsernameFromRequest_MissingCookie(t *testing.T) { - r := httptest.NewRequest(http.MethodGet, "/", nil) - if got := UsernameFromRequest(r); got != "" { - t.Fatalf("username = %q, want empty", got) - } -} - -func TestUsernameFromRequest_NonJSONPayload(t *testing.T) { - r := httptest.NewRequest(http.MethodGet, "/", nil) - // A well-formed Fernet token whose plaintext is not JSON. - r.AddCookie(&http.Cookie{Name: CookieName, Value: string(crypto.Encrypt([]byte("plain text")))}) - if got := UsernameFromRequest(r); got != "" { - t.Fatalf("username = %q, want empty", got) - } -} - -func TestMiddlewareAndForContext(t *testing.T) { - var seen string - h := Middleware()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - seen = ForContext(r.Context()) - })) - - r := httptest.NewRequest(http.MethodGet, "/", nil) - r.AddCookie(&http.Cookie{Name: CookieName, Value: sealCookie(t, "bigbes")}) - h.ServeHTTP(httptest.NewRecorder(), r) - if seen != "bigbes" { - t.Fatalf("ForContext = %q, want %q", seen, "bigbes") - } - - // Anonymous request: middleware still runs, ForContext yields "". - seen = "sentinel" - h.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/", nil)) - if seen != "" { - t.Fatalf("anonymous ForContext = %q, want empty", seen) - } -} - -func TestForContext_NoValue(t *testing.T) { - if got := ForContext(httptest.NewRequest(http.MethodGet, "/", nil).Context()); got != "" { - t.Fatalf("ForContext on bare context = %q, want empty", got) - } -} diff --git a/cmd/comparesrht/main.go b/cmd/comparesrht/main.go index 99d31f67381cb18d5eb912f7b51cb6e7217b8a22..331844ce336f7618490a100c568c9feed550abe5 100644 --- a/cmd/comparesrht/main.go +++ b/cmd/comparesrht/main.go @@ -38,6 +38,7 @@ import ( "go.bigb.es/auxilia/scribe" "sourcecraft.dev/bigbes/sr-ht-core/config" coreserver "sourcecraft.dev/bigbes/sr-ht-core/server" + "sourcecraft.dev/bigbes/sr-ht-ecore/login" "sourcecraft.dev/bigbes/sr-ht-compare/authz" "sourcecraft.dev/bigbes/sr-ht-compare/web" @@ -122,7 +123,7 @@ func main() { // Middleware chain per the web package contract (outermost first). This is // the hand-rolled substitute for WithDefaultMiddleware: no database, no - // redis, and authz.Middleware never issues a 401 so anonymous browsing + // redis, and login.Optional never issues a 401 so anonymous browsing // works. config.Middleware must be present because the GraphQL authorizer // resolves git.sr.ht's API origin from config.ForContext at request time. // @@ -142,7 +143,10 @@ func main() { r.Use(middleware.Recoverer) r.Use(middleware.Logger) r.Use(config.Middleware(conf, service)) - r.Use(authz.Middleware()) + // The instance's one cookie decode, with the default validator: a name + // this service narrowed further would be an account logged out of + // compare alone, and meta.sr.ht is the authority on which names exist. + r.Use(login.Optional()) app.Register(r) }) diff --git a/docs/inline-comments.md b/docs/inline-comments.md index 5cbad38e5ac4f89d2252ebb1d50508bb073507c7..e9a9df2baa85cf186aa683f9997cba6942d05386 100644 --- a/docs/inline-comments.md +++ b/docs/inline-comments.md @@ -125,7 +125,7 @@ Reuse the existing seams — do **not** invent a second authz path. (`authz.Authorizer.Repo`), which 404s a repo the viewer cannot see (private existence never leaks). Comments for a repo are only ever returned to a viewer who passed that check. -- **Write**: require an authenticated viewer — `authz.ForContext(ctx) != ""`. +- **Write**: require an authenticated viewer — `login.FromContext(ctx) != ""`. For the MVP, **any authenticated viewer who can read the repo may comment** (open code review). Edit/delete restricted to the comment's `author`; resolve allowed to the thread author or comment author. @@ -247,7 +247,8 @@ Confirm how the sibling Go services on this instance manage schema and match the `connection-string`). - Opt-out today: `cmd/comparesrht/main.go` (middleware `Group`, `validateConfig`). - Authz seams: `authz/authz.go` (`Authorizer`, `RepoInfo`), - `authz/identity.go` (`ForContext`), `web/handlers.go` (`s.resolve`). + `sr-ht-ecore/login` (`Optional`, `FromContext`), `web/handlers.go` + (`s.resolve`). - Routes / SSR contract: `web/router.go` (`Register`), `web/server.go`, `web/handlers.go` (`buildCompareJSON`, `#compare-data`). - Diff library API: `@pierre/diffs` `FileDiffOptions.renderAnnotation`, diff --git a/web/handlers.go b/web/handlers.go index 94fd901a15df8f324f691eb38eb15df02af578c9..9c283c4359a9f7debd322371b3af3c50ede259e2 100644 --- a/web/handlers.go +++ b/web/handlers.go @@ -13,6 +13,7 @@ import ( "github.com/go-chi/chi/v5" "go.bigb.es/auxilia/scribe" "sourcecraft.dev/bigbes/sr-ht-ecore/chrome" + "sourcecraft.dev/bigbes/sr-ht-ecore/login" "sourcecraft.dev/bigbes/sr-ht-compare/authz" "sourcecraft.dev/bigbes/sr-ht-compare/core" @@ -132,7 +133,7 @@ func (s *Server) fail(w http.ResponseWriter, r *http.Request, err error) { // authz metadata. Any error is already mapped to the right HTTP status by the // caller via fail. func (s *Server) resolve(ctx context.Context, owner, repo string) (*gitx.Repo, *authz.RepoInfo, error) { - viewer := authz.ForContext(ctx) + viewer := login.FromContext(ctx) info, err := s.authorizer.Repo(ctx, viewer, owner, repo) if err != nil { return nil, nil, err @@ -157,7 +158,7 @@ type indexData struct { func (s *Server) handleIndex(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - username := authz.ForContext(ctx) + username := login.FromContext(ctx) // The title is built from the chrome's own brand fields rather than from a // second read of site-name, so the tab and the nav cannot name the instance diff --git a/web/server.go b/web/server.go index 8222573e0dfa7cb957119b35daa691a4a04cf8ae..2af634133b9d23ec874a28caff1fede37c8830f5 100644 --- a/web/server.go +++ b/web/server.go @@ -3,7 +3,7 @@ // embeds a compact JSON payload plus the vendored esbuild bundle so the browser // renders the diff with @pierre/diffs and @pierre/trees. // -// The package owns no state of its own: identity comes from the authz cookie +// The package owns no state of its own: identity comes from ecore's login // middleware, authorization from an authz.Authorizer (git.sr.ht GraphQL), and // git data from gitx over bare repositories on disk. Every request that touches // a repository authorizes first (a not-found or forbidden repo is a 404, never @@ -38,9 +38,13 @@ // chi middleware.Recoverer // chi middleware.Logger (optional, but recommended) // config.Middleware(conf, "compare.sr.ht") // required: authz + gitx read it -// authz.Middleware() // required: never 401s; sets the viewer +// login.Optional() // required: never 401s; sets the viewer // -// config.Middleware must run before authz.Middleware is irrelevant to authz +// login.Optional and not login.Required: every page here is either public or a +// 404, and git.sr.ht decides which — a viewer this service refused would be a +// viewer git.sr.ht was never asked about. +// +// config.Middleware must run before login.Optional is irrelevant to login // itself (it only reads the cookie), but the GraphQL authorizer invoked inside // handlers needs config.ForContext(ctx) to resolve git.sr.ht's API origin, so // config.Middleware is mandatory on every request that reaches a handler. @@ -55,6 +59,7 @@ import ( "go.bigb.es/auxilia/culpa" "sourcecraft.dev/bigbes/sr-ht-ecore/assets" "sourcecraft.dev/bigbes/sr-ht-ecore/chrome" + "sourcecraft.dev/bigbes/sr-ht-ecore/login" "sourcecraft.dev/bigbes/sr-ht-ecore/pages" "sourcecraft.dev/bigbes/sr-ht-compare/authz" @@ -201,9 +206,10 @@ type viewData struct { // view builds the frame for one request: the shared chrome plus a title. // -// The username is whatever the authz cookie middleware resolved, which is "" for -// a viewer whose cookie is missing, expired or unreadable — so the nav offers -// login to exactly the viewers the handlers treat as anonymous. +// The username is whatever login.Optional resolved, which is "" for a viewer +// whose cookie is missing, expired, unreadable or carries a name that could not +// be one — so the nav offers login to exactly the viewers the handlers treat as +// anonymous. func (s *Server) view(r *http.Request, title string) viewData { - return viewData{Page: s.chromeSvc.Page(r, title, authz.ForContext(r.Context()))} + return viewData{Page: s.chromeSvc.Page(r, title, login.FromContext(r.Context()))} } diff --git a/web/web_test.go b/web/web_test.go index f6ffd1f949fb90d184fdfbac6b43045141c4dae5..5d5bc69fa68ec51a43c568f8a078f290d440bd58 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -22,6 +22,7 @@ import ( "sourcecraft.dev/bigbes/sr-ht-ecore/assets" "sourcecraft.dev/bigbes/sr-ht-ecore/csrf" "sourcecraft.dev/bigbes/sr-ht-ecore/ecoretest" + "sourcecraft.dev/bigbes/sr-ht-ecore/login" "sourcecraft.dev/bigbes/sr-ht-ecore/pages" "sourcecraft.dev/bigbes/sr-ht-compare/authz" @@ -30,7 +31,7 @@ import ( ) // TestMain seeds core-go's process-global crypto with ecore's fixed test -// keyset, which is what lets login below seal a unified-login cookie the authz +// keyset, which is what lets logIn below seal a unified-login cookie the login // middleware can open again. func TestMain(m *testing.M) { ecoretest.InitCrypto() @@ -153,15 +154,16 @@ func testServer(t *testing.T, root string, az authz.Authorizer) http.Handler { r := chi.NewRouter() r.Use(config.Middleware(conf, "compare.sr.ht")) - r.Use(authz.Middleware()) + r.Use(login.Optional()) srv.Register(r) return r } -// login seals a unified-login cookie for the given user onto a request. -func login(req *http.Request, user string) { +// logIn seals a unified-login cookie for the given user onto a request. It is +// not called login because that is the package that opens the cookie again. +func logIn(req *http.Request, user string) { payload, _ := json.Marshal(map[string]string{"name": user}) - req.AddCookie(&http.Cookie{Name: authz.CookieName, Value: string(crypto.Encrypt(payload))}) + req.AddCookie(&http.Cookie{Name: login.CookieName, Value: string(crypto.Encrypt(payload))}) } func demoAuthorizer() *stubAuthorizer { @@ -180,7 +182,7 @@ func get(t *testing.T, h http.Handler, target string, user string) *httptest.Res t.Helper() req := httptest.NewRequest(http.MethodGet, target, nil) if user != "" { - login(req, user) + logIn(req, user) } rec := httptest.NewRecorder() h.ServeHTTP(rec, req)