fix(ui): continue combined-flow SSO callbacks to sign-in steps

.changeset/combined-flow-sso-callback-sign-in-steps.md

@@ -0,0 +1,5 @@
+---
+'@clerk/ui': patch
+---
+
+Fixed `<SignIn />` in the combined sign-in-or-up flow sending users back to the start after an OAuth or SAML redirect when the sign-in still needed a step, such as a Clerk Protect check, a second factor, or a password reset. Sign-ups created from an OAuth sign-in, including after a Protect check, now continue to their remaining steps inside the component.

packages/ui/src/components/SignIn/__tests__/SignInCombinedFlowSSOCallback.test.tsx

@@ -0,0 +1,143 @@
+import type { HandleOAuthCallbackParams, SignInResource } from '@clerk/shared/types';
+import { waitFor } from '@testing-library/react';
+import React from 'react';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+import { bindCreateFixtures } from '@/test/create-fixtures';
+import { render } from '@/test/utils';
+
+import { PathRouter } from '../../../router';
+import { SignIn } from '../index';
+
+vi.mock('@clerk/shared/internal/clerk-js/protectCheck', () => ({
+  executeProtectCheck: vi.fn(),
+}));
+
+import { executeProtectCheck } from '@clerk/shared/internal/clerk-js/protectCheck';
+
+const { createFixtures } = bindCreateFixtures('SignIn');
+
+const mockExecute = executeProtectCheck as unknown as ReturnType<typeof vi.fn>;
+
+const signInProtectCheckFallbackUrl = `${window.location.origin}/sign-in#/protect-check`;
+
+type Fixtures = Awaited<ReturnType<typeof createFixtures>>['fixtures'];
+
+const setup = async (opts: { pendingOAuthTransfer?: boolean } = {}) => {
+  const { wrapper, fixtures, props } = await createFixtures(f => {
+    f.withEmailAddress();
+    f.withSocialProvider({ provider: 'google' });
+    f.withPasskey();
+    f.withPasskeySettings({ allow_autofill: true, show_sign_in_button: false });
+    f.startSignInWithProtectCheck(
+      opts.pendingOAuthTransfer ? { pendingOAuthTransfer: true, status: 'needs_identifier' } : undefined,
+    );
+  });
+  props.setProps({ routing: 'path', path: '/sign-in', withSignUp: true } as any);
+
+  // @ts-expect-error - This is not a public API
+  fixtures.clerk.__internal_isWebAuthnAutofillSupported = () => Promise.resolve(true);
+  fixtures.signIn.authenticateWithPasskey.mockReturnValue(new Promise(() => {}));
+  vi.mocked(fixtures.clerk.navigate).mockImplementation((to: string) => {
+    const url = new URL(to, window.location.href);
+    if (url.origin === window.location.origin) {
+      window.history.pushState({}, '', url.href);
+    }
+    return Promise.resolve();
+  });
+  vi.mocked(fixtures.clerk.handleRedirectCallback).mockImplementation(
+    async (params: HandleOAuthCallbackParams, navigate?: (to: string) => Promise<unknown>) =>
+      navigate!(params.signInProtectCheckUrl || signInProtectCheckFallbackUrl),
+  );
+
+  return { wrapper, fixtures };
+};
+
+const renderAtCallback = (wrapper: React.FC<{ children: React.ReactNode }>) => {
+  window.history.replaceState({}, '', '/sign-in/create/sso-callback');
+  return render(
+    <React.Suspense fallback={null}>
+      <PathRouter basePath='/sign-in'>
+        <SignIn />
+      </PathRouter>
+    </React.Suspense>,
+    { wrapper },
+  );
+};
+
+const expectNoReplacementSignIn = (fixtures: Fixtures) => {
+  expect(fixtures.signIn.authenticateWithPasskey).not.toHaveBeenCalled();
+  expect(fixtures.signIn.create).not.toHaveBeenCalled();
+};
+
+describe('SignIn combined-flow SSO callback gated by a Protect check', () => {
+  beforeEach(() => {
+    mockExecute.mockReset();
+    mockExecute.mockResolvedValue('proof-abc');
+  });
+
+  it('runs the challenge for the signed-in OAuth attempt and activates the session', async () => {
+    const { wrapper, fixtures } = await setup();
+    fixtures.signIn.submitProtectCheck.mockResolvedValue({
+      status: 'complete',
+      protectCheck: null,
+      createdSessionId: 'sess_1',
+    } as unknown as SignInResource);
+
+    renderAtCallback(wrapper);
+
+    await waitFor(() => {
+      expect(fixtures.signIn.submitProtectCheck).toHaveBeenCalledWith({ proofToken: 'proof-abc' });
+    });
+    await waitFor(() => {
+      expect(fixtures.clerk.setActive).toHaveBeenCalledWith(expect.objectContaining({ session: 'sess_1' }));
+    });
+    expect(window.location.pathname).toBe('/sign-in/protect-check');
+    expectNoReplacementSignIn(fixtures);
+  });
+
+  it('continues to the second factor after the challenge', async () => {
+    const { wrapper, fixtures } = await setup();
+    fixtures.signIn.submitProtectCheck.mockResolvedValue({
+      status: 'needs_second_factor',
+      protectCheck: null,
+      createdSessionId: null,
+    } as unknown as SignInResource);
+
+    renderAtCallback(wrapper);
+
+    await waitFor(() => {
+      expect(fixtures.clerk.navigate).toHaveBeenCalledWith(
+        expect.stringMatching(/^\/sign-in\/factor-two/),
+        expect.anything(),
+      );
+    });
+    expect(fixtures.signIn.submitProtectCheck).toHaveBeenCalledWith({ proofToken: 'proof-abc' });
+    expectNoReplacementSignIn(fixtures);
+  });
+
+  it('continues an incomplete OAuth transfer inside the embedded sign-up routes', async () => {
+    const { wrapper, fixtures } = await setup({ pendingOAuthTransfer: true });
+    fixtures.signIn.submitProtectCheck.mockResolvedValue({
+      status: 'needs_identifier',
+      protectCheck: null,
+      createdSessionId: null,
+      firstFactorVerification: { status: 'transferable' },
+    } as unknown as SignInResource);
+    vi.mocked(fixtures.clerk.__internal_resumeAfterProtectCheck).mockImplementation(
+      async (params: HandleOAuthCallbackParams = {}, navigate?: (to: string) => Promise<unknown>) =>
+        navigate!(params.continueSignUpUrl!),
+    );
+
+    renderAtCallback(wrapper);
+
+    await waitFor(() => {
+      expect(fixtures.clerk.navigate).toHaveBeenCalledWith(
+        expect.stringMatching(/^\/sign-in\/create\/continue/),
+        expect.anything(),
+      );
+    });
+    expect(fixtures.signIn.submitProtectCheck).toHaveBeenCalledWith({ proofToken: 'proof-abc' });
+    expectNoReplacementSignIn(fixtures);
+  });
+});

packages/ui/src/components/SignIn/__tests__/combinedFlowSSOCallbackRouting.test.tsx

@@ -0,0 +1,200 @@
+import type { Clerk, HandleOAuthCallbackParams } from '@clerk/shared/types';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import React from 'react';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+import { HashRouter, PathRouter, Route, useRouter, VirtualRouter } from '../../../router';
+import { buildCombinedFlowOAuthCallbackParams, buildSignInOAuthCallbackParams } from '../buildOAuthCallbackParams';
+
+vi.mock('@clerk/shared/react', () => {
+  return {
+    useClerk: () => {
+      return {
+        navigate: () => Promise.resolve(),
+      } as unknown as Clerk;
+    },
+  };
+});
+
+const rootParams = buildSignInOAuthCallbackParams({
+  signUpUrl: '/sign-in#/create',
+  signInUrl: '/sign-in',
+  signUpContinueUrl: '/sign-in#/create/continue',
+  signUpProtectCheckUrl: '/sign-in#/create/protect-check',
+  isCombinedFlow: true,
+} as any);
+
+const createParams = buildCombinedFlowOAuthCallbackParams({
+  signUpUrl: '/sign-in#/create',
+  signInUrl: '/sign-in',
+  secondFactorUrl: '/sign-in#/factor-two',
+} as any);
+
+const destinations = {
+  signInProtectCheckUrl: 'protect-check',
+  firstFactorUrl: 'factor-one',
+  secondFactorUrl: 'factor-two',
+  resetPasswordUrl: 'reset-password',
+  continueSignUpUrl: 'create/continue',
+  verifyEmailAddressUrl: 'create/verify-email-address',
+  verifyPhoneNumberUrl: 'create/verify-phone-number',
+  signUpProtectCheckUrl: 'create/protect-check',
+} as const;
+
+type Destination = keyof typeof destinations;
+
+const starts: Array<[string, HandleOAuthCallbackParams]> = [
+  ['create/sso-callback', createParams],
+  ['sso-callback', rootParams],
+  ['protect-check', rootParams],
+];
+
+const cases = starts.flatMap(([start, params]) =>
+  (Object.keys(destinations) as Destination[])
+    .filter(key => !(start === destinations[key]))
+    .map(key => [start, key, destinations[key], params[key] as string] as const),
+);
+
+const Marker = ({ at }: { at: string }) => <p>{`at:${at}`}</p>;
+
+const NavigateButton = ({ to }: { to: string }) => {
+  const router = useRouter();
+  return (
+    <button
+      type='button'
+      onClick={() => void router.navigate(to)}
+    >
+      go
+    </button>
+  );
+};
+
+const SignInRoutes = ({ start, to }: { start: string; to: string }) => {
+  const button = (at: string) => (start === at ? <NavigateButton to={to} /> : null);
+  return (
+    <>
+      <Route path='factor-one'>
+        <Marker at='factor-one' />
+      </Route>
+      <Route path='factor-two'>
+        <Marker at='factor-two' />
+      </Route>
+      <Route path='reset-password'>
+        <Marker at='reset-password' />
+      </Route>
+      <Route path='protect-check'>
+        <Marker at='protect-check' />
+        {button('protect-check')}
+      </Route>
+      <Route path='sso-callback'>{button('sso-callback')}</Route>
+      <Route path='create'>
+        <Route path='continue'>
+          <Marker at='create/continue' />
+        </Route>
+        <Route path='verify-email-address'>
+          <Marker at='create/verify-email-address' />
+        </Route>
+        <Route path='verify-phone-number'>
+          <Marker at='create/verify-phone-number' />
+        </Route>
+        <Route path='protect-check'>
+          <Marker at='create/protect-check' />
+        </Route>
+        <Route path='sso-callback'>{button('create/sso-callback')}</Route>
+      </Route>
+    </>
+  );
+};
+
+const routers: Array<[string, (start: string, to: string) => React.ReactElement]> = [
+  [
+    'path routing mounted at the root',
+    (start, to) => {
+      window.history.replaceState({}, '', `/sign-in/${start}`);
+      return (
+        <PathRouter basePath='/sign-in'>
+          <SignInRoutes
+            start={start}
+            to={to}
+          />
+        </PathRouter>
+      );
+    },
+  ],
+  [
+    'path routing mounted at a nested path',
+    (start, to) => {
+      window.history.replaceState({}, '', `/auth/sign-in/${start}`);
+      return (
+        <PathRouter basePath='/auth/sign-in'>
+          <SignInRoutes
+            start={start}
+            to={to}
+          />
+        </PathRouter>
+      );
+    },
+  ],
+  [
+    'path routing reached with a trailing slash',
+    (start, to) => {
+      window.history.replaceState({}, '', `/sign-in/${start}/`);
+      return (
+        <PathRouter basePath='/sign-in'>
+          <SignInRoutes
+            start={start}
+            to={to}
+          />
+        </PathRouter>
+      );
+    },
+  ],
+  [
+    'hash routing',
+    (start, to) => {
+      window.history.replaceState({}, '', `/#/${start}`);
+      return (
+        <HashRouter>
+          <SignInRoutes
+            start={start}
+            to={to}
+          />
+        </HashRouter>
+      );
+    },
+  ],
+  [
+    'virtual routing',
+    (start, to) => {
+      window.history.replaceState({}, '', '/');
+      return (
+        <VirtualRouter startPath={`/sign-in/${start}`}>
+          <Route path='sign-in'>
+            <SignInRoutes
+              start={start}
+              to={to}
+            />
+          </Route>
+        </VirtualRouter>
+      );
+    },
+  ],
+];
+
+describe('combined-flow SSO callback and protect-check navigation', () => {
+  beforeEach(() => {
+    window.history.replaceState({}, '', '/');
+  });
+
+  describe.each(routers)('with %s', (_, renderRouter) => {
+    it.each(cases)('from %s, %s reaches the %s step', async (start, _key, destination, to) => {
+      render(renderRouter(start, to));
+      expect(screen.queryByText(`at:${destination}`)).not.toBeInTheDocument();
+
+      await userEvent.click(screen.getByRole('button', { name: 'go' }));
+
+      expect(await screen.findByText(`at:${destination}`)).toBeInTheDocument();
+    });
+  });
+});