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

Merge pull request #3224 from nextcloud/backport/3217/stable22

Browse Source 
[stable22] Move circle checks to a unified service and improve member checks
This commit is contained in:
Julius Härtl
2021-08-03 11:41:03 +02:00
committed by GitHub
parent 5f4aa017b6 958d50d9b7
commit 74f0106718
4 changed files with 34 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
lib/Service/CirclesService.php
View File

@@ -26,6 +26,8 @@ declare(strict_types=1);
namespace OCA\Deck\Service; namespace OCA\Deck\Service;
use OCA\Circles\CirclesManager;
use OCA\Circles\Model\Member;
use OCP\App\IAppManager; use OCP\App\IAppManager;
/** /**
@@ -39,6 +41,10 @@ class CirclesService {
$this->circlesEnabled = $appManager->isEnabledForUser('circles'); $this->circlesEnabled = $appManager->isEnabledForUser('circles');
} }
public function isCirclesEnabled(): bool {
return $this->circlesEnabled;
}
public function getCircle($circleId) { public function getCircle($circleId) {
if (!$this->circlesEnabled) { if (!$this->circlesEnabled) {
return null; return null;
@@ -53,8 +59,13 @@ class CirclesService {
} }
try { try {
\OCA\Circles\Api\v1\Circles::getMember($circleId, $userId, 1, true); /** @var CirclesManager $circlesManager */
return true; $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) { } catch (\Exception $e) {
} }
return false; return false;

18
lib/Service/PermissionService.php
View File

@@ -24,6 +24,7 @@
namespace OCA\Deck\Service; namespace OCA\Deck\Service;
use OC\Cache\CappedMemoryCache; use OC\Cache\CappedMemoryCache;
use OCA\Circles\Model\Member;
use OCA\Deck\Db\Acl; use OCA\Deck\Db\Acl;
use OCA\Deck\Db\AclMapper; use OCA\Deck\Db\AclMapper;
use OCA\Deck\Db\Board; use OCA\Deck\Db\Board;
@@ -42,6 +43,8 @@ use OCP\Share\IManager;
class PermissionService { class PermissionService {
/** @var CirclesService */
private $circlesService;
/** @var BoardMapper */ /** @var BoardMapper */
private $boardMapper; private $boardMapper;
/** @var AclMapper */ /** @var AclMapper */
@@ -61,11 +64,11 @@ class PermissionService {
/** @var array */ /** @var array */
private $users = []; private $users = [];
private $circlesEnabled = false;
private $boardCache; private $boardCache;
public function __construct( public function __construct(
ILogger $logger, ILogger $logger,
CirclesService $circlesService,
AclMapper $aclMapper, AclMapper $aclMapper,
BoardMapper $boardMapper, BoardMapper $boardMapper,
IUserManager $userManager, IUserManager $userManager,
@@ -74,6 +77,7 @@ class PermissionService {
IConfig $config, IConfig $config,
$userId $userId
) { ) {
$this->circlesService = $circlesService;
$this->aclMapper = $aclMapper; $this->aclMapper = $aclMapper;
$this->boardMapper = $boardMapper; $this->boardMapper = $boardMapper;
$this->logger = $logger; $this->logger = $logger;
@@ -84,9 +88,6 @@ class PermissionService {
$this->userId = $userId; $this->userId = $userId;
$this->boardCache = new CappedMemoryCache(); $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); return $acl->getPermission($permission);
} }
if ($this->circlesEnabled && $acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) { if ($this->circlesService->isCirclesEnabled() && $acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) {
try { try {
\OCA\Circles\Api\v1\Circles::getMember($acl->getParticipant(), $this->userId, 1, true); return $this->circlesService->isUserInCircle($acl->getParticipant(), $userId) && $acl->getPermission($permission);
return $acl->getPermission($permission);
} catch (\Exception $e) { } catch (\Exception $e) {
$this->logger->info('Member not found in circle that was accessed. This should not happen.'); $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 { try {
$circle = \OCA\Circles\Api\v1\Circles::detailsCircle($acl->getParticipant(), true); $circle = \OCA\Circles\Api\v1\Circles::detailsCircle($acl->getParticipant(), true);
if ($circle === null) { if ($circle === null) {
@@ -287,7 +287,7 @@ class PermissionService {
} }
foreach ($circle->getInheritedMembers() as $member) { 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 // deck currently only supports user members in circles
continue; continue;
} }

13
tests/psalm-baseline.xml
View File

@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?> <?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="4.7.3@38c452ae584467e939d55377aaf83b5a26f19dd1"> <files psalm-version="4.8.1@f73f2299dbc59a3e6c4d66cff4605176e728ee69">
<file src="lib/Activity/ActivityManager.php"> <file src="lib/Activity/ActivityManager.php">
<TypeDoesNotContainType occurrences="1"> <TypeDoesNotContainType occurrences="1">
<code>$message !== null</code> <code>$message !== null</code>
@@ -147,8 +147,7 @@
<UndefinedClass occurrences="1"> <UndefinedClass occurrences="1">
<code>\OCA\Circles\Model\Circle</code> <code>\OCA\Circles\Model\Circle</code>
</UndefinedClass> </UndefinedClass>
<UndefinedDocblockClass occurrences="5"> <UndefinedDocblockClass occurrences="4">
<code>$this-&gt;object</code>
<code>$this-&gt;object</code> <code>$this-&gt;object</code>
<code>$this-&gt;object</code> <code>$this-&gt;object</code>
<code>$this-&gt;object</code> <code>$this-&gt;object</code>
@@ -202,10 +201,12 @@
</UndefinedDocblockClass> </UndefinedDocblockClass>
</file> </file>
<file src="lib/Service/CirclesService.php"> <file src="lib/Service/CirclesService.php">
<UndefinedClass occurrences="2"> <UndefinedClass occurrences="1">
<code>\OCA\Circles\Api\v1\Circles</code>
<code>\OCA\Circles\Api\v1\Circles</code> <code>\OCA\Circles\Api\v1\Circles</code>
</UndefinedClass> </UndefinedClass>
<UndefinedDocblockClass occurrences="1">
<code>$circlesManager</code>
</UndefinedDocblockClass>
</file> </file>
<file src="lib/Service/CommentService.php"> <file src="lib/Service/CommentService.php">
<UndefinedThisPropertyAssignment occurrences="2"> <UndefinedThisPropertyAssignment occurrences="2">
@@ -258,7 +259,7 @@
</file> </file>
<file src="lib/Service/PermissionService.php"> <file src="lib/Service/PermissionService.php">
<UndefinedClass occurrences="2"> <UndefinedClass occurrences="2">
<code>\OCA\Circles\Api\v1\Circles</code> <code>Member</code>
<code>\OCA\Circles\Api\v1\Circles</code> <code>\OCA\Circles\Api\v1\Circles</code>
</UndefinedClass> </UndefinedClass>
</file> </file>

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

@@ -35,6 +35,7 @@ use OCP\IConfig;
use OCP\IGroup; use OCP\IGroup;
use OCP\IGroupManager; use OCP\IGroupManager;
use OCP\ILogger; use OCP\ILogger;
use OCP\IRequest;
use OCP\IUser; use OCP\IUser;
use OCP\IUserManager; use OCP\IUserManager;
use OCP\Share\IManager; use OCP\Share\IManager;
@@ -62,7 +63,9 @@ class PermissionServiceTest extends \Test\TestCase {
public function setUp(): void { public function setUp(): void {
parent::setUp(); 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->aclMapper = $this->createMock(AclMapper::class);
$this->boardMapper = $this->createMock(BoardMapper::class); $this->boardMapper = $this->createMock(BoardMapper::class);
$this->userManager = $this->createMock(IUserManager::class); $this->userManager = $this->createMock(IUserManager::class);
@@ -72,6 +75,7 @@ class PermissionServiceTest extends \Test\TestCase {
$this->service = new PermissionService( $this->service = new PermissionService(
$this->logger, $this->logger,
$this->circlesService,
$this->aclMapper, $this->aclMapper,
$this->boardMapper, $this->boardMapper,
$this->userManager, $this->userManager,