Skip to content

Commit 1b37e2d

Browse files
committed
fix(plugin-auth): the MCP access-token verifier reads its key set through the signer's keyring reader, with one bounded fresh read for an unknown key
Claude-Session: https://claude.ai/code/session_01Nk17Z26X5DB59iQztDBADH Co-authored-by: Claude <noreply@anthropic.com>
1 parent 99e9d29 commit 1b37e2d

3 files changed

Lines changed: 461 additions & 16 deletions

File tree

Lines changed: 320 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,320 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#24186] The MCP access-token verifier's key set follows the cross-request
5+
* caching rule: it is read through the signer's own keyring reader, so a
6+
* keyring write this process observes reaches the next verification, the
7+
* reader's TTL bounds a write it cannot see, and a token signed by a key the
8+
* set does not carry earns one bounded fresh read before it is refused.
9+
*
10+
* ## The composition
11+
*
12+
* A real `AuthManager` on a real `ObjectQL` over `@objectstack/driver-sql` +
13+
* better-sqlite3, with this plugin's identity objects. Its embedded
14+
* authorization server is on (a loopback origin, the MCP surface on), so the
15+
* jwt plugin and its keyring reader are the ones a deployment runs. Tokens are
16+
* signed by that jwt plugin's own `signJWT` with the stored key, for this
17+
* deployment's issuer and MCP audience.
18+
*
19+
* ## What is pinned — one consequence per case
20+
*
21+
* 1. A signing key removed from the keyring stops verifying the tokens it
22+
* signed on the very next call, with no clock movement.
23+
* 2. A key that joins the keyring after the verifier's last read verifies on
24+
* the first call: through the write seam when the write passes this
25+
* process's engine, and through the one bounded fresh read when it does not
26+
* (written below the engine here, as a write on another node would be).
27+
* 3. A burst of tokens naming keys the keyring does not carry costs at most
28+
* one keyring read inside the floor interval, however the burst is spread.
29+
*
30+
* The bound's own edges — a read in flight is joined, the floor re-arms
31+
* exactly at its interval, a failed read still spends it — are pinned on a
32+
* counting key-set double at the foot of the file.
33+
*/
34+
35+
import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest';
36+
import { SignJWT, exportJWK, generateKeyPair } from 'jose';
37+
import { ObjectQL } from '@objectstack/objectql';
38+
import { SqlDriver } from '@objectstack/driver-sql';
39+
import { AuthManager } from './auth-manager.js';
40+
import { authIdentityObjects } from './manifest.js';
41+
import { retireHeldKeyrings, UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS } from './signing-key-read-cache.js';
42+
43+
const BASE = 'http://localhost:3000';
44+
const SECRET = 'test-secret-at-least-32-chars-long-24186';
45+
const SYS = { context: { isSystem: true } } as const;
46+
const USER = 'usr_24186';
47+
const CLIENT = 'client_24186';
48+
49+
const ENV_KEYS = ['OS_MCP_SERVER_ENABLED', 'OS_OIDC_PROVIDER_ENABLED'] as const;
50+
const savedEnv: Record<string, string | undefined> = {};
51+
const engines: ObjectQL[] = [];
52+
53+
beforeEach(() => {
54+
for (const k of ENV_KEYS) {
55+
savedEnv[k] = process.env[k];
56+
delete process.env[k];
57+
}
58+
process.env.OS_MCP_SERVER_ENABLED = 'true';
59+
});
60+
afterEach(async () => {
61+
vi.useRealTimers();
62+
vi.restoreAllMocks();
63+
for (const k of ENV_KEYS) {
64+
if (savedEnv[k] === undefined) delete process.env[k];
65+
else process.env[k] = savedEnv[k];
66+
}
67+
while (engines.length) {
68+
try {
69+
await (engines.pop() as unknown as { destroy?(): Promise<void> })?.destroy?.();
70+
} catch {
71+
/* noop */
72+
}
73+
}
74+
});
75+
76+
/** The `kid` a compact JWS names. */
77+
const kidOf = (jwt: string): string =>
78+
JSON.parse(Buffer.from(jwt.split('.')[0]!, 'base64url').toString('utf8')).kid;
79+
80+
/** A real AuthManager on a real engine, its keyring reads counted below the engine. */
81+
async function verifierRig() {
82+
vi.spyOn(console, 'warn').mockImplementation(() => {});
83+
const engine = new ObjectQL();
84+
engines.push(engine);
85+
const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true });
86+
engine.registerDriver(driver, true);
87+
await engine.init();
88+
for (const object of authIdentityObjects) {
89+
engine.registry.registerObject(object as never, '@objectstack/plugin-auth');
90+
}
91+
await engine.syncSchemas();
92+
vi.spyOn((engine as any).logger, 'info').mockImplementation(() => undefined);
93+
94+
// Every driver read of the keyring table, whichever verb carries it.
95+
let keyringDriverReads = 0;
96+
for (const verb of ['find', 'findOne'] as const) {
97+
const original = (driver as any)[verb].bind(driver);
98+
(driver as any)[verb] = async (object: string, ...rest: unknown[]) => {
99+
if (object === 'sys_jwks') keyringDriverReads += 1;
100+
return original(object, ...rest);
101+
};
102+
}
103+
104+
const manager = new AuthManager({ secret: SECRET, baseUrl: BASE, dataEngine: engine as never } as never);
105+
const api: any = await manager.getApi();
106+
107+
/** A token for this deployment's MCP resource, signed by the jwt plugin with the stored key. */
108+
const mint = async (): Promise<string> =>
109+
(await api.signJWT({
110+
body: {
111+
payload: {
112+
sub: USER,
113+
azp: CLIENT,
114+
scope: 'data:read',
115+
iss: manager.getAuthIssuer(),
116+
aud: manager.getMcpResourceUrl(),
117+
},
118+
},
119+
})).token;
120+
121+
/** A token for the same issuer and audience, signed by `privateKey` under `kid`. */
122+
const signWith = (privateKey: CryptoKey, kid: string): Promise<string> => {
123+
const now = Math.floor(Date.now() / 1000);
124+
return new SignJWT({ scope: 'data:read', azp: CLIENT })
125+
.setProtectedHeader({ alg: 'EdDSA', kid })
126+
.setIssuer(manager.getAuthIssuer())
127+
.setAudience(manager.getMcpResourceUrl())
128+
.setSubject(USER)
129+
.setIssuedAt(now)
130+
.setExpirationTime(now + 900)
131+
.sign(privateKey);
132+
};
133+
134+
/** What ONE keyring read costs at the driver on this engine — the unit the burst is counted in. */
135+
const oneKeyringRead = async (): Promise<number> => {
136+
const before = keyringDriverReads;
137+
retireHeldKeyrings(engine);
138+
await api.getJwks();
139+
return keyringDriverReads - before;
140+
};
141+
142+
return { engine, driver, manager, api, mint, signWith, oneKeyringRead, keyringReads: () => keyringDriverReads };
143+
}
144+
145+
describe('[#24186] the MCP verifier\'s key set follows the keyring, on a real engine', () => {
146+
it('a signing key removed from the keyring stops verifying its tokens on the next call, with no clock movement', async () => {
147+
const { engine, manager, mint } = await verifierRig();
148+
const token = await mint();
149+
expect(await manager.verifyMcpAccessToken(token)).toMatchObject({ userId: USER, clientId: CLIENT });
150+
151+
await engine.delete('sys_jwks', { where: { id: kidOf(token) }, ...SYS });
152+
153+
expect(await manager.verifyMcpAccessToken(token)).toBeNull();
154+
// The verifier itself is intact: a token from the key that replaced it verifies.
155+
const next = await mint();
156+
expect(kidOf(next)).not.toBe(kidOf(token));
157+
expect(await manager.verifyMcpAccessToken(next)).toMatchObject({ userId: USER });
158+
}, 60_000);
159+
160+
it('a key that joins the keyring through this engine after the last read verifies on the first call', async () => {
161+
const { engine, manager, mint } = await verifierRig();
162+
const first = await mint();
163+
expect(await manager.verifyMcpAccessToken(first)).toMatchObject({ userId: USER });
164+
165+
// The stored key expires a moment ago, so the next signature mints its successor.
166+
await engine.update('sys_jwks', { id: kidOf(first), expires_at: new Date(Date.now() - 1_000).toISOString() }, SYS);
167+
const second = await mint();
168+
expect(kidOf(second)).not.toBe(kidOf(first));
169+
170+
expect(await manager.verifyMcpAccessToken(second)).toMatchObject({ userId: USER });
171+
}, 60_000);
172+
173+
it('a key that joins the keyring where this engine cannot see it verifies on the first call, through one fresh read', async () => {
174+
const { driver, manager, mint, signWith, oneKeyringRead, keyringReads } = await verifierRig();
175+
expect(await manager.verifyMcpAccessToken(await mint())).toMatchObject({ userId: USER });
176+
const unit = await oneKeyringRead();
177+
expect(unit, 'the instrument sees a keyring read').toBeGreaterThan(0);
178+
179+
// A key row written BELOW the engine, as another node's write would land:
180+
// nothing in this process observes it. Its private half is never read here.
181+
const pair = await generateKeyPair('EdDSA', { crv: 'Ed25519', extractable: true });
182+
const kid = 'key_24186_unobserved';
183+
await driver.create('sys_jwks', {
184+
id: kid,
185+
public_key: JSON.stringify(await exportJWK(pair.publicKey)),
186+
private_key: '{}',
187+
alg: 'EdDSA',
188+
crv: 'Ed25519',
189+
created_at: new Date().toISOString(),
190+
});
191+
192+
const before = keyringReads();
193+
expect(await manager.verifyMcpAccessToken(await signWith(pair.privateKey as CryptoKey, kid)))
194+
.toMatchObject({ userId: USER, clientId: CLIENT });
195+
expect(keyringReads() - before, 'the first call paid exactly one fresh keyring read').toBe(unit);
196+
}, 60_000);
197+
198+
it('a burst of tokens naming keys the keyring does not carry costs at most one keyring read inside the floor interval', async () => {
199+
const { manager, mint, signWith, oneKeyringRead, keyringReads } = await verifierRig();
200+
expect(await manager.verifyMcpAccessToken(await mint())).toMatchObject({ userId: USER });
201+
const unit = await oneKeyringRead();
202+
expect(unit, 'the instrument sees a keyring read').toBeGreaterThan(0);
203+
204+
const stranger = await generateKeyPair('EdDSA', { crv: 'Ed25519' });
205+
const unknown = (i: number) => signWith(stranger.privateKey as CryptoKey, `key_24186_absent_${i}`);
206+
207+
const before = keyringReads();
208+
// Concurrent, then one after another: neither shape buys a second read.
209+
const concurrent = await Promise.all(
210+
Array.from({ length: 20 }, async (_, i) => manager.verifyMcpAccessToken(await unknown(i))),
211+
);
212+
const sequential: unknown[] = [];
213+
for (let i = 20; i < 40; i += 1) sequential.push(await manager.verifyMcpAccessToken(await unknown(i)));
214+
215+
expect([...concurrent, ...sequential].every((r) => r === null)).toBe(true);
216+
expect(keyringReads() - before).toBe(unit);
217+
// And a token the keyring does carry still verifies from the held set, for free.
218+
const known = await mint();
219+
const beforeKnown = keyringReads();
220+
expect(await manager.verifyMcpAccessToken(known)).toMatchObject({ userId: USER });
221+
expect(keyringReads() - beforeKnown).toBe(0);
222+
}, 60_000);
223+
});
224+
225+
describe('[#24186] the bound on the fresh read an unknown key earns', () => {
226+
const ISSUER = 'https://acme.example.com/api/v1/auth';
227+
const AUDIENCE = 'https://acme.example.com/api/v1/mcp';
228+
229+
/**
230+
* A manager whose key set is a counting double: every `getJwks()` call is
231+
* one read, numbered from 1, and `answer(n)` decides what read `n` returns
232+
* (or throws). With no reader behind it, every call is a read through.
233+
*/
234+
async function countingManager() {
235+
const known = await generateKeyPair('RS256');
236+
const rotated = await generateKeyPair('RS256');
237+
const before = { keys: [{ ...(await exportJWK(known.publicKey)), alg: 'RS256', kid: 'known' }] };
238+
const after = { keys: [...before.keys, { ...(await exportJWK(rotated.publicKey)), alg: 'RS256', kid: 'rotated' }] };
239+
const state = { reads: 0, answer: (_n: number): Promise<unknown> | unknown => before };
240+
const manager = new AuthManager({ secret: SECRET, baseUrl: 'https://acme.example.com' });
241+
vi.spyOn(manager, 'getApi').mockResolvedValue({
242+
getJwks: async () => {
243+
state.reads += 1;
244+
return state.answer(state.reads);
245+
},
246+
} as any);
247+
const token = (privateKey: CryptoKey, kid: string) => {
248+
const now = Math.floor(Date.now() / 1000);
249+
return new SignJWT({ scope: 'data:read', azp: CLIENT })
250+
.setProtectedHeader({ alg: 'RS256', kid })
251+
.setIssuer(ISSUER)
252+
.setAudience(AUDIENCE)
253+
.setSubject(USER)
254+
.setIssuedAt(now)
255+
.setExpirationTime(now + 900)
256+
.sign(privateKey);
257+
};
258+
return {
259+
manager,
260+
state,
261+
after,
262+
rotatedToken: () => token(rotated.privateKey as CryptoKey, 'rotated'),
263+
absentToken: (kid: string) => token(rotated.privateKey as CryptoKey, kid),
264+
};
265+
}
266+
267+
it('a token arriving while the fresh read is in flight shares it', async () => {
268+
const { manager, state, after, rotatedToken } = await countingManager();
269+
let open!: () => void;
270+
const gate = new Promise<void>((resolve) => { open = resolve; });
271+
// Read 2 is the fresh read: it answers the rotated set, once the gate opens.
272+
// Every other read answers the set from before the rotation.
273+
const original = state.answer;
274+
state.answer = async (n) => (n === 2 ? (await gate, after) : original(n));
275+
276+
const first = manager.verifyMcpAccessToken(await rotatedToken());
277+
await vi.waitFor(() => expect(state.reads).toBe(2));
278+
const second = manager.verifyMcpAccessToken(await rotatedToken());
279+
await vi.waitFor(() => expect(state.reads).toBe(3));
280+
// Let the second call reach the fresh read before it settles.
281+
await new Promise((resolve) => setTimeout(resolve, 25));
282+
open();
283+
284+
// The second call verifies only by joining read 2: a read of its own (read
285+
// 4) would answer the old set, and a refusal by the floor would be `null`.
286+
expect(await first).toMatchObject({ userId: USER });
287+
expect(await second).toMatchObject({ userId: USER });
288+
expect(state.reads).toBe(3);
289+
});
290+
291+
it('the floor re-arms exactly at its interval, and not a millisecond before', async () => {
292+
vi.useFakeTimers({ toFake: ['Date'] });
293+
vi.setSystemTime(new Date('2026-10-17T12:00:00Z'));
294+
const { manager, state, absentToken } = await countingManager();
295+
296+
expect(await manager.verifyMcpAccessToken(await absentToken('absent_1'))).toBeNull();
297+
expect(state.reads).toBe(2); // the set, and the one fresh read
298+
299+
vi.setSystemTime(Date.now() + UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS - 1);
300+
expect(await manager.verifyMcpAccessToken(await absentToken('absent_2'))).toBeNull();
301+
expect(state.reads).toBe(3); // the set only: still inside the floor
302+
303+
vi.setSystemTime(Date.now() + 1);
304+
expect(await manager.verifyMcpAccessToken(await absentToken('absent_3'))).toBeNull();
305+
expect(state.reads).toBe(5); // the set, and a fresh read again
306+
});
307+
308+
it('a fresh read that fails still spends the interval', async () => {
309+
const { manager, state, absentToken } = await countingManager();
310+
const original = state.answer;
311+
state.answer = (n) => {
312+
if (n === 2) throw new Error('keyring unreadable');
313+
return original(n);
314+
};
315+
316+
expect(await manager.verifyMcpAccessToken(await absentToken('absent_x'))).toBeNull();
317+
expect(await manager.verifyMcpAccessToken(await absentToken('absent_y'))).toBeNull();
318+
expect(state.reads).toBe(3); // the set and the failed fresh read, then the set alone
319+
});
320+
});

0 commit comments

Comments
 (0)