Repository navigation
fix: prevent double submission of login and TOTP forms - #1199
Draft
jrollinson wants to merge 1 commit into
Draft
jrollinson wants to merge 1 commit into
jrollinson wants to merge 1 commit into
Conversation
The TOTP form auto-submits when the sixth digit is entered (tinyauthapp#790), but a follow-up Enter keypress or password-manager form submit still fires a second POST /api/user/totp. The first request consumes the pending TOTP session, so the duplicate gets 401 and the mutation error handler raises a false verification-failed toast right before the redirect. The login form has the same unguarded path: autofill plus Enter double-submits /api/user/login, creating extra pending TOTP sessions and counting against the login rate limit. Ignore submissions while the mutation is pending or after it succeeded; retries after a failed verification still work. AI disclosure per AI_POLICY.md: drafted with AI assistance, small patch, to be reviewed and understood by the author before marking ready.
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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. Comment |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The TOTP form auto-submits when the sixth digit is entered (#790), but a follow-up Enter keypress or a password-manager form submit still triggers a second
POST /api/user/totp. The first request consumes the pending TOTP session, so the duplicate gets 401 (TOTP verification attempt without pending TOTP session) and the mutation'sonErrorraises the verification-failed toast during the 500 ms window before the redirect — users see a false "verification failed" after every successful TOTP login.The login form has the same unguarded path: password autofill plus Enter double-submits
POST /api/user/login, creating extra pending TOTP sessions and counting againstauth.loginMaxRetriesrate limiting.Observed on a self-hosted v5.2.0 deployment: server logs show a trailing
POST /api/user/totp401 after every successful verification, and doubledPOST /api/user/login200s in the same second.Fix
Ignore form submissions while the mutation is pending or after it succeeded (retry after a failed verification still works) — applied to both
TotpPageandLoginPage, covering the auto-submit, Enter, and external submit-button paths. The auto-submit UX from #790 is untouched.Verification
tsc -bandvite buildpasstotp-page.tsx/login-page.tsx/auth_service.go(a successful verification flipstotp_pendingoff, so the duplicate POST can only answer 401)AI disclosure
Per AI_POLICY.md: this patch was drafted with AI assistance. It is small (2 files), and the account holder is reviewing and taking ownership of it before this PR leaves draft status.