Skip to content

Commit 45c2cf9

Browse files
claude[bot]claude
andauthored
fix(plugin-auth): the MCP surface refuses a client_credentials token — principal-bound to a human (#17441)
* fix(plugin-auth): refuse a client_credentials token on the MCP surface `verifyMcpAccessToken` resolved a machine-to-machine access token to a principal while its own docblock declared such tokens rejected. The docblock's premise was that they "carry no `sub`"; the installed `@better-auth/oauth-provider` stamps `sub = user?.id ?? client.clientId`, so the premise is never true and the rejection it described could not fire. The discriminator is the `sub`/`client_id` pair as RFC 9068 defines it for a JWT access token: `client_id` is REQUIRED (§2.2) and `sub` is the resource owner when a grant had one, or an identifier for the client application when it did not (§2.2.3.1). A token whose `sub` equals its own `client_id`/`azp` therefore states that no human delegated it, and a token carrying neither client claim is refused too — the check cannot run on it and must not silently pass. Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW Co-authored-by: Claude <noreply@anthropic.com> * test(plugin-auth,runtime): pin the M2M refusal on a MINTED token The predecessor pin signed a token with no `sub` at all — a shape the provider does not mint — so it stayed green while every real client_credentials token was admitted. It is joined by a differential against a real authorization server: one server, one JWKS, two tokens, one variable (which grant produced them). The machine leg must assemble no principal; the human leg must resolve exactly as before, so a method that refuses everything cannot satisfy the pair. Claim-shape cases cover the two client spellings independently, including a token that disagrees with itself, and the no-client-claim case where the discriminator has no input. The runtime door owes the other half: a refused JWT bearer must assemble no execution context at all, not merely answer 401. Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 331a1a2 commit 45c2cf9

5 files changed

Lines changed: 335 additions & 9 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
'@objectstack/plugin-auth': patch
3+
---
4+
5+
MCP OAuth: refuse a `client_credentials` (machine-to-machine) access token
6+
7+
`AuthManager.verifyMcpAccessToken` resolved an M2M access token to a
8+
principal — a machine ran as an authenticated member, stamping a user id that
9+
belongs to no user into `created_by` / `updated_by` and owner columns — while
10+
the method's own contract declared such tokens rejected. The contract's
11+
premise was that they carry no `sub`; the OAuth provider stamps
12+
`sub = user?.id ?? client.clientId`, so the premise was never true and the
13+
rejection it described could never fire.
14+
15+
The subject and the client identity are now read as a pair, the way RFC 9068
16+
defines them for a JWT access token: `client_id` is REQUIRED (§2.2), and `sub`
17+
is the resource owner for a grant that had one or an identifier for the client
18+
application for a grant that did not (§2.2.3.1). A token whose `sub` equals its
19+
own `client_id` / `azp` therefore assembles no principal, and the MCP HTTP door
20+
answers `401`. A token carrying neither client claim is refused as well: the
21+
check has no input, and a check that cannot run must not silently pass.
22+
23+
Unchanged: interactive OAuth clients (authorization code + PKCE) resolve
24+
exactly as before, and the headless track is untouched — `x-api-key` /
25+
`Bearer osk_…` over HTTP and `OS_MCP_STDIO_API_KEY` over stdio are a separate
26+
chain with a separate credential shape, and remain the supported way for a
27+
machine to call this platform.

packages/plugins/plugin-auth/src/auth-manager.mcp-oauth-resource.test.ts

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,20 @@
3737
* run, one variable apart — so it reddens from either side: remove the
3838
* registration and the accepted half fails; widen it and the refused half
3939
* does.
40+
* 5. [#16418] The principal-binding block does the same for
41+
* `verifyMcpAccessToken`: it mints a REAL `client_credentials` token from
42+
* this server and hands it to a real AuthManager verifying against this
43+
* server's JWKS. The refusal it pins used to be asserted against a
44+
* HAND-BUILT token with no `sub` at all — a shape the provider does not
45+
* mint — so that assertion passed for years while the method admitted
46+
* every real M2M token. A minted token is the only subject that can tell
47+
* those two apart, and the user leg beside it is the differential: same
48+
* server, same JWKS, same audience, one variable (which grant produced the
49+
* token).
4050
*/
4151

4252
import { createRequire } from 'node:module';
53+
import { createHash } from 'node:crypto';
4354
import path from 'node:path';
4455
import fs from 'node:fs';
4556

@@ -291,6 +302,95 @@ function decodeJwtPayload(token: string): any {
291302
return JSON.parse(Buffer.from(parts[1]!, 'base64url').toString('utf8'));
292303
}
293304

305+
/**
306+
* The at-rest form the installed provider expects for a client secret. 1.7.2
307+
* defaults `storeClientSecret` to `"hashed"` whenever the jwt plugin is on
308+
* (it is here), and hashes with SHA-256 → unpadded base64url. Seeding the raw
309+
* secret instead produces `invalid_client`, i.e. NO token — which the mint
310+
* assertions below turn into a loud failure rather than a quiet "refused".
311+
*/
312+
function storedClientSecret(secret: string): string {
313+
return createHash('sha256').update(secret).digest('base64url');
314+
}
315+
316+
const M2M_CLIENT_ID = 'headless-integration-client';
317+
const M2M_CLIENT_SECRET = 'headless-integration-secret';
318+
319+
/**
320+
* Registers a CONFIDENTIAL `client_credentials` client on the running AS and
321+
* links it to the MCP resource — the shape #16418's trace names: a client row
322+
* carrying `client_credentials_scopes`, plus the `oauthClientResource` link
323+
* `enforcePerClientResources` requires.
324+
*
325+
* ⚠️ Seeded through the AS's OWN adapter, not by pushing a row into the store:
326+
* the memory adapter persists under the schema's `fieldName` mapping
327+
* (`client_credentials_scopes`, not `clientCredentialsScopes`), so a raw push
328+
* is not found and the grant fails as "missing client" — a refusal for the
329+
* wrong reason. It is also NOT registered through DCR, because 1.7.2 refuses
330+
* `client_credentials` in an unauthenticated registration and only an
331+
* administrative registration may set the scope ceiling.
332+
*/
333+
async function seedClientCredentialsClient(server: { auth: any; pluginSchema?: any }) {
334+
const ctx = await server.auth.$context;
335+
await ctx.adapter.create({
336+
model: 'oauthClient',
337+
data: {
338+
clientId: M2M_CLIENT_ID,
339+
clientSecret: storedClientSecret(M2M_CLIENT_SECRET),
340+
name: 'Headless integration',
341+
redirectUris: [REDIRECT_URI],
342+
grantTypes: ['client_credentials'],
343+
responseTypes: [],
344+
tokenEndpointAuthMethod: 'client_secret_post',
345+
scopes: ['data:read'],
346+
clientCredentialsScopes: ['data:read'],
347+
disabled: false,
348+
createdAt: new Date(),
349+
updatedAt: new Date(),
350+
},
351+
});
352+
await ctx.adapter.create({
353+
model: 'oauthClientResource',
354+
data: { clientId: M2M_CLIENT_ID, resourceId: MCP_RESOURCE, createdAt: new Date() },
355+
});
356+
}
357+
358+
/** Runs the real `client_credentials` grant and returns the minted token. */
359+
async function mintClientCredentialsToken(server: { auth: any }): Promise<string> {
360+
const res = await server.auth.handler(
361+
new Request(`${ISSUER}/oauth2/token`, {
362+
method: 'POST',
363+
headers: { 'content-type': 'application/x-www-form-urlencoded' },
364+
body: new URLSearchParams({
365+
grant_type: 'client_credentials',
366+
client_id: M2M_CLIENT_ID,
367+
client_secret: M2M_CLIENT_SECRET,
368+
scope: 'data:read',
369+
resource: MCP_RESOURCE,
370+
}).toString(),
371+
}),
372+
);
373+
const body: any = await res.json().catch(() => null);
374+
// ⛔ "the grant failed" must never be spellable as "the door refused it".
375+
expect(res.status, `the client_credentials grant did not mint a token: ${JSON.stringify(body)}`).toBe(200);
376+
expect(body?.access_token, 'no M2M access token was minted').toBeTruthy();
377+
return body.access_token as string;
378+
}
379+
380+
/**
381+
* An AuthManager whose JWKS comes from the RUNNING authorization server, so
382+
* `verifyMcpAccessToken` verifies signatures the same server produced. Issuer
383+
* and audience already agree by construction (both derive from BASE_URL).
384+
*/
385+
function managerVerifyingAgainst(server: { auth: any }): AuthManager {
386+
process.env.OS_MCP_SERVER_ENABLED = 'true';
387+
const m = new AuthManager({ secret: 'test-secret-at-least-32-chars-long', baseUrl: BASE_URL });
388+
vi.spyOn(m, 'getApi').mockResolvedValue({
389+
getJwks: async () => await server.auth.api.getJwks(),
390+
} as any);
391+
return m;
392+
}
393+
294394
describe('oauthProvider option surface liveness (installed 1.7.2)', () => {
295395
// Two-way control on the scanner itself: it must be able to answer BOTH
296396
// "present" and "absent", or a 0-hit reading proves nothing.
@@ -521,3 +621,86 @@ describe('MCP resource registration against the real provider (RFC 8707)', () =>
521621
expect(tokenBody?.access_token, 'no token may be minted for an unbound resource').toBeFalsy();
522622
});
523623
});
624+
625+
describe('[#16418] MCP is principal-bound — a minted client_credentials token resolves to NO principal', () => {
626+
it('mints a REAL M2M token whose `sub` is the client id and which carries no `sid` (the claim reading, off the token)', async () => {
627+
const opts = await captureProviderOptions();
628+
const server = await bootRealAuthorizationServer(opts);
629+
await seedClientCredentialsClient(server);
630+
631+
const payload = decodeJwtPayload(await mintClientCredentialsToken(server));
632+
633+
// Re-derive #3 from the card, kept live: the subject is read OFF THE
634+
// TOKEN, never inferred from the provider's source. This is the fact the
635+
// docblock used to deny ("carries no `sub`").
636+
expect(payload.sub, 'the M2M token must carry a subject at all').toBeTruthy();
637+
expect(payload.sub, "and that subject is the CLIENT — RFC 9068 §2.2.3.1's no-resource-owner shape").toBe(
638+
M2M_CLIENT_ID,
639+
);
640+
expect(payload.client_id).toBe(M2M_CLIENT_ID);
641+
expect(payload.azp).toBe(M2M_CLIENT_ID);
642+
// Measured absence, recorded because it names the discriminator this fix
643+
// deliberately did NOT choose: `sid` separates the two shapes today, but
644+
// it is upstream-optional (already gated per client on ID tokens), so
645+
// relying on it would 401 every human the moment a bump gated it here.
646+
expect(payload.sid, 'no session exists behind a client_credentials grant').toBeUndefined();
647+
});
648+
649+
it('DIFFERENTIAL: same server, same JWKS — the user token resolves, the M2M token does not', async () => {
650+
const opts = await captureProviderOptions();
651+
const server = await bootRealAuthorizationServer(opts);
652+
await seedClientCredentialsClient(server);
653+
const manager = managerVerifyingAgainst(server);
654+
655+
// -- machine leg -------------------------------------------------------
656+
const m2mToken = await mintClientCredentialsToken(server);
657+
expect(
658+
await manager.verifyMcpAccessToken(m2mToken),
659+
'a client_credentials token must assemble NO principal on the MCP surface — '
660+
+ 'headless callers use API keys (ADR-0101 D1), which is a separate chain entirely',
661+
).toBeNull();
662+
663+
// -- human leg (the negative control) ----------------------------------
664+
// The full flow on the SAME server: DCR → sign-up → authorize → consent →
665+
// token. If this half went red the refusal above would be worthless — a
666+
// method that refuses everything satisfies it.
667+
const reg = await registerDcrClient(server.auth);
668+
expect(reg.status, JSON.stringify(reg.body)).toBe(201);
669+
const cookie = await signUp(server.auth);
670+
const az = await authorizeWithResource(server.auth, reg.body.client_id, cookie, MCP_RESOURCE);
671+
expect(az.location).not.toContain('invalid_target');
672+
const code = await consentToCode(server.auth, az.location, cookie);
673+
const tokenRes = await server.auth.handler(
674+
new Request(`${ISSUER}/oauth2/token`, {
675+
method: 'POST',
676+
headers: { 'content-type': 'application/x-www-form-urlencoded' },
677+
body: new URLSearchParams({
678+
grant_type: 'authorization_code',
679+
code,
680+
redirect_uri: REDIRECT_URI,
681+
client_id: reg.body.client_id,
682+
code_verifier: PKCE_VERIFIER,
683+
resource: MCP_RESOURCE,
684+
}).toString(),
685+
}),
686+
);
687+
const tokenBody: any = await tokenRes.json().catch(() => null);
688+
expect(tokenRes.status, JSON.stringify(tokenBody)).toBe(200);
689+
const userToken: string = tokenBody.access_token;
690+
const userPayload = decodeJwtPayload(userToken);
691+
692+
expect(
693+
await manager.verifyMcpAccessToken(userToken),
694+
'an authorization-code token must still resolve — this narrows the M2M shape and nothing else',
695+
).toEqual({
696+
userId: userPayload.sub,
697+
scopes: ['openid', 'profile', 'email', 'offline_access', 'data:read'],
698+
clientId: reg.body.client_id,
699+
});
700+
701+
// The one variable between the two legs, stated as an assertion: the
702+
// human token's subject is NOT its client, the machine token's subject IS.
703+
expect(userPayload.sub).not.toBe(userPayload.client_id);
704+
expect(decodeJwtPayload(m2mToken).sub).toBe(decodeJwtPayload(m2mToken).client_id);
705+
});
706+
});

packages/plugins/plugin-auth/src/auth-manager.mcp-oauth.test.ts

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,9 @@ describe('verifyMcpAccessToken (local JWKS verification, fail-closed)', () => {
200200

201201
function signToken(overrides: Record<string, unknown> = {}, opts: { expired?: boolean } = {}) {
202202
const now = Math.floor(Date.now() / 1000);
203+
// `undefined` in an override DROPS the claim: jose serialises the payload
204+
// with JSON.stringify, which omits undefined values. That is how the
205+
// no-client-claim case below is expressed without a second signer.
203206
const jwt = new SignJWT({
204207
scope: 'data:read data:write',
205208
azp: 'client-abc',
@@ -249,9 +252,9 @@ describe('verifyMcpAccessToken (local JWKS verification, fail-closed)', () => {
249252
expect(await manager().verifyMcpAccessToken(token)).toBeNull();
250253
});
251254

252-
it('rejects a sub-less (client-credentials / M2M) token — MCP is principal-bound', async () => {
255+
it('rejects a sub-less token — a subject is the minimum a principal can be built from', async () => {
253256
const now = Math.floor(Date.now() / 1000);
254-
const token = await new SignJWT({ scope: 'data:read' })
257+
const token = await new SignJWT({ scope: 'data:read', azp: 'client-abc' })
255258
.setProtectedHeader({ alg: 'RS256', kid: 'test-key' })
256259
.setIssuer(ISSUER)
257260
.setAudience(AUDIENCE)
@@ -261,6 +264,51 @@ describe('verifyMcpAccessToken (local JWKS verification, fail-closed)', () => {
261264
expect(await manager().verifyMcpAccessToken(token)).toBeNull();
262265
});
263266

267+
// ── the client_credentials discriminator, by CLAIM SHAPE ─────────────────
268+
// These pin the rule on hand-built claim combinations, including ones no
269+
// grant produces today (a token disagreeing with itself across the two
270+
// client spellings). The behaviour on a token the REAL provider actually
271+
// mints for a `client_credentials` grant is pinned in
272+
// auth-manager.mcp-oauth-resource.test.ts, against a real authorization
273+
// server — the docblock's former "carries no `sub`" premise was green here
274+
// for years precisely because no minted token was ever handed to it.
275+
276+
it('rejects a token whose `sub` IS its `azp` — RFC 9068 §2.2.3.1: no resource owner was involved', async () => {
277+
const token = await signToken({ sub: 'client-abc', azp: 'client-abc' });
278+
expect(await manager().verifyMcpAccessToken(token)).toBeNull();
279+
});
280+
281+
it('rejects a token whose `sub` IS its `client_id` — the RFC 9068 §2.2 spelling of the same fact', async () => {
282+
const token = await signToken({ sub: 'client-abc', client_id: 'client-abc', azp: undefined });
283+
expect(await manager().verifyMcpAccessToken(token)).toBeNull();
284+
});
285+
286+
it('refuses on EITHER client spelling — a token that disagrees with itself is still refused', async () => {
287+
// `azp` says one client, `client_id` says another, and `sub` matches the
288+
// one a `??` chain would have discarded. Read as a pair, this is refused;
289+
// read through a precedence chain, it resolves.
290+
const token = await signToken({ sub: 'client-two', client_id: 'client-two', azp: 'client-one' });
291+
expect(await manager().verifyMcpAccessToken(token)).toBeNull();
292+
});
293+
294+
it('rejects a token carrying NEITHER `client_id` nor `azp` — the discriminator cannot run, so it must not pass', async () => {
295+
const token = await signToken({ azp: undefined });
296+
expect(await manager().verifyMcpAccessToken(token)).toBeNull();
297+
});
298+
299+
it('still resolves a delegated token that carries `client_id` and `azp` alongside a DIFFERENT `sub`', async () => {
300+
// The positive half of the pair rule, on the claim set a real
301+
// authorization-code token carries (measured: `client_id` === `azp`,
302+
// both != `sub`). Without this, the four refusals above are also
303+
// satisfied by a method that refuses everything.
304+
const token = await signToken({ sub: 'user-1', client_id: 'client-abc', azp: 'client-abc' });
305+
expect(await manager().verifyMcpAccessToken(token)).toEqual({
306+
userId: 'user-1',
307+
scopes: ['data:read', 'data:write'],
308+
clientId: 'client-abc',
309+
});
310+
});
311+
264312
it('rejects garbage / non-JWT input without touching the JWKS', async () => {
265313
const m = manager();
266314
expect(await m.verifyMcpAccessToken('')).toBeNull();

packages/plugins/plugin-auth/src/auth-manager.ts

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6130,9 +6130,33 @@ export class AuthManager {
61306130
* signature against our own JWKS, `iss` must be this deployment's issuer,
61316131
* `aud` must be the MCP resource URL (tokens minted for other audiences —
61326132
* userinfo, plain OIDC SSO — do NOT unlock MCP), `exp`/`nbf` enforced by
6133-
* jose. Client-credentials (M2M) tokens carry no `sub` and are rejected:
6134-
* the MCP surface is principal-bound by design; headless callers use API
6135-
* keys. Revocation note: JWT access tokens are not server-tracked, so
6133+
* jose.
6134+
*
6135+
* A `client_credentials` (M2M) token is REFUSED here — the MCP surface is
6136+
* principal-bound by design; headless callers use API keys. The
6137+
* discriminator is the `sub`/`client_id` PAIR, read as RFC 9068 defines it
6138+
* for a JWT access token: §2.2 makes `client_id` REQUIRED, and §2.2.3.1
6139+
* fixes what `sub` means beside it — the resource OWNER for a grant that
6140+
* had one, and "an identifier the authorization server uses to indicate the
6141+
* client application" for a grant that did not. So a token whose `sub`
6142+
* equals its own `client_id` (or its `azp` spelling) states, in the
6143+
* authorization server's own words, that NO human delegated it, and a token
6144+
* carrying neither client claim is refused as well: the check cannot run on
6145+
* it, and a check that cannot run must not silently pass (Route & surface
6146+
* ownership §3).
6147+
*
6148+
* ⛔ Not `sid`, and ⛔ not a `sys_user` lookup. `sid` does separate today's
6149+
* two token shapes, but it is an OIDC session-management convenience the
6150+
* installed provider ALREADY gates per-client on ID tokens
6151+
* (`enableEndSession || backchannelLogoutUri`) — a bump that gates it on
6152+
* access tokens too would 401 every human on this surface, which is the
6153+
* failure this method must not have. A `sys_user` read would make a
6154+
* deliberately LOCAL, I/O-free verifier depend on the data engine and turn
6155+
* a transient store error into a 401 for a legitimate human, and a row's
6156+
* existence is not humanity anyway (`isHumanUserRow` exists because
6157+
* `usr_system` is a row and not a person).
6158+
*
6159+
* Revocation note: JWT access tokens are not server-tracked, so
61366160
* revocation takes effect at expiry (≤1h default); refresh tokens ARE
61376161
* revocable immediately via `/oauth2/revoke`.
61386162
*
@@ -6163,14 +6187,34 @@ export class AuthManager {
61636187
audience: this.getMcpResourceUrl(),
61646188
});
61656189

6166-
const userId = typeof payload.sub === 'string' && payload.sub ? payload.sub : undefined;
6167-
if (!userId) return null;
6190+
const subject = typeof payload.sub === 'string' && payload.sub ? payload.sub : undefined;
6191+
if (!subject) return null;
6192+
6193+
// The two spellings of "which client is presenting this", read
6194+
// independently rather than through a `??` chain: a token that carries
6195+
// both and disagrees with itself must be refused on EITHER match, and
6196+
// collapsing them first would let the losing spelling smuggle the
6197+
// client id past the comparison below.
6198+
const clientIdClaim =
6199+
typeof (payload as any).client_id === 'string' && (payload as any).client_id
6200+
? ((payload as any).client_id as string)
6201+
: undefined;
6202+
const azp =
6203+
typeof (payload as any).azp === 'string' && (payload as any).azp
6204+
? ((payload as any).azp as string)
6205+
: undefined;
6206+
// No client identity at all → the human/machine discriminator has no
6207+
// input. Fail closed rather than admit an unclassifiable token.
6208+
if (!clientIdClaim && !azp) return null;
6209+
// `sub` IS the client → RFC 9068 §2.2.3.1's "no resource owner was
6210+
// involved" shape, i.e. a client_credentials grant. No principal.
6211+
if (subject === clientIdClaim || subject === azp) return null;
6212+
61686213
const scopes =
61696214
typeof payload.scope === 'string'
61706215
? payload.scope.split(' ').filter(Boolean)
61716216
: [];
6172-
const clientId = typeof (payload as any).azp === 'string' ? (payload as any).azp : undefined;
6173-
return { userId, scopes, ...(clientId ? { clientId } : {}) };
6217+
return { userId: subject, scopes, ...(azp ? { clientId: azp } : {}) };
61746218
} catch {
61756219
return null; // unknown/expired/wrong-audience/garbage → no principal
61766220
}

0 commit comments

Comments
 (0)