Skip to content

fix: MFA login fails with CSRF token validation error due to duplicate form submission#3371

Open
yog27ray wants to merge 1 commit into
parse-community:alphafrom
yog27ray:fix/mfa-login-duplicate-submit
Open

fix: MFA login fails with CSRF token validation error due to duplicate form submission#3371
yog27ray wants to merge 1 commit into
parse-community:alphafrom
yog27ray:fix/mfa-login-duplicate-submit

Conversation

@yog27ray

@yog27ray yog27ray commented Jun 13, 2026

Copy link
Copy Markdown

New Pull Request Checklist

Issue Description

Closes: #3370

Logging in with MFA (OTP) enabled fails with CSRF token validation failed. Please refresh the page and try again. (HTTP 403, EBADCSRFTOKEN). The password step works; the failure is on the OTP step. It reproduces on a single instance, in incognito, with no extensions, so it is not the multi-replica / sticky-session case from #3015.

Root Cause

LoginForm.react.js submits the form twice per click. The button is type="submit" (which natively submits the form as the click's default action) and its onClick also calls this.formRef.current.submit(), a second programmatic submission:

<input
  type="submit"
  onClick={() => {
    this.props.formSubmit();
    this.formRef.current.submit(); // redundant: type="submit" already submits
  }}
/>

On the password step both submissions return 302, so the duplicate is harmless. On the MFA step the first submission authenticates; Passport's req.logIn then calls req.session.regenerate() (session-fixation protection), which destroys the session holding the CSRF token (csrf-sync stores it in req.session). The browser cancels that response and never receives the regenerated cookie, so the second submission lands on the dead session, finds no CSRF token, and fails with 403.

Approach

Remove the redundant programmatic this.formRef.current.submit() and let the native type="submit" submit the form exactly once. The synchronous formSubmit() side effect still runs first, and Enter-key submission is preserved. The now-unused formRef (React.createRef() + ref=) is removed.

TODOs before merging

  • Add tests
  • Add changes to documentation (n/a, internal behavior fix)
  • Add security check
  • Add changes to API docs (n/a)

Summary by CodeRabbit

  • Bug Fixes

    • Fixed duplicate form submissions during MFA and CSRF-sensitive operations by streamlining the form submission flow.
  • Tests

    • Added comprehensive test coverage for form submission behavior, including verification of proper callback execution and prevention of unintended programmatic submissions.

@parse-github-assistant

Copy link
Copy Markdown

🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review.

Tip

  • Keep pull requests small. Large PRs will be rejected. Break complex features into smaller, incremental PRs.
  • Use Test Driven Development. Write failing tests before implementing functionality. Ensure tests pass.
  • Group code into logical blocks. Add a short comment before each block to explain its purpose.
  • We offer conceptual guidance. Coding is up to you. PRs must be merge-ready for human review.
  • Our review focuses on concept, not quality. PRs with code issues will be rejected. Use an AI agent.
  • Human review time is precious. Avoid review ping-pong. Inspect and test your AI-generated code.

Note

Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect.

Caution

Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code.

@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 09f635c3-83b1-4f7a-bb5e-05687d1105bb

📥 Commits

Reviewing files that changed from the base of the PR and between 137e5bd and cf368d1.

📒 Files selected for processing (2)
  • src/components/LoginForm/LoginForm.react.js
  • src/lib/tests/LoginForm.test.js

📝 Walkthrough

Walkthrough

LoginForm refactored to eliminate duplicate form submissions during MFA login. The component removes its formRef mechanism and the corresponding manual form.submit() call in the onClick handler, preventing a secondary submission that destroyed the session after Passport's session regeneration. Tests added to validate single submission behavior.

Changes

MFA Login Form Submission Fix

Layer / File(s) Summary
Remove formRef and duplicate submission from LoginForm
src/components/LoginForm/LoginForm.react.js
Constructor and formRef removed; <form> ref prop eliminated; submit button onClick handler now calls only this.props.formSubmit() instead of also invoking this.formRef.current.submit(), with comments explaining CSRF/MFA session regeneration behavior.
LoginForm submission behavior tests
src/lib/tests/LoginForm.test.js
Jest test suite verifies form renders with method="post", submit input is type="submit", formSubmit callback fires exactly once on click, native form.submit() is not called programmatically, and submission is blocked when disableSubmit is true.

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Title check ✅ Passed The PR title begins with 'fix:' prefix and clearly describes the main change—fixing duplicate form submission causing MFA login CSRF errors.
Description check ✅ Passed The PR description is comprehensive, covering issue reference (#3370), root cause analysis, approach, and test/documentation status. It follows the repository template structure with Issue, Approach, and Tasks sections.
Linked Issues check ✅ Passed Code changes (remove formRef and duplicate submission) directly address the root cause identified in #3370. Tests were added to verify the form submits once and the formSubmit callback is called correctly, meeting the linked issue's requirement.
Out of Scope Changes check ✅ Passed All changes are scoped to the linked issue: LoginForm refactor removes redundant submission, and new tests validate the fix. No unrelated modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Security Check ✅ Passed Reviewed LoginForm.react.js + test: removed redundant manual form submission and unused ref; no eval/dangerouslySetInnerHTML/JS injection patterns added, reducing duplicate-request CSRF risk.
Engage In Review Feedback ✅ Passed GitHub PR #3371 shows “Reviewers: No reviews”, so there were no review feedback comments requiring the author to engage or respond.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 OpenGrep (1.22.0)

OpenGrep fatal error (exit code 2): [00.12][ERROR]: Error: exception Unix_error: No such file or directory stat src/components/LoginForm/LoginForm.react.js
Raised by primitive operation at UTmp.replace_named_pipe_by_regular_file_if_needed in file "libs/commons/UTmp.ml", line 145, characters 8-27
Called from Scan_CLI.replace_target_roots_by_regular_files_where_needed.(fun) in file "src/osemgrep/cli_scan/Scan_CLI.ml", lines 1086-1087, characters 19-65
Called from List_.fast_map in file "libs/commons/List_.ml", line 81, characters 17


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MFA login fails with "CSRF token validation failed" due to duplicate form submission

1 participant