Implement access token context encoding framework - #3
Conversation
closes #37118 Signed-off-by: mposolda <mposolda@gmail.com>
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| Objects.requireNonNull(sessionType, "Null sessionType not allowed"); | ||
| Objects.requireNonNull(tokenType, "Null tokenType not allowed"); | ||
| Objects.requireNonNull(grantType, "Null grantType not allowed"); | ||
| Objects.requireNonNull(grantType, "Null rawTokenId not allowed"); |
There was a problem hiding this comment.
Raw token ID null check omitted
Low Severity
AccessTokenContext validates grantType twice and never validates rawTokenId. A null rawTokenId can be stored in the context, which can later produce malformed encoded IDs or unexpected failures while decoding, and the thrown validation message points to the wrong field.
| if (items.length != 2) return false; | ||
| // Grant type shortcut starts at character 4th char and is 2-chars long | ||
| if (items[0].substring(3, 5).equals(expectedGrantShortcut)) return false; | ||
| return isUUID().matches(items[1]); |
There was a problem hiding this comment.
Grant matcher logic is inverted
Medium Severity
isAccessTokenId rejects tokens when the extracted grant shortcut matches expectedGrantShortcut, and it slices items[0] with the wrong indexes. This makes the matcher accept incorrect grants and stop validating grant encoding in event assertions.
Additional Locations (1)
2 participants
Test 8nnn## Summary by CodeRabbitnn* New Featuresn * Introduced token context encoding system enabling metadata encoding and retrieval for access tokens.n * Added grant type shortcut support across OAuth2 grant types for improved token processing.nn* Bug Fixesn * Fixed variable naming error in token audience processing logic.nn✏️ Tip: You can customize this high-level summary in your review settings.nnn---nReplicated from ai-code-review-evaluation/keycloak-coderabbit#8
Note
High Risk
Changes the format of access token IDs and the core token issuance path, which can affect token validation/introspection, event logging, and any integrations assuming UUID-only token IDs; also adds new SPI wiring that must be present at runtime.
Overview
Introduces a new internal
tokenContextEncoderSPI (default implementation + tests) that encodes token context into the access tokenjti/ID as a compact prefix (session type, token type, grant shortcut) and can decode it later.Updates token issuance to use this encoder in
TokenManager.initToken, and threads the current OAuth2grant_typeintoClientSessionContext(newConstants.GRANT_TYPE, captured inOAuth2GrantType.Context, set in core grant flows including refresh/token-exchange/ROPC/pre-authorized code).Extends
OAuth2GrantTypeFactorywithgetShortcut()and implements shortcuts across built-in grant factories; also fixes a small variable naming typo in requested-audience handling and adjusts event assertion tests to expect the new token-id format.Written by Cursor Bugbot for commit c873872. Configure here.