Implement access token context encoding framework#8
Conversation
closes #37118 Signed-off-by: mposolda <mposolda@gmail.com>
WalkthroughThese changes introduce a new mechanism for encoding and decoding access token context metadata—such as session type, token type, and grant type—directly into token IDs using concise shortcuts. This involves new SPI, provider, and factory interfaces and implementations, updates to grant type factories to supply shortcuts, and modifications to token issuance and validation logic. Associated tests and event assertions are also updated to validate the new token ID format. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TokenManager
participant TokenContextEncoderProvider
participant ClientSessionContext
Client->>TokenManager: Request token (with grant type)
TokenManager->>ClientSessionContext: Retrieve session and grant info
TokenManager->>TokenContextEncoderProvider: getTokenContextFromClientSessionContext(...)
TokenContextEncoderProvider-->>TokenManager: AccessTokenContext
TokenManager->>TokenContextEncoderProvider: encodeTokenId(AccessTokenContext)
TokenContextEncoderProvider-->>TokenManager: Encoded Token ID
TokenManager-->>Client: Return token with encoded ID
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
services/src/main/java/org/keycloak/protocol/oidc/tokenexchange/StandardTokenExchangeProvider.java (1)
50-50: Remove unused import.The
TokenExchangeGrantTypeFactoryimport appears to be unused in this file.-import org.keycloak.protocol.oidc.grants.TokenExchangeGrantTypeFactory;services/src/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProviderFactory.java (1)
82-85: Improve error message to identify duplicate shortcuts.The validation detects duplicates but doesn't identify which shortcut or grant ID is duplicated, making debugging difficult.
// Validation if there are not duplicated shortcuts (for example when introducing new grant impl...) if (grantsByShortcuts.size() != grantsToShortcuts.size()) { - throw new IllegalStateException("Different lengths of maps. grantsByShortcuts.size=" + grantsByShortcuts.size() + ", grantsToShortcuts.size=" + grantsToShortcuts.size() + - ". Make sure that there is no OAuth2GrantType implementation with same ID or shortcut like other grants"); + // Find the duplicate + Map<String, String> duplicates = new HashMap<>(); + grantsByShortcuts.forEach((shortcut, grant) -> { + if (duplicates.containsKey(shortcut)) { + throw new IllegalStateException("Duplicate grant shortcut '" + shortcut + "' used by both '" + grant + "' and '" + duplicates.get(shortcut) + "'"); + } + duplicates.put(shortcut, grant); + }); + throw new IllegalStateException("Different lengths of maps. grantsByShortcuts.size=" + grantsByShortcuts.size() + ", grantsToShortcuts.size=" + grantsToShortcuts.size()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
server-spi-private/src/main/java/org/keycloak/models/Constants.java(1 hunks)server-spi-private/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantType.java(4 hunks)server-spi-private/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantTypeFactory.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java(4 hunks)services/src/main/java/org/keycloak/protocol/oidc/encode/AccessTokenContext.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProvider.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProviderFactory.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/encode/TokenContextEncoderProvider.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/encode/TokenContextEncoderProviderFactory.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/encode/TokenContextEncoderSpi.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/grants/AuthorizationCodeGrantTypeFactory.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/grants/ClientCredentialsGrantTypeFactory.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantTypeBase.java(2 hunks)services/src/main/java/org/keycloak/protocol/oidc/grants/PermissionGrantTypeFactory.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/grants/PreAuthorizedCodeGrantType.java(2 hunks)services/src/main/java/org/keycloak/protocol/oidc/grants/PreAuthorizedCodeGrantTypeFactory.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/grants/RefreshTokenGrantTypeFactory.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/grants/ResourceOwnerPasswordCredentialsGrantType.java(2 hunks)services/src/main/java/org/keycloak/protocol/oidc/grants/ResourceOwnerPasswordCredentialsGrantTypeFactory.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/grants/TokenExchangeGrantTypeFactory.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/grants/ciba/CibaGrantTypeFactory.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/grants/device/DeviceGrantTypeFactory.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/mappers/AbstractOIDCProtocolMapper.java(1 hunks)services/src/main/java/org/keycloak/protocol/oidc/tokenexchange/StandardTokenExchangeProvider.java(2 hunks)services/src/main/resources/META-INF/services/org.keycloak.protocol.oidc.encode.TokenContextEncoderProviderFactory(1 hunks)services/src/main/resources/META-INF/services/org.keycloak.provider.Spi(1 hunks)services/src/test/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProviderTest.java(1 hunks)testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AssertEvents.java(6 hunks)
🔇 Additional comments (45)
services/src/main/java/org/keycloak/protocol/oidc/encode/TokenContextEncoderProviderFactory.java (1)
27-28: LGTM! Clean factory interface following Keycloak SPI patterns.The interface correctly extends
ProviderFactory<TokenContextEncoderProvider>and follows the standard Keycloak SPI factory pattern.services/src/main/java/org/keycloak/protocol/oidc/encode/TokenContextEncoderSpi.java (1)
29-51: LGTM! Well-structured SPI implementation.The SPI correctly:
- Marks itself as internal, appropriate for infrastructure functionality
- Uses descriptive naming ("tokenContextEncoder")
- Properly references the provider and factory classes
- Follows standard Keycloak SPI patterns
services/src/main/java/org/keycloak/protocol/oidc/grants/ResourceOwnerPasswordCredentialsGrantType.java (2)
34-34: LGTM! Import addition supports the grant type standardization.The Constants import is necessary for accessing the standardized
GRANT_TYPEconstant.
134-134: LGTM! Proper integration with token context encoding framework.Setting the
GRANT_TYPEattribute in the client session context enables the token context encoder to embed grant type information into token IDs. The placement after session context creation and before token generation is correct.services/src/main/java/org/keycloak/protocol/oidc/grants/PreAuthorizedCodeGrantType.java (2)
32-32: LGTM! Import supports grant type standardization.The Constants import enables access to the standardized
GRANT_TYPEconstant for consistent attribute naming.
82-82: LGTM! Consistent integration with token context encoding.The addition properly sets the grant type attribute using the factory's
GRANT_TYPEconstant, maintaining consistency with the token context encoding framework across different grant types.services/src/main/java/org/keycloak/protocol/oidc/grants/ClientCredentialsGrantTypeFactory.java (1)
38-41: LGTM! Proper implementation of grant type shortcut.The
getShortcut()method correctly returns "cc" for client credentials grant, following the established pattern across grant type factories. The shortcut is concise and descriptive.services/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantTypeBase.java (2)
45-45: LGTM! Import addition supports the new token context encoding framework.The addition of
Constantsimport is necessary for the newGRANT_TYPEconstant used in the token context encoding enhancement.
109-109: LGTM! Grant type attribute setting enables token context encoding.Setting the grant type attribute in the client session context before token creation is a well-placed foundational change. This ensures that grant type information is consistently available for the new token context encoding framework across all OAuth2 grant type implementations.
services/src/main/java/org/keycloak/protocol/oidc/grants/TokenExchangeGrantTypeFactory.java (1)
40-43: LGTM! Shortcut implementation follows the established pattern.The
getShortcut()method implementation returning "te" for token exchange is concise and follows the naming convention. This integrates well with the token context encoding framework.server-spi-private/src/main/java/org/keycloak/models/Constants.java (1)
206-207: LGTM! Well-documented constant addition for grant type tracking.The
GRANT_TYPEconstant provides a centralized key for storing grant type information in client session contexts. The clear documentation and use ofOAuth2Constants.GRANT_TYPEas the value maintains consistency with the OAuth2 framework.services/src/main/java/org/keycloak/protocol/oidc/grants/PreAuthorizedCodeGrantTypeFactory.java (1)
58-61: LGTM! Consistent shortcut implementation for pre-authorized code grant type.The
getShortcut()method returning "pc" follows the established pattern and provides a logical abbreviation for the pre-authorized code grant type. This completes the integration with the token context encoding framework.services/src/main/resources/META-INF/services/org.keycloak.provider.Spi (1)
28-28: LGTM! Proper SPI registration for token context encoding framework.The addition of
org.keycloak.protocol.oidc.encode.TokenContextEncoderSpiproperly registers the new SPI for service loading. The placement maintains the file's organization and enables the token context encoding functionality.services/src/main/java/org/keycloak/protocol/oidc/grants/PermissionGrantTypeFactory.java (1)
38-41: LGTM! Clean implementation of the grant type shortcut.The implementation correctly provides "pg" as a concise shortcut for the permission grant type (UMA 2.0), which aligns with the token context encoding framework.
services/src/main/java/org/keycloak/protocol/oidc/grants/ResourceOwnerPasswordCredentialsGrantTypeFactory.java (1)
38-41: LGTM! Appropriate shortcut for resource owner password credentials.The "ro" shortcut is a logical abbreviation for the resource owner password credentials grant type and follows the consistent implementation pattern.
services/src/main/java/org/keycloak/protocol/oidc/tokenexchange/StandardTokenExchangeProvider.java (1)
237-237: LGTM! Proper grant type attribute setting for token context encoding.Setting the
Constants.GRANT_TYPEattribute ensures the token exchange grant type is properly recorded in the client session context, enabling the token context encoder to embed the correct grant type shortcut.services/src/main/java/org/keycloak/protocol/oidc/grants/RefreshTokenGrantTypeFactory.java (1)
33-33: Excellent pattern with constant for shortcut value.Using a public static final constant for the grant shortcut is a good practice that improves maintainability. The "rt" shortcut appropriately represents the refresh token grant type.
Also applies to: 40-43
services/src/main/java/org/keycloak/protocol/oidc/grants/device/DeviceGrantTypeFactory.java (1)
37-37: LGTM! Consistent implementation with appropriate shortcut.The constant-based approach for the grant shortcut is well-implemented. The "dg" shortcut is a clear abbreviation for the device grant type and maintains consistency with the framework pattern.
Also applies to: 44-47
services/src/main/resources/META-INF/services/org.keycloak.protocol.oidc.encode.TokenContextEncoderProviderFactory (1)
1-21: LGTM! Standard SPI service provider configuration.The service provider configuration follows Java SPI conventions correctly with the proper fully qualified class name registration.
services/src/main/java/org/keycloak/protocol/oidc/grants/ciba/CibaGrantTypeFactory.java (2)
37-37: LGTM! Appropriate shortcut for CIBA grant type.The shortcut "ci" is concise and meaningful for Client-Initiated Backchannel Authentication.
44-47: LGTM! Proper implementation of the interface contract.The method correctly implements the new
getShortcut()method from theOAuth2GrantTypeFactoryinterface.services/src/main/java/org/keycloak/protocol/oidc/mappers/AbstractOIDCProtocolMapper.java (1)
84-87: LGTM! Appropriate visibility change for reusability.Converting to a public static method is correct since it doesn't use instance state and allows broader usage across the token context encoding framework.
server-spi-private/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantTypeFactory.java (1)
29-33: LGTM! Well-documented interface extension.The new method is properly documented with clear requirements for unique shortcuts and explains the use case for character-limited contexts.
services/src/main/java/org/keycloak/protocol/oidc/grants/AuthorizationCodeGrantTypeFactory.java (2)
32-32: LGTM! Clear shortcut for authorization code grant.The shortcut "ac" is an appropriate and intuitive abbreviation for authorization code grant type.
39-42: LGTM! Consistent implementation pattern.The method implementation follows the established pattern across other grant type factories and correctly implements the interface contract.
services/src/main/java/org/keycloak/protocol/oidc/encode/TokenContextEncoderProvider.java (1)
25-38: Well-designed interface with clear separation of concerns.The interface provides a clean abstraction for token context encoding/decoding operations. The method names are descriptive and the javadoc clearly explains the purpose.
server-spi-private/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantType.java (4)
29-29: Good addition of OAuth2Constants import.Supports the new grant type extraction functionality.
85-85: Appropriate field addition for grant type.The grantType field supports the token context encoding framework by making grant type information readily available.
102-102: Correct initialization of grant type from form parameters.Properly extracts the grant type from OAuth2 form parameters using the standard constant.
173-175: Clean getter implementation.Provides appropriate access to the grant type field.
services/src/main/java/org/keycloak/protocol/oidc/encode/AccessTokenContext.java (3)
36-51: Well-designed SessionType enum with appropriate shortcuts.The enum provides clear session type categories with compact shortcuts suitable for token encoding.
53-67: Well-designed TokenType enum with appropriate shortcuts.The enum provides clear token type categories with compact shortcuts suitable for token encoding.
80-94: Clean getter implementations.All getter methods follow standard patterns and provide appropriate access to the immutable fields.
services/src/test/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProviderTest.java (4)
38-50: Good test setup with proper factory initialization.The setup correctly initializes the provider with grant type mappings that mirror the expected production configuration.
52-74: Comprehensive test coverage for successful token scenarios.Both tests verify complete round-trip encoding/decoding and validate all context fields are preserved correctly.
76-97: Good error handling and edge case coverage.Tests appropriately verify both incorrect grant type handling (with exceptions) and unknown grant type handling (with fallback behavior).
99-113: Excellent legacy token compatibility testing.Verifies that legacy tokens without encoded context are handled gracefully and that encoding unknown contexts is properly prevented.
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AssertEvents.java (2)
33-36: Appropriate imports for grant type factories.Correctly imports the factory classes needed for accessing grant type shortcuts in test assertions.
145-145: Good integration of token context validation into existing tests.The updates to use
isAccessTokenIdwith specific grant shortcuts enhance test precision by validating that token IDs contain the expected grant type information.Also applies to: 173-173, 182-182, 244-244
services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java (3)
67-68: LGTM! Necessary imports for the new token encoding framework.The imports are correctly added to support the new token context encoding functionality.
248-248: LGTM! Properly sets grant type for refresh token flows.Setting the grant type attribute ensures that refresh tokens have the correct context metadata for the new encoding framework.
867-869: Good cleanup! Fixed typo in variable name.The variable name has been corrected from
requestedAucienceClientstorequestedAudienceClients.services/src/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProvider.java (1)
66-98: Well-implemented token parsing with good error handling.The method properly handles:
- Legacy tokens without encoded metadata (backward compatibility)
- Validation of encoded context format and length
- Clear error messages for malformed tokens
services/src/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProviderFactory.java (2)
106-119: LGTM! Good implementation with dynamic grant type discovery.The method properly handles:
- Efficient lookup from cached mappings
- Dynamic discovery of new grant types deployed at runtime
- Thread-safe updates using ConcurrentHashMap
121-137: LGTM! Consistent implementation with proper dynamic discovery.The method maintains consistency with
getShortcutByGrantTypeand properly handles dynamic grant type discovery.
| public AccessTokenContext(SessionType sessionType, TokenType tokenType, String grantType, String rawTokenId) { | ||
| 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"); | ||
| this.sessionType = sessionType; | ||
| this.tokenType = tokenType; | ||
| this.grantType = grantType; | ||
| this.rawTokenId = rawTokenId; | ||
| } |
There was a problem hiding this comment.
Fix duplicate validation in constructor.
Line 73 validates grantType twice instead of validating rawTokenId. This could allow null rawTokenId values to pass through.
public AccessTokenContext(SessionType sessionType, TokenType tokenType, String grantType, String rawTokenId) {
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");
+ Objects.requireNonNull(rawTokenId, "Null rawTokenId not allowed");
this.sessionType = sessionType;
this.tokenType = tokenType;
this.grantType = grantType;
this.rawTokenId = rawTokenId;
}🤖 Prompt for AI Agents
In
services/src/main/java/org/keycloak/protocol/oidc/encode/AccessTokenContext.java
between lines 69 and 78, the constructor duplicates the null check for grantType
and misses checking rawTokenId. Replace the second Objects.requireNonNull call
that checks grantType with a check for rawTokenId to ensure rawTokenId is not
null.
| public AccessTokenContext getTokenContextFromClientSessionContext(ClientSessionContext clientSessionContext, String rawTokenId) { | ||
| AccessTokenContext.SessionType sessionType; | ||
| UserSessionModel userSession = clientSessionContext.getClientSession().getUserSession(); | ||
| if (userSession.getPersistenceState() == UserSessionModel.SessionPersistenceState.TRANSIENT) { | ||
| sessionType = AccessTokenContext.SessionType.TRANSIENT; | ||
| } else { | ||
| sessionType = userSession.isOffline() ? AccessTokenContext.SessionType.OFFLINE : AccessTokenContext.SessionType.ONLINE; | ||
| } | ||
|
|
||
| boolean useLightweightToken = AbstractOIDCProtocolMapper.getShouldUseLightweightToken(session); | ||
| AccessTokenContext.TokenType tokenType = useLightweightToken ? AccessTokenContext.TokenType.LIGHTWEIGHT : AccessTokenContext.TokenType.REGULAR; | ||
|
|
||
| String grantType = clientSessionContext.getAttribute(Constants.GRANT_TYPE, String.class); | ||
| if (grantType == null) { | ||
| grantType = UNKNOWN; | ||
| } | ||
|
|
||
| return new AccessTokenContext(sessionType, tokenType, grantType, rawTokenId); | ||
| } |
There was a problem hiding this comment.
Add null safety check for user session.
The method accesses clientSessionContext.getClientSession().getUserSession() without null checks. Consider adding validation to prevent potential NPE.
@Override
public AccessTokenContext getTokenContextFromClientSessionContext(ClientSessionContext clientSessionContext, String rawTokenId) {
AccessTokenContext.SessionType sessionType;
UserSessionModel userSession = clientSessionContext.getClientSession().getUserSession();
+ if (userSession == null) {
+ throw new IllegalStateException("User session is required for token context encoding");
+ }
if (userSession.getPersistenceState() == UserSessionModel.SessionPersistenceState.TRANSIENT) {
sessionType = AccessTokenContext.SessionType.TRANSIENT;
} else {🤖 Prompt for AI Agents
In
services/src/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProvider.java
around lines 45 to 63, the method accesses userSession from
clientSessionContext.getClientSession().getUserSession() without checking for
null, which can cause a NullPointerException. Add a null check for userSession
after retrieving it, and handle the null case appropriately, such as returning
null or throwing an exception, before proceeding with the rest of the method
logic.
| public String encodeTokenId(AccessTokenContext tokenContext) { | ||
| if (tokenContext.getSessionType() == AccessTokenContext.SessionType.UNKNOWN) { | ||
| throw new IllegalStateException("Cannot encode token with unknown sessionType"); | ||
| } | ||
| if (tokenContext.getTokenType() == AccessTokenContext.TokenType.UNKNOWN) { | ||
| throw new IllegalStateException("Cannot encode token with unknown tokenType"); | ||
| } | ||
|
|
||
| String grantShort = factory.getShortcutByGrantType(tokenContext.getGrantType()); | ||
| if (grantShort == null) { | ||
| throw new IllegalStateException("Cannot encode token with unknown grantType: " + tokenContext.getGrantType()); | ||
| } | ||
|
|
||
| return tokenContext.getSessionType().getShortcut() + | ||
| tokenContext.getTokenType().getShortcut() + | ||
| grantShort + | ||
| ':' + tokenContext.getRawTokenId(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null check for tokenContext parameter.
The method should validate the input parameter to prevent NPE.
@Override
public String encodeTokenId(AccessTokenContext tokenContext) {
+ if (tokenContext == null) {
+ throw new IllegalArgumentException("Token context cannot be null");
+ }
if (tokenContext.getSessionType() == AccessTokenContext.SessionType.UNKNOWN) {
throw new IllegalStateException("Cannot encode token with unknown sessionType");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public String encodeTokenId(AccessTokenContext tokenContext) { | |
| if (tokenContext.getSessionType() == AccessTokenContext.SessionType.UNKNOWN) { | |
| throw new IllegalStateException("Cannot encode token with unknown sessionType"); | |
| } | |
| if (tokenContext.getTokenType() == AccessTokenContext.TokenType.UNKNOWN) { | |
| throw new IllegalStateException("Cannot encode token with unknown tokenType"); | |
| } | |
| String grantShort = factory.getShortcutByGrantType(tokenContext.getGrantType()); | |
| if (grantShort == null) { | |
| throw new IllegalStateException("Cannot encode token with unknown grantType: " + tokenContext.getGrantType()); | |
| } | |
| return tokenContext.getSessionType().getShortcut() + | |
| tokenContext.getTokenType().getShortcut() + | |
| grantShort + | |
| ':' + tokenContext.getRawTokenId(); | |
| } | |
| @Override | |
| public String encodeTokenId(AccessTokenContext tokenContext) { | |
| if (tokenContext == null) { | |
| throw new IllegalArgumentException("Token context cannot be null"); | |
| } | |
| if (tokenContext.getSessionType() == AccessTokenContext.SessionType.UNKNOWN) { | |
| throw new IllegalStateException("Cannot encode token with unknown sessionType"); | |
| } | |
| if (tokenContext.getTokenType() == AccessTokenContext.TokenType.UNKNOWN) { | |
| throw new IllegalStateException("Cannot encode token with unknown tokenType"); | |
| } | |
| String grantShort = factory.getShortcutByGrantType(tokenContext.getGrantType()); | |
| if (grantShort == null) { | |
| throw new IllegalStateException("Cannot encode token with unknown grantType: " + tokenContext.getGrantType()); | |
| } | |
| return tokenContext.getSessionType().getShortcut() + | |
| tokenContext.getTokenType().getShortcut() + | |
| grantShort + | |
| ':' + tokenContext.getRawTokenId(); | |
| } |
🤖 Prompt for AI Agents
In
services/src/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProvider.java
around lines 101 to 118, the encodeTokenId method lacks a null check for the
tokenContext parameter, which can cause a NullPointerException. Add a null check
at the start of the method to throw an IllegalArgumentException or similar if
tokenContext is null, ensuring the method fails fast and clearly when given
invalid input.
| Map<String, String> grantsByShortcuts; | ||
| Map<String, String> grantsToShortcuts; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making grant mapping fields private.
The fields grantsByShortcuts and grantsToShortcuts have package-private visibility but should be private to ensure proper encapsulation.
- Map<String, String> grantsByShortcuts;
- Map<String, String> grantsToShortcuts;
+ private Map<String, String> grantsByShortcuts;
+ private Map<String, String> grantsToShortcuts;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Map<String, String> grantsByShortcuts; | |
| Map<String, String> grantsToShortcuts; | |
| private Map<String, String> grantsByShortcuts; | |
| private Map<String, String> grantsToShortcuts; |
🤖 Prompt for AI Agents
In
services/src/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProviderFactory.java
at lines 41 to 42, the fields grantsByShortcuts and grantsToShortcuts are
declared with package-private visibility. Change their access modifiers to
private to enforce encapsulation and restrict direct access from outside the
class.
|
|
||
| TokenContextEncoderProvider encoder = session.getProvider(TokenContextEncoderProvider.class); | ||
| AccessTokenContext tokenCtx = encoder.getTokenContextFromClientSessionContext(clientSessionCtx, KeycloakModelUtils.generateId()); | ||
| token.id(encoder.encodeTokenId(tokenCtx)); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify error handling for the token context encoder provider.
The code now depends on TokenContextEncoderProvider to generate token IDs. Consider handling the case where the provider might not be available or throws an exception during encoding.
🏁 Script executed:
#!/bin/bash
# Check if TokenContextEncoderProvider is always guaranteed to be available
ast-grep --pattern 'session.getProvider(TokenContextEncoderProvider.class)'
# Check if there are any null checks or error handling for the provider
rg -A 5 'getProvider\(TokenContextEncoderProvider\.class\)'Length of output: 1174
Ensure TokenContextEncoderProvider is always available or handled
The call to session.getProvider(TokenContextEncoderProvider.class) can return null if the provider isn’t registered, leading to a potential NPE when you invoke encoder.getTokenContextFromClientSessionContext(...) or encoder.encodeTokenId(...). Please add explicit error handling or make the provider requirement fail-fast:
• File: services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java
Lines 1051–1054
TokenContextEncoderProvider encoder = session.getProvider(TokenContextEncoderProvider.class);
if (encoder == null) {
throw new IllegalStateException("TokenContextEncoderProvider not available – ensure it’s registered in the session");
}
AccessTokenContext tokenCtx = encoder.getTokenContextFromClientSessionContext(clientSessionCtx, KeycloakModelUtils.generateId());
token.id(encoder.encodeTokenId(tokenCtx));Recommendations:
- Use
session.getProviderOrThrow(TokenContextEncoderProvider.class)if available. - Or add a null-check and throw a clear exception early.
🤖 Prompt for AI Agents
In services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java around
lines 1051 to 1054, the call to
session.getProvider(TokenContextEncoderProvider.class) may return null causing a
NullPointerException later. Fix this by either using
session.getProviderOrThrow(TokenContextEncoderProvider.class) if available, or
add a null check after getting the provider and throw an IllegalStateException
with a clear message if it is null, ensuring fail-fast behavior and preventing
NPEs.
| public static Matcher<String> isAccessTokenId(String expectedGrantShortcut) { | ||
| return new TypeSafeMatcher<String>() { | ||
| @Override | ||
| protected boolean matchesSafely(String item) { | ||
| String[] items = item.split(":"); | ||
| 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]); | ||
| } | ||
|
|
||
| @Override | ||
| public void describeTo(Description description) { | ||
| description.appendText("Not a Token ID with expected grant: " + expectedGrantShortcut); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
Fix logic error in token ID matcher.
The matcher incorrectly returns false when grant shortcuts match. Line 483 should return true when shortcuts match, not false.
public static Matcher<String> isAccessTokenId(String expectedGrantShortcut) {
return new TypeSafeMatcher<String>() {
@Override
protected boolean matchesSafely(String item) {
String[] items = item.split(":");
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;
+ if (!items[0].substring(3, 5).equals(expectedGrantShortcut)) return false;
return isUUID().matches(items[1]);
}
@Override
public void describeTo(Description description) {
description.appendText("Not a Token ID with expected grant: " + expectedGrantShortcut);
}
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static Matcher<String> isAccessTokenId(String expectedGrantShortcut) { | |
| return new TypeSafeMatcher<String>() { | |
| @Override | |
| protected boolean matchesSafely(String item) { | |
| String[] items = item.split(":"); | |
| 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]); | |
| } | |
| @Override | |
| public void describeTo(Description description) { | |
| description.appendText("Not a Token ID with expected grant: " + expectedGrantShortcut); | |
| } | |
| }; | |
| } | |
| public static Matcher<String> isAccessTokenId(String expectedGrantShortcut) { | |
| return new TypeSafeMatcher<String>() { | |
| @Override | |
| protected boolean matchesSafely(String item) { | |
| String[] items = item.split(":"); | |
| 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]); | |
| } | |
| @Override | |
| public void describeTo(Description description) { | |
| description.appendText("Not a Token ID with expected grant: " + expectedGrantShortcut); | |
| } | |
| }; | |
| } |
🤖 Prompt for AI Agents
In
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AssertEvents.java
between lines 476 and 492, the matchesSafely method incorrectly returns false
when the grant shortcut matches the expectedGrantShortcut. Change the condition
on line 483 so that it returns true when the grant shortcuts match, fixing the
logic to correctly identify valid token IDs.
2 participants
Test 8
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Tests