Skip to content

<fix>[identity]: ValidateSession clear SSOToken #4159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
zstack-robot-2 wants to merge 1 commit into from
+10 −0
Closed

<fix>[identity]: ValidateSession clear SSOToken #4159

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions identity/src/main/java/org/zstack/identity/Session.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ public class Session implements Component {
private DatabaseFacade dbf;
@Autowired
private EventFacade evtf;
@Autowired
private PluginRegistry pluginRgty;

private static List<LogoutExtensionPoint> logoutExtensionPoints;

private Future<Void> expiredSessionCollector;
private static Interner<String> sessionLock = Interners.newWeakInterner();
Expand Down Expand Up @@ -221,6 +225,11 @@ protected ErrorCode scripts() {
}

logout(s.getUuid());

for (LogoutExtensionPoint ext : logoutExtensionPoints) {
ext.beforeLogout(s);
}
Comment on lines 227 to +231
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 1, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 查找 LogoutExtensionPoint 接口定义及方法签名
fd -e java LogoutExtensionPoint --exec cat {}
# 查找其它实现/调用点以核对 before/after 约定
rg -nP --type=java '\bbeforeLogout\s*\(' -C2

Repository: MatheMatrix/zstack

Length of output: 1799

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) 取 Session.java 相关片段：logoutExtensionPoints 声明、赋值/start、以及调用点/被调用的上下文（checkSessionExpired/logout）
sed -n '200,320p' identity/src/main/java/org/zstack/identity/Session.java | nl -ba | sed -n '1,140p'
echo "-----"
rg -n "logoutExtensionPoints|checkSessionExpired|start\\(|logout\\(" identity/src/main/java/org/zstack/identity/Session.java
echo "-----"

# 2) 取 AccountManagerImpl.java 中 beforeLogout 的上下文：它是否在真正注销/删除之前调用
sed -n '430,540p' identity/src/main/java/org/zstack/identity/AccountManagerImpl.java | nl -ba | sed -n '1,140p'
echo "-----"
rg -n "beforeLogout\\(session\\)|LogoutExtensionPoint|logout|deleteSession|sessionLogout" identity/src/main/java/org/zstack/identity/AccountManagerImpl.java

Repository: MatheMatrix/zstack

Length of output: 105

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Session.java (logoutExtensionPoints lifecycle + call order) ==="
rg -n "logoutExtensionPoints|checkSessionExpired\\(|start\\(|logout\\(s\\.getUuid\\(\\)\\)|logout\\(" identity/src/main/java/org/zstack/identity/Session.java
echo "----- context around declaration"
sed -n '1,120p' identity/src/main/java/org/zstack/identity/Session.java | cat -n | sed -n '1,120p'
echo "----- context around start()/initialization of logoutExtensionPoints"
sed -n '240,320p' identity/src/main/java/org/zstack/identity/Session.java | cat -n
echo "----- context around the beforeLogout call site"
sed -n '200,260p' identity/src/main/java/org/zstack/identity/Session.java | cat -n

echo "=== AccountManagerImpl.java (beforeLogout semantics & order) ==="
rg -n "interface LogoutExtensionPoint|beforeLogout\\(session\\)|deleteSession\\(|logout\\(|sessionLogout\\(" identity/src/main/java/org/zstack/identity/AccountManagerImpl.java
echo "----- context around beforeLogout loop"
sed -n '440,520p' identity/src/main/java/org/zstack/identity/AccountManagerImpl.java | cat -n

Repository: MatheMatrix/zstack

Length of output: 16874

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Session.logout(String) implementation ==="
sed -n '140,220p' identity/src/main/java/org/zstack/identity/Session.java | cat -n

echo "=== AccountManagerImpl.logOutSession(...) implementation ==="
rg -n "void logOutSession\\(|logOutSession\\(" identity/src/main/java/org/zstack/identity/AccountManagerImpl.java
# show around the call site and definition
sed -n '1120,1180p' identity/src/main/java/org/zstack/identity/AccountManagerImpl.java | cat -n
# if there is another overload further down, also show later chunk around "logOutSession"
rg -n "logOutSession\\(" identity/src/main/java/org/zstack/identity/AccountManagerImpl.java | head

Repository: MatheMatrix/zstack

Length of output: 6641

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== 找出 checkSessionExpired / errorOnTimeout 的调用点 ==="
rg -n "Session\\.checkSessionExpired\\(|Session\\.errorOnTimeout\\(" identity/src/main/java | head -n 200

echo "----- 全局找 LogoutExtensionPoint.beforeLogout 调用顺序是否一致 -----"
rg -n "beforeLogout\\(.*\\)" identity/src/main/java | head -n 200

Repository: MatheMatrix/zstack

Length of output: 817

修正 LogoutExtensionPoint 的调用时序并避免 logoutExtensionPoints 初始化竞态

  • Session 的过期清理分支里先执行 Session.logout(s.getUuid())（会从缓存与 DB 中删除会话），随后才遍历 logoutExtensionPoints 调用 ext.beforeLogout(s)；与 AccountManagerImpl 的 APILogOutMsg 流程（先 beforeLogout，再 logOutSession -> Session.logout）不一致，beforeLogout 语义与实际顺序相反。
  • logoutExtensionPoints 为 static，start() 中在 startCleanUpStaleSessionTask() 之后才赋值，若 periodic task 在赋值前触发，可能出现 null 导致 NPE。
🔧 建议调整调用顺序 
-                    logout(s.getUuid());
-
-                    for (LogoutExtensionPoint ext : logoutExtensionPoints) {
-                        ext.beforeLogout(s);
-                    }
+                    for (LogoutExtensionPoint ext : logoutExtensionPoints) {
+                        ext.beforeLogout(s);
+                    }
+
+                    logout(s.getUuid());
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@identity/src/main/java/org/zstack/identity/Session.java` around lines 227 -
231, 在 Session 的过期清理分支中，将对 LogoutExtensionPoint 的调用顺序调整为先遍历并调用
ext.beforeLogout(s) 再调用 Session.logout(s.getUuid())，以与 AccountManagerImpl 的
APILogOutMsg 流程保持一致并恢复语义；同时修复 logoutExtensionPoints 可能为 null 的竞态：在 start() 中于
startCleanUpStaleSessionTask() 之前完成 logoutExtensionPoints
的初始化（或将其在声明时初始化），或在清理任务里对 logoutExtensionPoints 做空值检查并安全跳过，以避免 NPE。


return err(IdentityErrors.INVALID_SESSION, "Session expired");
}

Expand Down Expand Up @@ -266,6 +275,7 @@ public boolean start() {
setupGlobalConfig();
startCleanUpStaleSessionTask();
setupCanonicalEvents();
logoutExtensionPoints = pluginRgty.getExtensionList(LogoutExtensionPoint.class);
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 1, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

静态 logoutExtensionPoints 仅在 start() 中赋值，存在 NPE 与模式不一致风险

logoutExtensionPoints（第 46 行）为静态字段且无初始值，只在实例方法 start() 中赋值；而静态方法 checkSessionExpired（第 229 行）会遍历它。若 checkSessionExpired 在 Session 组件 start() 完成之前被调用（如启动早期或单元测试场景），将抛出 NullPointerException，进而中断会话校验。

此外，本文件中 login()（第 63 行）与 deleteSession()（第 180 行）均采用按需获取扩展的写法 getComponentLoader().getComponent(PluginRegistry.class).getExtensionList(...)，可天然规避初始化时序问题。建议在 checkSessionExpired 中沿用同一模式，移除静态缓存字段与此处赋值。

♻️ 方案一（推荐）：按需获取，移除静态缓存 
-    private static List<LogoutExtensionPoint> logoutExtensionPoints;
-                    for (LogoutExtensionPoint ext : logoutExtensionPoints) {
-                        ext.beforeLogout(s);
-                    }
+                    getComponentLoader().getComponent(PluginRegistry.class)
+                            .getExtensionList(LogoutExtensionPoint.class)
+                            .forEach(ext -> ext.beforeLogout(s));
-        logoutExtensionPoints = pluginRgty.getExtensionList(LogoutExtensionPoint.class);
         return true;

方案二（保留缓存）：在遍历前对 logoutExtensionPoints 做空值保护。

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@identity/src/main/java/org/zstack/identity/Session.java` at line 278, 静态字段
logoutExtensionPoints 仅在实例方法 start() 中赋值但被静态方法 checkSessionExpired
遍历，存在在组件未启动前调用导致的 NPE 风险；请移除该静态缓存字段并在 checkSessionExpired 中按需获取扩展点（使用
getComponentLoader().getComponent(PluginRegistry.class).getExtensionList(LogoutExtensionPoint.class)），与现有
login() 和 deleteSession() 的模式保持一致，或者若保留缓存则至少在 checkSessionExpired 遍历前做空值保护（null
检查）以避免 NPE；定位符：logoutExtensionPoints、start(), checkSessionExpired(), login(),
deleteSession() 及
getComponentLoader().getComponent(PluginRegistry.class).getExtensionList(...)。

return true;
}

Expand Down