diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 8f21f5dc0..9ea311095 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -24,6 +24,7 @@ namespace OCA\Deck\Db; use OC\Cache\CappedMemoryCache; +use OCA\Deck\Service\CirclesService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\IDBConnection; use OCP\IUserManager; @@ -36,10 +37,10 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { private $stackMapper; private $userManager; private $groupManager; + private $circlesService; private $logger; - private $circlesEnabled; - + /** @var CappedMemoryCache */ private $userBoardCache; public function __construct( @@ -49,6 +50,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { StackMapper $stackMapper, IUserManager $userManager, IGroupManager $groupManager, + CirclesService $circlesService, LoggerInterface $logger ) { parent::__construct($db, 'deck_boards', Board::class); @@ -57,12 +59,10 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { $this->stackMapper = $stackMapper; $this->userManager = $userManager; $this->groupManager = $groupManager; + $this->circlesService = $circlesService; $this->logger = $logger; $this->userBoardCache = new CappedMemoryCache(); - - - $this->circlesEnabled = \OC::$server->getAppManager()->isEnabledForUser('circles'); } @@ -181,12 +181,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { } public function findAllByCircles($userId, $limit = null, $offset = null, $since = -1,$includeArchived = true) { - if (!$this->circlesEnabled) { - return []; - } - $circles = array_map(function ($circle) { - return $circle->getUniqueId(); - }, \OCA\Circles\Api\v1\Circles::joinedCircles($userId, true)); + $circles = $this->circlesService->getUserCircles($userId); if (count($circles) === 0) { return []; } @@ -277,11 +272,11 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { return null; } if ($acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) { - if (!$this->circlesEnabled) { + if (!$this->circlesService->isCirclesEnabled()) { return null; } try { - $circle = \OCA\Circles\Api\v1\Circles::detailsCircle($acl->getParticipant(), true); + $circle = $this->circlesService->getCircle($acl->getParticipant()); if ($circle) { return new Circle($circle); } diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 5d228b73d..729570ab1 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -37,6 +37,7 @@ use OCA\Deck\Db\StackMapper; use OCA\Deck\Event\CardCreatedEvent; use OCA\Deck\Event\CardDeletedEvent; use OCA\Deck\Event\CardUpdatedEvent; +use OCA\Deck\NoPermissionException; use OCA\Deck\Notification\NotificationHelper; use OCA\Deck\Db\BoardMapper; use OCA\Deck\Db\LabelMapper; @@ -154,7 +155,12 @@ class CardService { } public function findCalendarEntries($boardId) { - $this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ); + try { + $this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ); + } catch (NoPermissionException $e) { + \OC::$server->getLogger()->error('Unable to check permission for a previously obtained board ' . $boardId, ['exception' => $e]); + return []; + } $cards = $this->cardMapper->findCalendarEntries($boardId); foreach ($cards as $card) { $this->enrich($card); diff --git a/lib/Service/CirclesService.php b/lib/Service/CirclesService.php index 7f5157a15..6a471b41d 100644 --- a/lib/Service/CirclesService.php +++ b/lib/Service/CirclesService.php @@ -27,8 +27,11 @@ declare(strict_types=1); namespace OCA\Deck\Service; use OCA\Circles\CirclesManager; +use OCA\Circles\Model\Circle; use OCA\Circles\Model\Member; +use OCA\Circles\Model\Probes\CircleProbe; use OCP\App\IAppManager; +use Throwable; /** * Wrapper around circles app API since it is not in a public namespace so we need to make sure that @@ -45,15 +48,24 @@ class CirclesService { return $this->circlesEnabled; } - public function getCircle($circleId) { + public function getCircle(string $circleId): ?Circle { if (!$this->circlesEnabled) { return null; } - return \OCA\Circles\Api\v1\Circles::detailsCircle($circleId, true); + try { + + // Enforce current user condition since we always want the full list of members + /** @var CirclesManager $circlesManager */ + $circlesManager = \OC::$server->get(CirclesManager::class); + $circlesManager->startSuperSession(); + return $circlesManager->getCircle($circleId); + } catch (Throwable $e) { + } + return null; } - public function isUserInCircle($circleId, $userId): bool { + public function isUserInCircle(string $circleId, string $userId): bool { if (!$this->circlesEnabled) { return false; } @@ -66,8 +78,32 @@ class CirclesService { $circle = $circlesManager->getCircle($circleId); $member = $circle->getInitiator(); return $member !== null && $member->getLevel() >= Member::LEVEL_MEMBER; - } catch (\Exception $e) { + } catch (Throwable $e) { } return false; } + + /** + * @param string $userId + * @return string[] circle single ids + */ + public function getUserCircles(string $userId): array { + if (!$this->circlesEnabled) { + return []; + } + + try { + /** @var CirclesManager $circlesManager */ + $circlesManager = \OC::$server->get(CirclesManager::class); + $federatedUser = $circlesManager->getFederatedUser($userId, Member::TYPE_USER); + $circlesManager->startSession($federatedUser); + $probe = new CircleProbe(); + $probe->mustBeMember(); + return array_map(function (Circle $circle) { + return $circle->getSingleId(); + }, $circlesManager->getCircles($probe)); + } catch (Throwable $e) { + } + return []; + } } diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 347863ece..e8206e774 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -280,14 +280,14 @@ class PermissionService { if ($this->circlesService->isCirclesEnabled() && $acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) { try { - $circle = \OCA\Circles\Api\v1\Circles::detailsCircle($acl->getParticipant(), true); + $circle = $this->circlesService->getCircle($acl->getParticipant()); if ($circle === null) { $this->logger->info('No circle found for acl rule ' . $acl->getId()); continue; } foreach ($circle->getInheritedMembers() as $member) { - if ($member->getUserType() !== 1 || $member->getLevel() >= Member::LEVEL_MEMBER) { + if ($member->getUserType() !== 1 || $member->getLevel() < Member::LEVEL_MEMBER) { // deck currently only supports user members in circles continue; } diff --git a/lib/Service/StackService.php b/lib/Service/StackService.php index 93de14018..ae0a72af7 100644 --- a/lib/Service/StackService.php +++ b/lib/Service/StackService.php @@ -35,6 +35,7 @@ use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Db\LabelMapper; use OCA\Deck\Db\Stack; use OCA\Deck\Db\StackMapper; +use OCA\Deck\NoPermissionException; use OCA\Deck\StatusException; class StackService { @@ -142,7 +143,12 @@ class StackService { } public function findCalendarEntries($boardId) { - $this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_READ); + try { + $this->permissionService->checkPermission(null, $boardId, Acl::PERMISSION_READ); + } catch (NoPermissionException $e) { + \OC::$server->getLogger()->error('Unable to check permission for a previously obtained board ' . $boardId, ['exception' => $e]); + return []; + } return $this->stackMapper->findAll($boardId); } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index e3710d52f..2591a2f5c 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -105,10 +105,6 @@ $boardId - - \OCA\Circles\Api\v1\Circles - \OCA\Circles\Api\v1\Circles - @@ -205,9 +201,11 @@ - \OCA\Circles\Api\v1\Circles + ?Circle - + + $circlesManager + $circlesManager $circlesManager @@ -262,8 +260,8 @@ + $circle Member - \OCA\Circles\Api\v1\Circles diff --git a/tests/unit/Db/AclMapperTest.php b/tests/unit/Db/AclMapperTest.php index f720fdb1a..5c0a08471 100644 --- a/tests/unit/Db/AclMapperTest.php +++ b/tests/unit/Db/AclMapperTest.php @@ -23,6 +23,7 @@ namespace OCA\Deck\Db; +use OCA\Deck\Service\CirclesService; use OCP\IGroupManager; use OCP\IUserManager; use Psr\Log\LoggerInterface; @@ -56,6 +57,7 @@ class AclMapperTest extends MapperTestUtility { \OC::$server->query(StackMapper::class), $this->userManager, $this->groupManager, + $this->createMock(CirclesService::class), $this->createMock(LoggerInterface::class) ); diff --git a/tests/unit/Db/BoardMapperTest.php b/tests/unit/Db/BoardMapperTest.php index 78cc0cccc..2f6ba5837 100644 --- a/tests/unit/Db/BoardMapperTest.php +++ b/tests/unit/Db/BoardMapperTest.php @@ -23,6 +23,7 @@ namespace OCA\Deck\Db; +use OCA\Deck\Service\CirclesService; use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IUserManager; @@ -63,6 +64,7 @@ class BoardMapperTest extends MapperTestUtility { \OC::$server->query(StackMapper::class), $this->userManager, $this->groupManager, + $this->createMock(CirclesService::class), $this->createMock(LoggerInterface::class) ); $this->aclMapper = \OC::$server->query(AclMapper::class);