Merge pull request #14570 from Budibase/feature/role-metadata-and-view-control

Role metadata and view controls

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<string, PermissionLevel[]> = {}
+  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(
+  ADMIN: new Role(BUILTIN_IDS.ADMIN, BuiltinPermissionID.ADMIN, {
-    BUILTIN_IDS.ADMIN,
+    displayName: "App admin",
-    "Admin",
+    description: "Can do everything",
-    BuiltinPermissionID.ADMIN
+    color: RoleColor.ADMIN,
-  ).addInheritance(BUILTIN_IDS.POWER),
+  }).addInheritance(BUILTIN_IDS.POWER),
-  POWER: new Role(
+  POWER: new Role(BUILTIN_IDS.POWER, BuiltinPermissionID.POWER, {
-    BUILTIN_IDS.POWER,
+    displayName: "App power user",
-    "Power",
+    description: "An app user with more access",
-    BuiltinPermissionID.POWER
+    color: RoleColor.POWER,
-  ).addInheritance(BUILTIN_IDS.BASIC),
+  }).addInheritance(BUILTIN_IDS.BASIC),
-  BASIC: new Role(
+  BASIC: new Role(BUILTIN_IDS.BASIC, BuiltinPermissionID.WRITE, {
-    BUILTIN_IDS.BASIC,
+    displayName: "App user",
-    "Basic",
+    description: "Any logged in user",
-    BuiltinPermissionID.WRITE
+    color: RoleColor.BASIC,
-  ).addInheritance(BUILTIN_IDS.PUBLIC),
+  }).addInheritance(BUILTIN_IDS.PUBLIC),
-  PUBLIC: new Role(BUILTIN_IDS.PUBLIC, "Public", BuiltinPermissionID.PUBLIC),
+  PUBLIC: new Role(BUILTIN_IDS.PUBLIC, BuiltinPermissionID.PUBLIC, {
-  BUILDER: new Role(BUILTIN_IDS.BUILDER, "Builder", BuiltinPermissionID.ADMIN),
+    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 } {

packages/backend-core/tests/core/utilities/mocks/licenses.ts

@@ -102,10 +102,6 @@ export const useAppBuilders = () => {
   return useFeature(Feature.APP_BUILDERS)
 }
 
-export const useViewPermissions = () => {
-  return useFeature(Feature.VIEW_PERMISSIONS)
-}
-
 export const useViewReadonlyColumns = () => {
   return useFeature(Feature.VIEW_READONLY_COLUMNS)
 }

packages/server/src/api/controllers/permission.ts

@@ -1,4 +1,4 @@
-import { permissions, roles, context, HTTPError } from "@budibase/backend-core"
+import { permissions, roles, context } from "@budibase/backend-core"
 import {
   UserCtx,
   Database,
@@ -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<string, ResourcePermissionInfo>
     ),
-    requiresPlanToModify: (
-      await sdk.permissions.allowsExplicitPermissions(resourceId)
-    ).minPlan,
   }
 }
 

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<void, FindRoleResponse>) {
 
 export async function save(ctx: UserCtx<SaveRoleRequest, SaveRoleResponse>) {
   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<SaveRoleRequest, SaveRoleResponse>) {
     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
   }

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

@@ -1,20 +1,5 @@
-const mockedSdk = sdk.permissions as jest.Mocked<typeof sdk.permissions>
-jest.mock("../../../sdk/app/permissions", () => ({
-  ...jest.requireActual("../../../sdk/app/permissions"),
-  resourceActionAllowed: jest.fn(),
-}))
-
-import sdk from "../../../sdk"
-
 import { roles } from "@budibase/backend-core"
-import {
+import { Document, PermissionLevel, Row, Table, ViewV2 } from "@budibase/types"
-  Document,
-  DocumentType,
-  PermissionLevel,
-  Row,
-  Table,
-  ViewV2,
-} from "@budibase/types"
 import * as setup from "./utilities"
 import { generator, mocks } from "@budibase/backend-core/tests"
 
@@ -40,7 +25,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 +96,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 +109,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", () => {
@@ -206,27 +144,7 @@ describe("/permission", () => {
       await config.api.viewV2.publicSearch(view.id, undefined, { status: 401 })
     })
 
-    it("should ignore the view permissions if the flag is not on", async () => {
+    it("should use the view permissions", async () => {
-      await config.api.permission.add({
-        roleId: STD_ROLE_ID,
-        resourceId: view.id,
-        level: PermissionLevel.READ,
-      })
-      await config.api.permission.revoke({
-        roleId: STD_ROLE_ID,
-        resourceId: table._id,
-        level: PermissionLevel.READ,
-      })
-      // replicate changes before checking permissions
-      await config.publish()
-
-      await config.api.viewV2.publicSearch(view.id, undefined, {
-        status: 401,
-      })
-    })
-
-    it("should use the view permissions if the flag is on", async () => {
-      mocks.licenses.useViewPermissions()
       await config.api.permission.add({
         roleId: STD_ROLE_ID,
         resourceId: view.id,

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

@@ -763,10 +763,6 @@ describe("/rowsActions", () => {
     })
 
     describe("role permission checks", () => {
-      beforeAll(() => {
-        mocks.licenses.useViewPermissions()
-      })
-
       afterAll(() => {
         mocks.licenses.useCloudFree()
       })

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

@@ -2297,7 +2297,6 @@ describe.each([
 
     describe("permissions", () => {
       beforeEach(async () => {
-        mocks.licenses.useViewPermissions()
         await Promise.all(
           Array.from({ length: 10 }, () => config.api.row.save(table._id!, {}))
         )

packages/server/src/api/routes/utils/validators.ts

@@ -208,6 +208,11 @@ export function roleValidator() {
       name: Joi.string()
         .regex(/^[a-zA-Z0-9_]*$/)
         .required(),
+      uiMetadata: Joi.object({
+        displayName: OPTIONAL_STRING,
+        color: OPTIONAL_STRING,
+        description: OPTIONAL_STRING,
+      }).optional(),
       // this is the base permission ID (for now a built in)
       permissionId: Joi.string()
         .valid(...Object.values(permissions.BuiltinPermissionID))

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<ResourceActionAllowedResult> {
-  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<ResourcePermissions> {
@@ -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
+      const rolePerms = roles.checkForRoleResourceArray(
-        ? roles.checkForRoleResourceArray(role.permissions || {}, resourceId)
+        role.permissions || {},
-        : {}
+        resourceId
+      )
       if (rolePerms[resourceId]?.indexOf(level as PermissionLevel) > -1) {
         permissions[level] = {
           role: roles.getExternalRoleID(role._id!, role.version),

packages/server/src/sdk/app/permissions/tests/permissions.spec.ts

@@ -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",
-      })
-    })
-  })
-})

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)",
+}

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: {

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

@@ -1,4 +1,4 @@
-import { PermissionLevel, PlanType } from "../../../sdk"
+import { PermissionLevel } from "../../../sdk"
 
 export interface ResourcePermissionInfo {
   role: string
@@ -8,7 +8,6 @@ export interface ResourcePermissionInfo {
 
 export interface GetResourcePermsResponse {
   permissions: Record<string, ResourcePermissionInfo>
-  requiresPlanToModify?: PlanType
 }
 
 export interface GetDependantResourcesResponse {

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 {}

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<string, PermissionLevel[]>
   version?: string
   name: string
+  uiMetadata?: RoleUIMetadata
 }

packages/types/src/sdk/licensing/feature.ts

@@ -13,6 +13,7 @@ export enum Feature {
   APP_BUILDERS = "appBuilders",
   OFFLINE = "offline",
   EXPANDED_PUBLIC_API = "expandedPublicApi",
+  // deprecated - no longer licensed
   VIEW_PERMISSIONS = "viewPermissions",
   VIEW_READONLY_COLUMNS = "viewReadonlyColumns",
   BUDIBASE_AI = "budibaseAI",

packages/worker/src/api/routes/global/tests/roles.spec.ts

@@ -35,8 +35,8 @@ describe("/api/global/roles", () => {
 
   const role = new roles.Role(
     db.generateRoleID(ROLE_NAME),
-    roles.BUILTIN_ROLE_IDS.BASIC,
+    permissions.BuiltinPermissionID.READ_ONLY,
-    permissions.BuiltinPermissionID.READ_ONLY
+    { displayName: roles.BUILTIN_ROLE_IDS.BASIC }
   )
 
   beforeAll(async () => {