diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 35a01ccdd6158..8a11fe0ee89d8 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/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index f3a3157851137..e96b99b11bfed 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -49,6 +49,8 @@ use OCP\IServerContainer; use OCP\Share\IShare; use OCP\UserStatus\IManager as IUserStatusManager; +use Psr\Log\LoggerInterface; +use Psr\Log\Test\LoggerInterfaceTest; /** * Class ApiTest @@ -128,6 +130,7 @@ private function createOCS($userId) { \OC::$server->getUserManager(), \OC::$server->getRootFolder(), \OC::$server->getURLGenerator(), + $this->getMockBuilder(LoggerInterface::class)->getMock(), $userId, $l, $config, diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index c7c2b6d8757ec..1c3213a4c693b 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, @@ -738,6 +745,7 @@ public function testGetShare(\OCP\Share\IShare $share, array $result) { $this->userManager, $this->rootFolder, $this->urlGenerator, + $this->loggerInterface, $this->currentUser, $this->l, $this->config, @@ -1362,6 +1370,7 @@ public function testGetShares(array $getSharesParameters, array $shares, array $ $this->userManager, $this->rootFolder, $this->urlGenerator, + $this->loggerInterface, $this->currentUser, $this->l, $this->config, @@ -1707,6 +1716,7 @@ public function testCreateShareUser() { $this->userManager, $this->rootFolder, $this->urlGenerator, + $this->loggerInterface, $this->currentUser, $this->l, $this->config, @@ -1809,6 +1819,7 @@ public function testCreateShareGroup() { $this->userManager, $this->rootFolder, $this->urlGenerator, + $this->loggerInterface, $this->currentUser, $this->l, $this->config, @@ -2190,6 +2201,7 @@ public function testCreateShareRemote() { $this->userManager, $this->rootFolder, $this->urlGenerator, + $this->loggerInterface, $this->currentUser, $this->l, $this->config, @@ -2260,6 +2272,7 @@ public function testCreateShareRemoteGroup() { $this->userManager, $this->rootFolder, $this->urlGenerator, + $this->loggerInterface, $this->currentUser, $this->l, $this->config, @@ -2514,6 +2527,7 @@ public function testCreateReshareOfFederatedMountNoDeletePermissions() { $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 e721a490e6def..4b38b7766039b 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1534,11 +1534,13 @@ $permissions & Constants::PERMISSION_READ - - \OCA\Circles\Api\v1\Circles + \OCA\Circles\Api\v1\Circles - + + $circleManager + $circleManager + $circleManager $this->getRoomShareHelper() $this->getRoomShareHelper() $this->getRoomShareHelper()