From 76281fc83074200f3455dfd91062066d193131a6 Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Fri, 18 Feb 2022 11:09:31 -0100 Subject: [PATCH] fixing moderator overview on reshares Signed-off-by: Maxence Lange --- .../lib/Controller/ShareAPIController.php | 49 ++++++++++++++----- .../Controller/ShareAPIControllerTest.php | 7 +++ build/psalm-baseline.xml | 8 +-- 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index fef71a868d54d..4242528af2b83 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -44,9 +44,9 @@ */ namespace OCA\Files_Sharing\Controller; +use OCA\Files\Helper; use OCA\Files_Sharing\Exceptions\SharingRightsException; use OCA\Files_Sharing\External\Storage; -use OCA\Files\Helper; use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSBadRequestException; @@ -56,9 +56,9 @@ use OCP\AppFramework\OCSController; use OCP\AppFramework\QueryException; use OCP\Constants; +use OCP\Files\Folder; use OCP\Files\InvalidPathException; use OCP\Files\IRootFolder; -use OCP\Files\Folder; use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\IConfig; @@ -71,12 +71,12 @@ use OCP\IUserManager; use OCP\Lock\ILockingProvider; use OCP\Lock\LockedException; -use OCP\Share; use OCP\Share\Exceptions\GenericShareException; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; use OCP\Share\IShare; use OCP\UserStatus\IManager as IUserStatusManager; +use Psr\Log\LoggerInterface; /** * Class Share20OCS @@ -95,6 +95,8 @@ class ShareAPIController extends OCSController { private $rootFolder; /** @var IURLGenerator */ private $urlGenerator; + /** @var LoggerInterface */ + private $logger; /** @var string */ private $currentUser; /** @var IL10N */ @@ -122,6 +124,7 @@ class ShareAPIController extends OCSController { * @param IUserManager $userManager * @param IRootFolder $rootFolder * @param IURLGenerator $urlGenerator + * @param LoggerInterface $logger , * @param string $userId * @param IL10N $l10n * @param IConfig $config @@ -137,6 +140,7 @@ public function __construct( IUserManager $userManager, IRootFolder $rootFolder, IURLGenerator $urlGenerator, + LoggerInterface $logger, string $userId = null, IL10N $l10n, IConfig $config, @@ -153,6 +157,7 @@ public function __construct( $this->request = $request; $this->rootFolder = $rootFolder; $this->urlGenerator = $urlGenerator; + $this->logger = $logger; $this->currentUser = $userId; $this->l = $l10n; $this->config = $config; @@ -523,7 +528,7 @@ public function createShare( $share->setSharedWith($shareWith); $share->setPermissions($permissions); } elseif ($shareType === IShare::TYPE_LINK - || $shareType === IShare::TYPE_EMAIL) { + || $shareType === IShare::TYPE_EMAIL) { // Can we even share links? if (!$this->shareManager->shareApiAllowLinks()) { @@ -542,9 +547,9 @@ public function createShare( } $permissions = Constants::PERMISSION_READ | - Constants::PERMISSION_CREATE | - Constants::PERMISSION_UPDATE | - Constants::PERMISSION_DELETE; + Constants::PERMISSION_CREATE | + Constants::PERMISSION_UPDATE | + Constants::PERMISSION_DELETE; } else { $permissions = Constants::PERMISSION_READ; } @@ -1742,7 +1747,7 @@ private function shareProviderResharingRights(string $userId, IShare $share, $no } if ($share->getShareType() === IShare::TYPE_CIRCLE && \OC::$server->getAppManager()->isEnabledForUser('circles') - && class_exists('\OCA\Circles\Api\v1\Circles')) { + && class_exists('\OCA\Circles\CirclesManager')) { $hasCircleId = (substr($share->getSharedWith(), -1) === ']'); $shareWithStart = ($hasCircleId ? strrpos($share->getSharedWith(), '[') + 1 : 0); $shareWithLength = ($hasCircleId ? -1 : strpos($share->getSharedWith(), ' ')); @@ -1752,12 +1757,30 @@ private function shareProviderResharingRights(string $userId, IShare $share, $no $sharedWith = substr($share->getSharedWith(), $shareWithStart, $shareWithLength); } try { - $member = \OCA\Circles\Api\v1\Circles::getMember($sharedWith, $userId, 1); - if ($member->getLevel() >= 4) { - return true; + // TODO: switch to ICirclesManager once we have it available within core + /** @var \OCA\Circles\CirclesManager $circleManager */ + $circleManager = $this->serverContainer->get('\OCA\Circles\CirclesManager'); + $circleManager->startSuperSession(); + + // We get the federatedUser linked to the userId (local user, so type=1) + // We browse the federatedUser's membership to confirm it exists and level is moderator + $federatedUser = $circleManager->getFederatedUser($userId, 1); + foreach($federatedUser->getMemberships() as $membership) { + if ($membership->getCircleId() === $sharedWith) { + return ($membership->getLevel() >= 4); + } } - return false; - } catch (QueryException $e) { + } catch (\Exception $e) { + $this->logger->info( + 'Exception while confirming resharing rights visibility', + [ + 'userId' => $userId, + 'sharedWith' => $sharedWith, + 'nodeId' => $node->getId(), + 'exception' => $e, + ] + ); + return false; } } diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 8bea67dff0533..0f39758f1afa3 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -60,6 +60,7 @@ use OCP\Share\Exceptions\GenericShareException; use OCP\Share\IManager; use OCP\Share\IShare; +use Psr\Log\LoggerInterface; use Test\TestCase; use OCP\UserStatus\IManager as IUserStatusManager; @@ -92,6 +93,9 @@ class ShareAPIControllerTest extends TestCase { /** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */ private $urlGenerator; + /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */ + private $loggerInterface; + /** @var string|\PHPUnit\Framework\MockObject\MockObject */ private $currentUser; @@ -130,6 +134,7 @@ protected function setUp(): void { $this->request = $this->createMock(IRequest::class); $this->rootFolder = $this->createMock(IRootFolder::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->loggerInterface = $this->createMock(LoggerInterface::class); $this->currentUser = 'currentUser'; $this->l = $this->createMock(IL10N::class); @@ -155,6 +160,7 @@ protected function setUp(): void { $this->userManager, $this->rootFolder, $this->urlGenerator, + $this->loggerInterface, $this->currentUser, $this->l, $this->config, @@ -178,6 +184,7 @@ private function mockFormatShare() { $this->userManager, $this->rootFolder, $this->urlGenerator, + $this->loggerInterface, $this->currentUser, $this->l, $this->config, diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 669be0a148a61..c9f33eddd7fe3 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1543,11 +1543,13 @@ $permissions & Constants::PERMISSION_READ - - \OCA\Circles\Api\v1\Circles + \OCA\Circles\Api\v1\Circles - + + $circleManager + $circleManager + $circleManager $this->getRoomShareHelper() $this->getRoomShareHelper() $this->getRoomShareHelper()