fix: redundant authentication code

lib/new-admin/graphql/modules/authentication.js

@@ -9,7 +9,7 @@ const authErrors = require('../errors/authentication')
 
 const REMEMBER_ME_AGE = 90 * T.day
 
-function authenticateUser(username, password) {
+const authenticateUser = (username, password) => {
   return users.getUserByUsername(username)
     .then(user => {
       const hashedPassword = user.password
@@ -26,26 +26,50 @@ function authenticateUser(username, password) {
     })
 }
 
+const destroySessionIfSameUser = (context, user) => {
+  const sessionUser = getUserFromCookie(context)
+  if (sessionUser && user.id === sessionUser.id)
+    context.req.session.destroy()
+}
+
+const destroySessionIfBeingUsed = (sessID, context) => {
+  if (sessID === context.req.session.id) {
+    context.req.session.destroy()
+  }
+}
+
+const getUserFromCookie = context => {
+  return context.req.session.user
+}
+
+const getLamassuCookie = context => {
+  return context.req.cookies && context.req.cookies.lid
+}
+
+const initializeSession = (context, user, rememberMe) => {
+  const finalUser = { id: user.id, username: user.username, role: user.role }
+  context.req.session.user = finalUser
+  if (rememberMe) context.req.session.cookie.maxAge = REMEMBER_ME_AGE
+}
+
 const getUserData = context => {
-  const lidCookie = context.req.cookies && context.req.cookies.lid
+  const lidCookie = getLamassuCookie(context)
   if (!lidCookie) return
 
-  const user = context.req.session.user
+  const user = getUserFromCookie(context)
   return user
 }
 
 const get2FASecret = (username, password) => {
   return authenticateUser(username, password).then(user => {
-    if (!user) throw new authErrors.InvalidCredentialsError()
-
     const secret = otplib.authenticator.generateSecret()
-    const otpauth = otplib.authenticator.keyuri(username, 'Lamassu Industries', secret)
+    const otpauth = otplib.authenticator.keyuri(user.username, 'Lamassu Industries', secret)
     return { secret, otpauth }
   })
 }
 
 const confirm2FA = (token, context) => {
-  const requestingUser = context.req.session.user
+  const requestingUser = getUserFromCookie(context)
 
   if (!requestingUser) throw new authErrors.InvalidCredentialsError()
 
@@ -94,56 +118,38 @@ const validateReset2FALink = token => {
 }
 
 const deleteSession = (sessionID, context) => {
-  if (sessionID === context.req.session.id) {
+  destroySessionIfBeingUsed(sessionID, context)
-    context.req.session.destroy()
-  }
   return sessionManager.deleteSessionById(sessionID)
 }
 
 const login = (username, password) => {
   return authenticateUser(username, password).then(user => {
-    if (!user) throw new authErrors.InvalidCredentialsError()
+    const twoFASecret = user.twofa_code
-    return users.getUserById(user.id).then(user => {
+    return twoFASecret ? 'INPUT2FA' : 'SETUP2FA'
-      const twoFASecret = user.twofa_code
-      return twoFASecret ? 'INPUT2FA' : 'SETUP2FA'
-    })
   })
 }
 
 const input2FA = (username, password, rememberMe, code, context) => {
   return authenticateUser(username, password)
-    .then(user => {
-      if (!user) throw new authErrors.InvalidCredentialsError()
-      return users.getUserById(user.id)
-    })
     .then(user => {
       const secret = user.twofa_code
       const isCodeValid = otplib.authenticator.verify({ token: code, secret: secret })
       if (!isCodeValid) throw new authErrors.InvalidTwoFactorError()
 
-      const finalUser = { id: user.id, username: user.username, role: user.role }
+      initializeSession(context, user, rememberMe)
-      context.req.session.user = finalUser
-      if (rememberMe) context.req.session.cookie.maxAge = REMEMBER_ME_AGE
-
       return true
     })
 }
 
 const setup2FA = (username, password, rememberMe, secret, codeConfirmation, context) => {
+  if (!secret) throw new authErrors.InvalidTwoFactorError()
+
+  const isCodeValid = otplib.authenticator.verify({ token: codeConfirmation, secret: secret })
+  if (!isCodeValid) throw new authErrors.InvalidTwoFactorError()
+
   return authenticateUser(username, password)
     .then(user => {
-      if (!user || !secret) throw new authErrors.InvalidCredentialsError()
+      initializeSession(context, user, rememberMe)
-
-      const isCodeValid = otplib.authenticator.verify({ token: codeConfirmation, secret: secret })
-      if (!isCodeValid) throw new authErrors.InvalidTwoFactorError()
-
-      return users.getUserById(user.id)
-    })
-    .then(user => {
-      const finalUser = { id: user.id, username: user.username, role: user.role }
-      context.req.session.user = finalUser
-      if (rememberMe) context.req.session.cookie.maxAge = REMEMBER_ME_AGE
-
       return users.save2FASecret(user.id, secret)
     })
     .then(() => true)
@@ -152,70 +158,71 @@ const setup2FA = (username, password, rememberMe, secret, codeConfirmation, cont
 const changeUserRole = (code, id, newRole, context) => {
   const action = (id, newRole) => users.changeUserRole(id, newRole)
 
-  if (!code) {
+  return users.getUserById(id)
-    return action(id, newRole)
+    .then(user => {
-  }
+      if (user.role !== 'superuser') {
-
+        return action(id, newRole)
-  return confirm2FA(code, context)
+      }
+    
+      return confirm2FA(code, context)
+    })
     .then(() => action(id, newRole))
 }
 
 const enableUser = (code, id, context) => {
   const action = id => users.enableUser(id)
 
-  if (!code) {
+  return users.getUserById(id)
-    return action(id)
+    .then(user => {
-  }
+      if (user.role !== 'superuser') {
-
+        return action(id)
-  return confirm2FA(code, context)
+      }
+    
+      return confirm2FA(code, context)
+    })
     .then(() => action(id))
 }
 
 const disableUser = (code, id, context) => {
   const action = id => users.disableUser(id)
 
-  if (!code) {
+  return users.getUserById(id)
-    return action(id)
+    .then(user => {
-  }
+      if (user.role !== 'superuser') {
-
+        return action(id)
-  return confirm2FA(code, context)
+      }
+    
+      return confirm2FA(code, context)
+    })
     .then(() => action(id))
 }
 
 const createResetPasswordToken = (code, userID, context) => {
-  const action = userID => {
+  const action = user => users.createAuthToken(user.id, 'reset_password')
-    return users.getUserById(userID)
-      .then(user => {
-        if (!user) throw new authErrors.InvalidCredentialsError()
-        return users.createAuthToken(user.id, 'reset_password')
-      })
-      .catch(err => console.error(err))
-  }
 
-  if (!code) {
+  return users.getUserById(userID)
-    return action(userID)
+    .then(user => {
-  }
+      if (user.role !== 'superuser') {
-
+        return action(user.id)
-  return confirm2FA(code, context)
+      }
-    .then(() => action(userID))
+    
+      return confirm2FA(code, context)
+        .then(() => action(user))
+    })
 }
 
 const createReset2FAToken = (code, userID, context) => {
-  const action = userID => {
+  const action = user => users.createAuthToken(user.id, 'reset_twofa')
-    return users.getUserById(userID)
-      .then(user => {
-        if (!user) throw new authErrors.InvalidCredentialsError()
-        return users.createAuthToken(user.id, 'reset_twofa')
-      })
-      .catch(err => console.error(err))
-  }
 
-  if (!code) {
+  return users.getUserById(userID)
-    return action(userID)
+    .then(user => {
-  }
+      if (user.role !== 'superuser') {
-
+        return action(user)
-  return confirm2FA(code, context)
+      }
-    .then(() => action(userID))
+    
+      return confirm2FA(code, context)
+        .then(() => action(user))
+    })
 }
 
 const createRegisterToken = (username, role) => {
@@ -240,8 +247,7 @@ const register = (token, username, password, role) => {
 const resetPassword = (token, userID, newPassword, context) => {
   return users.getUserById(userID)
     .then(user => {
-      if (!user) throw new authErrors.InvalidCredentialsError()
+      destroySessionIfSameUser(context, user)
-      if (context.req.session.user && user.id === context.req.session.user.id) context.req.session.destroy()
       return users.updatePassword(token, user.id, newPassword)
     })
     .then(() => true)
@@ -254,7 +260,7 @@ const reset2FA = (token, userID, code, secret, context) => {
 
   return users.getUserById(userID)
     .then(user => {
-      if (context.req.session.user && user.id === context.req.session.user.id) context.req.session.destroy()
+      destroySessionIfSameUser(context, user)
       return users.reset2FASecret(token, user.id, secret).then(() => true)
     })
     .catch(err => console.error(err))

lib/new-admin/services/login.js

@@ -1,15 +1,14 @@
 const db = require('../../db')
 
 function validateUser (username, password) {
-  const sql = 'SELECT id, username FROM users WHERE username=$1 AND password=$2'
+  return db.tx(t => {
-  const sqlUpdateLastAccessed = 'UPDATE users SET last_accessed = now() WHERE username=$1'
+    const q1 = t.one('SELECT * FROM users WHERE username=$1 AND password=$2', [username, password])
+    const q2 = t.none('UPDATE users SET last_accessed = now() WHERE username=$1', [username])
 
-  return db.one(sql, [username, password])
+    return t.batch([q1, q2])
-    .then(user => {
+      .then(([user, _]) => user)
-      return db.none(sqlUpdateLastAccessed, [user.username])
+      .catch(() => false)
-        .then(() => user)
+  })
-    })
-    .catch(() => false)
 }
 
 module.exports = {

lib/users.js

@@ -54,14 +54,14 @@ function getUsers () {
 
 function verifyAndUpdateUser (id, ua, ip) {
   const sql = `SELECT id, username, role, enabled FROM users WHERE id=$1 limit 1`
-  return db.oneOrNone(sql, [id]).then(user => {
+  return db.oneOrNone(sql, [id])
-    if (!user) return null
+    .then(user => {
+      if (!user) return null
 
-    const sql2 = `UPDATE users SET last_accessed=now(), last_accessed_from=$1, last_accessed_address=$2 WHERE id=$3 RETURNING id, role, enabled`
+      const sql2 = `UPDATE users SET last_accessed=now(), last_accessed_from=$1, last_accessed_address=$2 WHERE id=$3 RETURNING id, role, enabled`
-    return db.one(sql2, [ua, ip, id]).then(user => {
+      return db.one(sql2, [ua, ip, id])
-      return user
     })
-  })
+    .then(user => user)
 }
 
 function save2FASecret (id, secret) {