From f6fca596244fd4e02effddd79eaaf082d4a869b3 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 24 Aug 2026 20:08:50 -0400 Subject: [PATCH] docs(oauth): scope what the refresh-token opt-out actually changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review on #2114. The setting was described as taking effect "immediately", which is wrong twice over. `applyLiveServerSettings` treats OAuth as connection-time-only, so a connected client keeps the metadata it was built with — deliberately, and identically to every other field in that section (client id, secret, scopes, authorizationParams, endpoint overrides). Not wiring this one field live; saying "next connect" instead, in the docs, the checkbox description, and the type. And two pieces of state outlive the setting: the registration already held at the authorization server still lists the grant, and a refresh token issued earlier stays usable, because the SDK's `refreshAuthorization()` posts the stored token without consulting `grant_types`. So the stated cost is not guaranteed until stored OAuth state is cleared. Documented, rather than revoking credentials as a side effect of a checkbox — Clear stored OAuth state already does that explicitly. Also adds the manager->provider seam test the other bridges have: without it, deleting `requestRefreshToken:` from `createOAuthProvider` would leave the feature dead with every other test green. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev Signed-off-by: cliffhall --- .../ServerSettingsForm/ServerSettingsForm.tsx | 2 +- .../src/test/core/mcp/oauthManager.test.ts | 26 +++++++++++++++++++ core/mcp/types.ts | 7 +++++ docs/mcp-server-configuration.md | 6 +++-- 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx index 8c03176e..54b67a42 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx @@ -883,7 +883,7 @@ export function ServerSettingsForm({ /> onOAuthChange({ diff --git a/clients/web/src/test/core/mcp/oauthManager.test.ts b/clients/web/src/test/core/mcp/oauthManager.test.ts index 59ba1bfb..192e07d1 100644 --- a/clients/web/src/test/core/mcp/oauthManager.test.ts +++ b/clients/web/src/test/core/mcp/oauthManager.test.ts @@ -1687,6 +1687,32 @@ describe("OAuthManager", () => { expect(navigated.searchParams.get("state")).toBe("xyz"); }); + // #2068 — the manager→provider bridge for the refresh-token opt-out. Same + // gap as the authorizationParams case above: the provider test constructs + // `BaseOAuthClientProvider` directly and the runner test stops at the + // options object, so deleting `requestRefreshToken:` from + // `createOAuthProvider` would leave the feature dead with every other test + // still green. + it("forwards the refresh-token opt-out to the provider it builds", async () => { + const manager = new OAuthManager(createMockParams()); + manager.setOAuthConfig({ requestRefreshToken: false }); + + const provider = await manager.createOAuthProviderForTransport(); + expect(provider.clientMetadata.grant_types).toEqual([ + "authorization_code", + ]); + }); + + it("declares the refresh_token grant when the opt-out is not configured", async () => { + const manager = new OAuthManager(createMockParams()); + + const provider = await manager.createOAuthProviderForTransport(); + expect(provider.clientMetadata.grant_types).toEqual([ + "authorization_code", + "refresh_token", + ]); + }); + it("drops a reserved key configured on the manager", async () => { const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); const params = createMockParams(); diff --git a/core/mcp/types.ts b/core/mcp/types.ts index b4688385..5c8db1d7 100644 --- a/core/mcp/types.ts +++ b/core/mcp/types.ts @@ -793,6 +793,13 @@ export interface InspectorServerSettings { * persisted as `oauth.requestRefreshToken` only when explicitly off. Turning * it off also drops the SDK's automatic `offline_access` scope and the * `prompt=consent` it forces — the Entra admin-consent failure in #2068. + * + * Read at connect time like the rest of the OAuth block, so a live client + * keeps the metadata it was built with. It changes what the Inspector + * declares, not state the authorization server already holds: an existing + * registration still lists the grant, and a refresh token issued earlier + * stays usable (the SDK's refresh path never consults `grant_types`) until + * the stored OAuth state is cleared. */ oauthRequestRefreshToken?: boolean; /** diff --git a/docs/mcp-server-configuration.md b/docs/mcp-server-configuration.md index fd347ada..a9905bc3 100644 --- a/docs/mcp-server-configuration.md +++ b/docs/mcp-server-configuration.md @@ -193,9 +193,11 @@ These have no analog in the broader `mcp.json` ecosystem. Each is **omitted on w > **Reading an older file.** Before #1910 this field was a `[{ "key": …, "value": … }]` pair array. That shape is still **read**, so an existing `mcp.json` keeps working unchanged; it is never written back, so the field is rewritten as an object the next time the Inspector saves the entry. -`oauth.requestRefreshToken` (default `true`) controls whether the Inspector declares the `refresh_token` grant when it registers its OAuth client. Set it to `false` — or uncheck **Request refresh token** in Server Settings → Authorization — when that grant costs you more than it buys. Declaring it makes the SDK append `offline_access` to the requested scope whenever the authorization server advertises that scope, and appending `offline_access` in turn makes the SDK append `prompt=consent` to the authorization request. On Microsoft Entra ID, a forced `prompt=consent` from a non-admin user, for an app whose permissions require admin consent, is routed into the admin-consent workflow and rejected with `AADSTS90094` — even after a tenant admin has already granted consent ([#2068](https://github.com/modelcontextprotocol/inspector/issues/2068)). Turning the setting off breaks that chain at its source: no grant, no `offline_access`, no forced prompt. The cost is that an expired access token needs a fresh interactive sign-in rather than a silent refresh. Only `false` is written to disk, so a server that never touched the setting keeps a minimal entry. +`oauth.requestRefreshToken` (default `true`) controls whether the Inspector declares the `refresh_token` grant when it registers its OAuth client. Set it to `false` — or uncheck **Request refresh token** in Server Settings → Authorization — when that grant costs you more than it buys. Declaring it makes the SDK append `offline_access` to the requested scope whenever the authorization server advertises that scope, and appending `offline_access` in turn makes the SDK append `prompt=consent` to the authorization request. On Microsoft Entra ID, a forced `prompt=consent` from a non-admin user, for an app whose permissions require admin consent, is routed into the admin-consent workflow and rejected with `AADSTS90094` — even after a tenant admin has already granted consent ([#2068](https://github.com/modelcontextprotocol/inspector/issues/2068)). Turning the setting off breaks that chain at its source: no grant, no `offline_access`, no forced prompt. Going forward the Inspector then obtains no new refresh token, so an expired access token needs a fresh interactive sign-in rather than a silent refresh. Only `false` is written to disk, so a server that never touched the setting keeps a minimal entry. -> Turning it off changes the **next** authorization request immediately, because the SDK reads the grant list off the Inspector's live client metadata rather than off whatever was registered earlier. The registration held at the authorization server is not rewritten, though — if you also want the AS to stop listing the grant, clear the stored OAuth state for the server (Server Settings → Authorization → **Clear stored OAuth state**) so the next connect registers afresh. +> **It applies on the next connect, not to the live connection.** OAuth is connection-time configuration here — like the client credentials, the scopes, and the endpoint overrides beside it — so a client that is already connected keeps the metadata it was built with. Reconnect for the change to reach the authorization flow. + +> **Two pieces of state outlive the setting, and neither is cleared by unchecking it.** The registration held at the authorization server still lists the grant, because turning the setting off changes what the Inspector *declares*, not what the AS has already recorded. And a refresh token issued before the opt-out stays usable: the SDK's refresh path posts the stored token with `grant_type=refresh_token` without consulting the client metadata, so such a server can keep refreshing silently and you may not observe the cost described above at all. **Clear stored OAuth state** (Server Settings → Authorization) drops both — the next connect registers afresh and signs in interactively. `oauth.authorizationParams` is a string→string record of extra query parameters merged into the OAuth **authorization request** URL only — never the token request. Use it for provider-specific hints the core specs don't standardize (Keycloak's `kc_idp_hint`, OIDC's `login_hint` / `prompt` / `acr_values`, Auth0's `audience`). The protocol-critical parameters — `client_id`, `code_challenge`, `code_challenge_method`, `redirect_uri`, `resource`, `response_type`, `scope`, `state` — are **reserved**: the web form rejects them inline, and any that reach the merge anyway are dropped with a warning rather than overriding what the flow set (overriding them breaks PKCE, the CSRF state binding, or RFC 8707). Edit them in Server Settings → Authorization ("Additional authorization parameters"), beside Scopes.