Skip to content

Conversation

@akshayutture-augment
Copy link

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@akshayutture-augment
Copy link
Author

@greptileai

@greptile-apps
Copy link

greptile-apps bot commented Nov 14, 2025

Greptile Overview

Confidence Score: 3/5

  • This PR has a critical null check bug that must be fixed before merging
  • The implementation is architecturally sound with proper SPI design, comprehensive grant type coverage, and backward compatibility. However, the copy-paste error in AccessTokenContext.java:73 that validates grantType twice instead of rawTokenId is a critical bug that could allow null token IDs to pass validation, potentially causing downstream NPEs or security issues.
  • services/src/main/java/org/keycloak/protocol/oidc/encode/AccessTokenContext.java requires immediate fix for the null validation bug on line 73

Important Files Changed

File Analysis

Filename Score Overview
services/src/main/java/org/keycloak/protocol/oidc/encode/AccessTokenContext.java 3/5 New data class with critical null check bug on line 73 (validates grantType twice)
services/src/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProvider.java 5/5 Implements token context encoding/decoding logic with proper error handling
services/src/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProviderFactory.java 5/5 Factory with dynamic grant type mapping and duplicate shortcut validation
services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java 5/5 Integrates encoder in initToken method and adds refresh token grant type tracking
services/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantTypeBase.java 5/5 Sets grant type attribute in createTokenResponse for encoder context
services/src/test/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProviderTest.java 5/5 Unit tests cover encoding/decoding for various token types and grant types

Sequence Diagram

sequenceDiagram
    participant Client
    participant TokenEndpoint
    participant OAuth2GrantType
    participant OAuth2GrantTypeBase
    participant TokenManager
    participant TokenContextEncoder
    participant AccessTokenContext
    
    Client->>TokenEndpoint: Token Request (grant_type=...)
    TokenEndpoint->>OAuth2GrantType: process(Context)
    Note over OAuth2GrantType: Context extracts grant_type<br/>from form params
    OAuth2GrantType->>OAuth2GrantTypeBase: createTokenResponse()
    OAuth2GrantTypeBase->>OAuth2GrantTypeBase: Set grant_type in<br/>ClientSessionContext
    OAuth2GrantTypeBase->>TokenManager: createClientAccessToken()
    TokenManager->>TokenManager: initToken()
    TokenManager->>TokenContextEncoder: getTokenContextFromClientSessionContext()
    TokenContextEncoder->>TokenContextEncoder: Extract session type<br/>(online/offline/transient)
    TokenContextEncoder->>TokenContextEncoder: Extract token type<br/>(regular/lightweight)
    TokenContextEncoder->>TokenContextEncoder: Extract grant type<br/>from ClientSessionContext
    TokenContextEncoder->>AccessTokenContext: new AccessTokenContext(...)
    AccessTokenContext-->>TokenContextEncoder: context
    TokenContextEncoder-->>TokenManager: AccessTokenContext
    TokenManager->>TokenContextEncoder: encodeTokenId(context)
    TokenContextEncoder->>TokenContextEncoder: Format: {sessionType}{tokenType}{grantShortcut}:{rawId}<br/>Example: onrtac:1234
    TokenContextEncoder-->>TokenManager: encodedTokenId
    TokenManager->>TokenManager: token.id(encodedTokenId)
    TokenManager-->>OAuth2GrantTypeBase: AccessToken
    OAuth2GrantTypeBase-->>Client: Token Response with encoded ID
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

28 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

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");
Copy link

Choose a reason for hiding this comment

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

logic: copy-paste error - validates grantType twice instead of validating rawTokenId

Suggested change
Objects.requireNonNull(grantType, "Null rawTokenId not allowed");
Objects.requireNonNull(rawTokenId, "Null rawTokenId not allowed");
Prompt To Fix With AI
This is a comment left during a code review.
Path: services/src/main/java/org/keycloak/protocol/oidc/encode/AccessTokenContext.java
Line: 73:73

Comment:
**logic:** copy-paste error - validates `grantType` twice instead of validating `rawTokenId`

```suggestion
        Objects.requireNonNull(rawTokenId, "Null rawTokenId not allowed");
```

How can I resolve this? If you propose a fix, please make it concise.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants