History

Prefer OAuth when adding connections

Rhys Sullivan committed 1 month ago

Commit b6461fc1 parent b8c29704 files · +47 −15Browse files

Unified · Split

4 files
  1. .changeset/connection-oauth-default.md +5 −0
  2. e2e/scenarios/connection-setup-ux.test.ts +22 −0
  3. packages/react/src/components/add-account-modal.test.ts +18 −0
  4. packages/react/src/components/add-account-modal.tsx +2 −15
.changeset/connection-oauth-default.md+5−0
@@ -0,0 +1,5 @@
1---
2"executor": patch
3---
4
5Prefer browser sign-in when an integration offers OAuth.
e2e/scenarios/connection-setup-ux.test.ts+22−0
@@ -135,3 +135,25 @@ scenario( · 134 unmodified lines
135 }),135 }),
136 ),136 ),
137);137);
138
139scenario(
140 "Connection setup · a ready OAuth app is the default",
141 {},
142 Effect.scoped(
143 Effect.gen(function* () {
144 const { browser, identity, slug } = yield* fixture;
145 yield* browser.session(identity, async ({ page, step }) => {
146 await step("Add a connection with both a token and a registered sign-in app", async () => {
147 await visit(page, `/integrations/${slug}?addAccount=1`);
148 await page.getByRole("tab", { name: "OAuth2", exact: true }).waitFor();
149 expect(
150 await page
151 .getByRole("tab", { name: "OAuth2", exact: true })
152 .getAttribute("aria-selected"),
153 "sign-in is selected without first switching away from API token",
154 ).toBe("true");
155 });
156 });
157 }),
158 ),
159);
packages/react/src/components/add-account-modal.test.ts+18−0
@@ -19,6 +19,7 @@ import { · 18 unmodified lines
19 DEFAULT_CONNECTION_OWNER,19 DEFAULT_CONNECTION_OWNER,
20 hasDcr,20 hasDcr,
21 mergeCustomMethods,21 mergeCustomMethods,
22 preferredMethodId,
22 oauthIdentityLabelFromHealth,23 oauthIdentityLabelFromHealth,
23 runAutomaticOAuthConnect,24 runAutomaticOAuthConnect,
24 runCimdConnect,25 runCimdConnect,
@@ -1438,3 +1439,20 @@ describe("runDcrConnect", () => { · 1413 unmodified lines
1438 expect(String(registerArgs!.slug)).toBe("dcr-auth-example-com");1439 expect(String(registerArgs!.slug)).toBe("dcr-auth-example-com");
1439 });1440 });
1440});1441});
1442
1443describe("preferredMethodId", () => {
1444 it("opens sign-in before a token field and preserves token-only integrations", () => {
1445 const token = apiKeyMethod("token", "spec");
1446 const oauth: AuthMethod = {
1447 id: "oauth",
1448 label: "OAuth",
1449 kind: "oauth",
1450 source: "spec",
1451 template: AuthTemplateSlug.make("oauth"),
1452 placements: [],
1453 };
1454 expect(preferredMethodId([token, oauth])).toBe("oauth");
1455 expect(preferredMethodId([token])).toBe("token");
1456 expect(preferredMethodId([])).toBe("");
1457 });
1458});
packages/react/src/components/add-account-modal.tsx+2−15
@@ -628,13 +628,9 @@ export const connectionExistsMessage = (label: string): string => · 627 unmodified lines
628 * explicit choice. Personal: a connection is most often a personal credential. */628 * explicit choice. Personal: a connection is most often a personal credential. */
629export const DEFAULT_CONNECTION_OWNER: Owner = "user";629export const DEFAULT_CONNECTION_OWNER: Owner = "user";
630 630
631/** The method the modal opens on. OAuth needs a registered app (or a DCR631/** Prefer browser sign-in when offered; credential methods remain selectable. */
632 * round-trip) before "Connect" does anything; a key is one paste. When an
633 * integration declares both, starting on OAuth greets most users with
634 * "Register app" — a dead end — while the working method sits one tab over.
635 * Prefer the first non-OAuth method; OAuth stays one click away. */
636export const preferredMethodId = (methods: readonly AuthMethod[]): string =>632export const preferredMethodId = (methods: readonly AuthMethod[]): string =>
637 (methods.find((method) => method.kind !== "oauth") ?? methods[0])?.id ?? "";633 (methods.find((method) => method.kind === "oauth") ?? methods[0])?.id ?? "";
638 634
639const authMethodKey = (method: AuthMethod): string =>635const authMethodKey = (method: AuthMethod): string =>
640 method.source === "custom" ? `custom:${String(method.template)}` : `declared:${method.id}`;636 method.source === "custom" ? `custom:${String(method.template)}` : `declared:${method.id}`;
@@ -1582,15 +1578,6 @@ function AddAccountModalView(props: AddAccountModalProps) { · 941 unmodified lines
1582 [allMethods, methodId],1578 [allMethods, methodId],
1583 );1579 );
1584 1580
1585 useEffect(() => {
1586 if (allMethods.length === 0) {
1587 if (methodId !== "") setMethodId("");
1588 return;
1589 }
1590 if (allMethods.some((m: AuthMethod) => m.id === methodId)) return;
1591 setMethodId(allMethods[0]!.id);
1592 }, [allMethods, methodId]);
1593
1594 // Apply the handoff prefill ONCE per handoff key (tracked by ref). The1581 // Apply the handoff prefill ONCE per handoff key (tracked by ref). The
1595 // effect's deps include `allMethods`, which gets a new identity whenever the1582 // effect's deps include `allMethods`, which gets a new identity whenever the
1596 // integration refetches — and the wizard itself triggers a refetch mid-flow1583 // integration refetches — and the wizard itself triggers a refetch mid-flow