fix: Empty authData bypasses credential requirement on signup (GHSA-wjqw-r9x4-j59v)

[mtrezza]

Issue

Empty authData bypasses credential requirement on signup (GHSA-wjqw-r9x4-j59v)

Tasks

• Add tests
• Add changes
• Add security check
• Add benchmark

[parse-github-assistant]

🚀 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.

[parseplatformorg]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[coderabbitai]

📝 Walkthrough

Walkthrough

Refines signup authData handling and validation: adds tests for empty/invalid authData cases and updates RestWrite to compute a hasAuthData flag, changing when and how authData is validated or short-circuited during user signup and related flows.

Changes

Cohort / File(s)	Summary
Vulnerability tests spec/vulnerabilities.spec.js	Adds a new test suite "(GHSA-wjqw-r9x4-j59v) Empty authData session issuance bypass" covering signup rejection for empty/null/invalid authData and allowance when username+password are provided.
AuthData validation logic src/RestWrite.js	Introduces hasAuthData detection for meaningful provider entries; replaces previous authData-existence checks with provider-aware conditions, short-circuits handling for empty/falsy provider data, and refactors when handleAuthData/validation runs.

Sequence Diagram(s)

sequenceDiagram
Participant Client
Participant RestWrite
Participant Providers
Participant Database

Client->>RestWrite: signup request (username?, password?, authData)
RestWrite->>RestWrite: compute hasAuthData (is any provider object with keys?)
alt hasAuthData true
    RestWrite->>Providers: validate provider authData
    Providers-->>RestWrite: validation result
    RestWrite->>Database: create user + session
    Database-->>RestWrite: created userId, sessionToken
    RestWrite-->>Client: 201 with objectId/sessionToken
else hasAuthData false
    alt username+password present
        RestWrite->>Database: create user + session (skip provider validation)
        Database-->>RestWrite: created userId, sessionToken
        RestWrite-->>Client: 201 with objectId/sessionToken
    else
        RestWrite-->>Client: 400 USERNAME_MISSING or UNSUPPORTED_SERVICE
    end
end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• fix: Concurrent signup with same authentication creates duplicate users #10149: Modifies RestWrite authData validation and adds guards around duplicate/edge-case authData handling—closely related to hasAuthData logic changes.
• fix: Unlinking auth provider triggers auth data validation #10045: Adjusts authData validation/unlink behavior to avoid unnecessary provider validation for empty/null entries.
• fix: AuthData validation incorrectly triggered on unchanged providers #10025: Addresses authData re-validation and empty/stripped provider cases, overlapping with the signup/authData guard changes in this PR.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is entirely empty, missing all required template sections including Issue, Approach, and Tasks checklist required by the repository template.	Add a complete PR description following the template: include the Issue/link, describe the Approach with changes made, and check applicable Tasks (tests were added, document changes as needed).

✅ Passed checks (2 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title clearly and specifically identifies the main change: fixing a security vulnerability (GHSA-wjqw-r9x4-j59v) where empty authData was bypassing credential requirements during signup.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

📝 Coding Plan

• Generate coding plan for human review comments

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

spec/vulnerabilities.spec.js (1)

2938-2998: Strengthen rejection assertions to verify the intended failure path.

Using only toBeRejected() can mask false positives (any rejection passes). For security regression tests, assert status/error code/message.

✅ Example pattern

-      await expectAsync(
-        request({
-          method: 'POST',
-          url: 'http://localhost:8378/1/users',
-          headers: {
-            'Content-Type': 'application/json',
-            'X-Parse-Application-Id': 'test',
-            'X-Parse-REST-API-Key': 'rest',
-          },
-          body: JSON.stringify({ authData: {} }),
-        })
-      ).toBeRejected();
+      const res = await request({
+        method: 'POST',
+        url: 'http://localhost:8378/1/users',
+        headers: {
+          'Content-Type': 'application/json',
+          'X-Parse-Application-Id': 'test',
+          'X-Parse-REST-API-Key': 'rest',
+        },
+        body: JSON.stringify({ authData: {} }),
+      }).catch(e => e);
+      expect(res.status).toBe(400);
+      expect(res.data.code).toBe(Parse.Error.USERNAME_MISSING);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@spec/vulnerabilities.spec.js` around lines 2938 - 2998, The tests currently
use expectAsync(request(...)).toBeRejected() which only asserts any rejection;
update each failing test (the ones calling request POST /1/users with bodies
like { authData: {} }, { authData: { bogus: {} } }, and { authData: { bogus:
null } }) to explicitly verify the rejection reason: call request(...) inside a
try/catch (or use
expectAsync(...).toBeRejectedWith(jasmine.objectContaining(...))) and assert the
caught error contains the expected HTTP status (e.g., statusCode === 400 or
error.status === 400) and the response body/error message indicates invalid
authData or missing credentials so the test ensures the specific failure path
rather than any rejection.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/RestWrite.js`:
- Around line 472-474: The current guard uses hasAuthData to short-circuit
provider validation and skips validation for cases like { authData: { bogus: 'x'
} }; change the logic so that hasAuthData still gates credential flow but does
not bypass provider validation: only skip validation when the authData property
is entirely absent, and when authData exists ensure it's an object and run the
provider-specific validation (or reject if authData is present but not an
object). Update the conditional around hasAuthData /
Object.prototype.hasOwnProperty.call(this.data, 'authData') so that provider
validation functions (the authData validation routine used in this file) are
invoked whenever this.data.authData exists and is an object, and treat
non-object authData as invalid instead of skipping validation.

---

Nitpick comments:
In `@spec/vulnerabilities.spec.js`:
- Around line 2938-2998: The tests currently use
expectAsync(request(...)).toBeRejected() which only asserts any rejection;
update each failing test (the ones calling request POST /1/users with bodies
like { authData: {} }, { authData: { bogus: {} } }, and { authData: { bogus:
null } }) to explicitly verify the rejection reason: call request(...) inside a
try/catch (or use
expectAsync(...).toBeRejectedWith(jasmine.objectContaining(...))) and assert the
caught error contains the expected HTTP status (e.g., statusCode === 400 or
error.status === 400) and the response body/error message indicates invalid
authData or missing credentials so the test ensures the specific failure path
rather than any rejection.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 117f8a39-6a3b-4a1f-86e7-906391feb068

📥 Commits

Reviewing files that changed from the base of the PR and between 6476c57 and 96c6670.

📒 Files selected for processing (2)

• spec/vulnerabilities.spec.js
• src/RestWrite.js

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.57%. Comparing base (84db0a0) to head (3074910).
⚠️ Report is 65 commits behind head on alpha.

Additional details and impacted files

@@           Coverage Diff           @@
##            alpha   #10219   +/-   ##
=======================================
  Coverage   92.57%   92.57%           
=======================================
  Files         192      192           
  Lines       16296    16300    +4     
  Branches      199      199           
=======================================
+ Hits        15086    15090    +4     
  Misses       1193     1193           
  Partials       17       17           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[parseplatformorg]

🎉 This change has been released in version 9.6.0-alpha.29

[parseplatformorg]

🎉 This change has been released in version 9.6.0