refactor(experience): fall back to sign-in page when error (#5673)

* refactor(experience): fall back to sign-in page for edge cases

* refactor: add unit tests

* refactor: fix tests

packages/experience/src/pages/SocialSignInWebCallback/index.test.tsx

@@ -1,5 +1,5 @@
 import { waitFor } from '@testing-library/react';
-import { Route, Routes, useSearchParams } from 'react-router-dom';
+import { Navigate, Route, Routes, useSearchParams } from 'react-router-dom';
 
 import renderWithPageContext from '@/__mocks__/RenderWithPageContext';
 import SettingsProvider from '@/__mocks__/RenderWithPageContext/SettingsProvider';
@@ -28,11 +28,33 @@ jest.mock('@/apis/single-sign-on', () => ({
 jest.mock('react-router-dom', () => ({
   ...jest.requireActual('react-router-dom'),
   useSearchParams: jest.fn(),
+  Navigate: jest.fn(() => null),
 }));
 
 const mockUseSearchParameters = useSearchParams as jest.Mock;
+const mockNavigate = Navigate as jest.Mock;
 
 describe('SocialCallbackPage with code', () => {
+  describe('fallback', () => {
+    it('should redirect to /sign-in if connectorId is not found', async () => {
+      mockUseSearchParameters.mockReturnValue([new URLSearchParams('code=foo'), jest.fn()]);
+
+      renderWithPageContext(
+        <SettingsProvider>
+          <Routes>
+            <Route path="/sign-in/social/:connectorId" element={<SocialCallback />} />
+          </Routes>
+        </SettingsProvider>,
+        { initialEntries: ['/sign-in/social/invalid'] }
+      );
+
+      await waitFor(() => {
+        expect(signInWithSocial).not.toBeCalled();
+        expect(mockNavigate.mock.calls[0][0].to).toBe('/sign-in');
+      });
+    });
+  });
+
   describe('signIn with social', () => {
     const connectorId = socialConnectors[0]!.id;
     const state = generateState();

packages/experience/src/pages/SocialSignInWebCallback/index.tsx

@@ -21,7 +21,7 @@ const SocialSignInWebCallback = () => {
   }
 
   // Connector not found, return sign in page
-  return <Navigate to={experience.routes.signIn} />;
+  return <Navigate to={'/' + experience.routes.signIn} />;
 };
 
 export default SocialSignInWebCallback;

packages/experience/src/pages/SocialSignInWebCallback/use-single-sign-on-listener.ts

@@ -1,7 +1,7 @@
-import { SignInMode } from '@logto/schemas';
+import { SignInMode, experience } from '@logto/schemas';
 import { useCallback, useEffect, useState } from 'react';
 import { useTranslation } from 'react-i18next';
-import { useSearchParams } from 'react-router-dom';
+import { useNavigate, useSearchParams } from 'react-router-dom';
 
 import { singleSignOnAuthorization, singleSignOnRegistration } from '@/apis/single-sign-on';
 import useApi from '@/hooks/use-api';
@@ -16,11 +16,13 @@ const useSingleSignOnRegister = () => {
   const handleError = useErrorHandler();
   const request = useApi(singleSignOnRegistration);
   const { termsValidation } = useTerms();
+  const navigate = useNavigate();
 
   return useCallback(
     async (connectorId: string) => {
       // Agree to terms and conditions first before proceeding
       if (!(await termsValidation())) {
+        navigate('/' + experience.routes.signIn);
         return;
       }
 
@@ -36,7 +38,7 @@ const useSingleSignOnRegister = () => {
         window.location.replace(result.redirectTo);
       }
     },
-    [handleError, request, termsValidation]
+    [handleError, navigate, request, termsValidation]
   );
 };
 
@@ -59,6 +61,7 @@ const useSingleSignOnListener = (connectorId: string) => {
   const { signInMode } = useSieMethods();
 
   const handleError = useErrorHandler();
+  const navigate = useNavigate();
 
   const singleSignOnAuthorizationRequest = useApi(singleSignOnAuthorization);
   const registerSingleSignOnIdentity = useSingleSignOnRegister();
@@ -78,7 +81,7 @@ const useSingleSignOnListener = (connectorId: string) => {
             // Should not let user register new social account under sign-in only mode
             if (signInMode === SignInMode.SignIn) {
               setToast(error.message);
-
+              navigate('/' + experience.routes.signIn);
               return;
             }
 
@@ -94,6 +97,7 @@ const useSingleSignOnListener = (connectorId: string) => {
     },
     [
       handleError,
+      navigate,
       registerSingleSignOnIdentity,
       setToast,
       signInMode,
@@ -117,6 +121,7 @@ const useSingleSignOnListener = (connectorId: string) => {
     // Validate the state parameter
     if (!state || !stateValidation(state, connectorId)) {
       setToast(t('error.invalid_connector_auth'));
+      navigate('/' + experience.routes.signIn);
       return;
     }
 
@@ -124,6 +129,7 @@ const useSingleSignOnListener = (connectorId: string) => {
   }, [
     connectorId,
     isConsumed,
+    navigate,
     searchParameters,
     setSearchParameters,
     setToast,

packages/experience/src/pages/SocialSignInWebCallback/use-social-sign-in-listener.ts

@@ -1,5 +1,5 @@
 import type { RequestErrorBody } from '@logto/schemas';
-import { SignInMode } from '@logto/schemas';
+import { SignInMode, experience } from '@logto/schemas';
 import { useCallback, useEffect, useMemo, useState } from 'react';
 import { useTranslation } from 'react-i18next';
 import { useNavigate, useSearchParams } from 'react-router-dom';
@@ -63,12 +63,13 @@ const useSocialSignInListener = (connectorId: string) => {
         // Should not let user register new social account under sign-in only mode
         if (signInMode === SignInMode.SignIn) {
           setToast(error.message);
-
+          navigate('/' + experience.routes.signIn);
           return;
         }
 
         // Agree to terms and conditions first before proceeding
         if (!(await termsValidation())) {
+          navigate('/' + experience.routes.signIn);
           return;
         }
 
@@ -76,7 +77,14 @@ const useSocialSignInListener = (connectorId: string) => {
       },
       ...preSignInErrorHandler,
     }),
-    [preSignInErrorHandler, signInMode, termsValidation, accountNotExistErrorHandler, setToast]
+    [
+      preSignInErrorHandler,
+      signInMode,
+      termsValidation,
+      accountNotExistErrorHandler,
+      setToast,
+      navigate,
+    ]
   );
 
   const signInWithSocialHandler = useCallback(
@@ -119,6 +127,7 @@ const useSocialSignInListener = (connectorId: string) => {
 
     if (!state || !stateValidation(state, connectorId)) {
       setToast(t('error.invalid_connector_auth'));
+      navigate('/' + experience.routes.signIn);
       return;
     }
 
@@ -126,6 +135,7 @@ const useSocialSignInListener = (connectorId: string) => {
   }, [
     connectorId,
     isConsumed,
+    navigate,
     searchParameters,
     setSearchParameters,
     setToast,

packages/integration-tests/src/tests/experience/direct-sign-in.test.ts

@@ -2,6 +2,7 @@ import crypto from 'node:crypto';
 
 import { ConnectorType } from '@logto/connector-kit';
 import { SignInIdentifier, SsoProviderName } from '@logto/schemas';
+import { appendPath } from '@silverhand/essentials';
 
 import { mockSocialConnectorTarget } from '#src/__mocks__/connectors-mock.js';
 import { updateSignInExperience } from '#src/api/sign-in-experience.js';
@@ -9,6 +10,7 @@ import { createSsoConnector } from '#src/api/sso-connector.js';
 import { demoAppUrl, logtoUrl } from '#src/constants.js';
 import { clearConnectorsByTypes, setSocialConnector } from '#src/helpers/connector.js';
 import ExpectExperience from '#src/ui-helpers/expect-experience.js';
+import { dcls, dmodal } from '#src/utils.js';
 
 const randomString = () => crypto.randomBytes(8).toString('hex');
 
@@ -40,6 +42,8 @@ describe('direct sign-in', () => {
     // eslint-disable-next-line @silverhand/fp/no-mutation
     context.ssoConnectorId = ssoConnector.id;
     await updateSignInExperience({
+      termsOfUseUrl: 'https://example.com/terms',
+      privacyPolicyUrl: 'https://example.com/privacy',
       signUp: { identifiers: [], password: true, verify: false },
       signIn: {
         methods: [
@@ -57,12 +61,26 @@ describe('direct sign-in', () => {
   });
 
   it('should be landed to the social identity provider directly', async () => {
+    const socialUserId = 'foo_' + randomString();
     const experience = new ExpectExperience(await browser.newPage());
     const url = new URL(demoAppUrl);
 
     url.searchParams.set('direct_sign_in', `social:${mockSocialConnectorTarget}`);
     await experience.page.goto(url.href);
-    await experience.toProcessSocialSignIn({ socialUserId: 'foo', clickButton: false });
+    await experience.toProcessSocialSignIn({
+      socialUserId,
+      clickButton: false,
+    });
+
+    // Redirected back to the social callback page
+    experience.toMatchUrl(new RegExp(appendPath(new URL(logtoUrl), 'callback/social/.*').href));
+
+    // Should have popped up the terms of use and privacy policy dialog
+    await experience.toMatchElement([dmodal(), dcls('content')].join(' '), {
+      text: /terms of use/i,
+    });
+    await experience.toClickButton(/agree/i);
+
     experience.toMatchUrl(demoAppUrl);
     await experience.toClick('div[role=button]', /sign out/i);
     await experience.page.close();
@@ -71,11 +89,12 @@ describe('direct sign-in', () => {
   it('should be landed to the sso identity provider directly', async () => {
     const experience = new ExpectExperience(await browser.newPage());
     const url = new URL(demoAppUrl);
+    const socialUserId = 'foo_' + randomString();
 
     url.searchParams.set('direct_sign_in', `sso:${context.ssoConnectorId!}`);
     await experience.page.goto(url.href);
     await experience.toProcessSocialSignIn({
-      socialUserId: 'foo',
+      socialUserId,
       clickButton: false,
       authUrl: ssoOidcIssuer + '/auth',
     });
@@ -84,7 +103,7 @@ describe('direct sign-in', () => {
     // with the code and user ID in the query string.
     const callbackUrl = new URL(experience.page.url());
     expect(callbackUrl.searchParams.get('code')).toBe('mock-code');
-    expect(callbackUrl.searchParams.get('userId')).toBe('foo');
+    expect(callbackUrl.searchParams.get('userId')).toBe(socialUserId);
     expect(new URL(callbackUrl.pathname, callbackUrl.origin).href).toBe(demoAppUrl.href);
 
     await experience.page.close();

packages/integration-tests/src/tests/experience/social-sign-in.test.ts

@@ -0,0 +1,163 @@
+import crypto from 'node:crypto';
+
+import { ConnectorType } from '@logto/connector-kit';
+import { SignInIdentifier, SignInMode, SsoProviderName } from '@logto/schemas';
+import { appendPath } from '@silverhand/essentials';
+
+import { updateSignInExperience } from '#src/api/sign-in-experience.js';
+import { createSsoConnector } from '#src/api/sso-connector.js';
+import { demoAppUrl, logtoUrl } from '#src/constants.js';
+import {
+  clearConnectorsByTypes,
+  setEmailConnector,
+  setSocialConnector,
+} from '#src/helpers/connector.js';
+import ExpectExperience from '#src/ui-helpers/expect-experience.js';
+import { dcls, dmodal, generateEmail } from '#src/utils.js';
+
+const randomString = () => crypto.randomBytes(8).toString('hex');
+
+/**
+ * NOTE: This test suite assumes test cases will run sequentially (which is Jest default).
+ * Parallel execution will lead to errors.
+ */
+// Tip: See https://github.com/argos-ci/jest-puppeteer/blob/main/packages/expect-puppeteer/README.md
+// for convenient expect methods
+describe('social sign-in (with email identifier)', () => {
+  const context = new (class Context {
+    ssoConnectorId?: string;
+  })();
+  const ssoOidcIssuer = `${logtoUrl}/oidc`;
+  // eslint-disable-next-line @silverhand/fp/no-let
+  let experience: ExpectExperience;
+  const socialUserId = 'foo_' + randomString();
+
+  beforeAll(async () => {
+    await clearConnectorsByTypes([ConnectorType.Social, ConnectorType.Email, ConnectorType.Sms]);
+    await setEmailConnector();
+    await setSocialConnector();
+    const ssoConnector = await createSsoConnector({
+      providerName: SsoProviderName.OIDC,
+      connectorName: 'test-oidc-' + randomString(),
+      domains: [`foo${randomString()}.com`],
+      config: {
+        clientId: 'foo',
+        clientSecret: 'bar',
+        issuer: ssoOidcIssuer,
+      },
+    });
+    // eslint-disable-next-line @silverhand/fp/no-mutation
+    context.ssoConnectorId = ssoConnector.id;
+    await updateSignInExperience({
+      termsOfUseUrl: 'https://example.com/terms',
+      privacyPolicyUrl: 'https://example.com/privacy',
+      signUp: { identifiers: [SignInIdentifier.Email], password: true, verify: true },
+      signIn: {
+        methods: [
+          {
+            identifier: SignInIdentifier.Username,
+            password: true,
+            verificationCode: false,
+            isPasswordPrimary: true,
+          },
+          {
+            identifier: SignInIdentifier.Email,
+            password: true,
+            verificationCode: false,
+            isPasswordPrimary: true,
+          },
+        ],
+      },
+      signInMode: SignInMode.SignIn,
+      singleSignOnEnabled: true,
+      socialSignInConnectorTargets: ['mock-social'],
+    });
+  });
+
+  it('should be able to start the social sign-in flow', async () => {
+    // eslint-disable-next-line @silverhand/fp/no-mutation
+    experience = new ExpectExperience(await browser.newPage());
+    await experience.startWith(demoAppUrl, 'sign-in');
+    await experience.toProcessSocialSignIn({
+      socialUserId,
+    });
+
+    // Registration disabled, should be redirected back to the sign-in page
+    await experience.waitForToast('not been registered yet');
+    experience.toBeAt('sign-in');
+  });
+
+  it('should be able to cancel (disagree) the terms of use', async () => {
+    await updateSignInExperience({ signInMode: SignInMode.SignInAndRegister });
+    await experience.toProcessSocialSignIn({
+      socialUserId,
+    });
+    // Redirected back to the social callback page
+    experience.toMatchUrl(new RegExp(appendPath(new URL(logtoUrl), 'callback/social/.*').href));
+
+    // Should have popped up the terms of use and privacy policy dialog
+    await experience.toMatchElement([dmodal(), dcls('content')].join(' '), {
+      text: /terms of use/i,
+    });
+    await experience.toClickButton(/cancel/i);
+    experience.toBeAt('sign-in');
+  });
+
+  it('should be able to start the social sign-in flow again if state mismatch', async () => {
+    await experience.toProcessSocialSignIn({
+      socialUserId,
+      state: '', // Overriding the state to cause a mismatch
+    });
+    await experience.waitForToast(/invalid/);
+    experience.toBeAt('sign-in');
+  });
+
+  it('should be able to start the social sign-in flow again', async () => {
+    await experience.toProcessSocialSignIn({
+      socialUserId,
+    });
+
+    // Redirected back to the social callback page
+    experience.toMatchUrl(new RegExp(appendPath(new URL(logtoUrl), 'callback/social/.*').href));
+
+    // Should have popped up the terms of use and privacy policy dialog
+    await experience.toMatchElement([dmodal(), dcls('content')].join(' '), {
+      text: /terms of use/i,
+    });
+    await experience.toClickButton(/agree/i);
+  });
+
+  it('should be able to verify the required email address', async () => {
+    await experience.toFillInput('identifier', generateEmail(), { submit: true });
+    await experience.toCompleteVerification('continue', 'Email');
+
+    await experience.verifyThenEnd();
+  });
+
+  it('should directly sign in for existing users without pop-up terms of use', async () => {
+    // eslint-disable-next-line @silverhand/fp/no-mutation
+    experience = new ExpectExperience(await browser.newPage());
+    await experience.startWith(demoAppUrl, 'sign-in');
+    await experience.toProcessSocialSignIn({
+      socialUserId,
+    });
+    await experience.verifyThenEnd();
+  });
+
+  it('should directly sign in for new users when terms of use has not been set', async () => {
+    await updateSignInExperience({
+      termsOfUseUrl: '',
+      privacyPolicyUrl: '',
+    });
+
+    // eslint-disable-next-line @silverhand/fp/no-mutation
+    experience = new ExpectExperience(await browser.newPage());
+    await experience.startWith(demoAppUrl, 'sign-in');
+    await experience.toProcessSocialSignIn({
+      socialUserId: 'bar_' + randomString(),
+    });
+    await experience.toFillInput('identifier', generateEmail(), { submit: true });
+    await experience.toCompleteVerification('continue', 'Email');
+    await experience.verifyThenEnd();
+  });
+});

packages/integration-tests/src/ui-helpers/expect-experience.ts

@@ -249,6 +249,7 @@ export default class ExpectExperience extends ExpectPage {
     socialPhone,
     clickButton = true,
     authUrl = mockSocialAuthPageUrl,
+    state: stateOverride,
   }: {
     socialUserId: string;
     socialEmail?: string;
@@ -257,6 +258,8 @@ export default class ExpectExperience extends ExpectPage {
     clickButton?: boolean;
     /** The URL to wait for the social auth page. */
     authUrl?: string;
+    /** The state parameter to override. */
+    state?: string;
   }) {
     const authPageRequestListener = this.page.waitForRequest((request) =>
       request.url().startsWith(authUrl)
@@ -270,7 +273,7 @@ export default class ExpectExperience extends ExpectPage {
 
     const { searchParams: authSearchParams } = new URL(result.url());
     const redirectUri = authSearchParams.get('redirect_uri') ?? '';
-    const state = authSearchParams.get('state') ?? '';
+    const state = stateOverride ?? authSearchParams.get('state') ?? '';
 
     // Mock social redirects
     const callbackUrl = new URL(redirectUri);

packages/integration-tests/src/ui-helpers/expect-page.ts

@@ -53,7 +53,7 @@ export default class ExpectPage {
    * @param shouldNavigate Whether the click should trigger a navigation. Defaults to `true`.
    * @see {@link ExpectPage.toClick}
    */
-  async toClickButton(text: string, shouldNavigate = true) {
+  async toClickButton(text: string | RegExp, shouldNavigate = true) {
     return this.toClick('button', text, shouldNavigate);
   }
 
@@ -136,6 +136,16 @@ export default class ExpectPage {
     return expect(this.page).toMatchElement('*[role=alert]', { text });
   }
 
+  /**
+   * Expect the page to match an element with the given selector and text.
+   *
+   * @alias `expect(this.page).toMatchElement()`
+   * @see {@link jest.Matchers.toMatchElement}
+   */
+  async toMatchElement(...args: Parameters<jest.Matchers<unknown>['toMatchElement']>) {
+    return expect(this.page).toMatchElement(...args);
+  }
+
   /**
    * Expect the page's URL to match the given URL.
    *

packages/integration-tests/src/ui-helpers/index.ts

@@ -160,6 +160,8 @@ export const setupUsernameAndEmailExperience = async (passwordPolicy?: PartialPa
   await clearConnectorsByTypes([ConnectorType.Email, ConnectorType.Sms]);
   await setEmailConnector();
   await updateSignInExperience({
+    termsOfUseUrl: '',
+    privacyPolicyUrl: '',
     signInMode: SignInMode.SignInAndRegister,
     signUp: {
       identifiers: [SignInIdentifier.Username],

packages/integration-tests/src/utils.ts

@@ -113,6 +113,9 @@ export const cls = <C extends string>(className: C) => `[class*=_${className}]`
  */
 export const dcls = <C extends string>(className: C) => `div${cls(className)}` as const;
 
+/** Build the string for a CSS selector that matches a `<div>` element with `aria-modal=true`. */
+export const dmodal = () => `div[aria-modal=true]` as const;
+
 /**
  * Generate a random test name that starts with `test_` and followed by 4 random characters.
  *