Fix(app-store): add whether it is in debug mode and interface authorization check to the app store#680
Conversation
📝 WalkthroughWalkthrough为 IndexController 添加类级别 AccessTokenMiddleware。调整 ConfigProvider::__invoke 在非调试环境下直接返回空数组,在调试模式维持原有注解配置返回。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant AccessTokenMiddleware
participant IndexController
Client->>Router: HTTP Request (/app-store/index...)
Router->>AccessTokenMiddleware: Dispatch with priority 100
AccessTokenMiddleware->>AccessTokenMiddleware: 验证访问令牌
alt 验证通过
AccessTokenMiddleware->>IndexController: 调用控制器动作
IndexController-->>Client: 响应
else 验证失败
AccessTokenMiddleware-->>Client: 拒绝/错误响应
end
sequenceDiagram
participant Framework
participant ConfigProvider
Framework->>ConfigProvider: __invoke()
ConfigProvider->>ConfigProvider: 读取 APP_DEBUG
alt APP_DEBUG 为真
ConfigProvider-->>Framework: 返回原注解配置数组
else
ConfigProvider-->>Framework: 返回 []
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
plugin/mine-admin/app-store/src/Controller/IndexController.php (3)
27-28: 控制器级中间件会使整个 AppStore 接口在非调试环境不可用且统一要求令牌,请确认产品期望将 DeBugOnlyMiddleware 与 AccessTokenMiddleware 同时应用于控制器级别意味着:
- 非调试环境下该控制器所有接口都返回拒绝访问(当前实现为抛异常)。
- 调试环境下也仍会要求访问令牌。
请确认这是否符合产品目标。如果仅部分写操作(install/unInstall/upload 等)需要令牌或调试限制,建议在方法级单独声明中间件,避免一刀切。
如需我给出“方法级精确挂载中间件”的改造版代码,请告知目标路由清单(哪些要 Debug 限制、哪些要令牌、哪些公开)。
86-90: hasAccessToken 路由被控制器级中间件保护,可能导致“自检令牌存在性”陷入闭环当前 hasAccessToken 也会被 DeBugOnly + AccessToken 拦截:
- 非调试环境:直接拒绝访问,无法自检。
- 调试环境:仍需携带令牌才能访问,但该接口的语义正是“检查是否存在令牌”,容易形成“没有令牌就无法检查是否有令牌”的闭环。
可选方案(择一):
- 将 AccessTokenMiddleware 只应用到需要鉴权的写接口(方法级),放开 hasAccessToken 与只读接口。
- 在 AccessTokenMiddleware 内部维护白名单路由(例如 admin/plugin/store/hasAccessToken)直接放行。
- 若必须控制器级统一挂载,可在 hasAccessToken 方法改用仅 Debug 限制、不做令牌校验的单独中间件(但 Hyperf 默认无法“剔除父级中间件”,需改挂载策略)。
需要我起草方法级挂载方案的具体 diff 吗?
86-90: 空字符串被视为“存在令牌”,建议更严谨地判空env('MINE_ACCESS_TOKEN') 返回空字符串时,当前判断也会当作“存在”。建议判空字符串。
应用此 diff:
- return $this->success(['isHas' => env('MINE_ACCESS_TOKEN') !== null]); + return $this->success(['isHas' => (string) env('MINE_ACCESS_TOKEN', '') !== '']);app/Http/Common/Middleware/DeBugOnlyMiddleware.php (3)
21-22: 类命名与不可变性建议:DebugOnlyMiddleware 命名更自然,并标记 final
- 命名建议改为 DebugOnlyMiddleware(统一单词大小写)。
- 与项目中 AccessTokenMiddleware 一致,建议标记 final,防止被不必要继承。
如接受重命名,请同步更新引用处:
- plugin/mine-admin/app-store/src/Controller/IndexController.php: 导入与属性引用类名
21-41: 可测试性与一致性优化(可选):注入 ConfigInterface 而非全局 helper通过构造注入 Hyperf\Contract\ConfigInterface,避免对全局函数的硬依赖,便于单测中注入不同配置。
我可以提供基于依赖注入的精简改造版,如需要请告知。
21-41: 建议新增单元/集成测试覆盖调试开关两种分支
- debug=true:应放行并委托后续 handler。
- debug=false:应返回 403(或 404)而非 500。
我可以补充对应测试样例(含模拟请求与期望状态码),是否需要我起草测试代码?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
app/Http/Common/Middleware/DeBugOnlyMiddleware.php(1 hunks)plugin/mine-admin/app-store/src/Controller/IndexController.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/Http/Common/Middleware/DeBugOnlyMiddleware.php (1)
app/Exception/Handler/AbstractHandler.php (1)
handle(42-67)
plugin/mine-admin/app-store/src/Controller/IndexController.php (2)
app/Http/Common/Middleware/AccessTokenMiddleware.php (1)
AccessTokenMiddleware(18-24)app/Http/Common/Middleware/DeBugOnlyMiddleware.php (1)
DeBugOnlyMiddleware(21-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Test on PHP8.3 Swoole-v6.0.2
- GitHub Check: Test on PHP8.1 Swoole-v6.0.2
- GitHub Check: Test on PHP8.1 Swoole-v5.0.3
- GitHub Check: Test on PHP8.2 Swoole-v6.0.2
- GitHub Check: Test on PHP8.1 Swoole-v5.1.6
- GitHub Check: Test on PHP8.1 Swoole-master
- GitHub Check: Test on PHP8.2 Swoole-master
- GitHub Check: Test on PHP8.2 Swoole-v5.1.6
- GitHub Check: Test on PHP8.2 Swoole-v5.0.3
- GitHub Check: Test on PHP8.3 Swoole-v5.1.6
- GitHub Check: Test on PHP8.3 Swoole-master
- GitHub Check: Test on PHP8.1 Swow-develop
- GitHub Check: Test on PHP8.2 Swow-develop
- GitHub Check: Test on PHP8.3 Swow-develop
🔇 Additional comments (3)
plugin/mine-admin/app-store/src/Controller/IndexController.php (2)
15-16: 导入与注解使用合理,符合 Hyperf 属性中间件写法新增的命名空间导入与 Middleware 注解引入方式正确,无冗余依赖。
Also applies to: 21-21
27-28: 确认 priority 语义无误:priority 值越大中间件越先执行根据 Hyperf 官方文档及社区说明(v3.0.34+),priority 数值越大对应中间件执行优先级越高,即会越先执行。你在
IndexController.php中以101(DeBugOnlyMiddleware)高于100(AccessTokenMiddleware)配置,已满足预期执行顺序,无需调整。app/Http/Common/Middleware/DeBugOnlyMiddleware.php (1)
38-41: 无需变更:config('debug') 已正确映射 APP_DEBUG项目根目录的
config/config.php中已定义:// config/config.php return [ // ... 'debug' => env('APP_DEBUG', false), // ... ];因此
config('debug')正确获取环境变量APP_DEBUG,不存在config('app_debug')或config('app.debug')的配置项,无需修改。Likely an incorrect or invalid review comment.
…gOnlyMiddleware from IndexController
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugin/mine-admin/app-store/src/ConfigProvider.php (1)
19-22: 确认业务意图:非调试环境直接返回空配置将完全禁用该插件的注解扫描此早返回会让插件在非调试环境下不参与 annotations 扫描,等同于“彻底下线”该插件。若目标是生产环境完全关闭 App-Store,当前做法合理;若目标仅是限制访问(而仍希望路由/控制器被扫描注册),建议保留扫描并依赖 IndexController 上的 AccessTokenMiddleware/DebugOnlyMiddleware 做访问控制,而不是在 ConfigProvider 中返回空数组。
请确认预期行为。如果需要,我可以给出保留扫描、仅通过中间件拒绝访问的改法。
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
plugin/mine-admin/app-store/src/ConfigProvider.php(1 hunks)plugin/mine-admin/app-store/src/Controller/IndexController.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugin/mine-admin/app-store/src/Controller/IndexController.php
Summary by CodeRabbit