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

Remove dark magic middleware and check permission in services

Browse Source
This commit is contained in:
Julius Haertl
2017-01-13 16:36:36 +01:00
parent 6d0ebb7d73
commit b0627d8979
14 changed files with 158 additions and 297 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
lib/Controller/BoardController.php
View File

@@ -55,6 +55,10 @@ class BoardController extends Controller {
$this->userInfo = $this->getBoardPrerequisites();
}
/**
* TODO: move to boardservice
* @return array
*/
private function getBoardPrerequisites() {
$groups = $this->groupManager->getUserGroupIds(
$this->userManager->get($this->userId)
@@ -67,7 +71,6 @@ class BoardController extends Controller {
/**
* @NoAdminRequired
* @RequireNoPermission
*/
public function index() {
return $this->boardService->findAll($this->userInfo);
@@ -75,7 +78,6 @@ class BoardController extends Controller {
/**
* @NoAdminRequired
* @RequireReadPermission
* @param $boardId
* @return \OCP\AppFramework\Db\Entity
*/
@@ -85,7 +87,6 @@ class BoardController extends Controller {
/**
* @NoAdminRequired
* @RequireNoPermission
* @param $title
* @param $color
* @return \OCP\AppFramework\Db\Entity
@@ -96,7 +97,6 @@ class BoardController extends Controller {
/**
* @NoAdminRequired
* @RequireManagePermission
* @param $id
* @param $title
* @param $color
@@ -108,7 +108,6 @@ class BoardController extends Controller {
/**
* @NoAdminRequired
* @RequireManagePermission
* @param $boardId
* @return \OCP\AppFramework\Db\Entity
*/
@@ -118,7 +117,6 @@ class BoardController extends Controller {
/**
* @NoAdminRequired
* @RequireReadPermission
* @param $boardId
* @return array|bool
* @internal param $userId
@@ -135,7 +133,6 @@ class BoardController extends Controller {
/**
* @NoAdminRequired
* @RequireManagePermission
* @param $boardId
* @param $type
* @param $participant
@@ -150,7 +147,6 @@ class BoardController extends Controller {
/**
* @NoAdminRequired
* @RequireManagePermission
* @param $id
* @param $permissionWrite
* @param $permissionInvite
@@ -163,7 +159,6 @@ class BoardController extends Controller {
/**
* @NoAdminRequired
* @RequireManagePermission
* @param $aclId
* @return \OCP\AppFramework\Db\Entity
*/

10
lib/Controller/CardController.php
View File

@@ -42,7 +42,6 @@ class CardController extends Controller {
/**
* @NoAdminRequired
* @RequireReadPermission
* @param $cardId
* @return \OCP\AppFramework\Db\Entity
*/
@@ -52,7 +51,6 @@ class CardController extends Controller {
/**
* @NoAdminRequired
* @RequireEditPermission
* @param $cardId
* @param $stackId
* @param $order
@@ -64,7 +62,6 @@ class CardController extends Controller {
/**
* @NoAdminRequired
* @RequireEditPermission
* @param $cardId
* @param $title
* @return \OCP\AppFramework\Db\Entity
@@ -75,7 +72,6 @@ class CardController extends Controller {
/**
* @NoAdminRequired
* @RequireEditPermission
* @param $title
* @param $stackId
* @param $type
@@ -88,7 +84,6 @@ class CardController extends Controller {
/**
* @NoAdminRequired
* @RequireEditPermission
* @param $id
* @param $title
* @param $stackId
@@ -103,7 +98,6 @@ class CardController extends Controller {
/**
* @NoAdminRequired
* @RequireEditPermission
* @param $cardId
* @return \OCP\AppFramework\Db\Entity
*/
@@ -113,7 +107,6 @@ class CardController extends Controller {
/**
* @NoAdminRequired
* @RequireEditPermission
* @param $cardId
* @return \OCP\AppFramework\Db\Entity
*/
@@ -123,7 +116,6 @@ class CardController extends Controller {
/**
* @NoAdminRequired
* @RequireEditPermission
* @param $cardId
* @return \OCP\AppFramework\Db\Entity
*/
@@ -133,7 +125,6 @@ class CardController extends Controller {
/**
* @NoAdminRequired
* @RequireEditPermission
* @param $cardId
* @param $labelId
*/
@@ -143,7 +134,6 @@ class CardController extends Controller {
/**
* @NoAdminRequired
* @RequireEditPermission
* @param $cardId
* @param $labelId
*/

3
lib/Controller/LabelController.php
View File

@@ -44,7 +44,6 @@ class LabelController extends Controller {
/**
* @NoAdminRequired
* @RequireManagePermission
* @param $title
* @param $color
* @param $boardId
@@ -56,7 +55,6 @@ class LabelController extends Controller {
/**
* @NoAdminRequired
* @RequireManagePermission
* @param $id
* @param $title
* @param $color
@@ -68,7 +66,6 @@ class LabelController extends Controller {
/**
* @NoAdminRequired
* @RequireManagePermission
* @param $labelId
* @return \OCP\AppFramework\Db\Entity
*/

1
lib/Controller/ShareController.php
View File

@@ -55,7 +55,6 @@ class ShareController extends Controller {
/**
* @NoAdminRequired
* @RequireNoPermission
* @param $search
* @return array
*/

6
lib/Controller/StackController.php
View File

@@ -44,7 +44,6 @@ class StackController extends Controller {
/**
* @NoAdminRequired
* @RequireReadPermission
* @param $boardId
* @return array
*/
@@ -54,7 +53,6 @@ class StackController extends Controller {
/**
* @NoAdminRequired
* @RequireReadPermission
* @param $boardId
* @return array
*/
@@ -64,7 +62,6 @@ class StackController extends Controller {
/**
* @NoAdminRequired
* @RequireReadPermission
* @param $boardId
* @return
*/
@@ -74,7 +71,6 @@ class StackController extends Controller {
/**
* @NoAdminRequired
* @RequireManagePermission
* @param $title
* @param $boardId
* @param int $order
@@ -86,7 +82,6 @@ class StackController extends Controller {
/**
* @NoAdminRequired
* @RequireManagePermission
* @param $id
* @param $title
* @param $boardId
@@ -99,7 +94,6 @@ class StackController extends Controller {
/**
* @NoAdminRequired
* @RequireManagePermission
* @param $stackId
* @return \OCP\AppFramework\Db\Entity
*/

129
lib/Db/CardMapper.php
View File

@@ -27,104 +27,103 @@ use OCP\AppFramework\Db\Entity;
use OCP\IDBConnection;
class CardMapper extends DeckMapper implements IPermissionMapper {
private $labelMapper;
private $labelMapper;
public function __construct(IDBConnection $db, LabelMapper $labelMapper) {
parent::__construct($db, 'deck_cards', '\OCA\Deck\Db\Card');
$this->labelMapper = $labelMapper;
}
public function __construct(IDBConnection $db, LabelMapper $labelMapper) {
parent::__construct($db, 'deck_cards', '\OCA\Deck\Db\Card');
$this->labelMapper = $labelMapper;
}
public function insert(Entity $entity) {
$entity->setCreatedAt(time());
$entity->setLastModified(time());
return parent::insert($entity);
}
public function insert(Entity $entity) {
$entity->setCreatedAt(time());
$entity->setLastModified(time());
return parent::insert($entity);
}
public function update(Entity $entity) {
$entity->setLastModified(time());
return parent::update($entity);
}
public function update(Entity $entity) {
$entity->setLastModified(time());
return parent::update($entity);
}
/**
* @param $id
* @return Entity if not found
*/
public function find($id) {
$sql = 'SELECT * FROM `*PREFIX*deck_cards` ' .
'WHERE `id` = ?';
$card = $this->findEntity($sql, [$id]);
$labels = $this->labelMapper->findAssignedLabelsForCard($card->id);
$card->setLabels($labels);
return $card;
}
public function find($id) {
$sql = 'SELECT * FROM `*PREFIX*deck_cards` ' .
'WHERE `id` = ?';
$card = $this->findEntity($sql, [$id]);
$labels = $this->labelMapper->findAssignedLabelsForCard($card->id);
$card->setLabels($labels);
return $card;
}
public function findAll($stackId, $limit=null, $offset=null) {
$sql = 'SELECT * FROM `*PREFIX*deck_cards`
public function findAll($stackId, $limit = null, $offset = null) {
$sql = 'SELECT * FROM `*PREFIX*deck_cards`
WHERE `stack_id` = ? AND NOT archived ORDER BY `order`';
$entities = $this->findEntities($sql, [$stackId], $limit, $offset);
return $entities;
}
$entities = $this->findEntities($sql, [$stackId], $limit, $offset);
return $entities;
}
public function findAllArchived($stackId, $limit=null, $offset=null) {
$sql = 'SELECT * FROM `*PREFIX*deck_cards` WHERE `stack_id`=? AND archived ORDER BY `last_modified`';
$entities = $this->findEntities($sql, [$stackId], $limit, $offset);
return $entities;
}
public function findAllArchived($stackId, $limit = null, $offset = null) {
$sql = 'SELECT * FROM `*PREFIX*deck_cards` WHERE `stack_id`=? AND archived ORDER BY `last_modified`';
$entities = $this->findEntities($sql, [$stackId], $limit, $offset);
return $entities;
}
public function findAllByStack($stackId, $limit=null, $offset=null) {
public function findAllByStack($stackId, $limit = null, $offset = null) {
$sql = 'SELECT id FROM `*PREFIX*deck_cards`
WHERE `stack_id` = ?';
$entities = $this->findEntities($sql, [$stackId], $limit, $offset);
return $entities;
}
public function delete(Entity $entity) {
public function delete(Entity $entity) {
// delete assigned labels
$this->labelMapper->deleteLabelAssignmentsForCard($entity->getId());
// delete card
return parent::delete($entity);
}
return parent::delete($entity);
}
public function deleteByStack($stackId) {
$cards = $this->findAllByStack($stackId);
public function deleteByStack($stackId) {
$cards = $this->findAllByStack($stackId);
foreach ($cards as $card) {
$this->delete($card);
}
}
public function assignLabel($card, $label) {
$sql = 'INSERT INTO `*PREFIX*deck_assigned_labels` (`label_id`,`card_id`) VALUES (?,?)';
$stmt = $this->db->prepare($sql);
$stmt->bindParam(1, $label, \PDO::PARAM_INT);
$stmt->bindParam(2, $card, \PDO::PARAM_INT);
$stmt->execute();
}
public function assignLabel($card, $label) {
$sql = 'INSERT INTO `*PREFIX*deck_assigned_labels` (`label_id`,`card_id`) VALUES (?,?)';
$stmt = $this->db->prepare($sql);
$stmt->bindParam(1, $label, \PDO::PARAM_INT);
$stmt->bindParam(2, $card, \PDO::PARAM_INT);
$stmt->execute();
}
public function removeLabel($card, $label) {
$sql = 'DELETE FROM `*PREFIX*deck_assigned_labels` WHERE card_id = ? AND label_id = ?';
$stmt = $this->db->prepare($sql);
$stmt->bindParam(1, $card, \PDO::PARAM_INT);
$stmt->bindParam(2, $label, \PDO::PARAM_INT);
$stmt->execute();
}
public function removeLabel($card, $label) {
$sql = 'DELETE FROM `*PREFIX*deck_assigned_labels` WHERE card_id = ? AND label_id = ?';
$stmt = $this->db->prepare($sql);
$stmt->bindParam(1, $card, \PDO::PARAM_INT);
$stmt->bindParam(2, $label, \PDO::PARAM_INT);
$stmt->execute();
}
public function isOwner($userId, $cardId) {
$sql = 'SELECT owner FROM `*PREFIX*deck_boards` WHERE `id` IN (SELECT board_id FROM `*PREFIX*deck_stacks` WHERE id IN (SELECT stack_id FROM `*PREFIX*deck_cards` WHERE id = ?))';
$stmt = $this->execute($sql, [$cardId]);
$row = $stmt->fetch();
return ($row['owner'] === $userId);
}
public function isOwner($userId, $cardId) {
$sql = 'SELECT owner FROM `*PREFIX*deck_boards` WHERE `id` IN (SELECT board_id FROM `*PREFIX*deck_stacks` WHERE id IN (SELECT stack_id FROM `*PREFIX*deck_cards` WHERE id = ?))';
$stmt = $this->execute($sql, [$cardId]);
$row = $stmt->fetch();
return ($row['owner'] === $userId);
}
public function findBoardId($cardId) {
$sql = 'SELECT id FROM `*PREFIX*deck_boards` WHERE `id` IN (SELECT board_id FROM `*PREFIX*deck_stacks` WHERE id IN (SELECT stack_id FROM `*PREFIX*deck_cards` WHERE id = ?))';
$stmt = $this->execute($sql, [$cardId]);
$row = $stmt->fetch();
return $row['id'];
}
public function findBoardId($cardId) {
$sql = 'SELECT id FROM `*PREFIX*deck_boards` WHERE `id` IN (SELECT board_id FROM `*PREFIX*deck_stacks` WHERE id IN (SELECT stack_id FROM `*PREFIX*deck_cards` WHERE id = ?))';
$stmt = $this->execute($sql, [$cardId]);
$row = $stmt->fetch();
return $row['id'];
}
}

121
lib/Middleware/SharingMiddleware.php
View File

@@ -68,33 +68,6 @@ class SharingMiddleware extends Middleware {
$this->permissionService = $permissionService;
}
/**
* All permission checks for controller access
*
* The following method annotations are possible
* - RequireReadPermission
* - RequireEditPermission
* - RequireSharePermission
* - RequireManagePermission
* - RequireNoPermission
*
* Depending on the Controller class we call a corresponding mapper to find the board_id
* With the board_id we can check for ownership/permissions in the acl table
*
* @param \OCP\AppFramework\Controller $controller
* @param string $methodName
* @throws NoPermissionException
*/
public function beforeController($controller, $methodName) {
$userId = null;
if ($this->userSession->getUser()) {
$userId = $this->userSession->getUser()->getUID();
}
$method = $this->request->getMethod();
$params = $this->request->getParams();
$this->checkPermissions($userId, $controller, $method, $params, $methodName);
}
/**
* Return JSON error response if the user has no sufficient permission
*
@@ -114,98 +87,4 @@ class SharingMiddleware extends Middleware {
throw $exception;
}
/**
* Check permission depending on the route (controller/method)
*
* @param $userId
* @param $controller
* @param $method
* @param $params
* @param $methodName
* @return bool
* @throws NoPermissionException
* @throws \Exception
*/
private function checkPermissions($userId, $controller, $method, $params, $methodName) {
// no permission checks needed for plain html page or RequireNoPermission
if ($controller instanceof PageController ||
$this->reflector->hasAnnotation('RequireNoPermission')
) {
return true;
}
$mapper = null;
$id = null;
if ($controller instanceof BoardController) {
$mapper = $this->container->query('OCA\Deck\Db\BoardMapper');
$id = $params['boardId'];
}
if ($controller instanceof StackController) {
if ($method === "GET" || $method === "POST") {
$mapper = $this->container->query('OCA\Deck\Db\BoardMapper');
$id = $params['boardId'];
} else {
$mapper = $this->container->query('OCA\Deck\Db\StackMapper');
$id = $params['stackId'];
}
}
if ($controller instanceof CardController) {
if ($method === "POST" && !preg_match('/Label/', $methodName)) {
$mapper = $this->container->query('OCA\Deck\Db\StackMapper');
$id = $params['stackId'];
} else {
$mapper = $this->container->query('OCA\Deck\Db\CardMapper');
$id = $params['cardId'];
}
}
if ($controller instanceof LabelController) {
if ($method === "GET" || $method === "POST") {
$mapper = $this->container->query('OCA\Deck\Db\BoardMapper');
$id = $params['boardId'];
} else {
$mapper = $this->container->query('OCA\Deck\Db\LabelMapper');
$id = $params['labelId'];
}
}
// check if there is a mapper so we can find the corresponding board for the request
if ($mapper === null) {
throw new \Exception("No mappers specified for permission checks");
}
$boardId = $mapper->findBoardId($id);
if (!$boardId) {
throw new NotFoundException("Entity not found");
}
if ($this->reflector->hasAnnotation('RequireReadPermission')) {
if (!$this->permissionService->getPermission($boardId, Acl::PERMISSION_READ)) {
throw new NoPermissionException("User " . $userId . " has no permission to read.", $controller, $methodName);
}
}
if ($this->reflector->hasAnnotation('RequireEditPermission')) {
if (!$this->permissionService->getPermission($boardId, Acl::PERMISSION_EDIT)) {
throw new NoPermissionException("User " . $userId . " has no permission to edit.", $controller, $methodName);
}
}
if ($this->reflector->hasAnnotation('RequireSharePermission')) {
if (!$this->permissionService->getPermission($boardId, Acl::PERMISSION_SHARE)) {
throw new NoPermissionException("User " . $userId . " has no permission to share.", $controller, $methodName);
}
}
if ($this->reflector->hasAnnotation('RequireManagePermission')) {
if (!$this->permissionService->getPermission($boardId, Acl::PERMISSION_MANAGE)) {
throw new NoPermissionException("User " . $userId . " has no permission to manage.", $controller, $methodName);
}
}
// all permission checks succeeded
return true;
}
}

15
lib/Service/BoardService.php
View File

@@ -38,10 +38,10 @@ use OCA\Deck\Db\LabelMapper;
class BoardService {
private $boardMapper;
private $aclMapper;
private $labelMapper;
private $logger;
private $aclMapper;
private $l10n;
private $permissionService;
public function __construct(
BoardMapper $boardMapper,
@@ -49,14 +49,13 @@ class BoardService {
IL10N $l10n,
LabelMapper $labelMapper,
AclMapper $aclMapper,
IGroupManager $groupManager
PermissionService $permissionService
) {
$this->boardMapper = $boardMapper;
$this->labelMapper = $labelMapper;
$this->aclMapper = $aclMapper;
$this->logger = $logger;
$this->l10n = $l10n;
$this->groupManager = $groupManager;
$this->permissionService = $permissionService;
}
public function findAll($userInfo) {
@@ -67,6 +66,7 @@ class BoardService {
}
public function find($boardId) {
$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ);
return $this->boardMapper->find($boardId, true, true);
}
@@ -97,10 +97,12 @@ class BoardService {
}
public function delete($id) {
$this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_READ);
return $this->boardMapper->delete($this->find($id));
}
public function update($id, $title, $color) {
$this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_MANAGE);
$board = $this->find($id);
$board->setTitle($title);
$board->setColor($color);
@@ -109,6 +111,7 @@ class BoardService {
public function addAcl($boardId, $type, $participant, $write, $invite, $manage) {
$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_SHARE);
$acl = new Acl();
$acl->setBoardId($boardId);
$acl->setType($type);
@@ -120,6 +123,7 @@ class BoardService {
}
public function updateAcl($id, $write, $invite, $manage) {
$this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_SHARE);
$acl = $this->aclMapper->find($id);
$acl->setPermissionWrite($write);
$acl->setPermissionInvite($invite);
@@ -128,6 +132,7 @@ class BoardService {
}
public function deleteAcl($id) {
$this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_SHARE);
$acl = $this->aclMapper->find($id);
return $this->aclMapper->delete($acl);
}

20
lib/Service/CardService.php
View File

@@ -25,21 +25,31 @@ namespace OCA\Deck\Service;
use OCA\Deck\Db\Card;
use OCA\Deck\Db\CardMapper;
use OCA\Deck\Db\Acl;
use OCA\Deck\CardArchivedException;
use OCA\Deck\Db\StackMapper;
class CardService {
private $cardMapper;
public function __construct(CardMapper $cardMapper) {
public function __construct(
CardMapper $cardMapper,
StackMapper $stackMapper,
PermissionService $permissionService
) {
$this->cardMapper = $cardMapper;
$this->stackMapper = $stackMapper;
$this->permissionService = $permissionService;
}
public function find($cardId) {
$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ);
return $this->cardMapper->find($cardId);
}
public function create($title, $stackId, $type, $order, $owner) {
$this->permissionService->checkPermission($this->stackMapper, $stackId, Acl::PERMISSION_EDIT);
$card = new Card();
$card->setTitle($title);
$card->setStackId($stackId);
@@ -51,10 +61,12 @@ class CardService {
}
public function delete($id) {
$this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT);
return $this->cardMapper->delete($this->cardMapper->find($id));
}
public function update($id, $title, $stackId, $type, $order, $description, $owner) {
$this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT);
$card = $this->cardMapper->find($id);
if($card->getArchived()) {
throw new CardArchivedException();
@@ -69,6 +81,7 @@ class CardService {
}
public function rename($id, $title) {
$this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT);
$card = $this->cardMapper->find($id);
if($card->getArchived()) {
throw new CardArchivedException();
@@ -77,6 +90,7 @@ class CardService {
return $this->cardMapper->update($card);
}
public function reorder($id, $stackId, $order) {
$this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT);
$cards = $this->cardMapper->findAll($stackId);
$i = 0;
foreach ($cards as $card) {
@@ -102,18 +116,21 @@ class CardService {
}
public function archive($id) {
$this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT);
$card = $this->cardMapper->find($id);
$card->setArchived(true);
return $this->cardMapper->update($card);
}
public function unarchive($id) {
$this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT);
$card = $this->cardMapper->find($id);
$card->setArchived(false);
return $this->cardMapper->update($card);
}
public function assignLabel($cardId, $labelId) {
$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_EDIT);
$card = $this->cardMapper->find($cardId);
if($card->getArchived()) {
throw new CardArchivedException();
@@ -122,6 +139,7 @@ class CardService {
}
public function removeLabel($cardId, $labelId) {
$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_EDIT);
$card = $this->cardMapper->find($cardId);
if($card->getArchived()) {
throw new CardArchivedException();

16
lib/Service/LabelService.php
View File

@@ -24,8 +24,7 @@
namespace OCA\Deck\Service;
use OCA\Deck\Db\Label;
use OCP\ILogger;
use OCP\IL10N;
use OCA\Deck\Db\Acl;
use OCA\Deck\Db\LabelMapper;
@@ -34,19 +33,22 @@ class LabelService {
private $labelMapper;
private $logger;
public function __construct(ILogger $logger,
IL10N $l10n,
LabelMapper $labelMapper) {
public function __construct(
LabelMapper $labelMapper,
PermissionService $permissionService
) {
$this->labelMapper = $labelMapper;
$this->logger = $logger;
$this->permissionService = $permissionService;
}
public function find($labelId) {
$this->permissionService->checkPermission($this->labelMapper, $labelId, Acl::PERMISSION_READ);
$label = $this->labelMapper->find($labelId);
return $label;
}
public function create($title, $color, $boardId) {
$this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_MANAGE);
$label = new Label();
$label->setTitle($title);
$label->setColor($color);
@@ -55,10 +57,12 @@ class LabelService {
}
public function delete($id) {
$this->permissionService->checkPermission($this->labelMapper, $id, Acl::PERMISSION_MANAGE);
return $this->labelMapper->delete($this->find($id));
}
public function update($id, $title, $color) {
$this->permissionService->checkPermission($this->labelMapper, $id, Acl::PERMISSION_MANAGE);
$label = $this->find($id);
$label->setTitle($title);
$label->setColor($color);

35
lib/Service/PermissionService.php
View File

@@ -26,6 +26,11 @@ namespace OCA\Deck\Service;
use OCA\Deck\Db\Acl;
use OCA\Deck\Db\AclMapper;
use OCA\Deck\Db\BoardMapper;
use OCA\Deck\Db\Entity;
use OCA\Deck\Db\IPermissionMapper;
use OCA\Deck\NoPermissionException;
use OCA\Deck\NotFoundException;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\IGroupManager;
use OCP\ILogger;
@@ -84,6 +89,36 @@ class PermissionService {
return $this->userCan($acls, $permission);
}
/**
* check permissions for replacing dark magic middleware
*
* @param $mapper IPermissionMapper|null null if $id is a boardId
* @param $id int unique identifier of the Entity
* @param $permission int
* @return bool
* @throws NoPermissionException|NotFoundException
*/
public function checkPermission($mapper, $id, $permission) {
try {
if($mapper instanceof IPermissionMapper) {
$boardId = $mapper->findBoardId($id);
} else {
$boardId = $id;
}
if($boardId === null) {
throw new NotFoundException('No entity found');
}
if (!$this->getPermission($boardId, $permission)) {
$class = new \ReflectionClass($mapper);
$constants = array_flip($class->getConstants());
throw new NoPermissionException('Permission ' . $constants[$permission] . ' not granted.');
}
} catch (DoesNotExistException $exception) {
throw new NotFoundException('Permission denied');
}
return true;
}
/**
* @param $boardId
* @return bool

10
lib/Service/StackService.php
View File

@@ -23,6 +23,7 @@
namespace OCA\Deck\Service;
use OCA\Deck\Db\Acl;
use OCA\Deck\Db\CardMapper;
use OCA\Deck\Db\LabelMapper;
use OCP\ILogger;
@@ -41,17 +42,20 @@ class StackService {
private $cardMapper;
private $logger;
private $labelMapper;
private $permissionService;
public function __construct(StackMapper $stackMapper, CardMapper $cardMapper, LabelMapper $labelMapper, ILogger $logger,
IL10N $l10n,
ITimeFactory $timeFactory) {
ITimeFactory $timeFactory, PermissionService $permissionService) {
$this->stackMapper = $stackMapper;
$this->cardMapper = $cardMapper;
$this->labelMapper = $labelMapper;
$this->logger = $logger;
$this->permissionService = $permissionService;
}
public function findAll($boardId) {
$this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_READ);
$stacks = $this->stackMapper->findAll($boardId);
$labels = $this->labelMapper->getAssignedLabelsForBoard($boardId);
foreach ($stacks as $stackIndex => $stack) {
@@ -67,6 +71,7 @@ class StackService {
}
public function findAllArchived($boardId) {
$this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_READ);
$stacks = $this->stackMapper->findAll($boardId);
$labels = $this->labelMapper->getAssignedLabelsForBoard($boardId);
foreach ($stacks as $stackIndex => $stack) {
@@ -82,6 +87,7 @@ class StackService {
}
public function create($title, $boardId, $order) {
$this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_MANAGE);
$stack = new Stack();
$stack->setTitle($title);
$stack->setBoardId($boardId);
@@ -91,10 +97,12 @@ class StackService {
}
public function delete($id) {
$this->permissionService->checkPermission($this->stackMapper, $id, Acl::PERMISSION_MANAGE);
return $this->stackMapper->delete($this->stackMapper->find($id));
}
public function update($id, $title, $boardId, $order) {
$this->permissionService->checkPermission($this->stackMapper, $id, Acl::PERMISSION_MANAGE);
$stack = $this->stackMapper->find($id);
$stack->setTitle($title);
$stack->setBoardId($boardId);

62
tests/unit/Middleware/SharingMiddlewareTest.php
View File

@@ -71,68 +71,6 @@ class SharingMiddlewareTest extends \PHPUnit_Framework_TestCase {
);
}
public function dataBeforeController() {
return [
['GET', '\OCA\Deck\Controller\PageController', 'index', false, true, 123],
['GET', '\OCA\Deck\Controller\BoardController', 'index', false, true, 123],
['GET', '\OCA\Deck\Controller\BoardController', 'read', true, true, 123],
['GET', '\OCA\Deck\Controller\BoardController', 'read', true, true, 123, NoPermissionException::class],
['GET', '\OCA\Deck\Controller\BoardController', 'read', false, false, null, NotFoundException::class],
['GET', '\OCA\Deck\Controller\CardController', 'read', true, true, 123, NoPermissionException::class],
['POST', '\OCA\Deck\Controller\CardController', 'reorder', true, true, 123, NoPermissionException::class],
['PUT', '\OCA\Deck\Controller\BoardController', 'update', true, false, 123, NoPermissionException::class],
];
}
/**
* @dataProvider dataBeforeController
* @param $controllerClass
* @param $methodName
*/
public function testBeforeController($method, $controllerClass, $methodName, $getPermission, $success, $boardId, $exception=null) {
$controller = $this->getMockBuilder($controllerClass)
->disableOriginalConstructor()->getMock();
$mapper = $this->getMockBuilder(IPermissionMapper::class)
->disableOriginalConstructor()->getMock();
$mapper->expects($this->any())->method('findBoardId')->willReturn($boardId);
$mapper->expects($this->any())->method('isOwner')->willReturn(false);
$user = $this->getMockBuilder(IUser::class)
->disableOriginalConstructor()->getMock();
$user->expects($this->once())->method('getUID')->willReturn('user1');
$controllerInstance = \OC::$server->query($controllerClass);
$this->reflector->reflect($controllerInstance, $methodName);
$this->container->expects($this->any())
->method('query')->willReturn($mapper);
$this->userSession->expects($this->exactly(2))->method('getUser')->willReturn($user);
$this->request->expects($this->once())->method('getMethod')->willReturn($method);
if($getPermission === true) {
$this->permissionService
->expects($this->once())
->method('getPermission')
->willReturn($getPermission);
}
if($success) {
$this->sharingMiddleware->beforeController($controller, $methodName);
} else {
try {
$this->sharingMiddleware->beforeController($controller, $methodName);
} catch (\Exception $e) {
$this->assertInstanceOf($exception, $e);
}
}
}
public function setUpPermissions() {
$this->permissionService->expects($this->once())
->method('getPermission')
->with(123, Acl::PERMISSION_READ)
->willReturn(true);
}
public function dataAfterException() {
return [

4
tests/unit/Service/BoardServiceTest.php
View File

@@ -57,7 +57,7 @@ class BoardServiceTest extends \PHPUnit_Framework_TestCase {
->disableOriginalConstructor()->getMock();
$this->labelMapper = $this->getMockBuilder(LabelMapper::class)
->disableOriginalConstructor()->getMock();
$this->groupManager = $this->getMockBuilder(IGroupManager::class)
$this->permissionService = $this->getMockBuilder(PermissionService::class)
->disableOriginalConstructor()->getMock();
$this->service = new BoardService(
@@ -66,7 +66,7 @@ class BoardServiceTest extends \PHPUnit_Framework_TestCase {
$this->l10n,
$this->labelMapper,
$this->aclMapper,
$this->groupManager
$this->permissionService
);
}