From 06866111d3381a8c95876cb28cca412301d1d017 Mon Sep 17 00:00:00 2001 From: Martin McKeaveney Date: Mon, 17 Feb 2025 10:32:12 +0000 Subject: [PATCH 1/2] fix error message and extract tenantId from appId --- packages/backend-core/src/utils/tests/utils.spec.ts | 9 +++++++++ packages/backend-core/src/utils/utils.ts | 5 +++++ .../server/src/middleware/ensureTenantAppOwnership.ts | 9 ++++++--- .../middleware/tests/ensureTenantAppOwnership.spec.js | 7 +++++-- 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/backend-core/src/utils/tests/utils.spec.ts b/packages/backend-core/src/utils/tests/utils.spec.ts index b5c6283ce2..c7c1e4334f 100644 --- a/packages/backend-core/src/utils/tests/utils.spec.ts +++ b/packages/backend-core/src/utils/tests/utils.spec.ts @@ -67,6 +67,15 @@ describe("utils", () => { }) }) + it("gets appId from query params", async () => { + const ctx = structures.koa.newContext() + const expected = db.generateAppID() + ctx.query = { appId: expected } + + const actual = await utils.getAppIdFromCtx(ctx) + expect(actual).toBe(expected) + }) + it("doesn't get appId from url when previewing", async () => { const ctx = structures.koa.newContext() const appId = db.generateAppID() diff --git a/packages/backend-core/src/utils/utils.ts b/packages/backend-core/src/utils/utils.ts index 7f2e25b6d4..fcec61179f 100644 --- a/packages/backend-core/src/utils/utils.ts +++ b/packages/backend-core/src/utils/utils.ts @@ -101,6 +101,11 @@ export async function getAppIdFromCtx(ctx: Ctx) { appId = confirmAppId(pathId) } + // look in queryParams + if (!appId && ctx.query?.appId) { + appId = confirmAppId(ctx.query?.appId as string) + } + // lookup using custom url - prod apps only // filter out the builder preview path which collides with the prod app path // to ensure we don't load all apps excessively diff --git a/packages/server/src/middleware/ensureTenantAppOwnership.ts b/packages/server/src/middleware/ensureTenantAppOwnership.ts index 511342a0c2..23f35f5cb8 100644 --- a/packages/server/src/middleware/ensureTenantAppOwnership.ts +++ b/packages/server/src/middleware/ensureTenantAppOwnership.ts @@ -1,4 +1,4 @@ -import { tenancy, utils } from "@budibase/backend-core" +import { tenancy, utils, context } from "@budibase/backend-core" import { UserCtx } from "@budibase/types" async function ensureTenantAppOwnership(ctx: UserCtx, next: any) { @@ -6,9 +6,12 @@ async function ensureTenantAppOwnership(ctx: UserCtx, next: any) { if (!appId) { ctx.throw(400, "appId must be provided") } + + const appTenantId = context.getTenantIDFromAppID(appId) const tenantId = tenancy.getTenantId() - if (appId !== tenantId) { - ctx.throw(403, `App does not belong to tenant`) + + if (appTenantId !== tenantId) { + ctx.throw(403, "Unauthorized") } await next() } diff --git a/packages/server/src/middleware/tests/ensureTenantAppOwnership.spec.js b/packages/server/src/middleware/tests/ensureTenantAppOwnership.spec.js index 817893681b..e81dfc0398 100644 --- a/packages/server/src/middleware/tests/ensureTenantAppOwnership.spec.js +++ b/packages/server/src/middleware/tests/ensureTenantAppOwnership.spec.js @@ -2,6 +2,7 @@ import ensureTenantAppOwnership from "../ensureTenantAppOwnership" import { tenancy, utils } from "@budibase/backend-core" jest.mock("@budibase/backend-core", () => ({ + ...jest.requireActual("@budibase/backend-core"), tenancy: { getTenantId: jest.fn(), }, @@ -53,7 +54,9 @@ describe("Ensure Tenant Ownership Middleware", () => { expect(config.next).toHaveBeenCalled() }) - it("throws 403 when appId does not match tenant ID", async () => { + it("throws when tenant appId does not match tenant ID", async () => { + const appId = "app_dev_tenant3_fce449c4d75b4e4a9c7a6980d82a3e22" + utils.getAppIdFromCtx.mockResolvedValue(appId) tenancy.getTenantId.mockReturnValue("tenant_2") await config.executeMiddleware() @@ -61,7 +64,7 @@ describe("Ensure Tenant Ownership Middleware", () => { expect(utils.getAppIdFromCtx).toHaveBeenCalledWith(config.ctx) expect(config.throw).toHaveBeenCalledWith( 403, - "App does not belong to tenant" + "Unauthorized", ) }) From 268f67b17cd0938d7a8be442b6852893f9a8635e Mon Sep 17 00:00:00 2001 From: Martin McKeaveney Date: Mon, 17 Feb 2025 10:36:26 +0000 Subject: [PATCH 2/2] lint --- .../src/middleware/tests/ensureTenantAppOwnership.spec.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/server/src/middleware/tests/ensureTenantAppOwnership.spec.js b/packages/server/src/middleware/tests/ensureTenantAppOwnership.spec.js index e81dfc0398..5c500f8723 100644 --- a/packages/server/src/middleware/tests/ensureTenantAppOwnership.spec.js +++ b/packages/server/src/middleware/tests/ensureTenantAppOwnership.spec.js @@ -62,10 +62,7 @@ describe("Ensure Tenant Ownership Middleware", () => { await config.executeMiddleware() expect(utils.getAppIdFromCtx).toHaveBeenCalledWith(config.ctx) - expect(config.throw).toHaveBeenCalledWith( - 403, - "Unauthorized", - ) + expect(config.throw).toHaveBeenCalledWith(403, "Unauthorized") }) it("throws 400 when appId is missing", async () => {