From 2f0a31894c595bdc81c5398cdb255ab352e48353 Mon Sep 17 00:00:00 2001 From: Max Date: Fri, 1 Aug 2025 17:43:27 +0800 Subject: [PATCH] Enhance OAuth state management and validation in Signin API - Introduced UUID-based state parameter generation for improved uniqueness and security in OAuth flows. - Added validation for user-provided state parameters to ensure they conform to UUID format, with warnings included in the response. - Refactored the generateRandomState function to utilize UUID generation instead of cryptographic random bytes. - Updated OAuthAuthorizationURLResponse to include optional warnings about state format issues. --- openapi/signin/api.go | 30 +++++++++++++++++++++--------- openapi/signin/provider.go | 19 ------------------- openapi/signin/types.go | 5 +++-- 3 files changed, 24 insertions(+), 30 deletions(-) diff --git a/openapi/signin/api.go b/openapi/signin/api.go index a538a1db..4f116094 100644 --- a/openapi/signin/api.go +++ b/openapi/signin/api.go @@ -1,15 +1,15 @@ package signin import ( - "crypto/rand" - "encoding/hex" "fmt" "net/http" "net/url" + "regexp" "strings" "time" "github.com/gin-gonic/gin" + "github.com/google/uuid" "github.com/yaoapp/gou/session" "github.com/yaoapp/gou/store" "github.com/yaoapp/kun/log" @@ -256,6 +256,9 @@ func getOAuthAuthorizationURL(c *gin.Context) { return } + // Check if state is provided by user and validate format + var warnings []string + // Generate state if not provided if state == "" { var err error @@ -268,6 +271,11 @@ func getOAuthAuthorizationURL(c *gin.Context) { response.RespondWithError(c, response.StatusInternalServerError, errorResp) return } + } else { + // User provided state - check if it's in UUID format + if !isValidUUID(state) { + warnings = append(warnings, "State parameter is not in UUID format. For better uniqueness and security, consider using UUID format.") + } } // Set default redirect URI if not provided @@ -347,17 +355,21 @@ func getOAuthAuthorizationURL(c *gin.Context) { response.RespondWithSuccess(c, response.StatusOK, &OAuthAuthorizationURLResponse{ AuthorizationURL: authorizationURL, State: state, + Warnings: warnings, }) } -// generateRandomState generates a cryptographically secure random state parameter +// generateRandomState generates a UUID-based state parameter for better uniqueness func generateRandomState() (string, error) { - bytes := make([]byte, 16) - _, err := rand.Read(bytes) - if err != nil { - return "", err - } - return hex.EncodeToString(bytes), nil + u := uuid.New() + return u.String(), nil +} + +// isValidUUID checks if a string is a valid UUID format +func isValidUUID(s string) bool { + // UUID v4 format: 8-4-4-4-12 hexadecimal characters + uuidRegex := regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`) + return uuidRegex.MatchString(strings.ToLower(s)) } // getScheme returns the request scheme (http or https) diff --git a/openapi/signin/provider.go b/openapi/signin/provider.go index 2834cda0..e8e3c3dd 100644 --- a/openapi/signin/provider.go +++ b/openapi/signin/provider.go @@ -22,7 +22,6 @@ import ( "github.com/yaoapp/gou/application" "github.com/yaoapp/gou/http" "github.com/yaoapp/kun/log" - "github.com/yaoapp/kun/utils" oauthtypes "github.com/yaoapp/yao/openapi/oauth/types" ) @@ -411,20 +410,6 @@ func (p *Provider) getUserInfoFromEndpoint(accessToken string, tokenType string) return userInfo, nil } -// getUserInfoFromIDToken extracts user info from ID token (JWT) with signature verification -func (p *Provider) getUserInfoFromIDToken(idToken string) (*oauthtypes.OIDCUserInfo, error) { - // Verify JWT signature and get raw claims - rawClaims, err := p.verifyIDTokenAndGetClaims(idToken) - if err != nil { - return nil, fmt.Errorf("failed to verify ID token: %w", err) - } - - // Map the raw JWT claims to our standard user info structure - userInfo := p.mapUserInfoResponse(rawClaims) - - return userInfo, nil -} - // verifyIDTokenAndGetClaims verifies ID token signature and returns raw claims for user info mapping func (p *Provider) verifyIDTokenAndGetClaims(idToken string) (map[string]interface{}, error) { // Parse token to get header for key ID @@ -457,10 +442,6 @@ func (p *Provider) verifyIDTokenAndGetClaims(idToken string) (map[string]interfa return nil, fmt.Errorf("invalid JWT token") } - fmt.Println("--- TEST ---") - utils.Dump(token.Claims) - fmt.Println("---------------") - // Extract claims claims, ok := token.Claims.(jwt.MapClaims) if !ok { diff --git a/openapi/signin/types.go b/openapi/signin/types.go index 2cb898c8..44b8a5aa 100644 --- a/openapi/signin/types.go +++ b/openapi/signin/types.go @@ -100,8 +100,9 @@ type Endpoints struct { // OAuthAuthorizationURLResponse represents the response for OAuth authorization URL type OAuthAuthorizationURLResponse struct { - AuthorizationURL string `json:"authorization_url"` - State string `json:"state"` + AuthorizationURL string `json:"authorization_url"` + State string `json:"state"` + Warnings []string `json:"warnings,omitempty"` // Optional warnings about state format or other issues } // OAuthCallbackResponse represents the response for OAuth callback