From 4952747ae955b333a5c89b5c6ab413a44134627d Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 12 Sep 2024 15:40:37 +0100 Subject: [PATCH] Taking working from new-rbac-ui branch and separating it into its own PR, so that other work can be based on this from master. --- packages/backend-core/src/security/roles.ts | 51 ++++++++++------- .../server/src/api/controllers/permission.ts | 15 ----- packages/server/src/api/controllers/role.ts | 11 +++- .../src/api/routes/tests/permissions.spec.ts | 51 ----------------- .../server/src/sdk/app/permissions/index.ts | 57 ++----------------- .../app/permissions/tests/permissions.spec.ts | 53 ----------------- packages/shared-core/src/constants/colors.ts | 8 +++ packages/shared-core/src/constants/index.ts | 1 + packages/types/src/api/web/app/permission.ts | 1 - packages/types/src/api/web/role.ts | 3 +- packages/types/src/documents/app/role.ts | 7 +++ 11 files changed, 61 insertions(+), 197 deletions(-) delete mode 100644 packages/server/src/sdk/app/permissions/tests/permissions.spec.ts create mode 100644 packages/shared-core/src/constants/colors.ts diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 50850219a3..a7210ec2b8 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -7,8 +7,9 @@ import { doWithDB, } from "../db" import { getAppDB } from "../context" -import { Screen, Role as RoleDoc } from "@budibase/types" +import { Screen, Role as RoleDoc, RoleUIMetadata } from "@budibase/types" import cloneDeep from "lodash/fp/cloneDeep" +import { RoleColor } from "@budibase/shared-core" export const BUILTIN_ROLE_IDS = { ADMIN: "ADMIN", @@ -45,10 +46,12 @@ export class Role implements RoleDoc { inherits?: string version?: string permissions: Record = {} + uiMetadata?: RoleUIMetadata - constructor(id: string, name: string, permissionId: string) { + constructor(id: string, permissionId: string, uiMetadata?: RoleUIMetadata) { this._id = id - this.name = name + this.name = uiMetadata?.displayName || id + this.uiMetadata = uiMetadata this.permissionId = permissionId // version for managing the ID - removing the role_ when responding this.version = RoleIDVersion.NAME @@ -61,23 +64,31 @@ export class Role implements RoleDoc { } const BUILTIN_ROLES = { - ADMIN: new Role( - BUILTIN_IDS.ADMIN, - "Admin", - BuiltinPermissionID.ADMIN - ).addInheritance(BUILTIN_IDS.POWER), - POWER: new Role( - BUILTIN_IDS.POWER, - "Power", - BuiltinPermissionID.POWER - ).addInheritance(BUILTIN_IDS.BASIC), - BASIC: new Role( - BUILTIN_IDS.BASIC, - "Basic", - BuiltinPermissionID.WRITE - ).addInheritance(BUILTIN_IDS.PUBLIC), - PUBLIC: new Role(BUILTIN_IDS.PUBLIC, "Public", BuiltinPermissionID.PUBLIC), - BUILDER: new Role(BUILTIN_IDS.BUILDER, "Builder", BuiltinPermissionID.ADMIN), + ADMIN: new Role(BUILTIN_IDS.ADMIN, BuiltinPermissionID.ADMIN, { + displayName: "App admin", + description: "Can do everything", + color: RoleColor.ADMIN, + }).addInheritance(BUILTIN_IDS.POWER), + POWER: new Role(BUILTIN_IDS.POWER, BuiltinPermissionID.POWER, { + displayName: "App power user", + description: "An app user with more access", + color: RoleColor.POWER, + }).addInheritance(BUILTIN_IDS.BASIC), + BASIC: new Role(BUILTIN_IDS.BASIC, BuiltinPermissionID.WRITE, { + displayName: "App user", + description: "Any logged in user", + color: RoleColor.BASIC, + }).addInheritance(BUILTIN_IDS.PUBLIC), + PUBLIC: new Role(BUILTIN_IDS.PUBLIC, BuiltinPermissionID.PUBLIC, { + displayName: "Public user", + description: "Accessible to anyone", + color: RoleColor.PUBLIC, + }), + BUILDER: new Role(BUILTIN_IDS.BUILDER, BuiltinPermissionID.ADMIN, { + displayName: "Builder user", + description: "Users that can edit this app", + color: RoleColor.BUILDER, + }), } export function getBuiltinRoles(): { [key: string]: RoleDoc } { diff --git a/packages/server/src/api/controllers/permission.ts b/packages/server/src/api/controllers/permission.ts index 9444dfa251..0629ebc967 100644 --- a/packages/server/src/api/controllers/permission.ts +++ b/packages/server/src/api/controllers/permission.ts @@ -45,18 +45,6 @@ async function updatePermissionOnRole( }: { roleId: string; resourceId: string; level: PermissionLevel }, updateType: PermissionUpdateType ) { - const allowedAction = await sdk.permissions.resourceActionAllowed({ - resourceId, - level, - }) - - if (!allowedAction.allowed) { - throw new HTTPError( - `You are not allowed to '${allowedAction.level}' the resource type '${allowedAction.resourceType}'`, - 403 - ) - } - const db = context.getAppDB() const remove = updateType === PermissionUpdateType.REMOVE const isABuiltin = roles.isBuiltin(roleId) @@ -184,9 +172,6 @@ export async function getResourcePerms( }, {} as Record ), - requiresPlanToModify: ( - await sdk.permissions.allowsExplicitPermissions(resourceId) - ).minPlan, } } diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 605b462842..ee1c223952 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -19,7 +19,7 @@ import { UserMetadata, DocumentType, } from "@budibase/types" -import { sdk as sharedSdk } from "@budibase/shared-core" +import { RoleColor, sdk as sharedSdk } from "@budibase/shared-core" import sdk from "../../sdk" const UpdateRolesOptions = { @@ -62,7 +62,8 @@ export async function find(ctx: UserCtx) { export async function save(ctx: UserCtx) { const db = context.getAppDB() - let { _id, name, inherits, permissionId, version } = ctx.request.body + let { _id, name, inherits, permissionId, version, uiMetadata } = + ctx.request.body let isCreate = false const isNewVersion = version === roles.RoleIDVersion.NAME @@ -88,7 +89,11 @@ export async function save(ctx: UserCtx) { ctx.throw(400, "Cannot change custom role name") } - const role = new roles.Role(_id, name, permissionId).addInheritance(inherits) + const role = new roles.Role(_id, permissionId, { + displayName: uiMetadata?.displayName || name, + description: uiMetadata?.description || "Custom role", + color: uiMetadata?.color || RoleColor.DEFAULT_CUSTOM, + }).addInheritance(inherits) if (dbRole?.permissions && !role.permissions) { role.permissions = dbRole.permissions } diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 838e1aca0b..43df63cd33 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -1,8 +1,4 @@ const mockedSdk = sdk.permissions as jest.Mocked -jest.mock("../../../sdk/app/permissions", () => ({ - ...jest.requireActual("../../../sdk/app/permissions"), - resourceActionAllowed: jest.fn(), -})) import sdk from "../../../sdk" @@ -40,7 +36,6 @@ describe("/permission", () => { beforeEach(async () => { mocks.licenses.useCloudFree() - mockedSdk.resourceActionAllowed.mockResolvedValue({ allowed: true }) table = (await config.createTable()) as typeof table row = await config.createRow() @@ -112,29 +107,6 @@ describe("/permission", () => { expect(allRes.body[table._id]["read"]).toEqual(STD_ROLE_ID) expect(allRes.body[table._id]["write"]).toEqual(HIGHER_ROLE_ID) }) - - it("throw forbidden if the action is not allowed for the resource", async () => { - mockedSdk.resourceActionAllowed.mockResolvedValue({ - allowed: false, - resourceType: DocumentType.DATASOURCE, - level: PermissionLevel.READ, - }) - - await config.api.permission.add( - { - roleId: STD_ROLE_ID, - resourceId: table._id, - level: PermissionLevel.EXECUTE, - }, - { - status: 403, - body: { - message: - "You are not allowed to 'read' the resource type 'datasource'", - }, - } - ) - }) }) describe("remove", () => { @@ -148,29 +120,6 @@ describe("/permission", () => { const permsRes = await config.api.permission.get(table._id) expect(permsRes.permissions[STD_ROLE_ID]).toBeUndefined() }) - - it("throw forbidden if the action is not allowed for the resource", async () => { - mockedSdk.resourceActionAllowed.mockResolvedValue({ - allowed: false, - resourceType: DocumentType.DATASOURCE, - level: PermissionLevel.READ, - }) - - await config.api.permission.revoke( - { - roleId: STD_ROLE_ID, - resourceId: table._id, - level: PermissionLevel.EXECUTE, - }, - { - status: 403, - body: { - message: - "You are not allowed to 'read' the resource type 'datasource'", - }, - } - ) - }) }) describe("check public user allowed", () => { diff --git a/packages/server/src/sdk/app/permissions/index.ts b/packages/server/src/sdk/app/permissions/index.ts index dd4085d69e..a6e81652ee 100644 --- a/packages/server/src/sdk/app/permissions/index.ts +++ b/packages/server/src/sdk/app/permissions/index.ts @@ -1,10 +1,7 @@ import { db, roles } from "@budibase/backend-core" -import { features } from "@budibase/pro" import { - DocumentType, PermissionLevel, PermissionSource, - PlanType, VirtualDocumentType, } from "@budibase/types" import { extractViewInfoFromID, isViewID } from "../../../db/utils" @@ -15,36 +12,6 @@ import { import sdk from "../../../sdk" import { isV2 } from "../views" -type ResourceActionAllowedResult = - | { allowed: true } - | { - allowed: false - level: PermissionLevel - resourceType: DocumentType | VirtualDocumentType - } - -export async function resourceActionAllowed({ - resourceId, - level, -}: { - resourceId: string - level: PermissionLevel -}): Promise { - if (!isViewID(resourceId)) { - return { allowed: true } - } - - if (await features.isViewPermissionEnabled()) { - return { allowed: true } - } - - return { - allowed: false, - level, - resourceType: VirtualDocumentType.VIEW, - } -} - type ResourcePermissions = Record< string, { role: string; type: PermissionSource } @@ -58,20 +25,6 @@ export async function getInheritablePermissions( } } -export async function allowsExplicitPermissions(resourceId: string) { - if (isViewID(resourceId)) { - const allowed = await features.isViewPermissionEnabled() - const minPlan = !allowed ? PlanType.PREMIUM_PLUS : undefined - - return { - allowed, - minPlan, - } - } - - return { allowed: true } -} - export async function getResourcePerms( resourceId: string ): Promise { @@ -81,15 +34,13 @@ export async function getResourcePerms( const permsToInherit = await getInheritablePermissions(resourceId) - const allowsExplicitPerm = (await allowsExplicitPermissions(resourceId)) - .allowed - for (let level of CURRENTLY_SUPPORTED_LEVELS) { // update the various roleIds in the resource permissions for (let role of rolesList) { - const rolePerms = allowsExplicitPerm - ? roles.checkForRoleResourceArray(role.permissions || {}, resourceId) - : {} + const rolePerms = roles.checkForRoleResourceArray( + role.permissions || {}, + resourceId + ) if (rolePerms[resourceId]?.indexOf(level as PermissionLevel) > -1) { permissions[level] = { role: roles.getExternalRoleID(role._id!, role.version), diff --git a/packages/server/src/sdk/app/permissions/tests/permissions.spec.ts b/packages/server/src/sdk/app/permissions/tests/permissions.spec.ts deleted file mode 100644 index 4c233e68fa..0000000000 --- a/packages/server/src/sdk/app/permissions/tests/permissions.spec.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { PermissionLevel } from "@budibase/types" -import { mocks, structures } from "@budibase/backend-core/tests" -import { resourceActionAllowed } from ".." -import { generateViewID } from "../../../../db/utils" -import { initProMocks } from "../../../../tests/utilities/mocks/pro" - -initProMocks() - -describe("permissions sdk", () => { - beforeEach(() => { - mocks.licenses.useCloudFree() - }) - - describe("resourceActionAllowed", () => { - it("non view resources actions are always allowed", async () => { - const resourceId = structures.users.user()._id! - - const result = await resourceActionAllowed({ - resourceId, - level: PermissionLevel.READ, - }) - - expect(result).toEqual({ allowed: true }) - }) - - it("view resources actions allowed if the feature flag is enabled", async () => { - mocks.licenses.useViewPermissions() - const resourceId = generateViewID(structures.generator.guid()) - - const result = await resourceActionAllowed({ - resourceId, - level: PermissionLevel.READ, - }) - - expect(result).toEqual({ allowed: true }) - }) - - it("view resources actions allowed if the feature flag is disabled", async () => { - const resourceId = generateViewID(structures.generator.guid()) - - const result = await resourceActionAllowed({ - resourceId, - level: PermissionLevel.READ, - }) - - expect(result).toEqual({ - allowed: false, - level: "read", - resourceType: "view", - }) - }) - }) -}) diff --git a/packages/shared-core/src/constants/colors.ts b/packages/shared-core/src/constants/colors.ts new file mode 100644 index 0000000000..0abff46c3a --- /dev/null +++ b/packages/shared-core/src/constants/colors.ts @@ -0,0 +1,8 @@ +export enum RoleColor { + ADMIN = "var(--spectrum-global-color-static-red-400)", + POWER = "var(--spectrum-global-color-static-orange-400)", + BASIC = "var(--spectrum-global-color-static-green-400)", + PUBLIC = "var(--spectrum-global-color-static-blue-400)", + BUILDER = "var(--spectrum-global-color-static-magenta-600)", + DEFAULT_CUSTOM = "var(--spectrum-global-color-static-magenta-400)", +} diff --git a/packages/shared-core/src/constants/index.ts b/packages/shared-core/src/constants/index.ts index 78984aafa4..5a42fc5677 100644 --- a/packages/shared-core/src/constants/index.ts +++ b/packages/shared-core/src/constants/index.ts @@ -1,6 +1,7 @@ export * from "./api" export * from "./fields" export * from "./rows" +export * from "./colors" export const OperatorOptions = { Equals: { diff --git a/packages/types/src/api/web/app/permission.ts b/packages/types/src/api/web/app/permission.ts index 88ff4e9d2f..719be4f78e 100644 --- a/packages/types/src/api/web/app/permission.ts +++ b/packages/types/src/api/web/app/permission.ts @@ -8,7 +8,6 @@ export interface ResourcePermissionInfo { export interface GetResourcePermsResponse { permissions: Record - requiresPlanToModify?: PlanType } export interface GetDependantResourcesResponse { diff --git a/packages/types/src/api/web/role.ts b/packages/types/src/api/web/role.ts index c37dee60e0..644222b8f9 100644 --- a/packages/types/src/api/web/role.ts +++ b/packages/types/src/api/web/role.ts @@ -1,4 +1,4 @@ -import { Role } from "../../documents" +import { Role, RoleUIMetadata } from "../../documents" export interface SaveRoleRequest { _id?: string @@ -7,6 +7,7 @@ export interface SaveRoleRequest { inherits: string permissionId: string version: string + uiMetadata?: RoleUIMetadata } export interface SaveRoleResponse extends Role {} diff --git a/packages/types/src/documents/app/role.ts b/packages/types/src/documents/app/role.ts index 7ccfb2e7e9..6557b7e19d 100644 --- a/packages/types/src/documents/app/role.ts +++ b/packages/types/src/documents/app/role.ts @@ -1,10 +1,17 @@ import { Document } from "../document" import { PermissionLevel } from "../../sdk" +export interface RoleUIMetadata { + displayName?: string + color?: string + description?: string +} + export interface Role extends Document { permissionId: string inherits?: string permissions: Record version?: string name: string + uiMetadata?: RoleUIMetadata }