fix: JWT audience validation bypass in Google, Apple, and Facebook authentication adapters (GHSA-x6fw-778m-wr9v)#10113
Conversation
|
I will reformat the title to use the proper commit message syntax. |
|
🚀 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. |
✅ 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. |
📝 WalkthroughWalkthroughAdds configuration prechecks to auth adapters: Google and Apple now throw when Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Adapter as Auth Adapter
participant JWKS as JWKS/Key Resolver
participant Validator as JWT Validator
Client->>Adapter: call verifyIdToken(token)
Adapter->>Adapter: ensure clientId/appIds configured
alt missing config
Adapter-->>Client: throw OBJECT_NOT_FOUND ("... auth is not configured.")
else config present
Adapter->>Validator: validate token signature/claims
Validator->>JWKS: fetch key by kid (if needed)
JWKS-->>Validator: return public key
Validator-->>Adapter: return validated claims
Adapter->>Adapter: verify audience against clientId/appIds (if applicable)
Adapter-->>Client: return verified claims
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![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/AuthenticationAdapters.spec.js (1)
650-657: PR Title SuggestionBased on learnings, the PR title should follow Angular commit convention and be meaningful for the changelog. Consider:
fix(auth): require clientId configuration in Google, Apple, and Facebook auth adaptersThis clearly conveys the security improvement to developers reading the changelog.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/AuthenticationAdapters.spec.js` around lines 650 - 657, Update the PR title to follow the Angular commit convention and make the change clear in the changelog by setting it to: "fix(auth): require clientId configuration in Google, Apple, and Facebook auth adapters" so reviewers and release tooling correctly pick up this security-related fix affecting the Google/Apple/Facebook auth adapters (referenced in the test "should throw error when clientId is not configured" and the auth adapter validation logic).
🤖 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/AuthenticationAdapters.spec.js`:
- Around line 650-657: Update the PR title to follow the Angular commit
convention and make the change clear in the changelog by setting it to:
"fix(auth): require clientId configuration in Google, Apple, and Facebook auth
adapters" so reviewers and release tooling correctly pick up this
security-related fix affecting the Google/Apple/Facebook auth adapters
(referenced in the test "should throw error when clientId is not configured" and
the auth adapter validation logic).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c3bf1c53-e4f4-4116-8431-5809d904df98
📒 Files selected for processing (4)
spec/AuthenticationAdapters.spec.jssrc/Adapters/Auth/apple.jssrc/Adapters/Auth/facebook.jssrc/Adapters/Auth/google.js
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/index.spec.js (1)
662-689:⚠️ Potential issue | 🟡 MinorConvert to
async/awaitto prevent unhandled rejections from being swallowed.The
user.linkWith()promise at line 682 is not returned to the outer chain, so if it rejects, the error bypasses.catch(done.fail)and the test hangs. Converting toasync/awaitfixes this and keeps the test consistent with the repo's preferred style.Proposed fix
- it('should not fail when Google signin is introduced with clientId', done => { + it('should not fail when Google signin is introduced with clientId', async () => { const jwt = require('jsonwebtoken'); const authUtils = require('../lib/Adapters/Auth/utils'); - reconfigureServer({ + await reconfigureServer({ auth: { google: { clientId: 'secret' } }, - }) - .then(() => { - const fakeClaim = { - iss: 'https://accounts.google.com', - aud: 'secret', - exp: Date.now(), - sub: 'the_user_id', - }; - const fakeDecodedToken = { kid: '123', alg: 'RS256' }; - const fakeSigningKey = { kid: '123', rsaPublicKey: 'the_rsa_public_key' }; - spyOn(authUtils, 'getHeaderFromToken').and.callFake(() => fakeDecodedToken); - spyOn(authUtils, 'getSigningKey').and.resolveTo(fakeSigningKey); - spyOn(jwt, 'verify').and.callFake(() => fakeClaim); - const user = new Parse.User(); - user - .linkWith('google', { - authData: { id: 'the_user_id', id_token: 'the_token' }, - }) - .then(done); - }) - .catch(done.fail); + }); + + const fakeClaim = { + iss: 'https://accounts.google.com', + aud: 'secret', + exp: Date.now(), + sub: 'the_user_id', + }; + const fakeDecodedToken = { kid: '123', alg: 'RS256' }; + const fakeSigningKey = { kid: '123', rsaPublicKey: 'the_rsa_public_key' }; + spyOn(authUtils, 'getHeaderFromToken').and.callFake(() => fakeDecodedToken); + spyOn(authUtils, 'getSigningKey').and.resolveTo(fakeSigningKey); + spyOn(jwt, 'verify').and.callFake(() => fakeClaim); + + const user = new Parse.User(); + await expectAsync( + user.linkWith('google', { + authData: { id: 'the_user_id', id_token: 'the_token' }, + }) + ).toBeResolved(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/index.spec.js` around lines 662 - 689, The test registers asynchronous operations but doesn't return the promise from user.linkWith, causing unhandled rejections; convert the test callback to an async function, await reconfigureServer(...) and then await user.linkWith('google', { authData: ... }) instead of using done, remove the done/done.fail usage, and let thrown errors propagate so the test framework fails the test; keep the same stubs/spies on authUtils.getHeaderFromToken, authUtils.getSigningKey, and jwt.verify.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@spec/index.spec.js`:
- Around line 662-689: The test registers asynchronous operations but doesn't
return the promise from user.linkWith, causing unhandled rejections; convert the
test callback to an async function, await reconfigureServer(...) and then await
user.linkWith('google', { authData: ... }) instead of using done, remove the
done/done.fail usage, and let thrown errors propagate so the test framework
fails the test; keep the same stubs/spies on authUtils.getHeaderFromToken,
authUtils.getSigningKey, and jwt.verify.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 74c6a7de-8104-44d5-ba58-494514b8c70a
📒 Files selected for processing (2)
spec/index.spec.jssrc/Adapters/Auth/google.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Adapters/Auth/google.js
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10113 +/- ##
=======================================
Coverage 92.68% 92.69%
=======================================
Files 191 191
Lines 15879 15883 +4
Branches 180 180
=======================================
+ Hits 14717 14722 +5
+ Misses 1150 1149 -1
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/AuthenticationAdapters.spec.js (1)
1258-1265:⚠️ Potential issue | 🟡 MinorTest description doesn't match assertion.
The test description says "should throw error with missing id_token" but the test actually validates configuration error (empty options
{}). The expected error message "Facebook auth is not configured." confirms this is testing configuration, not missing token.Consider updating the description to match the actual test intent, similar to the enabled test at line 1477.
📝 Proposed fix
// TODO: figure out a way to run this test alongside facebook classic tests - xit('should throw error with missing id_token', async () => { + xit('should throw error when appIds is not configured for Limited Login', async () => { try { await facebook.validateAuthData({}, { appIds: ['secret'] });Note: This test passes
appIds: ['secret']but expects "Facebook auth is not configured." error. If the intent is to test missing token, the expected error should be "id token is invalid for this user." If the intent is to test missing configuration, pass{}as options.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/AuthenticationAdapters.spec.js` around lines 1258 - 1265, The test's description string "should throw error with missing id_token" doesn't match its behavior: it calls facebook.validateAuthData({}, { appIds: ['secret'] }) and expects a configuration error; either update the test title to reflect a configuration error (e.g., "should throw error when Facebook auth is not configured") or change the inputs/expectation to test a missing id_token (pass an options object with appIds and assert the "id token is invalid for this user." message); locate the test by its description and the call to facebook.validateAuthData to apply the fix.
🧹 Nitpick comments (1)
src/Adapters/Auth/apple.js (1)
76-81: Semantic mismatch:OBJECT_NOT_FOUNDfor configuration error.The error code
OBJECT_NOT_FOUNDtypically indicates a missing resource/object, not a misconfiguration. Consider using a more semantically appropriate error code likeParse.Error.INTERNAL_SERVER_ERRORorParse.Error.OTHER_CAUSEto better reflect that this is a server-side configuration issue rather than a client-side data lookup failure.This applies to all adapters in this PR (Apple, Google, Facebook).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Adapters/Auth/apple.js` around lines 76 - 81, Replace the semantically incorrect Parse.Error.OBJECT_NOT_FOUND thrown when clientId is missing in the Apple adapter (file: src/Adapters/Auth/apple.js, locate the clientId check) with a server-side configuration error code such as Parse.Error.INTERNAL_SERVER_ERROR or Parse.Error.OTHER_CAUSE; update the throw to use the chosen Parse.Error constant and keep the existing descriptive message ('Apple auth is not configured.'), and apply the same change pattern to the corresponding checks in the Google and Facebook adapter files referenced in this PR.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@spec/AuthenticationAdapters.spec.js`:
- Around line 1258-1265: The test's description string "should throw error with
missing id_token" doesn't match its behavior: it calls
facebook.validateAuthData({}, { appIds: ['secret'] }) and expects a
configuration error; either update the test title to reflect a configuration
error (e.g., "should throw error when Facebook auth is not configured") or
change the inputs/expectation to test a missing id_token (pass an options object
with appIds and assert the "id token is invalid for this user." message); locate
the test by its description and the call to facebook.validateAuthData to apply
the fix.
---
Nitpick comments:
In `@src/Adapters/Auth/apple.js`:
- Around line 76-81: Replace the semantically incorrect
Parse.Error.OBJECT_NOT_FOUND thrown when clientId is missing in the Apple
adapter (file: src/Adapters/Auth/apple.js, locate the clientId check) with a
server-side configuration error code such as Parse.Error.INTERNAL_SERVER_ERROR
or Parse.Error.OTHER_CAUSE; update the throw to use the chosen Parse.Error
constant and keep the existing descriptive message ('Apple auth is not
configured.'), and apply the same change pattern to the corresponding checks in
the Google and Facebook adapter files referenced in this PR.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b7a2dd9f-f6c9-48e7-87b4-4d1a39b6c979
📒 Files selected for processing (4)
spec/AuthenticationAdapters.spec.jssrc/Adapters/Auth/apple.jssrc/Adapters/Auth/facebook.jssrc/Adapters/Auth/google.js
mtrezza
changed the title
# [9.5.0-alpha.11](9.5.0-alpha.10...9.5.0-alpha.11) (2026-03-06) ### Bug Fixes * JWT audience validation bypass in Google, Apple, and Facebook authentication adapters ([GHSA-x6fw-778m-wr9v](GHSA-x6fw-778m-wr9v)) ([#10113](#10113)) ([9f8d3f3](9f8d3f3))
|
🎉 This change has been released in version 9.5.0-alpha.11 |
# [9.5.0](9.4.1...9.5.0) (2026-03-07) ### Bug Fixes * `PagesRouter` path traversal allows reading files outside configured pages directory ([GHSA-hm3f-q6rw-m6wh](GHSA-hm3f-q6rw-m6wh)) ([#10104](#10104)) ([e772543](e772543)) * Endpoint `/loginAs` allows `readOnlyMasterKey` to gain full read and write access as any user ([GHSA-79wj-8rqv-jvp5](GHSA-79wj-8rqv-jvp5)) ([#10098](#10098)) ([bc20945](bc20945)) * File creation and deletion bypasses `readOnlyMasterKey` write restriction ([GHSA-xfh7-phr7-gr2x](GHSA-xfh7-phr7-gr2x)) ([#10095](#10095)) ([036365a](036365a)) * File metadata endpoint bypasses `beforeFind` / `afterFind` trigger authorization ([GHSA-hwx8-q9cg-mqmc](GHSA-hwx8-q9cg-mqmc)) ([#10106](#10106)) ([72e7707](72e7707)) * GraphQL `__type` introspection bypass via inline fragments when public introspection is disabled ([GHSA-q5q9-2rhp-33qw](GHSA-q5q9-2rhp-33qw)) ([#10111](#10111)) ([61261a5](61261a5)) * JWT audience validation bypass in Google, Apple, and Facebook authentication adapters ([GHSA-x6fw-778m-wr9v](GHSA-x6fw-778m-wr9v)) ([#10113](#10113)) ([9f8d3f3](9f8d3f3)) * Malformed `$regex` query leaks database error details in API response ([GHSA-9cp7-3q5w-j92g](GHSA-9cp7-3q5w-j92g)) ([#10101](#10101)) ([9792d24](9792d24)) * Regular Expression Denial of Service (ReDoS) via `$regex` query in LiveQuery ([GHSA-mf3j-86qx-cq5j](https://github.com/parse-community/parse-server/security/advisories/GHSA-mf3j-86qx-cq5j)) ([#10118](#10118)) ([5e113c2](5e113c2)) ### Features * Add `Parse.File` option `maxUploadSize` to override the Parse Server option `maxUploadSize` per file upload ([#10093](#10093)) ([3d8807b](3d8807b)) * Add security check for server option `mountPlayground` for GraphQL development ([#10103](#10103)) ([2ae5db1](2ae5db1)) * Add server option `readOnlyMasterKeyIps` to restrict `readOnlyMasterKey` by IP ([#10115](#10115)) ([cbff6b4](cbff6b4)) * Add support for `Parse.File.setDirectory`, `setMetadata`, `setTags` with stream-based file upload ([#10092](#10092)) ([ca666b0](ca666b0)) * Allow to identify `readOnlyMasterKey` invocation of Cloud Function via `request.isReadOnly` ([#10100](#10100)) ([2c48751](2c48751)) * Deprecate GraphQL Playground that exposes master key in HTTP response ([#10112](#10112)) ([d54d800](d54d800))
|
🎉 This change has been released in version 9.5.0 |
2 participants
Pull Request
Issue
JWT audience validation bypass in Google, Apple, and Facebook authentication adapters (GHSA-x6fw-778m-wr9v)
Tasks
Summary by CodeRabbit
Bug Fixes
Tests