From c0c4010cf1ae37c744fb22716ec1fc051f868816 Mon Sep 17 00:00:00 2001 From: Julius Haertl Date: Sun, 6 Nov 2016 22:06:11 +0100 Subject: [PATCH] Check permissions in frontend --- appinfo/info.xml | 5 +- js/controller/CardController.js | 13 +- js/public/app.js | 13 +- lib/Controller/BoardController.php | 23 +-- lib/Db/AclMapper.php | 4 +- lib/Db/LabelMapper.php | 3 +- lib/Middleware/SharingMiddleware.php | 5 +- lib/Service/BoardService.php | 34 +---- lib/StatusException.php | 2 +- tests/unit/Service/BoardServiceTest.php | 187 ++++++++++++++++++++++++ 10 files changed, 219 insertions(+), 70 deletions(-) create mode 100644 tests/unit/Service/BoardServiceTest.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 1aa7bf6c7..e6130546e 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -6,9 +6,10 @@ My first ownCloud app agpl Julius Härtl - 0.0.1.15 + 0.1.0-beta1 Deck - Other + organization + office diff --git a/js/controller/CardController.js b/js/controller/CardController.js index afb08f6fe..9aa2e9812 100644 --- a/js/controller/CardController.js +++ b/js/controller/CardController.js @@ -47,12 +47,17 @@ app.controller('CardController', function ($scope, $rootScope, $routeParams, $lo $scope.status.cardRename=true; } }; - $scope.cardEditDescriptionShow = function() { - if($scope.archived || !BoardService.canEdit()) - return false; - else { + $scope.cardEditDescriptionShow = function($event) { + var node = $event.target.nodeName; + console.log($event); + console.log(BoardService); + if($scope.card.archived || !$scope.boardservice.canEdit()) { + console.log(node); + } else { + console.log("edit"); $scope.status.cardEditDescription=true; } + console.log($scope.status.canEditDescription); }; // handle rename to update information on the board as well $scope.cardRename = function(card) { diff --git a/js/public/app.js b/js/public/app.js index 09e77b084..6f8bf225d 100644 --- a/js/public/app.js +++ b/js/public/app.js @@ -369,12 +369,17 @@ app.controller('CardController', ["$scope", "$rootScope", "$routeParams", "$loca $scope.status.cardRename=true; } }; - $scope.cardEditDescriptionShow = function() { - if($scope.archived || !BoardService.canEdit()) - return false; - else { + $scope.cardEditDescriptionShow = function($event) { + var node = $event.target.nodeName; + console.log($event); + console.log(BoardService); + if($scope.card.archived || !$scope.boardservice.canEdit()) { + console.log(node); + } else { + console.log("edit"); $scope.status.cardEditDescription=true; } + console.log($scope.status.canEditDescription); }; // handle rename to update information on the board as well $scope.cardRename = function(card) { diff --git a/lib/Controller/BoardController.php b/lib/Controller/BoardController.php index 59089673a..774bfbe91 100644 --- a/lib/Controller/BoardController.php +++ b/lib/Controller/BoardController.php @@ -23,14 +23,10 @@ namespace OCA\Deck\Controller; -use OCA\Deck\Db\Acl; use OCA\Deck\Service\BoardService; - use OCA\Deck\Service\PermissionService; use OCP\IRequest; - use OCP\AppFramework\Controller; - use OCP\IUserManager; use OCP\IGroupManager; @@ -127,24 +123,7 @@ class BoardController extends Controller { * @internal param $userId */ public function getUserPermissions($boardId) { - $this->permissionService->getPermissions($boardId); - $board = $this->boardService->find($boardId); - if ($this->userId === $board->getOwner()) { - return [ - 'PERMISSION_READ' => true, - 'PERMISSION_EDIT' => true, - 'PERMISSION_MANAGE' => true, - 'PERMISSION_SHARE' => true, - ]; - } - - return [ - 'PERMISSION_READ' => $this->boardService->getPermission($boardId, $this->userId, Acl::PERMISSION_READ), - 'PERMISSION_EDIT' => $this->boardService->getPermission($boardId, $this->userId, Acl::PERMISSION_EDIT), - 'PERMISSION_MANAGE' => $this->boardService->getPermission($boardId, $this->userId, Acl::PERMISSION_MANAGE), - 'PERMISSION_SHARE' => $this->boardService->getPermission($boardId, $this->userId, Acl::PERMISSION_SHARE), - ]; - + return $this->permissionService->getPermissions($boardId); } /** diff --git a/lib/Db/AclMapper.php b/lib/Db/AclMapper.php index 79e0f8ea0..9b78dd211 100644 --- a/lib/Db/AclMapper.php +++ b/lib/Db/AclMapper.php @@ -26,17 +26,17 @@ namespace OCA\Deck\Db; use OCP\IDb; +use OCP\IDBConnection; class AclMapper extends DeckMapper implements IPermissionMapper { - public function __construct(IDb $db) { + public function __construct(IDBConnection $db) { parent::__construct($db, 'deck_board_acl', '\OCA\Deck\Db\Acl'); } public function findAll($boardId, $limit=null, $offset=null) { $sql = 'SELECT id, board_id, type, participant, permission_write, permission_invite, permission_manage FROM `*PREFIX*deck_board_acl` WHERE `board_id` = ? '; - //'UNION SELECT 0, id, \'user\', owner, 1, 1, 1, 1 FROM `*PREFIX*deck_boards` WHERE `id` = ? '; return $this->findEntities($sql, [$boardId], $limit, $offset); } diff --git a/lib/Db/LabelMapper.php b/lib/Db/LabelMapper.php index 9513e2822..5a86c0069 100644 --- a/lib/Db/LabelMapper.php +++ b/lib/Db/LabelMapper.php @@ -23,7 +23,6 @@ namespace OCA\Deck\Db; -use OCP\AppFramework\Db\Entity; use OCP\IDb; @@ -38,7 +37,7 @@ class LabelMapper extends DeckMapper implements IPermissionMapper { return $this->findEntities($sql, [$boardId], $limit, $offset); } - public function delete(Entity $entity) { + public function delete(\OCP\AppFramework\Db\Entity $entity) { // delete assigned labels $this->deleteLabelAssignments($entity->getId()); // delete label diff --git a/lib/Middleware/SharingMiddleware.php b/lib/Middleware/SharingMiddleware.php index 85e866f60..50b511180 100644 --- a/lib/Middleware/SharingMiddleware.php +++ b/lib/Middleware/SharingMiddleware.php @@ -59,7 +59,8 @@ class SharingMiddleware extends Middleware { IRequest $request, IUserSession $userSession, ControllerMethodReflector $reflector, - PermissionService $permissionService) { + PermissionService $permissionService + ) { $this->container = $container; $this->request = $request; $this->userSession = $userSession; @@ -178,7 +179,7 @@ class SharingMiddleware extends Middleware { } $boardId = $mapper->findBoardId($id); - if(!$boardId) { + if (!$boardId) { throw new NotFoundException("Entity not found"); } diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 56af36c24..515acf8e5 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -30,11 +30,9 @@ use OCP\IGroupManager; use OCP\ILogger; use OCP\IL10N; - use \OCA\Deck\Db\Board; use \OCA\Deck\Db\BoardMapper; use \OCA\Deck\Db\LabelMapper; -use OCP\IUserManager; class BoardService { @@ -50,26 +48,23 @@ class BoardService { IL10N $l10n, LabelMapper $labelMapper, AclMapper $aclMapper, - IUserManager $userManager, IGroupManager $groupManager) { $this->boardMapper = $boardMapper; $this->labelMapper = $labelMapper; $this->aclMapper = $aclMapper; $this->logger = $logger; $this->l10n = $l10n; - $this->userManager = $userManager; $this->groupManager = $groupManager; } public function findAll($userInfo) { $userBoards = $this->boardMapper->findAllByUser($userInfo['user']); $groupBoards = $this->boardMapper->findAllByGroups($userInfo['user'], $userInfo['groups']); - return array_merge($userBoards, $groupBoards); + return array_unique(array_merge($userBoards, $groupBoards)); } public function find($boardId) { - $board = $this->boardMapper->find($boardId, true, true); - return $board; + return $this->boardMapper->find($boardId, true, true); } public function create($title, $userId, $color) { @@ -84,7 +79,7 @@ class BoardService { '31CC7C' => $this->l10n->t('Finished'), '317CCC' => $this->l10n->t('To review'), 'FF7A66' => $this->l10n->t('Action needed'), - 'F1DB50' => $this->l10n->t('Maybe')]; + 'F1DB50' => $this->l10n->t('Later')]; $labels = []; foreach ($default_labels as $color=>$title) { $label = new Label(); @@ -134,27 +129,4 @@ class BoardService { return $this->aclMapper->delete($acl); } - /** - * @param $boardId - * @param $user - * @param $permission - * @return bool - */ - public function getPermission($boardId, $user, $permission) { - $acls = $this->aclMapper->findAll($boardId); - // check for users - foreach ($acls as $acl) { - if ($acl->getType() === "user" && $acl->getParticipant() === $user) { - return $acl->getPermission($permission); - } - } - // check for groups - $hasGroupPermission = false; - foreach ($acls as $acl) { - if (!$hasGroupPermission && $acl->getType() === "group" && $this->groupManager->isInGroup($user, $acl->getParticipant())) { - $hasGroupPermission = $acl->getPermission($permission); - } - } - return $hasGroupPermission; - } } \ No newline at end of file diff --git a/lib/StatusException.php b/lib/StatusException.php index b74d21b51..8527916a5 100644 --- a/lib/StatusException.php +++ b/lib/StatusException.php @@ -24,7 +24,7 @@ namespace OCA\Deck; -abstract class StatusException extends \Exception { +class StatusException extends \Exception { public function __construct($message) { parent::__construct($message); diff --git a/tests/unit/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php new file mode 100644 index 000000000..e16360513 --- /dev/null +++ b/tests/unit/Service/BoardServiceTest.php @@ -0,0 +1,187 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Deck\Service; + +use OCA\Deck\Db\Acl; +use OCA\Deck\Db\AclMapper; +use OCA\Deck\Db\Board; +use OCA\Deck\Db\BoardMapper; +use OCP\IGroupManager; +use OCP\ILogger; +use PHPUnit_Framework_TestCase; + +class BoardServiceTest extends \PHPUnit_Framework_TestCase { + + private $service; + private $logger; + private $aclMapper; + private $boardMapper; + private $groupManager; + private $userId = 'admin'; + + public function setUp() { + $this->logger = $this->request = $this->getMockBuilder(ILogger::class) + ->disableOriginalConstructor() + ->getMock(); + $this->aclMapper = $this->getMockBuilder(AclMapper::class) + ->disableOriginalConstructor()->getMock(); + $this->boardMapper = $this->getMockBuilder(BoardMapper::class) + ->disableOriginalConstructor()->getMock(); + $this->groupManager = $this->getMockBuilder(IGroupManager::class) + ->disableOriginalConstructor()->getMock(); + + $this->service = new PermissionService( + $this->logger, + $this->aclMapper, + $this->boardMapper, + $this->groupManager, + 'admin' + ); + } + + + public function testGetPermissionsOwner() { + $board = new Board(); + $board->setOwner('admin'); + $this->boardMapper->expects($this->once())->method('find')->with(123)->willReturn($board); + $this->aclMapper->expects($this->once()) + ->method('findAll') + ->willReturn(null); + $expected = [ + Acl::PERMISSION_READ => true, + Acl::PERMISSION_EDIT => true, + Acl::PERMISSION_MANAGE => true, + Acl::PERMISSION_SHARE => true, + ]; + $this->assertEquals($expected, $this->service->getPermissions(123)); + } + public function testGetPermissionsAcl() { + $board = new Board(); + $board->setOwner('admin'); + $this->boardMapper->expects($this->once())->method('find')->with(123)->willReturn($board); + $aclUser = new Acl(); + $aclUser->setType('user'); + $aclUser->setParticipant('admin'); + $aclUser->setPermissionWrite(true); + $aclUser->setPermissionInvite(true); + $aclUser->setPermissionManage(true); + $this->aclMapper->expects($this->once()) + ->method('findAll') + ->willReturn([$aclUser]); + $expected = [ + Acl::PERMISSION_READ => true, + Acl::PERMISSION_EDIT => true, + Acl::PERMISSION_MANAGE => true, + Acl::PERMISSION_SHARE => true, + ]; + $this->assertEquals($expected, $this->service->getPermissions(123)); + } + public function testGetPermissionsAclNo() { + $board = new Board(); + $board->setOwner('user1'); + $this->boardMapper->expects($this->once())->method('find')->with(123)->willReturn($board); + $this->aclMapper->expects($this->once()) + ->method('findAll') + ->willReturn([]); + $expected = [ + Acl::PERMISSION_READ => false, + Acl::PERMISSION_EDIT => false, + Acl::PERMISSION_MANAGE => false, + Acl::PERMISSION_SHARE => false, + ]; + $this->assertEquals($expected, $this->service->getPermissions(123)); + } + + public function testGetPermission() { + $board = new Board(); + $board->setOwner('admin'); + $this->boardMapper->expects($this->exactly(4))->method('find')->with(123)->willReturn($board); + $this->assertEquals(true, $this->service->getPermission(123, Acl::PERMISSION_READ)); + $this->assertEquals(true, $this->service->getPermission(123, Acl::PERMISSION_EDIT)); + $this->assertEquals(true, $this->service->getPermission(123, Acl::PERMISSION_MANAGE)); + $this->assertEquals(true, $this->service->getPermission(123, Acl::PERMISSION_SHARE)); + } + + public function testGetPermissionFail() { + $board = new Board(); + $board->setOwner('user1'); + $this->boardMapper->expects($this->exactly(4))->method('find')->with(234)->willReturn($board); + $this->aclMapper->expects($this->exactly(4))->method('findAll')->willReturn([]); + $this->assertEquals(false, $this->service->getPermission(234, Acl::PERMISSION_READ)); + $this->assertEquals(false, $this->service->getPermission(234, Acl::PERMISSION_EDIT)); + $this->assertEquals(false, $this->service->getPermission(234, Acl::PERMISSION_MANAGE)); + $this->assertEquals(false, $this->service->getPermission(234, Acl::PERMISSION_SHARE)); + } + + public function testUserIsBoardOwner() { + $board = new Board(); + $board->setOwner('admin'); + $this->boardMapper->expects($this->at(0))->method('find')->with(123)->willReturn($board); + $this->assertEquals(true, $this->service->userIsBoardOwner(123)); + $board = new Board(); + $board->setOwner('user1'); + $this->boardMapper->expects($this->at(0))->method('find')->with(234)->willReturn($board); + $this->assertEquals(false, $this->service->userIsBoardOwner(234)); + } + + public function testUserIsBoardOwnerNull() { + $this->boardMapper->expects($this->once())->method('find')->willReturn(null); + $this->assertEquals(false, $this->service->userIsBoardOwner(123)); + } + + public function dataTestUserCan() { + return [ + // participant permissions type + ['admin', false, false, false, 'user', true, false, false, false], + ['admin', true, false, false, 'user', true, true, false, false], + ['admin', true, true, false, 'user', true, true, true, false], + ['admin', true, true, false, 'user', true, true, true, false], + ['admin', true, true, true, 'user', true, true, true, true], + ['user1', false, false, false, 'user', false, false, false, false] + ]; + } + /** @dataProvider dataTestUserCan */ + public function testUserCan($participant, $edit, $share, $manage, $type, $canRead, $canEdit, $canShare, $canManage) { + $aclUser = new Acl(); + $aclUser->setType($type); + $aclUser->setParticipant($participant); + $aclUser->setPermissionWrite($edit); + $aclUser->setPermissionInvite($share); + $aclUser->setPermissionManage($manage); + $acls = [ + $aclUser + ]; + $this->assertEquals($canRead, $this->service->userCan($acls, Acl::PERMISSION_READ)); + $this->assertEquals($canEdit, $this->service->userCan($acls, Acl::PERMISSION_EDIT)); + $this->assertEquals($canShare, $this->service->userCan($acls, Acl::PERMISSION_SHARE)); + $this->assertEquals($canManage, $this->service->userCan($acls, Acl::PERMISSION_MANAGE)); + + + } + + public function testUserCanFail() { + $this->assertFalse($this->service->userCan([], Acl::PERMISSION_EDIT)); + } + +} \ No newline at end of file