Merge pull request #1062 from trheyi/main

Enhance OAuth state management and validation in Signin API

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)

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 {

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