Fixing issues with builtin roles living in the database as well as in code (easier to change in the future this way) - discovered by basic test case.

This commit is contained in:
mike12345567 2021-02-09 13:01:45 +00:00
parent c0aaaf0046
commit cc02397b01
9 changed files with 129 additions and 47 deletions

View File

@ -3,8 +3,15 @@ const {
PermissionLevels,
higherPermission,
} = require("../../utilities/security/permissions")
const {
isBuiltin,
getDBRoleID,
getExternalRoleID,
BUILTIN_ROLES,
} = require("../../utilities/security/roles")
const { getRoleParams } = require("../../db/utils")
const CouchDB = require("../../db")
const { cloneDeep } = require("lodash/fp")
const PermissionUpdateType = {
REMOVE: "remove",
@ -18,6 +25,8 @@ async function updatePermissionOnRole(
) {
const db = new CouchDB(appId)
const remove = updateType === PermissionUpdateType.REMOVE
const isABuiltin = isBuiltin(roleId)
const dbRoleId = getDBRoleID(roleId)
const body = await db.allDocs(
getRoleParams(null, {
include_docs: true,
@ -26,7 +35,12 @@ async function updatePermissionOnRole(
const dbRoles = body.rows.map(row => row.doc)
const docUpdates = []
// TODO NEED TO HANDLE BUILTINS HERE - THE dbRoles doesn't contain them
// 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])
builtin._id = getDBRoleID(builtin._id)
dbRoles.push(builtin)
}
// now try to find any roles which need updated, e.g. removing the
// resource from another role and then adding to the new role
@ -34,18 +48,18 @@ async function updatePermissionOnRole(
let updated = false
const rolePermissions = role.permissions ? role.permissions : {}
// handle the removal/updating the role which has this permission first
// the updating (role._id !== roleId) is required because a resource/level can
// the updating (role._id !== dbRoleId) is required because a resource/level can
// only be permitted in a single role (this reduces hierarchy confusion and simplifies
// the general UI for this, rather than needing to show everywhere it is used)
if (
(role._id !== roleId || remove) &&
(role._id !== dbRoleId || remove) &&
rolePermissions[resourceId] === level
) {
delete rolePermissions[resourceId]
updated = true
}
// handle the adding, we're on the correct role, at it to this
if (!remove && role._id === roleId) {
if (!remove && role._id === dbRoleId) {
rolePermissions[resourceId] = level
updated = true
}
@ -56,7 +70,11 @@ async function updatePermissionOnRole(
}
}
return db.bulkDocs(docUpdates)
const response = await db.bulkDocs(docUpdates)
return response.map(resp => ({
...resp,
_id: getExternalRoleID(resp._id),
}))
}
exports.fetchBuiltin = function(ctx) {
@ -81,8 +99,9 @@ exports.getResourcePerms = async function(ctx) {
for (let role of roles) {
// update the various roleIds in the resource permissions
if (role.permissions && role.permissions[resourceId]) {
resourcePerms[role._id] = higherPermission(
resourcePerms[role._id],
const roleId = getExternalRoleID(role._id)
resourcePerms[roleId] = higherPermission(
resourcePerms[roleId],
role.permissions[resourceId]
)
}

View File

@ -1,8 +1,11 @@
const CouchDB = require("../../db")
const {
BUILTIN_ROLES,
BUILTIN_ROLE_IDS,
Role,
getRole,
isBuiltin,
getExternalRoleID,
} = require("../../utilities/security/roles")
const {
generateRoleID,
@ -16,6 +19,14 @@ const UpdateRolesOptions = {
REMOVED: "removed",
}
// exclude internal roles like builder
const EXTERNAL_BUILTIN_ROLE_IDS = [
BUILTIN_ROLE_IDS.ADMIN,
BUILTIN_ROLE_IDS.POWER,
BUILTIN_ROLE_IDS.BASIC,
BUILTIN_ROLE_IDS.PUBLIC,
]
async function updateRolesOnUserTable(db, roleId, updateOption) {
const table = await db.get(ViewNames.USERS)
const schema = table.schema
@ -46,16 +57,22 @@ exports.fetch = async function(ctx) {
include_docs: true,
})
)
const customRoles = body.rows.map(row => row.doc)
const roles = body.rows.map(row => row.doc)
// exclude internal roles like builder
const staticRoles = [
BUILTIN_ROLES.ADMIN,
BUILTIN_ROLES.POWER,
BUILTIN_ROLES.BASIC,
BUILTIN_ROLES.PUBLIC,
]
ctx.body = [...staticRoles, ...customRoles]
// 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 dbBuiltin = roles.filter(
dbRole => getExternalRoleID(dbRole._id) === builtinRoleId
)[0]
if (dbBuiltin == null) {
roles.push(builtinRole)
} else {
dbBuiltin._id = getExternalRoleID(dbBuiltin._id)
roles.push(Object.assign(builtinRole, dbBuiltin))
}
}
ctx.body = roles
}
exports.find = async function(ctx) {
@ -67,6 +84,8 @@ exports.save = async function(ctx) {
let { _id, name, inherits, permissionId } = ctx.request.body
if (!_id) {
_id = generateRoleID()
} else if (isBuiltin(_id)) {
ctx.throw(400, "Cannot update builtin roles.")
}
const role = new Role(_id, name)
.addPermission(permissionId)
@ -84,6 +103,9 @@ exports.save = async function(ctx) {
exports.destroy = async function(ctx) {
const db = new CouchDB(ctx.user.appId)
const roleId = ctx.params.roleId
if (isBuiltin(roleId)) {
ctx.throw(400, "Cannot delete builtin role.")
}
// first check no users actively attached to role
const users = (
await db.allDocs(
@ -94,7 +116,7 @@ exports.destroy = async function(ctx) {
).rows.map(row => row.doc)
const usersWithRole = users.filter(user => user.roleId === roleId)
if (usersWithRole.length !== 0) {
ctx.throw("Cannot delete role when it is in use.")
ctx.throw(400, "Cannot delete role when it is in use.")
}
await db.remove(roleId, ctx.params.rev)

View File

@ -88,6 +88,21 @@ exports.createRole = async (request, appId) => {
return res.body
}
exports.addPermission = async (
request,
appId,
role,
resource,
level = "read"
) => {
const res = await request
.post(`/api/permission/${role}/${resource}/${level}`)
.set(exports.defaultHeaders(appId))
.expect("Content-Type", /json/)
.expect(200)
return res.body
}
exports.createLinkedTable = async (request, appId) => {
// get the ID to link to
const table = await exports.createTable(request, appId)

View File

@ -3,16 +3,18 @@ const {
createTable,
supertest,
defaultHeaders,
addPermission,
} = require("./couchTestUtils")
const { BUILTIN_ROLE_IDS } = require("../../../utilities/security/roles")
const STD_ROLE_ID = BUILTIN_ROLE_IDS.BASIC
const STD_ROLE_ID = BUILTIN_ROLE_IDS.PUBLIC
describe("/permission", () => {
let server
let request
let appId
let table
let perms
beforeAll(async () => {
;({ request, server } = await supertest())
@ -26,6 +28,7 @@ describe("/permission", () => {
let app = await createApplication(request)
appId = app.instance._id
table = await createTable(request, appId)
perms = await addPermission(request, appId, STD_ROLE_ID, table._id)
})
describe("levels", () => {
@ -42,10 +45,15 @@ describe("/permission", () => {
})
})
describe("add", () => {
describe("test", () => {
it("should be able to add permission to a role for the table", async () => {
expect(perms.length).toEqual(1)
expect(perms[0].id).toEqual(`${STD_ROLE_ID}`)
})
it("should get the resource permissions", async () => {
const res = await request
.post(`/api/permission/${STD_ROLE_ID}/${table._id}/read`)
.get(`/api/permission/${table._id}`)
.set(defaultHeaders(appId))
.expect("Content-Type", /json/)
.expect(200)

View File

@ -1,7 +1,5 @@
const {
createApplication,
createTable,
createView,
supertest,
defaultHeaders,
} = require("./couchTestUtils")
@ -20,8 +18,6 @@ describe("/roles", () => {
let server
let request
let appId
let table
let view
beforeAll(async () => {
;({ request, server } = await supertest())
@ -34,8 +30,6 @@ describe("/roles", () => {
beforeEach(async () => {
let app = await createApplication(request)
appId = app.instance._id
table = await createTable(request, appId)
view = await createView(request, appId, table._id)
})
describe("create", () => {

View File

@ -170,8 +170,8 @@ exports.getAppParams = (appId = null, otherProps = {}) => {
* Generates a new role ID.
* @returns {string} The new role ID which the role doc can be stored under.
*/
exports.generateRoleID = () => {
return `${DocumentTypes.ROLE}${SEPARATOR}${newid()}`
exports.generateRoleID = id => {
return `${DocumentTypes.ROLE}${SEPARATOR}${id || newid()}`
}
/**

View File

@ -64,12 +64,12 @@ module.exports = (permType, permLevel = null) => async (ctx, next) => {
ctx.throw(403, "Not Authorized")
}
if (
hasResource(ctx) &&
doesHaveResourcePermission(permissions, permLevel, ctx)
) {
return next()
}
// if (
// hasResource(ctx) &&
// doesHaveResourcePermission(permissions, permLevel, ctx)
// ) {
// return next()
// }
if (!doesHaveBasePermission(permType, permLevel, basePermissions)) {
ctx.throw(403, "User does not have permission")

View File

@ -21,13 +21,17 @@ class ResourceIdGetter {
main = this.main,
sub = this.sub
return (ctx, next) => {
if (main != null && ctx.request[parameter][main]) {
ctx.resourceId = ctx.request[parameter][main]
const request = ctx.request[parameter] || ctx[parameter]
if (request == null) {
return next()
}
if (sub != null && ctx.request[parameter][sub]) {
ctx.subResourceId = ctx.request[parameter][sub]
if (main != null && request[main]) {
ctx.resourceId = request[main]
}
next()
if (sub != null && request[sub]) {
ctx.subResourceId = request[sub]
}
return next()
}
}
}

View File

@ -1,10 +1,7 @@
const CouchDB = require("../../db")
const { cloneDeep } = require("lodash/fp")
const {
BUILTIN_PERMISSION_IDS,
higherPermission,
doesHaveBasePermission,
} = require("./permissions")
const { BUILTIN_PERMISSION_IDS, higherPermission } = require("./permissions")
const { generateRoleID, DocumentTypes, SEPARATOR } = require("../../db/utils")
const BUILTIN_IDS = {
ADMIN: "ADMIN",
@ -48,15 +45,15 @@ exports.BUILTIN_ROLES = {
}
exports.BUILTIN_ROLE_ID_ARRAY = Object.values(exports.BUILTIN_ROLES).map(
level => level._id
role => role._id
)
exports.BUILTIN_ROLE_NAME_ARRAY = Object.values(exports.BUILTIN_ROLES).map(
level => level.name
role => role.name
)
function isBuiltin(role) {
return exports.BUILTIN_ROLE_ID_ARRAY.indexOf(role) !== -1
return exports.BUILTIN_ROLE_ID_ARRAY.some(builtin => role.includes(builtin))
}
/**
@ -82,6 +79,8 @@ exports.getRole = async (appId, roleId) => {
const db = new CouchDB(appId)
const dbRole = await db.get(roleId)
role = Object.assign(role, dbRole)
// finalise the ID
role._id = exports.getExternalRoleID(role._id)
} catch (err) {
// only throw an error if there is no role at all
if (Object.keys(role).length === 0) {
@ -202,6 +201,27 @@ class AccessController {
}
}
/**
* Adds the "role_" for builtin role IDs which are to be written to the DB (for permissions).
*/
exports.getDBRoleID = roleId => {
if (roleId.startsWith(DocumentTypes.ROLE)) {
return roleId
}
return generateRoleID(roleId)
}
/**
* Remove the "role_" from builtin role IDs that have been written to the DB (for permissions).
*/
exports.getExternalRoleID = roleId => {
// for built in roles we want to remove the DB role ID element (role_)
if (roleId.startsWith(DocumentTypes.ROLE) && isBuiltin(roleId)) {
return roleId.split(`${DocumentTypes.ROLE}${SEPARATOR}`)[1]
}
return roleId
}
exports.AccessController = AccessController
exports.BUILTIN_ROLE_IDS = BUILTIN_IDS
exports.isBuiltin = isBuiltin