diff --git a/.github/workflows/pr-labeler.yml b/.github/workflows/pr-labeler.yml index e2fa9f2515..01ff12491e 100644 --- a/.github/workflows/pr-labeler.yml +++ b/.github/workflows/pr-labeler.yml @@ -21,6 +21,7 @@ jobs: l_max_size: "1000" fail_if_xl: "false" files_to_ignore: "yarn.lock" + message_if_xl: "" team-labeler: runs-on: ubuntu-latest diff --git a/README.md b/README.md index 64492b97e4..26ad9f80c2 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ Budibase is open-source - licensed as GPL v3. This should fill you with confiden

### Load data or start from scratch -Budibase pulls data from multiple sources, including MongoDB, CouchDB, PostgreSQL, MySQL, Airtable, S3, DynamoDB, or a REST API. And unlike other platforms, with Budibase you can start from scratch and create business apps with no data sources. [Request new datasources](https://github.com/Budibase/budibase/discussions?discussions_q=category%3AIdeas). +Budibase pulls data from multiple sources, including MongoDB, CouchDB, PostgreSQL, MariaDB, MySQL, Airtable, S3, DynamoDB, or a REST API. And unlike other platforms, with Budibase you can start from scratch and create business apps with no data sources. [Request new datasources](https://github.com/Budibase/budibase/discussions?discussions_q=category%3AIdeas).

Budibase data diff --git a/lerna.json b/lerna.json index bb0302353e..f24c0e095b 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.31.5", + "version": "2.31.7", "npmClient": "yarn", "packages": [ "packages/*", diff --git a/packages/builder/src/pages/builder/portal/_components/LockedFeature.svelte b/packages/builder/src/pages/builder/portal/_components/LockedFeature.svelte index e6f4075e2e..1df724099b 100644 --- a/packages/builder/src/pages/builder/portal/_components/LockedFeature.svelte +++ b/packages/builder/src/pages/builder/portal/_components/LockedFeature.svelte @@ -1,10 +1,12 @@ @@ -36,8 +40,9 @@ {:else}

+ {#if upgradeDisabled} + +
+ +
+ + {/if}
{/if} @@ -67,7 +82,11 @@ justify-content: flex-start; gap: var(--spacing-m); } - + .icon { + position: relative; + display: flex; + justify-content: center; + } .buttons { display: flex; flex-direction: row; diff --git a/packages/builder/src/pages/builder/portal/users/users/index.svelte b/packages/builder/src/pages/builder/portal/users/users/index.svelte index b91b2129e6..a1d5496ff6 100644 --- a/packages/builder/src/pages/builder/portal/users/users/index.svelte +++ b/packages/builder/src/pages/builder/portal/users/users/index.svelte @@ -127,7 +127,10 @@ name: user.firstName ? user.firstName + " " + user.lastName : "", userGroups, __selectable: - role.value === Constants.BudibaseRoles.Owner ? false : undefined, + role.value === Constants.BudibaseRoles.Owner || + $auth.user?.email === user.email + ? false + : true, apps: [...new Set(Object.keys(user.roles))], access: role.sortOrder, } @@ -392,7 +395,7 @@ allowSelectRows={!readonly} {customRenderers} loading={!$fetch.loaded || !groupsLoaded} - defaultSortColumn={"access"} + defaultSortColumn={"__selectable"} /> {/if} - - - {#if !empty && mounted} + {#if mounted} {/if} diff --git a/packages/client/src/components/preview/DNDPlaceholderOverlay.svelte b/packages/client/src/components/preview/DNDPlaceholderOverlay.svelte index 0ad4280e07..61cecc885b 100644 --- a/packages/client/src/components/preview/DNDPlaceholderOverlay.svelte +++ b/packages/client/src/components/preview/DNDPlaceholderOverlay.svelte @@ -6,8 +6,11 @@ let left, top, height, width const updatePosition = () => { - const node = - document.getElementsByClassName(DNDPlaceholderID)[0]?.childNodes[0] + let node = document.getElementsByClassName(DNDPlaceholderID)[0] + const insideGrid = node?.dataset.insideGrid === "true" + if (!insideGrid) { + node = document.getElementsByClassName(`${DNDPlaceholderID}-dom`)[0] + } if (!node) { height = 0 width = 0 diff --git a/packages/client/src/components/preview/HoverIndicator.svelte b/packages/client/src/components/preview/HoverIndicator.svelte index d204d77f49..f1c151ce16 100644 --- a/packages/client/src/components/preview/HoverIndicator.svelte +++ b/packages/client/src/components/preview/HoverIndicator.svelte @@ -19,7 +19,7 @@ newId = e.target.dataset.id } else { // Handle normal components - const element = e.target.closest(".interactive.component") + const element = e.target.closest(".interactive.component:not(.root)") newId = element?.dataset?.id } diff --git a/packages/client/src/stores/dnd.js b/packages/client/src/stores/dnd.js index 1bdd510eb7..43d4eeeed8 100644 --- a/packages/client/src/stores/dnd.js +++ b/packages/client/src/stores/dnd.js @@ -1,5 +1,5 @@ import { writable } from "svelte/store" -import { computed } from "../utils/computed.js" +import { derivedMemo } from "@budibase/frontend-core" const createDndStore = () => { const initialState = { @@ -78,11 +78,11 @@ export const dndStore = createDndStore() // performance by deriving any state that needs to be externally observed. // By doing this and using primitives, we can avoid invalidating other stores // or components which depend on DND state unless values actually change. -export const dndParent = computed(dndStore, x => x.drop?.parent) -export const dndIndex = computed(dndStore, x => x.drop?.index) -export const dndBounds = computed(dndStore, x => x.source?.bounds) -export const dndIsDragging = computed(dndStore, x => !!x.source) -export const dndIsNewComponent = computed( +export const dndParent = derivedMemo(dndStore, x => x.drop?.parent) +export const dndIndex = derivedMemo(dndStore, x => x.drop?.index) +export const dndBounds = derivedMemo(dndStore, x => x.source?.bounds) +export const dndIsDragging = derivedMemo(dndStore, x => !!x.source) +export const dndIsNewComponent = derivedMemo( dndStore, x => x.source?.newComponentType != null ) diff --git a/packages/client/src/stores/screens.js b/packages/client/src/stores/screens.js index 3c5ece0a6c..bc87216660 100644 --- a/packages/client/src/stores/screens.js +++ b/packages/client/src/stores/screens.js @@ -92,6 +92,8 @@ const createScreenStore = () => { width: `${$dndBounds?.width || 400}px`, height: `${$dndBounds?.height || 200}px`, opacity: 0, + "--default-width": $dndBounds?.width || 400, + "--default-height": $dndBounds?.height || 200, }, }, static: true, diff --git a/packages/client/src/utils/computed.js b/packages/client/src/utils/computed.js deleted file mode 100644 index aa89e7ad1b..0000000000 --- a/packages/client/src/utils/computed.js +++ /dev/null @@ -1,38 +0,0 @@ -import { writable } from "svelte/store" - -/** - * Extension of Svelte's built in "derived" stores, which the addition of deep - * comparison of non-primitives. Falls back to using shallow comparison for - * primitive types to avoid performance penalties. - * Useful for instances where a deep comparison is cheaper than an additional - * store invalidation. - * @param store the store to observer - * @param deriveValue the derivation function - * @returns {Writable<*>} a derived svelte store containing just the derived value - */ -export const computed = (store, deriveValue) => { - const initialValue = deriveValue(store) - const computedStore = writable(initialValue) - let lastKey = getKey(initialValue) - - store.subscribe(state => { - const value = deriveValue(state) - const key = getKey(value) - if (key !== lastKey) { - lastKey = key - computedStore.set(value) - } - }) - - return computedStore -} - -// Helper function to serialise any value into a primitive which can be cheaply -// and shallowly compared -const getKey = value => { - if (value == null || typeof value !== "object") { - return value - } else { - return JSON.stringify(value) - } -} diff --git a/packages/client/src/utils/grid.js b/packages/client/src/utils/grid.js index 1727b904ca..142a7ed55a 100644 --- a/packages/client/src/utils/grid.js +++ b/packages/client/src/utils/grid.js @@ -92,8 +92,12 @@ export const gridLayout = (node, metadata) => { } // Determine default width and height of component - let width = errored ? 500 : definition?.size?.width || 200 - let height = errored ? 60 : definition?.size?.height || 200 + let width = styles["--default-width"] ?? definition?.size?.width ?? 200 + let height = styles["--default-height"] ?? definition?.size?.height ?? 200 + if (errored) { + width = 500 + height = 60 + } width += 2 * GridSpacing height += 2 * GridSpacing let vars = { diff --git a/packages/client/src/utils/styleable.js b/packages/client/src/utils/styleable.js index 0f484a9ab9..884420a1fd 100644 --- a/packages/client/src/utils/styleable.js +++ b/packages/client/src/utils/styleable.js @@ -93,7 +93,7 @@ export const styleable = (node, styles = {}) => { node.addEventListener("mouseout", applyNormalStyles) // Add builder preview click listener - if (newStyles.interactive) { + if (newStyles.interactive && !newStyles.isRoot) { node.addEventListener("click", selectComponent, false) node.addEventListener("dblclick", editComponent, false) } diff --git a/packages/server/src/api/routes/tests/automation.spec.ts b/packages/server/src/api/routes/tests/automation.spec.ts index 1ade332f0a..b6233b24ad 100644 --- a/packages/server/src/api/routes/tests/automation.spec.ts +++ b/packages/server/src/api/routes/tests/automation.spec.ts @@ -15,6 +15,7 @@ import { Automation, FieldType, Table } from "@budibase/types" import { mocks } from "@budibase/backend-core/tests" import { FilterConditions } from "../../../automations/steps/filter" import { removeDeprecated } from "../../../automations/utils" +import { createAutomationBuilder } from "../../../automations/tests/utilities/AutomationTestBuilder" const MAX_RETRIES = 4 let { @@ -121,6 +122,104 @@ describe("/automations", () => { expect(events.automation.stepCreated).toHaveBeenCalledTimes(2) }) + it("Should ensure you can't have a branch as not a last step", async () => { + const automation = createAutomationBuilder({ + name: "String Equality Branching", + appId: config.getAppId(), + }) + .appAction({ fields: { status: "active" } }) + .branch({ + activeBranch: { + steps: stepBuilder => + stepBuilder.serverLog({ text: "Active user" }), + condition: { + equal: { "trigger.fields.status": "active" }, + }, + }, + }) + .serverLog({ text: "Inactive user" }) + .build() + + await config.api.automation.post(automation, { + status: 400, + body: { + message: + "Invalid body - Branch steps are only allowed as the last step", + }, + }) + }) + + it("Should check validation on an automation that has a branch step with no children", async () => { + const automation = createAutomationBuilder({ + name: "String Equality Branching", + appId: config.getAppId(), + }) + .appAction({ fields: { status: "active" } }) + .branch({}) + .serverLog({ text: "Inactive user" }) + .build() + + await config.api.automation.post(automation, { + status: 400, + body: { + message: + 'Invalid body - "definition.steps[0].inputs.branches" must contain at least 1 items', + }, + }) + }) + + it("Should check validation on a branch step with empty conditions", async () => { + const automation = createAutomationBuilder({ + name: "String Equality Branching", + appId: config.getAppId(), + }) + .appAction({ fields: { status: "active" } }) + .branch({ + activeBranch: { + steps: stepBuilder => + stepBuilder.serverLog({ text: "Active user" }), + condition: {}, + }, + }) + .build() + + await config.api.automation.post(automation, { + status: 400, + body: { + message: + 'Invalid body - "definition.steps[0].inputs.branches[0].condition" must have at least 1 key', + }, + }) + }) + + it("Should check validation on an branch that has a condition that is not valid", async () => { + const automation = createAutomationBuilder({ + name: "String Equality Branching", + appId: config.getAppId(), + }) + .appAction({ fields: { status: "active" } }) + .branch({ + activeBranch: { + steps: stepBuilder => + stepBuilder.serverLog({ text: "Active user" }), + condition: { + //@ts-ignore + INCORRECT: { "trigger.fields.status": "active" }, + }, + }, + }) + .serverLog({ text: "Inactive user" }) + .build() + + await config.api.automation.post(automation, { + status: 400, + body: { + message: + 'Invalid body - "definition.steps[0].inputs.branches[0].condition.INCORRECT" is not allowed', + }, + }) + }) + it("should apply authorization to endpoint", async () => { const automation = newAutomation() await checkBuilderEndpoint({ 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/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 9aa112cf4d..5e2a585b4a 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -1,6 +1,9 @@ import { auth, permissions } from "@budibase/backend-core" import { DataSourceOperation } from "../../../constants" import { + AutomationActionStepId, + AutomationStep, + AutomationStepType, EmptyFilterOption, SearchFilters, Table, @@ -88,7 +91,8 @@ export function datasourceValidator() { ) } -function filterObject() { +function filterObject(opts?: { unknown: boolean }) { + const { unknown = true } = opts || {} const conditionalFilteringObject = () => Joi.object({ conditions: Joi.array().items(Joi.link("#schema")).required(), @@ -115,7 +119,7 @@ function filterObject() { fuzzyOr: Joi.forbidden(), documentType: Joi.forbidden(), } - return Joi.object(filtersValidators).unknown(true).id("schema") + return Joi.object(filtersValidators).unknown(unknown).id("schema") } export function internalSearchValidator() { @@ -259,6 +263,11 @@ export function screenValidator() { } function generateStepSchema(allowStepTypes: string[]) { + const branchSchema = Joi.object({ + name: Joi.string().required(), + condition: filterObject({ unknown: false }).required().min(1), + }) + return Joi.object({ stepId: Joi.string().required(), id: Joi.string().required(), @@ -267,11 +276,35 @@ function generateStepSchema(allowStepTypes: string[]) { tagline: Joi.string().required(), icon: Joi.string().required(), params: Joi.object(), + inputs: Joi.when("stepId", { + is: AutomationActionStepId.BRANCH, + then: Joi.object({ + branches: Joi.array().items(branchSchema).min(1).required(), + children: Joi.object() + .pattern(Joi.string(), Joi.array().items(Joi.link("#step"))) + .required(), + }).required(), + otherwise: Joi.object(), + }), + args: Joi.object(), type: Joi.string() .required() .valid(...allowStepTypes), - }).unknown(true) + }) + .unknown(true) + .id("step") +} + +const validateStepsArray = ( + steps: AutomationStep[], + helpers: Joi.CustomHelpers +) => { + for (const step of steps.slice(0, -1)) { + if (step.stepId === AutomationActionStepId.BRANCH) { + return helpers.error("branchStepPosition") + } + } } export function automationValidator(existing = false) { @@ -284,9 +317,20 @@ export function automationValidator(existing = false) { definition: Joi.object({ steps: Joi.array() .required() - .items(generateStepSchema(["ACTION", "LOGIC"])), - trigger: generateStepSchema(["TRIGGER"]).allow(null), + .items( + generateStepSchema([ + AutomationStepType.ACTION, + AutomationStepType.LOGIC, + ]) + ) + .custom(validateStepsArray) + .messages({ + branchStepPosition: + "Branch steps are only allowed as the last step", + }), + trigger: generateStepSchema([AutomationStepType.TRIGGER]).allow(null), }) + .required() .unknown(true), }).unknown(true) diff --git a/packages/server/src/automations/tests/scenarios/scenarios.spec.ts b/packages/server/src/automations/tests/scenarios/scenarios.spec.ts index a0dab7f177..7fe4776d54 100644 --- a/packages/server/src/automations/tests/scenarios/scenarios.spec.ts +++ b/packages/server/src/automations/tests/scenarios/scenarios.spec.ts @@ -63,8 +63,8 @@ describe("Automation Scenarios", () => { }, }) .run() - - expect(results.steps[2].outputs.message).toContain("Branch 1.1") + expect(results.steps[3].outputs.status).toContain("branch1 branch taken") + expect(results.steps[4].outputs.message).toContain("Branch 1.1") }) it("should execute correct branch based on string equality", async () => { @@ -91,8 +91,10 @@ describe("Automation Scenarios", () => { }, }) .run() - - expect(results.steps[0].outputs.message).toContain("Active user") + expect(results.steps[0].outputs.status).toContain( + "activeBranch branch taken" + ) + expect(results.steps[1].outputs.message).toContain("Active user") }) it("should handle multiple conditions with AND operator", async () => { @@ -124,7 +126,7 @@ describe("Automation Scenarios", () => { }) .run() - expect(results.steps[0].outputs.message).toContain("Active admin user") + expect(results.steps[1].outputs.message).toContain("Active admin user") }) it("should handle multiple conditions with OR operator", async () => { @@ -162,7 +164,7 @@ describe("Automation Scenarios", () => { }) .run() - expect(results.steps[0].outputs.message).toContain("Special user") + expect(results.steps[1].outputs.message).toContain("Special user") }) }) @@ -362,6 +364,32 @@ describe("Automation Scenarios", () => { } ) }) + + it("should run an automation where a loop is used twice to ensure context correctness further down the tree", async () => { + const builder = createAutomationBuilder({ + name: "Test Trigger with Loop and Create Row", + }) + + const results = await builder + .appAction({ fields: {} }) + .loop({ + option: LoopStepType.ARRAY, + binding: [1, 2, 3], + }) + .serverLog({ text: "Message {{loop.currentItem}}" }) + .serverLog({ text: "{{steps.1.iterations}}" }) + .loop({ + option: LoopStepType.ARRAY, + binding: [1, 2, 3], + }) + .serverLog({ text: "{{loop.currentItem}}" }) + .serverLog({ text: "{{steps.3.iterations}}" }) + .run() + + // We want to ensure that bindings are corr + expect(results.steps[1].outputs.message).toContain("- 3") + expect(results.steps[3].outputs.message).toContain("- 3") + }) }) describe("Row Automations", () => { diff --git a/packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts b/packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts index 16cab73b75..f477efabe4 100644 --- a/packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts +++ b/packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts @@ -179,7 +179,7 @@ class AutomationBuilder extends BaseStepBuilder { private triggerOutputs: any private triggerSet: boolean = false - constructor(options: { name?: string } = {}) { + constructor(options: { name?: string; appId?: string } = {}) { super() this.automationConfig = { name: options.name || `Test Automation ${uuidv4()}`, @@ -188,7 +188,7 @@ class AutomationBuilder extends BaseStepBuilder { trigger: {} as AutomationTrigger, }, type: "automation", - appId: setup.getConfig().getAppId(), + appId: options.appId ?? setup.getConfig().getAppId(), } this.config = setup.getConfig() } @@ -261,13 +261,14 @@ class AutomationBuilder extends BaseStepBuilder { return this } - branch(branchConfig: BranchConfig): { - run: () => Promise - } { + branch(branchConfig: BranchConfig): this { this.addBranchStep(branchConfig) - return { - run: () => this.run(), - } + return this + } + + build(): Automation { + this.automationConfig.definition.steps = this.steps + return this.automationConfig } async run() { @@ -275,7 +276,7 @@ class AutomationBuilder extends BaseStepBuilder { throw new Error("Please add a trigger to this automation test") } this.automationConfig.definition.steps = this.steps - const automation = await this.config.createAutomation(this.automationConfig) + const automation = await this.config.createAutomation(this.build()) const results = await testAutomation( this.config, automation, @@ -295,6 +296,9 @@ class AutomationBuilder extends BaseStepBuilder { } } -export function createAutomationBuilder(options?: { name?: string }) { +export function createAutomationBuilder(options?: { + name?: string + appId?: string +}) { return new AutomationBuilder(options) } 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: {}, }, } diff --git a/packages/server/src/tests/utilities/api/automation.ts b/packages/server/src/tests/utilities/api/automation.ts index 9620e2011c..61bd915647 100644 --- a/packages/server/src/tests/utilities/api/automation.ts +++ b/packages/server/src/tests/utilities/api/automation.ts @@ -14,4 +14,14 @@ export class AutomationAPI extends TestAPI { ) return result } + post = async ( + body: Automation, + expectations?: Expectations + ): Promise => { + const result = await this._post(`/api/automations`, { + body, + expectations, + }) + return result + } } diff --git a/packages/server/src/threads/automation.ts b/packages/server/src/threads/automation.ts index eff8407104..f374ff159a 100644 --- a/packages/server/src/threads/automation.ts +++ b/packages/server/src/threads/automation.ts @@ -449,7 +449,11 @@ class Orchestrator { outputs: tempOutput, inputs: steps[stepToLoopIndex].inputs, }) - this.context.steps[currentIndex + 1] = tempOutput + this.context.steps[this.context.steps.length] = tempOutput + this.context.steps = this.context.steps.filter( + item => !item.hasOwnProperty.call(item, "currentItem") + ) + this.loopStepOutputs = [] } @@ -461,6 +465,19 @@ class Orchestrator { for (const branch of branches) { const condition = await this.evaluateBranchCondition(branch.condition) if (condition) { + let branchStatus = { + status: `${branch.name} branch taken`, + success: true, + } + + this.updateExecutionOutput( + branchStep.id, + branchStep.stepId, + branchStep.inputs, + branchStatus + ) + this.context.steps[this.context.steps.length] = branchStatus + const branchSteps = children?.[branch.name] || [] await this.executeSteps(branchSteps) break @@ -569,8 +586,8 @@ class Orchestrator { this.loopStepOutputs.push(outputs) } else { this.updateExecutionOutput(step.id, step.stepId, step.inputs, outputs) + this.context.steps[this.context.steps.length] = outputs } - this.context.steps[this.context.steps.length] = outputs } }