From 92d7ae172596317da64cb93c32cc3c8641e54668 Mon Sep 17 00:00:00 2001
From: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Date: Mon, 12 Jul 2021 10:24:54 +0200
Subject: [PATCH] fix(client): remove extra slash when redirecting to client
 (#42588)

Co-authored-by: Nicholas Carrigan (he/him) <nhcarrigan@gmail.com>
---
 api-server/src/server/boot/challenge.js     |  6 +++---
 api-server/src/server/component-passport.js | 24 +++++++++++++++------
 api-server/src/server/utils/redirection.js  | 11 ++++------
 cypress/support/commands.js                 |  1 +
 4 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/api-server/src/server/boot/challenge.js b/api-server/src/server/boot/challenge.js
index 544c2ce3950..98c3a0f64cf 100644
--- a/api-server/src/server/boot/challenge.js
+++ b/api-server/src/server/boot/challenge.js
@@ -17,8 +17,8 @@ import { fixCompletedChallengeItem } from '../../common/utils';
 import { getChallenges } from '../utils/get-curriculum';
 import {
   getRedirectParams,
-  getRedirectBase,
-  normalizeParams
+  normalizeParams,
+  getPrefixedLandingPath
 } from '../utils/redirection';
 
 const log = debug('fcc:boot:challenges');
@@ -341,7 +341,7 @@ export function createRedirectToCurrentChallenge(
     const { user } = req;
     const { origin, pathPrefix } = getRedirectParams(req, normalizeParams);
 
-    const redirectBase = getRedirectBase(origin, pathPrefix);
+    const redirectBase = getPrefixedLandingPath(origin, pathPrefix);
     if (!user) {
       return res.redirect(redirectBase + '/learn');
     }
diff --git a/api-server/src/server/component-passport.js b/api-server/src/server/component-passport.js
index 6f894d4b079..568387d638e 100644
--- a/api-server/src/server/component-passport.js
+++ b/api-server/src/server/component-passport.js
@@ -11,9 +11,9 @@ import passportProviders from './passport-providers';
 import { setAccessTokenToResponse } from './utils/getSetAccessToken';
 import {
   getReturnTo,
-  getRedirectBase,
+  getPrefixedLandingPath,
   getRedirectParams,
-  isRootPath
+  haveSamePath
 } from './utils/redirection';
 import { jwtSecret } from '../../../config/secrets';
 import { availableLangs } from '../../../config/i18n/all-langs';
@@ -100,9 +100,14 @@ export const devLoginRedirect = () => {
         };
       }
     );
-    returnTo += isRootPath(getRedirectBase(origin, pathPrefix), returnTo)
-      ? '/learn'
-      : '';
+
+    // if returnTo has a trailing slash, we need to remove it before comparing
+    // it to the prefixed landing path
+    if (returnTo.slice(-1) === '/') {
+      returnTo = returnTo.slice(0, -1);
+    }
+    const redirectBase = getPrefixedLandingPath(origin, pathPrefix);
+    returnTo += haveSamePath(redirectBase, returnTo) ? '/learn' : '';
     return res.redirect(returnTo);
   };
 };
@@ -142,7 +147,7 @@ we recommend using your email address: ${user.email} to sign in instead.
         const state = req && req.query && req.query.state;
         // returnTo, origin and pathPrefix are audited by getReturnTo
         let { returnTo, origin, pathPrefix } = getReturnTo(state, jwtSecret);
-        const redirectBase = getRedirectBase(origin, pathPrefix);
+        const redirectBase = getPrefixedLandingPath(origin, pathPrefix);
 
         // TODO: getReturnTo could return a success flag to show a flash message,
         // but currently it immediately gets overwritten by a second message. We
@@ -150,7 +155,12 @@ we recommend using your email address: ${user.email} to sign in instead.
         // multiple messages to appear at once.
 
         if (user.acceptedPrivacyTerms) {
-          returnTo += isRootPath(redirectBase, returnTo) ? '/learn' : '';
+          // if returnTo has a trailing slash, we need to remove it before comparing
+          // it to the prefixed landing path
+          if (returnTo.slice(-1) === '/') {
+            returnTo = returnTo.slice(0, -1);
+          }
+          returnTo += haveSamePath(redirectBase, returnTo) ? '/learn' : '';
           return res.redirectWithFlash(returnTo);
         } else {
           return res.redirectWithFlash(`${redirectBase}/email-sign-up`);
diff --git a/api-server/src/server/utils/redirection.js b/api-server/src/server/utils/redirection.js
index 4c7fb7d74bd..de441ee625d 100644
--- a/api-server/src/server/utils/redirection.js
+++ b/api-server/src/server/utils/redirection.js
@@ -50,10 +50,7 @@ function normalizeParams(
   return { returnTo, origin, pathPrefix };
 }
 
-// TODO: tests!
-// TODO: ensure origin and pathPrefix validation happens first
-// (it needs a dedicated function that can be called from here and getReturnTo)
-function getRedirectBase(origin, pathPrefix) {
+function getPrefixedLandingPath(origin, pathPrefix) {
   const redirectPathSegment = pathPrefix ? `/${pathPrefix}` : '';
   return `${origin}${redirectPathSegment}`;
 }
@@ -70,14 +67,14 @@ function getRedirectParams(req, _normalizeParams = normalizeParams) {
   return _normalizeParams({ returnTo: returnUrl.href, origin, pathPrefix });
 }
 
-function isRootPath(redirectBase, returnUrl) {
+function haveSamePath(redirectBase, returnUrl) {
   const base = new URL(redirectBase);
   const url = new URL(returnUrl);
   return base.pathname === url.pathname;
 }
 
 module.exports.getReturnTo = getReturnTo;
-module.exports.getRedirectBase = getRedirectBase;
+module.exports.getPrefixedLandingPath = getPrefixedLandingPath;
 module.exports.normalizeParams = normalizeParams;
 module.exports.getRedirectParams = getRedirectParams;
-module.exports.isRootPath = isRootPath;
+module.exports.haveSamePath = haveSamePath;
diff --git a/cypress/support/commands.js b/cypress/support/commands.js
index 11d357c1ec3..1f7113b7430 100644
--- a/cypress/support/commands.js
+++ b/cypress/support/commands.js
@@ -36,6 +36,7 @@
 Cypress.Commands.add('login', () => {
   cy.visit('/');
   cy.contains("Get started (it's free)").click();
+  cy.url().should('eq', Cypress.config().baseUrl + '/learn/');
   cy.contains('Welcome back');
 });