From 7322b402337a89078156b8c3d76c44d3bb29be60 Mon Sep 17 00:00:00 2001 From: Bharat Kathi Date: Mon, 31 Aug 2026 12:33:01 -0700 Subject: [PATCH 1/3] fix(oauth): bind authorization grants to bearer subject --- oauth/api/api.go | 5 +- oauth/api/auth.go | 71 +++++++++++++++++++++++++++ oauth/api/authorize.go | 58 +++++++++------------- oauth/api/authorize_test.go | 49 ++++++++++++++++++ web/src/pages/oauth/AuthorizePage.tsx | 2 - 5 files changed, 145 insertions(+), 40 deletions(-) create mode 100644 oauth/api/auth.go create mode 100644 oauth/api/authorize_test.go diff --git a/oauth/api/api.go b/oauth/api/api.go index aac3b6d5..9aa4c068 100644 --- a/oauth/api/api.go +++ b/oauth/api/api.go @@ -30,13 +30,14 @@ func InitializeRouter() *gin.Engine { MaxAge: 12 * time.Hour, AllowCredentials: true, })) + r.Use(UnauthorizedPanicHandler()) return r } func InitializeRoutes(router *gin.Engine) { router.GET("/oauth/ping", Ping) - router.GET("/oauth/authorize", ValidateAuthorize) - router.POST("/oauth/authorize", Authorize) + router.GET("/oauth/authorize", AuthChecker(), ValidateAuthorize) + router.POST("/oauth/authorize", AuthChecker(), Authorize) router.POST("/oauth/token", ExchangeToken) router.GET("/oauth/userinfo", UserInfo) router.POST("/oauth/userinfo", UserInfo) diff --git a/oauth/api/auth.go b/oauth/api/auth.go new file mode 100644 index 00000000..b27aed8e --- /dev/null +++ b/oauth/api/auth.go @@ -0,0 +1,71 @@ +package api + +import ( + "net/http" + "strings" + + "github.com/gaucho-racing/sentinel/oauth/pkg/logger" + "github.com/gaucho-racing/sentinel/oauth/pkg/sentinel" + "github.com/gin-gonic/gin" +) + +func AuthChecker() gin.HandlerFunc { + return func(c *gin.Context) { + authHeader := c.GetHeader("Authorization") + if !strings.HasPrefix(authHeader, "Bearer ") { + c.Next() + return + } + + token := strings.TrimPrefix(authHeader, "Bearer ") + if token == "" { + c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "invalid bearer token"}) + return + } + + var claims map[string]interface{} + if err := sentinel.Post("/api/core/token/validate", map[string]string{"token": token}, &claims); err != nil { + logger.SugarLogger.Errorf("Failed to validate token: %v", err) + c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "invalid bearer token"}) + return + } + + entityID, _ := claims["sub"].(string) + if entityID == "" { + c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "token subject is required"}) + return + } + + c.Set("Auth-Token", token) + c.Set("Auth-EntityID", entityID) + c.Next() + } +} + +func UnauthorizedPanicHandler() gin.HandlerFunc { + return func(c *gin.Context) { + defer func() { + if recovered := recover(); recovered != nil { + if recovered == "Unauthorized" { + c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "you are not authorized to access this resource"}) + return + } + logger.SugarLogger.Errorf("Unexpected panic: %v", recovered) + c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"error": "internal error"}) + } + }() + c.Next() + } +} + +func Require(c *gin.Context, condition bool) { + if !condition { + panic("Unauthorized") + } +} + +func GetRequestTokenEntityID(c *gin.Context) string { + entityID, _ := c.Get("Auth-EntityID") + value, _ := entityID.(string) + return value +} diff --git a/oauth/api/authorize.go b/oauth/api/authorize.go index 3886aca5..f449e7af 100644 --- a/oauth/api/authorize.go +++ b/oauth/api/authorize.go @@ -47,6 +47,9 @@ type validateAuthorizeResponse struct { // ValidateAuthorize validates the OAuth authorize request parameters // and returns application info for the frontend consent screen. func ValidateAuthorize(c *gin.Context) { + entityID := GetRequestTokenEntityID(c) + Require(c, entityID != "") + clientID := c.Query("client_id") if clientID == "" { c.JSON(http.StatusBadRequest, gin.H{"error": "client_id is required"}) @@ -101,36 +104,27 @@ func ValidateAuthorize(c *gin.Context) { return } - // Enforce the access gate here (not only at the authorize/token steps) so a - // user who doesn't qualify gets a clear error page up front, instead of a - // consent screen followed by a redirect back to the client with - // access_denied. entity_id is supplied by the SPA from the active session. - entityID := c.Query("entity_id") - if entityID != "" { - if err := service.CheckAccessGate(entityID, clientID); err != nil { - if errors.Is(err, service.ErrAccessDenied) { - c.JSON(http.StatusForbidden, gin.H{"error": "access_denied", "app_name": app.Name, "app_icon_url": app.IconURL}) - return - } - logger.SugarLogger.Errorf("access gate evaluation failed: %v", err) - c.JSON(http.StatusBadGateway, gin.H{"error": "server_error"}) + if err := service.CheckAccessGate(entityID, clientID); err != nil { + if errors.Is(err, service.ErrAccessDenied) { + c.JSON(http.StatusForbidden, gin.H{"error": "access_denied", "app_name": app.Name, "app_icon_url": app.IconURL}) return } + logger.SugarLogger.Errorf("access gate evaluation failed: %v", err) + c.JSON(http.StatusBadGateway, gin.H{"error": "server_error"}) + return } // Default to a consent prompt. If the user already authorized this exact // client+scope set within the last 24h, skip the screen and auto-approve. prompt := "consent" - if entityID != "" { - q := url.Values{} - q.Set("client_id", clientID) - q.Set("scope", scope) - q.Set("after", time.Now().Add(-24*time.Hour).Format(time.RFC3339)) - q.Set("limit", "1") - var logins []map[string]interface{} - if err := sentinel.Get(fmt.Sprintf("/api/core/entity/%s/logins?%s", entityID, q.Encode()), &logins); err == nil && len(logins) > 0 { - prompt = "none" - } + q := url.Values{} + q.Set("client_id", clientID) + q.Set("scope", scope) + q.Set("after", time.Now().Add(-24*time.Hour).Format(time.RFC3339)) + q.Set("limit", "1") + var logins []map[string]interface{} + if err := sentinel.Get(fmt.Sprintf("/api/core/entity/%s/logins?%s", entityID, q.Encode()), &logins); err == nil && len(logins) > 0 { + prompt = "none" } c.JSON(http.StatusOK, validateAuthorizeResponse{ @@ -143,13 +137,11 @@ func ValidateAuthorize(c *gin.Context) { }) } -type authorizeRequest struct { - EntityID string `json:"entity_id" binding:"required"` -} - // Authorize generates an authorization code after the user approves consent. -// The frontend sends the entity_id of the authenticated user. func Authorize(c *gin.Context) { + entityID := GetRequestTokenEntityID(c) + Require(c, entityID != "") + clientID := c.Query("client_id") redirectURI := c.Query("redirect_uri") scope := c.Query("scope") @@ -159,23 +151,17 @@ func Authorize(c *gin.Context) { return } - var req authorizeRequest - if err := c.ShouldBindJSON(&req); err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) - return - } - if !service.ValidateScopes(scope) || service.ScopesContain(scope, "sentinel:all") { c.JSON(http.StatusBadRequest, gin.H{"error": "invalid scope"}) return } - if err := service.CheckAccessGate(req.EntityID, clientID); err != nil { + if err := service.CheckAccessGate(entityID, clientID); err != nil { writeGateError(c, err) return } - authCode, err := service.GenerateAuthorizationCode(req.EntityID, clientID, scope, redirectURI, c.Query("nonce")) + authCode, err := service.GenerateAuthorizationCode(entityID, clientID, scope, redirectURI, c.Query("nonce")) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return diff --git a/oauth/api/authorize_test.go b/oauth/api/authorize_test.go new file mode 100644 index 00000000..97769c36 --- /dev/null +++ b/oauth/api/authorize_test.go @@ -0,0 +1,49 @@ +package api + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-gonic/gin" +) + +func TestAuthorizeRejectsCallerSuppliedEntityWithoutBearer(t *testing.T) { + gin.SetMode(gin.TestMode) + router := InitializeRouter() + InitializeRoutes(router) + + request := httptest.NewRequest( + http.MethodPost, + "/oauth/authorize?client_id=client&redirect_uri=https%3A%2F%2Fexample.com%2Fcallback&scope=openid", + strings.NewReader(`{"entity_id":"ent_victim"}`), + ) + request.Header.Set("Content-Type", "application/json") + response := httptest.NewRecorder() + + router.ServeHTTP(response, request) + + if response.Code != http.StatusUnauthorized { + t.Fatalf("expected %d, got %d: %s", http.StatusUnauthorized, response.Code, response.Body.String()) + } +} + +func TestValidateAuthorizeRequiresBearer(t *testing.T) { + gin.SetMode(gin.TestMode) + router := InitializeRouter() + InitializeRoutes(router) + + request := httptest.NewRequest( + http.MethodGet, + "/oauth/authorize?client_id=client&redirect_uri=https%3A%2F%2Fexample.com%2Fcallback&scope=openid&entity_id=ent_victim", + nil, + ) + response := httptest.NewRecorder() + + router.ServeHTTP(response, request) + + if response.Code != http.StatusUnauthorized { + t.Fatalf("expected %d, got %d: %s", http.StatusUnauthorized, response.Code, response.Body.String()) + } +} diff --git a/web/src/pages/oauth/AuthorizePage.tsx b/web/src/pages/oauth/AuthorizePage.tsx index 0f04cf36..1cbf09d6 100644 --- a/web/src/pages/oauth/AuthorizePage.tsx +++ b/web/src/pages/oauth/AuthorizePage.tsx @@ -97,7 +97,6 @@ export default function AuthorizePage() { client_id: clientId ?? "", redirect_uri: redirectUri, scope, - entity_id: session?.entityId ?? "", }) const res = await api.get(`/oauth/authorize?${search.toString()}`) return res.data @@ -129,7 +128,6 @@ export default function AuthorizePage() { if (nonce) search.set("nonce", nonce) const res = await api.post<{ code: string; redirect_uri: string }>( `/oauth/authorize?${search.toString()}`, - { entity_id: session?.entityId }, ) setSuccess(true) await new Promise((resolve) => From 6c95cf3d768584e73b030e9388165f3b8d869fee Mon Sep 17 00:00:00 2001 From: Bharat Kathi Date: Mon, 31 Aug 2026 13:07:18 -0700 Subject: [PATCH 2/3] fix(oauth): preserve upstream bearer validation errors --- oauth/api/auth.go | 15 ++++++++++-- oauth/api/auth_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 oauth/api/auth_test.go diff --git a/oauth/api/auth.go b/oauth/api/auth.go index b27aed8e..c0396348 100644 --- a/oauth/api/auth.go +++ b/oauth/api/auth.go @@ -1,6 +1,7 @@ package api import ( + "errors" "net/http" "strings" @@ -25,8 +26,7 @@ func AuthChecker() gin.HandlerFunc { var claims map[string]interface{} if err := sentinel.Post("/api/core/token/validate", map[string]string{"token": token}, &claims); err != nil { - logger.SugarLogger.Errorf("Failed to validate token: %v", err) - c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "invalid bearer token"}) + writeBearerValidationError(c, err) return } @@ -42,6 +42,17 @@ func AuthChecker() gin.HandlerFunc { } } +func writeBearerValidationError(c *gin.Context, err error) { + var apiErr *sentinel.APIError + if errors.As(err, &apiErr) && apiErr.Status == http.StatusUnauthorized { + c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "invalid bearer token"}) + return + } + + logger.SugarLogger.Errorf("Failed to validate bearer token: %v", err) + c.AbortWithStatusJSON(http.StatusBadGateway, gin.H{"error": "unable to validate bearer token"}) +} + func UnauthorizedPanicHandler() gin.HandlerFunc { return func(c *gin.Context) { defer func() { diff --git a/oauth/api/auth_test.go b/oauth/api/auth_test.go new file mode 100644 index 00000000..227c76a5 --- /dev/null +++ b/oauth/api/auth_test.go @@ -0,0 +1,52 @@ +package api + +import ( + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gaucho-racing/sentinel/oauth/pkg/logger" + "github.com/gaucho-racing/sentinel/oauth/pkg/sentinel" + "github.com/gin-gonic/gin" +) + +func TestWriteBearerValidationErrorDistinguishesInvalidTokenFromUpstreamFailure(t *testing.T) { + gin.SetMode(gin.TestMode) + logger.Init(true) + + tests := []struct { + name string + err error + wantStatus int + }{ + { + name: "invalid token", + err: &sentinel.APIError{Status: http.StatusUnauthorized}, + wantStatus: http.StatusUnauthorized, + }, + { + name: "core failure", + err: &sentinel.APIError{Status: http.StatusInternalServerError}, + wantStatus: http.StatusBadGateway, + }, + { + name: "transport failure", + err: &sentinel.APIError{Err: errors.New("connection refused")}, + wantStatus: http.StatusBadGateway, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + response := httptest.NewRecorder() + context, _ := gin.CreateTestContext(response) + + writeBearerValidationError(context, test.err) + + if response.Code != test.wantStatus { + t.Fatalf("expected %d, got %d: %s", test.wantStatus, response.Code, response.Body.String()) + } + }) + } +} From db943026b6562ccf27efd1e0b5fa76746ac14018 Mon Sep 17 00:00:00 2001 From: Bharat Kathi Date: Mon, 31 Aug 2026 13:17:19 -0700 Subject: [PATCH 3/3] chore(oauth): remove authorization tests --- oauth/api/auth_test.go | 52 ------------------------------------- oauth/api/authorize_test.go | 49 ---------------------------------- 2 files changed, 101 deletions(-) delete mode 100644 oauth/api/auth_test.go delete mode 100644 oauth/api/authorize_test.go diff --git a/oauth/api/auth_test.go b/oauth/api/auth_test.go deleted file mode 100644 index 227c76a5..00000000 --- a/oauth/api/auth_test.go +++ /dev/null @@ -1,52 +0,0 @@ -package api - -import ( - "errors" - "net/http" - "net/http/httptest" - "testing" - - "github.com/gaucho-racing/sentinel/oauth/pkg/logger" - "github.com/gaucho-racing/sentinel/oauth/pkg/sentinel" - "github.com/gin-gonic/gin" -) - -func TestWriteBearerValidationErrorDistinguishesInvalidTokenFromUpstreamFailure(t *testing.T) { - gin.SetMode(gin.TestMode) - logger.Init(true) - - tests := []struct { - name string - err error - wantStatus int - }{ - { - name: "invalid token", - err: &sentinel.APIError{Status: http.StatusUnauthorized}, - wantStatus: http.StatusUnauthorized, - }, - { - name: "core failure", - err: &sentinel.APIError{Status: http.StatusInternalServerError}, - wantStatus: http.StatusBadGateway, - }, - { - name: "transport failure", - err: &sentinel.APIError{Err: errors.New("connection refused")}, - wantStatus: http.StatusBadGateway, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - response := httptest.NewRecorder() - context, _ := gin.CreateTestContext(response) - - writeBearerValidationError(context, test.err) - - if response.Code != test.wantStatus { - t.Fatalf("expected %d, got %d: %s", test.wantStatus, response.Code, response.Body.String()) - } - }) - } -} diff --git a/oauth/api/authorize_test.go b/oauth/api/authorize_test.go deleted file mode 100644 index 97769c36..00000000 --- a/oauth/api/authorize_test.go +++ /dev/null @@ -1,49 +0,0 @@ -package api - -import ( - "net/http" - "net/http/httptest" - "strings" - "testing" - - "github.com/gin-gonic/gin" -) - -func TestAuthorizeRejectsCallerSuppliedEntityWithoutBearer(t *testing.T) { - gin.SetMode(gin.TestMode) - router := InitializeRouter() - InitializeRoutes(router) - - request := httptest.NewRequest( - http.MethodPost, - "/oauth/authorize?client_id=client&redirect_uri=https%3A%2F%2Fexample.com%2Fcallback&scope=openid", - strings.NewReader(`{"entity_id":"ent_victim"}`), - ) - request.Header.Set("Content-Type", "application/json") - response := httptest.NewRecorder() - - router.ServeHTTP(response, request) - - if response.Code != http.StatusUnauthorized { - t.Fatalf("expected %d, got %d: %s", http.StatusUnauthorized, response.Code, response.Body.String()) - } -} - -func TestValidateAuthorizeRequiresBearer(t *testing.T) { - gin.SetMode(gin.TestMode) - router := InitializeRouter() - InitializeRoutes(router) - - request := httptest.NewRequest( - http.MethodGet, - "/oauth/authorize?client_id=client&redirect_uri=https%3A%2F%2Fexample.com%2Fcallback&scope=openid&entity_id=ent_victim", - nil, - ) - response := httptest.NewRecorder() - - router.ServeHTTP(response, request) - - if response.Code != http.StatusUnauthorized { - t.Fatalf("expected %d, got %d: %s", http.StatusUnauthorized, response.Code, response.Body.String()) - } -}