ahtlon/deck
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Rewrite permission check and tests

Browse Source
This commit is contained in:
Julius Haertl
2017-01-16 00:14:28 +01:00
parent 75168332e9
commit b2113963d2
2 changed files with 148 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
lib/Service/PermissionService.php
View File

@@ -43,13 +43,12 @@ class PermissionService {
private $logger;
private $userId;
public function __construct(
ILogger $logger,
AclMapper $aclMapper,
BoardMapper $boardMapper,
IGroupManager $groupManager,
$userId
) {
public function __construct(ILogger $logger,
AclMapper $aclMapper,
BoardMapper $boardMapper,
IGroupManager $groupManager,
$userId
) {
$this->aclMapper = $aclMapper;
$this->boardMapper = $boardMapper;
$this->logger = $logger;
@@ -74,21 +73,6 @@ class PermissionService {
];
}
/**
* Check if the current user has specified permissions on a board
*
* @param $boardId
* @param $permission
* @return bool
*/
public function getPermission($boardId, $permission) {
if ($this->userIsBoardOwner($boardId)) {
return true;
}
$acls = $this->aclMapper->findAll($boardId);
return $this->userCan($acls, $permission);
}
/**
* check permissions for replacing dark magic middleware
*
@@ -96,7 +80,7 @@ class PermissionService {
* @param $id int unique identifier of the Entity
* @param $permission int
* @return bool
* @throws NoPermissionException|NotFoundException
* @throws NoPermissionException
*/
public function checkPermission($mapper, $id, $permission) {
try {
@@ -106,18 +90,27 @@ class PermissionService {
$boardId = $id;
}
if($boardId === null) {
throw new NotFoundException('No entity found');
// Throw NoPermission to not leak information about existing entries
throw new NoPermissionException('Permission denied');
}
if (!$this->getPermission($boardId, $permission)) {
$class = new \ReflectionClass($mapper);
$constants = array_flip($class->getConstants());
throw new NoPermissionException('Permission ' . $constants[$permission] . ' not granted.');
if ($this->userIsBoardOwner($boardId)) {
return true;
}
$acls = $this->aclMapper->findAll($boardId);
$result = $this->userCan($acls, $permission);
if ($result) {
return true;
}
} catch (DoesNotExistException $exception) {
throw new NotFoundException('Permission denied');
// Throw NoPermission to not leak information about existing entries
throw new NoPermissionException('Permission denied');
}
return true;
}
throw new NoPermissionException('Permission denied.');
}
/**
* @param $boardId
@@ -139,7 +132,7 @@ class PermissionService {
* @param $permission
* @return bool
*/
public function userCan($acls, $permission) {
public function userCan(array $acls, $permission) {
// check for users
foreach ($acls as $acl) {
if ($acl->getType() === "user" && $acl->getParticipant() === $this->userId) {

147
tests/unit/Service/PermissionServiceTest.php
View File

@@ -27,13 +27,16 @@ use OCA\Deck\Db\Acl;
use OCA\Deck\Db\AclMapper;
use OCA\Deck\Db\Board;
use OCA\Deck\Db\BoardMapper;
use OCA\Deck\Db\IPermissionMapper;
use OCA\Deck\NoPermissionException;
use OCP\IGroupManager;
use OCP\ILogger;
use PHPUnit_Framework_TestCase;
class PermissionServiceTest extends \PHPUnit_Framework_TestCase {
/** @var \PHPUnit_Framework_MockObject_MockObject|PermissionService */
private $service;
/** @var \PHPUnit_Framework_MockObject_MockObject|ILogger */
private $logger;
private $aclMapper;
private $boardMapper;
@@ -113,27 +116,6 @@ class PermissionServiceTest extends \PHPUnit_Framework_TestCase {
$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');
@@ -176,12 +158,127 @@ class PermissionServiceTest extends \PHPUnit_Framework_TestCase {
$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));
}
public function dataCheckPermission() {
return [
// see getAcls() for set permissions
[1, Acl::PERMISSION_READ, true],
[1, Acl::PERMISSION_EDIT, false],
[1, Acl::PERMISSION_MANAGE, false],
[1, Acl::PERMISSION_SHARE, false],
[2, Acl::PERMISSION_READ, true],
[2, Acl::PERMISSION_EDIT, true],
[2, Acl::PERMISSION_MANAGE, false],
[2, Acl::PERMISSION_SHARE, false],
[3, Acl::PERMISSION_READ, true],
[3, Acl::PERMISSION_EDIT, false],
[3, Acl::PERMISSION_MANAGE, true],
[3, Acl::PERMISSION_SHARE, false],
[4, Acl::PERMISSION_READ, true],
[4, Acl::PERMISSION_EDIT, false],
[4, Acl::PERMISSION_MANAGE, false],
[4, Acl::PERMISSION_SHARE, true],
[null, Acl::PERMISSION_READ, false],
[6, Acl::PERMISSION_READ, false],
[1, Acl::PERMISSION_READ, true, 'admin'],
[1, Acl::PERMISSION_EDIT, true, 'admin'],
[1, Acl::PERMISSION_MANAGE, true, 'admin'],
[1, Acl::PERMISSION_SHARE, true, 'admin'],
];
}
/** @dataProvider dataCheckPermission */
public function testCheckPermission($boardId, $permission, $result, $owner='foo') {
// Setup mapper
$mapper = $this->getMockBuilder(IPermissionMapper::class)->getMock();
// board owner
$mapper->expects($this->once())->method('findBoardId')->willReturn($boardId);
$board = new Board();
$board->setId($boardId);
$board->setOwner($owner);
$this->boardMapper->expects($this->any())->method('find')->willReturn($board);
// acl check
$acls = $this->getAcls($boardId);
$this->aclMapper->expects($this->any())->method('findAll')->willReturn($acls);
if($result) {
$actual = $this->service->checkPermission($mapper, 1234, $permission);
$this->assertTrue($actual);
} else {
$this->setExpectedException(NoPermissionException::class);
$this->service->checkPermission($mapper, 1234, $permission);
}
}
/** @dataProvider dataCheckPermission */
public function testCheckPermissionWithoutMapper($boardId, $permission, $result, $owner='foo') {
$mapper = null;
$board = new Board();
$board->setId($boardId);
$board->setOwner($owner);
$this->boardMapper->expects($this->any())->method('find')->willReturn($board);
$acls = $this->getAcls($boardId);
$this->aclMapper->expects($this->any())->method('findAll')->willReturn($acls);
if($result) {
$actual = $this->service->checkPermission($mapper, 1234, $permission);
$this->assertTrue($actual);
} else {
$this->setExpectedException(NoPermissionException::class);
$this->service->checkPermission($mapper, 1234, $permission);
}
}
public function testCheckPermissionNotFound() {
$mapper = $this->getMockBuilder(IPermissionMapper::class)->getMock();
$mapper->expects($this->once())->method('findBoardId')->willThrowException(new NoPermissionException(null));
$this->setExpectedException(NoPermissionException::class);
$this->service->checkPermission($mapper, 1234, Acl::PERMISSION_READ);
}
private function generateAcl($boardId, $type, $participant, $write, $manage, $share) {
$acl = new Acl();
$acl->setParticipant($participant);
$acl->setBoardId($boardId);
$acl->setType($type);
$acl->setPermissionWrite($write);
$acl->setPermissionInvite($share);
$acl->setPermissionManage($manage);
return $acl;
}
private function getAcls($boardId) {
$acls = [
$this->generateAcl(1, 'user', 'admin', false, false, false),
$this->generateAcl(2, 'user', 'admin', true, false, false),
$this->generateAcl(3, 'user', 'admin', false, true, false),
$this->generateAcl(4, 'user', 'admin', false, false, true),
$this->generateAcl(5, 'group', 'admin', false, false, false),
$this->generateAcl(6, 'user', 'foo', false, false, false)
];
$result = [];
foreach ($acls as $acl) {
if($acl->getBoardId() === $boardId) {
$result[] = $acl;
}
}
return $result;
}
}