modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

test(oauth): prove the opt-out against a real authorization server

Browse source 
Copilot review round 3 on #2114.

The unit tests asserted our own `grant_types` array and stopped there.
What actually unblocks an Entra tenant is what the SDK puts in the
authorize URL, and nothing covered that — so an SDK change that stopped
keying its augmentation off `grant_types` would leave AADSTS90094
intact with every test green.

Adds the complementary case to the existing e2e suite, against a real
AS advertising `offline_access`: with the grant declined, the authorize
URL carries neither `offline_access` nor `prompt=consent`, while the
configured scope still goes out. Verified non-vacuous by reverting the
provider conditional — it fails. The positive test alongside it now
also asserts `prompt=consent` is present, which was unasserted despite
being the literal failure in the issue.

Drops an unjustified `as unknown as ServerEntry` I added in the App
test — AGENTS.md forbids it, and it hid a real mismatch (a partial
settings object against a complete one). Typed `ServerEntry` /
`InspectorServerSettings` fixtures instead, so a field added to that
interface later is a compile error rather than a missing value at
runtime. The `constructedOAuth` helper reads through the constructor's
own parameter type and needs no cast at all now.

Scopes the two doc comments that still claimed the flag removes every
`offline_access` — `StoredMCPServer.oauth` and `OAuthProviderConfig`,
both contradicting the caveat already stated elsewhere.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-24 21:03:41 -04:00
commit 6303f8a6ce
3 changed files with 125 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

39
clients/web/src/App.test.tsx
View file

@ -654,6 +654,7 @@ import {
useServers,
} from "@inspector/core/react/useServers.js";
import type {
InspectorClientOptions,
InspectorServerSettings,
MessageEntry,
ServerEntry,
@ -702,20 +703,36 @@ const fetchLogInstances = (
// `requestRefreshToken` spread from App's `oauthFromServer` would leave the
// checkbox persisted but inert with every other new test still green.
describe("App wires the refresh-token opt-out into the client (#2068)", () => {
const HTTP_SERVER = {
// Fully-typed fixtures rather than a cast: `settings` is the whole
// `InspectorServerSettings`, so a field added to that interface later is a
// compile error here instead of a silently-absent value at runtime.
const HTTP_SERVER: ServerEntry = {
id: "A",
name: "PlotRocket",
config: { type: "streamable-http", url: "https://api.example.com/mcp" },
connection: { status: "disconnected" },
};
function mockServersWith(settings?: Partial<InspectorServerSettings>) {
const BASE_SETTINGS: InspectorServerSettings = {
headers: [],
env: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,
maxFetchRequests: 1000,
roots: [],
};
function mockServersWith(overrides?: Partial<InspectorServerSettings>) {
vi.mocked(useServers).mockReturnValue({
servers: [
{
...HTTP_SERVER,
...(settings ? { settings } : {}),
} as unknown as ServerEntry,
...(overrides
? { settings: { ...BASE_SETTINGS, ...overrides } }
: {}),
},
],
loading: false,
error: undefined,
@ -729,13 +746,13 @@ describe("App wires the refresh-token opt-out into the client (#2068)", () => {
});
}
/** The `oauth` option the mocked InspectorClient constructor was given. */
function constructedOAuth(): { requestRefreshToken?: boolean } | undefined {
const call = vi.mocked(McpIndex.InspectorClient).mock.calls[0];
const options = call?.[1] as
| { oauth?: { requestRefreshToken?: boolean } }
| undefined;
return options?.oauth;
/**
* The `oauth` option the mocked InspectorClient constructor was given. Read
* through the constructor's own parameter type, so a rename of the option
* fails to compile here rather than silently reading `undefined`.
*/
function constructedOAuth(): InspectorClientOptions["oauth"] {
return vi.mocked(McpIndex.InspectorClient).mock.calls[0]?.[1]?.oauth;
}
let previousUseServers: typeof useServers | undefined;

8
clients/web/src/test/core/auth/providers.test.ts
View file

@ -496,9 +496,11 @@ describe("OAuthNavigation", () => {
});
// The point of the opt-out: the SDK's determineScope() only appends
// `offline_access` when the client metadata declares `refresh_token`, and
// startAuthorization() only forces `prompt=consent` when `offline_access`
// is in scope. Dropping the grant is what breaks that chain.
// `offline_access` when the client metadata declares `refresh_token`.
// This asserts our half — the grant list. That the authorize URL then
// carries neither `offline_access` nor `prompt=consent` is proved against
// a real AS in the e2e suite (inspectorClient-oauth-e2e.test.ts), since
// the SDK, not this array, is what ultimately writes that URL.
it("drops the refresh_token grant when disabled", () => {
expect(
makeProviderWithRefresh(false).clientMetadata.grant_types,

75
clients/web/src/test/integration/mcp/inspectorClient-oauth-e2e.test.ts
View file

@ -1383,6 +1383,81 @@ describe("InspectorClient OAuth E2E", () => {
if (!authUrl) throw new Error("Expected authorization URL");
const scope = authUrl.searchParams.get("scope") ?? "";
expect(scope.split(/\s+/)).toContain("offline_access");
// The consequence of that scope, and the actual failure in #2068: the SDK
// forces an interactive consent prompt. Asserted here so the opt-out case
// below is a real before/after pair rather than a scope check alone.
expect(authUrl.searchParams.get("prompt")).toBe("consent");
});
// #2068 — the same server, the same AS advertising `offline_access`, with
// the grant declined. This is the end-to-end proof the unit tests cannot
// give: they assert our `grant_types` array, while the behavior that
// actually unblocks an Entra tenant is what the SDK puts in the authorize
// URL. If the SDK ever stopped keying its augmentation off `grant_types`,
// every other test here would still pass and AADSTS90094 would remain.
it("omits offline_access and prompt=consent when the refresh-token grant is declined", async () => {
const transport = transports[1]!;
const staticClientId = "test-no-offline-access-client";
const staticClientSecret = "test-no-offline-access-secret";
const serverConfig = {
...getDefaultServerConfig(),
serverType: transport.serverType,
...createOAuthTestServerConfig({
requireAuth: true,
supportRefreshTokens: true,
scopesSupported: ["mcp", "offline_access"],
staticClients: [
{
clientId: staticClientId,
clientSecret: staticClientSecret,
redirectUris: [testRedirectUrl],
},
],
}),
};
server = new TestServerHttp(serverConfig);
const port = await server.start();
const serverUrl = `http://localhost:${port}`;
await waitForOAuthWellKnown(serverUrl);
const oauthConfig = createTestOAuthConfig({
mode: "static",
clientId: staticClientId,
clientSecret: staticClientSecret,
redirectUrl: testRedirectUrl,
scope: "mcp",
});
client = new InspectorClient(
{
type: transport.clientType,
url: `${serverUrl}${transport.endpoint}`,
} as MCPServerConfig,
{
environment: {
transport: createTransportNode,
oauth: {
storage: oauthConfig.storage,
navigation: oauthConfig.navigation,
redirectUrlProvider: oauthConfig.redirectUrlProvider,
},
},
oauth: {
clientId: oauthConfig.clientId,
clientSecret: oauthConfig.clientSecret,
scope: oauthConfig.scope,
requestRefreshToken: false,
},
},
);
const authUrl = await client.authenticate();
if (!authUrl) throw new Error("Expected authorization URL");
const scope = authUrl.searchParams.get("scope") ?? "";
expect(scope.split(/\s+/)).not.toContain("offline_access");
expect(authUrl.searchParams.get("prompt")).toBeNull();
// The configured scope still goes out — the opt-out drops the SDK's
// addition, not what the user asked for.
expect(scope.split(/\s+/)).toContain("mcp");
});
});