diff --git a/src/index.ts b/src/index.ts index d54299a..a251331 100644 --- a/src/index.ts +++ b/src/index.ts @@ -10,7 +10,7 @@ import { OAuthResource } from './lib/resource.ts'; import { validateAndRefreshSession } from './lib/sessionValidator.ts'; import { clearOAuthSession } from './lib/handlers.ts'; import { HookManager } from './lib/hookManager.ts'; -import { DynamicProviderCache } from './lib/dynamicProviderCache.ts'; +import { DynamicProviderCache, DEFAULT_DYNAMIC_PROVIDER_CACHE_TTL_SECONDS } from './lib/dynamicProviderCache.ts'; import type { Scope, OAuthPluginConfig, ProviderRegistry, OAuthHooks } from './types.ts'; // Export HookManager class, OAuthResource class, and types @@ -39,6 +39,36 @@ export { getProvider } from './lib/providers/index.ts'; let pendingHooks: OAuthHooks | null = null; let activeHookManager: HookManager | null = null; +// Active dynamic-provider cache for the loaded plugin instance, so consumers +// can invalidate entries when their backing config changes. Per-worker-thread: +// this references the cache in the thread that ran handleApplication. +let activeDynamicProviderCache: DynamicProviderCache | null = null; + +/** + * Invalidate a single dynamically-resolved provider in the in-memory cache. + * Call this from application code after the backing config for `providerConfigId` + * changes (disabled, deleted, or credentials rotated) so the next request + * re-resolves it via the `onResolveProvider` hook instead of serving stale data. + * + * NOTE: the cache is per-worker-thread, so this evicts only in the thread that + * runs the call. Other threads converge within the configured TTL + * (`cacheDynamicProviders`, default {@link DEFAULT_DYNAMIC_PROVIDER_CACHE_TTL_SECONDS}s). + * For immediate cluster-wide effect, pair invalidation with a short TTL. + * + * @returns true if an entry was present and removed in this thread. + */ +export function invalidateDynamicProvider(providerConfigId: string): boolean { + return activeDynamicProviderCache?.delete(providerConfigId) ?? false; +} + +/** + * Clear every dynamically-resolved provider from the in-memory cache (this + * worker thread only — see {@link invalidateDynamicProvider} for cross-thread notes). + */ +export function clearDynamicProviderCache(): void { + activeDynamicProviderCache?.clear(); +} + /** * Register OAuth hooks programmatically * Call this from your application code to register lifecycle hooks @@ -84,6 +114,9 @@ export async function handleApplication(scope: Scope): Promise { let pluginDefaults: any = {}; // Store plugin defaults for dynamic provider resolution const dynamicProviderCache = new DynamicProviderCache(); // TTL cache for dynamically-resolved providers + // Expose this thread's cache for consumer-driven invalidation + activeDynamicProviderCache = dynamicProviderCache; + // Create hookManager instance scoped to this application const hookManager = new HookManager(logger); @@ -128,8 +161,10 @@ export async function handleApplication(scope: Scope): Promise { // Extract plugin defaults for dynamic provider resolution pluginDefaults = extractPluginDefaults(options); - // Update dynamic provider cache TTL (clears stale entries on config change) - dynamicProviderCache.updateTTL(options.cacheDynamicProviders ?? true); + // Update dynamic provider cache TTL (clears stale entries on config change). + // Defaults to a bounded TTL rather than forever so disabled/rotated + // dynamic providers stop being served without a restart (see #105). + dynamicProviderCache.updateTTL(options.cacheDynamicProviders ?? DEFAULT_DYNAMIC_PROVIDER_CACHE_TTL_SECONDS); // Update the resource with new providers if (Object.keys(providers).length === 0) { @@ -280,5 +315,8 @@ export async function handleApplication(scope: Scope): Promise { // Clean up on scope close scope.on('close', () => { logger?.info?.('OAuth plugin shutting down'); + if (activeDynamicProviderCache === dynamicProviderCache) { + activeDynamicProviderCache = null; + } }); } diff --git a/src/lib/dynamicProviderCache.ts b/src/lib/dynamicProviderCache.ts index 4264d5c..4776d9d 100644 --- a/src/lib/dynamicProviderCache.ts +++ b/src/lib/dynamicProviderCache.ts @@ -6,13 +6,29 @@ * config changes within a bounded window. * * cacheDynamicProviders values: - * true — cache forever (backward compatible default) + * true — cache forever (no eviction except explicit invalidate/clear) * false — never cache, always call hook * — cache for N seconds + * (unset) — DEFAULT_DYNAMIC_PROVIDER_CACHE_TTL_SECONDS (bounded) + * + * The cache is in-memory and per-worker-thread. A resolved provider therefore + * only goes stale when its TTL elapses (each thread re-resolves on its own) or + * when a consumer explicitly evicts it via `delete`/`clear`. Because an explicit + * eviction reaches only the thread that runs it, the TTL is the cross-thread + * convergence mechanism — keep it bounded so config changes (disable / delete / + * credential rotation) take effect cluster-wide. */ import type { ProviderRegistryEntry } from '../types.ts'; +/** + * Default TTL (seconds) when `cacheDynamicProviders` is not set. Bounded rather + * than infinite so a changed backing config is picked up within the window even + * without an explicit invalidation. The previous default was `true` (forever), + * which left disabled/rotated providers serving stale data until restart. + */ +export const DEFAULT_DYNAMIC_PROVIDER_CACHE_TTL_SECONDS = 300; + interface CacheEntry { entry: ProviderRegistryEntry; cachedAt: number; @@ -22,7 +38,7 @@ export class DynamicProviderCache { private cache = new Map(); private ttlMs: number; - constructor(ttl: boolean | number = true) { + constructor(ttl: boolean | number = DEFAULT_DYNAMIC_PROVIDER_CACHE_TTL_SECONDS) { this.ttlMs = DynamicProviderCache.parseTTL(ttl); } @@ -51,6 +67,14 @@ export class DynamicProviderCache { this.cache.set(name, { entry, cachedAt: Date.now() }); } + /** + * Evict a single entry. Returns true if it was present. Use when the backing + * config for `name` changes so the next request re-resolves it via the hook. + */ + delete(name: string): boolean { + return this.cache.delete(name); + } + clear(): void { this.cache.clear(); } diff --git a/src/types.ts b/src/types.ts index df03164..0a8b17a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -30,7 +30,7 @@ export interface OAuthPluginConfig { defaultRole?: string; /** Lifecycle hooks */ hooks?: OAuthHooks; - /** Cache providers resolved via onResolveProvider hook. true = forever (default), false = never, number = TTL in seconds */ + /** Cache providers resolved via onResolveProvider hook. true = forever, false = never, number = TTL in seconds. Default: 300s (bounded, so disabled/rotated providers stop being served without a restart). */ cacheDynamicProviders?: boolean | number; } diff --git a/test/dynamic-provider-invalidation.test.js b/test/dynamic-provider-invalidation.test.js new file mode 100644 index 0000000..a579122 --- /dev/null +++ b/test/dynamic-provider-invalidation.test.js @@ -0,0 +1,127 @@ +/** + * Tests for the module-level dynamic-provider cache invalidation wiring in + * src/index.ts: invalidateDynamicProvider(), clearDynamicProviderCache(), and + * the scope-close ownership guard. The DynamicProviderCache class itself is + * covered in test/lib/dynamicProviderCache.test.js — this exercises the + * handleApplication wiring around it. + * + * Everything runs in one test because the exported functions read module-level + * state (the active cache + active hookManager) that is shared across the file; + * node --test isolates each file in its own subprocess, so a single sequence + * keeps the state deterministic without cross-test contamination. + */ + +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { + handleApplication, + registerHooks, + invalidateDynamicProvider, + clearDynamicProviderCache, +} from '../dist/index.js'; + +function makeScope() { + const closeListeners = []; + let middleware = null; + const scope = { + logger: { info: () => {}, error: () => {}, warn: () => {}, debug: () => {} }, + options: { + _config: { + // A static provider so the plugin registers the real OAuthResource + // (not the no-providers error stub); the hook handles oac-* configs. + providers: { + github: { provider: 'github', clientId: 'gh-id', clientSecret: 'gh-secret' }, + }, + }, + getAll() { + return this._config; + }, + on() {}, + }, + server: { + // The session-validation middleware registers with no options; the MCP + // well-known handlers register with { urlPath }. Capture only the former. + http(fn, opts) { + if (!opts?.urlPath) middleware = fn; + return fn; + }, + }, + resources: { + set() {}, + }, + on(event, listener) { + if (event === 'close') closeListeners.push(listener); + }, + }; + return { scope, getMiddleware: () => middleware, closeListeners }; +} + +/** A session whose token is far from expiry so the validator does no refresh/network. */ +function oacSession(providerConfigId) { + const future = Date.now() + 60 * 60 * 1000; + return { + oauth: { + providerConfigId, + accessToken: 'access-token', + expiresAt: future, + refreshThreshold: future, + }, + }; +} + +describe('dynamic provider cache invalidation wiring', () => { + it('invalidate/clear evict resolved providers, and close releases the active cache', async () => { + // Before any plugin load there is no active cache → invalidate is a no-op. + assert.strictEqual(invalidateDynamicProvider('oac-1'), false); + + let resolveCalls = 0; + registerHooks({ + async onResolveProvider(providerName) { + if (!providerName.startsWith('oac-')) return null; + resolveCalls++; + return { + provider: 'generic', + clientId: 'oac-client', + clientSecret: 'oac-secret', + authorizationUrl: 'https://idp.test/authorize', + tokenUrl: 'https://idp.test/token', + userInfoUrl: 'https://idp.test/userinfo', + scope: 'openid', + }; + }, + }); + + const { scope, getMiddleware, closeListeners } = makeScope(); + await handleApplication(scope); + const middleware = getMiddleware(); + assert.ok(typeof middleware === 'function', 'middleware should be registered'); + + const next = (req) => req; + + // First request for oac-1 → cache miss → hook resolves and caches it. + await middleware({ session: oacSession('oac-1') }, next); + assert.strictEqual(resolveCalls, 1, 'hook resolves on first miss'); + + // Second request → served from cache, hook not called again. + await middleware({ session: oacSession('oac-1') }, next); + assert.strictEqual(resolveCalls, 1, 'second request is a cache hit'); + + // Invalidate the entry → evicts in this thread. + assert.strictEqual(invalidateDynamicProvider('oac-1'), true, 'invalidate evicts a present entry'); + assert.strictEqual(invalidateDynamicProvider('oac-1'), false, 'invalidate is false once evicted'); + + // Next request re-resolves via the hook (cache miss after eviction). + await middleware({ session: oacSession('oac-1') }, next); + assert.strictEqual(resolveCalls, 2, 'request after invalidate re-resolves'); + + // clearDynamicProviderCache() drops everything → next request re-resolves. + clearDynamicProviderCache(); + await middleware({ session: oacSession('oac-1') }, next); + assert.strictEqual(resolveCalls, 3, 'request after clear re-resolves'); + + // Scope close releases the active-cache reference → invalidate becomes a no-op. + assert.strictEqual(closeListeners.length, 1, 'a close listener is registered'); + closeListeners[0](); + assert.strictEqual(invalidateDynamicProvider('oac-1'), false, 'invalidate is a no-op after close'); + }); +}); diff --git a/test/lib/dynamicProviderCache.test.js b/test/lib/dynamicProviderCache.test.js index d911332..9c4aa6e 100644 --- a/test/lib/dynamicProviderCache.test.js +++ b/test/lib/dynamicProviderCache.test.js @@ -26,11 +26,26 @@ function advanceTime(ms) { describe('DynamicProviderCache', () => { describe('constructor TTL parsing', () => { - it('defaults to cache forever (true)', () => { + it('defaults to a bounded 300s TTL (not forever)', () => { const cache = new DynamicProviderCache(); const entry = mockEntry('okta'); cache.set('okta-org1', entry); - assert.strictEqual(cache.get('okta-org1'), entry); + + // Within the default window — entry is served + const restoreWithin = advanceTime(299_000); + try { + assert.strictEqual(cache.get('okta-org1'), entry); + } finally { + restoreWithin(); + } + + // Past the default window — entry has expired (proves it is NOT forever) + const restorePast = advanceTime(301_000); + try { + assert.strictEqual(cache.get('okta-org1'), undefined); + } finally { + restorePast(); + } }); it('true caches forever', () => { @@ -187,6 +202,25 @@ describe('DynamicProviderCache', () => { }); }); + describe('delete', () => { + it('evicts a single entry and reports whether it was present', () => { + const cache = new DynamicProviderCache(60); + const entry = mockEntry('okta'); + cache.set('okta-org1', entry); + cache.set('azure-org2', mockEntry('azure')); + + assert.strictEqual(cache.delete('okta-org1'), true); + assert.strictEqual(cache.get('okta-org1'), undefined); + // Other entries are untouched + assert.strictEqual(cache.size, 1); + }); + + it('returns false when the key is not present', () => { + const cache = new DynamicProviderCache(60); + assert.strictEqual(cache.delete('nonexistent'), false); + }); + }); + describe('clear', () => { it('removes all entries', () => { const cache = new DynamicProviderCache(60);