Repository: auth0/nextjs-auth0 (ts)

Issue:
withPageAuthRequired not redirecting to the correct url if there are multiple url params
<!--
**Please do not report security vulnerabilities here**. The Responsible Disclosure Program (https://auth0.com/whitehat) details the procedure for disclosing security issues.

Thank you in advance for helping us to improve this library! Please read through the template below and answer all relevant questions. Your additional work here is greatly appreciated and will help us respond as quickly as possible. For general support or usage questions, use the Auth0 Community (https://community.auth0.com/) or Auth0 Support (https://support.auth0.com/). Finally, to avoid duplicates, please search existing Issues before submitting one here.

By submitting an Issue to this repository, you agree to the terms within the Auth0 Code of Conduct (https://github.com/auth0/open-source-template/blob/main/CODE-OF-CONDUCT.md).
-->

### Describe the problem
I have an SSR page that is protected by `withPageAuthRequired`. The url for this page is in the format of `http://localhost:3000/events?city=some-city&start=xxxx-xx-xx&end=xxxx-xx-xx`. 

When I'm logged in I have no problem accessing this page. When I'm not logged in I am redirected to the login page as expected. The problem is after logging in, I am redirected to `http://localhost:3000/events?city=some-city`. The rest of the url is not included in the redirect. 

<!--
> Provide a clear and concise description of the issue
-->

### What was the expected behavior?
I should be redirected to the correct url which includes all the url/query string params. 

I checked the network tab on Chrome and this is the url requested when I'm being redirected to the login page: `http://localhost:3000/api/auth/login?returnTo=/events?city=some-city&start=xxxx-xx-xx&end=xxxx-xx-xx`

I expect the `returnTo` url param to equal `/events?city=some-city&start=xxxx-xx-xx&end=xxxx-xx-xx` but that is not the case. There are actually 3 url params here:

- `returnTo: /events?city=some-city`
- `start: xxxx-xx-xx`
- `end: xxxx-xx-xx`

The `returnTo` value will only include the url up to the first url/query string param.
<!--
> Tell us about the behavior you expected to see
-->

### Environment

<!--
> Please provide the following:
-->

- **Version of this library used:** 1.3.0
- **Which framework are you using, if applicable:**next 10.0.7


Pull request:
[SDK-2452] returnTo should be encoded as it contains url unsafe chars
### Description

Encoding the `returnTo` argument as it can contain unssafe url chars (for SSR and CSR `withPageAuthRequired`)

### References

fixes #351 

### Testing

- [x] This change adds test coverage for new/changed/fixed functionality

### Checklist

- [x] I have added documentation for new/changed functionality in this PR or in auth0.com/docs
- [ ] All active GitHub checks for tests, formatting, and security are passing
- [x] The correct base branch is being used, if not `main`


Patch:
diff --git a/src/frontend/with-page-auth-required.tsx b/src/frontend/with-page-auth-required.tsx
index 9ac11037..c3d76884 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 66e347c5..617ccccd 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) {


Test patch:
diff --git a/tests/frontend/with-page-auth-required.test.tsx b/tests/frontend/with-page-auth-required.test.tsx
index 8aace4ff..2c6d396e 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(<ProtectedPage />, { 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(<ProtectedPage />, { 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(<ProtectedPage />, { 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 5765e8e6..1e6c9900 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');
+  });
 });