feat(ui,react): Introduce OAuthConsent component#8289
Merged
wobsoriano merged 44 commits intomainfrom Apr 14, 2026
Merged
Conversation
Introduce a zero-config <OAuthConsent /> React component exported from @clerk/react and @clerk/nextjs that renders the OAuth consent screen for a signed-in user. The component reads client_id, scope, and redirect_uri from the URL by default and submits the consent decision via a native form POST to /v1/internal/oauth-consent. The accounts portal continues to work unchanged via the existing clerk.__internal_mountOAuthConsent path; the underlying UI component is a hybrid that uses context values when provided (accounts portal path) and falls back to the useOAuthConsent hook + URL parsing otherwise (public path). Highlights: - New public OAuthConsentProps type in @clerk/shared - OAuthConsentCtx refactored to lowercase oauth* casing with translation layer in ComponentContextProvider for accounts portal backward compat - Hybrid _OAuthConsent component with native form wrapping and submit buttons (proper a11y semantics) - URL parsing moved out of useOAuthConsent hook into the component for cleaner separation of concerns and SSR safety - New <OAuthConsent /> wrapper in @clerk/react re-exported from @clerk/nextjs - 7 unit tests covering public and accounts portal paths - Export snapshot updates for react-router and tanstack-react-start
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: c93b163 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new OAuthConsent component and re-exports it across Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx`:
- Around line 49-77: The form currently intercepts all submissions when either
ctx.onAllow or ctx.onDeny exists causing the missing callback to become a no-op;
change the gating logic to require both callbacks be present: replace
hasContextCallbacks = Boolean(ctx.onAllow || ctx.onDeny) with a check like
hasBothContextCallbacks = Boolean(ctx.onAllow && ctx.onDeny) and use
hasBothContextCallbacks in handleSubmit and in the code that suppresses the
hidden fallback inputs (the block around the hidden fallback inputs currently at
lines ~314-322) so that when only one callback is provided the other button
falls back to native POST.
- Around line 51-58: The actionUrl in OAuthConsent is missing forwarding of the
dev-browser JWT for development frontends, causing unauthenticated POSTs in dev;
modify the actionUrl builder in OAuthConsent so when the target is a dev
frontend (detect via clerk.frontendApi containing localhost/127.0.0.1 or a
CLERK_DEV_FRONTEND env flag), append a query param (e.g. dev_browser_jwt) with
the development JWT obtained from clerk (e.g. clerk.devBrowserJWT) or by calling
a helper like getDevBrowserJWT() before returning url.toString(), and keep the
existing session id param logic intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: e9b4495f-f3d8-4368-a9ff-f11a6dcb0edd
⛔ Files ignored due to path filters (2)
packages/react-router/src/__tests__/__snapshots__/exports.test.ts.snapis excluded by!**/*.snappackages/tanstack-react-start/src/__tests__/__snapshots__/exports.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (15)
.changeset/public-oauth-consent-component.mdpackages/nextjs/src/client-boundary/uiComponents.tsxpackages/nextjs/src/index.tspackages/react/src/components/index.tspackages/react/src/components/uiComponents.tsxpackages/shared/src/react/hooks/__tests__/useOAuthConsent.shared.spec.tspackages/shared/src/react/hooks/__tests__/useOAuthConsent.spec.tsxpackages/shared/src/react/hooks/useOAuthConsent.shared.tspackages/shared/src/react/hooks/useOAuthConsent.tsxpackages/shared/src/react/hooks/useOAuthConsent.types.tspackages/shared/src/types/clerk.tspackages/ui/src/components/OAuthConsent/OAuthConsent.tsxpackages/ui/src/components/OAuthConsent/__tests__/OAuthConsent.spec.tsxpackages/ui/src/contexts/ClerkUIComponentsContext.tsxpackages/ui/src/types.ts
💤 Files with no reviewable changes (3)
- packages/shared/src/react/hooks/useOAuthConsent.shared.ts
- packages/shared/src/react/hooks/tests/useOAuthConsent.shared.spec.ts
- packages/shared/src/react/hooks/tests/useOAuthConsent.spec.tsx
…g in OAuthConsent
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/ui/src/components/OAuthConsent/OAuthConsent.tsx (2)
49-53:⚠️ Potential issue | 🔴 CriticalPartial callback mode breaks one consent action.
Line 49 treats “either callback exists” as full context mode. Then Line 94 always prevents native submit, and Line 341 suppresses hidden fallback params. If only one callback is provided, the other button becomes a no-op.
Suggested fix
- const hasContextCallbacks = Boolean(ctx.onAllow || ctx.onDeny); + const hasAllowCallback = Boolean(ctx.onAllow); + const hasDenyCallback = Boolean(ctx.onDeny); + const hasBothContextCallbacks = hasAllowCallback && hasDenyCallback; - const isPublicFlow = !hasContextCallbacks; + const isPublicFlow = !hasBothContextCallbacks; const handleSubmit = (e: React.FormEvent<HTMLFormElement>) => { - if (!hasContextCallbacks) { - return; - } - e.preventDefault(); const submitter = (e.nativeEvent as SubmitEvent).submitter as HTMLButtonElement | null; - if (submitter?.value === 'true') { - ctx.onAllow?.(); - } else { - ctx.onDeny?.(); - } + const callback = submitter?.value === 'true' ? ctx.onAllow : ctx.onDeny; + if (!callback) { + return; + } + e.preventDefault(); + callback(); }; - {!hasContextCallbacks && + {!hasBothContextCallbacks && forwardedParams.map(([key, value]) => ( <input key={key} type='hidden' name={key} value={value} /> ))}As per coding guidelines, “Highlight only issues that could cause runtime errors, data loss, or severe maintainability issues.”
Also applies to: 93-104, 341-349
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx` around lines 49 - 53, The code treats "hasContextCallbacks" as true if either ctx.onAllow or ctx.onDeny exists, which breaks the other action when only one callback is provided; change the logic to require both callbacks (use both ctx.onAllow && ctx.onDeny) when deciding full-context behavior (adjust hasContextCallbacks/isPublicFlow accordingly), update the submit handler (the function that currently always prevents native submission) to only prevent default when both callbacks are present, and stop suppressing the hidden fallback params unless both callbacks exist so the fallback submit will work when one callback is missing.
78-85:⚠️ Potential issue | 🔴 CriticalNative POST auth is incomplete for development instances.
Line 83 explicitly leaves dev JWT forwarding as TODO. This keeps the dev consent POST path potentially unauthenticated and failing in development environments.
#!/bin/bash set -euo pipefail # Verify whether repo already has a canonical helper/field for dev browser JWT forwarding. rg -n -C3 --type=ts --type=tsx 'dev_browser_jwt|devBrowserJWT|getDevBrowserJWT|CLERK_DEV_FRONTEND|frontendApi' # Verify all OAuth consent POST action builders for consistency. rg -n -C3 --type=ts --type=tsx '/v1/internal/oauth-consent'As per coding guidelines, “Comment only when the issue must be resolved before merge, otherwise remain silent.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx` around lines 78 - 85, The actionUrl builder currently omits forwarding the dev browser JWT, causing dev POSTs to /v1/internal/oauth-consent to be unauthenticated; update the IIFE that constructs actionUrl to detect development instances and append the dev JWT (e.g., use the project's canonical helper such as getDevBrowserJWT or devBrowserJWT if present, or read CLERK_DEV_FRONTEND token) as a query param (e.g., dev_browser_jwt) when available, ensuring the URL includes url.searchParams.set('_clerk_dev_browser_jwt', token) or the agreed param name; reference the actionUrl constant and clerk.session usage so you add the token only in dev contexts and do not change production behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx`:
- Around line 49-53: The code treats "hasContextCallbacks" as true if either
ctx.onAllow or ctx.onDeny exists, which breaks the other action when only one
callback is provided; change the logic to require both callbacks (use both
ctx.onAllow && ctx.onDeny) when deciding full-context behavior (adjust
hasContextCallbacks/isPublicFlow accordingly), update the submit handler (the
function that currently always prevents native submission) to only prevent
default when both callbacks are present, and stop suppressing the hidden
fallback params unless both callbacks exist so the fallback submit will work
when one callback is missing.
- Around line 78-85: The actionUrl builder currently omits forwarding the dev
browser JWT, causing dev POSTs to /v1/internal/oauth-consent to be
unauthenticated; update the IIFE that constructs actionUrl to detect development
instances and append the dev JWT (e.g., use the project's canonical helper such
as getDevBrowserJWT or devBrowserJWT if present, or read CLERK_DEV_FRONTEND
token) as a query param (e.g., dev_browser_jwt) when available, ensuring the URL
includes url.searchParams.set('_clerk_dev_browser_jwt', token) or the agreed
param name; reference the actionUrl constant and clerk.session usage so you add
the token only in dev contexts and do not change production behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: b8ad3739-da04-4a58-860a-7f698aadc4dc
📒 Files selected for processing (1)
packages/ui/src/components/OAuthConsent/OAuthConsent.tsx
This comment has been minimized.
This comment has been minimized.
…lic flow Remove the "Authorization failed:" prefix from error messages to match the rest of the codebase's direct-message convention. Add a loading guard so the consent card does not render with empty values while the useOAuthConsent hook is still fetching. Returns null until data is available, letting the React wrapper's fallback handle loading UI.
…onsentProps alongside deprecated capital-A ones Both casings coexist: new lowercase fields (oauthApplicationName, etc.) are preferred, while the deprecated capital-A fields (oAuthApplicationName, etc.) remain for accounts portal backward compat. The translation layer in ComponentContextProvider uses the new fields with ?? fallback to the deprecated ones.
…elds used by accounts portal
…entProps to AvailableComponentProps Gate useOAuthConsent with enabled: !hasContextCallbacks so the hook skips the FAPI request when the accounts portal already provides all data via context. Add test assertion confirming no fetch on that path. Add __internal_OAuthConsentProps to the AvailableComponentProps union for type hygiene and consistency with other internal component props.
Member
Author
|
!snapshot |
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/ui/src/components/OAuthConsent/OAuthConsent.tsx (1)
55-57:⚠️ Potential issue | 🔴 CriticalRequire both context callbacks before switching out of the native POST flow.
A single
onAlloworonDenyis enough to disable the public-flow fetch, suppress the hidden fallback inputs, andpreventDefault()every submit. If a caller provides only one callback, the other button becomes a no-op instead of falling back to the native consent POST.Suggested fix
- // onAllow and onDeny are always provided as a pair by the accounts portal. - const hasContextCallbacks = Boolean(ctx.onAllow || ctx.onDeny); + const hasBothContextCallbacks = Boolean(ctx.onAllow && ctx.onDeny); ... const { data, isLoading, error } = useOAuthConsent({ oauthClientId, scope, // TODO: Remove this once account portal is refactored to use this component - enabled: !hasContextCallbacks, + enabled: !hasBothContextCallbacks, }); ... const handleSubmit = (e: React.FormEvent<HTMLFormElement>) => { - if (!hasContextCallbacks) { - return; - } - e.preventDefault(); const submitter = (e.nativeEvent as SubmitEvent).submitter as HTMLButtonElement | null; - if (submitter?.value === 'true') { - ctx.onAllow?.(); - } else { - ctx.onDeny?.(); + const callback = submitter?.value === 'true' ? ctx.onAllow : ctx.onDeny; + if (!callback) { + return; } + e.preventDefault(); + callback(); }; ... - {!hasContextCallbacks && + {!hasBothContextCallbacks && forwardedParams.map(([key, value]) => ( <input key={key} type='hidden' name={key} value={value} /> ))} - {!hasContextCallbacks && ctx.enableOrgSelection && selectedOrg && ( + {!hasBothContextCallbacks && ctx.enableOrgSelection && selectedOrg && ( <input type='hidden' name='organization_id' value={selectedOrg} /> )}Also applies to: 65-70, 131-141, 299-314
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx` around lines 55 - 57, The current logic treats the presence of either ctx.onAllow or ctx.onDeny as sufficient to disable the native POST flow; change this to require both callbacks be provided before switching flows by replacing Boolean(ctx.onAllow || ctx.onDeny) with Boolean(ctx.onAllow && ctx.onDeny) (update the hasContextCallbacks variable and any other places that check these callbacks), and use that stricter condition in the form submit handler (where preventDefault is called), in the rendering decision for the hidden fallback inputs, and in the button click handlers so that if only one callback is present the component remains in the native POST flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/components/OAuthConsent/utils.ts`:
- Around line 3-6: getRootDomain currently naively returns the last two labels
of the hostname which fails for multi-part public suffixes (e.g. *.co.uk),
IPv4/IPv6 literals, and localhost; update getRootDomain to parse the hostname
via the URL constructor, detect IP literals or single-label hosts and return
hostname as-is for those cases, and otherwise use a Public Suffix List-aware
resolver (e.g., the "psl" package) to compute the registered domain
(psl.get(hostname)) and fall back to hostname if psl returns null. Ensure the
function still throws only on invalid URLs and reference getRootDomain when
applying these changes.
---
Duplicate comments:
In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx`:
- Around line 55-57: The current logic treats the presence of either ctx.onAllow
or ctx.onDeny as sufficient to disable the native POST flow; change this to
require both callbacks be provided before switching flows by replacing
Boolean(ctx.onAllow || ctx.onDeny) with Boolean(ctx.onAllow && ctx.onDeny)
(update the hasContextCallbacks variable and any other places that check these
callbacks), and use that stricter condition in the form submit handler (where
preventDefault is called), in the rendering decision for the hidden fallback
inputs, and in the button click handlers so that if only one callback is present
the component remains in the native POST flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 0009687f-0a8e-44c5-ba5d-75335974666a
📒 Files selected for processing (34)
.changeset/public-oauth-consent-component.mddocs/superpowers/specs/2026-04-13-oauth-consent-org-selection.mdpackages/clerk-js/sandbox/app.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/modules/oauthApplication/__tests__/OAuthApplication.test.tspackages/clerk-js/src/core/modules/oauthApplication/index.tspackages/clerk-js/src/core/resources/__tests__/OAuthApplication.test.tspackages/clerk-js/src/core/resources/internal.tspackages/localizations/src/en-US.tspackages/nextjs/src/client-boundary/uiComponents.tsxpackages/nextjs/src/internal.tspackages/react-router/package.jsonpackages/react-router/src/internal.tspackages/react/src/components/uiComponents.tsxpackages/react/src/internal.tspackages/shared/src/react/hooks/useOAuthConsent.tsxpackages/shared/src/react/hooks/useOAuthConsent.types.tspackages/shared/src/types/clerk.tspackages/shared/src/types/localization.tspackages/shared/src/types/oauthApplication.tspackages/tanstack-react-start/package.jsonpackages/tanstack-react-start/src/internal.tspackages/ui/src/components/OAuthConsent/InlineAction.tsxpackages/ui/src/components/OAuthConsent/ListGroup.tsxpackages/ui/src/components/OAuthConsent/LogoGroup.tsxpackages/ui/src/components/OAuthConsent/OAuthConsent.tsxpackages/ui/src/components/OAuthConsent/OrgSelect.tsxpackages/ui/src/components/OAuthConsent/__tests__/InlineAction.test.tsxpackages/ui/src/components/OAuthConsent/__tests__/OAuthConsent.test.tsxpackages/ui/src/components/OAuthConsent/utils.tspackages/ui/src/contexts/ClerkUIComponentsContext.tsxpackages/ui/src/customizables/elementDescriptors.tspackages/ui/src/internal/appearance.tspackages/ui/src/types.ts
💤 Files with no reviewable changes (2)
- packages/clerk-js/src/core/resources/internal.ts
- packages/clerk-js/src/core/resources/tests/OAuthApplication.test.ts
✅ Files skipped from review due to trivial changes (4)
- packages/react-router/src/internal.ts
- packages/nextjs/src/internal.ts
- packages/tanstack-react-start/src/internal.ts
- packages/localizations/src/en-US.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/nextjs/src/client-boundary/uiComponents.tsx
- packages/shared/src/react/hooks/useOAuthConsent.types.ts
- packages/shared/src/react/hooks/useOAuthConsent.tsx
- packages/ui/src/contexts/ClerkUIComponentsContext.tsx
- packages/react/src/components/uiComponents.tsx
- .changeset/public-oauth-consent-component.md
- packages/ui/src/types.ts
- packages/shared/src/types/clerk.ts
Comment on lines
+3
to
+6
| export function getRootDomain(url: string): string { | ||
| try { | ||
| const { hostname } = new URL(url); | ||
| return hostname.split('.').slice(-2).join('.'); |
Contributor
There was a problem hiding this comment.
getRootDomain can display an incorrect destination host in consent warnings
On Line [6], taking only the last two hostname labels breaks for multi-part public suffixes (*.co.uk) and IP hosts, which can show misleading domain text in a trust/security prompt.
Suggested fix
export function getRootDomain(url: string): string {
try {
- const { hostname } = new URL(url);
- return hostname.split('.').slice(-2).join('.');
+ // Use the parsed hostname directly to avoid incorrect truncation
+ // for multi-part TLDs and IP literals.
+ return new URL(url).hostname;
} catch {
return '';
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function getRootDomain(url: string): string { | |
| try { | |
| const { hostname } = new URL(url); | |
| return hostname.split('.').slice(-2).join('.'); | |
| export function getRootDomain(url: string): string { | |
| try { | |
| // Use the parsed hostname directly to avoid incorrect truncation | |
| // for multi-part TLDs and IP literals. | |
| return new URL(url).hostname; | |
| } catch { | |
| return ''; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/OAuthConsent/utils.ts` around lines 3 - 6,
getRootDomain currently naively returns the last two labels of the hostname
which fails for multi-part public suffixes (e.g. *.co.uk), IPv4/IPv6 literals,
and localhost; update getRootDomain to parse the hostname via the URL
constructor, detect IP literals or single-label hosts and return hostname as-is
for those cases, and otherwise use a Public Suffix List-aware resolver (e.g.,
the "psl" package) to compute the registered domain (psl.get(hostname)) and fall
back to hostname if psl returns null. Ensure the function still throws only on
invalid URLs and reference getRootDomain when applying these changes.
Member
Author
|
!snapshot |
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx`:
- Around line 136-146: handleSubmit prevents native form submission and calls
ctx.onAllow/onDeny without passing the selected organization, so
ctx.enableOrgSelection changes (selectedOrg/effectiveOrg) never reach the
callback-backed flow; either restore native submission or modify the callback
contract and invocations to include the effective org. Update handleSubmit to
compute the selected org (same logic as where selected org is serialized for
POST) and pass it into ctx.onAllow(effectiveOrg) / ctx.onDeny(effectiveOrg) (and
update the ctx type for onAllow/onDeny accordingly), or alternatively stop
calling e.preventDefault() and allow the existing form POST to include
organization_id; ensure the same fix is applied to the other referenced places
that trigger consent via callbacks (the org selection logic around
selectedOrg/effectiveOrg and the serialization used at the POST path).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: f2de1501-201e-4126-88a0-0ab01497224c
📒 Files selected for processing (1)
packages/ui/src/components/OAuthConsent/OAuthConsent.tsx
Comment on lines
+136
to
+146
| const handleSubmit = (e: React.FormEvent<HTMLFormElement>) => { | ||
| if (!hasContextCallbacks) { | ||
| return; | ||
| } | ||
| e.preventDefault(); | ||
| const submitter = (e.nativeEvent as SubmitEvent).submitter as HTMLButtonElement | null; | ||
| if (submitter?.value === 'true') { | ||
| ctx.onAllow?.(); | ||
| } else { | ||
| ctx.onDeny?.(); | ||
| } |
Contributor
There was a problem hiding this comment.
organization_id is ignored on the callback-backed flow.
When ctx.enableOrgSelection is on, the user can change the org at Lines 228-233, but that value only gets serialized at Lines 313-318. In the accounts-portal path, handleSubmit at Lines 136-146 prevents the native POST and calls ctx.onAllow?.() / ctx.onDeny?.() with no org payload, so the selected org never affects the consent action. Either keep native submission here or extend the callback contract to carry effectiveOrg.
Also applies to: 228-233, 313-318
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx` around lines 136 -
146, handleSubmit prevents native form submission and calls ctx.onAllow/onDeny
without passing the selected organization, so ctx.enableOrgSelection changes
(selectedOrg/effectiveOrg) never reach the callback-backed flow; either restore
native submission or modify the callback contract and invocations to include the
effective org. Update handleSubmit to compute the selected org (same logic as
where selected org is serialized for POST) and pass it into
ctx.onAllow(effectiveOrg) / ctx.onDeny(effectiveOrg) (and update the ctx type
for onAllow/onDeny accordingly), or alternatively stop calling
e.preventDefault() and allow the existing form POST to include organization_id;
ensure the same fix is applied to the other referenced places that trigger
consent via callbacks (the org selection logic around selectedOrg/effectiveOrg
and the serialization used at the POST path).
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/ui/src/components/OAuthConsent/OAuthConsent.tsx (2)
50-52:⚠️ Potential issue | 🔴 CriticalCallback-backed consent still drops the selected organization.
effectiveOrgchanges when the user picks a different org, but Lines 148-151 invoke the consent callbacks without that value, and Lines 319-324 only serializeorganization_idfor the native POST path. In the accounts-portal flow, the org selector therefore has no effect on the submitted consent.Also applies to: 142-152, 234-239, 319-324
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx` around lines 50 - 52, The selected organization (selectedOrg/effectiveOrg) is not being passed into the consent callbacks or serialized for the callback-backed/accounts-portal path; update the places that invoke the consent handlers (onConsent / onDeny or whatever consent callback functions are used around the consent submission logic) to pass effectiveOrg (or its id) as an argument, and ensure the consent payload generation logic also includes organization_id when building the POST/callback payload for the callback-backed/accounts-portal flow so the selected org is sent in both native POST and callback-backed paths.
53-55:⚠️ Potential issue | 🔴 CriticalRequire both consent callbacks before hijacking the form.
Lines 53-55 switch into callback mode when either handler exists, but Lines 142-152 then block every submit and Lines 310-325 remove the native POST fallback inputs. If only one callback is provided, the other button becomes a no-op.
Also applies to: 142-152, 310-325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx` around lines 53 - 55, The consent form currently hijacks native POST fallback when only one of the portal callbacks is present because hasContextCallbacks is computed with ||; change it to require both callbacks (use ctx.onAllow && ctx.onDeny) so the form only switches to callback mode when both handlers exist, and update the related submit interception logic (the submit handler that blocks submits) and the code path that removes native POST fallback inputs (the rendering branch that strips hidden inputs) to be gated by the updated hasContextCallbacks boolean (or equivalent check) so the deny/allow buttons remain functional if either callback is missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx`:
- Around line 50-52: The selected organization (selectedOrg/effectiveOrg) is not
being passed into the consent callbacks or serialized for the
callback-backed/accounts-portal path; update the places that invoke the consent
handlers (onConsent / onDeny or whatever consent callback functions are used
around the consent submission logic) to pass effectiveOrg (or its id) as an
argument, and ensure the consent payload generation logic also includes
organization_id when building the POST/callback payload for the
callback-backed/accounts-portal flow so the selected org is sent in both native
POST and callback-backed paths.
- Around line 53-55: The consent form currently hijacks native POST fallback
when only one of the portal callbacks is present because hasContextCallbacks is
computed with ||; change it to require both callbacks (use ctx.onAllow &&
ctx.onDeny) so the form only switches to callback mode when both handlers exist,
and update the related submit interception logic (the submit handler that blocks
submits) and the code path that removes native POST fallback inputs (the
rendering branch that strips hidden inputs) to be gated by the updated
hasContextCallbacks boolean (or equivalent check) so the deny/allow buttons
remain functional if either callback is missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4e92224f-1baa-4ff5-b60f-c7e8df18b127
📒 Files selected for processing (1)
packages/ui/src/components/OAuthConsent/OAuthConsent.tsx
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/ui/src/components/OAuthConsent/OAuthConsent.tsx (2)
50-51:⚠️ Potential issue | 🔴 CriticalPartial context callbacks still break one consent action.
Using
||means if only one callback is provided, the other button becomes a no-op (form submission prevented, but no callback invoked). Per theOAuthConsentCtxJSDoc, both callbacks are expected together. Should use&&:- const hasContextCallbacks = Boolean(ctx.onAllow || ctx.onDeny); + const hasContextCallbacks = Boolean(ctx.onAllow && ctx.onDeny);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx` around lines 50 - 51, The current hasContextCallbacks uses Boolean(ctx.onAllow || ctx.onDeny) which treats a single provided callback as valid and causes the other consent action to become a silent no-op; change the check to require both callbacks (use ctx.onAllow && ctx.onDeny or Boolean(ctx.onAllow && ctx.onDeny)) so hasContextCallbacks accurately reflects the OAuthConsentCtx expectation that onAllow and onDeny are provided together and both buttons invoke their callbacks.
139-150:⚠️ Potential issue | 🔴 Critical
organization_idis still lost on the callback-backed flow.When
ctx.enableOrgSelectionis enabled, the user's org selection (effectiveOrg) is never passed toctx.onAllow()/ctx.onDeny(). The callback signature should be extended to accept the selected org, or the form should POST natively.const handleSubmit = (e: React.FormEvent<HTMLFormElement>) => { if (!hasContextCallbacks) { return; } e.preventDefault(); const submitter = (e.nativeEvent as SubmitEvent).submitter as HTMLButtonElement | null; if (submitter?.value === 'true') { - ctx.onAllow?.(); + ctx.onAllow?.(effectiveOrg); } else { - ctx.onDeny?.(); + ctx.onDeny?.(effectiveOrg); } };This also requires updating the
OAuthConsentCtxtype to accept the org parameter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx` around lines 139 - 150, The handleSubmit currently calls ctx.onAllow() / ctx.onDeny() without passing the selected organization, losing organization_id when enableOrgSelection is used; update the OAuthConsentCtx type to change onAllow and onDeny to accept the selected org (e.g., onAllow?: (effectiveOrg?: string) => void) and then modify the handleSubmit function to pass the current effectiveOrg value into ctx.onAllow(effectiveOrg) / ctx.onDeny(effectiveOrg); also search for other places that construct or call OAuthConsentCtx callbacks and update their call sites to handle the new org parameter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/ui/src/components/OAuthConsent/OAuthConsent.tsx`:
- Around line 50-51: The current hasContextCallbacks uses Boolean(ctx.onAllow ||
ctx.onDeny) which treats a single provided callback as valid and causes the
other consent action to become a silent no-op; change the check to require both
callbacks (use ctx.onAllow && ctx.onDeny or Boolean(ctx.onAllow && ctx.onDeny))
so hasContextCallbacks accurately reflects the OAuthConsentCtx expectation that
onAllow and onDeny are provided together and both buttons invoke their
callbacks.
- Around line 139-150: The handleSubmit currently calls ctx.onAllow() /
ctx.onDeny() without passing the selected organization, losing organization_id
when enableOrgSelection is used; update the OAuthConsentCtx type to change
onAllow and onDeny to accept the selected org (e.g., onAllow?: (effectiveOrg?:
string) => void) and then modify the handleSubmit function to pass the current
effectiveOrg value into ctx.onAllow(effectiveOrg) / ctx.onDeny(effectiveOrg);
also search for other places that construct or call OAuthConsentCtx callbacks
and update their call sites to handle the new org parameter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 5880c230-2fdb-4bee-8148-5da46918865b
📒 Files selected for processing (1)
packages/ui/src/components/OAuthConsent/OAuthConsent.tsx
Contributor
|
Hey @wobsoriano - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.3.14-snapshot.v20260414205636 --save-exact
npm i @clerk/astro@3.0.14-snapshot.v20260414205636 --save-exact
npm i @clerk/backend@3.2.10-snapshot.v20260414205636 --save-exact
npm i @clerk/chrome-extension@3.1.11-snapshot.v20260414205636 --save-exact
npm i @clerk/clerk-js@6.7.1-snapshot.v20260414205636 --save-exact
npm i @clerk/dev-cli@0.1.1-snapshot.v20260414205636 --save-exact
npm i @clerk/expo@3.1.11-snapshot.v20260414205636 --save-exact
npm i @clerk/expo-passkeys@1.0.12-snapshot.v20260414205636 --save-exact
npm i @clerk/express@2.1.2-snapshot.v20260414205636 --save-exact
npm i @clerk/fastify@3.1.12-snapshot.v20260414205636 --save-exact
npm i @clerk/hono@0.1.12-snapshot.v20260414205636 --save-exact
npm i @clerk/localizations@4.4.2-snapshot.v20260414205636 --save-exact
npm i @clerk/msw@0.0.12-snapshot.v20260414205636 --save-exact
npm i @clerk/nextjs@7.2.0-snapshot.v20260414205636 --save-exact
npm i @clerk/nuxt@2.2.1-snapshot.v20260414205636 --save-exact
npm i @clerk/react@6.4.0-snapshot.v20260414205636 --save-exact
npm i @clerk/react-router@3.1.0-snapshot.v20260414205636 --save-exact
npm i @clerk/shared@4.8.0-snapshot.v20260414205636 --save-exact
npm i @clerk/tanstack-react-start@1.1.0-snapshot.v20260414205636 --save-exact
npm i @clerk/testing@2.0.14-snapshot.v20260414205636 --save-exact
npm i @clerk/ui@1.6.0-snapshot.v20260414205636 --save-exact
npm i @clerk/upgrade@2.0.3-snapshot.v20260414205636 --save-exact
npm i @clerk/vue@2.0.13-snapshot.v20260414205636 --save-exact |
alexcarpenter
approved these changes
Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Summary
clerk.oauthApplicationfrom a hollow static resource class to a proper module class (matching theapiKeysandbillingpatterns), and addsbuildConsentActionUrl({ clientId })so custom-flow developers can get the consent form's POST URL without constructing it manually<OAuthConsent />component and auseOAuthConsenthook from the/internalpath for now<OAuthConsent />renders the consent screen usinguseOAuthConsentto fetch scope and application metadata from FAPI whenclient_idandredirect_uriare provided as URL parameters.This update is backwards compatible with accounts portal consent page. Once ready, we'll update accounts portal to use the component.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change