feat(api): allow redirects with a returnTo param (#40161)

api-server/server/boot/authentication.js

@@ -2,12 +2,15 @@ import passport from 'passport';
 import dedent from 'dedent';
 import { check } from 'express-validator/check';
 import { isEmail } from 'validator';
+import jwt from 'jsonwebtoken';
 
 import { homeLocation } from '../../../config/env';
+import { jwtSecret } from '../../../config/secrets';
+
 import {
   createPassportCallbackAuthenticator,
-  saveResponseAuthCookies,
+  devSaveResponseAuthCookies,
-  loginRedirect
+  devLoginRedirect
 } from '../component-passport';
 import { ifUserRedirectTo, ifNoUserRedirectTo } from '../utils/middleware';
 import { wrapHandledError } from '../utils/create-handled-error.js';
@@ -37,8 +40,8 @@ module.exports = function enableAuthentication(app) {
   app.enableAuth();
   const ifUserRedirect = ifUserRedirectTo();
   const ifNoUserRedirectHome = ifNoUserRedirectTo(homeLocation);
-  const saveAuthCookies = saveResponseAuthCookies();
+  const devSaveAuthCookies = devSaveResponseAuthCookies();
-  const loginSuccessRedirect = loginRedirect();
+  const devLoginSuccessRedirect = devLoginRedirect();
   const api = app.loopback.Router();
 
   // Use a local mock strategy for signing in if we are in dev mode.
@@ -48,26 +51,14 @@ module.exports = function enableAuthentication(app) {
     api.get(
       '/signin',
       passport.authenticate('devlogin'),
-      saveAuthCookies,
+      devSaveAuthCookies,
-      loginSuccessRedirect
+      devLoginSuccessRedirect
     );
   } else {
-    api.get(
+    api.get('/signin', ifUserRedirect, (req, res, next) => {
-      '/signin',
+      const state = jwt.sign({ returnTo: req.query.returnTo }, jwtSecret);
-      (req, res, next) => {
+      return passport.authenticate('auth0-login', { state })(req, res, next);
-        if (req && req.query && req.query.returnTo) {
+    });
-          req.query.returnTo = `${homeLocation}/${req.query.returnTo}`;
-        }
-        return next();
-      },
-      ifUserRedirect,
-      (req, res, next) => {
-        const state = req.query.returnTo
-          ? Buffer.from(req.query.returnTo).toString('base64')
-          : null;
-        return passport.authenticate('auth0-login', { state })(req, res, next);
-      }
-    );
 
     api.get(
       '/auth/auth0/callback',

api-server/server/component-passport.js

@@ -11,6 +11,8 @@ 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';
 
 const passportOptions = {
   emailOptional: true,
@@ -63,7 +65,7 @@ export function setupPassport(app) {
   });
 }
 
-export const saveResponseAuthCookies = () => {
+export const devSaveResponseAuthCookies = () => {
   return (req, res, next) => {
     const user = req.user;
 
@@ -78,12 +80,11 @@ export const saveResponseAuthCookies = () => {
   };
 };
 
-export const loginRedirect = () => {
+export const devLoginRedirect = () => {
   return (req, res) => {
     const successRedirect = req => {
-      if (!!req && req.session && req.session.returnTo) {
+      if (req && req.query && req.query.returnTo) {
-        delete req.session.returnTo;
+        return req.query.returnTo;
-        return `${homeLocation}/learn`;
       }
       return `${homeLocation}/learn`;
     };
@@ -101,10 +102,14 @@ export const createPassportCallbackAuthenticator = (strategy, config) => (
   res,
   next
 ) => {
-  const returnTo =
+  const state = req && req.query && req.query.state;
-    req && req.query && req.query.state
+  const { returnTo } = getReturnTo(state, jwtSecret);
-      ? Buffer.from(req.query.state, 'base64').toString('utf-8')
+
-      : `${homeLocation}/learn`;
+  // 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 },
@@ -116,7 +121,6 @@ export const createPassportCallbackAuthenticator = (strategy, config) => (
       if (!user || !userInfo) {
         return res.redirect('/signin');
       }
-      const redirect = `${returnTo}`;
 
       const { accessToken } = userInfo;
       const { provider } = config;
@@ -140,9 +144,10 @@ we recommend using your email address: ${user.email} to sign in instead.
         setAccessTokenToResponse({ accessToken }, req, res);
         req.login(user);
       }
-      // TODO: enable 'returnTo' for sign-up
+      // TODO: handle returning to /email-sign-up without relying on
+      // homeLocation
       if (user.acceptedPrivacyTerms) {
-        return res.redirectWithFlash(redirect);
+        return res.redirectWithFlash(returnTo);
       } else {
         return res.redirectWithFlash(`${homeLocation}/email-sign-up`);
       }

api-server/server/middleware.json

@@ -29,7 +29,6 @@
   "auth:before": {
     "express-flash": {},
     "./middlewares/express-extensions": {},
-    "./middlewares/add-return-to": {},
     "./middlewares/cookie-parser": {},
     "./middlewares/request-authorization": {}
   },

api-server/server/middlewares/add-return-to.js

@@ -1,37 +0,0 @@
-const pathsOfNoReturn = [
-  'link',
-  'auth',
-  'login',
-  'logout',
-  'signin',
-  'signup',
-  'fonts',
-  'favicon',
-  'js',
-  'css'
-];
-
-const pathsAllowedList = ['challenges', 'map', 'commit'];
-
-const pathsOfNoReturnRegex = new RegExp(pathsOfNoReturn.join('|'), 'i');
-const pathsAllowedRegex = new RegExp(pathsAllowedList.join('|'), 'i');
-
-export default function addReturnToUrl() {
-  return function(req, res, next) {
-    // Remember original destination before login.
-    var path = req.path.split('/')[1];
-
-    if (
-      req.method !== 'GET' ||
-      pathsOfNoReturnRegex.test(path) ||
-      !pathsAllowedRegex.test(path) ||
-      /hot/i.test(req.path)
-    ) {
-      return next();
-    }
-    req.session.returnTo = req.originalUrl.includes('/map')
-      ? '/'
-      : req.originalUrl;
-    return next();
-  };
-}

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

@@ -0,0 +1,23 @@
+const jwt = require('jsonwebtoken');
+const { allowedOrigins } = require('../../../config/cors-settings');
+const { homeLocation } = require('../../../config/env.json');
+
+function getReturnTo(encryptedReturnTo, secret) {
+  let returnTo;
+  let success = false;
+  try {
+    returnTo = jwt.verify(encryptedReturnTo, secret).returnTo;
+    // 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 {
+    returnTo = `${homeLocation}/learn`;
+  }
+
+  return { returnTo, success };
+}
+
+module.exports = getReturnTo;

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

@@ -0,0 +1,54 @@
+/* global describe expect it */
+
+const { homeLocation } = require('../../../config/env.json');
+const jwt = require('jsonwebtoken');
+
+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`;
+
+describe('get-return-to', () => {
+  describe('getReturnTo', () => {
+    it('should extract returnTo from a jwt', () => {
+      expect.assertions(1);
+
+      const encryptedReturnTo = jwt.sign(
+        { returnTo: validReturnTo },
+        validJWTSecret
+      );
+      expect(getReturnTo(encryptedReturnTo, validJWTSecret)).toStrictEqual({
+        returnTo: validReturnTo,
+        success: true
+      });
+    });
+
+    it('should return a default url if the secrets do not match', () => {
+      expect.assertions(1);
+
+      const encryptedReturnTo = jwt.sign(
+        { returnTo: validReturnTo },
+        invalidJWTSecret
+      );
+      expect(getReturnTo(encryptedReturnTo, validJWTSecret)).toStrictEqual({
+        returnTo: defaultReturnTo,
+        success: false
+      });
+    });
+
+    it('should return a default url for unknown origins', () => {
+      expect.assertions(1);
+      const encryptedReturnTo = jwt.sign(
+        { returnTo: invalidReturnTo },
+        validJWTSecret
+      );
+      expect(getReturnTo(encryptedReturnTo, validJWTSecret)).toStrictEqual({
+        returnTo: defaultReturnTo,
+        success: false
+      });
+    });
+  });
+});

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 } from '../../config/env.json';
+import { apiLocation, homeLocation } from '../../config/env.json';
 import {
   signInLoadingSelector,
   userSelector,
@@ -167,7 +167,7 @@ export function ShowSettings(props) {
   }
 
   if (!isSignedIn) {
-    navigate(`${apiLocation}/signin?returnTo=settings`);
+    navigate(`${apiLocation}/signin?returnTo=${homeLocation}/settings`);
     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 } from '../../config/env.json';
+import { apiLocation, homeLocation } from '../../config/env.json';
 
 import { ShowSettings } from './ShowSettings';
 
@@ -23,7 +23,7 @@ describe('<ShowSettings />', () => {
     shallow.render(<ShowSettings {...loggedOutProps} />);
     expect(navigate).toHaveBeenCalledTimes(1);
     expect(navigate).toHaveBeenCalledWith(
-      `${apiLocation}/signin?returnTo=settings`
+      `${apiLocation}/signin?returnTo=${homeLocation}/settings`
     );
     const result = shallow.getRenderOutput();
     // Renders Loader rather than ShowSettings