Merge pull request #14707 from Budibase/row-action/set-table-permission

Row action/set table permission

packages/server/src/api/controllers/rowAction/crud.ts

@@ -1,6 +1,7 @@
 import {
   CreateRowActionRequest,
   Ctx,
+  RowActionPermissions,
   RowActionResponse,
   RowActionsResponse,
   UpdateRowActionRequest,
@@ -18,25 +19,26 @@ async function getTable(ctx: Ctx) {
 
 export async function find(ctx: Ctx<void, RowActionsResponse>) {
   const table = await getTable(ctx)
+  const tableId = table._id!
 
-  if (!(await sdk.rowActions.docExists(table._id!))) {
+  if (!(await sdk.rowActions.docExists(tableId))) {
     ctx.body = {
       actions: {},
     }
     return
   }
 
-  const { actions } = await sdk.rowActions.getAll(table._id!)
+  const { actions } = await sdk.rowActions.getAll(tableId)
   const result: RowActionsResponse = {
     actions: Object.entries(actions).reduce<Record<string, RowActionResponse>>(
       (acc, [key, action]) => ({
         ...acc,
         [key]: {
           id: key,
-          tableId: table._id!,
+          tableId,
           name: action.name,
           automationId: action.automationId,
-          allowedViews: flattenAllowedViews(action.permissions.views),
+          allowedSources: flattenAllowedSources(tableId, action.permissions),
         },
       }),
       {}
@@ -49,17 +51,18 @@ export async function create(
   ctx: Ctx<CreateRowActionRequest, RowActionResponse>
 ) {
   const table = await getTable(ctx)
+  const tableId = table._id!
 
-  const createdAction = await sdk.rowActions.create(table._id!, {
+  const createdAction = await sdk.rowActions.create(tableId, {
     name: ctx.request.body.name,
   })
 
   ctx.body = {
-    tableId: table._id!,
+    tableId,
     id: createdAction.id,
     name: createdAction.name,
     automationId: createdAction.automationId,
-    allowedViews: undefined,
+    allowedSources: flattenAllowedSources(tableId, createdAction.permissions),
   }
   ctx.status = 201
 }
@@ -68,18 +71,19 @@ export async function update(
   ctx: Ctx<UpdateRowActionRequest, RowActionResponse>
 ) {
   const table = await getTable(ctx)
+  const tableId = table._id!
   const { actionId } = ctx.params
 
-  const action = await sdk.rowActions.update(table._id!, actionId, {
+  const action = await sdk.rowActions.update(tableId, actionId, {
     name: ctx.request.body.name,
   })
 
   ctx.body = {
-    tableId: table._id!,
+    tableId,
     id: action.id,
     name: action.name,
     automationId: action.automationId,
-    allowedViews: undefined,
+    allowedSources: flattenAllowedSources(tableId, action.permissions),
   }
 }
 
@@ -91,52 +95,89 @@ export async function remove(ctx: Ctx<void, void>) {
   ctx.status = 204
 }
 
+export async function setTablePermission(ctx: Ctx<void, RowActionResponse>) {
+  const table = await getTable(ctx)
+  const tableId = table._id!
+  const { actionId } = ctx.params
+
+  const action = await sdk.rowActions.setTablePermission(tableId, actionId)
+  ctx.body = {
+    tableId,
+    id: action.id,
+    name: action.name,
+    automationId: action.automationId,
+    allowedSources: flattenAllowedSources(tableId, action.permissions),
+  }
+}
+
+export async function unsetTablePermission(ctx: Ctx<void, RowActionResponse>) {
+  const table = await getTable(ctx)
+  const tableId = table._id!
+  const { actionId } = ctx.params
+
+  const action = await sdk.rowActions.unsetTablePermission(tableId, actionId)
+
+  ctx.body = {
+    tableId,
+    id: action.id,
+    name: action.name,
+    automationId: action.automationId,
+    allowedSources: flattenAllowedSources(tableId, action.permissions),
+  }
+}
+
 export async function setViewPermission(ctx: Ctx<void, RowActionResponse>) {
   const table = await getTable(ctx)
+  const tableId = table._id!
   const { actionId, viewId } = ctx.params
 
   const action = await sdk.rowActions.setViewPermission(
-    table._id!,
+    tableId,
     actionId,
     viewId
   )
   ctx.body = {
-    tableId: table._id!,
+    tableId,
     id: action.id,
     name: action.name,
     automationId: action.automationId,
-    allowedViews: flattenAllowedViews(action.permissions.views),
+    allowedSources: flattenAllowedSources(tableId, action.permissions),
   }
 }
 
 export async function unsetViewPermission(ctx: Ctx<void, RowActionResponse>) {
   const table = await getTable(ctx)
+  const tableId = table._id!
   const { actionId, viewId } = ctx.params
 
   const action = await sdk.rowActions.unsetViewPermission(
-    table._id!,
+    tableId,
     actionId,
     viewId
   )
 
   ctx.body = {
-    tableId: table._id!,
+    tableId,
     id: action.id,
     name: action.name,
     automationId: action.automationId,
-    allowedViews: flattenAllowedViews(action.permissions.views),
+    allowedSources: flattenAllowedSources(tableId, action.permissions),
   }
 }
 
-function flattenAllowedViews(
+function flattenAllowedSources(
-  permissions: Record<string, { runAllowed: boolean }>
+  tableId: string,
+  permissions: RowActionPermissions
 ) {
-  const allowedPermissions = Object.entries(permissions || {})
+  const allowedPermissions = []
-    .filter(([_, p]) => p.runAllowed)
+  if (permissions.table.runAllowed) {
-    .map(([viewId]) => viewId)
+    allowedPermissions.push(tableId)
-  if (!allowedPermissions.length) {
-    return undefined
   }
+  allowedPermissions.push(
+    ...Object.keys(permissions.views || {}).filter(
+      viewId => permissions.views[viewId].runAllowed
+    )
+  )
 
   return allowedPermissions
 }

packages/server/src/api/routes/rowAction.ts

@@ -51,6 +51,16 @@ router
     authorized(BUILDER),
     rowActionController.remove
   )
+  .post(
+    "/api/tables/:tableId/actions/:actionId/permissions",
+    authorized(BUILDER),
+    rowActionController.setTablePermission
+  )
+  .delete(
+    "/api/tables/:tableId/actions/:actionId/permissions",
+    authorized(BUILDER),
+    rowActionController.unsetTablePermission
+  )
   .post(
     "/api/tables/:tableId/actions/:actionId/permissions/:viewId",
     authorized(BUILDER),

packages/server/src/api/routes/tests/rowAction.spec.ts

@@ -133,6 +133,7 @@ describe("/rowsActions", () => {
         id: expect.stringMatching(/^row_action_\w+/),
         tableId: tableId,
         automationId: expectAutomationId(),
+        allowedSources: [tableId],
       })
 
       expect(await config.api.rowAction.find(tableId)).toEqual({
@@ -142,6 +143,7 @@ describe("/rowsActions", () => {
             id: res.id,
             tableId: tableId,
             automationId: expectAutomationId(),
+            allowedSources: [tableId],
           },
         },
       })
@@ -180,18 +182,21 @@ describe("/rowsActions", () => {
             id: responses[0].id,
             tableId,
             automationId: expectAutomationId(),
+            allowedSources: [tableId],
           },
           [responses[1].id]: {
             name: rowActions[1].name,
             id: responses[1].id,
             tableId,
             automationId: expectAutomationId(),
+            allowedSources: [tableId],
           },
           [responses[2].id]: {
             name: rowActions[2].name,
             id: responses[2].id,
             tableId,
             automationId: expectAutomationId(),
+            allowedSources: [tableId],
           },
         },
       })
@@ -224,6 +229,7 @@ describe("/rowsActions", () => {
         id: expect.any(String),
         tableId,
         automationId: expectAutomationId(),
+        allowedSources: [tableId],
       })
 
       expect(await config.api.rowAction.find(tableId)).toEqual({
@@ -233,6 +239,7 @@ describe("/rowsActions", () => {
             id: res.id,
             tableId: tableId,
             automationId: expectAutomationId(),
+            allowedSources: [tableId],
           },
         },
       })
@@ -354,6 +361,7 @@ describe("/rowsActions", () => {
         tableId,
         name: updatedName,
         automationId: actionData.automationId,
+        allowedSources: [tableId],
       })
 
       expect(await config.api.rowAction.find(tableId)).toEqual(
@@ -364,6 +372,7 @@ describe("/rowsActions", () => {
               id: actionData.id,
               tableId: actionData.tableId,
               automationId: actionData.automationId,
+              allowedSources: [tableId],
             },
           }),
         })
@@ -515,6 +524,81 @@ describe("/rowsActions", () => {
     })
   })
 
+  describe("set/unsetTablePermission", () => {
+    describe.each([
+      ["setTablePermission", config.api.rowAction.setTablePermission],
+      ["unsetTablePermission", config.api.rowAction.unsetTablePermission],
+    ])("unauthorisedTests for %s", (__, delegateTest) => {
+      unauthorisedTests((expectations, testConfig) =>
+        delegateTest(tableId, generator.guid(), expectations, testConfig)
+      )
+    })
+
+    let actionId1: string, actionId2: string
+
+    beforeEach(async () => {
+      for (const rowAction of createRowActionRequests(3)) {
+        await createRowAction(tableId, rowAction)
+      }
+      const persisted = await config.api.rowAction.find(tableId)
+
+      const actions = _.sampleSize(Object.keys(persisted.actions), 2)
+      actionId1 = actions[0]
+      actionId2 = actions[1]
+    })
+
+    it("can set table permission", async () => {
+      await config.api.rowAction.unsetTablePermission(tableId, actionId1)
+      await config.api.rowAction.unsetTablePermission(tableId, actionId2)
+      const actionResult = await config.api.rowAction.setTablePermission(
+        tableId,
+        actionId1
+      )
+      const expectedAction1 = expect.objectContaining({
+        allowedSources: [tableId],
+      })
+
+      const expectedActions = expect.objectContaining({
+        [actionId1]: expectedAction1,
+        [actionId2]: expect.objectContaining({
+          allowedSources: [],
+        }),
+      })
+      expect(actionResult).toEqual(expectedAction1)
+      expect((await config.api.rowAction.find(tableId)).actions).toEqual(
+        expectedActions
+      )
+    })
+
+    it("can unset table permission", async () => {
+      const actionResult = await config.api.rowAction.unsetTablePermission(
+        tableId,
+        actionId1
+      )
+
+      const expectedAction = expect.objectContaining({
+        allowedSources: [],
+      })
+      expect(actionResult).toEqual(expectedAction)
+      expect(
+        (await config.api.rowAction.find(tableId)).actions[actionId1]
+      ).toEqual(expectedAction)
+    })
+
+    it.each([
+      ["setTablePermission", config.api.rowAction.setTablePermission],
+      ["unsetTablePermission", config.api.rowAction.unsetTablePermission],
+    ])(
+      "cannot update permission for unexisting tables (%s)",
+      async (__, delegateTest) => {
+        const tableId = generator.guid()
+        await delegateTest(tableId, actionId1, {
+          status: 404,
+        })
+      }
+    )
+  })
+
   describe("set/unsetViewPermission", () => {
     describe.each([
       ["setViewPermission", config.api.rowAction.setViewPermission],
@@ -531,11 +615,9 @@ describe("/rowsActions", () => {
       )
     })
 
-    let tableIdForDescribe: string
     let actionId1: string, actionId2: string
     let viewId1: string, viewId2: string
-    beforeAll(async () => {
+    beforeEach(async () => {
-      tableIdForDescribe = tableId
       for (const rowAction of createRowActionRequests(3)) {
         await createRowAction(tableId, rowAction)
       }
@@ -557,11 +639,6 @@ describe("/rowsActions", () => {
       ).id
     })
 
-    beforeEach(() => {
-      // Hack to reuse tables for these given tests
-      tableId = tableIdForDescribe
-    })
-
     it("can set permission views", async () => {
       await config.api.rowAction.setViewPermission(tableId, viewId1, actionId1)
       const action1Result = await config.api.rowAction.setViewPermission(
@@ -576,10 +653,10 @@ describe("/rowsActions", () => {
       )
 
       const expectedAction1 = expect.objectContaining({
-        allowedViews: [viewId1, viewId2],
+        allowedSources: [tableId, viewId1, viewId2],
       })
       const expectedAction2 = expect.objectContaining({
-        allowedViews: [viewId1],
+        allowedSources: [tableId, viewId1],
       })
 
       const expectedActions = expect.objectContaining({
@@ -594,6 +671,8 @@ describe("/rowsActions", () => {
     })
 
     it("can unset permission views", async () => {
+      await config.api.rowAction.setViewPermission(tableId, viewId2, actionId1)
+      await config.api.rowAction.setViewPermission(tableId, viewId1, actionId2)
       const actionResult = await config.api.rowAction.unsetViewPermission(
         tableId,
         viewId1,
@@ -601,7 +680,7 @@ describe("/rowsActions", () => {
       )
 
       const expectedAction = expect.objectContaining({
-        allowedViews: [viewId2],
+        allowedSources: [tableId, viewId2],
       })
       expect(actionResult).toEqual(expectedAction)
       expect(
@@ -672,6 +751,7 @@ describe("/rowsActions", () => {
       )
 
       await config.publish()
+      // Travel time in order to "trim" the selected `getAutomationLogs`
       tk.travel(Date.now() + 100)
     })
 
@@ -712,6 +792,38 @@ describe("/rowsActions", () => {
       ])
     })
 
+    it("triggers from an allowed table", async () => {
+      expect(await getAutomationLogs()).toBeEmpty()
+      await config.api.rowAction.trigger(tableId, rowAction.id, { rowId })
+
+      const automationLogs = await getAutomationLogs()
+      expect(automationLogs).toEqual([
+        expect.objectContaining({
+          automationId: rowAction.automationId,
+        }),
+      ])
+    })
+
+    it("rejects triggering from a non-allowed table", async () => {
+      await config.api.rowAction.unsetTablePermission(tableId, rowAction.id)
+      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).toEqual([])
+    })
+
     it("rejects triggering from a non-allowed view", async () => {
       const viewId = (
         await config.api.viewV2.create(
@@ -901,7 +1013,7 @@ describe("/rowsActions", () => {
       })
 
       it.each(allowedRoleConfig)(
-        "does not allow running row actions for tables by default even",
+        "allow running row actions for tables by default",
         async (userRole, resourcePermission) => {
           await config.api.permission.add({
             level: PermissionLevel.READ,
@@ -918,15 +1030,12 @@ describe("/rowsActions", () => {
               rowAction.id,
               { rowId },
               {
-                status: 403,
+                status: 200,
-                body: {
-                  message: `Row action '${rowAction.id}' is not enabled for table '${tableId}'`,
-                },
               }
             )
 
             const automationLogs = await getAutomationLogs()
-            expect(automationLogs).toBeEmpty()
+            expect(automationLogs).toHaveLength(1)
           })
         }
       )

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: false },
+      table: { runAllowed: true },
       views: {},
     },
   }
@@ -163,6 +163,23 @@ async function guardView(tableId: string, viewId: string) {
   }
 }
 
+export async function setTablePermission(tableId: string, rowActionId: string) {
+  return await updateDoc(tableId, rowActionId, async actionsDoc => {
+    actionsDoc.actions[rowActionId].permissions.table.runAllowed = true
+    return actionsDoc
+  })
+}
+
+export async function unsetTablePermission(
+  tableId: string,
+  rowActionId: string
+) {
+  return await updateDoc(tableId, rowActionId, async actionsDoc => {
+    actionsDoc.actions[rowActionId].permissions.table.runAllowed = false
+    return actionsDoc
+  })
+}
+
 export async function setViewPermission(
   tableId: string,
   rowActionId: string,

packages/server/src/tests/utilities/api/rowAction.ts

@@ -72,6 +72,42 @@ export class RowActionAPI extends TestAPI {
     )
   }
 
+  setTablePermission = async (
+    tableId: string,
+    rowActionId: string,
+    expectations?: Expectations,
+    config?: { publicUser?: boolean }
+  ) => {
+    return await this._post<RowActionResponse>(
+      `/api/tables/${tableId}/actions/${rowActionId}/permissions`,
+      {
+        expectations: {
+          status: 200,
+          ...expectations,
+        },
+        ...config,
+      }
+    )
+  }
+
+  unsetTablePermission = async (
+    tableId: string,
+    rowActionId: string,
+    expectations?: Expectations,
+    config?: { publicUser?: boolean }
+  ) => {
+    return await this._delete<RowActionResponse>(
+      `/api/tables/${tableId}/actions/${rowActionId}/permissions`,
+      {
+        expectations: {
+          status: 200,
+          ...expectations,
+        },
+        ...config,
+      }
+    )
+  }
+
   setViewPermission = async (
     tableId: string,
     viewId: string,

packages/types/src/api/web/app/rowAction.ts

@@ -8,7 +8,7 @@ export interface RowActionResponse extends RowActionData {
   id: string
   tableId: string
   automationId: string
-  allowedViews: string[] | undefined
+  allowedSources: string[] | undefined
 }
 
 export interface RowActionsResponse {

packages/types/src/documents/app/rowAction.ts

@@ -8,8 +8,10 @@ export interface TableRowActions extends Document {
 export interface RowActionData {
   name: string
   automationId: string
-  permissions: {
+  permissions: RowActionPermissions
-    table: { runAllowed: boolean }
+}
-    views: Record<string, { runAllowed: boolean }>
+
-  }
+export interface RowActionPermissions {
+  table: { runAllowed: boolean }
+  views: Record<string, { runAllowed: boolean }>
 }