diff --git a/AGENTS.md b/AGENTS.md index a8a110b1..de3945eb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,7 +38,14 @@ v2/main/ │ │ # issuerBinding.ts SEP-2352 callback-leg failure │ │ # classification — separates a recoverable │ │ # "lost authorization state" from a genuine -│ │ # cross-AS issuer mismatch — #1808) +│ │ # cross-AS issuer mismatch — #1808; +│ │ # authorizationParams.ts per-server custom +│ │ # authorization-request parameters — the +│ │ # reserved-key list plus the merge applied in +│ │ # providers.redirectToAuthorization, the only +│ │ # seam that sees the SDK-built authorize URL; +│ │ # authorization request only, never the token +│ │ # request — #2018) │ │ ├── browser/ # Browser-side OAuth (sessionStorage, BrowserNavigation) │ │ ├── node/ # Node-side OAuth (NodeOAuthStorage, OAuthCallbackServer, │ │ │ # runner-interactive-oauth loopback callback flow) diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 09645d9a..72d8bd1f 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -63,6 +63,7 @@ import { ManagedRequestorTasksState } from "@inspector/core/mcp/state/managedReq import { ResourceSubscriptionsState } from "@inspector/core/mcp/state/resourceSubscriptionsState.js"; import { cleanRoots, + oauthAuthorizationParamsFromSettings, serializeMcpConfig, } from "@inspector/core/mcp/serverList.js"; import type { ClientConfig } from "@inspector/core/client/types.js"; @@ -2311,11 +2312,15 @@ function App() { .map((m) => [m.key, m.value]), ) : undefined; + const serverAuthorizationParams = savedSettings + ? oauthAuthorizationParamsFromSettings(savedSettings) + : undefined; const oauthFromServer = savedSettings && (savedSettings.oauthClientId || savedSettings.oauthClientSecret || savedSettings.oauthScopes || + serverAuthorizationParams || savedSettings.enterpriseManaged) ? { ...(savedSettings.oauthClientId && { @@ -2327,6 +2332,9 @@ function App() { ...(savedSettings.oauthScopes && { scope: savedSettings.oauthScopes, }), + ...(serverAuthorizationParams && { + authorizationParams: serverAuthorizationParams, + }), ...(savedSettings.enterpriseManaged && { enterpriseManaged: true, }), diff --git a/clients/web/src/components/elements/CopyButton/CopyButton.test.tsx b/clients/web/src/components/elements/CopyButton/CopyButton.test.tsx index 9a4b462a..0b663cae 100644 --- a/clients/web/src/components/elements/CopyButton/CopyButton.test.tsx +++ b/clients/web/src/components/elements/CopyButton/CopyButton.test.tsx @@ -10,6 +10,17 @@ describe("CopyButton", () => { expect(screen.getByText("⎘")).toBeInTheDocument(); }); + it("names itself 'Copy' with no label, and qualifies the name when given one", () => { + const { unmount } = renderWithMantine(); + expect(screen.getByRole("button", { name: "Copy" })).toBeInTheDocument(); + unmount(); + + renderWithMantine(); + expect( + screen.getByRole("button", { name: "Copy ID Token" }), + ).toBeInTheDocument(); + }); + it("triggers a click without throwing when clipboard is unavailable", async () => { const user = userEvent.setup(); renderWithMantine(); diff --git a/clients/web/src/components/elements/CopyButton/CopyButton.tsx b/clients/web/src/components/elements/CopyButton/CopyButton.tsx index 7a86ca08..5b87719d 100644 --- a/clients/web/src/components/elements/CopyButton/CopyButton.tsx +++ b/clients/web/src/components/elements/CopyButton/CopyButton.tsx @@ -11,6 +11,15 @@ export interface CopyButtonProps { * rows (e.g. beside a Code block). Icon size is unchanged. */ flush?: boolean; + /** + * Names what is being copied, for the accessible label only ("Copy ID + * Token"). Set it when more than one CopyButton can appear in the same + * region, so button-by-button screen-reader navigation can tell them apart; + * a lone control needs no qualifier. The visible tooltip stays "Copy" either + * way, and the accessible name keeps "Copy"/"Copied" as its first word so it + * still contains the visible text (WCAG 2.5.3). + */ + label?: string; } const CopyActionIcon = ActionIcon.withProps({ @@ -18,7 +27,8 @@ const CopyActionIcon = ActionIcon.withProps({ fz: 24, }); -export function CopyButton({ value, flush = false }: CopyButtonProps) { +export function CopyButton({ value, flush = false, label }: CopyButtonProps) { + const qualifier = label ? ` ${label}` : ""; return ( {({ copied, copy }) => ( @@ -26,7 +36,7 @@ export function CopyButton({ value, flush = false }: CopyButtonProps) { {copied ? "\u2713" : "\u2398"} diff --git a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.stories.tsx b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.stories.tsx index 3b1c919b..c6f1e598 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.stories.tsx +++ b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.stories.tsx @@ -3,7 +3,7 @@ import type { InitializeResult, } from "@modelcontextprotocol/client"; import type { Meta, StoryObj } from "@storybook/react-vite"; -import { expect, within } from "storybook/test"; +import { expect, userEvent, within } from "storybook/test"; import { ConnectionInfoContent, SERVER_INFO_NOT_REPORTED_LABEL, @@ -161,3 +161,37 @@ export const WithOAuth: Story = { }, }, }; + +// An authorization server that also returned an `id_token` (#2019). Shown for +// inspection only — decoding it is display-only, and it is never used as a +// credential. +export const WithOAuthIdToken: Story = { + args: { + initializeResult: { + protocolVersion: "2025-03-26", + serverInfo: { name: "OIDC-fronted Server", version: "3.0.0" }, + capabilities: { tools: { listChanged: true } }, + }, + clientCapabilities: { roots: { listChanged: true } }, + transport: "streamable-http", + oauth: { + protocol: "standard", + authorized: true, + authUrl: "https://auth.example.com/oauth2/authorize", + scopes: ["openid", "email", "read"], + accessToken: "opaque-access-token", + // header {"alg":"none"} / payload {"sub":"user-42","email":"a@b.test"} + idToken: + "eyJhbGciOiJub25lIn0.eyJzdWIiOiJ1c2VyLTQyIiwiZW1haWwiOiJhQGIudGVzdCJ9.", + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByText("ID Token")).toBeInTheDocument(); + // Only the ID token is a JWT here, so exactly one decode toggle is offered. + await userEvent.click( + canvas.getByRole("button", { name: "Decode JWT for ID Token" }), + ); + expect(canvas.getByText(/"sub": "user-42"/)).toBeInTheDocument(); + }, +}; diff --git a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx index 9f9a8ea4..f9f3d2fa 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx +++ b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx @@ -473,6 +473,95 @@ describe("ConnectionInfoContent", () => { expect(screen.queryByText("Auth URL")).not.toBeInTheDocument(); expect(screen.queryByText("Scopes")).not.toBeInTheDocument(); expect(screen.queryByText("Access Token")).not.toBeInTheDocument(); + expect(screen.queryByText("ID Token")).not.toBeInTheDocument(); + }); + + it("renders the ID Token row when the token set carries one", () => { + renderWithMantine( + , + ); + expect(screen.getByText("Access Token")).toBeInTheDocument(); + expect(screen.getByText("ID Token")).toBeInTheDocument(); + expect(screen.getByText("id-token-456")).toBeInTheDocument(); + }); + + it("gives the two token rows' controls distinct accessible names", () => { + // Both rows carry a copy control, and both tokens here are JWTs so both + // carry a decode toggle. Screen-reader button navigation has only the + // accessible name to go on, so the four must not collide (#2019 review). + const jwt = "eyJhbGciOiJub25lIn0.eyJzdWIiOiJ1c2VyIn0."; + renderWithMantine( + , + ); + + const names = screen + .getAllByRole("button") + .map((button) => button.getAttribute("aria-label") ?? button.textContent) + .filter((name): name is string => Boolean(name)); + expect(new Set(names).size).toBe(names.length); + + for (const name of [ + "Copy Access Token", + "Copy ID Token", + "Decode JWT for Access Token", + "Decode JWT for ID Token", + ]) { + expect(screen.getByRole("button", { name })).toBeInTheDocument(); + } + }); + + it("renders the ID Token row on its own when there is no access token", () => { + renderWithMantine( + , + ); + expect(screen.queryByText("Access Token")).not.toBeInTheDocument(); + expect(screen.getByText("ID Token")).toBeInTheDocument(); + }); + + it("omits the ID Token row when only an access token is present", () => { + renderWithMantine( + , + ); + expect(screen.getByText("Access Token")).toBeInTheDocument(); + expect(screen.queryByText("ID Token")).not.toBeInTheDocument(); }); it("does not render OAuth section when oauth prop is omitted", () => { diff --git a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.tsx b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.tsx index a7cd53f2..dde264b6 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.tsx +++ b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.tsx @@ -24,7 +24,7 @@ import { import { ContentViewer } from "../../elements/ContentViewer/ContentViewer"; import { EraBadge } from "../../elements/EraBadge/EraBadge"; import { isModernEra } from "../../elements/EraBadge/eraUtils"; -import { OAuthAccessTokenField } from "./OAuthAccessTokenField"; +import { OAuthTokenField } from "./OAuthTokenField"; export interface OAuthDetails { protocol: "standard" | "ema"; @@ -34,6 +34,13 @@ export interface OAuthDetails { authUrl?: string; scopes?: string[]; accessToken?: string; + /** + * OIDC `id_token` from the stored token set, when the authorization server + * returned one. Shown for inspection only — the MCP authorization spec is + * plain OAuth 2.1 and the Inspector never treats this as a credential + * (#2019). Absent when the token set carries none, so no empty row renders. + */ + idToken?: string; /** EMA only — install-level IdP session for legs 1–2. */ idpSession?: "none" | "logged_in" | "expired"; } @@ -370,20 +377,27 @@ export function ConnectionInfoContent({ {formatScopes(oauth.scopes)} )} - {oauth.accessToken ? ( - - ) : ( - onClearOAuth && ( - - - {CLEAR_OAUTH_STATE_AND_DISCONNECT_LABEL} - - - ) + )} + {/* Viewer only — an `id_token` the AS happened to return, decoded + on request. It carries no clear action: clearing OAuth state is + one action for the whole token set, owned by the access-token + row (or the standalone button below when there is none). */} + {oauth.idToken && ( + + )} + {!oauth.accessToken && onClearOAuth && ( + + + {CLEAR_OAUTH_STATE_AND_DISCONNECT_LABEL} + + )} diff --git a/clients/web/src/components/groups/ConnectionInfoContent/OAuthAccessTokenField.test.tsx b/clients/web/src/components/groups/ConnectionInfoContent/OAuthAccessTokenField.test.tsx deleted file mode 100644 index f1380f12..00000000 --- a/clients/web/src/components/groups/ConnectionInfoContent/OAuthAccessTokenField.test.tsx +++ /dev/null @@ -1,90 +0,0 @@ -import { describe, it, expect, vi } from "vitest"; -import userEvent from "@testing-library/user-event"; -import { renderWithMantine, screen } from "../../../test/renderWithMantine"; -import { CLEAR_OAUTH_STATE_AND_DISCONNECT_LABEL } from "./ConnectionInfoContent"; -import { OAuthAccessTokenField } from "./OAuthAccessTokenField"; - -const jwt = "eyJhbGciOiJub25lIn0.eyJzdWIiOiJ1c2VyIn0."; - -describe("OAuthAccessTokenField", () => { - it("renders token with copy control beside the content", () => { - renderWithMantine(); - expect(screen.getByText("Access Token")).toBeInTheDocument(); - expect(screen.getByText(/eyJhbGciOiJub25lIn0/)).toBeInTheDocument(); - expect(screen.getByText(/eyJzdWIiOiJ1c2VyIn0/)).toBeInTheDocument(); - expect(screen.getByRole("button", { name: "Copy" })).toBeInTheDocument(); - }); - - it("replaces raw token with decoded JSON and restores on toggle", async () => { - const user = userEvent.setup(); - renderWithMantine(); - - expect(screen.getByText(/eyJhbGciOiJub25lIn0/)).toBeInTheDocument(); - await user.click(screen.getByRole("button", { name: "Decode JWT" })); - expect(screen.getByText(/"sub": "user"/)).toBeInTheDocument(); - expect(screen.queryByText(/eyJhbGciOiJub25lIn0/)).not.toBeInTheDocument(); - - await user.click(screen.getByRole("button", { name: "Show token" })); - expect(screen.getByText(/eyJhbGciOiJub25lIn0/)).toBeInTheDocument(); - expect(screen.queryByText(/"sub": "user"/)).not.toBeInTheDocument(); - }); - - it("omits decode toggle for opaque tokens", () => { - renderWithMantine( - , - ); - expect( - screen.queryByRole("button", { name: "Decode JWT" }), - ).not.toBeInTheDocument(); - }); - - it("copies decoded JSON while decode view is shown", async () => { - const user = userEvent.setup(); - const writeText = vi.fn().mockResolvedValue(undefined); - Object.defineProperty(navigator, "clipboard", { - value: { writeText }, - configurable: true, - }); - - renderWithMantine(); - await user.click(screen.getByRole("button", { name: "Decode JWT" })); - await user.click(screen.getByRole("button", { name: "Copy" })); - - expect(writeText).toHaveBeenCalledWith( - expect.stringContaining('"sub": "user"'), - ); - expect(writeText).not.toHaveBeenCalledWith(jwt); - }); - - it("copies the raw token while token view is shown", async () => { - const user = userEvent.setup(); - const writeText = vi.fn().mockResolvedValue(undefined); - Object.defineProperty(navigator, "clipboard", { - value: { writeText }, - configurable: true, - }); - - renderWithMantine(); - await user.click(screen.getByRole("button", { name: "Copy" })); - - expect(writeText).toHaveBeenCalledWith(jwt); - }); - - it("renders clear action on the access token header row", async () => { - const user = userEvent.setup(); - const onClear = vi.fn(); - renderWithMantine( - , - ); - await user.click( - screen.getByRole("button", { - name: CLEAR_OAUTH_STATE_AND_DISCONNECT_LABEL, - }), - ); - expect(onClear).toHaveBeenCalledTimes(1); - }); -}); diff --git a/clients/web/src/components/groups/ConnectionInfoContent/OAuthTokenField.test.tsx b/clients/web/src/components/groups/ConnectionInfoContent/OAuthTokenField.test.tsx new file mode 100644 index 00000000..a46e6538 --- /dev/null +++ b/clients/web/src/components/groups/ConnectionInfoContent/OAuthTokenField.test.tsx @@ -0,0 +1,139 @@ +import { describe, it, expect, vi } from "vitest"; +import userEvent from "@testing-library/user-event"; +import { renderWithMantine, screen } from "../../../test/renderWithMantine"; +import { CLEAR_OAUTH_STATE_AND_DISCONNECT_LABEL } from "./ConnectionInfoContent"; +import { OAuthTokenField } from "./OAuthTokenField"; + +const jwt = "eyJhbGciOiJub25lIn0.eyJzdWIiOiJ1c2VyIn0."; + +describe("OAuthTokenField", () => { + it("renders token with copy control beside the content", () => { + renderWithMantine(); + expect(screen.getByText("Access Token")).toBeInTheDocument(); + expect(screen.getByText(/eyJhbGciOiJub25lIn0/)).toBeInTheDocument(); + expect(screen.getByText(/eyJzdWIiOiJ1c2VyIn0/)).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: "Copy Access Token" }), + ).toBeInTheDocument(); + }); + + it("replaces raw token with decoded JSON and restores on toggle", async () => { + const user = userEvent.setup(); + renderWithMantine(); + + expect(screen.getByText(/eyJhbGciOiJub25lIn0/)).toBeInTheDocument(); + await user.click( + screen.getByRole("button", { name: "Decode JWT for Access Token" }), + ); + expect(screen.getByText(/"sub": "user"/)).toBeInTheDocument(); + expect(screen.queryByText(/eyJhbGciOiJub25lIn0/)).not.toBeInTheDocument(); + + await user.click( + screen.getByRole("button", { name: "Show token for Access Token" }), + ); + expect(screen.getByText(/eyJhbGciOiJub25lIn0/)).toBeInTheDocument(); + expect(screen.queryByText(/"sub": "user"/)).not.toBeInTheDocument(); + }); + + it("omits decode toggle for opaque tokens", () => { + renderWithMantine( + , + ); + expect( + screen.queryByRole("button", { name: "Decode JWT for Access Token" }), + ).not.toBeInTheDocument(); + }); + + it("copies decoded JSON while decode view is shown", async () => { + const user = userEvent.setup(); + const writeText = vi.fn().mockResolvedValue(undefined); + Object.defineProperty(navigator, "clipboard", { + value: { writeText }, + configurable: true, + }); + + renderWithMantine(); + await user.click( + screen.getByRole("button", { name: "Decode JWT for Access Token" }), + ); + await user.click(screen.getByRole("button", { name: "Copy Access Token" })); + + expect(writeText).toHaveBeenCalledWith( + expect.stringContaining('"sub": "user"'), + ); + expect(writeText).not.toHaveBeenCalledWith(jwt); + }); + + it("copies the raw token while token view is shown", async () => { + const user = userEvent.setup(); + const writeText = vi.fn().mockResolvedValue(undefined); + Object.defineProperty(navigator, "clipboard", { + value: { writeText }, + configurable: true, + }); + + renderWithMantine(); + await user.click(screen.getByRole("button", { name: "Copy Access Token" })); + + expect(writeText).toHaveBeenCalledWith(jwt); + }); + + it("renders clear action on the access token header row", async () => { + const user = userEvent.setup(); + const onClear = vi.fn(); + renderWithMantine( + , + ); + await user.click( + screen.getByRole("button", { + name: CLEAR_OAUTH_STATE_AND_DISCONNECT_LABEL, + }), + ); + expect(onClear).toHaveBeenCalledTimes(1); + }); + + it("labels the row from the label prop and decodes an ID token", async () => { + const user = userEvent.setup(); + // header {"alg":"none"} / payload {"sub":"user"} — same shape an AS returns + // for an `id_token`; the decode path is identical to the access token's. + renderWithMantine(); + + expect(screen.getByText("ID Token")).toBeInTheDocument(); + expect(screen.queryByText("Access Token")).not.toBeInTheDocument(); + + await user.click( + screen.getByRole("button", { name: "Decode JWT for ID Token" }), + ); + expect(screen.getByText(/"sub": "user"/)).toBeInTheDocument(); + }); + + it("qualifies the decode and copy accessible names with the row label", () => { + // Two rows can render simultaneously (Access Token + ID Token), so the + // controls must not share accessible names — see #2019 review. + renderWithMantine(); + + expect( + screen.getByRole("button", { name: "Decode JWT for ID Token" }), + ).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: "Copy ID Token" }), + ).toBeInTheDocument(); + }); + + it("omits the clear action when no handler is given", () => { + renderWithMantine(); + expect( + screen.queryByRole("button", { + name: CLEAR_OAUTH_STATE_AND_DISCONNECT_LABEL, + }), + ).not.toBeInTheDocument(); + }); +}); diff --git a/clients/web/src/components/groups/ConnectionInfoContent/OAuthAccessTokenField.tsx b/clients/web/src/components/groups/ConnectionInfoContent/OAuthTokenField.tsx similarity index 70% rename from clients/web/src/components/groups/ConnectionInfoContent/OAuthAccessTokenField.tsx rename to clients/web/src/components/groups/ConnectionInfoContent/OAuthTokenField.tsx index 825a8507..abd5c4a9 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/OAuthAccessTokenField.tsx +++ b/clients/web/src/components/groups/ConnectionInfoContent/OAuthTokenField.tsx @@ -3,8 +3,18 @@ import { Button, Code, Flex, Stack, Text } from "@mantine/core"; import { decodeJwtPayload, isJwtFormat } from "@inspector/core/auth/ema/jwt.js"; import { CopyButton } from "../../elements/CopyButton/CopyButton"; -export interface OAuthAccessTokenFieldProps { - accessToken: string; +/** + * One labelled OAuth token row: the raw value, a copy control, and — when the + * value has the three-segment JWT shape — an in-place "Decode JWT" toggle. + * + * Shared by the Access Token and ID Token rows (#2019). The decode is + * display-only (`decodeJwtPayload` does no signature verification), so this is + * a viewer: neither token is treated as a credential here. + */ +export interface OAuthTokenFieldProps { + /** Row caption, e.g. "Access Token" / "ID Token". */ + label: string; + token: string; onClear?: () => void; clearLabel?: string; } @@ -64,16 +74,17 @@ function JwtTokenText({ token }: { token: string }) { ); } -export function OAuthAccessTokenField({ - accessToken, +export function OAuthTokenField({ + label, + token, onClear, clearLabel = "Clear", -}: OAuthAccessTokenFieldProps) { +}: OAuthTokenFieldProps) { const [showDecoded, setShowDecoded] = useState(false); - const isJwt = isJwtFormat(accessToken); + const isJwt = isJwtFormat(token); const jwtDecoded = useMemo( - () => (isJwt ? decodeJwtPayload(accessToken) : undefined), - [accessToken, isJwt], + () => (isJwt ? decodeJwtPayload(token) : undefined), + [token, isJwt], ); const decodedText = useMemo(() => { @@ -85,17 +96,18 @@ export function OAuthAccessTokenField({ ); }, [jwtDecoded]); - const copyValue = showDecoded && decodedText ? decodedText : accessToken; + const copyValue = showDecoded && decodedText ? decodedText : token; return ( - Access Token + {label} {jwtDecoded && ( setShowDecoded((open) => !open)} aria-pressed={showDecoded} + aria-label={`${showDecoded ? "Show token" : "Decode JWT"} for ${label}`} > {showDecoded ? "Show token" : "Decode JWT"} @@ -113,13 +125,13 @@ export function OAuthAccessTokenField({ {showDecoded && decodedText ? ( decodedText ) : isJwt ? ( - + ) : ( - accessToken + token )} - + ); diff --git a/clients/web/src/components/groups/ConnectionInfoContent/oauthDetailsFromConnectionState.test.ts b/clients/web/src/components/groups/ConnectionInfoContent/oauthDetailsFromConnectionState.test.ts index d6a89da6..d047ab23 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/oauthDetailsFromConnectionState.test.ts +++ b/clients/web/src/components/groups/ConnectionInfoContent/oauthDetailsFromConnectionState.test.ts @@ -34,6 +34,39 @@ describe("oauthDetailsFromConnectionState", () => { }); }); + it("surfaces an id_token when the token set carries one", () => { + const state: OAuthConnectionState = { + authorized: true, + protocol: "standard", + serverUrl: "https://mcp.example.com/mcp", + tokens: { + access_token: "tok", + token_type: "Bearer", + id_token: "header.payload.signature", + }, + }; + + expect(oauthDetailsFromConnectionState(state)).toEqual({ + protocol: "standard", + authorized: true, + accessToken: "tok", + idToken: "header.payload.signature", + }); + }); + + it("omits idToken when the token set carries none", () => { + const state: OAuthConnectionState = { + authorized: true, + protocol: "standard", + serverUrl: "https://mcp.example.com/mcp", + tokens: { access_token: "tok", token_type: "Bearer" }, + }; + + expect(oauthDetailsFromConnectionState(state)).not.toHaveProperty( + "idToken", + ); + }); + it("includes EMA idp session when present", () => { const state: OAuthConnectionState = { authorized: false, diff --git a/clients/web/src/components/groups/ConnectionInfoContent/oauthDetailsFromConnectionState.ts b/clients/web/src/components/groups/ConnectionInfoContent/oauthDetailsFromConnectionState.ts index 14c8847e..2e514633 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/oauthDetailsFromConnectionState.ts +++ b/clients/web/src/components/groups/ConnectionInfoContent/oauthDetailsFromConnectionState.ts @@ -22,6 +22,11 @@ export function oauthDetailsFromConnectionState( ...(state.tokens?.access_token && { accessToken: state.tokens.access_token, }), + // Surfaced for inspection when the AS returned one; omitted entirely + // otherwise so Connection Info renders no empty row (#2019). + ...(state.tokens?.id_token && { + idToken: state.tokens.id_token, + }), ...(state.protocol === "ema" && state.ema?.idpSession && { idpSession: state.ema.idpSession }), }; diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx index 43b400bf..d238228a 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx @@ -700,10 +700,272 @@ describe("ServerSettingsForm", () => { clientId: "a", clientSecret: "", scopes: "", + authorizationParams: [], enterpriseManaged: false, }); }); + // #2018 — custom authorization-request parameters. + it("shows the empty hint for authorization parameters when none are set", () => { + renderWithMantine( + , + ); + expect( + screen.getByText("Additional authorization parameters"), + ).toBeInTheDocument(); + expect( + screen.getByText("No additional parameters configured"), + ).toBeInTheDocument(); + }); + + it("adds an authorization-parameter row through onOAuthChange", async () => { + const user = userEvent.setup(); + const onOAuthChange = vi.fn(); + renderWithMantine( + , + ); + await user.click(screen.getByRole("button", { name: "+ Add Parameter" })); + expect(onOAuthChange).toHaveBeenCalledWith( + expect.objectContaining({ + authorizationParams: [{ key: "", value: "" }], + }), + ); + }); + + it("edits and removes an authorization-parameter row", async () => { + const user = userEvent.setup(); + const onOAuthChange = vi.fn(); + renderWithMantine( + , + ); + const valueInput = screen.getByDisplayValue("corp"); + await user.type(valueInput, "x"); + expect(onOAuthChange).toHaveBeenLastCalledWith( + expect.objectContaining({ + authorizationParams: [{ key: "kc_idp_hint", value: "corpx" }], + }), + ); + + onOAuthChange.mockClear(); + const removeButton = screen + .getAllByRole("button") + .find((b) => b.textContent === "X"); + await user.click(removeButton as HTMLElement); + expect(onOAuthChange).toHaveBeenLastCalledWith( + expect.objectContaining({ authorizationParams: [] }), + ); + }); + + it("clears an authorization-parameter key via its Clear button", async () => { + const user = userEvent.setup(); + const onOAuthChange = vi.fn(); + renderWithMantine( + , + ); + await user.click(clearButtonFor(screen.getByDisplayValue("kc_idp_hint"))); + expect(onOAuthChange).toHaveBeenLastCalledWith( + expect.objectContaining({ + authorizationParams: [{ key: "", value: "corp" }], + }), + ); + + onOAuthChange.mockClear(); + await user.click(clearButtonFor(screen.getByDisplayValue("corp"))); + expect(onOAuthChange).toHaveBeenLastCalledWith( + expect.objectContaining({ + authorizationParams: [{ key: "kc_idp_hint", value: "" }], + }), + ); + }); + + it("rejects a reserved authorization-parameter key inline and with a warning", () => { + renderWithMantine( + , + ); + expect( + screen.getByText( + '"state" is set by the authorization flow and cannot be overridden.', + ), + ).toBeInTheDocument(); + expect(screen.getByText("Reserved parameters ignored")).toBeInTheDocument(); + expect(screen.getByDisplayValue("state")).toHaveAttribute( + "aria-invalid", + "true", + ); + }); + + // #2018 — the section heading and the placeholders are not programmatically + // associated with these inputs, so each control carries its own aria-label and + // the reserved-key reason is the input's `error` string (which Mantine wires + // to the input via aria-describedby) rather than free-standing text. + it("gives each authorization-parameter control an accessible name", () => { + renderWithMantine( + , + ); + expect( + screen.getByRole("textbox", { + name: "Authorization parameter name, kc_idp_hint", + }), + ).toHaveValue("kc_idp_hint"); + expect( + screen.getByRole("textbox", { + name: "Authorization parameter value, kc_idp_hint", + }), + ).toHaveValue("corp"); + expect( + screen.getByRole("button", { + name: "Remove authorization parameter kc_idp_hint", + }), + ).toBeInTheDocument(); + }); + + it("falls back to a row number in the accessible name when the key is blank", () => { + renderWithMantine( + , + ); + expect( + screen.getByRole("textbox", { + name: "Authorization parameter name, row 1", + }), + ).toBeInTheDocument(); + }); + + // The reason must reach assistive tech through the input, not just as text on + // the page: Mantine emits the `error` string with an id the input references. + it("associates the reserved-key reason with the key input", () => { + renderWithMantine( + , + ); + const keyInput = screen.getByRole("textbox", { + name: "Authorization parameter name, state", + }); + const describedBy = keyInput.getAttribute("aria-describedby"); + expect(describedBy).toBeTruthy(); + const reason = describedBy + ?.split(" ") + .map((id) => document.getElementById(id)?.textContent ?? "") + .join(" "); + expect(reason).toContain( + '"state" is set by the authorization flow and cannot be overridden.', + ); + }); + + // #2018 — the EMA leg authorizes against the enterprise IdP, a different + // authorization server, and deliberately sends none of these parameters. The + // description has to say so, or the form shows a configured value as active + // when it will not be sent. + it("warns that authorization parameters are unused under enterprise-managed auth", () => { + const { rerender } = renderWithMantine( + , + ); + expect( + screen.getByText(/Not sent while Enterprise-managed authorization is on/), + ).toBeInTheDocument(); + + rerender( + , + ); + expect( + screen.queryByText( + /Not sent while Enterprise-managed authorization is on/, + ), + ).not.toBeInTheDocument(); + }); + + it("does not flag a blank authorization-parameter row as reserved", () => { + renderWithMantine( + , + ); + expect( + screen.queryByText("Reserved parameters ignored"), + ).not.toBeInTheDocument(); + }); + + it("pluralizes the reserved-parameter warning for multiple keys", () => { + renderWithMantine( + , + ); + expect(screen.getByText(/state, scope are set/)).toBeInTheDocument(); + }); + it("invokes onOAuthChange with the chosen insufficient-scope policy (SEP-2350)", async () => { const user = userEvent.setup(); const onOAuthChange = vi.fn(); @@ -744,6 +1006,7 @@ describe("ServerSettingsForm", () => { clientId: "", clientSecret: "", scopes: "", + authorizationParams: [], enterpriseManaged: true, }); }); @@ -848,6 +1111,7 @@ describe("ServerSettingsForm", () => { clientId: "", clientSecret: "z", scopes: "", + authorizationParams: [], enterpriseManaged: false, }); }); diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx index e20ddd44..276c9cf9 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx @@ -1,6 +1,7 @@ import { Accordion, ActionIcon, + Alert, Button, Checkbox, Flex, @@ -26,6 +27,11 @@ import { MODERN_LOG_LEVELS, } from "@inspector/core/mcp/types.js"; import { isOAuthCapableServerType } from "@inspector/core/mcp/config.js"; +import { + RESERVED_AUTHORIZATION_PARAMS, + authorizationParamKeyError, + isReservedAuthorizationParam, +} from "@inspector/core/auth/authorizationParams.js"; import { ADVERTISABLE_EXTENSIONS } from "@inspector/core/mcp/extensions.js"; import type { Root } from "@modelcontextprotocol/client"; @@ -184,6 +190,25 @@ const EmptyHint = Text.withProps({ fs: "italic", }); +// Matches the label/description pair Mantine renders for a single input, so the +// authorization-parameter row list reads as one labelled field next to Scopes. +const FieldLabel = Text.withProps({ + size: "sm", + fw: 500, +}); + +const FieldDescription = Text.withProps({ + size: "xs", + c: "dimmed", +}); + +// `align="flex-start"` keeps the key/value/remove controls top-aligned when the +// key input grows an error message, so the row doesn't jump as it appears. +const AuthorizationParamRow = Group.withProps({ + grow: true, + align: "flex-start", +}); + const ClearStoredOAuthButton = Button.withProps({ variant: "light", color: "red", @@ -245,6 +270,74 @@ function KeyValueRows({ ); } +// Reserved-key rejection is inline per row (#2018): the reason is passed as the +// key input's `error` **string**, so Mantine renders it in the input's own error +// slot and wires the `aria-describedby` association — a bare boolean would mark +// the field invalid without telling a screen reader why. The section-level Alert +// below repeats it so the rejection stays visible when the offending row is +// scrolled out of view. +// +// Each control carries a per-row `aria-label`: the section heading and the +// placeholders are not programmatically associated with the inputs, so without +// one an assistive technology cannot tell the key box from the value box, and +// the remove button announces only "X". +function AuthorizationParamRows({ + params, + onChange, + onRemove, +}: { + params: { key: string; value: string }[]; + onChange: (index: number, key: string, value: string) => void; + onRemove: (index: number) => void; +}) { + return ( + <> + {params.map((param, index) => { + const rowLabel = param.key.trim() || `row ${index + 1}`; + return ( + + + onChange(index, e.currentTarget.value, param.value) + } + rightSection={ + param.key ? ( + onChange(index, "", param.value)} + /> + ) : null + } + /> + + onChange(index, param.key, e.currentTarget.value) + } + rightSection={ + param.value ? ( + onChange(index, param.key, "")} /> + ) : null + } + /> + onRemove(index)} + > + X + + + ); + })} + + ); +} + function RootRows({ roots, onChange, @@ -374,16 +467,52 @@ export function ServerSettingsForm({ onTimeoutChange(field, numValue); }; + // #2018 — custom authorization-request parameters. The rows ride the single + // `onOAuthChange` callback with the rest of the Authorization section rather + // than growing three more props on this form. + const authorizationParams = settings.oauthAuthorizationParams ?? []; + function currentOAuth(): OAuthSettings { return { clientId: settings.oauthClientId ?? "", clientSecret: settings.oauthClientSecret ?? "", scopes: settings.oauthScopes ?? "", + authorizationParams, enterpriseManaged: settings.enterpriseManaged ?? false, onInsufficientScope: settings.oauthOnInsufficientScope, }; } + const rejectedParamKeys = authorizationParams + .map((p) => p.key.trim()) + .filter((key) => isReservedAuthorizationParam(key)); + + function changeAuthorizationParams( + params: { key: string; value: string }[], + ): void { + onOAuthChange({ ...currentOAuth(), authorizationParams: params }); + } + + function handleAddAuthorizationParam(): void { + changeAuthorizationParams([...authorizationParams, { key: "", value: "" }]); + } + + function handleRemoveAuthorizationParam(index: number): void { + changeAuthorizationParams( + authorizationParams.filter((_, i) => i !== index), + ); + } + + function handleAuthorizationParamChange( + index: number, + key: string, + value: string, + ): void { + changeAuthorizationParams( + authorizationParams.map((p, i) => (i === index ? { key, value } : p)), + ); + } + return ( // Stays inline: Accordion is a compound, `multiple`-discriminated generic, // so `.withProps({ multiple: true, ... })` loses its JSX call signature @@ -699,6 +828,41 @@ export function ServerSettingsForm({ ) : null } /> + + Additional authorization parameters + + Extra query parameters appended to the authorization request + only — never the token request. Use them for provider-specific + hints such as kc_idp_hint, login_hint, prompt, acr_values, or + audience. + {settings.enterpriseManaged + ? " Not sent while Enterprise-managed authorization is on: that flow authorizes against the enterprise IdP, a different authorization server." + : ""} + + {authorizationParams.length === 0 ? ( + No additional parameters configured + ) : ( + + )} + {rejectedParamKeys.length > 0 ? ( + + {`${rejectedParamKeys.join(", ")} ${ + rejectedParamKeys.length === 1 ? "is" : "are" + } set by the authorization flow and will not be sent. Overriding ${ + rejectedParamKeys.length === 1 ? "it" : "them" + } would break PKCE, the CSRF state binding, or the resource indicator. Reserved: ${RESERVED_AUTHORIZATION_PARAMS.join( + ", ", + )}.`} + + ) : null} + + + Add Parameter + +