fix(Google Login): Prevents uninvited users gaining access (#1517 - LLC-10)

#1517
https://learningpool.atlassian.net/browse/LLC-10

Author: crazy-grizzly

api/src/auth/jwt.js

@@ -53,7 +53,7 @@ const payloadDefaults = ({
 /**
  * @param {*} - payload
  * @param {*} opts - (optional)
- * @returns {Promise<any>}
+ * @returns {Promise<string>}
  */
 const createJWT = ({
   userId,
@@ -84,7 +84,7 @@ const createUserTokenPayload = (user, provider) => payloadDefaults({
 /**
  * @param {*} user
  * @param {*} provider
- * @returns {Promise<any>}
+ * @returns {Promise<string>}
  */
 const createUserJWT = (user, provider) =>
   createJWT(
@@ -103,7 +103,7 @@ const createUserRefreshTokenPayload = (user, provider) => payloadDefaults({
 /**
  * @param {*} user
  * @param {*} provider
- * @returns {Promise<any>}
+ * @returns {Promise<string>}
  */
 const createUserRefreshJWT = (user, provider) =>
   createJWT(
@@ -150,7 +150,7 @@ const createOrgRefreshTokenPayload = (user, organisationId, provider) => payload
  * @param {*} user
  * @param {*} organisationId
  * @param {*} provider
- * @returns {Promise<any>}
+ * @returns {Promise<string>}
  */
 const createOrgRefreshJWT = (user, organisationId, provider) =>
   createJWT(

api/src/auth/passport.js

@@ -212,13 +212,25 @@ if (
           profile.emails,
           email => email.verified === true
         );
-        User.findOrCreate({ email: userEmail.value }, (err, user) => {
-          assert.ifError(err);
-          user.googleId = profile.id;
-          user.imageUrl = get(profile, 'photos.0.value');
-          user.name = profile.displayName;
-          user.save((err, savedUser) => done(err, savedUser));
-        });
+
+        User.findOne(
+          {
+            email: userEmail.value
+          },
+          (err, user) => {
+            assert.ifError(err);
+
+            if (!user) {
+              return done(null, false, { message: 'User does not exist' });
+            }
+
+            user.googleId = profile.id;
+            user.imageUrl = get(profile, 'photos.0.value');
+            user.name = profile.displayName;
+
+            user.save((err, savedUser) => done(err, savedUser));
+          },
+        );
       }
     )
   );

api/src/controllers/AuthController.js

@@ -2,6 +2,7 @@ import passport from 'passport';
 import jsonwebtoken from 'jsonwebtoken';
 import { v4 as uuid } from 'uuid';
 import ms from 'ms';
+import status from 'http-status';
 import logger from 'lib/logger';
 import User from 'lib/models/user';
 import OAuthToken from 'lib/models/oAuthToken';
@@ -261,12 +262,18 @@ const jwtOrganisation = (req, res) => {
   }
 };
 
-const googleSuccess = (req, res) => {
+const googleSuccess = (user, response) => {
   // we have successfully signed into google
   // create a JWT and set it in the query params (only way to return it with a redirect)
-  createUserJWT(req.user, 'google')
-    .then(token => res.redirect(`/api${AUTH_JWT_SUCCESS}?access_token=${token}`))
-    .catch(err => res.status(500).send(err));
+  createUserJWT(user, 'google')
+    .then(token => response.redirect(`/api${AUTH_JWT_SUCCESS}?access_token=${token}`))
+    .catch(
+      (err) => {
+        response
+          .status(status.INTERNAL_SERVER_ERROR)
+          .send(err);
+      }
+    );
 };
 
 const clientInfo = (req, res) => {

api/src/routes/HttpRoutes.js

@@ -117,6 +117,13 @@ router.get(
   AuthController.clientInfo
 );
 
+router.get(
+  routes.OAUTH2_FAILED,
+  (request, response) => {
+    response.send('Authorization failed');
+  },
+);
+
 router.post(
   routes.OAUTH2_TOKEN,
   AuthController.issueOAuth2AccessToken
@@ -136,10 +143,26 @@ if (process.env.GOOGLE_ENABLED) {
     routes.AUTH_JWT_GOOGLE,
     passport.authenticate('google', GOOGLE_AUTH_OPTIONS)
   );
+
   router.get(
     routes.AUTH_JWT_GOOGLE_CALLBACK,
-    passport.authenticate('google', DEFAULT_PASSPORT_OPTIONS),
-    AuthController.googleSuccess
+    (request, response, next) => {
+      passport.authenticate(
+        'google',
+        DEFAULT_PASSPORT_OPTIONS,
+        (error, user, info) => {
+          const defaultErrorMessage = 'Something bad happened';
+
+          if (!user) {
+            response.redirect(`/api${routes.OAUTH2_FAILED}?error=${get(info, 'message', defaultErrorMessage)}`);
+
+            return;
+          }
+
+          AuthController.googleSuccess(user, response);
+        },
+      )(request, response, next);
+    },
   );
 }
 

lib/constants/auth.js

@@ -19,8 +19,8 @@ import getAuthFromRequest from 'lib/helpers/getAuthFromRequest';
 
 export const GOOGLE_AUTH_OPTIONS = {
   scope: ['https://www.googleapis.com/auth/plus.login', 'email'],
-  approvalPrompt: 'auto',
-  session: false
+  session: false,
+  prompt: 'select_account'
 };
 
 export const DEFAULT_PASSPORT_OPTIONS = {

lib/constants/routes.js

@@ -15,6 +15,7 @@ export const AUTH_JWT_SUCCESS = '/auth/jwt/success';
 export const AUTH_CLIENT_INFO = '/auth/client/info';
 
 export const OAUTH2_TOKEN = '/oauth2/token';
+export const OAUTH2_FAILED = '/oauth2/failed';
 
 export const SENDSMS = '/sendsms';
 

lib/models/user.js

@@ -17,6 +17,73 @@ import getOrgFromAuthInfo from 'lib/services/auth/authInfoSelectors/getOrgFromAu
 import getUserIdFromAuthInfo from 'lib/services/auth/authInfoSelectors/getUserIdFromAuthInfo';
 import { validatePasswordUtil } from 'lib/utils/validators/User';
 
+/**
+ * @typedef {object} PasswordHistoryItem
+ *  @property {string} hash
+ *  @property {Date} date
+ */
+
+/**
+ * @typedef {object} ResetTokensItem
+ *  @property {string} token
+ *  @property {Date} expires
+ */
+
+/**
+ * @typedef {object} UserSettings
+ *  @property {boolean} CONFIRM_BEFORE_DELETE
+ */
+
+/**
+ * @typedef {object} OrganisationSettings
+ *  @property {object} organisation - TODO: define type
+ *  @property {string[]} scopes
+ *  @property {object[]} roles - TODO: define type
+ *  @property {string} filter
+ *  @property {string} oldFilter
+ *  @property {string} timezone
+ */
+
+/**
+ * @typedef {object} OwnerOrganisationSettings
+ *  @property {boolean} LOCKOUT_ENABLED
+ *  @property {number} LOCKOUT_ATTEMPS
+ *  @property {number} LOCKOUT_SECONDS
+ *  @property {boolean} PASSWORD_HISTORY_CHECK
+ *  @property {number} PASSWORD_HISTORY_TOTAL
+ *  @property {number} PASSWORD_MIN_LENGTH
+ *  @property {boolean} PASSWORD_REQUIRE_ALPHA
+ *  @property {boolean} PASSWORD_REQUIRE_NUMBER
+ *  @property {boolean} PASSWORD_USE_CUSTOM_REGEX
+ *  @property {string} PASSWORD_CUSTOM_REGEX
+ *  @property {string} PASSWORD_CUSTOM_MESSAGE
+ */
+
+/**
+ * Plain object structure without mongoose model methods
+ * @typedef {object} User
+ *  @property {string} name
+ *  @property {string} email
+ *  @property {object[]} organisations - TODO: define type
+ *  @property {OrganisationSettings[]} organisationSettings
+ *  @property {string} imageUrl
+ *  @property {string} googleId
+ *  @property {string} password
+ *  @property {object} ownerOrganisation - TODO: define type
+ *  @property {OwnerOrganisationSettings} ownerOrganisationSettings
+ *  @property {UserSettings} settings
+ *  @property {string[]} scopes
+ *  @property {boolean} verified
+ *  @property {ResetTokensItem[]} resetTokens
+ *  @property {PasswordHistoryItem[]} passwordHistory
+ *  @property {Date} authLastAttempt
+ *  @property {number} authFailedAttempts
+ *  @property {Date} authLockoutExpiry
+ *  @property {boolean} hasBeenMigrated
+ */
+
+/** @typedef {module:mongoose.Model<User>} UserModel */
+
 /**
  * @param {string} value
  * @returns {true} if validation is success
@@ -63,6 +130,7 @@ async function validatePassword(value) {
 }
 
 // TODO: Remove `organisationSettings.oldFilter` and `hasBeenMigrated` after we confirm success of $in migration
+/** @class UserSchema */
 const schema = new mongoose.Schema({
   name: { type: String },
   email: {
@@ -110,6 +178,7 @@ const schema = new mongoose.Schema({
   // "owned" users when the organisation's settings are updated
   ownerOrganisationSettings: {
     LOCKOUT_ENABLED: { type: Boolean, default: true },
+    // TODO: fix typo 🤨
     LOCKOUT_ATTEMPS: { type: Number, default: 5 }, // number of attempts before lock out
     LOCKOUT_SECONDS: { type: Number, default: 1800 }, // 30 minute lock out period
 

package.json

@@ -87,6 +87,7 @@
     "highland": "^2.8.1",
     "html-to-text": "^2.1.0",
     "http-proxy": "^1.12.0",
+    "http-status": "^1.4.2",
     "immutable": "^3.8.1",
     "ioredis": "^3.2.2",
     "js-cookie": "^2.1.3",

ui/src/containers/App/index.js

@@ -10,6 +10,7 @@ import { routeNodeSelector } from 'redux-router5';
 import { startsWithSegment } from 'router5.helpers';
 import get from 'lodash/get';
 import createAsyncComponent from 'ui/utils/createAsyncComponent';
+import SaveBarErrors from 'ui/containers/SaveBarErrors';
 
 const renderPage = (routeName) => {
   const testRoute = startsWithSegment(routeName);
@@ -62,6 +63,7 @@ const component = ({ route }) => {
       <Helmet {...config.app.head} />
       {renderPage(name)}
       <Toasts />
+      <SaveBarErrors />
     </div>
   );
 };

ui/src/pages/LoginPage/index.js

@@ -35,6 +35,14 @@ const enhance = compose(
   })
 );
 
+/**
+ * @param oAuthLoginStart - {@see reduxOAuthLoginStart}
+ * @param loginStart - {@see reduxLoginStart}
+ * @param loginRequestState - {@see loginRequestStateSelector}
+ * @param loginError - {@see loginErrorSelector}
+ * @param googleAuth - {@see getAppDataSelector}
+ * @returns {*}
+ */
 const render = ({
   oAuthLoginStart,
   loginStart,
@@ -52,8 +60,11 @@ const render = ({
     if (email && password) loginStart({ username: email, password }).catch(() => { });
   };
 
-  const onClickOAuthLogin = (e) => {
-    if (e) e.preventDefault();
+  const onClickOAuthLogin = (event) => {
+    if (event) {
+      event.preventDefault();
+    }
+
     oAuthLoginStart('google').catch(() => { });
   };