Skip to content

Implement access token context encoding framework - #3

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

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

Conversation

@akshayutture-augment

@akshayutture-augment akshayutture-augment commented Nov 17, 2025

Copy link
Copy Markdown

No description provided.

closes #37118

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

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Comment augment review to trigger a new review at any time.

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
Copy Markdown

Choose a reason for hiding this comment

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

In the AccessTokenContext constructor, the null checks validate grantType twice and never validate rawTokenId, which defeats the intended validation and could hide null rawTokenId inputs.

🤖 Was this useful? React with 👍 or 👎

return new TypeSafeMatcher<String>() {
@Override
protected boolean matchesSafely(String item) {
String[] items = item.split(":");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The isAccessTokenId matcher appears to invert the equality check and uses an off-by-one substring for the grant shortcut, so it will fail when the grant actually matches; consider checking the correct indices and returning true when the shortcut matches.

🤖 Was this useful? React with 👍 or 👎

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.

2 participants