Skip to content

Commit 456f125

Browse files
RhysSullivanRhysSullivan
committed
Make spec-detected auth methods immutable in the add flow
A method seeded from spec/probe detection rendered the full None/API key/ OAuth selector and editable fields, so a user could silently switch a detected method (e.g. a Bearer-token API) to OAuth with empty endpoints and end up with a broken integration. Detected methods now render read-only in a disabled state: a lock icon, the auth kind named explicitly (OAuth / API key / No auth, since MCP seeds a detected method as just "Detected"), the declared config shown read-only, and "Pulled from spec. Remove to override." The only action is to remove the row and add a custom one; hand-added methods stay fully editable. This lives in the shared AuthMethodListEditor, so the OpenAPI and MCP add flows both get it. Detection is keyed off an explicit row flag rather than the seed slug, since MCP seeds a detected method without one. Also fix PlacementLine: its inline-flex container trimmed the whitespace at child edges, dropping the space after "Authorization:" and the trailing space in a "Bearer " prefix and rendering "Authorization:Bearer••••••". Render it as plain inline with preserved whitespace.
1 parent f173b2a commit 456f125

3 files changed

Lines changed: 319 additions & 32 deletions

File tree

‎e2e/selfhost/detected-auth-immutable-ui.test.ts‎

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
// Selfhost-only (browser): a spec/probe-DETECTED auth method is immutable in
2+
// the add flow. The shared AuthMethodListEditor renders detected methods as a
3+
// disabled, read-only summary ("Pulled from spec. Remove to override.") with no
4+
// kind selector, so a user can't silently retype the spec's method into a kind
5+
// nothing backs. A method the user adds by hand stays fully editable. Both the
6+
// MCP and OpenAPI add flows compose the same editor, so one behavior, two
7+
// surfaces. Selfhost runs with EXECUTOR_ALLOW_LOCAL_NETWORK so the probe/analyze
8+
// can reach the loopback fixtures. Video is the artifact.
9+
import { randomBytes } from "node:crypto";
10+
import { createServer } from "node:http";
11+
12+
import { expect } from "@effect/vitest";
13+
import { Effect } from "effect";
14+
import { makeGreetingMcpServer, serveMcpServerWithOAuth } from "@executor-js/plugin-mcp/testing";
15+
import { OAuthTestServer } from "@executor-js/sdk/testing";
16+
17+
import { scenario } from "../src/scenario";
18+
import { Browser, Target } from "../src/services";
19+
20+
const REMOVE_HINT = "Pulled from spec. Remove to override.";
21+
22+
scenario(
23+
"Detected auth · an MCP probe's OAuth method is immutable in the add flow",
24+
{},
25+
Effect.scoped(
26+
Effect.gen(function* () {
27+
const target = yield* Target;
28+
const browser = yield* Browser;
29+
// OAuth-protected server: the probe 401s with resource metadata, so the
30+
// method list seeds a single detected OAuth row (discovered metadata).
31+
const server = yield* serveMcpServerWithOAuth(
32+
() => makeGreetingMcpServer({ name: `oauth-mcp-${randomBytes(3).toString("hex")}` }),
33+
{ path: "/mcp" },
34+
);
35+
const identity = yield* target.newIdentity();
36+
37+
yield* browser.session(identity, async ({ page, step }) => {
38+
await step("Open the add-MCP flow pointed at the OAuth server", async () => {
39+
await page.goto(`/integrations/add/mcp?url=${encodeURIComponent(server.endpoint)}`, {
40+
waitUntil: "networkidle",
41+
});
42+
await page.getByText("How does this server authenticate?").waitFor();
43+
await page.getByText("Method 1 · Detected").waitFor();
44+
});
45+
46+
await step("The detected method is locked: read-only, named, no selector", async () => {
47+
// The kind is named explicitly ("OAuth"), the discovered-OAuth summary
48+
// and override hint sit inside a disabled block, and there is NO
49+
// editable kind selector (the FilterTabs render as buttons).
50+
await page.getByText("OAuth", { exact: true }).first().waitFor();
51+
await page.getByText("OAuth metadata is discovered from this server").waitFor();
52+
await page.getByText(REMOVE_HINT).waitFor();
53+
expect(
54+
await page.locator("[aria-disabled]").count(),
55+
"the detected method renders a disabled (non-interactive) block",
56+
).toBeGreaterThan(0);
57+
expect(
58+
await page.getByRole("button", { name: "API key", exact: true }).count(),
59+
"no editable kind selector is shown for the detected method",
60+
).toBe(0);
61+
});
62+
63+
await step("A hand-added method keeps the full editable selector", async () => {
64+
await page.getByRole("button", { name: "Add method" }).click();
65+
await page.getByText("Method 2").waitFor();
66+
// The added row defaults to API key and exposes the None/API key/OAuth
67+
// kind tabs (buttons) — the detected row above still shows none.
68+
expect(
69+
await page.getByRole("button", { name: "API key", exact: true }).count(),
70+
"the added method exposes the kind selector",
71+
).toBeGreaterThan(0);
72+
await page.getByText(REMOVE_HINT).waitFor();
73+
});
74+
});
75+
}),
76+
).pipe(Effect.provide(OAuthTestServer.layer())),
77+
);
78+
79+
/** A real 127.0.0.1 server that serves a static OpenAPI spec for the add flow. */
80+
const serveSpec = (body: string) =>
81+
Effect.acquireRelease(
82+
Effect.callback<{ readonly url: string; readonly close: () => void }>((resume) => {
83+
const server = createServer((_request, response) => {
84+
response.writeHead(200, { "content-type": "application/json" });
85+
response.end(body);
86+
});
87+
server.listen(0, "127.0.0.1", () => {
88+
const address = server.address();
89+
const port = typeof address === "object" && address ? address.port : 0;
90+
resume(
91+
Effect.succeed({
92+
url: `http://127.0.0.1:${port}/spec.json`,
93+
close: () => {
94+
server.close();
95+
server.closeAllConnections();
96+
},
97+
}),
98+
);
99+
});
100+
}),
101+
(server) => Effect.sync(server.close),
102+
);
103+
104+
const apiKeyAndOAuthSpec = (): string =>
105+
JSON.stringify({
106+
openapi: "3.0.3",
107+
info: { title: "Acme Immutable Auth Fixture", version: "1.0.0" },
108+
servers: [{ url: "https://api.acme.test" }],
109+
security: [{ bearerAuth: [] }, { acmeOAuth: ["read"] }],
110+
components: {
111+
securitySchemes: {
112+
bearerAuth: { type: "http", scheme: "bearer" },
113+
acmeOAuth: {
114+
type: "oauth2",
115+
flows: {
116+
authorizationCode: {
117+
authorizationUrl: "https://api.acme.test/oauth/authorize",
118+
tokenUrl: "https://api.acme.test/oauth/token",
119+
scopes: { read: "Read access" },
120+
},
121+
},
122+
},
123+
},
124+
},
125+
paths: {
126+
"/widgets": {
127+
get: {
128+
operationId: "listWidgets",
129+
summary: "List widgets",
130+
responses: { "200": { description: "ok" } },
131+
},
132+
},
133+
},
134+
});
135+
136+
scenario(
137+
"Detected auth · OpenAPI spec-detected methods are immutable in the add flow",
138+
{},
139+
Effect.scoped(
140+
Effect.gen(function* () {
141+
const target = yield* Target;
142+
const browser = yield* Browser;
143+
const spec = yield* serveSpec(apiKeyAndOAuthSpec());
144+
const identity = yield* target.newIdentity();
145+
146+
yield* browser.session(identity, async ({ page, step }) => {
147+
await step("Analyze a spec that declares both API key and OAuth", async () => {
148+
await page.goto(`/integrations/add/openapi`, { waitUntil: "networkidle" });
149+
await page
150+
.getByPlaceholder(/openapi\.json/i)
151+
.first()
152+
.fill(spec.url);
153+
await page.getByText("How does this API authenticate?").waitFor();
154+
await page.getByText("Method 2").waitFor();
155+
});
156+
157+
await step("Both detected methods are locked, named, read-only", async () => {
158+
// Two detected methods, each with the override hint and its kind named
159+
// ("API key" / "OAuth"); the OAuth one shows the spec's real endpoints
160+
// read-only. No editable kind selector (FilterTabs render as buttons).
161+
expect(
162+
await page.getByText(REMOVE_HINT).count(),
163+
"both detected methods show the remove-to-override hint",
164+
).toBe(2);
165+
await page.getByText("https://api.acme.test/oauth/authorize").waitFor();
166+
await page.getByText("API key", { exact: true }).first().waitFor();
167+
await page.getByText("OAuth", { exact: true }).first().waitFor();
168+
expect(
169+
await page.getByRole("button", { name: "OAuth", exact: true }).count(),
170+
"no editable kind selector is shown for the detected methods",
171+
).toBe(0);
172+
});
173+
174+
await step("A hand-added method keeps the full editable selector", async () => {
175+
await page.getByRole("button", { name: "Add method" }).click();
176+
await page.getByText("Method 3").waitFor();
177+
expect(
178+
await page.getByRole("button", { name: "API key", exact: true }).count(),
179+
"the added method exposes the kind selector",
180+
).toBeGreaterThan(0);
181+
});
182+
});
183+
}),
184+
),
185+
);

‎packages/react/src/components/auth-method-list-editor.tsx‎

Lines changed: 128 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@
1212
// ---------------------------------------------------------------------------
1313

1414
import { useCallback, useEffect, useRef, useState } from "react";
15-
import { PlusIcon, XIcon } from "lucide-react";
15+
import { LockIcon, PlusIcon, XIcon } from "lucide-react";
1616

17+
import { PlacementLine } from "../lib/auth-placements";
1718
import { Button } from "./button";
1819
import { FieldLabel } from "./field";
1920
import {
@@ -35,6 +36,11 @@ export interface AuthMethodSeed {
3536

3637
export interface AuthMethodRow {
3738
readonly value: AuthTemplateEditorValue;
39+
/** True when this row came from detection (a seed), false when the user added
40+
* it. Detected rows are immutable — the spec/probe declared them — so the
41+
* editor renders them read-only. Not inferred from `seedSlug`: some plugins
42+
* (MCP) seed a detected method with a label but no slug. */
43+
readonly seeded: boolean;
3844
readonly seedSlug?: string;
3945
readonly seedLabel?: string;
4046
}
@@ -59,6 +65,7 @@ export function useAuthMethodList(seeds: readonly AuthMethodSeed[]): AuthMethodL
5965
seeds.map(
6066
(seed: AuthMethodSeed): AuthMethodRow => ({
6167
value: seed.value,
68+
seeded: true,
6269
...(seed.slug !== undefined ? { seedSlug: seed.slug } : {}),
6370
...(seed.label !== undefined ? { seedLabel: seed.label } : {}),
6471
}),
@@ -79,7 +86,10 @@ export function useAuthMethodList(seeds: readonly AuthMethodSeed[]): AuthMethodL
7986
}, []);
8087

8188
const addRow = useCallback(() => {
82-
setRows((current: readonly AuthMethodRow[]) => [...current, { value: emptyApiKeyValue() }]);
89+
setRows((current: readonly AuthMethodRow[]) => [
90+
...current,
91+
{ value: emptyApiKeyValue(), seeded: false },
92+
]);
8393
}, []);
8494

8595
return { rows, setRowAt, removeRowAt, addRow };
@@ -115,36 +125,53 @@ export function AuthMethodListEditor(props: AuthMethodListEditorProps) {
115125
) : null
116126
) : (
117127
<div className="flex flex-col gap-3">
118-
{list.rows.map((row: AuthMethodRow, index: number) => (
119-
<div
120-
key={index}
121-
className="space-y-2 rounded-lg border border-border/60 bg-muted/20 p-3"
122-
>
123-
<div className="flex items-center justify-between">
124-
<span className="text-xs font-medium text-muted-foreground">
125-
Method {index + 1}
126-
{row.seedLabel ? ` · ${row.seedLabel}` : ""}
127-
</span>
128-
<Button
129-
type="button"
130-
variant="ghost"
131-
size="icon-sm"
132-
aria-label="Remove method"
133-
className="text-muted-foreground hover:text-foreground"
134-
onClick={() => list.removeRowAt(index)}
135-
>
136-
<XIcon />
137-
</Button>
128+
{list.rows.map((row: AuthMethodRow, index: number) => {
129+
// A row seeded from detection is the spec's own auth declaration:
130+
// it's IMMUTABLE here. We render it read-only (no kind selector, no
131+
// editable fields) so a user can't silently retype the spec's
132+
// method into something nothing backs (e.g. flipping a Bearer-token
133+
// API to OAuth with empty endpoints). The escape hatch is to remove
134+
// the row and add a custom one. Manually added rows (no seed) get
135+
// the full editor.
136+
const detected = row.seeded;
137+
return (
138+
<div
139+
key={index}
140+
className="space-y-2 rounded-lg border border-border/60 bg-muted/20 p-3"
141+
>
142+
<div className="flex items-center justify-between">
143+
<span className="flex items-center gap-1.5 text-xs font-medium text-muted-foreground">
144+
{detected ? <LockIcon className="size-3 shrink-0" aria-hidden /> : null}
145+
<span>
146+
Method {index + 1}
147+
{row.seedLabel ? ` · ${row.seedLabel}` : ""}
148+
</span>
149+
</span>
150+
<Button
151+
type="button"
152+
variant="ghost"
153+
size="icon-sm"
154+
aria-label="Remove method"
155+
className="text-muted-foreground hover:text-foreground"
156+
onClick={() => list.removeRowAt(index)}
157+
>
158+
<XIcon />
159+
</Button>
160+
</div>
161+
{detected ? (
162+
<DetectedMethodSummary value={row.value} oauthMetadata={oauthMetadata} />
163+
) : (
164+
<AuthTemplateEditor
165+
value={row.value}
166+
onChange={(next: AuthTemplateEditorValue) => list.setRowAt(index, next)}
167+
{...(allowedKinds ? { allowedKinds } : {})}
168+
{...(presets ? { presets } : {})}
169+
{...(oauthMetadata ? { oauthMetadata } : {})}
170+
/>
171+
)}
138172
</div>
139-
<AuthTemplateEditor
140-
value={row.value}
141-
onChange={(next: AuthTemplateEditorValue) => list.setRowAt(index, next)}
142-
{...(allowedKinds ? { allowedKinds } : {})}
143-
{...(presets ? { presets } : {})}
144-
{...(oauthMetadata ? { oauthMetadata } : {})}
145-
/>
146-
</div>
147-
))}
173+
);
174+
})}
148175
</div>
149176
)}
150177
{list.rows.length > 0 && props.footerHint ? (
@@ -153,3 +180,73 @@ export function AuthMethodListEditor(props: AuthMethodListEditorProps) {
153180
</section>
154181
);
155182
}
183+
184+
/** One read-only `label value` line, mono value, for the detected summary. */
185+
function SpecField(props: { readonly label: string; readonly value: string }) {
186+
return (
187+
<div className="flex gap-2 text-xs">
188+
<span className="w-20 shrink-0 text-muted-foreground">{props.label}</span>
189+
<span className="break-all font-mono text-foreground/80">{props.value}</span>
190+
</div>
191+
);
192+
}
193+
194+
/** Read-only view of a spec-detected method: shows what the spec declared
195+
* (placements / OAuth endpoints) as a DISABLED, non-interactive block. The
196+
* detected method is immutable here, so the summary is styled like a disabled
197+
* field (muted, not-allowed cursor, text not selectable) to communicate that
198+
* plainly. The only action is to remove the row (the header's X) and add a
199+
* custom method to override. */
200+
function DetectedMethodSummary(props: {
201+
readonly value: AuthTemplateEditorValue;
202+
readonly oauthMetadata?: "editable" | "discovered";
203+
}) {
204+
const { value, oauthMetadata } = props;
205+
// Name the auth kind explicitly: a detection label like MCP's "Detected"
206+
// doesn't say whether it's OAuth or an API key, so surface it here.
207+
const kindLabel =
208+
value.kind === "oauth" ? "OAuth" : value.kind === "apikey" ? "API key" : "No auth";
209+
return (
210+
<div className="space-y-2">
211+
<p className="font-mono text-[11px] uppercase tracking-wide text-muted-foreground">
212+
{kindLabel}
213+
</p>
214+
<div
215+
aria-disabled
216+
className="cursor-not-allowed select-none space-y-1 rounded-md border border-border/60 bg-muted/40 px-3 py-2.5 text-muted-foreground"
217+
>
218+
{value.kind === "none" && (
219+
<p className="text-xs">No credential — tools are callable without an account.</p>
220+
)}
221+
222+
{value.kind === "apikey" &&
223+
(value.placements.length > 0 ? (
224+
<div className="flex flex-wrap gap-x-4 gap-y-1">
225+
{value.placements.map((placement, i: number) => (
226+
<PlacementLine key={i} placement={placement} />
227+
))}
228+
</div>
229+
) : null)}
230+
231+
{value.kind === "oauth" &&
232+
(oauthMetadata === "discovered" ? (
233+
<p className="text-xs">
234+
OAuth metadata is discovered from this server when you connect an account.
235+
</p>
236+
) : (
237+
<div className="space-y-1">
238+
{value.authorizationUrl ? (
239+
<SpecField label="Authorize" value={value.authorizationUrl} />
240+
) : null}
241+
{value.tokenUrl ? <SpecField label="Token" value={value.tokenUrl} /> : null}
242+
{value.scopes.length > 0 ? (
243+
<SpecField label="Scopes" value={value.scopes.join(", ")} />
244+
) : null}
245+
</div>
246+
))}
247+
</div>
248+
249+
<p className="text-[11px] text-muted-foreground">Pulled from spec. Remove to override.</p>
250+
</div>
251+
);
252+
}

0 commit comments

Comments
 (0)