Skip to content

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

Merged
mtrezza merged 3 commits intoparse-community:alphafrom
mtrezza:fix/GHSA-wjqw-r9x4-j59v-v9
Mar 16, 2026
Merged

fix: Empty authData bypasses credential requirement on signup (GHSA-wjqw-r9x4-j59v)#10219
mtrezza merged 3 commits intoparse-community:alphafrom
mtrezza:fix/GHSA-wjqw-r9x4-j59v-v9

Conversation

@mtrezza
Copy link
Member

@mtrezza mtrezza commented Mar 16, 2026

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
Copy link

parse-github-assistant bot commented Mar 16, 2026

🚀 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
Copy link
Contributor

parseplatformorg commented Mar 16, 2026

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
Copy link

coderabbitai bot commented Mar 16, 2026

📝 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

🚥 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

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

mtrezza added 2 commits March 16, 2026 13:15
- Separate credential gating from provider validation skip
- Non-object authData values now still go through provider validation
- Strengthen test assertions with specific status codes and error codes
@codecov
Copy link

codecov bot commented Mar 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.57%. Comparing base (84db0a0) to head (3074910).
⚠️ Report is 3 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.

@mtrezza mtrezza changed the title fix: GHSA-wjqw-r9x4-j59v v9 fix: Empty authData bypasses credential requirement on signup (GHSA-wjqw-r9x4-j59v) Mar 16, 2026
@mtrezza mtrezza merged commit 5dcbf41 into parse-community:alpha Mar 16, 2026
22 of 24 checks passed
parseplatformorg pushed a commit that referenced this pull request Mar 16, 2026
# [9.6.0-alpha.29](9.6.0-alpha.28...9.6.0-alpha.29) (2026-03-16)

### Bug Fixes

* Empty authData bypasses credential requirement on signup ([GHSA-wjqw-r9x4-j59v](GHSA-wjqw-r9x4-j59v)) ([#10219](#10219)) ([5dcbf41](5dcbf41))
parseplatformorg pushed a commit that referenced this pull request Mar 16, 2026
# [9.6.0-alpha.29](9.6.0-alpha.28...9.6.0-alpha.29) (2026-03-16)

### Bug Fixes

* Empty authData bypasses credential requirement on signup ([GHSA-wjqw-r9x4-j59v](GHSA-wjqw-r9x4-j59v)) ([#10219](#10219)) ([5dcbf41](5dcbf41))
@parseplatformorg
Copy link
Contributor

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

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

Labels

state:released-alpha Released as alpha version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants