fix(i18n): redirect based on Referer header (#40512)

Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>

api-server/server/boot/authentication.js

@@ -16,6 +16,7 @@ import { ifUserRedirectTo, ifNoUserRedirectTo } from '../utils/middleware';
 import { wrapHandledError } from '../utils/create-handled-error.js';
 import { removeCookies } from '../utils/getSetAccessToken';
 import { decodeEmail } from '../../common/utils';
+import { getParamsFromReq } from '../utils/get-return-to';
 
 const isSignUpDisabled = !!process.env.DISABLE_SIGNUP;
 if (isSignUpDisabled) {
@@ -56,7 +57,8 @@ module.exports = function enableAuthentication(app) {
     );
   } else {
     api.get('/signin', ifUserRedirect, (req, res, next) => {
-      const state = jwt.sign({ returnTo: req.query.returnTo }, jwtSecret);
+      const { returnTo, origin, pathPrefix } = getParamsFromReq(req);
+      const state = jwt.sign({ returnTo, origin, pathPrefix }, jwtSecret);
       return passport.authenticate('auth0-login', { state })(req, res, next);
     });
 

api-server/server/boot/challenge.js

@@ -12,12 +12,15 @@ import { ObjectID } from 'mongodb';
 import isNumeric from 'validator/lib/isNumeric';
 import isURL from 'validator/lib/isURL';
 
-import { homeLocation } from '../../../config/env';
-
 import { ifNoUserSend } from '../utils/middleware';
 import { dasherize } from '../../../utils/slugs';
 import { fixCompletedChallengeItem } from '../../common/utils';
 import { getChallenges } from '../utils/get-curriculum';
+import {
+  getParamsFromReq,
+  getRedirectBase,
+  normalizeParams
+} from '../utils/get-return-to';
 
 const log = debug('fcc:boot:challenges');
 
@@ -29,7 +32,9 @@ export default async function bootChallenge(app, done) {
     await getChallenges()
   );
   const redirectToCurrentChallenge = createRedirectToCurrentChallenge(
-    challengeUrlResolver
+    challengeUrlResolver,
+    normalizeParams,
+    getParamsFromReq
   );
 
   api.post(
@@ -59,7 +64,6 @@ export default async function bootChallenge(app, done) {
   app.use(router);
   done();
 }
-const learnURL = `${homeLocation}/learn`;
 
 const jsProjects = [
   'aaa48de84e1ecc7c742e1124',
@@ -327,15 +331,22 @@ function backendChallengeCompleted(req, res, next) {
     .subscribe(() => {}, next);
 }
 
+// TODO: extend tests to cover www.freecodecamp.org/language and
+// chinese.freecodecamp.org
 export function createRedirectToCurrentChallenge(
   challengeUrlResolver,
-  { _homeLocation = homeLocation, _learnUrl = learnURL } = {}
+  normalizeParams,
+  getParamsFromReq
 ) {
   return async function redirectToCurrentChallenge(req, res, next) {
     const { user } = req;
+    const { origin, pathPrefix } = normalizeParams(getParamsFromReq(req));
+
+    const redirectBase = getRedirectBase(origin, pathPrefix);
     if (!user) {
-      return res.redirect(_learnUrl);
+      return res.redirect(redirectBase + '/learn');
     }
+
     const challengeId = user && user.currentChallengeId;
     const challengeUrl = await challengeUrlResolver(challengeId).catch(next);
     if (challengeUrl === '/learn') {
@@ -346,6 +357,6 @@ export function createRedirectToCurrentChallenge(
         db may not be properly seeded.
       `);
     }
-    return res.redirect(`${_homeLocation}${challengeUrl}`);
+    return res.redirect(`${redirectBase}${challengeUrl}`);
   };
 }

api-server/server/boot_tests/challenge.test.js

@@ -330,11 +330,18 @@ describe('boot/challenge', () => {
   describe('redirectToCurrentChallenge', () => {
     const mockHomeLocation = 'https://www.example.com';
     const mockLearnUrl = `${mockHomeLocation}/learn`;
+    const mockgetParamsFromReq = () => ({
+      returnTo: mockLearnUrl,
+      origin: mockHomeLocation,
+      pathPrefix: ''
+    });
+    const mockNormalizeParams = params => params;
 
     it('redirects to the learn base url for non-users', async done => {
       const redirectToCurrentChallenge = createRedirectToCurrentChallenge(
         () => {},
-        { _homeLocation: mockHomeLocation, _learnUrl: mockLearnUrl }
+        mockNormalizeParams,
+        mockgetParamsFromReq
       );
       const req = mockReq();
       const res = mockRes();
@@ -356,7 +363,8 @@ describe('boot/challenge', () => {
       const expectedUrl = `${mockHomeLocation}${requestedChallengeUrl}`;
       const redirectToCurrentChallenge = createRedirectToCurrentChallenge(
         challengeUrlResolver,
-        { _homeLocation: mockHomeLocation, _learnUrl: mockLearnUrl }
+        mockNormalizeParams,
+        mockgetParamsFromReq
       );
       const req = mockReq({
         user: mockUser
@@ -379,7 +387,8 @@ describe('boot/challenge', () => {
       );
       const redirectToCurrentChallenge = createRedirectToCurrentChallenge(
         challengeUrlResolver,
-        { _homeLocation: mockHomeLocation, _learnUrl: mockLearnUrl }
+        mockNormalizeParams,
+        mockgetParamsFromReq
       );
       const req = mockReq({
         user: { ...mockUser, currentChallengeId: '' }

api-server/server/component-passport.js

@@ -4,15 +4,18 @@ import {
   // prettier ignore
   PassportConfigurator
 } from '@freecodecamp/loopback-component-passport';
-import url from 'url';
 import dedent from 'dedent';
 
 import { getUserById } from './utils/user-stats';
-import { homeLocation } from '../../config/env';
 import passportProviders from './passport-providers';
 import { setAccessTokenToResponse } from './utils/getSetAccessToken';
 import { jwtSecret } from '../../config/secrets';
-import getReturnTo from './utils/get-return-to';
+import {
+  getReturnTo,
+  getRedirectBase,
+  getParamsFromReq,
+  isRootPath
+} from './utils/get-return-to';
 
 const passportOptions = {
   emailOptional: true,
@@ -82,18 +85,12 @@ export const devSaveResponseAuthCookies = () => {
 
 export const devLoginRedirect = () => {
   return (req, res) => {
-    const successRedirect = req => {
-      if (req && req.query && req.query.returnTo) {
-        return req.query.returnTo;
-      }
-      return `${homeLocation}/learn`;
-    };
-
-    let redirect = url.parse(successRedirect(req), true);
-    delete redirect.search;
-
-    redirect = url.format(redirect);
-    return res.redirect(redirect);
+    // this mirrors the production approach, but without any validation
+    let { returnTo, origin, pathPrefix } = getParamsFromReq(req);
+    returnTo += isRootPath(getRedirectBase(origin, pathPrefix), returnTo)
+      ? '/learn'
+      : '';
+    return res.redirect(returnTo);
   };
 };
 
@@ -102,14 +99,6 @@ export const createPassportCallbackAuthenticator = (strategy, config) => (
   res,
   next
 ) => {
-  const state = req && req.query && req.query.state;
-  const { returnTo } = getReturnTo(state, jwtSecret);
-
-  // TODO: getReturnTo returns a {returnTo, success} object, so we can use
-  // 'success' to show a flash message, but currently it immediately gets
-  // overwritten by a second message. We should either change the message if
-  // !success or allow multiple messages to appear at once.
-
   return passport.authenticate(
     strategy,
     { session: false },
@@ -144,12 +133,22 @@ we recommend using your email address: ${user.email} to sign in instead.
         setAccessTokenToResponse({ accessToken }, req, res);
         req.login(user);
       }
-      // TODO: handle returning to /email-sign-up without relying on
-      // homeLocation
+
+      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);
+
+      // TODO: getReturnTo could return a success flag to show a flash message,
+      // but currently it immediately gets overwritten by a second message. We
+      // should either change the message if the flag is present or allow
+      // multiple messages to appear at once.
+
       if (user.acceptedPrivacyTerms) {
+        returnTo += isRootPath(redirectBase, returnTo) ? '/learn' : '';
         return res.redirectWithFlash(returnTo);
       } else {
-        return res.redirectWithFlash(`${homeLocation}/email-sign-up`);
+        return res.redirectWithFlash(`${redirectBase}/email-sign-up`);
       }
     }
   )(req, res, next);

api-server/server/utils/get-return-to.js

@@ -1,23 +1,84 @@
 const jwt = require('jsonwebtoken');
+const { availableLangs } = require('../../../client/i18n/allLangs');
 const { allowedOrigins } = require('../../../config/cors-settings');
+// homeLocation is being used as a fallback, here. If the one provided by the
+// client is invalid we default to this.
 const { homeLocation } = require('../../../config/env.json');
 
 function getReturnTo(encryptedReturnTo, secret) {
-  let returnTo;
-  let success = false;
+  let params;
   try {
-    returnTo = jwt.verify(encryptedReturnTo, secret).returnTo;
+    params = jwt.verify(encryptedReturnTo, secret);
+  } catch (e) {
+    // TODO: report to Sentry? Probably not. Remove entirely?
+    console.log(e);
+    // something went wrong, use default params
+    params = {
+      returnTo: `${homeLocation}/learn`,
+      origin: homeLocation,
+      pathPrefix: ''
+    };
+  }
+
+  return normalizeParams(params);
+}
+
+// TODO: tests!
+function normalizeParams({ returnTo, origin, pathPrefix }) {
+  // coerce to strings, just in case something weird and nefarious is happening
+  returnTo = '' + returnTo;
+  origin = '' + origin;
+  pathPrefix = '' + pathPrefix;
   // we add the '/' to prevent returns to
   // www.freecodecamp.org.somewhere.else.com
-    if (!allowedOrigins.some(origin => returnTo.startsWith(origin + '/'))) {
-      throw Error();
-    }
-    success = true;
-  } catch {
+  if (
+    !returnTo ||
+    !allowedOrigins.some(allowed => returnTo.startsWith(allowed + '/'))
+  ) {
     returnTo = `${homeLocation}/learn`;
   }
-
-  return { returnTo, success };
+  // this can be strict equality.
+  if (!origin || !allowedOrigins.includes(origin)) {
+    origin = homeLocation;
+  }
+  // default to '' if the locale isn't recognised
+  pathPrefix = availableLangs.client.includes(pathPrefix) ? pathPrefix : '';
+  return { returnTo, origin, pathPrefix };
 }
 
-module.exports = getReturnTo;
+// TODO: use this to redirect to current challenge
+// 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) {
+  const redirectPathSegment = pathPrefix ? `/${pathPrefix}` : '';
+  return `${origin}${redirectPathSegment}`;
+}
+
+// TODO: this might be cleaner if we just use a URL for returnTo (call it
+// returnURL for clarity) rather than pulling out origin and returning it
+// separately
+function getParamsFromReq(req) {
+  const url = req.header('Referer');
+  // since we do not always redirect the user back to the page they were on
+  // we need client locale and origin to construct the redirect url.
+  const returnUrl = new URL(url);
+  const origin = returnUrl.origin;
+  // if this is not one of the client languages, validation will convert
+  // this to '' before it is used.
+  const pathPrefix = returnUrl.pathname.split('/')[0];
+  return { returnTo: returnUrl.href, origin, pathPrefix };
+}
+
+function isRootPath(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.normalizeParams = normalizeParams;
+module.exports.getParamsFromReq = getParamsFromReq;
+module.exports.isRootPath = isRootPath;

api-server/server/utils/get-return-to.test.js

@@ -3,13 +3,21 @@
 const { homeLocation } = require('../../../config/env.json');
 const jwt = require('jsonwebtoken');
 
-const getReturnTo = require('./get-return-to');
+const { getReturnTo } = require('./get-return-to');
 
 const validJWTSecret = 'this is a super secret string';
 const invalidJWTSecret = 'This is not correct secret';
 const validReturnTo = 'https://www.freecodecamp.org/settings';
 const invalidReturnTo = 'https://www.freecodecamp.org.fake/settings';
 const defaultReturnTo = `${homeLocation}/learn`;
+const defaultOrigin = homeLocation;
+const defaultPrefix = '';
+
+const defaultObject = {
+  returnTo: defaultReturnTo,
+  origin: defaultOrigin,
+  pathPrefix: defaultPrefix
+};
 
 describe('get-return-to', () => {
   describe('getReturnTo', () => {
@@ -21,8 +29,8 @@ describe('get-return-to', () => {
         validJWTSecret
       );
       expect(getReturnTo(encryptedReturnTo, validJWTSecret)).toStrictEqual({
-        returnTo: validReturnTo,
-        success: true
+        ...defaultObject,
+        returnTo: validReturnTo
       });
     });
 
@@ -33,10 +41,9 @@ describe('get-return-to', () => {
         { returnTo: validReturnTo },
         invalidJWTSecret
       );
-      expect(getReturnTo(encryptedReturnTo, validJWTSecret)).toStrictEqual({
-        returnTo: defaultReturnTo,
-        success: false
-      });
+      expect(getReturnTo(encryptedReturnTo, validJWTSecret)).toStrictEqual(
+        defaultObject
+      );
     });
 
     it('should return a default url for unknown origins', () => {
@@ -45,10 +52,9 @@ describe('get-return-to', () => {
         { returnTo: invalidReturnTo },
         validJWTSecret
       );
-      expect(getReturnTo(encryptedReturnTo, validJWTSecret)).toStrictEqual({
-        returnTo: defaultReturnTo,
-        success: false
-      });
+      expect(getReturnTo(encryptedReturnTo, validJWTSecret)).toStrictEqual(
+        defaultObject
+      );
     });
   });
 });

client/src/client-only-routes/ShowSettings.js

@@ -5,7 +5,7 @@ import { createSelector } from 'reselect';
 import { Grid, Button } from '@freecodecamp/react-bootstrap';
 import Helmet from 'react-helmet';
 
-import { apiLocation, homeLocation } from '../../config/env.json';
+import { apiLocation } from '../../config/env.json';
 import {
   signInLoadingSelector,
   userSelector,
@@ -169,7 +169,7 @@ export function ShowSettings(props) {
   }
 
   if (!isSignedIn) {
-    navigate(`${apiLocation}/signin?returnTo=${homeLocation}/settings`);
+    navigate(`${apiLocation}/signin`);
     return <Loader fullScreen={true} />;
   }
 

client/src/client-only-routes/ShowSettings.test.js

@@ -1,7 +1,7 @@
 /* global jest, expect */
 import React from 'react';
 import ShallowRenderer from 'react-test-renderer/shallow';
-import { apiLocation, homeLocation } from '../../config/env.json';
+import { apiLocation } from '../../config/env.json';
 
 import { ShowSettings } from './ShowSettings';
 
@@ -22,9 +22,7 @@ describe('<ShowSettings />', () => {
     const shallow = new ShallowRenderer();
     shallow.render(<ShowSettings {...loggedOutProps} />);
     expect(navigate).toHaveBeenCalledTimes(1);
-    expect(navigate).toHaveBeenCalledWith(
-      `${apiLocation}/signin?returnTo=${homeLocation}/settings`
-    );
+    expect(navigate).toHaveBeenCalledWith(`${apiLocation}/signin`);
     const result = shallow.getRenderOutput();
     // Renders Loader rather than ShowSettings
     expect(result.type.displayName).toBe('Loader');

client/src/components/Header/components/Login.js

@@ -27,9 +27,7 @@ function Login(props) {
     children,
     isSignedIn
   } = props;
-  const href = isSignedIn
-    ? `${homeLocation}/learn`
-    : `${apiLocation}/signin?returnTo=${homeLocation}/learn`;
+  const href = isSignedIn ? `${homeLocation}/learn` : `${apiLocation}/signin`;
   return (
     <Button
       bsStyle='default'

config/env.js

@@ -11,10 +11,10 @@ if (process.env.PIPELINE_ENV !== 'true') {
 }
 
 const {
-  HOME_LOCATION: home,
-  API_LOCATION: api,
-  FORUM_LOCATION: forum,
-  NEWS_LOCATION: news,
+  HOME_LOCATION: homeLocation,
+  API_LOCATION: apiLocation,
+  FORUM_LOCATION: forumLocation,
+  NEWS_LOCATION: newsLocation,
   CLIENT_LOCALE: clientLocale,
   CURRICULUM_LOCALE: curriculumLocale,
   SHOW_LOCALE_DROPDOWN_MENU: showLocaleDropdownMenu,
@@ -27,10 +27,10 @@ const {
 } = process.env;
 
 const locations = {
-  homeLocation: home,
-  apiLocation: api,
-  forumLocation: forum,
-  newsLocation: news
+  homeLocation,
+  apiLocation,
+  forumLocation,
+  newsLocation
 };
 
 module.exports = Object.assign(locations, {