From 4da377f426beca8cf0c1c22d07dd0b8f541aac17 Mon Sep 17 00:00:00 2001 From: kiswa Date: Sat, 28 May 2016 12:29:36 +0000 Subject: [PATCH] Continued route security updates and tests --- src/api/controllers/AutoActions.php | 36 +++++-- src/api/controllers/Columns.php | 49 +++++++-- src/api/index.php | 28 ++--- test/api/controllers/AutoActionsTest.php | 109 ++++++++++++++----- test/api/controllers/BoardsTest.php | 1 + test/api/controllers/ColumnsTest.php | 127 ++++++++++++++++++----- 6 files changed, 268 insertions(+), 82 deletions(-) diff --git a/src/api/controllers/AutoActions.php b/src/api/controllers/AutoActions.php index ef3088e..4057b1f 100644 --- a/src/api/controllers/AutoActions.php +++ b/src/api/controllers/AutoActions.php @@ -4,8 +4,16 @@ use RedBeanPHP\R; class AutoActions extends BaseController { public function getAllActions($request, $response, $args) { + $status = $this->secureRoute($request, $response, + SecurityLevel::User); + if ($status !== 200) { + return $this->jsonResponse($response, $status); + } + $actionBeans = R::findAll('auto_action'); + // TODO: Filter by boards user has access to + if(count($actionBeans)) { $this->apiJson->setSuccess(); @@ -25,9 +33,18 @@ class AutoActions extends BaseController { } public function addAction($request, $response, $args) { + $status = $this->secureRoute($request, $response, + SecurityLevel::BoardAdmin); + if ($status !== 200) { + return $this->jsonResponse($response, $status); + } + $action = new AutoAction($this->container); $action->loadFromJson($request->getBody()); + $actor = new User($this->container, Auth::GetUserId($request)); + // TODO: Verify BoardAdmin has board access + if (!$action->save()) { $this->logger->addError('Add Action: ', [$action]); $this->apiJson->addAlert('error', @@ -37,9 +54,8 @@ class AutoActions extends BaseController { return $this->jsonResponse($response); } - // TODO: Get existing user to log user_id and name - $this->dbLogger->logChange($this->container, 0, - '$user->name added automatic action.', + $this->dbLogger->logChange($this->container, $actor->id, + $actor->username . ' added automatic action.', '', json_encode($action), 'action', $action->id); $this->apiJson->setSuccess(); @@ -49,9 +65,18 @@ class AutoActions extends BaseController { } public function removeAction($request, $response, $args) { + $status = $this->secureRoute($request, $response, + SecurityLevel::BoardAdmin); + if ($status !== 200) { + return $this->jsonResponse($response, $status); + } + $id = (int)$args['id']; $action = new AutoAction($this->container, $id); + $actor = new User($this->container, Auth::GetUserId($request)); + // TODO: Verify BoardAdmin has board access + if($action->id !== $id) { $this->logger->addError('Remove Action: ', [$action]); $this->apiJson->addAlert('error', 'Error removing action. ' . @@ -63,9 +88,8 @@ class AutoActions extends BaseController { $before = $action; $action->delete(); - // TODO: Get existing user to log user_id and name - $this->dbLogger->logChange($this->container, 0, - '$user->name removed action ' . $before->id, + $this->dbLogger->logChange($this->container, $actor->id, + $actor->username .' removed action ' . $before->id . '.', json_encode($before), '', 'action', $id); $this->apiJson->setSuccess(); diff --git a/src/api/controllers/Columns.php b/src/api/controllers/Columns.php index 77cfb54..f71461b 100644 --- a/src/api/controllers/Columns.php +++ b/src/api/controllers/Columns.php @@ -4,7 +4,14 @@ use RedBeanPHP\R; class Columns extends BaseController { public function getColumn($request, $response, $args) { + $status = $this->secureRoute($request, $response, + SecurityLevel::User); + if ($status !== 200) { + return $this->jsonResponse($response, $status); + } + $column = new Column($this->container, (int)$args['id']); + // TODO: Verify user has board access if ($column->id === 0) { $this->logger->addError('Attempt to load column ' . @@ -22,6 +29,15 @@ class Columns extends BaseController { } public function addColumn($request, $response, $args) { + $status = $this->secureRoute($request, $response, + SecurityLevel::BoardAdmin); + if ($status !== 200) { + return $this->jsonResponse($response, $status); + } + + // TODO: Verify user has board access + $actor = new User($this->container, Auth::GetUserId($request)); + $column = new Column($this->container); $column->loadFromJson($request->getBody()); @@ -33,9 +49,8 @@ class Columns extends BaseController { return $this->jsonResponse($response); } - // TODO: Get existing user to log user_id and name - $this->dbLogger->logChange($this->container, 0, - '$user->name added column ' . $column->name . '.', + $this->dbLogger->logChange($this->container, $actor->id, + $actor->username . ' added column ' . $column->name . '.', '', json_encode($column), 'column', $column->id); $this->apiJson->setSuccess(); @@ -46,6 +61,15 @@ class Columns extends BaseController { } public function updateColumn($request, $response, $args) { + $status = $this->secureRoute($request, $response, + SecurityLevel::BoardAdmin); + if ($status !== 200) { + return $this->jsonResponse($response, $status); + } + + // TODO: Verify user has board access + $actor = new User($this->container, Auth::GetUserId($request)); + $column = new Column($this->container, (int)$args['id']); $update = new Column($this->container); $update->loadFromJson($request->getBody()); @@ -60,9 +84,8 @@ class Columns extends BaseController { $update->save(); - // TODO: Get existing user to log user_id and name - $this->dbLogger->logChange($this->container, 0, - '$user->name updated column ' . $update->name, + $this->dbLogger->logChange($this->container, $actor->id, + $actor->username . ' updated column ' . $update->name, json_encode($column), json_encode($update), 'column', $update->id); @@ -74,6 +97,15 @@ class Columns extends BaseController { } public function removeColumn($request, $response, $args) { + $status = $this->secureRoute($request, $response, + SecurityLevel::BoardAdmin); + if ($status !== 200) { + return $this->jsonResponse($response, $status); + } + + // TODO: Verify user has board access + $actor = new User($this->container, Auth::GetUserId($request)); + $id = (int)$args['id']; $column = new Column($this->container, $id); @@ -88,9 +120,8 @@ class Columns extends BaseController { $before = $column; $column->delete(); - // TODO: Get existing user to log user_id and name - $this->dbLogger->logChange($this->container, 0, - '$user->name removed column ' . $before->name, + $this->dbLogger->logChange($this->container, $actor->id, + $actor->username . ' removed column ' . $before->name, json_encode($before), '', 'column', $id); $this->apiJson->setSuccess(); diff --git a/src/api/index.php b/src/api/index.php index 7f4eee3..8a603b8 100644 --- a/src/api/index.php +++ b/src/api/index.php @@ -25,23 +25,23 @@ $app->post ('/autoactions', 'AutoActions:addAction'); // BoardAdmi $app->delete('/autoactions/{id}', 'AutoActions:removeAction'); // BoardAdmin (with board access) $app->get ('/columns/{id}', 'Columns:getColumn'); // User (with board access) -$app->post ('/columns', 'Columns:addColumn'); // BoardAdmin -$app->post ('/columns/{id}', 'Columns:updateColumn'); // BoardAdmin -$app->delete('/columns/{id}', 'Columns:removeColumn'); // BoardAdmin +$app->post ('/columns', 'Columns:addColumn'); // BoardAdmin (with board access) +$app->post ('/columns/{id}', 'Columns:updateColumn'); // BoardAdmin (with board access) +$app->delete('/columns/{id}', 'Columns:removeColumn'); // BoardAdmin (with board access) -$app->get ('/tasks/{id}', 'Tasks:getTask'); // User -$app->post ('/tasks', 'Tasks:addTask'); // User -$app->post ('/tasks/{id}', 'Tasks:updateTask'); // User -$app->delete('/tasks/{id}', 'Tasks:removeTask'); // User +$app->get ('/tasks/{id}', 'Tasks:getTask'); // User (with board access) +$app->post ('/tasks', 'Tasks:addTask'); // User (with board access) +$app->post ('/tasks/{id}', 'Tasks:updateTask'); // User (with board access) +$app->delete('/tasks/{id}', 'Tasks:removeTask'); // User (with board access) -$app->get ('/comments/{id}', 'Comments:getComment'); // User -$app->post ('/comments', 'Comments:addComment'); // User -$app->post ('/comments/{id}', 'Comments:updateComment'); // BoardAdmin or submitter -$app->delete('/comments/{id}', 'Comments:removeComment'); // BoardAdmin or submitter +$app->get ('/comments/{id}', 'Comments:getComment'); // User (with board access) +$app->post ('/comments', 'Comments:addComment'); // User (with board access) +$app->post ('/comments/{id}', 'Comments:updateComment'); // BoardAdmin or submitter (with board access) +$app->delete('/comments/{id}', 'Comments:removeComment'); // BoardAdmin or submitter (with board access) -$app->get ('/attachments/{id}', 'Attachments:getAttachment'); // User -$app->post ('/attachments', 'Attachments:addAttachment'); // User -$app->delete('/attachments/{id}', 'Attachments:removeAttachment'); // BoardAdmin or submitter +$app->get ('/attachments/{id}', 'Attachments:getAttachment'); // User (with board access) +$app->post ('/attachments', 'Attachments:addAttachment'); // User (with board access) +$app->delete('/attachments/{id}', 'Attachments:removeAttachment'); // BoardAdmin or submitter (with board access) $app->get ('/users', 'Users:getAllUsers'); // User (by board access) $app->get ('/users/{id}', 'Users:getUser'); // User (by board access) diff --git a/test/api/controllers/AutoActionsTest.php b/test/api/controllers/AutoActionsTest.php index 946047b..7db05cb 100644 --- a/test/api/controllers/AutoActionsTest.php +++ b/test/api/controllers/AutoActionsTest.php @@ -13,67 +13,120 @@ class AutoActionsTest extends PHPUnit_Framework_TestCase { public function setUp() { RedBeanPHP\R::nuke(); + Auth::CreateInitialAdmin(new ContainerMock()); + $this->actions = new AutoActions(new ContainerMock()); } public function testGetAllActions() { - $expected = new ApiJson(); - $expected->addAlert('info', 'No automatic actions in database.'); + $request = new RequestMock(); + $request->header = [DataMock::getJwt()]; - $actual = - $this->actions->getAllActions(null, new ResponseMock(), null); - $this->assertEquals($expected, $actual); + $actual = $this->actions->getAllActions($request, + new ResponseMock(), null); + $this->assertEquals('No automatic actions in database.', + $actual->alerts[0]['text']); $this->createAutoAction(); + $this->actions = new AutoActions(new ContainerMock()); - $actions = - $this->actions->getAllActions(null, new ResponseMock(), null); - $this->assertTrue(count($actions->data) === 1); - $this->assertTrue($actions->status === 'success'); + $request->header = [DataMock::getJwt()]; + + $actions = $this->actions->getAllActions($request, + new ResponseMock(), null); + + $this->assertEquals(2, count($actions->data)); + $this->assertEquals('success', $actions->status); } public function testAddRemoveAction() { - $expected = new ApiJson(); - $actual = $this->createAutoAction(); - - $expected->setSuccess(); - $expected->addAlert('success', 'Automatic action added.'); - - $this->assertEquals($expected, $actual); - - $expected->addAlert('success', 'Automatic action removed.'); + $this->assertEquals('success', $actual->status); $args = []; $args['id'] = '1'; - $actual = $this->actions->removeAction(null, new ResponseMock(), $args); + $this->actions = new AutoActions(new ContainerMock()); + $request = new RequestMock(); + $request->header = [DataMock::getJwt()]; - $this->assertEquals($expected, $actual); + $actual = $this->actions->removeAction($request, + new ResponseMock(), $args); + + $this->assertEquals('Automatic action removed.', + $actual->alerts[0]['text']); } - public function testAddBadAction() { + public function testGetAllActionsUnprivileged() { + $res = DataMock::createUnpriviligedUser(); + $this->assertEquals('success', $res->status); + + $request = new RequestMock(); + $request->header = [DataMock::getJwt(2)]; + + $actual = $this->actions->getAllActions($request, + new ResponseMock(), null); + + $this->assertEquals('Insufficient privileges.', + $actual->alerts[0]['text']); + } + + public function testAddRemoveActionUnprivileged() { + $res = DataMock::createUnpriviligedUser(); + $this->assertEquals('success', $res->status); + + $request = new RequestMock(); + $request->header = [DataMock::getJwt(2)]; + + $action = DataMock::getAutoAction(); + $action->id = 0; + + $request->payload = $action; + + $actual = $this->actions->addAction($request, + new ResponseMock(), null); + $this->assertEquals('Insufficient privileges.', + $actual->alerts[0]['text']); + + $args = []; + $args['id'] = '1'; + + $this->actions = new AutoActions(new ContainerMock()); + $request = new RequestMock(); + $request->header = [DataMock::getJwt(2)]; + + $actual = $this->actions->removeAction($request, + new ResponseMock(), $args); + + $this->assertEquals('Insufficient privileges.', + $actual->alerts[0]['text']); + } + + public function testAddRemoveBadAction() { $request = new RequestMock(); $request->invalidPayload = true; + $request->header = [DataMock::getJwt()]; $response = $this->actions->addAction($request, new ResponseMock(), null); - $this->assertTrue($response->status === 'failure'); - $this->assertTrue($response->alerts[0]['type'] === 'error'); - } + $this->assertEquals('failure', $response->status); + $this->assertEquals('error', $response->alerts[0]['type']); - public function testRemoveBadAction() { $args = []; $args['id'] = 5; // No such action - $response = $this->actions->removeAction(null, + $request->header = [DataMock::getJwt()]; + + $response = $this->actions->removeAction($request, new ResponseMock(), $args); - $this->assertTrue($response->status === 'failure'); + $this->assertEquals('failure', $response->status); } private function createAutoAction() { $request = new RequestMock(); + $request->header = [DataMock::getJwt()]; + $action = DataMock::getAutoAction(); $action->id = 0; @@ -81,7 +134,7 @@ class AutoActionsTest extends PHPUnit_Framework_TestCase { $response = $this->actions->addAction($request, new ResponseMock(), null); - $this->assertTrue($response->status === 'success'); + $this->assertEquals('success', $response->status); return $response; } diff --git a/test/api/controllers/BoardsTest.php b/test/api/controllers/BoardsTest.php index 7e9410f..f5464f6 100644 --- a/test/api/controllers/BoardsTest.php +++ b/test/api/controllers/BoardsTest.php @@ -120,6 +120,7 @@ class BoardsTest extends PHPUnit_Framework_TestCase { $actual->alerts[0]['text']); $this->boards = new Boards(new ContainerMock()); + $request->header = [DataMock::getJwt(2)]; $actual = $this->boards->removeBoard($request, new ResponseMock(), $args); diff --git a/test/api/controllers/ColumnsTest.php b/test/api/controllers/ColumnsTest.php index b78ef7d..a45b85b 100644 --- a/test/api/controllers/ColumnsTest.php +++ b/test/api/controllers/ColumnsTest.php @@ -14,66 +14,112 @@ class ColumnsTest extends PHPUnit_Framework_TestCase { public function setUp() { RedBeanPHP\R::nuke(); + Auth::CreateInitialAdmin(new ContainerMock()); + $this->columns = new Columns(new ContainerMock()); } public function testGetColumn() { - $expected = new ApiJson(); - $expected->addAlert('error', 'No column found for ID 1.'); + $request = new RequestMock(); + $request->header = [DataMock::getJwt()]; $args = []; $args['id'] = 1; - $actual = $this->columns->getColumn(null, + $actual = $this->columns->getColumn($request, new ResponseMock(), $args); - $this->assertEquals($expected, $actual); + $this->assertEquals('failure', $actual->status); $this->createColumn(); - $actual = $this->columns->getColumn(null, + $this->columns = new Columns(new ContainerMock()); + $request->header = [DataMock::getJwt()]; + + $actual = $this->columns->getColumn($request, new ResponseMock(), $args); - $this->assertTrue($actual->status === 'success'); - $this->assertTrue(count($actual->data) === 1); + $this->assertEquals('success', $actual->status); + $this->assertEquals(2, count($actual->data)); + } + + public function testGetColumnUnprivileged() { + $res = DataMock::createUnpriviligedUser(); + $this->assertEquals('success', $res->status); + + $request = new RequestMock(); + $request->header = [DataMock::getJwt(2)]; + + $actual = $this->columns->getColumn($request, + new ResponseMock(), null); + + $this->assertEquals('Insufficient privileges.', + $actual->alerts[0]['text']); } public function testAddRemoveColumn() { - $expected = new ApiJson(); - $actual = $this->createColumn(); - - $expected->setSuccess(); - $expected->addAlert('success', 'Column col1 added.'); - - $this->assertEquals($expected, $actual); - - $expected->addAlert('success', 'Column col1 removed.'); + $this->assertEquals('success', $actual->status); $args = []; $args['id'] = 1; - $actual = $this->columns->removeColumn(null, + $request = new RequestMock(); + $request->header = [DataMock::getJwt()]; + + $actual = $this->columns->removeColumn($request, new ResponseMock(), $args); - $this->assertEquals($expected, $actual); + $this->assertEquals('success', $actual->status); + } + + public function testAddRemoveColumnUnprivileged() { + $res = DataMock::createUnpriviligedUser(); + $this->assertEquals('success', $res->status); + + $request = new RequestMock(); + $request->header = [DataMock::getJwt(2)]; + + $column = DataMock::getColumn(); + $column->id = 0; + + $request->payload = $column; + + $actual = $this->columns->addColumn($request, + new ResponseMock(), null); + $this->assertEquals('Insufficient privileges.', + $actual->alerts[0]['text']); + + $args = []; + $args['id'] = 1; + + $request->header = [DataMock::getJwt(2)]; + + $actual = $this->columns->removeColumn($request, + new ResponseMock(), $args); + $this->assertEquals('Insufficient privileges.', + $actual->alerts[0]['text']); } public function testAddBadColumn() { $request = new RequestMock(); $request->invalidPayload = true; + $request->header = [DataMock::getJwt()]; $response = $this->columns->addColumn($request, new ResponseMock(), null); - $this->assertTrue($response->status === 'failure'); - $this->assertTrue($response->alerts[0]['type'] === 'error'); + $this->assertEquals('failure', $response->status); + $this->assertEquals('error', $response->alerts[0]['type']); } public function testRemoveBadColumn() { $args = []; $args['id'] = 5; // No such column - $response = $this->columns->removeColumn(null, + $request = new RequestMock(); + $request->header = [DataMock::getJwt()]; + + $response = $this->columns->removeColumn($request, new ResponseMock(), $args); - $this->assertTrue($response->status === 'failure'); + $this->assertEquals('failure', $response->status); } public function testUpdateColumn() { @@ -87,19 +133,50 @@ class ColumnsTest extends PHPUnit_Framework_TestCase { $request = new RequestMock(); $request->payload = $column; + $request->header = [DataMock::getJwt()]; $response = $this->columns->updateColumn($request, new ResponseMock(), $args); - $this->assertTrue($response->status === 'success'); + $this->assertEquals('success', $response->status); $request->payload = new stdClass(); + $request->header = [DataMock::getJwt()]; + $response = $this->columns->updateColumn($request, new ResponseMock(), $args); - $this->assertTrue($response->alerts[2]['type'] === 'error'); + $this->assertEquals('error', $response->alerts[2]['type']); + } + + /** + * @group single + */ + public function testUpdateColumnUnprivileged() { + $res = DataMock::createUnpriviligedUser(); + $this->assertEquals('success', $res->status); + + $this->createColumn(); + $this->columns = new Columns(new ContainerMock()); + + $column = DataMock::getColumn(); + $column->name = 'updated'; + + $args = []; + $args['id'] = $column->id; + + $request = new RequestMock(); + $request->payload = $column; + $request->header = [DataMock::getJwt(2)]; + + $actual = $this->columns->updateColumn($request, + new ResponseMock(), $args); + $this->assertEquals('Insufficient privileges.', + $actual->alerts[0]['text']); } private function createColumn() { $request = new RequestMock(); + $request->header = [DataMock::getJwt()]; + $column = DataMock::getColumn(); $column->id = 0; @@ -107,7 +184,7 @@ class ColumnsTest extends PHPUnit_Framework_TestCase { $response = $this->columns->addColumn($request, new ResponseMock(), null); - $this->assertTrue($response->status === 'success'); + $this->assertEquals('success', $response->status); return $response; }