diff --git a/packages/server/src/api/routes/tests/rowAction.spec.ts b/packages/server/src/api/routes/tests/rowAction.spec.ts index d43f5075d0..3f4447c50d 100644 --- a/packages/server/src/api/routes/tests/rowAction.spec.ts +++ b/packages/server/src/api/routes/tests/rowAction.spec.ts @@ -5,11 +5,10 @@ import { CreateRowActionRequest, DocumentType, PermissionLevel, - Row, RowActionResponse, } from "@budibase/types" import * as setup from "./utilities" -import { generator } from "@budibase/backend-core/tests" +import { generator, mocks } from "@budibase/backend-core/tests" import { Expectations } from "../../../tests/utilities/api/base" import { roles } from "@budibase/backend-core" import { automations } from "@budibase/pro" @@ -651,13 +650,27 @@ describe("/rowsActions", () => { }) describe("trigger", () => { - let row: Row + let viewId: string + let rowId: string let rowAction: RowActionResponse beforeEach(async () => { - row = await config.api.row.save(tableId, {}) + const row = await config.api.row.save(tableId, {}) + rowId = row._id! rowAction = await createRowAction(tableId, createRowActionRequest()) + viewId = ( + await config.api.viewV2.create( + setup.structures.viewV2.createRequest(tableId) + ) + ).id + + await config.api.rowAction.setViewPermission( + tableId, + viewId, + rowAction.id + ) + await config.publish() tk.travel(Date.now() + 100) }) @@ -673,9 +686,7 @@ describe("/rowsActions", () => { it("can trigger an automation given valid data", async () => { expect(await getAutomationLogs()).toBeEmpty() - await config.api.rowAction.trigger(tableId, rowAction.id, { - rowId: row._id!, - }) + await config.api.rowAction.trigger(viewId, rowAction.id, { rowId }) const automationLogs = await getAutomationLogs() expect(automationLogs).toEqual([ @@ -687,8 +698,11 @@ describe("/rowsActions", () => { inputs: null, outputs: { fields: {}, - row: await config.api.row.get(tableId, row._id!), - table: await config.api.table.get(tableId), + row: await config.api.row.get(tableId, rowId), + table: { + ...(await config.api.table.get(tableId)), + views: expect.anything(), + }, automation: expect.objectContaining({ _id: rowAction.automationId, }), @@ -709,9 +723,7 @@ describe("/rowsActions", () => { await config.api.rowAction.trigger( viewId, rowAction.id, - { - rowId: row._id!, - }, + { rowId }, { status: 403, body: { @@ -738,10 +750,9 @@ describe("/rowsActions", () => { ) await config.publish() + expect(await getAutomationLogs()).toBeEmpty() - await config.api.rowAction.trigger(viewId, rowAction.id, { - rowId: row._id!, - }) + await config.api.rowAction.trigger(viewId, rowAction.id, { rowId }) const automationLogs = await getAutomationLogs() expect(automationLogs).toEqual([ @@ -750,5 +761,170 @@ describe("/rowsActions", () => { }), ]) }) + + describe("role permission checks", () => { + beforeAll(() => { + mocks.licenses.useViewPermissions() + }) + + afterAll(() => { + mocks.licenses.useCloudFree() + }) + + function createUser(role: string) { + return config.createUser({ + admin: { global: false }, + builder: {}, + roles: { [config.getProdAppId()]: role }, + }) + } + + const allowedRoleConfig = (() => { + function getRolesLowerThan(role: string) { + const result = Object.values(roles.BUILTIN_ROLE_IDS).filter( + r => r !== role && roles.lowerBuiltinRoleID(r, role) === r + ) + return result + } + return Object.values(roles.BUILTIN_ROLE_IDS).flatMap(r => + [r, ...getRolesLowerThan(r)].map(p => [r, p]) + ) + })() + + const disallowedRoleConfig = (() => { + function getRolesHigherThan(role: string) { + const result = Object.values(roles.BUILTIN_ROLE_IDS).filter( + r => r !== role && roles.lowerBuiltinRoleID(r, role) === role + ) + return result + } + return Object.values(roles.BUILTIN_ROLE_IDS).flatMap(r => + getRolesHigherThan(r).map(p => [r, p]) + ) + })() + + describe.each([ + [ + "view (with implicit views)", + async () => { + const viewId = ( + await config.api.viewV2.create( + setup.structures.viewV2.createRequest(tableId) + ) + ).id + + await config.api.rowAction.setViewPermission( + tableId, + viewId, + rowAction.id + ) + return { permissionResource: viewId, triggerResouce: viewId } + }, + ], + [ + "view (without implicit views)", + async () => { + const viewId = ( + await config.api.viewV2.create( + setup.structures.viewV2.createRequest(tableId) + ) + ).id + + await config.api.rowAction.setViewPermission( + tableId, + viewId, + rowAction.id + ) + return { permissionResource: tableId, triggerResouce: viewId } + }, + ], + ])("checks for %s", (_, getResources) => { + it.each(allowedRoleConfig)( + "allows triggering if the user has read permission (user %s, table %s)", + async (userRole, resourcePermission) => { + const { permissionResource, triggerResouce } = await getResources() + + await config.api.permission.add({ + level: PermissionLevel.READ, + resourceId: permissionResource, + roleId: resourcePermission, + }) + + const normalUser = await createUser(userRole) + + await config.withUser(normalUser, async () => { + await config.publish() + await config.api.rowAction.trigger( + triggerResouce, + rowAction.id, + { rowId }, + { status: 200 } + ) + }) + } + ) + + it.each(disallowedRoleConfig)( + "rejects if the user does not have table read permission (user %s, table %s)", + async (userRole, resourcePermission) => { + const { permissionResource, triggerResouce } = await getResources() + await config.api.permission.add({ + level: PermissionLevel.READ, + resourceId: permissionResource, + roleId: resourcePermission, + }) + + const normalUser = await createUser(userRole) + + await config.withUser(normalUser, async () => { + await config.publish() + await config.api.rowAction.trigger( + triggerResouce, + rowAction.id, + { rowId }, + { + status: 403, + body: { message: "User does not have permission" }, + } + ) + + const automationLogs = await getAutomationLogs() + expect(automationLogs).toBeEmpty() + }) + } + ) + }) + + it.each(allowedRoleConfig)( + "does not allow running row actions for tables by default even", + async (userRole, resourcePermission) => { + await config.api.permission.add({ + level: PermissionLevel.READ, + resourceId: tableId, + roleId: resourcePermission, + }) + + const normalUser = await createUser(userRole) + + await config.withUser(normalUser, async () => { + await config.publish() + await config.api.rowAction.trigger( + tableId, + rowAction.id, + { rowId }, + { + status: 403, + body: { + message: `Row action '${rowAction.id}' is not enabled for table '${tableId}'`, + }, + } + ) + + const automationLogs = await getAutomationLogs() + expect(automationLogs).toBeEmpty() + }) + } + ) + }) }) }) diff --git a/packages/server/src/middleware/authorized.ts b/packages/server/src/middleware/authorized.ts index b23a9846b7..3bead7f80d 100644 --- a/packages/server/src/middleware/authorized.ts +++ b/packages/server/src/middleware/authorized.ts @@ -88,7 +88,7 @@ const authorized = opts = { schema: false }, resourcePath?: string ) => - async (ctx: any, next: any) => { + async (ctx: UserCtx, next: any) => { // webhooks don't need authentication, each webhook unique // also internal requests (between services) don't need authorized if (isWebhookEndpoint(ctx) || ctx.internal) { diff --git a/packages/server/src/middleware/triggerRowActionAuthorised.ts b/packages/server/src/middleware/triggerRowActionAuthorised.ts index be5c6a97e1..17f22b7000 100644 --- a/packages/server/src/middleware/triggerRowActionAuthorised.ts +++ b/packages/server/src/middleware/triggerRowActionAuthorised.ts @@ -1,40 +1,52 @@ import { Next } from "koa" -import { Ctx } from "@budibase/types" -import { paramSubResource } from "./resourceId" +import { PermissionLevel, PermissionType, UserCtx } from "@budibase/types" import { docIds } from "@budibase/backend-core" import * as utils from "../db/utils" import sdk from "../sdk" +import { authorizedResource } from "./authorized" export function triggerRowActionAuthorised( sourcePath: string, actionPath: string ) { - return async (ctx: Ctx, next: Next) => { - // Reusing the existing middleware to extract the value - paramSubResource(sourcePath, actionPath)(ctx, () => {}) - const { resourceId: sourceId, subResourceId: rowActionId } = ctx + return async (ctx: UserCtx, next: Next) => { + async function getResourceIds() { + const sourceId: string = ctx.params[sourcePath] + const rowActionId: string = ctx.params[actionPath] - const isTableId = docIds.isTableId(sourceId) - const isViewId = utils.isViewID(sourceId) - if (!isTableId && !isViewId) { - ctx.throw(400, `'${sourceId}' is not a valid source id`) + const isTableId = docIds.isTableId(sourceId) + const isViewId = utils.isViewID(sourceId) + if (!isTableId && !isViewId) { + ctx.throw(400, `'${sourceId}' is not a valid source id`) + } + + const tableId = isTableId + ? sourceId + : utils.extractViewInfoFromID(sourceId).tableId + const viewId = isTableId ? undefined : sourceId + return { tableId, viewId, rowActionId } } - const tableId = isTableId - ? sourceId - : utils.extractViewInfoFromID(sourceId).tableId + const { tableId, viewId, rowActionId } = await getResourceIds() + // Check if the user has permissions to the table/view + await authorizedResource( + !viewId ? PermissionType.TABLE : PermissionType.VIEW, + PermissionLevel.READ, + sourcePath + )(ctx, () => {}) + + // Check is the row action can run for the given view/table const rowAction = await sdk.rowActions.get(tableId, rowActionId) - - if (isTableId && !rowAction.permissions.table.runAllowed) { + if (!viewId && !rowAction.permissions.table.runAllowed) { ctx.throw( 403, - `Row action '${rowActionId}' is not enabled for table '${sourceId}'` + `Row action '${rowActionId}' is not enabled for table '${tableId}'` ) - } else if (isViewId && !rowAction.permissions.views[sourceId]?.runAllowed) { + } else if (viewId && !rowAction.permissions.views[viewId]?.runAllowed) { ctx.throw( 403, - `Row action '${rowActionId}' is not enabled for view '${sourceId}'` + `Row action '${rowActionId}' is not enabled for view '${viewId}'` ) } diff --git a/packages/server/src/sdk/app/rowActions.ts b/packages/server/src/sdk/app/rowActions.ts index e963a4317b..9a7d402df0 100644 --- a/packages/server/src/sdk/app/rowActions.ts +++ b/packages/server/src/sdk/app/rowActions.ts @@ -75,7 +75,7 @@ export async function create(tableId: string, rowAction: { name: string }) { name: action.name, automationId: automation._id!, permissions: { - table: { runAllowed: true }, + table: { runAllowed: false }, views: {}, }, }