Link app context with tenancy, remove app tenancy middleware

package.json

@@ -31,9 +31,12 @@
     "nuke:docker": "lerna run --parallel dev:stack:nuke",
     "clean": "lerna clean",
     "kill-port": "kill-port 4001",
-    "dev": "yarn run kill-port && lerna link && lerna run --parallel dev:builder --concurrency 1",
-    "dev:noserver": "lerna link && lerna run dev:stack:up && lerna run --parallel dev:builder --concurrency 1 --ignore @budibase/server --ignore @budibase/worker",
-    "dev:server": "lerna run --parallel dev:builder --concurrency 1 --scope @budibase/worker --scope @budibase/server",
+    "kill-builder": "kill-port 3000",
+    "kill-server": "kill-port 4001 4002",
+    "kill-all": "yarn run kill-builder && yarn run kill-server",
+    "dev": "yarn run kill-all && lerna link && lerna run --parallel dev:builder --concurrency 1",
+    "dev:noserver": "yarn run kill-builder && lerna link && lerna run dev:stack:up && lerna run --parallel dev:builder --concurrency 1 --ignore @budibase/server --ignore @budibase/worker",
+    "dev:server": "yarn run kill-server && lerna run --parallel dev:builder --concurrency 1 --scope @budibase/worker --scope @budibase/server",
     "test": "lerna run test",
     "lint:eslint": "eslint packages",
     "lint:prettier": "prettier --check \"packages/**/*.{js,ts,svelte}\"",

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

@@ -1,5 +1,6 @@
 const env = require("../environment")
 const { Headers } = require("../../constants")
+const { SEPARATOR, DocumentTypes } = require("../db/constants")
 const cls = require("./FunctionContext")
 const { getCouch } = require("../db")
 const { getProdAppID, getDevelopmentAppID } = require("../db/conversions")
@@ -42,8 +43,39 @@ exports.doInTenant = (tenantId, task) => {
   })
 }
 
+/**
+ * Given an app ID this will attempt to retrieve the tenant ID from it.
+ * @return {null|string} The tenant ID found within the app ID.
+ */
+exports.getTenantIDFromAppID = appId => {
+  if (!appId) {
+    return null
+  }
+  const split = appId.split(SEPARATOR)
+  const hasDev = split[1] === DocumentTypes.DEV
+  if ((hasDev && split.length === 3) || (!hasDev && split.length === 2)) {
+    return null
+  }
+  if (hasDev) {
+    return split[2]
+  } else {
+    return split[1]
+  }
+}
+
+const setAppTenantId = appId => {
+  const appTenantId = this.getTenantIDFromAppID(appId) || this.DEFAULT_TENANT_ID
+  this.updateTenantId(appTenantId)
+}
+
 exports.doInAppContext = (appId, task) => {
+  if (!appId) {
+    throw new Error("appId is required")
+  }
   return cls.run(() => {
+    // set the app tenant id
+    setAppTenantId(appId)
+
     // set the app ID
     cls.setOnContext(ContextKeys.APP_ID, appId)
 

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

@@ -9,11 +9,7 @@ const {
   APP_PREFIX,
   APP_DEV,
 } = require("./constants")
-const {
-  getTenantId,
-  getTenantIDFromAppID,
-  getGlobalDBName,
-} = require("../tenancy")
+const { getTenantId, getGlobalDBName } = require("../tenancy")
 const fetch = require("node-fetch")
 const { getCouch } = require("./index")
 const { getAppMetadata } = require("../cache/appMetadata")
@@ -39,7 +35,6 @@ exports.DocumentTypes = DocumentTypes
 exports.APP_PREFIX = APP_PREFIX
 exports.APP_DEV = exports.APP_DEV_PREFIX = APP_DEV
 exports.SEPARATOR = SEPARATOR
-exports.getTenantIDFromAppID = getTenantIDFromAppID
 exports.isDevApp = isDevApp
 exports.isProdAppID = isProdAppID
 exports.isDevAppID = isDevAppID

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

@@ -1,27 +0,0 @@
-const {
-  isMultiTenant,
-  updateTenantId,
-  isTenantIdSet,
-  DEFAULT_TENANT_ID,
-  updateAppId,
-} = require("../tenancy")
-const ContextFactory = require("../context/FunctionContext")
-const { getTenantIDFromAppID } = require("../db/utils")
-
-module.exports = () => {
-  return ContextFactory.getMiddleware(ctx => {
-    // if not in multi-tenancy mode make sure its default and exit
-    if (!isMultiTenant()) {
-      updateTenantId(DEFAULT_TENANT_ID)
-      return
-    }
-    // if tenant ID already set no need to continue
-    if (isTenantIdSet()) {
-      return
-    }
-    const appId = ctx.appId ? ctx.appId : ctx.user ? ctx.user.appId : null
-    const tenantId = getTenantIDFromAppID(appId) || DEFAULT_TENANT_ID
-    updateTenantId(tenantId)
-    updateAppId(appId)
-  })
-}

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

@@ -6,7 +6,6 @@ const { authError } = require("./passport/utils")
 const authenticated = require("./authenticated")
 const auditLog = require("./auditLog")
 const tenancy = require("./tenancy")
-const appTenancy = require("./appTenancy")
 const internalApi = require("./internalApi")
 const datasourceGoogle = require("./passport/datasource/google")
 const csrf = require("./csrf")
@@ -19,7 +18,6 @@ module.exports = {
   authenticated,
   auditLog,
   tenancy,
-  appTenancy,
   authError,
   internalApi,
   datasource: {

packages/backend-core/src/tenancy/tenancy.js

@@ -1,6 +1,11 @@
 const { getDB } = require("../db")
-const { SEPARATOR, StaticDatabases, DocumentTypes } = require("../db/constants")
-const { getTenantId, DEFAULT_TENANT_ID, isMultiTenant } = require("../context")
+const { SEPARATOR, StaticDatabases } = require("../db/constants")
+const {
+  getTenantId,
+  DEFAULT_TENANT_ID,
+  isMultiTenant,
+  getTenantIDFromAppID,
+} = require("../context")
 const env = require("../environment")
 
 const TENANT_DOC = StaticDatabases.PLATFORM_INFO.docs.tenants
@@ -118,26 +123,6 @@ exports.getTenantUser = async identifier => {
   }
 }
 
-/**
- * Given an app ID this will attempt to retrieve the tenant ID from it.
- * @return {null|string} The tenant ID found within the app ID.
- */
-exports.getTenantIDFromAppID = appId => {
-  if (!appId) {
-    return null
-  }
-  const split = appId.split(SEPARATOR)
-  const hasDev = split[1] === DocumentTypes.DEV
-  if ((hasDev && split.length === 3) || (!hasDev && split.length === 2)) {
-    return null
-  }
-  if (hasDev) {
-    return split[2]
-  } else {
-    return split[1]
-  }
-}
-
 exports.isUserInAppTenant = (appId, user = null) => {
   let userTenantId
   if (user) {
@@ -145,7 +130,7 @@ exports.isUserInAppTenant = (appId, user = null) => {
   } else {
     userTenantId = getTenantId()
   }
-  const tenantId = exports.getTenantIDFromAppID(appId) || DEFAULT_TENANT_ID
+  const tenantId = getTenantIDFromAppID(appId) || DEFAULT_TENANT_ID
   return tenantId === userTenantId
 }
 

packages/server/src/api/controllers/auth.js

@@ -7,7 +7,7 @@ const { getAppDB, getAppId } = require("@budibase/backend-core/context")
 exports.fetchSelf = async ctx => {
   let userId = ctx.user.userId || ctx.user._id
   /* istanbul ignore next */
-  if (!userId) {
+  if (!userId || !ctx.isAuthenticated) {
     ctx.body = {}
     return
   }

packages/server/src/api/index.js

@@ -3,7 +3,6 @@ const {
   buildAuthMiddleware,
   auditLog,
   buildTenancyMiddleware,
-  buildAppTenancyMiddleware,
 } = require("@budibase/backend-core/auth")
 const currentApp = require("../middleware/currentapp")
 const compress = require("koa-compress")
@@ -52,8 +51,6 @@ router
     })
   )
   .use(currentApp)
-  // this middleware will try to use the app ID to determine the tenancy
-  .use(buildAppTenancyMiddleware())
   .use(auditLog)
 
 // error handling middleware

packages/server/src/middleware/currentapp.js

@@ -71,21 +71,22 @@ module.exports = async (ctx, next) => {
   }
 
   return doInAppContext(appId, async () => {
-    let noCookieSet = false
+    let skipCookie = false
     // if the user not in the right tenant then make sure they have no permissions
     // need to judge this only based on the request app ID,
     if (
       env.MULTI_TENANCY &&
       ctx.user &&
       requestAppId &&
-      !isUserInAppTenant(requestAppId)
+      !isUserInAppTenant(requestAppId, ctx.user)
     ) {
       // don't error, simply remove the users rights (they are a public user)
       delete ctx.user.builder
       delete ctx.user.admin
       delete ctx.user.roles
+      ctx.isAuthenticated = false
       roleId = BUILTIN_ROLE_IDS.PUBLIC
-      noCookieSet = true
+      skipCookie = true
     }
 
     ctx.appId = appId
@@ -105,7 +106,7 @@ module.exports = async (ctx, next) => {
       (requestAppId !== appId ||
         appCookie == null ||
         appCookie.appId !== requestAppId) &&
-      !noCookieSet
+      !skipCookie
     ) {
       setCookie(ctx, { appId }, Cookies.CurrentApp)
     }

packages/server/src/middleware/tests/authorized.spec.js

@@ -35,9 +35,7 @@ class TestConfiguration {
   }
 
   executeMiddleware() {
-    return doInAppContext(APP_ID, () => {
-      return this.middleware(this.ctx, this.next)
-    })
+    return this.middleware(this.ctx, this.next)
   }
 
   setUser(user) {

packages/server/src/middleware/tests/currentapp.spec.js

@@ -38,7 +38,7 @@ function mockAuthWithNoCookie() {
     },
   }))
   jest.mock("@budibase/backend-core/utils", () => ({
-    getAppId: jest.fn(),
+    getAppIdFromCtx: jest.fn(),
     setCookie: jest.fn(),
     getCookie: jest.fn(),
   }))
@@ -51,7 +51,7 @@ function mockAuthWithCookie() {
   jest.resetModules()
   mockWorker()
   jest.mock("@budibase/backend-core/utils", () => ({
-    getAppId: () => {
+    getAppIdFromCtx: () => {
       return "app_test"
     },
     setCookie: jest.fn(),
@@ -143,7 +143,7 @@ describe("Current app middleware", () => {
     it("should perform correct when no cookie exists", async () => {
       mockReset()
       jest.mock("@budibase/backend-core/utils", () => ({
-        getAppId: () => {
+        getAppIdFromCtx: () => {
           return "app_test"
         },
         setCookie: jest.fn(),
@@ -158,7 +158,7 @@ describe("Current app middleware", () => {
     it("lastly check what occurs when cookie doesn't need updated", async () => {
       mockReset()
       jest.mock("@budibase/backend-core/utils", () => ({
-        getAppId: () => {
+        getAppIdFromCtx: () => {
           return "app_test"
         },
         setCookie: jest.fn(),

packages/server/src/tests/utilities/TestConfiguration.js

@@ -80,7 +80,7 @@ class TestConfiguration {
       return request.body
     }
     // check if already in a context
-    if (context.getAppId() == null) {
+    if (context.getAppId() == null && this.appId !== null) {
       return context.doInAppContext(this.appId, async () => {
         return run()
       })