diff --git a/packages/auth/src/security/permissions.js b/packages/auth/src/security/permissions.js index d0308d783e..3b05c10e20 100644 --- a/packages/auth/src/security/permissions.js +++ b/packages/auth/src/security/permissions.js @@ -131,38 +131,14 @@ exports.getBuiltinPermissionByID = id => { return perms.find(perm => perm._id === id) } -exports.doesHaveResourcePermission = ( - permissions, - permLevel, - { resourceId, subResourceId } -) => { - // set foundSub to not subResourceId, incase there is no subResource - let foundMain = false, - foundSub = false - for (let [resource, levels] of Object.entries(permissions)) { - if (resource === resourceId && levels.indexOf(permLevel) !== -1) { - foundMain = true - } - if ( - subResourceId && - resource === subResourceId && - levels.indexOf(permLevel) !== -1 - ) { - foundSub = true - } - // this will escape if foundMain only when no sub resource - if (foundMain && foundSub) { - break - } - } - return foundMain || foundSub -} - -exports.doesHaveBasePermission = (permType, permLevel, permissionIds) => { +exports.doesHaveBasePermission = (permType, permLevel, rolesHierarchy) => { + const basePermissions = [ + ...new Set(rolesHierarchy.map(role => role.permissionId)), + ] const builtins = Object.values(BUILTIN_PERMISSIONS) let permissions = flatten( builtins - .filter(builtin => permissionIds.indexOf(builtin._id) !== -1) + .filter(builtin => basePermissions.indexOf(builtin._id) !== -1) .map(builtin => builtin.permissions) ) for (let permission of permissions) { diff --git a/packages/auth/src/security/roles.js b/packages/auth/src/security/roles.js index 71fbc10132..8529dde6f4 100644 --- a/packages/auth/src/security/roles.js +++ b/packages/auth/src/security/roles.js @@ -170,47 +170,18 @@ async function getAllUserRoles(appId, userRoleId) { * to determine if a user can access something that requires a specific role. * @param {string} appId The ID of the application from which roles should be obtained. * @param {string} userRoleId The user's role ID, this can be found in their access token. + * @param {object} opts Various options, such as whether to only retrieve the IDs (default true). * @returns {Promise} returns an ordered array of the roles, with the first being their * highest level of access and the last being the lowest level. */ -exports.getUserRoleHierarchy = async (appId, userRoleId) => { +exports.getUserRoleHierarchy = async ( + appId, + userRoleId, + opts = { idOnly: true } +) => { // special case, if they don't have a role then they are a public user - return (await getAllUserRoles(appId, userRoleId)).map(role => role._id) -} - -/** - * Get all of the user permissions which could be found across the role hierarchy - * @param appId The ID of the application from which roles should be obtained. - * @param userRoleId The user's role ID, this can be found in their access token. - * @returns {Promise<{basePermissions: string[], permissions: Object}>} the base - * permission IDs as well as any custom resource permissions. - */ -exports.getUserPermissions = async (appId, userRoleId) => { - const rolesHierarchy = await getAllUserRoles(appId, userRoleId) - const basePermissions = [ - ...new Set(rolesHierarchy.map(role => role.permissionId)), - ] - const permissions = {} - for (let role of rolesHierarchy) { - if (role.permissions) { - for (let [resource, levels] of Object.entries(role.permissions)) { - if (!permissions[resource]) { - permissions[resource] = [] - } - const permsSet = new Set(permissions[resource]) - if (Array.isArray(levels)) { - levels.forEach(level => permsSet.add(level)) - } else { - permsSet.add(levels) - } - permissions[resource] = [...permsSet] - } - } - } - return { - basePermissions, - permissions, - } + const roles = await getAllUserRoles(appId, userRoleId) + return opts.idOnly ? roles.map(role => role._id) : roles } /** @@ -246,6 +217,39 @@ exports.getAllRoles = async appId => { return roles } +/** + * This retrieves the required role/ + * @param appId + * @param permLevel + * @param resourceId + * @param subResourceId + * @return {Promise<{permissions}|Object>} + */ +exports.getRequiredResourceRole = async ( + appId, + permLevel, + { resourceId, subResourceId } +) => { + const roles = await exports.getAllRoles(appId) + let main = [], + sub = [] + for (let role of roles) { + // no permissions, ignore it + if (!role.permissions) { + continue + } + const mainRes = role.permissions[resourceId] + const subRes = role.permissions[subResourceId] + if (mainRes && mainRes.indexOf(permLevel) !== -1) { + main.push(role._id) + } else if (subRes && subRes.indexOf(permLevel) !== -1) { + sub.push(role._id) + } + } + // for now just return the IDs + return main.concat(sub) +} + class AccessController { constructor(appId) { this.appId = appId diff --git a/packages/server/src/middleware/authorized.js b/packages/server/src/middleware/authorized.js index d91311e165..4e9c2c9ef8 100644 --- a/packages/server/src/middleware/authorized.js +++ b/packages/server/src/middleware/authorized.js @@ -1,7 +1,10 @@ -const { getUserPermissions } = require("@budibase/auth/roles") +const { + getUserRoleHierarchy, + getRequiredResourceRole, + BUILTIN_ROLE_IDS, +} = require("@budibase/auth/roles") const { PermissionTypes, - doesHaveResourcePermission, doesHaveBasePermission, } = require("@budibase/auth/permissions") const builderMiddleware = require("./builder") @@ -28,13 +31,7 @@ module.exports = await builderMiddleware(ctx, permType) const isAuthed = ctx.isAuthenticated - const { basePermissions, permissions } = await getUserPermissions( - ctx.appId, - ctx.roleId - ) - // builders for now have permission to do anything - // TODO: in future should consider separating permissions with an require("@budibase/auth").isClient check let isBuilder = ctx.user && ctx.user.builder && ctx.user.builder.global const isBuilderApi = permType === PermissionTypes.BUILDER if (isBuilder) { @@ -43,20 +40,30 @@ module.exports = return ctx.throw(403, "Not Authorized") } - if ( - hasResource(ctx) && - doesHaveResourcePermission(permissions, permLevel, ctx) - ) { - return next() + // need to check this first, in-case public access, don't check authed until last + const roleId = ctx.roleId || BUILTIN_ROLE_IDS.PUBLIC + const hierarchy = await getUserRoleHierarchy(ctx.appId, roleId, { + idOnly: false, + }) + const permError = "User does not have permission" + let possibleRoleIds = [] + if (hasResource(ctx)) { + possibleRoleIds = await getRequiredResourceRole(ctx.appId, permLevel, ctx) + } + // check if we found a role, if not fallback to base permissions + if (possibleRoleIds.length > 0) { + const found = hierarchy.find( + role => possibleRoleIds.indexOf(role._id) !== -1 + ) + return found ? next() : ctx.throw(403, permError) + } else if (!doesHaveBasePermission(permType, permLevel, hierarchy)) { + ctx.throw(403, permError) } + // if they are not authed, then anything using the authorized middleware will fail if (!isAuthed) { ctx.throw(403, "Session not authenticated") } - if (!doesHaveBasePermission(permType, permLevel, basePermissions)) { - ctx.throw(403, "User does not have permission") - } - return next() } diff --git a/packages/server/src/middleware/tests/authorized.spec.js b/packages/server/src/middleware/tests/authorized.spec.js index 3c3bf9bb36..d7052b323b 100644 --- a/packages/server/src/middleware/tests/authorized.spec.js +++ b/packages/server/src/middleware/tests/authorized.spec.js @@ -10,6 +10,7 @@ jest.mock("../../environment", () => ({ const authorizedMiddleware = require("../authorized") const env = require("../../environment") const { PermissionTypes, PermissionLevels } = require("@budibase/auth/permissions") +require("@budibase/auth").init(require("../../db")) class TestConfiguration { constructor(role) { @@ -21,6 +22,7 @@ class TestConfiguration { request: { url: "" }, + appId: "", auth: {}, next: this.next, throw: this.throw