From 3fd557bf0827f677177cba42dea0a392970cbb13 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 11 Feb 2021 18:13:09 +0000 Subject: [PATCH] Flipping RBAC implementation to use levels -> role for resource perms API and resource -> level -> role for full fetch (please note full fetch will only work for resources that have a custom permission in the system somewhere, everything else simply defaults to standard. --- .../server/src/api/controllers/permission.js | 28 ++++++------ .../src/api/routes/tests/permissions.spec.js | 11 ++--- .../src/utilities/security/permissions.js | 8 ++++ .../server/src/utilities/security/roles.js | 44 ++++++++++++------- 4 files changed, 55 insertions(+), 36 deletions(-) diff --git a/packages/server/src/api/controllers/permission.js b/packages/server/src/api/controllers/permission.js index 2ecfe806cf..66c3baa774 100644 --- a/packages/server/src/api/controllers/permission.js +++ b/packages/server/src/api/controllers/permission.js @@ -25,7 +25,7 @@ const PermissionUpdateType = { const SUPPORTED_LEVELS = [PermissionLevels.WRITE, PermissionLevels.READ] function getPermissionType(resourceId) { - const docType = DocumentTypes.filter(docType => + const docType = Object.values(DocumentTypes).filter(docType => resourceId.startsWith(docType) )[0] switch (docType) { @@ -54,7 +54,10 @@ function getBasePermissions(resourceId) { } const perms = getBuiltinPermissionByID(role.permissionId) const typedPermission = perms.permissions.find(perm => perm.type === type) - if (typedPermission) { + if ( + typedPermission && + SUPPORTED_LEVELS.indexOf(typedPermission.level) !== -1 + ) { const level = typedPermission.level permissions[level] = lowerBuiltinRoleID(permissions[level], roleId) if (isPermissionLevelHigherThanRead(level)) { @@ -148,18 +151,15 @@ exports.fetch = async function(ctx) { let permissions = {} // create an object with structure role ID -> resource ID -> level for (let role of roles) { - if (role.permissions) { - const roleId = getExternalRoleID(role._id) - if (permissions[roleId] == null) { - permissions[roleId] = {} - } - // TODO: need to work this out - for (let [resource, level] of Object.entries(role.permissions)) { - permissions[roleId][resource] = higherPermission( - permissions[roleId][resource], - level - ) + if (!role.permissions) { + continue + } + const roleId = getExternalRoleID(role._id) + for (let [resource, level] of Object.entries(role.permissions)) { + if (permissions[resource] == null) { + permissions[resource] = getBasePermissions(resource) } + permissions[resource][level] = roleId } } ctx.body = permissions @@ -178,7 +178,7 @@ exports.getResourcePerms = async function(ctx) { for (let level of SUPPORTED_LEVELS) { // update the various roleIds in the resource permissions for (let role of roles) { - if (role.permissions && role.permissions[resourceId]) { + if (role.permissions && role.permissions[resourceId] === level) { resourcePerms[level] = getExternalRoleID(role._id) } } diff --git a/packages/server/src/api/routes/tests/permissions.spec.js b/packages/server/src/api/routes/tests/permissions.spec.js index 8353eb271d..bb1f072efc 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.js +++ b/packages/server/src/api/routes/tests/permissions.spec.js @@ -71,21 +71,22 @@ describe("/permission", () => { .set(defaultHeaders(appId)) .expect("Content-Type", /json/) .expect(200) - expect(res.body[STD_ROLE_ID]).toEqual("read") + expect(res.body["read"]).toEqual(STD_ROLE_ID) + expect(res.body["write"]).toEqual(HIGHER_ROLE_ID) }) it("should get resource permissions with multiple roles", async () => { perms = await addPermission(request, appId, HIGHER_ROLE_ID, table._id, "write") const res = await getTablePermissions() - expect(res.body[HIGHER_ROLE_ID]).toEqual("write") - expect(res.body[STD_ROLE_ID]).toEqual("read") + expect(res.body["read"]).toEqual(STD_ROLE_ID) + expect(res.body["write"]).toEqual(HIGHER_ROLE_ID) const allRes = await request .get(`/api/permission`) .set(defaultHeaders(appId)) .expect("Content-Type", /json/) .expect(200) - expect(allRes.body[HIGHER_ROLE_ID][table._id]).toEqual("write") - expect(allRes.body[STD_ROLE_ID][table._id]).toEqual("read") + expect(allRes.body[table._id]["write"]).toEqual(HIGHER_ROLE_ID) + expect(allRes.body[table._id]["read"]).toEqual(STD_ROLE_ID) }) }) diff --git a/packages/server/src/utilities/security/permissions.js b/packages/server/src/utilities/security/permissions.js index 6752f790e6..d826c13966 100644 --- a/packages/server/src/utilities/security/permissions.js +++ b/packages/server/src/utilities/security/permissions.js @@ -63,6 +63,7 @@ function getAllowedLevels(userPermLevel) { } exports.BUILTIN_PERMISSION_IDS = { + PUBLIC: "public", READ_ONLY: "read_only", WRITE: "write", ADMIN: "admin", @@ -70,6 +71,13 @@ exports.BUILTIN_PERMISSION_IDS = { } exports.BUILTIN_PERMISSIONS = { + PUBLIC: { + _id: exports.BUILTIN_PERMISSION_IDS.PUBLIC, + name: "Public", + permissions: [ + new Permission(PermissionTypes.WEBHOOK, PermissionLevels.EXECUTE) + ], + }, READ_ONLY: { _id: exports.BUILTIN_PERMISSION_IDS.READ_ONLY, name: "Read only", diff --git a/packages/server/src/utilities/security/roles.js b/packages/server/src/utilities/security/roles.js index c6336dec97..eadc96d59d 100644 --- a/packages/server/src/utilities/security/roles.js +++ b/packages/server/src/utilities/security/roles.js @@ -37,7 +37,7 @@ exports.BUILTIN_ROLES = { .addPermission(BUILTIN_PERMISSION_IDS.WRITE) .addInheritance(BUILTIN_IDS.PUBLIC), PUBLIC: new Role(BUILTIN_IDS.PUBLIC, "Public").addPermission( - BUILTIN_PERMISSION_IDS.READ_ONLY + BUILTIN_PERMISSION_IDS.PUBLIC ), BUILDER: new Role(BUILTIN_IDS.BUILDER, "Builder").addPermission( BUILTIN_PERMISSION_IDS.ADMIN @@ -56,27 +56,37 @@ function isBuiltin(role) { return exports.BUILTIN_ROLE_ID_ARRAY.some(builtin => role.includes(builtin)) } +/** + * Works through the inheritance ranks to see how far up the builtin stack this ID is. + */ +function builtinRoleToNumber(id) { + 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], + count = 0 + do { + if (!role) { + break + } + role = exports.BUILTIN_ROLES[role.inherits] + count++ + } while (role !== null) + return count +} + /** * Returns whichever builtin roleID is lower. */ exports.lowerBuiltinRoleID = (roleId1, roleId2) => { - const MAX = Object.values(BUILTIN_IDS).length + 1 - function toNum(id) { - if (id === BUILTIN_IDS.ADMIN || id === BUILTIN_IDS.BUILDER) { - return MAX - } - let role = exports.BUILTIN_ROLES[id], - count = 0 - do { - if (!role) { - break - } - role = exports.BUILTIN_ROLES[role.inherits] - count++ - } while (role !== null) - return count + if (!roleId1) { + return roleId2 } - return toNum(roleId1) > toNum(roleId2) ? roleId2 : roleId1 + if (!roleId2) { + return roleId1 + } + return builtinRoleToNumber(roleId1) > builtinRoleToNumber(roleId2) ? roleId2 : roleId1 } /**