Skip to content

[TT-14359] fix nested scopes for identity base field #7522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andrei-tyk merged 7 commits into from
Nov 10, 2025
Merged

[TT-14359] fix nested scopes for identity base field #7522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1c4dbbb
TT-14359, fixed nested claims for identity base field
andrei-tyk Nov 7, 2025
7a0a404
TT-14359, fixed nested claims for identity base field
andrei-tyk Nov 7, 2025
4e6527e
Merge branch 'master' into TT-14359-fix-nested-scopes-for-identity-ba…
andrei-tyk Nov 7, 2025
c4a3336
TT-14359, refactoring for ease of reading
andrei-tyk Nov 7, 2025
c207dad
Merge remote-tracking branch 'origin/TT-14359-fix-nested-scopes-for-i…
andrei-tyk Nov 7, 2025
04e4b5a
Merge branch 'master' into TT-14359-fix-nested-scopes-for-identity-ba…
andrei-tyk Nov 10, 2025
2d93816
Merge branch 'master' into TT-14359-fix-nested-scopes-for-identity-ba…
andrei-tyk Nov 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 70 additions & 44 deletions gateway/mw_jwt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,43 +385,43 @@
}
jwkSet, ok := cachedItem.(*jose.JSONWebKeySet)
if !ok {
k.Logger().Warnf("Invalid JWK cache format for APIID %s, URL %s — ignoring", k.Spec.APIID, jwkURI.URL)
k.Logger().Warnf("Invalid JWK cache format for APIID %s, URL %s ? ignoring", k.Spec.APIID, jwkURI.URL)

Check warning on line 388 in gateway/mw_jwt.go

View check run for this annotation

probelabs / Visor: quality

style Issue 

In a log message, an em dash (`—`) was replaced with a question mark (`?`), which makes the message's intent slightly less clear. This appears to be an unintentional typo. The same change is present on line 424.
Raw output 
It is recommended to revert the `?` to the original `—` or rephrase the message to improve clarity, for example: `...for APIID %s, URL %s, ignoring`.
}
jwkSets = append(jwkSets, jwkSet)
}
return jwkSets
}

func (k *JWTMiddleware) getSecretFromMultipleJWKURIs(jwkURIs []apidef.JWK, kidVal interface{}, keyType string) (interface{}, error) {
if !k.Spec.APIDefinition.IsOAS {
err := errors.New("this feature is only available when using OAS API")
k.Logger().WithError(err).Infof("Failed to process api")

return nil, err
}

var (
jwkSets []*jose.JSONWebKeySet
fallbackJWKURIs []apidef.JWK
kid, ok = kidVal.(string)
)

if !ok {
return nil, ErrKIDNotAString
}

cacheAPIDef := k.specCacheKey(k.Spec, JWKsAPIDef)
cacheOutdated := false

cachedAPIDefRaw, foundDef := k.loadOrCreateJWKCache().Get(cacheAPIDef)
if foundDef {
cachedAPIDef, ok := cachedAPIDefRaw.(*apidef.APIDefinition)
if !ok {
cacheOutdated = true
}

if jwkURLsChanged(cachedAPIDef.JWTJwksURIs, jwkURIs) {
k.Logger().Infof("Detected change in JWK URLs — refreshing cache for APIID %s", k.Spec.APIID)
k.Logger().Infof("Detected change in JWK URLs ? refreshing cache for APIID %s", k.Spec.APIID)

Check warning on line 424 in gateway/mw_jwt.go

View check run for this annotation

probelabs / Visor: security

style Issue 

The log messages on lines 391 and 424 contain a '?' which appears to be a typo. The original character was '—', and using '?' here can be confusing. Log messages should be clear and consistent.
Raw output 
Replace the '?' with '—' in both log messages for clarity and consistency with the surrounding codebase.
cacheOutdated = true
} else {
jwkSets = k.collectCachedJWKsFromCache(jwkURIs)
Expand Down Expand Up @@ -508,36 +508,28 @@

func (k *JWTMiddleware) getPolicyIDFromToken(claims jwt.MapClaims) (string, bool) {
if k.Spec.IsOAS {
policyID := ""
found := false
fieldNames := k.Spec.OAS.GetJWTConfiguration().BasePolicyClaims
if len(fieldNames) == 0 && k.Spec.OAS.GetJWTConfiguration().PolicyFieldName != "" {
fieldNames = append(fieldNames, k.Spec.OAS.GetJWTConfiguration().PolicyFieldName)
}
for _, c := range fieldNames {
policyID, found = claims[c].(string)
if found {
break
for _, claimField := range fieldNames {
if policyID, found := getClaimValue(claims, claimField); found {
k.Logger().Debugf("Found policy in claim: %s", claimField)
return policyID, true
}
}

if !found || policyID == "" {
k.Logger().Debugf("Could not identify a policy to apply to this token from fields: %v", fieldNames)
return "", false
}
return policyID, true
k.Logger().Debugf("Could not identify a policy to apply to this token from fields: %v", fieldNames)
return "", false
} else {
policyID, foundPolicy := claims[k.Spec.JWTPolicyFieldName].(string)
if !foundPolicy {
k.Logger().Debugf("Could not identify a policy to apply to this token from field: %s", k.Spec.JWTPolicyFieldName)
return "", false
// Legacy path - also support nested claims
if policyID, found := getClaimValue(claims, k.Spec.JWTPolicyFieldName); found {
k.Logger().Debugf("Found policy in claim: %s", k.Spec.JWTPolicyFieldName)
return policyID, true
}

if policyID == "" {
k.Logger().Errorf("Policy field %s has empty value", k.Spec.JWTPolicyFieldName)
return "", false
}
return policyID, true
k.Logger().Debugf("Could not identify a policy to apply to this token from field: %s", k.Spec.JWTPolicyFieldName)
return "", false
}
}

Expand Down Expand Up @@ -629,6 +621,31 @@
return *scopeSlice
}

// getClaimValue attempts to retrieve a string value from JWT claims using a two-step lookup:
// 1. First, it checks for a literal key (backward compatibility for keys with dots in their names)
// 2. If not found and the field contains a dot, it attempts nested lookup (e.g., "user.id" -> claims["user"]["id"])
//
// Returns the claim value and a boolean indicating if it was found.
func getClaimValue(claims jwt.MapClaims, claimField string) (string, bool) {
// STEP 1: Try literal key first (backward compatibility)
// Handles edge case where claim key contains literal dots (e.g., "user.id" as a key)
if value, found := claims[claimField].(string); found && value != "" {
return value, true
}

// STEP 2: Try nested lookup (new feature)
// Only if literal key wasn't found AND the field contains a dot
if strings.Contains(claimField, ".") {
if value := nestedMapLookup(claims, strings.Split(claimField, ".")...); value != nil {
if strValue, ok := value.(string); ok && strValue != "" {
return strValue, true
}
}
}

return "", false
}

func nestedMapLookup(m map[string]interface{}, ks ...string) interface{} {
var c interface{} = m
for _, k := range ks {
Expand Down Expand Up @@ -1505,36 +1522,45 @@

// getUserIDFromClaim parses jwt claims and get the userID from provided identityBaseField.
func getUserIDFromClaim(claims jwt.MapClaims, identityBaseField string, shouldFallback bool) (string, error) {
var (
userID string
found bool
)

if identityBaseField != "" {
if userID, found = claims[identityBaseField].(string); found {
if len(userID) > 0 {
log.WithField("userId", userID).Debug("Found User Id in Base Field")
return userID, nil
}
if userID, found := getClaimValue(claims, identityBaseField); found {
log.WithField("userId", userID).Debug("Found User Id in Base Field")
return userID, nil
}

err := fmt.Errorf("%w, claim: %s", ErrEmptyUserIDInClaim, identityBaseField)
log.Error(err)
return "", err
// Check if the field exists but is empty
if value, exists := claims[identityBaseField]; exists {
if strValue, ok := value.(string); ok && strValue == "" {
err := fmt.Errorf("%w, claim: %s", ErrEmptyUserIDInClaim, identityBaseField)
log.Error(err)
return "", err
}
}

if !found {
log.WithField("Base Field", identityBaseField).Warning("Base Field claim not found, trying to find user ID in 'sub' claim.")
// Also check nested path for empty string
if strings.Contains(identityBaseField, ".") {
if value := nestedMapLookup(claims, strings.Split(identityBaseField, ".")...); value != nil {
if strValue, ok := value.(string); ok && strValue == "" {
err := fmt.Errorf("%w, claim: %s", ErrEmptyUserIDInClaim, identityBaseField)
log.Error(err)

Check warning on line 1545 in gateway/mw_jwt.go

View check run for this annotation

probelabs / Visor: performance

performance Issue 

The function `getUserIDFromClaim` performs redundant lookups for JWT claims, which can impact performance on the request hot path. After an initial call to `getClaimValue` fails, the code proceeds to perform the same literal and nested lookups again to check for an empty value. This results in unnecessary CPU usage and memory allocations (from `strings.Split`) for every request where the identity claim is missing, empty, or of the wrong type.
Raw output 
Refactor the logic to perform the claim lookup only once. The result of the single lookup should be inspected to determine if the claim is valid, empty, or not found, thus avoiding the repeated work. A potential approach is to perform the literal lookup, check the result, then perform the nested lookup if necessary, all within this function, instead of calling `getClaimValue` and then repeating its internal logic.
return "", err
}
}

Check failure on line 1548 in gateway/mw_jwt.go

View check run for this annotation

probelabs / Visor: architecture

architecture Issue 

The function `getUserIDFromClaim` duplicates the logic for looking up JWT claims that is already present in the new `getClaimValue` function. This violates the Don't Repeat Yourself (DRY) principle and introduces a maintenance burden. The function re-implements both literal and nested claim lookups to differentiate between a 'not found' claim and a 'found but empty' claim, a distinction that the `getClaimValue` helper abstracts away. This leads to redundant code that is difficult to maintain.
Raw output 
Refactor the claim lookup logic into a new, lower-level helper function that can be shared by both `getClaimValue` and `getUserIDFromClaim`. This new function should perform the raw lookup (both literal and nested) and return the `interface{}` value, allowing each caller to implement its own specific validation logic (e.g., checking for empty strings) without duplicating the lookup mechanism. This will improve separation of concerns and code reusability.
}

log.WithField("Base Field", identityBaseField).Warning("Base Field claim not found, trying to find user ID in 'sub' claim.")
}

if userID, found = claims[SUB].(string); shouldFallback && found {
if len(userID) > 0 {
log.WithField("userId", userID).Debug("Found User Id in 'sub' claim")
return userID, nil
}
if shouldFallback {

Check failure on line 1554 in gateway/mw_jwt.go

View check run for this annotation

probelabs / Visor: quality

logic Issue 

The logic to find a claim's value is duplicated between the `getClaimValue` function and the `getUserIDFromClaim` function. `getClaimValue` returns `false` for both a non-existent claim and a claim that exists but has an empty string value. This forces `getUserIDFromClaim` to re-implement both literal and nested lookups to distinguish between these two cases, as an empty value should result in an error while a non-existent value should trigger a fallback. This duplication makes the code inefficient (e.g., `nestedMapLookup` may be called twice for the same claim) and harder to maintain.
Raw output 
Modify `getClaimValue` to return the found value as `interface{}` along with a boolean indicating if it was found. This would centralize the lookup logic. The calling functions would then be responsible for type assertion and value validation (e.g., checking for empty strings). This would eliminate the need for `getUserIDFromClaim` to perform its own redundant lookups.
if userID, found := claims[SUB].(string); found {
if len(userID) > 0 {
log.WithField("userId", userID).Debug("Found User Id in 'sub' claim")
return userID, nil
}

log.Error(ErrEmptyUserIDInSubClaim)
return "", ErrEmptyUserIDInSubClaim
log.Error(ErrEmptyUserIDInSubClaim)
return "", ErrEmptyUserIDInSubClaim
}
}

log.Error(ErrNoSuitableUserIDClaimFound)
Expand Down
Loading
Loading