History

Stop editing an OAuth app from clearing its recorded origin

createClient is delete-then-create, so the edit flow resubmitting originIntegration: null was wiping the intent stamp the picker matches on. The form now takes an explicit intentIntegration that the edit dialog fills from the summary origin; fresh registrations keep stamping the current integration.

Rhys Sullivan committed 4 months ago

Commit 9b8b2b01 parent e9ee6663 files · +63 −4Browse files

Unified · Split

3 files
  1. packages/react/src/components/add-account-modal.tsx +9 −0
  2. packages/react/src/components/oauth-client-form.test.ts +29 −1
  3. packages/react/src/components/oauth-client-form.tsx +25 −3
packages/react/src/components/add-account-modal.tsx+9−0
@@ -1593,6 +1593,15 @@ function AddAccountModalView(props: AddAccountModalProps) { · 1592 unmodified lines
15931593 existingSlugs={clientSummaries.map((c: OAuthClientSummary) => String(c.slug))}
15941594 fixedSlug={editingClient.slug}
15951595 fixedOwner={editingClient.owner}
1596+ // Preserve the app's already-recorded origin verbatim (only a
1597+ // "manual" origin carries a recorded integration; DCR apps
1598+ // aren't edited through this form). Editing must never
1599+ // re-derive this from the current dialog's integration.
1600+ intentIntegration={
1601+ editingClient.origin.kind === "manual"
1602+ ? (editingClient.origin.integration ?? null)
1603+ : null
1604+ }
15961605 prefill={{
15971606 authorizationUrl: editingClient.authorizationUrl,
15981607 tokenUrl: editingClient.tokenUrl,
packages/react/src/components/oauth-client-form.test.ts+29−1
@@ -1,6 +1,11 @@
11 import { describe, expect, it } from "@effect/vitest";
2+import { IntegrationSlug } from "@executor-js/sdk/shared";
23
3−import { canSubmitOAuthClientForm, registrationScopes } from "./oauth-client-form";
4+import {
5+ canSubmitOAuthClientForm,
6+ registrationScopes,
7+ resolveOriginIntegration,
8+} from "./oauth-client-form";
49
510 const validBase = {
611 submitting: false,
@@ -36,6 +41,29 @@ describe("registrationScopes", () => { · 29 unmodified lines
3641 });
3742 });
3843
44+describe("resolveOriginIntegration", () => {
45+ const INTEG = IntegrationSlug.make("linear_mcp");
46+ const OTHER = IntegrationSlug.make("github");
47+
48+ it("stamps the current integration for a fresh registration (no explicit intent)", () => {
49+ expect(resolveOriginIntegration(undefined, INTEG)).toBe(INTEG);
50+ });
51+
52+ it("stamps null for a fresh registration outside any integration context", () => {
53+ expect(resolveOriginIntegration(undefined, undefined)).toBeNull();
54+ });
55+
56+ it("preserves an explicit null intent when editing — does NOT re-stamp the current integration", () => {
57+ // Regression: editing used to send `fixedSlug ? null : integrationSlug`,
58+ // which clobbered an app's recorded origin with null on every edit.
59+ expect(resolveOriginIntegration(null, INTEG)).toBeNull();
60+ });
61+
62+ it("preserves an explicit non-null intent when editing, even if it differs from the current integration", () => {
63+ expect(resolveOriginIntegration(OTHER, INTEG)).toBe(OTHER);
64+ });
65+});
66+
3967 describe("canSubmitOAuthClientForm", () => {
4068 it("allows authorization-code public clients with no client secret", () => {
4169 expect(
packages/react/src/components/oauth-client-form.tsx+25−3
@@ -76,6 +76,19 @@ export const registrationScopes = ( · 75 unmodified lines
7676 discoveredScopes: readonly string[],
7777 ): readonly string[] => (declaredScopes.length > 0 ? declaredScopes : discoveredScopes);
7878
79+/** The `originIntegration` to send with a `createClient` payload. When the
80+ * caller passes an explicit `intentIntegration` (edit flow: the app's
81+ * preserved recorded intent, which may legitimately be `null`), it wins
82+ * verbatim — editing must never silently overwrite a stamp. Otherwise this is
83+ * a fresh registration from an integration's dialog, which stamps the current
84+ * integration as recorded intent (or `null` outside any integration
85+ * context). */
86+export const resolveOriginIntegration = (
87+ intentIntegration: IntegrationSlug | null | undefined,
88+ integrationSlug: IntegrationSlug | undefined,
89+): IntegrationSlug | null =>
90+ intentIntegration !== undefined ? intentIntegration : (integrationSlug ?? null);
91+
7992 export const canSubmitOAuthClientForm = (input: {
8093 readonly submitting: boolean;
8194 readonly name: string;
@@ -101,6 +114,13 @@ export function OAuthClientForm(props: { · 19 unmodified lines
101114 * when editing (an existing app's origin is fixed) or when there is no single
102115 * integration context. */
103116 readonly integrationSlug?: IntegrationSlug;
117+ /** Explicit recorded-intent stamp to send verbatim, overriding the default
118+ * derivation from `integrationSlug`. Set this when editing an existing app
119+ * so its ALREADY-recorded origin (which may be `null`) is preserved rather
120+ * than re-derived from the current dialog's integration context. Omit for a
121+ * fresh registration, where the default (stamp `integrationSlug`, or `null`
122+ * outside an integration context) is correct. */
123+ readonly intentIntegration?: IntegrationSlug | null;
104124 /** Existing client slugs, so the generated slug stays unique across apps. */
105125 readonly existingSlugs: readonly string[];
106126 /** Endpoints/scopes declared by the integration's OAuth method. */
@@ -125,6 +145,7 @@ export function OAuthClientForm(props: { · 18 unmodified lines
125145 const {
126146 integrationName,
127147 integrationSlug,
148+ intentIntegration,
128149 existingSlugs,
129150 prefill,
130151 fixedSlug,
@@ -307,9 +328,10 @@ export function OAuthClientForm(props: { · 176 unmodified lines
307328 clientId: clientId.trim(),
308329 clientSecret: clientSecret.trim(),
309330 resource,
310− // Editing keeps the app's original origin; only a fresh registration
311− // from an integration's dialog stamps recorded intent.
312− originIntegration: fixedSlug ? null : (integrationSlug ?? null),
331+ // Editing preserves the app's already-recorded origin (via
332+ // `intentIntegration`, passed verbatim by the caller); a fresh
333+ // registration from an integration's dialog stamps recorded intent.
334+ originIntegration: resolveOriginIntegration(intentIntegration, integrationSlug),
313335 },
314336 reactivityKeys: oauthClientWriteKeys,
315337 });