<fix>[identity]: validate session logout token #2650
Conversation
Resolves: ZSTAC-77668 Change-Id: I6166696f686d6f677363766678646c7567746168
Walkthrough在会话过期检查中，新增在注销前调用插件注册表中的 LogoutExtensionPoint.beforeLogout(SessionInventory) 钩子；随后继续执行原有的注销并返回 INVALID_SESSION 错误。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Identity as Identity.Session
participant Registry as PluginRegistry
participant Ext as LogoutExtensionPoint(s)
participant Auth as Logout Service
Note over Identity: 检测到会话过期
Identity->>Registry: 获取所有 LogoutExtensionPoint
loop 对每个扩展点
Identity->>Ext: beforeLogout(SessionInventory)
end
Identity->>Auth: logout(sessionUuid)
Auth-->>Identity: 完成
Identity-->>Identity: 返回 INVALID_SESSION
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
identity/src/main/java/org/zstack/identity/Session.java (3)
223-227: 并发情形下 beforeLogout 可能被多次触发，需确保扩展幂等两个请求并发命中同一过期会话时，均会走到该分支并调用 beforeLogout，然后其中一个成功删除记录。建议在扩展实现侧保证幂等，或在调用前做一次快速存在性检查（非必需）。
223-227: 补充轻量可观测性日志为定位扩展侧问题与调用耗时，建议在 debug/trace 级别记录扩展调用开始/结束与耗时（保持默认关闭，不影响性能）。
223-227: 统一 beforeLogout 回调：定时/批量删除会绕过扩展回调验证结果：代码中有显式调用 Session.logout(...) 的路径（identity/.../Session.java:141、identity/.../login/LoginManagerImpl.java:257、identity/.../AccountManagerImpl.java:1145），且 Session.logout 内也会触发 SessionLogoutExtensionPoint 并对 SessionVO 做 hardDelete（约在 Session.java 行 177–180）。但存在直接批量删除会话的路径，会绕过 logout 回调：identity/.../Session.java:294–297 中执行了 "delete from SessionVO s where s.uuid in :uuids" 并直接 dq.executeUpdate()。结论：并非所有会话终结都会触发 beforeLogout，定时/批量清理会跳过回调。
建议（非破坏性改进）：评估统一触发点——例如在 deleteSession()/批量删除前补齐 before 钩子，或将定时清理改为逐条调用 Session.logout，以保证扩展一致性；可先作为后续增强保留最小改动策略。
📜 Review details
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
identity/src/main/java/org/zstack/identity/Session.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - 代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写
Files:
identity/src/main/java/org/zstack/identity/Session.java
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名：
- API 名称必须唯一，不能重复。
- API 消息类需要继承
APIMessage；其返回类必须继承APIReply或APIEvent，并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest，并满足如下规范：
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时，使用
{variableName}格式。- HTTP 方法对应：
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档，并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名：
- 使用 UpperCamelCase 风格。
- 特殊情况：
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量：
- 使用 lowerCamelCase 风格。
常量命名：
- 全部大写，使用下划线分隔单词。
- 要求表达清楚，避免使用含糊或不准确的名称。
包名：
- 统一使用小写，使用点分隔符，每个部分应是一个具有自然语义的英文单词（参考 Spring 框架的结构）。
命名细节：
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量，防止混淆。
- 命名缩写：
- 不允许使用不必要的缩写，如：
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达：
- 避免使用布尔型参数造成含义不明确。例如：
- 对于
stopAgent(boolean ignoreError)，建议拆分为不同函数（如stopAgentIgnoreError()），或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图，并在名称中体现数据类型或用途（例如在常量与变量名称中，将类型词放在末尾）。
- 避免使用魔法值（Magic Value）：
直接使用未经定义的数值或字符串（如 if (status == 5)）应替换为枚举或常量。
示例：
// 错误示例：魔法值
if (user.getStatus() == 5) { ... }
// 正确示例：常量或枚举
public static final int STATUS_ACTIVE = 5;
if (user.getStatus() == STATUS_ACTIVE) { ... }
// 或使用枚举
enum UserStatus { ACTIVE, INACTIVE }
注释：
- 代码应尽量做到自解释，对少于两行的说明可以直接写在代码中。
- 对于较长的注释，需要仔细校对并随代码更新，确保内容正确。
- 接口方法不应有多余的修饰符（例如
public），且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用：
- 应尽量减少 if...else 结构的使用，建议：
- 限制嵌套层级最多为两层，且内层不应再出现
else分支。- 尽早返回（Early Return），将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断：
- if 条件表达不宜过长或过于复杂，必要时可以将条件抽成 boolean 变量描述。
代码块长度：
...
Files:
identity/src/main/java/org/zstack/identity/Session.java
🧠 Learnings (1)
📓 Common learnings
Learnt from: zstack-robot-1
PR: MatheMatrix/zstack#2360
File: network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java:449-490
Timestamp: 2025-08-04T04:48:19.103Z
Learning: ZStack项目在cherry-pick操作中，即使发现了性能优化机会（如IP地址批量保存的内存优化），也严格遵循不做额外修改的政策，优先保证cherry-pick的完整性和一致性。
Learnt from: ZStack-Robot
PR: MatheMatrix/zstack#2489
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.java:471-545
Timestamp: 2025-08-22T05:36:40.467Z
Learning: In ZStack项目中，对于核心逻辑相关的代码，团队倾向于暂时不进行重构，即使代码复杂度较高，优先保证核心功能的稳定性。
Learnt from: ZStack-Robot
PR: MatheMatrix/zstack#2325
File: compute/src/main/java/org/zstack/compute/vm/VmMigrateCallExtensionFlow.java:29-29
Timestamp: 2025-07-24T05:53:10.246Z
Learning: 在ZStack项目的重构过程中，团队采用务实的优先级策略：优先完成影响运行时功能的代码重构，对于注释中的旧引用等非功能性问题可以延后处理，因为这些不会影响系统的实际运行。
| PluginRegistry pluginRgty = getComponentLoader().getComponent(PluginRegistry.class); | ||
|
|
||
| for (LogoutExtensionPoint ext : pluginRgty.getExtensionList(LogoutExtensionPoint.class)) { | ||
| ext.beforeLogout(s); | ||
| } |
There was a problem hiding this comment.
在过期注销前缺少故障隔离：单个扩展抛异常会阻断注销与错误返回
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.
| 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.
Resolves: ZSTAC-77668
Change-Id: I6166696f686d6f677363766678646c7567746168
sync from gitlab !8437