Require password proof when disabling TOTP#33
Conversation
Send current-password step-up proof with the TOTP disable request so the UI matches the hardened backend contract. Made-with: Cursor
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd35904e39
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) : null} | ||
|
|
||
| {!status.enabled ? ( | ||
| {!setup ? ( |
There was a problem hiding this comment.
Switching the password field guard to !setup hides the current-password input as soon as setup starts, but enable() still derives oldAuthHash from currentPassword. If the server returns invalid_credentials at this stage (for example, password changed in another session between setup and verify), the UI shows an error but provides no way to correct the password because the field is hidden while setup is truthy, so the flow is effectively stuck until a full refresh.
Useful? React with 👍 / 👎.
Keep the current-password field available during active setup so users can correct stale password proof before enabling TOTP. Made-with: Cursor
|
@codex review Addressed the inline P2 by keeping the current-password field available during active TOTP setup, so users can correct stale password proof before retrying enable. Added regression coverage for editing the field while setup is active. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38cd41a1e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) : null} | ||
|
|
||
| {!status.enabled ? ( | ||
| {!recoveryCodes ? ( |
There was a problem hiding this comment.
The new disable() flow now always derives oldAuthHash, but this render guard hides the current-password field whenever recoveryCodes exists. After a successful enable, recoveryCodes is set and currentPassword is cleared, while the “Disable two-factor authentication” button remains available; clicking disable then repeatedly fails with password_required and the user has no way to correct it in-session (without a full reload).
Useful? React with 👍 / 👎.
Keep the current-password field visible through setup, recovery-code display, and disable states so step-up retries cannot get stuck. Made-with: Cursor
|
@codex review Addressed the second inline P2 by making the current-password field always available in the TOTP card body, including while recovery codes are displayed and disable remains available. Added regression coverage for that recovery-code state. |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Test plan
Made with Cursor