fix(plugin-auth): register the MCP resource so RFC 8707 authorize can succeed

.changeset/mcp-oauth-resource-registration.md

@@ -0,0 +1,19 @@
+---
+"@objectstack/plugin-auth": minor
+---
+
+MCP OAuth can complete again: the MCP resource is registered as an RFC 8707 resource and DCR-registered clients are linked to it, so `authorize?resource=<mcp url>` no longer answers `invalid_target`.
+
+On 17.3.0 no MCP client could ever obtain a token. `plugin-auth` configured `@better-auth/oauth-provider` with `validAudiences: [authIssuer, mcpResourceUrl]`, an option the pinned 1.7.2 does not read — the string does not occur once in its dist. In 1.7.2 a requested `resource` is resolved from the `oauthResource` table (`sys_oauth_resource`) and `enforcePerClientResources` defaults to `true`, so the client must also be linked in `oauthClientResource` (`sys_oauth_client_resource`). Neither row was ever written, so every client that sends `resource=` — Claude Code does — was refused at `/oauth2/authorize` with `invalid_target: requested resource <mcp url> is not configured`. Discovery, dynamic client registration and the login page all worked; the flow died one step before consent.
+
+- **`resources: [mcpResourceUrl]`** seeds the `sys_oauth_resource` row from the provider's own `init`. Seeding is idempotent and defaults to `insertOnly`, so an administrator's later edits to the row's token policy are never reverted by a restart.
+- **`clientRegistrationDefaultResources: [mcpResourceUrl]`** links each newly registered client to that resource inside the DCR transaction. This is the only place the link can be made: a client registers anonymously about one second before the browser login, leaving no window for an administrator to insert the row by hand.
+- **`enforcePerClientResources` is left at its `true` default.** The per-client linkage check stays on — the fix makes the link exist rather than switching the check off. A client with no link row is still refused with `invalid_target`, and a test asserts that.
+- **`validAudiences` is removed.** It was passed and read by nobody, which is precisely how the defect survived a version bump: it looked like configuration and enforced nothing.
+
+Two boot-path defects the resource seed uncovered are fixed in the same change, because seeding is the first thing this package ever wrote from a plugin `init`:
+
+- **`getAuthInstance()` now settles better-auth's plugin `init` hooks before it resolves.** `betterAuth()` returns synchronously and runs those hooks behind `auth.$context`, so a failure inside one had no catcher and escaped as an unhandled rejection — which Node terminates the process for by default. A boot failure now rejects the call that asked for the instance.
+- **The no-`dataEngine` development fallback builds its own in-memory adapter instead of letting better-auth build one.** better-auth 1.7.2 keys that store by the schema *key* while every read resolves by `modelName`, so on that path every model this package renames was unreachable — `user`/`sys_user` as much as `oauthResource`/`sys_oauth_resource` — answering `Model <name> not found`. Production never took this branch (it uses the ObjectQL adapter); development and tests did.
+
+No configuration change is required. Deployments that already ran 17.3.0 get the resource row on the next boot; MCP clients that failed to connect need to reconnect so a fresh registration picks up the link.

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

@@ -7,9 +7,13 @@
  * Token verification tests use REAL jose-signed JWTs against a locally
  * generated JWKS (mocked `getApi().getJwks`), so the crypto path — signature,
  * issuer, audience, expiry — is exercised for real, fail-closed on each axis.
- * The full discovery → DCR → PKCE browser flow is covered end-to-end against
- * a live dev server (see the PR's verification notes); better-auth's own
- * endpoint behavior is not re-tested here.
+ *
+ * ⚠️ The `oauthProvider plugin wiring` block below reads the options object
+ * this package passes to a MOCKED `oauthProvider`. That subject can only
+ * answer "did we pass X", never "does the provider honour X" — so ⛔ never
+ * assert protocol behaviour here. Anything whose truth depends on what the
+ * installed provider DOES belongs in auth-manager.mcp-oauth-resource.test.ts,
+ * which boots the real provider and drives the flow end to end.
  */
 
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
@@ -272,7 +276,7 @@ describe('verifyMcpAccessToken (local JWKS verification, fail-closed)', () => {
   });
 });
 
-describe('oauthProvider plugin wiring (DCR + scopes + audiences)', () => {
+describe('oauthProvider plugin wiring (DCR + scopes — options we pass, not behaviour)', () => {
   async function capturePluginOpts(env: Record<string, string>): Promise<any> {
     for (const [k, v] of Object.entries(env)) process.env[k] = v;
     (betterAuth as any).mockImplementation((config: any) => ({ handler: vi.fn(), api: {}, _cfg: config }));
@@ -297,19 +301,13 @@ describe('oauthProvider plugin wiring (DCR + scopes + audiences)', () => {
     expect(opts.allowUnauthenticatedClientRegistration).toBe(true);
     for (const scope of MCP_OAUTH_SCOPES) expect(opts.scopes).toContain(scope);
     expect(opts.scopes).toEqual(expect.arrayContaining(['openid', 'profile', 'email', 'offline_access']));
-    // RFC 8707: the MCP resource must be a valid audience or token minting fails.
-    expect(opts.validAudiences).toContain('https://acme.example.com/api/v1/mcp');
-    expect(opts.validAudiences).toContain('https://acme.example.com/api/v1/auth');
-  });
-
-  it('silences the false-positive oauthAuthServerConfig warning (#3420)', async () => {
-    // registerOidcDiscoveryRoutes mounts /.well-known/oauth-authorization-server
-    // (and the /api/v1/auth path-insertion variant) at the issuer ROOT ourselves,
-    // so better-auth's boot-time "Please ensure … exists" reminder is a false
-    // positive. It must be silenced via the documented option, or the stock
-    // showcase prints it (twice) on every `os dev`. Regression guard for the fix.
-    const opts = await capturePluginOpts({ OS_MCP_SERVER_ENABLED: 'true' });
-    expect(opts.silenceWarnings).toEqual({ oauthAuthServerConfig: true });
+    // ⛔ RFC 8707 audience binding is NOT asserted here. The subject available
+    // in this describe block is the options object we passed in, and the
+    // provider never consumes it — an assertion on it is green whether or not
+    // the installed version reads the option, which is exactly how a dead
+    // `validAudiences` survived a version bump. The refutable form lives in
+    // auth-manager.mcp-oauth-resource.test.ts, which boots the REAL provider
+    // and drives `authorize?resource=<mcp url>` to a minted token.
   });
 
   it('OS_OIDC_DCR_ENABLED=false forces DCR off even with MCP on', async () => {

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

@@ -333,7 +333,15 @@ describe('AuthManager', () => {
       }));
     });
 
-    it('should return undefined (in-memory fallback) when no dataEngine is provided', async () => {
+    // ⛔ This used to assert `database === undefined` — "let better-auth build
+    // its own in-memory store". That store is keyed by the schema KEY while
+    // every read resolves by `modelName`, so on better-auth 1.7.2 EVERY model
+    // this package renames (`user`/`sys_user`, `oauthResource`/
+    // `sys_oauth_resource`, …) was unreachable on that path with
+    // "Model <name> not found". The old assertion could not see that: it read
+    // the value we passed, never what the value does. This one drives the
+    // factory and asks the adapter for a renamed model.
+    it('the no-dataEngine fallback yields an adapter that can resolve a RENAMED model', async () => {
       let capturedConfig: any;
       (betterAuth as any).mockImplementation((config: any) => {
         capturedConfig = config;
@@ -348,9 +356,17 @@ describe('AuthManager', () => {
       });
 
       await manager.getAuthInstance();
-
-      expect(capturedConfig.database).toBeUndefined();
       warnSpy.mockRestore();
+
+      // An AdapterFactory, not `undefined`.
+      expect(typeof capturedConfig.database).toBe('function');
+
+      const adapter = capturedConfig.database(capturedConfig);
+      // `sys_user` is `user` renamed via `modelName`; a store keyed by the
+      // schema key answers "Model sys_user not found" here instead of `null`.
+      await expect(
+        adapter.findOne({ model: 'sys_user', where: [{ field: 'id', value: 'nobody' }] }),
+      ).resolves.toBeNull();
     });
   });
 

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

@@ -1256,7 +1256,7 @@ export class AuthManager {
       basePath: this.configuredBasePath(),
 
       // Database adapter configuration
-      database: this.createDatabaseConfig(),
+      database: await this.createDatabaseConfig(),
 
       // Model/field mapping: camelCase (better-auth) → snake_case (ObjectStack)
       // These declarations tell better-auth the actual table/column names used
@@ -2417,7 +2417,27 @@ export class AuthManager {
       } : {}),
     };
 
-    return betterAuth(betterAuthConfig);
+    const auth = betterAuth(betterAuthConfig);
+
+    // ⛔ Do not return before better-auth's plugin `init` hooks have settled.
+    //
+    // `betterAuth()` returns synchronously and runs those hooks behind
+    // `auth.$context`, so anything a plugin does at init is a promise NOBODY
+    // holds. That was harmless while init did no I/O. It stopped being harmless
+    // when the oauth-provider began seeding the RFC 8707 `sys_oauth_resource`
+    // row from its own `init`: a failure there had no catcher and surfaced as
+    // an UNHANDLED REJECTION — which Node terminates the process for by default
+    // — and, in tests, as a boot write racing its engine's teardown and failing
+    // with "No driver available for object 'sys_oauth_resource'" long after the
+    // test that triggered it had passed.
+    //
+    // Awaiting it here makes the seed part of "the instance is ready": a boot
+    // failure now rejects THIS call, where callers can see and handle it,
+    // instead of escaping the stack. `$context` is absent when better-auth is
+    // mocked, and `await undefined` is a no-op, so this is safe on that path.
+    await (auth as { $context?: Promise<unknown> } | undefined)?.$context;
+
+    return auth;
   }
 
   /**
@@ -3463,34 +3483,54 @@ export class AuthManager {
         loginPage: this.getConsolePageUrl('/login'),
         consentPage: this.getConsolePageUrl('/oauth/consent'),
         schema: buildOauthProviderPluginSchema(),
-        // better-auth's oauth-provider cannot see the well-known documents we
-        // mount ourselves at the issuer ROOT (RFC 8414 §3 requires them there,
-        // not under the auth basePath) — registerOidcDiscoveryRoutes serves
-        // /.well-known/oauth-authorization-server AND the path-insertion variant
-        // (`…/api/v1/auth`) the notice names. Its "Please ensure … exists"
-        // reminder is therefore a false positive on every stock example.
-        //
-        // #3420 root cause of the DOUBLE print: the notice fires in the
-        // oauth-provider plugin's `init(ctx)`, which better-auth runs once per
-        // `betterAuth()` construction — and the instance is built more than once
-        // at boot (an initial lazy build, then a rebuild once boot-time auth
-        // *settings* are applied — applyConfigPatch() nulls the cached instance
-        // so the next request rebuilds with the new policy). Gating the emitter
-        // here silences the one requirement we've already satisfied across every
-        // build path, independent of how many times auth is constructed, so an
-        // official dev boot stays warning-free.
-        silenceWarnings: { oauthAuthServerConfig: true },
+        // ⛔ No `silenceWarnings` here. It was added for the #3420 double
+        // print of oauth-provider's "Please ensure /.well-known/… exists"
+        // notice — a false positive, because registerOidcDiscoveryRoutes
+        // mounts those documents at the issuer ROOT where RFC 8414 §3 requires
+        // them. The pinned 1.7.2 emits no such notice: neither the option name
+        // nor the `oauthAuthServerConfig` key nor the notice text occurs
+        // anywhere in `@better-auth/oauth-provider` or `better-auth`, so the
+        // option silenced nothing and was the same dead-option shape as the
+        // `validAudiences` defect below. If a future bump reintroduces the
+        // notice, re-add the silencer with a fresh reading — do NOT restore it
+        // on the strength of this comment.
         // ── MCP OAuth track (#2698) ────────────────────────────────
         // Coarse tool-family scopes for the platform's own MCP endpoint,
         // advertised alongside the standard OIDC scopes. Names are
         // single-sourced in @objectstack/spec so AS / resource server /
         // tool layer cannot drift.
         scopes: ['openid', 'profile', 'email', 'offline_access', ...MCP_OAUTH_SCOPES],
         // MCP clients bind tokens to the resource via RFC 8707
-        // (`resource=<mcp url>`); the AS only mints audiences it knows.
-        // The auth base (better-auth's default audience) stays valid for
-        // plain OIDC SSO flows.
-        validAudiences: [this.getAuthIssuer(), this.getMcpResourceUrl()],
+        // (`resource=<mcp url>`). In @better-auth/oauth-provider 1.7.2 a
+        // requested `resource` is resolved from the `oauthResource` table
+        // (`sys_oauth_resource`) — a miss is refused at /oauth2/authorize with
+        // `invalid_target: requested resource <id> is not configured` — and
+        // `enforcePerClientResources` defaults to TRUE, so the client must
+        // additionally be linked in `oauthClientResource`
+        // (`sys_oauth_client_resource`). Both rows have to exist before the
+        // first Connect, so both are declared here:
+        //
+        //   • `resources` seeds the sys_oauth_resource row from the plugin's
+        //     own `init` (idempotent, `resourceSeedMode: "insertOnly"` by
+        //     default, so an admin's later CRUD edits are never reverted);
+        //   • `clientRegistrationDefaultResources` links every newly
+        //     registered client to it inside the DCR transaction — a client
+        //     that registers anonymously one second before the login cannot
+        //     be linked by an admin in between.
+        //
+        // ⛔ `enforcePerClientResources` is deliberately NOT passed: the
+        // per-client linkage check stays at its `true` default. The fix makes
+        // the link happen; it does not switch the check off. A client with no
+        // link row is still refused, and
+        // auth-manager.mcp-oauth-resource.test.ts asserts exactly that.
+        //
+        // ⛔ Do not reintroduce `validAudiences`: 1.7.2 reads no such option
+        // (0 occurrences in its dist), and audience validation now runs
+        // through the resource table instead. A field that is passed and read
+        // by nobody looks like configuration and enforces nothing — that is
+        // how this defect survived a version bump.
+        resources: [this.getMcpResourceUrl()],
+        clientRegistrationDefaultResources: [this.getMcpResourceUrl()],
         // RFC 7591 Dynamic Client Registration. `allowUnauthenticated…` is
         // required: MCP clients register BEFORE any user is logged in (the
         // whole point of the self-serve flow). Registration is rate-limited
@@ -3765,7 +3805,7 @@ export class AuthManager {
    * silently.  We therefore wrap the ObjectQL adapter in a factory function
    * so it is correctly recognised as a `DBAdapterInstance`.
    */
-  private createDatabaseConfig(): any {
+  private async createDatabaseConfig(): Promise<any> {
     // Use ObjectQL adapter factory if dataEngine is provided
     if (this.config.dataEngine) {
       // createObjectQLAdapterFactory returns an AdapterFactory
@@ -3783,9 +3823,41 @@ export class AuthManager {
       'Please provide a dataEngine instance (e.g., ObjectQL) in AuthManagerOptions.'
     );
 
-    // Return a minimal in-memory configuration as fallback
-    // This allows the system to work in development/testing without a real database
-    return undefined; // better-auth will use its default in-memory adapter
+    // ⛔ NOT `undefined`, and ⛔ do not "simplify" it back to that.
+    //
+    // Handing better-auth no `database` makes it build its own in-memory store
+    // in `getBaseAdapter`, and that store is keyed by the schema KEY while
+    // every read resolves by `modelName`. Measured on better-auth 1.7.2:
+    //
+    //   getAuthTables(options) -> { oauthResource: { modelName: 'sys_oauth_resource' }, … }
+    //   its memoryDB           -> { oauthResource: [] }            // keyed by KEY
+    //   the adapter then asks  -> 'sys_oauth_resource'             // resolved by modelName
+    //   => Error: Model sys_oauth_resource not found
+    //
+    // So on that path EVERY model this package renames is unreachable —
+    // `user`/`sys_user` included. It stayed invisible for as long as nothing
+    // touched a renamed model during boot; the RFC 8707 resource seed does,
+    // from the oauth-provider plugin's `init`, where the throw surfaces as an
+    // UNHANDLED REJECTION rather than a failed request.
+    //
+    // Keying the store by `modelName` is what the adapter actually reads, so
+    // this fixes the dev/test fallback instead of working around it. Production
+    // never reaches this branch — it returns the ObjectQL factory above.
+    //
+    // The import is dynamic on purpose (the rest of better-auth is loaded the
+    // same way here); `createAuthInstance` awaits this method, and better-auth
+    // then calls the returned factory SYNCHRONOUSLY, so the module has to be
+    // resolved before we hand it over, not inside it.
+    const [{ memoryAdapter }, { getAuthTables }] = await Promise.all([
+      import('better-auth/adapters/memory'),
+      import('@better-auth/core/db'),
+    ]);
+    return (options: any) => {
+      const tables = getAuthTables(options) as Record<string, { modelName?: string }>;
+      const db: Record<string, unknown[]> = {};
+      for (const [key, table] of Object.entries(tables)) db[table?.modelName ?? key] = [];
+      return memoryAdapter(db)(options);
+    };
   }
 
   /**