Skip to content

<fix>[identity]: validate session logout token #2650

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

Open
ZStack-Robot wants to merge 1 commit into
base: 5.4.0
Choose a base branch
from
Open

<fix>[identity]: validate session logout token #2650

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
6 changes: 6 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 @@ -220,6 +220,12 @@ protected ErrorCode scripts() {
return null;
}

PluginRegistry pluginRgty = getComponentLoader().getComponent(PluginRegistry.class);

for (LogoutExtensionPoint ext : pluginRgty.getExtensionList(LogoutExtensionPoint.class)) {
ext.beforeLogout(s);
}
Comment on lines +223 to +227
Copy link

@coderabbitai coderabbitai bot Sep 18, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

在过期注销前缺少故障隔离：单个扩展抛异常会阻断注销与错误返回

beforeLogout() 位于请求链路的过期校验路径上，一旦任一扩展抛出异常，将导致后续 logout() 未执行、请求拿不到期望的 INVALID_SESSION。建议捕获并记录，继续执行注销，避免扩展影响核心行为。

可按以下方式加护栏（保持最小改动）：

-PluginRegistry pluginRgty = getComponentLoader().getComponent(PluginRegistry.class);
-for (LogoutExtensionPoint ext : pluginRgty.getExtensionList(LogoutExtensionPoint.class)) {
-    ext.beforeLogout(s);
-}
+PluginRegistry pluginRgty = getComponentLoader().getComponent(PluginRegistry.class);
+for (LogoutExtensionPoint ext : pluginRgty.getExtensionList(LogoutExtensionPoint.class)) {
+    try {
+        ext.beforeLogout(s);
+    } catch (Throwable t) {
+        logger.warn(String.format(
+                "ignore error from %s.beforeLogout on session[uuid:%s]",
+                ext.getClass().getSimpleName(), s.getUuid()), t);
+    }
+}
📝 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.

Suggested change
PluginRegistry pluginRgty = getComponentLoader().getComponent(PluginRegistry.class);
for (LogoutExtensionPoint ext : pluginRgty.getExtensionList(LogoutExtensionPoint.class)) {
ext.beforeLogout(s);
}
PluginRegistry pluginRgty = getComponentLoader().getComponent(PluginRegistry.class);
for (LogoutExtensionPoint ext : pluginRgty.getExtensionList(LogoutExtensionPoint.class)) {
try {
ext.beforeLogout(s);
} catch (Throwable t) {
logger.warn(String.format(
"ignore error from %s.beforeLogout on session[uuid:%s]",
ext.getClass().getSimpleName(), s.getUuid()), t);
}
}
🤖 Prompt for AI Agents 
In identity/src/main/java/org/zstack/identity/Session.java around lines 223 to
227, the loop calling each LogoutExtensionPoint.beforeLogout(s) lacks fault
isolation so any extension throwing an exception aborts logout and prevents
returning INVALID_SESSION; wrap each ext.beforeLogout(s) invocation in a
try-catch that catches Throwable (or Exception), log the exception with context
(which extension and session info) and continue to the next extension so logout
proceeds regardless of individual extension failures.


logout(s.getUuid());
return err(IdentityErrors.INVALID_SESSION, "Session expired");
}
Expand Down