Skip to content

[TT-15359]improve backwards compatibility of Jwt claim validation #7294

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

Jump to bottom
Merged
merged 1 commit into from
Aug 12, 2025
Merged

[TT-15359]improve backwards compatibility of Jwt claim validation #7294

merged 1 commit into from
Aug 12, 2025
+149 −104

Conversation

kofoworola
Copy link
Contributor

@kofoworola kofoworola commented Aug 11, 2025 •
edited by lghiur
Loading

User description

TT-15359
Summary Core Registered Claims Validation
Type Story Story
Status In Code Review
Points N/A
Labels jira_escalated

Description

TT-15359

This pull request improves backward compatibility for JWT claim validation in the Tyk Gateway by:

  • Adding fallback logic in the OAS (OpenAPI Specification) JWT configuration:
  • Uses PolicyFieldName if BasePolicyClaims is empty.
  • Uses IdentityBaseField if SubjectClaims is empty.
  • Uses Scopes.ClaimName if Scopes.Claims is empty.
  • Extending and reorganizing unit tests to cover these new fallback behaviors.
  • Ensuring that new logic preserves existing backward-compatible behavior when parsing JWTs.

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

PR Type

Enhancement, Tests

Description

  • Add OAS fallbacks for deprecated fields

  • Prefer single-field configs when arrays empty

  • Expand unit tests for OAS JWT parsing

  • Preserve backward compatibility behavior

Diagram Walkthrough

flowchart LR
  OASCfg["OAS JWT config"] -- "BasePolicyClaims empty, use PolicyFieldName" --> PolicyID["Resolve policy ID"]
  OASCfg -- "SubjectClaims empty, use IdentityBaseField" --> UserID["Resolve user ID"]
  OASCfg -- "Scopes.Claims empty, use Scopes.ClaimName" --> ScopeClaim["Resolve scope claim name"]
  Tests["Unit tests"] -- "cover new fallbacks" --> Verification["Behavior verified"]
Loading

File Walkthrough
Relevant files
Enhancement
mw_jwt.go
Add OAS JWT claim fallback logic                                                 

gateway/mw_jwt.go

  • Fallback to PolicyFieldName when BasePolicyClaims empty
  • Fallback to IdentityBaseField when SubjectClaims empty
  • Fallback to Scopes.ClaimName when Scopes.Claims empty
 +9/-0     
Tests
mw_jwt_test.go
Extend tests for OAS JWT claim fallbacks                                 

gateway/mw_jwt_test.go

  • Add tests for policy fallback via PolicyFieldName
  • Split and expand OAS user ID tests with IdentityBaseField
  • Add tests for scopes fallback via Scopes.ClaimName
 +140/-104

@kofoworola kofoworola requested a review from lghiur August 11, 2025 11:38
@buger
Copy link
Member

buger commented Aug 11, 2025 •
edited by jira bot
Loading

Let's make that PR title a 💯 shall we? 💪

Your PR title and story title look slightly different. Just checking in to know if it was intentional!

Story Title Core Registered Claims Validation
PR Title [TT-15359]improve backwards compatibility of Jwt claim validation

Check out this guide to learn more about PR best-practices.

@github-actions GitHub Actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Backward-compat logic

The new fallbacks to deprecated fields (PolicyFieldName, IdentityBaseField, Scopes.ClaimName) look correct, but confirm that precedence is intentional: arrays take priority over single field fallbacks and that empty strings in claims are still treated as errors where appropriate.

 
		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
			}
		}

		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
	} 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
		}

		if policyID == "" {
			k.Logger().Errorf("Policy field %s has empty value", k.Spec.JWTPolicyFieldName)
			return "", false
		}
		return policyID, true
	}
}

func (k *JWTMiddleware) getBasePolicyID(r *http.Request, claims jwt.MapClaims) (policyID string, found bool) {
	if k.Spec.JWTPolicyFieldName != "" {
		policyID, found = k.getPolicyIDFromToken(claims)
		return
	} else if k.Spec.JWTClientIDBaseField != "" {
		clientID, clientIDFound := claims[k.Spec.JWTClientIDBaseField].(string)
		if !clientIDFound {
			k.Logger().Debug("Could not identify a policy to apply to this token from field")
			return
		}

		// Check for a regular token that matches this client ID
		clientSession, exists := k.CheckSessionAndIdentityForValidKey(clientID, r)
		clientID = clientSession.KeyID
		if !exists {
			return
		}

		pols := clientSession.PolicyIDs()
		if len(pols) < 1 {
			return
		}

		// Use the policy from the client ID
		return pols[0], true
	}

	return
}

func (k *JWTMiddleware) getUserIdFromClaim(claims jwt.MapClaims) (string, error) {
	if k.Spec.IsOAS {
		return k.getUserIDFromClaimOAS(claims)
	} else {
		return getUserIDFromClaim(claims, k.Spec.JWTIdentityBaseField, true)
	}
}

func (k *JWTMiddleware) getUserIDFromClaimOAS(claims jwt.MapClaims) (string, error) {
	identityBaseFields := k.Spec.OAS.GetJWTConfiguration().SubjectClaims
	if len(identityBaseFields) == 0 && k.Spec.OAS.GetJWTConfiguration().IdentityBaseField != "" {
		identityBaseFields = append(identityBaseFields, k.Spec.OAS.GetJWTConfiguration().IdentityBaseField)
	}
	checkedSub := false
	for _, identityBaseField := range identityBaseFields {
		if identityBaseField == SUB {
			checkedSub = true
		}

		id, err := getUserIDFromClaim(claims, identityBaseField, false)
		if err != nil {
			if errors.Is(ErrNoSuitableUserIDClaimFound, err) {
				continue
			}
			return "", err
		}
		return id, nil
	}
	// fallBack to Sub if SUB has not been checked yet
	if !checkedSub {
		return getUserIDFromClaim(claims, SUB, false)
	}
	return "", ErrNoSuitableUserIDClaimFound
}

func toScopeStringsSlice(v interface{}, scopeSlice *[]string, nested bool) []string {
	if scopeSlice == nil {
		scopeSlice = &[]string{}
	}

	switch e := v.(type) {
	case string:
		if !nested {
			splitStringScopes := strings.Split(e, " ")
			*scopeSlice = append(*scopeSlice, splitStringScopes...)
		} else {
			*scopeSlice = append(*scopeSlice, e)
		}

	case []interface{}:
		for _, scopeElement := range e {
			toScopeStringsSlice(scopeElement, scopeSlice, true)
		}
	}

	return *scopeSlice
}

func nestedMapLookup(m map[string]interface{}, ks ...string) interface{} {
	var c interface{} = m
	for _, k := range ks {
		if _, ok := c.(map[string]interface{}); !ok {
			//fmt.Errorf("key not found; remaining keys: %v", ks)
			return nil
		}
		c = getMapContext(c, k)
	}
	return c
}

func getMapContext(m interface{}, k string) (rval interface{}) {
	switch e := m.(type) {
	case map[string]interface{}:
		return e[k]
	default:
		return e
	}
}

func getScopeFromClaim(claims jwt.MapClaims, scopeClaimName string) []string {
	lookedUp := nestedMapLookup(claims, strings.Split(scopeClaimName, ".")...)

	return toScopeStringsSlice(lookedUp, nil, false)
}

func mapScopeToPolicies(mapping map[string]string, scope []string) []string {
	polIDs := []string{}

	// add all policies matched from scope-policy mapping
	policiesToApply := map[string]bool{}
	for _, scopeItem := range scope {
		if policyID, ok := mapping[scopeItem]; ok {
			policiesToApply[policyID] = true
			log.Debugf("Found a matching policy for scope item: %s", scopeItem)
		} else {
			log.Errorf("Couldn't find a matching policy for scope item: %s", scopeItem)
		}
	}
	for id := range policiesToApply {
		polIDs = append(polIDs, id)
	}

	return polIDs
}

func (k *JWTMiddleware) getOAuthClientIDFromClaim(claims jwt.MapClaims) string {
	for _, claimName := range oauthClientIDClaims {
		if val, ok := claims[claimName]; ok {
			return val.(string)
		}
	}
	return ""
}

// processCentralisedJWT Will check a JWT token centrally against the secret stored in the API Definition.
func (k *JWTMiddleware) processCentralisedJWT(r *http.Request, token *jwt.Token) (error, int) {
	k.Logger().Debug("JWT authority is centralised")

	claims := token.Claims.(jwt.MapClaims)
	baseFieldData, err := k.getUserIdFromClaim(claims)
	if err != nil {
		k.reportLoginFailure("[NOT FOUND]", r)
		return err, http.StatusForbidden
	}

	// Generate a virtual token
	data := []byte(baseFieldData)
	keyID := fmt.Sprintf("%x", md5.Sum(data))
	sessionID := k.Gw.generateToken(k.Spec.OrgID, keyID)
	updateSession := false

	k.Logger().Debug("JWT Temporary session ID is: ", sessionID)

	// CheckSessionAndIdentityForValidKey returns a session with keyID populated
	session, exists := k.CheckSessionAndIdentityForValidKey(sessionID, r)

	sessionID = session.KeyID
	isDefaultPol := false
	basePolicyID := ""
	foundPolicy := false
	if !exists {
		// Create it
		k.Logger().Debug("Key does not exist, creating")

		// We need a base policy as a template, either get it from the token itself OR a proxy client ID within Tyk
		basePolicyID, foundPolicy = k.getBasePolicyID(r, claims)
		if !foundPolicy {
			if len(k.Spec.JWTDefaultPolicies) == 0 {
				k.reportLoginFailure(baseFieldData, r)
				return errors.New("key not authorized: no matching policy found"), http.StatusForbidden
			} else {
				isDefaultPol = true
				basePolicyID = k.Spec.JWTDefaultPolicies[0]
			}
		}

		session, err = k.Gw.generateSessionFromPolicy(basePolicyID,
			k.Spec.OrgID,
			true)

		// If base policy is one of the defaults, apply other ones as well
		if isDefaultPol {
			for _, pol := range k.Spec.JWTDefaultPolicies {
				if !contains(session.ApplyPolicies, pol) {
					session.ApplyPolicies = append(session.ApplyPolicies, pol)
				}
			}
		}

		if err := k.ApplyPolicies(&session); err != nil {
			return errors.New("failed to create key: " + err.Error()), http.StatusInternalServerError
		}

		if err != nil {
			k.reportLoginFailure(baseFieldData, r)
			k.Logger().Error("Could not find a valid policy to apply to this token!")
			return errors.New("key not authorized: no matching policy"), http.StatusForbidden
		}

		//override session expiry with JWT if longer lived
		if f, ok := claims["exp"].(float64); ok {
			if int64(f)-session.Expires > 0 {
				session.Expires = int64(f)
			}
		}

		session.MetaData = map[string]interface{}{"TykJWTSessionID": sessionID}
		session.Alias = baseFieldData

		// Update the session in the session manager in case it gets called again
		updateSession = true
		k.Logger().Debug("Policy applied to key")
	} else {
		// extract policy ID from JWT token
		basePolicyID, foundPolicy = k.getBasePolicyID(r, claims)
		if !foundPolicy {
			if len(k.Spec.JWTDefaultPolicies) == 0 {
				k.reportLoginFailure(baseFieldData, r)
				return errors.New("key not authorized: no matching policy found"), http.StatusForbidden
			} else {
				isDefaultPol = true
				basePolicyID = k.Spec.JWTDefaultPolicies[0]
			}
		}
		// check if we received a valid policy ID in claim
		k.Gw.policiesMu.RLock()
		policy, ok := k.Gw.policiesByID[basePolicyID]
		k.Gw.policiesMu.RUnlock()
		if !ok {
			k.reportLoginFailure(baseFieldData, r)
			k.Logger().Error("Policy ID found is invalid!")
			return errors.New("key not authorized: no matching policy"), http.StatusForbidden
		}
		// check if token for this session was switched to another valid policy
		pols := session.PolicyIDs()
		if len(pols) == 0 {
			k.reportLoginFailure(baseFieldData, r)
			k.Logger().Error("No policies for the found session. Failing Request.")
			return errors.New("key not authorized: no matching policy found"), http.StatusForbidden
		}

		defaultPolicyListChanged := false

		if isDefaultPol {
			// check a policy is removed/added from/to default policies

			for _, pol := range session.PolicyIDs() {
				if !contains(k.Spec.JWTDefaultPolicies, pol) && basePolicyID != pol {
					defaultPolicyListChanged = true
				}
			}

			for _, defPol := range k.Spec.JWTDefaultPolicies {
				if !contains(session.PolicyIDs(), defPol) {
					defaultPolicyListChanged = true
				}
			}
		}

		if !contains(pols, basePolicyID) || defaultPolicyListChanged {
			if policy.OrgID != k.Spec.OrgID {
				k.reportLoginFailure(baseFieldData, r)
				k.Logger().Error("Policy ID found is invalid (wrong ownership)!")
				return errors.New("key not authorized: no matching policy"), http.StatusForbidden
			}
			// apply new policy to session and update session
			updateSession = true
			session.SetPolicies(basePolicyID)

			if isDefaultPol {
				for _, pol := range k.Spec.JWTDefaultPolicies {
					if !contains(session.ApplyPolicies, pol) {
						session.ApplyPolicies = append(session.ApplyPolicies, pol)
					}
				}
			}

			if err := k.ApplyPolicies(&session); err != nil {
				k.reportLoginFailure(baseFieldData, r)
				k.Logger().WithError(err).Error("Could not apply new policy to session")
				return errors.New("key not authorized: could not apply new policy"), http.StatusForbidden
			}
		}

		//override session expiry with JWT if longer lived
		if f, ok := claims["exp"].(float64); ok {
			if int64(f)-session.Expires > 0 {
				session.Expires = int64(f)
				updateSession = true
			}
		}
	}

	// apply policies from scope if scope-to-policy mapping is specified for this API
	if len(k.Spec.GetScopeToPolicyMapping()) != 0 {
		scopeClaimName := k.Spec.GetScopeClaimName()
		if k.Spec.IsOAS {
			scopeClaimName = k.getScopeClaimNameOAS(claims)
		}
		if scopeClaimName == "" {
			scopeClaimName = "scope"
		}

		if scope := getScopeFromClaim(claims, scopeClaimName); len(scope) > 0 {
			polIDs := []string{
				basePolicyID, // add base policy as a first one
			}

			// // If specified, scopes should not use default policy
			if isDefaultPol {
				polIDs = []string{}
			}

			// add all policies matched from scope-policy mapping
			mappedPolIDs := mapScopeToPolicies(k.Spec.GetScopeToPolicyMapping(), scope)
			if len(mappedPolIDs) > 0 {
				k.Logger().Debugf("Identified policy(s) to apply to this token from scope claim: %s", scopeClaimName)
			} else {
				k.Logger().Errorf("Couldn't identify policy(s) to apply to this token from scope claim: %s", scopeClaimName)
			}

			polIDs = append(polIDs, mappedPolIDs...)
			if len(polIDs) == 0 {
				k.reportLoginFailure(baseFieldData, r)
				k.Logger().Error("no matching policy found in scope claim")
				return errors.New("key not authorized: no matching policy found in scope claim"), http.StatusForbidden
			}

			// check if we need to update session
			if !updateSession {
				updateSession = !session.PoliciesEqualTo(polIDs)
			}

			session.SetPolicies(polIDs...)

			// multiple policies assigned to a key, check if it is applicable
			if err := k.ApplyPolicies(&session); err != nil {
				k.reportLoginFailure(baseFieldData, r)
				k.Logger().WithError(err).Error("Could not several policies from scope-claim mapping to JWT to session")
				return errors.New("key not authorized: could not apply several policies"), http.StatusForbidden
			}

		}
	}

	oauthClientID := ""
	// Get the OAuth client ID if available:
	if !k.Spec.IDPClientIDMappingDisabled {
		oauthClientID = k.getOAuthClientIDFromClaim(claims)
	}

	if session.OauthClientID != oauthClientID {
		session.OauthClientID = oauthClientID
		updateSession = true
	}

	if !k.Spec.IDPClientIDMappingDisabled && oauthClientID != "" {
		// Initialize the OAuthManager if empty:
		if k.Spec.OAuthManager == nil {
			prefix := generateOAuthPrefix(k.Spec.APIID)
			storageManager := k.Gw.getGlobalMDCBStorageHandler(prefix, false)
			storageManager.Connect()

			storageDriver := &storage.RedisCluster{KeyPrefix: prefix, HashKeys: false, ConnectionHandler: k.Gw.StorageConnectionHandler}
			storageDriver.Connect()

			k.Spec.OAuthManager = &OAuthManager{
				OsinServer: k.Gw.TykOsinNewServer(&osin.ServerConfig{},
					&RedisOsinStorageInterface{
						storageManager,
						k.Gw.GlobalSessionManager,
						storageDriver,
						k.Spec.OrgID,
						k.Gw,
					}),
			}
		}

		// Retrieve OAuth client data from storage and inject developer ID into the session object:
		client, err := k.Spec.OAuthManager.Storage().GetClient(oauthClientID)
		if err == nil {
			userData := client.GetUserData()
			if userData != nil {
				data, ok := userData.(map[string]interface{})
				if ok {
					updateSession = session.TagsFromMetadata(data)

					if err := k.ApplyPolicies(&session); err != nil {
						return errors.New("failed to apply policies in session metadata: " + err.Error()), http.StatusInternalServerError
					}
				}
			}
		} else {
			k.Logger().WithError(err).Debug("Couldn't get OAuth client")
		}
	}

	// ensure to set the sessionID
	session.KeyID = sessionID
	k.Logger().Debug("Key found")
	switch k.Spec.BaseIdentityProvidedBy {
	case apidef.JWTClaim, apidef.UnsetAuth:
		ctxSetSession(r, &session, updateSession, k.Gw.GetConfig().HashKeys)
		if updateSession {
			k.Gw.SessionCache.Set(session.KeyHash(), session.Clone(), cache.DefaultExpiration)
		}
	}
	ctxSetJWTContextVars(k.Spec, r, token)

	return nil, http.StatusOK
}

func (k *JWTMiddleware) getScopeClaimNameOAS(claims jwt.MapClaims) string {
	claimNames := k.Spec.OAS.GetJWTConfiguration().Scopes.Claims
	if len(claimNames) == 0 && k.Spec.OAS.GetJWTConfiguration().Scopes.ClaimName != "" {
		claimNames = []string{k.Spec.OAS.GetJWTConfiguration().Scopes.ClaimName}
	}
Type assertions

policyID, found = claims[c].(string) assumes the claim is a string; consider whether claims might be numeric or arrays (common for scopes). If non-string types are possible, add normalization or clearer error handling to avoid silent skips.

 
for _, c := range fieldNames {
	policyID, found = claims[c].(string)
	if found {
		break

@github-actions GitHub Actions
Copy link
Contributor

API Changes

no api changes detected

@github-actions GitHub Actions
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Nil-check scopes before access

Guard against Scopes being nil before accessing its fields to prevent panics. Cache
the JWT configuration once and only read Claims/ClaimName if Scopes is non-nil.

gateway/mw_jwt.go [840-843]

-claimNames := k.Spec.OAS.GetJWTConfiguration().Scopes.Claims
-if len(claimNames) == 0 && k.Spec.OAS.GetJWTConfiguration().Scopes.ClaimName != "" {
-	claimNames = []string{k.Spec.OAS.GetJWTConfiguration().Scopes.ClaimName}
+cfg := k.Spec.OAS.GetJWTConfiguration()
+var claimNames []string
+if cfg.Scopes != nil {
+	claimNames = cfg.Scopes.Claims
+	if len(claimNames) == 0 && cfg.Scopes.ClaimName != "" {
+		claimNames = []string{cfg.Scopes.ClaimName}
+	}
 }
Suggestion importance[1-10]: 8

__

Why: Adding a nil check for Scopes prevents a potential panic and improves robustness; the proposed code matches the context and addresses a legitimate runtime risk.

Medium
Cache JWT config reference

Avoid repeated calls to GetJWTConfiguration() to prevent accidental nil-pointer
dereferences if the configuration is absent. Cache the configuration once and read
both BasePolicyClaims and PolicyFieldName from it.

gateway/mw_jwt.go [386-389]

-fieldNames := k.Spec.OAS.GetJWTConfiguration().BasePolicyClaims
-if len(fieldNames) == 0 && k.Spec.OAS.GetJWTConfiguration().PolicyFieldName != "" {
-	fieldNames = append(fieldNames, k.Spec.OAS.GetJWTConfiguration().PolicyFieldName)
+cfg := k.Spec.OAS.GetJWTConfiguration()
+fieldNames := cfg.BasePolicyClaims
+if len(fieldNames) == 0 && cfg.PolicyFieldName != "" {
+	fieldNames = append(fieldNames, cfg.PolicyFieldName)
 }
Suggestion importance[1-10]: 6

__

Why: Caching GetJWTConfiguration() into cfg reduces repeated calls and slightly improves clarity; it's accurate to the code context and low risk, but a minor optimization rather than a critical fix.

Low
Use local JWT config var

Store the JWT configuration in a local variable before accessing its fields to avoid
multiple calls and potential nil dereferences. This also makes the fallback to
IdentityBaseField safer and clearer.

gateway/mw_jwt.go [456-459]

-identityBaseFields := k.Spec.OAS.GetJWTConfiguration().SubjectClaims
-if len(identityBaseFields) == 0 && k.Spec.OAS.GetJWTConfiguration().IdentityBaseField != "" {
-	identityBaseFields = append(identityBaseFields, k.Spec.OAS.GetJWTConfiguration().IdentityBaseField)
+cfg := k.Spec.OAS.GetJWTConfiguration()
+identityBaseFields := cfg.SubjectClaims
+if len(identityBaseFields) == 0 && cfg.IdentityBaseField != "" {
+	identityBaseFields = append(identityBaseFields, cfg.IdentityBaseField)
 }
Suggestion importance[1-10]: 6

__

Why: Storing the JWT config locally simplifies the code and avoids repeated calls; correct and helpful but not essential to functionality.

Low

@github-actions GitHub Actions
Copy link
Contributor

📦 Impact Review Snapshot

Effort Downstream Updates Compatibility Docs TL;DR
2/5 ✅ 🟢 📖 Adds backward compatibility fallbacks for JWT claim validation fields
## Impact Assessment

This PR adds backward compatibility fallbacks for JWT claim validation in the OAS implementation. The changes ensure that when newer array-based fields are empty, the system falls back to using older single-field configurations:

  1. If BasePolicyClaims is empty, use PolicyFieldName
  2. If SubjectClaims is empty, use IdentityBaseField
  3. If Scopes.Claims is empty, use Scopes.ClaimName

These changes are purely additive and enhance backward compatibility rather than breaking it. The PR doesn't modify any API schemas or interfaces, only internal behavior to handle empty arrays more gracefully.

## Required Updates

No immediate updates are required in downstream repositories as this change is backward compatible and doesn't modify any schemas or interfaces. The changes are internal to the JWT middleware implementation and don't affect how other systems interact with Tyk Gateway.

However, documentation in the portal repository could be updated to reflect these fallback behaviors for clarity.

## Compatibility Concerns

No compatibility concerns identified. This PR specifically improves backward compatibility by ensuring older single-field configurations continue to work even when newer array-based fields are introduced. The changes maintain the existing behavior while adding fallback paths.

The type assertions used (claims[c].(string)) assume claim values are strings, which is consistent with existing code. The PR doesn't introduce any new assumptions about data types.

## Summary & Recommendations
  • This PR enhances backward compatibility without introducing breaking changes
  • No immediate downstream updates are required, but documentation could be updated to reflect these fallbacks
  • The changes are well-tested with expanded unit tests for the fallback scenarios
  • No suggestions to provide – change LGTM.

Tip: Mention me again using /dependency <request>.
Powered by Probe AI
Tyk Gateway Dependency Impact Reviewer

@github-actions GitHub Actions
Copy link
Contributor

🛡️ Security Snapshot

Effort Risk Level Tests Compliance TL;DR
Low 🟢 ✅ ✔️ Backward compatibility changes for JWT claim validation with no security concerns
## Security Impact Analysis

The PR adds fallback mechanisms for JWT claim validation in three areas:

  1. In getPolicyIDFromToken: When BasePolicyClaims array is empty, it falls back to using the deprecated PolicyFieldName field
  2. In getUserIDFromClaimOAS: When SubjectClaims array is empty, it falls back to using the deprecated IdentityBaseField field
  3. In getScopeClaimNameOAS: When Scopes.Claims array is empty, it falls back to using the deprecated Scopes.ClaimName field

These changes maintain backward compatibility while preserving the precedence of newer configuration options. The code maintains proper validation and error handling for empty values.

## Identified Vulnerabilities

No vulnerabilities were identified in the implementation. The changes:

  • Maintain proper validation of JWT claims
  • Preserve existing error handling for empty or invalid claims
  • Follow the same security patterns as the existing code
  • Don't introduce new attack vectors or weaken existing security controls
## Security Recommendations

While no security issues were found, consider:

  • Adding comments in the code to clarify the fallback behavior and deprecation status
  • Ensuring documentation is updated to reflect these backward compatibility mechanisms
  • Consider adding logging when fallbacks are used to help identify configurations using deprecated fields
## OWASP Compliance

The changes don't impact OWASP compliance:

  • No impact on authentication or authorization security (A2:2021 - Cryptographic Failures)
  • Maintains proper JWT validation (A7:2021 - Identification and Authentication Failures)
  • No changes to access control mechanisms (A1:2021 - Broken Access Control)
## Summary
  • The changes provide backward compatibility for JWT claim validation without compromising security
  • The implementation maintains proper validation and error handling
  • The precedence is correctly set (array-based configurations take priority over single field configurations)
  • No security issues identified – change LGTM.

Tip: Mention me again using /security <request>.
Powered by Probe AI
Security Impact Reviewer Prompt

@github-actions GitHub Actions
Copy link
Contributor

🚀 Performance Snapshot

Effort Perf Risk Hot Paths Benchmarks TL;DR
Low 🟢 ✅ ❔ Minimal impact fallbacks for backward compatibility in JWT claim validation
## Performance Impact Analysis

The PR adds three fallback mechanisms in the JWT middleware to maintain backward compatibility:

  1. In getPolicyIDFromToken: Fallback to PolicyFieldName when BasePolicyClaims array is empty
  2. In getUserIDFromClaimOAS: Fallback to IdentityBaseField when SubjectClaims array is empty
  3. In getScopeClaimNameOAS: Fallback to Scopes.ClaimName when Scopes.Claims array is empty

These changes add minimal overhead (a single length check and conditional append) only when the newer array-based configurations are empty, ensuring smooth migration from older configurations.

## Critical Areas

The changes affect JWT claim validation which is on the critical authentication path, but the impact is negligible:

  • The additional checks only execute when arrays are empty (fallback path)
  • The code only adds a single conditional branch with minimal operations
  • No additional memory allocations in the hot path when arrays are populated
  • Type assertions remain unchanged, maintaining existing performance characteristics
## Optimization Recommendations

The implementation is already well-optimized for the use case. No further optimizations are necessary as:

  • The fallback logic is simple and efficient
  • The changes maintain the existing code structure and flow
  • The PR includes comprehensive tests that verify both primary and fallback paths
  • The implementation correctly preserves precedence (array values take priority over single field fallbacks)
## Summary
  • The PR adds minimal overhead only in fallback scenarios
  • The JWT validation hot path remains efficient for the primary use case
  • The changes are well-tested and maintain backward compatibility
  • No performance concerns identified in the implementation

No suggestions to provide – change LGTM.

Tip: Mention me again using /performance <request>.
Powered by Probe AI
Performance Impact Reviewer Prompt

@kofoworola kofoworola force-pushed the chore/improve-jwt-backwards branch from 68156c7 to a100dc3 Compare August 11, 2025 17:03
@github-actions GitHub Actions
Copy link
Contributor

🚀 Performance Snapshot

Effort Perf Risk Hot Paths Benchmarks TL;DR
Low 🟢 ✅ ❔ Minimal impact fallbacks for backward compatibility in JWT claim validation
## Performance Impact Analysis

The PR adds three fallback mechanisms in the JWT middleware to maintain backward compatibility:

  1. In getPolicyIDFromToken: Fallback to PolicyFieldName when BasePolicyClaims array is empty
  2. In getUserIDFromClaimOAS: Fallback to IdentityBaseField when SubjectClaims array is empty
  3. In getScopeClaimNameOAS: Fallback to Scopes.ClaimName when Scopes.Claims array is empty

These changes add minimal overhead (a single length check and conditional append) only when the newer array-based configurations are empty, ensuring smooth migration from older configurations.

## Critical Areas

The changes affect JWT claim validation which is on the critical authentication path, but the impact is negligible:

  • The additional checks only execute when arrays are empty (fallback path)
  • The code only adds a single conditional branch with minimal operations
  • No additional memory allocations in the hot path when arrays are populated
  • Type assertions remain unchanged, maintaining existing performance characteristics
## Optimization Recommendations

The implementation is already well-optimized for the use case. No further optimizations are necessary as:

  • The fallback logic is simple and efficient
  • The changes maintain the existing code structure and flow
  • The PR includes comprehensive tests that verify both primary and fallback paths
  • The implementation correctly preserves precedence (array values take priority over single field fallbacks)
## Summary
  • The PR adds minimal overhead only in fallback scenarios
  • The JWT validation hot path remains efficient for the primary use case
  • The changes are well-tested and maintain backward compatibility
  • No performance concerns identified in the implementation

No suggestions to provide – change LGTM.

Tip: Mention me again using /performance <request>.
Powered by Probe AI
Performance Impact Reviewer Prompt

@github-actions GitHub Actions
Copy link
Contributor

🛡️ Security Snapshot

Effort Risk Level Tests Compliance TL;DR
Low 🟢 ✅ ✔️ Backward compatibility fallbacks for JWT claim validation with no security concerns
## Security Impact Analysis

This PR adds fallback mechanisms for JWT claim validation in the OAS implementation to improve backward compatibility. The changes ensure that when newer array-based fields are empty, the system falls back to using older single-field configurations:

  1. In getPolicyIDFromToken: When BasePolicyClaims array is empty, falls back to PolicyFieldName
  2. In getUserIDFromClaimOAS: When SubjectClaims array is empty, falls back to IdentityBaseField
  3. In getScopeClaimNameOAS: When Scopes.Claims array is empty, falls back to Scopes.ClaimName

These changes maintain proper validation and error handling for empty values, preserving the security properties of the original implementation.

## Identified Vulnerabilities

No vulnerabilities were identified in the implementation. The changes:

  • Maintain proper validation of JWT claims
  • Preserve existing error handling for empty or invalid claims
  • Follow the same security patterns as the existing code
  • Don't introduce new attack vectors or weaken existing security controls
  • Properly handle type assertions with appropriate error checking
## Security Recommendations

While no security issues were found, consider these minor improvements:

  1. Cache the JWT configuration in a local variable to avoid multiple calls to GetJWTConfiguration(), which could help prevent potential nil pointer dereferences
  2. Add nil checks for Scopes before accessing its fields to prevent panics
  3. Add comments to clarify the fallback behavior for future maintainers
## OWASP Compliance

The changes don't impact OWASP compliance:

  • No impact on authentication or authorization security (A2:2021 - Cryptographic Failures)
  • Maintains proper JWT validation (A7:2021 - Identification and Authentication Failures)
  • No changes to access control mechanisms (A1:2021 - Broken Access Control)
  • The type assertions used (claims[c].(string)) are consistent with existing code patterns
## Summary
  • The changes provide backward compatibility for JWT claim validation without compromising security
  • The implementation maintains proper validation and error handling
  • The precedence is correctly set (array-based configurations take priority over single field configurations)
  • The PR is well-tested with comprehensive unit tests covering the fallback scenarios
  • No security issues identified – change LGTM.

Tip: Mention me again using /security <request>.
Powered by Probe AI
Security Impact Reviewer Prompt

@github-actions GitHub Actions
Copy link
Contributor

📦 Impact Review Snapshot

Effort Downstream Updates Compatibility Docs TL;DR
Low ✅ 🟢 📖 Adds backward compatibility fallbacks for JWT claim validation with no schema changes
## Impact Assessment

This PR adds backward compatibility fallbacks for JWT claim validation in the OAS implementation. The changes ensure that when newer array-based fields are empty, the system falls back to using older single-field configurations:

  1. Uses PolicyFieldName if BasePolicyClaims array is empty
  2. Uses IdentityBaseField if SubjectClaims array is empty
  3. Uses Scopes.ClaimName if Scopes.Claims array is empty

These changes are purely additive and enhance backward compatibility rather than breaking it. The implementation maintains the existing precedence (array values take priority over single field values) while adding graceful fallbacks.

## Required Updates

No immediate updates are required in downstream repositories as this change is backward compatible and doesn't modify any schemas or interfaces. The changes are internal to the JWT middleware implementation and don't affect how other systems interact with Tyk Gateway.

  • tyk-operator: No changes needed as the API definition schema remains unchanged
  • tyk-charts: No configuration changes required
  • portal: No updates needed, though documentation could be updated to reflect these fallback behaviors
  • tyk-sink: No changes needed as the JWT validation behavior remains compatible
## Compatibility Concerns

No compatibility concerns identified. This PR specifically improves backward compatibility by ensuring older single-field configurations continue to work even when newer array-based fields are introduced. The changes maintain the existing behavior while adding fallback paths.

The implementation correctly preserves precedence - array-based configurations take priority, and fallbacks are only used when arrays are empty. This ensures consistent behavior with existing configurations.

## Summary & Recommendations
  • This PR enhances backward compatibility without introducing breaking changes
  • No immediate downstream updates are required
  • The changes are well-tested with expanded unit tests for the fallback scenarios
  • Consider updating documentation to reflect these fallbacks for clarity

No suggestions to provide – change LGTM.

Tip: Mention me again using /dependency <request>.
Powered by Probe AI
Tyk Gateway Dependency Impact Reviewer

@github-actions GitHub Actions
Copy link
Contributor

🚦 Connectivity Review Snapshot

Effort Tests Security Perf TL;DR
Low ✅ 🔒 none 🟢 Adds backward compatibility fallbacks for JWT claim validation with no connectivity impact
## Connectivity Assessment
  • Redis Connections: No changes to Redis connection handling or storage patterns. The PR only adds fallback logic for JWT claim validation.
  • RPC Connections: No impact on RPC connections or MDCB mode functionality. The changes are isolated to JWT claim validation logic.
  • Synchronization Mechanisms: No changes to synchronization mechanisms or pub/sub channels. The PR maintains existing behavior while adding fallbacks.
## Test Coverage Validation
  • Redis Tests: The PR doesn't modify Redis storage or connection logic, so no specific Redis tests were needed.
  • RPC Tests: No RPC-related changes, so no RPC tests were required.
  • Failure Scenario Tests: The PR includes comprehensive tests for the fallback scenarios, ensuring that when newer array-based fields are empty, the system properly falls back to using older single-field configurations.
## Security & Performance Impact
  • Authentication Changes: The changes enhance backward compatibility without modifying the security model. The JWT validation process remains secure with proper claim validation.
  • Performance Considerations: Minimal performance impact as the fallback logic only adds simple conditional checks that execute only when arrays are empty.
  • Error Handling: Error handling remains consistent with the existing implementation, maintaining proper error reporting and logging.
## Summary & Recommendations
  • The PR adds backward compatibility fallbacks for JWT claim validation in a clean, non-intrusive way.
  • The implementation correctly preserves precedence (array values take priority over single field fallbacks).
  • The changes are well-tested with specific test cases for each fallback scenario.
  • No suggestions to provide – change LGTM.

Tip: Mention me again using /connectivity <request>.
Powered by Probe AI
Connectivity Issues Reviewer Prompt for Tyk Gateway

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@kofoworola kofoworola merged commit 35d2b2d into master Aug 12, 2025
43 of 44 checks passed
@kofoworola kofoworola deleted the chore/improve-jwt-backwards branch August 12, 2025 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pvormste pvormste pvormste approved these changes

@lghiur lghiur Awaiting requested review from lghiur

@edsonmichaque edsonmichaque Awaiting requested review from edsonmichaque

@andrei-tyk andrei-tyk Awaiting requested review from andrei-tyk

Assignees
No one assigned
Labels
Review effort 2/5 Story Tyk Product Development
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kofoworola @buger @pvormste