From 571380e243aa93a540f52d25407679b49549a3a2 Mon Sep 17 00:00:00 2001 From: Thomas Lynch Date: Fri, 20 Jan 2023 18:40:17 +1100 Subject: [PATCH 1/7] remove applying inheritance on setting, match editstaff/editaccount --- models/forms/editrole.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/forms/editrole.js b/models/forms/editrole.js index 8cf40a0f..f3c95541 100644 --- a/models/forms/editrole.js +++ b/models/forms/editrole.js @@ -10,7 +10,7 @@ module.exports = async (req, res) => { let rolePermissions = new Permission(res.locals.editingRole.permissions); rolePermissions.handleBody(req.body, res.locals.permissions); - rolePermissions.applyInheritance(); + // rolePermissions.applyInheritance(); const existingRoleName = roleManager.roleNameMap[rolePermissions.base64]; if (existingRoleName) { From d68a32b8310e59eff751c42efdafa9158d151961 Mon Sep 17 00:00:00 2001 From: Thomas Lynch Date: Fri, 20 Jan 2023 18:57:27 +1100 Subject: [PATCH 2/7] Add some additional improved permission.js tests, and null check rather than !Metadata in permission handleBody (because else 0 would be true) --- lib/permission/permission.js | 2 +- lib/permission/permission.test.js | 24 +++++++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/lib/permission/permission.js b/lib/permission/permission.js index 4d6a9427..6459bdd4 100644 --- a/lib/permission/permission.js +++ b/lib/permission/permission.js @@ -30,7 +30,7 @@ class Permission extends BigBitfield { const handlingBits = boardOnly ? Permissions._MANAGE_BOARD_BITS : Object.keys(Metadata); for (let bit of handlingBits) { // If perm has no "parent" bit, or current user has the parent permission, set each bit based on the form input - const allowedParent = !Metadata[bit].parent + const allowedParent = Metadata[bit].parent == null || editorPermission.get(Metadata[bit].parent); if (allowedParent && !Metadata[bit].block) { this.set(parseInt(bit), (body[`permission_bit_${bit}`] != null)); diff --git a/lib/permission/permission.test.js b/lib/permission/permission.test.js index 401fa4f9..1e2e459d 100644 --- a/lib/permission/permission.test.js +++ b/lib/permission/permission.test.js @@ -67,6 +67,28 @@ describe('testing permissions', () => { expect(Permission.allPermissions.every(b => NO_PERMISSION.get(b))).toBe(true); }); - //todo: what othe rpermissions test should be added? + test('handleBody() by somebody with editorPermission NOT having Permissions.ROOT cannot set Permissions.ROOT', () => { + const TEST_PERMISSION = new Permission(); + TEST_PERMISSION.handleBody({ + 'permission_bit_0': 0, + }, ANON); + expect(TEST_PERMISSION.get(0)).toBe(false); + }); + + test('handleBody() by somebody with editorPermission having Permissions.ROOT CAN set Permissions.ROOT', () => { + const TEST_PERMISSION = new Permission(); + TEST_PERMISSION.handleBody({ + 'permission_bit_0': 0, + }, ROOT); + expect(TEST_PERMISSION.get(0)).toBe(true); + }); + + test('handleBody() does not allow setting permission outside of _MANAGE_BOARD_BITS when boardOnly=true, even with permission', () => { + const TEST_PERMISSION = new Permission(); + TEST_PERMISSION.handleBody({ + 'permission_bit_0': 0, + }, ROOT, true); + expect(TEST_PERMISSION.get(0)).toBe(false); + }); }); From f9672375b3646a02926fbc867419b96183451518 Mon Sep 17 00:00:00 2001 From: Thomas Lynch Date: Fri, 20 Jan 2023 18:59:26 +1100 Subject: [PATCH 3/7] update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10dc8681..12a19768 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ### 0.11.2 - Convert the assets page form handling to the newer checkSchema code. - Don't show the "Edit" option in the post dropdowns for public pages. + - Improve duplicate checking when editing roles to only explicitly match updated roles rather than applying inheritance first. - Bugfix "my permission" page not displaying correctly and board staff permission editing not applying. - Improve required parent permission display to show "requires X" in the tooltip of disabled checkboxes. From ef4de0fb07a79f10c83319b87e170cd9fa204221 Mon Sep 17 00:00:00 2001 From: Thomas Lynch Date: Fri, 20 Jan 2023 19:27:45 +1100 Subject: [PATCH 4/7] editAccount no longer allows account editors to apply roles to users with root permission --- controllers/forms/editaccount.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/controllers/forms/editaccount.js b/controllers/forms/editaccount.js index e315063e..b548d3dd 100644 --- a/controllers/forms/editaccount.js +++ b/controllers/forms/editaccount.js @@ -5,6 +5,8 @@ const editAccount = require(__dirname+'/../../models/forms/editaccount.js') , dynamicResponse = require(__dirname+'/../../lib/misc/dynamic.js') , paramConverter = require(__dirname+'/../../lib/middleware/input/paramconverter.js') , roleManager = require(__dirname+'/../../lib/permission/rolemanager.js') + , { Permissions } = require(__dirname+'/../../lib/permission/permissions.js') + , Permission = require(__dirname+'/../../lib/permission/permission.js') , { alphaNumericRegex, checkSchema, lengthBody, inArrayBody, existsBody } = require(__dirname+'/../../lib/input/schema.js'); module.exports = { @@ -22,12 +24,21 @@ module.exports = { { result: async () => { res.locals.editingAccount = await Accounts.findOne(req.body.username); return res.locals.editingAccount != null; - }, expected: true, error: 'Invalid account username' }, + }, expected: true, blocking: true, error: 'Invalid account username' }, { result: (res.locals.user.username === req.body.username), expected: false, error: 'You can\'t edit your own permissions' }, { result: !existsBody(req.body.template) //no template, OR the template is a valid one || inArrayBody(req.body.template, [roleManager.roles.ANON.base64, roleManager.roles.GLOBAL_STAFF.base64, roleManager.roles.ADMIN.base64, roleManager.roles.BOARD_STAFF.base64, roleManager.roles.BOARD_OWNER.base64]), expected: true, error: 'Invalid template selection' }, + { result: () => { + //not applying a template, OR the user doesn't have root perms, has to be a function to execute after the async result above. + if (!existsBody(req.body.template)) { + return true; + } + const editingPermission = new Permission(res.locals.editingAccount.permissions); + return !editingPermission.get(Permissions.ROOT); + }, + expected: true, error: 'You can\'t apply template permissions to a ROOT user.' }, ]); if (errors.length > 0) { From cc2291616577ab4637fa5323790f96c292ad27cf Mon Sep 17 00:00:00 2001 From: Thomas Lynch Date: Fri, 20 Jan 2023 19:28:11 +1100 Subject: [PATCH 5/7] gulp password task now sets admin as root --- gulpfile.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gulpfile.js b/gulpfile.js index 98145cc1..c401d778 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -143,6 +143,9 @@ async function password() { const { Accounts } = require(__dirname+'/db/'); const randomPassword = randomBytes(20).toString('base64'); await Accounts.changePassword('admin', randomPassword); + const ROOT = new Permission(); + ROOT.setAll(Permission.allPermissions); + await Accounts.setAccountPermissions('admin', ROOT); console.log('=====LOGIN DETAILS=====\nusername: admin\npassword:', randomPassword, '\n======================='); } From 262954c41c9449023e32c6f016dc51a66589bc5d Mon Sep 17 00:00:00 2001 From: Thomas Lynch Date: Fri, 20 Jan 2023 19:47:43 +1100 Subject: [PATCH 6/7] remove comment todo that is already done --- lib/permission/permissions.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/permission/permissions.js b/lib/permission/permissions.js index 0a1aaefc..ec89db5a 100644 --- a/lib/permission/permissions.js +++ b/lib/permission/permissions.js @@ -50,7 +50,6 @@ const Permissions = Object.seal(Object.freeze(Object.preventExtensions({ }))); -//todo: make these keyed by the bits? but then how to get the name param for form fields? might change that const Metadata = Object.seal(Object.freeze(Object.preventExtensions({ [Permissions.ROOT]: { title: 'Root', label: 'Root', desc: 'Full control. Use with caution!', parent: Permissions.ROOT }, From f16de9a3f320125c6135d07235832fcaf2c9d3ed Mon Sep 17 00:00:00 2001 From: Thomas Lynch Date: Fri, 20 Jan 2023 19:51:13 +1100 Subject: [PATCH 7/7] update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12a19768..95166d43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ### 0.11.2 - Convert the assets page form handling to the newer checkSchema code. - Don't show the "Edit" option in the post dropdowns for public pages. + - No longer apply permissions inheritance after editing to prevent confusion. - Improve duplicate checking when editing roles to only explicitly match updated roles rather than applying inheritance first. - Bugfix "my permission" page not displaying correctly and board staff permission editing not applying. - Improve required parent permission display to show "requires X" in the tooltip of disabled checkboxes.