chore(backend): Fix JWKS cache (#3321)

* chore(backend): Fix JWKS cache

* chore(backend): Finish implementation

* chore(backend): Fix keys tests

* fix(backend): Add test to assert the cache doesn't get wiped out

* fix(backend): Simplify test

* chore(backend): Remove unnecessary comment

* chore(backend): Add @deprecated comment

* chore(backend): Remove unnecessary error

* fix(backend): Ensure fetch if kid isn't found in cache

* chore(backend): Re-add exported errors to avoid a breaking change

* Update integration/tests/handshake.test.ts

.changeset/chatty-cooks-notice.md

@@ -0,0 +1,5 @@
+---
+'@clerk/backend': patch
+---
+
+Fix bug in JWKS cache logic that caused a race condition resulting in no JWK being available.

packages/backend/src/errors.ts

@@ -20,6 +20,7 @@ export const TokenVerificationErrorReason = {
   RemoteJWKInvalid: 'jwk-remote-invalid',
   RemoteJWKMissing: 'jwk-remote-missing',
   JWKFailedToResolve: 'jwk-failed-to-resolve',
+  JWKKidMismatch: 'jwk-kid-mismatch',
 };
 
 export type TokenVerificationErrorReason =

packages/backend/src/tokens/__tests__/keys.test.ts

@@ -172,8 +172,9 @@ export default (QUnit: QUnit) => {
       } catch (err) {
         if (err instanceof Error) {
           assert.propEqual(err, {
-            reason: 'jwk-remote-missing',
-            action: 'Contact support@clerk.com',
+            reason: 'jwk-kid-mismatch',
+            action:
+              'Go to your Dashboard and validate your secret and public keys are correct. Contact support@clerk.com if the issue persists.',
           });
           assert.propContains(err, {
             message: `Unable to find a signing key in JWKS that matches the kid='${kid}' of the provided session token. Please make sure that the __session cookie or the HTTP authorization header contain a Clerk-generated session JWT. The following kid is available: ${mockRsaJwkKid}, local`,
@@ -184,5 +185,38 @@ export default (QUnit: QUnit) => {
         }
       }
     });
+
+    test('cache TTLs do not conflict', async assert => {
+      fakeClock.runAll();
+
+      fakeFetch.onCall(0).returns(jsonOk(mockJwks));
+      let jwk = await loadClerkJWKFromRemote({
+        secretKey: 'deadbeef',
+        kid: mockRsaJwkKid,
+        skipJwksCache: true,
+      });
+      assert.propEqual(jwk, mockRsaJwk);
+
+      // just less than an hour, the cache TTL
+      fakeClock.tick(60 * 60 * 1000 - 5);
+
+      // re-fetch, 5m cache is expired
+      fakeFetch.onCall(1).returns(jsonOk(mockJwks));
+      jwk = await loadClerkJWKFromRemote({
+        secretKey: 'deadbeef',
+        kid: mockRsaJwkKid,
+      });
+      assert.propEqual(jwk, mockRsaJwk);
+
+      // cache should be cleared, but 5m ttl is still valid
+      fakeClock.next();
+
+      // re-fetch, 5m cache is expired
+      jwk = await loadClerkJWKFromRemote({
+        secretKey: 'deadbeef',
+        kid: mockRsaJwkKid,
+      });
+      assert.propEqual(jwk, mockRsaJwk);
+    });
   });
 };

packages/backend/src/tokens/keys.ts

@@ -1,4 +1,4 @@
-import { API_URL, API_VERSION, JWKS_CACHE_TTL_MS, MAX_CACHE_LAST_UPDATED_AT_SECONDS } from '../constants';
+import { API_URL, API_VERSION, MAX_CACHE_LAST_UPDATED_AT_SECONDS } from '../constants';
 import {
   TokenVerificationError,
   TokenVerificationErrorAction,
@@ -26,19 +26,9 @@ function getCacheValues() {
   return Object.values(cache);
 }
 
-function setInCache(jwk: JsonWebKeyWithKid, jwksCacheTtlInMs: number) {
+function setInCache(jwk: JsonWebKeyWithKid, shouldExpire = true) {
   cache[jwk.kid] = jwk;
-  lastUpdatedAt = Date.now();
-
-  if (jwksCacheTtlInMs >= 0) {
-    setTimeout(() => {
-      if (jwk) {
-        delete cache[jwk.kid];
-      } else {
-        cache = {};
-      }
-    }, jwksCacheTtlInMs);
-  }
+  lastUpdatedAt = shouldExpire ? Date.now() : -1;
 }
 
 const LocalJwkKid = 'local';
@@ -83,7 +73,7 @@ export function loadClerkJWKFromLocal(localKey?: string): JsonWebKey {
         n: modulus,
         e: 'AQAB',
       },
-      -1, // local key never expires in cache
+      false, // local key never expires in cache
     );
   }
 
@@ -92,6 +82,9 @@ export function loadClerkJWKFromLocal(localKey?: string): JsonWebKey {
 
 export type LoadClerkJWKFromRemoteOptions = {
   kid: string;
+  /**
+   * @deprecated This cache TTL is deprecated and will be removed in the next major version. Specifying a cache TTL is now a no-op.
+   */
   jwksCacheTtlInMs?: number;
   skipJwksCache?: boolean;
   secretKey?: string;
@@ -108,19 +101,16 @@ export type LoadClerkJWKFromRemoteOptions = {
  * @param {Object} options
  * @param {string} options.kid - The id of the key that the JWT was signed with
  * @param {string} options.alg - The algorithm of the JWT
- * @param {number} options.jwksCacheTtlInMs - The TTL of the jwks cache (defaults to 1 hour)
  * @returns {JsonWebKey} key
  */
 export async function loadClerkJWKFromRemote({
   secretKey,
   apiUrl = API_URL,
   apiVersion = API_VERSION,
   kid,
-  jwksCacheTtlInMs = JWKS_CACHE_TTL_MS,
   skipJwksCache,
 }: LoadClerkJWKFromRemoteOptions): Promise<JsonWebKey> {
-  const needsFetch = !getFromCache(kid) || cacheHasExpired();
-  if (skipJwksCache || needsFetch) {
+  if (skipJwksCache || cacheHasExpired() || !getFromCache(kid)) {
     if (!secretKey) {
       throw new TokenVerificationError({
         action: TokenVerificationErrorAction.ContactSupport,
@@ -139,7 +129,7 @@ export async function loadClerkJWKFromRemote({
       });
     }
 
-    keys.forEach(key => setInCache(key, jwksCacheTtlInMs));
+    keys.forEach(key => setInCache(key));
   }
 
   const jwk = getFromCache(kid);
@@ -152,11 +142,9 @@ export async function loadClerkJWKFromRemote({
       .join(', ');
 
     throw new TokenVerificationError({
-      action: TokenVerificationErrorAction.ContactSupport,
-      message: `Unable to find a signing key in JWKS that matches the kid='${kid}' of the provided session token. Please make sure that the __session cookie or the HTTP authorization header contain a Clerk-generated session JWT.${
-        jwkKeys ? ` The following kid is available: ${jwkKeys}` : ''
-      }`,
-      reason: TokenVerificationErrorReason.RemoteJWKMissing,
+      action: `Go to your Dashboard and validate your secret and public keys are correct. ${TokenVerificationErrorAction.ContactSupport} if the issue persists.`,
+      message: `Unable to find a signing key in JWKS that matches the kid='${kid}' of the provided session token. Please make sure that the __session cookie or the HTTP authorization header contain a Clerk-generated session JWT. The following kid is available: ${jwkKeys}`,
+      reason: TokenVerificationErrorReason.JWKKidMismatch,
     });
   }
 
@@ -208,7 +196,19 @@ async function fetchJWKSFromBAPI(apiUrl: string, key: string, apiVersion: string
 }
 
 function cacheHasExpired() {
-  return Date.now() - lastUpdatedAt >= MAX_CACHE_LAST_UPDATED_AT_SECONDS * 1000;
+  // If lastUpdatedAt is -1, it means that we're using a local JWKS and it never expires
+  if (lastUpdatedAt === -1) {
+    return false;
+  }
+
+  // If the cache has expired, clear the value so we don't attempt to make decisions based on stale data
+  const isExpired = Date.now() - lastUpdatedAt >= MAX_CACHE_LAST_UPDATED_AT_SECONDS * 1000;
+
+  if (isExpired) {
+    cache = {};
+  }
+
+  return isExpired;
 }
 
 type ErrorFields = {