* Server Settings: disambiguate EMA OAuth credential fields (#1692) Under enterprise-managed authorization the OAuth Settings "Client ID" / "Client Secret" fields hold the *resource authorization server* credentials (EMA leg 3 — the Test Client from the xaa.dev resource registration), not the app/IdP pair configured in Client Settings. The unqualified labels sat under a toggle naming the enterprise IdP, pulling readers toward the wrong pair. - When enterpriseManaged is on, relabel to "Resource AS Client ID" / "Resource AS Client Secret" and add a description naming the resource authorization server and pointing the app pair back to Client Settings. - Document the Scopes delimiter: space-separated (RFC 6749), with a warning that commas produce one invalid token, plus a placeholder example. Adds tests for both label states and the scopes hint, plus an EnterpriseManagedOAuth story. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * pr-screenshots: OAuth EMA field disambiguation proof shots (#1692) Add before/after screenshots of the Server Settings → OAuth Settings fields (EMA off = plain Client ID/Secret; EMA on = Resource AS Client ID/Secret) plus the new space-separated Scopes hint, with a README section documenting them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Address review: genericize EMA copy, reuse enterpriseManaged const (#1692) - Drop the hardcoded xaa.dev staging host from the resource-AS field description (it's a general EMA concept, not env-specific); phrase it as "the resource authorization server's registered client credential (EMA leg 3)". Also trims the copy's overlap with the checkbox description. - Reuse the existing `enterpriseManaged` const for the EMA Checkbox's `checked` prop instead of recomputing `settings.enterpriseManaged ?? false`. - Update the test assertion and pr-screenshots README/EMA-on screenshot to match the new copy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 * Client Settings: mirror EMA disambiguation (IdP-qualified labels) (#1692) The Server Settings OAuth fields now label the leg-3 resource-AS pair explicitly; make Client Settings parallel so each modal names its pair and points at where the other lives. - Relabel the EMA credential fields to "IdP Client ID" / "IdP Client Secret" (parallel to Server Settings' "Resource AS Client ID/Secret"), so the label alone disambiguates without relying on the description. - Add the reciprocal cross-reference to each description: these are the enterprise IdP pair (EMA legs 1–2), not the per-server resource authorization server credentials, which go in Server Settings → OAuth Settings. - Update ClientSettingsForm/ClientSettingsModal tests for the new labels; add a pr-screenshots section + Client Settings screenshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall
GitHub
parent
c51d6aabd2
commit
402d7b204d
6 changed files with 131 additions and 12 deletions
|
|
@ -475,7 +475,7 @@ describe("ClientSettingsForm interactions", () => {
|
|||
/>,
|
||||
);
|
||||
|
||||
await user.type(screen.getByLabelText("Client ID"), "x");
|
||||
await user.type(screen.getByLabelText("IdP Client ID"), "x");
|
||||
expect(onSettingsChange).toHaveBeenLastCalledWith(expect.any(Function));
|
||||
expect(
|
||||
resolveSettingsChange(
|
||||
|
|
@ -485,7 +485,7 @@ describe("ClientSettingsForm interactions", () => {
|
|||
).toBe("x");
|
||||
|
||||
onSettingsChange.mockClear();
|
||||
await user.type(screen.getByLabelText("Client Secret"), "s");
|
||||
await user.type(screen.getByLabelText("IdP Client Secret"), "s");
|
||||
expect(onSettingsChange).toHaveBeenLastCalledWith(expect.any(Function));
|
||||
expect(
|
||||
resolveSettingsChange(
|
||||
|
|
|
|||
|
|
@ -177,8 +177,8 @@ export function ClientSettingsForm({
|
|||
}
|
||||
/>
|
||||
<TextInput
|
||||
label="Client ID"
|
||||
description="Client id registered with your enterprise IdP."
|
||||
label="IdP Client ID"
|
||||
description="Client id registered with your enterprise IdP (EMA legs 1–2) — not the per-server resource authorization server credentials, which go in Server Settings → OAuth Settings."
|
||||
value={settings.clientId}
|
||||
onChange={(e) => patch({ clientId: e.currentTarget.value })}
|
||||
error={showClientIdError}
|
||||
|
|
@ -190,8 +190,8 @@ export function ClientSettingsForm({
|
|||
}
|
||||
/>
|
||||
<TextInput
|
||||
label="Client Secret"
|
||||
description="Client secret for your enterprise IdP (if required)."
|
||||
label="IdP Client Secret"
|
||||
description="Client secret for your enterprise IdP, if required (EMA legs 1–2) — not the per-server resource authorization server credentials, which go in Server Settings → OAuth Settings."
|
||||
type="password"
|
||||
value={settings.clientSecret}
|
||||
onChange={(e) =>
|
||||
|
|
|
|||
|
|
@ -95,8 +95,8 @@ describe("ClientSettingsModal", () => {
|
|||
/>,
|
||||
);
|
||||
expect(screen.getByLabelText("Issuer")).toBeInTheDocument();
|
||||
expect(screen.getByLabelText("Client ID")).toBeInTheDocument();
|
||||
expect(screen.getByLabelText("Client Secret")).toBeInTheDocument();
|
||||
expect(screen.getByLabelText("IdP Client ID")).toBeInTheDocument();
|
||||
expect(screen.getByLabelText("IdP Client Secret")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("calls onSettingsChange when typing issuer", async () => {
|
||||
|
|
@ -260,7 +260,7 @@ describe("ClientSettingsModal", () => {
|
|||
await clickClose(user); // blocked on blank clientId
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
|
||||
await user.type(screen.getByLabelText("Client ID"), "client-1");
|
||||
await user.type(screen.getByLabelText("IdP Client ID"), "client-1");
|
||||
await clickClose(user); // now complete -> closes
|
||||
expect(onClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -257,6 +257,19 @@ export const WithOAuth: Story = {
|
|||
},
|
||||
};
|
||||
|
||||
export const EnterpriseManagedOAuth: Story = {
|
||||
args: {
|
||||
expandedSections: ["oauth"],
|
||||
settings: {
|
||||
...defaultSettings,
|
||||
enterpriseManaged: true,
|
||||
oauthClientId: "resource-test-client-id",
|
||||
oauthClientSecret: "resource-test-client-secret",
|
||||
oauthScopes: "mcp tools:read env:read",
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
export const AllConfigured: Story = {
|
||||
args: {
|
||||
settings: {
|
||||
|
|
|
|||
|
|
@ -748,6 +748,57 @@ describe("ServerSettingsForm", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("uses unqualified Client ID / Client Secret labels when EMA is off (#1692)", () => {
|
||||
renderWithMantine(
|
||||
<ServerSettingsForm
|
||||
{...baseHandlers}
|
||||
settings={emptySettings}
|
||||
expandedSections={["oauth"]}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByLabelText("Client ID")).toBeInTheDocument();
|
||||
expect(screen.getByLabelText("Client Secret")).toBeInTheDocument();
|
||||
expect(screen.queryByLabelText("Resource AS Client ID")).toBeNull();
|
||||
});
|
||||
|
||||
it("relabels the OAuth fields to Resource AS credentials when EMA is on (#1692)", () => {
|
||||
renderWithMantine(
|
||||
<ServerSettingsForm
|
||||
{...baseHandlers}
|
||||
settings={{ ...emptySettings, enterpriseManaged: true }}
|
||||
expandedSections={["oauth"]}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByLabelText("Resource AS Client ID")).toBeInTheDocument();
|
||||
expect(
|
||||
screen.getByLabelText("Resource AS Client Secret"),
|
||||
).toBeInTheDocument();
|
||||
expect(screen.queryByLabelText("Client ID")).toBeNull();
|
||||
// The resource-authorization-server description is present (rendered twice —
|
||||
// once per field).
|
||||
expect(
|
||||
screen.getAllByText(
|
||||
/resource authorization server's registered client credential/i,
|
||||
).length,
|
||||
).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("documents the space-separated Scopes delimiter (#1692)", () => {
|
||||
renderWithMantine(
|
||||
<ServerSettingsForm
|
||||
{...baseHandlers}
|
||||
settings={emptySettings}
|
||||
expandedSections={["oauth"]}
|
||||
/>,
|
||||
);
|
||||
expect(
|
||||
screen.getByText(/Space-separated OAuth scopes/i),
|
||||
).toBeInTheDocument();
|
||||
expect(
|
||||
screen.getByPlaceholderText("mcp tools:read env:read"),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("hides the OAuth Settings section for stdio servers", () => {
|
||||
renderWithMantine(
|
||||
<ServerSettingsForm
|
||||
|
|
|
|||
|
|
@ -305,6 +305,21 @@ export function ServerSettingsForm({
|
|||
onRootChange,
|
||||
}: ServerSettingsFormProps) {
|
||||
const oauthCapable = isOAuthCapableServerType(serverType);
|
||||
// Under enterprise-managed auth these fields hold the *resource authorization
|
||||
// server* credentials (leg 3 — its registered client), not the app/IdP pair
|
||||
// configured in Client Settings. The toggle's label mentions the IdP, which
|
||||
// pulls the reader toward the wrong pair — so qualify the labels and add a
|
||||
// description when EMA is on (#1692).
|
||||
const enterpriseManaged = settings.enterpriseManaged ?? false;
|
||||
const clientIdLabel = enterpriseManaged
|
||||
? "Resource AS Client ID"
|
||||
: "Client ID";
|
||||
const clientSecretLabel = enterpriseManaged
|
||||
? "Resource AS Client Secret"
|
||||
: "Client Secret";
|
||||
const resourceAsDescription = enterpriseManaged
|
||||
? "The resource authorization server's registered client credential (EMA leg 3) — not the app client id/secret, which belong in Client Settings."
|
||||
: undefined;
|
||||
// The modern per-request "Log Level per Request" control is meaningless on a
|
||||
// legacy connection (#1629). Hide it when legacy is pinned, or when an `auto`
|
||||
// server negotiated legacy at connect time. A pinned `modern` server, and an
|
||||
|
|
@ -591,7 +606,7 @@ export function ServerSettingsForm({
|
|||
<Checkbox
|
||||
label="Enterprise-managed authorization"
|
||||
description="Connect via the configured enterprise IdP instead of interactive OAuth to the MCP authorization server. OAuth fields below are resource authorization server credentials."
|
||||
checked={settings.enterpriseManaged ?? false}
|
||||
checked={enterpriseManaged}
|
||||
onChange={(e) =>
|
||||
onOAuthChange({
|
||||
...currentOAuth(),
|
||||
|
|
@ -604,7 +619,8 @@ export function ServerSettingsForm({
|
|||
authentication
|
||||
</HintText>
|
||||
<TextInput
|
||||
label="Client ID"
|
||||
label={clientIdLabel}
|
||||
description={resourceAsDescription}
|
||||
value={settings.oauthClientId ?? ""}
|
||||
onChange={(e) =>
|
||||
onOAuthChange({
|
||||
|
|
@ -624,7 +640,8 @@ export function ServerSettingsForm({
|
|||
}
|
||||
/>
|
||||
<TextInput
|
||||
label="Client Secret"
|
||||
label={clientSecretLabel}
|
||||
description={resourceAsDescription}
|
||||
value={settings.oauthClientSecret ?? ""}
|
||||
type="password"
|
||||
onChange={(e) =>
|
||||
|
|
@ -646,6 +663,8 @@ export function ServerSettingsForm({
|
|||
/>
|
||||
<TextInput
|
||||
label="Scopes"
|
||||
description="Space-separated OAuth scopes (RFC 6749). Do not use commas — a comma-separated entry is sent as one invalid token and rejected by the authorization server."
|
||||
placeholder="mcp tools:read env:read"
|
||||
value={settings.oauthScopes ?? ""}
|
||||
onChange={(e) =>
|
||||
onOAuthChange({
|
||||
|
|
|
|||
Loading…
Reference in a new issue