fix: login redirect should preserve query parameters (#2012)

https://www.notion.so/Ensure-redirect-from-Pricing-page-takes-you-to-post-login-modal-of-selected-tier-e9fb5131d7b2486596de0d8c17f77161

This PR is a follow up to a conversation BenGo (not tagging him here to
avoid notification noise) and I had a couple of weeks ago. The high
level summary is:

Something like the pricing page could use a `requiresAuth: true`
behavior. This would function similar to the `redirectTo: '...'`
behavior, except it would capture the url and handle the redirecting
automatically. This is important for tiered pricing because otherwise we
can't include which tier the customer selected on the public pricing
page before we redirect to the authenticated accounts payment page
because the `redirectTo` must be static.

packages/website/components/contexts/authorizationContext.js

@@ -12,6 +12,8 @@ import { isLoggedIn, getMagic } from 'lib/magic';
  * @property {string} [redirectTo] If redirectTo is set, redirect if the user was not found
  * @property {boolean} [redirectIfFound] If redirectIfFound is also set, redirect if the user was found.
  * @property {boolean} [authOnLoad] Whether or not to check for magic authorization, defaults to true
+ * @property {boolean} [requiresAuth] Whether or not to perform automatic login and redirect back
+ *                                    to this page if unauthenticated
  */
 
 /**

packages/website/components/general/restrictedRoute.js

@@ -1,3 +1,5 @@
+import { stringify } from 'querystring';
+
 import { useRouter } from 'next/router';
 import { useEffect } from 'react';
 
@@ -11,6 +13,8 @@ import { useAuthorization } from 'components/contexts/authorizationContext';
  * @property {boolean} [isRestricted] whether or not this route is restricted to auhtenticated users
  * @property {string} [redirectTo] If redirectTo is set, redirect if the user was not found
  * @property {boolean} [redirectIfFound] If redirectIfFound is also set, redirect if the user was found.
+ * @property {boolean} [requiresAuth] If requiresAuth is set and redirectTo isn't set then,
+ *                                    redirect to the login page setting the return uri to this page
  */
 
 /**
@@ -20,12 +24,12 @@ import { useAuthorization } from 'components/contexts/authorizationContext';
  *
  * @returns
  */
-const RestrictedRoute = ({ isRestricted = false, children, redirectTo, redirectIfFound = false }) => {
-  const { push } = useRouter();
+const RestrictedRoute = ({ isRestricted = false, children, redirectTo, redirectIfFound = false, requiresAuth }) => {
+  const { push, asPath } = useRouter();
   const { isLoading, isFetching, isLoggedIn } = useAuthorization();
 
   useEffect(() => {
-    if (!redirectTo || isLoading || isFetching) {
+    if (!(redirectTo || requiresAuth) || isLoading || isFetching) {
       return;
     }
     if (
@@ -34,11 +38,13 @@ const RestrictedRoute = ({ isRestricted = false, children, redirectTo, redirectI
       // If redirectIfFound is also set, redirect if the user was found
       (redirectIfFound && isLoggedIn)
     ) {
-      push(redirectTo);
+      push(redirectTo ?? '');
+    } else if (requiresAuth && !isLoggedIn) {
+      push(`/login/?${stringify({ redirect_uri: asPath })}`);
     }
-  }, [redirectTo, redirectIfFound, isFetching, isLoading, isLoggedIn, push]);
+  }, [redirectTo, redirectIfFound, isFetching, isLoading, isLoggedIn, push, asPath, requiresAuth]);
 
-  const shouldWaitForLoggedIn = isRestricted && !isLoggedIn;
+  const shouldWaitForLoggedIn = (isRestricted || requiresAuth) && !isLoggedIn;
 
   return !shouldWaitForLoggedIn ? (
     children

packages/website/components/types.d.ts

@@ -4,6 +4,7 @@ export interface RestrictedRouteProps {
   isRestricted?: boolean;
   redirectTo?: string;
   redirectIfFound?: boolean;
+  requiresAuth?: boolean;
   pageBgColor?: string; // TODO: Remove if unused
   navBgColor?: string; // TODO: Remove if unused
   footerBgColor?: string; // TODO: Remove if unused

packages/website/pages/account/payment.js

@@ -218,8 +218,7 @@ export function getStaticProps() {
   return {
     props: {
       title: 'Payment',
-      isRestricted: true,
-      redirectTo: '/login/?redirect_uri=/account/payment',
+      requiresAuth: true,
       stripePublishableKey,
       breadcrumbs: [crumbs.index, crumbs.payment],
     },

packages/website/tests/accountPayment.e2e.spec.ts

@@ -11,20 +11,29 @@ test.beforeEach(async ({ page }) => {
 
 test.describe('/account/payment', () => {
   test('redirects through login and back when not initially authenticated', async ({ page }, testInfo) => {
-    const accountPaymentPathname = '/account/payment';
+    const accountPaymentPathname = '/account/payment/';
+    const accountQuery = '?plan=lite'
+    const accountUrl = `${accountPaymentPathname}${accountQuery}`;
+
     // try to go to page that requires authn
-    await page.goto(accountPaymentPathname, { waitUntil: 'networkidle' });
+    await page.goto(accountUrl, { waitUntil: 'networkidle' });
     // wait for redirect to a Log in page
     await page.locator('text=Log in').waitFor();
     const pageUrl = new URL(page.url());
+
     expect(pageUrl.pathname).toEqual('/login/');
-    expect(pageUrl.searchParams.get('redirect_uri')).toEqual(accountPaymentPathname);
+    expect(pageUrl.searchParams.get('redirect_uri')).toEqual(accountUrl);
     // fill login
     await LoginTester().login(page, {
       email: MAGIC_SUCCESS_EMAIL,
     });
+
+    console.log(`pathname: ${new URL(page.url()).pathname}`);
+    console.log(`search: ${new URL(page.url()).search}`);
+
     // should be back at our initial target destination
-    expect(new URL(page.url()).pathname).toEqual(accountPaymentPathname + '/');
+    expect(new URL(page.url()).pathname).toEqual(accountPaymentPathname);
+    expect(new URL(page.url()).search).toEqual(accountQuery);
     await page.screenshot({
       fullPage: true,
       path: await E2EScreenshotPath(testInfo, `accountPayment-noauth`),