From 3f744d596d4bf2f38e5eb1dc18c9792b9cc84eae Mon Sep 17 00:00:00 2001 From: Val Date: Sat, 20 Jun 2026 16:37:09 -0700 Subject: [PATCH] Fix ID token and session token JWK conflcit in KV store Fixes #31 The current implementation uses a signle `WorkersKVStore` for the user-provided KV key. Both the ID token verifier and session cookie verifier use this same store, but they each need a different JWK. The ID token verification keys are fetched from while the session cookie verification keys are fetched from . Whichever one gets used first gets cached in the KV store, then both ID token and session cookie verifiers will use those same cached keys until TTL expires. This means one of them will get the wrong keys and reject all verifications. This PR fixes this. I tried to go with the approach that is the least breaking possible. I didn't want to change the public interface of `Auth` nor `WorkersKVStore`. Instead this PR adds a `scoped(scope: string)` function to `KeyStorer` (optional for backwards compatibility), and implements it in both `WorkersKVStore` and `WorkersKVStoreSingle`. It returns an instance of the same class with `:${scope}` appended to the original key. In the case of `WorkersKVStoreSingle`, `.scoped()` also registers a singleton for that scope. Then in `BaseAuth`, we scope the `KeyStorer` in the contructor: ```js function scopedKeyStorer(keyStorer: KeyStorer, scope: string): KeyStorer { if (typeof keyStorer.scoped === 'function') { return keyStorer.scoped(scope); } return keyStorer; } this.idTokenVerifier = createIdTokenVerifier(projectId, scopedKeyStorer(keyStore, 'id-token')); this.sessionCookieVerifier = createSessionCookieVerifier(projectId, scopedKeyStorer(keyStore, 'session-cookie')); ``` Note: the `scopedKeyStorer` function is used in case the user passes a custom `KeyStorer` that doesn't have the `scoped` method we just added. Then it'll work just like before with the ID token and session cookie conflict. Upgrading to this patched version will result in the previous cache key being ignored, and the appropriate keys to be fetched in the new scoped stores on first use. The previous cache key will stay in the KV until its TTL, then it'll be garbage collected without any manual cleanup necessary. --- src/auth.ts | 6 +++--- src/index.ts | 4 ++++ src/key-store.ts | 19 ++++++++++++++++-- tests/auth.test.ts | 24 ++++++++++++++++++++++- tests/key-store.test.ts | 43 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 tests/key-store.test.ts diff --git a/src/auth.ts b/src/auth.ts index 94fb4c5..7ec8825 100644 --- a/src/auth.ts +++ b/src/auth.ts @@ -4,7 +4,7 @@ import type { EmulatorEnv } from './emulator'; import { useEmulator } from './emulator'; import type { ErrorInfo } from './errors'; import { AppErrorCodes, AuthClientErrorCode, FirebaseAppError, FirebaseAuthError } from './errors'; -import type { KeyStorer } from './key-store'; +import { scopedKeyStorer, type KeyStorer } from './key-store'; import type { FirebaseIdToken, FirebaseTokenVerifier } from './token-verifier'; import { createIdTokenVerifier, createSessionCookieVerifier } from './token-verifier'; import type { UserRecord } from './user-record'; @@ -17,8 +17,8 @@ export class BaseAuth { private readonly _authApiClient?: AuthApiClient; constructor(projectId: string, keyStore: KeyStorer, credential?: Credential) { - this.idTokenVerifier = createIdTokenVerifier(projectId, keyStore); - this.sessionCookieVerifier = createSessionCookieVerifier(projectId, keyStore); + this.idTokenVerifier = createIdTokenVerifier(projectId, scopedKeyStorer(keyStore, 'id-token')); + this.sessionCookieVerifier = createSessionCookieVerifier(projectId, scopedKeyStorer(keyStore, 'session-cookie')); if (credential) { this._authApiClient = new AuthApiClient(projectId, credential); diff --git a/src/index.ts b/src/index.ts index 3b790b2..7260219 100644 --- a/src/index.ts +++ b/src/index.ts @@ -42,6 +42,10 @@ export class WorkersKVStoreSingle extends WorkersKVStore { super(cacheKey, cfKVNamespace); } + public scoped(scope: string): WorkersKVStoreSingle { + return WorkersKVStoreSingle.getOrInitialize(`${this.cacheKey}:${scope}`, this.cfKVNamespace); + } + static getOrInitialize(cacheKey: string, cfKVNamespace: KVNamespace): WorkersKVStoreSingle { if (!WorkersKVStoreSingle.instance) { WorkersKVStoreSingle.instance = new Map(); diff --git a/src/key-store.ts b/src/key-store.ts index 1cdcf19..166380a 100644 --- a/src/key-store.ts +++ b/src/key-store.ts @@ -1,6 +1,14 @@ export interface KeyStorer { get(): Promise; put(value: string, expirationTtl: number): Promise; + scoped?(scope: string): KeyStorer; +} + +export function scopedKeyStorer(keyStorer: KeyStorer, scope: string): KeyStorer { + if (typeof keyStorer.scoped === 'function') { + return keyStorer.scoped(scope); + } + return keyStorer; } /** @@ -8,10 +16,17 @@ export interface KeyStorer { */ export class WorkersKVStore implements KeyStorer { constructor( - private readonly cacheKey: string, - private readonly cfKVNamespace: KVNamespace + protected readonly cacheKey: string, + protected readonly cfKVNamespace: KVNamespace ) {} + /** + * Returns a view of this store that caches public keys under a derived KV key. + */ + public scoped(scope: string): WorkersKVStore { + return new WorkersKVStore(`${this.cacheKey}:${scope}`, this.cfKVNamespace); + } + public async get(): Promise { return await this.cfKVNamespace.get(this.cacheKey, 'json'); } diff --git a/tests/auth.test.ts b/tests/auth.test.ts index c04a438..d14187f 100644 --- a/tests/auth.test.ts +++ b/tests/auth.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { ApiSettings } from '../src/api-requests'; import { BaseAuth } from '../src/auth'; import { AuthApiClient } from '../src/auth-api-requests'; @@ -24,6 +24,28 @@ const sessionCookieUids = [ generateRandomString(20), ]; +describe('BaseAuth key store scoping', () => { + it('scopes key store separately for id-token and session-cookie verifiers', () => { + const scoped = vi.fn( + (_scope: string): KeyStorer => ({ + get: vi.fn(), + put: vi.fn(), + }) + ); + const keyStorer: KeyStorer = { + get: vi.fn(), + put: vi.fn(), + scoped, + }; + + new BaseAuth(projectId, keyStorer); + + expect(scoped).toHaveBeenCalledTimes(2); + expect(scoped).toHaveBeenCalledWith('id-token'); + expect(scoped).toHaveBeenCalledWith('session-cookie'); + }); +}); + describe('createSessionCookie()', () => { const expiresIn = 24 * 60 * 60 * 1000; diff --git a/tests/key-store.test.ts b/tests/key-store.test.ts new file mode 100644 index 0000000..1b89524 --- /dev/null +++ b/tests/key-store.test.ts @@ -0,0 +1,43 @@ +import { Miniflare } from 'miniflare'; +import { describe, expect, it } from 'vitest'; +import { WorkersKVStoreSingle } from '../src/index'; +import { WorkersKVStore } from '../src/key-store'; + +const nullScript = 'export default { fetch: () => new Response(null, { status: 404 }) };'; +const mf = new Miniflare({ + modules: true, + script: nullScript, + kvNamespaces: ['TEST_NAMESPACE'], +}); + +describe('WorkersKVStore.scoped', () => { + it('suffixes the KV key with the scope', async () => { + const TEST_NAMESPACE = await mf.getKVNamespace('TEST_NAMESPACE'); + const baseKey = 'scoped-cache-key'; + const keyStore = new WorkersKVStore(baseKey, TEST_NAMESPACE); + + const idTokenStore = keyStore.scoped('id-token'); + const sessionCookieStore = keyStore.scoped('session-cookie'); + + await idTokenStore.put(JSON.stringify([{ kid: 'id-kid' }]), 3600); + await sessionCookieStore.put(JSON.stringify([{ kid: 'session-kid' }]), 3600); + + expect(await TEST_NAMESPACE.get(`${baseKey}:id-token`, 'json')).toEqual([{ kid: 'id-kid' }]); + expect(await TEST_NAMESPACE.get(`${baseKey}:session-cookie`, 'json')).toEqual([{ kid: 'session-kid' }]); + expect(await TEST_NAMESPACE.get(baseKey)).toBeNull(); + }); +}); + +describe('WorkersKVStoreSingle.scoped', () => { + it('returns a singleton per scoped cache key', async () => { + const TEST_NAMESPACE = await mf.getKVNamespace('TEST_NAMESPACE'); + const keyStore = WorkersKVStoreSingle.getOrInitialize('base-key', TEST_NAMESPACE); + + const idTokenStore = keyStore.scoped('id-token'); + const idTokenStoreAgain = keyStore.scoped('id-token'); + const sessionCookieStore = keyStore.scoped('session-cookie'); + + expect(idTokenStore).toBe(idTokenStoreAgain); + expect(sessionCookieStore).not.toBe(idTokenStore); + }); +});