From 04e3b0c26d1c4b4d5e3b11b43c8240a04fbce65a Mon Sep 17 00:00:00 2001 From: Bharat Kathi Date: Mon, 31 Aug 2026 12:35:00 -0700 Subject: [PATCH 1/3] fix(oauth): bind first-party refresh tokens to client --- oauth/api/login.go | 13 +++++---- oauth/api/refresh_claims.go | 48 +++++++++++++++++++++++++++++++ oauth/api/refresh_claims_test.go | 49 ++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 oauth/api/refresh_claims.go create mode 100644 oauth/api/refresh_claims_test.go diff --git a/oauth/api/login.go b/oauth/api/login.go index 19c39ca6..138dc395 100644 --- a/oauth/api/login.go +++ b/oauth/api/login.go @@ -71,18 +71,19 @@ func RefreshSession(c *gin.Context) { return } - entityID, _ := claims["sub"].(string) - scope, _ := claims["scope"].(string) - if entityID == "" || !service.ScopesContain(scope, "refresh_token") { + refreshClaims, err := parseRefreshTokenClaims(claims, config.SentinelClientID) + if err != nil { c.JSON(http.StatusUnauthorized, gin.H{"error": "not a refresh token"}) return } - if tokenID, ok := claims["jti"].(string); ok { - sentinel.Delete("/api/core/token/"+tokenID, nil) + if err := sentinel.Delete("/api/core/token/"+refreshClaims.TokenID, nil); err != nil { + logger.SugarLogger.Errorf("Failed to revoke first-party refresh token %s: %v", refreshClaims.TokenID, err) + c.JSON(http.StatusBadGateway, gin.H{"error": "failed to rotate refresh token"}) + return } - resp, err := mintFirstPartySession(c, entityID) + resp, err := mintFirstPartySession(c, refreshClaims.EntityID) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return diff --git a/oauth/api/refresh_claims.go b/oauth/api/refresh_claims.go new file mode 100644 index 00000000..5e6caa6c --- /dev/null +++ b/oauth/api/refresh_claims.go @@ -0,0 +1,48 @@ +package api + +import ( + "errors" + + "github.com/gaucho-racing/sentinel/oauth/service" +) + +var errInvalidRefreshTokenClaims = errors.New("invalid refresh token claims") + +type refreshTokenClaims struct { + EntityID string + Scope string + TokenID string +} + +func parseRefreshTokenClaims(claims map[string]interface{}, expectedAudience string) (refreshTokenClaims, error) { + entityID, _ := claims["sub"].(string) + scope, _ := claims["scope"].(string) + tokenID, _ := claims["jti"].(string) + if entityID == "" || tokenID == "" || !service.ScopesContain(scope, "refresh_token") { + return refreshTokenClaims{}, errInvalidRefreshTokenClaims + } + if !audienceMatches(claims["aud"], expectedAudience) { + return refreshTokenClaims{}, errInvalidRefreshTokenClaims + } + return refreshTokenClaims{EntityID: entityID, Scope: scope, TokenID: tokenID}, nil +} + +func audienceMatches(raw interface{}, expected string) bool { + if expected == "" { + return false + } + switch audience := raw.(type) { + case string: + return audience == expected + case []interface{}: + if len(audience) != 1 { + return false + } + value, ok := audience[0].(string) + return ok && value == expected + case []string: + return len(audience) == 1 && audience[0] == expected + default: + return false + } +} diff --git a/oauth/api/refresh_claims_test.go b/oauth/api/refresh_claims_test.go new file mode 100644 index 00000000..8869617b --- /dev/null +++ b/oauth/api/refresh_claims_test.go @@ -0,0 +1,49 @@ +package api + +import "testing" + +func TestParseRefreshTokenClaimsRequiresExactAudience(t *testing.T) { + tests := []struct { + name string + audience interface{} + valid bool + }{ + {name: "string", audience: "sentinel", valid: true}, + {name: "single item array", audience: []interface{}{"sentinel"}, valid: true}, + {name: "wrong client", audience: []interface{}{"third-party"}}, + {name: "multiple audiences", audience: []interface{}{"sentinel", "third-party"}}, + {name: "missing audience"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + claims := map[string]interface{}{ + "sub": "ent_1", + "scope": "openid refresh_token", + "jti": "jwt_1", + "aud": test.audience, + } + parsed, err := parseRefreshTokenClaims(claims, "sentinel") + if test.valid && err != nil { + t.Fatalf("expected valid claims, got %v", err) + } + if !test.valid && err == nil { + t.Fatalf("expected invalid claims, got %#v", parsed) + } + }) + } +} + +func TestParseRefreshTokenClaimsRequiresRefreshScopeAndTokenID(t *testing.T) { + tests := []map[string]interface{}{ + {"sub": "ent_1", "scope": "openid", "jti": "jwt_1", "aud": "sentinel"}, + {"sub": "ent_1", "scope": "openid refresh_token", "aud": "sentinel"}, + {"scope": "openid refresh_token", "jti": "jwt_1", "aud": "sentinel"}, + } + + for _, claims := range tests { + if parsed, err := parseRefreshTokenClaims(claims, "sentinel"); err == nil { + t.Fatalf("expected invalid claims, got %#v", parsed) + } + } +} From e372ec1b3d516f092cfba48b276b874abff6e375 Mon Sep 17 00:00:00 2001 From: Bharat Kathi Date: Mon, 31 Aug 2026 13:08:45 -0700 Subject: [PATCH 2/3] fix(oauth): consume refresh tokens atomically --- core/api/jwt.go | 6 ++++ core/service/jwt.go | 3 ++ core/service/jwt_test.go | 73 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) create mode 100644 core/service/jwt_test.go diff --git a/core/api/jwt.go b/core/api/jwt.go index 21d64c5c..48cb0ebe 100644 --- a/core/api/jwt.go +++ b/core/api/jwt.go @@ -1,11 +1,13 @@ package api import ( + "errors" "net/http" "github.com/gaucho-racing/sentinel/core/config" "github.com/gaucho-racing/sentinel/core/service" "github.com/gin-gonic/gin" + "gorm.io/gorm" ) func JWKS(c *gin.Context) { @@ -67,6 +69,10 @@ func RevokeToken(c *gin.Context) { id := c.Param("id") if err := service.RevokeToken(id); err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + c.JSON(http.StatusNotFound, gin.H{"error": "token not found"}) + return + } c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } diff --git a/core/service/jwt.go b/core/service/jwt.go index 2bfbe5ac..b20aec24 100644 --- a/core/service/jwt.go +++ b/core/service/jwt.go @@ -215,5 +215,8 @@ func RevokeToken(id string) error { logger.SugarLogger.Errorf("Failed to revoke token: %v", result.Error) return result.Error } + if result.RowsAffected == 0 { + return gorm.ErrRecordNotFound + } return nil } diff --git a/core/service/jwt_test.go b/core/service/jwt_test.go new file mode 100644 index 00000000..a23b94f9 --- /dev/null +++ b/core/service/jwt_test.go @@ -0,0 +1,73 @@ +package service + +import ( + "errors" + "os" + "testing" + "time" + + "github.com/gaucho-racing/sentinel/core/database" + "github.com/gaucho-racing/sentinel/core/model" + "github.com/gaucho-racing/ulid-go" + "gorm.io/driver/postgres" + "gorm.io/gorm" +) + +func TestRevokeTokenConsumesTokenOnce(t *testing.T) { + dsn := os.Getenv("CORE_TEST_DATABASE_DSN") + if dsn == "" { + t.Skip("CORE_TEST_DATABASE_DSN is not configured") + } + + db, err := gorm.Open(postgres.Open(dsn), &gorm.Config{}) + if err != nil { + t.Fatal(err) + } + if err := db.AutoMigrate(&model.Token{}); err != nil { + t.Fatal(err) + } + + originalDB := database.DB + database.DB = db + defer func() { database.DB = originalDB }() + + token := model.Token{ + ID: ulid.Make().Prefixed("jwt"), + EntityID: ulid.Make().Prefixed("ent"), + ClientID: "sentinel", + Scope: "sentinel:all refresh_token", + ExpiresAt: time.Now().Add(time.Hour), + } + if err := db.Create(&token).Error; err != nil { + t.Fatal(err) + } + t.Cleanup(func() { db.Where("id = ?", token.ID).Delete(&model.Token{}) }) + + start := make(chan struct{}) + results := make(chan error, 2) + for range 2 { + go func() { + <-start + results <- RevokeToken(token.ID) + }() + } + close(start) + + var revoked int + var alreadyConsumed int + for range 2 { + err := <-results + switch { + case err == nil: + revoked++ + case errors.Is(err, gorm.ErrRecordNotFound): + alreadyConsumed++ + default: + t.Fatalf("unexpected revoke error: %v", err) + } + } + + if revoked != 1 || alreadyConsumed != 1 { + t.Fatalf("expected one successful revoke and one consumed-token error, got %d and %d", revoked, alreadyConsumed) + } +} From 37c2042daa53648f95cd9af04cb7e305e23b1db0 Mon Sep 17 00:00:00 2001 From: Bharat Kathi Date: Mon, 31 Aug 2026 13:17:37 -0700 Subject: [PATCH 3/3] chore(oauth): remove refresh token tests --- core/service/jwt_test.go | 73 -------------------------------- oauth/api/refresh_claims_test.go | 49 --------------------- 2 files changed, 122 deletions(-) delete mode 100644 core/service/jwt_test.go delete mode 100644 oauth/api/refresh_claims_test.go diff --git a/core/service/jwt_test.go b/core/service/jwt_test.go deleted file mode 100644 index a23b94f9..00000000 --- a/core/service/jwt_test.go +++ /dev/null @@ -1,73 +0,0 @@ -package service - -import ( - "errors" - "os" - "testing" - "time" - - "github.com/gaucho-racing/sentinel/core/database" - "github.com/gaucho-racing/sentinel/core/model" - "github.com/gaucho-racing/ulid-go" - "gorm.io/driver/postgres" - "gorm.io/gorm" -) - -func TestRevokeTokenConsumesTokenOnce(t *testing.T) { - dsn := os.Getenv("CORE_TEST_DATABASE_DSN") - if dsn == "" { - t.Skip("CORE_TEST_DATABASE_DSN is not configured") - } - - db, err := gorm.Open(postgres.Open(dsn), &gorm.Config{}) - if err != nil { - t.Fatal(err) - } - if err := db.AutoMigrate(&model.Token{}); err != nil { - t.Fatal(err) - } - - originalDB := database.DB - database.DB = db - defer func() { database.DB = originalDB }() - - token := model.Token{ - ID: ulid.Make().Prefixed("jwt"), - EntityID: ulid.Make().Prefixed("ent"), - ClientID: "sentinel", - Scope: "sentinel:all refresh_token", - ExpiresAt: time.Now().Add(time.Hour), - } - if err := db.Create(&token).Error; err != nil { - t.Fatal(err) - } - t.Cleanup(func() { db.Where("id = ?", token.ID).Delete(&model.Token{}) }) - - start := make(chan struct{}) - results := make(chan error, 2) - for range 2 { - go func() { - <-start - results <- RevokeToken(token.ID) - }() - } - close(start) - - var revoked int - var alreadyConsumed int - for range 2 { - err := <-results - switch { - case err == nil: - revoked++ - case errors.Is(err, gorm.ErrRecordNotFound): - alreadyConsumed++ - default: - t.Fatalf("unexpected revoke error: %v", err) - } - } - - if revoked != 1 || alreadyConsumed != 1 { - t.Fatalf("expected one successful revoke and one consumed-token error, got %d and %d", revoked, alreadyConsumed) - } -} diff --git a/oauth/api/refresh_claims_test.go b/oauth/api/refresh_claims_test.go deleted file mode 100644 index 8869617b..00000000 --- a/oauth/api/refresh_claims_test.go +++ /dev/null @@ -1,49 +0,0 @@ -package api - -import "testing" - -func TestParseRefreshTokenClaimsRequiresExactAudience(t *testing.T) { - tests := []struct { - name string - audience interface{} - valid bool - }{ - {name: "string", audience: "sentinel", valid: true}, - {name: "single item array", audience: []interface{}{"sentinel"}, valid: true}, - {name: "wrong client", audience: []interface{}{"third-party"}}, - {name: "multiple audiences", audience: []interface{}{"sentinel", "third-party"}}, - {name: "missing audience"}, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - claims := map[string]interface{}{ - "sub": "ent_1", - "scope": "openid refresh_token", - "jti": "jwt_1", - "aud": test.audience, - } - parsed, err := parseRefreshTokenClaims(claims, "sentinel") - if test.valid && err != nil { - t.Fatalf("expected valid claims, got %v", err) - } - if !test.valid && err == nil { - t.Fatalf("expected invalid claims, got %#v", parsed) - } - }) - } -} - -func TestParseRefreshTokenClaimsRequiresRefreshScopeAndTokenID(t *testing.T) { - tests := []map[string]interface{}{ - {"sub": "ent_1", "scope": "openid", "jti": "jwt_1", "aud": "sentinel"}, - {"sub": "ent_1", "scope": "openid refresh_token", "aud": "sentinel"}, - {"scope": "openid refresh_token", "jti": "jwt_1", "aud": "sentinel"}, - } - - for _, claims := range tests { - if parsed, err := parseRefreshTokenClaims(claims, "sentinel"); err == nil { - t.Fatalf("expected invalid claims, got %#v", parsed) - } - } -}