From 381bb43a8cb71613074d027f93bbe027f243bd6e Mon Sep 17 00:00:00 2001 From: Jacek Date: Wed, 6 May 2026 15:35:36 -0500 Subject: [PATCH 01/17] fix: reset core 3 oauth retry state --- .changeset/curly-cameras-laugh.md | 5 ++ .../clerk-js/src/core/resources/SignIn.ts | 6 +- .../core/resources/__tests__/SignIn.test.ts | 79 ++++++++++++++++++- .../__tests__/runAsyncResourceTask.test.ts | 26 ++++++ .../src/utils/runAsyncResourceTask.ts | 24 +++++- 5 files changed, 134 insertions(+), 6 deletions(-) create mode 100644 .changeset/curly-cameras-laugh.md diff --git a/.changeset/curly-cameras-laugh.md b/.changeset/curly-cameras-laugh.md new file mode 100644 index 00000000000..8c3a345a3a9 --- /dev/null +++ b/.changeset/curly-cameras-laugh.md @@ -0,0 +1,5 @@ +--- +"@clerk/clerk-js": patch +--- + +Create a fresh Core 3 sign-in attempt when retrying OAuth SSO after an abandoned provider redirect, and reset async resource fetch status when restoring from BFCache. diff --git a/packages/clerk-js/src/core/resources/SignIn.ts b/packages/clerk-js/src/core/resources/SignIn.ts index f6540d06eb9..faa506dfbe2 100644 --- a/packages/clerk-js/src/core/resources/SignIn.ts +++ b/packages/clerk-js/src/core/resources/SignIn.ts @@ -1145,7 +1145,11 @@ class SignInFuture implements SignInFutureResource { routes.actionCompleteRedirectUrl = wrappedRoutes.redirectUrl; } - if (!this.#resource.id) { + if ( + !this.#resource.id || + strategy !== 'enterprise_sso' || + this.#resource.firstFactorVerification.status === 'unverified' + ) { 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..6421145ae23 100644 --- a/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts @@ -2153,6 +2153,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 +2274,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(); diff --git a/packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts b/packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts index 2fd83b89f76..bb7b121088f 100644 --- a/packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts +++ b/packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts @@ -62,4 +62,30 @@ describe('runAsyncTask', () => { status: 'idle', }); }); + + it('resets fetch status when a pending task is restored from bfcache', async () => { + const emitSpy = vi.spyOn(eventBus, 'emit'); + let resolveTask: () => void; + const task = vi.fn( + () => + new Promise(resolve => { + resolveTask = resolve; + }), + ); + + const pendingTask = runAsyncResourceTask(resource, task); + await Promise.resolve(); + + const pageShowEvent = new Event('pageshow') as PageTransitionEvent; + Object.defineProperty(pageShowEvent, 'persisted', { value: true }); + window.dispatchEvent(pageShowEvent); + + expect(emitSpy).toHaveBeenNthCalledWith(3, 'resource:fetch', { + resource, + status: 'idle', + }); + + resolveTask!(); + await pendingTask; + }); }); diff --git a/packages/clerk-js/src/utils/runAsyncResourceTask.ts b/packages/clerk-js/src/utils/runAsyncResourceTask.ts index 5a2200a8288..98558994939 100644 --- a/packages/clerk-js/src/utils/runAsyncResourceTask.ts +++ b/packages/clerk-js/src/utils/runAsyncResourceTask.ts @@ -11,6 +11,22 @@ export async function runAsyncResourceTask( resource: BaseResource, task: () => Promise, ): Promise<{ result?: T; error: ClerkError | null }> { + const resetFetchStatus = () => { + eventBus.emit('resource:fetch', { + resource, + status: 'idle', + }); + }; + const resetFetchStatusOnPageShow = (event: PageTransitionEvent) => { + if (event.persisted) { + resetFetchStatus(); + } + }; + + if (typeof window !== 'undefined' && typeof window.addEventListener === 'function') { + window.addEventListener('pageshow', resetFetchStatusOnPageShow); + } + eventBus.emit('resource:error', { resource, error: null }); eventBus.emit('resource:fetch', { resource, @@ -25,9 +41,9 @@ export async function runAsyncResourceTask( // TODO @userland-errors: return { error: err }; } finally { - eventBus.emit('resource:fetch', { - resource, - status: 'idle', - }); + if (typeof window !== 'undefined' && typeof window.removeEventListener === 'function') { + window.removeEventListener('pageshow', resetFetchStatusOnPageShow); + } + resetFetchStatus(); } } From ab7de706f2f4f68d820a4b76d0657bd1cf2d7813 Mon Sep 17 00:00:00 2001 From: Jacek Date: Wed, 6 May 2026 17:08:12 -0500 Subject: [PATCH 02/17] chore: tighten changeset wording --- .changeset/curly-cameras-laugh.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/curly-cameras-laugh.md b/.changeset/curly-cameras-laugh.md index 8c3a345a3a9..cbda5dafdbc 100644 --- a/.changeset/curly-cameras-laugh.md +++ b/.changeset/curly-cameras-laugh.md @@ -2,4 +2,4 @@ "@clerk/clerk-js": patch --- -Create a fresh Core 3 sign-in attempt when retrying OAuth SSO after an abandoned provider redirect, and reset async resource fetch status when restoring from BFCache. +Fix Core 3 OAuth retry routing to the previously selected provider after an abandoned redirect or BFCache restore. From 655ed7ec2657c571972285be7b9914bf35b1b182 Mon Sep 17 00:00:00 2001 From: Jacek Date: Wed, 6 May 2026 17:18:56 -0500 Subject: [PATCH 03/17] chore: scope oauth retry fix --- .changeset/curly-cameras-laugh.md | 2 +- .../__tests__/runAsyncResourceTask.test.ts | 26 ------------------- .../src/utils/runAsyncResourceTask.ts | 24 +++-------------- 3 files changed, 5 insertions(+), 47 deletions(-) diff --git a/.changeset/curly-cameras-laugh.md b/.changeset/curly-cameras-laugh.md index cbda5dafdbc..b7d34bc84ff 100644 --- a/.changeset/curly-cameras-laugh.md +++ b/.changeset/curly-cameras-laugh.md @@ -2,4 +2,4 @@ "@clerk/clerk-js": patch --- -Fix Core 3 OAuth retry routing to the previously selected provider after an abandoned redirect or BFCache restore. +Fix Core 3 OAuth retry routing to the previously selected provider after an abandoned redirect. diff --git a/packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts b/packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts index bb7b121088f..2fd83b89f76 100644 --- a/packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts +++ b/packages/clerk-js/src/utils/__tests__/runAsyncResourceTask.test.ts @@ -62,30 +62,4 @@ describe('runAsyncTask', () => { status: 'idle', }); }); - - it('resets fetch status when a pending task is restored from bfcache', async () => { - const emitSpy = vi.spyOn(eventBus, 'emit'); - let resolveTask: () => void; - const task = vi.fn( - () => - new Promise(resolve => { - resolveTask = resolve; - }), - ); - - const pendingTask = runAsyncResourceTask(resource, task); - await Promise.resolve(); - - const pageShowEvent = new Event('pageshow') as PageTransitionEvent; - Object.defineProperty(pageShowEvent, 'persisted', { value: true }); - window.dispatchEvent(pageShowEvent); - - expect(emitSpy).toHaveBeenNthCalledWith(3, 'resource:fetch', { - resource, - status: 'idle', - }); - - resolveTask!(); - await pendingTask; - }); }); diff --git a/packages/clerk-js/src/utils/runAsyncResourceTask.ts b/packages/clerk-js/src/utils/runAsyncResourceTask.ts index 98558994939..5a2200a8288 100644 --- a/packages/clerk-js/src/utils/runAsyncResourceTask.ts +++ b/packages/clerk-js/src/utils/runAsyncResourceTask.ts @@ -11,22 +11,6 @@ export async function runAsyncResourceTask( resource: BaseResource, task: () => Promise, ): Promise<{ result?: T; error: ClerkError | null }> { - const resetFetchStatus = () => { - eventBus.emit('resource:fetch', { - resource, - status: 'idle', - }); - }; - const resetFetchStatusOnPageShow = (event: PageTransitionEvent) => { - if (event.persisted) { - resetFetchStatus(); - } - }; - - if (typeof window !== 'undefined' && typeof window.addEventListener === 'function') { - window.addEventListener('pageshow', resetFetchStatusOnPageShow); - } - eventBus.emit('resource:error', { resource, error: null }); eventBus.emit('resource:fetch', { resource, @@ -41,9 +25,9 @@ export async function runAsyncResourceTask( // TODO @userland-errors: return { error: err }; } finally { - if (typeof window !== 'undefined' && typeof window.removeEventListener === 'function') { - window.removeEventListener('pageshow', resetFetchStatusOnPageShow); - } - resetFetchStatus(); + eventBus.emit('resource:fetch', { + resource, + status: 'idle', + }); } } From f9f078420a91392708b70c9933ebc10d251cae7a Mon Sep 17 00:00:00 2001 From: Jacek Date: Wed, 6 May 2026 17:46:23 -0500 Subject: [PATCH 04/17] chore: re-trigger ci for updated title From 0d62e94fe7ba3aaf39ddd66849e7723fef38fe3e Mon Sep 17 00:00:00 2001 From: Jacek Date: Thu, 7 May 2026 07:41:47 -0500 Subject: [PATCH 05/17] refactor: clarify sign in sso retry condition --- .../clerk-js/src/core/resources/SignIn.ts | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/clerk-js/src/core/resources/SignIn.ts b/packages/clerk-js/src/core/resources/SignIn.ts index faa506dfbe2..cfb8d826919 100644 --- a/packages/clerk-js/src/core/resources/SignIn.ts +++ b/packages/clerk-js/src/core/resources/SignIn.ts @@ -1145,11 +1145,25 @@ class SignInFuture implements SignInFutureResource { routes.actionCompleteRedirectUrl = wrappedRoutes.redirectUrl; } - if ( - !this.#resource.id || - strategy !== 'enterprise_sso' || - this.#resource.firstFactorVerification.status === 'unverified' - ) { + const hasExistingSignIn = Boolean(this.#resource.id); + const isEnterpriseSSO = strategy === 'enterprise_sso'; + const hasPendingFirstFactorVerification = this.#resource.firstFactorVerification.status === 'unverified'; + + const shouldStartNewSignIn = (() => { + if (!hasExistingSignIn) { + return true; + } + + if (!isEnterpriseSSO) { + return true; + } + + // Enterprise SSO can prepare a first factor on an existing sign-in, + // unless the current sign-in is still holding an abandoned redirect verification. + return hasPendingFirstFactorVerification; + })(); + + if (shouldStartNewSignIn) { await this._create({ strategy, ...routes, From d3bef865e6d7dccd964d0d763cb1937ae3d86782 Mon Sep 17 00:00:00 2001 From: Jacek Date: Thu, 7 May 2026 08:04:07 -0500 Subject: [PATCH 06/17] refactor: simplify sign in sso retry condition --- .../clerk-js/src/core/resources/SignIn.ts | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/packages/clerk-js/src/core/resources/SignIn.ts b/packages/clerk-js/src/core/resources/SignIn.ts index cfb8d826919..0ddbda22f75 100644 --- a/packages/clerk-js/src/core/resources/SignIn.ts +++ b/packages/clerk-js/src/core/resources/SignIn.ts @@ -1145,25 +1145,14 @@ class SignInFuture implements SignInFutureResource { routes.actionCompleteRedirectUrl = wrappedRoutes.redirectUrl; } - const hasExistingSignIn = Boolean(this.#resource.id); - const isEnterpriseSSO = strategy === 'enterprise_sso'; - const hasPendingFirstFactorVerification = this.#resource.firstFactorVerification.status === 'unverified'; - - const shouldStartNewSignIn = (() => { - if (!hasExistingSignIn) { - return true; - } - - if (!isEnterpriseSSO) { - return true; - } - - // Enterprise SSO can prepare a first factor on an existing sign-in, - // unless the current sign-in is still holding an abandoned redirect verification. - return hasPendingFirstFactorVerification; - })(); - - if (shouldStartNewSignIn) { + // Enterprise SSO can prepare a first factor on an existing sign-in, + // unless the current sign-in is still holding an abandoned redirect verification. + const canReuseExistingSignIn = + Boolean(this.#resource.id) && + strategy === 'enterprise_sso' && + this.#resource.firstFactorVerification.status !== 'unverified'; + + if (!canReuseExistingSignIn) { await this._create({ strategy, ...routes, From bf2254901041e4dfa965ceb9034c6d6b02518a1d Mon Sep 17 00:00:00 2001 From: Jacek Date: Thu, 7 May 2026 08:58:22 -0500 Subject: [PATCH 07/17] refactor: always create new sign-in for sso flow --- .../clerk-js/src/core/resources/SignIn.ts | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/packages/clerk-js/src/core/resources/SignIn.ts b/packages/clerk-js/src/core/resources/SignIn.ts index 0ddbda22f75..487ccb12f76 100644 --- a/packages/clerk-js/src/core/resources/SignIn.ts +++ b/packages/clerk-js/src/core/resources/SignIn.ts @@ -1145,20 +1145,11 @@ class SignInFuture implements SignInFutureResource { routes.actionCompleteRedirectUrl = wrappedRoutes.redirectUrl; } - // Enterprise SSO can prepare a first factor on an existing sign-in, - // unless the current sign-in is still holding an abandoned redirect verification. - const canReuseExistingSignIn = - Boolean(this.#resource.id) && - strategy === 'enterprise_sso' && - this.#resource.firstFactorVerification.status !== 'unverified'; - - if (!canReuseExistingSignIn) { - await this._create({ - strategy, - ...routes, - identifier, - }); - } + await this._create({ + strategy, + ...routes, + identifier, + }); if (strategy === 'enterprise_sso') { await this.#resource.__internal_basePost({ From 40261a6e99bd43dee08bc91ed99e24289eeb0c5d Mon Sep 17 00:00:00 2001 From: Jacek Date: Thu, 7 May 2026 10:46:00 -0500 Subject: [PATCH 08/17] test: rename sso test to reflect unconditional create --- packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6421145ae23..65f654c7443 100644 --- a/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts @@ -2153,7 +2153,7 @@ describe('SignIn', () => { }); }); - it('creates a new OAuth sign-in when retrying after a previous provider redirect was abandoned', async () => { + it("creates a fresh sign-in on every sso() call, even when a prior provider's redirect is still pending", async () => { vi.stubGlobal('window', { location: { origin: 'https://example.com' } }); const mockPopup = { location: { href: '' } } as Window; From daa5c4aae5a5dec6357bcabb5926c436e52ae3cd Mon Sep 17 00:00:00 2001 From: Jacek Date: Thu, 7 May 2026 11:31:42 -0500 Subject: [PATCH 09/17] fix: preserve enterprise sso sign-in reuse --- .../clerk-js/src/core/resources/SignIn.ts | 14 +++-- .../core/resources/__tests__/SignIn.test.ts | 55 ++++++++++++++++++- 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/packages/clerk-js/src/core/resources/SignIn.ts b/packages/clerk-js/src/core/resources/SignIn.ts index 487ccb12f76..e81632516ba 100644 --- a/packages/clerk-js/src/core/resources/SignIn.ts +++ b/packages/clerk-js/src/core/resources/SignIn.ts @@ -1145,11 +1145,15 @@ class SignInFuture implements SignInFutureResource { routes.actionCompleteRedirectUrl = wrappedRoutes.redirectUrl; } - await this._create({ - strategy, - ...routes, - identifier, - }); + const shouldCreateSignIn = !this.#resource.id || strategy !== 'enterprise_sso'; + + if (shouldCreateSignIn) { + await this._create({ + strategy, + ...routes, + identifier, + }); + } if (strategy === 'enterprise_sso') { await this.#resource.__internal_basePost({ 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 65f654c7443..d95b60343dc 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,59 @@ 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('handles relative redirectUrl by converting to absolute', async () => { vi.stubGlobal('window', { location: { origin: 'https://example.com' } }); @@ -2153,7 +2206,7 @@ describe('SignIn', () => { }); }); - it("creates a fresh sign-in on every sso() call, even when a prior provider's redirect is still pending", async () => { + 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; From e0526fd27a12c0f7893726b89642772c37d36f4a Mon Sep 17 00:00:00 2001 From: Jacek Date: Thu, 7 May 2026 11:44:17 -0500 Subject: [PATCH 10/17] test: cover enterprise sso retry after abandoned redirect --- .../clerk-js/src/core/resources/SignIn.ts | 3 + .../core/resources/__tests__/SignIn.test.ts | 72 +++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/packages/clerk-js/src/core/resources/SignIn.ts b/packages/clerk-js/src/core/resources/SignIn.ts index e81632516ba..76070ee456a 100644 --- a/packages/clerk-js/src/core/resources/SignIn.ts +++ b/packages/clerk-js/src/core/resources/SignIn.ts @@ -1145,6 +1145,9 @@ class SignInFuture implements SignInFutureResource { routes.actionCompleteRedirectUrl = wrappedRoutes.redirectUrl; } + // Enterprise SSO can be entered with a pre-existing sign-in (e.g. from a ticket + // or identifier-based discovery), in which case `prepare_first_factor` must run + // against that resource. All other strategies always start fresh. const shouldCreateSignIn = !this.#resource.id || strategy !== 'enterprise_sso'; if (shouldCreateSignIn) { 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 d95b60343dc..e49e369dc83 100644 --- a/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts @@ -2163,6 +2163,78 @@ describe('SignIn', () => { }); }); + 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' } }); From 129d610471fffb7fbe86c8e6f414686654daa586 Mon Sep 17 00:00:00 2001 From: Jacek Date: Fri, 8 May 2026 12:36:18 -0500 Subject: [PATCH 11/17] docs: explain why enterprise sso reuse is safe and oauth is not --- packages/clerk-js/src/core/resources/SignIn.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/clerk-js/src/core/resources/SignIn.ts b/packages/clerk-js/src/core/resources/SignIn.ts index 76070ee456a..924b930a49e 100644 --- a/packages/clerk-js/src/core/resources/SignIn.ts +++ b/packages/clerk-js/src/core/resources/SignIn.ts @@ -1145,9 +1145,11 @@ class SignInFuture implements SignInFutureResource { routes.actionCompleteRedirectUrl = wrappedRoutes.redirectUrl; } - // Enterprise SSO can be entered with a pre-existing sign-in (e.g. from a ticket - // or identifier-based discovery), in which case `prepare_first_factor` must run - // against that resource. All other strategies always start fresh. + // 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) { From 78cc8b8bfa2ee1ac374fcffd3359737d114a2043 Mon Sep 17 00:00:00 2001 From: Jacek Date: Fri, 8 May 2026 14:08:59 -0500 Subject: [PATCH 12/17] test(integration): cover oauth retry after abandoned redirect --- .../custom-flows-react-vite/src/main.tsx | 6 +- .../src/routes/SignIn.tsx | 31 ++++--- integration/tests/custom-flows/oauth.test.ts | 80 +++++++++++++++++++ 3 files changed, 105 insertions(+), 12 deletions(-) create mode 100644 integration/tests/custom-flows/oauth.test.ts diff --git a/integration/templates/custom-flows-react-vite/src/main.tsx b/integration/templates/custom-flows-react-vite/src/main.tsx index 33b3d38e758..29ffd5e20cf 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,10 @@ 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..081f232a64a 100644 --- a/integration/templates/custom-flows-react-vite/src/routes/SignIn.tsx +++ b/integration/templates/custom-flows-react-vite/src/routes/SignIn.tsx @@ -5,7 +5,7 @@ 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 { useClerk, useSignIn, useUser } from '@clerk/react'; import { useState } from 'react'; import { NavLink, useNavigate } from 'react-router'; @@ -16,10 +16,16 @@ 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 social = (clerk as any)?.__internal_environment?.userSettings?.social ?? {}; + const oauthProviders = Object.entries(social as Record) + .filter(([key, value]) => key.startsWith('oauth_') && value?.enabled) + .map(([, value]) => ({ strategy: value.strategy, name: value.name })); + + const handleOauth = async (strategy: string) => { await signIn.sso({ - strategy, + strategy: strategy as Parameters[0]['strategy'], redirectUrl: '/sso-callback', redirectUrlComplete: '/protected', }); @@ -268,14 +274,17 @@ export function SignIn({ className, ...props }: React.ComponentProps<'div'>) {
- + {oauthProviders.map(provider => ( + + ))}
diff --git a/integration/tests/custom-flows/oauth.test.ts b/integration/tests/custom-flows/oauth.test.ts new file mode 100644 index 00000000000..642429e32bc --- /dev/null +++ b/integration/tests/custom-flows/oauth.test.ts @@ -0,0 +1,80 @@ +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 }); + + 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 that creates the sign-in. + const firstPostPromise = page.waitForRequest( + req => req.method() === 'POST' && /\/v1\/client\/sign_ins(\?|$)/.test(req.url()), + ); + await oauthButton.first().click(); + await firstPostPromise; + + // Wait until we're on the OAuth provider's consent screen, then abandon via back navigation. + await u.page.getByText('Sign in to oauth-provider').waitFor(); + await u.page.goBack(); + await oauthButton.first().waitFor(); + + // Second attempt: must POST to /client/sign_ins again. If reuse logic kicked in incorrectly, + // the SignInFuture would skip create and silently no-op (status null, no redirect URL), + // so the absence of a second POST is exactly the SDK-75 regression we're guarding against. + 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 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(); + }); +}); From af50e36fa0662eb0b822079820db28c35e42131d Mon Sep 17 00:00:00 2001 From: Jacek Date: Fri, 8 May 2026 15:32:23 -0500 Subject: [PATCH 13/17] fix(integration): make custom-flows oauth buttons reactive and visible in retry state --- .../custom-flows-react-vite/src/main.tsx | 7 ++- .../src/routes/SignIn.tsx | 46 ++++++++++++------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/integration/templates/custom-flows-react-vite/src/main.tsx b/integration/templates/custom-flows-react-vite/src/main.tsx index 29ffd5e20cf..b133724280a 100644 --- a/integration/templates/custom-flows-react-vite/src/main.tsx +++ b/integration/templates/custom-flows-react-vite/src/main.tsx @@ -46,7 +46,12 @@ createRoot(document.getElementById('root')!).render( /> } + 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 081f232a64a..21c89aaa8cf 100644 --- a/integration/templates/custom-flows-react-vite/src/routes/SignIn.tsx +++ b/integration/templates/custom-flows-react-vite/src/routes/SignIn.tsx @@ -6,7 +6,7 @@ import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/com import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; import { useClerk, useSignIn, useUser } from '@clerk/react'; -import { useState } from 'react'; +import { useEffect, useState } from 'react'; import { NavLink, useNavigate } from 'react-router'; type AvailableStrategy = 'email_code' | 'phone_code' | 'password' | 'reset_password_email_code'; @@ -18,10 +18,17 @@ export function SignIn({ className, ...props }: React.ComponentProps<'div'>) { const navigate = useNavigate(); const clerk = useClerk(); - const social = (clerk as any)?.__internal_environment?.userSettings?.social ?? {}; - const oauthProviders = Object.entries(social as Record) - .filter(([key, value]) => key.startsWith('oauth_') && value?.enabled) - .map(([, value]) => ({ strategy: value.strategy, name: value.name })); + 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({ @@ -31,6 +38,22 @@ export function SignIn({ className, ...props }: React.ComponentProps<'div'>) { }); }; + const oauthButtons = ( + <> + {oauthProviders.map(provider => ( + + ))} + + ); + const handleSubmit = async (formData: FormData) => { const identifier = formData.get('identifier'); if (!identifier) { @@ -109,6 +132,7 @@ export function SignIn({ className, ...props }: React.ComponentProps<'div'>) {
+ {oauthButtons} {signIn.supportedFirstFactors .filter(({ strategy }) => strategy !== 'reset_password_email_code') .map(({ strategy }) => ( @@ -274,17 +298,7 @@ export function SignIn({ className, ...props }: React.ComponentProps<'div'>) {
- {oauthProviders.map(provider => ( - - ))} + {oauthButtons}
From 474dea0d8932d1f8126ef6686f6a9479c9505b18 Mon Sep 17 00:00:00 2001 From: Jacek Date: Fri, 8 May 2026 15:43:37 -0500 Subject: [PATCH 14/17] test(integration): abort oauth redirect to deterministically test sdk-75 retry --- integration/tests/custom-flows/oauth.test.ts | 32 +++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/integration/tests/custom-flows/oauth.test.ts b/integration/tests/custom-flows/oauth.test.ts index 642429e32bc..ab689be889a 100644 --- a/integration/tests/custom-flows/oauth.test.ts +++ b/integration/tests/custom-flows/oauth.test.ts @@ -40,27 +40,43 @@ test.describe('Custom Flows OAuth @custom', () => { 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 that creates the sign-in. + // 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; - // Wait until we're on the OAuth provider's consent screen, then abandon via back navigation. - await u.page.getByText('Sign in to oauth-provider').waitFor(); - await u.page.goBack(); + // 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 reuse logic kicked in incorrectly, - // the SignInFuture would skip create and silently no-op (status null, no redirect URL), - // so the absence of a second POST is exactly the SDK-75 regression we're guarding against. + // 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()), ); @@ -68,7 +84,7 @@ test.describe('Custom Flows OAuth @custom', () => { const secondPost = await secondPostPromise; expect(secondPost.method()).toBe('POST'); - // Complete the OAuth flow and assert we're signed in on the app instance. + // 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(); From 7a3b4ea3e49836edfe0941159e3f67c53eaea073 Mon Sep 17 00:00:00 2001 From: Jacek Date: Fri, 8 May 2026 15:54:55 -0500 Subject: [PATCH 15/17] fix(integration): use correct future-api redirect params for sso --- .../templates/custom-flows-react-vite/src/routes/SignIn.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 21c89aaa8cf..2cb1e08f061 100644 --- a/integration/templates/custom-flows-react-vite/src/routes/SignIn.tsx +++ b/integration/templates/custom-flows-react-vite/src/routes/SignIn.tsx @@ -33,8 +33,8 @@ export function SignIn({ className, ...props }: React.ComponentProps<'div'>) { const handleOauth = async (strategy: string) => { await signIn.sso({ strategy: strategy as Parameters[0]['strategy'], - redirectUrl: '/sso-callback', - redirectUrlComplete: '/protected', + redirectUrl: '/protected', + redirectCallbackUrl: '/sso-callback', }); }; From 55af2cebc26727dd8308a528b38b048773ef5ab1 Mon Sep 17 00:00:00 2001 From: Jacek Date: Mon, 1 Jun 2026 12:57:31 -0500 Subject: [PATCH 16/17] fix(clerk-js): only recreate SSO sign-in when switching providers Reuse the existing sign-in unless its pending verification would replay a stale redirect for a different strategy, so flows like ticket-then-OAuth keep their attached state instead of being discarded. --- .../clerk-js/src/core/resources/SignIn.ts | 20 +++++++---- .../core/resources/__tests__/SignIn.test.ts | 36 +++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/packages/clerk-js/src/core/resources/SignIn.ts b/packages/clerk-js/src/core/resources/SignIn.ts index 21f40d7906e..d41c9d44159 100644 --- a/packages/clerk-js/src/core/resources/SignIn.ts +++ b/packages/clerk-js/src/core/resources/SignIn.ts @@ -1145,12 +1145,20 @@ class SignInFuture implements SignInFutureResource { routes.actionCompleteRedirectUrl = wrappedRoutes.redirectUrl; } - // 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'; + // Reuse the existing sign-in by default so any state already attached to it carries + // into the SSO attempt (a ticket from `signIn.create({ strategy: 'ticket' })`, a + // discovered identifier, and so on). The one case we cannot reuse is when doing so + // would replay a stale provider redirect: an OAuth redirect URL only comes back from + // `_create` and cannot be refreshed in place, so if the resource already holds a + // pending verification for a different strategy than the one we're starting (e.g. + // `oauth_google` followed by `oauth_github`) we have to start fresh (SDK-75). + // `enterprise_sso` is always safe to reuse because the `prepare_first_factor` call + // below refreshes its redirect against the existing sign-in. + const { strategy: pendingStrategy, externalVerificationRedirectURL: pendingRedirectUrl } = + this.#resource.firstFactorVerification; + const wouldReplayStaleRedirect = + strategy !== 'enterprise_sso' && !!pendingRedirectUrl && pendingStrategy !== strategy; + const shouldCreateSignIn = !this.#resource.id || wouldReplayStaleRedirect; if (shouldCreateSignIn) { await this._create({ 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 d5d3b8ca510..c27188a0a5e 100644 --- a/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts @@ -2163,6 +2163,42 @@ describe('SignIn', () => { }); }); + it('reuses an existing ticket sign-in when starting OAuth rather than creating a new one', 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: 'oauth_google' }], + }, + }); + BaseResource._fetch = mockFetch; + + const signIn = new SignIn(); + await signIn.__internal_future.ticket({ ticket: 'ticket_123' }); + const result = await signIn.__internal_future.sso({ + strategy: 'oauth_google', + redirectUrl: 'https://complete.example.com', + redirectCallbackUrl: '/sso-callback', + }); + + // The ticket sign-in carries no conflicting OAuth verification, so the OAuth call + // reuses it instead of POSTing a fresh `/client/sign_ins` (which would drop the ticket). + expect(result.error).toBeNull(); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + 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' } }); From be8f21533395affa757a21845697cc3292072b89 Mon Sep 17 00:00:00 2001 From: Jacek Date: Mon, 1 Jun 2026 13:15:29 -0500 Subject: [PATCH 17/17] fix(clerk-js): force fresh SSO sign-in on any pending OAuth redirect The previous refinement only recreated when the pending OAuth strategy differed from the new one, which reused the stale redirect when retrying the same provider after an abandoned attempt and reintroduced SDK-75. Any lingering OAuth redirect is stale (it only comes from _create and can't be refreshed), so a pending redirect now always forces a fresh create; ticket/identifier flows have none and still reuse. --- .../clerk-js/src/core/resources/SignIn.ts | 18 ++--- .../core/resources/__tests__/SignIn.test.ts | 79 +++++++++++++++++++ 2 files changed, 87 insertions(+), 10 deletions(-) diff --git a/packages/clerk-js/src/core/resources/SignIn.ts b/packages/clerk-js/src/core/resources/SignIn.ts index d41c9d44159..3b19d06d56a 100644 --- a/packages/clerk-js/src/core/resources/SignIn.ts +++ b/packages/clerk-js/src/core/resources/SignIn.ts @@ -1146,18 +1146,16 @@ class SignInFuture implements SignInFutureResource { } // Reuse the existing sign-in by default so any state already attached to it carries - // into the SSO attempt (a ticket from `signIn.create({ strategy: 'ticket' })`, a - // discovered identifier, and so on). The one case we cannot reuse is when doing so - // would replay a stale provider redirect: an OAuth redirect URL only comes back from - // `_create` and cannot be refreshed in place, so if the resource already holds a - // pending verification for a different strategy than the one we're starting (e.g. - // `oauth_google` followed by `oauth_github`) we have to start fresh (SDK-75). + // into the SSO attempt (e.g. a ticket from `signIn.create({ strategy: 'ticket' })`, + // or a discovered identifier). OAuth is the exception: its redirect URL only comes + // back from `_create` and cannot be refreshed in place, so any redirect already + // lingering on the resource is stale. Reusing it would replay a previous attempt's + // redirect, including a retry of the same provider after an abandoned redirect + // (SDK-75), so whenever an OAuth call finds a pending redirect we start fresh. // `enterprise_sso` is always safe to reuse because the `prepare_first_factor` call // below refreshes its redirect against the existing sign-in. - const { strategy: pendingStrategy, externalVerificationRedirectURL: pendingRedirectUrl } = - this.#resource.firstFactorVerification; - const wouldReplayStaleRedirect = - strategy !== 'enterprise_sso' && !!pendingRedirectUrl && pendingStrategy !== strategy; + const hasPendingRedirect = !!this.#resource.firstFactorVerification.externalVerificationRedirectURL; + const wouldReplayStaleRedirect = strategy !== 'enterprise_sso' && hasPendingRedirect; const shouldCreateSignIn = !this.#resource.id || wouldReplayStaleRedirect; if (shouldCreateSignIn) { 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 c27188a0a5e..2ecd83eec75 100644 --- a/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/SignIn.test.ts @@ -2390,6 +2390,85 @@ describe('SignIn', () => { expect(mockPopup.location.href).toBe('https://github.com/login/oauth/authorize'); }); + it('creates a fresh OAuth sign-in when retrying the same provider after an abandoned redirect (SDK-75)', 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_google_2', + first_factor_verification: { + status: 'unverified', + external_verification_redirect_url: 'https://accounts.google.com/o/oauth2/auth?attempt=2', + }, + }, + }); + mockFetch.mockResolvedValueOnce({ + client: null, + response: { + id: 'signin_google_2', + status: 'complete', + }, + }); + BaseResource._fetch = mockFetch; + + vi.mocked(_futureAuthenticateWithPopup).mockImplementation((_clerk, params) => { + params.popup.location.href = params.externalVerificationRedirectURL.toString(); + return Promise.resolve(); + }); + + // Existing sign-in left over from a first, abandoned Google attempt (stale redirect). + const signIn = new SignIn({ + id: 'signin_google_1', + 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?attempt=1', + }, + } as any); + + const result = await signIn.__internal_future.sso({ + strategy: 'oauth_google', + redirectUrl: 'https://complete.example.com', + redirectCallbackUrl: '/sso-callback', + popup: mockPopup, + }); + + // Same provider, but the stale redirect must not be replayed: a fresh sign-in is + // POSTed and we navigate to the new redirect rather than the abandoned one. + expect(result.error).toBeNull(); + expect(mockFetch).toHaveBeenNthCalledWith(1, { + method: 'POST', + path: '/client/sign_ins', + body: expect.objectContaining({ + strategy: 'oauth_google', + }), + }); + expect(mockPopup.location.href).toBe('https://accounts.google.com/o/oauth2/auth?attempt=2'); + }); + it('uses popup when provided', async () => { vi.stubGlobal('window', { location: { origin: 'https://example.com' } });