revert(bug): Revert settings redirect in graphql to fix ios pairing bug

mozilla · Aug 28, 2023 · a018d63 · a018d63
1 parent 195ca61
commit a018d63
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 20 deletions.
diff --git a/packages/fxa-content-server/app/scripts/lib/router.js b/packages/fxa-content-server/app/scripts/lib/router.js
@@ -374,14 +374,7 @@ Router = Router.extend({
       const searchParams = new URLSearchParams(this.window.location.search);
       const redirectUrl = searchParams.get('redirect_to');
       if (redirectUrl) {
-        // Ignore query params when validating the redirect url
-        const parsedRedirectUrl = redirectUrl.split('?')[0];
-        if (
-          !this.isValidRedirect(
-            parsedRedirectUrl,
-            this.config.redirectAllowlist
-          )
-        ) {
+        if (!this.isValidRedirect(redirectUrl, this.config.redirectAllowlist)) {
           throw new Error('Invalid redirect!');
         }
         return this.navigateAway(redirectUrl);

diff --git a/packages/fxa-settings/src/lib/gql.test.ts b/packages/fxa-settings/src/lib/gql.test.ts
@@ -1,7 +1,7 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-import { errorHandler } from './gql';
+import { errorHandler, pagesRequiringAuthentication } from './gql';
 import { ErrorResponse } from '@apollo/client/link/error';
 import { Operation, NextLink, ServerError } from '@apollo/client/core';
 import { GraphQLError } from 'graphql';
@@ -11,8 +11,8 @@ let errorResponse: ErrorResponse;
 let mockReplace: Mock;
 
 describe('errorHandler', () => {
-  const pageWhichRequiresAuthentication = 'https://accounts.firefox.com/settings';
-  const pageWhichDoesNotRequireAuthentication = 'https://accounts.firefox.com/signin';
+  const pageWhichRequiresAuthentication = pagesRequiringAuthentication[0];
+  const pageWhichDoesNotRequireAuthentication = 'foo';
   beforeAll(() => {
     mockReplace = jest.fn();
     Object.defineProperty(window, 'location', {
@@ -43,7 +43,7 @@ describe('errorHandler', () => {
     errorHandler(errorResponse);
 
     expect(window.location.replace).toHaveBeenCalledWith(
-      '/signin?redirect_to=https%3A%2F%2Faccounts.firefox.com%2Fsettings'
+      `/signin?redirect_to=${pageWhichRequiresAuthentication}`
     );
   });
 
@@ -60,7 +60,7 @@ describe('errorHandler', () => {
     errorHandler(errorResponse);
 
     expect(window.location.replace).toHaveBeenCalledWith(
-     '/signin?redirect_to=https%3A%2F%2Faccounts.firefox.com%2Fsettings'
+      `/signin?redirect_to=${pageWhichRequiresAuthentication}`
     );
   });
 
@@ -86,8 +86,7 @@ describe('errorHandler', () => {
       value: {
         replace: mockReplace,
         search: '',
-        pathname: 'signin',
-        href: pageWhichDoesNotRequireAuthentication,
+        pathname: pageWhichDoesNotRequireAuthentication,
       },
     });
 

diff --git a/packages/fxa-settings/src/lib/gql.ts b/packages/fxa-settings/src/lib/gql.ts
@@ -40,12 +40,8 @@ export const errorHandler: ErrorHandler = ({ graphQLErrors, networkError }) => {
     }
   }
   if (reauth && currentPageRequiresAuthentication) {
-    // When doing a redirect and going to signin, we want to ensure that original query params
-    // are sent as well, otw we would strip out utms and context params
-    const queryParams = new URLSearchParams(window.location.search);
-    queryParams.set('redirect_to', window.location.href);
     window.location.replace(
-      `/signin?${queryParams.toString()}`
+      `/signin?redirect_to=${encodeURIComponent(window.location.pathname)}`
     );
   } else {
     if (!reauth) {