diff --git a/AGENTS.md b/AGENTS.md index 0baea459..cfd08caa 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 509fc079..97aefe73 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"; @@ -2301,11 +2302,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 && { @@ -2317,6 +2322,9 @@ function App() { ...(savedSettings.oauthScopes && { scope: savedSettings.oauthScopes, }), + ...(serverAuthorizationParams && { + authorizationParams: serverAuthorizationParams, + }), ...(savedSettings.enterpriseManaged && { enterpriseManaged: true, }), diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx index 43b400bf..b66d76ac 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx @@ -700,10 +700,165 @@ 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", + ); + }); + + 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 +899,7 @@ describe("ServerSettingsForm", () => { clientId: "", clientSecret: "", scopes: "", + authorizationParams: [], enterpriseManaged: true, }); }); @@ -848,6 +1004,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..6a3bee1c 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,23 @@ 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", +}); + +const ReservedParamError = Text.withProps({ + size: "xs", + c: "var(--inspector-danger-text)", +}); + const ClearStoredOAuthButton = Button.withProps({ variant: "light", color: "red", @@ -245,6 +268,63 @@ function KeyValueRows({ ); } +// Reserved-key rejection is inline per row (#2018): the key input goes red with +// the reason under it, and the section-level warning below repeats it so the +// rejection is visible even when the offending row is scrolled out of view. +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) => ( + + + + 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 + + {authorizationParamKeyError(param.key) ? ( + + {authorizationParamKeyError(param.key)} + + ) : null} + + ))} + + ); +} + function RootRows({ roots, onChange, @@ -374,16 +454,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 +815,38 @@ 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. + + {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 + +