You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
bug: /token returns HTTP 200 with an unusable access_token while a 2FA challenge is pending — silent break for password-grant machine clients #2322
When enterprise 2FA applies to an account (the user is enrolled, or a role policy requires enrollment), POST /token with a correct username/password still returns HTTP 200 with an access_token + token_type: "bearer" — but that token carries the MFA challenge scope and is rejected with 401 by every endpoint except /api/enterprise/2fa/login/*. Any OAuth2 password-grant client (monitoring dashboards, ops tooling, scripts) reads access_token from a 200 and believes authentication succeeded; the failure only surfaces as unexplained 401s on later calls. The break is silent and presents as instance malfunction rather than an auth-flow change.
POST /token → 200
{"access_token": "<jwt>", "challenge_token": "<jwt>", "token_type": "bearer",
"mfa_required": true, "mfa_enrolled": true, "enrollment_required": false}
GET /api/version (Authorization: Bearer <that access_token>) → 401
{"detail": "Could not validate credentials"}
Root Cause
The MFA-challenge response reuses the success shape of the OAuth2 password grant. A field named access_token with token_type: "bearer" in a 200 response is, by contract, a usable bearer token — here it is a challenge-scoped token that get_current_user deliberately rejects everywhere (the MFA_CHALLENGE_SCOPE check). Verified empirically on an enrolled admin account; per the router code the same shape is returned for a policy-required-but-unenrolled user (forced-enrollment flow), so machine clients break the moment the policy is enabled, before anyone even enrolls.
Reproduction Steps
On an enterprise-entitled instance, enroll TOTP 2FA on an account (or enable "Require 2FA for admins")
POST /token with that account's correct credentials → observe 200 + access_token
Use the returned access_token on any authenticated endpoint (e.g. GET /api/version) → 401
Suggested Fix
Signal the pending second factor as an error for the grant, per RFC 6749 error semantics: return 401/403 with {"detail": "mfa_required", "challenge_token": "..."} and no access_token field. If the frontend depends on the current 200 shape, keep it only on a UI-specific login endpoint and fix /token (the OAuth2-shaped surface) to fail loudly. At minimum, remove/null the access_token field in the challenge response so clients fail at login time, not downstream.
Picking this up. While tracing it I found the "Root Cause" section is off in one load-bearing detail, and it changes what's left to build — flagging before writing code rather than silently building something different.
access_token is null. The challenge token is never presented as a bearer by a naive client, so the MFA_CHALLENGE_SCOPE rejection path is never reached — it's a red herring.
The real chain producing the reported 401: client reads resp.json()["access_token"] → gets None → sends Authorization: Bearer None → get_current_user fails to decode it → 401 {"detail": "Could not validate credentials"}. Same symptom, different mechanism.
Consequence for the suggested fix
The issue's stated minimum — "At minimum, remove/null the access_token field in the challenge response" — is already shipped. Token's own docstring already says the challenge response carries "no access_token". What remains from the suggestion is only the RFC 6749 part: the route answers HTTP 200 with token_type: "bearer" for a grant that did not succeed.
Two findings not in the issue
1. The email login route already gets this right, by accident.POST /api/auth/email/verify (routers/auth.py:571) hits the same gate but carries no response_model and returns the raw challenge dict — so there is no access_token key at all and no token_type. A client doing r.json()["access_token"] gets a KeyError at the login call, loud and one frame from the cause. Two routes, same trigger, opposite failure modes.
2. Trinity ships the broken client.src/mcp-server/src/client.ts:141-147:
if(!response.ok)thrownewError(`Authentication failed: ${response.statusText}`);constdata=(awaitresponse.json())asTokenResponse;this.token=data.access_token;// undefined on a 200 challenge
Status checked, payload not — this.token becomes undefined and every later call 401s. Legacy/dev path (prod runs MCP_REQUIRE_API_KEY=true, so the MCP server holds no password), but it's a first-party instance of exactly the pattern the issue describes.
Also worth stating: tests/ contains zero references to mfa_gate, mfa_required, or challenge_token. There is no coverage on any of this today.
The worse trigger, for prioritisation
The issue frames enrolment as the trigger. The policy arm is worse: TwoFactorProvider.gate_decision returns required=True for every admin/creator-role account the moment require_for_admin / require_for_creator is set, and gate_login fires on enrolled or required. So flipping the policy breaks every unattended admin/creator credential instantly, before anyone has enrolled anything — one toggle, no user action, and the rollout hasn't started yet. Worth leading the fix with.
What I'm scoping into this issue
The half that needs no product decision — align the password route with the behaviour the email route already has:
/token (and its /api/token alias) stops emitting an empty access_token and a token_type: "bearer" on the challenge response. Failure moves to the login call instead of an unrelated 401 hours later.
Harden src/mcp-server/src/client.ts to verify it actually received a session.
First tests for the gate — both login routes, both the enrolled and the policy-required arms.
Verified blast radius: stores/auth.js:194 and :255 both branch on mfa_required and return before reading access_token, so the browser flow is unaffected. The other token_type hits in the tree are unrelated (Settings.vue:3032 is GitHub PAT validation; the public docs describe the success case).
Two questions I'm deliberately not answering
Both are product/architecture calls, so I'm leaving them here rather than guessing:
Should /token return a non-2xx for a deferred login? RFC 6749 says a failed grant is an error response. Doing it properly means stores/auth.js must read the challenge out of an error body at both call sites — a breaking change for any client already handling the current 200 shape. My scope above deliberately stops short of this.
Should a role policy sweep machine credentials into a human-interactive second factor at all? If the answer is no, the fix is larger than the response shape and this issue is the wrong container for it.
@vybe — happy to take either or both into scope if you want them here; otherwise I'll ship the three items above and we file the rest separately.
Activity
obasilakis commentedon Aug 20, 2026
/claim
github-actions commentedon Aug 20, 2026
Claimed by @obasilakis. Label
status-in-progressadded.Next steps:
feature/2322-<short-slug>Fixes #2322obasilakis commentedon Aug 20, 2026
Root-cause correction before I implement this
Picking this up. While tracing it I found the "Root Cause" section is off in one load-bearing detail, and it changes what's left to build — flagging before writing code rather than silently building something different.
The token field is empty, not challenge-scoped
The issue reports:
{"access_token": "<jwt>", "challenge_token": "<jwt>", "token_type": "bearer", "mfa_required": true, ...}and attributes the downstream 401 to
dependencies.pyrejectingMFA_CHALLENGE_SCOPE.What the code actually does on
dev:services/mfa_gate.py::gate_loginreturns{mfa_required, mfa_enrolled, enrollment_required, challenge_token}— it never setsaccess_tokenmodels.py::Tokendeclaresaccess_token: Optional[str] = Nonerouters/auth.py:318is@router.post("/token", response_model=Token)with noresponse_model_exclude_noneSo the serialized challenge response is:
{"access_token": null, "token_type": "bearer", "mfa_required": true, "mfa_enrolled": true, "enrollment_required": false, "challenge_token": "eyJ..."}access_tokenis null. The challenge token is never presented as a bearer by a naive client, so theMFA_CHALLENGE_SCOPErejection path is never reached — it's a red herring.The real chain producing the reported 401: client reads
resp.json()["access_token"]→ getsNone→ sendsAuthorization: Bearer None→get_current_userfails to decode it →401 {"detail": "Could not validate credentials"}. Same symptom, different mechanism.Consequence for the suggested fix
The issue's stated minimum — "At minimum, remove/null the
access_tokenfield in the challenge response" — is already shipped.Token's own docstring already says the challenge response carries "noaccess_token". What remains from the suggestion is only the RFC 6749 part: the route answers HTTP 200 withtoken_type: "bearer"for a grant that did not succeed.Two findings not in the issue
1. The email login route already gets this right, by accident.
POST /api/auth/email/verify(routers/auth.py:571) hits the same gate but carries noresponse_modeland returns the raw challenge dict — so there is noaccess_tokenkey at all and notoken_type. A client doingr.json()["access_token"]gets aKeyErrorat the login call, loud and one frame from the cause. Two routes, same trigger, opposite failure modes.2. Trinity ships the broken client.
src/mcp-server/src/client.ts:141-147:Status checked, payload not —
this.tokenbecomesundefinedand every later call 401s. Legacy/dev path (prod runsMCP_REQUIRE_API_KEY=true, so the MCP server holds no password), but it's a first-party instance of exactly the pattern the issue describes.Also worth stating:
tests/contains zero references tomfa_gate,mfa_required, orchallenge_token. There is no coverage on any of this today.The worse trigger, for prioritisation
The issue frames enrolment as the trigger. The policy arm is worse:
TwoFactorProvider.gate_decisionreturnsrequired=Truefor every admin/creator-role account the momentrequire_for_admin/require_for_creatoris set, andgate_loginfires onenrolled or required. So flipping the policy breaks every unattended admin/creator credential instantly, before anyone has enrolled anything — one toggle, no user action, and the rollout hasn't started yet. Worth leading the fix with.What I'm scoping into this issue
The half that needs no product decision — align the password route with the behaviour the email route already has:
/token(and its/api/tokenalias) stops emitting an emptyaccess_tokenand atoken_type: "bearer"on the challenge response. Failure moves to the login call instead of an unrelated 401 hours later.src/mcp-server/src/client.tsto verify it actually received a session.Verified blast radius:
stores/auth.js:194and:255both branch onmfa_requiredandreturnbefore readingaccess_token, so the browser flow is unaffected. The othertoken_typehits in the tree are unrelated (Settings.vue:3032is GitHub PAT validation; the public docs describe the success case).Two questions I'm deliberately not answering
Both are product/architecture calls, so I'm leaving them here rather than guessing:
/tokenreturn a non-2xx for a deferred login? RFC 6749 says a failed grant is an error response. Doing it properly meansstores/auth.jsmust read the challenge out of an error body at both call sites — a breaking change for any client already handling the current 200 shape. My scope above deliberately stops short of this.@vybe — happy to take either or both into scope if you want them here; otherwise I'll ship the three items above and we file the rest separately.
fix(auth): address /review findings — doc call-sites, token echo, ema…
docs(security): CSO --diff audit report for #2322 (0 findings)
fix(auth): a deferred grant is an error for the grant — /token answer…
docs(security): record R1 as closed in the CSO report addendum (#2322)
docs(flows): the /sync-feature-flows sweep this PR skipped (#2322)
docs(learnings): correct the #2322 entry — it was wrong about its own…
fix(auth): a login that issued no session must not look like one that…
fix(auth): address /review findings — doc call-sites, token echo, ema…
docs(security): CSO --diff audit report for #2322 (0 findings)
fix(auth): a deferred grant is an error for the grant — /token answer…
docs(security): record R1 as closed in the CSO report addendum (#2322)
docs(flows): the /sync-feature-flows sweep this PR skipped (#2322)
docs(learnings): correct the #2322 entry — it was wrong about its own…
fix(auth): a login that issued no session must not look like one that…