Implement access token context encoding framework #8

wangyue6761 · 2026-01-06T15:28:07Z

Recreated from upstream PR ai-code-review-evaluation#8

Original title: Implement access token context encoding framework

Test 8

closes #37118 Signed-off-by: mposolda <[email protected]>

wangyue6761 · 2026-01-07T11:12:57Z

@cpgbot review

wangyue6761

Triggered by @cpgbot review. Total issues: 10.

wangyue6761 · 2026-01-07T11:19:29Z

...ite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AssertEvents.java

+                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;


ERROR RiskType.SYNTAX_STATIC_ERRORS (confidence 0.90) line (483, 483)

逻辑错误：isAccessTokenId方法中的子字符串比较条件反转。第483行代码在items[0].substring(3, 5).equals(expectedGrantShortcut)为true时返回false，这违背了方法的预期行为。根据方法名和描述，该方法应验证token ID是否包含预期的grant shortcut，相等时应返回true（或至少不返回false）。当前逻辑会导致包含正确grant shortcut的token ID被错误地拒绝。

Suggestion: 将第483行代码从 'if (items[0].substring(3, 5).equals(expectedGrantShortcut)) return false;' 修改为 'if (!items[0].substring(3, 5).equals(expectedGrantShortcut)) return false;' 或根据整体逻辑调整。

wangyue6761 · 2026-01-07T11:19:29Z

services/src/main/java/org/keycloak/protocol/oidc/encode/AccessTokenContext.java

+        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");


WARNING RiskType.SYNTAX_STATIC_ERRORS (confidence 0.90) line (73, 73)

构造函数第73行重复检查grantType参数而非rawTokenId参数。代码为Objects.requireNonNull(grantType, "Null rawTokenId not allowed");，错误信息字符串是'Null rawTokenId not allowed'但实际检查的是grantType变量。这导致rawTokenId参数未进行null检查，违反构造函数设计意图，可能允许null值传入而不会抛出预期的NullPointerException。

Suggestion: 将第73行改为 Objects.requireNonNull(rawTokenId, "Null rawTokenId not allowed"); 以正确检查rawTokenId参数是否为null。

wangyue6761 · 2026-01-07T11:19:29Z

...c/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProviderFactory.java

+        return tokenTypesByShortcut.get(tokenTypeShortcut);
+    }
+
+    protected String getShortcutByGrantType(String grantType) {


WARNING RiskType.CONCURRENCY_TIMING_CORRECTNESS (confidence 0.85) line (106, 119)

getShortcutByGrantType方法存在check-then-act竞态条件。多个线程可能同时检查到grantsToShortcuts映射中不存在某个grantType（第107-108行），然后都进入第110-115行的更新逻辑。虽然使用了ConcurrentHashMap，但check-then-act模式缺乏原子性保护。具体问题：1) 多个线程可能重复执行相同的工厂查找操作；2) 两个映射的更新（第113-114行）不是原子操作，可能导致映射不一致；3) 在动态部署新grant类型时，竞态窗口可能导致重复插入或映射状态不一致。

Suggestion: 使用ConcurrentHashMap的computeIfAbsent方法替换check-then-act模式，确保查找和更新的原子性。例如：return grantsToShortcuts.computeIfAbsent(grantType, k -> { OAuth2GrantTypeFactory factory = (OAuth2GrantTypeFactory) sessionFactory.getProviderFactory(OAuth2GrantType.class, k); return factory != null ? factory.getShortcut() : null; }); 同样，getGrantTypeByShortcut方法也存在类似问题，需要同步修复。

wangyue6761 · 2026-01-07T11:19:29Z

...ices/src/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProvider.java

+            String encodedContext = encodedTokenId.substring(0, indexOf);
+            String rawId = encodedTokenId.substring(indexOf + 1);
+
+            if (encodedContext.length() != 6) {


WARNING RiskType.ROBUSTNESS_BOUNDARY_CONDITIONS (confidence 0.85) line (74, 76)

方法getTokenContextFromTokenId在第74-76行硬编码了长度检查encodedContext.length() != 6。这个值6基于当前SessionType、TokenType和GrantType的shortcut长度（每个2字符）。证据：1) AccessTokenContext.SessionType枚举的shortcut都是2字符（"on", "of", "tr", "un"）；2) AccessTokenContext.TokenType枚举的shortcut都是2字符（"rt", "lt", "un"）；3) OAuth2GrantTypeFactory实现如AuthorizationCodeGrantTypeFactory返回"ac"（2字符），ClientCredentialsGrantTypeFactory返回"cc"（2字符）。虽然当前所有shortcut都是2字符，但硬编码长度6使得代码对未来的扩展不友好，如果未来需要支持更长的shortcut，需要修改这个硬编码值。

Suggestion: 建议将硬编码值6提取为常量，例如private static final int ENCODED_CONTEXT_LENGTH = 6;，并在注释中说明这个值是基于SessionType、TokenType和GrantType的shortcut长度之和（每个2字符）。或者，更好的方法是计算预期的长度：int expectedLength = AccessTokenContext.SessionType.ONLINE.getShortcut().length() + AccessTokenContext.TokenType.REGULAR.getShortcut().length() + 2;（其中2是grant type shortcut的典型长度）。

wangyue6761 · 2026-01-07T11:19:29Z

...ices/src/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProvider.java

+    @Override
+    public AccessTokenContext getTokenContextFromTokenId(String encodedTokenId) {
+        int indexOf = encodedTokenId.indexOf(':');
+        if (indexOf == -1) {


WARNING RiskType.ROBUSTNESS_BOUNDARY_CONDITIONS (confidence 0.85) line (68, 70)

方法 getTokenContextFromTokenId(String encodedTokenId) 在第67行直接调用 encodedTokenId.indexOf(':') 而没有检查 encodedTokenId 是否为 null。如果传入 null 参数，会抛出 NullPointerException。此外，空字符串作为输入可能导致意外的行为（如空字符串作为 rawTokenId 传递）。接口契约没有指定 @Nullable/@NotNull 注解，调用者可能传递 null 值。

Suggestion: 在方法开始处添加 null 检查：if (encodedTokenId == null) { throw new IllegalArgumentException("encodedTokenId cannot be null"); }。同时考虑处理空字符串的情况，根据业务逻辑决定是抛出异常还是返回特定的 AccessTokenContext。

wangyue6761 · 2026-01-07T11:19:29Z

server-spi-private/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantType.java

-            this.event = context.event;
-            this.cors = context.cors;
-            this.tokenManager = context.tokenManager;
+            this.grantType = formParams.getFirst(OAuth2Constants.GRANT_TYPE);


WARNING RiskType.ROBUSTNESS_BOUNDARY_CONDITIONS (confidence 0.80) line (102, 102)

grantType字段从formParams.getFirst(OAuth2Constants.GRANT_TYPE)初始化，没有空值检查。当formParams中缺少GRANT_TYPE参数时，grantType将为null。后续在OAuth2GrantTypeBase.java:109和DefaultTokenContextEncoderProvider.java:109等位置直接使用getGrantType()返回值，可能导致NullPointerException或异常消息中的null值。Jakarta EE的MultivaluedMap.getFirst()在键不存在时返回null是标准行为。

Suggestion: 在初始化grantType时添加空值检查，或者在使用getGrantType()的地方添加防御性检查。例如：1) 在构造函数中添加验证：if (grantType == null) throw new IllegalArgumentException("grant_type parameter is required"); 2) 或者将grantType字段标记为@nullable并在文档中说明。

wangyue6761 · 2026-01-07T11:19:29Z

...c/main/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProviderFactory.java

+        return grantShortcut;
+    }
+
+    protected String getGrantTypeByShortcut(String shortcut) {


WARNING RiskType.CONCURRENCY_TIMING_CORRECTNESS (confidence 0.75) line (121, 137)

getGrantTypeByShortcut方法存在check-then-act竞态条件。多个并发线程可能同时发现映射中缺少某个shortcut（第122行检查），然后都执行第125-133行的查询和更新操作。虽然使用ConcurrentHashMap确保单个put操作原子性，但整个'检查-查询-更新'序列缺乏同步保护。这可能导致：1) 性能浪费（重复执行相同的查询）；2) 潜在的getProviderFactoriesStream调用开销；3) 理论上可能干扰第82-85行的映射大小验证逻辑。共享状态为grantsByShortcuts和grantsToShortcuts（ConcurrentHashMap实例），在Keycloak多线程服务器环境中被并发访问。

Suggestion: 建议使用ConcurrentHashMap的computeIfAbsent方法原子化整个'检查-计算-更新'操作，或者使用双重检查锁定模式。例如：return grantsByShortcuts.computeIfAbsent(shortcut, k -> sessionFactory.getProviderFactoriesStream(OAuth2GrantType.class).map(fct -> (OAuth2GrantTypeFactory) fct).filter(fct -> shortcut.equals(fct.getShortcut())).map(OAuth2GrantTypeFactory::getId).findFirst().orElse(null));

wangyue6761 · 2026-01-07T11:19:29Z

.../src/test/java/org/keycloak/protocol/oidc/encode/DefaultTokenContextEncoderProviderTest.java

+    public void before() {
+        DefaultTokenContextEncoderProviderFactory factory = new DefaultTokenContextEncoderProviderFactory();
+        factory.init(null);
+        factory.grantsByShortcuts = new HashMap<>();


WARNING RiskType.INTENT_SEMANTIC_CONSISTENCY (confidence 0.70) line (42, 48)

测试中硬编码了grant shortcut映射('ro'->PASSWORD, 'cc'->CLIENT_CREDENTIALS)，虽然这两个映射与对应的ResourceOwnerPasswordCredentialsGrantTypeFactory和ClientCredentialsGrantTypeFactory实现一致，但测试没有覆盖其他grant类型工厂（如AuthorizationCodeGrantTypeFactory返回'ac', PermissionGrantTypeFactory返回'pg', PreAuthorizedCodeGrantTypeFactory返回'pc'等）。生产代码DefaultTokenContextEncoderProviderFactory.postInit()会动态加载所有OAuth2GrantTypeFactory实现来构建完整映射，测试的硬编码方式可能导致映射不完整，无法验证生产代码的动态加载逻辑和所有grant类型的正确映射。

Suggestion: 建议修改测试，使其更贴近生产代码的逻辑：1) 要么模拟生产代码的动态加载过程，验证所有grant类型工厂的映射；2) 要么在测试注释中明确说明只测试特定grant类型的映射，并添加TODO注释说明其他grant类型的测试覆盖；3) 或者重构测试以使用真实的工厂实例而不是硬编码映射。

wangyue6761 · 2026-01-07T11:19:29Z

services/src/main/java/org/keycloak/protocol/oidc/mappers/AbstractOIDCProtocolMapper.java


-    boolean getShouldUseLightweightToken(KeycloakSession session) {
+    public static boolean getShouldUseLightweightToken(KeycloakSession session) {
        Object attributeValue = session.getAttribute(Constants.USE_LIGHTWEIGHT_ACCESS_TOKEN_ENABLED);


WARNING RiskType.ROBUSTNESS_BOUNDARY_CONDITIONS (confidence 0.55) line (85, 86)

session.getContext().getClient()可能返回null，导致NullPointerException。在getShouldUseLightweightToken方法中，直接调用session.getContext().getClient().getAttribute()而没有空值检查。

Suggestion: 在调用getClient().getAttribute()之前添加空值检查，例如：ClientModel client = session.getContext().getClient(); if (client != null) { ... }

wangyue6761 · 2026-01-07T11:19:29Z

server-spi-private/src/main/java/org/keycloak/protocol/oidc/grants/OAuth2GrantType.java

        protected EventBuilder event;
        protected Cors cors;
        protected Object tokenManager;
+        protected String grantType;


WARNING RiskType.LIFECYCLE_STATE_CONSISTENCY (confidence 0.45) line (85, 85)

The grantType field is added to the Context class; need to verify if all existing grant type implementations handle this new field correctly.

Suggestion: 检查所有继承OAuth2GrantTypeBase的授权类型实现，确保它们都正确调用了createTokenResponse方法或手动设置了Constants.GRANT_TYPE属性。特别关注ResourceOwnerPasswordCredentialsGrantType等可能直接构建令牌响应的实现。

Encoding context to access token IDs (#37634)

c873872

closes #37118 Signed-off-by: mposolda <[email protected]>

wangyue6761 commented Jan 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement access token context encoding framework #8

Implement access token context encoding framework #8

Uh oh!

wangyue6761 commented Jan 6, 2026

Uh oh!

wangyue6761 commented Jan 7, 2026

Uh oh!

wangyue6761 left a comment

Uh oh!

wangyue6761 Jan 7, 2026

Uh oh!

wangyue6761 Jan 7, 2026

Uh oh!

wangyue6761 Jan 7, 2026

Uh oh!

wangyue6761 Jan 7, 2026

Uh oh!

wangyue6761 Jan 7, 2026

Uh oh!

wangyue6761 Jan 7, 2026

Uh oh!

wangyue6761 Jan 7, 2026

Uh oh!

wangyue6761 Jan 7, 2026

Uh oh!

wangyue6761 Jan 7, 2026

Uh oh!

wangyue6761 Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Implement access token context encoding framework #8

Are you sure you want to change the base?

Implement access token context encoding framework #8

Uh oh!

Conversation

wangyue6761 commented Jan 6, 2026

Uh oh!

wangyue6761 commented Jan 7, 2026

Uh oh!

wangyue6761 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants