Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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;
}

Expand All @@ -36,7 +38,7 @@ const useSingleSignOnRegister = () => {
window.location.replace(result.redirectTo);
}
},
[handleError, request, termsValidation]
[handleError, navigate, request, termsValidation]
);
};

Expand All @@ -59,6 +61,7 @@ const useSingleSignOnListener = (connectorId: string) => {
const { signInMode } = useSieMethods();

const handleError = useErrorHandler();
const navigate = useNavigate();

const singleSignOnAuthorizationRequest = useApi(singleSignOnAuthorization);
const registerSingleSignOnIdentity = useSingleSignOnRegister();
Expand All @@ -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;
}

Expand All @@ -94,6 +97,7 @@ const useSingleSignOnListener = (connectorId: string) => {
},
[
handleError,
navigate,
registerSingleSignOnIdentity,
setToast,
signInMode,
Expand All @@ -117,13 +121,15 @@ const useSingleSignOnListener = (connectorId: string) => {
// Validate the state parameter
if (!state || !stateValidation(state, connectorId)) {
setToast(t('error.invalid_connector_auth'));
navigate('/' + experience.routes.signIn);
return;
}

void singleSignOnHandler(connectorId, rest);
}, [
connectorId,
isConsumed,
navigate,
searchParameters,
setSearchParameters,
setToast,
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -63,20 +63,28 @@ 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;
}

await accountNotExistErrorHandler(error);
},
...preSignInErrorHandler,
}),
[preSignInErrorHandler, signInMode, termsValidation, accountNotExistErrorHandler, setToast]
[
preSignInErrorHandler,
signInMode,
termsValidation,
accountNotExistErrorHandler,
setToast,
navigate,
]
);

const signInWithSocialHandler = useCallback(
Expand Down Expand Up @@ -119,13 +127,15 @@ const useSocialSignInListener = (connectorId: string) => {

if (!state || !stateValidation(state, connectorId)) {
setToast(t('error.invalid_connector_auth'));
navigate('/' + experience.routes.signIn);
return;
}

void signInWithSocialHandler(connectorId, rest);
}, [
connectorId,
isConsumed,
navigate,
searchParameters,
setSearchParameters,
setToast,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ 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';
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');

Expand Down Expand Up @@ -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: [
Expand All @@ -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();
Expand All @@ -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',
});
Expand All @@ -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();
Expand Down