From ab0651b69d00bc5eef2363ebf295b7493e79c777 Mon Sep 17 00:00:00 2001 From: Mateo Miccino Date: Thu, 7 Nov 2024 17:20:00 -0300 Subject: [PATCH] feat: add invalid redirection page, forward `targetConfigId` and add tests (#123) * feat: Add invalid redirection page * add tests, use navigationWithParams on all the places, forward targetConfigId * lint * apply review with tests format using describes * add redirectTo from review * remove targetConfigId from redirection.ts * change 'describe' in the tests to be more specific to the review --------- Co-authored-by: Lautaro Petaccio --- .../Pages/CallbackPage/CallbackPage.tsx | 24 +-- .../Pages/DefaultPage/DefaultPage.tsx | 10 +- .../InvalidRedirectionPage.module.css | 11 ++ .../InvalidRedirectionPage.tsx | 24 +++ .../Pages/InvalidRedirectionPage/index.ts | 1 + src/components/Pages/LoginPage/LoginPage.tsx | 24 +-- .../Pages/RequestPage/RequestPage.tsx | 14 +- src/components/Pages/SetupPage/SetupPage.tsx | 32 +--- src/hooks/navigation.spec.ts | 175 ++++++++++++++++++ src/hooks/navigation.ts | 53 ++++++ src/hooks/redirection.spec.ts | 47 +++-- src/hooks/redirection.ts | 38 +++- src/main.tsx | 2 + src/shared/locations.ts | 5 + 14 files changed, 370 insertions(+), 90 deletions(-) create mode 100644 src/components/Pages/InvalidRedirectionPage/InvalidRedirectionPage.module.css create mode 100644 src/components/Pages/InvalidRedirectionPage/InvalidRedirectionPage.tsx create mode 100644 src/components/Pages/InvalidRedirectionPage/index.ts create mode 100644 src/hooks/navigation.spec.ts create mode 100644 src/hooks/navigation.ts create mode 100644 src/shared/locations.ts diff --git a/src/components/Pages/CallbackPage/CallbackPage.tsx b/src/components/Pages/CallbackPage/CallbackPage.tsx index 51afc6d..368c56a 100644 --- a/src/components/Pages/CallbackPage/CallbackPage.tsx +++ b/src/components/Pages/CallbackPage/CallbackPage.tsx @@ -1,15 +1,16 @@ import { useCallback, useContext, useEffect, useState } from 'react' -import { useNavigate } from 'react-router-dom' import { ProviderType } from '@dcl/schemas' import { Button } from 'decentraland-ui/dist/components/Button/Button' import { Modal } from 'decentraland-ui/dist/components/Modal/Modal' import { getConfiguration, connection } from 'decentraland-connect' +import { useNavigateWithSearchParams } from '../../../hooks/navigation' import { useAfterLoginRedirection } from '../../../hooks/redirection' import { useTargetConfig } from '../../../hooks/targetConfig' import usePageTracking from '../../../hooks/usePageTracking' import { getAnalytics } from '../../../modules/analytics/segment' import { TrackingEvents } from '../../../modules/analytics/types' import { fetchProfile } from '../../../modules/profile' +import { locations } from '../../../shared/locations' import { wait } from '../../../shared/time' import { ConnectionModal, ConnectionModalState } from '../../ConnectionModal' import { FeatureFlagsContext } from '../../FeatureFlagsProvider' @@ -20,8 +21,8 @@ const MAGIC_KEY = getConfiguration().magic.apiKey export const CallbackPage = () => { usePageTracking() - const redirectTo = useAfterLoginRedirection() - const navigate = useNavigate() + const { url: redirectTo, redirect } = useAfterLoginRedirection() + const navigate = useNavigateWithSearchParams() const [state, setConnectionModalState] = useState(ConnectionModalState.WAITING_FOR_CONFIRMATION) const { flags } = useContext(FeatureFlagsContext) const [targetConfig] = useTargetConfig() @@ -50,7 +51,7 @@ export const CallbackPage = () => { setConnectionModalState(ConnectionModalState.WAITING_FOR_CONFIRMATION) } catch (error) { console.log(error) - navigate('/login') + navigate(locations.login()) } }, [navigate]) @@ -79,24 +80,17 @@ export const CallbackPage = () => { // If the connected account does not have a profile, redirect the user to the setup page to create a new one. // The setup page should then redirect the user to the url provided as query param if available. if (!profile) { - window.location.href = '/auth/setup' + (redirectTo ? `?redirectTo=${redirectTo}` : '') - return + return navigate(locations.setup(redirectTo)) } } } - if (redirectTo) { - // If redirection url is present, redirect the user to that url. - window.location.href = redirectTo - } else { - // Navigate to the landing page if there is no other place to redirect. - window.location.href = '/' - } + redirect() } catch (error) { console.log(error) - navigate('/login') + navigate(locations.login()) } - }, [navigate, redirectTo, flags]) + }, [navigate, redirectTo, redirect, flags]) if (state === ConnectionModalState.WAITING_FOR_CONFIRMATION) { return ( diff --git a/src/components/Pages/DefaultPage/DefaultPage.tsx b/src/components/Pages/DefaultPage/DefaultPage.tsx index 907d61a..7dae302 100644 --- a/src/components/Pages/DefaultPage/DefaultPage.tsx +++ b/src/components/Pages/DefaultPage/DefaultPage.tsx @@ -1,19 +1,19 @@ import { useEffect } from 'react' -import { useLocation, useNavigate } from 'react-router-dom' import { Loader } from 'decentraland-ui/dist/components/Loader/Loader' +import { useNavigateWithSearchParams } from '../../../hooks/navigation' import { getCurrentConnectionData } from '../../../shared/connection' +import { locations } from '../../../shared/locations' import styles from './DefaultPage.module.css' export const DefaultPage = () => { - const navigate = useNavigate() - const location = useLocation() + const navigate = useNavigateWithSearchParams() useEffect(() => { getCurrentConnectionData().then(connectionData => { if (connectionData) { - window.location.href = '/' + window.location.href = locations.home() } else { - navigate({ pathname: '/login', search: location.search }) + navigate(locations.login()) } }) }, [getCurrentConnectionData, navigate]) diff --git a/src/components/Pages/InvalidRedirectionPage/InvalidRedirectionPage.module.css b/src/components/Pages/InvalidRedirectionPage/InvalidRedirectionPage.module.css new file mode 100644 index 0000000..5c50d35 --- /dev/null +++ b/src/components/Pages/InvalidRedirectionPage/InvalidRedirectionPage.module.css @@ -0,0 +1,11 @@ +.warningImage { + height: 64px; + width: 64px; + margin-bottom: 24px; +} + +:global(.ui.modal > .content).content { + display: flex; + flex-direction: column; + align-items: center; +} diff --git a/src/components/Pages/InvalidRedirectionPage/InvalidRedirectionPage.tsx b/src/components/Pages/InvalidRedirectionPage/InvalidRedirectionPage.tsx new file mode 100644 index 0000000..a712f6e --- /dev/null +++ b/src/components/Pages/InvalidRedirectionPage/InvalidRedirectionPage.tsx @@ -0,0 +1,24 @@ +import { Link } from 'react-router-dom' +import { Button } from 'decentraland-ui/dist/components/Button/Button' +import { Modal } from 'decentraland-ui/dist/components/Modal/Modal' +import { ModalNavigation } from 'decentraland-ui/dist/components/ModalNavigation/ModalNavigation' +import warningSrc from '../../../assets/images/warning.svg' +import { locations } from '../../../shared/locations' +import style from './InvalidRedirectionPage.module.css' + +export const InvalidRedirectionPage = () => { + return ( + + + + +

The site you were redirected to is invalid.

+ + + + + + ) +} diff --git a/src/components/Pages/InvalidRedirectionPage/index.ts b/src/components/Pages/InvalidRedirectionPage/index.ts new file mode 100644 index 0000000..81bf8f9 --- /dev/null +++ b/src/components/Pages/InvalidRedirectionPage/index.ts @@ -0,0 +1 @@ +export * from './InvalidRedirectionPage' diff --git a/src/components/Pages/LoginPage/LoginPage.tsx b/src/components/Pages/LoginPage/LoginPage.tsx index d830240..8b0fa2f 100644 --- a/src/components/Pages/LoginPage/LoginPage.tsx +++ b/src/components/Pages/LoginPage/LoginPage.tsx @@ -1,5 +1,4 @@ import { useState, useCallback, useMemo, useEffect, useContext } from 'react' -import { useSearchParams } from 'react-router-dom' import Image1 from '../../../assets/images/background/image1.webp' import Image10 from '../../../assets/images/background/image10.webp' import Image2 from '../../../assets/images/background/image2.webp' @@ -10,6 +9,7 @@ import Image6 from '../../../assets/images/background/image6.webp' import Image7 from '../../../assets/images/background/image7.webp' import Image8 from '../../../assets/images/background/image8.webp' import Image9 from '../../../assets/images/background/image9.webp' +import { useNavigateWithSearchParams } from '../../../hooks/navigation' import { useAfterLoginRedirection } from '../../../hooks/redirection' import { useTargetConfig } from '../../../hooks/targetConfig' import usePageTracking from '../../../hooks/usePageTracking' @@ -17,6 +17,7 @@ import { getAnalytics } from '../../../modules/analytics/segment' import { ClickEvents, ConnectionType, TrackingEvents } from '../../../modules/analytics/types' import { fetchProfile } from '../../../modules/profile' import { isErrorWithMessage, isErrorWithName } from '../../../shared/errors' +import { locations } from '../../../shared/locations' import { wait } from '../../../shared/time' import { Connection, ConnectionOptionType } from '../../Connection' import { ConnectionModal, ConnectionModalState } from '../../ConnectionModal' @@ -30,14 +31,14 @@ const BACKGROUND_IMAGES = [Image1, Image2, Image3, Image4, Image5, Image6, Image export const LoginPage = () => { usePageTracking() - const [searchParams] = useSearchParams() + const navigate = useNavigateWithSearchParams() const [connectionModalState, setConnectionModalState] = useState(ConnectionModalState.CONNECTING_WALLET) const [showLearnMore, setShowLearnMore] = useState(false) const [showMagicLearnMore, setShowMagicLearnMore] = useState(false) const [showConnectionModal, setShowConnectionModal] = useState(false) const [currentConnectionType, setCurrentConnectionType] = useState() const [isMobile] = useState(getIsMobile()) - const redirectTo = useAfterLoginRedirection() + const { url: redirectTo, redirect } = useAfterLoginRedirection() const showGuestOption = redirectTo && new URL(redirectTo).pathname.includes('/play') const [currentBackgroundIndex, setCurrentBackgroundIndex] = useState(0) const { flags } = useContext(FeatureFlagsContext) @@ -126,22 +127,13 @@ export const LoginPage = () => { // If the connected account does not have a profile, redirect the user to the setup page to create a new one. // The setup page should then redirect the user to the url provided as query param if available. if (!profile) { - window.location.href = '/auth/setup' + (redirectTo ? `?redirectTo=${redirectTo}` : '') - setShowConnectionModal(false) - return + navigate(locations.setup(redirectTo)) + return setShowConnectionModal(false) } } } - if (redirectTo) { - // If a redirection url was provided in the query params, redirect the user to that url. - window.location.href = redirectTo - } else { - // Redirect the user to the root url if there is no other place to redirect. - // TODO: Maybe we should add something to the root page, or simply redirect to the profile app. - window.location.href = '/' - } - + redirect() setShowConnectionModal(false) } catch (error) { console.log('Error', isErrorWithMessage(error) ? error.message : JSON.stringify(error)) @@ -154,7 +146,7 @@ export const LoginPage = () => { } } }, - [setConnectionModalState, setShowConnectionModal, setCurrentConnectionType, redirectTo, searchParams, flags] + [setConnectionModalState, setShowConnectionModal, setCurrentConnectionType, redirectTo, navigate, redirect, flags] ) const handleOnCloseConnectionModal = useCallback(() => { diff --git a/src/components/Pages/RequestPage/RequestPage.tsx b/src/components/Pages/RequestPage/RequestPage.tsx index 48ed74b..fefef14 100644 --- a/src/components/Pages/RequestPage/RequestPage.tsx +++ b/src/components/Pages/RequestPage/RequestPage.tsx @@ -1,5 +1,5 @@ import { ReactNode, useCallback, useEffect, useRef, useState } from 'react' -import { useNavigate, useParams } from 'react-router-dom' +import { useParams } from 'react-router-dom' import { Profile } from 'dcl-catalyst-client/dist/client/specs/catalyst.schemas' import { ethers, BrowserProvider } from 'ethers' import Icon from 'semantic-ui-react/dist/commonjs/elements/Icon/Icon' @@ -8,6 +8,7 @@ import { Button } from 'decentraland-ui/dist/components/Button/Button' import { CommunityBubble } from 'decentraland-ui/dist/components/CommunityBubble' import { Loader } from 'decentraland-ui/dist/components/Loader/Loader' import { connection } from 'decentraland-connect' +import { useNavigateWithSearchParams } from '../../../hooks/navigation' import { useTargetConfig } from '../../../hooks/targetConfig' import usePageTracking from '../../../hooks/usePageTracking' import { getAnalytics } from '../../../modules/analytics/segment' @@ -16,6 +17,7 @@ import { config } from '../../../modules/config' import { fetchProfile } from '../../../modules/profile' import { getCurrentConnectionData } from '../../../shared/connection' import { isErrorWithMessage, isRpcError } from '../../../shared/errors' +import { locations } from '../../../shared/locations' import { CustomWearablePreview } from '../../CustomWearablePreview' import styles from './RequestPage.module.css' @@ -40,7 +42,7 @@ enum View { export const RequestPage = () => { usePageTracking() const params = useParams() - const navigate = useNavigate() + const navigate = useNavigateWithSearchParams() const providerRef = useRef() const [view, setView] = useState(View.LOADING_REQUEST) const [isLoading, setIsLoading] = useState(false) @@ -56,12 +58,12 @@ export const RequestPage = () => { // Goes to the login page where the user will have to connect a wallet. const toLoginPage = useCallback(() => { - navigate(`/login?redirectTo=${encodeURIComponent(`/auth/requests/${requestId}?targetConfigId=${targetConfigId}`)}`) - }, [requestId, targetConfigId]) + navigate(locations.login(`/auth/requests/${requestId}?targetConfigId=${targetConfigId}`)) + }, [requestId]) const toSetupPage = useCallback(() => { - navigate(`/setup?redirectTo=${encodeURIComponent(`/auth/requests/${requestId}?targetConfigId=${targetConfigId}`)}`) - }, [requestId, targetConfigId]) + navigate(locations.setup(`/auth/requests/${requestId}?targetConfigId=${targetConfigId}`)) + }, [requestId]) useEffect(() => { ;(async () => { diff --git a/src/components/Pages/SetupPage/SetupPage.tsx b/src/components/Pages/SetupPage/SetupPage.tsx index d8aabc9..3f2589a 100644 --- a/src/components/Pages/SetupPage/SetupPage.tsx +++ b/src/components/Pages/SetupPage/SetupPage.tsx @@ -11,11 +11,13 @@ import backImg from '../../../assets/images/back.svg' import diceImg from '../../../assets/images/dice.svg' import logoImg from '../../../assets/images/logo.svg' import wrongImg from '../../../assets/images/wrong.svg' +import { useNavigateWithSearchParams } from '../../../hooks/navigation' import { useAfterLoginRedirection } from '../../../hooks/redirection' import { getAnalytics } from '../../../modules/analytics/segment' import { ClickEvents, TrackingEvents } from '../../../modules/analytics/types' import { fetchProfile } from '../../../modules/profile' import { getCurrentConnectionData } from '../../../shared/connection' +import { locations } from '../../../shared/locations' import { CustomWearablePreview } from '../../CustomWearablePreview' import { FeatureFlagsContext } from '../../FeatureFlagsProvider' import { deployProfileFromDefault, subscribeToNewsletter } from './utils' @@ -40,6 +42,7 @@ const ErrorMessage = (props: { message: string; className?: string }) => { } export const SetupPage = () => { + const navigate = useNavigateWithSearchParams() const [initialized, setInitialized] = useState(false) const [view, setView] = useState(View.RANDOMIZE) const [profile, setProfile] = useState(getRandomDefaultProfile()) @@ -54,7 +57,7 @@ export const SetupPage = () => { const { initialized: initializedFlags } = useContext(FeatureFlagsContext) - const redirectTo = useAfterLoginRedirection() + const { url: redirectTo, redirect } = useAfterLoginRedirection() // Validate the name. const nameError = useMemo(() => { @@ -205,11 +208,7 @@ export const SetupPage = () => { }) // Redirect to the site defined in the search params. - if (redirectTo) { - window.location.href = redirectTo - } else { - window.location.href = '/' - } + redirect() } catch (e) { setDeploying(false) @@ -217,17 +216,13 @@ export const SetupPage = () => { console.error('There was an error deploying the profile', (e as Error).message) } }, - [nameError, emailError, agreeError, name, email, agree, profile, redirectTo] + [nameError, emailError, agreeError, name, email, agree, profile, redirect] ) // Initialization effect. // Will run some checks to see if the user can proceed with the simplified avatar setup flow. useEffect(() => { ;(async () => { - const toLogin = () => { - window.location.href = `/auth/login${redirectTo ? `?redirectTo=${redirectTo}` : ''}` - } - // Check if the wallet is connected. try { const connectionData = await getCurrentConnectionData() @@ -238,8 +233,7 @@ export const SetupPage = () => { identityRef.current = connectionData.identity } catch (e) { console.warn('No previous connection found') - toLogin() - return + return navigate(locations.login(redirectTo)) } const profile = await fetchProfile(accountRef.current) @@ -247,20 +241,12 @@ export const SetupPage = () => { // Check that the connected account does not have a profile already. if (profile) { console.warn('Profile already exists') - - if (redirectTo) { - console.log('Redirecting to', redirectTo) - window.location.href = redirectTo - } else { - window.location.href = '/' - } - - return + return redirect() } setInitialized(true) })() - }, [redirectTo]) + }, [redirect, navigate]) if (!initialized || !initializedFlags) { return ( diff --git a/src/hooks/navigation.spec.ts b/src/hooks/navigation.spec.ts new file mode 100644 index 0000000..a5acb2c --- /dev/null +++ b/src/hooks/navigation.spec.ts @@ -0,0 +1,175 @@ +import { useNavigate, useLocation } from 'react-router-dom' +import { renderHook, act } from '@testing-library/react-hooks' +import { useNavigateWithSearchParams } from './navigation' + +jest.mock('react-router-dom') + +describe('useNavigateWithSearchParams', () => { + const mockNavigate = jest.fn() + const mockLocation = { search: '' } + + beforeEach(() => { + jest.clearAllMocks() + ;(useNavigate as jest.Mock).mockReturnValue(mockNavigate) + ;(useLocation as jest.Mock).mockReturnValue(mockLocation) + }) + + describe('when targetConfigId parameter is not present', () => { + beforeEach(() => { + ;(useLocation as jest.Mock).mockReturnValue({ search: '' }) + }) + + it('should navigate to the given path without modifying search params', () => { + const { result } = renderHook(() => useNavigateWithSearchParams()) + + act(() => { + result.current('/test-path') + }) + + expect(mockNavigate).toHaveBeenCalledWith({ + pathname: '/test-path', + search: undefined + }) + }) + + it('should navigate to the given path with its own search params', () => { + const { result } = renderHook(() => useNavigateWithSearchParams()) + + act(() => { + result.current('/test-path?param1=value1¶m2=value2') + }) + + expect(mockNavigate).toHaveBeenCalledWith({ + pathname: '/test-path', + search: '?param1=value1¶m2=value2' + }) + }) + }) + + describe('when targetConfigId parameter is present', () => { + beforeEach(() => { + ;(useLocation as jest.Mock).mockReturnValue({ search: '?targetConfigId=123' }) + }) + + it('should append targetConfigId to the given path', () => { + const { result } = renderHook(() => useNavigateWithSearchParams()) + + act(() => { + result.current('/test-path') + }) + + expect(mockNavigate).toHaveBeenCalledWith({ + pathname: '/test-path', + search: '?targetConfigId=123' + }) + }) + + it('should merge targetConfigId with existing search params', () => { + const { result } = renderHook(() => useNavigateWithSearchParams()) + + act(() => { + result.current('/test-path?param1=value1') + }) + + expect(mockNavigate).toHaveBeenCalledWith({ + pathname: '/test-path', + search: '?param1=value1&targetConfigId=123' + }) + }) + + it('should overwrite targetConfigId in the path with the one from current location', () => { + const { result } = renderHook(() => useNavigateWithSearchParams()) + + act(() => { + result.current('/test-path?param1=value1&targetConfigId=456') + }) + + expect(mockNavigate).toHaveBeenCalledWith({ + pathname: '/test-path', + search: '?param1=value1&targetConfigId=123' + }) + }) + + it('should correctly merge multiple search params with targetConfigId', () => { + const { result } = renderHook(() => useNavigateWithSearchParams()) + + act(() => { + result.current('/test-path?param1=value1¶m2=value2') + }) + + expect(mockNavigate).toHaveBeenCalledWith({ + pathname: '/test-path', + search: '?param1=value1¶m2=value2&targetConfigId=123' + }) + }) + }) + + describe('when navigating with different path formats', () => { + it('should handle full URLs with different origins', () => { + const { result } = renderHook(() => useNavigateWithSearchParams()) + + act(() => { + result.current('http://example.com/test-path?param1=value1') + }) + + expect(mockNavigate).toHaveBeenCalledWith({ + pathname: '/test-path', + search: '?param1=value1' + }) + }) + + it('should navigate correctly with a relative path', () => { + const { result } = renderHook(() => useNavigateWithSearchParams()) + + act(() => { + result.current('test-path') + }) + + expect(mockNavigate).toHaveBeenCalledWith({ + pathname: '/test-path', + search: undefined + }) + }) + }) + + describe('when targetConfigId parameter has special characters', () => { + beforeEach(() => { + ;(useLocation as jest.Mock).mockReturnValue({ search: '?targetConfigId=%2F%2Fmalicious.com' }) + }) + + it('should encode targetConfigId with special characters', () => { + const { result } = renderHook(() => useNavigateWithSearchParams()) + + act(() => { + result.current('/test-path') + }) + + expect(mockNavigate).toHaveBeenCalledWith({ + pathname: '/test-path', + search: '?targetConfigId=%2F%2Fmalicious.com' + }) + }) + }) + + describe('when handling errors in navigation', () => { + it('should throw an error when provided path is malformed', () => { + const { result } = renderHook(() => useNavigateWithSearchParams()) + + expect(() => { + act(() => { + result.current('http://') + }) + }).toThrow('Invalid path provided') + }) + + it('should throw an error when navigate is called with an empty path', () => { + const { result } = renderHook(() => useNavigateWithSearchParams()) + + expect(() => { + act(() => { + result.current('') + }) + }).toThrow('Path cannot be empty') + }) + }) +}) diff --git a/src/hooks/navigation.ts b/src/hooks/navigation.ts new file mode 100644 index 0000000..be5998e --- /dev/null +++ b/src/hooks/navigation.ts @@ -0,0 +1,53 @@ +import { useCallback } from 'react' +import { useNavigate, useLocation } from 'react-router-dom' + +export const useNavigateWithSearchParams = () => { + const navigate = useNavigate() + const location = useLocation() + + const enhancedNavigation = useCallback( + (path: string) => { + if (!path) { + throw new Error('Path cannot be empty') + } + + let urlFromPath: URL + try { + urlFromPath = new URL(path, window.location.origin) + + // Additional validation: Check if pathname is valid + if (!urlFromPath.pathname) { + throw new Error('Invalid path provided') + } + } catch (error) { + throw new Error('Invalid path provided') + } + + const pathname = urlFromPath.pathname + + // Extract search parameters from the provided path + const searchParams = new URLSearchParams(urlFromPath.search) + + // Get 'targetConfigId' from the current location's search parameters + const currentSearchParams = new URLSearchParams(location.search) + const targetConfigId = currentSearchParams.get('targetConfigId') + + // If 'targetConfigId' exists, add it to the search parameters + if (targetConfigId) { + searchParams.set('targetConfigId', targetConfigId) + } + + // Convert search parameters back to a string + const searchString = searchParams.toString() + + // Navigate to the new path with the updated search parameters + navigate({ + pathname, + search: searchString ? `?${searchString}` : undefined + }) + }, + [navigate, location] + ) + + return enhancedNavigation +} diff --git a/src/hooks/redirection.spec.ts b/src/hooks/redirection.spec.ts index 151ea26..695ef48 100644 --- a/src/hooks/redirection.spec.ts +++ b/src/hooks/redirection.spec.ts @@ -16,9 +16,9 @@ describe('when using the redirection hook', () => { mockedUseLocation.mockReturnValue({ search: '' } as Location) }) - it('should return undefined', () => { + it('should return the default site', () => { const { result } = renderHook(() => useAfterLoginRedirection()) - expect(result.current).toBeUndefined() + expect(result.current.url).toBe('http://localhost/') }) }) @@ -27,9 +27,9 @@ describe('when using the redirection hook', () => { mockedUseLocation.mockReturnValue({ search: 'redirectTo=https://test.com' } as Location) }) - it('should return undefined', () => { + it('should return the invalid redirection URL', () => { const { result } = renderHook(() => useAfterLoginRedirection()) - expect(result.current).toBeUndefined() + expect(result.current.url).toBe('http://localhost/auth/invalidRedirection') }) }) @@ -40,7 +40,7 @@ describe('when using the redirection hook', () => { it('should return the local path using the current domain', () => { const { result } = renderHook(() => useAfterLoginRedirection()) - expect(result.current).toBe('http://localhost/test') + expect(result.current.url).toBe('http://localhost/test') }) }) @@ -51,7 +51,7 @@ describe('when using the redirection hook', () => { it('should return the local path using the current domain', () => { const { result } = renderHook(() => useAfterLoginRedirection()) - expect(result.current).toBe('http://localhost/test') + expect(result.current.url).toBe('http://localhost/test') }) }) @@ -60,9 +60,9 @@ describe('when using the redirection hook', () => { mockedUseLocation.mockReturnValue({ search: 'redirectTo=//test.com' } as Location) }) - it('should return undefined', () => { + it('should return the invalid redirection URL', () => { const { result } = renderHook(() => useAfterLoginRedirection()) - expect(result.current).toBeUndefined() + expect(result.current.url).toBe('http://localhost/auth/invalidRedirection') }) }) @@ -71,9 +71,9 @@ describe('when using the redirection hook', () => { mockedUseLocation.mockReturnValue({ search: 'redirectTo=%2F%2Ftest.com' } as Location) }) - it('should return undefined', () => { + it('should return the invalid redirection URL', () => { const { result } = renderHook(() => useAfterLoginRedirection()) - expect(result.current).toBeUndefined() + expect(result.current.url).toBe('http://localhost/auth/invalidRedirection') }) }) @@ -82,9 +82,9 @@ describe('when using the redirection hook', () => { mockedUseLocation.mockReturnValue({ search: 'redirectTo=%252F%252Ftest.com' } as Location) }) - it('should return undefined', () => { + it('should return the invalid redirection URL', () => { const { result } = renderHook(() => useAfterLoginRedirection()) - expect(result.current).toBeUndefined() + expect(result.current.url).toBe('http://localhost/auth/invalidRedirection') }) }) @@ -95,7 +95,7 @@ describe('when using the redirection hook', () => { it('should return the local path using the current domain', () => { const { result } = renderHook(() => useAfterLoginRedirection()) - expect(result.current).toBe('http://localhost/@test.com') + expect(result.current.url).toBe('http://localhost/@test.com') }) }) @@ -104,9 +104,9 @@ describe('when using the redirection hook', () => { mockedUseLocation.mockReturnValue({ search: 'redirectTo=https://test.com/@localhost' } as Location) }) - it('should return undefined', () => { + it('should return the invalid redirection URL', () => { const { result } = renderHook(() => useAfterLoginRedirection()) - expect(result.current).toBeUndefined() + expect(result.current.url).toBe('http://localhost/auth/invalidRedirection') }) }) @@ -115,9 +115,9 @@ describe('when using the redirection hook', () => { mockedUseLocation.mockReturnValue({ search: 'redirectTo=https://example.com%5C%5C@localhost' } as Location) }) - it('should return undefined', () => { + it('should return the invalid redirection URL', () => { const { result } = renderHook(() => useAfterLoginRedirection()) - expect(result.current).toBeUndefined() + expect(result.current.url).toBe('http://localhost/auth/invalidRedirection') }) }) @@ -128,7 +128,18 @@ describe('when using the redirection hook', () => { it('should return the URL', () => { const { result } = renderHook(() => useAfterLoginRedirection()) - expect(result.current).toBe('http://localhost/test') + expect(result.current.url).toBe('http://localhost/test') + }) + }) + + describe('and the redirectTo parameter is missing', () => { + beforeEach(() => { + mockedUseLocation.mockReturnValue({ search: '' } as Location) + }) + + it('should return the default site', () => { + const { result } = renderHook(() => useAfterLoginRedirection()) + expect(result.current.url).toBe('http://localhost/') }) }) }) diff --git a/src/hooks/redirection.ts b/src/hooks/redirection.ts index f084c63..957c3b5 100644 --- a/src/hooks/redirection.ts +++ b/src/hooks/redirection.ts @@ -1,31 +1,55 @@ +import { useCallback } from 'react' import { useLocation } from 'react-router-dom' +import { locations } from '../shared/locations' export const useAfterLoginRedirection = () => { const location = useLocation() const search = new URLSearchParams(location.search) + + // Extract 'redirectTo' from current search parameters const redirectToSearchParam = search.get('redirectTo') - const redirectTo = redirectToSearchParam ? decodeURIComponent(redirectToSearchParam) : null - if (redirectTo === null) { - return undefined + // Initialize redirectTo with a default value + let redirectTo = locations.home() + + // Decode 'redirectTo' if it exists + if (redirectToSearchParam) { + try { + redirectTo = decodeURIComponent(redirectToSearchParam) + } catch (error) { + console.error("Can't decode redirectTo parameter") + } } + let sanitizedRedirectTo = locations.home() + try { - let redirectToURL: URL + let redirectToURL + // Create a URL object from 'redirectTo' if (redirectTo.startsWith('/')) { redirectToURL = new URL(redirectTo, window.location.origin) } else { redirectToURL = new URL(redirectTo) } + // Check if the hostname matches to prevent open redirects if (redirectToURL.hostname !== window.location.hostname) { - return undefined + redirectToURL = new URL('/auth/invalidRedirection', window.location.origin) } - return redirectToURL.href + // Set the sanitized redirect URL + sanitizedRedirectTo = redirectToURL.href } catch (error) { console.error("Can't parse redirectTo URL") - return undefined + // Optionally, redirect to an error page or the home page + sanitizedRedirectTo = locations.home() } + + // Create the redirect function + const redirect = useCallback(() => { + window.location.href = sanitizedRedirectTo + }, [sanitizedRedirectTo]) + + return { url: sanitizedRedirectTo, redirect } } diff --git a/src/main.tsx b/src/main.tsx index 891e510..153bb5b 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -8,6 +8,7 @@ import { RequestPage } from './components/Pages/RequestPage' import { SetupPage } from './components/Pages/SetupPage' import { DefaultPage } from './components/Pages/DefaultPage' import { CallbackPage } from './components/Pages/CallbackPage' +import { InvalidRedirectionPage } from './components/Pages/InvalidRedirectionPage' import { LoginPage } from './components/Pages/LoginPage' import { FeatureFlagsProvider } from './components/FeatureFlagsProvider' import { config } from './modules/config' @@ -24,6 +25,7 @@ ReactDOM.render( } /> + } /> } /> } /> } /> diff --git a/src/shared/locations.ts b/src/shared/locations.ts new file mode 100644 index 0000000..2c37440 --- /dev/null +++ b/src/shared/locations.ts @@ -0,0 +1,5 @@ +export const locations = { + home: () => '/', + login: (redirectTo?: string) => `/login${redirectTo ? `?redirectTo=${encodeURIComponent(redirectTo)}` : ''}`, + setup: (redirectTo?: string) => `/setup${redirectTo ? `?redirectTo=${encodeURIComponent(redirectTo)}` : ''}` +}