From ef157ae9ba3a23ccd5c09a8a291c5f4ba016d81d Mon Sep 17 00:00:00 2001 From: Olivier Dumont Date: Tue, 30 Dec 2025 12:36:30 +0100 Subject: [PATCH] Fix critical security issue: verify JWT signature in access token validation The validateAccessToken method was only decoding the JWT payload without verifying the signature, allowing attackers to forge tokens. This fix: - Adds ValidateAccessToken method to OIDCService that properly verifies JWT signature using RSA public key - Validates issuer, expiration, and required claims - Updates controller to use the secure validation method - Removes insecure manual JWT parsing code --- internal/controller/oidc_controller.go | 45 +-------------------- internal/service/oidc_service.go | 54 ++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 43 deletions(-) diff --git a/internal/controller/oidc_controller.go b/internal/controller/oidc_controller.go index 74f98e9..f72c617 100644 --- a/internal/controller/oidc_controller.go +++ b/internal/controller/oidc_controller.go @@ -401,48 +401,7 @@ func (controller *OIDCController) getAccessToken(c *gin.Context) string { func (controller *OIDCController) validateAccessToken(accessToken string) (*config.UserContext, error) { // Validate the JWT token using the OIDC service's public key - // This is a simplified validation - in production, you'd want to store - // access tokens and validate them properly, check token revocation, etc. - - // For now, we'll use a helper method in the OIDC service to validate tokens - // Since we don't have a direct method, we'll parse and validate manually - // In a production system, you'd want to add a ValidateAccessToken method to the service - - // Parse the JWT token - parts := strings.Split(accessToken, ".") - if len(parts) != 3 { - return nil, fmt.Errorf("invalid token format") - } - - // Decode payload - payload, err := base64.RawURLEncoding.DecodeString(parts[1]) - if err != nil { - return nil, fmt.Errorf("failed to decode token payload: %w", err) - } - - var claims map[string]interface{} - if err := json.Unmarshal(payload, &claims); err != nil { - return nil, fmt.Errorf("failed to unmarshal claims: %w", err) - } - - // Extract user info from claims - username, _ := claims["sub"].(string) - if username == "" { - return nil, fmt.Errorf("missing sub claim") - } - - // Extract email and name if available - email, _ := claims["email"].(string) - name, _ := claims["name"].(string) - - // Create user context - userContext := &config.UserContext{ - Username: username, - Email: email, - Name: name, - IsLoggedIn: true, - } - - return userContext, nil + // This properly verifies the signature, issuer, and expiration + return controller.oidc.ValidateAccessToken(accessToken) } diff --git a/internal/service/oidc_service.go b/internal/service/oidc_service.go index c456830..f6b7ae5 100644 --- a/internal/service/oidc_service.go +++ b/internal/service/oidc_service.go @@ -281,6 +281,60 @@ func (oidc *OIDCService) GenerateAccessToken(userContext *config.UserContext, cl return accessToken, nil } +func (oidc *OIDCService) ValidateAccessToken(accessToken string) (*config.UserContext, error) { + token, err := jwt.Parse(accessToken, func(token *jwt.Token) (interface{}, error) { + if _, ok := token.Method.(*jwt.SigningMethodRSA); !ok { + return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) + } + return oidc.publicKey, nil + }) + + if err != nil { + return nil, fmt.Errorf("failed to parse access token: %w", err) + } + + if !token.Valid { + return nil, errors.New("invalid access token") + } + + claims, ok := token.Claims.(jwt.MapClaims) + if !ok { + return nil, errors.New("invalid token claims") + } + + // Verify issuer + iss, ok := claims["iss"].(string) + if !ok || iss != oidc.config.Issuer { + return nil, errors.New("invalid issuer") + } + + // Check expiration + exp, ok := claims["exp"].(float64) + if !ok || time.Now().Unix() > int64(exp) { + return nil, errors.New("access token expired") + } + + // Extract user info from claims + username, ok := claims["sub"].(string) + if !ok || username == "" { + return nil, errors.New("missing sub claim") + } + + // Extract email and name if available + email, _ := claims["email"].(string) + name, _ := claims["name"].(string) + + // Create user context + userContext := &config.UserContext{ + Username: username, + Email: email, + Name: name, + IsLoggedIn: true, + } + + return userContext, nil +} + func (oidc *OIDCService) GenerateIDToken(userContext *config.UserContext, clientID string, nonce string) (string, error) { expiry := oidc.config.IDTokenExpiry if expiry <= 0 {