History

Lead with manual entry when a server rejects automatic registration

After transparent DCR is rejected (e.g. a server that only approves loopback redirect URIs), the recovery "Register an OAuth app" form still led with "Register automatically", sending the user straight back into the path that just failed. Pass the rejection reason into OAuthClientForm so it suppresses the auto path, restates why, and leads with manual client id/secret entry plus the callback URL to allow-list.

Rhys Sullivan committed 4 months ago

Commit 8fe2a971 parent a414b712 files · +22 −3Browse files

Unified · Split

2 files
  1. packages/react/src/components/add-account-modal.tsx +1 −0
  2. packages/react/src/components/oauth-client-form.tsx +21 −3
packages/react/src/components/add-account-modal.tsx+1−0
@@ -1348,6 +1348,7 @@ function AddAccountModalView(props: AddAccountModalProps) { · 1347 unmodified lines
1348 existingSlugs={[...oauthApps, ...oauthOtherApps].map((app: OAuthClientOption) =>1348 existingSlugs={[...oauthApps, ...oauthOtherApps].map((app: OAuthClientOption) =>
1349 String(app.slug),1349 String(app.slug),
1350 )}1350 )}
1351 autoRegisterRejectedReason={dcrFallbackMessage}
1351 prefill={{1352 prefill={{
1352 authorizationUrl:1353 authorizationUrl:
1353 oauthHandoffPrefill?.authorizationUrl ??1354 oauthHandoffPrefill?.authorizationUrl ??
packages/react/src/components/oauth-client-form.tsx+21−3
@@ -88,6 +88,11 @@ export function OAuthClientForm(props: { · 87 unmodified lines
88 readonly onCreated: (result: { readonly owner: Owner; readonly slug: OAuthClientSlug }) => void;88 readonly onCreated: (result: { readonly owner: Owner; readonly slug: OAuthClientSlug }) => void;
89 readonly onCancel?: () => void;89 readonly onCancel?: () => void;
90 readonly surface?: "card" | "plain";90 readonly surface?: "card" | "plain";
91 /** When set, the server's automatic (DCR) registration is known to be rejected
92 * for this host (e.g. it refused our redirect URI). The form then suppresses
93 * the "Register automatically" path and leads with manual client entry; the
94 * string is the actionable reason shown to the user. */
95 readonly autoRegisterRejectedReason?: string | null;
91}) {96}) {
92 const {97 const {
93 integrationName,98 integrationName,
@@ -98,6 +103,7 @@ export function OAuthClientForm(props: { · 4 unmodified lines
98 onCreated,103 onCreated,
99 onCancel,104 onCancel,
100 surface = "card",105 surface = "card",
106 autoRegisterRejectedReason = null,
101 } = props;107 } = props;
102 // Non-org hosts (local/desktop) have one local workspace. Offer only Local,108 // Non-org hosts (local/desktop) have one local workspace. Offer only Local,
103 // so the owner dropdown (which hides on a single option) disappears.109 // so the owner dropdown (which hides on a single option) disappears.
@@ -178,6 +184,11 @@ export function OAuthClientForm(props: { · 74 unmodified lines
178 tokenUrl.trim().length > 0 &&184 tokenUrl.trim().length > 0 &&
179 grant === "authorization_code";185 grant === "authorization_code";
180 186
187 // When the server already rejected automatic registration for this host (e.g.
188 // it refused our non-loopback redirect URI), don't lead the user back into the
189 // path that just failed: suppress the auto CTA and lead with manual entry.
190 const showAutoRegister = canRegisterDynamic && autoRegisterRejectedReason === null;
191
181 const handleDiscover = async () => {192 const handleDiscover = async () => {
182 const url = issuerUrl.trim();193 const url = issuerUrl.trim();
183 if (url.length === 0) {194 if (url.length === 0) {
@@ -283,12 +294,19 @@ export function OAuthClientForm(props: { · 99 unmodified lines
283 <div className="space-y-1">294 <div className="space-y-1">
284 <p className="text-sm font-medium">Register an OAuth app</p>295 <p className="text-sm font-medium">Register an OAuth app</p>
285 <p className="text-xs text-muted-foreground">296 <p className="text-xs text-muted-foreground">
286 {canRegisterDynamic297 {showAutoRegister
287 ? "Register automatically below, or enter a client id/secret manually."298 ? "Register automatically below, or enter a client id/secret manually."
288 : "Paste a client id/secret. We only ask for endpoints when they aren't already known."}299 : "Paste a client id/secret. We only ask for endpoints when they aren't already known."}
289 </p>300 </p>
290 </div>301 </div>
291 302
303 {autoRegisterRejectedReason ? (
304 <div className="space-y-1 rounded-lg border border-destructive/40 bg-destructive/5 p-3">
305 <p className="text-sm font-medium text-destructive">Automatic registration unavailable</p>
306 <p className="text-xs text-muted-foreground">{autoRegisterRejectedReason}</p>
307 </div>
308 ) : null}
309
292 {/* app name */}310 {/* app name */}
293 <div className="space-y-1.5">311 <div className="space-y-1.5">
294 <Label htmlFor="oauth-app-name" className="text-xs text-muted-foreground">312 <Label htmlFor="oauth-app-name" className="text-xs text-muted-foreground">
@@ -305,7 +323,7 @@ export function OAuthClientForm(props: { · 10 unmodified lines
305 323
306 {/* register automatically (RFC 7591 DCR) — the primary path when the324 {/* register automatically (RFC 7591 DCR) — the primary path when the
307 server advertises a registration endpoint: no client id/secret needed */}325 server advertises a registration endpoint: no client id/secret needed */}
308 {canRegisterDynamic ? (326 {showAutoRegister ? (
309 <div className="space-y-2 rounded-lg border border-ring/40 bg-accent/30 p-3">327 <div className="space-y-2 rounded-lg border border-ring/40 bg-accent/30 p-3">
310 <p className="text-sm font-medium">No client ID needed</p>328 <p className="text-sm font-medium">No client ID needed</p>
311 <p className="text-xs text-muted-foreground">329 <p className="text-xs text-muted-foreground">
@@ -365,7 +383,7 @@ export function OAuthClientForm(props: { · 53 unmodified lines
365 </div>383 </div>
366 384
367 {/* divider before the manual (secondary) path when DCR is available */}385 {/* divider before the manual (secondary) path when DCR is available */}
368 {canRegisterDynamic ? (386 {showAutoRegister ? (
369 <div className="flex items-center gap-2 text-xs text-muted-foreground">387 <div className="flex items-center gap-2 text-xs text-muted-foreground">
370 <span className="h-px flex-1 bg-border/60" />388 <span className="h-px flex-1 bg-border/60" />
371 or enter a client ID manually389 or enter a client ID manually

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t