feat(clerk-js,ui): Add OAuth transport support for external auth flows#8831
feat(clerk-js,ui): Add OAuth transport support for external auth flows#8831wobsoriano wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 871776e 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds an internal OAuthTransport type and Clerk hooks, routes SignIn/SignUp redirect flows through a transport-aware helper, updates UI components to supply internal callback params and transport-aware behavior, and adds tests and a changeset marking minor bumps. ChangesOAuth transport implementation across shared, clerk-js, and UI
Sequence Diagram(s)sequenceDiagram
participant Resource as SignIn/SignUp Resource
participant AuthUtil as _authenticateWithTransport
participant OAuthTransport
participant Clerk
Resource->>AuthUtil: authenticateWithRedirect(params)
AuthUtil->>OAuthTransport: getRedirectUrl()
AuthUtil->>Resource: authenticateMethod(..., { redirectUrl })
Resource-->>AuthUtil: verificationUrl
AuthUtil->>OAuthTransport: open(verificationUrl)
OAuthTransport-->>AuthUtil: { callbackUrl }
AuthUtil->>Resource: reload({ rotatingTokenNonce? })
AuthUtil->>Clerk: __internal_handleResourceCallback(resource, callbackParams)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
API Changes Report
Summary
@clerk/clerk-jsCurrent version: 6.16.1 Subpath
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/UserProfile/ConnectedAccountsSection.tsx (1)
132-155:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't fall through to
navigate('')when reconnect returns no redirect URL.
response.verification?.externalVerificationRedirectURLis optional here, but the web fallback coerces the missing case intonavigate(''), and the transport branch just returns. That turns an incomplete reconnect response into a broken/no-op flow instead of an actionable error. Please guard the URL first and send the missing-data case throughhandleError.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/components/UserProfile/ConnectedAccountsSection.tsx` around lines 132 - 155, The web fallback currently calls navigate('') when response.verification?.externalVerificationRedirectURL is missing; update the logic in ConnectedAccountsSection.tsx after the createExternalAccount / account.reauthorize call to first check that response?.verification?.externalVerificationRedirectURL (or its href) exists and is non-empty, and only call navigate(...) when it does; if it is missing, call handleError(...) with an appropriate error/context (same error path you expect for incomplete reconnects) and do not fall through to navigate; mirror the transport branch behavior (which returns early) by returning after handling the error or successful open so you don't continue into navigate on missing data.
🧹 Nitpick comments (2)
packages/ui/src/components/UserProfile/__tests__/ConnectedAccountsSection.test.tsx (1)
140-177: ⚡ Quick winAdd a transport reconnect test for the new
ConnectedAccount.reconnectpath.The added transport coverage only exercises the add-connection menu. The reconnect flow now has separate transport behavior (
getRedirectUrl,reauthorizevscreateExternalAccount,open,user.reload), but none of that is asserted in this suite yet.As per coding guidelines,
**/*.{test,spec}.{ts,tsx}: Unit tests are required for all new functionality.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/components/UserProfile/__tests__/ConnectedAccountsSection.test.tsx` around lines 140 - 177, Add a unit test in ConnectedAccountsSection.test.tsx that covers the new reconnect path on ConnectedAccount.reconnect: set up a fixture with an existing external account, register a mock __internal_oauthTransport that implements getRedirectUrl and reauthorize (mockResolvedValue), render the component and trigger theReconnect UI action (click the reconnect button), then assert that transport.getRedirectUrl/reauthorize were called with the expected URL/params, that user.reload was called, and that the add-connection flow helpers (createExternalAccount/open) were not invoked for this reconnect path; reference the test helpers/objects fixtures.clerk, fixtures.clerk.__internal_oauthTransport, ConnectedAccount.reconnect, and fixtures.clerk.user!.reload when implementing the assertions.Source: Coding guidelines
packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts (1)
38-79: ⚡ Quick winConsider additional test coverage for transport behavior.
The test validates the happy path but could be more comprehensive:
Missing assertion: The test name claims the transport routes "instead of windowNavigate," but there's no assertion verifying that
windowNavigate(or similar browser redirect mechanisms) is NOT called. Consider adding a spy to verify the normal redirect path is bypassed.Error handling: No coverage for what happens when
transport.openrejects or whenhandleResourceCallbackfails. These paths should be tested to ensure proper error propagation.Backwards compatibility: Consider adding a test case where no transport is registered to verify the existing redirect behavior still works (regression protection).
📋 Suggested additional test cases
it('falls back to normal redirect when no transport is registered', async () => { const mockNavigate = vi.fn(); SignUp.clerk = { buildUrlWithAuth: vi.fn(u => u), // No __internal_oauthTransport __internal_environment: { displayConfig: { captchaOauthBypass: [] } }, } as any; // Mock windowNavigate or similar const navigateSpy = vi.spyOn(window, 'location', 'set'); const mockFetch = vi.fn().mockResolvedValueOnce({ client: null, response: { id: 'signup_123', verifications: { external_account: { status: 'unverified', external_verification_redirect_url: 'https://provider.example/auth', }, }, }, }); BaseResource._fetch = mockFetch; const signUp = new SignUp(); await signUp.authenticateWithRedirect({ strategy: 'oauth_google', redirectUrl: '/sso-callback', redirectUrlComplete: '/', } as any); // Verify normal redirect occurred expect(navigateSpy).toHaveBeenCalled(); }); it('handles transport rejection gracefully', async () => { const open = vi.fn().mockRejectedValue(new Error('Transport unavailable')); SignUp.clerk = { buildUrlWithAuth: vi.fn(u => u), __internal_oauthTransport: { getRedirectUrl: () => 'myapp://sso-callback', open }, __internal_handleResourceCallback: vi.fn(), __internal_environment: { displayConfig: { captchaOauthBypass: [] } }, } as any; const mockFetch = vi.fn().mockResolvedValueOnce({ client: null, response: { id: 'signup_123', verifications: { external_account: { status: 'unverified', external_verification_redirect_url: 'https://provider.example/auth', }, }, }, }); BaseResource._fetch = mockFetch; const signUp = new SignUp(); await expect( signUp.authenticateWithRedirect({ strategy: 'oauth_google', redirectUrl: '/sso-callback', redirectUrlComplete: '/', __internal_callbackParams: { signInUrl: '/sign-in' }, } as any) ).rejects.toThrow('Transport unavailable'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts` around lines 38 - 79, Test currently only asserts the happy path through the OAuth transport (SignUp.authenticateWithRedirect using SignUp.clerk.__internal_oauthTransport.open) but misses verifying the normal browser redirect is not invoked and lacks failure/back-compat cases; add three tests: (1) spy on the browser navigation mechanism (e.g., spyOn(window, 'location', 'set') or the project's windowNavigate helper) and assert it is NOT called when __internal_oauthTransport is present, (2) a test where __internal_oauthTransport.open rejects and assert authenticateWithRedirect rejects with that error (and that __internal_handleResourceCallback is not called), and (3) a fallback test with no __internal_oauthTransport registered that asserts the code falls back to the normal redirect path (using BaseResource._fetch to return the external_verification_redirect_url and verifying the window navigation spy was called).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/components/UserProfile/ConnectedAccountsMenu.tsx`:
- Around line 48-64: Both branches call createExternalAccount but only proceed
when res.verification?.externalVerificationRedirectURL exists; instead of
silently returning (transport branch) or doing nothing (web branch) when that
URL is missing, explicitly validate and surface the failure so the existing
catch/error path handles it: after calling createExternalAccount (in both the
clerk.__internal_oauthTransport branch and the window.location.href branch), if
the URL is falsy throw a descriptive Error('Missing
externalVerificationRedirectURL') (or invoke the shared error handler if one
exists) rather than returning or continuing, so the outer try/catch will route
the failure consistently; keep the existing successful flows that call
clerk.__internal_oauthTransport.open, user.reload, card.setIdle(strategy),
sleep, and navigate unchanged.
---
Outside diff comments:
In `@packages/ui/src/components/UserProfile/ConnectedAccountsSection.tsx`:
- Around line 132-155: The web fallback currently calls navigate('') when
response.verification?.externalVerificationRedirectURL is missing; update the
logic in ConnectedAccountsSection.tsx after the createExternalAccount /
account.reauthorize call to first check that
response?.verification?.externalVerificationRedirectURL (or its href) exists and
is non-empty, and only call navigate(...) when it does; if it is missing, call
handleError(...) with an appropriate error/context (same error path you expect
for incomplete reconnects) and do not fall through to navigate; mirror the
transport branch behavior (which returns early) by returning after handling the
error or successful open so you don't continue into navigate on missing data.
---
Nitpick comments:
In `@packages/clerk-js/src/core/resources/__tests__/SignUp.test.ts`:
- Around line 38-79: Test currently only asserts the happy path through the
OAuth transport (SignUp.authenticateWithRedirect using
SignUp.clerk.__internal_oauthTransport.open) but misses verifying the normal
browser redirect is not invoked and lacks failure/back-compat cases; add three
tests: (1) spy on the browser navigation mechanism (e.g., spyOn(window,
'location', 'set') or the project's windowNavigate helper) and assert it is NOT
called when __internal_oauthTransport is present, (2) a test where
__internal_oauthTransport.open rejects and assert authenticateWithRedirect
rejects with that error (and that __internal_handleResourceCallback is not
called), and (3) a fallback test with no __internal_oauthTransport registered
that asserts the code falls back to the normal redirect path (using
BaseResource._fetch to return the external_verification_redirect_url and
verifying the window navigation spy was called).
In
`@packages/ui/src/components/UserProfile/__tests__/ConnectedAccountsSection.test.tsx`:
- Around line 140-177: Add a unit test in ConnectedAccountsSection.test.tsx that
covers the new reconnect path on ConnectedAccount.reconnect: set up a fixture
with an existing external account, register a mock __internal_oauthTransport
that implements getRedirectUrl and reauthorize (mockResolvedValue), render the
component and trigger theReconnect UI action (click the reconnect button), then
assert that transport.getRedirectUrl/reauthorize were called with the expected
URL/params, that user.reload was called, and that the add-connection flow
helpers (createExternalAccount/open) were not invoked for this reconnect path;
reference the test helpers/objects fixtures.clerk,
fixtures.clerk.__internal_oauthTransport, ConnectedAccount.reconnect, and
fixtures.clerk.user!.reload when implementing the assertions.
🪄 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), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 34b8dd7e-d1ba-47ad-933e-29e071fb8609
📒 Files selected for processing (23)
.changeset/oauth-transport.mdpackages/clerk-js/src/core/__tests__/clerk.test.tspackages/clerk-js/src/core/clerk.tspackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/core/resources/SignUp.tspackages/clerk-js/src/core/resources/__tests__/SignIn.test.tspackages/clerk-js/src/core/resources/__tests__/SignUp.test.tspackages/clerk-js/src/utils/__tests__/authenticateWithTransport.test.tspackages/clerk-js/src/utils/authenticateWithTransport.tspackages/shared/src/types/clerk.tspackages/shared/src/types/index.tspackages/shared/src/types/oauthTransport.tspackages/shared/src/types/redirects.tspackages/ui/src/components/SignIn/SignInSocialButtons.tsxpackages/ui/src/components/SignIn/__tests__/SignInSocialButtons.test.tsxpackages/ui/src/components/SignIn/__tests__/buildOAuthCallbackParams.test.tspackages/ui/src/components/SignIn/buildOAuthCallbackParams.tspackages/ui/src/components/SignIn/index.tsxpackages/ui/src/components/SignUp/SignUpSocialButtons.tsxpackages/ui/src/components/SignUp/__tests__/SignUpSocialButtons.test.tsxpackages/ui/src/components/UserProfile/ConnectedAccountsMenu.tsxpackages/ui/src/components/UserProfile/ConnectedAccountsSection.tsxpackages/ui/src/components/UserProfile/__tests__/ConnectedAccountsSection.test.tsx
| try { | ||
| if (clerk.__internal_oauthTransport) { | ||
| const res = await createExternalAccount(String(await clerk.__internal_oauthTransport.getRedirectUrl())); | ||
| const url = res?.verification?.externalVerificationRedirectURL; | ||
| if (url) { | ||
| await clerk.__internal_oauthTransport.open(url); | ||
| await user.reload(); | ||
| } | ||
| }) | ||
| .catch(err => { | ||
| handleError(err, [], card.setError); | ||
| card.setIdle(strategy); | ||
| }); | ||
| void sleep(2000).then(() => card.setIdle(strategy)); | ||
| return; | ||
| } | ||
|
|
||
| const res = await createExternalAccount(window.location.href); | ||
| if (res && res.verification?.externalVerificationRedirectURL) { | ||
| void sleep(2000).then(() => card.setIdle(strategy)); | ||
| void navigate(res.verification.externalVerificationRedirectURL.href); | ||
| } |
There was a problem hiding this comment.
Handle the missing verification URL before treating connect as success.
This branch only transitions out of loading when res.verification?.externalVerificationRedirectURL exists. If createExternalAccount() resolves without that URL, the web path leaves the action stuck loading, while the transport path quietly returns after the idle timer. Please validate the URL explicitly and route the failure through the existing error path instead of letting the flow no-op.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/components/UserProfile/ConnectedAccountsMenu.tsx` around
lines 48 - 64, Both branches call createExternalAccount but only proceed when
res.verification?.externalVerificationRedirectURL exists; instead of silently
returning (transport branch) or doing nothing (web branch) when that URL is
missing, explicitly validate and surface the failure so the existing catch/error
path handles it: after calling createExternalAccount (in both the
clerk.__internal_oauthTransport branch and the window.location.href branch), if
the URL is falsy throw a descriptive Error('Missing
externalVerificationRedirectURL') (or invoke the shared error handler if one
exists) rather than returning or continuing, so the outer try/catch will route
the failure consistently; keep the existing successful flows that call
clerk.__internal_oauthTransport.open, user.reload, card.setIdle(strategy),
sleep, and navigate unchanged.
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@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: |
| const transport = SignIn.clerk.__internal_oauthTransport; | ||
| if (transport) { | ||
| return _authenticateWithTransport({ | ||
| clerk: SignIn.clerk, | ||
| transport, | ||
| resource: this, | ||
| authenticateMethod: this.authenticateWithRedirectOrPopup, | ||
| params, | ||
| callbackParams: params.__internal_callbackParams ?? {}, | ||
| }); | ||
| } |
There was a problem hiding this comment.
If no transport is registered, the existing redirect behavior is preserved. If a transport is registered, we reuse the same private authenticateWithRedirectOrPopup initiation path through _authenticateWithTransport
| * Exact callback params the SignIn `sso-callback` route passes to SSOCallback. | ||
| * Excludes `navigateOnSetActive`, which is transport-only. | ||
| */ | ||
| export function buildSignInOAuthCallbackParams(ctx: SignInContextType): HandleOAuthCallbackParams { |
There was a problem hiding this comment.
mirror sthe props passed by the normal sso-callback routes. . In Electron, the renderer does not navigate to that route, so the social button passes the same callback context up front before the external transport opens
| const redirectUrl = ctx.ssoCallbackUrl; | ||
| const redirectUrlComplete = ctx.afterSignInUrl || '/'; | ||
| const shouldUsePopup = ctx.oauthFlow === 'popup' || (ctx.oauthFlow === 'auto' && originPrefersPopup()); | ||
| const shouldUsePopup = |
There was a problem hiding this comment.
when an OAuth transport is registered, it takes precedence over oauthFlow. oauthFlow chooses between browser redirect and browser popup. Electron supplies a runtime transport instead, so popup/redirect should not be selected inside the renderer.
| * | ||
| * @internal | ||
| */ | ||
| export async function _authenticateWithTransport(opts: { |
There was a problem hiding this comment.
this is the core seam for Electron. It does not reimplement signIn.create, signUp.create, captcha retry, or other OAuth initiation logic. It calls the existing resource authenticateWithRedirectOrPopup method and only replaces the navigation callback so we can capture the provider verification URL and hand it to an external transport
The electron side of this is in #8835, where preload/main provide the transport implementation
| const redirectUrl = isModal ? appendModalState({ url: window.location.href, componentName }) : window.location.href; | ||
|
|
||
| try { | ||
| const redirectUrl = clerk.__internal_oauthTransport |
There was a problem hiding this comment.
connected accounts do not go through authenticateWithRedirectOrPopup, so this component needs a small local transport branch. The branch only swaps the callback URL and external URL opening
1 participant
Description
Adds an internal OAuth transport seam that lets Clerk UI auth flows hand off OAuth to an external runtime, such as Electron opening the system browser, while reusing the existing Clerk redirect/popup initiation logic.
This is intended to support
@clerk/electronwithout making the core UI components directly know about Electron.Electron cannot safely use the normal embedded browser redirect behavior for OAuth/SSO. The Electron SDK needs a way to open OAuth in the system browser and return to the app through a deep link, without duplicating Clerk’s OAuth initiation logic in UI code.
This keeps the runtime-specific behavior behind one internal transport primitive instead of adding Electron-specific branching throughout the components.
What changed
__internal_oauthTransportas an internal Clerk option.authenticateWithRedirectOrPopupflow and captures the generated OAuth verification URL instead of navigating in-browser.Usage
The Electron SDK can register an internal OAuth transport on
ClerkProvider. The UI components still use the normal Clerk APIs, but OAuth handoff happens through Electron IPC instead of browser navigation.The Electron preload/main process owns
window.__clerk_internal_electron: it opens the OAuth URL in the system browser, waits for the deep-link callback, then resolves with the callback URL so Clerk can resume the existing redirect callback flow.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests