From 33d1af2a1a65d379c792ab7421094b026c251b2c Mon Sep 17 00:00:00 2001 From: Rik Smale <13023439+WikiRik@users.noreply.github.com> Date: Sat, 13 Jan 2024 22:02:51 +0100 Subject: [PATCH] fix(board): ensure that boards can be managed with local permissions (#570) Co-authored-by: WikiRik Co-authored-by: Leon Vreling --- lib/boards.js | 6 +- lib/helpers.js | 30 +++++++++- lib/middlewares.js | 26 ++++++++- test/api/board-creation.test.js | 2 +- test/api/board-deleting.test.js | 2 +- test/api/board-listing.test.js | 26 +++++++++ test/assets/core-manage-permissions-full.json | 55 +++++++++++++++++++ test/scripts/mock.js | 44 +++++++++++++++ 8 files changed, 182 insertions(+), 9 deletions(-) create mode 100644 test/assets/core-manage-permissions-full.json diff --git a/lib/boards.js b/lib/boards.js index 146e4cf2..9339d0b1 100644 --- a/lib/boards.js +++ b/lib/boards.js @@ -10,7 +10,7 @@ const mailer = require('./mailer'); const config = require('../config'); exports.createBoard = async (req, res) => { - if (!req.permissions.manage_boards) { + if (!req.permissions.manage_boards[req.body.body_id] && !req.permissions.manage_boards.global) { return errors.makeForbiddenError(res, 'You are not allowed to create boards.'); } @@ -148,7 +148,7 @@ exports.getBoard = async (req, res) => { }; exports.updateBoard = async (req, res) => { - if (!req.permissions.manage_boards) { + if (!req.permissions.manage_boards[req.board.body_id] && !req.permissions.manage_boards.global) { return errors.makeForbiddenError(res, 'You are not allowed to update boards.'); } @@ -161,7 +161,7 @@ exports.updateBoard = async (req, res) => { }; exports.deleteBoard = async (req, res) => { - if (!req.permissions.manage_boards) { + if (!req.permissions.manage_boards[req.board.body_id] && !req.permissions.manage_boards.global) { return errors.makeForbiddenError(res, 'You are not allowed to delete boards.'); } diff --git a/lib/helpers.js b/lib/helpers.js index d7b19be4..31ac07be 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -34,11 +34,35 @@ function hasPermission(permissionsList, combinedPermission) { return permissionsList.some((permission) => permission.combined.endsWith(combinedPermission)); } -exports.getPermissions = (user, corePermissions) => { - return { - manage_boards: hasPermission(corePermissions, 'manage_network:boards'), +// A helper to get bodies list where I have some permission +// from POST /my_permissions +function getBodiesListFromPermissions(result) { + if (!Array.isArray(result)) { + return []; + } + + return result + .filter((elt) => elt.body_id) + .map((elt) => elt.body_id) + .filter((elt, index, array) => array.indexOf(elt) === index); +} + +exports.getPermissions = (user, corePermissions, managePermissions) => { + const permissions = { view_board: hasPermission(corePermissions, 'view:board') }; + + permissions.manage_boards = { + global: hasPermission(corePermissions, 'global:manage_network:boards') + }; + + const manageBoardsList = getBodiesListFromPermissions(managePermissions); + const userBodies = user && Array.isArray(user.bodies) ? user.bodies : []; + for (const body of userBodies) { + permissions.manage_boards[body.id] = manageBoardsList.includes(body.id); + } + + return permissions; }; // A helper to add data to gauge Prometheus metric. diff --git a/lib/middlewares.js b/lib/middlewares.js index c94a682e..41e7f1ae 100644 --- a/lib/middlewares.js +++ b/lib/middlewares.js @@ -1,7 +1,10 @@ +const request = require('request-promise-native'); + const core = require('./core'); const errors = require('./errors'); const helpers = require('./helpers'); const logger = require('./logger'); +const config = require('../config'); const Bugsnag = require('./bugsnag'); const packageInfo = require('../package.json'); @@ -16,6 +19,26 @@ exports.authenticateUser = async (req, res, next) => { core.getMyPermissions(req) ]); + // Fetching permissions for board management, the list of bodies + // where do you have the 'manage_network:boards' permission for it. + const manageRequest = await request({ + url: config.core.url + ':' + config.core.port + '/my_permissions', + method: 'POST', + headers: { + 'X-Requested-With': 'XMLHttpRequest', + 'X-Auth-Token': req.headers['x-auth-token'], + }, + simple: false, + json: true, + body: { + action: 'manage_network', + object: 'boards' + }, + resolveWithFullResponse: true + }); + + req.manageRequest = manageRequest; + if (typeof userBody !== 'object') { throw new Error('Malformed response when fetching user: ' + userBody); } @@ -38,7 +61,8 @@ exports.authenticateUser = async (req, res, next) => { req.user = userBody.data; req.corePermissions = permissionsBody.data; - req.permissions = helpers.getPermissions(req.user, req.corePermissions); + if (req.manageRequest.body && req.manageRequest.body.success) req.managePermissions = manageRequest.body.data; + req.permissions = helpers.getPermissions(req.user, req.corePermissions, req.managePermissions); return next(); }; diff --git a/test/api/board-creation.test.js b/test/api/board-creation.test.js index 6b66d68a..cb349f75 100644 --- a/test/api/board-creation.test.js +++ b/test/api/board-creation.test.js @@ -223,7 +223,7 @@ describe('Board creation', () => { }); test('should succeed if everything is okay', async () => { - const board = generator.generateBoard(); + const board = generator.generateBoard({}); const res = await request({ uri: '/bodies/' + body.id + '/boards', diff --git a/test/api/board-deleting.test.js b/test/api/board-deleting.test.js index 6ac11509..5795e9c8 100644 --- a/test/api/board-deleting.test.js +++ b/test/api/board-deleting.test.js @@ -38,7 +38,7 @@ describe('Board deleting', () => { }); test('should succeed if everything is okay', async () => { - const board = await generator.createBoard(); + const board = await generator.createBoard({}); const res = await request({ uri: '/bodies/' + board.body_id + '/boards/' + board.id, diff --git a/test/api/board-listing.test.js b/test/api/board-listing.test.js index c3216741..1d56d340 100644 --- a/test/api/board-listing.test.js +++ b/test/api/board-listing.test.js @@ -37,6 +37,32 @@ describe('Board listing', () => { expect(res.body.success).toEqual(false); }); + test('should fail for the current board of body if no permission', async () => { + mock.mockAll({ mainPermissions: { noPermissions: true } }); + + const res = await request({ + uri: '/bodies/1/boards/current', + method: 'GET', + headers: { 'X-Auth-Token': 'blablabla' } + }); + + expect(res.statusCode).toEqual(403); + expect(res.body.success).toEqual(false); + }); + + test('should fail for the current board of body if no permission', async () => { + mock.mockAll({ mainPermissions: { noPermissions: true } }); + + const res = await request({ + uri: '/bodies/1/boards/current', + method: 'GET', + headers: { 'X-Auth-Token': 'blablabla' } + }); + + expect(res.statusCode).toEqual(403); + expect(res.body.success).toEqual(false); + }); + test('should list all boards on / GET', async () => { await generator.createBoard(); await generator.createBoard(); diff --git a/test/assets/core-manage-permissions-full.json b/test/assets/core-manage-permissions-full.json new file mode 100644 index 00000000..041f7681 --- /dev/null +++ b/test/assets/core-manage-permissions-full.json @@ -0,0 +1,55 @@ +{ + "success": true, + "data": [ + { + "seo_url": 134, + "permissions": [], + "parent_circle_id": 2, + "parent_circle": { + "seo_url": 2, + "permissions": null, + "parent_circle_id": null, + "parent_circle": null, + "name": "General board circle", + "joinable": false, + "id": 2, + "description": "This is the toplevel circle for all boards in the system. It grants boards administrative rights for their body.", + "child_circles": null, + "body_id": null, + "body": null + }, + "name": "Board AEGEE-Alicante", + "joinable": false, + "id": 134, + "description": "Board circle of AEGEE-Alicante", + "child_circles": [], + "body_id": 9, + "body": null + }, + { + "seo_url": 134, + "permissions": [], + "parent_circle_id": 2, + "parent_circle": { + "seo_url": 2, + "permissions": null, + "parent_circle_id": null, + "parent_circle": null, + "name": "General board circle", + "joinable": false, + "id": 2, + "description": "This is the toplevel circle for all boards in the system. It grants boards administrative rights for their body.", + "child_circles": null, + "body_id": null, + "body": null + }, + "name": "Board AEGEE-Alicante", + "joinable": false, + "id": 134, + "description": "Board circle of AEGEE-Alicante", + "child_circles": [], + "body_id": 34, + "body": null + } + ] +} \ No newline at end of file diff --git a/test/scripts/mock.js b/test/scripts/mock.js index 35f95a6d..dc577d5c 100644 --- a/test/scripts/mock.js +++ b/test/scripts/mock.js @@ -84,6 +84,48 @@ exports.mockCoreMainPermissions = (options) => { .replyWithFile(200, path.join(__dirname, '..', 'assets', 'core-permissions-full.json')); }; +exports.mockCoreManagePermissions = (options) => { + if (options.netError) { + return nock(`${config.core.url}:${config.core.port}`) + .persist() + .post('/my_permissions') + .replyWithError('Some random error.'); + } + + if (options.badResponse) { + return nock(`${config.core.url}:${config.core.port}`) + .persist() + .post('/my_permissions') + .reply(500, 'Some error happened.'); + } + + if (options.unsuccessfulResponse) { + return nock(`${config.core.url}:${config.core.port}`) + .persist() + .post('/my_permissions') + .reply(500, { success: false, message: 'Some error' }); + } + + if (options.unauthorized) { + return nock(`${config.core.url}:${config.core.port}`) + .persist() + .post('/my_permissions') + .replyWithFile(401, path.join(__dirname, '..', 'assets', 'core-unauthorized.json')); + } + + if (options.noPermissions) { + return nock(`${config.core.url}:${config.core.port}`) + .persist() + .post('/my_permissions') + .replyWithFile(200, path.join(__dirname, '..', 'assets', 'core-empty.json')); + } + + return nock(`${config.core.url}:${config.core.port}`) + .persist() + .post('/my_permissions') + .replyWithFile(200, path.join(__dirname, '..', 'assets', 'core-manage-permissions-full.json')); +}; + exports.mockCoreBody = (options) => { if (options.netError) { return nock(`${config.core.url}:${config.core.port}`) @@ -172,6 +214,7 @@ exports.mockAll = (options = {}) => { nock.cleanAll(); const omsCoreStub = exports.mockCore(options.core || {}); const omsMainPermissionsStub = exports.mockCoreMainPermissions(options.mainPermissions || {}); + const omsManagePermissionsStub = exports.mockCoreManagePermissions(options.managePermissions || {}); const omsCoreBodyStub = exports.mockCoreBody(options.body || {}); const omsCoreMemberStub = exports.mockCoreMember(options.member || {}); const omsMailerStub = exports.mockCoreMailer(options.mailer || {}); @@ -179,6 +222,7 @@ exports.mockAll = (options = {}) => { return { omsCoreStub, omsMainPermissionsStub, + omsManagePermissionsStub, omsCoreBodyStub, omsCoreMemberStub, omsMailerStub