diff --git a/apps/files_sharing/lib/Controller/ExternalSharesController.php b/apps/files_sharing/lib/Controller/ExternalSharesController.php index 79acd0bca67c0..c297b9e017900 100644 --- a/apps/files_sharing/lib/Controller/ExternalSharesController.php +++ b/apps/files_sharing/lib/Controller/ExternalSharesController.php @@ -15,6 +15,9 @@ use OCP\AppFramework\Http\JSONResponse; use OCP\BackgroundJob\IJobList; use OCP\IRequest; +use OCP\IUser; +use OCP\IUserSession; +use RuntimeException; /** * Class ExternalSharesController @@ -26,17 +29,26 @@ public function __construct( string $appName, IRequest $request, private readonly Manager $externalManager, - private IJobList $jobList, + private readonly IJobList $jobList, + private readonly IUserSession $userSession, ) { parent::__construct($appName, $request); } + private function getUser(): IUser { + $user = $this->userSession->getUser(); + if ($user === null) { + throw new RuntimeException('No user for non-public page'); + } + return $user; + } + /** * @NoOutgoingFederatedSharingRequired */ #[NoAdminRequired] public function index(): JSONResponse { - return new JSONResponse($this->externalManager->getOpenShares()); + return new JSONResponse($this->externalManager->getOpenShares($this->getUser())); } /** @@ -44,9 +56,9 @@ public function index(): JSONResponse { */ #[NoAdminRequired] public function create(string $id): JSONResponse { - $externalShare = $this->externalManager->getShare($id); + $externalShare = $this->externalManager->getShare($id, $this->getUser()); if ($externalShare !== false) { - $this->externalManager->acceptShare($externalShare); + $this->externalManager->acceptShare($externalShare, $this->getUser()); $this->jobList->add(ExternalShareScanJob::class, [$externalShare->getUser(), $externalShare->getMountpoint()]); } return new JSONResponse(); @@ -57,9 +69,9 @@ public function create(string $id): JSONResponse { */ #[NoAdminRequired] public function destroy(string $id): JSONResponse { - $externalShare = $this->externalManager->getShare($id); + $externalShare = $this->externalManager->getShare($id, $this->getUser()); if ($externalShare !== false) { - $this->externalManager->declineShare($externalShare); + $this->externalManager->declineShare($externalShare, $this->getUser()); } return new JSONResponse(); } diff --git a/apps/files_sharing/lib/Controller/RemoteController.php b/apps/files_sharing/lib/Controller/RemoteController.php index eff5654292810..adc37a5bec0ee 100644 --- a/apps/files_sharing/lib/Controller/RemoteController.php +++ b/apps/files_sharing/lib/Controller/RemoteController.php @@ -19,7 +19,10 @@ use OCP\AppFramework\OCSController; use OCP\Files\IRootFolder; use OCP\IRequest; +use OCP\IUser; +use OCP\IUserSession; use Psr\Log\LoggerInterface; +use RuntimeException; /** * @psalm-import-type Files_SharingRemoteShare from ResponseDefinitions @@ -34,12 +37,20 @@ public function __construct( IRequest $request, private readonly Manager $externalManager, private readonly LoggerInterface $logger, - private readonly ?string $userId, + private readonly IUserSession $userSession, private readonly IRootFolder $rootFolder, ) { parent::__construct($appName, $request); } + private function getUser(): IUser { + $user = $this->userSession->getUser(); + if ($user === null) { + throw new RuntimeException('No user for non-public page'); + } + return $user; + } + /** * Get list of pending remote shares * @@ -49,7 +60,7 @@ public function __construct( */ #[NoAdminRequired] public function getOpenShares(): DataResponse { - $shares = $this->externalManager->getOpenShares(); + $shares = $this->externalManager->getOpenShares($this->getUser()); $shares = array_map($this->extendShareInfo(...), $shares); return new DataResponse($shares); } @@ -65,13 +76,13 @@ public function getOpenShares(): DataResponse { */ #[NoAdminRequired] public function acceptShare(string $id): DataResponse { - $externalShare = $this->externalManager->getShare($id); + $externalShare = $this->externalManager->getShare($id, $this->getUser()); if ($externalShare === false) { $this->logger->error('Could not accept federated share with id: ' . $id . ' Share not found.', ['app' => 'files_sharing']); throw new OCSNotFoundException('Wrong share ID, share does not exist.'); } - if (!$this->externalManager->acceptShare($externalShare)) { + if (!$this->externalManager->acceptShare($externalShare, $this->getUser())) { $this->logger->error('Could not accept federated share with id: ' . $id, ['app' => 'files_sharing']); throw new OCSNotFoundException('Wrong share ID, share does not exist.'); } @@ -90,13 +101,13 @@ public function acceptShare(string $id): DataResponse { */ #[NoAdminRequired] public function declineShare(string $id): DataResponse { - $externalShare = $this->externalManager->getShare($id); + $externalShare = $this->externalManager->getShare($id, $this->getUser()); if ($externalShare === false) { $this->logger->error('Could not decline federated share with id: ' . $id . ' Share not found.', ['app' => 'files_sharing']); throw new OCSNotFoundException('Wrong share ID, share does not exist.'); } - if (!$this->externalManager->declineShare($externalShare)) { + if (!$this->externalManager->declineShare($externalShare, $this->getUser())) { $this->logger->error('Could not decline federated share with id: ' . $id, ['app' => 'files_sharing']); throw new OCSNotFoundException('Wrong share ID, share does not exist.'); } @@ -112,7 +123,7 @@ private function extendShareInfo(ExternalShare $share): array { $shareData = $share->jsonSerialize(); $shareData['parent'] = $shareData['parent'] !== '-1' ? $shareData['parent'] : null; - $userFolder = $this->rootFolder->getUserFolder($this->userId); + $userFolder = $this->rootFolder->getUserFolder($this->getUser()->getUID()); try { $mountPointNode = $userFolder->get($share->getMountpoint()); @@ -139,7 +150,7 @@ private function extendShareInfo(ExternalShare $share): array { */ #[NoAdminRequired] public function getShares(): DataResponse { - $shares = $this->externalManager->getAcceptedShares(); + $shares = $this->externalManager->getAcceptedShares($this->getUser()); $shares = array_map(fn (ExternalShare $share) => $this->extendShareInfo($share), $shares); return new DataResponse($shares); } @@ -155,7 +166,7 @@ public function getShares(): DataResponse { */ #[NoAdminRequired] public function getShare(string $id): DataResponse { - $shareInfo = $this->externalManager->getShare($id); + $shareInfo = $this->externalManager->getShare($id, $this->getUser()); if ($shareInfo === false) { throw new OCSNotFoundException('share does not exist'); @@ -177,15 +188,15 @@ public function getShare(string $id): DataResponse { */ #[NoAdminRequired] public function unshare(string $id): DataResponse { - $shareInfo = $this->externalManager->getShare($id); + $shareInfo = $this->externalManager->getShare($id, $this->getUser()); if ($shareInfo === false) { throw new OCSNotFoundException('Share does not exist'); } - $mountPoint = '/' . $this->userId . '/files' . $shareInfo->getMountpoint(); + $mountPoint = '/' . $this->getUser()->getUID() . '/files' . $shareInfo->getMountpoint(); - if ($this->externalManager->removeShare($mountPoint) === true) { + if ($this->externalManager->removeShare($this->getUser(), $mountPoint) === true) { return new DataResponse(); } else { throw new OCSForbiddenException('Could not unshare'); diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index c17cb1a4f98c8..0f4f16d0ffae1 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -22,15 +22,11 @@ use OCP\Files\ISetupManager; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; -use OCP\Files\Storage\IStorageFactory; use OCP\Http\Client\IClientService; -use OCP\ICertificateManager; -use OCP\IConfig; use OCP\IDBConnection; use OCP\IGroup; use OCP\IGroupManager; use OCP\IUser; -use OCP\IUserSession; use OCP\Notification\IManager; use OCP\OCS\IDiscoveryService; use OCP\Share\IShare; @@ -38,28 +34,21 @@ use Psr\Log\LoggerInterface; class Manager { - private ?IUser $user; - public function __construct( - private IDBConnection $connection, - private \OC\Files\Mount\Manager $mountManager, - private IStorageFactory $storageLoader, - private IClientService $clientService, - private IManager $notificationManager, - private IDiscoveryService $discoveryService, - private ICloudFederationProviderManager $cloudFederationProviderManager, - private ICloudFederationFactory $cloudFederationFactory, - private IGroupManager $groupManager, - IUserSession $userSession, - private IEventDispatcher $eventDispatcher, - private LoggerInterface $logger, - private IRootFolder $rootFolder, - private ISetupManager $setupManager, - private ICertificateManager $certificateManager, - private ExternalShareMapper $externalShareMapper, - private IConfig $config, + private readonly IDBConnection $connection, + private readonly \OC\Files\Mount\Manager $mountManager, + private readonly IClientService $clientService, + private readonly IManager $notificationManager, + private readonly IDiscoveryService $discoveryService, + private readonly ICloudFederationProviderManager $cloudFederationProviderManager, + private readonly ICloudFederationFactory $cloudFederationFactory, + private readonly IGroupManager $groupManager, + private readonly IEventDispatcher $eventDispatcher, + private readonly LoggerInterface $logger, + private readonly IRootFolder $rootFolder, + private readonly ISetupManager $setupManager, + private readonly ExternalShareMapper $externalShareMapper, ) { - $this->user = $userSession->getUser(); } /** @@ -69,61 +58,32 @@ public function __construct( * @throws NotPermittedException * @throws UserNotFoundException */ - public function addShare(ExternalShare $externalShare, IUser|IGroup|null $shareWith = null): ?Mount { - $shareWith = $shareWith ?? $this->user; - - if ($externalShare->getAccepted() !== IShare::STATUS_ACCEPTED) { - // To avoid conflicts with the mount point generation later, - // we only use a temporary mount point name here. The real - // mount point name will be generated when accepting the share, - // using the original share item name. - $tmpMountPointName = '{{TemporaryMountPointName#' . $externalShare->getName() . '}}'; - $externalShare->setMountpoint($tmpMountPointName); - $externalShare->setShareWith($shareWith); - - $i = 1; - while (true) { - try { - $this->externalShareMapper->insert($externalShare); - break; - } catch (Exception $e) { - if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { - $externalShare->setMountpoint($tmpMountPointName . '-' . $i); - $i++; - } else { - throw $e; - } + public function addShare(ExternalShare $externalShare, IUser|IGroup $shareWith): void { + // To avoid conflicts with the mount point generation later, + // we only use a temporary mount point name here. The real + // mount point name will be generated when accepting the share, + // using the original share item name. + $tmpMountPointName = '{{TemporaryMountPointName#' . $externalShare->getName() . '}}'; + $externalShare->setMountpoint($tmpMountPointName); + $externalShare->setShareWith($shareWith); + + $i = 1; + while (true) { + try { + $this->externalShareMapper->insert($externalShare); + break; + } catch (Exception $e) { + if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + $externalShare->setMountpoint($tmpMountPointName . '-' . $i); + $i++; + } else { + throw $e; } } - - return null; } - - $user = $shareWith instanceof IUser ? $shareWith : $this->user; - - $userFolder = $this->rootFolder->getUserFolder($user->getUID()); - $mountPoint = $userFolder->getNonExistingName($externalShare->getName()); - - $mountPoint = Filesystem::normalizePath('/' . $mountPoint); - $externalShare->setMountpoint($mountPoint); - $externalShare->setShareWith($user); - $this->externalShareMapper->insert($externalShare); - - $options = [ - 'remote' => $externalShare->getRemote(), - 'token' => $externalShare->getRefreshToken(), - 'password' => $externalShare->getPassword(), - 'access_token' => $externalShare->getAccessToken(), - 'access_token_expires' => $externalShare->getAccessTokenExpires(), - 'mountpoint' => $externalShare->getMountpoint(), - 'owner' => $externalShare->getOwner(), - 'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates'), - ]; - return $this->mountShare($options, $user); } - public function getShare(string $id, ?IUser $user = null): ExternalShare|false { - $user = $user ?? $this->user; + public function getShare(string $id, IUser $user): ExternalShare|false { try { $externalShare = $this->externalShareMapper->getById($id); } catch (DoesNotExistException $e) { @@ -222,16 +182,7 @@ private function updateSubShare(ExternalShare $externalShare, IUser $user, ?stri * * @return bool True if the share could be accepted, false otherwise */ - public function acceptShare(ExternalShare $externalShare, ?IUser $user = null): bool { - // If we're auto-accepting a share, we need to know the user id - // as there is no session available while processing the share - // from the remote server request. - $user = $user ?? $this->user; - if ($user === null) { - $this->logger->error('No user specified for accepting share'); - return false; - } - + public function acceptShare(ExternalShare $externalShare, IUser $user): bool { $result = false; $this->setupManager->setupForUser($user); $folder = $this->rootFolder->getUserFolder($user->getUID()); @@ -279,13 +230,7 @@ public function acceptShare(ExternalShare $externalShare, ?IUser $user = null): * * @return bool True if the share could be declined, false otherwise */ - public function declineShare(ExternalShare $externalShare, ?Iuser $user = null): bool { - $user = $user ?? $this->user; - if ($user === null) { - $this->logger->error('No user specified for declining share'); - return false; - } - + public function declineShare(ExternalShare $externalShare, Iuser $user): bool { $result = false; if ($externalShare->getShareType() === IShare::TYPE_USER) { @@ -311,13 +256,7 @@ public function declineShare(ExternalShare $externalShare, ?Iuser $user = null): return $result; } - public function processNotification(ExternalShare $remoteShare, ?IUser $user = null): void { - $user = $user ?? $this->user; - if ($user === null) { - $this->logger->error('No user specified for processing notification'); - return; - } - + public function processNotification(ExternalShare $remoteShare, IUser $user): void { $filter = $this->notificationManager->createNotification(); $filter->setApp('files_sharing') ->setUser($user->getUID()) @@ -400,33 +339,18 @@ protected function tryOCMEndPoint(ExternalShare $externalShare, string $feedback /** * remove '/user/files' from the path and trailing slashes */ - protected function stripPath(string $path): string { - $prefix = '/' . $this->user->getUID() . '/files'; + protected function stripPath(IUser $user, string $path): string { + $prefix = '/' . $user->getUID() . '/files'; return rtrim(substr($path, strlen($prefix)), '/'); } - public function getMount(array $data, ?IUser $user = null): Mount { - $user = $user ?? $this->user; - $data['manager'] = $this; - $mountPoint = '/' . $user->getUID() . '/files' . $data['mountpoint']; - $data['mountpoint'] = $mountPoint; - $data['certificateManager'] = $this->certificateManager; - return new Mount(Storage::class, $mountPoint, $data, $this, $this->storageLoader); - } - - protected function mountShare(array $data, ?IUser $user = null): Mount { - $mount = $this->getMount($data, $user); - $this->mountManager->addMount($mount); - return $mount; - } - public function getMountManager(): \OC\Files\Mount\Manager { return $this->mountManager; } - public function setMountPoint(string $source, string $target): bool { - $source = $this->stripPath($source); - $target = $this->stripPath($target); + public function setMountPoint(IUser $user, string $source, string $target): bool { + $source = $this->stripPath($user, $source); + $target = $this->stripPath($user, $target); $sourceHash = md5($source); $targetHash = md5($target); @@ -435,16 +359,16 @@ public function setMountPoint(string $source, string $target): bool { ->set('mountpoint', $qb->createNamedParameter($target)) ->set('mountpoint_hash', $qb->createNamedParameter($targetHash)) ->where($qb->expr()->eq('mountpoint_hash', $qb->createNamedParameter($sourceHash))) - ->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($this->user->getUID()))); + ->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($user->getUID()))); $result = (bool)$qb->executeStatement(); - $this->eventDispatcher->dispatchTyped(new InvalidateMountCacheEvent($this->user)); + $this->eventDispatcher->dispatchTyped(new InvalidateMountCacheEvent($user)); return $result; } - public function removeShare(string $mountPoint): bool { + public function removeShare(IUser $user, string $mountPoint): bool { try { $mountPointObj = $this->mountManager->find($mountPoint); } catch (NotFoundException $e) { @@ -457,11 +381,11 @@ public function removeShare(string $mountPoint): bool { } $id = $mountPointObj->getStorage()->getCache()->getId(''); - $mountPoint = $this->stripPath($mountPoint); + $mountPoint = $this->stripPath($user, $mountPoint); try { try { - $externalShare = $this->externalShareMapper->getByMountPointAndUser($mountPoint, $this->user); + $externalShare = $this->externalShareMapper->getByMountPointAndUser($mountPoint, $user); } catch (DoesNotExistException $e) { // ignore $this->removeReShares((string)$id); @@ -546,9 +470,9 @@ public function removeGroupShares(IGroup $group): bool { * * @return list list of open server-to-server shares */ - public function getOpenShares(): array { + public function getOpenShares(IUser $user): array { try { - return $this->externalShareMapper->getShares($this->user, IShare::STATUS_PENDING); + return $this->externalShareMapper->getShares($user, IShare::STATUS_PENDING); } catch (Exception $e) { $this->logger->emergency('Error when retrieving shares', ['exception' => $e]); return []; @@ -560,9 +484,9 @@ public function getOpenShares(): array { * * @return list list of accepted server-to-server shares */ - public function getAcceptedShares(): array { + public function getAcceptedShares(IUser $user): array { try { - return $this->externalShareMapper->getShares($this->user, IShare::STATUS_ACCEPTED); + return $this->externalShareMapper->getShares($user, IShare::STATUS_ACCEPTED); } catch (Exception $e) { $this->logger->emergency('Error when retrieving shares', ['exception' => $e]); return []; diff --git a/apps/files_sharing/lib/External/Mount.php b/apps/files_sharing/lib/External/Mount.php index 6f9a578fe6d16..882f04e826039 100644 --- a/apps/files_sharing/lib/External/Mount.php +++ b/apps/files_sharing/lib/External/Mount.php @@ -13,6 +13,7 @@ use OC\Files\Storage\StorageFactory; use OCA\Files_Sharing\ISharedMountPoint; use OCP\Files\Mount\IMovableMount; +use OCP\IUser; use Override; class Mount extends MountPoint implements IMovableMount, ISharedMountPoint { @@ -21,6 +22,7 @@ public function __construct( string $mountpoint, array $options, protected Manager $manager, + private readonly IUser $user, ?StorageFactory $loader = null, ) { parent::__construct($storage, $mountpoint, $options, $loader, null, null, MountProvider::class); @@ -28,7 +30,7 @@ public function __construct( #[Override] public function moveMount(string $target): bool { - $result = $this->manager->setMountPoint($this->mountPoint, $target); + $result = $this->manager->setMountPoint($this->user, $this->mountPoint, $target); $this->setMountPoint($target); return $result; @@ -36,7 +38,7 @@ public function moveMount(string $target): bool { #[Override] public function removeMount(): bool { - return $this->manager->removeShare($this->mountPoint); + return $this->manager->removeShare($this->user, $this->mountPoint); } #[Override] diff --git a/apps/files_sharing/lib/External/MountProvider.php b/apps/files_sharing/lib/External/MountProvider.php index d26ea83d55ee8..c6dd1e4777523 100644 --- a/apps/files_sharing/lib/External/MountProvider.php +++ b/apps/files_sharing/lib/External/MountProvider.php @@ -54,8 +54,9 @@ private function getMount(IUser $user, array $data, IStorageFactory $storageFact $data['certificateManager'] = Server::get(ICertificateManager::class); $data['HttpClientService'] = Server::get(IClientService::class); $data['verify'] = !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates'); + $data['recipient'] = $user; - return new Mount(self::STORAGE, $mountPoint, $data, $manager, $storageFactory); + return new Mount(self::STORAGE, $mountPoint, $data, $manager, $user, $storageFactory); } #[\Override] diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index 1622f1ea8f032..35a9b8a802dff 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -34,6 +34,7 @@ use OCP\IAppConfig; use OCP\ICacheFactory; use OCP\IConfig; +use OCP\IUser; use OCP\IUserSession; use OCP\OCM\Exceptions\OCMArgumentException; use OCP\OCM\Exceptions\OCMProviderException; @@ -53,6 +54,7 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, private IConfig $config; protected IAppConfig $appConfig; private IShareManager $shareManager; + private IUser $recipientUser; private bool $tokenRefreshed = false; /** Unix timestamp until which the current access token is considered valid (0 = unknown/expired) */ private int $tokenExpiresAt = 0; @@ -65,7 +67,16 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, private const int REFRESH_BACKOFF_SECONDS = 5; /** - * @param array{HttpClientService: IClientService, manager: ExternalShareManager, cloudId: ICloudId, mountpoint: string, token: string, access_token: ?string, access_token_expires: ?int}|array $options + * @param array{ + * HttpClientService: IClientService, + * manager: ExternalShareManager, + * cloudId: ICloudId, + * mountpoint: string, + * token: string, + * recipient: IUser, + * access_token: ?string, + * access_token_expires: ?int + * }|array $options */ public function __construct($options) { $this->memcacheFactory = Server::get(ICacheFactory::class); @@ -77,6 +88,7 @@ public function __construct($options) { $this->config = Server::get(IConfig::class); $this->appConfig = Server::get(IAppConfig::class); $this->shareManager = Server::get(IShareManager::class); + $this->recipientUser = $options['recipient']; // use default path to webdav if not found on discovery try { @@ -326,7 +338,7 @@ public function checkStorageAvailability(): void { // valid Nextcloud instance means that the public share no longer exists // since this is permanent (re-sharing the file will create a new token) // we remove the invalid storage - $this->manager->removeShare($this->mountPoint); + $this->manager->removeShare($this->recipientUser, $this->mountPoint); $this->manager->getMountManager()->removeMount($this->mountPoint); throw new StorageInvalidException('Remote share not found', 0, $e); } else { @@ -335,7 +347,7 @@ public function checkStorageAvailability(): void { } } catch (ForbiddenException $e) { // auth error, remove share for now (provide a dialog in the future) - $this->manager->removeShare($this->mountPoint); + $this->manager->removeShare($this->recipientUser, $this->mountPoint); $this->manager->getMountManager()->removeMount($this->mountPoint); throw new StorageInvalidException('Auth error when getting remote share'); } catch (\GuzzleHttp\Exception\ConnectException $e) { diff --git a/apps/files_sharing/tests/Controller/ExternalShareControllerTest.php b/apps/files_sharing/tests/Controller/ExternalShareControllerTest.php index 61b29fa771cfd..73a02ad2f1552 100644 --- a/apps/files_sharing/tests/Controller/ExternalShareControllerTest.php +++ b/apps/files_sharing/tests/Controller/ExternalShareControllerTest.php @@ -14,6 +14,8 @@ use OCP\AppFramework\Http\JSONResponse; use OCP\BackgroundJob\IJobList; use OCP\IRequest; +use OCP\IUser; +use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; /** @@ -25,20 +27,27 @@ class ExternalShareControllerTest extends \Test\TestCase { private IRequest&MockObject $request; private Manager&MockObject $externalManager; private IJobList&MockObject $jobList; + private IUser $user; protected function setUp(): void { parent::setUp(); $this->request = $this->createMock(IRequest::class); $this->externalManager = $this->createMock(Manager::class); $this->jobList = $this->createMock(IJobList::class); + $this->user = $this->createMock(IUser::class); + $this->user->method('getUID')->willReturn('user'); } public function getExternalShareController(): ExternalSharesController { + $session = $this->createMock(IUserSession::class); + $session->method('getUser') + ->willReturn($this->user); return new ExternalSharesController( 'files_sharing', $this->request, $this->externalManager, $this->jobList, + $session, ); } @@ -56,12 +65,12 @@ public function testCreate(): void { $this->externalManager ->expects($this->once()) ->method('getShare') - ->with('4') + ->with('4', $this->user) ->willReturn($share); $this->externalManager ->expects($this->once()) ->method('acceptShare') - ->with($share); + ->with($share, $this->user); $this->jobList ->expects($this->once()) ->method('add'); @@ -74,12 +83,12 @@ public function testDestroy(): void { $this->externalManager ->expects($this->once()) ->method('getShare') - ->with('4') + ->with('4', $this->user) ->willReturn($share); $this->externalManager ->expects($this->once()) ->method('declineShare') - ->with($share); + ->with($share, $this->user); $this->assertEquals(new JSONResponse(), $this->getExternalShareController()->destroy('4')); } diff --git a/apps/files_sharing/tests/External/ManagerTest.php b/apps/files_sharing/tests/External/ManagerTest.php index 68fb4839cbf36..0b9b08eac7bb4 100644 --- a/apps/files_sharing/tests/External/ManagerTest.php +++ b/apps/files_sharing/tests/External/ManagerTest.php @@ -38,7 +38,6 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; -use OCP\IUserSession; use OCP\OCM\IOCMDiscoveryService; use OCP\OCS\IDiscoveryService; use OCP\Server; @@ -121,7 +120,7 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->logger->expects($this->never())->method('emergency'); - $this->manager = $this->createManagerForUser($this->user); + $this->manager = $this->createManager(); $this->testMountProvider = new MountProvider(Server::get(IDBConnection::class), function () { return $this->manager; @@ -157,31 +156,23 @@ protected function tearDown(): void { parent::tearDown(); } - private function createManagerForUser(IUser $user): Manager&MockObject { - $userSession = $this->createMock(IUserSession::class); - $userSession->method('getUser') - ->willReturn($user); - + private function createManager(): Manager&MockObject { return $this->getMockBuilder(Manager::class) ->setConstructorArgs( [ Server::get(IDBConnection::class), $this->mountManager, - new StorageFactory(), $this->clientService, Server::get(\OCP\Notification\IManager::class), $this->discoveryService, $this->cloudFederationProviderManager, $this->cloudFederationFactory, $this->groupManager, - $userSession, $this->eventDispatcher, $this->logger, $this->rootFolder, $this->setupManagerEncTrait, - $this->certificateManager, $this->externalShareMapper, - $this->config, ] )->onlyMethods(['tryOCMEndPoint'])->getMock(); } @@ -250,8 +241,8 @@ public function doTestAddShare(ExternalShare $shareData1, IUser|IGroup $userOrGr } // Add a share for "user" - $this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], [$shareData1, $userOrGroup])); - $openShares = $this->manager->getOpenShares(); + $this->manager->addShare($shareData1, $userOrGroup); + $openShares = $this->manager->getOpenShares($this->user); $this->assertCount(1, $openShares); $this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1->getName() . '}}', $userOrGroup); @@ -267,8 +258,8 @@ public function doTestAddShare(ExternalShare $shareData1, IUser|IGroup $userOrGr $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1->getName() . '}}'); // Add a second share for "user" with the same name - $this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], [$shareData2, $userOrGroup])); - $openShares = $this->manager->getOpenShares(); + $this->manager->addShare($shareData2, $userOrGroup); + $openShares = $this->manager->getOpenShares($this->user); $this->assertCount(2, $openShares); $this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1->getName() . '}}', $userOrGroup); // New share falls back to "-1" appendix, because the name is already taken @@ -307,7 +298,7 @@ public function doTestAddShare(ExternalShare $shareData1, IUser|IGroup $userOrGr } // Accept the first share - $this->assertTrue($this->manager->acceptShare($openShares[0])); + $this->assertTrue($this->manager->acceptShare($openShares[0], $this->user)); // Check remaining shares - Accepted $acceptedShares = $this->externalShareMapper->getShares($this->user, IShare::STATUS_ACCEPTED); @@ -315,7 +306,7 @@ public function doTestAddShare(ExternalShare $shareData1, IUser|IGroup $userOrGr $shareData1->setAccepted(true); $this->assertExternalShareEntry($shareData1, $acceptedShares[0], 1, $shareData1->getName(), $this->user); // Check remaining shares - Open - $openShares = $this->manager->getOpenShares(); + $openShares = $this->manager->getOpenShares($this->user); $this->assertCount(1, $openShares); $this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2->getName() . '}}-1', $userOrGroup); @@ -325,8 +316,9 @@ public function doTestAddShare(ExternalShare $shareData1, IUser|IGroup $userOrGr $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1->getName() . '}}-1'); // Add another share for "user" with the same name - $this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], [$shareData3, $userOrGroup])); - $openShares = $this->manager->getOpenShares(); + + $this->manager->addShare($shareData3, $userOrGroup); + $openShares = $this->manager->getOpenShares($this->user); $this->assertCount(2, $openShares); $this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2->getName() . '}}-1', $userOrGroup); if (!$isGroup) { @@ -360,7 +352,7 @@ public function doTestAddShare(ExternalShare $shareData1, IUser|IGroup $userOrGr } // Decline the third share - $this->assertTrue($this->manager->declineShare($openShares[1])); + $this->assertTrue($this->manager->declineShare($openShares[1], $this->user)); $this->setupMounts(); $this->assertMount($shareData1->getName()); @@ -373,7 +365,7 @@ public function doTestAddShare(ExternalShare $shareData1, IUser|IGroup $userOrGr $shareData1->setAccepted(true); $this->assertExternalShareEntry($shareData1, $acceptedShares[0], 1, $shareData1->getName(), $this->user); // Check remaining shares - Open - $openShares = $this->manager->getOpenShares(); + $openShares = $this->manager->getOpenShares($this->user); if ($isGroup) { // declining a group share adds it back to pending instead of deleting it $this->assertCount(2, $openShares); @@ -431,7 +423,7 @@ public function doTestAddShare(ExternalShare $shareData1, IUser|IGroup $userOrGr } private function verifyAcceptedGroupShare(ExternalShare $share): void { - $openShares = $this->manager->getOpenShares(); + $openShares = $this->manager->getOpenShares($this->user); $this->assertCount(0, $openShares); $acceptedShares = $this->externalShareMapper->getShares($this->user, IShare::STATUS_ACCEPTED); $this->assertCount(1, $acceptedShares); @@ -445,7 +437,7 @@ private function verifyDeclinedGroupShare(ExternalShare $share, ?string $tempMou if ($tempMount === null) { $tempMount = '{{TemporaryMountPointName#/SharedFolder}}'; } - $openShares = $this->manager->getOpenShares(); + $openShares = $this->manager->getOpenShares($this->user); $this->assertCount(1, $openShares); $acceptedShares = $this->externalShareMapper->getShares($this->user, IShare::STATUS_ACCEPTED); $this->assertCount(0, $acceptedShares); @@ -470,7 +462,7 @@ private function createTestUserShare(string $userId = 'user1'): ExternalShare { $share->setAccepted(IShare::STATUS_PENDING); $share->setRemoteId('2346'); - $this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], [$share, $user])); + $this->manager->addShare($share, $user); return $share; } @@ -490,7 +482,7 @@ private function createTestGroupShare(string $groupId = 'group1'): array { $share->setAccepted(IShare::STATUS_PENDING); $share->setRemoteId('2342'); - $this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], [$share, $groupId === 'group1' ? $this->group1 : $this->group2])); + $this->manager->addShare($share, $groupId === 'group1' ? $this->group1 : $this->group2); $allShares = $this->externalShareMapper->getShares($this->user, null); $groupShare = null; @@ -509,85 +501,85 @@ private function createTestGroupShare(string $groupId = 'group1'): array { public function testAcceptOriginalGroupShare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->acceptShare($groupShare)); + $this->assertTrue($this->manager->acceptShare($groupShare, $this->user)); $this->verifyAcceptedGroupShare($shareData); // a second time - $this->assertTrue($this->manager->acceptShare($groupShare)); + $this->assertTrue($this->manager->acceptShare($groupShare, $this->user)); $this->verifyAcceptedGroupShare($shareData); } public function testAcceptGroupShareAgainThroughGroupShare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->acceptShare($groupShare)); + $this->assertTrue($this->manager->acceptShare($groupShare, $this->user)); $this->verifyAcceptedGroupShare($shareData); // decline again, this keeps the sub-share - $this->assertTrue($this->manager->declineShare($groupShare)); + $this->assertTrue($this->manager->declineShare($groupShare, $this->user)); $this->verifyDeclinedGroupShare($shareData, '/SharedFolder'); // this will return sub-entries - $openShares = $this->manager->getOpenShares(); + $openShares = $this->manager->getOpenShares($this->user); $this->assertCount(1, $openShares); // accept through group share - $this->assertTrue($this->manager->acceptShare($groupShare)); + $this->assertTrue($this->manager->acceptShare($groupShare, $this->user)); $this->verifyAcceptedGroupShare($shareData, '/SharedFolder'); // accept a second time - $this->assertTrue($this->manager->acceptShare($groupShare)); + $this->assertTrue($this->manager->acceptShare($groupShare, $this->user)); $this->verifyAcceptedGroupShare($shareData, '/SharedFolder'); } public function testAcceptGroupShareAgainThroughSubShare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->acceptShare($groupShare)); + $this->assertTrue($this->manager->acceptShare($groupShare, $this->user)); $this->verifyAcceptedGroupShare($shareData); // decline again, this keeps the sub-share - $this->assertTrue($this->manager->declineShare($groupShare)); + $this->assertTrue($this->manager->declineShare($groupShare, $this->user)); $this->verifyDeclinedGroupShare($shareData, '/SharedFolder'); // this will return sub-entries - $openShares = $this->manager->getOpenShares(); + $openShares = $this->manager->getOpenShares($this->user); $this->assertCount(1, $openShares); // accept through sub-share - $this->assertTrue($this->manager->acceptShare($openShares[0])); + $this->assertTrue($this->manager->acceptShare($openShares[0], $this->user)); $this->verifyAcceptedGroupShare($shareData); // accept a second time - $this->assertTrue($this->manager->acceptShare($openShares[0])); + $this->assertTrue($this->manager->acceptShare($openShares[0], $this->user)); $this->verifyAcceptedGroupShare($shareData); } public function testDeclineOriginalGroupShare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->declineShare($groupShare)); + $this->assertTrue($this->manager->declineShare($groupShare, $this->user)); $this->verifyDeclinedGroupShare($shareData); // a second time - $this->assertTrue($this->manager->declineShare($groupShare)); + $this->assertTrue($this->manager->declineShare($groupShare, $this->user)); $this->verifyDeclinedGroupShare($shareData); } public function testDeclineGroupShareAgainThroughGroupShare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->acceptShare($groupShare)); + $this->assertTrue($this->manager->acceptShare($groupShare, $this->user)); $this->verifyAcceptedGroupShare($shareData); // decline again, this keeps the sub-share - $this->assertTrue($this->manager->declineShare($groupShare)); + $this->assertTrue($this->manager->declineShare($groupShare, $this->user)); $this->verifyDeclinedGroupShare($shareData, '/SharedFolder'); // a second time - $this->assertTrue($this->manager->declineShare($groupShare)); + $this->assertTrue($this->manager->declineShare($groupShare, $this->user)); $this->verifyDeclinedGroupShare($shareData, '/SharedFolder'); } public function testDeclineGroupShareAgainThroughSubshare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->acceptShare($groupShare)); + $this->assertTrue($this->manager->acceptShare($groupShare, $this->user)); $this->verifyAcceptedGroupShare($shareData); // this will return sub-entries @@ -595,57 +587,57 @@ public function testDeclineGroupShareAgainThroughSubshare(): void { $this->assertCount(1, $allShares); // decline again through sub-share - $this->assertTrue($this->manager->declineShare($allShares[0])); + $this->assertTrue($this->manager->declineShare($allShares[0], $this->user)); $this->verifyDeclinedGroupShare($shareData, '/SharedFolder'); // a second time - $this->assertTrue($this->manager->declineShare($allShares[0])); + $this->assertTrue($this->manager->declineShare($allShares[0], $this->user)); $this->verifyDeclinedGroupShare($shareData, '/SharedFolder'); } public function testDeclineGroupShareAgainThroughMountPoint(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->acceptShare($groupShare)); + $this->assertTrue($this->manager->acceptShare($groupShare, $this->user)); $this->verifyAcceptedGroupShare($shareData); // decline through mount point name - $this->assertTrue($this->manager->removeShare($this->user->getUID() . '/files/' . $shareData->getName())); + $this->assertTrue($this->manager->removeShare($this->user, $this->user->getUID() . '/files/' . $shareData->getName())); $this->verifyDeclinedGroupShare($shareData, '/SharedFolder'); // second time must fail as the mount point is gone - $this->assertFalse($this->manager->removeShare($this->user->getUID() . '/files/' . $shareData->getName())); + $this->assertFalse($this->manager->removeShare($this->user, $this->user->getUID() . '/files/' . $shareData->getName())); } public function testDeclineThenAcceptGroupShareAgainThroughGroupShare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); // decline, this creates a declined sub-share - $this->assertTrue($this->manager->declineShare($groupShare)); + $this->assertTrue($this->manager->declineShare($groupShare, $this->user)); $this->verifyDeclinedGroupShare($shareData); // accept through sub-share - $this->assertTrue($this->manager->acceptShare($groupShare)); + $this->assertTrue($this->manager->acceptShare($groupShare, $this->user)); $this->verifyAcceptedGroupShare($shareData, '/SharedFolder'); // accept a second time - $this->assertTrue($this->manager->acceptShare($groupShare)); + $this->assertTrue($this->manager->acceptShare($groupShare, $this->user)); $this->verifyAcceptedGroupShare($shareData, '/SharedFolder'); } public function testDeclineThenAcceptGroupShareAgainThroughSubShare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); // decline, this creates a declined sub-share - $this->assertTrue($this->manager->declineShare($groupShare)); + $this->assertTrue($this->manager->declineShare($groupShare, $this->user)); $this->verifyDeclinedGroupShare($shareData); // this will return sub-entries - $openShares = $this->manager->getOpenShares(); + $openShares = $this->manager->getOpenShares($this->user); // accept through sub-share - $this->assertTrue($this->manager->acceptShare($openShares[0])); + $this->assertTrue($this->manager->acceptShare($openShares[0], $this->user)); $this->verifyAcceptedGroupShare($shareData); // accept a second time - $this->assertTrue($this->manager->acceptShare($openShares[0])); + $this->assertTrue($this->manager->acceptShare($openShares[0], $this->user)); $this->verifyAcceptedGroupShare($shareData); } @@ -656,15 +648,16 @@ public function testDeleteUserShares(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $shares = $this->manager->getOpenShares(); - $this->assertCount(2, $shares); - - $this->assertTrue($this->manager->acceptShare($groupShare)); $user2 = $this->createMock(IUser::class); $user2->method('getUID')->willReturn('user2'); + $shares = $this->manager->getOpenShares($this->user); + $this->assertCount(2, $shares); + + $this->assertTrue($this->manager->acceptShare($groupShare, $this->user)); + // user 2 shares - $manager2 = $this->createManagerForUser($user2); + $manager2 = $this->createManager(); $share = new ExternalShare(); $share->generateId(); $share->setRemote('http://localhost'); @@ -676,22 +669,22 @@ public function testDeleteUserShares(): void { $share->setAccepted(IShare::STATUS_PENDING); $share->setRemoteId('2342'); - $this->assertCount(1, $manager2->getOpenShares()); - $this->assertSame(null, call_user_func_array([$manager2, 'addShare'], [$share, $user2])); - $this->assertCount(2, $manager2->getOpenShares()); + $this->assertCount(1, $manager2->getOpenShares($user2)); + $manager2->addShare($share, $user2); + $this->assertCount(2, $manager2->getOpenShares($user2)); $userShare = $this->externalShareMapper->getById($userShare->getId()); // Simpler to compare $this->manager->expects($this->once())->method('tryOCMEndPoint')->with($userShare, 'decline')->willReturn([]); $this->manager->removeUserShares($this->user); - $user1Shares = $this->manager->getOpenShares(); + $user1Shares = $this->manager->getOpenShares($this->user); // user share is gone, group is still there $this->assertCount(1, $user1Shares); $this->assertEquals($user1Shares[0]->getShareType(), IShare::TYPE_GROUP); // user 2 shares untouched - $user2Shares = $manager2->getOpenShares(); + $user2Shares = $manager2->getOpenShares($user2); $this->assertCount(2, $user2Shares); $this->assertEquals($user2Shares[0]->getShareType(), IShare::TYPE_GROUP); $this->assertEquals($user2Shares[0]->getUser(), 'group1'); @@ -704,16 +697,16 @@ public function testDeleteGroupShares(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $shares = $this->manager->getOpenShares(); - $this->assertCount(2, $shares); - - $this->assertTrue($this->manager->acceptShare($groupShare)); - $user = $this->createMock(IUser::class); $user->method('getUID')->willReturn('user2'); + $shares = $this->manager->getOpenShares($this->user); + $this->assertCount(2, $shares); + + $this->assertTrue($this->manager->acceptShare($groupShare, $this->user)); + // user 2 shares - $manager2 = $this->createManagerForUser($user); + $manager2 = $this->createManager(); $share = new ExternalShare(); $share->generateId(); @@ -726,20 +719,20 @@ public function testDeleteGroupShares(): void { $share->setAccepted(IShare::STATUS_PENDING); $share->setRemoteId('2343'); - $this->assertCount(1, $manager2->getOpenShares()); - $this->assertSame(null, call_user_func_array([$manager2, 'addShare'], [$share, $user])); - $this->assertCount(2, $manager2->getOpenShares()); + $this->assertCount(1, $manager2->getOpenShares($user)); + $manager2->addShare($share, $user); + $this->assertCount(2, $manager2->getOpenShares($user)); $this->manager->expects($this->never())->method('tryOCMEndPoint'); $this->manager->removeGroupShares($this->group1); - $user1Shares = $this->manager->getOpenShares(); + $user1Shares = $this->manager->getOpenShares($this->user); // user share is gone, group is still there $this->assertCount(1, $user1Shares); $this->assertEquals($user1Shares[0]->getShareType(), IShare::TYPE_USER); // user 2 shares untouched - $user2Shares = $manager2->getOpenShares(); + $user2Shares = $manager2->getOpenShares($user); $this->assertCount(1, $user2Shares); $this->assertEquals($user2Shares[0]->getShareType(), IShare::TYPE_USER); $this->assertEquals($user2Shares[0]->getUser(), 'user2'); diff --git a/apps/files_sharing/tests/External/ManagerUpdateAccessTokenTest.php b/apps/files_sharing/tests/External/ManagerUpdateAccessTokenTest.php index c4a5fe2db8b3d..765cbe4af7b6d 100644 --- a/apps/files_sharing/tests/External/ManagerUpdateAccessTokenTest.php +++ b/apps/files_sharing/tests/External/ManagerUpdateAccessTokenTest.php @@ -19,10 +19,7 @@ use OCP\Federation\ICloudFederationProviderManager; use OCP\Files\IRootFolder; use OCP\Files\ISetupManager; -use OCP\Files\Storage\IStorageFactory; use OCP\Http\Client\IClientService; -use OCP\ICertificateManager; -use OCP\IConfig; use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IUserSession; @@ -49,21 +46,17 @@ protected function setUp(): void { $this->manager = new Manager( $this->createMock(IDBConnection::class), $this->createMock(\OC\Files\Mount\Manager::class), - $this->createMock(IStorageFactory::class), $this->createMock(IClientService::class), $this->createMock(INotificationManager::class), $this->createMock(IDiscoveryService::class), $this->createMock(ICloudFederationProviderManager::class), $this->createMock(ICloudFederationFactory::class), $this->createMock(IGroupManager::class), - $userSession, $this->createMock(IEventDispatcher::class), $this->logger, $this->createMock(IRootFolder::class), $this->createMock(ISetupManager::class), - $this->createMock(ICertificateManager::class), $this->externalShareMapper, - $this->createMock(IConfig::class), ); } diff --git a/apps/files_sharing/tests/ExternalStorageTest.php b/apps/files_sharing/tests/ExternalStorageTest.php index 38040a47d71f2..660fae06fe764 100644 --- a/apps/files_sharing/tests/ExternalStorageTest.php +++ b/apps/files_sharing/tests/ExternalStorageTest.php @@ -15,6 +15,7 @@ use OCP\Http\Client\IClientService; use OCP\Http\Client\IResponse; use OCP\ICertificateManager; +use OCP\IUser; use OCP\OCM\IOCMDiscoveryService; use OCP\OCM\IOCMProvider; use OCP\Server; @@ -85,6 +86,7 @@ private function getTestStorage($uri) { ->willReturn('/public.php/webdav'); $ocmProvider->method('getEndPoint') ->willReturn($uri); + $user = $this->createMock(IUser::class); return new TestSharingExternalStorage( [ @@ -98,6 +100,7 @@ private function getTestStorage($uri) { 'certificateManager' => $certificateManager, 'HttpClientService' => $httpClientService, 'discoveryService' => $discoveryService, + 'recipient' => $user, ] ); }