diff --git a/src/frontend/with-page-auth-required.tsx b/src/frontend/with-page-auth-required.tsx index 9ac110377..c3d768844 100644 --- a/src/frontend/with-page-auth-required.tsx +++ b/src/frontend/with-page-auth-required.tsx @@ -84,7 +84,7 @@ const withPageAuthRequired: WithPageAuthRequired = (Component, options = {}) => useEffect(() => { if ((user && !error) || isLoading) return; - window.location.assign(`${loginUrl}?returnTo=${returnTo}`); + window.location.assign(`${loginUrl}?returnTo=${encodeURIComponent(returnTo)}`); }, [user, error, isLoading]); if (error) return onError(error); diff --git a/src/helpers/with-page-auth-required.ts b/src/helpers/with-page-auth-required.ts index 66e347c59..617ccccd3 100644 --- a/src/helpers/with-page-auth-required.ts +++ b/src/helpers/with-page-auth-required.ts @@ -106,7 +106,12 @@ export default function withPageAuthRequiredFactory(loginUrl: string, getSession // 10 - redirect // 9.5.4 - unstable_redirect // 9.4 - res.setHeaders - return { redirect: { destination: `${loginUrl}?returnTo=${returnTo || ctx.resolvedUrl}`, permanent: false } }; + return { + redirect: { + destination: `${loginUrl}?returnTo=${encodeURIComponent(returnTo || ctx.resolvedUrl)}`, + permanent: false + } + }; } let ret: any = { props: {} }; if (getServerSideProps) { diff --git a/tests/frontend/with-page-auth-required.test.tsx b/tests/frontend/with-page-auth-required.test.tsx index 8aace4ff5..2c6d396e0 100644 --- a/tests/frontend/with-page-auth-required.test.tsx +++ b/tests/frontend/with-page-auth-required.test.tsx @@ -92,7 +92,11 @@ describe('with-page-auth-required csr', () => { const ProtectedPage = withPageAuthRequired(MyPage, { returnTo: '/foo' }); render(, { wrapper: withUserProvider() }); - await waitFor(() => expect(window.location.assign).toHaveBeenCalledWith(expect.stringContaining('?returnTo=/foo'))); + await waitFor(() => + expect(window.location.assign).toHaveBeenCalledWith( + expect.stringContaining(`?returnTo=${encodeURIComponent('/foo')}`) + ) + ); }); it('should use a custom login url', async () => { @@ -117,9 +121,24 @@ describe('with-page-auth-required csr', () => { render(, { wrapper: withUserProvider() }); await waitFor(() => - expect(window.location.assign).toHaveBeenCalledWith(expect.stringContaining('?returnTo=/foo/bar')) + expect(window.location.assign).toHaveBeenCalledWith( + expect.stringContaining(`?returnTo=${encodeURIComponent('/foo/bar')}`) + ) ); routerMock.basePath = basePath; routerMock.asPath = asPath; }); + + it('should preserve multiple query params in the returnTo URL', async () => { + (global as any).fetch = fetchUserUnsuccessfulMock; + const MyPage = (): JSX.Element => <>Private; + const ProtectedPage = withPageAuthRequired(MyPage, { returnTo: '/foo?bar=baz&qux=quux' }); + + render(, { wrapper: withUserProvider() }); + await waitFor(() => { + expect(window.location.assign).toHaveBeenCalled(); + }); + const url = new URL((window.location.assign as jest.Mock).mock.calls[0][0], 'https://example.com'); + expect(url.searchParams.get('returnTo')).toEqual('/foo?bar=baz&qux=quux'); + }); }); diff --git a/tests/helpers/with-page-auth-required.test.ts b/tests/helpers/with-page-auth-required.test.ts index 5765e8e62..1e6c9900a 100644 --- a/tests/helpers/with-page-auth-required.test.ts +++ b/tests/helpers/with-page-auth-required.test.ts @@ -1,3 +1,4 @@ +import { URL } from 'url'; import { login, setup, teardown } from '../fixtures/setup'; import { withoutApi } from '../fixtures/default-settings'; import { get } from '../auth0-session/fixtures/helpers'; @@ -11,7 +12,7 @@ describe('with-page-auth-required ssr', () => { res: { statusCode, headers } } = await get(baseUrl, '/protected', { fullResponse: true }); expect(statusCode).toBe(307); - expect(headers.location).toBe('/api/auth/login?returnTo=/protected'); + expect(decodeURIComponent(headers.location)).toBe('/api/auth/login?returnTo=/protected'); }); test('allow access to a page with a valid session', async () => { @@ -31,7 +32,7 @@ describe('with-page-auth-required ssr', () => { res: { statusCode, headers } } = await get(baseUrl, '/protected', { fullResponse: true }); expect(statusCode).toBe(307); - expect(headers.location).toBe('/api/auth/login?returnTo=/foo'); + expect(decodeURIComponent(headers.location)).toBe('/api/auth/login?returnTo=/foo'); }); test('accept custom server-side props', async () => { @@ -56,7 +57,7 @@ describe('with-page-auth-required ssr', () => { res: { statusCode, headers } } = await get(baseUrl, '/protected', { fullResponse: true }); expect(statusCode).toBe(307); - expect(headers.location).toBe('/api/foo?returnTo=/protected'); + expect(decodeURIComponent(headers.location)).toBe('/api/foo?returnTo=/protected'); delete process.env.NEXT_PUBLIC_AUTH0_LOGIN; }); @@ -68,4 +69,14 @@ describe('with-page-auth-required ssr', () => { } = await get(baseUrl, '/csr-protected', { cookieJar, fullResponse: true }); expect(statusCode).toBe(200); }); + + test('should preserve multiple query params in the returnTo URL', async () => { + const baseUrl = await setup(withoutApi, { withPageAuthRequiredOptions: { returnTo: '/foo?bar=baz&qux=quux' } }); + const { + res: { statusCode, headers } + } = await get(baseUrl, '/protected', { fullResponse: true }); + expect(statusCode).toBe(307); + const url = new URL(headers.location, baseUrl); + expect(url.searchParams.get('returnTo')).toEqual('/foo?bar=baz&qux=quux'); + }); });