Merge pull request #1118 from Budibase/bug/rbac-issue

RBAC issue - occasionally not allowing updating of resource permissions

packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte

@@ -30,7 +30,9 @@
 <div class="popover">
   <h5>Who Can Access This Data?</h5>
   <div class="note">
-    <Label extraSmall grey>Specify the minimum access level role for this data.</Label>
+    <Label extraSmall grey>
+      Specify the minimum access level role for this data.
+    </Label>
   </div>
   <Spacer large />
   <div class="row">

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

@@ -1,5 +1,5 @@
 const {
-  BUILTIN_PERMISSIONS,
+  getBuiltinPermissions,
   PermissionLevels,
   isPermissionLevelHigherThanRead,
   higherPermission,
@@ -8,11 +8,10 @@ const {
   isBuiltin,
   getDBRoleID,
   getExternalRoleID,
-  BUILTIN_ROLES,
+  getBuiltinRoles,
 } = require("../../utilities/security/roles")
 const { getRoleParams } = require("../../db/utils")
 const CouchDB = require("../../db")
-const { cloneDeep } = require("lodash/fp")
 const {
   CURRENTLY_SUPPORTED_LEVELS,
   getBasePermissions,
@@ -65,7 +64,7 @@ async function updatePermissionOnRole(
 
   // the permission is for a built in, make sure it exists
   if (isABuiltin && !dbRoles.some(role => role._id === dbRoleId)) {
-    const builtin = cloneDeep(BUILTIN_ROLES[roleId])
+    const builtin = getBuiltinRoles()[roleId]
     builtin._id = getDBRoleID(builtin._id)
     dbRoles.push(builtin)
   }
@@ -110,7 +109,7 @@ async function updatePermissionOnRole(
 }
 
 exports.fetchBuiltin = function(ctx) {
-  ctx.body = Object.values(BUILTIN_PERMISSIONS)
+  ctx.body = Object.values(getBuiltinPermissions())
 }
 
 exports.fetchLevels = function(ctx) {

packages/server/src/api/controllers/role.js

@@ -1,6 +1,6 @@
 const CouchDB = require("../../db")
 const {
-  BUILTIN_ROLES,
+  getBuiltinRoles,
   BUILTIN_ROLE_IDS,
   Role,
   getRole,
@@ -58,10 +58,11 @@ exports.fetch = async function(ctx) {
     })
   )
   let roles = body.rows.map(row => row.doc)
+  const builtinRoles = getBuiltinRoles()
 
   // need to combine builtin with any DB record of them (for sake of permissions)
   for (let builtinRoleId of EXTERNAL_BUILTIN_ROLE_IDS) {
-    const builtinRole = BUILTIN_ROLES[builtinRoleId]
+    const builtinRole = builtinRoles[builtinRoleId]
     const dbBuiltin = roles.filter(
       dbRole => getExternalRoleID(dbRole._id) === builtinRoleId
     )[0]

packages/server/src/middleware/authenticated.js

@@ -1,6 +1,6 @@
 const jwt = require("jsonwebtoken")
 const STATUS_CODES = require("../utilities/statusCodes")
-const { getRole, BUILTIN_ROLES } = require("../utilities/security/roles")
+const { getRole, getBuiltinRoles } = require("../utilities/security/roles")
 const { AuthTypes } = require("../constants")
 const {
   getAppId,
@@ -20,6 +20,7 @@ module.exports = async (ctx, next) => {
   // we hold it in state as a
   let appId = getAppId(ctx)
   const cookieAppId = ctx.cookies.get(getCookieName("currentapp"))
+  const builtinRoles = getBuiltinRoles()
   if (appId && cookieAppId !== appId) {
     setCookie(ctx, appId, "currentapp")
   } else if (cookieAppId) {
@@ -40,7 +41,7 @@ module.exports = async (ctx, next) => {
     ctx.appId = appId
     ctx.user = {
       appId,
-      role: BUILTIN_ROLES.PUBLIC,
+      role: builtinRoles.PUBLIC,
     }
     await next()
     return

packages/server/src/utilities/security/permissions.js

@@ -1,4 +1,5 @@
 const { flatten } = require("lodash")
+const { cloneDeep } = require("lodash/fp")
 
 const PermissionLevels = {
   READ: "read",
@@ -70,7 +71,7 @@ exports.BUILTIN_PERMISSION_IDS = {
   POWER: "power",
 }
 
-exports.BUILTIN_PERMISSIONS = {
+const BUILTIN_PERMISSIONS = {
   PUBLIC: {
     _id: exports.BUILTIN_PERMISSION_IDS.PUBLIC,
     name: "Public",
@@ -121,8 +122,12 @@ exports.BUILTIN_PERMISSIONS = {
   },
 }
 
+exports.getBuiltinPermissions = () => {
+  return cloneDeep(BUILTIN_PERMISSIONS)
+}
+
 exports.getBuiltinPermissionByID = id => {
-  const perms = Object.values(exports.BUILTIN_PERMISSIONS)
+  const perms = Object.values(BUILTIN_PERMISSIONS)
   return perms.find(perm => perm._id === id)
 }
 
@@ -155,7 +160,7 @@ exports.doesHaveResourcePermission = (
 }
 
 exports.doesHaveBasePermission = (permType, permLevel, permissionIds) => {
-  const builtins = Object.values(exports.BUILTIN_PERMISSIONS)
+  const builtins = Object.values(BUILTIN_PERMISSIONS)
   let permissions = flatten(
     builtins
       .filter(builtin => permissionIds.indexOf(builtin._id) !== -1)

packages/server/src/utilities/security/roles.js

@@ -26,7 +26,7 @@ Role.prototype.addInheritance = function(inherits) {
   return this
 }
 
-exports.BUILTIN_ROLES = {
+const BUILTIN_ROLES = {
   ADMIN: new Role(BUILTIN_IDS.ADMIN, "Admin")
     .addPermission(BUILTIN_PERMISSION_IDS.ADMIN)
     .addInheritance(BUILTIN_IDS.POWER),
@@ -44,11 +44,15 @@ exports.BUILTIN_ROLES = {
   ),
 }
 
-exports.BUILTIN_ROLE_ID_ARRAY = Object.values(exports.BUILTIN_ROLES).map(
+exports.getBuiltinRoles = () => {
+  return cloneDeep(BUILTIN_ROLES)
+}
+
+exports.BUILTIN_ROLE_ID_ARRAY = Object.values(BUILTIN_ROLES).map(
   role => role._id
 )
 
-exports.BUILTIN_ROLE_NAME_ARRAY = Object.values(exports.BUILTIN_ROLES).map(
+exports.BUILTIN_ROLE_NAME_ARRAY = Object.values(BUILTIN_ROLES).map(
   role => role.name
 )
 
@@ -60,17 +64,18 @@ function isBuiltin(role) {
  * Works through the inheritance ranks to see how far up the builtin stack this ID is.
  */
 function builtinRoleToNumber(id) {
+  const builtins = exports.getBuiltinRoles()
   const MAX = Object.values(BUILTIN_IDS).length + 1
   if (id === BUILTIN_IDS.ADMIN || id === BUILTIN_IDS.BUILDER) {
     return MAX
   }
-  let role = exports.BUILTIN_ROLES[id],
+  let role = builtins[id],
     count = 0
   do {
     if (!role) {
       break
     }
-    role = exports.BUILTIN_ROLES[role.inherits]
+    role = builtins[role.inherits]
     count++
   } while (role !== null)
   return count
@@ -107,7 +112,7 @@ exports.getRole = async (appId, roleId) => {
   // but can be extended by a doc stored about them (e.g. permissions)
   if (isBuiltin(roleId)) {
     role = cloneDeep(
-      Object.values(exports.BUILTIN_ROLES).find(role => role._id === roleId)
+      Object.values(BUILTIN_ROLES).find(role => role._id === roleId)
     )
   }
   try {

packages/server/src/utilities/security/utilities.js

@@ -6,7 +6,7 @@ const {
 } = require("../../utilities/security/permissions")
 const {
   lowerBuiltinRoleID,
-  BUILTIN_ROLES,
+  getBuiltinRoles,
 } = require("../../utilities/security/roles")
 const { DocumentTypes } = require("../../db/utils")
 
@@ -44,7 +44,7 @@ exports.getPermissionType = resourceId => {
 exports.getBasePermissions = resourceId => {
   const type = exports.getPermissionType(resourceId)
   const permissions = {}
-  for (let [roleId, role] of Object.entries(BUILTIN_ROLES)) {
+  for (let [roleId, role] of Object.entries(getBuiltinRoles())) {
     if (!role.permissionId) {
       continue
     }