Skip to content

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

Merged
andrei-tyk merged 7 commits intomasterfrom
TT-14359-fix-nested-scopes-for-identity-base-field
Nov 10, 2025
Merged

[TT-14359] fix nested scopes for identity base field#7522
andrei-tyk merged 7 commits intomasterfrom
TT-14359-fix-nested-scopes-for-identity-base-field

Conversation

@andrei-tyk
Copy link
Copy Markdown
Contributor

@andrei-tyk andrei-tyk commented Nov 7, 2025

Description

This PR fixes TT-14359 by adding support for nested JWT claims in the JWTIdentityBaseField and JWTPolicyFieldName configuration fields, allowing users to specify claim paths like user.id or
authorization.policy to extract values from nested JSON structures in JWT tokens. The implementation maintains full backward compatibility by first checking for literal claim keys (including keys that
contain actual dots in their names) before attempting nested lookup, and includes proper fallback behavior to the sub claim when nested identity fields are not found. The changes extend the existing
nestedMapLookup function (previously only used for scopes) to also support identity base field and policy field resolution, with comprehensive test coverage including single-level nesting, multi-level
nesting, fallback scenarios, empty string handling, non-string value handling, multiple policy claim priority, backward compatibility verification, and the specific customer scenario from the ticket
where claims are nested under a test object.

Related Issue

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

Ticket Details

TT-14359
Status In Code Review
Summary Support nested locations for JWT policy and subject claims

Generated at: 2025-11-10 09:55:12

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 7, 2025

API Changes

no api changes detected

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Nov 7, 2025

🔍 Code Analysis Results

This PR introduces support for nested JWT claims for identity and policy fields, allowing for more flexible integration with identity providers that issue tokens with complex, structured data.

The core of this change is enabling the JWTIdentityBaseField and JWTPolicyFieldName configuration fields to accept dot-separated paths (e.g., user.id or authorization.policy) to extract values from nested JSON objects within a JWT.

To ensure full backward compatibility, the implementation introduces a two-step lookup process encapsulated in a new getClaimValue function:

  1. It first attempts a literal key lookup to find a claim with the exact name provided. This handles existing configurations and edge cases where claim names contain literal dots.
  2. If the literal lookup fails and the field name contains a dot, it proceeds with a nested lookup, traversing the claims map to find the specified value.

The functions getPolicyIDFromToken and getUserIDFromClaim have been refactored to use this new helper, centralizing the claim resolution logic. The PR also includes a comprehensive new test suite that validates single and multi-level nesting, fallback behavior to the sub claim, error handling for empty or non-string values, and backward compatibility.

Files Changed Analysis

  • gateway/mw_jwt.go (+70, -44): Contains the primary logic changes. The getPolicyIDFromToken and getUserIDFromClaim functions were refactored to use the new getClaimValue helper function, which centralizes the literal-then-nested lookup logic.
  • gateway/mw_jwt_nested_claims_test.go (+448, -0): A new test file is added, providing extensive coverage for the new functionality. The large number of additions demonstrates a thorough testing strategy, covering numerous use cases and edge cases to ensure the change is robust.

Architecture & Impact Assessment

  • What this PR accomplishes: It enhances the JWT middleware to support nested claims for identity and policy lookups. This increases flexibility and makes it easier to integrate Tyk with identity providers that use structured JWTs.
  • Key technical changes introduced: A new getClaimValue helper function implements a backward-compatible, two-step claim resolution strategy (literal lookup, then nested lookup). This change reuses the existing nestedMapLookup function, ensuring a consistent approach to claim parsing within the middleware.
  • Affected system components: The changes are scoped to the JWT Middleware in the Tyk Gateway. The impact is limited to APIs that use JWT authentication. Due to the backward-compatible design, no changes are required for existing API definitions.

JWT Claim Resolution Flow

The diagram below illustrates the updated logic for resolving a claim value.

graph TD
    A[Start Claim Resolution] --> B[getClaimValue(claims, fieldName)];
    B --> C{Attempt Literal Lookup: claims[fieldName]};
    C --> D{Found & Not Empty String?};
    D -- Yes --> E[Return Value];
    D -- No --> F{Field Name contains "."?};
    F -- No --> G[Claim Not Found / Fallback];
    F -- Yes --> H[Perform Nested Lookup];
    H --> I{Found & Not Empty String?};
    I -- Yes --> E;
    I -- No --> G;
    E --> Z[End];
    G --> Z[End];
Loading

Scope Discovery & Context Expansion

  • The changes are well-contained within the gateway module, specifically the JWT middleware implementation in mw_jwt.go.
  • The PR reuses the nestedMapLookup function, which was already present and used for resolving JWT scopes. This extends a consistent pattern for claim parsing to the identity and policy fields.
  • The impact is limited to the processing of inbound JWTs for APIs with JWT authentication enabled. Other authentication modes and gateway functionalities are unaffected.
  • The new test file gateway/mw_jwt_nested_claims_test.go provides clear examples of the intended functionality and edge cases considered, such as handling literal dots in claim names, which is crucial for backward compatibility.
Metadata
  • Review Effort: 2 / 5
  • Primary Label: enhancement

Powered by Visor from Probelabs

Last updated: 2025-11-10T10:01:20.082Z | Triggered by: synchronize | Commit: 2d93816

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Nov 7, 2025

🔍 Code Analysis Results

Security Issues (1)

Severity Location Issue
🟡 Warning gateway/mw_jwt.go:391-424
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.
💡 SuggestionReplace the '?' with '—' in both log messages for clarity and consistency with the surrounding codebase.

Architecture Issues (1)

Severity Location Issue
🟠 Error gateway/mw_jwt.go:1531-1548
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.
💡 SuggestionRefactor 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.

Performance Issues (1)

Severity Location Issue
🟡 Warning gateway/mw_jwt.go:1532-1545
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.
💡 SuggestionRefactor 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.

Quality Issues (2)

Severity Location Issue
🟠 Error gateway/mw_jwt.go:1525-1554
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.
💡 SuggestionModify `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.
🟡 Warning gateway/mw_jwt.go:388
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.
💡 SuggestionIt is recommended to revert the `?` to the original `—` or rephrase the message to improve clarity, for example: `...for APIID %s, URL %s, ignoring`.

✅ Dependency Check Passed

No dependency issues found – changes LGTM.

✅ Connectivity Check Passed

No connectivity issues found – changes LGTM.


Powered by Visor from Probelabs

Last updated: 2025-11-10T10:01:21.091Z | Triggered by: synchronize | Commit: 2d93816

💡 TIP: You can chat with Visor using /visor ask <your question>

@andrei-tyk andrei-tyk changed the title [Tt-14359] fix nested scopes for identity base field [TT-14359] fix nested scopes for identity base field Nov 7, 2025
@andrei-tyk andrei-tyk enabled auto-merge (squash) November 10, 2025 09:30
@sonarqubecloud
Copy link
Copy Markdown

@andrei-tyk andrei-tyk merged commit 26c5240 into master Nov 10, 2025
49 of 53 checks passed
@andrei-tyk andrei-tyk deleted the TT-14359-fix-nested-scopes-for-identity-base-field branch November 10, 2025 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants