Skip to content

feat: 为 WebUI 新增忘记密码功能,基于终端随机确认码的双因子安全验证#8420

Open
Sisyphbaous-DT-Project wants to merge 6 commits into
AstrBotDevs:masterfrom
Sisyphbaous-DT-Project:feat/forgot-password-confirmation-code
Open

feat: 为 WebUI 新增忘记密码功能,基于终端随机确认码的双因子安全验证#8420
Sisyphbaous-DT-Project wants to merge 6 commits into
AstrBotDevs:masterfrom
Sisyphbaous-DT-Project:feat/forgot-password-confirmation-code

Conversation

@Sisyphbaous-DT-Project
Copy link
Copy Markdown
Contributor

@Sisyphbaous-DT-Project Sisyphbaous-DT-Project commented May 29, 2026

功能说明

为 AstrBot WebUI 新增"忘记密码"自助重置功能。用户在 WebUI 登录页点击"忘记密码"后,通过 终端随机确认码进行身份验证,验证通过后 AstrBot 自动重启并生成新的随机密码输出到终端日志中。

安全设计

本功能采用双因子安全模型,要求攻击者必须同时拥有两种访问权限:

  1. WebUI 访问权限 — 能够访问登录页面并发起重置请求
  2. 终端/日志访问权限 — 能够读取后端输出到终端的 6 位随机确认码

安全机制细节

机制 说明
随机确认码 每次 init 生成 6 位随机码(字符集 ABCDEFGHJKLMNPQRSTUVWXYZ23456789,排除易混淆字符 O/0/1/I),仅输出到终端日志
确认码过期 有效期 5 分钟,超时需重新获取
速率限制 5 分钟内最多发起 3 次 init 请求,防止暴力破解
标记文件 验证通过后创建 data/.reset_dashboard_password,AstrBot 启动时检测并触发密码重置,重置后自动删除该文件
自动重启 提交确认码后自动调用 core_lifecycle.restart() 重启,前端轮询等待服务恢复后自动刷新
验证失败限制 连续输入 3 次错误确认码后该确认码作废,需重新获取,防止暴力猜测
Demo 模式 Demo 模式下禁用此功能

用户操作流程

登录页 → 点击"忘记密码"
  → 后端生成确认码,输出到终端日志
  → 前端弹窗,用户输入 6 位确认码
  → 二次确认弹窗(含警告 + 重启提示)
  → 确认 → 后端验证 → 创建标记文件 → AstrBot 重启
  → 前端显示"正在重启"遮罩,轮询等待恢复
  → 恢复后自动刷新页面,新密码已输出到终端日志

改动内容

后端

astrbot/core/config/astrbot_config.py

  • 新增 DASHBOARD_RESET_FLAG_FILE:标记文件路径 data/.reset_dashboard_password
  • _is_dashboard_password_reset_requested():同时检查环境变量 ASTRBOT_DASHBOARD_RESET_PASSWORD 和标记文件
  • 密码重置后自动删除标记文件

astrbot/dashboard/routes/auth.py

  • 新增 POST /api/auth/forgot-password/init
    • 使用 secrets 模块生成密码学安全的 6 位随机确认码
    • 所有时间计算使用 time.monotonic() 替代 time.time() 防止系统时钟回拨影响
    • 5 分钟过期,速率限制(5 分钟 3 次)
    • 生成新确认码时重置失败次数
  • 新增 POST /api/auth/forgot-password
    • 验证确认码 → 创建标记文件 → 延迟重启
    • 验证失败累加计数,连续 3 次错误后作废确认码
    • 后台重启任务保持强引用防止被 GC 回收
  • AuthRoute.__init__ 新增 core_lifecycle 参数,用于在验证通过后触发 restart()

astrbot/dashboard/server.py

  • auth_middleware 放行 /api/auth/forgot-password/init(未经认证可访问)
  • AuthRoute 实例化时传入 core_lifecycle

前端

dashboard/src/views/authentication/authForms/AuthLogin.vue

  • 登录表单底部新增"忘记密码"链接按钮
  • 两阶段弹窗流程:输入确认码 → 二次确认(含警告 + 重启提示)
  • 新增 pollForRestart():每 2 秒探测 /api/auth/setup-status,最多 60 次
  • 重启期间显示遮罩,恢复后自动刷新

dashboard/src/stores/auth.ts

  • 新增 forgotPasswordInit():调用 /api/auth/forgot-password/init
  • 新增 forgotPassword(code):调用 /api/auth/forgot-password 提交确认码

i18n

dashboard/src/i18n/locales/{zh-CN,en-US,ru-RU}/features/auth.json

  • 新增 forgotPassword 字段,包含翻译键:labeltitlecodeHintcodeLabelcodeInvalidconfirmTitlewarningTitlewarningTextrestartHintconfirmReset
  • 新增页面级翻译键:cancelnextrestartingrestartingTitlerestartingHint

修改的文件

文件 新增行数
astrbot/core/config/astrbot_config.py +35
astrbot/dashboard/routes/auth.py +124
astrbot/dashboard/server.py +4
dashboard/src/views/authentication/authForms/AuthLogin.vue +166
dashboard/src/stores/auth.ts +24
dashboard/src/i18n/locales/zh-CN/features/auth.json +17
dashboard/src/i18n/locales/en-US/features/auth.json +17
dashboard/src/i18n/locales/ru-RU/features/auth.json +17
tests/test_dashboard.py +188
总计 +592

测试结果

新增 5 个 forgot-password 测试,全部通过

tests/test_dashboard.py::test_forgot_password_rejects_without_init PASSED
tests/test_dashboard.py::test_forgot_password_rejects_wrong_code PASSED
tests/test_dashboard.py::test_forgot_password_code_expires_after_three_wrong_attempts PASSED
tests/test_dashboard.py::test_forgot_password_creates_flag_file PASSED
tests/test_dashboard.py::test_forgot_password_flag_file_triggers_reset_on_startup PASSED

========== 5 passed in 4.94s ==========

覆盖场景:

  1. 未初始化确认码直接提交 → 拒绝
  2. 提交错误确认码 → 拒绝
  3. 连续 3 次错误确认码 → 确认码作废,第 4 次提示重新获取
  4. 提交正确确认码 → 创建标记文件 + 触发 restart
  5. 启动时标记文件已存在 → 触发密码重置 + 删除标记文件

d85f1e23668d275fc361292c40a5ca09 40b89c4e10c7df0ca030634052689227 ff26efdb4e64608501f3d94608923440 8b1e29fa0d7d4d7b7d30fe99af565346 d7b4d88e2123e6a12098dd42be65ed0e 89473b1d90eb5ad4c15fe5ed02758466

Checklist / 检查清单

  • 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
    / 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。

  • 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
    / 我的更改经过了良好的测试,并已在上方提供了"验证步骤"和"运行截图"

  • 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.toml 文件相应位置。

  • 😮 My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码。

  • 🔒 This is NOT a breaking change.
    / 这不是一个破坏性变更。

Summary by Sourcery

Add a secure self-service WebUI forgot-password flow backed by a terminal-displayed confirmation code and automatic password reset on restart.

New Features:

  • Introduce backend endpoints to initialize and verify a 6-character confirmation code for dashboard password reset, with rate limiting and demo-mode safeguards.
  • Add a WebUI forgot-password UX on the login page, including code entry, warning confirmation, and an automatic restart-wait overlay tied to setup-status polling.
  • Support dashboard password reset via a flag file or environment variable, regenerating a random password on startup and exposing it for administrators.

Enhancements:

  • Wire the core lifecycle into authentication routes to allow asynchronous service restarts after successful password reset verification.

Tests:

  • Add integration tests covering forgot-password initialization, wrong-code handling with attempt limits, flag-file creation, and password reset consumption on startup.

- 新增 POST /api/auth/forgot-password/init 端点,生成 6 位随机确认码输出到终端
- POST /api/auth/forgot-password 改为验证随机码,增加 5 分钟过期和速率限制(5 分钟 3 次)
- 验证通过后创建 .reset_dashboard_password 标记文件,AstrBot 自动重启并生成新随机密码
- 前端改为两阶段流程:点击按钮 → 调用 init API → 输入 6 位确认码 → 二次确认 → 重启轮询
- 新增 4 个 forgot_password 测试:无 init 拒绝、错误码拒绝、正确码创建 flag file 及重启、启动时 flag file 触发重置
- 更新 zh-CN/en-US/ru-RU i18n 翻译,将确认文本相关键替换为确认码相关键
- 使用 secrets.choice 替代 random.choices 生成密码学安全的随机确认码
- 新增确认码验证失败次数限制(连续 3 次错误后作废)
- 所有时间计算改用 time.monotonic() 防止系统时钟回拨影响
- asyncio.create_task 保持强引用防止被 GC 回收
- 前端 pollForRestart 检查 res.ok 处理反代 502/503 状态码
- 新增测试覆盖连续 3 次验证失败后确认码作废场景
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. area:webui The bug / feature is about webui(dashboard) of astrbot. labels May 29, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a 'Forgot Password' feature for the AstrBot dashboard, allowing users to request a password reset via a 6-digit confirmation code printed to the terminal. The backend handles code generation, validation, rate limiting, and triggers an automatic restart to apply the reset, while the frontend provides a two-step dialog flow and a restart overlay. Unit tests have also been added to cover these new endpoints and behaviors. The feedback suggests improving the user experience by allowing users to edit their entered code upon failure instead of forcing a full reset, and trimming whitespace from the confirmation code on both the frontend and backend.

Comment thread dashboard/src/views/authentication/authForms/AuthLogin.vue
Comment thread dashboard/src/views/authentication/authForms/AuthLogin.vue
Comment thread astrbot/dashboard/routes/auth.py
- closeConfirmDialog 改为返回第一步弹窗,保留已输入的确认码,用户可修改后重试
- submitForgotStep1 对确认码 trim 处理,防止复制粘贴带入首尾空格
- 后端 forgot_password 对 code 做 strip 处理,defense in depth
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • The forgot-password rate limiting and attempt counters are stored on the AuthRoute instance in memory, so they will reset on any process restart or multi-worker deployment; consider whether you need a more durable or shared store (or at least documenting that this protection is per-process and ephemeral).
  • The new forgot-password endpoints currently mix English and Chinese error messages (e.g. demo mode vs rate limit and code validation); aligning these to i18n keys like the UI strings would make backend responses consistent and easier to localize.
  • In _delayed_restart, self.core_lifecycle is assumed non-None even though the constructor allows it to be omitted; either make core_lifecycle required for this feature or guard against it being None to avoid potential attribute errors.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The forgot-password rate limiting and attempt counters are stored on the AuthRoute instance in memory, so they will reset on any process restart or multi-worker deployment; consider whether you need a more durable or shared store (or at least documenting that this protection is per-process and ephemeral).
- The new forgot-password endpoints currently mix English and Chinese error messages (e.g. demo mode vs rate limit and code validation); aligning these to i18n keys like the UI strings would make backend responses consistent and easier to localize.
- In `_delayed_restart`, `self.core_lifecycle` is assumed non-None even though the constructor allows it to be omitted; either make `core_lifecycle` required for this feature or guard against it being `None` to avoid potential attribute errors.

## Individual Comments

### Comment 1
<location path="astrbot/dashboard/routes/auth.py" line_range="303-304" />
<code_context>
+            return Response().error("创建重置标记失败,请检查文件权限").__dict__
+
+        # Trigger restart asynchronously so the HTTP response can be sent first
+        if self.core_lifecycle is not None:
+            self._restart_task = asyncio.create_task(self._delayed_restart())
+
+        return Response().ok(None, "密码重置请求已接受,AstrBot 即将重启").__dict__
</code_context>
<issue_to_address>
**suggestion:** Storing the restart task on the instance might be unnecessary and potentially misleading.

`_restart_task` is never read, which can imply lifecycle management that doesn’t exist. If you don’t intend to await or cancel this task, just call `asyncio.create_task(self._delayed_restart())` without storing it, or otherwise make the fire-and-forget nature explicit (e.g., via a local variable).

Suggested implementation:

```python
        # Trigger restart asynchronously so the HTTP response can be sent first (fire-and-forget)
        if self.core_lifecycle is not None:
            asyncio.create_task(self._delayed_restart())

```

If there is an attribute declaration like `self._restart_task = None` in `__init__` or elsewhere in this class that was added for this feature, it should be removed as it is no longer used.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread astrbot/dashboard/routes/auth.py
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 6 issues, and left some high level feedback:

  • The reset-code and rate-limit state for the forgot-password flow is stored only in-memory on the AuthRoute instance; in multi-worker or multi-process deployments this can lead to inconsistent behavior (code generated on one worker, validated on another), so consider persisting this state to a shared store or making the flow explicitly single-process-only (e.g., via configuration or logging).
  • In AstrBotConfig.init you reset DASHBOARD_RESET_PASSWORD_ENV to "0" after handling a reset request; if an operator sets this env var intentionally (e.g., via systemd), you may be surprising them by mutating process env at runtime—consider treating the env flag as read-only and relying solely on the flag file for one-shot resets, or documenting and isolating this mutation more clearly.
  • The frontend restart polling uses AbortSignal.timeout in pollForRestart, which is not supported in some older browsers and runtimes; if you need broader compatibility, consider wrapping this in a feature check or replacing it with a manual timeout using setTimeout and AbortController.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The reset-code and rate-limit state for the forgot-password flow is stored only in-memory on the AuthRoute instance; in multi-worker or multi-process deployments this can lead to inconsistent behavior (code generated on one worker, validated on another), so consider persisting this state to a shared store or making the flow explicitly single-process-only (e.g., via configuration or logging).
- In AstrBotConfig.__init__ you reset DASHBOARD_RESET_PASSWORD_ENV to "0" after handling a reset request; if an operator sets this env var intentionally (e.g., via systemd), you may be surprising them by mutating process env at runtime—consider treating the env flag as read-only and relying solely on the flag file for one-shot resets, or documenting and isolating this mutation more clearly.
- The frontend restart polling uses AbortSignal.timeout in pollForRestart, which is not supported in some older browsers and runtimes; if you need broader compatibility, consider wrapping this in a feature check or replacing it with a manual timeout using setTimeout and AbortController.

## Individual Comments

### Comment 1
<location path="dashboard/src/views/authentication/authForms/AuthLogin.vue" line_range="95-104" />
<code_context>
+  }
+}
+
+async function pollForRestart() {
+  let attempts = 0;
+  const maxAttempts = 60;
+  const interval = 2000;
+
+  while (attempts < maxAttempts) {
+    await new Promise((resolve) => setTimeout(resolve, interval));
+    attempts++;
+    try {
+      const res = await fetch('/api/auth/setup-status', { method: 'GET', signal: AbortSignal.timeout(3000) });
+      if (!res.ok) throw new Error('Server not ready');
+      restartPending.value = false;
+      window.location.reload();
+      return;
+    } catch {
</code_context>
<issue_to_address>
**issue (bug_risk):** AbortSignal.timeout usage may not be available in all target browsers

`AbortSignal.timeout` isn’t supported in all target browsers and can throw, breaking the polling loop. To keep behavior consistent, gate its use with a feature check or switch to an `AbortController` plus `setTimeout` pattern for the timeout signal.
</issue_to_address>

### Comment 2
<location path="astrbot/dashboard/routes/auth.py" line_range="269-270" />
<code_context>
+                .__dict__
+            )
+
+        post_data = await request.json
+        if not isinstance(post_data, dict):
+            return Response().error("Invalid request payload").__dict__
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Accessing request.json directly may be framework-specific or incorrect

`post_data = await request.json` assumes `json` is an awaitable attribute. In many frameworks this is a method (`await request.json()`) or accessed differently. Please verify this matches your framework’s API and the pattern used elsewhere, otherwise this will fail at runtime.
</issue_to_address>

### Comment 3
<location path="astrbot/core/config/astrbot_config.py" line_range="83-92" />
<code_context>
             )
         # 检查配置完整性,并插入
         has_new = self.check_config_integrity(default_config, conf)
+        dashboard_reset_requested = self._is_dashboard_password_reset_requested()
         if (
             "dashboard" in conf
             and isinstance(conf["dashboard"], dict)
-            and not conf["dashboard"].get("pbkdf2_password")
-            and not conf["dashboard"].get("password")
+            and (
+                dashboard_reset_requested
+                or (
+                    not conf["dashboard"].get("pbkdf2_password")
+                    and not conf["dashboard"].get("password")
+                )
+            )
         ):
             self._reset_generated_dashboard_password(conf)
+            if dashboard_reset_requested:
+                os.environ[DASHBOARD_RESET_PASSWORD_ENV] = "0"
             has_new = True
</code_context>
<issue_to_address>
**question (bug_risk):** Resetting DASHBOARD_RESET_PASSWORD_ENV inside the process may not clear it for the next run

Setting `os.environ[DASHBOARD_RESET_PASSWORD_ENV] = "0"` only affects the current process and its children. If the env var is set by the shell/service, it will still be set again on the next start, so `_is_dashboard_password_reset_requested` will keep returning true. If this is meant as a one‑time reset signal, consider using the env var only to create/remove a persistent flag (e.g., a file) that you then check instead, or clearly document that the caller is responsible for unsetting the env var between runs.
</issue_to_address>

### Comment 4
<location path="dashboard/src/stores/auth.ts" line_range="112-110" />
<code_context>
         return false;
       }
     },
+    async forgotPasswordInit(): Promise<void> {
+      try {
+        const res = await axios.post('/api/auth/forgot-password/init');
+        if (res.data.status === 'error') {
+          return Promise.reject(res.data.message);
+        }
+        return Promise.resolve();
+      } catch (error) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Wrapping axios calls in explicit Promise.resolve/reject is redundant and obscures error types

These `async` functions don’t need `Promise.resolve` / `Promise.reject`; just `return` on success and `throw` on error and the caller’s `await`/`try...catch` will behave the same. Also, you’re sometimes rejecting with a string (`res.data.message`) and other times letting the axios error object bubble, while the caller expects `err?.response?.data?.message`. It’d be clearer to normalize errors, e.g. `if (res.data.status === 'error') throw new Error(res.data.message);` and otherwise let axios throw, so the caller always sees a consistent error shape.

Suggested implementation:

```typescript
    async forgotPasswordInit(): Promise<void> {
      const res = await axios.post('/api/auth/forgot-password/init');
      if (res.data.status === 'error') {
        throw new Error(res.data.message);
      }
    },

```

```typescript
        const res = await axios.post('/api/auth/forgot-password', {
          code: code
        });
        if (res.data.status === 'error') {
          throw new Error(res.data.message);

```

Depending on how callers currently handle these errors, you may also want to:
1. Update any `catch (err)` blocks that expect `err?.response?.data?.message` to instead use `err?.message` for these two flows.
2. Optionally, if you want to preserve the `err.response.data.message` shape, you could throw an object with a `response.data.message` property instead of `new Error`, but that would diverge from the example in your review comment.
</issue_to_address>

### Comment 5
<location path="astrbot/dashboard/routes/auth.py" line_range="53" />
<code_context>


 class AuthRoute(Route):
-    def __init__(self, context: RouteContext, db) -> None:
+    def __init__(self, context: RouteContext, db, core_lifecycle=None) -> None:
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the password-reset state/logic and restart orchestration into small helper components so AuthRoute only coordinates HTTP flow.

You can decouple the mutable reset state and restart orchestration from `AuthRoute` to reduce complexity while preserving behavior.

### 1. Encapsulate reset state in a small helper

Right now `AuthRoute` owns:

- `_reset_code`
- `_reset_code_expiry`
- `_reset_failed_attempts`
- `_reset_attempts`
- `_generate_reset_code`
- `_is_rate_limited`
- `_record_attempt`

You can move all of that into a dedicated helper that `AuthRoute` delegates to. This keeps the route’s flow linear and the state/logic localized.

Example (small, focused class in the same module or a new `password_reset.py`):

```python
# password_reset.py (or near AuthRoute)

import secrets
import time
from typing import List


class PasswordResetManager:
    def __init__(self) -> None:
        self._code: str | None = None
        self._expiry: float = 0.0
        self._failed_attempts: int = 0
        self._attempts: List[float] = []

    def _generate_code(self) -> str:
        charset = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"
        return "".join(secrets.choice(charset) for _ in range(6))

    def is_rate_limited(self, max_attempts: int = 3, window_seconds: float = 300.0) -> bool:
        now = time.monotonic()
        self._attempts = [t for t in self._attempts if now - t < window_seconds]
        return len(self._attempts) >= max_attempts

    def record_attempt(self) -> None:
        self._attempts.append(time.monotonic())

    def start_reset(self, ttl_seconds: float = 300.0) -> str:
        """Generate and store a new confirmation code, returning it."""
        code = self._generate_code()
        self._code = code
        self._expiry = time.monotonic() + ttl_seconds
        self._failed_attempts = 0
        return code

    def verify_code(self, code: str, max_failed_attempts: int = 3) -> tuple[bool, str | None]:
        """
        Returns (ok, error_message).
        If ok is True, the code is consumed and internal state cleared.
        """
        if self._code is None:
            return False, "请先点击忘记密码获取确认码"

        now = time.monotonic()
        if now > self._expiry:
            self._code = None
            return False, "确认码已过期,请重新获取"

        if code.upper() != self._code.upper():
            self._failed_attempts += 1
            if self._failed_attempts >= max_failed_attempts:
                self._code = None
                return False, "确认码错误次数过多,已失效,请重新获取"
            remaining = max_failed_attempts - self._failed_attempts
            return False, f"确认码不正确,还可以尝试 {remaining} 次"

        # success: consume code
        self._code = None
        return True, None
```

Then `AuthRoute` only needs a single field and delegates:

```python
class AuthRoute(Route):
    def __init__(self, context: RouteContext, db, core_lifecycle=None) -> None:
        super().__init__(context)
        self.db = db
        self.core_lifecycle = core_lifecycle
        self._reset_mgr = PasswordResetManager()
        # routes mapping as before...
```

Usage in handlers:

```python
async def forgot_password_init(self):
    if DEMO_MODE:
        return Response().error("You are not permitted to do this operation in demo mode").__dict__

    if self._reset_mgr.is_rate_limited():
        return Response().error("请求过于频繁,请 5 分钟后再试").__dict__

    self._reset_mgr.record_attempt()
    code = self._reset_mgr.start_reset()

    logger.info(
        "Password reset requested. Confirmation code: %s "
        "(valid for 5 minutes). Enter this code in the WebUI to proceed.",
        code,
    )
    return Response().ok(None, "确认码已生成,请查看终端日志获取 6 位确认码").__dict__
```

```python
async def forgot_password(self):
    if DEMO_MODE:
        return Response().error("You are not permitted to do this operation in demo mode").__dict__

    post_data = await request.json
    # validation of payload and string length as you have now ...
    code = code.strip()

    ok, err = self._reset_mgr.verify_code(code)
    if not ok:
        return Response().error(err).__dict__

    # rest of your flag file + restart logic unchanged...
```

This keeps all reset-related state and branching inside `PasswordResetManager`, and `AuthRoute` becomes mostly “glue” and HTTP-specific validation.

### 2. Move restart orchestration into a small helper

The restart logic is currently embedded in `AuthRoute` via `_delayed_restart` and `self.core_lifecycle`. You can move that out into a small helper function (in a lifecycle/util module) and call it from the route:

```python
# lifecycle_utils.py (or similar)

import asyncio
from astrbot import logger


async def delayed_restart(core_lifecycle, delay: float = 1.0) -> None:
    await asyncio.sleep(delay)
    try:
        await core_lifecycle.restart()
    except Exception as e:
        logger.error(f"Auto-restart after password reset failed: {e}")


def schedule_restart(core_lifecycle, delay: float = 1.0) -> None:
    if core_lifecycle is None:
        return
    asyncio.create_task(delayed_restart(core_lifecycle, delay))
```

Then in `AuthRoute.forgot_password`:

```python
from .lifecycle_utils import schedule_restart  # adjust import path

async def forgot_password(self):
    # ... after writing DASHBOARD_RESET_FLAG_FILE successfully ...

    schedule_restart(self.core_lifecycle, delay=1.0)
    return Response().ok(None, "密码重置请求已接受,AstrBot 即将重启").__dict__
```

This removes restart-specific concerns from `AuthRoute`, makes the route easier to read, and the reset flow easier to reason about, while preserving all behavior and using the same in-memory state model you introduced.
</issue_to_address>

### Comment 6
<location path="astrbot/core/config/astrbot_config.py" line_range="82" />
<code_context>
             )
         # 检查配置完整性,并插入
         has_new = self.check_config_integrity(default_config, conf)
+        dashboard_reset_requested = self._is_dashboard_password_reset_requested()
         if (
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the dashboard password reset decision and side effects into a helper method so the constructor only orchestrates the reset flow.

You can reduce the branching complexity in `__init__` by encapsulating all reset-related logic into a dedicated helper. That keeps the constructor focused on “what” happens (maybe reset) instead of “how” (env/flag, clearing, conditions).

For example:

```python
def __init__(...):
    ...
    has_new = self.check_config_integrity(default_config, conf)
    if self._maybe_reset_dashboard_password(conf, legacy_dashboard_password_change_required):
        has_new = True
    self.update(conf)
```

Then move all the reset decision-making and side effects into a single helper:

```python
def _maybe_reset_dashboard_password(
    self,
    conf: dict,
    legacy_dashboard_password_change_required: bool,
) -> bool:
    if "dashboard" not in conf or not isinstance(conf["dashboard"], dict):
        return False

    dashboard = conf["dashboard"]
    reset_requested = self._is_dashboard_password_reset_requested()
    no_password_set = not dashboard.get("pbkdf2_password") and not dashboard.get("password")

    # Case 1: no password configured (initial/reset generation)
    if no_password_set:
        self._reset_generated_dashboard_password(conf)
        if reset_requested:
            os.environ[DASHBOARD_RESET_PASSWORD_ENV] = "0"
        return True

    # Case 2: legacy config marked as requiring password change
    if legacy_dashboard_password_change_required and dashboard.get("pbkdf2_password"):
        self._reset_generated_dashboard_password(conf)
        return True

    return False
```

You can then keep `_is_dashboard_password_reset_requested` as-is, and keep the flag-file cleanup inside `_reset_generated_dashboard_password`, so all side effects (“do the reset & finalize flags”) are only triggered from `_maybe_reset_dashboard_password`, not from `__init__` directly. This removes the nested `(! ... or (! ...))` logic from the constructor and centralizes the env/flag handling into one place.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread dashboard/src/views/authentication/authForms/AuthLogin.vue Outdated
Comment thread astrbot/dashboard/routes/auth.py
Comment thread astrbot/core/config/astrbot_config.py
Comment thread dashboard/src/stores/auth.ts
Comment thread astrbot/dashboard/routes/auth.py
Comment thread astrbot/core/config/astrbot_config.py
防止 AuthRoute 在 core_lifecycle 未设置时被意外调用导致 AttributeError
AbortSignal.timeout 在 Safari < 16.4、旧版 Chrome/Edge 中不支持,会抛 TypeError
中断 pollForRestart 轮询循环。改为 AbortController + setTimeout 模式,
finally 块中清理定时器避免内存泄漏。
@Sisyphbaous-DT-Project
Copy link
Copy Markdown
Contributor Author

@sourcery-ai review

@Sisyphbaous-DT-Project
Copy link
Copy Markdown
Contributor Author

Sisyphbaous-DT-Project commented May 29, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • The forgot-password confirmation code and rate-limiting state are kept only in-memory on the AuthRoute instance, which means they won’t work consistently across multiple dashboard workers/processes or survive a web-server reload; consider persisting this state (e.g., in your existing DB or a shared cache) or explicitly constraining the deployment model.
  • The tests reach into AuthRoute’s private _reset_code field via view_func.__self__, which couples the tests tightly to the route implementation details; consider exposing a small test-only hook or refactoring code lookup so the tests don’t depend on private attributes.
  • In the frontend forgotPasswordInit/forgotPassword actions you sometimes reject with a raw message string and sometimes with the Axios error object, while the caller assumes err.response?.data?.message; you may want to normalize errors at the store layer so the component can handle them in a consistent way.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The forgot-password confirmation code and rate-limiting state are kept only in-memory on the `AuthRoute` instance, which means they won’t work consistently across multiple dashboard workers/processes or survive a web-server reload; consider persisting this state (e.g., in your existing DB or a shared cache) or explicitly constraining the deployment model.
- The tests reach into `AuthRoute`’s private `_reset_code` field via `view_func.__self__`, which couples the tests tightly to the route implementation details; consider exposing a small test-only hook or refactoring code lookup so the tests don’t depend on private attributes.
- In the frontend `forgotPasswordInit`/`forgotPassword` actions you sometimes `reject` with a raw message string and sometimes with the Axios error object, while the caller assumes `err.response?.data?.message`; you may want to normalize errors at the store layer so the component can handle them in a consistent way.

## Individual Comments

### Comment 1
<location path="astrbot/dashboard/routes/auth.py" line_range="53" />
<code_context>


 class AuthRoute(Route):
-    def __init__(self, context: RouteContext, db) -> None:
+    def __init__(self, context: RouteContext, db, core_lifecycle=None) -> None:
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the password reset and rate-limiting state/logic into a dedicated helper class so AuthRoute’s methods become thin HTTP orchestrators that delegate to it.

You can keep all current behavior but reduce the complexity on `AuthRoute` by pushing the reset/rate-limit state into a tiny helper object and turning the route methods into thin orchestrators.

### 1. Extract password-reset state/logic into a helper

Move all of:

- `_reset_code`
- `_reset_code_expiry`
- `_reset_failed_attempts`
- `_reset_attempts`
- `_generate_reset_code`
- `_is_rate_limited`
- `_record_attempt`

into a dedicated class, e.g. in the same module or a nearby one:

```python
class PasswordResetManager:
    def __init__(
        self,
        *,
        code_ttl: float = 300.0,
        max_requests: int = 3,
        request_window: float = 300.0,
        max_code_attempts: int = 3,
    ) -> None:
        self._code: str | None = None
        self._code_expiry: float = 0.0
        self._failed_attempts: int = 0
        self._attempt_timestamps: list[float] = []
        self._code_ttl = code_ttl
        self._max_requests = max_requests
        self._request_window = request_window
        self._max_code_attempts = max_code_attempts

    def _generate_code(self) -> str:
        charset = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"
        return "".join(secrets.choice(charset) for _ in range(6))

    def _is_rate_limited(self) -> bool:
        now = time.monotonic()
        self._attempt_timestamps = [
            t for t in self._attempt_timestamps
            if now - t < self._request_window
        ]
        return len(self._attempt_timestamps) >= self._max_requests

    def request_code(self) -> tuple[bool, str | None, str]:
        """Return (ok, error_message, code)."""
        if self._is_rate_limited():
            return False, "请求过于频繁,请 5 分钟后再试", ""

        self._attempt_timestamps.append(time.monotonic())
        code = self._generate_code()
        self._code = code
        self._code_expiry = time.monotonic() + self._code_ttl
        self._failed_attempts = 0
        return True, "", code

    def validate_code(self, code: str) -> tuple[bool, str]:
        """Return (ok, error_message). Clears code on success or too many failures."""
        if self._code is None:
            return False, "请先点击忘记密码获取确认码"

        now = time.monotonic()
        if now > self._code_expiry:
            self._code = None
            return False, "确认码已过期,请重新获取"

        if code.upper() != self._code.upper():
            self._failed_attempts += 1
            if self._failed_attempts >= self._max_code_attempts:
                self._code = None
                return False, "确认码错误次数过多,已失效,请重新获取"
            remaining = self._max_code_attempts - self._failed_attempts
            return False, f"确认码不正确,还可以尝试 {remaining}"

        # success
        self._code = None
        return True, ""
```

Then the `AuthRoute` only holds a single field:

```python
class AuthRoute(Route):
    def __init__(self, context: RouteContext, db, core_lifecycle=None) -> None:
        super().__init__(context)
        self.db = db
        self.core_lifecycle = core_lifecycle
        self._password_reset = PasswordResetManager()
        ...
```

### 2. Simplify route methods to orchestration-only

Refactor `forgot_password_init` and `forgot_password` to delegate to the helper:

```python
    async def forgot_password_init(self):
        if DEMO_MODE:
            return (
                Response()
                .error("You are not permitted to do this operation in demo mode")
                .__dict__
            )

        ok, err, code = self._password_reset.request_code()
        if not ok:
            return Response().error(err).__dict__

        logger.info(
            "Password reset requested. Confirmation code: %s "
            "(valid for 5 minutes). Enter this code in the WebUI to proceed.",
            code,
        )
        return (
            Response().ok(None, "确认码已生成,请查看终端日志获取 6 位确认码").__dict__
        )

    async def forgot_password(self):
        if DEMO_MODE:
            return (
                Response()
                .error("You are not permitted to do this operation in demo mode")
                .__dict__
            )

        post_data = await request.json
        if not isinstance(post_data, dict):
            return Response().error("Invalid request payload").__dict__

        code = post_data.get("code", "")
        if not isinstance(code, str) or len(code.strip()) != 6:
            return Response().error("确认码格式不正确").__dict__

        ok, err = self._password_reset.validate_code(code.strip())
        if not ok:
            return Response().error(err).__dict__

        # unchanged from your implementation
        try:
            with open(DASHBOARD_RESET_FLAG_FILE, "w", encoding="utf-8") as f:
                f.write("1")
        except OSError as e:
            logger.error(f"Failed to create password reset flag file: {e}")
            return Response().error("创建重置标记失败,请检查文件权限").__dict__

        if self.core_lifecycle is not None:
            self._restart_task = asyncio.create_task(self._delayed_restart())

        return Response().ok(None, "密码重置请求已接受,AstrBot 即将重启").__dict__
```

This keeps all existing behavior (including rate limiting and attempt limits) but:

- `AuthRoute` no longer manages several pieces of mutable cross-request state directly.
- The reset flow is encapsulated and testable in isolation (`PasswordResetManager`).
- The route class reads as a simple HTTP orchestrator, which should address the “too many concerns” feedback without changing semantics.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread astrbot/dashboard/routes/auth.py
@Soulter Soulter requested a review from Raven95676 May 30, 2026 09:43
@Soulter
Copy link
Copy Markdown
Member

Soulter commented May 30, 2026

修一下 code format

将 auth.py 中第 291 行的超长 return 语句拆分为多行,使 ruff format --check 通过。
@Sisyphbaous-DT-Project
Copy link
Copy Markdown
Contributor Author

f281e78686a659c973d39bb9d834e186

@LIghtJUNction
Copy link
Copy Markdown
Member

好啊,这样挺方便的

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:webui The bug / feature is about webui(dashboard) of astrbot. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants