fix(backend): Remove cache timer to fix empty cache bug (#3332)

Author: brkalow

.changeset/cyan-gifts-cheer.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/tokens/keys.test.ts

@@ -188,8 +188,10 @@ export default (QUnit: QUnit) => {
       }
     });
 
-    test('throws an error when JWKS can not be fetched from Backend or Frontend API and cache updated less than 5 minutes ago', async assert => {
+    test('throws an error when no JWK matches the provided kid', async assert => {
+      fakeFetch.onCall(0).returns(jsonOk(mockJwks));
       const kid = 'ins_whatever';
+
       try {
         await loadClerkJWKFromRemote({
           apiKey: 'deadbeef',
@@ -203,7 +205,7 @@ export default (QUnit: QUnit) => {
             action: 'Contact [email protected]',
           });
           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 are available: local, ${mockRsaJwkKid}`,
+            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 are available: ${mockRsaJwkKid}`,
           });
         } else {
           // This should never be reached. If it does, the suite should fail
@@ -212,30 +214,37 @@ export default (QUnit: QUnit) => {
       }
     });
 
-    test('throws an error when no JWK matches the provided kid', async assert => {
+    test('cache TTLs do not conflict', async assert => {
+      fakeClock.runAll();
+
       fakeFetch.onCall(0).returns(jsonOk(mockJwks));
-      const kid = 'ins_whatever';
+      let jwk = await loadClerkJWKFromRemote({
+        secretKey: 'deadbeef',
+        kid: mockRsaJwkKid,
+        skipJwksCache: true,
+      });
+      assert.propEqual(jwk, mockRsaJwk);
 
-      try {
-        await loadClerkJWKFromRemote({
-          apiKey: 'deadbeef',
-          kid,
-        });
-        assert.false(true);
-      } catch (err) {
-        if (err instanceof Error) {
-          assert.propEqual(err, {
-            reason: 'jwk-remote-missing',
-            action: 'Contact [email protected]',
-          });
-          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 are available: local, ${mockRsaJwkKid}`,
-          });
-        } else {
-          // This should never be reached. If it does, the suite should fail
-          assert.false(true);
-        }
-      }
+      // 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

@@ -27,22 +27,9 @@ function getCacheValues() {
   return Object.values(cache);
 }
 
-function setInCache(
-  jwk: JsonWebKeyWithKid,
-  jwksCacheTtlInMs: number = 1000 * 60 * 60, // 1 hour
-) {
+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';
@@ -87,7 +74,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
     );
   }
 
@@ -128,10 +115,12 @@ export async function loadClerkJWKFromRemote({
   apiVersion = API_VERSION,
   issuer,
   kid,
-  jwksCacheTtlInMs = 1000 * 60 * 60, // 1 hour,
   skipJwksCache,
 }: LoadClerkJWKFromRemoteOptions): Promise<JsonWebKey> {
-  const shouldRefreshCache = !getFromCache(kid) && reachedMaxCacheUpdatedAt();
+  if (cacheHasExpired()) {
+    cache = {};
+  }
+  const shouldRefreshCache = !getFromCache(kid);
   if (skipJwksCache || shouldRefreshCache) {
     let fetcher;
     const key = secretKey || apiKey;
@@ -158,7 +147,7 @@ export async function loadClerkJWKFromRemote({
       });
     }
 
-    keys.forEach(key => setInCache(key, jwksCacheTtlInMs));
+    keys.forEach(key => setInCache(key));
   }
 
   const jwk = getFromCache(kid);
@@ -240,6 +229,14 @@ async function fetchJWKSFromBAPI(apiUrl: string, key: string, apiVersion: string
   return response.json();
 }
 
-function reachedMaxCacheUpdatedAt() {
-  return Date.now() - lastUpdatedAt >= MAX_CACHE_LAST_UPDATED_AT_SECONDS * 1000;
+function cacheHasExpired() {
+  // 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;
+
+  return isExpired;
 }