Skip to content

Implement access token context encoding framework#3

Open
ShashankFC wants to merge 1 commit into
feature-token-context-baselinefrom
feature-token-context-implementation
Open

Implement access token context encoding framework#3
ShashankFC wants to merge 1 commit into
feature-token-context-baselinefrom
feature-token-context-implementation

Conversation

@ShashankFC

@ShashankFC ShashankFC commented Feb 13, 2026

Copy link
Copy Markdown

User description

Test 8

Summary by CodeRabbit

  • New Features

    • Introduced token context encoding system enabling metadata encoding and retrieval for access tokens.
    • Added grant type shortcut support across OAuth2 grant types for improved token processing.
  • Bug Fixes

    • Fixed variable naming error in token audience processing logic.

✏️ Tip: You can customize this high-level summary in your review settings.


Replicated from ai-code-review-evaluation/keycloak-coderabbit#8


CodeAnt-AI Description

Implement compact token-context encoding in access token IDs and add tests

What Changed

  • Access token IDs now include a 6-character prefix encoding session type (online/offline/transient), token type (regular/lightweight), and a 2-char grant shortcut, allowing the server to recover token context from the ID alone.
  • New internal SPI and default provider added to encode/decode token context; grant factories were extended with stable 2-char shortcuts and validation to avoid duplicate shortcuts.
  • Token creation and several grant flows now store or use the grant type in client session context so created token IDs include the correct grant shortcut.
  • Added unit tests validating encoding/decoding of token IDs and updated test helpers to assert token IDs contain expected grant shortcuts.
  • Fixed a naming bug for requested audience clients and exposed a getter for grant type in grant context where needed.

Impact

✅ Decode token session/type/grant from ID without DB lookup
✅ Fewer token-introspection lookups to recover token origin
✅ Clearer test assertions on token IDs

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

closes #37118

Signed-off-by: mposolda <mposolda@gmail.com>
@ShashankFC

Copy link
Copy Markdown
Author

@CodeAnt-AI: review

@codeant-ai

codeant-ai Bot commented Feb 13, 2026

Copy link
Copy Markdown

CodeAnt AI is running the review.

@codeant-ai codeant-ai Bot added the size:XL This PR changes 500-999 lines, ignoring generated files label Feb 13, 2026
@codeant-ai

codeant-ai Bot commented Feb 13, 2026

Copy link
Copy Markdown

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Possible NullPointerException
    The code in getTokenContextFromClientSessionContext assumes clientSessionContext.getClientSession()
    and the returned UserSessionModel are non-null and immediately calls methods on them. For some flows
    (e.g. client_credentials or other client-only flows) client session or user session may be null,
    which would cause a NullPointerException at runtime. Consider explicit null checks and a sensible
    fallback (UNKNOWN session type) for those cases.

  • Constructor Bug
    The constructor uses Objects.requireNonNull for grantType twice; the second call should validate rawTokenId. As-is, a null rawTokenId will not be detected and a misleading exception message is used.

  • Validation message mismatch
    AccessTokenContext constructor currently throws "Null rawTokenId not allowed" when checking grantType (duplicate). This mismatch may confuse debugging and logs; ensure validation messages match checked values.

  • Input validation / brittle parsing
    getTokenContextFromTokenId assumes non-null encodedTokenId and a strict format (contains ':' and
    encoded-context length == 6). Passing null, empty string, or wrong format will produce NPE or
    IllegalArgumentException with a message but no early guard. Add null/empty checks and clearer errors,
    and consider making the expected encoded-context length explicit or configurable.

  • Map consistency / dynamic refresh race
    The factory dynamically refreshes grant shortcut maps in getShortcutByGrantType/getGrantTypeByShortcut
    and also verifies map sizes during postInit. While ConcurrentHashMap avoids basic races, the size-based
    duplicate detection in postInit could be fragile during dynamic deploys/hot plugins. Consider additional
    validation logic (e.g. explicit duplicate detection during registration) and consistent atomic updates
    when adding new mapping entries.

Comment on lines +482 to +483
// Grant type shortcut starts at character 4th char and is 2-chars long
if (items[0].substring(3, 5).equals(expectedGrantShortcut)) return false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The isAccessTokenId matcher currently checks the wrong substring indices and inverts the equality condition for the grant shortcut, causing it to reject correctly encoded token IDs for the expected grant type and potentially accept incorrect ones, which will make tests and any assertions using it behave incorrectly. [logic error]

Severity Level: Major ⚠️
- ❌ OAuth code-to-token tests fail on TOKEN_ID assertions.
- ❌ Device and refresh flow tests using TOKEN_ID matchers can fail.
- ⚠️ New grant shortcut token encoding remains unverified by tests.
Suggested change
// Grant type shortcut starts at character 4th char and is 2-chars long
if (items[0].substring(3, 5).equals(expectedGrantShortcut)) return false;
// Grant type shortcut is encoded in the last 2 characters of the 6-character prefix
String grantShortcut = items[0].substring(4, 6);
if (!grantShortcut.equals(expectedGrantShortcut)) return false;
Steps of Reproduction ✅
1. Run the test suite that uses `AssertEvents.expectCodeToToken(...)`, for example `mvn
test -Dtest=RefreshTokenTest` in module `testsuite/integration-arquillian/tests/base`
(caller found at
`testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/RefreshTokenTest.java:219`
where `events.expectCodeToToken(codeId, sessionId).assertEvent();` is invoked).

2. During the test, an access token is obtained via the OAuth client (e.g.,
`oauth.doAccessTokenRequest(code)` in `RefreshTokenTest.refreshTokenRequest`, lines 29–32
in the snippet starting at file line 210) and the server emits an event with a
`Details.TOKEN_ID` that includes a grant shortcut like
`AuthorizationCodeGrantTypeFactory.GRANT_SHORTCUT` (`"ac"` defined in
`services/src/main/java/org/keycloak/protocol/oidc/grants/AuthorizationCodeGrantTypeFactory.java:32`).

3. The test then calls `events.expectCodeToToken(codeId, sessionId)` (defined in
`AssertEvents.java:142–149`), which sets a detail matcher `.detail(Details.TOKEN_ID,
isAccessTokenId(AuthorizationCodeGrantTypeFactory.GRANT_SHORTCUT))`, and finally
`.assertEvent()`; inside `ExpectedEvent.assertEvent(EventRepresentation actual)`
(`AssertEvents.java:421–445`), the actual `TOKEN_ID` from the event is matched against
`isAccessTokenId(...)` via Hamcrest `assertThat`.

4. In `isAccessTokenId` (`AssertEvents.java:476–484`), the code splits the token ID, then
executes `if (items[0].substring(3, 5).equals(expectedGrantShortcut)) return false;`, so
for a correctly encoded token whose prefix contains the expected grant shortcut,
`matchesSafely` returns `false`, causing `assertThat("Unexpected value for TOKEN_ID",
actualValue, is(d.getValue()))` to fail and the test to error, even though the token ID is
correct.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AssertEvents.java
**Line:** 482:483
**Comment:**
	*Logic Error: The `isAccessTokenId` matcher currently checks the wrong substring indices and inverts the equality condition for the grant shortcut, causing it to reject correctly encoded token IDs for the expected grant type and potentially accept incorrect ones, which will make tests and any assertions using it behave incorrectly.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

@codeant-ai

codeant-ai Bot commented Feb 13, 2026

Copy link
Copy Markdown

CodeAnt AI finished running the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL This PR changes 500-999 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants