diff --git a/api-server/server/boot/authentication.js b/api-server/server/boot/authentication.js
index 19f01f10e63..636667f2ea9 100644
--- a/api-server/server/boot/authentication.js
+++ b/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,
- loginRedirect
+ devSaveResponseAuthCookies,
+ 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 loginSuccessRedirect = loginRedirect();
+ const devSaveAuthCookies = devSaveResponseAuthCookies();
+ 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,
- loginSuccessRedirect
+ devSaveAuthCookies,
+ devLoginSuccessRedirect
);
} else {
- api.get(
- '/signin',
- (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('/signin', ifUserRedirect, (req, res, next) => {
+ const state = jwt.sign({ returnTo: req.query.returnTo }, jwtSecret);
+ return passport.authenticate('auth0-login', { state })(req, res, next);
+ });
api.get(
'/auth/auth0/callback',
diff --git a/api-server/server/component-passport.js b/api-server/server/component-passport.js
index fa12146774b..1b7ad1ffb73 100644
--- a/api-server/server/component-passport.js
+++ b/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) {
- delete req.session.returnTo;
- return `${homeLocation}/learn`;
+ if (req && req.query && req.query.returnTo) {
+ return req.query.returnTo;
}
return `${homeLocation}/learn`;
};
@@ -101,10 +102,14 @@ export const createPassportCallbackAuthenticator = (strategy, config) => (
res,
next
) => {
- const returnTo =
- req && req.query && req.query.state
- ? Buffer.from(req.query.state, 'base64').toString('utf-8')
- : `${homeLocation}/learn`;
+ 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 },
@@ -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`);
}
diff --git a/api-server/server/middleware.json b/api-server/server/middleware.json
index 445290954d6..3588498b119 100644
--- a/api-server/server/middleware.json
+++ b/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": {}
},
diff --git a/api-server/server/middlewares/add-return-to.js b/api-server/server/middlewares/add-return-to.js
deleted file mode 100644
index 47d7bbfc03b..00000000000
--- a/api-server/server/middlewares/add-return-to.js
+++ /dev/null
@@ -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();
- };
-}
diff --git a/api-server/server/utils/get-return-to.js b/api-server/server/utils/get-return-to.js
new file mode 100644
index 00000000000..ba90ea46108
--- /dev/null
+++ b/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;
diff --git a/api-server/server/utils/get-return-to.test.js b/api-server/server/utils/get-return-to.test.js
new file mode 100644
index 00000000000..c535e687f07
--- /dev/null
+++ b/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
+ });
+ });
+ });
+});
diff --git a/client/src/client-only-routes/ShowSettings.js b/client/src/client-only-routes/ShowSettings.js
index e6ade34b0f8..dc23eb839bc 100644
--- a/client/src/client-only-routes/ShowSettings.js
+++ b/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 ;
}
diff --git a/client/src/client-only-routes/ShowSettings.test.js b/client/src/client-only-routes/ShowSettings.test.js
index 91051a0c804..3e7cd4ec96d 100644
--- a/client/src/client-only-routes/ShowSettings.test.js
+++ b/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('', () => {
shallow.render();
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