fix: JWT Algorithm Confusion in Google Auth Adapter (GHSA-4q3h-vp4r-prv2)#10073
Conversation
|
I will reformat the title to use the proper commit message syntax. |
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughThe changes implement a security fix for JWT algorithm confusion vulnerabilities across authentication adapters. The update hardcodes the RS256 algorithm during JWT verification instead of trusting the token header, replaces manual JWKS handling with the dedicated jwks-rsa library in the Google adapter, and enhances error handling for invalid signing keys. Tests are updated to verify the new verification flow and security enforcement. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Adapter as Auth Adapter
participant JWKSClient as JWKS Client
participant JWTLib as JWT Library
Client->>Adapter: verifyIdToken(token, options)
Adapter->>Adapter: getHeaderFromToken(token)
Adapter->>Adapter: Extract keyId from header
Adapter->>JWKSClient: getSigningKey(keyId)
JWKSClient->>JWKSClient: Check cache
alt Key in cache
JWKSClient-->>JWKSClient: Return cached key
else Key not in cache
JWKSClient->>JWKSClient: Fetch from JWKS endpoint
JWKSClient->>JWKSClient: Cache result
end
JWKSClient-->>Adapter: Return publicKey
Adapter->>JWTLib: jwt.verify(token, publicKey, {algorithms: ['RS256']})
JWTLib->>JWTLib: Verify signature with RS256 only
alt Valid signature
JWTLib-->>Adapter: Decoded payload
Adapter-->>Client: Verified token data
else Invalid signature or wrong algorithm
JWTLib-->>Adapter: Verification error
Adapter-->>Client: Error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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 |
✅ 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. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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)
972-976: Pre-existing mock inconsistency:getHeaderFromTokenreturns nested object, causingkeyIdto beundefined.In this test (and several other unchanged Apple/Facebook tests),
getHeaderFromTokenis mocked to return the fullfakeDecodedTokenobject (with a nested.headerproperty) rather thanfakeDecodedToken.header. Since the adapter destructures{ kid: keyId }from the return value,keyIdbecomesundefined. The test still passes becausegetSigningKeyis broadly mocked andjwt.verifyfails early on the malformed token—but the mock doesn't accurately reflect the real call.This is pre-existing and not introduced by this PR, but while you're touching adjacent lines it would be a good cleanup to align these mocks with the flat-object pattern used in the newly added tests (e.g., lines 959, 529, 1318).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/AuthenticationAdapters.spec.js` around lines 972 - 976, The mocks for getHeaderFromToken return a nested object which makes the adapter's destructuring ({ kid: keyId }) yield undefined; update the tests (including this one and other Apple/Facebook tests) to have spyOn(authUtils, 'getHeaderFromToken') return the flat header object (i.e., fakeDecodedToken.header) instead of the whole fakeDecodedToken so keyId is set correctly, and ensure getSigningKey remains mocked via authUtils.getSigningKey to resolve to fakeSigningKey to match the real call shape.
🤖 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 972-976: The mocks for getHeaderFromToken return a nested object
which makes the adapter's destructuring ({ kid: keyId }) yield undefined; update
the tests (including this one and other Apple/Facebook tests) to have
spyOn(authUtils, 'getHeaderFromToken') return the flat header object (i.e.,
fakeDecodedToken.header) instead of the whole fakeDecodedToken so keyId is set
correctly, and ensure getSigningKey remains mocked via authUtils.getSigningKey
to resolve to fakeSigningKey to match the real call shape.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 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
mtrezza
merged commit 9b94083
into
parse-community:release-8.x.x
## [8.6.3](8.6.2...8.6.3) (2026-02-23) ### Bug Fixes * JWT Algorithm Confusion in Google Auth Adapter ([GHSA-4q3h-vp4r-prv2](https://github.com/parse-community/parse-server/security/advisories/GHSA-4q3h-vp4r-prv2)) ([#10073](#10073)) ([9b94083](9b94083))
|
🎉 This change has been released in version 8.6.3 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-8.x.x #10073 +/- ##
=================================================
+ Coverage 92.58% 92.60% +0.02%
=================================================
Files 191 191
Lines 15543 15509 -34
Branches 177 177
=================================================
- Hits 14390 14362 -28
+ Misses 1141 1135 -6
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Pull Request
Issue
JWT Algorithm Confusion in Google Auth Adapter (GHSA-4q3h-vp4r-prv2).
Tasks
Summary by CodeRabbit