Merge pull request #2901 from Budibase/fix/custom-sso-enforce-cross-tenancy

Don't let user exist in multiple tenants when using custom sso

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

@@ -20,6 +20,10 @@ const getErrorMessage = () => {
   return done.mock.calls[0][2].message
 }
 
+const saveUser = async (user) => {
+  return await db.put(user)
+}
+
 describe("third party common", () => {
   describe("authenticateThirdParty", () => { 
     let thirdPartyUser
@@ -36,7 +40,7 @@ describe("third party common", () => {
   
     describe("validation", () => {
       const testValidation = async (message) => {
-        await authenticateThirdParty(thirdPartyUser, false, done)
+        await authenticateThirdParty(thirdPartyUser, false, done, saveUser)
         expect(done.mock.calls.length).toBe(1)
         expect(getErrorMessage()).toContain(message)
       }
@@ -78,7 +82,7 @@ describe("third party common", () => {
     describe("when the user doesn't exist", () => {
       describe("when a local account is required", () => {
         it("returns an error message", async () => {
-          await authenticateThirdParty(thirdPartyUser, true, done)
+          await authenticateThirdParty(thirdPartyUser, true, done, saveUser)
           expect(done.mock.calls.length).toBe(1)
           expect(getErrorMessage()).toContain("Email does not yet exist. You must set up your local budibase account first.")
         })
@@ -86,7 +90,7 @@ describe("third party common", () => {
       
       describe("when a local account isn't required", () => {
         it("creates and authenticates the user", async () => {
-          await authenticateThirdParty(thirdPartyUser, false, done)
+          await authenticateThirdParty(thirdPartyUser, false, done, saveUser)
           const user = expectUserIsAuthenticated()
           expectUserIsSynced(user, thirdPartyUser)
           expect(user.roles).toStrictEqual({})
@@ -123,7 +127,7 @@ describe("third party common", () => {
         })
 
         it("syncs and authenticates the user", async () => {
-          await authenticateThirdParty(thirdPartyUser, true, done)
+          await authenticateThirdParty(thirdPartyUser, true, done, saveUser)
         
           const user = expectUserIsAuthenticated()
           expectUserIsSynced(user, thirdPartyUser)
@@ -139,7 +143,7 @@ describe("third party common", () => {
         })
 
         it("syncs and authenticates the user", async () => {
-          await authenticateThirdParty(thirdPartyUser, true, done)
+          await authenticateThirdParty(thirdPartyUser, true, done, saveUser)
         
           const user = expectUserIsAuthenticated()
           expectUserIsSynced(user, thirdPartyUser)

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

@@ -1,6 +1,7 @@
 const env = require("../../environment")
 const jwt = require("jsonwebtoken")
 const { generateGlobalUserID } = require("../../db/utils")
+const { saveUser } = require("../../utils")
 const { authError } = require("./utils")
 const { newid } = require("../../hashing")
 const { createASession } = require("../../security/sessions")
@@ -14,7 +15,8 @@ const fetch = require("node-fetch")
 exports.authenticateThirdParty = async function (
   thirdPartyUser,
   requireLocalAccount = true,
-  done
+  done,
+  saveUserFn = saveUser
 ) {
   if (!thirdPartyUser.provider) {
     return authError(done, "third party user provider required")
@@ -71,7 +73,13 @@ exports.authenticateThirdParty = async function (
   dbUser = await syncUser(dbUser, thirdPartyUser)
 
   // create or sync the user
-  const response = await db.put(dbUser)
+  let response
+  try {
+    response = await saveUserFn(dbUser, getTenantId(), false, false)
+  } catch (err) {
+    return authError(done, err)
+  }
+
   dbUser._rev = response.rev
 
   // authenticate

packages/auth/src/tenancy/tenancy.js

@@ -107,3 +107,13 @@ exports.lookupTenantId = async userId => {
   }
   return tenantId
 }
+
+// lookup, could be email or userId, either will return a doc
+exports.getTenantUser = async identifier => {
+  const db = getDB(PLATFORM_INFO_DB)
+  try {
+    return await db.get(identifier)
+  } catch (err) {
+    return null
+  }
+}

packages/auth/src/utils.js

@@ -1,10 +1,24 @@
-const { DocumentTypes, SEPARATOR, ViewNames } = require("./db/utils")
+const {
+  DocumentTypes,
+  SEPARATOR,
+  ViewNames,
+  generateGlobalUserID,
+} = require("./db/utils")
 const jwt = require("jsonwebtoken")
 const { options } = require("./middleware/passport/jwt")
 const { createUserEmailView } = require("./db/views")
-const { Headers } = require("./constants")
+const { Headers, UserStatus } = require("./constants")
-const { getGlobalDB } = require("./tenancy")
+const {
+  getGlobalDB,
+  updateTenantId,
+  getTenantUser,
+  tryAddTenant,
+} = require("./tenancy")
 const environment = require("./environment")
+const accounts = require("./cloud/accounts")
+const { hash } = require("./hashing")
+const userCache = require("./cache/user")
+const env = require("./environment")
 
 const APP_PREFIX = DocumentTypes.APP + SEPARATOR
 
@@ -131,3 +145,93 @@ exports.getGlobalUserByEmail = async email => {
     }
   }
 }
+
+exports.saveUser = async (
+  user,
+  tenantId,
+  hashPassword = true,
+  requirePassword = true
+) => {
+  if (!tenantId) {
+    throw "No tenancy specified."
+  }
+  // need to set the context for this request, as specified
+  updateTenantId(tenantId)
+  // specify the tenancy incase we're making a new admin user (public)
+  const db = getGlobalDB(tenantId)
+  let { email, password, _id } = user
+  // make sure another user isn't using the same email
+  let dbUser
+  if (email) {
+    // check budibase users inside the tenant
+    dbUser = await exports.getGlobalUserByEmail(email)
+    if (dbUser != null && (dbUser._id !== _id || Array.isArray(dbUser))) {
+      throw `Email address ${email} already in use.`
+    }
+
+    // check budibase users in other tenants
+    if (env.MULTI_TENANCY) {
+      dbUser = await getTenantUser(email)
+      if (dbUser != null && dbUser.tenantId !== tenantId) {
+        throw `Email address ${email} already in use.`
+      }
+    }
+
+    // check root account users in account portal
+    if (!env.SELF_HOSTED && !env.DISABLE_ACCOUNT_PORTAL) {
+      const account = await accounts.getAccount(email)
+      if (account && account.verified && account.tenantId !== tenantId) {
+        throw `Email address ${email} already in use.`
+      }
+    }
+  } else {
+    dbUser = await db.get(_id)
+  }
+
+  // get the password, make sure one is defined
+  let hashedPassword
+  if (password) {
+    hashedPassword = hashPassword ? await hash(password) : password
+  } else if (dbUser) {
+    hashedPassword = dbUser.password
+  } else if (requirePassword) {
+    throw "Password must be specified."
+  }
+
+  _id = _id || generateGlobalUserID()
+  user = {
+    createdAt: Date.now(),
+    ...dbUser,
+    ...user,
+    _id,
+    password: hashedPassword,
+    tenantId,
+  }
+  // make sure the roles object is always present
+  if (!user.roles) {
+    user.roles = {}
+  }
+  // add the active status to a user if its not provided
+  if (user.status == null) {
+    user.status = UserStatus.ACTIVE
+  }
+  try {
+    const response = await db.put({
+      password: hashedPassword,
+      ...user,
+    })
+    await tryAddTenant(tenantId, _id, email)
+    await userCache.invalidateUser(response.id)
+    return {
+      _id: response.id,
+      _rev: response.rev,
+      email,
+    }
+  } catch (err) {
+    if (err.status === 409) {
+      throw "User exists already"
+    } else {
+      throw err
+    }
+  }
+}

packages/worker/src/api/controllers/global/users.js

@@ -1,29 +1,24 @@
 const {
-  generateGlobalUserID,
   getGlobalUserParams,
   StaticDatabases,
   generateNewUsageQuotaDoc,
 } = require("@budibase/auth/db")
-const { hash, getGlobalUserByEmail } = require("@budibase/auth").utils
+const { hash, getGlobalUserByEmail, saveUser } = require("@budibase/auth").utils
-const { UserStatus, EmailTemplatePurpose } = require("../../../constants")
+const { EmailTemplatePurpose } = require("../../../constants")
 const { checkInviteCode } = require("../../../utilities/redis")
 const { sendEmail } = require("../../../utilities/email")
 const { user: userCache } = require("@budibase/auth/cache")
 const { invalidateSessions } = require("@budibase/auth/sessions")
-const CouchDB = require("../../../db")
 const accounts = require("@budibase/auth/accounts")
 const {
   getGlobalDB,
   getTenantId,
+  getTenantUser,
   doesTenantExist,
-  tryAddTenant,
-  updateTenantId,
 } = require("@budibase/auth/tenancy")
 const { removeUserFromInfoDB } = require("@budibase/auth/deprovision")
 const env = require("../../../environment")
 
-const PLATFORM_INFO_DB = StaticDatabases.PLATFORM_INFO.name
-
 async function allUsers() {
   const db = getGlobalDB()
   const response = await db.allDocs(
@@ -34,96 +29,6 @@ async function allUsers() {
   return response.rows.map(row => row.doc)
 }
 
-async function saveUser(
-  user,
-  tenantId,
-  hashPassword = true,
-  requirePassword = true
-) {
-  if (!tenantId) {
-    throw "No tenancy specified."
-  }
-  // need to set the context for this request, as specified
-  updateTenantId(tenantId)
-  // specify the tenancy incase we're making a new admin user (public)
-  const db = getGlobalDB(tenantId)
-  let { email, password, _id } = user
-  // make sure another user isn't using the same email
-  let dbUser
-  if (email) {
-    // check budibase users inside the tenant
-    dbUser = await getGlobalUserByEmail(email)
-    if (dbUser != null && (dbUser._id !== _id || Array.isArray(dbUser))) {
-      throw `Email address ${email} already in use.`
-    }
-
-    // check budibase users in other tenants
-    if (env.MULTI_TENANCY) {
-      dbUser = await getTenantUser(email)
-      if (dbUser != null && dbUser.tenantId !== tenantId) {
-        throw `Email address ${email} already in use.`
-      }
-    }
-
-    // check root account users in account portal
-    if (!env.SELF_HOSTED && !env.DISABLE_ACCOUNT_PORTAL) {
-      const account = await accounts.getAccount(email)
-      if (account && account.verified && account.tenantId !== tenantId) {
-        throw `Email address ${email} already in use.`
-      }
-    }
-  } else {
-    dbUser = await db.get(_id)
-  }
-
-  // get the password, make sure one is defined
-  let hashedPassword
-  if (password) {
-    hashedPassword = hashPassword ? await hash(password) : password
-  } else if (dbUser) {
-    hashedPassword = dbUser.password
-  } else if (requirePassword) {
-    throw "Password must be specified."
-  }
-
-  _id = _id || generateGlobalUserID()
-  user = {
-    createdAt: Date.now(),
-    ...dbUser,
-    ...user,
-    _id,
-    password: hashedPassword,
-    tenantId,
-  }
-  // make sure the roles object is always present
-  if (!user.roles) {
-    user.roles = {}
-  }
-  // add the active status to a user if its not provided
-  if (user.status == null) {
-    user.status = UserStatus.ACTIVE
-  }
-  try {
-    const response = await db.put({
-      password: hashedPassword,
-      ...user,
-    })
-    await tryAddTenant(tenantId, _id, email)
-    await userCache.invalidateUser(response.id)
-    return {
-      _id: response.id,
-      _rev: response.rev,
-      email,
-    }
-  } catch (err) {
-    if (err.status === 409) {
-      throw "User exists already"
-    } else {
-      throw err
-    }
-  }
-}
-
 exports.save = async ctx => {
   try {
     ctx.body = await saveUser(ctx.request.body, getTenantId())
@@ -310,16 +215,6 @@ exports.find = async ctx => {
   ctx.body = user
 }
 
-// lookup, could be email or userId, either will return a doc
-const getTenantUser = async identifier => {
-  const db = new CouchDB(PLATFORM_INFO_DB)
-  try {
-    return await db.get(identifier)
-  } catch (err) {
-    return null
-  }
-}
-
 exports.tenantUserLookup = async ctx => {
   const id = ctx.params.id
   const user = await getTenantUser(id)