Skip to content

Fix(app-store): add whether it is in debug mode and interface authorization check to the app store#680

Merged
zds-s merged 2 commits into
mineadmin:masterfrom
people-sea:fix/plugin/app-store-auth-check-debug
Aug 18, 2025
Merged

Fix(app-store): add whether it is in debug mode and interface authorization check to the app store#680
zds-s merged 2 commits into
mineadmin:masterfrom
people-sea:fix/plugin/app-store-auth-check-debug

Conversation

@people-sea

@people-sea people-sea commented Aug 18, 2025

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Refactor
    • 对应用商店的 Index 控制器启用访问令牌校验中间件;未携带或无效令牌的请求将被拒绝。
  • Chores
    • 配置在非调试环境下默认不加载对应注解配置,仅在调试模式生效,减少运行时开销与启动负担。

@coderabbitai

coderabbitai Bot commented Aug 18, 2025

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

为 IndexController 添加类级别 AccessTokenMiddleware。调整 ConfigProvider::__invoke 在非调试环境下直接返回空数组,在调试模式维持原有注解配置返回。

Changes

Cohort / File(s) Summary
控制器中引入鉴权中间件
plugin/mine-admin/app-store/src/Controller/IndexController.php
新增 use 引用并在类上添加 #[Middleware(middleware: AccessTokenMiddleware::class, priority: 100)],其余接口与注入不变。
配置提供者增加环境短路
plugin/mine-admin/app-store/src/ConfigProvider.php
在 __invoke 中加入对 APP_DEBUG 的判定:非真时提前返回空数组;为真时按原有逻辑返回注解配置。对外签名未变。

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
Loading
sequenceDiagram
  participant Framework
  participant ConfigProvider

  Framework->>ConfigProvider: __invoke()
  ConfigProvider->>ConfigProvider: 读取 APP_DEBUG
  alt APP_DEBUG 为真
    ConfigProvider-->>Framework: 返回原注解配置数组
  else
    ConfigProvider-->>Framework: 返回 []
  end
Loading

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 Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or PR title anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@boring-cyborg boring-cyborg Bot added the Http label Aug 18, 2025
@people-sea
people-sea requested a review from zds-s August 18, 2025 05:23
@people-sea
people-sea marked this pull request as ready for review August 18, 2025 05:23
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug labels Aug 18, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 0a61d77 and 8da2625.

📒 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.

Comment thread app/Http/Common/Middleware/DeBugOnlyMiddleware.php Outdated
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Aug 18, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 8da2625 and 02bfaf2.

📒 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

Comment thread plugin/mine-admin/app-store/src/ConfigProvider.php
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Aug 18, 2025
@people-sea
people-sea enabled auto-merge (squash) August 18, 2025 06:18
@people-sea
people-sea disabled auto-merge August 18, 2025 06:20
@zds-s
zds-s merged commit 0a600ca into mineadmin:master Aug 18, 2025
4 checks passed
@people-sea people-sea changed the title Fix(app-store): add DeBugOnlyMiddleware and apply debug/auth check to IndexController Fix(app-store): add whether it is in debug mode and interface authorization check to the app store Aug 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Http lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants