diff --git a/packages/backend-core/src/security/permissions.ts b/packages/backend-core/src/security/permissions.ts index 4ed2cd39541..929ae929097 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 7d9a11db483..df7f41e6cea 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 39348646fb8..f98833c7cd8 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 10477119837..7f5ac6b92f8 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -18,7 +18,7 @@ import { UserCtx, UserMetadata, DocumentType, - PermissionLevel, + BuiltinPermissionID, } from "@budibase/types" import { RoleColor, sdk as sharedSdk, helpers } from "@budibase/shared-core" import sdk from "../../sdk" @@ -134,7 +134,13 @@ 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) { + ctx.throw(400, "Role requires permissionId to be specified.") } const role = new roles.Role(_id, name, permissionId, { diff --git a/packages/server/src/api/routes/tests/application.spec.ts b/packages/server/src/api/routes/tests/application.spec.ts index 47b6776610a..729f8993799 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 7a9bb2f373c..193d018c248 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 adb83ca7932..ee5ba7840d8 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" @@ -58,6 +52,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", () => { @@ -149,7 +156,7 @@ describe("/roles", () => { _id: id1, name: id1, permissions: {}, - permissionId: "write", + permissionId: BuiltinPermissionID.WRITE, version: "name", inherits: ["POWER"], }) @@ -157,7 +164,7 @@ describe("/roles", () => { _id: id2, permissions: {}, name: id2, - permissionId: "write", + permissionId: BuiltinPermissionID.WRITE, version: "name", inherits: [id1], }) @@ -176,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(BuiltinPermissionID.READ_ONLY) + }) }) describe("fetch", () => { @@ -210,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 () => { @@ -316,7 +336,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( @@ -356,19 +376,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 894710ca272..82a550f2fdc 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 02a1a2d0600..862d8c30c5c 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,8 +215,8 @@ export function roleValidator() { }).optional(), // this is the base permission ID (for now a built in) permissionId: Joi.string() - .valid(...Object.values(permissions.BuiltinPermissionID)) - .required(), + .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 b63d6d1a78b..49046d9eda0 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 051a196a5bc..003b37f8f24 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 df439e84e7b..f1c573c8494 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 22f4ab9cd38..b624a5c6175 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 638de97f483..bb36af4170c 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", @@ -5,6 +7,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 28977d85215..4e7acb2741d 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 } )