fix(plugin-auth): the MCP surface refuses a client_credentials token — principal-bound to a human

.changeset/mcp-token-human-principal.md

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

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

@@ -37,9 +37,20 @@
  *    run, one variable apart — so it reddens from either side: remove the
  *    registration and the accepted half fails; widen it and the refused half
  *    does.
+ * 5. [#16418] The principal-binding block does the same for
+ *    `verifyMcpAccessToken`: it mints a REAL `client_credentials` token from
+ *    this server and hands it to a real AuthManager verifying against this
+ *    server's JWKS. The refusal it pins used to be asserted against a
+ *    HAND-BUILT token with no `sub` at all — a shape the provider does not
+ *    mint — so that assertion passed for years while the method admitted
+ *    every real M2M token. A minted token is the only subject that can tell
+ *    those two apart, and the user leg beside it is the differential: same
+ *    server, same JWKS, same audience, one variable (which grant produced the
+ *    token).
  */
 
 import { createRequire } from 'node:module';
+import { createHash } from 'node:crypto';
 import path from 'node:path';
 import fs from 'node:fs';
 
@@ -291,6 +302,95 @@ function decodeJwtPayload(token: string): any {
   return JSON.parse(Buffer.from(parts[1]!, 'base64url').toString('utf8'));
 }
 
+/**
+ * The at-rest form the installed provider expects for a client secret. 1.7.2
+ * defaults `storeClientSecret` to `"hashed"` whenever the jwt plugin is on
+ * (it is here), and hashes with SHA-256 → unpadded base64url. Seeding the raw
+ * secret instead produces `invalid_client`, i.e. NO token — which the mint
+ * assertions below turn into a loud failure rather than a quiet "refused".
+ */
+function storedClientSecret(secret: string): string {
+  return createHash('sha256').update(secret).digest('base64url');
+}
+
+const M2M_CLIENT_ID = 'headless-integration-client';
+const M2M_CLIENT_SECRET = 'headless-integration-secret';
+
+/**
+ * Registers a CONFIDENTIAL `client_credentials` client on the running AS and
+ * links it to the MCP resource — the shape #16418's trace names: a client row
+ * carrying `client_credentials_scopes`, plus the `oauthClientResource` link
+ * `enforcePerClientResources` requires.
+ *
+ * ⚠️ Seeded through the AS's OWN adapter, not by pushing a row into the store:
+ * the memory adapter persists under the schema's `fieldName` mapping
+ * (`client_credentials_scopes`, not `clientCredentialsScopes`), so a raw push
+ * is not found and the grant fails as "missing client" — a refusal for the
+ * wrong reason. It is also NOT registered through DCR, because 1.7.2 refuses
+ * `client_credentials` in an unauthenticated registration and only an
+ * administrative registration may set the scope ceiling.
+ */
+async function seedClientCredentialsClient(server: { auth: any; pluginSchema?: any }) {
+  const ctx = await server.auth.$context;
+  await ctx.adapter.create({
+    model: 'oauthClient',
+    data: {
+      clientId: M2M_CLIENT_ID,
+      clientSecret: storedClientSecret(M2M_CLIENT_SECRET),
+      name: 'Headless integration',
+      redirectUris: [REDIRECT_URI],
+      grantTypes: ['client_credentials'],
+      responseTypes: [],
+      tokenEndpointAuthMethod: 'client_secret_post',
+      scopes: ['data:read'],
+      clientCredentialsScopes: ['data:read'],
+      disabled: false,
+      createdAt: new Date(),
+      updatedAt: new Date(),
+    },
+  });
+  await ctx.adapter.create({
+    model: 'oauthClientResource',
+    data: { clientId: M2M_CLIENT_ID, resourceId: MCP_RESOURCE, createdAt: new Date() },
+  });
+}
+
+/** Runs the real `client_credentials` grant and returns the minted token. */
+async function mintClientCredentialsToken(server: { auth: any }): Promise<string> {
+  const res = await server.auth.handler(
+    new Request(`${ISSUER}/oauth2/token`, {
+      method: 'POST',
+      headers: { 'content-type': 'application/x-www-form-urlencoded' },
+      body: new URLSearchParams({
+        grant_type: 'client_credentials',
+        client_id: M2M_CLIENT_ID,
+        client_secret: M2M_CLIENT_SECRET,
+        scope: 'data:read',
+        resource: MCP_RESOURCE,
+      }).toString(),
+    }),
+  );
+  const body: any = await res.json().catch(() => null);
+  // ⛔ "the grant failed" must never be spellable as "the door refused it".
+  expect(res.status, `the client_credentials grant did not mint a token: ${JSON.stringify(body)}`).toBe(200);
+  expect(body?.access_token, 'no M2M access token was minted').toBeTruthy();
+  return body.access_token as string;
+}
+
+/**
+ * An AuthManager whose JWKS comes from the RUNNING authorization server, so
+ * `verifyMcpAccessToken` verifies signatures the same server produced. Issuer
+ * and audience already agree by construction (both derive from BASE_URL).
+ */
+function managerVerifyingAgainst(server: { auth: any }): AuthManager {
+  process.env.OS_MCP_SERVER_ENABLED = 'true';
+  const m = new AuthManager({ secret: 'test-secret-at-least-32-chars-long', baseUrl: BASE_URL });
+  vi.spyOn(m, 'getApi').mockResolvedValue({
+    getJwks: async () => await server.auth.api.getJwks(),
+  } as any);
+  return m;
+}
+
 describe('oauthProvider option surface liveness (installed 1.7.2)', () => {
   // Two-way control on the scanner itself: it must be able to answer BOTH
   // "present" and "absent", or a 0-hit reading proves nothing.
@@ -521,3 +621,86 @@ describe('MCP resource registration against the real provider (RFC 8707)', () =>
     expect(tokenBody?.access_token, 'no token may be minted for an unbound resource').toBeFalsy();
   });
 });
+
+describe('[#16418] MCP is principal-bound — a minted client_credentials token resolves to NO principal', () => {
+  it('mints a REAL M2M token whose `sub` is the client id and which carries no `sid` (the claim reading, off the token)', async () => {
+    const opts = await captureProviderOptions();
+    const server = await bootRealAuthorizationServer(opts);
+    await seedClientCredentialsClient(server);
+
+    const payload = decodeJwtPayload(await mintClientCredentialsToken(server));
+
+    // Re-derive #3 from the card, kept live: the subject is read OFF THE
+    // TOKEN, never inferred from the provider's source. This is the fact the
+    // docblock used to deny ("carries no `sub`").
+    expect(payload.sub, 'the M2M token must carry a subject at all').toBeTruthy();
+    expect(payload.sub, "and that subject is the CLIENT — RFC 9068 §2.2.3.1's no-resource-owner shape").toBe(
+      M2M_CLIENT_ID,
+    );
+    expect(payload.client_id).toBe(M2M_CLIENT_ID);
+    expect(payload.azp).toBe(M2M_CLIENT_ID);
+    // Measured absence, recorded because it names the discriminator this fix
+    // deliberately did NOT choose: `sid` separates the two shapes today, but
+    // it is upstream-optional (already gated per client on ID tokens), so
+    // relying on it would 401 every human the moment a bump gated it here.
+    expect(payload.sid, 'no session exists behind a client_credentials grant').toBeUndefined();
+  });
+
+  it('DIFFERENTIAL: same server, same JWKS — the user token resolves, the M2M token does not', async () => {
+    const opts = await captureProviderOptions();
+    const server = await bootRealAuthorizationServer(opts);
+    await seedClientCredentialsClient(server);
+    const manager = managerVerifyingAgainst(server);
+
+    // -- machine leg -------------------------------------------------------
+    const m2mToken = await mintClientCredentialsToken(server);
+    expect(
+      await manager.verifyMcpAccessToken(m2mToken),
+      'a client_credentials token must assemble NO principal on the MCP surface — '
+        + 'headless callers use API keys (ADR-0101 D1), which is a separate chain entirely',
+    ).toBeNull();
+
+    // -- human leg (the negative control) ----------------------------------
+    // The full flow on the SAME server: DCR → sign-up → authorize → consent →
+    // token. If this half went red the refusal above would be worthless — a
+    // method that refuses everything satisfies it.
+    const reg = await registerDcrClient(server.auth);
+    expect(reg.status, JSON.stringify(reg.body)).toBe(201);
+    const cookie = await signUp(server.auth);
+    const az = await authorizeWithResource(server.auth, reg.body.client_id, cookie, MCP_RESOURCE);
+    expect(az.location).not.toContain('invalid_target');
+    const code = await consentToCode(server.auth, az.location, cookie);
+    const tokenRes = await server.auth.handler(
+      new Request(`${ISSUER}/oauth2/token`, {
+        method: 'POST',
+        headers: { 'content-type': 'application/x-www-form-urlencoded' },
+        body: new URLSearchParams({
+          grant_type: 'authorization_code',
+          code,
+          redirect_uri: REDIRECT_URI,
+          client_id: reg.body.client_id,
+          code_verifier: PKCE_VERIFIER,
+          resource: MCP_RESOURCE,
+        }).toString(),
+      }),
+    );
+    const tokenBody: any = await tokenRes.json().catch(() => null);
+    expect(tokenRes.status, JSON.stringify(tokenBody)).toBe(200);
+    const userToken: string = tokenBody.access_token;
+    const userPayload = decodeJwtPayload(userToken);
+
+    expect(
+      await manager.verifyMcpAccessToken(userToken),
+      'an authorization-code token must still resolve — this narrows the M2M shape and nothing else',
+    ).toEqual({
+      userId: userPayload.sub,
+      scopes: ['openid', 'profile', 'email', 'offline_access', 'data:read'],
+      clientId: reg.body.client_id,
+    });
+
+    // The one variable between the two legs, stated as an assertion: the
+    // human token's subject is NOT its client, the machine token's subject IS.
+    expect(userPayload.sub).not.toBe(userPayload.client_id);
+    expect(decodeJwtPayload(m2mToken).sub).toBe(decodeJwtPayload(m2mToken).client_id);
+  });
+});

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

@@ -200,6 +200,9 @@ describe('verifyMcpAccessToken (local JWKS verification, fail-closed)', () => {
 
   function signToken(overrides: Record<string, unknown> = {}, opts: { expired?: boolean } = {}) {
     const now = Math.floor(Date.now() / 1000);
+    // `undefined` in an override DROPS the claim: jose serialises the payload
+    // with JSON.stringify, which omits undefined values. That is how the
+    // no-client-claim case below is expressed without a second signer.
     const jwt = new SignJWT({
       scope: 'data:read data:write',
       azp: 'client-abc',
@@ -249,9 +252,9 @@ describe('verifyMcpAccessToken (local JWKS verification, fail-closed)', () => {
     expect(await manager().verifyMcpAccessToken(token)).toBeNull();
   });
 
-  it('rejects a sub-less (client-credentials / M2M) token — MCP is principal-bound', async () => {
+  it('rejects a sub-less token — a subject is the minimum a principal can be built from', async () => {
     const now = Math.floor(Date.now() / 1000);
-    const token = await new SignJWT({ scope: 'data:read' })
+    const token = await new SignJWT({ scope: 'data:read', azp: 'client-abc' })
       .setProtectedHeader({ alg: 'RS256', kid: 'test-key' })
       .setIssuer(ISSUER)
       .setAudience(AUDIENCE)
@@ -261,6 +264,51 @@ describe('verifyMcpAccessToken (local JWKS verification, fail-closed)', () => {
     expect(await manager().verifyMcpAccessToken(token)).toBeNull();
   });
 
+  // ── the client_credentials discriminator, by CLAIM SHAPE ─────────────────
+  // These pin the rule on hand-built claim combinations, including ones no
+  // grant produces today (a token disagreeing with itself across the two
+  // client spellings). The behaviour on a token the REAL provider actually
+  // mints for a `client_credentials` grant is pinned in
+  // auth-manager.mcp-oauth-resource.test.ts, against a real authorization
+  // server — the docblock's former "carries no `sub`" premise was green here
+  // for years precisely because no minted token was ever handed to it.
+
+  it('rejects a token whose `sub` IS its `azp` — RFC 9068 §2.2.3.1: no resource owner was involved', async () => {
+    const token = await signToken({ sub: 'client-abc', azp: 'client-abc' });
+    expect(await manager().verifyMcpAccessToken(token)).toBeNull();
+  });
+
+  it('rejects a token whose `sub` IS its `client_id` — the RFC 9068 §2.2 spelling of the same fact', async () => {
+    const token = await signToken({ sub: 'client-abc', client_id: 'client-abc', azp: undefined });
+    expect(await manager().verifyMcpAccessToken(token)).toBeNull();
+  });
+
+  it('refuses on EITHER client spelling — a token that disagrees with itself is still refused', async () => {
+    // `azp` says one client, `client_id` says another, and `sub` matches the
+    // one a `??` chain would have discarded. Read as a pair, this is refused;
+    // read through a precedence chain, it resolves.
+    const token = await signToken({ sub: 'client-two', client_id: 'client-two', azp: 'client-one' });
+    expect(await manager().verifyMcpAccessToken(token)).toBeNull();
+  });
+
+  it('rejects a token carrying NEITHER `client_id` nor `azp` — the discriminator cannot run, so it must not pass', async () => {
+    const token = await signToken({ azp: undefined });
+    expect(await manager().verifyMcpAccessToken(token)).toBeNull();
+  });
+
+  it('still resolves a delegated token that carries `client_id` and `azp` alongside a DIFFERENT `sub`', async () => {
+    // The positive half of the pair rule, on the claim set a real
+    // authorization-code token carries (measured: `client_id` === `azp`,
+    // both != `sub`). Without this, the four refusals above are also
+    // satisfied by a method that refuses everything.
+    const token = await signToken({ sub: 'user-1', client_id: 'client-abc', azp: 'client-abc' });
+    expect(await manager().verifyMcpAccessToken(token)).toEqual({
+      userId: 'user-1',
+      scopes: ['data:read', 'data:write'],
+      clientId: 'client-abc',
+    });
+  });
+
   it('rejects garbage / non-JWT input without touching the JWKS', async () => {
     const m = manager();
     expect(await m.verifyMcpAccessToken('')).toBeNull();

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

@@ -6130,9 +6130,33 @@ export class AuthManager {
    * signature against our own JWKS, `iss` must be this deployment's issuer,
    * `aud` must be the MCP resource URL (tokens minted for other audiences —
    * userinfo, plain OIDC SSO — do NOT unlock MCP), `exp`/`nbf` enforced by
-   * jose. Client-credentials (M2M) tokens carry no `sub` and are rejected:
-   * the MCP surface is principal-bound by design; headless callers use API
-   * keys. Revocation note: JWT access tokens are not server-tracked, so
+   * jose.
+   *
+   * A `client_credentials` (M2M) token is REFUSED here — the MCP surface is
+   * principal-bound by design; headless callers use API keys. The
+   * discriminator is the `sub`/`client_id` PAIR, read as RFC 9068 defines it
+   * for a JWT access token: §2.2 makes `client_id` REQUIRED, and §2.2.3.1
+   * fixes what `sub` means beside it — the resource OWNER for a grant that
+   * had one, and "an identifier the authorization server uses to indicate the
+   * client application" for a grant that did not. So a token whose `sub`
+   * equals its own `client_id` (or its `azp` spelling) states, in the
+   * authorization server's own words, that NO human delegated it, and a token
+   * carrying neither client claim is refused as well: the check cannot run on
+   * it, and a check that cannot run must not silently pass (Route & surface
+   * ownership §3).
+   *
+   * ⛔ Not `sid`, and ⛔ not a `sys_user` lookup. `sid` does separate today's
+   * two token shapes, but it is an OIDC session-management convenience the
+   * installed provider ALREADY gates per-client on ID tokens
+   * (`enableEndSession || backchannelLogoutUri`) — a bump that gates it on
+   * access tokens too would 401 every human on this surface, which is the
+   * failure this method must not have. A `sys_user` read would make a
+   * deliberately LOCAL, I/O-free verifier depend on the data engine and turn
+   * a transient store error into a 401 for a legitimate human, and a row's
+   * existence is not humanity anyway (`isHumanUserRow` exists because
+   * `usr_system` is a row and not a person).
+   *
+   * Revocation note: JWT access tokens are not server-tracked, so
    * revocation takes effect at expiry (≤1h default); refresh tokens ARE
    * revocable immediately via `/oauth2/revoke`.
    *
@@ -6163,14 +6187,34 @@ export class AuthManager {
         audience: this.getMcpResourceUrl(),
       });
 
-      const userId = typeof payload.sub === 'string' && payload.sub ? payload.sub : undefined;
-      if (!userId) return null;
+      const subject = typeof payload.sub === 'string' && payload.sub ? payload.sub : undefined;
+      if (!subject) return null;
+
+      // The two spellings of "which client is presenting this", read
+      // independently rather than through a `??` chain: a token that carries
+      // both and disagrees with itself must be refused on EITHER match, and
+      // collapsing them first would let the losing spelling smuggle the
+      // client id past the comparison below.
+      const clientIdClaim =
+        typeof (payload as any).client_id === 'string' && (payload as any).client_id
+          ? ((payload as any).client_id as string)
+          : undefined;
+      const azp =
+        typeof (payload as any).azp === 'string' && (payload as any).azp
+          ? ((payload as any).azp as string)
+          : undefined;
+      // No client identity at all → the human/machine discriminator has no
+      // input. Fail closed rather than admit an unclassifiable token.
+      if (!clientIdClaim && !azp) return null;
+      // `sub` IS the client → RFC 9068 §2.2.3.1's "no resource owner was
+      // involved" shape, i.e. a client_credentials grant. No principal.
+      if (subject === clientIdClaim || subject === azp) return null;
+
       const scopes =
         typeof payload.scope === 'string'
           ? payload.scope.split(' ').filter(Boolean)
           : [];
-      const clientId = typeof (payload as any).azp === 'string' ? (payload as any).azp : undefined;
-      return { userId, scopes, ...(clientId ? { clientId } : {}) };
+      return { userId: subject, scopes, ...(azp ? { clientId: azp } : {}) };
     } catch {
       return null; // unknown/expired/wrong-audience/garbage → no principal
     }