Guided-auth removal, OAuth smoke testing, and TUI/CLI support (#1531)
* refactor(web): update OAuth E2E tests to use new authentication method - Replaced `runGuidedAuth` with `authenticate` in the InspectorClient OAuth end-to-end tests to support quick authentication with CIMD pre-registration. - Updated comments to reflect the change in authentication method and its support for HTTP test metadata URLs. - Ensured that the tests continue to validate the expected authorization URL after the change. Additionally, imported `ensureCimdClientRegistration` in the OAuthManager to streamline client registration during the authentication process. * Guided-auth removal, OAuth smoke testing, and TUI/CLI support * Added CLI OAuth runner flags tests * Added clarifying comment on port 0 binding * README notes on TUI oauth callback port, prettier fixes for CLI tests * CIMD validation uses inline like EMA IdP * Fixed TUI test failures from merge * test: bring 5 OAuth files over the 90 per-file coverage gate The guided-auth-removal work added/touched several files that fell below the repo's >=90 per-file coverage gate (npm run coverage). CI doesn't run the gate (it's local-only), so validate + test:integration passed while these gaps slipped through. Bring them all back over the line: - core/auth/node/runner-oauth-callback.ts: test bad-URL/non-http-scheme throws, no-port->80 default, and IPv6 hostname bracketing. Annotate the provably-dead guards (empty host, out-of-range port, String(err) fallback, pathname "/" fallback) -- all rejected by new URL() upstream -- with justified v8 ignore comments instead of lowering the gate. - core/auth/cimd.ts: test the AS-doesn't-support-CIMD early return. - core/react/useClientSettingsDraft.ts: test the functional-updater path, the stale-onChange-after-close prev===null guard, and the timer-fires-after-close value===null guard. - ClientSettingsForm.tsx: test the CIMD checkbox toggle and CIMD URL clear. - clients/tui/src/utils/oauthDisplay.ts: test static/dcr registration kinds, the expired IdP session, and the configured-scope/empty-scope branches of formatScopes. npm run coverage now passes (exit 0) across web, cli, tui, launcher. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: cliffhall <cliff@futurescale.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
4a927d3978
commit
13dafe457b
3 changed files with 5702 additions and 4444 deletions
51
clients/web/src/utils/clearServerOAuthState.test.ts
Normal file
51
clients/web/src/utils/clearServerOAuthState.test.ts
Normal file
|
|
@ -0,0 +1,51 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { getBrowserOAuthStorage } from "@inspector/core/auth/browser/index.js";
|
||||
import { clearServerOAuthState } from "./clearServerOAuthState";
|
||||
|
||||
describe("clearServerOAuthState", () => {
|
||||
beforeEach(() => {
|
||||
getBrowserOAuthStorage().clear("https://mcp.example.com/mcp");
|
||||
});
|
||||
|
||||
it("clears storage by server URL when not the active connection", async () => {
|
||||
const storage = getBrowserOAuthStorage();
|
||||
await storage.saveTokens("https://mcp.example.com/mcp", {
|
||||
access_token: "tok",
|
||||
token_type: "Bearer",
|
||||
});
|
||||
|
||||
const cleared = clearServerOAuthState({
|
||||
config: { type: "streamable-http", url: "https://mcp.example.com/mcp" },
|
||||
isActiveConnection: false,
|
||||
});
|
||||
|
||||
expect(cleared).toBe(true);
|
||||
expect(
|
||||
await storage.getTokens("https://mcp.example.com/mcp"),
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it("uses the live client when clearing the active connection", () => {
|
||||
const inspectorClient = {
|
||||
clearOAuthTokens: vi.fn(),
|
||||
};
|
||||
|
||||
const cleared = clearServerOAuthState({
|
||||
config: { type: "streamable-http", url: "https://mcp.example.com/mcp" },
|
||||
inspectorClient: inspectorClient as never,
|
||||
isActiveConnection: true,
|
||||
});
|
||||
|
||||
expect(cleared).toBe(true);
|
||||
expect(inspectorClient.clearOAuthTokens).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("returns false for stdio servers", () => {
|
||||
expect(
|
||||
clearServerOAuthState({
|
||||
config: { type: "stdio", command: "node", args: [] },
|
||||
isActiveConnection: false,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
32
clients/web/src/utils/clearServerOAuthState.ts
Normal file
32
clients/web/src/utils/clearServerOAuthState.ts
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
import { getBrowserOAuthStorage } from "@inspector/core/auth/browser/index.js";
|
||||
import { getOAuthServerUrl } from "@inspector/core/mcp/config.js";
|
||||
import type { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
|
||||
import type { MCPServerConfig } from "@inspector/core/mcp/types.js";
|
||||
|
||||
export interface ClearServerOAuthStateParams {
|
||||
config: MCPServerConfig;
|
||||
/** When set and this server is the active connection, clear via the live client. */
|
||||
inspectorClient?: InspectorClient | null;
|
||||
isActiveConnection: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Clear persisted OAuth state (tokens, DCR/CIMD client id, PKCE, etc.) for an
|
||||
* HTTP MCP server. When clearing the active connection, uses the live client so
|
||||
* in-memory flow state is reset too.
|
||||
*/
|
||||
export function clearServerOAuthState(
|
||||
params: ClearServerOAuthStateParams,
|
||||
): boolean {
|
||||
const serverUrl = getOAuthServerUrl(params.config);
|
||||
if (!serverUrl) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (params.isActiveConnection && params.inspectorClient) {
|
||||
params.inspectorClient.clearOAuthTokens();
|
||||
} else {
|
||||
getBrowserOAuthStorage().clear(serverUrl);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
|
@ -4,6 +4,8 @@
|
|||
* independently of the React component that orchestrates it.
|
||||
*/
|
||||
|
||||
export { isUnauthorizedError } from "@inspector/core/auth/utils.js";
|
||||
|
||||
/**
|
||||
* The pathname the auth server redirects back to after the user authorizes.
|
||||
* `App.tsx`'s `redirectUrlProvider` points OAuth flows at
|
||||
|
|
@ -14,31 +16,10 @@ export const OAUTH_CALLBACK_PATH = "/oauth/callback";
|
|||
|
||||
/**
|
||||
* sessionStorage key holding the id of the server whose OAuth flow is in
|
||||
* flight. The OAuth `state` parameter only carries `{execution}:{authId}`, not which
|
||||
* flight. The OAuth `state` parameter carries the auth session id; the full
|
||||
* configured server initiated the flow, and the full-page redirect to the auth
|
||||
* server wipes all in-memory React state. The id is stashed here right before
|
||||
* redirecting so the post-callback page load can rebuild the right
|
||||
* `InspectorClient` and resume the connection.
|
||||
*/
|
||||
export const OAUTH_PENDING_SERVER_KEY = "mcp-inspector:oauth-pending-server-id";
|
||||
|
||||
/**
|
||||
* True when a thrown connect error represents an upstream 401. The remote
|
||||
* transport preserves the status on the error object
|
||||
* (`remoteClientTransport` sets `error.status`); this structured check is the
|
||||
* primary path. As a fallback for cases where the status is lost crossing an
|
||||
* SDK boundary, we match the transport's own formatted wording —
|
||||
* `"Remote (connect|send|events stream) failed (401): …"` — anchored on
|
||||
* `failed …(401)` rather than a bare `(401)`, so an unrelated `(401)` spliced
|
||||
* into an error message (e.g. from a tool result) can't trip the OAuth flow.
|
||||
* A 401 on connect to an HTTP/SSE server is the signal to start OAuth.
|
||||
*/
|
||||
export function isUnauthorizedError(err: unknown): boolean {
|
||||
if (typeof err === "object" && err !== null) {
|
||||
const status = (err as { status?: number; code?: number }).status;
|
||||
const code = (err as { code?: number }).code;
|
||||
if (status === 401 || code === 401) return true;
|
||||
}
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
return /\bfailed\b[^\n]*\(401\)/i.test(message);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue