From 976c6e74814da6b51da8627d71bb396b7638eac8 Mon Sep 17 00:00:00 2001 From: Priyanshi Singh <257472766+priyanshi-singh-22@users.noreply.github.com> Date: Tue, 17 Feb 2026 22:16:30 +0530 Subject: [PATCH 1/4] Fixing Extra_claims validation that breaks for non-string claim values --- internal/api/handlers/v0/auth/oidc.go | 44 +++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/internal/api/handlers/v0/auth/oidc.go b/internal/api/handlers/v0/auth/oidc.go index 0cef699b3..ef258fc1b 100644 --- a/internal/api/handlers/v0/auth/oidc.go +++ b/internal/api/handlers/v0/auth/oidc.go @@ -224,6 +224,50 @@ func (h *OIDCHandler) validateExtraClaims(claims *OIDCClaims) error { return fmt.Errorf("claim validation failed: required claim %s not found", key) } + //Handle array values + if actualArray, ok := actualValue.([]any); ok { + // Check if expected value is also an array + if expectedArray, ok := expectedValue.([]any); ok { + // Both are arrays - check if any actual value exists in expected array + found := false + for _, actVal := range actualArray { + for _, expVal := range expectedArray { + if actVal == expVal { + found = true + break + } + } + if found { + break + } + } + if !found { + return fmt.Errorf("claim validation failed: %s no matching values found between actual %v and expected %v", key, actualArray, expectedArray) + } + continue + } + + // Expected is scalar, actual is array + if len(actualArray) == 1 { + // Normalize single-element arrays to scalars + actualValue = actualArray[0] + } else if len(actualArray) > 1 { + // Check if expected value exists in the array + found := false + for _, item := range actualArray { + if item == expectedValue { + found = true + break + } + } + if !found { + return fmt.Errorf("claim validation failed: %s expected %v to be in array %v", key, expectedValue, actualArray) + } + continue + } + } + + // Compare values if both are scalars if actualValue != expectedValue { return fmt.Errorf("claim validation failed: %s expected %v, got %v", key, expectedValue, actualValue) } From 838f6651c7732b8de1a9aed2e4e455b7170b551a Mon Sep 17 00:00:00 2001 From: Priyanshi Singh <257472766+priyanshi-singh-22@users.noreply.github.com> Date: Tue, 17 Feb 2026 22:44:59 +0530 Subject: [PATCH 2/4] Adding new extra-claim validation test cases --- internal/api/handlers/v0/auth/oidc_test.go | 178 +++++++++++++++++++++ 1 file changed, 178 insertions(+) diff --git a/internal/api/handlers/v0/auth/oidc_test.go b/internal/api/handlers/v0/auth/oidc_test.go index 4ad8acb1d..1976cd647 100644 --- a/internal/api/handlers/v0/auth/oidc_test.go +++ b/internal/api/handlers/v0/auth/oidc_test.go @@ -81,6 +81,184 @@ func TestOIDCHandler_ExchangeToken(t *testing.T) { token: "invalid-domain-token", expectedError: true, }, + { + name: "successful validation with standard claim 'sub'", + config: &config.Config{ + OIDCEnabled: true, + OIDCIssuer: "https://cigna.oktapreview.com", + OIDCClientID: "api://glbcore", + OIDCExtraClaims: `[{"sub":"0oa2ly86ida9z3vgF0h8"}]`, + OIDCPublishPerms: "*", + JWTPrivateKey: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + }, + mockValidator: &MockGenericOIDCValidator{ + validateFunc: func(_ context.Context, _ string) (*auth.OIDCClaims, error) { + return &auth.OIDCClaims{ + Subject: "0oa2ly86ida9z3vgF0h8", + ExtraClaims: map[string]any{ + "email": "user@cigna.com", + }, + }, nil + }, + }, + token: "valid-okta-token", + expectedError: false, + }, + { + name: "failed validation with wrong standard claim 'sub'", + config: &config.Config{ + OIDCEnabled: true, + OIDCIssuer: "https://cigna.oktapreview.com", + OIDCClientID: "api://glbcore", + OIDCExtraClaims: `[{"sub":"0oa2ly86ida9z3vgF0h8"}]`, + OIDCPublishPerms: "*", + JWTPrivateKey: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + }, + mockValidator: &MockGenericOIDCValidator{ + validateFunc: func(_ context.Context, _ string) (*auth.OIDCClaims, error) { + return &auth.OIDCClaims{ + Subject: "wrong_subject_value", + ExtraClaims: map[string]any{ + "email": "user@cigna.com", + }, + }, nil + }, + }, + token: "invalid-sub-token", + expectedError: true, + }, + { + name: "successful validation with array claim - scalar expected value in array", + config: &config.Config{ + OIDCEnabled: true, + OIDCIssuer: "https://accounts.google.com", + OIDCClientID: "test-client-id", + OIDCExtraClaims: `[{"groups":"admin"}]`, + OIDCPublishPerms: "*", + JWTPrivateKey: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + }, + mockValidator: &MockGenericOIDCValidator{ + validateFunc: func(_ context.Context, _ string) (*auth.OIDCClaims, error) { + return &auth.OIDCClaims{ + ExtraClaims: map[string]any{ + "groups": []any{"admin", "users", "developers"}, + }, + }, nil + }, + }, + token: "valid-array-claim-token", + expectedError: false, + }, + { + name: "failed validation with array claim - scalar not in array", + config: &config.Config{ + OIDCEnabled: true, + OIDCIssuer: "https://accounts.google.com", + OIDCClientID: "test-client-id", + OIDCExtraClaims: `[{"groups":"super-admin"}]`, + OIDCPublishPerms: "*", + JWTPrivateKey: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + }, + mockValidator: &MockGenericOIDCValidator{ + validateFunc: func(_ context.Context, _ string) (*auth.OIDCClaims, error) { + return &auth.OIDCClaims{ + ExtraClaims: map[string]any{ + "groups": []any{"admin", "users", "developers"}, + }, + }, nil + }, + }, + token: "invalid-array-claim-token", + expectedError: true, + }, + { + name: "successful validation with array to array comparison - overlapping values", + config: &config.Config{ + OIDCEnabled: true, + OIDCIssuer: "https://accounts.google.com", + OIDCClientID: "test-client-id", + OIDCExtraClaims: `[{"roles":["admin","moderator"]}]`, + OIDCPublishPerms: "*", + JWTPrivateKey: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + }, + mockValidator: &MockGenericOIDCValidator{ + validateFunc: func(_ context.Context, _ string) (*auth.OIDCClaims, error) { + return &auth.OIDCClaims{ + ExtraClaims: map[string]any{ + "roles": []any{"admin", "users"}, + }, + }, nil + }, + }, + token: "valid-array-array-token", + expectedError: false, + }, + { + name: "failed validation with array to array comparison - no overlapping values", + config: &config.Config{ + OIDCEnabled: true, + OIDCIssuer: "https://accounts.google.com", + OIDCClientID: "test-client-id", + OIDCExtraClaims: `[{"roles":["super-admin","owner"]}]`, + OIDCPublishPerms: "*", + JWTPrivateKey: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + }, + mockValidator: &MockGenericOIDCValidator{ + validateFunc: func(_ context.Context, _ string) (*auth.OIDCClaims, error) { + return &auth.OIDCClaims{ + ExtraClaims: map[string]any{ + "roles": []any{"admin", "users"}, + }, + }, nil + }, + }, + token: "invalid-array-array-token", + expectedError: true, + }, + { + name: "successful validation with single-element array normalization", + config: &config.Config{ + OIDCEnabled: true, + OIDCIssuer: "https://accounts.google.com", + OIDCClientID: "test-client-id", + OIDCExtraClaims: `[{"department":"engineering"}]`, + OIDCPublishPerms: "*", + JWTPrivateKey: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + }, + mockValidator: &MockGenericOIDCValidator{ + validateFunc: func(_ context.Context, _ string) (*auth.OIDCClaims, error) { + return &auth.OIDCClaims{ + ExtraClaims: map[string]any{ + "department": []any{"engineering"}, // Single element array + }, + }, nil + }, + }, + token: "valid-single-array-token", + expectedError: false, + }, + { + name: "failed validation with missing claim", + config: &config.Config{ + OIDCEnabled: true, + OIDCIssuer: "https://accounts.google.com", + OIDCClientID: "test-client-id", + OIDCExtraClaims: `[{"required_claim":"expected_value"}]`, + OIDCPublishPerms: "*", + JWTPrivateKey: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + }, + mockValidator: &MockGenericOIDCValidator{ + validateFunc: func(_ context.Context, _ string) (*auth.OIDCClaims, error) { + return &auth.OIDCClaims{ + ExtraClaims: map[string]any{ + "other_claim": "some_value", + }, + }, nil + }, + }, + token: "missing-claim-token", + expectedError: true, + }, } for _, tt := range tests { From 3884579d794dac88700707f73c989dada171617f Mon Sep 17 00:00:00 2001 From: Priyanshi Singh <257472766+priyanshi-singh-22@users.noreply.github.com> Date: Thu, 19 Feb 2026 14:02:05 +0530 Subject: [PATCH 3/4] Modifying the test cases in TestOIDCHandler_ExchangeToken --- internal/api/handlers/v0/auth/oidc_test.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/internal/api/handlers/v0/auth/oidc_test.go b/internal/api/handlers/v0/auth/oidc_test.go index 1976cd647..24efde5a2 100644 --- a/internal/api/handlers/v0/auth/oidc_test.go +++ b/internal/api/handlers/v0/auth/oidc_test.go @@ -82,21 +82,22 @@ func TestOIDCHandler_ExchangeToken(t *testing.T) { expectedError: true, }, { - name: "successful validation with standard claim 'sub'", + name: "successful validation with extra claim 'client_id'", config: &config.Config{ OIDCEnabled: true, OIDCIssuer: "https://cigna.oktapreview.com", OIDCClientID: "api://glbcore", - OIDCExtraClaims: `[{"sub":"0oa2ly86ida9z3vgF0h8"}]`, + OIDCExtraClaims: `[{"client_id":"matched_client_id_value"}]`, OIDCPublishPerms: "*", JWTPrivateKey: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", }, mockValidator: &MockGenericOIDCValidator{ validateFunc: func(_ context.Context, _ string) (*auth.OIDCClaims, error) { return &auth.OIDCClaims{ - Subject: "0oa2ly86ida9z3vgF0h8", + Subject: "user-subject-123", ExtraClaims: map[string]any{ - "email": "user@cigna.com", + "email": "user@cigna.com", + "client_id": "matched_client_id_value", }, }, nil }, @@ -105,26 +106,27 @@ func TestOIDCHandler_ExchangeToken(t *testing.T) { expectedError: false, }, { - name: "failed validation with wrong standard claim 'sub'", + name: "failed validation with wrong extra claim 'client_id'", config: &config.Config{ OIDCEnabled: true, OIDCIssuer: "https://cigna.oktapreview.com", OIDCClientID: "api://glbcore", - OIDCExtraClaims: `[{"sub":"0oa2ly86ida9z3vgF0h8"}]`, + OIDCExtraClaims: `[{"client_id":"client_id_value"}]`, OIDCPublishPerms: "*", JWTPrivateKey: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", }, mockValidator: &MockGenericOIDCValidator{ validateFunc: func(_ context.Context, _ string) (*auth.OIDCClaims, error) { return &auth.OIDCClaims{ - Subject: "wrong_subject_value", + Subject: "user-subject-123", ExtraClaims: map[string]any{ - "email": "user@cigna.com", + "email": "user@cigna.com", + "client_id": "wrong_client_id_value", }, }, nil }, }, - token: "invalid-sub-token", + token: "invalid-client-id-token", expectedError: true, }, { From f7b2e7ca5243e917ca4d1dfe6beb71bce0b40bc5 Mon Sep 17 00:00:00 2001 From: Priyanshi Singh <257472766+priyanshi-singh-22@users.noreply.github.com> Date: Tue, 24 Mar 2026 19:02:57 +0530 Subject: [PATCH 4/4] Linting errors fixed --- internal/api/handlers/v0/auth/oidc.go | 134 +++++++++++++++----------- 1 file changed, 80 insertions(+), 54 deletions(-) diff --git a/internal/api/handlers/v0/auth/oidc.go b/internal/api/handlers/v0/auth/oidc.go index ef258fc1b..abf65f66e 100644 --- a/internal/api/handlers/v0/auth/oidc.go +++ b/internal/api/handlers/v0/auth/oidc.go @@ -210,73 +210,99 @@ func (h *OIDCHandler) validateExtraClaims(claims *OIDCClaims) error { return nil // No extra validation required } - // Parse extra claims configuration - var extraClaimsRules []map[string]any - if err := json.Unmarshal([]byte(h.config.OIDCExtraClaims), &extraClaimsRules); err != nil { - return fmt.Errorf("invalid extra claims configuration: %w", err) + extraClaimsRules, err := h.parseExtraClaimsRules() + if err != nil { + return fmt.Errorf("failed to parse extra claims rules: %w", err) } // Validate each rule for _, rule := range extraClaimsRules { - for key, expectedValue := range rule { - actualValue, exists := claims.ExtraClaims[key] - if !exists { - return fmt.Errorf("claim validation failed: required claim %s not found", key) - } + if err := h.validateSingleRule(claims, rule); err != nil { + return fmt.Errorf("claim validation failed for rule %v: %w", rule, err) + } + } - //Handle array values - if actualArray, ok := actualValue.([]any); ok { - // Check if expected value is also an array - if expectedArray, ok := expectedValue.([]any); ok { - // Both are arrays - check if any actual value exists in expected array - found := false - for _, actVal := range actualArray { - for _, expVal := range expectedArray { - if actVal == expVal { - found = true - break - } - } - if found { - break - } - } - if !found { - return fmt.Errorf("claim validation failed: %s no matching values found between actual %v and expected %v", key, actualArray, expectedArray) - } - continue - } + return nil +} - // Expected is scalar, actual is array - if len(actualArray) == 1 { - // Normalize single-element arrays to scalars - actualValue = actualArray[0] - } else if len(actualArray) > 1 { - // Check if expected value exists in the array - found := false - for _, item := range actualArray { - if item == expectedValue { - found = true - break - } - } - if !found { - return fmt.Errorf("claim validation failed: %s expected %v to be in array %v", key, expectedValue, actualArray) - } - continue - } - } +// parseExtraClaimsRules parses the extra claims configuration from JSON +func (h *OIDCHandler) parseExtraClaimsRules() ([]map[string]any, error) { + var extraClaimsRules []map[string]any + if err := json.Unmarshal([]byte(h.config.OIDCExtraClaims), &extraClaimsRules); err != nil { + return nil, fmt.Errorf("invalid extra claims configuration: %w", err) + } + return extraClaimsRules, nil +} - // Compare values if both are scalars - if actualValue != expectedValue { - return fmt.Errorf("claim validation failed: %s expected %v, got %v", key, expectedValue, actualValue) - } +// validateSingleRule validates a single claim rule against the OIDC claims +func (h *OIDCHandler) validateSingleRule(claims *OIDCClaims, rule map[string]any) error { + for key, expectedValue := range rule { + actualValue, exists := claims.ExtraClaims[key] + if !exists { + return fmt.Errorf("claim validation failed: required claim %s not found", key) + } + + if err := h.compareClaimValues(key, actualValue, expectedValue); err != nil { + return err } } + return nil +} + +// compareClaimValues compares actual and expected claim values with support for arrays +func (h *OIDCHandler) compareClaimValues(key string, actualValue, expectedValue any) error { + actualArray, actualIsArray := actualValue.([]any) + if actualIsArray { + return h.compareArrayClaimValues(key, actualArray, expectedValue) + } + + // Compare scalar values + if actualValue != expectedValue { + return fmt.Errorf("claim validation failed: %s expected %v, got %v", key, expectedValue, actualValue) + } return nil } +// compareArrayClaimValues handles comparison when actual value is an array +func (h *OIDCHandler) compareArrayClaimValues(key string, actualArray []any, expectedValue any) error { + expectedArray, expectedIsArray := expectedValue.([]any) + if expectedIsArray { + return h.compareArrayToArray(key, actualArray, expectedArray) + } + + return h.compareArrayToScalar(key, actualArray, expectedValue) +} + +// compareArrayToArray compares two arrays looking for any matching values +func (h *OIDCHandler) compareArrayToArray(key string, actualArray, expectedArray []any) error { + for _, actVal := range actualArray { + for _, expVal := range expectedArray { + if actVal == expVal { + return nil // Found a match + } + } + } + return fmt.Errorf("claim validation failed: %s no matching values found between actual %v and expected %v", key, actualArray, expectedArray) +} + +// compareArrayToScalar compares an array to a scalar value +func (h *OIDCHandler) compareArrayToScalar(key string, actualArray []any, expectedValue any) error { + if len(actualArray) == 1 { + // Normalize single-element arrays to scalars and compare + return h.compareClaimValues(key, actualArray[0], expectedValue) + } + + // Check if expected value exists in the array + for _, item := range actualArray { + if item == expectedValue { + return nil // Found a match + } + } + + return fmt.Errorf("claim validation failed: %s expected %v to be in array %v", key, expectedValue, actualArray) +} + // buildPermissions builds permissions based on OIDC claims and configuration func (h *OIDCHandler) buildPermissions(_ *OIDCClaims) []auth.Permission { var permissions []auth.Permission