fix(backend): Scope the JWKS cache per Clerk instance

.changeset/scope-jwks-cache-per-instance.md

@@ -0,0 +1,9 @@
+---
+'@clerk/backend': patch
+---
+
+Scope the JWKS cache per Clerk instance. The cache was keyed on the JWT `kid` alone and shared across the whole process, so an application verifying tokens for more than one Clerk instance (for example the Dynamic Keys / multi-tenant pattern) could resolve a signing key that was fetched for a different instance. Keys are now cached separately per secret key and API URL, so a token can only be verified against the instance whose credentials fetched its signing key.
+
+Networkless verification with `jwtKey` had the same flaw: the JWK derived from the PEM was cached by `kid` alone, so a process verifying tokens with different `jwtKey` values could resolve a key derived from another instance's PEM. The JWK is now always derived from the `jwtKey` that was passed in.
+
+The `jwk-kid-mismatch` error message no longer lists the key IDs currently held in the cache.

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

@@ -37,45 +37,40 @@ describe('tokens.loadClerkJWKFromLocal(localKey)', () => {
     expect(jwk).toMatchObject(mockPEMJwk);
   });
 
-  it('caches PEM keys separately for different kids', () => {
+  it('derives a separate JWK per kid', () => {
     const jwk1 = loadClerkJwkFromPem({ kid: 'ins_1', pem: mockPEMKey }) as JsonWebKey & { kid: string };
     expect(jwk1.kid).toBe('local-ins_1');
     expect(jwk1.n).toBe(mockPEMJwk.n);
 
     const jwk2 = loadClerkJwkFromPem({ kid: 'ins_2', pem: mockPEMJwtKey }) as JsonWebKey & { kid: string };
     expect(jwk2.kid).toBe('local-ins_2');
     expect(jwk2.n).toBe(mockPEMJwk.n);
+  });
 
-    // Verify both are cached independently
-    const jwk1Cached = loadClerkJwkFromPem({ kid: 'ins_1', pem: mockPEMKey });
-    const jwk2Cached = loadClerkJwkFromPem({ kid: 'ins_2', pem: mockPEMJwtKey });
+  // Regression test for SDK-148. A cache keyed on `kid` alone (an untrusted token-header
+  // value) served the first caller's key to every later caller presenting the same kid,
+  // regardless of the pem they supplied.
+  it('always derives the JWK from the provided pem, even for a previously seen kid', () => {
+    const otherModulus = 'x'.repeat(342);
+    const otherPem = `MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA${otherModulus}IDAQAB`;
 
-    expect(jwk1Cached).toBe(jwk1);
-    expect(jwk2Cached).toBe(jwk2); // Same object reference means its cached
-  });
+    const jwkA = loadClerkJwkFromPem({ kid: 'ins_same_kid', pem: mockPEMKey }) as JsonWebKey & { kid: string };
+    expect(jwkA.n).toBe(mockPEMJwk.n);
 
-  it('returns cached JWK on subsequent calls with same kid', () => {
-    const jwk1 = loadClerkJwkFromPem({ kid: 'cache-test', pem: mockPEMKey });
-    const jwk2 = loadClerkJwkFromPem({ kid: 'cache-test', pem: mockPEMKey });
-    // Should return the exact same reference
-    expect(jwk1).toBe(jwk2);
+    const jwkB = loadClerkJwkFromPem({ kid: 'ins_same_kid', pem: otherPem }) as JsonWebKey & { kid: string };
+    expect(jwkB.n).toBe(otherModulus);
   });
 
-  it('uses "local-" prefix to avoid cache collision with remote keys', () => {
+  it('uses "local-" prefix to distinguish the JWK from remote keys', () => {
     const localJwk = loadClerkJwkFromPem({ kid: 'test-kid', pem: mockPEMKey }) as JsonWebKey & { kid: string };
     expect(localJwk.kid).toBe('local-test-kid');
   });
 
-  it('creates separate cache entries for different kids even with same PEM', () => {
-    // Two JWT keys might theoretically use the same PEM (unlikely but possible)
+  it('derives separate JWKs for different kids even with same PEM', () => {
     const jwkA = loadClerkJwkFromPem({ kid: 'ins_key_a', pem: mockPEMKey }) as JsonWebKey & { kid: string };
     const jwkB = loadClerkJwkFromPem({ kid: 'ins_key_b', pem: mockPEMKey }) as JsonWebKey & { kid: string };
 
-    // They should be different objects
-    expect(jwkA).not.toBe(jwkB);
-    // But have the same modulus
     expect(jwkA.n).toBe(jwkB.n);
-    // And different prefixed kids
     expect(jwkA.kid).toBe('local-ins_key_a');
     expect(jwkB.kid).toBe('local-ins_key_b');
   });
@@ -200,8 +195,156 @@ describe('tokens.loadClerkJWKFromRemote(options)', () => {
         kid,
       }),
     ).rejects.toThrowError(
-      "Unable to find a signing key in JWKS that matches the kid='ins_whatever' 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: ins_2GIoQhbUpy0hX7B2cVkuTMinXoD",
+      "Unable to find a signing key in JWKS that matches the kid='ins_whatever' of the provided session token. Please make sure that the __session cookie or the HTTP authorization header contain a Clerk-generated session JWT.",
+    );
+  });
+
+  // The cached kids are instance ids; enumerating them discloses which co-tenants
+  // are warm in a shared process.
+  it('does not enumerate cached kids in the error message', async () => {
+    server.use(
+      http.get(
+        'https://api.clerk.com/v1/jwks',
+        validateHeaders(() => {
+          return HttpResponse.json(mockJwks);
+        }),
+      ),
+    );
+
+    const error = await loadClerkJWKFromRemote({ secretKey: 'deadbeef', kid: 'ins_whatever' }).catch(e => e);
+
+    expect(error).toBeInstanceOf(TokenVerificationError);
+    expect(error.message).not.toContain(mockRsaJwkKid);
+  });
+
+  // Regression test for SDK-148. The cache was keyed on `kid` alone. Since a Clerk `kid`
+  // is the instance id and the lookup short-circuits before `secretKey` is consulted, a
+  // key fetched for one instance was served to another instance's verifier, and
+  // `verifyJwt` never asserts `iss`.
+  it('does not serve a cached key to a different secretKey', async () => {
+    const instanceAKid = 'ins_tenant_a';
+    let secretKeysUsed: string[] = [];
+
+    server.use(
+      http.get(
+        'https://api.clerk.com/v1/jwks',
+        validateHeaders(({ request }) => {
+          secretKeysUsed.push((request.headers.get('Authorization') ?? '').replace('Bearer ', ''));
+          // Each instance's JWKS contains only its own signing key.
+          return HttpResponse.json({ keys: [{ ...mockRsaJwk, kid: instanceAKid }] });
+        }),
+      ),
     );
+
+    // Instance A warms the cache with its own key.
+    const jwk = await loadClerkJWKFromRemote({ secretKey: 'sk_test_a', kid: instanceAKid });
+    expect(jwk).toMatchObject({ kid: instanceAKid });
+    expect(secretKeysUsed).toEqual(['sk_test_a']);
+
+    // Instance B asking for instance A's kid must miss the cache and fetch under its
+    // own secretKey.
+    secretKeysUsed = [];
+    server.use(
+      http.get(
+        'https://api.clerk.com/v1/jwks',
+        validateHeaders(({ request }) => {
+          secretKeysUsed.push((request.headers.get('Authorization') ?? '').replace('Bearer ', ''));
+          return HttpResponse.json({ keys: [{ ...mockRsaJwk, kid: 'ins_tenant_b' }] });
+        }),
+      ),
+    );
+
+    await expect(() => loadClerkJWKFromRemote({ secretKey: 'sk_test_b', kid: instanceAKid })).rejects.toThrowError(
+      TokenVerificationError,
+    );
+    expect(secretKeysUsed).toEqual(['sk_test_b']);
+  });
+
+  it('keeps a separate cache TTL per instance', async () => {
+    let fetchCount = 0;
+    server.use(
+      http.get(
+        'https://api.clerk.com/v1/jwks',
+        validateHeaders(() => {
+          fetchCount++;
+          return HttpResponse.json(mockJwks);
+        }),
+      ),
+    );
+
+    await loadClerkJWKFromRemote({ secretKey: 'sk_ttl_a', kid: mockRsaJwkKid });
+    expect(fetchCount).toBe(1);
+
+    // A second instance must not ride on the first instance's fresh TTL.
+    await loadClerkJWKFromRemote({ secretKey: 'sk_ttl_b', kid: mockRsaJwkKid });
+    expect(fetchCount).toBe(2);
+
+    // Each instance now serves from its own cache.
+    await loadClerkJWKFromRemote({ secretKey: 'sk_ttl_a', kid: mockRsaJwkKid });
+    await loadClerkJWKFromRemote({ secretKey: 'sk_ttl_b', kid: mockRsaJwkKid });
+    expect(fetchCount).toBe(2);
+  });
+
+  it('keeps a separate cache per apiUrl', async () => {
+    const fetches = { com: 0, test: 0 };
+    server.use(
+      http.get(
+        'https://api.clerk.com/v1/jwks',
+        validateHeaders(() => {
+          fetches.com++;
+          return HttpResponse.json(mockJwks);
+        }),
+      ),
+      http.get(
+        'https://api.clerk.test/v1/jwks',
+        validateHeaders(() => {
+          fetches.test++;
+          return HttpResponse.json(mockJwks);
+        }),
+      ),
+    );
+
+    await loadClerkJWKFromRemote({ secretKey: 'sk_api_url', kid: mockRsaJwkKid });
+    expect(fetches).toEqual({ com: 1, test: 0 });
+
+    // The same kid under another apiUrl must not be served from the first scope's cache.
+    await loadClerkJWKFromRemote({ secretKey: 'sk_api_url', apiUrl: 'https://api.clerk.test', kid: mockRsaJwkKid });
+    expect(fetches).toEqual({ com: 1, test: 1 });
+
+    await loadClerkJWKFromRemote({ secretKey: 'sk_api_url', kid: mockRsaJwkKid });
+    await loadClerkJWKFromRemote({ secretKey: 'sk_api_url', apiUrl: 'https://api.clerk.test', kid: mockRsaJwkKid });
+    expect(fetches).toEqual({ com: 1, test: 1 });
+  });
+
+  it('keeps a separate cache per apiVersion', async () => {
+    const fetches = { v1: 0, v2: 0 };
+    server.use(
+      http.get(
+        'https://api.clerk.com/v1/jwks',
+        validateHeaders(() => {
+          fetches.v1++;
+          return HttpResponse.json(mockJwks);
+        }),
+      ),
+      http.get(
+        'https://api.clerk.com/v2/jwks',
+        validateHeaders(() => {
+          fetches.v2++;
+          return HttpResponse.json(mockJwks);
+        }),
+      ),
+    );
+
+    await loadClerkJWKFromRemote({ secretKey: 'sk_api_version', kid: mockRsaJwkKid });
+    expect(fetches).toEqual({ v1: 1, v2: 0 });
+
+    // The same kid under another apiVersion must not be served from the first scope's cache.
+    await loadClerkJWKFromRemote({ secretKey: 'sk_api_version', apiVersion: 'v2', kid: mockRsaJwkKid });
+    expect(fetches).toEqual({ v1: 1, v2: 1 });
+
+    await loadClerkJWKFromRemote({ secretKey: 'sk_api_version', kid: mockRsaJwkKid });
+    await loadClerkJWKFromRemote({ secretKey: 'sk_api_version', apiVersion: 'v2', kid: mockRsaJwkKid });
+    expect(fetches).toEqual({ v1: 1, v2: 1 });
   });
 
   it('cache TTLs do not conflict', async () => {

packages/backend/src/tokens/keys.ts

@@ -19,20 +19,36 @@ type JsonWebKeyWithKid = JsonWebKey & { kid: string };
 
 type JsonWebKeyCache = Record<string, JsonWebKeyWithKid>;
 
-let cache: JsonWebKeyCache = {};
-let lastUpdatedAt = 0;
-
-function getFromCache(kid: string) {
-  return cache[kid];
-}
+type RemoteJwksCache = {
+  keys: JsonWebKeyCache;
+  lastUpdatedAt: number;
+};
 
-function getCacheValues() {
-  return Object.values(cache);
-}
+/**
+ * Remote JWKS caches, one per Clerk instance. A single process-wide cache keyed by `kid`
+ * alone hands one instance's signing key to another instance's verification: a Clerk `kid`
+ * is the instance id, the lookup short-circuits before `secretKey` is consulted, and
+ * `verifyJwt` does not assert `iss`. That let a session token minted by instance B
+ * authenticate against instance A in any process serving both.
+ */
+const remoteCaches = new Map<string, RemoteJwksCache>();
 
-function setInCache(cacheKey: string, jwk: JsonWebKeyWithKid, shouldExpire = true) {
-  cache[cacheKey] = jwk;
-  lastUpdatedAt = shouldExpire ? Date.now() : -1;
+/**
+ * The scope is held in memory only as a Map key. It is never logged or surfaced in errors.
+ */
+function getRemoteCache(scope: string): RemoteJwksCache {
+  let cache = remoteCaches.get(scope);
+  if (!cache) {
+    // Evict expired scopes on new-scope creation so one-off scopes cannot grow the Map forever.
+    for (const [key, entry] of remoteCaches) {
+      if (cacheHasExpired(entry)) {
+        remoteCaches.delete(key);
+      }
+    }
+    cache = { keys: {}, lastUpdatedAt: 0 };
+    remoteCaches.set(scope, cache);
+  }
+  return cache;
 }
 
 const PEM_HEADER = '-----BEGIN PUBLIC KEY-----';
@@ -47,21 +63,12 @@ type LoadClerkJwkFromPemOptions = {
 
 /**
  * Loads a local PEM key usually from process.env and transform it to JsonWebKey format.
- * The result is cached on the module level to avoid unnecessary computations in subsequent invocations.
+ * Derived fresh on every call: a cache keyed on `kid` (which comes from the untrusted token
+ * header) served one instance's key to another instance's verifier, and derivation is cheap.
  */
 export function loadClerkJwkFromPem(params: LoadClerkJwkFromPemOptions): JsonWebKey {
   const { kid, pem } = params;
 
-  // We use a cache key that includes the local prefix in order to avoid
-  // cache conflicts when loadClerkJwkFromPem and loadClerkJWKFromRemote
-  // are called with the same kid
-  const prefixedKid = `local-${kid}`;
-  const cachedJwk = getFromCache(prefixedKid);
-
-  if (cachedJwk) {
-    return cachedJwk;
-  }
-
   if (!pem) {
     throw new TokenVerificationError({
       action: TokenVerificationErrorAction.SetClerkJWTKey,
@@ -80,8 +87,8 @@ export function loadClerkJwkFromPem(params: LoadClerkJwkFromPemOptions): JsonWeb
     .replace(/\//g, '_');
 
   // https://datatracker.ietf.org/doc/html/rfc7517
-  const jwk = { kid: prefixedKid, kty: 'RSA', alg: 'RS256', n: modulus, e: 'AQAB' };
-  setInCache(prefixedKid, jwk, false); // local key never expires in cache
+  // The 'local-' kid prefix distinguishes locally derived JWKs from remote ones.
+  const jwk: JsonWebKeyWithKid = { kid: `local-${kid}`, kty: 'RSA', alg: 'RS256', n: modulus, e: 'AQAB' };
   return jwk;
 }
 
@@ -131,7 +138,9 @@ export type LoadClerkJWKFromRemoteOptions = {
 export async function loadClerkJWKFromRemote(params: LoadClerkJWKFromRemoteOptions): Promise<JsonWebKey> {
   const { secretKey, apiUrl = API_URL, apiVersion = API_VERSION, kid, skipJwksCache } = params;
 
-  if (skipJwksCache || cacheHasExpired() || !getFromCache(kid)) {
+  const cache = getRemoteCache(`${apiUrl}|${apiVersion}|${secretKey ?? ''}`);
+
+  if (skipJwksCache || cacheHasExpired(cache) || !cache.keys[kid]) {
     if (!secretKey) {
       throw new TokenVerificationError({
         action: TokenVerificationErrorAction.ContactSupport,
@@ -150,21 +159,20 @@ export async function loadClerkJWKFromRemote(params: LoadClerkJWKFromRemoteOptio
       });
     }
 
-    keys.forEach(key => setInCache(key.kid, key));
+    keys.forEach(key => {
+      cache.keys[key.kid] = key;
+    });
+    cache.lastUpdatedAt = Date.now();
   }
 
-  const jwk = getFromCache(kid);
+  const jwk = cache.keys[kid];
 
   if (!jwk) {
-    const cacheValues = getCacheValues();
-    const jwkKeys = cacheValues
-      .map(jwk => jwk.kid)
-      .sort()
-      .join(', ');
-
+    // The available kids are deliberately omitted: they are instance ids, and enumerating
+    // them would disclose which co-tenants are warm in a shared process.
     throw new TokenVerificationError({
       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}`,
+      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.`,
       reason: TokenVerificationErrorReason.JWKKidMismatch,
     });
   }
@@ -218,17 +226,12 @@ async function fetchJWKSFromBAPI(apiUrl: string, key: string, apiVersion: string
   return response.json();
 }
 
-function cacheHasExpired() {
-  // If lastUpdatedAt is -1, it means that we're using a local JWKS and it never expires
-  if (lastUpdatedAt === -1) {
-    return false;
-  }
-
+function cacheHasExpired(cache: RemoteJwksCache) {
   // 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;
+  const isExpired = Date.now() - cache.lastUpdatedAt >= MAX_CACHE_LAST_UPDATED_AT_SECONDS * 1000;
 
   if (isExpired) {
-    cache = {};
+    cache.keys = {};
   }
 
   return isExpired;