@@ 1,33 1,32 @@
package authn
import (
- "encoding/json"
"log/slog"
"net/http"
"go.bigb.es/auxilia/scribe"
"sourcecraft.dev/bigbes/sr-ht-core/auth"
- "sourcecraft.dev/bigbes/sr-ht-core/crypto"
-)
-// CookieName is the unified-login cookie shared across every SourceHut service.
-const CookieName = "sr.ht.unified-login.v1"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/login"
+)
-// OptionalCookieMiddleware reads the sr.ht.unified-login.v1 cookie and, when it
-// is present and valid, attaches the resolved caller to the request context
-// (retrievable with CallerFromContext). It NEVER rejects a request: a missing,
-// malformed, undecryptable, or unresolvable cookie leaves the request
+// OptionalCookieMiddleware reads the unified-login cookie and, when it names a
+// user this service can resolve, attaches the resolved caller to the request
+// context (retrievable with CallerFromContext). It NEVER rejects a request: a
+// missing, malformed, undecryptable, or unresolvable cookie leaves the request
// anonymous. This is what allows public browsing and public clones to work
// without credentials — unlike core-go's auth.Middleware, which 401s any
// request lacking a cookie or Authorization header.
//
-// It replicates the logic of core-go's unexported cookieAuth: decrypt the
-// fernet-sealed cookie with crypto.DecryptWithoutExpiration, decode the
-// {"name": ...} payload, and resolve the user via the meta backend (core-go's
-// auth.LookupUser path). Suspended users are resolved normally; the suspension
-// flag is carried on the caller (via AsCoreCaller) and gates writes at the
-// access-control layer rather than being rejected here.
+// The two halves of that sentence are two packages, and the split is the point.
+// Decoding the cookie is instance-wide — one session, one seal, one grammar for
+// the name inside it — and lives in sr-ht-ecore's login. Turning the name into a
+// row is dolt.sr.ht's alone: our user table, our mirror-on-first-sight, our
+// answer for a user meta has but we have never seen. Only the second half is
+// here, which is also why this is not simply login.Optional: what the rest of
+// the service reads out of the context is an *auth.AuthContext with a UserID,
+// not a username.
//
// Requires crypto.InitCrypto to have run (server.New does this at startup) and,
// for the user lookup, config.Middleware + database.Middleware installed
@@ 46,31 45,28 @@ func OptionalCookieMiddleware() func(http.Handler) http.Handler {
// resolveCookie returns the caller authenticated by the request's unified-login
// cookie, or nil if there is no cookie or it cannot be resolved for any reason.
// Every failure path returns nil (anonymous) — none is fatal.
+//
+// Suspended users are resolved normally; the suspension flag is carried on the
+// caller (via AsCoreCaller) and gates writes at the access-control layer rather
+// than being rejected here.
func resolveCookie(r *http.Request) *auth.AuthContext {
- cookie, err := r.Cookie(CookieName)
- if err != nil {
- return nil // no cookie: anonymous
- }
-
- payload := crypto.DecryptWithoutExpiration([]byte(cookie.Value))
- if payload == nil {
- return nil // bad/forged/undecryptable cookie: anonymous
- }
-
- var authCookie auth.AuthCookie
- if err := json.Unmarshal(payload, &authCookie); err != nil {
- return nil // malformed payload: anonymous
- }
- if authCookie.Name == "" {
- return nil // no username in payload: anonymous
+ // No cookie, a forged one, a payload that is not core-go's JSON, or a name
+ // that could not be an account name: all "" and all anonymous. Nothing is
+ // logged, because the cookie value is attacker-supplied and arrives on every
+ // request — a warning per bad decode is a log flood anyone can turn on.
+ username := login.UsernameFromRequest(r)
+ if username == "" {
+ return nil
}
var ac auth.AuthContext
- if err := meta.LookupUser(r.Context(), authCookie.Name, &ac); err != nil {
+ if err := meta.LookupUser(r.Context(), username, &ac); err != nil {
// meta/database unreachable or unknown user: degrade to anonymous
- // rather than failing the request (browsing must keep working).
+ // rather than failing the request (browsing must keep working). This
+ // one *is* logged: the name has already passed login's grammar, so it
+ // is bounded text, and an unreachable meta is an operator's problem.
slog.WarnContext(r.Context(), "resolving the login cookie's user failed",
- "component", "authn", "username", authCookie.Name, scribe.Err(err))
+ "component", "authn", "username", username, scribe.Err(err))
return nil
}
ac.AuthMethod = auth.AUTH_COOKIE
@@ 5,7 5,12 @@ import (
"net/http/httptest"
"testing"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
"sourcecraft.dev/bigbes/sr-ht-core/auth"
+
+ "sourcecraft.dev/bigbes/sr-ht-ecore/login"
)
// captureHandler records the caller present on the request context when reached.
@@ 30,44 35,48 @@ func runCookieMiddleware(t *testing.T, cookie *http.Cookie) (*auth.AuthContext,
rec := httptest.NewRecorder()
h.ServeHTTP(rec, req)
- if !reached {
- t.Fatal("middleware must always call next (never rejects)")
- }
+ require.True(t, reached, "middleware must always call next (never rejects)")
return got, rec.Code
}
-func TestOptionalCookieMiddleware_NoCookie(t *testing.T) {
- withStubBackend(t, &stubBackend{users: map[string]auth.AuthContext{}})
- got, code := runCookieMiddleware(t, nil)
- if got != nil {
- t.Fatalf("expected anonymous, got %+v", got)
- }
- if code != http.StatusOK {
- t.Fatalf("expected 200, got %d", code)
- }
-}
-
func TestOptionalCookieMiddleware_ValidCookie(t *testing.T) {
withStubBackend(t, &stubBackend{users: map[string]auth.AuthContext{
"bigbes": sampleUser(1, "bigbes", auth.USER_TYPE_USER),
}})
got, code := runCookieMiddleware(t, &http.Cookie{
- Name: CookieName,
+ Name: login.CookieName,
Value: forgeCookie(t, "bigbes"),
})
- if code != http.StatusOK {
- t.Fatalf("expected 200, got %d", code)
- }
- if got == nil {
- t.Fatal("expected an authenticated caller")
- }
- if got.Username != "bigbes" || got.UserID != 1 {
- t.Fatalf("wrong caller resolved: %+v", got)
- }
- if got.AuthMethod != auth.AUTH_COOKIE {
- t.Fatalf("AuthMethod = %q, want %q", got.AuthMethod, auth.AUTH_COOKIE)
- }
+ require.Equal(t, http.StatusOK, code)
+ require.NotNil(t, got, "expected an authenticated caller")
+ assert.Equal(t, "bigbes", got.Username)
+ assert.Equal(t, 1, got.UserID)
+ assert.Equal(t, auth.AUTH_COOKIE, got.AuthMethod)
+}
+
+// TestOptionalCookieMiddleware_StripsCanonicalSigil pins a difference from the
+// decode this package used to carry: a payload holding the canonical "~name"
+// form resolves to the bare name rather than being looked up with the sigil
+// still on it — which meta would answer for nobody.
+func TestOptionalCookieMiddleware_StripsCanonicalSigil(t *testing.T) {
+ stub := &stubBackend{users: map[string]auth.AuthContext{
+ "bigbes": sampleUser(1, "bigbes", auth.USER_TYPE_USER),
+ }}
+ withStubBackend(t, stub)
+
+ got, code := runCookieMiddleware(t, &http.Cookie{
+ Name: login.CookieName,
+ Value: forgeCookie(t, "~bigbes"),
+ })
+ require.Equal(t, http.StatusOK, code)
+ require.NotNil(t, got)
+ assert.Equal(t, "bigbes", got.Username)
+ // The backend is asked for the bare name, not "~bigbes". Asserting on the
+ // resolved caller alone would not show this: the stub normalises the sigil
+ // away itself, as meta.LookupUser does, so both spellings resolve either
+ // way and only the recorded argument says which one was sent.
+ assert.Equal(t, []string{"bigbes"}, stub.lookedUp)
}
func TestOptionalCookieMiddleware_SuspendedStillResolves(t *testing.T) {
@@ 75,55 84,73 @@ func TestOptionalCookieMiddleware_SuspendedStillResolves(t *testing.T) {
"susp": sampleUser(2, "susp", auth.USER_TYPE_SUSPENDED),
}})
got, _ := runCookieMiddleware(t, &http.Cookie{
- Name: CookieName,
+ Name: login.CookieName,
Value: forgeCookie(t, "susp"),
})
- if got == nil {
- t.Fatal("suspended users must still resolve (reads are allowed)")
- }
- if !AsCoreCaller(got).Suspended {
- t.Fatal("resolved caller must be flagged suspended")
- }
+ require.NotNil(t, got, "suspended users must still resolve (reads are allowed)")
+ assert.True(t, AsCoreCaller(got).Suspended, "resolved caller must be flagged suspended")
}
-func TestOptionalCookieMiddleware_GarbageCookie(t *testing.T) {
- withStubBackend(t, &stubBackend{users: map[string]auth.AuthContext{}})
- got, code := runCookieMiddleware(t, &http.Cookie{
- Name: CookieName,
- Value: "not-a-valid-fernet-token",
- })
- if got != nil {
- t.Fatalf("garbage cookie must degrade to anonymous, got %+v", got)
+// TestOptionalCookieMiddleware_UnusableCookieIsAnonymous is this package's own
+// contract rather than login's: whatever the decode answers, the middleware
+// resolves nobody and still calls next. It is what keeps public browsing and
+// public clones working when anything about the session goes wrong.
+//
+// The name cases are the second thing this package used not to do. A cookie
+// naming "../../etc/passwd" or an empty string decoded to that string and went
+// straight to the user lookup; now it never reaches the backend at all, and the
+// assertion below is that it did not.
+func TestOptionalCookieMiddleware_UnusableCookieIsAnonymous(t *testing.T) {
+ cases := []struct {
+ name string
+ cookie func(t *testing.T) *http.Cookie
+ }{
+ {"no cookie at all", func(*testing.T) *http.Cookie { return nil }},
+ {"undecryptable value", func(*testing.T) *http.Cookie {
+ return &http.Cookie{Name: login.CookieName, Value: "not-a-valid-fernet-token"}
+ }},
+ {"empty name in payload", func(t *testing.T) *http.Cookie {
+ return &http.Cookie{Name: login.CookieName, Value: forgeCookie(t, "")}
+ }},
+ {"path separator in name", func(t *testing.T) *http.Cookie {
+ return &http.Cookie{Name: login.CookieName, Value: forgeCookie(t, "../../etc/passwd")}
+ }},
+ {"newline in name", func(t *testing.T) *http.Cookie {
+ return &http.Cookie{Name: login.CookieName, Value: forgeCookie(t, "alice\nWARN forged")}
+ }},
}
- if code != http.StatusOK {
- t.Fatalf("must not reject: expected 200, got %d", code)
+
+ for _, c := range cases {
+ t.Run(c.name, func(t *testing.T) {
+ stub := &stubBackend{users: map[string]auth.AuthContext{
+ "bigbes": sampleUser(1, "bigbes", auth.USER_TYPE_USER),
+ }}
+ withStubBackend(t, stub)
+
+ got, code := runCookieMiddleware(t, c.cookie(t))
+ assert.Nil(t, got, "must degrade to anonymous")
+ assert.Equal(t, http.StatusOK, code, "must not reject")
+ assert.Empty(t, stub.lookedUp, "an unusable cookie must not reach the user lookup")
+ })
}
}
func TestOptionalCookieMiddleware_UnknownUser(t *testing.T) {
withStubBackend(t, &stubBackend{users: map[string]auth.AuthContext{}})
got, code := runCookieMiddleware(t, &http.Cookie{
- Name: CookieName,
+ Name: login.CookieName,
Value: forgeCookie(t, "ghost"),
})
- if got != nil {
- t.Fatalf("unknown user must degrade to anonymous, got %+v", got)
- }
- if code != http.StatusOK {
- t.Fatalf("must not reject: expected 200, got %d", code)
- }
+ assert.Nil(t, got, "unknown user must degrade to anonymous")
+ assert.Equal(t, http.StatusOK, code, "must not reject")
}
func TestOptionalCookieMiddleware_BackendDownIsAnonymous(t *testing.T) {
withStubBackend(t, &stubBackend{lookupErr: errBackendDown})
got, code := runCookieMiddleware(t, &http.Cookie{
- Name: CookieName,
+ Name: login.CookieName,
Value: forgeCookie(t, "bigbes"),
})
- if got != nil {
- t.Fatalf("backend failure must degrade to anonymous, got %+v", got)
- }
- if code != http.StatusOK {
- t.Fatalf("must not 500: expected 200, got %d", code)
- }
+ assert.Nil(t, got, "backend failure must degrade to anonymous")
+ assert.Equal(t, http.StatusOK, code, "must not 500")
}