Skip to content

fix: allow trusted TOTP devices on v1 auth login#9163

Open
VectorPeak wants to merge 1 commit into
AstrBotDevs:masterfrom
VectorPeak:fix/v1-totp-trusted-device-cookie
Open

fix: allow trusted TOTP devices on v1 auth login#9163
VectorPeak wants to merge 1 commit into
AstrBotDevs:masterfrom
VectorPeak:fix/v1-totp-trusted-device-cookie

Conversation

@VectorPeak

@VectorPeak VectorPeak commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

This PR fixes the trusted-device TOTP flow for the v1 dashboard login endpoint. The dashboard login UI uses /api/v1/auth/login as the primary login path, but the trusted-device cookie was scoped to the legacy /api/auth path. Because of that cookie path mismatch, a user who selected "trust this device" during v1 login could still be asked for TOTP again on the next v1 login.

Modifications / 改动点

This change scopes the TOTP trusted-device cookie to /api, so it covers both supported dashboard auth login surfaces:

/api/auth/login
/api/v1/auth/login

It also clears the previous legacy-scoped trusted-device cookie at Path=/api/auth when issuing the new cookie. That avoids leaving two same-name trusted-device cookies with different paths in browsers that already received the old cookie.

The existing trusted-device token issuance and database validation logic is unchanged. Password verification, TOTP code verification, recovery-code behavior, JWT cookie handling, trusted-device token format, trusted-device database storage, API key auth, and legacy auth compatibility are not changed.

What Problem This Solves

The dashboard login flow sends login requests through authApi.login(), which first calls the generated OpenAPI v1 client:

POST /api/v1/auth/login

The legacy endpoint is only used as a fallback.

When TOTP is enabled, the user can complete the TOTP step with trust_device_flag: true. The backend correctly verifies the TOTP code and issues a trusted-device token. However, _set_trusted_device_cookie() previously wrote that token as:

Set-Cookie: astrbot_totp_trusted_device=...; Path=/api/auth; HttpOnly; SameSite=strict

A cookie scoped to /api/auth covers legacy auth routes such as /api/auth/login, but it does not cover the v1 auth route /api/v1/auth/login. A standard browser or cookie jar therefore does not send astrbot_totp_trusted_device back to the v1 login endpoint. On the next v1 login attempt, _login() receives no trusted-device token, AuthService.login() treats the device as untrusted, and the response still requires TOTP.

In practice, this made the user-visible "trust this device" option work for the legacy auth path, but not for the current v1 dashboard login path.

Evidence

Before the fix, a cookie scoped to /api/auth was sent to legacy auth paths, but not to v1 auth paths:

http://testserver.local/api/auth/login => astrbot_totp_trusted_device=trusted-token
http://testserver.local/api/auth/totp/setup => astrbot_totp_trusted_device=trusted-token
http://testserver.local/api/v1/auth/login => <no cookie>
http://testserver.local/api/v1/auth/setup-status => <no cookie>

The affected user-visible flow was:

1. User logs in through the dashboard UI.
2. The UI calls POST /api/v1/auth/login.
3. TOTP is required.
4. User enters a valid TOTP code and enables "trust this device".
5. Backend issues astrbot_totp_trusted_device with Path=/api/auth.
6. User later logs in again through the same dashboard UI.
7. Browser sends the request to /api/v1/auth/login without the trusted-device cookie.
8. Backend returns 401 with totp_required again.

The regression test now verifies the fixed v1 flow:

POST /api/v1/auth/login with valid TOTP + trust_device_flag=true
POST /api/v1/auth/login again without a TOTP code
expected: 200 ok

Possible call chain / impact

Dashboard login form
  -> authStore.login(username, password, code, trustDevice)
  -> authApi.login(...)
  -> openApiV1.login(...)
  -> POST /api/v1/auth/login

astrbot/dashboard/api/auth.py
  -> login(...)
  -> _login(...)
  -> request.cookies.get("astrbot_totp_trusted_device")
  -> AuthService.login(..., trusted_device_cookie_token=...)

astrbot/dashboard/services/auth_service.py
  -> is_totp_trusted_device_valid(...)
  -> missing trusted-device cookie means TOTP is required again

Risk boundary: the trusted-device cookie remains HttpOnly and SameSite=Strict, and its Secure behavior still follows the existing dashboard JWT secure-cookie setting. The path change broadens browser attachment from /api/auth to /api, but backend trusted-device validation remains required and the cookie is only consumed by the login flow.

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

Screenshots or Test Results / 运行截图或测试结果

Focused regression tests:

.\.venv\Scripts\python.exe -m pytest tests/test_dashboard.py::test_auth_login_sets_trusted_device_cookie_when_flag_true tests/test_dashboard.py::test_auth_login_skips_totp_when_trusted_cookie_valid tests/test_dashboard.py::test_v1_auth_login_skips_totp_when_trusted_cookie_valid -q

Result:

3 passed, 1 warning

Formatting and lint checks:

.\.venv\Scripts\python.exe -m ruff format --check astrbot\dashboard\api\auth.py tests\test_dashboard.py
.\.venv\Scripts\python.exe -m ruff check astrbot\dashboard\api\auth.py tests\test_dashboard.py
git diff --check

Result:

2 files already formatted
All checks passed!

GitHub checks after the initial PR push passed before the follow-up compatibility cleanup:

Run pytest suite: pass
format-check: pass
build: pass
CodeQL: pass
Smoke tests: pass on Ubuntu, macOS, and Windows for Python 3.10-3.14

Checklist / 检查清单

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

  • 👀 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.
    / 我的更改没有引入恶意代码。

@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. area:webui The bug / feature is about webui(dashboard) of astrbot. labels Jul 6, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


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.

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

Copy link
Copy Markdown
Contributor

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 updates the path of the trusted device cookie from "/api/auth" to "/api" in the authentication API. It also updates the existing test suite to reflect this change and introduces a new test, test_v1_auth_login_skips_totp_when_trusted_cookie_valid, to ensure that the trusted device cookie is correctly applied and verified under the new path during v1 login flows. I have no feedback to provide as there are no review comments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@VectorPeak VectorPeak force-pushed the fix/v1-totp-trusted-device-cookie branch from 539aea8 to 8e5d708 Compare July 6, 2026 13:41
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:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant