From 958d50d9b72e995e9e580dcf5cca9f274f2cd1f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 12 Jul 2021 17:55:53 +0200 Subject: [PATCH] Move circle checks to a unified service and improve member checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Service/CirclesService.php | 15 +++++++++++++-- lib/Service/PermissionService.php | 18 +++++++++--------- tests/psalm-baseline.xml | 13 +++++++------ tests/unit/Service/PermissionServiceTest.php | 6 +++++- 4 files changed, 34 insertions(+), 18 deletions(-) diff --git a/lib/Service/CirclesService.php b/lib/Service/CirclesService.php index 4df7a7987..7f5157a15 100644 --- a/lib/Service/CirclesService.php +++ b/lib/Service/CirclesService.php @@ -26,6 +26,8 @@ declare(strict_types=1); namespace OCA\Deck\Service; +use OCA\Circles\CirclesManager; +use OCA\Circles\Model\Member; use OCP\App\IAppManager; /** @@ -39,6 +41,10 @@ class CirclesService { $this->circlesEnabled = $appManager->isEnabledForUser('circles'); } + public function isCirclesEnabled(): bool { + return $this->circlesEnabled; + } + public function getCircle($circleId) { if (!$this->circlesEnabled) { return null; @@ -53,8 +59,13 @@ class CirclesService { } try { - \OCA\Circles\Api\v1\Circles::getMember($circleId, $userId, 1, true); - return true; + /** @var CirclesManager $circlesManager */ + $circlesManager = \OC::$server->get(CirclesManager::class); + $federatedUser = $circlesManager->getFederatedUser($userId, Member::TYPE_USER); + $circlesManager->startSession($federatedUser); + $circle = $circlesManager->getCircle($circleId); + $member = $circle->getInitiator(); + return $member !== null && $member->getLevel() >= Member::LEVEL_MEMBER; } catch (\Exception $e) { } return false; diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index ff4191e82..e5a6edfcd 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -24,6 +24,7 @@ namespace OCA\Deck\Service; use OC\Cache\CappedMemoryCache; +use OCA\Circles\Model\Member; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; use OCA\Deck\Db\Board; @@ -42,6 +43,8 @@ use OCP\Share\IManager; class PermissionService { + /** @var CirclesService */ + private $circlesService; /** @var BoardMapper */ private $boardMapper; /** @var AclMapper */ @@ -61,11 +64,11 @@ class PermissionService { /** @var array */ private $users = []; - private $circlesEnabled = false; private $boardCache; public function __construct( ILogger $logger, + CirclesService $circlesService, AclMapper $aclMapper, BoardMapper $boardMapper, IUserManager $userManager, @@ -74,6 +77,7 @@ class PermissionService { IConfig $config, $userId ) { + $this->circlesService = $circlesService; $this->aclMapper = $aclMapper; $this->boardMapper = $boardMapper; $this->logger = $logger; @@ -84,9 +88,6 @@ class PermissionService { $this->userId = $userId; $this->boardCache = new CappedMemoryCache(); - - $this->circlesEnabled = \OC::$server->getAppManager()->isEnabledForUser('circles') && - (version_compare(\OC::$server->getAppManager()->getAppVersion('circles'), '0.17.1') >= 0); } /** @@ -210,10 +211,9 @@ class PermissionService { return $acl->getPermission($permission); } - if ($this->circlesEnabled && $acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) { + if ($this->circlesService->isCirclesEnabled() && $acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) { try { - \OCA\Circles\Api\v1\Circles::getMember($acl->getParticipant(), $this->userId, 1, true); - return $acl->getPermission($permission); + return $this->circlesService->isUserInCircle($acl->getParticipant(), $userId) && $acl->getPermission($permission); } catch (\Exception $e) { $this->logger->info('Member not found in circle that was accessed. This should not happen.'); } @@ -278,7 +278,7 @@ class PermissionService { } } - if ($this->circlesEnabled && $acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) { + if ($this->circlesService->isCirclesEnabled() && $acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) { try { $circle = \OCA\Circles\Api\v1\Circles::detailsCircle($acl->getParticipant(), true); if ($circle === null) { @@ -287,7 +287,7 @@ class PermissionService { } foreach ($circle->getInheritedMembers() as $member) { - if ($member->getUserType() !== 1) { + if ($member->getUserType() !== 1 || $member->getLevel() >= Member::LEVEL_MEMBER) { // deck currently only supports user members in circles continue; } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index d8fd03aea..aed23e9b9 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,5 +1,5 @@ - + $message !== null @@ -147,8 +147,7 @@ \OCA\Circles\Model\Circle - - $this->object + $this->object $this->object $this->object @@ -202,10 +201,12 @@ - - \OCA\Circles\Api\v1\Circles + \OCA\Circles\Api\v1\Circles + + $circlesManager + @@ -258,7 +259,7 @@ - \OCA\Circles\Api\v1\Circles + Member \OCA\Circles\Api\v1\Circles diff --git a/tests/unit/Service/PermissionServiceTest.php b/tests/unit/Service/PermissionServiceTest.php index 28601ed01..f92adc059 100644 --- a/tests/unit/Service/PermissionServiceTest.php +++ b/tests/unit/Service/PermissionServiceTest.php @@ -35,6 +35,7 @@ use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; use OCP\ILogger; +use OCP\IRequest; use OCP\IUser; use OCP\IUserManager; use OCP\Share\IManager; @@ -62,7 +63,9 @@ class PermissionServiceTest extends \Test\TestCase { public function setUp(): void { parent::setUp(); - $this->logger = $this->request = $this->createMock(ILogger::class); + $this->logger = $this->createMock(ILogger::class); + $this->request = $this->createMock(IRequest::class); + $this->circlesService = $this->createMock(CirclesService::class); $this->aclMapper = $this->createMock(AclMapper::class); $this->boardMapper = $this->createMock(BoardMapper::class); $this->userManager = $this->createMock(IUserManager::class); @@ -72,6 +75,7 @@ class PermissionServiceTest extends \Test\TestCase { $this->service = new PermissionService( $this->logger, + $this->circlesService, $this->aclMapper, $this->boardMapper, $this->userManager,