From 16f2e11d39cdaba0f20c8e3de9664e5d58cf9cf9 Mon Sep 17 00:00:00 2001 From: bryan newbold Date: Tue, 29 Apr 2025 00:24:58 -0700 Subject: [PATCH 1/6] MVP atproto inter-service auth token support --- atproto/auth/jwt.go | 147 +++++++++++++++++++++++++++++++++ atproto/auth/jwt_signing.go | 88 ++++++++++++++++++++ atproto/auth/jwt_test.go | 156 ++++++++++++++++++++++++++++++++++++ 3 files changed, 391 insertions(+) create mode 100644 atproto/auth/jwt.go create mode 100644 atproto/auth/jwt_signing.go create mode 100644 atproto/auth/jwt_test.go diff --git a/atproto/auth/jwt.go b/atproto/auth/jwt.go new file mode 100644 index 00000000..f29b298c --- /dev/null +++ b/atproto/auth/jwt.go @@ -0,0 +1,147 @@ +package auth + +import ( + "context" + "encoding/base64" + "errors" + "fmt" + "log/slog" + "math/rand" + "time" + + "github.com/bluesky-social/indigo/atproto/crypto" + "github.com/bluesky-social/indigo/atproto/identity" + "github.com/bluesky-social/indigo/atproto/syntax" + + "github.com/golang-jwt/jwt/v5" +) + +// TODO: check for uniqueness of JTI (random nonce) to prevent token replay + +type ServiceAuthValidator struct { + // Service DID reference for this validator: a DID with optional #-separated fragment + Audience string + Dir identity.Directory +} + +type serviceAuthClaims struct { + jwt.RegisteredClaims + + LexMethod string `json:"lxm,omitempty"` +} + +func (s *ServiceAuthValidator) Validate(ctx context.Context, tokenString string, lexMethod *syntax.NSID) (syntax.DID, error) { + + opts := []jwt.ParserOption{ + jwt.WithValidMethods(supportedAlgs), + jwt.WithAudience(s.Audience), + jwt.WithExpirationRequired(), + jwt.WithIssuedAt(), + jwt.WithLeeway(5 * time.Second), // TODO: configurable? better default? + } + + token, err := jwt.ParseWithClaims(tokenString, &serviceAuthClaims{}, s.fetchIssuerKeyFunc(ctx), opts...) + if err != nil && errors.Is(err, jwt.ErrTokenSignatureInvalid) { + // if signature validation fails, purge the directory and try again + // TODO: probably need to cache or rate-limit this? + + // do an unvalidated extraction of 'iss' from JWT + insecure := jwt.NewParser(jwt.WithoutClaimsValidation()) + t, _, err := insecure.ParseUnverified(tokenString, &jwt.MapClaims{}) + claims, ok := t.Claims.(*jwt.MapClaims) + if !ok { + return "", jwt.ErrTokenInvalidClaims + } + iss, err := claims.GetIssuer() + if err != nil { + return "", err + } + did, err := syntax.ParseDID(iss) + if err != nil { + return "", fmt.Errorf("%w: invalid DID: %w", jwt.ErrTokenInvalidIssuer, err) + } + + slog.Info("purging directory and retrying service auth signature validation", "did", did) + err = s.Dir.Purge(ctx, did.AtIdentifier()) + if err != nil { + slog.Error("purging identity directory", "did", did, "err", err) + } + token, err = jwt.ParseWithClaims(tokenString, &serviceAuthClaims{}, s.fetchIssuerKeyFunc(ctx), opts...) + } + if err != nil { + return "", err + } + claims, ok := token.Claims.(*serviceAuthClaims) + if !ok { + // TODO: is this the best error here? + return "", jwt.ErrTokenInvalidClaims + } + + if lexMethod != nil && claims.LexMethod != lexMethod.String() { + return "", fmt.Errorf("%w: Lexicon endpoint (LXM)", jwt.ErrTokenInvalidClaims) + } + + // NOTE: KeyFunc has already parsed issuer, so we know it is a valid DID + did := syntax.DID(claims.Issuer) + return did, nil +} + +// resolves public key from identity directory +func (s *ServiceAuthValidator) fetchIssuerKeyFunc(ctx context.Context) func(token *jwt.Token) (any, error) { + return func(token *jwt.Token) (any, error) { + claims, ok := token.Claims.(*serviceAuthClaims) + if !ok { + return nil, fmt.Errorf("%w: missing 'iss'", jwt.ErrTokenInvalidClaims) + } + iss, err := claims.GetIssuer() + if err != nil { + return nil, fmt.Errorf("%w: missing 'iss'", jwt.ErrTokenInvalidClaims) + } + did, err := syntax.ParseDID(iss) + if err != nil { + return nil, fmt.Errorf("%w: invalid DID: %w", jwt.ErrTokenInvalidIssuer, err) + } + // NOTE: this will do handle resolution by default + ident, err := s.Dir.LookupDID(ctx, did) + if err != nil { + return nil, fmt.Errorf("%w: resolving DID (%s): %w", jwt.ErrTokenInvalidIssuer, did, err) + } + return ident.PublicKey() + } +} + +func randomNonce() string { + buf := make([]byte, 16) + rand.Read(buf) + return base64.RawURLEncoding.EncodeToString(buf) +} + +func SignServiceAuth(iss syntax.DID, aud string, ttl time.Duration, lexMethod *syntax.NSID, priv crypto.PrivateKey) (string, error) { + claims := serviceAuthClaims{ + RegisteredClaims: jwt.RegisteredClaims{ + ExpiresAt: jwt.NewNumericDate(time.Now().Add(ttl)), + IssuedAt: jwt.NewNumericDate(time.Now()), + Issuer: iss.String(), + Audience: []string{aud}, + ID: randomNonce(), + }, + } + if lexMethod != nil { + claims.LexMethod = lexMethod.String() + } + + var sm *signingMethodAtproto + + // NOTE: could also have a crypto.PrivateKey.Alg() method which returns a string + switch priv.(type) { + case *crypto.PrivateKeyP256: + sm = signingMethodES256 + case *crypto.PrivateKeyK256: + sm = signingMethodES256K + default: + return "", fmt.Errorf("unknown signing key type") + } + + token := jwt.NewWithClaims(sm, claims) + return token.SignedString(priv) +} diff --git a/atproto/auth/jwt_signing.go b/atproto/auth/jwt_signing.go new file mode 100644 index 00000000..d0142d35 --- /dev/null +++ b/atproto/auth/jwt_signing.go @@ -0,0 +1,88 @@ +package auth + +import ( + "crypto" + + atcrypto "github.com/bluesky-social/indigo/atproto/crypto" + "github.com/golang-jwt/jwt/v5" +) + +var ( + signingMethodES256K *signingMethodAtproto + signingMethodES256 *signingMethodAtproto + supportedAlgs []string +) + +// Implementation of jwt.SigningMethod for the `atproto/crypto` types. +type signingMethodAtproto struct { + alg string + hash crypto.Hash + toOutSig toOutSig + sigLen int +} + +type toOutSig func(sig []byte) []byte + +func init() { + // tells JWT library to serialize 'aud' as regular string, not array of strings (when signing) + jwt.MarshalSingleStringAsArray = false + + signingMethodES256K = &signingMethodAtproto{ + alg: "ES256K", + hash: crypto.SHA256, + toOutSig: toES256K, + sigLen: 64, + } + jwt.RegisterSigningMethod(signingMethodES256K.Alg(), func() jwt.SigningMethod { + return signingMethodES256K + }) + signingMethodES256 = &signingMethodAtproto{ + alg: "ES256", + hash: crypto.SHA256, + toOutSig: toES256, + sigLen: 64, + } + jwt.RegisterSigningMethod(signingMethodES256.Alg(), func() jwt.SigningMethod { + return signingMethodES256 + }) + supportedAlgs = []string{signingMethodES256K.Alg(), signingMethodES256.Alg()} +} + +func (sm *signingMethodAtproto) Verify(signingString string, sig []byte, key interface{}) error { + pub, ok := key.(atcrypto.PublicKey) + if !ok { + return jwt.ErrInvalidKeyType + } + + if !sm.hash.Available() { + return jwt.ErrHashUnavailable + } + + if len(sig) != sm.sigLen { + return jwt.ErrTokenSignatureInvalid + } + + // NOTE: important to use using "lenient" variant here + return pub.HashAndVerifyLenient([]byte(signingString), sig) +} + +func (sm *signingMethodAtproto) Sign(signingString string, key interface{}) ([]byte, error) { + priv, ok := key.(atcrypto.PrivateKey) + if !ok { + return nil, jwt.ErrInvalidKeyType + } + + return priv.HashAndSign([]byte(signingString)) +} + +func (sm *signingMethodAtproto) Alg() string { + return sm.alg +} + +func toES256K(sig []byte) []byte { + return sig[:64] +} + +func toES256(sig []byte) []byte { + return sig[:64] +} diff --git a/atproto/auth/jwt_test.go b/atproto/auth/jwt_test.go new file mode 100644 index 00000000..62004c7e --- /dev/null +++ b/atproto/auth/jwt_test.go @@ -0,0 +1,156 @@ +package auth + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/bluesky-social/indigo/atproto/crypto" + "github.com/bluesky-social/indigo/atproto/identity" + "github.com/bluesky-social/indigo/atproto/syntax" + + "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/assert" +) + +// Returns an early-2024 timestamp as a point in time for validating known JWTs (which contain expires-at) +func testTime() time.Time { + return time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC) +} + +func validateMinimal(token string, iss, aud string, pub crypto.PublicKey) error { + + p := jwt.NewParser( + jwt.WithValidMethods(supportedAlgs), + jwt.WithTimeFunc(testTime), + jwt.WithIssuer(iss), + jwt.WithAudience(aud), + ) + _, err := p.Parse(token, func(tok *jwt.Token) (any, error) { + return pub, nil + }) + if err != nil { + return fmt.Errorf("failed to parse auth header JWT: %w", err) + } + return nil +} + +func TestSignatureMethods(t *testing.T) { + assert := assert.New(t) + + jwtTestFixtures := []struct { + name string + pubkey string + iss string + aud string + jwt string + }{ + { + name: "secp256k1 (K-256)", + pubkey: "did:key:zQ3shscXNYZQZSPwegiv7uQZZV5kzATLBRtgJhs7uRY7pfSk4", + iss: "did:example:iss", + aud: "did:example:aud", + jwt: "eyJ0eXAiOiJKV1QiLCJhbGciOiJFUzI1NksifQ.eyJpc3MiOiJkaWQ6ZXhhbXBsZTppc3MiLCJhdWQiOiJkaWQ6ZXhhbXBsZTphdWQiLCJleHAiOjE3MTM1NzEwMTJ9.J_In_PQCMjygeeoIKyjybORD89ZnEy1bZTd--sdq_78qv3KCO9181ZAh-2Pl0qlXZjfUlxgIa6wiak2NtsT98g", + }, + { + name: "secp256k1 (K-256)", + pubkey: "did:key:zQ3shqKrpHzQ5HDfhgcYMWaFcpBK3SS39wZLdTjA5GeakX8G5", + iss: "did:example:iss", + aud: "did:example:aud", + jwt: "eyJ0eXAiOiJKV1QiLCJhbGciOiJFUzI1NksifQ.eyJhdWQiOiJkaWQ6ZXhhbXBsZTphdWQiLCJpc3MiOiJkaWQ6ZXhhbXBsZTppc3MiLCJleHAiOjE3MTM1NzExMzJ9.itNeYcF5oFMZIGxtnbJhE4McSniv_aR-Yk1Wj8uWk1K8YjlS2fzuJMo0-fILV3payETxn6r45f0FfpTaqY0EZQ", + }, + { + name: "P-256", + pubkey: "did:key:zDnaeXRDKRCEUoYxi8ZJS2pDsgfxUh3pZiu3SES9nbY4DoART", + iss: "did:example:iss", + aud: "did:example:aud", + jwt: "eyJ0eXAiOiJKV1QiLCJhbGciOiJFUzI1NiJ9.eyJpc3MiOiJkaWQ6ZXhhbXBsZTppc3MiLCJhdWQiOiJkaWQ6ZXhhbXBsZTphdWQiLCJleHAiOjE3MTM1NzE1NTR9.FFRLm7SGbDUp6cL0WoCs0L5oqNkjCXB963TqbgI-KxIjbiqMQATVCalcMJx17JGTjMmfVHJP6Op_V4Z0TTjqog", + }, + } + + for _, fix := range jwtTestFixtures { + + pubk, err := crypto.ParsePublicDIDKey(fix.pubkey) + if err != nil { + t.Fatal(err) + } + + assert.NoError(validateMinimal(fix.jwt, fix.iss, fix.aud, pubk)) + } +} + +func testSigningValidation(t *testing.T, priv crypto.PrivateKey) { + assert := assert.New(t) + ctx := context.Background() + + iss := syntax.DID("did:example:iss") + aud := "did:example:aud#svc" + lxm := syntax.NSID("com.example.api") + + priv, err := crypto.GeneratePrivateKeyP256() + if err != nil { + t.Fatal(err) + } + pub, err := priv.PublicKey() + if err != nil { + t.Fatal(err) + } + + dir := identity.NewMockDirectory() + dir.Insert(identity.Identity{ + DID: iss, + Keys: map[string]identity.Key{ + "atproto": identity.Key{ + Type: "Multikey", + PublicKeyMultibase: pub.Multibase(), + }, + }, + }) + + v := ServiceAuthValidator{ + Audience: aud, + Dir: &dir, + } + + t1, err := SignServiceAuth(iss, aud, time.Minute, nil, priv) + if err != nil { + t.Fatal(err) + } + d1, err := v.Validate(ctx, t1, nil) + assert.NoError(err) + assert.Equal(d1, iss) + _, err = v.Validate(ctx, t1, &lxm) + assert.Error(err) + + t2, err := SignServiceAuth(iss, aud, time.Minute, &lxm, priv) + if err != nil { + t.Fatal(err) + } + d2, err := v.Validate(ctx, t2, nil) + assert.NoError(err) + assert.Equal(d2, iss) + _, err = v.Validate(ctx, t2, &lxm) + assert.NoError(err) + + _, err = v.Validate(ctx, t2, nil) + assert.NoError(err) + _, err = v.Validate(ctx, t2, &lxm) + assert.NoError(err) +} + +func TestP256SigningValidation(t *testing.T) { + priv, err := crypto.GeneratePrivateKeyP256() + if err != nil { + t.Fatal(err) + } + testSigningValidation(t, priv) +} + +func TestK256SigningValidation(t *testing.T) { + priv, err := crypto.GeneratePrivateKeyK256() + if err != nil { + t.Fatal(err) + } + testSigningValidation(t, priv) +} -- 2.51.2 From ae36f112875c87ba80bb10482023b7985c6e8880 Mon Sep 17 00:00:00 2001 From: bryan newbold Date: Tue, 29 Apr 2025 00:25:22 -0700 Subject: [PATCH 2/6] go mod tidy --- go.mod | 1 + go.sum | 2 ++ 2 files changed, 3 insertions(+) diff --git a/go.mod b/go.mod index f96122c7..5778f195 100644 --- a/go.mod +++ b/go.mod @@ -18,6 +18,7 @@ require ( github.com/go-redis/cache/v9 v9.0.0 github.com/gocql/gocql v1.7.0 github.com/golang-jwt/jwt v3.2.2+incompatible + github.com/golang-jwt/jwt/v5 v5.2.2 github.com/gorilla/websocket v1.5.1 github.com/hashicorp/go-retryablehttp v0.7.5 github.com/hashicorp/golang-lru/arc/v2 v2.0.6 diff --git a/go.sum b/go.sum index 6583afdb..f542823c 100644 --- a/go.sum +++ b/go.sum @@ -117,6 +117,8 @@ github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= github.com/golang-jwt/jwt v3.2.2+incompatible h1:IfV12K8xAKAnZqdXVzCZ+TOjboZ2keLg81eXfW3O+oY= github.com/golang-jwt/jwt v3.2.2+incompatible/go.mod h1:8pz2t5EyA70fFQQSrl6XZXzqecmYZeUEB8OUGHkxJ+I= +github.com/golang-jwt/jwt/v5 v5.2.2 h1:Rl4B7itRWVtYIHFrSNd7vhTiz9UpLdi6gZhZ3wEeDy8= +github.com/golang-jwt/jwt/v5 v5.2.2/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.4.0-rc.1/go.mod h1:ceaxUfeHdC40wWswd/P6IGgMaK3YpKi5j83Wpe3EHw8= github.com/golang/protobuf v1.4.0-rc.1.0.20200221234624-67d41d38c208/go.mod h1:xKAWHe0F5eneWXFV3EuXVDTCmh+JuBKY0li0aMyXATA= -- 2.51.2 From 2bb06bfb67b77713ec6127da98fe004099b13c87 Mon Sep 17 00:00:00 2001 From: bryan newbold Date: Tue, 29 Apr 2025 01:44:37 -0700 Subject: [PATCH 3/6] HTTP middleware for admin auth and inter-service auth --- atproto/auth/http.go | 79 +++++++++++++++++++ atproto/auth/http_test.go | 161 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 240 insertions(+) create mode 100644 atproto/auth/http.go create mode 100644 atproto/auth/http_test.go diff --git a/atproto/auth/http.go b/atproto/auth/http.go new file mode 100644 index 00000000..36592fc2 --- /dev/null +++ b/atproto/auth/http.go @@ -0,0 +1,79 @@ +package auth + +import ( + "context" + "crypto/subtle" + "net/http" + "strings" + + "github.com/bluesky-social/indigo/atproto/syntax" +) + +// HTTP Middleware for atproto admin auth, which is HTTP Basic auth with the username "admin". +// +// This supports multiple admin passwords, which makes it easier to rotate service secrets. +// +// This can be used with `echo.WrapMiddleware` (part of the echo web framework) +func AdminAuthMiddleware(handler http.HandlerFunc, adminPasswords []string) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + username, password, ok := r.BasicAuth() + if ok && username == "admin" { + for _, pw := range adminPasswords { + if subtle.ConstantTimeCompare([]byte(pw), []byte(password)) == 1 { + handler(w, r) + return + } + } + } + w.Header().Set("WWW-Authenticate", `Basic realm="admin", charset="UTF-8"`) + // TODO: XRPC error body? + http.Error(w, "Unauthorized", http.StatusUnauthorized) + } +} + +// HTTP Middleware for inter-service auth, which is HTTP Bearer with JWT. +// +// 'mandatory' indicates whether valid inter-service auth must be present, or just optional. +func (v *ServiceAuthValidator) Middleware(handler http.HandlerFunc, mandatory bool) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + + if hdr := r.Header.Get("Authorization"); hdr != "" { + parts := strings.Split(hdr, " ") + if parts[0] != "Bearer" || len(parts) != 2 { + // TODO: XRPC error body? + w.Header().Set("WWW-Authenticate", "Bearer") + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return + } + + var lxm *syntax.NSID + uparts := strings.Split(r.URL.Path, "/") + // TODO: should this "fail closed"? eg, reject if not a valid XRPC endpoint + if len(uparts) >= 3 && uparts[1] == "xrpc" { + nsid, err := syntax.ParseNSID(uparts[2]) + if nil == err { + lxm = &nsid + } + } + + did, err := v.Validate(r.Context(), parts[1], lxm) + if err != nil { + w.Header().Set("WWW-Authenticate", "Bearer") + http.Error(w, "Unauthorized", http.StatusUnauthorized) + // TODO: XRPC error body? + return + } + ctx := context.WithValue(r.Context(), "did", did) + handler(w, r.WithContext(ctx)) + return + } + + if mandatory { + // TODO: XRPC error body? + w.Header().Set("WWW-Authenticate", "Bearer") + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return + } + handler(w, r) + } +} diff --git a/atproto/auth/http_test.go b/atproto/auth/http_test.go new file mode 100644 index 00000000..378ed542 --- /dev/null +++ b/atproto/auth/http_test.go @@ -0,0 +1,161 @@ +package auth + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/bluesky-social/indigo/atproto/crypto" + "github.com/bluesky-social/indigo/atproto/identity" + "github.com/bluesky-social/indigo/atproto/syntax" + + "github.com/stretchr/testify/assert" +) + +func webHome(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + w.WriteHeader(http.StatusOK) + did, ok := ctx.Value("did").(syntax.DID) + if ok { + w.Write([]byte(did.String())) + } else { + w.Write([]byte("hello world")) + } +} + +func TestAdminAuthMiddleware(t *testing.T) { + assert := assert.New(t) + + pw1 := "secret123" + pw2 := "secret789" + + req := httptest.NewRequest(http.MethodGet, "/", nil) + middle := AdminAuthMiddleware(webHome, []string{pw1, pw2}) + + { + resp := httptest.NewRecorder() + middle(resp, req) + assert.Equal(http.StatusUnauthorized, resp.Code) + } + + { + resp := httptest.NewRecorder() + req.SetBasicAuth("admin", pw1) + middle(resp, req) + assert.Equal(http.StatusOK, resp.Code) + } + + { + resp := httptest.NewRecorder() + req.SetBasicAuth("admin", pw2) + middle(resp, req) + assert.Equal(http.StatusOK, resp.Code) + } + + { + resp := httptest.NewRecorder() + req.SetBasicAuth("wrong", pw2) + middle(resp, req) + assert.Equal(http.StatusUnauthorized, resp.Code) + } + + { + resp := httptest.NewRecorder() + req.SetBasicAuth("admin", "wrong") + middle(resp, req) + assert.Equal(http.StatusUnauthorized, resp.Code) + } +} + +func TestServiceAuthMiddleware(t *testing.T) { + assert := assert.New(t) + + iss := syntax.DID("did:example:iss") + aud := "did:example:aud#svc" + lxm := syntax.NSID("com.example.api") + + priv, err := crypto.GeneratePrivateKeyP256() + if err != nil { + t.Fatal(err) + } + pub, err := priv.PublicKey() + if err != nil { + t.Fatal(err) + } + + dir := identity.NewMockDirectory() + dir.Insert(identity.Identity{ + DID: iss, + Keys: map[string]identity.Key{ + "atproto": identity.Key{ + Type: "Multikey", + PublicKeyMultibase: pub.Multibase(), + }, + }, + }) + + v := ServiceAuthValidator{ + Audience: aud, + Dir: &dir, + } + + { + // optional middleware, no auth + req := httptest.NewRequest(http.MethodGet, "/xrpc/com.example.api", nil) + middle := v.Middleware(webHome, false) + resp := httptest.NewRecorder() + middle(resp, req) + assert.Equal(http.StatusOK, resp.Code) + assert.Equal("hello world", string(resp.Body.Bytes())) + } + + { + // mandatory middleware, no auth + req := httptest.NewRequest(http.MethodGet, "/xrpc/com.example.api", nil) + middle := v.Middleware(webHome, true) + resp := httptest.NewRecorder() + middle(resp, req) + assert.Equal(http.StatusUnauthorized, resp.Code) + } + + { + // mandatory middleware, valid auth + tok, err := SignServiceAuth(iss, aud, time.Minute, &lxm, priv) + if err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodGet, "/xrpc/com.example.api", nil) + req.Header.Set("Authorization", "Bearer "+tok) + middle := v.Middleware(webHome, true) + resp := httptest.NewRecorder() + middle(resp, req) + assert.Equal(http.StatusOK, resp.Code) + assert.Equal(iss.String(), string(resp.Body.Bytes())) + } + + { + // mangled header + req := httptest.NewRequest(http.MethodGet, "/xrpc/com.example.api", nil) + req.Header.Set("Authorization", "Bearer dummy") + middle := v.Middleware(webHome, false) + resp := httptest.NewRecorder() + middle(resp, req) + assert.Equal(http.StatusUnauthorized, resp.Code) + } + + { + // wrong path + tok, err := SignServiceAuth(iss, aud, time.Minute, &lxm, priv) + if err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodGet, "/xrpc/com.example.other.api", nil) + req.Header.Set("Authorization", "Bearer "+tok) + middle := v.Middleware(webHome, true) + resp := httptest.NewRecorder() + middle(resp, req) + assert.Equal(http.StatusUnauthorized, resp.Code) + } +} -- 2.51.2 From c91628aa2de41e7f786e5ae8baa608ac519b401b Mon Sep 17 00:00:00 2001 From: bryan newbold Date: Mon, 2 Jun 2025 23:44:51 -0700 Subject: [PATCH 4/6] updates from code review --- atproto/auth/http_test.go | 18 ++++++------------ atproto/auth/jwt.go | 8 ++++---- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/atproto/auth/http_test.go b/atproto/auth/http_test.go index 378ed542..ad3577f6 100644 --- a/atproto/auth/http_test.go +++ b/atproto/auth/http_test.go @@ -11,6 +11,7 @@ import ( "github.com/bluesky-social/indigo/atproto/syntax" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func webHome(w http.ResponseWriter, r *http.Request) { @@ -71,19 +72,16 @@ func TestAdminAuthMiddleware(t *testing.T) { func TestServiceAuthMiddleware(t *testing.T) { assert := assert.New(t) + require := require.New(t) iss := syntax.DID("did:example:iss") aud := "did:example:aud#svc" lxm := syntax.NSID("com.example.api") priv, err := crypto.GeneratePrivateKeyP256() - if err != nil { - t.Fatal(err) - } + require.NoError(err) pub, err := priv.PublicKey() - if err != nil { - t.Fatal(err) - } + require.NoError(err) dir := identity.NewMockDirectory() dir.Insert(identity.Identity{ @@ -123,9 +121,7 @@ func TestServiceAuthMiddleware(t *testing.T) { { // mandatory middleware, valid auth tok, err := SignServiceAuth(iss, aud, time.Minute, &lxm, priv) - if err != nil { - t.Fatal(err) - } + require.NoError(err) req := httptest.NewRequest(http.MethodGet, "/xrpc/com.example.api", nil) req.Header.Set("Authorization", "Bearer "+tok) middle := v.Middleware(webHome, true) @@ -148,9 +144,7 @@ func TestServiceAuthMiddleware(t *testing.T) { { // wrong path tok, err := SignServiceAuth(iss, aud, time.Minute, &lxm, priv) - if err != nil { - t.Fatal(err) - } + require.NoError(err) req := httptest.NewRequest(http.MethodGet, "/xrpc/com.example.other.api", nil) req.Header.Set("Authorization", "Bearer "+tok) middle := v.Middleware(webHome, true) diff --git a/atproto/auth/jwt.go b/atproto/auth/jwt.go index f29b298c..c3d7acc5 100644 --- a/atproto/auth/jwt.go +++ b/atproto/auth/jwt.go @@ -2,11 +2,11 @@ package auth import ( "context" + "crypto/rand" "encoding/base64" "errors" "fmt" "log/slog" - "math/rand" "time" "github.com/bluesky-social/indigo/atproto/crypto" @@ -91,11 +91,11 @@ func (s *ServiceAuthValidator) fetchIssuerKeyFunc(ctx context.Context) func(toke return func(token *jwt.Token) (any, error) { claims, ok := token.Claims.(*serviceAuthClaims) if !ok { - return nil, fmt.Errorf("%w: missing 'iss'", jwt.ErrTokenInvalidClaims) + return nil, jwt.ErrTokenInvalidClaims } iss, err := claims.GetIssuer() if err != nil { - return nil, fmt.Errorf("%w: missing 'iss'", jwt.ErrTokenInvalidClaims) + return nil, fmt.Errorf("%w: missing 'iss' claim", jwt.ErrTokenInvalidIssuer) } did, err := syntax.ParseDID(iss) if err != nil { @@ -139,7 +139,7 @@ func SignServiceAuth(iss syntax.DID, aud string, ttl time.Duration, lexMethod *s case *crypto.PrivateKeyK256: sm = signingMethodES256K default: - return "", fmt.Errorf("unknown signing key type") + return "", fmt.Errorf("unknown signing key type: %T", priv) } token := jwt.NewWithClaims(sm, claims) -- 2.51.2 From f3a0794d7f7f48902515c8571138e9b679fb3d51 Mon Sep 17 00:00:00 2001 From: bryan newbold Date: Sun, 15 Jun 2025 16:15:32 -0700 Subject: [PATCH 5/6] configurable JWT timestamp leeway --- atproto/auth/jwt.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/atproto/auth/jwt.go b/atproto/auth/jwt.go index c3d7acc5..429c82af 100644 --- a/atproto/auth/jwt.go +++ b/atproto/auth/jwt.go @@ -20,8 +20,9 @@ import ( type ServiceAuthValidator struct { // Service DID reference for this validator: a DID with optional #-separated fragment - Audience string - Dir identity.Directory + Audience string + Dir identity.Directory + TimestampLeeway time.Duration } type serviceAuthClaims struct { @@ -32,12 +33,17 @@ type serviceAuthClaims struct { func (s *ServiceAuthValidator) Validate(ctx context.Context, tokenString string, lexMethod *syntax.NSID) (syntax.DID, error) { + leeway := s.TimestampLeeway + if leeway == 0 { + leeway = 5 * time.Second + } + opts := []jwt.ParserOption{ jwt.WithValidMethods(supportedAlgs), jwt.WithAudience(s.Audience), jwt.WithExpirationRequired(), jwt.WithIssuedAt(), - jwt.WithLeeway(5 * time.Second), // TODO: configurable? better default? + jwt.WithLeeway(leeway), } token, err := jwt.ParseWithClaims(tokenString, &serviceAuthClaims{}, s.fetchIssuerKeyFunc(ctx), opts...) @@ -73,7 +79,7 @@ func (s *ServiceAuthValidator) Validate(ctx context.Context, tokenString string, } claims, ok := token.Claims.(*serviceAuthClaims) if !ok { - // TODO: is this the best error here? + // TODO: is the error message returned descriptive enough? return "", jwt.ErrTokenInvalidClaims } -- 2.51.2 From 9cd1355802d23e5f57bc526fe8831854de14bbda Mon Sep 17 00:00:00 2001 From: bryan newbold Date: Sun, 15 Jun 2025 16:24:47 -0700 Subject: [PATCH 6/6] use atproto JSON error responses --- atproto/auth/http.go | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/atproto/auth/http.go b/atproto/auth/http.go index 36592fc2..6b51d216 100644 --- a/atproto/auth/http.go +++ b/atproto/auth/http.go @@ -3,6 +3,8 @@ package auth import ( "context" "crypto/subtle" + "encoding/json" + "fmt" "net/http" "strings" @@ -26,8 +28,12 @@ func AdminAuthMiddleware(handler http.HandlerFunc, adminPasswords []string) http } } w.Header().Set("WWW-Authenticate", `Basic realm="admin", charset="UTF-8"`) - // TODO: XRPC error body? - http.Error(w, "Unauthorized", http.StatusUnauthorized) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + json.NewEncoder(w).Encode(map[string]string{ + "error": "Unauthorized", + "message": "atproto admin auth required, but missing or incorrect password", + }) } } @@ -40,9 +46,13 @@ func (v *ServiceAuthValidator) Middleware(handler http.HandlerFunc, mandatory bo if hdr := r.Header.Get("Authorization"); hdr != "" { parts := strings.Split(hdr, " ") if parts[0] != "Bearer" || len(parts) != 2 { - // TODO: XRPC error body? w.Header().Set("WWW-Authenticate", "Bearer") - http.Error(w, "Unauthorized", http.StatusUnauthorized) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + json.NewEncoder(w).Encode(map[string]string{ + "error": "Unauthorized", + "message": "atproto service auth required, but missing or incorrect formatting", + }) return } @@ -59,8 +69,12 @@ func (v *ServiceAuthValidator) Middleware(handler http.HandlerFunc, mandatory bo did, err := v.Validate(r.Context(), parts[1], lxm) if err != nil { w.Header().Set("WWW-Authenticate", "Bearer") - http.Error(w, "Unauthorized", http.StatusUnauthorized) - // TODO: XRPC error body? + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + json.NewEncoder(w).Encode(map[string]string{ + "error": "Unauthorized", + "message": fmt.Sprintf("invalid service auth: %s", err), + }) return } ctx := context.WithValue(r.Context(), "did", did) @@ -69,9 +83,13 @@ func (v *ServiceAuthValidator) Middleware(handler http.HandlerFunc, mandatory bo } if mandatory { - // TODO: XRPC error body? w.Header().Set("WWW-Authenticate", "Bearer") - http.Error(w, "Unauthorized", http.StatusUnauthorized) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + json.NewEncoder(w).Encode(map[string]string{ + "error": "Unauthorized", + "message": "atproto service auth required", + }) return } handler(w, r) -- 2.51.2