History

Fix auth-method dialog overflow with long API keys

A long unbroken key pasted as a placement prefix stretched the add-method dialog (preview line) and the connect modal (merged credential affix) past the viewport. Wrap the placement preview mid-token and truncate the merged affix inside the field.

Rhys Sullivan committed 3 months ago

Commit a11c9c01 parent 7d6fcea3 files · +52 −5Browse files

Unified · Split

3 files
  1. e2e/selfhost/connect-modal-credential-ux.test.ts +44 −1
  2. packages/react/src/components/add-account-modal.tsx +4 −2
  3. packages/react/src/lib/auth-placements.tsx +4 −2
e2e/selfhost/connect-modal-credential-ux.test.ts+44−1
@@ -3,10 +3,14 @@ · 2 unmodified lines
3// it reads as the header value being built ("Authorization: Bearer ▏token");3// it reads as the header value being built ("Authorization: Bearer ▏token");
4// - the "Add authentication method" editor offers placement presets;4// - the "Add authentication method" editor offers placement presets;
5// - a prefix with no trailing space (sent joined to the value) warns, and the5// - a prefix with no trailing space (sent joined to the value) warns, and the
6// warning clears once the space is restored.6// warning clears once the space is restored;
7// - a very long pasted key wraps inside the dialog instead of stretching it
8// past the viewport (the preview breaks mid-token, the merged affix
9// truncates).
7// Video is the artifact.10// Video is the artifact.
8import { randomBytes } from "node:crypto";11import { randomBytes } from "node:crypto";
9 12
13import { expect } from "@effect/vitest";
10import { Effect } from "effect";14import { Effect } from "effect";
11import { composePluginApi } from "@executor-js/api/server";15import { composePluginApi } from "@executor-js/api/server";
12import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api";16import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api";
@@ -17,6 +21,16 @@ import { Api, Browser, Target } from "../src/services"; · 4 unmodified lines
17 21
18const api = composePluginApi([openApiHttpPlugin()] as const);22const api = composePluginApi([openApiHttpPlugin()] as const);
19 23
24/** Horizontal overflow of the open dialog in px (0 = fits). Infinity when no
25 * dialog is open, so an assertion against it always fails loudly. */
26const dialogOverflow = (page: {
27 readonly evaluate: <T>(fn: () => T) => Promise<T>;
28}): Promise<number> =>
29 page.evaluate(() => {
30 const dialog = document.querySelector('[role="dialog"]');
31 return dialog === null ? Number.POSITIVE_INFINITY : dialog.scrollWidth - dialog.clientWidth;
32 });
33
20const bearerSpec = (): string =>34const bearerSpec = (): string =>
21 JSON.stringify({35 JSON.stringify({
22 openapi: "3.0.3",36 openapi: "3.0.3",
@@ -75,6 +89,35 @@ scenario( · 52 unmodified lines
75 await page.getByPlaceholder("Bearer ").first().fill("Bearer ");89 await page.getByPlaceholder("Bearer ").first().fill("Bearer ");
76 await page.getByText("Prefix has no trailing space").waitFor({ state: "detached" });90 await page.getByText("Prefix has no trailing space").waitFor({ state: "detached" });
77 });91 });
92
93 await step("A long pasted key never widens the dialog", async () => {
94 // A key mistakenly pasted into Prefix is the worst case: one
95 // unbroken 700-char token rendered into the preview line. The
96 // dialog must keep its width (the preview wraps mid-token).
97 const longKey = `${"x".repeat(300)}Bearer${"x".repeat(400)}`;
98 // The preview line renders only once the placement is named.
99 await page.getByPlaceholder("Authorization").first().fill("Authorization");
100 await page.getByPlaceholder("Bearer ").first().fill(longKey);
101 await page.getByText("Preview").waitFor();
102 // ≤ 1px: subpixel rounding can report a fractional scrollWidth.
103 expect(
104 await dialogOverflow(page),
105 "the add-method dialog must not scroll horizontally",
106 ).toBeLessThanOrEqual(1);
107 });
108
109 await step("The merged affix truncates a long saved prefix", async () => {
110 // Saving the method returns to the connect modal with the new
111 // method selected; its credential field merges the (700-char)
112 // prefix as the affix, which must truncate inside the field.
113 await page.getByRole("button", { name: "Add method", exact: true }).click();
114 await page.getByRole("heading", { name: /Add connection/ }).waitFor();
115 await page.getByText("Authorization: x").first().waitFor();
116 expect(
117 await dialogOverflow(page),
118 "the connect modal must not scroll horizontally",
119 ).toBeLessThanOrEqual(1);
120 });
78 });121 });
79 }),122 }),
80 apiClient.openapi123 apiClient.openapi
packages/react/src/components/add-account-modal.tsx+4−2
@@ -270,8 +270,10 @@ function PasteCredentialInputs(props: { · 269 unmodified lines
270 // from offering to GENERATE a password here; the app's own 1Password270 // from offering to GENERATE a password here; the app's own 1Password
271 // picker covers filling an existing secret.271 // picker covers filling an existing secret.
272 <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">272 <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">
273 <span className="flex shrink-0 select-none items-center whitespace-pre border-r border-input bg-muted/40 px-3 text-muted-foreground/70">273 {/* min-w-0 + inner ellipsis: a method saved with a very long
274 {props.affix}274 prefix must truncate inside the field, not stretch it. */}
275 <span className="flex min-w-0 max-w-[60%] select-none items-center border-r border-input bg-muted/40 px-3 text-muted-foreground/70">
276 <span className="overflow-hidden text-ellipsis whitespace-pre">{props.affix}</span>
275 </span>277 </span>
276 {/* oxlint-disable-next-line react/forbid-elements */}278 {/* oxlint-disable-next-line react/forbid-elements */}
277 <input279 <input
packages/react/src/lib/auth-placements.tsx+4−2
@@ -116,9 +116,11 @@ export function PlacementLine(props: { readonly placement: Placement; readonly m · 115 unmodified lines
116 // each child, which would drop the space after "Authorization:" and the116 // each child, which would drop the space after "Authorization:" and the
117 // trailing space carried by a prefix like "Bearer ", rendering117 // trailing space carried by a prefix like "Bearer ", rendering
118 // "Authorization:Bearer••••••". whitespace-pre-wrap keeps those spaces while118 // "Authorization:Bearer••••••". whitespace-pre-wrap keeps those spaces while
119 // still allowing the line to wrap.119 // still allowing the line to wrap. wrap-anywhere lets an unbroken value (a
120 // long API key pasted as the prefix) break mid-token — without it the line's
121 // min-content width is the whole key and it stretches the dialog off-screen.
120 return (122 return (
121 <span className="whitespace-pre-wrap font-mono text-xs text-muted-foreground">123 <span className="wrap-anywhere whitespace-pre-wrap font-mono text-xs text-muted-foreground">
122 {lead}124 {lead}
123 {placement.prefix ? (125 {placement.prefix ? (
124 <span className="text-muted-foreground/60">{placement.prefix}</span>126 <span className="text-muted-foreground/60">{placement.prefix}</span>