diff --git a/.changeset/curly-cameras-laugh.md b/.changeset/curly-cameras-laugh.md new file mode 100644 index 00000000000..b7d34bc84ff --- /dev/null +++ b/.changeset/curly-cameras-laugh.md @@ -0,0 +1,5 @@ +--- +"@clerk/clerk-js": patch +--- + +Fix Core 3 OAuth retry routing to the previously selected provider after an abandoned redirect. diff --git a/integration/templates/custom-flows-react-vite/src/main.tsx b/integration/templates/custom-flows-react-vite/src/main.tsx index 33b3d38e758..b133724280a 100644 --- a/integration/templates/custom-flows-react-vite/src/main.tsx +++ b/integration/templates/custom-flows-react-vite/src/main.tsx @@ -2,7 +2,7 @@ import { StrictMode } from 'react'; import { createRoot } from 'react-dom/client'; import { BrowserRouter, Route, Routes } from 'react-router'; import './index.css'; -import { ClerkProvider } from '@clerk/react'; +import { AuthenticateWithRedirectCallback, ClerkProvider } from '@clerk/react'; import { Home } from './routes/Home'; import { SignIn } from './routes/SignIn'; import { SignUp } from './routes/SignUp'; @@ -44,6 +44,15 @@ createRoot(document.getElementById('root')!).render( path='/protected' element={} /> + + } + /> diff --git a/integration/templates/custom-flows-react-vite/src/routes/SignIn.tsx b/integration/templates/custom-flows-react-vite/src/routes/SignIn.tsx index 27eead90579..2cb1e08f061 100644 --- a/integration/templates/custom-flows-react-vite/src/routes/SignIn.tsx +++ b/integration/templates/custom-flows-react-vite/src/routes/SignIn.tsx @@ -5,8 +5,8 @@ import { Button } from '@/components/ui/button'; import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card'; import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; -import { useSignIn, useUser } from '@clerk/react'; -import { useState } from 'react'; +import { useClerk, useSignIn, useUser } from '@clerk/react'; +import { useEffect, useState } from 'react'; import { NavLink, useNavigate } from 'react-router'; type AvailableStrategy = 'email_code' | 'phone_code' | 'password' | 'reset_password_email_code'; @@ -16,15 +16,44 @@ export function SignIn({ className, ...props }: React.ComponentProps<'div'>) { const [selectedStrategy, setSelectedStrategy] = useState(null); const { isSignedIn } = useUser(); const navigate = useNavigate(); + const clerk = useClerk(); - const handleOauth = async (strategy: 'oauth_google') => { + const computeProviders = () => { + const social = (clerk as any)?.__internal_environment?.userSettings?.social ?? {}; + return Object.entries(social as Record) + .filter(([key, value]) => key.startsWith('oauth_') && value?.enabled) + .map(([, value]) => ({ strategy: value.strategy, name: value.name })); + }; + const [oauthProviders, setOauthProviders] = useState<{ strategy: string; name: string }[]>(computeProviders); + useEffect(() => { + setOauthProviders(computeProviders()); + return clerk.addListener?.(() => setOauthProviders(computeProviders())); + }, [clerk]); + + const handleOauth = async (strategy: string) => { await signIn.sso({ - strategy, - redirectUrl: '/sso-callback', - redirectUrlComplete: '/protected', + strategy: strategy as Parameters[0]['strategy'], + redirectUrl: '/protected', + redirectCallbackUrl: '/sso-callback', }); }; + const oauthButtons = ( + <> + {oauthProviders.map(provider => ( + + ))} + + ); + const handleSubmit = async (formData: FormData) => { const identifier = formData.get('identifier'); if (!identifier) { @@ -103,6 +132,7 @@ export function SignIn({ className, ...props }: React.ComponentProps<'div'>) {
+ {oauthButtons} {signIn.supportedFirstFactors .filter(({ strategy }) => strategy !== 'reset_password_email_code') .map(({ strategy }) => ( @@ -268,14 +298,7 @@ export function SignIn({ className, ...props }: React.ComponentProps<'div'>) {
- + {oauthButtons}
diff --git a/integration/tests/custom-flows/oauth.test.ts b/integration/tests/custom-flows/oauth.test.ts new file mode 100644 index 00000000000..ab689be889a --- /dev/null +++ b/integration/tests/custom-flows/oauth.test.ts @@ -0,0 +1,96 @@ +import { createClerkClient } from '@clerk/backend'; +import { expect, test } from '@playwright/test'; + +import type { Application } from '../../models/application'; +import { appConfigs } from '../../presets'; +import { instanceKeys } from '../../presets/envs'; +import type { FakeUser } from '../../testUtils'; +import { createTestUtils } from '../../testUtils'; +import { createUserService } from '../../testUtils/usersService'; + +test.describe('Custom Flows OAuth @custom', () => { + test.describe.configure({ mode: 'serial' }); + + let app: Application; + let fakeUser: FakeUser; + + test.beforeAll(async () => { + test.setTimeout(150_000); + app = await appConfigs.customFlows.reactVite.clone().commit(); + await app.setup(); + await app.withEnv(appConfigs.envs.withEmailCodes); + await app.dev(); + + const client = createClerkClient({ + secretKey: instanceKeys.get('oauth-provider').sk, + publishableKey: instanceKeys.get('oauth-provider').pk, + }); + const users = createUserService(client); + fakeUser = users.createFakeUser({ withUsername: true }); + await users.createBapiUser(fakeUser); + }); + + test.afterAll(async () => { + const u = createTestUtils({ app }); + await fakeUser.deleteIfExists(); + await u.services.users.deleteIfExists({ email: fakeUser.email }); + await app.teardown(); + }); + + test('SDK-75: retrying OAuth after an abandoned redirect creates a fresh sign-in', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + + // Block the OAuth provider redirect on the first attempt only. clerk-js sets + // `firstFactorVerification.status='unverified'` and `externalVerificationRedirectURL` + // the moment the POST resolves — before the navigation runs — so aborting the + // navigation deterministically reproduces the SDK-75 abandoned-redirect state + // without depending on browser back/BFCache semantics. + let blockOnce = true; + await page.route('**/oauth/authorize**', async route => { + if (blockOnce && route.request().isNavigationRequest()) { + blockOnce = false; + await route.abort('aborted'); + return; + } + await route.continue(); + }); + + await u.page.goToRelative('/sign-in'); + await u.page.waitForClerkJsLoaded(); + + const oauthButton = u.page.getByRole('button', { name: /^Sign in with / }); + await oauthButton.first().waitFor(); + + // First attempt: capture the POST, then let the redirect get aborted. + const firstPostPromise = page.waitForRequest( + req => req.method() === 'POST' && /\/v1\/client\/sign_ins(\?|$)/.test(req.url()), + ); + await oauthButton.first().click(); + await firstPostPromise; + + // The redirect was aborted, so we stay on the app's sign-in page with stale + // OAuth state lingering in the SignIn resource. Wait for the OAuth button to + // be re-enabled (fetchStatus settles back to 'idle' once the navigation aborts). + await u.page.waitForURL(url => url.toString().startsWith(app.serverUrl) && url.pathname.includes('/sign-in')); + await oauthButton.first().waitFor(); + + // Second attempt: must POST to /client/sign_ins again. If the previous reuse + // logic kicked in (pre-fix), SignInFuture.sso would skip create and silently + // no-op — so the second POST not happening is exactly the regression. + const secondPostPromise = page.waitForRequest( + req => req.method() === 'POST' && /\/v1\/client\/sign_ins(\?|$)/.test(req.url()), + ); + await oauthButton.first().click(); + const secondPost = await secondPostPromise; + expect(secondPost.method()).toBe('POST'); + + // Complete the OAuth flow end-to-end and assert we're signed in on the app instance. + await u.page.getByText('Sign in to oauth-provider').waitFor(); + await u.po.signIn.setIdentifier(fakeUser.email); + await u.po.signIn.continue(); + await u.po.signIn.enterTestOtpCode(); + + await u.page.waitForAppUrl('/protected'); + await u.po.expect.toBeSignedIn(); + }); +}); diff --git a/packages/clerk-js/src/core/resources/SignIn.ts b/packages/clerk-js/src/core/resources/SignIn.ts index f6540d06eb9..924b930a49e 100644 --- a/packages/clerk-js/src/core/resources/SignIn.ts +++ b/packages/clerk-js/src/core/resources/SignIn.ts @@ -1145,7 +1145,14 @@ class SignInFuture implements SignInFutureResource { routes.actionCompleteRedirectUrl = wrappedRoutes.redirectUrl; } - if (!this.#resource.id) { + // Enterprise SSO has a `prepare_first_factor` call below that runs against the + // existing sign-in and refreshes server state, so reuse is safe for ticket-based + // and identifier-discovery flows. OAuth strategies have no equivalent refresh — + // the redirect URL only comes back from `_create` — so reusing a stale resource + // would replay the previous provider's redirect (SDK-75). Always start fresh. + const shouldCreateSignIn = !this.#resource.id || strategy !== 'enterprise_sso'; + + if (shouldCreateSignIn) { await this._create({ strategy, ...routes, diff --git a/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts b/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts index 76d82b08de8..e49e369dc83 100644 --- a/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts @@ -2110,6 +2110,131 @@ describe('SignIn', () => { }); }); + it('reuses an existing ticket sign-in when preparing enterprise SSO', async () => { + vi.stubGlobal('window', { location: { origin: 'https://example.com' } }); + + SignIn.clerk = { + buildUrlWithAuth: vi.fn().mockReturnValue('https://example.com/sso-callback'), + __internal_environment: { + displayConfig: { + captchaOauthBypass: [], + }, + }, + } as any; + + const mockFetch = vi + .fn() + .mockResolvedValueOnce({ + client: null, + response: { + id: 'signin_ticket', + status: 'needs_first_factor', + supported_first_factors: [{ strategy: 'enterprise_sso' }], + }, + }) + .mockResolvedValueOnce({ + client: null, + response: { + id: 'signin_ticket', + first_factor_verification: { + status: 'unverified', + external_verification_redirect_url: 'https://sso.example.com/auth', + }, + }, + }); + BaseResource._fetch = mockFetch; + + const signIn = new SignIn(); + await signIn.__internal_future.ticket({ ticket: 'ticket_123' }); + await signIn.__internal_future.sso({ + strategy: 'enterprise_sso', + redirectUrl: 'https://complete.example.com', + redirectCallbackUrl: '/sso-callback', + }); + + expect(mockFetch).toHaveBeenNthCalledWith(2, { + method: 'POST', + path: '/client/sign_ins/signin_ticket/prepare_first_factor', + body: { + strategy: 'enterprise_sso', + redirectUrl: 'https://example.com/sso-callback', + actionCompleteRedirectUrl: 'https://complete.example.com', + }, + }); + }); + + it('reuses an existing enterprise SSO sign-in and uses the fresh redirect URL when retrying after an abandoned attempt', async () => { + vi.stubGlobal('window', { location: { origin: 'https://example.com' } }); + + const mockPopup = { location: { href: '' } } as Window; + + SignIn.clerk = { + buildUrlWithAuth: vi.fn().mockReturnValue('https://example.com/sso-callback'), + __internal_environment: { + displayConfig: { + captchaOauthBypass: [], + }, + }, + } as any; + + const mockFetch = vi + .fn() + .mockResolvedValueOnce({ + client: null, + response: { + id: 'signin_enterprise', + first_factor_verification: { + status: 'unverified', + external_verification_redirect_url: 'https://sso.example.com/auth/fresh', + }, + }, + }) + .mockResolvedValueOnce({ + client: null, + response: { + id: 'signin_enterprise', + status: 'complete', + }, + }); + BaseResource._fetch = mockFetch; + + vi.mocked(_futureAuthenticateWithPopup).mockImplementation((_clerk, params) => { + params.popup.location.href = params.externalVerificationRedirectURL.toString(); + return Promise.resolve(); + }); + + const signIn = new SignIn({ + id: 'signin_enterprise', + object: 'sign_in', + status: 'needs_first_factor', + first_factor_verification: { + status: 'unverified', + strategy: 'enterprise_sso', + external_verification_redirect_url: 'https://sso.example.com/auth/stale', + }, + } as any); + + const result = await signIn.__internal_future.sso({ + strategy: 'enterprise_sso', + redirectUrl: 'https://complete.example.com', + redirectCallbackUrl: '/sso-callback', + popup: mockPopup, + }); + + expect(result.error).toBeNull(); + expect(mockFetch).toHaveBeenNthCalledWith(1, { + method: 'POST', + path: '/client/sign_ins/signin_enterprise/prepare_first_factor', + body: expect.objectContaining({ + strategy: 'enterprise_sso', + }), + }); + expect(mockFetch).not.toHaveBeenCalledWith( + expect.objectContaining({ method: 'POST', path: '/client/sign_ins' }), + ); + expect(mockPopup.location.href).toBe('https://sso.example.com/auth/fresh'); + }); + it('handles relative redirectUrl by converting to absolute', async () => { vi.stubGlobal('window', { location: { origin: 'https://example.com' } }); @@ -2153,6 +2278,82 @@ describe('SignIn', () => { }); }); + it('creates a new OAuth sign-in when retrying after a previous provider redirect was abandoned', async () => { + vi.stubGlobal('window', { location: { origin: 'https://example.com' } }); + + const mockPopup = { location: { href: '' } } as Window; + const mockBuildUrlWithAuth = vi.fn().mockImplementation(url => { + if (url.startsWith('/')) { + return 'https://example.com' + url; + } + return url; + }); + + SignIn.clerk = { + buildUrlWithAuth: mockBuildUrlWithAuth, + buildUrl: vi.fn().mockImplementation(path => 'https://example.com' + path), + frontendApi: 'clerk.example.com', + __internal_environment: { + displayConfig: { + captchaOauthBypass: [], + }, + }, + } as any; + + const mockFetch = vi.fn(); + mockFetch.mockResolvedValueOnce({ + client: null, + response: { + id: 'signin_github', + first_factor_verification: { + status: 'unverified', + external_verification_redirect_url: 'https://github.com/login/oauth/authorize', + }, + }, + }); + mockFetch.mockResolvedValueOnce({ + client: null, + response: { + id: 'signin_github', + status: 'complete', + }, + }); + BaseResource._fetch = mockFetch; + + vi.mocked(_futureAuthenticateWithPopup).mockImplementation((_clerk, params) => { + params.popup.location.href = params.externalVerificationRedirectURL.toString(); + return Promise.resolve(); + }); + + const signIn = new SignIn({ + id: 'signin_google', + object: 'sign_in', + status: 'needs_first_factor', + first_factor_verification: { + status: 'unverified', + strategy: 'oauth_google', + external_verification_redirect_url: 'https://accounts.google.com/o/oauth2/auth', + }, + } as any); + + const result = await signIn.__internal_future.sso({ + strategy: 'oauth_github', + redirectUrl: 'https://complete.example.com', + redirectCallbackUrl: '/sso-callback', + popup: mockPopup, + }); + + expect(result.error).toBeNull(); + expect(mockFetch).toHaveBeenNthCalledWith(1, { + method: 'POST', + path: '/client/sign_ins', + body: expect.objectContaining({ + strategy: 'oauth_github', + }), + }); + expect(mockPopup.location.href).toBe('https://github.com/login/oauth/authorize'); + }); + it('uses popup when provided', async () => { vi.stubGlobal('window', { location: { origin: 'https://example.com' } }); @@ -2198,9 +2399,10 @@ describe('SignIn', () => { }); BaseResource._fetch = mockFetch; - vi.mocked(_futureAuthenticateWithPopup).mockImplementation(async (_clerk, params) => { + vi.mocked(_futureAuthenticateWithPopup).mockImplementation((_clerk, params) => { // Simulate the actual behavior of setting popup href params.popup.location.href = params.externalVerificationRedirectURL.toString(); + return Promise.resolve(); }); const signIn = new SignIn();