Feat: support downscoping for ID-JAG assertions

[mlajkim]

Description

The Athenz ZTS server currently requires an ID-JAG with a mandatory scope; otherwise, it will return the following error:

Invalid jag assertion - missing scope

This PR changes the scope handling so that instead of strictly enforcing the mandatory scope from the assertion, it now uses the requester's requested scope, originally granted to the authorized accessor. This allows a requester (e.g., an AI Agent) to voluntarily downscope its own permissions for safer access, aligning with the RFC 7521 standard, which states the following:

"As such, the requested scope MUST be equal to or less than the scope originally granted to the authorized accessor. The authorization server MUST limit the scope of the issued access token to be equal to or less than the scope originally granted..." — RFC 7521, Section 4.1

What's done

• Ensured permissions are downscoped only when requested
• Added more tests

Related Issue: #3100

Contribution Checklist:

• The pull request does not introduce any breaking changes
• I have read the contribution guidelines.
• Create an issue and link to the pull request.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a crucial security enhancement by enforcing scope subsetting for JWT-bearer requests. It ensures that any requested scope is a valid, narrower version of the scope granted in the original assertion, thereby preventing privilege escalation and adhering to standard security practices for token exchange.

Highlights

• Scope Subset Validation: Implemented validation to ensure that if a 'scope' parameter is provided in a JWT-bearer request, it must be a strict subset of the scope present in the assertion (JAG) token, aligning with RFC7523.
• Assertion Scope Handling: Added logic to explicitly check for the presence of an assertion scope when a request scope is provided, preventing requests with scopes when the assertion itself has none.
• Token Scope Override: Modified the jagTokenObj to use the requested, validated subset of scopes for downstream processing, effectively down-scoping the token.

Changelog

• servers/zts/src/main/java/com/yahoo/athenz/zts/token/AccessTokenRequest.java
  • Added imports for java.util.Arrays, java.util.HashSet, and java.util.Set to support collection operations.
  • Introduced new validation logic within validateJWTBearerRequest to check if the requested scope is a subset of the assertion scope.
  • Updated the jagTokenObj to reflect the down-scoped token if a valid subset was requested.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The pull request adds support for down-scoping JWT-bearer tokens by providing a scope parameter, as per RFC7523. The implementation is mostly correct, but there's a potential issue with how scope strings are parsed, specifically when they contain only whitespace. This could lead to incorrect behavior in edge cases. I've provided a suggestion to make the scope parsing more robust.

[mlajkim]

/gemini review

[gemini-code-assist]

Code Review

This pull request adds scope subset validation for ID-JAG token requests. However, the current implementation has a potential NullPointerException if the assertion token lacks a scope claim, which could lead to a minor Denial of Service (DoS) attack. A fix for this medium-severity issue has been suggested. Additionally, a medium-severity suggestion is provided to improve robustness by handling duplicate scopes in the request.

[gemini-code-assist]

Code Review

The pull request adds scope validation for ID-JAG tokens, ensuring that a requested scope is a subset of the scope in the assertion token. My review identifies a potential NullPointerException if the assertion token does not contain a scope, which is a high-severity issue. I've provided a fix to handle this case gracefully. Additionally, I've suggested an improvement to handle duplicate scopes in the request, ensuring the resulting token has a normalized, unique set of scopes, which aligns better with OAuth 2.0 specifications.

[mlajkim]

/gemini review

[gemini-code-assist]

Code Review

This pull request adds support for scope subsetting for ID-JAG tokens as per RFC 7521. When a scope is provided in a JWT Bearer Grant request, the code now validates that the requested scope is a subset of the scope in the original assertion token. My review includes a suggestion to use LinkedHashSet instead of HashSet to preserve the order of scopes, ensuring deterministic behavior.

[mlajkim]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a critical security enhancement by validating that the requested scope in a JWT Bearer Grant flow is a subset of the scope granted in the original assertion token, preventing a client from escalating its privileges. The implementation correctly parses the space-delimited scopes and performs the subset check. No specific security issues or vulnerabilities were identified in the provided code changes, and the potential edge case regarding trimmedScope was evaluated and determined to be handled correctly. There are a couple of suggestions to improve code style and maintainability.

[mlajkim]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a critical security enhancement for JWT Bearer grant type requests. It adds validation to ensure that if a scope is requested, it must be a subset of or equal to the scope present in the assertion token (ID-JAG). This prevents a client from escalating privileges by requesting a token with a broader scope than originally granted. The implementation appears correct, and the accompanying review comment correctly identifies the crucial need for unit tests to verify the new logic and prevent regressions. Overall, the changes are a significant improvement to the security of the token exchange flow.

[mlajkim]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements scope validation and downscoping for JAG token exchanges, aligning with RFC 7521. My review includes a suggestion for code simplification and a note about adding unit tests for this new security-critical logic.

_{Note: Security Review did not run due to the size of the PR.}

[mlajkim]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a critical security enhancement by enforcing that the requested scope for an access token is a subset of the scope in the JWT Assertion for Grant (JAG) token. This prevents scope escalation. The implementation correctly validates the scopes and down-scopes the token if a valid subset is requested. The accompanying tests are comprehensive and cover various scenarios using a data provider, which is a great improvement. I have one suggestion to improve the robustness of the scope validation.

_{Note: Security Review did not run due to the size of the PR.}

[mlajkim]

/gemini summary

[gemini-code-assist]

Summary of Changes

This pull request significantly strengthens the security posture of the Athenz Token Service (ZTS) by introducing critical validation for ID-JAG token exchanges. It mandates the presence of a scope in ID-JAG assertions and enforces strict downscoping rules, ensuring that any requested Access Token scope is a valid subset of the originally asserted scope. This prevents potential privilege escalation and aligns the system with established OAuth 2.0 security best practices.

Highlights

• Enforced Mandatory Scope for ID-JAG Tokens: Athenz now strictly requires a scope parameter in ID-JAG tokens to determine the role for the issued Access Token, even though RFC 7523 defines it as optional.
• Implemented Scope Downscoping Validation: New logic ensures that if a scope is requested during an Access Token exchange, it must be a subset of the scope originally granted in the ID-JAG token, adhering to RFC 7521.
• Enhanced Test Coverage for Scope Validation: Comprehensive parameterized tests were added to validate various scenarios of scope requests, including valid subsets, supersets, and error conditions, ensuring the robustness of the new security measures.

Changelog

• servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSImpl.java
  • Added new validation logic to ensure that if a scope is provided in an Access Token exchange request, it must be a subset of the scope present in the ID-JAG token.
  • Updated the scopeStd variable to reflect the validated and potentially downscoped value for the issued Access Token.
  • Introduced detailed comments explaining the mandatory nature of the scope and the downscoping requirements based on RFCs.
  • Modified debug logging to include assertion scope and requested scope for better traceability.
• servers/zts/src/test/java/com/yahoo/athenz/zts/ZTSImplAccessTokenTest.java
  • Introduced a DataProvider named jagTokenExchangeCases to supply various test data for scope validation scenarios.
  • Refactored the testProcessJAGTokenExchangeRequestSuccessWithSpecificRoles method into a new parameterized test testProcessJAGTokenExchangeRequest to utilize the DataProvider.
  • Expanded test coverage to include numerous cases for valid and invalid scope requests, ensuring correct behavior for subset validation, error handling, and principal role inclusion.

Activity

• gemini-code-assist[bot] provided an initial summary of the PR.
• mlajkim repeatedly requested /gemini review and /gemini summary.
• gemini-code-assist[bot] provided several review comments, including:
• Suggestions for robustly handling whitespace-only scope strings.
• Identification of potential NullPointerException issues with getScopeStd() and recommendations for null and empty string checks.
• Recommendations to use LinkedHashSet for preserving scope order and ensuring uniqueness.
• Suggestions to simplify scope checks using StringUtil.isBlank().
• A critical request for adding comprehensive unit tests for the new scope validation logic, which mlajkim acknowledged and later confirmed as handled.
• mlajkim addressed several review comments, confirming they were "handled".

[mlajkim]

@havetisyan, let me know what you think when you get a chance.