From a6b6842e2be49a82d31b5dbb25d102560b9eeb0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 3 May 2017 12:05:15 +0200 Subject: [PATCH 1/3] No not fail on nonexisting acl users/groups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- appinfo/info.xml | 7 ++- lib/AppInfo/Application.php | 31 ++++++++++++- lib/Db/AclMapper.php | 5 ++ lib/Db/Board.php | 1 - lib/Db/BoardMapper.php | 21 ++++++++- lib/Migration/UnknownUsers.php | 83 ++++++++++++++++++++++++++++++++++ 6 files changed, 143 insertions(+), 5 deletions(-) create mode 100644 lib/Migration/UnknownUsers.php diff --git a/appinfo/info.xml b/appinfo/info.xml index e9ac89995..df6da2ec0 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -16,7 +16,7 @@ 💥 This is still alpha software: it may not be stable enough for production! - 0.1.3 + 0.1.3.1 agpl Julius Härtl Deck @@ -30,4 +30,9 @@ + + + OCA\Deck\Migration\UnknownUsers + + diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index befeb8f6e..8572102ee 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -23,9 +23,14 @@ namespace OCA\Deck\AppInfo; +use OCA\Deck\Db\Acl; +use OCA\Deck\Db\AclMapper; use OCP\AppFramework\App; use OCA\Deck\Middleware\SharingMiddleware; - +use OCP\IGroup; +use OCP\IGroupManager; +use OCP\IUser; +use OCP\IUserManager; class Application extends App { @@ -48,6 +53,30 @@ class Application extends App { }); $container->registerMiddleware('SharingMiddleware'); + // Delete user/group acl entries when they get deleted + /** @var IUserManager $userManager */ + $userManager = $server->getUserManager(); + $userManager->listen('\OC\User', 'postDelete', function(IUser $user) use ($container) { + /** @var AclMapper $aclMapper */ + $aclMapper = $container->query(AclMapper::class); + $acls = $aclMapper->findByParticipant(Acl::PERMISSION_TYPE_USER, $user->getUID()); + foreach ($acls as $acl) { + $aclMapper->delete($acl); + } + }); + + /** @var IUserManager $userManager */ + $groupManager = $server->getGroupManager(); + $groupManager->listen('\OC\Group', 'postDelete', function(IGroup $group) use ($container) { + /** @var AclMapper $aclMapper */ + $aclMapper = $container->query(AclMapper::class); + $aclMapper->findByParticipant(Acl::PERMISSION_TYPE_GROUP, $group->getGID()); + $acls = $aclMapper->findByParticipant(Acl::PERMISSION_TYPE_GROUP, $group->getGID()); + foreach ($acls as $acl) { + $aclMapper->delete($acl); + } + }); + } public function registerNavigationEntry() { diff --git a/lib/Db/AclMapper.php b/lib/Db/AclMapper.php index bd1ba187c..2edc5d497 100644 --- a/lib/Db/AclMapper.php +++ b/lib/Db/AclMapper.php @@ -48,4 +48,9 @@ class AclMapper extends DeckMapper implements IPermissionMapper { return $entity->getBoardId(); } + public function findByParticipant($type, $participant) { + $sql = 'SELECT * from *PREFIX*deck_board_acl WHERE type = ? AND participant = ?'; + return $this->findEntities($sql, [$type, $participant]); + } + } diff --git a/lib/Db/Board.php b/lib/Db/Board.php index 868b54fde..4d3b5e42d 100644 --- a/lib/Db/Board.php +++ b/lib/Db/Board.php @@ -52,7 +52,6 @@ class Board extends RelationalEntity implements JsonSerializable { if ($this->shared === -1) { unset($json['shared']); } - $json['owner'] = $this->resolveOwner(); return $json; } diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 9d982070d..5de370094 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -130,6 +130,11 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { return $entries; } + public function findAll() { + $sql = 'SELECT id from *PREFIX*deck_boards;'; + return $this->findEntities($sql, []); + } + public function delete(/** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */ \OCP\AppFramework\Db\Entity $entity) { // delete acl @@ -166,10 +171,22 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { $groupManager = $this->groupManager; $acl->resolveRelation('participant', function($participant) use (&$acl, &$userManager, &$groupManager) { if($acl->getType() === Acl::PERMISSION_TYPE_USER) { - return new User($userManager->get($acl->getParticipant($participant))); + $user = $userManager->get($participant); + if($user !== null) { + return new User($user); + } else { + \OC::$server->getLogger()->debug('User ' . $acl->getId() . ' not found when mapping acl ' . $acl->getParticipant()); + return $participant; + } } if($acl->getType() === Acl::PERMISSION_TYPE_GROUP) { - return new Group($groupManager->get($acl->getParticipant($participant))); + $group = $groupManager->get($participant); + if($group !== null) { + return new Group($group); + } else { + \OC::$server->getLogger()->debug('Group ' . $acl->getId() . ' not found when mapping acl ' . $acl->getParticipant()); + return $participant; + } } throw new \Exception('Unknown permission type for mapping Acl'); }); diff --git a/lib/Migration/UnknownUsers.php b/lib/Migration/UnknownUsers.php new file mode 100644 index 000000000..7298d9e98 --- /dev/null +++ b/lib/Migration/UnknownUsers.php @@ -0,0 +1,83 @@ + + * + * @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\Migration; + +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\IUserManager; +use OCP\Migration\IRepairStep; +use OCP\Migration\IOutput; + +class UnknownUsers implements IRepairStep { + + private $userManager; + private $groupManager; + private $aclMapper; + private $boardMapper; + + public function __construct(IUserManager $userManager, IGroupManager $groupManager, AclMapper $aclMapper, BoardMapper $boardMapper) { + $this->userManager = $userManager; + $this->groupManager = $groupManager; + $this->aclMapper = $aclMapper; + $this->boardMapper = $boardMapper; + } + + /* + * @inheritdoc + */ + public function getName() { + return 'Delete orphaned ACL rules'; + } + + /** + * @inheritdoc + */ + public function run(IOutput $output) { + $boards = $this->boardMapper->findAll(); + /** @var Board $board */ + foreach ($boards as $board) { + $acls = $this->aclMapper->findAll($board->getId()); + /** @var Acl $acl */ + foreach ($acls as $acl) { + if($acl->getType() === Acl::PERMISSION_TYPE_USER) { + $user = $this->userManager->get($acl->getParticipant()); + if($user === null) { + $this->aclMapper->delete($acl); + } + } + if($acl->getType() === Acl::PERMISSION_TYPE_GROUP) { + $group = $this->groupManager->get($acl->getParticipant()); + if($group === null) { + $this->aclMapper->delete($acl); + } + } + + } + } + } +} From 860cbab1d30734fd102a2d2678ab382f57f9da50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 4 May 2017 11:26:45 +0200 Subject: [PATCH 2/3] Add proper error handling when user/group cannot be found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Db/BoardMapper.php | 10 +++++++--- lib/Db/CardMapper.php | 7 ++++++- lib/Db/RelationalObject.php | 3 ++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 5de370094..e1b2d0598 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -176,7 +176,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { return new User($user); } else { \OC::$server->getLogger()->debug('User ' . $acl->getId() . ' not found when mapping acl ' . $acl->getParticipant()); - return $participant; + return null; } } if($acl->getType() === Acl::PERMISSION_TYPE_GROUP) { @@ -185,7 +185,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { return new Group($group); } else { \OC::$server->getLogger()->debug('Group ' . $acl->getId() . ' not found when mapping acl ' . $acl->getParticipant()); - return $participant; + return null; } } throw new \Exception('Unknown permission type for mapping Acl'); @@ -198,7 +198,11 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { public function mapOwner(Board &$board) { $userManager = $this->userManager; $board->resolveRelation('owner', function($owner) use (&$userManager) { - return new User($userManager->get($owner)); + $user = $userManager->get($owner); + if($user !== null) { + return new User($user); + } + return null; }); } diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index 1542a95ea..e5f4fa284 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -31,6 +31,7 @@ use OCP\IUserManager; class CardMapper extends DeckMapper implements IPermissionMapper { private $labelMapper; + private $userManager; public function __construct(IDBConnection $db, LabelMapper $labelMapper, IUserManager $userManager) { parent::__construct($db, 'deck_cards', '\OCA\Deck\Db\Card'); @@ -131,7 +132,11 @@ class CardMapper extends DeckMapper implements IPermissionMapper { public function mapOwner(Card &$card) { $userManager = $this->userManager; $card->resolveRelation('owner', function($owner) use (&$userManager) { - return new User($userManager->get($owner)); + $user = $userManager->get($owner); + if($user !== null) { + return new User($user); + } + return null; }); } diff --git a/lib/Db/RelationalObject.php b/lib/Db/RelationalObject.php index e5740e754..d5a6fc18e 100644 --- a/lib/Db/RelationalObject.php +++ b/lib/Db/RelationalObject.php @@ -25,7 +25,8 @@ namespace OCA\Deck\Db; class RelationalObject implements \JsonSerializable { - private $primaryKey; + protected $primaryKey; + protected $object; /** * RelationalObject constructor. From 12ebffb88572f0b75ae1e8538fb29af78058c28a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 4 May 2017 12:10:17 +0200 Subject: [PATCH 3/3] Do not use getDisplayName for groups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit it is not supported in Nextcloud 11 https://github.com/nextcloud/server/pull/4690 https://github.com/nextcloud/server/pull/2833 Signed-off-by: Julius Härtl --- lib/Db/Group.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Db/Group.php b/lib/Db/Group.php index f668282b7..4b23fae19 100644 --- a/lib/Db/Group.php +++ b/lib/Db/Group.php @@ -35,7 +35,7 @@ class Group extends RelationalObject { public function getObjectSerialization() { return [ 'uid' => $this->object->getGID(), - 'displayname' => $this->object->getDisplayName() + 'displayname' => $this->object->getGID() ]; } } \ No newline at end of file