fix: Validate authData input in dot-notation updates, login provider checks, and challenge endpoint

[mtrezza]

Issue

Input validation bugs in authData handling:

• Dot-notation update keys bypass authData field name validation, allowing injection of malformed provider names
• Login crashes with null dereference when stored authData contains unknown providers
• Challenge endpoint crashes with null dereference when provider values are null or non-object

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.

[coderabbitai]

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: c0b4cf10-2a52-44ba-b132-0ed9ced369de

📥 Commits

Reviewing files that changed from the base of the PR and between 0c7290c and f17555a.

📒 Files selected for processing (2)

• spec/vulnerabilities.spec.js
• src/Auth.js

🚧 Files skipped from review as they are similar to previous changes (2)

• spec/vulnerabilities.spec.js
• src/Auth.js

---

📝 Walkthrough

Walkthrough

Adds stricter authData validation across login, challenge, and database update flows and new vulnerability tests. Auth provider list is filtered to require valid validators/adapters; PUT/PATCH rejects any field beginning with authData; challenge requests reject null/non-object provider values.

Changes

Cohort / File(s)	Summary
Vulnerability Tests spec/vulnerabilities.spec.js	Added tests for GHSA-g55g-hrp7-h6vq (dotted authData provider injection and corrupted provider handling) and GHSA-2c6m-7356-pw67 (null/non-object provider values in challenge requests).
Auth Flow Validation src/Auth.js	Filter saved user providers by validating each provider via config.authDataManager.getValidatorForProvider() and requiring a valid adapter; excludes providers lacking adapters before policy checks.
Database Update Validation src/Controllers/DatabaseController.js	Reject any update field whose name starts with authData (broader than previous authData.<provider>.id check), returning INVALID_KEY_NAME for such fields.
Challenge Request Validation src/Routers/UsersRouter.js	Enforce that all authData values are non-null objects and safely guard access to provider id; ensure at most one provider contains an id, rejecting malformed shapes early in challenge flow.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Client as Client
participant Router as UsersRouter
participant Auth as Auth
participant DB as DatabaseController

Client->>Router: POST /requestPasswordReset (authData or challenge)
Router->>Router: Validate authData keys are objects (non-null)
alt invalid authData
    Router-->>Client: 400 error (invalid authData shape)
else valid authData
    Router->>Auth: extract providers (guarded id access)
    Auth->>Auth: filter savedUserProviders via getValidatorForProvider + adapter presence
    alt no valid provider / multiple ids
        Auth-->>Client: 400 error (invalid provider selection)
    else proceed
        Auth->>DB: read/update user (reject fields starting with authData on updates)
        DB-->>Auth: success / error
        Auth-->>Client: success / appropriate error

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix: Account takeover via operator injection in authentication data identifier (GHSA-5fw2-8jcv-xh87) #10185: Related changes to Auth.js and added tests for malformed authData/provider entries.
• fix: Empty authData bypasses credential requirement on signup (GHSA-wjqw-r9x4-j59v) #10219: Similar enhancements to authData validation and handling of null/non-object provider values in challenge flow.
• SQL injection via query field name when using PostgreSQL (GHSA-c442-97qw-j6c6) #10177: Related updates to DatabaseController key-name validation logic for internal fields like authData.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description is empty; the required template sections (Issue, Approach, Tasks) are not filled out at all.	Add a comprehensive description including the issue being fixed, the approach taken to validate authData in the three affected areas, and mark which tasks were completed (tests were added).

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes across all modified files: validating authData input in dot-notation updates, login provider checks, and challenge endpoint validation.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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.}

[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]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Controllers/DatabaseController.js (1)

606-611: ⚠️ Potential issue | 🟠 Major

Scope authData.* dot-notation blocking to _User only.

At Line 606, this blocks authData.<...> updates for every class, not just _User. That can break legitimate custom-class object fields named authData.

Suggested fix

-                if (fieldName.match(/^authData\./)) {
+                if (className === '_User' && fieldName.match(/^authData\./)) {
                   throw new Parse.Error(
                     Parse.Error.INVALID_KEY_NAME,
                     `Invalid field name for update: ${fieldName}`
                   );
                 }

🤖 Prompt for AI Agents

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

In `@src/Controllers/DatabaseController.js` around lines 606 - 611, The current
check unconditionally rejects field names matching /^authData\./, which blocks
legitimate fields on non-_User classes; update the logic in the function
handling updates (the block that reads `if (fieldName.match(/^authData\./)) {
... }`) to only throw the Parse.Error when the target class is '_User' (e.g.,
check the request or params className variable equals '_User' before rejecting),
leaving `authData.*` allowed for custom classes.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@spec/vulnerabilities.spec.js`:
- Around line 3064-3079: The test "rejects challenge request with null provider
value without 500" currently asserts expect(res.status).toBeLessThan(500), which
would also pass for 200; update the assertion to ensure a rejection client error
(e.g., assert res.status is >= 400 and < 500) so it fails on success responses,
and apply the same change to the other similar test around lines 3081-3096;
locate the tests by their titles/it descriptions and replace the weak status
check with a strict client-error range (or explicit not-200) assertion.

In `@src/Auth.js`:
- Around line 526-534: savedUserProviders can include entries whose
validator.adapter is undefined, causing a crash when reading
provider.adapter.policy; update the pipeline that builds savedUserProviders (the
map using config.authDataManager.getValidatorForProvider and the subsequent
.filter(Boolean)) to explicitly exclude validators with no adapter (i.e., only
return { name: provider, adapter: validator.adapter } when validator &&
validator.adapter) or add a second filter that checks entry.adapter is truthy so
that any providers without a defined adapter are removed before policy access.

---

Outside diff comments:
In `@src/Controllers/DatabaseController.js`:
- Around line 606-611: The current check unconditionally rejects field names
matching /^authData\./, which blocks legitimate fields on non-_User classes;
update the logic in the function handling updates (the block that reads `if
(fieldName.match(/^authData\./)) { ... }`) to only throw the Parse.Error when
the target class is '_User' (e.g., check the request or params className
variable equals '_User' before rejecting), leaving `authData.*` allowed for
custom classes.

🪄 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: 3ea0cfe9-e0bd-4ef8-a65e-7a118690f014

📥 Commits

Reviewing files that changed from the base of the PR and between 9667fd7 and 0c7290c.

📒 Files selected for processing (4)

• spec/vulnerabilities.spec.js
• src/Auth.js
• src/Controllers/DatabaseController.js
• src/Routers/UsersRouter.js

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.59%. Comparing base (5dcbf41) to head (f17555a).
⚠️ Report is 1 commits behind head on alpha.

Additional details and impacted files

@@            Coverage Diff             @@
##            alpha   #10221      +/-   ##
==========================================
+ Coverage   92.57%   92.59%   +0.01%     
==========================================
  Files         192      192              
  Lines       16300    16307       +7     
  Branches      199      199              
==========================================
+ Hits        15090    15099       +9     
+ Misses       1193     1191       -2     
  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.