fix: OAuth2 adapter app ID validation sends wrong token to introspection endpoint (GHSA-69xg-f649-w5g2)

[mtrezza]

Pull Request

• Report security issues confidentially.
• Any contribution is under this license.

Issue

OAuth2 adapter app ID validation sends wrong token to introspection endpoint (GHSA-69xg-f649-w5g2)

Tasks

• Add tests
• Add changes to documentation (guides, repository pages, code comments)
• Add security check
• Add new Parse Error codes to Parse JS SDK

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

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]

📝 Walkthrough

Walkthrough

Adds a test verifying the real access token is sent to the introspection endpoint during app ID validation. Changes the OAuth2Adapter.validateAppId signature from (authData) to (appIds, authData); the new parameter is added but not used in the function body.

Changes

Cohort / File(s)	Summary
Test Addition spec/Adapters/Auth/oauth2.spec.js	Adds a test that overrides global.fetch to intercept introspection requests, extracts the token from the request body, asserts the captured token equals the provided authData.access_token, and checks that multiple introspection requests occurred. Restores original fetch.
Method Signature Update src/Adapters/Auth/oauth2.js	Changes OAuth2Adapter.validateAppId signature from validateAppId(authData) to validateAppId(appIds, authData); implementation remains unchanged and the new appIds parameter is currently unused.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is largely incomplete. While it references the security issue and marks tests as added, the Approach section is missing entirely, and most task items remain unchecked despite code changes being present.	Complete the Approach section to describe the changes made, and verify/update the Tasks checklist to reflect actual work completed (documentation, security checks, etc.).

✅ 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 security issue being fixed (GHSA-69xg-f649-w5g2) and the main problem (OAuth2 adapter sends wrong token to introspection endpoint).

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

---

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)

src/Adapters/Auth/oauth2.js (1)

75-85: Honor the new appIds argument.

This method now accepts appIds, but the actual validation still reads from this.appIds. That leaves the new contract partially ignored and can silently bypass any per-call app ID list passed by the caller.

♻️ Proposed tweak

   async validateAppId(appIds, authData) {
     if (!this.appidField) {
       return;
     }
 
     const response = await this.requestTokenInfo(authData.access_token);
+    const allowedAppIds = appIds ?? this.appIds;
 
     const appIdFieldValue = response[this.appidField];
     const isValidAppId = Array.isArray(appIdFieldValue)
-      ? appIdFieldValue.some(appId => this.appIds.includes(appId))
-      : this.appIds.includes(appIdFieldValue);
+      ? appIdFieldValue.some(appId => allowedAppIds.includes(appId))
+      : allowedAppIds.includes(appIdFieldValue);

🤖 Prompt for AI Agents

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

In `@src/Adapters/Auth/oauth2.js` around lines 75 - 85, In validateAppId, you're
ignoring the new appIds parameter and always using this.appIds; change the logic
to honor the per-call appIds by deriving a local allowedAppIds = appIds ||
this.appIds and then use allowedAppIds for the membership checks (i.e., when
appIdFieldValue is an array use appIdFieldValue.some(appId =>
allowedAppIds.includes(appId)), otherwise
allowedAppIds.includes(appIdFieldValue)); keep the existing behavior when appIds
is not provided by falling back to this.appIds.

🤖 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/Adapters/Auth/oauth2.spec.js`:
- Around line 342-369: The test overrides global.fetch (saved to originalFetch)
but never restores it, making other specs order-dependent; update the test
around Parse.User.logInWith and capturedTokens to restore global.fetch =
originalFetch in a finally block (or use an afterEach hook) so the originalFetch
is reassigned regardless of test success/failure, ensuring the mocked fetch only
lives for the duration of this test.

---

Nitpick comments:
In `@src/Adapters/Auth/oauth2.js`:
- Around line 75-85: In validateAppId, you're ignoring the new appIds parameter
and always using this.appIds; change the logic to honor the per-call appIds by
deriving a local allowedAppIds = appIds || this.appIds and then use
allowedAppIds for the membership checks (i.e., when appIdFieldValue is an array
use appIdFieldValue.some(appId => allowedAppIds.includes(appId)), otherwise
allowedAppIds.includes(appIdFieldValue)); keep the existing behavior when appIds
is not provided by falling back to this.appIds.

🪄 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: 09021cb5-3a10-4bc0-94b1-822d4eb07aac

📥 Commits

Reviewing files that changed from the base of the PR and between 19a3f05 and 3230e5c.

📒 Files selected for processing (2)

• spec/Adapters/Auth/oauth2.spec.js
• src/Adapters/Auth/oauth2.js

[parseplatformorg]

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

Status	Scanner	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.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.61%. Comparing base (0d0a554) to head (fa17c18).
⚠️ Report is 100 commits behind head on alpha.

Additional details and impacted files

@@           Coverage Diff           @@
##            alpha   #10187   +/-   ##
=======================================
  Coverage   92.61%   92.61%           
=======================================
  Files         192      192           
  Lines       16258    16258           
  Branches      190      190           
=======================================
  Hits        15057    15057           
  Misses       1188     1188           
  Partials       13       13           

☔ 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.13

[parseplatformorg]

🎉 This change has been released in version 9.6.0