From c6a413f3991d660436f0c2b7deb0191bd47df482 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 23 Oct 2024 14:47:06 +0100 Subject: [PATCH 1/9] Quick fix for role validation, permissionId is no longer required. --- packages/server/src/api/controllers/role.ts | 4 ++++ packages/server/src/api/routes/tests/role.spec.ts | 13 +++++++++++++ packages/server/src/api/routes/utils/validators.ts | 2 +- packages/types/src/api/web/role.ts | 2 +- 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 1047711983..35f221293f 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -137,6 +137,10 @@ export async function save(ctx: UserCtx) { permissionId = PermissionLevel.WRITE } + if (!permissionId) { + ctx.throw(400, "Role requires permissionId to be specified.") + } + const role = new roles.Role(_id, name, permissionId, { displayName: uiMetadata?.displayName || name, description: uiMetadata?.description || "Custom role", diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index adb83ca793..e3dfcb40f1 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -58,6 +58,19 @@ describe("/roles", () => { }) expect(res.inherits).toEqual([BUILTIN_ROLE_IDS.BASIC]) }) + + it("save role without permissionId", async () => { + const res = await config.api.roles.save( + { + ...basicRole(), + permissionId: undefined, + }, + { + status: 200, + } + ) + expect(res.permissionId).toEqual(PermissionLevel.WRITE) + }) }) describe("update", () => { diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 02a1a2d060..962735ded7 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -215,7 +215,7 @@ export function roleValidator() { // this is the base permission ID (for now a built in) permissionId: Joi.string() .valid(...Object.values(permissions.BuiltinPermissionID)) - .required(), + .optional(), permissions: Joi.object() .pattern( /.*/, diff --git a/packages/types/src/api/web/role.ts b/packages/types/src/api/web/role.ts index df439e84e7..aa5dd2f31b 100644 --- a/packages/types/src/api/web/role.ts +++ b/packages/types/src/api/web/role.ts @@ -6,7 +6,7 @@ export interface SaveRoleRequest { _rev?: string name: string inherits?: string | string[] - permissionId: string + permissionId?: string permissions?: Record version?: string uiMetadata?: RoleUIMetadata From 8f21802e6ea5a3cf0cea9b06f8276d9b59250277 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 23 Oct 2024 15:08:59 +0100 Subject: [PATCH 2/9] Refactor, correct to the BuiltinPermissionID rather than PermissionLevel, these are different. --- .../backend-core/src/security/permissions.ts | 14 +++---- packages/backend-core/src/security/roles.ts | 7 ++-- .../src/security/tests/permissions.spec.ts | 7 ++-- packages/server/src/api/controllers/role.ts | 5 ++- .../src/api/routes/tests/application.spec.ts | 6 +-- .../src/api/routes/tests/permissions.spec.ts | 15 +++++-- .../server/src/api/routes/tests/role.spec.ts | 41 +++++++++++-------- .../src/api/routes/tests/screen.spec.ts | 8 ++-- .../server/src/api/routes/utils/validators.ts | 3 +- .../server/src/tests/utilities/structures.ts | 5 ++- .../src/helpers/tests/roles.spec.ts | 4 +- packages/types/src/api/web/role.ts | 4 +- packages/types/src/documents/app/role.ts | 4 +- packages/types/src/sdk/permissions.ts | 9 ++++ .../src/api/routes/global/tests/roles.spec.ts | 6 +-- 15 files changed, 83 insertions(+), 55 deletions(-) diff --git a/packages/backend-core/src/security/permissions.ts b/packages/backend-core/src/security/permissions.ts index 4ed2cd3954..929ae92909 100644 --- a/packages/backend-core/src/security/permissions.ts +++ b/packages/backend-core/src/security/permissions.ts @@ -1,4 +1,8 @@ -import { PermissionLevel, PermissionType } from "@budibase/types" +import { + PermissionLevel, + PermissionType, + BuiltinPermissionID, +} from "@budibase/types" import flatten from "lodash/flatten" import cloneDeep from "lodash/fp/cloneDeep" @@ -57,14 +61,6 @@ export function getAllowedLevels(userPermLevel: PermissionLevel): string[] { } } -export enum BuiltinPermissionID { - PUBLIC = "public", - READ_ONLY = "read_only", - WRITE = "write", - ADMIN = "admin", - POWER = "power", -} - export const BUILTIN_PERMISSIONS: { [key in keyof typeof BuiltinPermissionID]: { _id: (typeof BuiltinPermissionID)[key] diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 7d9a11db48..df7f41e6ce 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -1,5 +1,4 @@ import semver from "semver" -import { BuiltinPermissionID, PermissionLevel } from "./permissions" import { prefixRoleID, getRoleParams, @@ -14,6 +13,8 @@ import { RoleUIMetadata, Database, App, + BuiltinPermissionID, + PermissionLevel, } from "@budibase/types" import cloneDeep from "lodash/fp/cloneDeep" import { RoleColor, helpers } from "@budibase/shared-core" @@ -50,7 +51,7 @@ export class Role implements RoleDoc { _id: string _rev?: string name: string - permissionId: string + permissionId: BuiltinPermissionID inherits?: string | string[] version?: string permissions: Record = {} @@ -59,7 +60,7 @@ export class Role implements RoleDoc { constructor( id: string, name: string, - permissionId: string, + permissionId: BuiltinPermissionID, uiMetadata?: RoleUIMetadata ) { this._id = id diff --git a/packages/backend-core/src/security/tests/permissions.spec.ts b/packages/backend-core/src/security/tests/permissions.spec.ts index 39348646fb..f98833c7cd 100644 --- a/packages/backend-core/src/security/tests/permissions.spec.ts +++ b/packages/backend-core/src/security/tests/permissions.spec.ts @@ -1,6 +1,7 @@ import cloneDeep from "lodash/cloneDeep" import * as permissions from "../permissions" import { BUILTIN_ROLE_IDS } from "../roles" +import { BuiltinPermissionID } from "@budibase/types" describe("levelToNumber", () => { it("should return 0 for EXECUTE", () => { @@ -77,7 +78,7 @@ describe("doesHaveBasePermission", () => { const rolesHierarchy = [ { roleId: BUILTIN_ROLE_IDS.ADMIN, - permissionId: permissions.BuiltinPermissionID.ADMIN, + permissionId: BuiltinPermissionID.ADMIN, }, ] expect( @@ -91,7 +92,7 @@ describe("doesHaveBasePermission", () => { const rolesHierarchy = [ { roleId: BUILTIN_ROLE_IDS.PUBLIC, - permissionId: permissions.BuiltinPermissionID.PUBLIC, + permissionId: BuiltinPermissionID.PUBLIC, }, ] expect( @@ -129,7 +130,7 @@ describe("getBuiltinPermissions", () => { describe("getBuiltinPermissionByID", () => { it("returns correct permission object for valid ID", () => { const expectedPermission = { - _id: permissions.BuiltinPermissionID.PUBLIC, + _id: BuiltinPermissionID.PUBLIC, name: "Public", permissions: [ new permissions.Permission( diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 35f221293f..f8af71fe5c 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -19,6 +19,7 @@ import { UserMetadata, DocumentType, PermissionLevel, + BuiltinPermissionID, } from "@budibase/types" import { RoleColor, sdk as sharedSdk, helpers } from "@budibase/shared-core" import sdk from "../../sdk" @@ -134,7 +135,9 @@ export async function save(ctx: UserCtx) { } // assume write permission level for newly created roles if (isCreate && !permissionId) { - permissionId = PermissionLevel.WRITE + permissionId = BuiltinPermissionID.WRITE + } else if (!permissionId && dbRole?.permissionId) { + permissionId = dbRole.permissionId } if (!permissionId) { diff --git a/packages/server/src/api/routes/tests/application.spec.ts b/packages/server/src/api/routes/tests/application.spec.ts index 47b6776610..729f899379 100644 --- a/packages/server/src/api/routes/tests/application.spec.ts +++ b/packages/server/src/api/routes/tests/application.spec.ts @@ -16,7 +16,7 @@ import * as setup from "./utilities" import { AppStatus } from "../../../db/utils" import { events, utils, context, features } from "@budibase/backend-core" import env from "../../../environment" -import { type App } from "@budibase/types" +import { type App, BuiltinPermissionID } from "@budibase/types" import tk from "timekeeper" import * as uuid from "uuid" import { structures } from "@budibase/backend-core/tests" @@ -80,7 +80,7 @@ describe("/applications", () => { const role = await config.api.roles.save({ name: "Test", inherits: "PUBLIC", - permissionId: "read_only", + permissionId: BuiltinPermissionID.READ_ONLY, version: "name", }) @@ -112,7 +112,7 @@ describe("/applications", () => { const role = await config.api.roles.save({ name: roleName, inherits: "PUBLIC", - permissionId: "read_only", + permissionId: BuiltinPermissionID.READ_ONLY, version: "name", }) diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 7a9bb2f373..193d018c24 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -1,5 +1,12 @@ import { roles } from "@budibase/backend-core" -import { Document, PermissionLevel, Role, Row, Table } from "@budibase/types" +import { + BuiltinPermissionID, + Document, + PermissionLevel, + Role, + Row, + Table, +} from "@budibase/types" import * as setup from "./utilities" import { generator, mocks } from "@budibase/backend-core/tests" @@ -304,7 +311,7 @@ describe("/permission", () => { role1 = await config.api.roles.save( { name: "test_1", - permissionId: PermissionLevel.WRITE, + permissionId: BuiltinPermissionID.WRITE, inherits: BUILTIN_ROLE_IDS.BASIC, }, { status: 200 } @@ -312,7 +319,7 @@ describe("/permission", () => { role2 = await config.api.roles.save( { name: "test_2", - permissionId: PermissionLevel.WRITE, + permissionId: BuiltinPermissionID.WRITE, inherits: BUILTIN_ROLE_IDS.BASIC, }, { status: 200 } @@ -345,7 +352,7 @@ describe("/permission", () => { it("should be able to fetch two tables, with different roles, using multi-inheritance", async () => { const role3 = await config.api.roles.save({ name: "role3", - permissionId: PermissionLevel.WRITE, + permissionId: BuiltinPermissionID.WRITE, inherits: [role1._id!, role2._id!], }) diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index e3dfcb40f1..1eb59c5a0c 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -1,15 +1,9 @@ -import { - roles, - events, - permissions, - db as dbCore, -} from "@budibase/backend-core" +import { roles, events, db as dbCore } from "@budibase/backend-core" import * as setup from "./utilities" -import { PermissionLevel } from "@budibase/types" +import { PermissionLevel, BuiltinPermissionID } from "@budibase/types" const { basicRole } = setup.structures const { BUILTIN_ROLE_IDS } = roles -const { BuiltinPermissionID } = permissions const LOOP_ERROR = "Role inheritance contains a loop, this is not supported" @@ -162,7 +156,7 @@ describe("/roles", () => { _id: id1, name: id1, permissions: {}, - permissionId: "write", + permissionId: BuiltinPermissionID.WRITE, version: "name", inherits: ["POWER"], }) @@ -170,7 +164,7 @@ describe("/roles", () => { _id: id2, permissions: {}, name: id2, - permissionId: "write", + permissionId: BuiltinPermissionID.WRITE, version: "name", inherits: [id1], }) @@ -189,10 +183,25 @@ describe("/roles", () => { inherits: [BUILTIN_ROLE_IDS.ADMIN], }) // remove the roles so that it will default back to DB roles, then save again - delete res.inherits - const updatedRes = await config.api.roles.save(res) + const updatedRes = await config.api.roles.save({ + ...res, + inherits: undefined, + }) expect(updatedRes.inherits).toEqual([BUILTIN_ROLE_IDS.ADMIN]) }) + + it("handle updating a role, without its permissionId", async () => { + const res = await config.api.roles.save({ + ...basicRole(), + permissionId: BuiltinPermissionID.READ_ONLY, + }) + // permission ID can be removed during update + const updatedRes = await config.api.roles.save({ + ...res, + permissionId: undefined, + }) + expect(updatedRes.permissionId).toEqual(PermissionLevel.READ) + }) }) describe("fetch", () => { @@ -329,7 +338,7 @@ describe("/roles", () => { await config.api.roles.save({ name: customRoleName, inherits: roles.BUILTIN_ROLE_IDS.BASIC, - permissionId: permissions.BuiltinPermissionID.READ_ONLY, + permissionId: BuiltinPermissionID.READ_ONLY, version: "name", }) await config.withHeaders( @@ -369,19 +378,19 @@ describe("/roles", () => { const { _id: roleId1 } = await config.api.roles.save({ name: role1, inherits: roles.BUILTIN_ROLE_IDS.BASIC, - permissionId: permissions.BuiltinPermissionID.WRITE, + permissionId: BuiltinPermissionID.WRITE, version: "name", }) const { _id: roleId2 } = await config.api.roles.save({ name: role2, inherits: roles.BUILTIN_ROLE_IDS.POWER, - permissionId: permissions.BuiltinPermissionID.POWER, + permissionId: BuiltinPermissionID.POWER, version: "name", }) await config.api.roles.save({ name: role3, inherits: [roleId1!, roleId2!], - permissionId: permissions.BuiltinPermissionID.READ_ONLY, + permissionId: BuiltinPermissionID.READ_ONLY, version: "name", }) const headers = await config.roleHeaders({ diff --git a/packages/server/src/api/routes/tests/screen.spec.ts b/packages/server/src/api/routes/tests/screen.spec.ts index 894710ca27..82a550f2fd 100644 --- a/packages/server/src/api/routes/tests/screen.spec.ts +++ b/packages/server/src/api/routes/tests/screen.spec.ts @@ -1,7 +1,7 @@ import { checkBuilderEndpoint } from "./utilities/TestFunctions" import * as setup from "./utilities" import { events, roles } from "@budibase/backend-core" -import { Screen, PermissionLevel, Role } from "@budibase/types" +import { Screen, Role, BuiltinPermissionID } from "@budibase/types" const { basicScreen } = setup.structures @@ -40,17 +40,17 @@ describe("/screens", () => { role1 = await config.api.roles.save({ name: "role1", inherits: roles.BUILTIN_ROLE_IDS.BASIC, - permissionId: PermissionLevel.WRITE, + permissionId: BuiltinPermissionID.WRITE, }) role2 = await config.api.roles.save({ name: "role2", inherits: roles.BUILTIN_ROLE_IDS.BASIC, - permissionId: PermissionLevel.WRITE, + permissionId: BuiltinPermissionID.WRITE, }) multiRole = await config.api.roles.save({ name: "multiRole", inherits: [role1._id!, role2._id!], - permissionId: PermissionLevel.WRITE, + permissionId: BuiltinPermissionID.WRITE, }) screen1 = await config.api.screen.save( { diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 962735ded7..862d8c30c5 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -8,6 +8,7 @@ import { SearchFilters, Table, WebhookActionType, + BuiltinPermissionID, } from "@budibase/types" import Joi, { CustomValidator } from "joi" import { ValidSnippetNameRegex, helpers } from "@budibase/shared-core" @@ -214,7 +215,7 @@ export function roleValidator() { }).optional(), // this is the base permission ID (for now a built in) permissionId: Joi.string() - .valid(...Object.values(permissions.BuiltinPermissionID)) + .valid(...Object.values(BuiltinPermissionID)) .optional(), permissions: Joi.object() .pattern( diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts index b63d6d1a78..49046d9eda 100644 --- a/packages/server/src/tests/utilities/structures.ts +++ b/packages/server/src/tests/utilities/structures.ts @@ -1,4 +1,4 @@ -import { permissions, roles, utils } from "@budibase/backend-core" +import { roles, utils } from "@budibase/backend-core" import { createHomeScreen } from "../../constants/screens" import { EMPTY_LAYOUT } from "../../constants/layouts" import { cloneDeep } from "lodash/fp" @@ -33,6 +33,7 @@ import { TableSourceType, Webhook, WebhookActionType, + BuiltinPermissionID, } from "@budibase/types" import { LoopInput } from "../../definitions/automations" import { merge } from "lodash" @@ -515,7 +516,7 @@ export function basicRole(): Role { return { name: `NewRole_${utils.newid()}`, inherits: roles.BUILTIN_ROLE_IDS.BASIC, - permissionId: permissions.BuiltinPermissionID.READ_ONLY, + permissionId: BuiltinPermissionID.WRITE, permissions: {}, version: "name", } diff --git a/packages/shared-core/src/helpers/tests/roles.spec.ts b/packages/shared-core/src/helpers/tests/roles.spec.ts index 051a196a5b..003b37f8f2 100644 --- a/packages/shared-core/src/helpers/tests/roles.spec.ts +++ b/packages/shared-core/src/helpers/tests/roles.spec.ts @@ -1,5 +1,5 @@ import { checkForRoleInheritanceLoops } from "../roles" -import { Role } from "@budibase/types" +import { BuiltinPermissionID, Role } from "@budibase/types" /** * This unit test exists as this utility will be used in the frontend and backend, confirmation @@ -19,7 +19,7 @@ function role(id: string, inherits: string | string[]): TestRole { _id: id, inherits: inherits, name: "ROLE", - permissionId: "PERMISSION", + permissionId: BuiltinPermissionID.WRITE, permissions: {}, // not needed for this test } allRoles.push(role) diff --git a/packages/types/src/api/web/role.ts b/packages/types/src/api/web/role.ts index aa5dd2f31b..f1c573c849 100644 --- a/packages/types/src/api/web/role.ts +++ b/packages/types/src/api/web/role.ts @@ -1,12 +1,12 @@ import { Role, RoleUIMetadata } from "../../documents" -import { PermissionLevel } from "../../sdk" +import { PermissionLevel, BuiltinPermissionID } from "../../sdk" export interface SaveRoleRequest { _id?: string _rev?: string name: string inherits?: string | string[] - permissionId?: string + permissionId?: BuiltinPermissionID permissions?: Record version?: string uiMetadata?: RoleUIMetadata diff --git a/packages/types/src/documents/app/role.ts b/packages/types/src/documents/app/role.ts index 22f4ab9cd3..b624a5c617 100644 --- a/packages/types/src/documents/app/role.ts +++ b/packages/types/src/documents/app/role.ts @@ -1,5 +1,5 @@ import { Document } from "../document" -import { PermissionLevel } from "../../sdk" +import { PermissionLevel, BuiltinPermissionID } from "../../sdk" export interface RoleUIMetadata { displayName?: string @@ -8,7 +8,7 @@ export interface RoleUIMetadata { } export interface Role extends Document { - permissionId: string + permissionId: BuiltinPermissionID inherits?: string | string[] permissions: Record version?: string diff --git a/packages/types/src/sdk/permissions.ts b/packages/types/src/sdk/permissions.ts index 638de97f48..d27876156e 100644 --- a/packages/types/src/sdk/permissions.ts +++ b/packages/types/src/sdk/permissions.ts @@ -5,6 +5,15 @@ export enum PermissionLevel { ADMIN = "admin", } +// used within the role, specifies base permissions +export enum BuiltinPermissionID { + PUBLIC = "public", + READ_ONLY = "read_only", + WRITE = "write", + ADMIN = "admin", + POWER = "power", +} + // these are the global types, that govern the underlying default behaviour export enum PermissionType { APP = "app", diff --git a/packages/worker/src/api/routes/global/tests/roles.spec.ts b/packages/worker/src/api/routes/global/tests/roles.spec.ts index 28977d8521..4e7acb2741 100644 --- a/packages/worker/src/api/routes/global/tests/roles.spec.ts +++ b/packages/worker/src/api/routes/global/tests/roles.spec.ts @@ -1,6 +1,6 @@ import { structures, TestConfiguration } from "../../../../tests" -import { context, db, permissions, roles } from "@budibase/backend-core" -import { App, Database } from "@budibase/types" +import { context, db, roles } from "@budibase/backend-core" +import { App, Database, BuiltinPermissionID } from "@budibase/types" jest.mock("@budibase/backend-core", () => { const core = jest.requireActual("@budibase/backend-core") @@ -44,7 +44,7 @@ describe("/api/global/roles", () => { const role = new roles.Role( db.generateRoleID(ROLE_NAME), ROLE_NAME, - permissions.BuiltinPermissionID.READ_ONLY, + BuiltinPermissionID.READ_ONLY, { displayName: roles.BUILTIN_ROLE_IDS.BASIC } ) From ca0dc030ade5f1c941a66437e2f228bdda73d546 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 23 Oct 2024 15:08:59 +0100 Subject: [PATCH 3/9] Make sure processSearchFilters handles an undefined input. --- packages/shared-core/src/utils.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/shared-core/src/utils.ts b/packages/shared-core/src/utils.ts index 5db4ead1dc..8e22c10c15 100644 --- a/packages/shared-core/src/utils.ts +++ b/packages/shared-core/src/utils.ts @@ -138,9 +138,16 @@ export function isSupportedUserSearch(query: SearchFilters) { return true } -export const processSearchFilters = ( +export function processSearchFilters(filterArray: undefined): undefined +export function processSearchFilters( filterArray: LegacyFilter[] -): Required => { +): Required +export function processSearchFilters( + filterArray?: LegacyFilter[] +): Required | undefined { + if (!filterArray) { + return undefined + } const { allOr, onEmptyFilter, filters } = splitFiltersArray(filterArray) return { logicalOperator: UILogicalOperator.ALL, From 3ddd8e10cc4e026710b5f1d6ab2b1272d46f9d79 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 23 Oct 2024 15:10:53 +0100 Subject: [PATCH 4/9] Comment and test case. --- packages/server/src/api/routes/tests/role.spec.ts | 2 +- packages/types/src/sdk/permissions.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index 1eb59c5a0c..a4e672e961 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -200,7 +200,7 @@ describe("/roles", () => { ...res, permissionId: undefined, }) - expect(updatedRes.permissionId).toEqual(PermissionLevel.READ) + expect(updatedRes.permissionId).toEqual(BuiltinPermissionID.READ_ONLY) }) }) diff --git a/packages/types/src/sdk/permissions.ts b/packages/types/src/sdk/permissions.ts index d27876156e..bb36af4170 100644 --- a/packages/types/src/sdk/permissions.ts +++ b/packages/types/src/sdk/permissions.ts @@ -1,3 +1,5 @@ +// used in resource permissions - permissions can be at one of these levels +// endpoints will set what type of permission they require (e.g. searching requires READ) export enum PermissionLevel { READ = "read", WRITE = "write", From 9ebd890c04902dcb2d810c307e46cef004ef8980 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 23 Oct 2024 15:13:58 +0100 Subject: [PATCH 5/9] Handle empty arrays too. --- packages/shared-core/src/utils.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/shared-core/src/utils.ts b/packages/shared-core/src/utils.ts index 8e22c10c15..01afbc2539 100644 --- a/packages/shared-core/src/utils.ts +++ b/packages/shared-core/src/utils.ts @@ -135,17 +135,19 @@ export function isSupportedUserSearch(query: SearchFilters) { return false } } + return true } export function processSearchFilters(filterArray: undefined): undefined +export function processSearchFilters(filterArray: []): undefined export function processSearchFilters( filterArray: LegacyFilter[] ): Required export function processSearchFilters( filterArray?: LegacyFilter[] ): Required | undefined { - if (!filterArray) { + if (!filterArray || filterArray.length === 0) { return undefined } const { allOr, onEmptyFilter, filters } = splitFiltersArray(filterArray) From e28357c45120c01025da3fb2ab0b381f76ad4a1c Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 23 Oct 2024 15:21:37 +0100 Subject: [PATCH 6/9] Linting. --- packages/server/src/api/controllers/role.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index f8af71fe5c..7f5ac6b92f 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -18,7 +18,6 @@ import { UserCtx, UserMetadata, DocumentType, - PermissionLevel, BuiltinPermissionID, } from "@budibase/types" import { RoleColor, sdk as sharedSdk, helpers } from "@budibase/shared-core" From f2a844c3b5606e39257f64f86ce37e0e3ed20626 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 23 Oct 2024 15:24:32 +0100 Subject: [PATCH 7/9] Fix test. --- packages/shared-core/src/filters.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index e0703321e8..e6cab22911 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -467,7 +467,7 @@ export function buildQuery( } if (Array.isArray(filter)) { - filter = processSearchFilters(filter) + filter = processSearchFilters(filter) ?? [] } const operator = logicalOperatorFromUI( From 8ebb02eb7976b0a88a847d28509b32a782888473 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 23 Oct 2024 15:30:46 +0100 Subject: [PATCH 8/9] Test fix. --- packages/server/src/api/routes/tests/role.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index a4e672e961..ee5ba7840d 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -232,9 +232,7 @@ describe("/roles", () => { const customRoleFetched = res.find(r => r._id === customRole.name) expect(customRoleFetched).toBeDefined() expect(customRoleFetched!.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC) - expect(customRoleFetched!.permissionId).toEqual( - BuiltinPermissionID.READ_ONLY - ) + expect(customRoleFetched!.permissionId).toEqual(BuiltinPermissionID.WRITE) }) it("should be able to get the role with a permission added", async () => { From f7b84ca7eab2d9be82ff84b9e620fc8318b96aec Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 23 Oct 2024 15:32:07 +0100 Subject: [PATCH 9/9] Fix some type skullduggery. --- packages/shared-core/src/filters.ts | 13 ++++++------- packages/shared-core/src/utils.ts | 5 ----- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index e6cab22911..003e6b5082 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -455,19 +455,18 @@ export function splitFiltersArray(filters: LegacyFilter[]) { * Legacy support remains for the old **SearchFilter[]** format. * These will be migrated to an appropriate **SearchFilters** object, if encountered */ -export function buildQuery(filter: undefined): undefined -export function buildQuery( - filter: UISearchFilter | LegacyFilter[] -): SearchFilters export function buildQuery( filter?: UISearchFilter | LegacyFilter[] -): SearchFilters | undefined { +): SearchFilters { if (!filter) { - return + return {} } if (Array.isArray(filter)) { - filter = processSearchFilters(filter) ?? [] + filter = processSearchFilters(filter) + if (!filter) { + return {} + } } const operator = logicalOperatorFromUI( diff --git a/packages/shared-core/src/utils.ts b/packages/shared-core/src/utils.ts index 01afbc2539..0e49db9c7c 100644 --- a/packages/shared-core/src/utils.ts +++ b/packages/shared-core/src/utils.ts @@ -139,11 +139,6 @@ export function isSupportedUserSearch(query: SearchFilters) { return true } -export function processSearchFilters(filterArray: undefined): undefined -export function processSearchFilters(filterArray: []): undefined -export function processSearchFilters( - filterArray: LegacyFilter[] -): Required export function processSearchFilters( filterArray?: LegacyFilter[] ): Required | undefined {