Code review updates

packages/backend-core/src/auth.js

@@ -15,6 +15,7 @@ const {
   tenancy,
   appTenancy,
   authError,
+  ssoCallbackUrl,
   csrf,
   internalApi,
 } = require("./middleware")
@@ -72,11 +73,10 @@ async function refreshOIDCAccessToken(db, chosenConfig, refreshToken) {
 
 async function refreshGoogleAccessToken(db, config, refreshToken) {
   let callbackUrl = await google.getCallbackUrl(db, config)
-  const googleConfig = await google.fetchStrategyConfig(config)
 
   let strategy
   try {
-    strategy = await google.strategyFactory(googleConfig, callbackUrl)
+    strategy = await google.strategyFactory(config, callbackUrl)
   } catch (err) {
     console.error(err)
     throw new Error("Error constructing OIDC refresh strategy", err)
@@ -168,4 +168,5 @@ module.exports = {
   internalApi,
   refreshOAuthToken,
   updateUserOAuth,
+  ssoCallbackUrl,
 }

packages/backend-core/src/db/utils.ts

@@ -386,10 +386,9 @@ export const getScopedFullConfig = async function (
   if (type === Configs.SETTINGS) {
     if (scopedConfig && scopedConfig.doc) {
       // overrides affected by environment variables
-      scopedConfig.doc.config.platformUrl = await getPlatformUrl(
-        { tenantAware: true },
-        db
-      )
+      scopedConfig.doc.config.platformUrl = await getPlatformUrl({
+        tenantAware: true,
+      })
       scopedConfig.doc.config.analyticsEnabled =
         await events.analytics.enabled()
     } else {
@@ -398,7 +397,7 @@ export const getScopedFullConfig = async function (
         doc: {
           _id: generateConfigID({ type, user, workspace }),
           config: {
-            platformUrl: await getPlatformUrl({ tenantAware: true }, db),
+            platformUrl: await getPlatformUrl({ tenantAware: true }),
             analyticsEnabled: await events.analytics.enabled(),
           },
         },
@@ -409,10 +408,7 @@ export const getScopedFullConfig = async function (
   return scopedConfig && scopedConfig.doc
 }
 
-export const getPlatformUrl = async (
-  opts = { tenantAware: true },
-  db = null
-) => {
+export const getPlatformUrl = async (opts = { tenantAware: true }) => {
   let platformUrl = env.PLATFORM_URL || "http://localhost:10000"
 
   if (!env.SELF_HOSTED && env.MULTI_TENANCY && opts.tenantAware) {
@@ -422,11 +418,11 @@ export const getPlatformUrl = async (
       platformUrl = platformUrl.replace("://", `://${tenantId}.`)
     }
   } else if (env.SELF_HOSTED) {
-    const dbx = db ? db : getGlobalDB()
+    const db = getGlobalDB()
     // get the doc directly instead of with getScopedConfig to prevent loop
     let settings
     try {
-      settings = await dbx.get(generateConfigID({ type: Configs.SETTINGS }))
+      settings = await db.get(generateConfigID({ type: Configs.SETTINGS }))
     } catch (e: any) {
       if (e.status !== 404) {
         throw e

packages/backend-core/src/middleware/authenticated.js

@@ -94,7 +94,6 @@ module.exports = (
               user = await getUser(userId, session.tenantId)
             }
             user.csrfToken = session.csrfToken
-            delete user.password
             authenticated = true
           } catch (err) {
             error = err

packages/backend-core/src/middleware/index.js

@@ -2,7 +2,7 @@ const jwt = require("./passport/jwt")
 const local = require("./passport/local")
 const google = require("./passport/google")
 const oidc = require("./passport/oidc")
-const { authError } = require("./passport/utils")
+const { authError, ssoCallbackUrl } = require("./passport/utils")
 const authenticated = require("./authenticated")
 const auditLog = require("./auditLog")
 const tenancy = require("./tenancy")
@@ -20,6 +20,7 @@ module.exports = {
   tenancy,
   authError,
   internalApi,
+  ssoCallbackUrl,
   datasource: {
     google: datasourceGoogle,
   },

packages/backend-core/src/middleware/passport/google.js

@@ -2,7 +2,6 @@ const GoogleStrategy = require("passport-google-oauth").OAuth2Strategy
 const { ssoCallbackUrl } = require("./utils")
 const { authenticateThirdParty } = require("./third-party-common")
 const { Configs } = require("../../../constants")
-const environment = require("../../environment")
 
 const buildVerifyFn = saveUserFn => {
   return (accessToken, refreshToken, profile, done) => {
@@ -60,15 +59,6 @@ exports.strategyFactory = async function (config, callbackUrl, saveUserFn) {
   }
 }
 
-exports.fetchStrategyConfig = async function (googleConfig) {
-  return (
-    googleConfig || {
-      clientID: environment.GOOGLE_CLIENT_ID,
-      clientSecret: environment.GOOGLE_CLIENT_SECRET,
-    }
-  )
-}
-
 exports.getCallbackUrl = async function (db, config) {
   return ssoCallbackUrl(db, config, Configs.GOOGLE)
 }

packages/backend-core/src/middleware/passport/local.js

@@ -6,7 +6,7 @@ const users = require("../../users")
 const { authError } = require("./utils")
 const { newid } = require("../../hashing")
 const { createASession } = require("../../security/sessions")
-const { getTenantId, getGlobalDB } = require("../../tenancy")
+const { getTenantId } = require("../../tenancy")
 
 const INVALID_ERR = "Invalid credentials"
 const SSO_NO_PASSWORD = "SSO user does not have a password set"
@@ -56,19 +56,6 @@ exports.authenticate = async function (ctx, email, password, done) {
     const sessionId = newid()
     const tenantId = getTenantId()
 
-    if (dbUser.provider || dbUser.providerType || dbUser.pictureUrl) {
-      delete dbUser.provider
-      delete dbUser.providerType
-      delete dbUser.pictureUrl
-
-      try {
-        const db = getGlobalDB()
-        await db.put(dbUser)
-      } catch (err) {
-        console.error("OAuth elements could not be purged")
-      }
-    }
-
     await createASession(dbUser._id, { sessionId, tenantId })
 
     dbUser.token = jwt.sign(

packages/backend-core/src/middleware/passport/oidc.js

@@ -103,9 +103,9 @@ exports.strategyFactory = async function (config, saveUserFn) {
   }
 }
 
-exports.fetchStrategyConfig = async function (config, callbackUrl) {
+exports.fetchStrategyConfig = async function (enrichedConfig, callbackUrl) {
   try {
-    const { clientID, clientSecret, configUrl } = config
+    const { clientID, clientSecret, configUrl } = enrichedConfig
 
     if (!clientID || !clientSecret || !callbackUrl || !configUrl) {
       //check for remote config and all required elements

packages/backend-core/src/middleware/passport/utils.js

@@ -1,4 +1,4 @@
-const { getGlobalDB, isMultiTenant, getTenantId } = require("../../tenancy")
+const { isMultiTenant, getTenantId } = require("../../tenancy")
 const { getScopedConfig } = require("../../db/utils")
 const { Configs } = require("../../constants")
 
@@ -23,9 +23,7 @@ exports.ssoCallbackUrl = async (db, config, type) => {
   if (config && config.callbackURL) {
     return config.callbackURL
   }
-
-  const dbx = db ? db : getGlobalDB()
-  const publicConfig = await getScopedConfig(dbx, {
+  const publicConfig = await getScopedConfig(db, {
     type: Configs.SETTINGS,
   })
 

packages/worker/src/api/controllers/global/auth.ts

@@ -1,49 +1,22 @@
 const core = require("@budibase/backend-core")
-const { getScopedConfig } = require("@budibase/backend-core/db")
-const { google } = require("@budibase/backend-core/middleware")
-const { oidc } = require("@budibase/backend-core/middleware")
 const { Configs, EmailTemplatePurpose } = require("../../../constants")
 const { sendEmail, isEmailConfigured } = require("../../../utilities/email")
 const { setCookie, getCookie, clearCookie, hash, platformLogout } = core.utils
 const { Cookies, Headers } = core.constants
-const { passport } = core.auth
+const { passport, ssoCallbackUrl, google, oidc } = core.auth
 const { checkResetPasswordCode } = require("../../../utilities/redis")
-const {
-  getGlobalDB,
-  getTenantId,
-  isMultiTenant,
-} = require("@budibase/backend-core/tenancy")
+const { getGlobalDB } = require("@budibase/backend-core/tenancy")
 const env = require("../../../environment")
 import { events, users as usersCore, context } from "@budibase/backend-core"
 import { users } from "../../../sdk"
 import { User } from "@budibase/types"
 
-const ssoCallbackUrl = async (config: any, type: any) => {
-  // incase there is a callback URL from before
-  if (config && config.callbackURL) {
-    return config.callbackURL
-  }
-
-  const db = getGlobalDB()
-  const publicConfig = await getScopedConfig(db, {
-    type: Configs.SETTINGS,
-  })
-
-  let callbackUrl = `/api/global/auth`
-  if (isMultiTenant()) {
-    callbackUrl += `/${getTenantId()}`
-  }
-  callbackUrl += `/${type}/callback`
-
-  return `${publicConfig.platformUrl}${callbackUrl}`
-}
-
 export const googleCallbackUrl = async (config: any) => {
-  return ssoCallbackUrl(config, "google")
+  return ssoCallbackUrl(getGlobalDB(), config, "google")
 }
 
 export const oidcCallbackUrl = async (config: any) => {
-  return ssoCallbackUrl(config, "oidc")
+  return ssoCallbackUrl(getGlobalDB(), config, "oidc")
 }
 
 async function authInternal(ctx: any, user: any, err = null, info = null) {
@@ -70,7 +43,7 @@ async function authInternal(ctx: any, user: any, err = null, info = null) {
 export const authenticate = async (ctx: any, next: any) => {
   return passport.authenticate(
     "local",
-    async (err: any, user: any, info: any) => {
+    async (err: any, user: User, info: any) => {
       await authInternal(ctx, user, err, info)
       await context.identity.doInUserContext(user, async () => {
         await events.auth.login("local")