M authn/authn_test.go => authn/authn_test.go +2 -1
@@ 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)
D authn/cookie.go => authn/cookie.go +0 -72
@@ 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
-}
D authn/cookie_test.go => authn/cookie_test.go +0 -92
@@ 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)
- }
- })
- }
-}
M authn/resolver.go => authn/resolver.go +25 -1
@@ 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
}
M authn/resolver_test.go => authn/resolver_test.go +31 -0
@@ 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)
M graph/graph_test.go => graph/graph_test.go +3 -1
@@ 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 ----------------------------------------------------
M web/web_test.go => web/web_test.go +3 -1
@@ 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 {