Merge google/oidc user authentication and surface user relevant error messages during authentication

packages/auth/src/middleware/passport/google.js

@@ -1,75 +1,24 @@
-const env = require("../../environment")
-const jwt = require("jsonwebtoken")
-const database = require("../../db")
 const GoogleStrategy = require("passport-google-oauth").OAuth2Strategy
-const {
-  StaticDatabases,
-  generateGlobalUserID,
-  ViewNames,
-} = require("../../db/utils")
 
-async function authenticate(token, tokenSecret, profile, done) {
+const { authenticateThirdParty } = require("./third-party-common")
-  // Check the user exists in the instance DB by email
+
-  const db = database.getDB(StaticDatabases.GLOBAL.name)
+async function authenticate(accessToken, refreshToken, profile, done) {
-
+  const thirdPartyUser = {
-  let dbUser
+    provider: profile.provider, // should always be 'google'
-
+    providerType: "google",
-  const userId = generateGlobalUserID(profile.id)
+    userId: profile.id,
-
+    profile: profile,
-  try {
+    email: profile._json.email,
-    // use the google profile id
+    oauth2: {
-    dbUser = await db.get(userId)
+      accessToken: accessToken,
-  } catch (err) {
+      refreshToken: refreshToken
-    const user = {
-      _id: userId,
-      provider: profile.provider,
-      roles: {},
-      ...profile._json,
-    }
-
-    // check if an account with the google email address exists locally
-    const users = await db.query(`database/${ViewNames.USER_BY_EMAIL}`, {
-      key: profile._json.email,
-      include_docs: true,
-    })
-
-    // Google user already exists by email
-    if (users.rows.length > 0) {
-      const existing = users.rows[0].doc
-
-      // remove the local account to avoid conflicts
-      await db.remove(existing._id, existing._rev)
-
-      // merge with existing account
-      user.roles = existing.roles
-      user.builder = existing.builder
-      user.admin = existing.admin
-
-      const response = await db.post(user)
-      dbUser = user
-      dbUser._rev = response.rev
-    } else {
-      return done(
-        new Error(
-          "email does not yet exist. You must set up your local budibase account first."
-        ),
-        false
-      )
     }
   }
 
-  // authenticate
+  return authenticateThirdParty(
-  const payload = {
+    thirdPartyUser, 
-    userId: dbUser._id,
+    true, // require local accounts to exist
-    builder: dbUser.builder,
+    done)
-    email: dbUser.email,
-  }
-
-  dbUser.token = jwt.sign(payload, env.JWT_SECRET, {
-    expiresIn: "1 day",
-  })
-
-  return done(null, dbUser)
 }
 
 /**

packages/auth/src/middleware/passport/jwt.js

@@ -1,5 +1,6 @@
 const { Cookies } = require("../../constants")
 const env = require("../../environment")
+const { authError } = require("./utils")
 
 exports.options = {
   secretOrKey: env.JWT_SECRET,
@@ -12,6 +13,6 @@ exports.authenticate = async function (jwt, done) {
   try {
     return done(null, jwt)
   } catch (err) {
-    return done(new Error("JWT invalid."), false)
+    return authError(done, "JWT invalid", err)
   }
 }

packages/auth/src/middleware/passport/local.js

@@ -3,6 +3,7 @@ const { UserStatus } = require("../../constants")
 const { compare } = require("../../hashing")
 const env = require("../../environment")
 const { getGlobalUserByEmail } = require("../../utils")
+const { authError } = require("./utils")
 
 const INVALID_ERR = "Invalid Credentials"
 
@@ -16,17 +17,17 @@ exports.options = {}
  * @returns The authenticated user, or errors if they occur
  */
 exports.authenticate = async function (email, password, done) {
-  if (!email) return done(null, false, "Email Required.")
+  if (!email) return authError(done, "Email Required")
-  if (!password) return done(null, false, "Password Required.")
+  if (!password) return authError(done, "Password Required")
 
   const dbUser = await getGlobalUserByEmail(email)
   if (dbUser == null) {
-    return done(null, false, { message: "User not found" })
+    return authError(done, "User not found")
   }
 
   // check that the user is currently inactive, if this is the case throw invalid
   if (dbUser.status === UserStatus.INACTIVE) {
-    return done(null, false, { message: INVALID_ERR })
+    return authError(done, INVALID_ERR)
   }
 
   // authenticate
@@ -43,6 +44,6 @@ exports.authenticate = async function (email, password, done) {
 
     return done(null, dbUser)
   } else {
-    done(new Error(INVALID_ERR), false)
+    return authError(done, INVALID_ERR)
   }
 }

packages/auth/src/middleware/passport/oidc.js

@@ -1,28 +1,18 @@
-const env = require("../../environment")
-const jwt = require("jsonwebtoken")
-const database = require("../../db")
 const fetch = require("node-fetch")
 const OIDCStrategy = require("@techpass/passport-openidconnect").Strategy
-const {
+const { authenticateThirdParty } = require("./third-party-common")
-  StaticDatabases,
-  generateGlobalUserID,
-  ViewNames,
-} = require("../../db/utils")
 
 /**
- * Attempt to parse the users email address. 
- * 
- * It is not guaranteed that the email will be returned by the user info endpoint (e.g. github connected account used in azure ad).
- * Fallback to the id token where possible.
- * 
  * @param {*} profile The structured profile created by passport using the user info endpoint
- * @param {*} jwtClaims The raw claims returned in the id token
+ * @param {*} jwtClaims The claims returned in the id token
  */
 function getEmail(profile, jwtClaims) {
+  // profile not guaranteed to contain email e.g. github connected azure ad account
   if (profile._json.email) {
     return profile._json.email
   }
 
+  // fallback to id token
   if (jwtClaims.email) {
     return jwtClaims.email
   }
@@ -31,7 +21,6 @@ function getEmail(profile, jwtClaims) {
 }
 
 /**
- * 
  * @param {*} issuer The identity provider base URL
  * @param {*} sub The user ID
  * @param {*} profile The user profile information. Created by passport from the /userinfo response
@@ -54,67 +43,23 @@ async function authenticate(
   params,
   done
 ) {
-  // Check the user exists in the instance DB by email
+  const thirdPartyUser = {
-  const db = database.getDB(StaticDatabases.GLOBAL.name)
+    // store the issuer info to enable sync in future
-
+    provider: issuer, 
-  let dbUser
+    providerType: "oidc",
-
+    userId: profile.id,
-  const userId = generateGlobalUserID(profile.id)
+    profile: profile,
-
+    email: getEmail(profile, jwtClaims),
-  try {
+    oauth2: {
-    // use the OIDC profile id
+      accessToken: accessToken,
-    dbUser = await db.get(userId)
+      refreshToken: refreshToken
-  } catch (err) {
-    const user = {
-      _id: userId,
-      provider: profile.provider,
-      roles: {},
-      ...profile._json,
-    }
-
-    // check if an account with the OIDC email address exists locally
-    const email = getEmail(profile, jwtClaims)
-    if (!email) {
-      return done(null, false, { message: "No email address found" })
-    }
-
-    const users = await db.query(`database/${ViewNames.USER_BY_EMAIL}`, {
-      key: email,
-      include_docs: true,
-    })
-
-    // OIDC user already exists by email
-    if (users.rows.length > 0) {
-      const existing = users.rows[0].doc
-
-      // remove the local account to avoid conflicts
-      await db.remove(existing._id, existing._rev)
-
-      // merge with existing account
-      user.roles = existing.roles
-      user.builder = existing.builder
-      user.admin = existing.admin
-
-      const response = await db.post(user)
-      dbUser = user
-      dbUser._rev = response.rev
-    } else {
-      return done(null, false, { message: "Email does not yet exist. You must set up your local budibase account first." })
     }
   }
 
-  // authenticate
+  return authenticateThirdParty(
-  const payload = {
+    thirdPartyUser, 
-    userId: dbUser._id,
+    false, // don't require local accounts to exist
-    builder: dbUser.builder,
+    done)
-    email: dbUser.email,
-  }
-
-  dbUser.token = jwt.sign(payload, env.JWT_SECRET, {
-    expiresIn: "1 day",
-  })
-
-  return done(null, dbUser)
 }
 
 /**

packages/auth/src/middleware/passport/third-party-common.js

@@ -0,0 +1,124 @@
+const env = require("../../environment")
+const jwt = require("jsonwebtoken")
+const database = require("../../db")
+const {
+  StaticDatabases,
+  generateGlobalUserID,
+  ViewNames,
+} = require("../../db/utils")
+const { authError } = require("./utils")
+
+/**
+ * Common authentication logic for third parties. e.g. OAuth, OIDC. 
+ */
+exports.authenticateThirdParty = async function (
+    thirdPartyUser,
+    requireLocalAccount = true,
+    done
+  ) {
+    if (!thirdPartyUser.provider) return authError(done, "third party user provider required")
+    if (!thirdPartyUser.userId) return authError(done, "third party user id required")
+    if (!thirdPartyUser.email) return authError(done, "third party user email required")
+    
+    const db = database.getDB(StaticDatabases.GLOBAL.name)
+  
+    let dbUser
+    
+    // use the third party id
+    const userId = generateGlobalUserID(thirdPartyUser.userId)  
+  
+    try {
+      dbUser = await db.get(userId)
+    } catch (err) {
+      // abort when not 404 error
+      if (!err.status || err.status !== 404) {
+        return authError(done, "Unexpected error when retrieving existing user", err)
+      }
+  
+      // check user already exists by email
+      const users = await db.query(`database/${ViewNames.USER_BY_EMAIL}`, {
+        key: thirdPartyUser.email,
+        include_docs: true,
+      })
+      let userExists = users.rows.length > 0
+
+      if (requireLocalAccount && !userExists) {
+        return authError(done, "Email does not yet exist. You must set up your local budibase account first.")         
+      }
+
+      // create the user to save
+      let user
+      if (userExists) {
+        const existing = users.rows[0].doc
+        user = constructMergedUser(userId, existing, thirdPartyUser)
+        
+        // remove the local account to avoid conflicts
+        await db.remove(existing._id, existing._rev)
+      } else {
+        user = constructNewUser(userId, thirdPartyUser)
+      }
+
+      // save the user
+      const response = await db.post(user)
+      dbUser = user
+      dbUser._rev = response.rev
+    }
+  
+    // authenticate
+    const payload = {
+      userId: dbUser._id,
+      builder: dbUser.builder,
+      email: dbUser.email,
+    }
+  
+    dbUser.token = jwt.sign(payload, env.JWT_SECRET, {
+      expiresIn: "1 day",
+    })
+  
+    return done(null, dbUser)
+}
+
+/**
+ * @returns a user object constructed from existing and third party information
+ */
+function constructMergedUser(userId, existing, thirdPartyUser) {
+  const user = constructNewUser(userId, thirdPartyUser)
+
+  // merge with existing account
+  user.roles = existing.roles
+  user.builder = existing.builder
+  user.admin = existing.admin
+
+  return user
+}
+
+/**
+ * @returns a user object constructed from third party information
+ */
+function constructNewUser(userId, thirdPartyUser) {
+  const user = {
+    _id: userId,
+    provider: thirdPartyUser.provider,
+    providerType: thirdPartyUser.providerType,
+    roles: {}
+  }
+
+  // persist profile information
+  // @reviewers: Historically stored at the root level of the user
+  //             Nest to prevent conflicts with future fields
+  //             Is this okay to change? 
+  if (thirdPartyUser.profile) {
+    user.thirdPartyProfile = {
+      ...thirdPartyUser.profile._json
+    }
+  }
+
+  // persist oauth tokens for future use 
+  if (thirdPartyUser.oauth2) {
+    user.oauth2 = {
+        ...thirdPartyUser.oauth2
+    }
+  }
+
+  return user
+}

packages/auth/src/middleware/passport/utils.js

@@ -0,0 +1,14 @@
+/**
+ * Utility to handle authentication errors. 
+ * 
+ * @param {*} done The passport callback. 
+ * @param {*} message Message that will be returned in the response body
+ * @param {*} err (Optional) error that will be logged
+ */
+exports.authError = function (done, message, err = null) {
+    return done(
+        err, 
+        null, // never return a user
+        { message: message }
+    )
+}

packages/worker/src/api/controllers/admin/auth.js

@@ -13,6 +13,7 @@ const GLOBAL_DB = authPkg.StaticDatabases.GLOBAL.name
 
 function authInternal(ctx, user, err = null, info = null) {
   if (err) {
+    console.error("Authentication error", err)
     return ctx.throw(403, info? info : "Unauthorized")
   }
 
@@ -32,8 +33,8 @@ function authInternal(ctx, user, err = null, info = null) {
 }
 
 exports.authenticate = async (ctx, next) => {
-  return passport.authenticate("local", async (err, user) => {
+  return passport.authenticate("local", async (err, user, info) => {
-    authInternal(ctx, user, err)
+    authInternal(ctx, user, err, info)
 
     delete user.token
 
@@ -123,8 +124,8 @@ exports.googleAuth = async (ctx, next) => {
   return passport.authenticate(
     strategy,
     { successRedirect: "/", failureRedirect: "/error" },
-    async (err, user) => {
+    async (err, user, info) => {
-      authInternal(ctx, user, err)
+      authInternal(ctx, user, err, info)
 
       ctx.redirect("/")
     }