From bcc84bf1fdab595f415d2be7ab1d93c24e8977d2 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Thu, 24 Mar 2022 13:04:49 +0000 Subject: [PATCH] Link app context with tenancy, remove app tenancy middleware --- package.json | 9 +- packages/backend-core/src/context/index.js | 32 + packages/backend-core/src/db/utils.js | 7 +- .../backend-core/src/middleware/appTenancy.js | 27 - packages/backend-core/src/middleware/index.js | 2 - packages/backend-core/src/tenancy/tenancy.js | 31 +- packages/client/stats.html | 2689 ----------------- packages/server/src/api/controllers/auth.js | 2 +- packages/server/src/api/index.js | 3 - packages/server/src/middleware/currentapp.js | 9 +- .../src/middleware/tests/authorized.spec.js | 4 +- .../src/middleware/tests/currentapp.spec.js | 8 +- .../src/tests/utilities/TestConfiguration.js | 2 +- 13 files changed, 59 insertions(+), 2766 deletions(-) delete mode 100644 packages/backend-core/src/middleware/appTenancy.js delete mode 100644 packages/client/stats.html diff --git a/package.json b/package.json index 69c7967409..26dc9c6de7 100644 --- a/package.json +++ b/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}\"", diff --git a/packages/backend-core/src/context/index.js b/packages/backend-core/src/context/index.js index 968ad4eefb..ba9a7831db 100644 --- a/packages/backend-core/src/context/index.js +++ b/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) diff --git a/packages/backend-core/src/db/utils.js b/packages/backend-core/src/db/utils.js index 6d6f9a782b..feb17c4129 100644 --- a/packages/backend-core/src/db/utils.js +++ b/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 diff --git a/packages/backend-core/src/middleware/appTenancy.js b/packages/backend-core/src/middleware/appTenancy.js deleted file mode 100644 index b0430a0051..0000000000 --- a/packages/backend-core/src/middleware/appTenancy.js +++ /dev/null @@ -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) - }) -} diff --git a/packages/backend-core/src/middleware/index.js b/packages/backend-core/src/middleware/index.js index 5878479152..6c4c0d8883 100644 --- a/packages/backend-core/src/middleware/index.js +++ b/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: { diff --git a/packages/backend-core/src/tenancy/tenancy.js b/packages/backend-core/src/tenancy/tenancy.js index 8360198b60..24acc16862 100644 --- a/packages/backend-core/src/tenancy/tenancy.js +++ b/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 } diff --git a/packages/client/stats.html b/packages/client/stats.html deleted file mode 100644 index 99e666d039..0000000000 --- a/packages/client/stats.html +++ /dev/null @@ -1,2689 +0,0 @@ - - - - - - - - RollUp Visualizer - - - -
- - - - - diff --git a/packages/server/src/api/controllers/auth.js b/packages/server/src/api/controllers/auth.js index 30c0e5d09c..03480f62fb 100644 --- a/packages/server/src/api/controllers/auth.js +++ b/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 } diff --git a/packages/server/src/api/index.js b/packages/server/src/api/index.js index 8b0c091346..39a7fd6cb7 100644 --- a/packages/server/src/api/index.js +++ b/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 diff --git a/packages/server/src/middleware/currentapp.js b/packages/server/src/middleware/currentapp.js index 1fae78533a..bc9462759c 100644 --- a/packages/server/src/middleware/currentapp.js +++ b/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) } diff --git a/packages/server/src/middleware/tests/authorized.spec.js b/packages/server/src/middleware/tests/authorized.spec.js index 9cfa9d368f..cf15b28458 100644 --- a/packages/server/src/middleware/tests/authorized.spec.js +++ b/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) { diff --git a/packages/server/src/middleware/tests/currentapp.spec.js b/packages/server/src/middleware/tests/currentapp.spec.js index 4e53a6a4c0..bee07cbf91 100644 --- a/packages/server/src/middleware/tests/currentapp.spec.js +++ b/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(), diff --git a/packages/server/src/tests/utilities/TestConfiguration.js b/packages/server/src/tests/utilities/TestConfiguration.js index 1d280fdd4b..6ebafa8421 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.js +++ b/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() })