From 88fee5d62b9992cd190221819a487222af87732d Mon Sep 17 00:00:00 2001
From: MaxGenash <max.henash@bigcommerce.com>
Date: Fri, 4 Dec 2020 12:45:49 +0200
Subject: [PATCH] fix: add handling cases when redirect link is already
 stripped in normalizeRedirectUrl

---
 server/lib/utils.js                        | 23 +++++-----
 server/lib/utils.spec.js                   | 49 +++++++++++++++++++---
 server/plugins/renderer/renderer.module.js |  2 +-
 3 files changed, 56 insertions(+), 18 deletions(-)

diff --git a/server/lib/utils.js b/server/lib/utils.js
index df392c04..41d33175 100644
--- a/server/lib/utils.js
+++ b/server/lib/utils.js
@@ -17,24 +17,25 @@ function stripDomainFromCookies(cookies) {
 /**
  * Strip domain from redirectUrl if it matches the current storeUrl, if not, leave it.
  *
- * @param request
- * @param redirectUrl
+ * @param {string} redirectUrl
+ * @param {{ normalStoreUrl, storeUrl}} config
  * @returns {string}
  */
-function normalizeRedirectUrl(request, redirectUrl) {
-    const storeHost = new URL(request.app.normalStoreUrl).host;
-    const secureStoreHost = new URL(request.app.storeUrl).host;
+function normalizeRedirectUrl(redirectUrl, config) {
+    if (!redirectUrl || !redirectUrl.startsWith('http')) {
+        return redirectUrl; // already stripped, skip
+    }
+
+    const storeHost = new URL(config.normalStoreUrl).host;
+    const secureStoreHost = new URL(config.storeUrl).host;
     const redirectUrlObj = new URL(redirectUrl);
-    let stripHost = false;
 
     if (redirectUrlObj.host === storeHost || redirectUrlObj.host === secureStoreHost) {
-        stripHost = true;
-    }
-
-    if (stripHost) {
+        // Need to strip
         return redirectUrlObj.pathname + redirectUrlObj.search + redirectUrlObj.hash;
     }
-    return redirectUrl;
+
+    return redirectUrl; // Different host, shouldn't strip
 }
 
 /**
diff --git a/server/lib/utils.spec.js b/server/lib/utils.spec.js
index c480e1dc..4959aadb 100644
--- a/server/lib/utils.spec.js
+++ b/server/lib/utils.spec.js
@@ -1,21 +1,58 @@
-const utils = require('./utils');
+const { uuid2int, int2uuid, normalizeRedirectUrl } = require('./utils');
 
 describe('utils', () => {
     describe('uuid2int', () => {
         it('should return an int value presentation', () => {
-            expect(utils.uuid2int('00000000-0000-0000-0000-000000000001')).toEqual(1);
-            expect(utils.uuid2int('00000000-0000-0000-0000-000000000102')).toEqual(102);
+            expect(uuid2int('00000000-0000-0000-0000-000000000001')).toEqual(1);
+            expect(uuid2int('00000000-0000-0000-0000-000000000102')).toEqual(102);
         });
 
         it('should throw an error if an invalid uuid is used', () => {
-            expect(() => utils.uuid2int('00002')).toThrow(Error);
+            expect(() => uuid2int('00002')).toThrow(Error);
         });
     });
 
     describe('int2uuid', () => {
         it('should return an uuid value presentation', () => {
-            expect(utils.int2uuid(1)).toEqual('00000000-0000-0000-0000-000000000001');
-            expect(utils.int2uuid(505)).toEqual('00000000-0000-0000-0000-000000000505');
+            expect(int2uuid(1)).toEqual('00000000-0000-0000-0000-000000000001');
+            expect(int2uuid(505)).toEqual('00000000-0000-0000-0000-000000000505');
+        });
+    });
+
+    describe('normalizeRedirectUrl', () => {
+        it('should return the original value if the redirectUrl is already striped', () => {
+            const redirectUrl = '/products?filter=name#3';
+            expect(normalizeRedirectUrl(redirectUrl, {})).toEqual(redirectUrl);
+        });
+
+        it('should return the original value if the redirectUrl has a host different from hosts in config', () => {
+            const redirectUrl = 'https://google.com/search?q=this-product';
+            const config = {
+                normalStoreUrl: 'https://store-12345678.mybigcommerce.com',
+                storeUrl: 'https://my-awesome-store.com',
+            };
+
+            expect(normalizeRedirectUrl(redirectUrl, config)).toEqual(redirectUrl);
+        });
+
+        it('should return the url without host if the redirectUrl has a host = to normalStoreUrl', () => {
+            const redirectUrl = 'https://store-12345678.mybigcommerce.com/products?filter=name#3';
+            const config = {
+                normalStoreUrl: 'https://store-12345678.mybigcommerce.com',
+                storeUrl: 'https://my-awesome-store.com',
+            };
+
+            expect(normalizeRedirectUrl(redirectUrl, config)).toEqual('/products?filter=name#3');
+        });
+
+        it('should return the url without host if the redirectUrl has a host = to storeUrl', () => {
+            const redirectUrl = 'https://my-awesome-store.com/products?filter=name#3';
+            const config = {
+                normalStoreUrl: 'https://store-12345678.mybigcommerce.com',
+                storeUrl: 'https://my-awesome-store.com',
+            };
+
+            expect(normalizeRedirectUrl(redirectUrl, config)).toEqual('/products?filter=name#3');
         });
     });
 });
diff --git a/server/plugins/renderer/renderer.module.js b/server/plugins/renderer/renderer.module.js
index 0d8413e0..504401a2 100644
--- a/server/plugins/renderer/renderer.module.js
+++ b/server/plugins/renderer/renderer.module.js
@@ -291,7 +291,7 @@ internals.redirect = async (response, request) => {
         throw new Error('StatusCode is set to 30x but there is no location header to redirect to.');
     }
 
-    response.headers.set('location', utils.normalizeRedirectUrl(request, location));
+    response.headers.set('location', utils.normalizeRedirectUrl(location, request.app));
 
     // return a redirect response
     return new RedirectResponse(location, response.headers.raw(), response.status);