From b7d1bf89a0bba51749babd45ce9f9c14b72f2470 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sun, 9 Aug 2026 00:23:21 +0300 Subject: [PATCH] login: decode the unified-login cookie through ecore --- authn/authn_test.go | 3 +- authn/cookie.go | 72 --------------------------------- authn/cookie_test.go | 92 ------------------------------------------ authn/resolver.go | 26 +++++++++++- authn/resolver_test.go | 31 ++++++++++++++ graph/graph_test.go | 4 +- web/web_test.go | 4 +- 7 files changed, 64 insertions(+), 168 deletions(-) delete mode 100644 authn/cookie.go delete mode 100644 authn/cookie_test.go diff --git a/authn/authn_test.go b/authn/authn_test.go index 81856bce9c7b8bfaff1ec80a40b9419c8351bc13..5be6c36fc97120c20b3652b0b409f9a4d2d07768 100644 --- a/authn/authn_test.go +++ b/authn/authn_test.go @@ -13,6 +13,7 @@ import ( "github.com/vaughan0/go-ini" "sourcecraft.dev/bigbes/sr-ht-core/auth" "sourcecraft.dev/bigbes/sr-ht-core/crypto" + "sourcecraft.dev/bigbes/sr-ht-ecore/login" ) // testConf is the synthesized instance config every test runs against: a fresh @@ -121,7 +122,7 @@ func testInstance(t *testing.T) Instance { func request(cookie string, headers map[string]string) *http.Request { r := httptest.NewRequest(http.MethodGet, "/", nil) if cookie != "" { - r.AddCookie(&http.Cookie{Name: CookieName, Value: cookie}) + r.AddCookie(&http.Cookie{Name: login.CookieName, Value: cookie}) } for k, v := range headers { r.Header.Set(k, v) diff --git a/authn/cookie.go b/authn/cookie.go deleted file mode 100644 index eed441bfbbdabd4b23423905f9e047bba76a500f..0000000000000000000000000000000000000000 --- a/authn/cookie.go +++ /dev/null @@ -1,72 +0,0 @@ -package authn - -import ( - "encoding/json" - "net/http" - "strings" - - "sourcecraft.dev/bigbes/sr-ht-core/auth" - "sourcecraft.dev/bigbes/sr-ht-core/crypto" - - "sourcecraft.dev/bigbes/sr-ht-spec/core" -) - -// CookieName is the unified-login cookie shared by every service on the -// instance. Its value is a Fernet token sealed by meta.sr.ht with the -// [sr.ht] network-key, which is why spec.srht.bigb.es must live under the -// shared *.srht.bigb.es cookie domain — otherwise the cookie is never sent to -// us and every viewer looks anonymous. -const CookieName = "sr.ht.unified-login.v1" - -// UsernameFromRequest returns the username carried by the unified-login cookie, -// or "" for an anonymous viewer. -// -// Every failure path — no cookie, forged or truncated ciphertext, a payload -// that is not JSON, a payload with no name — returns "" rather than an error. -// This service never serves an error page on identity grounds; it decides what -// an anonymous viewer may see instead. Returning an error here would turn a -// stale cookie from a browser tab left open over a key rotation into a broken -// site rather than a logged-out one. -// -// Requires crypto.InitCrypto to have run (server.New does it at startup). -func UsernameFromRequest(r *http.Request) string { - c, err := r.Cookie(CookieName) - if err != nil { - return "" // no cookie: anonymous - } - return UsernameFromCookie(c.Value) -} - -// UsernameFromCookie is UsernameFromRequest for a cookie value already in hand -// — the form the hooks' RPC path and tests need, where there is no -// *http.Request to read from. -// -// Note the deliberate use of DecryptWithoutExpiration, matching core-go's own -// cookieAuth: the unified-login cookie carries no service-side TTL, and its -// lifetime is the browser cookie's Expires plus meta.sr.ht's ability to rotate -// the network key. Adding a TTL here would log the owner out of this one -// service on a schedule no other service shares. -func UsernameFromCookie(value string) string { - if value == "" { - return "" - } - payload := crypto.DecryptWithoutExpiration([]byte(value)) - if payload == nil { - return "" // forged, tampered, or sealed with a key we no longer hold - } - - var claims auth.AuthCookie - if err := json.Unmarshal(payload, &claims); err != nil { - return "" // well-sealed but malformed payload - } - - // Cookies carry the bare username; strip a leading '~' defensively in case - // something upstream stored the canonical "~user" form. - name := strings.TrimPrefix(claims.Name, "~") - if err := core.ValidateOwner(name); err != nil { - // An unusable username is not an identity. Rejecting here keeps a - // hostile cookie payload out of path construction and log lines. - return "" - } - return name -} diff --git a/authn/cookie_test.go b/authn/cookie_test.go deleted file mode 100644 index 84f7cc5c3f20103dd6e48f4dd4423555b0761668..0000000000000000000000000000000000000000 --- a/authn/cookie_test.go +++ /dev/null @@ -1,92 +0,0 @@ -package authn - -import ( - "testing" - - "sourcecraft.dev/bigbes/sr-ht-core/crypto" -) - -func TestUsernameFromRequest_ValidCookieRoundTrips(t *testing.T) { - got := UsernameFromRequest(request(sealCookie(t, "bigbes"), nil)) - if got != "bigbes" { - t.Fatalf("username = %q, want %q", got, "bigbes") - } -} - -func TestUsernameFromRequest_StripsTilde(t *testing.T) { - got := UsernameFromRequest(request(sealCookie(t, "~bigbes"), nil)) - if got != "bigbes" { - t.Fatalf("username = %q, want %q", got, "bigbes") - } -} - -// A cookie whose ciphertext has been altered must fail the Fernet HMAC and read -// as anonymous — not as an error, and certainly not as an identity. -func TestUsernameFromRequest_TamperedCookieIsAnonymous(t *testing.T) { - got := UsernameFromRequest(request(tamper(t, sealCookie(t, "bigbes")), nil)) - if got != "" { - t.Fatalf("tampered cookie yielded %q, want anonymous", got) - } -} - -func TestUsernameFromRequest_AbsentCookieIsAnonymous(t *testing.T) { - if got := UsernameFromRequest(request("", nil)); got != "" { - t.Fatalf("absent cookie yielded %q, want anonymous", got) - } -} - -func TestUsernameFromRequest_GarbageIsAnonymous(t *testing.T) { - for name, value := range map[string]string{ - "not base64": "not-a-valid-fernet-token", - "empty": "", - "truncated fernet": sealCookie(t, "bigbes")[:10], - } { - t.Run(name, func(t *testing.T) { - if got := UsernameFromCookie(value); got != "" { - t.Fatalf("garbage cookie yielded %q, want anonymous", got) - } - }) - } -} - -// A cookie sealed under a key we no longer hold — the shape of both a rotated -// network key and an outright forgery. It must expire the session, not the -// request. -func TestUsernameFromCookie_ForeignKeyIsAnonymous(t *testing.T) { - value := sealCookieWithKey(t, &rotatedKey, "bigbes") - if got := UsernameFromCookie(value); got != "" { - t.Fatalf("cookie under a foreign key yielded %q, want anonymous", got) - } - // Sanity: the same payload under the live key does resolve, so the test - // above is proving the key check and not a broken helper. - if got := UsernameFromCookie(sealCookie(t, "bigbes")); got != "bigbes" { - t.Fatalf("control cookie yielded %q, want %q", got, "bigbes") - } -} - -func TestUsernameFromCookie_NonJSONPayloadIsAnonymous(t *testing.T) { - value := string(crypto.Encrypt([]byte("plain text, well sealed"))) - if got := UsernameFromCookie(value); got != "" { - t.Fatalf("non-JSON payload yielded %q, want anonymous", got) - } -} - -// A well-sealed cookie can still carry a name we must refuse to treat as an -// identity: empty, or something that would not survive being used as a path -// segment or a log field. -func TestUsernameFromCookie_UnusableNameIsAnonymous(t *testing.T) { - for _, name := range []string{ - "", - "..", - "../../etc/passwd", - "has space", - "Uppercase", - "-leading-dash", - } { - t.Run(name, func(t *testing.T) { - if got := UsernameFromCookie(sealCookie(t, name)); got != "" { - t.Fatalf("cookie name %q yielded %q, want anonymous", name, got) - } - }) - } -} diff --git a/authn/resolver.go b/authn/resolver.go index 210b9af647fa2adacddd0a16daa4b0bc20a439aa..c9664488b39e0b2f23767e332fdc7c92e3ad0202 100644 --- a/authn/resolver.go +++ b/authn/resolver.go @@ -8,10 +8,34 @@ import ( "strings" "go.bigb.es/auxilia/scribe" + "sourcecraft.dev/bigbes/sr-ht-ecore/login" "sourcecraft.dev/bigbes/sr-ht-spec/core" ) +// cookieDecode is how this service reads the instance's unified-login cookie: +// sr-ht-ecore's one decoder, told to use core.ValidateOwner as its name rule. +// +// The decode itself is not ours and never was — decrypt without expiration, +// unmarshal core-go's claims, strip one leading '~', treat every failure as +// anonymity — and six services each keeping a copy of those four steps is how +// one of them ends up missing the fifth. The fifth is the validator, and it is +// the one thing here that stays spec.sr.ht's: a decoded name goes on to be +// joined into a repository path under [spec.sr.ht] repos, and core.ValidateOwner +// is the rule the rest of this service builds those paths against. Passing it +// means there is one grammar rather than two that agree until one of them is +// widened. +// +// Resolved once, at package level, because it is read on every request and +// login.Option is a build step. +var cookieDecode = []login.Option{login.WithValidator(validCookieName)} + +// validCookieName adapts core.ValidateOwner to login's predicate shape. An +// error is "not a usable identity", which login turns into an anonymous viewer +// — never an error page, because a name this service cannot use is the same +// thing to a browser as no cookie at all. +func validCookieName(name string) bool { return core.ValidateOwner(name) == nil } + // Resolver turns a request into a Principal. It holds the instance owner // username — the one name a cookie has to match to carry authority — and, when // the instance is configured for it, the tokens.sr.ht plane every agent @@ -115,7 +139,7 @@ func (rs *Resolver) Resolve(ctx context.Context, r *http.Request) (Principal, er r.Header.Get(HeaderAgent), r.Header.Get(HeaderAgentSession)) } - username := UsernameFromRequest(r) + username := login.UsernameFromRequest(r, cookieDecode...) if username == "" { return Anonymous(), nil } diff --git a/authn/resolver_test.go b/authn/resolver_test.go index 50187aab4a4a49b950766318213323e2651d7983..041a9501a0d7974b81a9f60e711e06752a610d05 100644 --- a/authn/resolver_test.go +++ b/authn/resolver_test.go @@ -8,6 +8,8 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "sourcecraft.dev/bigbes/sr-ht-ecore/bearer" ) @@ -110,6 +112,35 @@ func TestResolve_NonOwnerCookieIsAnonymous(t *testing.T) { } } +// The validator seam. login decodes the cookie, and what this service supplies +// is core.ValidateOwner as the rule for what may count as a name — the same rule +// every repository path under [spec.sr.ht] repos is built against. Each of these +// is a well-sealed cookie carrying a name meta.sr.ht could not have issued, and +// each must read as anonymous rather than reach filepath.Join. +// +// login ships a conservative default of its own, so this is not the difference +// between validating and not validating; it pins that the rule in force is ours, +// which is what stops the two grammars from drifting apart on the day one of +// them is widened. +func TestResolve_CookieNameIsHeldToValidateOwner(t *testing.T) { + rs := newTestResolver(t) + for _, name := range []string{ + "", + "..", + "../../etc/passwd", + "has space", + "Uppercase", + "-leading-dash", + } { + t.Run(name, func(t *testing.T) { + p, err := rs.Resolve(context.Background(), request(sealCookie(t, name), nil)) + require.NoError(t, err, "an unusable cookie name must never error") + assert.True(t, p.IsAnonymous(), "principal = %+v, want anonymous", p) + assert.Empty(t, p.CookieUser, "an unusable name must not survive as a viewer name either") + }) + } +} + func TestResolve_AgentTokenAccepted(t *testing.T) { f := newPlaneFixture(t, http.StatusNoContent) diff --git a/graph/graph_test.go b/graph/graph_test.go index 3b2e19916947a0795d7ecbcd18c0e83c00e7a079..b6aed3a1a3d5da2b9d2693d2866c2a76ca2a7c49 100644 --- a/graph/graph_test.go +++ b/graph/graph_test.go @@ -19,6 +19,8 @@ import ( "sourcecraft.dev/bigbes/sr-ht-core/crypto" "sourcecraft.dev/bigbes/sr-ht-ecore/bearer" "sourcecraft.dev/bigbes/sr-ht-ecore/ecoretest" + // Aliased: this file's own helper for signing a request in is called login. + ecorelogin "sourcecraft.dev/bigbes/sr-ht-ecore/login" "sourcecraft.dev/bigbes/sr-ht-spec/authn" "sourcecraft.dev/bigbes/sr-ht-spec/core" @@ -411,7 +413,7 @@ func ok(t *testing.T, r response, v any) { // meta.sr.ht writes, sealed with the shared network key. func login(req *http.Request, user string) { payload, _ := json.Marshal(map[string]string{"name": user}) - req.AddCookie(&http.Cookie{Name: authn.CookieName, Value: string(crypto.Encrypt(payload))}) + req.AddCookie(&http.Cookie{Name: ecorelogin.CookieName, Value: string(crypto.Encrypt(payload))}) } // ---- the read contract ---------------------------------------------------- diff --git a/web/web_test.go b/web/web_test.go index 33107cb8c3fc1220a4bd694cb8ee186606566a23..ee25c11488d8cecc806779fb7e53ab488631c3e9 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -21,6 +21,8 @@ import ( "sourcecraft.dev/bigbes/sr-ht-ecore/bearer" "sourcecraft.dev/bigbes/sr-ht-ecore/csrf" "sourcecraft.dev/bigbes/sr-ht-ecore/ecoretest" + // Aliased: this file's own helper for signing a request in is called login. + ecorelogin "sourcecraft.dev/bigbes/sr-ht-ecore/login" "sourcecraft.dev/bigbes/sr-ht-spec/authn" "sourcecraft.dev/bigbes/sr-ht-spec/core" @@ -478,7 +480,7 @@ func testServerWith(t *testing.T, reader *fakeReader) (http.Handler, *fakeReader // same shape meta.sr.ht writes, sealed with the shared network key. func login(req *http.Request, user string) { payload, _ := json.Marshal(map[string]string{"name": user}) - req.AddCookie(&http.Cookie{Name: authn.CookieName, Value: string(crypto.Encrypt(payload))}) + req.AddCookie(&http.Cookie{Name: ecorelogin.CookieName, Value: string(crypto.Encrypt(payload))}) } func get(t *testing.T, h http.Handler, target, user string) *httptest.ResponseRecorder {