From 37c00f24bd7b254cffe7c4d1b8d1423f9a48e5b4 Mon Sep 17 00:00:00 2001 From: Martin McKeaveney Date: Wed, 10 Feb 2021 18:18:31 +0000 Subject: [PATCH 01/11] control RBAC from data section --- .../builder/src/builderStore/store/backend.js | 24 ++++++ .../backend/DataTable/DataTable.svelte | 2 + .../backend/DataTable/ViewDataTable.svelte | 2 + .../buttons/ManageAccessButton.svelte | 27 ++++++ .../popovers/ManageAccessPopover.svelte | 83 +++++++++++++++++++ packages/server/src/api/routes/view.js | 3 + 6 files changed, 141 insertions(+) create mode 100644 packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte create mode 100644 packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte diff --git a/packages/builder/src/builderStore/store/backend.js b/packages/builder/src/builderStore/store/backend.js index cb8d154350..d2a19f8726 100644 --- a/packages/builder/src/builderStore/store/backend.js +++ b/packages/builder/src/builderStore/store/backend.js @@ -328,6 +328,30 @@ export const getBackendUiStore = () => { return response }, }, + permissions: { + fetch: async () => { + const response = await api.get("/api/permission") + const json = await response.json() + return json + }, + fetchLevels: async () => { + const response = await api.get("/api/permission/levels") + const json = await response.json() + return json + }, + forResource: async resourceId => { + const response = await api.get(`/api/permission/${resourceId}`) + const json = await response.json() + return json + }, + save: async ({ role, resource, level }) => { + const response = await api.post( + `/api/permission/${role}/${resource}/${level}` + ) + const json = await response.json() + return json + }, + }, } return store diff --git a/packages/builder/src/components/backend/DataTable/DataTable.svelte b/packages/builder/src/components/backend/DataTable/DataTable.svelte index 436a3b4dee..937cb7931a 100644 --- a/packages/builder/src/components/backend/DataTable/DataTable.svelte +++ b/packages/builder/src/components/backend/DataTable/DataTable.svelte @@ -5,6 +5,7 @@ import CreateViewButton from "./buttons/CreateViewButton.svelte" import ExportButton from "./buttons/ExportButton.svelte" import EditRolesButton from "./buttons/EditRolesButton.svelte" + import ManageAccessButton from "./buttons/ManageAccessButton.svelte" import * as api from "./api" import Table from "./Table.svelte" import { TableNames } from "constants" @@ -48,6 +49,7 @@ modalContentComponent={isUsersTable ? CreateEditUser : CreateEditRow} /> + {/if} {#if isUsersTable} diff --git a/packages/builder/src/components/backend/DataTable/ViewDataTable.svelte b/packages/builder/src/components/backend/DataTable/ViewDataTable.svelte index 2ace2bb338..6597dcd481 100644 --- a/packages/builder/src/components/backend/DataTable/ViewDataTable.svelte +++ b/packages/builder/src/components/backend/DataTable/ViewDataTable.svelte @@ -6,6 +6,7 @@ import GroupByButton from "./buttons/GroupByButton.svelte" import FilterButton from "./buttons/FilterButton.svelte" import ExportButton from "./buttons/ExportButton.svelte" + import ManageAccessButton from "./buttons/ManageAccessButton.svelte" export let view = {} @@ -54,4 +55,5 @@ {/if} + diff --git a/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte b/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte new file mode 100644 index 0000000000..c6c6bddecf --- /dev/null +++ b/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte @@ -0,0 +1,27 @@ + + +
+ + + Manage Access + +
+ + + + + diff --git a/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte b/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte new file mode 100644 index 0000000000..bcb75863f3 --- /dev/null +++ b/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte @@ -0,0 +1,83 @@ + + +
+
Manage Access
+ {#each levels as level} + + {/each} +
+ + +
+
+ + diff --git a/packages/server/src/api/routes/view.js b/packages/server/src/api/routes/view.js index 0ae12f687c..f6d1a55803 100644 --- a/packages/server/src/api/routes/view.js +++ b/packages/server/src/api/routes/view.js @@ -2,6 +2,7 @@ const Router = require("@koa/router") const viewController = require("../controllers/view") const rowController = require("../controllers/row") const authorized = require("../../middleware/authorized") +const { paramResource } = require("../../middleware/resourceId") const { BUILDER, PermissionTypes, @@ -15,12 +16,14 @@ router .get("/api/views/export", authorized(BUILDER), viewController.exportView) .get( "/api/views/:viewName", + paramResource("viewName"), authorized(PermissionTypes.VIEW, PermissionLevels.READ), rowController.fetchView ) .get("/api/views", authorized(BUILDER), viewController.fetch) .delete( "/api/views/:viewName", + paramResource("viewName"), authorized(BUILDER), usage, viewController.destroy From 701e2b218e8896a901d48454794f244106ebda69 Mon Sep 17 00:00:00 2001 From: Martin McKeaveney Date: Wed, 10 Feb 2021 22:23:27 +0000 Subject: [PATCH 02/11] RBAC popover complete --- .../builder/src/builderStore/store/backend.js | 9 ++ .../backend/DataTable/DataTable.svelte | 2 +- .../backend/DataTable/ViewDataTable.svelte | 2 +- .../buttons/ManageAccessButton.svelte | 20 ++- .../DataTable/modals/CreateEditColumn.svelte | 2 +- .../popovers/ManageAccessPopover.svelte | 114 ++++++++++++------ .../builder/src/constants/backend/index.js | 8 ++ 7 files changed, 116 insertions(+), 41 deletions(-) diff --git a/packages/builder/src/builderStore/store/backend.js b/packages/builder/src/builderStore/store/backend.js index d2a19f8726..912e458995 100644 --- a/packages/builder/src/builderStore/store/backend.js +++ b/packages/builder/src/builderStore/store/backend.js @@ -30,6 +30,7 @@ export const getBackendUiStore = () => { const queries = await queriesResponse.json() const integrationsResponse = await api.get("/api/integrations") const integrations = await integrationsResponse.json() + const permissionLevels = await store.actions.permissions.fetchLevels() store.update(state => { state.selectedDatabase = db @@ -37,6 +38,7 @@ export const getBackendUiStore = () => { state.datasources = datasources state.queries = queries state.integrations = integrations + state.permissionLevels = permissionLevels return state }) }, @@ -351,6 +353,13 @@ export const getBackendUiStore = () => { const json = await response.json() return json }, + delete: async ({ role, resource, level }) => { + const response = await api.delete( + `/api/permission/${role}/${resource}/${level}` + ) + const json = await response.json() + return json + }, }, } diff --git a/packages/builder/src/components/backend/DataTable/DataTable.svelte b/packages/builder/src/components/backend/DataTable/DataTable.svelte index 937cb7931a..577fda62a8 100644 --- a/packages/builder/src/components/backend/DataTable/DataTable.svelte +++ b/packages/builder/src/components/backend/DataTable/DataTable.svelte @@ -48,8 +48,8 @@ title={isUsersTable ? 'Create New User' : 'Create New Row'} modalContentComponent={isUsersTable ? CreateEditUser : CreateEditRow} /> - + {/if} {#if isUsersTable} diff --git a/packages/builder/src/components/backend/DataTable/ViewDataTable.svelte b/packages/builder/src/components/backend/DataTable/ViewDataTable.svelte index 6597dcd481..f875fa8849 100644 --- a/packages/builder/src/components/backend/DataTable/ViewDataTable.svelte +++ b/packages/builder/src/components/backend/DataTable/ViewDataTable.svelte @@ -54,6 +54,6 @@ {#if view.calculation} {/if} - + diff --git a/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte b/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte index c6c6bddecf..2540267d72 100644 --- a/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte +++ b/packages/builder/src/components/backend/DataTable/buttons/ManageAccessButton.svelte @@ -1,5 +1,7 @@
- + Manage Access
- + diff --git a/packages/builder/src/constants/backend/index.js b/packages/builder/src/constants/backend/index.js index 80eaf613f8..afb474499b 100644 --- a/packages/builder/src/constants/backend/index.js +++ b/packages/builder/src/constants/backend/index.js @@ -92,3 +92,11 @@ export const HostingTypes = { CLOUD: "cloud", SELF: "self", } + +export const Roles = { + ADMIN: "ADMIN", + POWER: "POWER", + BASIC: "BASIC", + PUBLIC: "PUBLIC", + BUILDER: "BUILDER", +} From 111ff61bbc5901a93c3c4ee52fc99b9bdebe36be Mon Sep 17 00:00:00 2001 From: Martin McKeaveney Date: Wed, 10 Feb 2021 22:25:30 +0000 Subject: [PATCH 03/11] tidy up --- packages/builder/src/builderStore/store/backend.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/builder/src/builderStore/store/backend.js b/packages/builder/src/builderStore/store/backend.js index 912e458995..d6d712083a 100644 --- a/packages/builder/src/builderStore/store/backend.js +++ b/packages/builder/src/builderStore/store/backend.js @@ -331,11 +331,6 @@ export const getBackendUiStore = () => { }, }, permissions: { - fetch: async () => { - const response = await api.get("/api/permission") - const json = await response.json() - return json - }, fetchLevels: async () => { const response = await api.get("/api/permission/levels") const json = await response.json() From 1abc4dd1f7064e847ec38ce96e4b0a2db445b1e5 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 11 Feb 2021 10:24:37 +0000 Subject: [PATCH 04/11] WIP - storing progress on RBAC changes. --- .../server/src/api/controllers/permission.js | 45 ++++++++++++++++++- .../src/utilities/security/permissions.js | 5 +++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/permission.js b/packages/server/src/api/controllers/permission.js index 1e6bd1869c..09ef11f55e 100644 --- a/packages/server/src/api/controllers/permission.js +++ b/packages/server/src/api/controllers/permission.js @@ -1,7 +1,9 @@ const { BUILTIN_PERMISSIONS, PermissionLevels, + PermissionTypes, higherPermission, + getBuiltinPermissionByID, } = require("../../utilities/security/permissions") const { isBuiltin, @@ -9,7 +11,7 @@ const { getExternalRoleID, BUILTIN_ROLES, } = require("../../utilities/security/roles") -const { getRoleParams } = require("../../db/utils") +const { getRoleParams, DocumentTypes } = require("../../db/utils") const CouchDB = require("../../db") const { cloneDeep } = require("lodash/fp") @@ -18,6 +20,47 @@ const PermissionUpdateType = { ADD: "add", } +function getBasePermissions(resourceId) { + const docType = DocumentTypes.filter(docType => + resourceId.startsWith(docType) + )[0] + const levelsToFind = [PermissionLevels.WRITE, PermissionLevels.READ] + let type + switch (docType) { + case DocumentTypes.TABLE: + case DocumentTypes.ROW: + type = PermissionTypes.TABLE + break + case DocumentTypes.AUTOMATION: + type = PermissionTypes.AUTOMATION + break + case DocumentTypes.WEBHOOK: + type = PermissionTypes.WEBHOOK + break + case DocumentTypes.QUERY: + case DocumentTypes.DATASOURCE: + type = PermissionTypes.QUERY + break + default: + // views don't have an ID, will end up here + type = PermissionTypes.VIEW + break + } + + const permissions = {} + for (let [roleId, role] of Object.entries(BUILTIN_ROLES)) { + if (!role.permissionId) { + continue + } + const perms = getBuiltinPermissionByID(role.permissionId) + const typedPermission = perms.permissions.find(perm => perm.type === type) + if (typedPermission) { + // TODO: need to get the lowest role + // TODO: store the read/write with the lowest role + } + } +} + // utility function to stop this repetition - permissions always stored under roles async function getAllDBRoles(db) { const body = await db.allDocs( diff --git a/packages/server/src/utilities/security/permissions.js b/packages/server/src/utilities/security/permissions.js index c0bc26cb8f..dba4b99593 100644 --- a/packages/server/src/utilities/security/permissions.js +++ b/packages/server/src/utilities/security/permissions.js @@ -97,6 +97,11 @@ exports.BUILTIN_PERMISSIONS = { }, } +exports.getBuiltinPermissionByID = id => { + const perms = Object.values(exports.BUILTIN_PERMISSIONS) + return perms.find(perm => perm._id === id) +} + exports.doesHaveResourcePermission = ( permissions, permLevel, From 6c4c70e62b278efc171e2a58e47e5a71c9357e16 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 11 Feb 2021 13:29:15 +0000 Subject: [PATCH 05/11] Some updates to RBAC backend, try to make switch to object support level -> roleID. --- .../server/src/api/controllers/permission.js | 44 +++++++++++-------- .../src/utilities/security/permissions.js | 37 +++++++++------- .../server/src/utilities/security/roles.js | 25 +++++++++++ 3 files changed, 72 insertions(+), 34 deletions(-) diff --git a/packages/server/src/api/controllers/permission.js b/packages/server/src/api/controllers/permission.js index 09ef11f55e..286ea5667c 100644 --- a/packages/server/src/api/controllers/permission.js +++ b/packages/server/src/api/controllers/permission.js @@ -4,11 +4,13 @@ const { PermissionTypes, higherPermission, getBuiltinPermissionByID, + isPermissionLevelHigherThanRead, } = require("../../utilities/security/permissions") const { isBuiltin, getDBRoleID, getExternalRoleID, + lowerBuiltinRoleID, BUILTIN_ROLES, } = require("../../utilities/security/roles") const { getRoleParams, DocumentTypes } = require("../../db/utils") @@ -20,33 +22,31 @@ const PermissionUpdateType = { ADD: "add", } -function getBasePermissions(resourceId) { +const SUPPORTED_LEVELS = [PermissionLevels.WRITE, PermissionLevels.READ] + +function getPermissionType(resourceId) { const docType = DocumentTypes.filter(docType => resourceId.startsWith(docType) )[0] - const levelsToFind = [PermissionLevels.WRITE, PermissionLevels.READ] - let type switch (docType) { case DocumentTypes.TABLE: case DocumentTypes.ROW: - type = PermissionTypes.TABLE - break + return PermissionTypes.TABLE case DocumentTypes.AUTOMATION: - type = PermissionTypes.AUTOMATION - break + return PermissionTypes.AUTOMATION case DocumentTypes.WEBHOOK: - type = PermissionTypes.WEBHOOK - break + return PermissionTypes.WEBHOOK case DocumentTypes.QUERY: case DocumentTypes.DATASOURCE: - type = PermissionTypes.QUERY - break + return PermissionTypes.QUERY default: // views don't have an ID, will end up here - type = PermissionTypes.VIEW - break + return PermissionTypes.VIEW } +} +async function getBasePermissions(resourceId) { + const type = getPermissionType(resourceId) const permissions = {} for (let [roleId, role] of Object.entries(BUILTIN_ROLES)) { if (!role.permissionId) { @@ -55,10 +55,17 @@ function getBasePermissions(resourceId) { const perms = getBuiltinPermissionByID(role.permissionId) const typedPermission = perms.permissions.find(perm => perm.type === type) if (typedPermission) { - // TODO: need to get the lowest role - // TODO: store the read/write with the lowest role + const level = typedPermission.level + permissions[level] = lowerBuiltinRoleID(permissions[level], roleId) + if (isPermissionLevelHigherThanRead(level)) { + permissions[PermissionLevels.READ] = lowerBuiltinRoleID( + permissions[PermissionLevels.READ], + roleId + ) + } } } + return permissions } // utility function to stop this repetition - permissions always stored under roles @@ -132,7 +139,7 @@ exports.fetchBuiltin = function(ctx) { exports.fetchLevels = function(ctx) { // for now only provide the read/write perms externally - ctx.body = [PermissionLevels.WRITE, PermissionLevels.READ] + ctx.body = SUPPORTED_LEVELS } exports.fetch = async function(ctx) { @@ -167,11 +174,12 @@ exports.getResourcePerms = async function(ctx) { ) const roles = body.rows.map(row => row.doc) const resourcePerms = {} - for (let role of roles) { + for (let level of SUPPORTED_LEVELS) { + for (let role of roles) // update the various roleIds in the resource permissions if (role.permissions && role.permissions[resourceId]) { const roleId = getExternalRoleID(role._id) - resourcePerms[roleId] = higherPermission( + resourcePerms[level] = higherPermission( resourcePerms[roleId], role.permissions[resourceId] ) diff --git a/packages/server/src/utilities/security/permissions.js b/packages/server/src/utilities/security/permissions.js index dba4b99593..6752f790e6 100644 --- a/packages/server/src/utilities/security/permissions.js +++ b/packages/server/src/utilities/security/permissions.js @@ -23,6 +23,22 @@ function Permission(type, level) { this.type = type } +function levelToNumber(perm) { + switch (perm) { + // not everything has execute privileges + case PermissionLevels.EXECUTE: + return 0 + case PermissionLevels.READ: + return 1 + case PermissionLevels.WRITE: + return 2 + case PermissionLevels.ADMIN: + return 3 + default: + return -1 + } +} + /** * Given the specified permission level for the user return the levels they are allowed to carry out. * @param {string} userPermLevel The permission level of the user. @@ -149,22 +165,11 @@ exports.doesHaveBasePermission = (permType, permLevel, permissionIds) => { } exports.higherPermission = (perm1, perm2) => { - function toNum(perm) { - switch (perm) { - // not everything has execute privileges - case PermissionLevels.EXECUTE: - return 0 - case PermissionLevels.READ: - return 1 - case PermissionLevels.WRITE: - return 2 - case PermissionLevels.ADMIN: - return 3 - default: - return -1 - } - } - return toNum(perm1) > toNum(perm2) ? perm1 : perm2 + return levelToNumber(perm1) > levelToNumber(perm2) ? perm1 : perm2 +} + +exports.isPermissionLevelHigherThanRead = level => { + return levelToNumber(level) > 1 } // utility as a lot of things need simply the builder permission diff --git a/packages/server/src/utilities/security/roles.js b/packages/server/src/utilities/security/roles.js index 447c049e1d..0cd9d0621e 100644 --- a/packages/server/src/utilities/security/roles.js +++ b/packages/server/src/utilities/security/roles.js @@ -222,6 +222,31 @@ exports.getExternalRoleID = roleId => { return roleId } +/** + * Returns whichever roleID is lower. + */ +exports.lowerRoleID = async (appId, roleId1, roleId2) => { + // TODO: need to make this function work + const MAX = Object.values(BUILTIN_IDS).length + 1 + async function toNum(id) { + if (id === BUILTIN_IDS.ADMIN || id === BUILTIN_IDS.BUILDER) { + return MAX + } + let role = await exports.getRole(appId, id), + count = 0 + do { + if (!role) { + break + } + role = exports.BUILTIN_ROLES[role.inherits] + count++ + } while (role !== null) + return count + } + const [num1, num2] = Promise.all([toNum(roleId1), toNum(roleId2)]) + return num1 > num2 ? roleId2 : roleId1 +} + exports.AccessController = AccessController exports.BUILTIN_ROLE_IDS = BUILTIN_IDS exports.isBuiltin = isBuiltin From a2ce35b2f660fea7287080e75bd8ccb4267e5bdf Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 11 Feb 2021 13:38:07 +0000 Subject: [PATCH 06/11] Fixing minor issue with switch to level -> roleID. --- .../server/src/api/controllers/permission.js | 16 +++---- .../server/src/utilities/security/roles.js | 48 +++++++++---------- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/packages/server/src/api/controllers/permission.js b/packages/server/src/api/controllers/permission.js index 286ea5667c..2ecfe806cf 100644 --- a/packages/server/src/api/controllers/permission.js +++ b/packages/server/src/api/controllers/permission.js @@ -45,7 +45,7 @@ function getPermissionType(resourceId) { } } -async function getBasePermissions(resourceId) { +function getBasePermissions(resourceId) { const type = getPermissionType(resourceId) const permissions = {} for (let [roleId, role] of Object.entries(BUILTIN_ROLES)) { @@ -153,6 +153,7 @@ exports.fetch = async function(ctx) { 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], @@ -173,16 +174,13 @@ exports.getResourcePerms = async function(ctx) { }) ) const roles = body.rows.map(row => row.doc) - const resourcePerms = {} + const resourcePerms = getBasePermissions(resourceId) for (let level of SUPPORTED_LEVELS) { - for (let role of roles) // update the various roleIds in the resource permissions - if (role.permissions && role.permissions[resourceId]) { - const roleId = getExternalRoleID(role._id) - resourcePerms[level] = higherPermission( - resourcePerms[roleId], - role.permissions[resourceId] - ) + for (let role of roles) { + if (role.permissions && role.permissions[resourceId]) { + resourcePerms[level] = getExternalRoleID(role._id) + } } } ctx.body = resourcePerms diff --git a/packages/server/src/utilities/security/roles.js b/packages/server/src/utilities/security/roles.js index 0cd9d0621e..c6336dec97 100644 --- a/packages/server/src/utilities/security/roles.js +++ b/packages/server/src/utilities/security/roles.js @@ -56,6 +56,29 @@ function isBuiltin(role) { return exports.BUILTIN_ROLE_ID_ARRAY.some(builtin => role.includes(builtin)) } +/** + * 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 + } + return toNum(roleId1) > toNum(roleId2) ? roleId2 : roleId1 +} + /** * Gets the role object, this is mainly useful for two purposes, to check if the level exists and * to check if the role inherits any others. @@ -222,31 +245,6 @@ exports.getExternalRoleID = roleId => { return roleId } -/** - * Returns whichever roleID is lower. - */ -exports.lowerRoleID = async (appId, roleId1, roleId2) => { - // TODO: need to make this function work - const MAX = Object.values(BUILTIN_IDS).length + 1 - async function toNum(id) { - if (id === BUILTIN_IDS.ADMIN || id === BUILTIN_IDS.BUILDER) { - return MAX - } - let role = await exports.getRole(appId, id), - count = 0 - do { - if (!role) { - break - } - role = exports.BUILTIN_ROLES[role.inherits] - count++ - } while (role !== null) - return count - } - const [num1, num2] = Promise.all([toNum(roleId1), toNum(roleId2)]) - return num1 > num2 ? roleId2 : roleId1 -} - exports.AccessController = AccessController exports.BUILTIN_ROLE_IDS = BUILTIN_IDS exports.isBuiltin = isBuiltin From 9ea045624848b9038b227d4502bbd2df09394556 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 11 Feb 2021 18:13:09 +0000 Subject: [PATCH 07/11] 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 } /** From a1075881a615b76ae447507aba7e8390c0f31987 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 12 Feb 2021 09:55:37 +0000 Subject: [PATCH 08/11] Linting. --- packages/server/src/utilities/security/permissions.js | 2 +- packages/server/src/utilities/security/roles.js | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/server/src/utilities/security/permissions.js b/packages/server/src/utilities/security/permissions.js index d826c13966..342654f9ba 100644 --- a/packages/server/src/utilities/security/permissions.js +++ b/packages/server/src/utilities/security/permissions.js @@ -75,7 +75,7 @@ exports.BUILTIN_PERMISSIONS = { _id: exports.BUILTIN_PERMISSION_IDS.PUBLIC, name: "Public", permissions: [ - new Permission(PermissionTypes.WEBHOOK, PermissionLevels.EXECUTE) + new Permission(PermissionTypes.WEBHOOK, PermissionLevels.EXECUTE), ], }, READ_ONLY: { diff --git a/packages/server/src/utilities/security/roles.js b/packages/server/src/utilities/security/roles.js index eadc96d59d..660f190d6f 100644 --- a/packages/server/src/utilities/security/roles.js +++ b/packages/server/src/utilities/security/roles.js @@ -86,7 +86,9 @@ exports.lowerBuiltinRoleID = (roleId1, roleId2) => { if (!roleId2) { return roleId1 } - return builtinRoleToNumber(roleId1) > builtinRoleToNumber(roleId2) ? roleId2 : roleId1 + return builtinRoleToNumber(roleId1) > builtinRoleToNumber(roleId2) + ? roleId2 + : roleId1 } /** From ec3ef225b91dbacfa98cb408d7a248b92924adfe Mon Sep 17 00:00:00 2001 From: Martin McKeaveney Date: Fri, 12 Feb 2021 10:12:17 +0000 Subject: [PATCH 09/11] simplify UI --- .../builder/src/builderStore/store/backend.js | 7 -- .../popovers/ManageAccessPopover.svelte | 77 +++++-------------- 2 files changed, 18 insertions(+), 66 deletions(-) diff --git a/packages/builder/src/builderStore/store/backend.js b/packages/builder/src/builderStore/store/backend.js index d6d712083a..6731aea51c 100644 --- a/packages/builder/src/builderStore/store/backend.js +++ b/packages/builder/src/builderStore/store/backend.js @@ -348,13 +348,6 @@ export const getBackendUiStore = () => { const json = await response.json() return json }, - delete: async ({ role, resource, level }) => { - const response = await api.delete( - `/api/permission/${role}/${resource}/${level}` - ) - const json = await response.json() - return json - }, }, } diff --git a/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte b/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte index 8a1859a2d6..d8266b8f3b 100644 --- a/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte +++ b/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte @@ -4,42 +4,26 @@ import { Roles } from "constants/backend" import api from "builderStore/api" import { notifier } from "builderStore/store/notifications" - import { Button, Label, Select, Spacer } from "@budibase/bbui" + import { Button, Label, Input, Select, Spacer } from "@budibase/bbui" export let resourceId export let permissions - export let levels export let onClosed - // Draft level and role for editing - let level = levels[0] - let role = Roles.BASIC - - $: permissionKeys = Object.keys(permissions) - - async function addPermission() { + async function changePermission(level, role) { await backendUiStore.actions.permissions.save({ level, role, resource: resourceId, }) - // Show updated permissions in UI + // Show updated permissions in UI: REMOVE permissions = await backendUiStore.actions.permissions.forResource( resourceId ) - notifier.success("Access rule saved.") - - // Reset the draft permissions - level = levels[0] - role = Roles.BASIC - } - - async function deletePermission(level, role) { - await backendUiStore.actions.permissions.delete({ level, role, resourceId }) - delete permissions[role] - notifier.danger("Removed access rule.") - permissions = permissions + notifier.success("Updated permissions.") + // TODO: update permissions + // permissions[] } @@ -49,39 +33,24 @@
-
- {#if permissionKeys.length === 0} - - {/if} - {#each permissionKeys as role} - - - deletePermission(permissions[role], role)} /> + {#each Object.keys(permissions) as level} + + {/each}
-
- - -
- - -
-
@@ -107,19 +76,9 @@ margin-top: var(--spacing-l); } - .draft-permission { + .row { display: grid; grid-template-columns: 1fr 1fr; grid-gap: var(--spacing-m); } - - .row { - display: grid; - grid-template-columns: 1fr 1fr 20px; - grid-gap: var(--spacing-s); - } - - .delete { - cursor: pointer; - } From 1a8fe9b02cffdfd8d99bc6b6a0c7487ff18f95c5 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 12 Feb 2021 12:02:07 +0000 Subject: [PATCH 10/11] Some more fixes for RBAC as well as fixing the duplication of roles. --- .../popovers/ManageAccessPopover.svelte | 1 + .../server/src/api/controllers/permission.js | 98 ++++++++----------- packages/server/src/api/controllers/role.js | 4 +- .../src/utilities/security/utilities.js | 70 +++++++++++++ 4 files changed, 115 insertions(+), 58 deletions(-) create mode 100644 packages/server/src/utilities/security/utilities.js diff --git a/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte b/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte index d8266b8f3b..fab0ee8fd0 100644 --- a/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte +++ b/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte @@ -29,6 +29,7 @@
Who Can Access This Data?
+
diff --git a/packages/server/src/api/controllers/permission.js b/packages/server/src/api/controllers/permission.js index 66c3baa774..c505332c61 100644 --- a/packages/server/src/api/controllers/permission.js +++ b/packages/server/src/api/controllers/permission.js @@ -1,72 +1,42 @@ const { BUILTIN_PERMISSIONS, PermissionLevels, - PermissionTypes, - higherPermission, - getBuiltinPermissionByID, isPermissionLevelHigherThanRead, + higherPermission, } = require("../../utilities/security/permissions") const { isBuiltin, getDBRoleID, getExternalRoleID, - lowerBuiltinRoleID, BUILTIN_ROLES, } = require("../../utilities/security/roles") -const { getRoleParams, DocumentTypes } = require("../../db/utils") +const { getRoleParams } = require("../../db/utils") const CouchDB = require("../../db") const { cloneDeep } = require("lodash/fp") +const { + CURRENTLY_SUPPORTED_LEVELS, + getBasePermissions, +} = require("../../utilities/security/utilities") const PermissionUpdateType = { REMOVE: "remove", ADD: "add", } -const SUPPORTED_LEVELS = [PermissionLevels.WRITE, PermissionLevels.READ] +const SUPPORTED_LEVELS = CURRENTLY_SUPPORTED_LEVELS -function getPermissionType(resourceId) { - const docType = Object.values(DocumentTypes).filter(docType => - resourceId.startsWith(docType) - )[0] - switch (docType) { - case DocumentTypes.TABLE: - case DocumentTypes.ROW: - return PermissionTypes.TABLE - case DocumentTypes.AUTOMATION: - return PermissionTypes.AUTOMATION - case DocumentTypes.WEBHOOK: - return PermissionTypes.WEBHOOK - case DocumentTypes.QUERY: - case DocumentTypes.DATASOURCE: - return PermissionTypes.QUERY - default: - // views don't have an ID, will end up here - return PermissionTypes.VIEW +// quick function to perform a bit of weird logic, make sure fetch calls +// always say a write role also has read permission +function fetchLevelPerms(permissions, level, roleId) { + if (!permissions) { + permissions = {} } -} - -function getBasePermissions(resourceId) { - const type = getPermissionType(resourceId) - const permissions = {} - for (let [roleId, role] of Object.entries(BUILTIN_ROLES)) { - if (!role.permissionId) { - continue - } - const perms = getBuiltinPermissionByID(role.permissionId) - const typedPermission = perms.permissions.find(perm => perm.type === type) - if ( - typedPermission && - SUPPORTED_LEVELS.indexOf(typedPermission.level) !== -1 - ) { - const level = typedPermission.level - permissions[level] = lowerBuiltinRoleID(permissions[level], roleId) - if (isPermissionLevelHigherThanRead(level)) { - permissions[PermissionLevels.READ] = lowerBuiltinRoleID( - permissions[PermissionLevels.READ], - roleId - ) - } - } + permissions[level] = roleId + if ( + isPermissionLevelHigherThanRead(level) && + !permissions[PermissionLevels.READ] + ) { + permissions[PermissionLevels.READ] = roleId } return permissions } @@ -118,7 +88,10 @@ async function updatePermissionOnRole( } // handle the adding, we're on the correct role, at it to this if (!remove && role._id === dbRoleId) { - rolePermissions[resourceId] = level + rolePermissions[resourceId] = higherPermission( + rolePermissions[resourceId], + level + ) updated = true } // handle the update, add it to bulk docs to perform at end @@ -156,13 +129,20 @@ exports.fetch = async function(ctx) { } 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 + permissions[resource] = fetchLevelPerms( + permissions[resource], + level, + roleId + ) } } - ctx.body = permissions + // apply the base permissions + const finalPermissions = {} + for (let [resource, permission] of Object.entries(permissions)) { + const basePerms = getBasePermissions(resource) + finalPermissions[resource] = Object.assign(basePerms, permission) + } + ctx.body = finalPermissions } exports.getResourcePerms = async function(ctx) { @@ -174,16 +154,20 @@ exports.getResourcePerms = async function(ctx) { }) ) const roles = body.rows.map(row => row.doc) - const resourcePerms = getBasePermissions(resourceId) + let permissions = {} 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] === level) { - resourcePerms[level] = getExternalRoleID(role._id) + permissions = fetchLevelPerms( + permissions, + level, + getExternalRoleID(role._id) + ) } } } - ctx.body = resourcePerms + ctx.body = Object.assign(getBasePermissions(resourceId), permissions) } exports.addPermission = async function(ctx) { diff --git a/packages/server/src/api/controllers/role.js b/packages/server/src/api/controllers/role.js index 59afcc06de..440dbfde35 100644 --- a/packages/server/src/api/controllers/role.js +++ b/packages/server/src/api/controllers/role.js @@ -57,7 +57,7 @@ exports.fetch = async function(ctx) { include_docs: true, }) ) - const roles = body.rows.map(row => row.doc) + let roles = body.rows.map(row => row.doc) // need to combine builtin with any DB record of them (for sake of permissions) for (let builtinRoleId of EXTERNAL_BUILTIN_ROLE_IDS) { @@ -68,6 +68,8 @@ exports.fetch = async function(ctx) { if (dbBuiltin == null) { roles.push(builtinRole) } else { + // remove role and all back after combining with the builtin + roles = roles.filter(role => role._id !== dbBuiltin._id) dbBuiltin._id = getExternalRoleID(dbBuiltin._id) roles.push(Object.assign(builtinRole, dbBuiltin)) } diff --git a/packages/server/src/utilities/security/utilities.js b/packages/server/src/utilities/security/utilities.js new file mode 100644 index 0000000000..9d191b9572 --- /dev/null +++ b/packages/server/src/utilities/security/utilities.js @@ -0,0 +1,70 @@ +const { + PermissionLevels, + PermissionTypes, + getBuiltinPermissionByID, + isPermissionLevelHigherThanRead, +} = require("../../utilities/security/permissions") +const { + lowerBuiltinRoleID, + BUILTIN_ROLES, +} = require("../../utilities/security/roles") +const { DocumentTypes } = require("../../db/utils") + +const CURRENTLY_SUPPORTED_LEVELS = [ + PermissionLevels.WRITE, + PermissionLevels.READ, +] + +exports.getPermissionType = resourceId => { + const docType = Object.values(DocumentTypes).filter(docType => + resourceId.startsWith(docType) + )[0] + switch (docType) { + case DocumentTypes.TABLE: + case DocumentTypes.ROW: + return PermissionTypes.TABLE + case DocumentTypes.AUTOMATION: + return PermissionTypes.AUTOMATION + case DocumentTypes.WEBHOOK: + return PermissionTypes.WEBHOOK + case DocumentTypes.QUERY: + case DocumentTypes.DATASOURCE: + return PermissionTypes.QUERY + default: + // views don't have an ID, will end up here + return PermissionTypes.VIEW + } +} + +/** + * works out the basic permissions based on builtin roles for a resource, using its ID + * @param resourceId + * @returns {{}} + */ +exports.getBasePermissions = resourceId => { + const type = exports.getPermissionType(resourceId) + const permissions = {} + for (let [roleId, role] of Object.entries(BUILTIN_ROLES)) { + if (!role.permissionId) { + continue + } + const perms = getBuiltinPermissionByID(role.permissionId) + const typedPermission = perms.permissions.find(perm => perm.type === type) + if ( + typedPermission && + CURRENTLY_SUPPORTED_LEVELS.indexOf(typedPermission.level) !== -1 + ) { + const level = typedPermission.level + permissions[level] = lowerBuiltinRoleID(permissions[level], roleId) + if (isPermissionLevelHigherThanRead(level)) { + permissions[PermissionLevels.READ] = lowerBuiltinRoleID( + permissions[PermissionLevels.READ], + roleId + ) + } + } + } + return permissions +} + +exports.CURRENTLY_SUPPORTED_LEVELS = CURRENTLY_SUPPORTED_LEVELS From 73169ab9112f61a9d362ee97209bffc5ee78a989 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 12 Feb 2021 12:05:01 +0000 Subject: [PATCH 11/11] Quick CSS change for manage access popover. --- .../DataTable/popovers/ManageAccessPopover.svelte | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte b/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte index fab0ee8fd0..e8f6f6df0b 100644 --- a/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte +++ b/packages/builder/src/components/backend/DataTable/popovers/ManageAccessPopover.svelte @@ -29,7 +29,9 @@
Who Can Access This Data?
- +
+ +
@@ -82,4 +84,9 @@ grid-template-columns: 1fr 1fr; grid-gap: var(--spacing-m); } + + .note { + margin-top: 10px; + margin-bottom: 0; + }