Skip to content

Commit

Permalink
fix(board): ensure that boards can be managed with local permissions (#…
Browse files Browse the repository at this point in the history
…570)

Co-authored-by: WikiRik <WikiRik@users.noreply.github.com>
Co-authored-by: Leon Vreling <l.m.vreling@student.tue.nl>
  • Loading branch information
3 people authored Jan 13, 2024
1 parent 08dab16 commit 33d1af2
Show file tree
Hide file tree
Showing 8 changed files with 182 additions and 9 deletions.
6 changes: 3 additions & 3 deletions lib/boards.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}

Expand Down Expand Up @@ -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.');
}

Expand All @@ -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.');
}

Expand Down
30 changes: 27 additions & 3 deletions lib/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
26 changes: 25 additions & 1 deletion lib/middlewares.js
Original file line number Diff line number Diff line change
@@ -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');

Expand All @@ -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);
}
Expand All @@ -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();
};
Expand Down
2 changes: 1 addition & 1 deletion test/api/board-creation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion test/api/board-deleting.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
26 changes: 26 additions & 0 deletions test/api/board-listing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
55 changes: 55 additions & 0 deletions test/assets/core-manage-permissions-full.json
Original file line number Diff line number Diff line change
@@ -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
}
]
}
44 changes: 44 additions & 0 deletions test/scripts/mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`)
Expand Down Expand Up @@ -172,13 +214,15 @@ 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 || {});

return {
omsCoreStub,
omsMainPermissionsStub,
omsManagePermissionsStub,
omsCoreBodyStub,
omsCoreMemberStub,
omsMailerStub
Expand Down

0 comments on commit 33d1af2

Please sign in to comment.