diff --git a/packages/server/src/api/routes/backup.ts b/packages/server/src/api/routes/backup.ts index c7f32d6043..e94aeb8874 100644 --- a/packages/server/src/api/routes/backup.ts +++ b/packages/server/src/api/routes/backup.ts @@ -2,7 +2,6 @@ import Router from "@koa/router" import * as controller from "../controllers/backup" import authorized from "../../middleware/authorized" import { permissions } from "@budibase/backend-core" -import { UserCtx } from "@budibase/types" import ensureTenantAppOwnership from "../../middleware/ensureTenantAppOwnership" const router: Router = new Router() @@ -10,7 +9,7 @@ const router: Router = new Router() router.post( "/api/backups/export", authorized(permissions.BUILDER), - ensureTenantAppOwnership((ctx: UserCtx) => ctx.query.appId as string), + ensureTenantAppOwnership, controller.exportAppDump ) diff --git a/packages/server/src/middleware/ensureTenantAppOwnership.ts b/packages/server/src/middleware/ensureTenantAppOwnership.ts index 3ad5ff2c97..8f609f682a 100644 --- a/packages/server/src/middleware/ensureTenantAppOwnership.ts +++ b/packages/server/src/middleware/ensureTenantAppOwnership.ts @@ -1,16 +1,17 @@ -import { tenancy } from "@budibase/backend-core" +import { tenancy, utils } from "@budibase/backend-core" import { UserCtx } from "@budibase/types" -function ensureTenantAppOwnership(appIdGetter: (ctx: UserCtx) => string) { - return async (ctx: UserCtx, next: any) => { - const appId = appIdGetter(ctx) - const exportAppId = tenancy.getTenantIDFromAppID(appId) - const tenantId = tenancy.getTenantId() - if (exportAppId !== tenantId) { - ctx.throw(403, `Cannot export app from another tenant`) - } - await next() +async function ensureTenantAppOwnership(ctx: UserCtx, next: any) { + const appId = await utils.getAppIdFromCtx(ctx) + if (!appId) { + ctx.throw(400, "appId must be provided") } + const exportAppId = tenancy.getTenantIDFromAppID(appId) + const tenantId = tenancy.getTenantId() + if (exportAppId !== tenantId) { + ctx.throw(403, `Cannot export app from another tenant`) + } + await next() } export default ensureTenantAppOwnership diff --git a/packages/server/src/middleware/tests/ensureTenantAppOwnership.spec.js b/packages/server/src/middleware/tests/ensureTenantAppOwnership.spec.js index ff53ed0b8f..b600c5a2f2 100644 --- a/packages/server/src/middleware/tests/ensureTenantAppOwnership.spec.js +++ b/packages/server/src/middleware/tests/ensureTenantAppOwnership.spec.js @@ -1,26 +1,31 @@ import ensureTenantAppOwnership from "../ensureTenantAppOwnership" -import { tenancy } from "@budibase/backend-core" +import { tenancy, utils } from "@budibase/backend-core" jest.mock("@budibase/backend-core", () => ({ tenancy: { getTenantId: jest.fn(), getTenantIDFromAppID: jest.fn(), }, + utils: { + getAppIdFromCtx: jest.fn(), + }, })) class TestConfiguration { - constructor() { + constructor(appId = "app_123") { this.next = jest.fn() this.throw = jest.fn() - this.middleware = ensureTenantAppOwnership(() => "app_123") + this.middleware = ensureTenantAppOwnership this.ctx = { next: this.next, throw: this.throw, } + + utils.getAppIdFromCtx.mockResolvedValue(appId) } - executeMiddleware() { + async executeMiddleware() { return this.middleware(this.ctx, this.next) } @@ -45,6 +50,8 @@ describe("Ensure Tenant Ownership Middleware", () => { tenancy.getTenantId.mockReturnValue("tenant_1") await config.executeMiddleware() + + expect(utils.getAppIdFromCtx).toHaveBeenCalledWith(config.ctx) expect(config.next).toHaveBeenCalled() }) @@ -53,9 +60,19 @@ describe("Ensure Tenant Ownership Middleware", () => { tenancy.getTenantId.mockReturnValue("tenant_1") await config.executeMiddleware() + + expect(utils.getAppIdFromCtx).toHaveBeenCalledWith(config.ctx) expect(config.throw).toHaveBeenCalledWith( 403, "Cannot export app from another tenant" ) }) + + it("throws 400 when appId is missing", async () => { + utils.getAppIdFromCtx.mockResolvedValue(null) + + await config.executeMiddleware() + + expect(config.throw).toHaveBeenCalledWith(400, "appId must be provided") + }) })