fix: MFA single-use token bypass via concurrent authData login requests (GHSA-w73w-g5xw-rwhf)#10326
Merged
mtrezza merged 1 commit intoparse-community:alphafrom Mar 26, 2026
Conversation
|
🚀 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
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. |
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughAdded a test suite validating MFA recovery code reuse prevention under concurrent authData-only logins, and modified RestWrite's authData handling to snapshot pre-mutation values and enforce optimistic-locking constraints during database updates, translating certain update failures to a standardized error response. Changes
Sequence DiagramsequenceDiagram
participant Client as Client (10 Concurrent)
participant RestWrite
participant _User as _User DB
Note over Client,_User: Concurrent Login Attempts with Same Recovery Token
par Winning Request
Client->>RestWrite: POST /login (authData.mfa.recovery)
activate RestWrite
RestWrite->>RestWrite: snapshot authData
RestWrite->>RestWrite: apply mutatedAuthData (consume recovery code)
RestWrite->>_User: UPDATE query with optimistic-lock<br/>(authData.mfa.recovery == [original_token])
activate _User
_User-->>RestWrite: ✓ Success (1 match, 1 updated)
deactivate _User
RestWrite-->>Client: 200 OK
deactivate RestWrite
and Losing Requests (9x)
Client->>RestWrite: POST /login (authData.mfa.recovery)
activate RestWrite
RestWrite->>RestWrite: snapshot authData
RestWrite->>RestWrite: apply mutatedAuthData
RestWrite->>_User: UPDATE query with optimistic-lock<br/>(authData.mfa.recovery == [original_token])
activate _User
_User-->>RestWrite: ✗ OBJECT_NOT_FOUND<br/>(lock predicate failed)
deactivate _User
RestWrite->>RestWrite: translate to SCRIPT_FAILED
RestWrite-->>Client: 400 Invalid auth data
deactivate RestWrite
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10326 +/- ##
=======================================
Coverage 92.52% 92.52%
=======================================
Files 192 192
Lines 16505 16517 +12
Branches 227 227
=======================================
+ Hits 15271 15283 +12
Misses 1214 1214
Partials 20 20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spec/vulnerabilities.spec.js (1)
4387-4399: Assert the rejection contract too.This still passes as long as nine requests reject, even if they reject for an unrelated reason. Asserting
SCRIPT_FAILED/Invalid auth dataon the losers and the originalobjectIdon the winner would make the regression much tighter.🧪 Suggested assertion tightening
const succeeded = results.filter(r => r.status === 'fulfilled'); const failed = results.filter(r => r.status === 'rejected'); // Exactly one request should succeed; all others should fail expect(succeeded.length).toBe(1); expect(failed.length).toBe(9); + expect(succeeded[0].value.data.objectId).toBe(user.id); + failed.forEach(({ reason }) => { + expect(reason.data.code).toBe(Parse.Error.SCRIPT_FAILED); + expect(reason.data.error).toBe('Invalid auth data'); + }); // Verify the recovery code has been consumed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/vulnerabilities.spec.js` around lines 4387 - 4399, Tighten the assertions after the concurrent loginWithRecovery() calls by checking rejection reasons and the successful winner: verify each failed result in results (where r.status === 'rejected') has the expected error code/message (e.g., 'SCRIPT_FAILED' or 'Invalid auth data') and that the single fulfilled result contains the original user's objectId; keep the existing verification of consumed recovery code via user.fetch() and recoveryCode but add checks on results to assert the precise rejection contract and the winner's returned objectId to prevent unrelated failures from passing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@spec/vulnerabilities.spec.js`:
- Around line 4387-4399: Tighten the assertions after the concurrent
loginWithRecovery() calls by checking rejection reasons and the successful
winner: verify each failed result in results (where r.status === 'rejected') has
the expected error code/message (e.g., 'SCRIPT_FAILED' or 'Invalid auth data')
and that the single fulfilled result contains the original user's objectId; keep
the existing verification of consumed recovery code via user.fetch() and
recoveryCode but add checks on results to assert the precise rejection contract
and the winner's returned objectId to prevent unrelated failures from passing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3201f086-ca8f-4f27-b36c-6cf821aac8c6
📒 Files selected for processing (2)
spec/vulnerabilities.spec.jssrc/RestWrite.js
mtrezza
changed the title
parseplatformorg
pushed a commit
that referenced
this pull request
Mar 26, 2026
# [9.7.0-alpha.8](9.7.0-alpha.7...9.7.0-alpha.8) (2026-03-26) ### Bug Fixes * MFA single-use token bypass via concurrent authData login requests ([GHSA-w73w-g5xw-rwhf](GHSA-w73w-g5xw-rwhf)) ([#10326](#10326)) ([e7efbeb](e7efbeb))
Contributor
|
🎉 This change has been released in version 9.7.0-alpha.8 |
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
2 participants
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.
Issue
MFA single-use token bypass via concurrent authData login requests (GHSA-w73w-g5xw-rwhf)