Skip to content

Commit 2692f42

Browse files
committed
Refine the connect modal's API-key credential UX
- Merged credential field: the placement's lead + prefix is a fixed, non-editable addon inside the field, so it reads as the header value being built ("Authorization: Bearer <token>") and the separate preview line is dropped. autoComplete/ignore attrs stop the browser and password managers from offering to GENERATE a password here; the app's own 1Password picker still covers filling an existing secret. - Attached tabs: the auth-method tabs join the panel as one card (full-width rounded-top header, hairline divider) instead of a pill bar floating above a separate card. - Prefix trailing-space warning (amber): a non-empty prefix with no trailing space is sent joined to the value ("Bearer" + token -> "Bearertoken"). Advisory, not blocking.
1 parent 456f125 commit 2692f42

3 files changed

Lines changed: 189 additions & 20 deletions

File tree

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Selfhost-only (browser): the connect modal's API-key credential UX.
2+
// - the credential field MERGES the placement's lead + prefix as an affix, so
3+
// it reads as the header value being built ("Authorization: Bearer ▏token");
4+
// - the "Add authentication method" editor offers placement presets;
5+
// - a prefix with no trailing space (sent joined to the value) warns, and the
6+
// warning clears once the space is restored.
7+
// Video is the artifact.
8+
import { randomBytes } from "node:crypto";
9+
10+
import { Effect } from "effect";
11+
import { composePluginApi } from "@executor-js/api/server";
12+
import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api";
13+
import { IntegrationSlug } from "@executor-js/sdk/shared";
14+
15+
import { scenario } from "../src/scenario";
16+
import { Api, Browser, Target } from "../src/services";
17+
18+
const api = composePluginApi([openApiHttpPlugin()] as const);
19+
20+
const bearerSpec = (): string =>
21+
JSON.stringify({
22+
openapi: "3.0.3",
23+
info: { title: "Bearer Fixture", version: "1.0.0" },
24+
servers: [{ url: "https://api.bearerfix.test" }],
25+
security: [{ bearerAuth: [] }],
26+
components: { securitySchemes: { bearerAuth: { type: "http", scheme: "bearer" } } },
27+
paths: {
28+
"/ping": { get: { operationId: "ping", responses: { "200": { description: "ok" } } } },
29+
},
30+
});
31+
32+
scenario(
33+
"Connect modal · API key credential UX: merged affix, add-method, prefix warning",
34+
{},
35+
Effect.scoped(
36+
Effect.gen(function* () {
37+
const target = yield* Target;
38+
const { client: makeApiClient } = yield* Api;
39+
const browser = yield* Browser;
40+
const identity = yield* target.newIdentity();
41+
const apiClient = yield* makeApiClient(api, identity);
42+
const slug = `connect_ux_${randomBytes(4).toString("hex")}`;
43+
44+
yield* Effect.ensuring(
45+
Effect.gen(function* () {
46+
yield* apiClient.openapi.addSpec({
47+
payload: { spec: { kind: "blob", value: bearerSpec() }, slug },
48+
});
49+
50+
yield* browser.session(identity, async ({ page, step }) => {
51+
await step("Open the connect modal", async () => {
52+
await page.goto(`/integrations/${slug}?addAccount=1`, { waitUntil: "networkidle" });
53+
await page.getByRole("heading", { name: /Add connection/ }).waitFor();
54+
});
55+
56+
await step("The credential field merges the placement prefix", async () => {
57+
// The placement's lead + prefix renders as a non-editable affix
58+
// inside the field, so there is no separate preview line.
59+
await page.getByText("Authorization: Bearer").first().waitFor();
60+
});
61+
62+
await step("The add-method editor offers placement presets", async () => {
63+
await page.getByRole("button", { name: "Add authentication method" }).click();
64+
await page.getByRole("heading", { name: "Add authentication method" }).waitFor();
65+
await page.getByRole("button", { name: "Bearer header" }).waitFor();
66+
await page.getByRole("button", { name: "API key query" }).waitFor();
67+
});
68+
69+
await step("A prefix with no trailing space warns", async () => {
70+
await page.getByPlaceholder("Bearer ").first().fill("Bearer");
71+
await page.getByText("Prefix has no trailing space").waitFor();
72+
});
73+
74+
await step("Restoring the trailing space clears the warning", async () => {
75+
await page.getByPlaceholder("Bearer ").first().fill("Bearer ");
76+
await page.getByText("Prefix has no trailing space").waitFor({ state: "detached" });
77+
});
78+
});
79+
}),
80+
apiClient.openapi
81+
.removeSpec({ params: { slug: IntegrationSlug.make(slug) } })
82+
.pipe(Effect.ignore),
83+
);
84+
}),
85+
),
86+
);

packages/react/src/components/add-account-modal.tsx

Lines changed: 80 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,10 @@ function PasteCredentialInputs(props: {
154154
/** Force the per-input label even for a single input (env vars name their
155155
* field rather than relying on a generic "value / token" placeholder). */
156156
readonly showLabels?: boolean;
157+
/** Single-input only: the placement's lead + prefix (e.g. "Authorization:
158+
* Bearer "), rendered as a non-editable affix INSIDE the field so the input
159+
* reads as the header value being built. Replaces the separate preview line. */
160+
readonly affix?: string | null;
157161
readonly values: Readonly<Record<string, string>>;
158162
readonly onChange: (values: Record<string, string>) => void;
159163
}) {
@@ -173,7 +177,11 @@ function PasteCredentialInputs(props: {
173177
<Input
174178
id={`credential-input-${input.variable}`}
175179
type="password"
176-
autoComplete="new-password"
180+
autoComplete="off"
181+
data-1p-ignore
182+
data-lpignore="true"
183+
data-bwignore
184+
data-form-type="other"
177185
placeholder={`paste ${input.label}`}
178186
value={props.values[input.variable] ?? ""}
179187
onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
@@ -201,20 +209,55 @@ function PasteCredentialInputs(props: {
201209
{labelled && (
202210
<Label className="font-mono text-xs text-muted-foreground">{input.label}</Label>
203211
)}
204-
<Input
205-
type="password"
206-
autoComplete="new-password"
207-
placeholder={labelled ? "secret value" : "paste the value / token"}
208-
value={props.values[input.variable] ?? ""}
209-
onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
210-
props.onChange({
211-
...props.values,
212-
[input.variable]: e.target.value,
213-
})
214-
}
215-
className="font-mono"
216-
data-ph-block
217-
/>
212+
{props.affix ? (
213+
// Merged field: the placement's lead + prefix is a FIXED addon (its
214+
// own muted segment, divider, non-selectable), and the user types
215+
// only the token after it — the field reads as the header value being
216+
// built. Only the border reacts to focus (no full-field glow). The
217+
// autoComplete/ignore attrs stop the browser and password managers
218+
// from offering to GENERATE a password here; the app's own 1Password
219+
// picker covers filling an existing secret.
220+
<div className="flex h-9 w-full min-w-0 items-stretch overflow-hidden rounded-md border border-input bg-transparent font-mono text-sm shadow-xs transition-colors focus-within:border-ring dark:bg-input/30">
221+
<span className="flex shrink-0 select-none items-center whitespace-pre border-r border-input bg-muted/40 px-3 text-muted-foreground/70">
222+
{props.affix}
223+
</span>
224+
{/* oxlint-disable-next-line react/forbid-elements */}
225+
<input
226+
type="password"
227+
autoComplete="off"
228+
data-1p-ignore
229+
data-lpignore="true"
230+
data-bwignore
231+
data-form-type="other"
232+
placeholder="token"
233+
value={props.values[input.variable] ?? ""}
234+
onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
235+
props.onChange({ ...props.values, [input.variable]: e.target.value })
236+
}
237+
className="min-w-0 flex-1 bg-transparent px-3 outline-none placeholder:text-muted-foreground/60"
238+
data-ph-block
239+
/>
240+
</div>
241+
) : (
242+
<Input
243+
type="password"
244+
autoComplete="off"
245+
data-1p-ignore
246+
data-lpignore="true"
247+
data-bwignore
248+
data-form-type="other"
249+
placeholder={labelled ? "secret value" : "paste the value / token"}
250+
value={props.values[input.variable] ?? ""}
251+
onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
252+
props.onChange({
253+
...props.values,
254+
[input.variable]: e.target.value,
255+
})
256+
}
257+
className="font-mono"
258+
data-ph-block
259+
/>
260+
)}
218261
</div>
219262
))}
220263
</div>
@@ -292,6 +335,8 @@ function CredentialValueFields(props: {
292335
readonly inputs: readonly CredentialInput[];
293336
readonly singleInput: boolean;
294337
readonly showLabels?: boolean;
338+
/** Single-input only: placement lead + prefix to merge into the field. */
339+
readonly affix?: string | null;
295340
/** Allow an external provider (1Password) origin. Off for env methods: the
296341
* wire's external `from` is keyed under the canonical `token` variable, but
297342
* an env credential must be keyed under its env var name, so a 1Password
@@ -346,6 +391,7 @@ function CredentialValueFields(props: {
346391
inputs={props.inputs}
347392
singleInput={props.singleInput}
348393
showLabels={props.showLabels}
394+
affix={props.affix}
349395
values={props.values}
350396
onChange={props.onValuesChange}
351397
/>
@@ -882,6 +928,20 @@ function AddAccountModalView(props: AddAccountModalProps) {
882928
method != null &&
883929
method.placements.length > 0 &&
884930
method.placements.every((p) => p.carrier === "env");
931+
// Single-input header/query methods: the placement's lead + prefix (e.g.
932+
// "Authorization: Bearer ") merges INTO the credential field as a non-editable
933+
// affix, so the input reads as the header value being built and the separate
934+
// preview line is dropped. Env and multi-input methods keep the plain field.
935+
const singleCredentialAffix = useMemo<string | null>(() => {
936+
if (!method || !singleInput || isEnvMethod) return null;
937+
const placement = method.placements.find((p) => p.carrier !== "env");
938+
if (!placement) return null;
939+
const lead =
940+
placement.carrier === "header"
941+
? `${placement.name || "Authorization"}: `
942+
: `?${placement.name || "api_key"}=`;
943+
return `${lead}${placement.prefix ?? ""}`;
944+
}, [method, singleInput, isEnvMethod]);
885945
// DCR-capable: the integration advertises dynamic registration (MCP oauth2),
886946
// OR carries a discovery URL we can probe at connect time. When DCR-capable
887947
// and not yet fallen back, we skip the app picker entirely (Option A).
@@ -1394,9 +1454,9 @@ function AddAccountModalView(props: AddAccountModalProps) {
13941454
<Tabs
13951455
value={methodId}
13961456
onValueChange={selectMethod}
1397-
className="w-full min-w-0 max-w-full gap-3"
1457+
className="w-full min-w-0 max-w-full gap-0"
13981458
>
1399-
<TabsList className="flex h-10 w-fit min-w-0 max-w-full justify-start overflow-x-auto overflow-y-hidden p-1 [scrollbar-width:thin]">
1459+
<TabsList className="flex h-10 w-full min-w-0 max-w-full justify-start overflow-x-auto overflow-y-hidden rounded-b-none rounded-t-md border border-b-0 border-border/60 bg-muted/30 p-1 [scrollbar-width:thin]">
14001460
<div className="flex w-max shrink-0 items-stretch gap-1">
14011461
{allMethods.map((m: AuthMethod) => (
14021462
<div
@@ -1452,9 +1512,9 @@ function AddAccountModalView(props: AddAccountModalProps) {
14521512
{dcrActive ? null : (
14531513
<TabsContent
14541514
value={methodId}
1455-
className="mt-0 min-w-0 space-y-5 rounded-md border border-border/60 bg-muted/15 p-4"
1515+
className="mt-0 min-w-0 space-y-5 rounded-b-md rounded-t-none border border-border/60 bg-muted/15 p-4"
14561516
>
1457-
{method?.placements && !isEnvMethod && singleInput
1517+
{method?.placements && !isEnvMethod && singleInput && !singleCredentialAffix
14581518
? (() => {
14591519
const shown = method.placements.filter((p) => p.carrier !== "env");
14601520
return shown.length > 0 ? (
@@ -1573,6 +1633,7 @@ function AddAccountModalView(props: AddAccountModalProps) {
15731633
inputs={credentialInputs}
15741634
singleInput={singleInput}
15751635
showLabels={isEnvMethod}
1636+
affix={singleCredentialAffix}
15761637
allowExternalProvider={!isEnvMethod}
15771638
values={values}
15781639
onValuesChange={setValues}

packages/react/src/components/placement-editor.tsx

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { PlusIcon, XIcon } from "lucide-react";
1+
import { PlusIcon, TriangleAlertIcon, XIcon } from "lucide-react";
22

33
import {
44
emptyPlacement,
@@ -47,6 +47,10 @@ export function PlacementEditor(props: {
4747
const remove = (index: number): void =>
4848
onChange(placements.filter((_p: Placement, j: number) => j !== index));
4949

50+
// A non-empty prefix with no trailing space is sent JOINED to the credential
51+
// (e.g. "Bearer" + token -> "Bearertoken"). Almost always a mistake: warn.
52+
const barePrefix = placements.some((p) => p.prefix.length > 0 && !p.prefix.endsWith(" "));
53+
5054
return (
5155
<div className="flex flex-col gap-3">
5256
<div className="flex flex-wrap gap-1.5">
@@ -147,6 +151,24 @@ export function PlacementEditor(props: {
147151
) : null}
148152
</div>
149153
))}
154+
{barePrefix ? (
155+
<section className="grid grid-cols-[auto_1fr] gap-x-2.5 gap-y-1 rounded-md border border-l-[3px] border-amber-300/70 border-l-amber-500 bg-amber-50 px-3 py-2.5 text-[12px] leading-5 dark:border-amber-500/25 dark:border-l-amber-500/80 dark:bg-amber-500/10">
156+
<TriangleAlertIcon
157+
className="mt-0.5 size-4 shrink-0 text-amber-600 dark:text-amber-400"
158+
aria-hidden
159+
/>
160+
<div className="min-w-0 space-y-1">
161+
<p className="font-medium text-amber-900 dark:text-amber-100">
162+
Prefix has no trailing space
163+
</p>
164+
<p className="text-amber-900/80 dark:text-amber-100/80">
165+
It is sent joined to the value (<span className="font-mono">Bearer••••••</span>). Most
166+
APIs expect a space, like <span className="font-mono">"Bearer "</span>.
167+
</p>
168+
</div>
169+
</section>
170+
) : null}
171+
150172
<Button
151173
type="button"
152174
variant="outline"

0 commit comments

Comments
 (0)