From f57db6afad43680caeac0c48823087cb29ef4de5 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 12 Feb 2021 20:34:54 +0000 Subject: [PATCH] Fixing an issue with RBAC, there was a mutable issue where a server builtin resource was getting updated, fixed this by not exposing the mutable structure, instead exposing a function which provides a new object everytime. --- .../server/src/api/controllers/permission.js | 9 ++++----- packages/server/src/api/controllers/role.js | 5 +++-- packages/server/src/middleware/authenticated.js | 5 +++-- .../src/utilities/security/permissions.js | 11 ++++++++--- packages/server/src/utilities/security/roles.js | 17 +++++++++++------ .../server/src/utilities/security/utilities.js | 4 ++-- 6 files changed, 31 insertions(+), 20 deletions(-) diff --git a/packages/server/src/api/controllers/permission.js b/packages/server/src/api/controllers/permission.js index c505332c61..56c9d3f8c6 100644 --- a/packages/server/src/api/controllers/permission.js +++ b/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) { diff --git a/packages/server/src/api/controllers/role.js b/packages/server/src/api/controllers/role.js index 440dbfde35..2c29d1030e 100644 --- a/packages/server/src/api/controllers/role.js +++ b/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] diff --git a/packages/server/src/middleware/authenticated.js b/packages/server/src/middleware/authenticated.js index e4e678abad..659baa8f6c 100644 --- a/packages/server/src/middleware/authenticated.js +++ b/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 diff --git a/packages/server/src/utilities/security/permissions.js b/packages/server/src/utilities/security/permissions.js index 342654f9ba..083de730b5 100644 --- a/packages/server/src/utilities/security/permissions.js +++ b/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) diff --git a/packages/server/src/utilities/security/roles.js b/packages/server/src/utilities/security/roles.js index 660f190d6f..38297f59b2 100644 --- a/packages/server/src/utilities/security/roles.js +++ b/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, 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 { diff --git a/packages/server/src/utilities/security/utilities.js b/packages/server/src/utilities/security/utilities.js index 9d191b9572..f22b11dbd3 100644 --- a/packages/server/src/utilities/security/utilities.js +++ b/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 }