Merge pull request #5134 from Budibase/fix/5103

Fix for RBAC on apps built from templates containing public screens

packages/backend-core/src/security/roles.js

@@ -1,5 +1,5 @@
 const { cloneDeep } = require("lodash/fp")
-const { BUILTIN_PERMISSION_IDS } = require("./permissions")
+const { BUILTIN_PERMISSION_IDS, PermissionLevels } = require("./permissions")
 const {
   generateRoleID,
   getRoleParams,
@@ -180,6 +180,20 @@ exports.getUserRoleHierarchy = async (userRoleId, opts = { idOnly: true }) => {
   return opts.idOnly ? roles.map(role => role._id) : roles
 }
 
+// this function checks that the provided permissions are in an array format
+// some templates/older apps will use a simple string instead of array for roles
+// convert the string to an array using the theory that write is higher than read
+exports.checkForRoleResourceArray = (rolePerms, resourceId) => {
+  if (rolePerms && !Array.isArray(rolePerms[resourceId])) {
+    const permLevel = rolePerms[resourceId]
+    rolePerms[resourceId] = [permLevel]
+    if (permLevel === PermissionLevels.WRITE) {
+      rolePerms[resourceId].push(PermissionLevels.READ)
+    }
+  }
+  return rolePerms
+}
+
 /**
  * Given an app ID this will retrieve all of the roles that are currently within that app.
  * @return {Promise<object[]>} An array of the role objects that were found.
@@ -209,15 +223,27 @@ exports.getAllRoles = async appId => {
       roles.push(Object.assign(builtinRole, dbBuiltin))
     }
   }
+  // check permissions
+  for (let role of roles) {
+    if (!role.permissions) {
+      continue
+    }
+    for (let resourceId of Object.keys(role.permissions)) {
+      role.permissions = exports.checkForRoleResourceArray(
+        role.permissions,
+        resourceId
+      )
+    }
+  }
   return roles
 }
 
 /**
- * This retrieves the required role
+ * This retrieves the required role for a resource
- * @param permLevel
+ * @param permLevel The level of request
- * @param resourceId
+ * @param resourceId The resource being requested
- * @param subResourceId
+ * @param subResourceId The sub resource being requested
- * @return {Promise<{permissions}|Object>}
+ * @return {Promise<{permissions}|Object>} returns the permissions required to access.
  */
 exports.getRequiredResourceRole = async (
   permLevel,

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

@@ -4,6 +4,7 @@ const {
   getDBRoleID,
   getExternalRoleID,
   getBuiltinRoles,
+  checkForRoleResourceArray,
 } = require("@budibase/backend-core/roles")
 const { getRoleParams } = require("../../db/utils")
 const {
@@ -144,12 +145,11 @@ exports.getResourcePerms = async function (ctx) {
   for (let level of SUPPORTED_LEVELS) {
     // update the various roleIds in the resource permissions
     for (let role of roles) {
-      const rolePerms = role.permissions
+      const rolePerms = checkForRoleResourceArray(role.permissions, resourceId)
       if (
         rolePerms &&
         rolePerms[resourceId] &&
-        (rolePerms[resourceId] === level ||
+        rolePerms[resourceId].indexOf(level) !== -1
-          rolePerms[resourceId].indexOf(level) !== -1)
       ) {
         permissions[level] = getExternalRoleID(role._id)
       }

packages/server/src/api/routes/table.js

@@ -40,7 +40,7 @@ router
   .get(
     "/api/tables/:tableId",
     paramResource("tableId"),
-    authorized(PermissionTypes.TABLE, PermissionLevels.READ),
+    authorized(PermissionTypes.TABLE, PermissionLevels.READ, { schema: true }),
     tableController.find
   )
   /**

packages/server/src/middleware/authorized.js

@@ -5,6 +5,7 @@ const {
 } = require("@budibase/backend-core/roles")
 const {
   PermissionTypes,
+  PermissionLevels,
   doesHaveBasePermission,
 } = require("@budibase/backend-core/permissions")
 const builderMiddleware = require("./builder")
@@ -64,7 +65,7 @@ const checkAuthorizedResource = async (
 }
 
 module.exports =
-  (permType, permLevel = null) =>
+  (permType, permLevel = null, opts = { schema: false }) =>
   async (ctx, next) => {
     // webhooks don't need authentication, each webhook unique
     // also internal requests (between services) don't need authorized
@@ -81,15 +82,25 @@ module.exports =
     await builderMiddleware(ctx, permType)
 
     // get the resource roles
-    let resourceRoles = []
+    let resourceRoles = [],
+      otherLevelRoles
+    const otherLevel =
+      permLevel === PermissionLevels.READ
+        ? PermissionLevels.WRITE
+        : PermissionLevels.READ
     const appId = getAppId()
     if (appId && hasResource(ctx)) {
       resourceRoles = await getRequiredResourceRole(permLevel, ctx)
+      if (opts && opts.schema) {
+        otherLevelRoles = await getRequiredResourceRole(otherLevel, ctx)
+      }
     }
 
     // if the resource is public, proceed
-    const isPublicResource = resourceRoles.includes(BUILTIN_ROLE_IDS.PUBLIC)
+    if (
-    if (isPublicResource) {
+      resourceRoles.includes(BUILTIN_ROLE_IDS.PUBLIC) ||
+      (otherLevelRoles && otherLevelRoles.includes(BUILTIN_ROLE_IDS.PUBLIC))
+    ) {
       return next()
     }
 
@@ -98,8 +109,17 @@ module.exports =
       return ctx.throw(403, "Session not authenticated")
     }
 
+    try {
       // check authorized
       await checkAuthorized(ctx, resourceRoles, permType, permLevel)
+    } catch (err) {
+      // this is a schema, check if
+      if (opts && opts.schema && permLevel) {
+        await checkAuthorized(ctx, otherLevelRoles, permType, otherLevel)
+      } else {
+        throw err
+      }
+    }
 
     // csrf protection
     return csrf(ctx, next)