Repository navigation
fix(utils): replace jwks-rsa with Web Crypto JWKS parsing - #3311
Open
lahirumaramba wants to merge 2 commits into
Open
lahirumaramba wants to merge 2 commits into
lahirumaramba wants to merge 2 commits into
Conversation
In v14.0.0, upgrading `jwks-rsa` to v4 introduced a synchronous
`require('jose')` call against ESM-only `jose` v6. In Node `<22.12.0`
and on serverless runtimes like Vercel that disable `require(esm)` by
default, importing `firebase-admin/app-check`, `firebase-admin/auth`,
or `firebase-admin/phone-number-verification` crashes at startup with
`ERR_REQUIRE_ESM`.
Replace `jwks-rsa` in `JwksFetcher` with our existing `HttpClient` and
standard `crypto.subtle` JWK-to-SPKI conversion for RSA and EC signing
keys, and remove `jwks-rsa` from `package.json`.
Fixes #3181
Contributor
There was a problem hiding this comment.
Code Review
This pull request removes the jwks-rsa dependency and replaces it with a custom implementation for fetching and parsing JSON Web Key Sets (JWKS) using the Web Crypto API in src/utils/jwt.ts. It also updates the corresponding unit tests to cover the new logic. The review feedback suggests two key improvements: first, using Node.js's native Buffer to convert the public key SPKI to base64 more efficiently and idiomatically; second, adding a safety check to ensure resp.data is a non-null object before accessing its properties to prevent a potential runtime crash.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3181
In
v14.0.0, upgradingjwks-rsato v4 introduced a synchronous CommonJSrequire('jose')call againstjosev6, which only ships ES modules. On Node<22.12.0, and on serverless platforms like Vercel that start Node 22 and Node 24 with--no-experimental-require-moduleby default, importingfirebase-admin/app-check,firebase-admin/phone-number-verification, or evenfirebase-admin/authcrashes immediately on startup withERR_REQUIRE_ESM.This PR removes
jwks-rsacompletely and fetches/converts JWKS public signing keys directly using our existingHttpClientand standard Web Crypto (globalThis.crypto.subtle). That preserves the 6-hour key cache andAppOptions.httpAgentsupport, eliminates the CJS/ESM boundary failure across all Node 22+ and serverless runtimes, and dropsjwks-rsa,jose, and their transitive runtime dependencies.Technical Details
JwksFetcherinsrc/utils/jwt.tsfetches the JWKS JSON payload viaHttpClient(forwardinghttpAgentidentically toUrlKeyFetcher) and converts public signing JWKs into SPKI PEM strings viacrypto.subtle.importKey('jwk', ...)andcrypto.subtle.exportKey('spki', ...).RS256,RS384,RS512,PS256,PS384,PS512) and EC (P-256,P-384,P-521) signing keys.n/eandx/y) and skips non-signing (use !== 'sig') or private JWKs (d,p,q,dp,dq,qi,oth) prior to import so malformed JWKs cannot produce degenerate public keys.jwt,app-check,auth, andphone-number-verification, as well as loading undernode --no-experimental-require-moduleand Nodev22.0.0in both CommonJS and ESM.