From 8359185a22e704146eb7a48fad182241045adfa2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 24 Aug 2023 11:23:30 +0200 Subject: [PATCH] Add unhappy paths tests --- packages/server/src/middleware/authorized.ts | 18 ++++--- .../src/middleware/tests/authorized.spec.ts | 49 +++++++++++++++++-- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/packages/server/src/middleware/authorized.ts b/packages/server/src/middleware/authorized.ts index 3d4c44a108..52ced1b9cc 100644 --- a/packages/server/src/middleware/authorized.ts +++ b/packages/server/src/middleware/authorized.ts @@ -82,8 +82,14 @@ const resourceIdTranformers: Partial< > = { [PermissionType.VIEW]: async ctx => { const { resourceId } = ctx + if (!resourceId) { + ctx.throw(400, `Cannot obtain the view id`) + return + } + if (!isViewID(resourceId)) { - ctx.throw(400, `"${resourceId}" is not a valid viewId`) + ctx.throw(400, `"${resourceId}" is not a valid view id`) + return } if (await features.isViewPermissionEnabled()) { @@ -121,17 +127,17 @@ const authorized = permLevel === PermissionLevel.READ ? PermissionLevel.WRITE : PermissionLevel.READ - const appId = context.getAppId() if (resourcePath) { // Reusing the existing middleware to extract the value paramResource(resourcePath)(ctx, () => {}) } - if (appId && hasResource(ctx)) { - if (resourceIdTranformers[permType]) { - await resourceIdTranformers[permType]!(ctx) - } + if (resourceIdTranformers[permType]) { + await resourceIdTranformers[permType]!(ctx) + } + + if (hasResource(ctx)) { resourceRoles = await roles.getRequiredResourceRole(permLevel!, ctx) if (opts && opts.schema) { otherLevelRoles = await roles.getRequiredResourceRole(otherLevel, ctx) diff --git a/packages/server/src/middleware/tests/authorized.spec.ts b/packages/server/src/middleware/tests/authorized.spec.ts index 900928f0bc..4d6f281294 100644 --- a/packages/server/src/middleware/tests/authorized.spec.ts +++ b/packages/server/src/middleware/tests/authorized.spec.ts @@ -3,13 +3,16 @@ jest.mock("../../environment", () => ({ isTest: () => true, // @ts-ignore isProd: () => this.prod, - _set: function (key: string, value: string) { + _set: function (_key: string, value: string) { this.prod = value === "production" }, })) + +import { PermissionType, PermissionLevel } from "@budibase/types" + import authorizedMiddleware from "../authorized" import env from "../../environment" -import { PermissionType, PermissionLevel } from "@budibase/types" +import { generateTableID, generateViewID } from "../../db/utils" const APP_ID = "" @@ -51,7 +54,7 @@ class TestConfiguration { this.middleware = authorizedMiddleware(...perms) } - setResourceId(id: string) { + setResourceId(id?: string) { this.ctx.resourceId = id } @@ -85,6 +88,7 @@ describe("Authorization middleware", () => { }) beforeEach(() => { + jest.clearAllMocks() config = new TestConfiguration() }) @@ -172,5 +176,44 @@ describe("Authorization middleware", () => { "User does not have permission" ) }) + + describe("view type", () => { + const tableId = generateTableID() + const viewId = generateViewID(tableId) + + beforeEach(() => { + config.setMiddlewareRequiredPermission( + PermissionType.VIEW, + PermissionLevel.READ + ) + config.setResourceId(viewId) + + config.setUser({ + role: { + _id: "", + }, + }) + }) + + it("throw an exception if the resource id is not provided", async () => { + config.setResourceId(undefined) + await config.executeMiddleware() + expect(config.throw).toHaveBeenNthCalledWith( + 1, + 400, + "Cannot obtain the view id" + ) + }) + + it("throw an exception if the resource id is not a valid view id", async () => { + config.setResourceId(tableId) + await config.executeMiddleware() + expect(config.throw).toHaveBeenNthCalledWith( + 1, + 400, + `"${tableId}" is not a valid view id` + ) + }) + }) }) })