diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index e15d57181a369..eb8718e83da83 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -500,7 +500,6 @@ private function confirmSignedOrigin(?IIncomingSignedRequest $signedRequest, str * * @param IIncomingSignedRequest|null $signedRequest * @param string $resourceType - * @param string $sharedSecret * * @throws IncomingRequestException * @throws BadRequestException @@ -524,7 +523,7 @@ private function confirmNotificationIdentity( return; } } catch (\Exception $e) { - throw new IncomingRequestException($e->getMessage()); + throw new IncomingRequestException($e->getMessage(), previous: $e); } $this->confirmNotificationEntry($signedRequest, $identity); diff --git a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php index 7fdd718cbfe8b..2fc262a1e67da 100644 --- a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php +++ b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php @@ -7,9 +7,7 @@ */ namespace OCA\FederatedFileSharing\Controller; -use OCA\FederatedFileSharing\AddressHandler; use OCA\FederatedFileSharing\FederatedShareProvider; -use OCA\FederatedFileSharing\Notifications; use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; @@ -29,7 +27,6 @@ use OCP\HintException; use OCP\IDBConnection; use OCP\IRequest; -use OCP\IUserManager; use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Server; use OCP\Share; @@ -44,10 +41,6 @@ public function __construct( IRequest $request, private FederatedShareProvider $federatedShareProvider, private IDBConnection $connection, - private Share\IManager $shareManager, - private Notifications $notifications, - private AddressHandler $addressHandler, - private IUserManager $userManager, private ICloudIdManager $cloudIdManager, private LoggerInterface $logger, private ICloudFederationFactory $cloudFederationFactory, @@ -66,10 +59,10 @@ public function __construct( * @param string|null $owner Display name of the receiver * @param string|null $sharedBy Display name of the sender * @param string|null $shareWith ID of the user that receives the share - * @param int|null $remoteId ID of the remote + * @param string|null $remoteId ID of the remote * @param string|null $sharedByFederatedId Federated ID of the sender * @param string|null $ownerFederatedId Federated ID of the receiver - * @return Http\DataResponse, array{}> + * @return DataResponse, array{}> * @throws OCSException * * 200: Share created successfully @@ -83,10 +76,10 @@ public function createShare( ?string $owner = null, ?string $sharedBy = null, ?string $shareWith = null, - ?int $remoteId = null, + ?string $remoteId = null, ?string $sharedByFederatedId = null, ?string $ownerFederatedId = null, - ) { + ): DataResponse { if ($ownerFederatedId === null) { $ownerFederatedId = $this->cloudIdManager->getCloudId($owner, $this->cleanupRemote($remote))->getId(); } @@ -132,11 +125,11 @@ public function createShare( /** * create re-share on behalf of another user * - * @param int $id ID of the share + * @param string $id ID of the share * @param string|null $token Shared secret between servers * @param string|null $shareWith ID of the user that receives the share * @param int|null $remoteId ID of the remote - * @return Http\DataResponse + * @return DataResponse * @throws OCSBadRequestException Re-sharing is not possible * @throws OCSException * @@ -144,7 +137,7 @@ public function createShare( */ #[NoCSRFRequired] #[PublicPage] - public function reShare(int $id, ?string $token = null, ?string $shareWith = null, ?int $remoteId = 0) { + public function reShare(string $id, ?string $token = null, ?string $shareWith = null, ?int $remoteId = 0): DataResponse { if ($token === null || $shareWith === null || $remoteId === null @@ -181,9 +174,9 @@ public function reShare(int $id, ?string $token = null, ?string $shareWith = nul /** * accept server-to-server share * - * @param int $id ID of the remote share + * @param string $id ID of the remote share * @param string|null $token Shared secret between servers - * @return Http\DataResponse, array{}> + * @return DataResponse, array{}> * @throws OCSException * @throws ShareNotFound * @throws HintException @@ -192,7 +185,7 @@ public function reShare(int $id, ?string $token = null, ?string $shareWith = nul */ #[NoCSRFRequired] #[PublicPage] - public function acceptShare(int $id, ?string $token = null) { + public function acceptShare(string $id, ?string $token = null): DataResponse { $notification = [ 'sharedSecret' => $token, 'message' => 'Recipient accept the share' @@ -216,16 +209,16 @@ public function acceptShare(int $id, ?string $token = null) { /** * decline server-to-server share * - * @param int $id ID of the remote share + * @param string $id ID of the remote share * @param string|null $token Shared secret between servers - * @return Http\DataResponse, array{}> + * @return DataResponse, array{}> * @throws OCSException * * 200: Share declined successfully */ #[NoCSRFRequired] #[PublicPage] - public function declineShare(int $id, ?string $token = null) { + public function declineShare(string $id, ?string $token = null) { $notification = [ 'sharedSecret' => $token, 'message' => 'Recipient declined the share' @@ -249,16 +242,16 @@ public function declineShare(int $id, ?string $token = null) { /** * remove server-to-server share if it was unshared by the owner * - * @param int $id ID of the share + * @param string $id ID of the share * @param string|null $token Shared secret between servers - * @return Http\DataResponse, array{}> + * @return DataResponse, array{}> * @throws OCSException * * 200: Share unshared successfully */ #[NoCSRFRequired] #[PublicPage] - public function unshare(int $id, ?string $token = null) { + public function unshare(string $id, ?string $token = null) { if (!$this->isS2SEnabled()) { throw new OCSException('Server does not support federated cloud sharing', 503); } @@ -275,7 +268,7 @@ public function unshare(int $id, ?string $token = null) { return new DataResponse(); } - private function cleanupRemote($remote) { + private function cleanupRemote(string $remote): string { $remote = substr($remote, strpos($remote, '://') + 3); return rtrim($remote, '/'); @@ -285,16 +278,16 @@ private function cleanupRemote($remote) { /** * federated share was revoked, either by the owner or the re-sharer * - * @param int $id ID of the share + * @param string $id ID of the share * @param string|null $token Shared secret between servers - * @return Http\DataResponse, array{}> + * @return DataResponse, array{}> * @throws OCSBadRequestException Revoking the share is not possible * * 200: Share revoked successfully */ #[NoCSRFRequired] #[PublicPage] - public function revoke(int $id, ?string $token = null) { + public function revoke(string $id, ?string $token = null) { try { $provider = $this->cloudFederationProviderManager->getCloudFederationProvider('file'); $notification = ['sharedSecret' => $token]; @@ -324,19 +317,19 @@ private function isS2SEnabled($incoming = false) { } /** - * update share information to keep federated re-shares in sync + * Update share information to keep federated re-shares in sync. * - * @param int $id ID of the share + * @param string $id ID of the share * @param string|null $token Shared secret between servers * @param int|null $permissions New permissions - * @return Http\DataResponse, array{}> + * @return DataResponse, array{}> * @throws OCSBadRequestException Updating permissions is not possible * * 200: Permissions updated successfully */ #[NoCSRFRequired] #[PublicPage] - public function updatePermissions(int $id, ?string $token = null, ?int $permissions = null) { + public function updatePermissions(string $id, ?string $token = null, ?int $permissions = null) { $ncPermissions = $permissions; try { @@ -385,7 +378,7 @@ protected function ncPermissions2ocmPermissions($ncPermissions) { * @param string|null $token Shared secret between servers * @param string|null $remote Address of the remote * @param string|null $remote_id ID of the remote - * @return Http\DataResponse + * @return DataResponse * @throws OCSBadRequestException Moving share is not possible * * 200: Share moved successfully diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 47ed3e893e927..1d1c00772c5bf 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -27,6 +27,7 @@ use OCP\Share\IShare; use OCP\Share\IShareProvider; use OCP\Share\IShareProviderSupportsAllSharesInFolder; +use Override; use Psr\Log\LoggerInterface; /** @@ -62,24 +63,19 @@ public function __construct( ) { } - /** - * Return the identifier of this provider. - * - * @return string Containing only [a-zA-Z0-9] - */ - public function identifier() { + #[Override] + public function identifier(): string { return 'ocFederatedSharing'; } /** * Share a path * - * @param IShare $share - * @return IShare The share object * @throws ShareNotFound * @throws \Exception */ - public function create(IShare $share) { + #[Override] + public function create(IShare $share): IShare { $shareWith = $share->getSharedWith(); $itemSource = $share->getNodeId(); $itemType = $share->getNodeType(); @@ -168,14 +164,12 @@ public function create(IShare $share) { } /** - * create federated share and inform the recipient + * Create federated share and inform the recipient. * - * @param IShare $share - * @return int * @throws ShareNotFound * @throws \Exception */ - protected function createFederatedShare(IShare $share) { + protected function createFederatedShare(IShare $share): string { $token = $this->tokenHandler->generateToken(); $shareId = $this->addShareToDB( $share->getNodeId(), @@ -293,9 +287,8 @@ protected function getShareFromExternalShareTable(IShare $share) { * @param string $token * @param int $shareType * @param \DateTime $expirationDate - * @return int */ - private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $shareType, $expirationDate) { + private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $shareType, $expirationDate): string { $qb = $this->dbConnection->getQueryBuilder(); $qb->insert('share') ->setValue('share_type', $qb->createNamedParameter($shareType)) @@ -317,16 +310,13 @@ private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ui $qb->setValue('file_target', $qb->createNamedParameter('')); $qb->executeStatement(); - return $qb->getLastInsertId(); + return (string)$qb->getLastInsertId(); } /** - * Update a share - * - * @param IShare $share - * @return IShare The share object + * Update a share. */ - public function update(IShare $share) { + public function update(IShare $share): IShare { /* * We allow updating the permissions of federated shares */ @@ -348,13 +338,12 @@ public function update(IShare $share) { } /** - * send the updated permission to the owner/initiator, if they are not the same + * Send the updated permission to the owner/initiator, if they are not the same. * - * @param IShare $share * @throws ShareNotFound * @throws HintException */ - protected function sendPermissionUpdate(IShare $share) { + protected function sendPermissionUpdate(IShare $share): void { $remoteId = $this->getRemoteId($share); // if the local user is the owner we send the permission change to the initiator if ($this->userManager->userExists($share->getShareOwner())) { @@ -367,12 +356,9 @@ protected function sendPermissionUpdate(IShare $share) { /** - * update successful reShare with the correct token - * - * @param int $shareId - * @param string $token + * Update successful reShare with the correct token. */ - protected function updateSuccessfulReShare($shareId, $token) { + protected function updateSuccessfulReShare(string $shareId, string $token): void { $query = $this->dbConnection->getQueryBuilder(); $query->update('share') ->where($query->expr()->eq('id', $query->createNamedParameter($shareId))) @@ -381,12 +367,9 @@ protected function updateSuccessfulReShare($shareId, $token) { } /** - * store remote ID in federated reShare table - * - * @param $shareId - * @param $remoteId + * Store remote ID in federated reShare table. */ - public function storeRemoteId(int $shareId, string $remoteId): void { + public function storeRemoteId(string $shareId, string $remoteId): void { $query = $this->dbConnection->getQueryBuilder(); $query->insert('federated_reshares') ->values( @@ -399,10 +382,8 @@ public function storeRemoteId(int $shareId, string $remoteId): void { } /** - * get share ID on remote server for federated re-shares + * Get share ID on remote server for federated re-shares. * - * @param IShare $share - * @return string * @throws ShareNotFound */ public function getRemoteId(IShare $share): string { @@ -512,11 +493,9 @@ public function removeShareFromTable(IShare $share) { } /** - * remove share from table - * - * @param string $shareId + * Remove share from table. */ - private function removeShareFromTableById($shareId) { + private function removeShareFromTableById(string $shareId): void { $qb = $this->dbConnection->getQueryBuilder(); $qb->delete('share') ->where($qb->expr()->eq('id', $qb->createNamedParameter($shareId))) @@ -748,14 +727,8 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset) { return $shares; } - /** - * Get a share by token - * - * @param string $token - * @return IShare - * @throws ShareNotFound - */ - public function getShareByToken($token) { + #[Override] + public function getShareByToken(string $token): IShare { $qb = $this->dbConnection->getQueryBuilder(); $cursor = $qb->select('*') @@ -812,9 +785,9 @@ private function getRawShare($id) { * @throws InvalidShare * @throws ShareNotFound */ - private function createShareObject($data) { + private function createShareObject($data): IShare { $share = new Share($this->rootFolder, $this->userManager); - $share->setId((int)$data['id']) + $share->setId((string)$data['id']) ->setShareType((int)$data['share_type']) ->setPermissions((int)$data['permissions']) ->setTarget($data['file_target']) @@ -894,7 +867,7 @@ public function userDeleted($uid, $shareType) { $qb = $this->dbConnection->getQueryBuilder(); $qb->delete('share_external') - ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_GROUP))) + ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_USER))) ->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($uid))) ->executeStatement(); } diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index 176301a1f2b90..9121cb1f2a62e 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -9,15 +9,18 @@ use NCU\Federation\ISignedCloudFederationProvider; use OC\AppFramework\Http; use OC\Files\Filesystem; +use OC\Files\SetupManager; use OCA\FederatedFileSharing\AddressHandler; use OCA\FederatedFileSharing\FederatedShareProvider; use OCA\Federation\TrustedServers; use OCA\Files_Sharing\Activity\Providers\RemoteShares; +use OCA\Files_Sharing\External\ExternalShare; +use OCA\Files_Sharing\External\ExternalShareMapper; use OCA\Files_Sharing\External\Manager; use OCA\GlobalSiteSelector\Service\SlaveService; +use OCA\Polls\Db\Share; use OCP\Activity\IManager as IActivityManager; use OCP\App\IAppManager; -use OCP\AppFramework\QueryException; use OCP\Constants; use OCP\Federation\Exceptions\ActionNotSupportedException; use OCP\Federation\Exceptions\AuthenticationFailedException; @@ -31,9 +34,9 @@ use OCP\Files\NotFoundException; use OCP\HintException; use OCP\IConfig; -use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IURLGenerator; +use OCP\IUser; use OCP\IUserManager; use OCP\Notification\IManager as INotificationManager; use OCP\Server; @@ -41,55 +44,44 @@ use OCP\Share\IManager; use OCP\Share\IProviderFactory; use OCP\Share\IShare; +use OCP\Snowflake\IGenerator; use OCP\Util; +use Override; use Psr\Log\LoggerInterface; use SensitiveParameter; class CloudFederationProviderFiles implements ISignedCloudFederationProvider { - /** - * CloudFederationProvider constructor. - */ public function __construct( - private IAppManager $appManager, - private FederatedShareProvider $federatedShareProvider, - private AddressHandler $addressHandler, - private IUserManager $userManager, - private IManager $shareManager, - private ICloudIdManager $cloudIdManager, - private IActivityManager $activityManager, - private INotificationManager $notificationManager, - private IURLGenerator $urlGenerator, - private ICloudFederationFactory $cloudFederationFactory, - private ICloudFederationProviderManager $cloudFederationProviderManager, - private IDBConnection $connection, - private IGroupManager $groupManager, - private IConfig $config, - private Manager $externalShareManager, - private LoggerInterface $logger, - private IFilenameValidator $filenameValidator, + private readonly IAppManager $appManager, + private readonly FederatedShareProvider $federatedShareProvider, + private readonly AddressHandler $addressHandler, + private readonly IUserManager $userManager, + private readonly IManager $shareManager, + private readonly ICloudIdManager $cloudIdManager, + private readonly IActivityManager $activityManager, + private readonly INotificationManager $notificationManager, + private readonly IURLGenerator $urlGenerator, + private readonly ICloudFederationFactory $cloudFederationFactory, + private readonly ICloudFederationProviderManager $cloudFederationProviderManager, + private readonly IGroupManager $groupManager, + private readonly IConfig $config, + private readonly Manager $externalShareManager, + private readonly LoggerInterface $logger, + private readonly IFilenameValidator $filenameValidator, private readonly IProviderFactory $shareProviderFactory, + private readonly SetupManager $setupManager, + private readonly IGenerator $snowflakeGenerator, + private readonly ExternalShareMapper $externalShareMapper, ) { } - /** - * @return string - */ - public function getShareType() { + #[Override] + public function getShareType(): string { return 'file'; } - /** - * share received from another server - * - * @param ICloudFederationShare $share - * @return string provider specific unique ID of the share - * - * @throws ProviderCouldNotAddShareException - * @throws QueryException - * @throws HintException - * @since 14.0.0 - */ - public function shareReceived(ICloudFederationShare $share) { + #[Override] + public function shareReceived(ICloudFederationShare $share): string { if (!$this->isS2SEnabled(true)) { throw new ProviderCouldNotAddShareException('Server does not support federated cloud sharing', '', Http::STATUS_SERVICE_UNAVAILABLE); } @@ -99,7 +91,8 @@ public function shareReceived(ICloudFederationShare $share) { throw new ProviderCouldNotAddShareException('Unsupported protocol for data exchange.', '', Http::STATUS_NOT_IMPLEMENTED); } - [$ownerUid, $remote] = $this->addressHandler->splitUserRemote($share->getOwner()); + [, $remote] = $this->addressHandler->splitUserRemote($share->getOwner()); + // for backward compatibility make sure that the remote url stored in the // database ends with a trailing slash if (!str_ends_with($remote, '/')) { @@ -109,17 +102,15 @@ public function shareReceived(ICloudFederationShare $share) { $token = $share->getShareSecret(); $name = $share->getResourceName(); $owner = $share->getOwnerDisplayName() ?: $share->getOwner(); - $sharedBy = $share->getSharedByDisplayName(); $shareWith = $share->getShareWith(); $remoteId = $share->getProviderId(); $sharedByFederatedId = $share->getSharedBy(); $ownerFederatedId = $share->getOwner(); $shareType = $this->mapShareTypeToNextcloud($share->getShareType()); - // if no explicit information about the person who created the share was send + // if no explicit information about the person who created the share was sent // we assume that the share comes from the owner if ($sharedByFederatedId === null) { - $sharedBy = $owner; $sharedByFederatedId = $ownerFederatedId; } @@ -128,7 +119,9 @@ public function shareReceived(ICloudFederationShare $share) { throw new ProviderCouldNotAddShareException('The mountpoint name contains invalid characters.', '', Http::STATUS_BAD_REQUEST); } - // FIXME this should be a method in the user management instead + $user = null; + $group = null; + if ($shareType === IShare::TYPE_USER) { $this->logger->debug('shareWith before, ' . $shareWith, ['app' => 'files_sharing']); Util::emitHook( @@ -138,20 +131,32 @@ public function shareReceived(ICloudFederationShare $share) { ); $this->logger->debug('shareWith after, ' . $shareWith, ['app' => 'files_sharing']); - if (!$this->userManager->userExists($shareWith)) { + $user = $this->userManager->get($shareWith); + if ($user === null) { throw new ProviderCouldNotAddShareException('User does not exists', '', Http::STATUS_BAD_REQUEST); } - \OC_Util::setupFS($shareWith); + $this->setupManager->setupForUser($user); + } else { + $group = $this->groupManager->get($shareWith); + if ($group === null) { + throw new ProviderCouldNotAddShareException('Group does not exists', '', Http::STATUS_BAD_REQUEST); + } } - if ($shareType === IShare::TYPE_GROUP && !$this->groupManager->groupExists($shareWith)) { - throw new ProviderCouldNotAddShareException('Group does not exists', '', Http::STATUS_BAD_REQUEST); - } + $externalShare = new ExternalShare(); + $externalShare->setId($this->snowflakeGenerator->nextId()); + $externalShare->setRemote($remote); + $externalShare->setRemoteId($remoteId); + $externalShare->setShareToken($token); + $externalShare->setPassword(''); + $externalShare->setName($name); + $externalShare->setOwner($owner); + $externalShare->setShareType($shareType); + $externalShare->setAccepted(IShare::STATUS_PENDING); try { - $this->externalShareManager->addShare($remote, $token, '', $name, $owner, $shareType, false, $shareWith, $remoteId); - $shareId = Server::get(IDBConnection::class)->lastInsertId('*PREFIX*share_external'); + $this->externalShareManager->addShare($externalShare, $user ?: $group); // get DisplayName about the owner of the share $ownerDisplayName = $this->getUserDisplayName($ownerFederatedId); @@ -166,41 +171,40 @@ public function shareReceived(ICloudFederationShare $share) { } } - if ($shareType === IShare::TYPE_USER) { $event = $this->activityManager->generateEvent(); $event->setApp('files_sharing') ->setType('remote_share') ->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_RECEIVED, [$ownerFederatedId, trim($name, '/'), $ownerDisplayName]) ->setAffectedUser($shareWith) - ->setObject('remote_share', $shareId, $name); + ->setObject('remote_share', $externalShare->getId(), $name); Server::get(IActivityManager::class)->publish($event); - $this->notifyAboutNewShare($shareWith, $shareId, $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName); + $this->notifyAboutNewShare($shareWith, $externalShare->getId(), $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName); // If auto-accept is enabled, accept the share if ($this->federatedShareProvider->isFederatedTrustedShareAutoAccept() && $trustedServers?->isTrustedServer($remote) === true) { - $this->externalShareManager->acceptShare($shareId, $shareWith); + $this->externalShareManager->acceptShare($externalShare, $user); } } else { - $groupMembers = $this->groupManager->get($shareWith)->getUsers(); + $groupMembers = $group->getUsers(); foreach ($groupMembers as $user) { $event = $this->activityManager->generateEvent(); $event->setApp('files_sharing') ->setType('remote_share') ->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_RECEIVED, [$ownerFederatedId, trim($name, '/'), $ownerDisplayName]) ->setAffectedUser($user->getUID()) - ->setObject('remote_share', $shareId, $name); + ->setObject('remote_share', $externalShare->getId(), $name); Server::get(IActivityManager::class)->publish($event); - $this->notifyAboutNewShare($user->getUID(), $shareId, $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName); + $this->notifyAboutNewShare($user->getUID(), $externalShare->getId(), $ownerFederatedId, $sharedByFederatedId, $name, $ownerDisplayName); // If auto-accept is enabled, accept the share if ($this->federatedShareProvider->isFederatedTrustedShareAutoAccept() && $trustedServers?->isTrustedServer($remote) === true) { - $this->externalShareManager->acceptShare($shareId, $user->getUID()); + $this->externalShareManager->acceptShare($externalShare, $user); } } } - return $shareId; + return $externalShare->getId(); } catch (\Exception $e) { $this->logger->error('Server can not add remote share.', [ 'app' => 'files_sharing', @@ -213,56 +217,28 @@ public function shareReceived(ICloudFederationShare $share) { throw new ProviderCouldNotAddShareException('server can not add remote share, missing parameter', '', HTTP::STATUS_BAD_REQUEST); } - /** - * notification received from another server - * - * @param string $notificationType (e.g. SHARE_ACCEPTED) - * @param string $providerId id of the share - * @param array $notification payload of the notification - * @return array data send back to the sender - * - * @throws ActionNotSupportedException - * @throws AuthenticationFailedException - * @throws BadRequestException - * @throws HintException - * @since 14.0.0 - */ - public function notificationReceived($notificationType, $providerId, array $notification) { - switch ($notificationType) { - case 'SHARE_ACCEPTED': - return $this->shareAccepted($providerId, $notification); - case 'SHARE_DECLINED': - return $this->shareDeclined($providerId, $notification); - case 'SHARE_UNSHARED': - return $this->unshare($providerId, $notification); - case 'REQUEST_RESHARE': - return $this->reshareRequested($providerId, $notification); - case 'RESHARE_UNDO': - return $this->undoReshare($providerId, $notification); - case 'RESHARE_CHANGE_PERMISSION': - return $this->updateResharePermissions($providerId, $notification); - } - - - throw new BadRequestException([$notificationType]); + #[Override] + public function notificationReceived(string $notificationType, string $providerId, array $notification): array { + return match ($notificationType) { + 'SHARE_ACCEPTED' => $this->shareAccepted($providerId, $notification), + 'SHARE_DECLINED' => $this->shareDeclined($providerId, $notification), + 'SHARE_UNSHARED' => $this->unshare($providerId, $notification), + 'REQUEST_RESHARE' => $this->reshareRequested($providerId, $notification), + 'RESHARE_UNDO' => $this->undoReshare($providerId, $notification), + 'RESHARE_CHANGE_PERMISSION' => $this->updateResharePermissions($providerId, $notification), + default => throw new BadRequestException([$notificationType]), + }; } /** - * map OCM share type (strings) to Nextcloud internal share types (integer) - * - * @param string $shareType - * @return int + * Map OCM share type (strings) to Nextcloud internal share types (integer) + * @return IShare::TYPE_GROUP|IShare::TYPE_USER */ - private function mapShareTypeToNextcloud($shareType) { - $result = IShare::TYPE_USER; - if ($shareType === 'group') { - $result = IShare::TYPE_GROUP; - } - - return $result; + private function mapShareTypeToNextcloud(string $shareType): int { + return $shareType === 'group' ? IShare::TYPE_GROUP : IShare::TYPE_USER; } - private function notifyAboutNewShare($shareWith, $shareId, $ownerFederatedId, $sharedByFederatedId, $name, $displayName): void { + private function notifyAboutNewShare(string $shareWith, string $shareId, $ownerFederatedId, $sharedByFederatedId, string $name, string $displayName): void { $notification = $this->notificationManager->createNotification(); $notification->setApp('files_sharing') ->setUser($shareWith) @@ -286,15 +262,14 @@ private function notifyAboutNewShare($shareWith, $shareId, $ownerFederatedId, $s /** * process notification that the recipient accepted a share * - * @param string $id - * @param array $notification + * @param array{sharedSecret?: string} $notification * @return array * @throws ActionNotSupportedException * @throws AuthenticationFailedException * @throws BadRequestException * @throws HintException */ - private function shareAccepted($id, array $notification) { + private function shareAccepted(string $id, array $notification): array { if (!$this->isS2SEnabled()) { throw new ActionNotSupportedException('Server does not support federated cloud sharing'); } @@ -333,21 +308,22 @@ private function shareAccepted($id, array $notification) { } /** - * @param IShare $share * @throws ShareNotFound */ - protected function executeAcceptShare(IShare $share) { + protected function executeAcceptShare(IShare $share): void { + $user = $this->getCorrectUser($share); + try { - $fileId = (int)$share->getNode()->getId(); - [$file, $link] = $this->getFile($this->getCorrectUid($share), $fileId); - } catch (\Exception $e) { + $fileId = $share->getNode()->getId(); + [$file, $link] = $this->getFile($user, $fileId); + } catch (\Exception) { throw new ShareNotFound(); } $event = $this->activityManager->generateEvent(); $event->setApp('files_sharing') ->setType('remote_share') - ->setAffectedUser($this->getCorrectUid($share)) + ->setAffectedUser($user->getUID()) ->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_ACCEPTED, [$share->getSharedWith(), [$fileId => $file]]) ->setObject('files', $fileId, $file) ->setLink($link); @@ -357,8 +333,7 @@ protected function executeAcceptShare(IShare $share) { /** * process notification that the recipient declined a share * - * @param string $id - * @param array $notification + * @param array{sharedSecret?: string} $notification * @return array * @throws ActionNotSupportedException * @throws AuthenticationFailedException @@ -367,7 +342,7 @@ protected function executeAcceptShare(IShare $share) { * @throws HintException * */ - protected function shareDeclined($id, array $notification) { + protected function shareDeclined(string $id, array $notification): array { if (!$this->isS2SEnabled()) { throw new ActionNotSupportedException('Server does not support federated cloud sharing'); } @@ -394,7 +369,6 @@ protected function shareDeclined($id, array $notification) { 'sharedSecret' => $token, 'message' => 'Recipient declined the re-share' ] - ); $this->cloudFederationProviderManager->sendNotification($remote, $notification); } @@ -407,23 +381,24 @@ protected function shareDeclined($id, array $notification) { /** * delete declined share and create a activity * - * @param IShare $share * @throws ShareNotFound */ - protected function executeDeclineShare(IShare $share) { + protected function executeDeclineShare(IShare $share): void { $this->federatedShareProvider->removeShareFromTable($share); + $user = $this->getCorrectUser($share); + try { - $fileId = (int)$share->getNode()->getId(); - [$file, $link] = $this->getFile($this->getCorrectUid($share), $fileId); - } catch (\Exception $e) { + $fileId = $share->getNode()->getId(); + [$file, $link] = $this->getFile($user, $fileId); + } catch (\Exception) { throw new ShareNotFound(); } $event = $this->activityManager->generateEvent(); $event->setApp('files_sharing') ->setType('remote_share') - ->setAffectedUser($this->getCorrectUid($share)) + ->setAffectedUser($user->getUID()) ->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_DECLINED, [$share->getSharedWith(), [$fileId => $file]]) ->setObject('files', $fileId, $file) ->setLink($link); @@ -433,13 +408,12 @@ protected function executeDeclineShare(IShare $share) { /** * received the notification that the owner unshared a file from you * - * @param string $id - * @param array $notification + * @param array{sharedSecret?: string} $notification * @return array * @throws AuthenticationFailedException * @throws BadRequestException */ - private function undoReshare($id, array $notification) { + private function undoReshare(string $id, array $notification): array { if (!isset($notification['sharedSecret'])) { throw new BadRequestException(['sharedSecret']); } @@ -455,13 +429,12 @@ private function undoReshare($id, array $notification) { /** * unshare file from self * - * @param string $id - * @param array $notification + * @param array{sharedSecret?: string} $notification * @return array * @throws ActionNotSupportedException * @throws BadRequestException */ - private function unshare($id, array $notification) { + private function unshare(string $id, array $notification): array { if (!$this->isS2SEnabled(true)) { throw new ActionNotSupportedException('incoming shares disabled!'); } @@ -471,56 +444,29 @@ private function unshare($id, array $notification) { } $token = $notification['sharedSecret']; - $qb = $this->connection->getQueryBuilder(); - $qb->select('*') - ->from('share_external') - ->where( - $qb->expr()->andX( - $qb->expr()->eq('remote_id', $qb->createNamedParameter($id)), - $qb->expr()->eq('share_token', $qb->createNamedParameter($token)) - ) - ); - - $result = $qb->executeQuery(); - $share = $result->fetchAssociative(); - $result->closeCursor(); + $share = $this->externalShareMapper->getShareByRemoteIdAndToken($id, $token); - if ($token && $id && !empty($share)) { - $remote = $this->cleanupRemote($share['remote']); + if ($token && $id && $share !== null) { + $remote = $this->cleanupRemote($share->getRemote()); - $owner = $this->cloudIdManager->getCloudId($share['owner'], $remote); - $mountpoint = $share['mountpoint']; - $user = $share['user']; + $owner = $this->cloudIdManager->getCloudId($share->getOwner(), $remote); + $mountpoint = $share->getMountpoint(); + $user = $share->getUser(); - $qb = $this->connection->getQueryBuilder(); - $qb->delete('share_external') - ->where( - $qb->expr()->andX( - $qb->expr()->eq('remote_id', $qb->createNamedParameter($id)), - $qb->expr()->eq('share_token', $qb->createNamedParameter($token)) - ) - ); - - $qb->executeStatement(); - - // delete all child in case of a group share - $qb = $this->connection->getQueryBuilder(); - $qb->delete('share_external') - ->where($qb->expr()->eq('parent', $qb->createNamedParameter((int)$share['id']))); - $qb->executeStatement(); + $this->externalShareMapper->delete($share); $ownerDisplayName = $this->getUserDisplayName($owner->getId()); - if ((int)$share['share_type'] === IShare::TYPE_USER) { - if ($share['accepted']) { + if ($share->getShareType() === IShare::TYPE_USER) { + if ($share->getAccepted()) { $path = trim($mountpoint, '/'); } else { - $path = trim($share['name'], '/'); + $path = trim($share->getName(), '/'); } $notification = $this->notificationManager->createNotification(); $notification->setApp('files_sharing') - ->setUser($share['user']) - ->setObject('remote_share', (string)$share['id']); + ->setUser($share->getUser()) + ->setObject('remote_share', $share->getId()); $this->notificationManager->markProcessed($notification); $event = $this->activityManager->generateEvent(); @@ -528,7 +474,7 @@ private function unshare($id, array $notification) { ->setType('remote_share') ->setSubject(RemoteShares::SUBJECT_REMOTE_SHARE_UNSHARED, [$owner->getId(), $path, $ownerDisplayName]) ->setAffectedUser($user) - ->setObject('remote_share', (int)$share['id'], $path); + ->setObject('remote_share', $share->getId(), $path); Server::get(IActivityManager::class)->publish($event); } } @@ -536,7 +482,7 @@ private function unshare($id, array $notification) { return []; } - private function cleanupRemote($remote) { + private function cleanupRemote(string $remote): string { $remote = substr($remote, strpos($remote, '://') + 3); return rtrim($remote, '/'); @@ -545,15 +491,14 @@ private function cleanupRemote($remote) { /** * recipient of a share request to re-share the file with another user * - * @param string $id - * @param array $notification + * @param array{sharedSecret?: string, shareWith?: string, senderId?: string} $notification * @return array * @throws AuthenticationFailedException * @throws BadRequestException * @throws ProviderCouldNotAddShareException * @throws ShareNotFound */ - protected function reshareRequested($id, array $notification) { + protected function reshareRequested(string $id, array $notification) { if (!isset($notification['sharedSecret'])) { throw new BadRequestException(['sharedSecret']); } @@ -595,7 +540,7 @@ protected function reshareRequested($id, array $notification) { $share->setSharedBy($share->getSharedWith()); $share->setSharedWith($shareWith); $result = $this->federatedShareProvider->create($share); - $this->federatedShareProvider->storeRemoteId((int)$result->getId(), $senderId); + $this->federatedShareProvider->storeRemoteId($result->getId(), $senderId); return ['token' => $result->getToken(), 'providerId' => $result->getId()]; } else { throw new ProviderCouldNotAddShareException('resharing not allowed for share: ' . $id); @@ -603,71 +548,22 @@ protected function reshareRequested($id, array $notification) { } /** - * update permission of a re-share so that the share dialog shows the right + * Update permission of a re-share so that the share dialog shows the right * permission if the owner or the sender changes the permission * - * @param string $id - * @param array $notification - * @return array + * @return string[] * @throws AuthenticationFailedException * @throws BadRequestException */ - protected function updateResharePermissions($id, array $notification) { + protected function updateResharePermissions(string $id, array $notification): array { throw new HintException('Updating reshares not allowed'); } /** - * translate OCM Permissions to Nextcloud permissions - * - * @param array $ocmPermissions - * @return int - * @throws BadRequestException - */ - protected function ocmPermissions2ncPermissions(array $ocmPermissions) { - $ncPermissions = 0; - foreach ($ocmPermissions as $permission) { - switch (strtolower($permission)) { - case 'read': - $ncPermissions += Constants::PERMISSION_READ; - break; - case 'write': - $ncPermissions += Constants::PERMISSION_CREATE + Constants::PERMISSION_UPDATE; - break; - case 'share': - $ncPermissions += Constants::PERMISSION_SHARE; - break; - default: - throw new BadRequestException(['permission']); - } - } - - return $ncPermissions; - } - - /** - * update permissions in database - * - * @param IShare $share - * @param int $permissions - */ - protected function updatePermissionsInDatabase(IShare $share, $permissions) { - $query = $this->connection->getQueryBuilder(); - $query->update('share') - ->where($query->expr()->eq('id', $query->createNamedParameter($share->getId()))) - ->set('permissions', $query->createNamedParameter($permissions)) - ->executeStatement(); - } - - - /** - * get file - * - * @param string $user - * @param int $fileSource - * @return array with internal path of the file and a absolute link to it + * @return list{?string, string} with internal path of the file and a absolute link to it */ - private function getFile($user, $fileSource) { - \OC_Util::setupFS($user); + private function getFile(IUser $user, int $fileSource): array { + $this->setupManager->setupForUser($user); try { $file = Filesystem::getPath($fileSource); @@ -681,30 +577,26 @@ private function getFile($user, $fileSource) { } /** - * check if we are the initiator or the owner of a re-share and return the correct UID - * - * @param IShare $share - * @return string + * Check if we are the initiator or the owner of a re-share and return the correct UID */ - protected function getCorrectUid(IShare $share) { - if ($this->userManager->userExists($share->getShareOwner())) { - return $share->getShareOwner(); + protected function getCorrectUser(IShare $share): IUser { + if ($user = $this->userManager->get($share->getShareOwner())) { + return $user; } - return $share->getSharedBy(); + $user = $this->userManager->get($share->getSharedBy()); + if ($user === null) { + throw new \RuntimeException('Neither the share owner or the share initiator exist'); + } + return $user; } - - /** * check if we got the right share * - * @param IShare $share - * @param string $token - * @return bool * @throws AuthenticationFailedException */ - protected function verifyShare(IShare $share, $token) { + protected function verifyShare(IShare $share, string $token): bool { if ( $share->getShareType() === IShare::TYPE_REMOTE && $share->getToken() === $token @@ -728,12 +620,9 @@ protected function verifyShare(IShare $share, $token) { /** - * check if server-to-server sharing is enabled - * - * @param bool $incoming - * @return bool + * Check if server-to-server sharing is enabled */ - private function isS2SEnabled($incoming = false) { + private function isS2SEnabled(bool $incoming = false): bool { $result = $this->appManager->isEnabledForUser('files_sharing'); if ($incoming) { @@ -745,19 +634,11 @@ private function isS2SEnabled($incoming = false) { return $result; } - - /** - * get the supported share types, e.g. "user", "group", etc. - * - * @return array - * - * @since 14.0.0 - */ - public function getSupportedShareTypes() { + #[Override] + public function getSupportedShareTypes(): array { return ['user', 'group']; } - public function getUserDisplayName(string $userId): string { // check if gss is enabled and available if (!$this->appManager->isEnabledForAnyone('globalsiteselector') @@ -768,7 +649,7 @@ public function getUserDisplayName(string $userId): string { try { $slaveService = Server::get(SlaveService::class); } catch (\Throwable $e) { - Server::get(LoggerInterface::class)->error( + $this->logger->error( $e->getMessage(), ['exception' => $e] ); @@ -778,13 +659,7 @@ public function getUserDisplayName(string $userId): string { return $slaveService->getUserDisplayName($this->cloudIdManager->removeProtocolFromUrl($userId), false); } - /** - * @inheritDoc - * - * @param string $sharedSecret - * @param array $payload - * @return string - */ + #[Override] public function getFederationIdFromSharedSecret( #[SensitiveParameter] string $sharedSecret, @@ -800,7 +675,7 @@ public function getFederationIdFromSharedSecret( return ''; } - return $share['user'] . '@' . $share['remote']; + return $share->getUser() . '@' . $share->getRemote(); } // if uid_owner is a local account, the request comes from the recipient diff --git a/apps/federatedfilesharing/openapi.json b/apps/federatedfilesharing/openapi.json index 411ff856b18c5..bd172c574cdba 100644 --- a/apps/federatedfilesharing/openapi.json +++ b/apps/federatedfilesharing/openapi.json @@ -192,8 +192,7 @@ "description": "ID of the user that receives the share" }, "remoteId": { - "type": "integer", - "format": "int64", + "type": "string", "nullable": true, "default": null, "description": "ID of the remote" @@ -313,8 +312,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -405,7 +403,7 @@ "/ocs/v2.php/cloud/shares/{id}/permissions": { "post": { "operationId": "request_handler-update-permissions", - "summary": "update share information to keep federated re-shares in sync", + "summary": "Update share information to keep federated re-shares in sync.", "tags": [ "request_handler" ], @@ -450,8 +448,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -566,8 +563,7 @@ "description": "ID of the remote share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -664,8 +660,7 @@ "description": "ID of the remote share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -752,8 +747,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -840,8 +834,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { diff --git a/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php b/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php index e293f440fe385..ca09587cbe897 100644 --- a/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php +++ b/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php @@ -8,10 +8,8 @@ */ namespace OCA\FederatedFileSharing\Tests; -use OCA\FederatedFileSharing\AddressHandler; use OCA\FederatedFileSharing\Controller\RequestHandlerController; use OCA\FederatedFileSharing\FederatedShareProvider; -use OCA\FederatedFileSharing\Notifications; use OCP\AppFramework\Http\DataResponse; use OCP\EventDispatcher\IEventDispatcher; use OCP\Federation\ICloudFederationFactory; @@ -21,8 +19,6 @@ use OCP\Federation\ICloudIdManager; use OCP\IDBConnection; use OCP\IRequest; -use OCP\IUserManager; -use OCP\Share; use OCP\Share\IShare; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -42,15 +38,11 @@ class RequestHandlerControllerTest extends \Test\TestCase { private RequestHandlerController $requestHandler; private FederatedShareProvider&MockObject $federatedShareProvider; - private Notifications&MockObject $notifications; - private AddressHandler&MockObject $addressHandler; - private IUserManager&MockObject $userManager; private IShare&MockObject $share; private ICloudIdManager&MockObject $cloudIdManager; private LoggerInterface&MockObject $logger; private IRequest&MockObject $request; private IDBConnection&MockObject $connection; - private Share\IManager&MockObject $shareManager; private ICloudFederationFactory&MockObject $cloudFederationFactory; private ICloudFederationProviderManager&MockObject $cloudFederationProviderManager; private ICloudFederationProvider&MockObject $cloudFederationProvider; @@ -67,13 +59,9 @@ protected function setUp(): void { $this->federatedShareProvider->expects($this->any())->method('getShareById') ->willReturn($this->share); - $this->notifications = $this->createMock(Notifications::class); - $this->addressHandler = $this->createMock(AddressHandler::class); - $this->userManager = $this->createMock(IUserManager::class); $this->cloudIdManager = $this->createMock(ICloudIdManager::class); $this->request = $this->createMock(IRequest::class); $this->connection = $this->createMock(IDBConnection::class); - $this->shareManager = $this->createMock(Share\IManager::class); $this->cloudFederationFactory = $this->createMock(ICloudFederationFactory::class); $this->cloudFederationProviderManager = $this->createMock(ICloudFederationProviderManager::class); $this->cloudFederationProvider = $this->createMock(ICloudFederationProvider::class); @@ -88,10 +76,6 @@ protected function setUp(): void { $this->request, $this->federatedShareProvider, $this->connection, - $this->shareManager, - $this->notifications, - $this->addressHandler, - $this->userManager, $this->cloudIdManager, $this->logger, $this->cloudFederationFactory, @@ -106,7 +90,7 @@ public function testCreateShare(): void { $this->user2, 'name', '', - 1, + '1', $this->ownerCloudId, $this->owner, $this->user1CloudId, @@ -125,13 +109,13 @@ public function testCreateShare(): void { $this->cloudFederationProvider->expects($this->once())->method('shareReceived') ->with($this->cloudFederationShare); - $result = $this->requestHandler->createShare('localhost', 'token', 'name', $this->owner, $this->user1, $this->user2, 1, $this->user1CloudId, $this->ownerCloudId); + $result = $this->requestHandler->createShare('localhost', 'token', 'name', $this->owner, $this->user1, $this->user2, '1', $this->user1CloudId, $this->ownerCloudId); $this->assertInstanceOf(DataResponse::class, $result); } public function testDeclineShare(): void { - $id = 42; + $id = '42'; $notification = [ 'sharedSecret' => 'token', @@ -154,7 +138,7 @@ public function testDeclineShare(): void { public function testAcceptShare(): void { - $id = 42; + $id = '42'; $notification = [ 'sharedSecret' => 'token', diff --git a/apps/files_external/tests/Config/UserPlaceholderHandlerTest.php b/apps/files_external/tests/Config/UserPlaceholderHandlerTest.php index 13d25ce5075a6..0e4805e86dcf5 100644 --- a/apps/files_external/tests/Config/UserPlaceholderHandlerTest.php +++ b/apps/files_external/tests/Config/UserPlaceholderHandlerTest.php @@ -14,9 +14,11 @@ use OCP\IUserSession; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; +use Test\TestCase; -class UserPlaceholderHandlerTest extends \Test\TestCase { +class UserPlaceholderHandlerTest extends TestCase { protected IUser&MockObject $user; protected IUserSession&MockObject $session; protected IManager&MockObject $shareManager; @@ -34,6 +36,9 @@ protected function setUp(): void { $this->session = $this->createMock(IUserSession::class); $this->shareManager = $this->createMock(IManager::class); $this->request = $this->createMock(IRequest::class); + $this->request->method('getParam') + ->with('token') + ->willReturn('foo'); $this->userManager = $this->createMock(IUserManager::class); $this->handler = new UserPlaceholderHandler($this->session, $this->shareManager, $this->request, $this->userManager); @@ -53,16 +58,17 @@ public static function optionProvider(): array { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('optionProvider')] + #[DataProvider('optionProvider')] public function testHandle(string|array $option, string|array $expected): void { $this->setUser(); $this->assertSame($expected, $this->handler->handle($option)); } - #[\PHPUnit\Framework\Attributes\DataProvider('optionProvider')] + #[DataProvider('optionProvider')] public function testHandleNoUser(string|array $option): void { $this->shareManager->expects($this->once()) ->method('getShareByToken') + ->with('foo') ->willThrowException(new ShareNotFound()); $this->assertSame($option, $this->handler->handle($option)); } diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index dda5b5c7f7728..48f197f9bf937 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -50,6 +50,8 @@ 'OCA\\Files_Sharing\\Exceptions\\SharingRightsException' => $baseDir . '/../lib/Exceptions/SharingRightsException.php', 'OCA\\Files_Sharing\\ExpireSharesJob' => $baseDir . '/../lib/ExpireSharesJob.php', 'OCA\\Files_Sharing\\External\\Cache' => $baseDir . '/../lib/External/Cache.php', + 'OCA\\Files_Sharing\\External\\ExternalShare' => $baseDir . '/../lib/External/ExternalShare.php', + 'OCA\\Files_Sharing\\External\\ExternalShareMapper' => $baseDir . '/../lib/External/ExternalShareMapper.php', 'OCA\\Files_Sharing\\External\\Manager' => $baseDir . '/../lib/External/Manager.php', 'OCA\\Files_Sharing\\External\\Mount' => $baseDir . '/../lib/External/Mount.php', 'OCA\\Files_Sharing\\External\\MountProvider' => $baseDir . '/../lib/External/MountProvider.php', @@ -82,6 +84,7 @@ 'OCA\\Files_Sharing\\Migration\\Version24000Date20220404142216' => $baseDir . '/../lib/Migration/Version24000Date20220404142216.php', 'OCA\\Files_Sharing\\Migration\\Version31000Date20240821142813' => $baseDir . '/../lib/Migration/Version31000Date20240821142813.php', 'OCA\\Files_Sharing\\Migration\\Version32000Date20251017081948' => $baseDir . '/../lib/Migration/Version32000Date20251017081948.php', + 'OCA\\Files_Sharing\\Migration\\Version33000Date20251030081948' => $baseDir . '/../lib/Migration/Version33000Date20251030081948.php', 'OCA\\Files_Sharing\\MountProvider' => $baseDir . '/../lib/MountProvider.php', 'OCA\\Files_Sharing\\Notification\\Listener' => $baseDir . '/../lib/Notification/Listener.php', 'OCA\\Files_Sharing\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php', diff --git a/apps/files_sharing/composer/composer/autoload_static.php b/apps/files_sharing/composer/composer/autoload_static.php index 4f65cfe110512..110a64fb3acfa 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -65,6 +65,8 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\Exceptions\\SharingRightsException' => __DIR__ . '/..' . '/../lib/Exceptions/SharingRightsException.php', 'OCA\\Files_Sharing\\ExpireSharesJob' => __DIR__ . '/..' . '/../lib/ExpireSharesJob.php', 'OCA\\Files_Sharing\\External\\Cache' => __DIR__ . '/..' . '/../lib/External/Cache.php', + 'OCA\\Files_Sharing\\External\\ExternalShare' => __DIR__ . '/..' . '/../lib/External/ExternalShare.php', + 'OCA\\Files_Sharing\\External\\ExternalShareMapper' => __DIR__ . '/..' . '/../lib/External/ExternalShareMapper.php', 'OCA\\Files_Sharing\\External\\Manager' => __DIR__ . '/..' . '/../lib/External/Manager.php', 'OCA\\Files_Sharing\\External\\Mount' => __DIR__ . '/..' . '/../lib/External/Mount.php', 'OCA\\Files_Sharing\\External\\MountProvider' => __DIR__ . '/..' . '/../lib/External/MountProvider.php', @@ -97,6 +99,7 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\Migration\\Version24000Date20220404142216' => __DIR__ . '/..' . '/../lib/Migration/Version24000Date20220404142216.php', 'OCA\\Files_Sharing\\Migration\\Version31000Date20240821142813' => __DIR__ . '/..' . '/../lib/Migration/Version31000Date20240821142813.php', 'OCA\\Files_Sharing\\Migration\\Version32000Date20251017081948' => __DIR__ . '/..' . '/../lib/Migration/Version32000Date20251017081948.php', + 'OCA\\Files_Sharing\\Migration\\Version33000Date20251030081948' => __DIR__ . '/..' . '/../lib/Migration/Version33000Date20251030081948.php', 'OCA\\Files_Sharing\\MountProvider' => __DIR__ . '/..' . '/../lib/MountProvider.php', 'OCA\\Files_Sharing\\Notification\\Listener' => __DIR__ . '/..' . '/../lib/Notification/Listener.php', 'OCA\\Files_Sharing\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php', diff --git a/apps/files_sharing/lib/Command/CleanupRemoteStorages.php b/apps/files_sharing/lib/Command/CleanupRemoteStorages.php index 18d1c2a11570e..1200bfde5af8b 100644 --- a/apps/files_sharing/lib/Command/CleanupRemoteStorages.php +++ b/apps/files_sharing/lib/Command/CleanupRemoteStorages.php @@ -22,13 +22,13 @@ class CleanupRemoteStorages extends Command { public function __construct( - protected IDBConnection $connection, - private ICloudIdManager $cloudIdManager, + protected readonly IDBConnection $connection, + private readonly ICloudIdManager $cloudIdManager, ) { parent::__construct(); } - protected function configure() { + protected function configure(): void { $this ->setName('sharing:cleanup-remote-storages') ->setDescription('Cleanup shared storage entries that have no matching entry in the shares_external table') @@ -74,10 +74,11 @@ public function execute(InputInterface $input, OutputInterface $output): int { } } } - return 0; + + return Command::SUCCESS; } - public function countFiles($numericId, OutputInterface $output) { + public function countFiles($numericId, OutputInterface $output): void { $queryBuilder = $this->connection->getQueryBuilder(); $queryBuilder->select($queryBuilder->func()->count('fileid')) ->from('filecache') @@ -92,7 +93,7 @@ public function countFiles($numericId, OutputInterface $output) { $output->writeln("$count files can be deleted for storage $numericId"); } - public function deleteStorage($id, $numericId, OutputInterface $output) { + public function deleteStorage($id, $numericId, OutputInterface $output): void { $queryBuilder = $this->connection->getQueryBuilder(); $queryBuilder->delete('storages') ->where($queryBuilder->expr()->eq( @@ -106,7 +107,7 @@ public function deleteStorage($id, $numericId, OutputInterface $output) { $this->deleteFiles($numericId, $output); } - public function deleteFiles($numericId, OutputInterface $output) { + public function deleteFiles($numericId, OutputInterface $output): void { $queryBuilder = $this->connection->getQueryBuilder(); $queryBuilder->delete('filecache') ->where($queryBuilder->expr()->eq( @@ -119,7 +120,11 @@ public function deleteFiles($numericId, OutputInterface $output) { $output->writeln("deleted $count files"); } - public function getRemoteStorages() { + /** + * @return array + * @throws \OCP\DB\Exception + */ + private function getRemoteStorages(): array { $queryBuilder = $this->connection->getQueryBuilder(); $queryBuilder->select(['id', 'numeric_id']) ->from('storages') @@ -148,7 +153,10 @@ public function getRemoteStorages() { return $remoteStorages; } - public function getRemoteShareIds() { + /** + * @return array + */ + private function getRemoteShareIds(): array { $queryBuilder = $this->connection->getQueryBuilder(); $queryBuilder->select(['id', 'share_token', 'owner', 'remote']) ->from('share_external'); @@ -159,7 +167,6 @@ public function getRemoteShareIds() { while ($row = $result->fetchAssociative()) { $cloudId = $this->cloudIdManager->getCloudId($row['owner'], $row['remote']); $remote = $cloudId->getRemote(); - $remoteShareIds[$row['id']] = 'shared::' . md5($row['share_token'] . '@' . $remote); } $result->closeCursor(); diff --git a/apps/files_sharing/lib/Controller/ExternalSharesController.php b/apps/files_sharing/lib/Controller/ExternalSharesController.php index fa828a9d2c2cb..7f266ea53a3b7 100644 --- a/apps/files_sharing/lib/Controller/ExternalSharesController.php +++ b/apps/files_sharing/lib/Controller/ExternalSharesController.php @@ -7,6 +7,7 @@ */ namespace OCA\Files_Sharing\Controller; +use OCA\Files_Sharing\External\Manager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\JSONResponse; @@ -21,42 +22,40 @@ class ExternalSharesController extends Controller { public function __construct( string $appName, IRequest $request, - private \OCA\Files_Sharing\External\Manager $externalManager, + private readonly Manager $externalManager, ) { parent::__construct($appName, $request); } /** * @NoOutgoingFederatedSharingRequired - * - * @return JSONResponse */ #[NoAdminRequired] - public function index() { + public function index(): JSONResponse { return new JSONResponse($this->externalManager->getOpenShares()); } /** * @NoOutgoingFederatedSharingRequired - * - * @param int $id - * @return JSONResponse */ #[NoAdminRequired] - public function create($id) { - $this->externalManager->acceptShare($id); + public function create(string $id): JSONResponse { + $externalShare = $this->externalManager->getShare($id); + if ($externalShare !== false) { + $this->externalManager->acceptShare($externalShare); + } return new JSONResponse(); } /** * @NoOutgoingFederatedSharingRequired - * - * @param integer $id - * @return JSONResponse */ #[NoAdminRequired] - public function destroy($id) { - $this->externalManager->declineShare($id); + public function destroy(string $id): JSONResponse { + $externalShare = $this->externalManager->getShare($id); + if ($externalShare !== false) { + $this->externalManager->declineShare($externalShare); + } return new JSONResponse(); } } diff --git a/apps/files_sharing/lib/Controller/RemoteController.php b/apps/files_sharing/lib/Controller/RemoteController.php index 8c15cd8463e4c..597323f674cc3 100644 --- a/apps/files_sharing/lib/Controller/RemoteController.php +++ b/apps/files_sharing/lib/Controller/RemoteController.php @@ -7,7 +7,7 @@ */ namespace OCA\Files_Sharing\Controller; -use OC\Files\View; +use OCA\Files_Sharing\External\ExternalShare; use OCA\Files_Sharing\External\Manager; use OCA\Files_Sharing\ResponseDefinitions; use OCP\AppFramework\Http; @@ -16,25 +16,25 @@ use OCP\AppFramework\OCS\OCSForbiddenException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; +use OCP\Files\IRootFolder; use OCP\IRequest; use Psr\Log\LoggerInterface; /** * @psalm-import-type Files_SharingRemoteShare from ResponseDefinitions + * @api */ class RemoteController extends OCSController { /** - * Remote constructor. - * - * @param string $appName - * @param IRequest $request - * @param Manager $externalManager + * Remote controller constructor. */ public function __construct( - $appName, + string $appName, IRequest $request, - private Manager $externalManager, - private LoggerInterface $logger, + private readonly Manager $externalManager, + private readonly LoggerInterface $logger, + private readonly ?string $userId, + private readonly IRootFolder $rootFolder, ) { parent::__construct($appName, $request); } @@ -47,71 +47,85 @@ public function __construct( * 200: Pending remote shares returned */ #[NoAdminRequired] - public function getOpenShares() { - return new DataResponse($this->externalManager->getOpenShares()); + public function getOpenShares(): DataResponse { + $shares = $this->externalManager->getOpenShares(); + $shares = array_map(fn (ExternalShare $share) => $this->extendShareInfo($share), $shares); + return new DataResponse($shares); } /** - * Accept a remote share + * Accept a remote share. * - * @param int $id ID of the share + * @param string $id ID of the share * @return DataResponse, array{}> * @throws OCSNotFoundException Share not found * * 200: Share accepted successfully */ #[NoAdminRequired] - public function acceptShare($id) { - if ($this->externalManager->acceptShare($id)) { - return new DataResponse(); + public function acceptShare(string $id): DataResponse { + $externalShare = $this->externalManager->getShare($id); + 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.'); } - $this->logger->error('Could not accept federated share with id: ' . $id, - ['app' => 'files_sharing']); + if (!$this->externalManager->acceptShare($externalShare)) { + $this->logger->error('Could not accept federated share with id: ' . $id, ['app' => 'files_sharing']); + throw new OCSNotFoundException('Wrong share ID, share does not exist.'); + } - throw new OCSNotFoundException('wrong share ID, share does not exist.'); + return new DataResponse(); } /** - * Decline a remote share + * Decline a remote share. * - * @param int $id ID of the share + * @param string $id ID of the share * @return DataResponse, array{}> * @throws OCSNotFoundException Share not found * * 200: Share declined successfully */ #[NoAdminRequired] - public function declineShare($id) { - if ($this->externalManager->declineShare($id)) { - return new DataResponse(); + public function declineShare(string $id): DataResponse { + $externalShare = $this->externalManager->getShare($id); + 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.'); } - // Make sure the user has no notification for something that does not exist anymore. - $this->externalManager->processNotification($id); + if (!$this->externalManager->declineShare($externalShare)) { + $this->logger->error('Could not decline federated share with id: ' . $id, ['app' => 'files_sharing']); + throw new OCSNotFoundException('Wrong share ID, share does not exist.'); + } - throw new OCSNotFoundException('wrong share ID, share does not exist.'); + return new DataResponse(); } /** - * @param array $share Share with info from the share_external table - * @return array enriched share info with data from the filecache + * @param ExternalShare $share Share with info from the share_external table + * @return Files_SharingRemoteShare Enriched share info with data from the filecache */ - private static function extendShareInfo($share) { - $view = new View('/' . \OC_User::getUser() . '/files/'); - $info = $view->getFileInfo($share['mountpoint']); + private function extendShareInfo(ExternalShare $share): array { + $shareData = $share->jsonSerialize(); - if ($info === false) { - return $share; + $shareData['parent'] = $shareData['parent'] !== '-1' ? $shareData['parent'] : null; + $userFolder = $this->rootFolder->getUserFolder($this->userId); + + try { + $mountPointNode = $userFolder->get($share->getMountpoint()); + } catch (\Exception) { + return $shareData; } - $share['mimetype'] = $info->getMimetype(); - $share['mtime'] = $info->getMTime(); - $share['permissions'] = $info->getPermissions(); - $share['type'] = $info->getType(); - $share['file_id'] = $info->getId(); + $shareData['mimetype'] = $mountPointNode->getMimetype(); + $shareData['mtime'] = $mountPointNode->getMTime(); + $shareData['permissions'] = $mountPointNode->getPermissions(); + $shareData['type'] = $mountPointNode->getType(); + $shareData['file_id'] = $mountPointNode->getId(); - return $share; + return $shareData; } /** @@ -122,30 +136,29 @@ private static function extendShareInfo($share) { * 200: Accepted remote shares returned */ #[NoAdminRequired] - public function getShares() { + public function getShares(): DataResponse { $shares = $this->externalManager->getAcceptedShares(); - $shares = array_map(self::extendShareInfo(...), $shares); - + $shares = array_map(fn (ExternalShare $share) => $this->extendShareInfo($share), $shares); return new DataResponse($shares); } /** * Get info of a remote share * - * @param int $id ID of the share + * @param string $id ID of the share * @return DataResponse * @throws OCSNotFoundException Share not found * * 200: Share returned */ #[NoAdminRequired] - public function getShare($id) { + public function getShare(string $id): DataResponse { $shareInfo = $this->externalManager->getShare($id); if ($shareInfo === false) { throw new OCSNotFoundException('share does not exist'); } else { - $shareInfo = self::extendShareInfo($shareInfo); + $shareInfo = $this->extendShareInfo($shareInfo); return new DataResponse($shareInfo); } } @@ -153,7 +166,7 @@ public function getShare($id) { /** * Unshare a remote share * - * @param int $id ID of the share + * @param string $id ID of the share * @return DataResponse, array{}> * @throws OCSNotFoundException Share not found * @throws OCSForbiddenException Unsharing is not possible @@ -161,14 +174,14 @@ public function getShare($id) { * 200: Share unshared successfully */ #[NoAdminRequired] - public function unshare($id) { + public function unshare(string $id): DataResponse { $shareInfo = $this->externalManager->getShare($id); if ($shareInfo === false) { throw new OCSNotFoundException('Share does not exist'); } - $mountPoint = '/' . \OC_User::getUser() . '/files' . $shareInfo['mountpoint']; + $mountPoint = '/' . $this->userId . '/files' . $shareInfo->getMountpoint(); if ($this->externalManager->removeShare($mountPoint) === true) { return new DataResponse(); diff --git a/apps/files_sharing/lib/Controller/ShareesAPIController.php b/apps/files_sharing/lib/Controller/ShareesAPIController.php index 8d88af0c19f3d..6f2185d87e9c7 100644 --- a/apps/files_sharing/lib/Controller/ShareesAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareesAPIController.php @@ -39,14 +39,11 @@ */ class ShareesAPIController extends OCSController { - /** @var int */ - protected $offset = 0; - - /** @var int */ - protected $limit = 10; + protected int $offset = 0; + protected int $limit = 10; /** @var Files_SharingShareesSearchResult */ - protected $result = [ + protected array $result = [ 'exact' => [ 'users' => [], 'groups' => [], @@ -67,8 +64,6 @@ class ShareesAPIController extends OCSController { 'lookupEnabled' => false, ]; - protected $reachedEndFor = []; - public function __construct( string $appName, IRequest $request, diff --git a/apps/files_sharing/lib/External/ExternalShare.php b/apps/files_sharing/lib/External/ExternalShare.php new file mode 100644 index 0000000000000..db2c08f9dafa3 --- /dev/null +++ b/apps/files_sharing/lib/External/ExternalShare.php @@ -0,0 +1,139 @@ +addType('id', Types::STRING); // Stored as a bigint + $this->addType('parent', Types::STRING); // Stored as a bigint + $this->addType('shareType', Types::INTEGER); + $this->addType('remote', Types::STRING); + $this->addType('remoteId', Types::STRING); + $this->addType('shareToken', Types::STRING); + $this->addType('password', Types::STRING); + $this->addType('name', Types::STRING); + $this->addType('owner', Types::STRING); + $this->addType('user', Types::STRING); + $this->addType('mountpoint', Types::STRING); + $this->addType('mountpointHash', Types::STRING); + $this->addType('accepted', Types::INTEGER); + } + + public function setMountpoint(string $mountPoint): void { + $this->setter('mountpoint', [$mountPoint]); + $this->setMountpointHash(md5($mountPoint)); + } + + public function setName(string $name): void { + $name = Filesystem::normalizePath('/' . $name); + $this->setter('name', [$name]); + } + + public function setShareWith(IUser|IGroup $shareWith): void { + $this->setUser($shareWith instanceof IGroup ? $shareWith->getGID() : $shareWith->getUID()); + } + + /** + * @return Files_SharingRemoteShare + */ + public function jsonSerialize(): array { + $parent = $this->getParent(); + return [ + 'id' => $this->getId(), + 'parent' => $parent, + 'share_type' => $this->getShareType() ?? IShare::TYPE_USER, // unfortunately nullable on the DB level, but never null. + 'remote' => $this->getRemote(), + 'remote_id' => $this->getRemoteId(), + 'share_token' => $this->getShareToken(), + 'name' => $this->getName(), + 'owner' => $this->getOwner(), + 'user' => $this->getUser(), + 'mountpoint' => $this->getMountpoint(), + 'accepted' => $this->getAccepted(), + + // Added later on + 'file_id' => null, + 'mimetype' => null, + 'permissions' => null, + 'mtime' => null, + 'type' => null, + ]; + } + + /** + * @internal For unit tests + * @return ExternalShare + */ + public function clone(): self { + $newShare = new ExternalShare(); + $newShare->setParent($this->getParent()); + $newShare->setShareType($this->getShareType()); + $newShare->setRemote($this->getRemote()); + $newShare->setRemoteId($this->getRemoteId()); + $newShare->setShareToken($this->getShareToken()); + $newShare->setPassword($this->getPassword()); + $newShare->setName($this->getName()); + $newShare->setOwner($this->getOwner()); + $newShare->setMountpoint($this->getMountpoint()); + $newShare->setAccepted($this->getAccepted()); + $newShare->setPassword($this->getPassword()); + return $newShare; + } +} diff --git a/apps/files_sharing/lib/External/ExternalShareMapper.php b/apps/files_sharing/lib/External/ExternalShareMapper.php new file mode 100644 index 0000000000000..3349981e84d2c --- /dev/null +++ b/apps/files_sharing/lib/External/ExternalShareMapper.php @@ -0,0 +1,252 @@ + + */ +class ExternalShareMapper extends QBMapper { + private const TABLE_NAME = 'share_external'; + + public function __construct( + IDBConnection $db, + private readonly IGroupManager $groupManager, + ) { + parent::__construct($db, self::TABLE_NAME); + } + + /** + * @throws DoesNotExistException + */ + public function getById(string $id): ExternalShare { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from(self::TABLE_NAME) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($id))) + ->setMaxResults(1); + return $this->findEntity($qb); + } + + /** + * Get share by token. + * + * @throws DoesNotExistException + */ + public function getShareByToken(string $token): ExternalShare { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from(self::TABLE_NAME) + ->where($qb->expr()->eq('share_token', $qb->createNamedParameter($token, IQueryBuilder::PARAM_STR))) + ->setMaxResults(1); + return $this->findEntity($qb); + } + + /** + * Get share by parent id and user. + * + * @throws DoesNotExistException + */ + public function getUserShare(ExternalShare $parentShare, IUser $user): ExternalShare { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from(self::TABLE_NAME) + ->where($qb->expr()->andX( + $qb->expr()->eq('parent', $qb->createNamedParameter($parentShare->getId())), + $qb->expr()->eq('user', $qb->createNamedParameter($user->getUID(), IQueryBuilder::PARAM_STR)), + )); + return $this->findEntity($qb); + } + + public function getByMountPointAndUser(string $mountPoint, IUser $user) { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from(self::TABLE_NAME) + ->where($qb->expr()->andX( + $qb->expr()->eq('mountpoint_hash', $qb->createNamedParameter(md5($mountPoint))), + $qb->expr()->eq('user', $qb->createNamedParameter($user->getUID(), IQueryBuilder::PARAM_STR)), + )); + return $this->findEntity($qb); + } + + /** + * @return \Generator + * @throws Exception + */ + public function getUserShares(IUser $user): \Generator { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from(self::TABLE_NAME) + ->where($qb->expr()->eq('user', $qb->createNamedParameter($user->getUID(), IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_USER, IQueryBuilder::PARAM_INT))); + + return $this->yieldEntities($qb); + } + + public function deleteUserShares(IUser $user): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete(self::TABLE_NAME) + // user field can specify a user or a group + ->where($qb->expr()->eq('user', $qb->createNamedParameter($user->getUID()))) + ->andWhere( + $qb->expr()->orX( + // delete direct shares + $qb->expr()->eq('share_type', $qb->expr()->literal(IShare::TYPE_USER)), + // delete sub-shares of group shares for that user + $qb->expr()->andX( + $qb->expr()->eq('share_type', $qb->expr()->literal(IShare::TYPE_GROUP)), + $qb->expr()->neq('parent', $qb->expr()->literal(-1)), + ) + ) + ); + $qb->executeStatement(); + } + + /** + * @throws Exception + */ + public function deleteGroupShares(IGroup $group): void { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from(self::TABLE_NAME) + ->where($qb->expr()->eq('user', $qb->createNamedParameter($group->getGID()))) + ->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_GROUP))); + + $this->yieldEntities($qb); + + $delete = $this->db->getQueryBuilder(); + $delete->delete(self::TABLE_NAME) + ->where( + $qb->expr()->orX( + $qb->expr()->eq('id', $qb->createParameter('share_id')), + $qb->expr()->eq('parent', $qb->createParameter('share_parent_id')) + ) + ); + + foreach ($this->yieldEntities($qb) as $share) { + $delete->setParameter('share_id', $share->getId()); + $delete->setParameter('share_parent_id', $share->getId()); + $delete->executeStatement(); + } + } + + /** + * @return \Generator + */ + public function getAllShares(): \Generator { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from(self::TABLE_NAME); + + return $this->yieldEntities($qb); + } + + /** + * Return a list of shares for the user. + * + * @psalm-param IShare::STATUS_PENDING|IShare::STATUS_ACCEPTED|null $status Filter share by their status or return all shares of the user if null. + * @return list list of open server-to-server shares + * @throws Exception + */ + public function getShares(IUser $user, ?int $status): array { + // Not allowing providing a user here, + // as we only want to retrieve shares for the current user. + $groups = $this->groupManager->getUserGroups($user); + $userGroups = []; + foreach ($groups as $group) { + $userGroups[] = $group->getGID(); + } + + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from('share_external') + ->where( + $qb->expr()->orX( + $qb->expr()->eq('user', $qb->createNamedParameter($user->getUID())), + $qb->expr()->in( + 'user', + $qb->createNamedParameter($userGroups, IQueryBuilder::PARAM_STR_ARRAY) + ) + ) + ) + ->orderBy('id', 'ASC'); + + $shares = $this->findEntities($qb); + + // remove parent group share entry if we have a specific user share entry for the user + $toRemove = []; + foreach ($shares as $share) { + if ($share->getShareType() === IShare::TYPE_GROUP && $share->getParent() !== '-1') { + $toRemove[] = $share->getParent(); + } + } + $shares = array_filter($shares, function (ExternalShare $share) use ($toRemove): bool { + return !in_array($share->getId(), $toRemove, true); + }); + + if (!is_null($status)) { + $shares = array_filter($shares, function (ExternalShare $share) use ($status): bool { + return $share->getAccepted() === $status; + }); + } + return array_values($shares); + } + + public function deleteAll(): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete('share_external') + ->executeStatement(); + } + + #[Override] + public function delete(Entity $entity): ExternalShare { + /** @var ExternalShare $share */ + $share = $entity; + $qb = $this->db->getQueryBuilder(); + + $qb->delete(self::TABLE_NAME) + // delete the share itself + ->where($qb->expr()->eq('id', $qb->createNamedParameter($share->getId()))) + // delete all child in case of a group share + ->orWhere($qb->expr()->eq('parent', $qb->createNamedParameter($share->getId()))) + ->executeStatement(); + return $share; + } + + public function getShareByRemoteIdAndToken(string $id, mixed $token): ?ExternalShare { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from(self::TABLE_NAME) + ->where( + $qb->expr()->andX( + $qb->expr()->eq('remote_id', $qb->createNamedParameter($id)), + $qb->expr()->eq('share_token', $qb->createNamedParameter($token)) + ) + ); + + try { + return $this->findEntity($qb); + } catch (Exception) { + return null; + } + } +} diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index 43cd7f8d12541..0958626b96d9d 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -9,37 +9,36 @@ namespace OCA\Files_Sharing\External; use OC\Files\Filesystem; +use OC\Files\SetupManager; +use OC\User\NoUserException; use OCA\FederatedFileSharing\Events\FederatedShareAddedEvent; use OCA\Files_Sharing\Helper; -use OCA\Files_Sharing\ResponseDefinitions; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\DB\Exception; -use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\Federation\ICloudFederationFactory; use OCP\Federation\ICloudFederationProviderManager; -use OCP\Files; use OCP\Files\Events\InvalidateMountCacheEvent; +use OCP\Files\Folder; +use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; +use OCP\Files\NotPermittedException; use OCP\Files\Storage\IStorageFactory; use OCP\Http\Client\IClientService; +use OCP\ICertificateManager; use OCP\IDBConnection; +use OCP\IGroup; use OCP\IGroupManager; -use OCP\IUserManager; +use OCP\IUser; use OCP\IUserSession; use OCP\Notification\IManager; use OCP\OCS\IDiscoveryService; -use OCP\Share; use OCP\Share\IShare; +use OCP\Snowflake\IGenerator; use Psr\Log\LoggerInterface; -/** - * @psalm-import-type Files_SharingRemoteShare from ResponseDefinitions - */ class Manager { - public const STORAGE = '\OCA\Files_Sharing\External\Storage'; - - /** @var string|null */ - private $uid; + private ?IUser $user; public function __construct( private IDBConnection $connection, @@ -51,212 +50,114 @@ public function __construct( private ICloudFederationProviderManager $cloudFederationProviderManager, private ICloudFederationFactory $cloudFederationFactory, private IGroupManager $groupManager, - private IUserManager $userManager, IUserSession $userSession, private IEventDispatcher $eventDispatcher, private LoggerInterface $logger, + private IRootFolder $rootFolder, + private SetupManager $setupManager, + private ICertificateManager $certificateManager, + private ExternalShareMapper $externalShareMapper, + private IGenerator $snowflakeGenerator, ) { - $user = $userSession->getUser(); - $this->uid = $user ? $user->getUID() : null; + $this->user = $userSession->getUser(); } /** - * add new server-to-server share + * Add new server-to-server share. * - * @param string $remote - * @param string $token - * @param string $password - * @param string $name - * @param string $owner - * @param int $shareType - * @param boolean $accepted - * @param string $user - * @param string $remoteId - * @param int $parent - * @return Mount|null - * @throws \Doctrine\DBAL\Exception + * @throws Exception + * @throws NotPermittedException + * @throws NoUserException */ - public function addShare($remote, $token, $password, $name, $owner, $shareType, $accepted = false, $user = null, $remoteId = '', $parent = -1) { - $user = $user ?? $this->uid; - $accepted = $accepted ? IShare::STATUS_ACCEPTED : IShare::STATUS_PENDING; - $name = Filesystem::normalizePath('/' . $name); + public function addShare(ExternalShare $externalShare, IUser|IGroup|null $shareWith = null): ?Mount { + $shareWith = $shareWith ?? $this->user; - if ($accepted !== IShare::STATUS_ACCEPTED) { + 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#' . $name . '}}'; - $mountPoint = $tmpMountPointName; - $hash = md5($tmpMountPointName); - $data = [ - 'remote' => $remote, - 'share_token' => $token, - 'password' => $password, - 'name' => $name, - 'owner' => $owner, - 'user' => $user, - 'mountpoint' => $mountPoint, - 'mountpoint_hash' => $hash, - 'accepted' => $accepted, - 'remote_id' => $remoteId, - 'share_type' => $shareType, - ]; + $tmpMountPointName = '{{TemporaryMountPointName#' . $externalShare->getName() . '}}'; + $externalShare->setMountpoint($tmpMountPointName); + $externalShare->setShareWith($shareWith); $i = 1; - while (!$this->connection->insertIfNotExist('*PREFIX*share_external', $data, ['user', 'mountpoint_hash'])) { - // The external share already exists for the user - $data['mountpoint'] = $tmpMountPointName . '-' . $i; - $data['mountpoint_hash'] = md5($data['mountpoint']); - $i++; + 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; } - $mountPoint = Files::buildNotExistingFileName('/', $name); - $mountPoint = Filesystem::normalizePath('/' . $mountPoint); - $hash = md5($mountPoint); + $user = $shareWith instanceof IUser ? $shareWith : $this->user; - $this->writeShareToDb($remote, $token, $password, $name, $owner, $user, $mountPoint, $hash, $accepted, $remoteId, $parent, $shareType); + $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' => $remote, - 'token' => $token, - 'password' => $password, - 'mountpoint' => $mountPoint, - 'owner' => $owner + 'remote' => $externalShare->getRemote(), + 'token' => $externalShare->getShareToken(), + 'password' => $externalShare->getPassword(), + 'mountpoint' => $externalShare->getMountpoint(), + 'owner' => $externalShare->getOwner(), ]; return $this->mountShare($options, $user); } - /** - * Write remote share to the database. - * - * @throws Exception - */ - private function writeShareToDb(string $remote, string $token, ?string $password, string $name, string $owner, string $user, string $mountPoint, string $hash, int $accepted, string $remoteId, int $parent, int $shareType): void { - $qb = $this->connection->getQueryBuilder(); - $qb->insert('share_external') - ->values([ - 'remote' => $qb->createNamedParameter($remote, IQueryBuilder::PARAM_STR), - 'share_token' => $qb->createNamedParameter($token, IQueryBuilder::PARAM_STR), - 'password' => $qb->createNamedParameter($password, IQueryBuilder::PARAM_STR), - 'name' => $qb->createNamedParameter($name, IQueryBuilder::PARAM_STR), - 'owner' => $qb->createNamedParameter($owner, IQueryBuilder::PARAM_STR), - 'user' => $qb->createNamedParameter($user, IQueryBuilder::PARAM_STR), - 'mountpoint' => $qb->createNamedParameter($mountPoint, IQueryBuilder::PARAM_STR), - 'mountpoint_hash' => $qb->createNamedParameter($hash, IQueryBuilder::PARAM_STR), - 'accepted' => $qb->createNamedParameter($accepted, IQueryBuilder::PARAM_INT), - 'remote_id' => $qb->createNamedParameter($remoteId, IQueryBuilder::PARAM_STR), - 'parent' => $qb->createNamedParameter($parent, IQueryBuilder::PARAM_INT), - 'share_type' => $qb->createNamedParameter($shareType, IQueryBuilder::PARAM_INT), - ]) - ->executeStatement(); - } - - private function fetchShare(int $id): array|false { - $qb = $this->connection->getQueryBuilder(); - $result = $qb->select('id', 'remote', 'remote_id', 'share_token', 'name', 'owner', 'user', 'mountpoint', 'accepted', 'parent', 'share_type', 'password', 'mountpoint_hash') - ->from('share_external') - ->where($qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT))) - ->executeQuery(); - $share = $result->fetchAssociative(); - $result->closeCursor(); - return $share; - } - - /** - * get share by token - * - * @param string $token - * @return mixed share of false - */ - private function fetchShareByToken(string $token): array|false { - $qb = $this->connection->getQueryBuilder(); - $result = $qb->select('id', 'remote', 'remote_id', 'share_token', 'name', 'owner', 'user', 'mountpoint', 'accepted', 'parent', 'share_type', 'password', 'mountpoint_hash') - ->from('share_external') - ->where($qb->expr()->eq('share_token', $qb->createNamedParameter($token, IQueryBuilder::PARAM_STR))) - ->executeQuery(); - $share = $result->fetchAssociative(); - $result->closeCursor(); - return $share; - } - - private function fetchUserShare(int $parentId, string $uid): ?array { - $qb = $this->connection->getQueryBuilder(); - $result = $qb->select('id', 'remote', 'remote_id', 'share_token', 'name', 'owner', 'user', 'mountpoint', 'accepted', 'parent', 'share_type', 'password', 'mountpoint_hash') - ->from('share_external') - ->where($qb->expr()->andX( - $qb->expr()->eq('parent', $qb->createNamedParameter($parentId, IQueryBuilder::PARAM_INT)), - $qb->expr()->eq('user', $qb->createNamedParameter($uid, IQueryBuilder::PARAM_STR)), - )) - ->executeQuery(); - $share = $result->fetchAssociative(); - $result->closeCursor(); - if ($share !== false) { - return $share; - } - return null; - } - - public function getShare(int $id, ?string $user = null): array|false { - $user = $user ?? $this->uid; - $share = $this->fetchShare($id); - if ($share === false) { + public function getShare(string $id, ?IUser $user = null): ExternalShare|false { + $user = $user ?? $this->user; + try { + $externalShare = $this->externalShareMapper->getById($id); + } catch (DoesNotExistException $e) { return false; } // check if the user is allowed to access it - if ($this->canAccessShare($share, $user)) { - return $share; + if ($this->canAccessShare($externalShare, $user)) { + return $externalShare; } return false; } - /** - * Get share by token - * - * @param string $token - * @return array|false - */ - public function getShareByToken(string $token): array|false { - $share = $this->fetchShareByToken($token); - - // We do not check if the user is allowed to access it here, - // as this is not used from a user context. - if ($share === false) { - return false; - } - - return $share; - } - - private function canAccessShare(array $share, string $user): bool { - $validShare = isset($share['share_type']) && isset($share['user']); - - if (!$validShare) { + private function canAccessShare(ExternalShare $share, IUser $user): bool { + $isValid = $share->getShareType() === null; + if ($isValid) { + // Invalid share type return false; } // If the share is a user share, check if the user is the recipient - if ((int)$share['share_type'] === IShare::TYPE_USER - && $share['user'] === $user) { + if ($share->getShareType() === IShare::TYPE_USER && $share->getUser() === $user->getUID()) { return true; } // If the share is a group share, check if the user is in the group - if ((int)$share['share_type'] === IShare::TYPE_GROUP) { - $parentId = (int)$share['parent']; - if ($parentId !== -1) { + if ($share->getShareType() === IShare::TYPE_GROUP) { + $parentId = $share->getParent(); + if ($parentId !== '-1') { // we just retrieved a sub-share, switch to the parent entry for verification - $groupShare = $this->fetchShare($parentId); + $groupShare = $this->externalShareMapper->getById($parentId); } else { $groupShare = $share; } - $user = $this->userManager->get($user); - if ($this->groupManager->get($groupShare['user'])->inGroup($user)) { + if ($this->groupManager->get($groupShare->getUser())->inGroup($user)) { return true; } } @@ -264,227 +165,178 @@ private function canAccessShare(array $share, string $user): bool { return false; } + public function getShareByToken(string $token): ExternalShare|false { + try { + return $this->externalShareMapper->getShareByToken($token); + } catch (DoesNotExistException) { + return false; + } + } + /** - * Updates accepted flag in the database + * @throws Exception */ - private function updateAccepted(int $shareId, bool $accepted): void { - $qb = $this->connection->getQueryBuilder(); - $qb->update('share_external') - ->set('accepted', $qb->createNamedParameter($accepted ? 1 : 0, IQueryBuilder::PARAM_INT)) - ->where($qb->expr()->eq('id', $qb->createNamedParameter($shareId, IQueryBuilder::PARAM_INT))) - ->executeStatement(); + private function updateSubShare(ExternalShare $externalShare, IUser $user, ?string $mountPoint, int $accepted): ExternalShare { + $parentId = $externalShare->getParent(); + if ($parentId !== '-1') { + // this is the sub-share + $subShare = $externalShare; + } else { + try { + $subShare = $this->externalShareMapper->getUserShare($externalShare, $user); + } catch (DoesNotExistException) { + $subShare = new ExternalShare(); + $subShare->setId($this->snowflakeGenerator->nextId()); + $subShare->setRemote($externalShare->getRemote()); + $subShare->setPassword($externalShare->getPassword()); + $subShare->setName($externalShare->getName()); + $subShare->setOwner($externalShare->getOwner()); + $subShare->setUser($user->getUID()); + $subShare->setMountpoint($mountPoint ?? $externalShare->getMountpoint()); + $subShare->setAccepted($accepted); + $subShare->setRemoteId($externalShare->getRemoteId()); + $subShare->setParent($externalShare->getId()); + $subShare->setShareType($externalShare->getShareType()); + $subShare->setShareToken($externalShare->getShareToken()); + $this->externalShareMapper->insert($subShare); + } + } + + if ($subShare->getAccepted() !== $accepted) { + $subShare->setAccepted($accepted); + if ($mountPoint !== null) { + $subShare->setMountpoint($mountPoint); + } + $this->externalShareMapper->update($subShare); + } + + return $subShare; } /** - * accept server-to-server share + * Accept server-to-server share. * * @return bool True if the share could be accepted, false otherwise */ - public function acceptShare(int $id, ?string $user = null): bool { + 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->uid; + $user = $user ?? $this->user; if ($user === null) { $this->logger->error('No user specified for accepting share'); return false; } - $share = $this->getShare($id, $user); $result = false; - - if ($share) { - \OC_Util::setupFS($user); - $shareFolder = Helper::getShareFolder(null, $user); - $mountPoint = Files::buildNotExistingFileName($shareFolder, $share['name']); - $mountPoint = Filesystem::normalizePath($mountPoint); - $hash = md5($mountPoint); - $userShareAccepted = false; - - if ((int)$share['share_type'] === IShare::TYPE_USER) { - $qb = $this->connection->getQueryBuilder(); - $qb->update('share_external') - ->set('accepted', $qb->createNamedParameter(1)) - ->set('mountpoint', $qb->createNamedParameter($mountPoint)) - ->set('mountpoint_hash', $qb->createNamedParameter($hash)) - ->where($qb->expr()->andX( - $qb->expr()->eq('id', $qb->createNamedParameter($id)), - $qb->expr()->eq('user', $qb->createNamedParameter($user)) - )); - $userShareAccepted = $qb->executeStatement(); - } else { - $parentId = (int)$share['parent']; - if ($parentId !== -1) { - // this is the sub-share - $subshare = $share; - } else { - $subshare = $this->fetchUserShare($id, $user); - } - - if ($subshare !== null) { - try { - $qb = $this->connection->getQueryBuilder(); - $qb->update('share_external') - ->set('accepted', $qb->createNamedParameter(1)) - ->set('mountpoint', $qb->createNamedParameter($mountPoint)) - ->set('mountpoint_hash', $qb->createNamedParameter($hash)) - ->where($qb->expr()->andX( - $qb->expr()->eq('id', $qb->createNamedParameter($subshare['id'])), - $qb->expr()->eq('user', $qb->createNamedParameter($user)) - )) - ->executeStatement(); - $result = true; - } catch (Exception $e) { - $this->logger->emergency('Could not update share', ['exception' => $e]); - $result = false; - } - } else { - try { - $this->writeShareToDb( - $share['remote'], - $share['share_token'], - $share['password'], - $share['name'], - $share['owner'], - $user, - $mountPoint, $hash, 1, - $share['remote_id'], - $id, - $share['share_type']); - $result = true; - } catch (Exception $e) { - $this->logger->emergency('Could not create share', ['exception' => $e]); - $result = false; - } - } + $this->setupManager->setupForUser($user); + $folder = $this->rootFolder->getUserFolder($user->getUID()); + + $shareFolder = Helper::getShareFolder(null, $user->getUID()); + $shareFolder = $folder->get($shareFolder); + /** @var Folder $shareFolder */ + $mountPoint = $shareFolder->getNonExistingName($externalShare->getName()); + $mountPoint = Filesystem::normalizePath($mountPoint); + $userShareAccepted = false; + + if ($externalShare->getShareType() === IShare::TYPE_USER) { + if ($externalShare->getUser() === $user->getUID()) { + $externalShare->setAccepted(IShare::STATUS_ACCEPTED); + $externalShare->setMountpoint($mountPoint); + $this->externalShareMapper->update($externalShare); + $userShareAccepted = true; } - - if ($userShareAccepted !== false) { - $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'accept'); - $event = new FederatedShareAddedEvent($share['remote']); - $this->eventDispatcher->dispatchTyped($event); - $this->eventDispatcher->dispatchTyped(new InvalidateMountCacheEvent($this->userManager->get($user))); + } else { + try { + $this->updateSubShare($externalShare, $user, $mountPoint, IShare::STATUS_ACCEPTED); $result = true; + } catch (Exception $e) { + $this->logger->emergency('Could not create sub-share', ['exception' => $e]); + $this->processNotification($externalShare, $user); + return false; } } - // Make sure the user has no notification for something that does not exist anymore. - $this->processNotification($id, $user); + if ($userShareAccepted !== false) { + $this->sendFeedbackToRemote($externalShare, 'accept'); + $event = new FederatedShareAddedEvent($externalShare->getRemote()); + $this->eventDispatcher->dispatchTyped($event); + $this->eventDispatcher->dispatchTyped(new InvalidateMountCacheEvent($user)); + $result = true; + } + // Make sure the user has no notification for something that does not exist anymore. + $this->processNotification($externalShare, $user); return $result; } /** - * decline server-to-server share + * Decline server-to-server share * * @return bool True if the share could be declined, false otherwise */ - public function declineShare(int $id, ?string $user = null): bool { - $user = $user ?? $this->uid; + 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; } - $share = $this->getShare($id, $user); $result = false; - if ($share && (int)$share['share_type'] === IShare::TYPE_USER) { - $qb = $this->connection->getQueryBuilder(); - $qb->delete('share_external') - ->where($qb->expr()->andX( - $qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)), - $qb->expr()->eq('user', $qb->createNamedParameter($user, IQueryBuilder::PARAM_STR)) - )) - ->executeStatement(); - $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'decline'); - - $this->processNotification($id, $user); - $result = true; - } elseif ($share && (int)$share['share_type'] === IShare::TYPE_GROUP) { - $parentId = (int)$share['parent']; - if ($parentId !== -1) { - // this is the sub-share - $subshare = $share; - } else { - $subshare = $this->fetchUserShare($id, $user); + if ($externalShare->getShareType() === IShare::TYPE_USER) { + if ($externalShare->getUser() === $user->getUID()) { + $this->externalShareMapper->delete($externalShare); + $this->sendFeedbackToRemote($externalShare, 'decline'); + $this->processNotification($externalShare, $user); + $result = true; } - - if ($subshare !== null) { - try { - $this->updateAccepted((int)$subshare['id'], false); - $result = true; - } catch (Exception $e) { - $this->logger->emergency('Could not update share', ['exception' => $e]); - $result = false; - } - } else { - try { - $this->writeShareToDb( - $share['remote'], - $share['share_token'], - $share['password'], - $share['name'], - $share['owner'], - $user, - $share['mountpoint'], - $share['mountpoint_hash'], - 0, - $share['remote_id'], - $id, - $share['share_type']); - $result = true; - } catch (Exception $e) { - $this->logger->emergency('Could not create share', ['exception' => $e]); - $result = false; - } + } elseif ($externalShare->getShareType() === IShare::TYPE_GROUP) { + try { + $this->updateSubShare($externalShare, $user, null, IShare::STATUS_PENDING); + $result = true; + } catch (Exception $e) { + $this->logger->emergency('Could not create sub-share', ['exception' => $e]); + $this->processNotification($externalShare, $user); + return false; } - $this->processNotification($id, $user); } + // Make sure the user has no notification for something that does not exist anymore. + $this->processNotification($externalShare, $user); return $result; } - public function processNotification(int $remoteShare, ?string $user = null): void { - $user = $user ?? $this->uid; + 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; } - $share = $this->fetchShare($remoteShare); - if ($share === false) { - return; - } - $filter = $this->notificationManager->createNotification(); $filter->setApp('files_sharing') - ->setUser($user) - ->setObject('remote_share', (string)$remoteShare); + ->setUser($user->getUID()) + ->setObject('remote_share', $remoteShare->getId()); $this->notificationManager->markProcessed($filter); } /** - * inform remote server whether server-to-server share was accepted/declined + * Inform remote server whether server-to-server share was accepted/declined * - * @param string $remote - * @param string $token - * @param string $remoteId Share id on the remote host - * @param string $feedback - * @return boolean + * @param 'accept'|'decline' $feedback */ - private function sendFeedbackToRemote($remote, $token, $remoteId, $feedback) { - $result = $this->tryOCMEndPoint($remote, $token, $remoteId, $feedback); - + private function sendFeedbackToRemote(ExternalShare $externalShare, string $feedback): bool { + $result = $this->tryOCMEndPoint($externalShare, $feedback); if (is_array($result)) { return true; } - $federationEndpoints = $this->discoveryService->discover($remote, 'FEDERATED_SHARING'); + $federationEndpoints = $this->discoveryService->discover($externalShare->getRemote(), 'FEDERATED_SHARING'); $endpoint = $federationEndpoints['share'] ?? '/ocs/v2.php/cloud/shares'; - $url = rtrim($remote, '/') . $endpoint . '/' . $remoteId . '/' . $feedback . '?format=' . Share::RESPONSE_FORMAT; - $fields = ['token' => $token]; + $url = rtrim($externalShare->getRemote(), '/') . $endpoint . '/' . $externalShare->getRemoteId() . '/' . $feedback . '?format=json'; + $fields = ['token' => $externalShare->getShareToken()]; $client = $this->clientService->newClient(); @@ -506,44 +358,39 @@ private function sendFeedbackToRemote($remote, $token, $remoteId, $feedback) { } /** - * try send accept message to ocm end-point + * Try to send accept message to ocm end-point * - * @param string $remoteDomain - * @param string $token - * @param string $remoteId id of the share - * @param string $feedback + * @param 'accept'|'decline' $feedback * @return array|false */ - protected function tryOCMEndPoint($remoteDomain, $token, $remoteId, $feedback) { + protected function tryOCMEndPoint(ExternalShare $externalShare, string $feedback) { switch ($feedback) { case 'accept': $notification = $this->cloudFederationFactory->getCloudFederationNotification(); $notification->setMessage( 'SHARE_ACCEPTED', 'file', - $remoteId, + $externalShare->getRemoteId(), [ - 'sharedSecret' => $token, + 'sharedSecret' => $externalShare->getShareToken(), 'message' => 'Recipient accept the share' ] ); - return $this->cloudFederationProviderManager->sendNotification($remoteDomain, $notification); + return $this->cloudFederationProviderManager->sendNotification($externalShare->getRemote(), $notification); case 'decline': $notification = $this->cloudFederationFactory->getCloudFederationNotification(); $notification->setMessage( 'SHARE_DECLINED', 'file', - $remoteId, + $externalShare->getRemoteId(), [ - 'sharedSecret' => $token, + 'sharedSecret' => $externalShare->getShareToken(), 'message' => 'Recipient declined the share' ] - ); - return $this->cloudFederationProviderManager->sendNotification($remoteDomain, $notification); + return $this->cloudFederationProviderManager->sendNotification($externalShare->getRemote(), $notification); } - return false; } @@ -551,24 +398,20 @@ protected function tryOCMEndPoint($remoteDomain, $token, $remoteId, $feedback) { * remove '/user/files' from the path and trailing slashes */ protected function stripPath(string $path): string { - $prefix = '/' . $this->uid . '/files'; + $prefix = '/' . $this->user->getUID() . '/files'; return rtrim(substr($path, strlen($prefix)), '/'); } - public function getMount(array $data, ?string $user = null) { - $user = $user ?? $this->uid; + public function getMount(array $data, ?IUser $user = null): Mount { + $user = $user ?? $this->user; $data['manager'] = $this; - $mountPoint = '/' . $user . '/files' . $data['mountpoint']; + $mountPoint = '/' . $user->getUID() . '/files' . $data['mountpoint']; $data['mountpoint'] = $mountPoint; - $data['certificateManager'] = \OC::$server->getCertificateManager(); - return new Mount(self::STORAGE, $mountPoint, $data, $this, $this->storageLoader); + $data['certificateManager'] = $this->certificateManager; + return new Mount(Storage::class, $mountPoint, $data, $this, $this->storageLoader); } - /** - * @param array $data - * @return Mount - */ - protected function mountShare(array $data, ?string $user = null): Mount { + protected function mountShare(array $data, ?IUser $user = null): Mount { $mount = $this->getMount($data, $user); $this->mountManager->addMount($mount); return $mount; @@ -589,16 +432,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->uid))); + ->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($this->user->getUID()))); $result = (bool)$qb->executeStatement(); - $this->eventDispatcher->dispatchTyped(new InvalidateMountCacheEvent($this->userManager->get($this->uid))); + $this->eventDispatcher->dispatchTyped(new InvalidateMountCacheEvent($this->user)); return $result; } - public function removeShare($mountPoint): bool { + public function removeShare(string $mountPoint): bool { try { $mountPointObj = $this->mountManager->find($mountPoint); } catch (NotFoundException $e) { @@ -612,31 +455,28 @@ public function removeShare($mountPoint): bool { $id = $mountPointObj->getStorage()->getCache()->getId(''); $mountPoint = $this->stripPath($mountPoint); - $hash = md5($mountPoint); try { - $qb = $this->connection->getQueryBuilder(); - $qb->select('remote', 'share_token', 'remote_id', 'share_type', 'id') - ->from('share_external') - ->where($qb->expr()->eq('mountpoint_hash', $qb->createNamedParameter($hash))) - ->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($this->uid))); - $result = $qb->executeQuery(); - $share = $result->fetchAssociative(); - $result->closeCursor(); - if ($share !== false && (int)$share['share_type'] === IShare::TYPE_USER) { + try { + $externalShare = $this->externalShareMapper->getByMountPointAndUser($mountPoint, $this->user); + } catch (DoesNotExistException $e) { + // ignore + $this->removeReShares((string)$id); + return true; + } + + if ($externalShare->getShareType() === IShare::TYPE_USER) { try { - $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'decline'); + $this->sendFeedbackToRemote($externalShare, 'decline'); } catch (\Throwable $e) { // if we fail to notify the remote (probably cause the remote is down) // we still want the share to be gone to prevent undeletable remotes } - $qb = $this->connection->getQueryBuilder(); - $qb->delete('share_external') - ->where('id', $qb->createNamedParameter((int)$share['id'])) - ->executeStatement(); - } elseif ($share !== false && (int)$share['share_type'] === IShare::TYPE_GROUP) { - $this->updateAccepted((int)$share['id'], false); + $this->externalShareMapper->delete($externalShare); + } elseif ($externalShare->getShareType() === IShare::TYPE_GROUP) { + $externalShare->setAccepted(IShare::STATUS_PENDING); + $this->externalShareMapper->update($externalShare); } $this->removeReShares((string)$id); @@ -669,42 +509,17 @@ protected function removeReShares(string $mountPointId): void { } /** - * remove all shares for user $uid if the user was deleted - * - * @param string $uid + * Remove all shares for user $uid if the user was deleted. */ - public function removeUserShares($uid): bool { + public function removeUserShares(IUser $user): bool { try { - $qb = $this->connection->getQueryBuilder(); - $qb->select('id', 'remote', 'share_type', 'share_token', 'remote_id') - ->from('share_external') - ->where($qb->expr()->eq('user', $qb->createNamedParameter($uid))) - ->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_USER))); - $result = $qb->executeQuery(); - $shares = $result->fetchAllAssociative(); - $result->closeCursor(); - + $shares = $this->externalShareMapper->getUserShares($user); foreach ($shares as $share) { - $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'decline'); + $this->sendFeedbackToRemote($share, 'decline'); } - $qb = $this->connection->getQueryBuilder(); - $qb->delete('share_external') - // user field can specify a user or a group - ->where($qb->expr()->eq('user', $qb->createNamedParameter($uid))) - ->andWhere( - $qb->expr()->orX( - // delete direct shares - $qb->expr()->eq('share_type', $qb->expr()->literal(IShare::TYPE_USER)), - // delete sub-shares of group shares for that user - $qb->expr()->andX( - $qb->expr()->eq('share_type', $qb->expr()->literal(IShare::TYPE_GROUP)), - $qb->expr()->neq('parent', $qb->expr()->literal(-1)), - ) - ) - ); - $qb->executeStatement(); - } catch (\Doctrine\DBAL\Exception $ex) { + $this->externalShareMapper->deleteUserShares($user); + } catch (Exception $ex) { $this->logger->emergency('Could not delete user shares', ['exception' => $ex]); return false; } @@ -712,34 +527,10 @@ public function removeUserShares($uid): bool { return true; } - public function removeGroupShares($gid): bool { + public function removeGroupShares(IGroup $group): bool { try { - $qb = $this->connection->getQueryBuilder(); - $qb->select('id', 'remote', 'share_type', 'share_token', 'remote_id') - ->from('share_external') - ->where($qb->expr()->eq('user', $qb->createNamedParameter($gid))) - ->andWhere($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_GROUP))); - $result = $qb->executeQuery(); - $shares = $result->fetchAllAssociative(); - $result->closeCursor(); - - $deletedGroupShares = []; - $qb = $this->connection->getQueryBuilder(); - // delete group share entry and matching sub-entries - $qb->delete('share_external') - ->where( - $qb->expr()->orX( - $qb->expr()->eq('id', $qb->createParameter('share_id')), - $qb->expr()->eq('parent', $qb->createParameter('share_parent_id')) - ) - ); - - foreach ($shares as $share) { - $qb->setParameter('share_id', $share['id']); - $qb->setParameter('share_parent_id', $share['id']); - $qb->executeStatement(); - } - } catch (\Doctrine\DBAL\Exception $ex) { + $this->externalShareMapper->deleteGroupShares($group); + } catch (Exception $ex) { $this->logger->emergency('Could not delete user shares', ['exception' => $ex]); return false; } @@ -748,79 +539,28 @@ public function removeGroupShares($gid): bool { } /** - * return a list of shares which are not yet accepted by the user - * - * @return list list of open server-to-server shares - */ - public function getOpenShares() { - return $this->getShares(false); - } - - /** - * return a list of shares which are accepted by the user + * Return a list of shares which are not yet accepted by the user. * - * @return list list of accepted server-to-server shares + * @return list list of open server-to-server shares */ - public function getAcceptedShares() { - return $this->getShares(true); + public function getOpenShares(): array { + try { + return $this->externalShareMapper->getShares($this->user, IShare::STATUS_PENDING); + } catch (Exception $e) { + $this->logger->emergency('Error when retrieving shares', ['exception' => $e]); + return []; + } } /** - * return a list of shares for the user + * Return a list of shares which are accepted by the user. * - * @param bool|null $accepted True for accepted only, - * false for not accepted, - * null for all shares of the user - * @return list list of open server-to-server shares + * @return list list of accepted server-to-server shares */ - private function getShares($accepted) { - // Not allowing providing a user here, - // as we only want to retrieve shares for the current user. - $user = $this->userManager->get($this->uid); - $groups = $this->groupManager->getUserGroups($user); - $userGroups = []; - foreach ($groups as $group) { - $userGroups[] = $group->getGID(); - } - - $qb = $this->connection->getQueryBuilder(); - $qb->select('id', 'share_type', 'parent', 'remote', 'remote_id', 'share_token', 'name', 'owner', 'user', 'mountpoint', 'accepted') - ->from('share_external') - ->where( - $qb->expr()->orX( - $qb->expr()->eq('user', $qb->createNamedParameter($this->uid)), - $qb->expr()->in( - 'user', - $qb->createNamedParameter($userGroups, IQueryBuilder::PARAM_STR_ARRAY) - ) - ) - ) - ->orderBy('id', 'ASC'); - + public function getAcceptedShares(): array { try { - $result = $qb->executeQuery(); - /** @var list $shares */ - $shares = $result->fetchAllAssociative(); - $result->closeCursor(); - - // remove parent group share entry if we have a specific user share entry for the user - $toRemove = []; - foreach ($shares as $share) { - if ((int)$share['share_type'] === IShare::TYPE_GROUP && (int)$share['parent'] > 0) { - $toRemove[] = $share['parent']; - } - } - $shares = array_filter($shares, function ($share) use ($toRemove) { - return !in_array($share['id'], $toRemove, true); - }); - - if (!is_null($accepted)) { - $shares = array_filter($shares, function ($share) use ($accepted) { - return (bool)$share['accepted'] === $accepted; - }); - } - return array_values($shares); - } catch (\Doctrine\DBAL\Exception $e) { + return $this->externalShareMapper->getShares($this->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 f50c379f85fcd..49ca047ffb1ff 100644 --- a/apps/files_sharing/lib/External/Mount.php +++ b/apps/files_sharing/lib/External/Mount.php @@ -10,23 +10,17 @@ use OC\Files\Mount\MountPoint; use OC\Files\Mount\MoveableMount; use OC\Files\Storage\Storage; +use OC\Files\Storage\StorageFactory; use OCA\Files_Sharing\ISharedMountPoint; +use Override; class Mount extends MountPoint implements MoveableMount, ISharedMountPoint { - - /** - * @param string|Storage $storage - * @param string $mountpoint - * @param array $options - * @param \OCA\Files_Sharing\External\Manager $manager - * @param \OC\Files\Storage\StorageFactory $loader - */ public function __construct( - $storage, - $mountpoint, - $options, - protected $manager, - $loader = null, + string|Storage $storage, + string $mountpoint, + array $options, + protected Manager $manager, + ?StorageFactory $loader = null, ) { parent::__construct($storage, $mountpoint, $options, $loader, null, null, MountProvider::class); } @@ -35,9 +29,8 @@ public function __construct( * Move the mount point to $target * * @param string $target the target mount point - * @return bool */ - public function moveMount($target) { + public function moveMount($target): bool { $result = $this->manager->setMountPoint($this->mountPoint, $target); $this->setMountPoint($target); @@ -51,13 +44,8 @@ public function removeMount(): bool { return $this->manager->removeShare($this->mountPoint); } - /** - * Get the type of mount point, used to distinguish things like shares and external storage - * in the web interface - * - * @return string - */ - public function getMountType() { + #[Override] + public function getMountType(): string { return 'shared'; } } diff --git a/apps/files_sharing/lib/External/MountProvider.php b/apps/files_sharing/lib/External/MountProvider.php index 5a0021db6b7d5..5e132c22903ce 100644 --- a/apps/files_sharing/lib/External/MountProvider.php +++ b/apps/files_sharing/lib/External/MountProvider.php @@ -1,5 +1,7 @@ managerProvider = $managerProvider; } - public function getMount(IUser $user, $data, IStorageFactory $storageFactory) { + private function getMount(IUser $user, array $data, IStorageFactory $storageFactory): Mount { $managerProvider = $this->managerProvider; $manager = $managerProvider(); $data['manager'] = $manager; $mountPoint = '/' . $user->getUID() . '/files/' . ltrim($data['mountpoint'], '/'); $data['mountpoint'] = $mountPoint; $data['cloudId'] = $this->cloudIdManager->getCloudId($data['owner'], $data['remote']); - $data['certificateManager'] = \OC::$server->getCertificateManager(); + $data['certificateManager'] = Server::get(ICertificateManager::class); $data['HttpClientService'] = Server::get(IClientService::class); + return new Mount(self::STORAGE, $mountPoint, $data, $manager, $storageFactory); } - public function getMountsForUser(IUser $user, IStorageFactory $loader) { + public function getMountsForUser(IUser $user, IStorageFactory $loader): array { $qb = $this->connection->getQueryBuilder(); - $qb->select('remote', 'share_token', 'password', 'mountpoint', 'owner') + $qb->select('id', 'remote', 'share_token', 'password', 'mountpoint', 'owner') ->from('share_external') ->where($qb->expr()->eq('user', $qb->createNamedParameter($user->getUID()))) ->andWhere($qb->expr()->eq('accepted', $qb->createNamedParameter(IShare::STATUS_ACCEPTED, IQueryBuilder::PARAM_INT))); diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index 509b23f68b4a1..e8de4bc63b5d3 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -192,7 +192,7 @@ public function test(): bool { * @throws StorageNotAvailableException * @throws StorageInvalidException */ - public function checkStorageAvailability() { + public function checkStorageAvailability(): void { // see if we can find out why the share is unavailable try { $this->getShareInfo(0); @@ -230,9 +230,7 @@ public function file_exists(string $path): bool { } /** - * Check if the configured remote is a valid federated share provider - * - * @return bool + * Check if the configured remote is a valid-federated share provider */ protected function testRemote(): bool { try { diff --git a/apps/files_sharing/lib/Hooks.php b/apps/files_sharing/lib/Hooks.php index e90b9f5c23dac..9f00afa1079c5 100644 --- a/apps/files_sharing/lib/Hooks.php +++ b/apps/files_sharing/lib/Hooks.php @@ -9,16 +9,22 @@ use OC\Files\Filesystem; use OC\Files\View; +use OCP\IUserManager; use OCP\Server; class Hooks { - public static function deleteUser($params) { - $manager = Server::get(External\Manager::class); + public static function deleteUser(array $params): void { + $userManager = Server::get(IUserManager::class); + $user = $userManager->get($params['uid']); + if ($user === null) { + return; + } - $manager->removeUserShares($params['uid']); + $manager = Server::get(External\Manager::class); + $manager->removeUserShares($user); } - public static function unshareChildren($params) { + public static function unshareChildren(array $params): void { $path = Filesystem::getView()->getAbsolutePath($params['path']); $view = new View('/'); diff --git a/apps/files_sharing/lib/Migration/Version33000Date20251030081948.php b/apps/files_sharing/lib/Migration/Version33000Date20251030081948.php new file mode 100644 index 0000000000000..ad69f6004121c --- /dev/null +++ b/apps/files_sharing/lib/Migration/Version33000Date20251030081948.php @@ -0,0 +1,29 @@ +dropAutoincrementColumn('share_external', 'id'); + return null; + } +} diff --git a/apps/files_sharing/lib/ResponseDefinitions.php b/apps/files_sharing/lib/ResponseDefinitions.php index 71a2b25a70cf8..58277577eabdd 100644 --- a/apps/files_sharing/lib/ResponseDefinitions.php +++ b/apps/files_sharing/lib/ResponseDefinitions.php @@ -81,15 +81,15 @@ * } * * @psalm-type Files_SharingRemoteShare = array{ - * accepted: bool, + * accepted: int, * file_id: int|null, - * id: int, + * id: string, * mimetype: string|null, * mountpoint: string, * mtime: int|null, * name: string, * owner: string, - * parent: int|null, + * parent: string|null, * permissions: int|null, * remote: string, * remote_id: string, diff --git a/apps/files_sharing/openapi.json b/apps/files_sharing/openapi.json index 380a7194188ae..dc89752f091b9 100644 --- a/apps/files_sharing/openapi.json +++ b/apps/files_sharing/openapi.json @@ -408,7 +408,8 @@ ], "properties": { "accepted": { - "type": "boolean" + "type": "integer", + "format": "int64" }, "file_id": { "type": "integer", @@ -416,8 +417,7 @@ "nullable": true }, "id": { - "type": "integer", - "format": "int64" + "type": "string" }, "mimetype": { "type": "string", @@ -438,8 +438,7 @@ "type": "string" }, "parent": { - "type": "integer", - "format": "int64", + "type": "string", "nullable": true }, "permissions": { @@ -3932,7 +3931,7 @@ "/ocs/v2.php/apps/files_sharing/api/v1/remote_shares/pending/{id}": { "post": { "operationId": "remote-accept-share", - "summary": "Accept a remote share", + "summary": "Accept a remote share.", "tags": [ "remote" ], @@ -3951,8 +3950,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -4055,7 +4053,7 @@ }, "delete": { "operationId": "remote-decline-share", - "summary": "Decline a remote share", + "summary": "Decline a remote share.", "tags": [ "remote" ], @@ -4074,8 +4072,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -4199,8 +4196,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -4324,8 +4320,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { diff --git a/apps/files_sharing/tests/CapabilitiesTest.php b/apps/files_sharing/tests/CapabilitiesTest.php index 914be0702c318..adc1909cf7c5d 100644 --- a/apps/files_sharing/tests/CapabilitiesTest.php +++ b/apps/files_sharing/tests/CapabilitiesTest.php @@ -19,11 +19,9 @@ use OCP\IConfig; use OCP\IDateTimeZone; use OCP\IGroupManager; -use OCP\IURLGenerator; use OCP\IUserManager; use OCP\IUserSession; use OCP\L10N\IFactory; -use OCP\Mail\IMailer; use OCP\Security\IHasher; use OCP\Security\ISecureRandom; use OCP\Share\IProviderFactory; @@ -90,9 +88,6 @@ private function getResults(array $map, array $typedMap = [], bool $federationEn $this->createMock(IProviderFactory::class), $this->createMock(IUserManager::class), $this->createMock(IRootFolder::class), - $this->createMock(IMailer::class), - $this->createMock(IURLGenerator::class), - $this->createMock(\OC_Defaults::class), $this->createMock(IEventDispatcher::class), $this->createMock(IUserSession::class), $this->createMock(KnownUserService::class), diff --git a/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php b/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php index d9c832a31851e..c105157b1a985 100644 --- a/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php +++ b/apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php @@ -8,25 +8,24 @@ namespace OCA\Files_Sharing\Tests\Command; use OCA\Files_Sharing\Command\CleanupRemoteStorages; +use OCA\Files_Sharing\External\ExternalShare; +use OCA\Files_Sharing\External\ExternalShareMapper; use OCP\Federation\ICloudId; use OCP\Federation\ICloudIdManager; use OCP\IDBConnection; use OCP\Server; +use OCP\Snowflake\IGenerator; +use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Test\TestCase; -/** - * Class CleanupRemoteStoragesTest - * - * - * @package OCA\Files_Sharing\Tests\Command - */ -#[\PHPUnit\Framework\Attributes\Group('DB')] +#[Group('DB')] class CleanupRemoteStoragesTest extends TestCase { protected IDBConnection $connection; + protected ExternalShareMapper $mapper; protected CleanupRemoteStorages $command; private ICloudIdManager&MockObject $cloudIdManager; @@ -44,21 +43,12 @@ protected function setUp(): void { parent::setUp(); $this->connection = Server::get(IDBConnection::class); + $this->mapper = Server::get(ExternalShareMapper::class); $storageQuery = Server::get(IDBConnection::class)->getQueryBuilder(); $storageQuery->insert('storages') ->setValue('id', $storageQuery->createParameter('id')); - $shareExternalQuery = Server::get(IDBConnection::class)->getQueryBuilder(); - $shareExternalQuery->insert('share_external') - ->setValue('share_token', $shareExternalQuery->createParameter('share_token')) - ->setValue('remote', $shareExternalQuery->createParameter('remote')) - ->setValue('name', $shareExternalQuery->createParameter('name')) - ->setValue('owner', $shareExternalQuery->createParameter('owner')) - ->setValue('user', $shareExternalQuery->createParameter('user')) - ->setValue('mountpoint', $shareExternalQuery->createParameter('mountpoint')) - ->setValue('mountpoint_hash', $shareExternalQuery->createParameter('mountpoint_hash')); - $filesQuery = Server::get(IDBConnection::class)->getQueryBuilder(); $filesQuery->insert('filecache') ->setValue('storage', $filesQuery->createParameter('storage')) @@ -73,15 +63,15 @@ protected function setUp(): void { } if (isset($storage['share_token'])) { - $shareExternalQuery - ->setParameter('share_token', $storage['share_token']) - ->setParameter('remote', $storage['remote']) - ->setParameter('name', 'irrelevant') - ->setParameter('owner', 'irrelevant') - ->setParameter('user', $storage['user']) - ->setParameter('mountpoint', 'irrelevant') - ->setParameter('mountpoint_hash', 'irrelevant'); - $shareExternalQuery->executeStatement(); + $externalShare = new ExternalShare(); + $externalShare->setId(Server::get(IGenerator::class)->nextId()); + $externalShare->setShareToken($storage['share_token']); + $externalShare->setRemote($storage['remote']); + $externalShare->setName('irrelevant'); + $externalShare->setOwner('irrelevant'); + $externalShare->setUser($storage['user']); + $externalShare->setMountpoint('irrelevant'); + $this->mapper->insert($externalShare); } if (isset($storage['files_count'])) { diff --git a/apps/files_sharing/tests/Controller/ExternalShareControllerTest.php b/apps/files_sharing/tests/Controller/ExternalShareControllerTest.php index 7e054d9a6dcf1..74978984fd5f1 100644 --- a/apps/files_sharing/tests/Controller/ExternalShareControllerTest.php +++ b/apps/files_sharing/tests/Controller/ExternalShareControllerTest.php @@ -8,10 +8,9 @@ namespace OCA\Files_Sharing\Tests\Controllers; use OCA\Files_Sharing\Controller\ExternalSharesController; +use OCA\Files_Sharing\External\ExternalShare; use OCA\Files_Sharing\External\Manager; use OCP\AppFramework\Http\JSONResponse; -use OCP\Http\Client\IClientService; -use OCP\IConfig; use OCP\IRequest; use PHPUnit\Framework\MockObject\MockObject; @@ -21,21 +20,13 @@ * @package OCA\Files_Sharing\Controllers */ class ExternalShareControllerTest extends \Test\TestCase { - /** @var IRequest */ - private $request; - /** @var \OCA\Files_Sharing\External\Manager */ - private $externalManager; - /** @var IConfig|MockObject */ - private $config; - /** @var IClientService */ - private $clientService; + private IRequest&MockObject $request; + private Manager $externalManager; protected function setUp(): void { parent::setUp(); $this->request = $this->createMock(IRequest::class); $this->externalManager = $this->createMock(Manager::class); - $this->clientService = $this->createMock(IClientService::class); - $this->config = $this->createMock(IConfig::class); } /** @@ -46,8 +37,6 @@ public function getExternalShareController() { 'files_sharing', $this->request, $this->externalManager, - $this->clientService, - $this->config, ); } @@ -61,20 +50,32 @@ public function testIndex(): void { } public function testCreate(): void { + $share = $this->createMock(ExternalShare::class); + $this->externalManager + ->expects($this->once()) + ->method('getShare') + ->with('4') + ->willReturn($share); $this->externalManager ->expects($this->once()) ->method('acceptShare') - ->with(4); + ->with($share); - $this->assertEquals(new JSONResponse(), $this->getExternalShareController()->create(4)); + $this->assertEquals(new JSONResponse(), $this->getExternalShareController()->create('4')); } public function testDestroy(): void { + $share = $this->createMock(ExternalShare::class); + $this->externalManager + ->expects($this->once()) + ->method('getShare') + ->with('4') + ->willReturn($share); $this->externalManager ->expects($this->once()) ->method('declineShare') - ->with(4); + ->with($share); - $this->assertEquals(new JSONResponse(), $this->getExternalShareController()->destroy(4)); + $this->assertEquals(new JSONResponse(), $this->getExternalShareController()->destroy('4')); } } diff --git a/apps/files_sharing/tests/ExpireSharesJobTest.php b/apps/files_sharing/tests/ExpireSharesJobTest.php index 00defc040bb86..adb6f6d2b814a 100644 --- a/apps/files_sharing/tests/ExpireSharesJobTest.php +++ b/apps/files_sharing/tests/ExpireSharesJobTest.php @@ -11,11 +11,14 @@ use OCA\Files_Sharing\ExpireSharesJob; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Constants; +use OCP\Files\IRootFolder; use OCP\IDBConnection; +use OCP\IUser; use OCP\IUserManager; use OCP\Server; use OCP\Share\IManager; use OCP\Share\IShare; +use PHPUnit\Framework\Attributes\DataProvider; /** * Class ExpireSharesJobTest @@ -26,32 +29,30 @@ #[\PHPUnit\Framework\Attributes\Group('DB')] class ExpireSharesJobTest extends \Test\TestCase { - /** @var ExpireSharesJob */ - private $job; + private ExpireSharesJob $job; - /** @var IDBConnection */ - private $connection; + private IDBConnection $connection; + private IRootFolder $rootFolder; - /** @var string */ - private $user1; + private IUser $user1; - /** @var string */ - private $user2; + private IUser $user2; protected function setUp(): void { parent::setUp(); $this->connection = Server::get(IDBConnection::class); + $this->rootFolder = Server::get(IRootFolder::class); // clear occasional leftover shares from other tests $qb = $this->connection->getQueryBuilder(); $qb->delete('share')->executeStatement(); - $this->user1 = $this->getUniqueID('user1_'); - $this->user2 = $this->getUniqueID('user2_'); + $user1 = $this->getUniqueID('user1_'); + $user2 = $this->getUniqueID('user2_'); $userManager = Server::get(IUserManager::class); - $userManager->createUser($this->user1, 'longrandompassword'); - $userManager->createUser($this->user2, 'longrandompassword'); + $this->user1 = $userManager->createUser($user1, 'longrandompassword'); + $this->user2 = $userManager->createUser($user2, 'longrandompassword'); \OC::registerShareHooks(Server::get(SystemConfig::class)); @@ -62,22 +63,15 @@ protected function tearDown(): void { $qb = $this->connection->getQueryBuilder(); $qb->delete('share')->executeStatement(); - $userManager = Server::get(IUserManager::class); - $user1 = $userManager->get($this->user1); - if ($user1) { - $user1->delete(); - } - $user2 = $userManager->get($this->user2); - if ($user2) { - $user2->delete(); - } + $this->user1->delete(); + $this->user2->delete(); $this->logout(); parent::tearDown(); } - private function getShares() { + private function getShares(): array { $shares = []; $qb = $this->connection->getQueryBuilder(); @@ -108,26 +102,25 @@ public static function dataExpireLinkShare() { } /** - * * @param bool addExpiration Should we add an expire date * @param string $interval The dateInterval * @param bool $addInterval If true add to the current time if false subtract * @param bool $shouldExpire Should this share be expired */ - #[\PHPUnit\Framework\Attributes\DataProvider('dataExpireLinkShare')] - public function testExpireLinkShare($addExpiration, $interval, $addInterval, $shouldExpire): void { - $this->loginAsUser($this->user1); + #[DataProvider('dataExpireLinkShare')] + public function testExpireLinkShare(bool $addExpiration, string $interval, bool $addInterval, bool $shouldExpire): void { + $this->loginAsUser($this->user1->getUID()); - $user1Folder = \OC::$server->getUserFolder($this->user1); + $user1Folder = $this->rootFolder->getUserFolder($this->user1->getUID()); $testFolder = $user1Folder->newFolder('test'); - $shareManager = Server::get(\OCP\Share\IManager::class); + $shareManager = Server::get(IManager::class); $share = $shareManager->newShare(); $share->setNode($testFolder) ->setShareType(IShare::TYPE_LINK) ->setPermissions(Constants::PERMISSION_READ) - ->setSharedBy($this->user1); + ->setSharedBy($this->user1->getUID()); $shareManager->createShare($share); @@ -173,19 +166,19 @@ public function testExpireLinkShare($addExpiration, $interval, $addInterval, $sh } public function testDoNotExpireOtherShares(): void { - $this->loginAsUser($this->user1); + $this->loginAsUser($this->user1->getUID()); - $user1Folder = \OC::$server->getUserFolder($this->user1); + $user1Folder = $this->rootFolder->getUserFolder($this->user1->getUID()); $testFolder = $user1Folder->newFolder('test'); - $shareManager = Server::get(\OCP\Share\IManager::class); + $shareManager = Server::get(IManager::class); $share = $shareManager->newShare(); $share->setNode($testFolder) ->setShareType(IShare::TYPE_USER) ->setPermissions(Constants::PERMISSION_READ) - ->setSharedBy($this->user1) - ->setSharedWith($this->user2); + ->setSharedBy($this->user1->getUID()) + ->setSharedWith($this->user2->getUID()); $shareManager->createShare($share); diff --git a/apps/files_sharing/tests/External/CacheTest.php b/apps/files_sharing/tests/External/CacheTest.php index c23101ea49ef1..7af8af4fb5fa5 100644 --- a/apps/files_sharing/tests/External/CacheTest.php +++ b/apps/files_sharing/tests/External/CacheTest.php @@ -18,6 +18,7 @@ use OCP\ICacheFactory; use OCP\IURLGenerator; use OCP\IUserManager; +use PHPUnit\Framework\MockObject\MockObject; /** * Class Cache @@ -27,26 +28,11 @@ */ #[\PHPUnit\Framework\Attributes\Group('DB')] class CacheTest extends TestCase { - /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ - protected $contactsManager; - - /** - * @var Storage - **/ - private $storage; - - /** - * @var Cache - */ - private $cache; - - /** - * @var string - */ - private $remoteUser; - - /** @var ICloudIdManager */ - private $cloudIdManager; + protected IManager&MockObject $contactsManager; + private Storage&MockObject $storage; + private Cache $cache; + private string $remoteUser; + private ICloudIdManager $cloudIdManager; protected function setUp(): void { parent::setUp(); @@ -62,7 +48,7 @@ protected function setUp(): void { ); $this->remoteUser = $this->getUniqueID('remoteuser'); - $this->storage = $this->getMockBuilder('\OCA\Files_Sharing\External\Storage') + $this->storage = $this->getMockBuilder(\OCA\Files_Sharing\External\Storage::class) ->disableOriginalConstructor() ->getMock(); $this->storage diff --git a/apps/files_sharing/tests/External/ManagerTest.php b/apps/files_sharing/tests/External/ManagerTest.php index e1c40b18865b6..64afb91b0809c 100644 --- a/apps/files_sharing/tests/External/ManagerTest.php +++ b/apps/files_sharing/tests/External/ManagerTest.php @@ -9,9 +9,12 @@ use OC\Federation\CloudIdManager; use OC\Files\Mount\MountPoint; +use OC\Files\SetupManager; use OC\Files\SetupManagerFactory; use OC\Files\Storage\StorageFactory; use OC\Files\Storage\Temporary; +use OCA\Files_Sharing\External\ExternalShare; +use OCA\Files_Sharing\External\ExternalShareMapper; use OCA\Files_Sharing\External\Manager; use OCA\Files_Sharing\External\MountProvider; use OCA\Files_Sharing\Tests\TestCase; @@ -19,11 +22,14 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\Federation\ICloudFederationFactory; use OCP\Federation\ICloudFederationProviderManager; +use OCP\Files\Folder; +use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\Http\Client\IClient; use OCP\Http\Client\IClientService; use OCP\Http\Client\IResponse; use OCP\ICacheFactory; +use OCP\ICertificateManager; use OCP\IDBConnection; use OCP\IGroup; use OCP\IGroupManager; @@ -34,6 +40,7 @@ use OCP\OCS\IDiscoveryService; use OCP\Server; use OCP\Share\IShare; +use OCP\Snowflake\IGenerator; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Test\Traits\UserTrait; @@ -48,8 +55,9 @@ class ManagerTest extends TestCase { use UserTrait; - protected string $uid; protected IUser $user; + protected IGroup&MockObject $group1; + protected IGroup&MockObject $group2; protected MountProvider $testMountProvider; protected IEventDispatcher&MockObject $eventDispatcher; protected LoggerInterface&MockObject $logger; @@ -61,12 +69,14 @@ class ManagerTest extends TestCase { protected ICloudFederationFactory&MockObject $cloudFederationFactory; protected IGroupManager&MockObject $groupManager; protected IUserManager&MockObject $userManager; + protected SetupManager&MockObject $setupManager; + protected ICertificateManager&MockObject $certificateManager; + private ExternalShareMapper $externalShareMapper; protected function setUp(): void { parent::setUp(); - $this->uid = $this->getUniqueID('user'); - $this->user = $this->createUser($this->uid, ''); + $this->user = $this->createUser($this->getUniqueID('user'), ''); $this->mountManager = new \OC\Files\Mount\Manager($this->createMock(SetupManagerFactory::class)); $this->clientService = $this->getMockBuilder(IClientService::class) ->disableOriginalConstructor()->getMock(); @@ -75,6 +85,19 @@ protected function setUp(): void { $this->groupManager = $this->createMock(IGroupManager::class); $this->userManager = $this->createMock(IUserManager::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->setupManager = $this->createMock(SetupManager::class); + $this->rootFolder = $this->createMock(IRootFolder::class); + $this->rootFolder->method('getUserFolder') + ->willReturnCallback(function (string $userId): Folder { + $folder = $this->createMock(Folder::class); + $folder->method('get') + ->willReturn($folder); + $folder->method('getNonExistingName') + ->willReturnCallback(fn (string $name): string => $name); + return $folder; + }); + + $this->externalShareMapper = new ExternalShareMapper(Server::get(IDBConnection::class), $this->groupManager); $this->contactsManager = $this->createMock(IManager::class); // needed for MountProvider() initialization @@ -82,10 +105,12 @@ protected function setUp(): void { ->method('search') ->willReturn([]); + $this->certificateManager = $this->createMock(ICertificateManager::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->logger->expects($this->never())->method('emergency'); - $this->manager = $this->createManagerForUser($this->uid); + $this->manager = $this->createManagerForUser($this->user); $this->testMountProvider = new MountProvider(Server::get(IDBConnection::class), function () { return $this->manager; @@ -97,20 +122,20 @@ protected function setUp(): void { $this->userManager, )); - $group1 = $this->createMock(IGroup::class); - $group1->expects($this->any())->method('getGID')->willReturn('group1'); - $group1->expects($this->any())->method('inGroup')->with($this->user)->willReturn(true); + $this->group1 = $this->createMock(IGroup::class); + $this->group1->expects($this->any())->method('getGID')->willReturn('group1'); + $this->group1->expects($this->any())->method('inGroup')->with($this->user)->willReturn(true); - $group2 = $this->createMock(IGroup::class); - $group2->expects($this->any())->method('getGID')->willReturn('group2'); - $group2->expects($this->any())->method('inGroup')->with($this->user)->willReturn(true); + $this->group2 = $this->createMock(IGroup::class); + $this->group2->expects($this->any())->method('getGID')->willReturn('group2'); + $this->group2->expects($this->any())->method('inGroup')->with($this->user)->willReturn(true); $this->userManager->expects($this->any())->method('get')->willReturn($this->user); - $this->groupManager->expects($this->any())->method(('getUserGroups'))->willReturn([$group1, $group2]); + $this->groupManager->expects($this->any())->method(('getUserGroups'))->willReturn([$this->group1, $this->group2]); $this->groupManager->expects($this->any())->method(('get')) ->willReturnMap([ - ['group1', $group1], - ['group2', $group2], + ['group1', $this->group1], + ['group2', $this->group2], ]); } @@ -121,10 +146,7 @@ protected function tearDown(): void { parent::tearDown(); } - private function createManagerForUser($userId) { - $user = $this->createMock(IUser::class); - $user->method('getUID') - ->willReturn($userId); + private function createManagerForUser(IUser $user): Manager&MockObject { $userSession = $this->createMock(IUserSession::class); $userSession->method('getUser') ->willReturn($user); @@ -141,15 +163,19 @@ private function createManagerForUser($userId) { $this->cloudFederationProviderManager, $this->cloudFederationFactory, $this->groupManager, - $this->userManager, $userSession, $this->eventDispatcher, $this->logger, + $this->rootFolder, + $this->setupManager, + $this->certificateManager, + $this->externalShareMapper, + Server::get(IGenerator::class), ] )->onlyMethods(['tryOCMEndPoint'])->getMock(); } - private function setupMounts() { + private function setupMounts(): void { $this->clearMounts(); $mounts = $this->testMountProvider->getMountsForUser($this->user, new StorageFactory()); foreach ($mounts as $mount) { @@ -157,47 +183,43 @@ private function setupMounts() { } } - private function clearMounts() { + private function clearMounts(): void { $this->mountManager->clear(); $this->mountManager->addMount(new MountPoint(Temporary::class, '', [])); } public function testAddUserShare(): void { - $this->doTestAddShare([ - 'remote' => 'http://localhost', - 'token' => 'token1', - 'password' => '', - 'name' => '/SharedFolder', - 'owner' => 'foobar', - 'shareType' => IShare::TYPE_USER, - 'accepted' => false, - 'user' => $this->uid, - 'remoteId' => '2342' - ], false); + $userShare = new ExternalShare(); + $userShare->setId(Server::get(IGenerator::class)->nextId()); + $userShare->setRemote('http://localhost'); + $userShare->setShareToken('token1'); + $userShare->setPassword(''); + $userShare->setName('/SharedFolder'); + $userShare->setOwner('foobar'); + $userShare->setShareType(IShare::TYPE_USER); + $userShare->setAccepted(IShare::STATUS_PENDING); + $userShare->setRemoteId('2342'); + + $this->doTestAddShare($userShare, $this->user); } public function testAddGroupShare(): void { - $this->doTestAddShare([ - 'remote' => 'http://localhost', - 'token' => 'token1', - 'password' => '', - 'name' => '/SharedFolder', - 'owner' => 'foobar', - 'shareType' => IShare::TYPE_GROUP, - 'accepted' => false, - 'user' => 'group1', - 'remoteId' => '2342' - ], true); + $groupShare = new ExternalShare(); + $groupShare->setId(Server::get(IGenerator::class)->nextId()); + $groupShare->setRemote('http://localhost'); + $groupShare->setOwner('foobar'); + $groupShare->setShareType(IShare::TYPE_GROUP); + $groupShare->setAccepted(IShare::STATUS_PENDING); + $groupShare->setRemoteId('2342'); + $groupShare->setShareToken('token1'); + $groupShare->setPassword(''); + $groupShare->setName('/SharedFolder'); + $this->doTestAddShare($groupShare, $this->group1, isGroup: true); } - public function doTestAddShare($shareData1, $isGroup = false) { - $shareData2 = $shareData1; - $shareData2['token'] = 'token2'; - $shareData3 = $shareData1; - $shareData3['token'] = 'token3'; - + public function doTestAddShare(ExternalShare $shareData1, IUser|IGroup $userOrGroup, bool $isGroup = false): void { if ($isGroup) { - $this->manager->expects($this->never())->method('tryOCMEndPoint'); + $this->manager->expects($this->never())->method('tryOCMEndPoint')->willReturn(false); } else { $this->manager->expects(self::atLeast(2)) ->method('tryOCMEndPoint') @@ -208,27 +230,34 @@ public function doTestAddShare($shareData1, $isGroup = false) { } // Add a share for "user" - $this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData1)); + $this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], [$shareData1, $userOrGroup])); $openShares = $this->manager->getOpenShares(); $this->assertCount(1, $openShares); - $this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1['name'] . '}}', $shareData1['user']); + $this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1->getName() . '}}', $userOrGroup); + + $shareData2 = $shareData1->clone(); + $shareData2->setShareToken('token2'); + $shareData2->setId(\OCP\Server::get(IGenerator::class)->nextId()); + $shareData3 = $shareData1->clone(); + $shareData3->setShareToken('token3'); + $shareData3->setId(\OCP\Server::get(IGenerator::class)->nextId()); $this->setupMounts(); $this->assertNotMount('SharedFolder'); - $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); + $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)); + $this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], [$shareData2, $userOrGroup])); $openShares = $this->manager->getOpenShares(); $this->assertCount(2, $openShares); - $this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1['name'] . '}}', $shareData1['user']); + $this->assertExternalShareEntry($shareData1, $openShares[0], 1, '{{TemporaryMountPointName#' . $shareData1->getName() . '}}', $userOrGroup); // New share falls back to "-1" appendix, because the name is already taken - $this->assertExternalShareEntry($shareData2, $openShares[1], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1', $shareData2['user']); + $this->assertExternalShareEntry($shareData2, $openShares[1], 2, '{{TemporaryMountPointName#' . $shareData2->getName() . '}}-1', $userOrGroup); $this->setupMounts(); $this->assertNotMount('SharedFolder'); - $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); - $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); + $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1->getName() . '}}'); + $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1->getName() . '}}-1'); $newClientCalls = []; $this->clientService @@ -253,44 +282,44 @@ public function doTestAddShare($shareData1, $isGroup = false) { ])); $client->expects($this->once()) ->method('post') - ->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[0]['remote_id']), $this->anything()) + ->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[0]->getRemoteId()), $this->anything()) ->willReturn($response); } // Accept the first share - $this->assertTrue($this->manager->acceptShare($openShares[0]['id'])); + $this->assertTrue($this->manager->acceptShare($openShares[0])); // Check remaining shares - Accepted - $acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]); + $acceptedShares = $this->externalShareMapper->getShares($this->user, IShare::STATUS_ACCEPTED); $this->assertCount(1, $acceptedShares); - $shareData1['accepted'] = true; - $this->assertExternalShareEntry($shareData1, $acceptedShares[0], 1, $shareData1['name'], $this->uid); + $shareData1->setAccepted(true); + $this->assertExternalShareEntry($shareData1, $acceptedShares[0], 1, $shareData1->getName(), $this->user); // Check remaining shares - Open $openShares = $this->manager->getOpenShares(); $this->assertCount(1, $openShares); - $this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1', $shareData2['user']); + $this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2->getName() . '}}-1', $userOrGroup); $this->setupMounts(); - $this->assertMount($shareData1['name']); - $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); - $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); + $this->assertMount($shareData1->getName()); + $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1->getName() . '}}'); + $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)); + $this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], [$shareData3, $userOrGroup])); $openShares = $this->manager->getOpenShares(); $this->assertCount(2, $openShares); - $this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1', $shareData2['user']); + $this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2->getName() . '}}-1', $userOrGroup); if (!$isGroup) { // New share falls back to the original name (no "-\d", because the name is not taken) - $this->assertExternalShareEntry($shareData3, $openShares[1], 3, '{{TemporaryMountPointName#' . $shareData3['name'] . '}}', $shareData3['user']); + $this->assertExternalShareEntry($shareData3, $openShares[1], 3, '{{TemporaryMountPointName#' . $shareData3->getName() . '}}', $userOrGroup); } else { - $this->assertExternalShareEntry($shareData3, $openShares[1], 3, '{{TemporaryMountPointName#' . $shareData3['name'] . '}}-2', $shareData3['user']); + $this->assertExternalShareEntry($shareData3, $openShares[1], 3, '{{TemporaryMountPointName#' . $shareData3->getName() . '}}-2', $userOrGroup); } $this->setupMounts(); - $this->assertMount($shareData1['name']); - $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); - $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); + $this->assertMount($shareData1->getName()); + $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1->getName() . '}}'); + $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1->getName() . '}}-1'); if (!$isGroup) { $client = $this->createMock(IClient::class); @@ -306,45 +335,45 @@ public function doTestAddShare($shareData1, $isGroup = false) { ])); $client->expects($this->once()) ->method('post') - ->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[1]['remote_id'] . '/decline'), $this->anything()) + ->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[1]->getRemoteId() . '/decline'), $this->anything()) ->willReturn($response); } // Decline the third share - $this->assertTrue($this->manager->declineShare($openShares[1]['id'])); + $this->assertTrue($this->manager->declineShare($openShares[1])); $this->setupMounts(); - $this->assertMount($shareData1['name']); - $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); - $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); + $this->assertMount($shareData1->getName()); + $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1->getName() . '}}'); + $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1->getName() . '}}-1'); // Check remaining shares - Accepted - $acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]); + $acceptedShares = $this->externalShareMapper->getShares($this->user, IShare::STATUS_ACCEPTED); $this->assertCount(1, $acceptedShares); - $shareData1['accepted'] = true; - $this->assertExternalShareEntry($shareData1, $acceptedShares[0], 1, $shareData1['name'], $this->uid); + $shareData1->setAccepted(true); + $this->assertExternalShareEntry($shareData1, $acceptedShares[0], 1, $shareData1->getName(), $this->user); // Check remaining shares - Open $openShares = $this->manager->getOpenShares(); if ($isGroup) { // declining a group share adds it back to pending instead of deleting it $this->assertCount(2, $openShares); // this is a group share that is still open - $this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1', $shareData2['user']); + $this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2->getName() . '}}-1', $userOrGroup); // this is the user share sub-entry matching the group share which got declined - $this->assertExternalShareEntry($shareData3, $openShares[1], 2, '{{TemporaryMountPointName#' . $shareData3['name'] . '}}-2', $this->uid); + $this->assertExternalShareEntry($shareData3, $openShares[1], 2, '{{TemporaryMountPointName#' . $shareData3->getName() . '}}-2', $this->user); } else { $this->assertCount(1, $openShares); - $this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2['name'] . '}}-1', $this->uid); + $this->assertExternalShareEntry($shareData2, $openShares[0], 2, '{{TemporaryMountPointName#' . $shareData2->getName() . '}}-1', $this->user); } $this->setupMounts(); - $this->assertMount($shareData1['name']); - $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); - $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); + $this->assertMount($shareData1->getName()); + $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1->getName() . '}}'); + $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1->getName() . '}}-1'); if ($isGroup) { // no http requests here - $this->manager->removeGroupShares('group1'); + $this->manager->removeGroupShares($this->group1); } else { $client1 = $this->createMock(IClient::class); $client2 = $this->createMock(IClient::class); @@ -362,111 +391,119 @@ public function doTestAddShare($shareData1, $isGroup = false) { $client1->expects($this->once()) ->method('post') - ->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[0]['remote_id'] . '/decline'), $this->anything()) + ->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $openShares[0]->getRemoteId() . '/decline'), $this->anything()) ->willReturn($response); $client2->expects($this->once()) ->method('post') - ->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $acceptedShares[0]['remote_id'] . '/decline'), $this->anything()) + ->with($this->stringStartsWith('http://localhost/ocs/v2.php/cloud/shares/' . $acceptedShares[0]->getRemoteId() . '/decline'), $this->anything()) ->willReturn($response); - $this->manager->removeUserShares($this->uid); + $this->manager->removeUserShares($this->user); } - $this->assertEmpty(self::invokePrivate($this->manager, 'getShares', [null]), 'Asserting all shares for the user have been deleted'); + $this->assertEmpty($this->externalShareMapper->getShares($this->user, null), 'Asserting all shares for the user have been deleted'); $this->clearMounts(); self::invokePrivate($this->manager, 'setupMounts'); - $this->assertNotMount($shareData1['name']); - $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); - $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); + $this->assertNotMount($shareData1->getName()); + $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1->getName() . '}}'); + $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1->getName() . '}}-1'); } - private function verifyAcceptedGroupShare($shareData) { + private function verifyAcceptedGroupShare(ExternalShare $share): void { $openShares = $this->manager->getOpenShares(); $this->assertCount(0, $openShares); - $acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]); + $acceptedShares = $this->externalShareMapper->getShares($this->user, IShare::STATUS_ACCEPTED); $this->assertCount(1, $acceptedShares); - $shareData['accepted'] = true; - $this->assertExternalShareEntry($shareData, $acceptedShares[0], 0, $shareData['name'], $this->uid); + $share->setAccepted(IShare::STATUS_ACCEPTED); + $this->assertExternalShareEntry($share, $acceptedShares[0], 0, $share->getName(), $this->user); $this->setupMounts(); - $this->assertMount($shareData['name']); + $this->assertMount($share->getName()); } - private function verifyDeclinedGroupShare($shareData, $tempMount = null) { + private function verifyDeclinedGroupShare(ExternalShare $share, ?string $tempMount = null): void { if ($tempMount === null) { $tempMount = '{{TemporaryMountPointName#/SharedFolder}}'; } $openShares = $this->manager->getOpenShares(); $this->assertCount(1, $openShares); - $acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]); + $acceptedShares = $this->externalShareMapper->getShares($this->user, IShare::STATUS_ACCEPTED); $this->assertCount(0, $acceptedShares); - $this->assertExternalShareEntry($shareData, $openShares[0], 0, $tempMount, $this->uid); + $share->setAccepted(IShare::STATUS_PENDING); + $this->assertExternalShareEntry($share, $openShares[0], 0, $tempMount, $this->user); $this->setupMounts(); - $this->assertNotMount($shareData['name']); + $this->assertNotMount($share->getName()); $this->assertNotMount($tempMount); } - private function createTestUserShare($userId = 'user1') { - $shareData = [ - 'remote' => 'http://localhost', - 'token' => 'token1', - 'password' => '', - 'name' => '/SharedFolder', - 'owner' => 'foobar', - 'shareType' => IShare::TYPE_USER, - 'accepted' => false, - 'user' => $userId, - 'remoteId' => '2342' - ]; - - $this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData)); - - return $shareData; + private function createTestUserShare(string $userId = 'user1'): ExternalShare { + $user = $this->createMock(IUser::class); + $user->expects($this->any())->method('getUID')->willReturn($userId); + $share = new ExternalShare(); + $share->setId(Server::get(IGenerator::class)->nextId()); + $share->setRemote('http://localhost'); + $share->setShareToken('token1'); + $share->setPassword(''); + $share->setName('/SharedFolder'); + $share->setOwner('foobar'); + $share->setShareType(IShare::TYPE_USER); + $share->setAccepted(IShare::STATUS_PENDING); + $share->setRemoteId('2346'); + + $this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], [$share, $user])); + + return $share; } - private function createTestGroupShare($groupId = 'group1') { - $shareData = [ - 'remote' => 'http://localhost', - 'token' => 'token1', - 'password' => '', - 'name' => '/SharedFolder', - 'owner' => 'foobar', - 'shareType' => IShare::TYPE_GROUP, - 'accepted' => false, - 'user' => $groupId, - 'remoteId' => '2342' - ]; - - $this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData)); - - $allShares = self::invokePrivate($this->manager, 'getShares', [null]); + + /** + * @return array{0: ExternalShare, 1: ExternalShare} + */ + private function createTestGroupShare(string $groupId = 'group1'): array { + $share = new ExternalShare(); + $share->setId(Server::get(IGenerator::class)->nextId()); + $share->setRemote('http://localhost'); + $share->setShareToken('token1'); + $share->setPassword(''); + $share->setName('/SharedFolder'); + $share->setOwner('foobar'); + $share->setShareType(IShare::TYPE_GROUP); + $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])); + + $allShares = $this->externalShareMapper->getShares($this->user, null); + $groupShare = null; foreach ($allShares as $share) { - if ($share['user'] === $groupId) { + if ($share->getUser() === $groupId) { // this will hold the main group entry $groupShare = $share; break; } } - return [$shareData, $groupShare]; + $this->assertEquals($share->getId(), $groupShare->getId()); + + return [$share, $groupShare]; } public function testAcceptOriginalGroupShare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->acceptShare($groupShare['id'])); + $this->assertTrue($this->manager->acceptShare($groupShare)); $this->verifyAcceptedGroupShare($shareData); // a second time - $this->assertTrue($this->manager->acceptShare($groupShare['id'])); + $this->assertTrue($this->manager->acceptShare($groupShare)); $this->verifyAcceptedGroupShare($shareData); } public function testAcceptGroupShareAgainThroughGroupShare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->acceptShare($groupShare['id'])); + $this->assertTrue($this->manager->acceptShare($groupShare)); $this->verifyAcceptedGroupShare($shareData); // decline again, this keeps the sub-share - $this->assertTrue($this->manager->declineShare($groupShare['id'])); + $this->assertTrue($this->manager->declineShare($groupShare)); $this->verifyDeclinedGroupShare($shareData, '/SharedFolder'); // this will return sub-entries @@ -474,21 +511,21 @@ public function testAcceptGroupShareAgainThroughGroupShare(): void { $this->assertCount(1, $openShares); // accept through group share - $this->assertTrue($this->manager->acceptShare($groupShare['id'])); + $this->assertTrue($this->manager->acceptShare($groupShare)); $this->verifyAcceptedGroupShare($shareData, '/SharedFolder'); // accept a second time - $this->assertTrue($this->manager->acceptShare($groupShare['id'])); + $this->assertTrue($this->manager->acceptShare($groupShare)); $this->verifyAcceptedGroupShare($shareData, '/SharedFolder'); } public function testAcceptGroupShareAgainThroughSubShare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->acceptShare($groupShare['id'])); + $this->assertTrue($this->manager->acceptShare($groupShare)); $this->verifyAcceptedGroupShare($shareData); // decline again, this keeps the sub-share - $this->assertTrue($this->manager->declineShare($groupShare['id'])); + $this->assertTrue($this->manager->declineShare($groupShare)); $this->verifyDeclinedGroupShare($shareData, '/SharedFolder'); // this will return sub-entries @@ -496,212 +533,209 @@ public function testAcceptGroupShareAgainThroughSubShare(): void { $this->assertCount(1, $openShares); // accept through sub-share - $this->assertTrue($this->manager->acceptShare($openShares[0]['id'])); + $this->assertTrue($this->manager->acceptShare($openShares[0])); $this->verifyAcceptedGroupShare($shareData); // accept a second time - $this->assertTrue($this->manager->acceptShare($openShares[0]['id'])); + $this->assertTrue($this->manager->acceptShare($openShares[0])); $this->verifyAcceptedGroupShare($shareData); } public function testDeclineOriginalGroupShare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->declineShare($groupShare['id'])); + $this->assertTrue($this->manager->declineShare($groupShare)); $this->verifyDeclinedGroupShare($shareData); // a second time - $this->assertTrue($this->manager->declineShare($groupShare['id'])); + $this->assertTrue($this->manager->declineShare($groupShare)); $this->verifyDeclinedGroupShare($shareData); } public function testDeclineGroupShareAgainThroughGroupShare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->acceptShare($groupShare['id'])); + $this->assertTrue($this->manager->acceptShare($groupShare)); $this->verifyAcceptedGroupShare($shareData); // decline again, this keeps the sub-share - $this->assertTrue($this->manager->declineShare($groupShare['id'])); + $this->assertTrue($this->manager->declineShare($groupShare)); $this->verifyDeclinedGroupShare($shareData, '/SharedFolder'); // a second time - $this->assertTrue($this->manager->declineShare($groupShare['id'])); + $this->assertTrue($this->manager->declineShare($groupShare)); $this->verifyDeclinedGroupShare($shareData, '/SharedFolder'); } public function testDeclineGroupShareAgainThroughSubshare(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->acceptShare($groupShare['id'])); + $this->assertTrue($this->manager->acceptShare($groupShare)); $this->verifyAcceptedGroupShare($shareData); // this will return sub-entries - $allShares = self::invokePrivate($this->manager, 'getShares', [null]); + $allShares = $this->externalShareMapper->getShares($this->user, null); $this->assertCount(1, $allShares); // decline again through sub-share - $this->assertTrue($this->manager->declineShare($allShares[0]['id'])); + $this->assertTrue($this->manager->declineShare($allShares[0])); $this->verifyDeclinedGroupShare($shareData, '/SharedFolder'); // a second time - $this->assertTrue($this->manager->declineShare($allShares[0]['id'])); + $this->assertTrue($this->manager->declineShare($allShares[0])); $this->verifyDeclinedGroupShare($shareData, '/SharedFolder'); } public function testDeclineGroupShareAgainThroughMountPoint(): void { [$shareData, $groupShare] = $this->createTestGroupShare(); - $this->assertTrue($this->manager->acceptShare($groupShare['id'])); + $this->assertTrue($this->manager->acceptShare($groupShare)); $this->verifyAcceptedGroupShare($shareData); // decline through mount point name - $this->assertTrue($this->manager->removeShare($this->uid . '/files/' . $shareData['name'])); + $this->assertTrue($this->manager->removeShare($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->uid . '/files/' . $shareData['name'])); + $this->assertFalse($this->manager->removeShare($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['id'])); + $this->assertTrue($this->manager->declineShare($groupShare)); $this->verifyDeclinedGroupShare($shareData); - // this will return sub-entries - $openShares = $this->manager->getOpenShares(); - // accept through sub-share - $this->assertTrue($this->manager->acceptShare($groupShare['id'])); + $this->assertTrue($this->manager->acceptShare($groupShare)); $this->verifyAcceptedGroupShare($shareData, '/SharedFolder'); // accept a second time - $this->assertTrue($this->manager->acceptShare($groupShare['id'])); + $this->assertTrue($this->manager->acceptShare($groupShare)); $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['id'])); + $this->assertTrue($this->manager->declineShare($groupShare)); $this->verifyDeclinedGroupShare($shareData); // this will return sub-entries $openShares = $this->manager->getOpenShares(); // accept through sub-share - $this->assertTrue($this->manager->acceptShare($openShares[0]['id'])); + $this->assertTrue($this->manager->acceptShare($openShares[0])); $this->verifyAcceptedGroupShare($shareData); // accept a second time - $this->assertTrue($this->manager->acceptShare($openShares[0]['id'])); + $this->assertTrue($this->manager->acceptShare($openShares[0])); $this->verifyAcceptedGroupShare($shareData); } public function testDeleteUserShares(): void { // user 1 shares - $shareData = $this->createTestUserShare($this->uid); + $userShare = $this->createTestUserShare($this->user->getUID()); [$shareData, $groupShare] = $this->createTestGroupShare(); $shares = $this->manager->getOpenShares(); $this->assertCount(2, $shares); - $this->assertTrue($this->manager->acceptShare($groupShare['id'])); + $this->assertTrue($this->manager->acceptShare($groupShare)); + $user2 = $this->createMock(IUser::class); + $user2->method('getUID')->willReturn('user2'); // user 2 shares - $manager2 = $this->createManagerForUser('user2'); - $shareData2 = [ - 'remote' => 'http://localhost', - 'token' => 'token1', - 'password' => '', - 'name' => '/SharedFolder', - 'owner' => 'foobar', - 'shareType' => IShare::TYPE_USER, - 'accepted' => false, - 'user' => 'user2', - 'remoteId' => '2342' - ]; + $manager2 = $this->createManagerForUser($user2); + $share = new ExternalShare(); + $share->setId(Server::get(IGenerator::class)->nextId()); + $share->setRemote('http://localhost'); + $share->setShareToken('token1'); + $share->setPassword(''); + $share->setName('/SharedFolder'); + $share->setOwner('foobar'); + $share->setShareType(IShare::TYPE_USER); + $share->setAccepted(IShare::STATUS_PENDING); + $share->setRemoteId('2342'); $this->assertCount(1, $manager2->getOpenShares()); - $this->assertSame(null, call_user_func_array([$manager2, 'addShare'], $shareData2)); + $this->assertSame(null, call_user_func_array([$manager2, 'addShare'], [$share, $user2])); $this->assertCount(2, $manager2->getOpenShares()); - $this->manager->expects($this->once())->method('tryOCMEndPoint')->with('http://localhost', 'token1', '2342', 'decline')->willReturn([]); - $this->manager->removeUserShares($this->uid); + $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(); // user share is gone, group is still there $this->assertCount(1, $user1Shares); - $this->assertEquals($user1Shares[0]['share_type'], IShare::TYPE_GROUP); + $this->assertEquals($user1Shares[0]->getShareType(), IShare::TYPE_GROUP); // user 2 shares untouched $user2Shares = $manager2->getOpenShares(); $this->assertCount(2, $user2Shares); - $this->assertEquals($user2Shares[0]['share_type'], IShare::TYPE_GROUP); - $this->assertEquals($user2Shares[0]['user'], 'group1'); - $this->assertEquals($user2Shares[1]['share_type'], IShare::TYPE_USER); - $this->assertEquals($user2Shares[1]['user'], 'user2'); + $this->assertEquals($user2Shares[0]->getShareType(), IShare::TYPE_GROUP); + $this->assertEquals($user2Shares[0]->getUser(), 'group1'); + $this->assertEquals($user2Shares[1]->getShareType(), IShare::TYPE_USER); + $this->assertEquals($user2Shares[1]->getUser(), 'user2'); } public function testDeleteGroupShares(): void { - $shareData = $this->createTestUserShare($this->uid); + $shareData = $this->createTestUserShare($this->user->getUID()); [$shareData, $groupShare] = $this->createTestGroupShare(); $shares = $this->manager->getOpenShares(); $this->assertCount(2, $shares); - $this->assertTrue($this->manager->acceptShare($groupShare['id'])); + $this->assertTrue($this->manager->acceptShare($groupShare)); + + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('user2'); // user 2 shares - $manager2 = $this->createManagerForUser('user2'); - $shareData2 = [ - 'remote' => 'http://localhost', - 'token' => 'token1', - 'password' => '', - 'name' => '/SharedFolder', - 'owner' => 'foobar', - 'shareType' => IShare::TYPE_USER, - 'accepted' => false, - 'user' => 'user2', - 'remoteId' => '2342' - ]; + $manager2 = $this->createManagerForUser($user); + + $share = new ExternalShare(); + $share->setId(Server::get(IGenerator::class)->nextId()); + $share->setRemote('http://localhost'); + $share->setShareToken('token1'); + $share->setPassword(''); + $share->setName('/SharedFolder'); + $share->setOwner('foobar'); + $share->setShareType(IShare::TYPE_USER); + $share->setAccepted(IShare::STATUS_PENDING); + $share->setRemoteId('2343'); $this->assertCount(1, $manager2->getOpenShares()); - $this->assertSame(null, call_user_func_array([$manager2, 'addShare'], $shareData2)); + $this->assertSame(null, call_user_func_array([$manager2, 'addShare'], [$share, $user])); $this->assertCount(2, $manager2->getOpenShares()); $this->manager->expects($this->never())->method('tryOCMEndPoint'); - $this->manager->removeGroupShares('group1'); + $this->manager->removeGroupShares($this->group1); $user1Shares = $this->manager->getOpenShares(); // user share is gone, group is still there $this->assertCount(1, $user1Shares); - $this->assertEquals($user1Shares[0]['share_type'], IShare::TYPE_USER); + $this->assertEquals($user1Shares[0]->getShareType(), IShare::TYPE_USER); // user 2 shares untouched $user2Shares = $manager2->getOpenShares(); $this->assertCount(1, $user2Shares); - $this->assertEquals($user2Shares[0]['share_type'], IShare::TYPE_USER); - $this->assertEquals($user2Shares[0]['user'], 'user2'); + $this->assertEquals($user2Shares[0]->getShareType(), IShare::TYPE_USER); + $this->assertEquals($user2Shares[0]->getUser(), 'user2'); } - /** - * @param array $expected - * @param array $actual - * @param int $share - * @param string $mountPoint - */ - protected function assertExternalShareEntry($expected, $actual, $share, $mountPoint, $targetEntity) { - $this->assertEquals($expected['remote'], $actual['remote'], 'Asserting remote of a share #' . $share); - $this->assertEquals($expected['token'], $actual['share_token'], 'Asserting token of a share #' . $share); - $this->assertEquals($expected['name'], $actual['name'], 'Asserting name of a share #' . $share); - $this->assertEquals($expected['owner'], $actual['owner'], 'Asserting owner of a share #' . $share); - $this->assertEquals($expected['accepted'], (int)$actual['accepted'], 'Asserting accept of a share #' . $share); - $this->assertEquals($targetEntity, $actual['user'], 'Asserting user of a share #' . $share); - $this->assertEquals($mountPoint, $actual['mountpoint'], 'Asserting mountpoint of a share #' . $share); + protected function assertExternalShareEntry(ExternalShare $expected, ExternalShare $actual, int $share, string $mountPoint, IUser|IGroup $targetEntity): void { + $this->assertEquals($expected->getRemote(), $actual->getRemote(), 'Asserting remote of a share #' . $share); + $this->assertEquals($expected->getShareToken(), $actual->getShareToken(), 'Asserting token of a share #' . $share); + $this->assertEquals($expected->getName(), $actual->getName(), 'Asserting name of a share #' . $share); + $this->assertEquals($expected->getOwner(), $actual->getOwner(), 'Asserting owner of a share #' . $share); + $this->assertEquals($expected->getAccepted(), $actual->getAccepted(), 'Asserting accept of a share #' . $share); + $this->assertEquals($targetEntity instanceof IGroup ? $targetEntity->getGID() : $targetEntity->getUID(), $actual->getUser(), 'Asserting user of a share #' . $share); + $this->assertEquals($mountPoint, $actual->getMountpoint(), 'Asserting mountpoint of a share #' . $share); } - private function assertMount($mountPoint) { + private function assertMount(string $mountPoint): void { $mountPoint = rtrim($mountPoint, '/'); $mount = $this->mountManager->find($this->getFullPath($mountPoint)); $this->assertInstanceOf('\OCA\Files_Sharing\External\Mount', $mount); @@ -711,7 +745,7 @@ private function assertMount($mountPoint) { $this->assertInstanceOf('\OCA\Files_Sharing\External\Storage', $storage); } - private function assertNotMount($mountPoint) { + private function assertNotMount(string $mountPoint): void { $mountPoint = rtrim($mountPoint, '/'); try { $mount = $this->mountManager->find($this->getFullPath($mountPoint)); @@ -722,7 +756,7 @@ private function assertNotMount($mountPoint) { } } - private function getFullPath($path) { - return '/' . $this->uid . '/files' . $path; + private function getFullPath(string $path): string { + return '/' . $this->user->getUID() . '/files' . $path; } } diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index 16e3068b15b90..fa1ac5b46e0f8 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -991,7 +991,7 @@ protected function removeShareFromTable(int $shareId): void { */ protected function createShareObject(array $data): IShare { $share = new Share($this->rootFolder, $this->userManager); - $share->setId((int)$data['id']) + $share->setId($data['id']) ->setShareType((int)$data['share_type']) ->setPermissions((int)$data['permissions']) ->setTarget($data['file_target']) diff --git a/build/integration/features/bootstrap/AppConfiguration.php b/build/integration/features/bootstrap/AppConfiguration.php index e8580ed537b55..0ebe05f41c84e 100644 --- a/build/integration/features/bootstrap/AppConfiguration.php +++ b/build/integration/features/bootstrap/AppConfiguration.php @@ -7,6 +7,7 @@ */ use Behat\Behat\Hook\Scope\AfterScenarioScope; use Behat\Behat\Hook\Scope\BeforeScenarioScope; +use Behat\Gherkin\Node\TableNode; use PHPUnit\Framework\Assert; use Psr\Http\Message\ResponseInterface; @@ -19,8 +20,8 @@ trait AppConfiguration { /** @var ResponseInterface */ private $response = null; - abstract public function sendingTo($verb, $url); - abstract public function sendingToWith($verb, $url, $body); + abstract public function sendingTo(string $verb, string $url); + abstract public function sendingToWith(string $verb, string $url, ?TableNode $body); abstract public function theOCSStatusCodeShouldBe($statusCode); abstract public function theHTTPStatusCodeShouldBe($statusCode); diff --git a/build/integration/features/bootstrap/BasicStructure.php b/build/integration/features/bootstrap/BasicStructure.php index e6ba54a72129d..76dc7bcf272b4 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -74,26 +74,23 @@ public function __construct($baseUrl, $admin, $regular_user_password) { /** * @Given /^using api version "(\d+)"$/ - * @param string $version */ - public function usingApiVersion($version) { + public function usingApiVersion(int $version): void { $this->apiVersion = (int)$version; } /** * @Given /^As an "([^"]*)"$/ - * @param string $user */ - public function asAn($user) { + public function asAn(string $user): void { $this->currentUser = $user; } /** * @Given /^Using server "(LOCAL|REMOTE)"$/ - * @param string $server * @return string Previous used server */ - public function usingServer($server) { + public function usingServer(string $server): string { $previousServer = $this->currentServer; if ($server === 'LOCAL') { $this->baseUrl = $this->localBaseUrl; @@ -108,21 +105,16 @@ public function usingServer($server) { /** * @When /^sending "([^"]*)" to "([^"]*)"$/ - * @param string $verb - * @param string $url */ - public function sendingTo($verb, $url) { + public function sendingTo(string $verb, string $url): void { $this->sendingToWith($verb, $url, null); } /** * Parses the xml or json answer to get ocs response which doesn't match with * http one in v1 of the api. - * - * @param ResponseInterface $response - * @return string */ - public function getOCSResponseCode($response): int { + public function getOCSResponseCode(ResponseInterface $response): int { if ($response === null) { throw new \RuntimeException('No response available'); } @@ -141,11 +133,8 @@ public function getOCSResponseCode($response): int { /** * This function is needed to use a vertical fashion in the gherkin tables. - * - * @param array $arrayOfArrays - * @return array */ - public function simplifyArray($arrayOfArrays) { + public function simplifyArray(array $arrayOfArrays): array { $a = array_map(function ($subArray) { return $subArray[0]; }, $arrayOfArrays); @@ -154,11 +143,8 @@ public function simplifyArray($arrayOfArrays) { /** * @When /^sending "([^"]*)" to "([^"]*)" with$/ - * @param string $verb - * @param string $url - * @param TableNode $body */ - public function sendingToWith($verb, $url, $body) { + public function sendingToWith(string $verb, string $url, ?TableNode $body): void { $fullUrl = $this->baseUrl . "v{$this->apiVersion}.php" . $url; $client = new Client(); $options = []; @@ -191,13 +177,7 @@ public function sendingToWith($verb, $url, $body) { } } - /** - * @param string $verb - * @param string $url - * @param TableNode|array|null $body - * @param array $headers - */ - protected function sendRequestForJSON(string $verb, string $url, $body = null, array $headers = []): void { + protected function sendRequestForJSON(string $verb, string $url, TableNode|array|null $body = null, array $headers = []): void { $fullUrl = $this->baseUrl . "v{$this->apiVersion}.php" . $url; $client = new Client(); $options = []; diff --git a/build/integration/features/bootstrap/CommandLine.php b/build/integration/features/bootstrap/CommandLine.php index 0ffa1b9623d4c..f814c758683ff 100644 --- a/build/integration/features/bootstrap/CommandLine.php +++ b/build/integration/features/bootstrap/CommandLine.php @@ -11,22 +11,20 @@ trait CommandLine { /** @var int return code of last command */ - private $lastCode; + private int $lastCode = 0; /** @var string stdout of last command */ - private $lastStdOut; + private string $lastStdOut = ''; /** @var string stderr of last command */ - private $lastStdErr; - - /** @var string */ - protected $ocPath = '../..'; + private string $lastStdErr = ''; + protected string $ocPath = '../..'; /** * Invokes an OCC command * - * @param []string $args OCC command, the part behind "occ". For example: "files:transfer-ownership" + * @param string[] $args OCC command, the part behind "occ". For example: "files:transfer-ownership" * @return int exit code */ - public function runOcc($args = [], string $inputString = '') { + public function runOcc(array $args = [], string $inputString = ''): int { $args = array_map(function ($arg) { return escapeshellarg($arg); }, $args); @@ -57,7 +55,7 @@ public function runOcc($args = [], string $inputString = '') { /** * @Given /^invoking occ with "([^"]*)"$/ */ - public function invokingTheCommand($cmd) { + public function invokingTheCommand(string $cmd): void { $args = explode(' ', $cmd); $this->runOcc($args); } @@ -65,7 +63,7 @@ public function invokingTheCommand($cmd) { /** * @Given /^invoking occ with "([^"]*)" with input "([^"]+)"$/ */ - public function invokingTheCommandWith($cmd, $inputString) { + public function invokingTheCommandWith(string $cmd, string $inputString): void { $args = explode(' ', $cmd); $this->runOcc($args, $inputString); } @@ -73,7 +71,7 @@ public function invokingTheCommandWith($cmd, $inputString) { /** * Find exception texts in stderr */ - public function findExceptions() { + public function findExceptions(): array { $exceptions = []; $captureNext = false; // the exception text usually appears after an "[Exception"] row @@ -94,7 +92,7 @@ public function findExceptions() { /** * @Then /^the command was successful$/ */ - public function theCommandWasSuccessful() { + public function theCommandWasSuccessful(): void { $exceptions = $this->findExceptions(); if ($this->lastCode !== 0) { $msg = 'The command was not successful, exit code was ' . $this->lastCode . '.'; @@ -111,7 +109,7 @@ public function theCommandWasSuccessful() { /** * @Then /^the command failed with exit code ([0-9]+)$/ */ - public function theCommandFailedWithExitCode($exitCode) { + public function theCommandFailedWithExitCode($exitCode): void { if ($this->lastCode !== (int)$exitCode) { throw new \Exception('The command was expected to fail with exit code ' . $exitCode . ' but got ' . $this->lastCode); } @@ -120,7 +118,7 @@ public function theCommandFailedWithExitCode($exitCode) { /** * @Then /^the command failed with exception text "([^"]*)"$/ */ - public function theCommandFailedWithException($exceptionText) { + public function theCommandFailedWithException(string $exceptionText): void { $exceptions = $this->findExceptions(); if (empty($exceptions)) { throw new \Exception('The command did not throw any exceptions'); @@ -134,21 +132,21 @@ public function theCommandFailedWithException($exceptionText) { /** * @Then /^the command output contains the text "([^"]*)"$/ */ - public function theCommandOutputContainsTheText($text) { - Assert::assertStringContainsString($text, $this->lastStdOut, 'The command did not output the expected text on stdout'); + public function theCommandOutputContainsTheText(string $text): void { + Assert::assertStringContainsString($text, $this->lastStdOut, 'The command did not output the expected text on stdout.'); } /** * @Then /^the command output does not contain the text "([^"]*)"$/ */ - public function theCommandOutputDoesNotContainTheText($text) { - Assert::assertStringNotContainsString($text, $this->lastStdOut, 'The command did output the not expected text on stdout'); + public function theCommandOutputDoesNotContainTheText(string $text): void { + Assert::assertStringNotContainsString($text, $this->lastStdOut, 'The command did output the not expected text on stdout.'); } /** * @Then /^the command error output contains the text "([^"]*)"$/ */ - public function theCommandErrorOutputContainsTheText($text) { - Assert::assertStringContainsString($text, $this->lastStdErr, 'The command did not output the expected text on stderr'); + public function theCommandErrorOutputContainsTheText(string $text): void { + Assert::assertStringContainsString($text, $this->lastStdErr, 'The command did not output the expected text on stderr.'); } } diff --git a/build/integration/features/bootstrap/FederationContext.php b/build/integration/features/bootstrap/FederationContext.php index 95dc8119ad6a3..d2832e268addb 100644 --- a/build/integration/features/bootstrap/FederationContext.php +++ b/build/integration/features/bootstrap/FederationContext.php @@ -33,7 +33,7 @@ class FederationContext implements Context, SnippetAcceptingContext { * The server is started also after the scenarios to ensure that it is * properly cleaned up if stopped. */ - public function startFederatedServer() { + public function startFederatedServer(): void { if (self::$phpFederatedServerPid !== '') { return; } @@ -46,7 +46,7 @@ public function startFederatedServer() { /** * @BeforeScenario */ - public function cleanupRemoteStorages() { + public function cleanupRemoteStorages(): void { // Ensure that dangling remote storages from previous tests will not // interfere with the current scenario. // The storages must be cleaned before each scenario; they can not be @@ -59,13 +59,10 @@ public function cleanupRemoteStorages() { /** * @Given /^User "([^"]*)" from server "(LOCAL|REMOTE)" shares "([^"]*)" with user "([^"]*)" from server "(LOCAL|REMOTE)"$/ * - * @param string $sharerUser - * @param string $sharerServer "LOCAL" or "REMOTE" - * @param string $sharerPath - * @param string $shareeUser - * @param string $shareeServer "LOCAL" or "REMOTE" + * @param 'LOCAL'|'REMOTE' $sharerServer "LOCAL" or "REMOTE" + * @param 'LOCAL'|'REMOTE' $shareeServer */ - public function federateSharing($sharerUser, $sharerServer, $sharerPath, $shareeUser, $shareeServer) { + public function federateSharing(string $sharerUser, string $sharerServer, string $sharerPath, string $shareeUser, string $shareeServer): void { if ($shareeServer == 'REMOTE') { $shareWith = "$shareeUser@" . substr($this->remoteBaseUrl, 0, -4); } else { @@ -80,13 +77,10 @@ public function federateSharing($sharerUser, $sharerServer, $sharerPath, $sharee /** * @Given /^User "([^"]*)" from server "(LOCAL|REMOTE)" shares "([^"]*)" with group "([^"]*)" from server "(LOCAL|REMOTE)"$/ * - * @param string $sharerUser - * @param string $sharerServer "LOCAL" or "REMOTE" - * @param string $sharerPath - * @param string $shareeUser - * @param string $shareeServer "LOCAL" or "REMOTE" + * @param 'LOCAL'|'REMOTE' $sharerServer "LOCAL" or "REMOTE" + * @param 'LOCAL'|'REMOTE' $shareeServer */ - public function federateGroupSharing($sharerUser, $sharerServer, $sharerPath, $shareeGroup, $shareeServer) { + public function federateGroupSharing(string $sharerUser, string $sharerServer, string $sharerPath, string $shareeGroup, string $shareeServer): void { if ($shareeServer == 'REMOTE') { $shareWith = "$shareeGroup@" . substr($this->remoteBaseUrl, 0, -4); } else { @@ -99,11 +93,8 @@ public function federateGroupSharing($sharerUser, $sharerServer, $sharerPath, $s /** * @Then remote share :count is returned with - * - * @param int $number - * @param TableNode $body */ - public function remoteShareXIsReturnedWith(int $number, TableNode $body) { + public function remoteShareXIsReturnedWith(int $number, TableNode $body): void { $this->theHTTPStatusCodeShouldBe('200'); $this->theOCSStatusCodeShouldBe('100'); @@ -130,16 +121,15 @@ public function remoteShareXIsReturnedWith(int $number, TableNode $body) { /** * @When /^User "([^"]*)" from server "(LOCAL|REMOTE)" accepts last pending share$/ - * @param string $user - * @param string $server */ - public function acceptLastPendingShare($user, $server) { + public function acceptLastPendingShare(string $user, string $server): void { $previous = $this->usingServer($server); $this->asAn($user); $this->sendingToWith('GET', '/apps/files_sharing/api/v1/remote_shares/pending', null); $this->theHTTPStatusCodeShouldBe('200'); $this->theOCSStatusCodeShouldBe('100'); - $share_id = simplexml_load_string($this->response->getBody())->data[0]->element[0]->id; + $shares = simplexml_load_string($this->response->getBody())->data[0]->element; + $share_id = $shares[count($shares) - 1]->id; $this->sendingToWith('POST', "/apps/files_sharing/api/v1/remote_shares/pending/{$share_id}", null); $this->theHTTPStatusCodeShouldBe('200'); $this->theOCSStatusCodeShouldBe('100'); @@ -150,9 +140,8 @@ public function acceptLastPendingShare($user, $server) { /** * @When /^user "([^"]*)" deletes last accepted remote share$/ - * @param string $user */ - public function deleteLastAcceptedRemoteShare($user) { + public function deleteLastAcceptedRemoteShare(string $user): void { $this->asAn($user); $this->sendingToWith('DELETE', '/apps/files_sharing/api/v1/remote_shares/' . $this->lastAcceptedRemoteShareId, null); } @@ -160,7 +149,7 @@ public function deleteLastAcceptedRemoteShare($user) { /** * @When /^remote server is stopped$/ */ - public function remoteServerIsStopped() { + public function remoteServerIsStopped(): void { if (self::$phpFederatedServerPid === '') { return; } @@ -173,7 +162,7 @@ public function remoteServerIsStopped() { /** * @BeforeScenario @TrustedFederation */ - public function theServersAreTrustingEachOther() { + public function theServersAreTrustingEachOther(): void { $this->asAn('admin'); // Trust the remote server on the local server $this->usingServer('LOCAL'); @@ -190,7 +179,7 @@ public function theServersAreTrustingEachOther() { /** * @AfterScenario @TrustedFederation */ - public function theServersAreNoLongerTrustingEachOther() { + public function theServersAreNoLongerTrustingEachOther(): void { $this->asAn('admin'); // Untrust the remote servers on the local server $this->usingServer('LOCAL'); @@ -213,7 +202,7 @@ public function theServersAreNoLongerTrustingEachOther() { } } - protected function resetAppConfigs() { + protected function resetAppConfigs(): void { $this->deleteServerConfig('files_sharing', 'incoming_server2server_group_share_enabled'); $this->deleteServerConfig('files_sharing', 'outgoing_server2server_group_share_enabled'); $this->deleteServerConfig('files_sharing', 'federated_trusted_share_auto_accept'); diff --git a/build/integration/features/bootstrap/Sharing.php b/build/integration/features/bootstrap/Sharing.php index d93f114f27bdb..2f3303ac1a1dd 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -17,27 +17,19 @@ trait Sharing { use Provisioning; - /** @var int */ - private $sharingApiVersion = 1; - - /** @var SimpleXMLElement */ - private $lastShareData = null; + private int $sharingApiVersion = 1; + private ?SimpleXMLElement $lastShareData = null; /** @var SimpleXMLElement[] */ - private $storedShareData = []; - - /** @var int */ - private $savedShareId = null; - + private array $storedShareData = []; + private ?string $savedShareId = null; /** @var ResponseInterface */ private $response; /** * @Given /^as "([^"]*)" creating a share with$/ - * @param string $user - * @param TableNode|null $body */ - public function asCreatingAShareWith($user, $body) { + public function asCreatingAShareWith(string $user, ?TableNode $body): void { $fullUrl = $this->baseUrl . "v{$this->apiVersion}.php/apps/files_sharing/api/v{$this->sharingApiVersion}/shares"; $client = new Client(); $options = [ @@ -78,29 +70,28 @@ public function asCreatingAShareWith($user, $body) { /** * @When /^save the last share data as "([^"]*)"$/ */ - public function saveLastShareData($name) { + public function saveLastShareData(string $name): void { $this->storedShareData[$name] = $this->lastShareData; } /** * @When /^restore the last share data from "([^"]*)"$/ */ - public function restoreLastShareData($name) { + public function restoreLastShareData(string $name): void { $this->lastShareData = $this->storedShareData[$name]; } /** * @When /^creating a share with$/ - * @param TableNode|null $body */ - public function creatingShare($body) { + public function creatingShare(?TableNode $body): void { $this->asCreatingAShareWith($this->currentUser, $body); } /** * @When /^accepting last share$/ */ - public function acceptingLastShare() { + public function acceptingLastShare(): void { $share_id = $this->lastShareData->data[0]->id; $url = "/apps/files_sharing/api/v{$this->sharingApiVersion}/shares/pending/$share_id"; $this->sendingToWith('POST', $url, null); @@ -110,10 +101,8 @@ public function acceptingLastShare() { /** * @When /^user "([^"]*)" accepts last share$/ - * - * @param string $user */ - public function userAcceptsLastShare(string $user) { + public function userAcceptsLastShare(string $user): void { // "As userXXX" and "user userXXX accepts last share" steps are not // expected to be used in the same scenario, but restore the user just // in case. @@ -133,7 +122,7 @@ public function userAcceptsLastShare(string $user) { /** * @Then /^last link share can be downloaded$/ */ - public function lastLinkShareCanBeDownloaded() { + public function lastLinkShareCanBeDownloaded(): void { if (count($this->lastShareData->data->element) > 0) { $url = $this->lastShareData->data[0]->url; } else { @@ -146,7 +135,7 @@ public function lastLinkShareCanBeDownloaded() { /** * @Then /^last share can be downloaded$/ */ - public function lastShareCanBeDownloaded() { + public function lastShareCanBeDownloaded(): void { if (count($this->lastShareData->data->element) > 0) { $token = $this->lastShareData->data[0]->token; } else { @@ -305,6 +294,18 @@ public function createShare($user, } } + public function getFieldValueInResponse($field) { + $data = simplexml_load_string($this->response->getBody())->data[0]; + if (count($data->element) > 0) { + foreach ($data as $element) { + return (string)$element->$field; + } + + return false; + } + return $data->$field; + } + public function isFieldInResponse($field, $contentExpected) { $data = simplexml_load_string($this->response->getBody())->data[0]; if ((string)$field == 'expiration') { @@ -521,7 +522,7 @@ public function checkShareFields($body) { /** * @Then the list of returned shares has :count shares */ - public function theListOfReturnedSharesHasShares(int $count) { + public function theListOfReturnedSharesHasShares(int $count): void { $this->theHTTPStatusCodeShouldBe('200'); $this->theOCSStatusCodeShouldBe('100'); diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index 2cb37002ac02c..c88efa485224d 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -33,14 +33,14 @@ trait WebDav { /** * @Given /^using dav path "([^"]*)"$/ */ - public function usingDavPath($davPath) { + public function usingDavPath(string $davPath): void { $this->davPath = $davPath; } /** * @Given /^using old dav path$/ */ - public function usingOldDavPath() { + public function usingOldDavPath(): void { $this->davPath = 'remote.php/webdav'; $this->usingOldDavPath = true; } @@ -48,7 +48,7 @@ public function usingOldDavPath() { /** * @Given /^using new dav path$/ */ - public function usingNewDavPath() { + public function usingNewDavPath(): void { $this->davPath = 'remote.php/dav'; $this->usingOldDavPath = false; } @@ -56,7 +56,7 @@ public function usingNewDavPath() { /** * @Given /^using new public dav path$/ */ - public function usingNewPublicDavPath() { + public function usingNewPublicDavPath(): void { $this->davPath = 'public.php/dav'; $this->usingOldDavPath = false; } diff --git a/build/integration/run-docker.sh b/build/integration/run-docker.sh index 044898077d64b..a53e4e1c5e47a 100755 --- a/build/integration/run-docker.sh +++ b/build/integration/run-docker.sh @@ -141,6 +141,8 @@ function prepareDocker() { --exclude="./config/config.php" \ --exclude="./config/*.config.php" \ --exclude="./data" \ + --exclude="./apps-extra" \ + --exclude="./apps-extra2" \ --exclude="./data-autotest" \ --exclude="./tests" \ --exclude="node_modules" \ diff --git a/build/integration/run.sh b/build/integration/run.sh index 30dd0646b1081..9b4b14d5ff442 100755 --- a/build/integration/run.sh +++ b/build/integration/run.sh @@ -18,6 +18,8 @@ HIDE_OC_LOGS=$2 INSTALLED=$($OCC status | grep installed: | cut -d " " -f 5) if [ "$INSTALLED" == "true" ]; then + # Run in debug mode + $OCC config:system:set debug --value=true --type=boolean # Disable appstore to avoid spamming from CI $OCC config:system:set appstoreenabled --value=false --type=boolean # Disable bruteforce protection because the integration tests do trigger them diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 75beecd1639ed..022d6eb47c0c9 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1214,17 +1214,6 @@ - - - - - - - - - - - @@ -1235,13 +1224,6 @@ - - - - - - - @@ -1257,10 +1239,6 @@ - - - - &$shareWith] )]]> - - - - - - - - - - - - - @@ -1545,15 +1510,6 @@ - - - - - - - - - @@ -1594,25 +1550,10 @@ - - - - - - - - - - - - - - - @@ -2441,12 +2382,6 @@ - - - getId()]]> - - - @@ -4091,17 +4026,7 @@ dbprettyname]]> - - - getId()]]> - getId()]]> - - - - - - @@ -4211,14 +4136,8 @@ - - - - - - diff --git a/lib/private/Activity/Event.php b/lib/private/Activity/Event.php index 0ccad1d0a4e66..6a2bc5dba7777 100644 --- a/lib/private/Activity/Event.php +++ b/lib/private/Activity/Event.php @@ -48,8 +48,7 @@ class Event implements IEvent { protected $messageRichParameters = []; /** @var string */ protected $objectType = ''; - /** @var int */ - protected $objectId = 0; + protected string|int $objectId = 0; /** @var string */ protected $objectName = ''; /** @var string */ @@ -319,13 +318,16 @@ public function getRichMessageParameters(): array { /** * {@inheritDoc} */ - public function setObject(string $objectType, int $objectId, string $objectName = ''): IEvent { + public function setObject(string $objectType, string|int $objectId, string $objectName = ''): IEvent { if (isset($objectType[255])) { throw new InvalidValueException('objectType'); } if (isset($objectName[4000])) { throw new InvalidValueException('objectName'); } + if (is_string($objectId) && isset($objectId[19])) { + throw new InvalidValueException('objectId'); + } $this->objectType = $objectType; $this->objectId = $objectId; $this->objectName = $objectName; @@ -340,9 +342,9 @@ public function getObjectType(): string { } /** - * @return int + * @return int|string */ - public function getObjectId(): int { + public function getObjectId(): string|int { return $this->objectId; } diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index 3558664758816..fef0d634f0562 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -391,13 +391,50 @@ public function delete() { /** * Add a suffix to the name in case the file exists * - * @param string $name + * @param string $filename * @return string * @throws NotPermittedException */ - public function getNonExistingName($name) { - $uniqueName = \OC_Helper::buildNotExistingFileNameForView($this->getPath(), $name, $this->view); - return trim($this->getRelativePath($uniqueName), '/'); + public function getNonExistingName($filename) { + $path = $this->getPath(); + if ($path === '/') { + $path = ''; + } + if ($pos = strrpos($filename, '.')) { + $name = substr($filename, 0, $pos); + $ext = substr($filename, $pos); + } else { + $name = $filename; + $ext = ''; + } + + $newpath = $path . '/' . $filename; + if ($this->view->file_exists($newpath)) { + if (preg_match_all('/\((\d+)\)/', $name, $matches, PREG_OFFSET_CAPTURE)) { + /** @var array, array> $matches */ + //Replace the last "(number)" with "(number+1)" + $last_match = count($matches[0]) - 1; + $counter = $matches[1][$last_match][0] + 1; + $offset = $matches[0][$last_match][1]; + $match_length = strlen($matches[0][$last_match][0]); + } else { + $counter = 2; + $match_length = 0; + $offset = false; + } + do { + if ($offset) { + //Replace the last "(number)" with "(number+1)" + $newname = substr_replace($name, '(' . $counter . ')', $offset, $match_length); + } else { + $newname = $name . ' (' . $counter . ')'; + } + $newpath = $path . '/' . $newname . $ext; + $counter++; + } while ($this->view->file_exists($newpath)); + } + + return trim($this->getRelativePath($newpath), '/'); } /** diff --git a/lib/private/Files/Node/LazyFolder.php b/lib/private/Files/Node/LazyFolder.php index c4188b80ee5ec..c23a7d03ada9e 100644 --- a/lib/private/Files/Node/LazyFolder.php +++ b/lib/private/Files/Node/LazyFolder.php @@ -493,7 +493,7 @@ public function isCreatable() { /** * @inheritDoc */ - public function getNonExistingName($name) { + public function getNonExistingName($filename) { return $this->__call(__FUNCTION__, func_get_args()); } diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 5300e6e1baad2..bc74002fcbc09 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -989,7 +989,7 @@ public function getShareByToken($token) { */ private function createShare($data) { $share = new Share($this->rootFolder, $this->userManager); - $share->setId((int)$data['id']) + $share->setId($data['id']) ->setShareType((int)$data['share_type']) ->setPermissions((int)$data['permissions']) ->setTarget($data['file_target']) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 956c24a92afa2..deaeb52b668d0 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -10,6 +10,7 @@ use OC\Core\AppInfo\ConfigLexicon; use OC\Files\Mount\MoveableMount; use OC\KnownUser\KnownUserService; +use OC\Share\Helper; use OC\Share20\Exception\ProviderException; use OCA\Files_Sharing\AppInfo\Application; use OCA\Files_Sharing\SharedStorage; @@ -29,12 +30,10 @@ use OCP\IDateTimeZone; use OCP\IGroupManager; use OCP\IL10N; -use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; use OCP\L10N\IFactory; -use OCP\Mail\IMailer; use OCP\Security\Events\ValidatePasswordPolicyEvent; use OCP\Security\IHasher; use OCP\Security\ISecureRandom; @@ -56,6 +55,7 @@ use OCP\Share\IShareProviderSupportsAccept; use OCP\Share\IShareProviderSupportsAllSharesInFolder; use OCP\Share\IShareProviderWithNotification; +use Override; use Psr\Log\LoggerInterface; /** @@ -77,9 +77,6 @@ public function __construct( private IProviderFactory $factory, private IUserManager $userManager, private IRootFolder $rootFolder, - private IMailer $mailer, - private IURLGenerator $urlGenerator, - private \OC_Defaults $defaults, private IEventDispatcher $dispatcher, private IUserSession $userSession, private KnownUserService $knownUserService, @@ -96,20 +93,18 @@ public function __construct( /** * Convert from a full share id to a tuple (providerId, shareId) * - * @param string $id * @return string[] */ - private function splitFullId($id) { + private function splitFullId(string $id): array { return explode(':', $id, 2); } /** * Verify if a password meets all requirements * - * @param string $password * @throws HintException */ - protected function verifyPassword($password) { + protected function verifyPassword(?string $password): void { if ($password === null) { // No password is set, check if this is allowed. if ($this->shareApiLinkEnforcePassword()) { @@ -138,7 +133,7 @@ protected function verifyPassword($password) { * * @suppress PhanUndeclaredClassMethod */ - protected function generalCreateChecks(IShare $share, bool $isUpdate = false) { + protected function generalCreateChecks(IShare $share, bool $isUpdate = false): void { if ($share->getShareType() === IShare::TYPE_USER) { // We expect a valid user as sharedWith for user shares if (!$this->userManager->userExists($share->getSharedWith())) { @@ -278,7 +273,7 @@ protected function generalCreateChecks(IShare $share, bool $isUpdate = false) { * @throws \InvalidArgumentException * @throws \Exception */ - protected function validateExpirationDateInternal(IShare $share) { + protected function validateExpirationDateInternal(IShare $share): IShare { $isRemote = $share->getShareType() === IShare::TYPE_REMOTE || $share->getShareType() === IShare::TYPE_REMOTE_GROUP; $expirationDate = $share->getExpirationDate(); @@ -369,7 +364,7 @@ protected function validateExpirationDateInternal(IShare $share) { * @throws \InvalidArgumentException * @throws \Exception */ - protected function validateExpirationDateLink(IShare $share) { + protected function validateExpirationDateLink(IShare $share): IShare { $expirationDate = $share->getExpirationDate(); $isEnforced = $this->shareApiLinkDefaultExpireDateEnforced(); @@ -446,10 +441,9 @@ protected function validateExpirationDateLink(IShare $share) { /** * Check for pre share requirements for user shares * - * @param IShare $share * @throws \Exception */ - protected function userCreateChecks(IShare $share) { + protected function userCreateChecks(IShare $share): void { // Check if we can share with group members only if ($this->shareWithGroupMembersOnly()) { $sharedBy = $this->userManager->get($share->getSharedBy()); @@ -508,10 +502,9 @@ protected function userCreateChecks(IShare $share) { /** * Check for pre share requirements for group shares * - * @param IShare $share * @throws \Exception */ - protected function groupCreateChecks(IShare $share) { + protected function groupCreateChecks(IShare $share): void { // Verify group shares are allowed if (!$this->allowGroupSharing()) { throw new \Exception($this->l->t('Group sharing is now allowed')); @@ -554,10 +547,9 @@ protected function groupCreateChecks(IShare $share) { /** * Check for pre share requirements for link shares * - * @param IShare $share * @throws \Exception */ - protected function linkCreateChecks(IShare $share) { + protected function linkCreateChecks(IShare $share): void { // Are link shares allowed? if (!$this->shareApiAllowLinks()) { throw new \Exception($this->l->t('Link sharing is not allowed')); @@ -578,10 +570,8 @@ protected function linkCreateChecks(IShare $share) { * See: https://github.com/owncloud/core/issues/22295 * * FIXME: Remove once multiple link shares can be properly displayed - * - * @param IShare $share */ - protected function setLinkParent(IShare $share) { + protected function setLinkParent(IShare $share): void { $storage = $share->getNode()->getStorage(); if ($storage->instanceOfStorage(SharedStorage::class)) { /** @var \OCA\Files_Sharing\SharedStorage $storage */ @@ -589,10 +579,7 @@ protected function setLinkParent(IShare $share) { } } - /** - * @param File|Folder $path - */ - protected function pathCreateChecks($path) { + protected function pathCreateChecks(Node $path): void { // Make sure that we do not share a path that contains a shared mountpoint if ($path instanceof \OCP\Files\Folder) { $mounts = $this->mountManager->findIn($path->getPath()); @@ -611,10 +598,9 @@ protected function pathCreateChecks($path) { /** * Check if the user that is sharing can actually share * - * @param IShare $share * @throws \Exception */ - protected function canShare(IShare $share) { + protected function canShare(IShare $share): void { if (!$this->shareApiEnabled()) { throw new \Exception($this->l->t('Sharing is disabled')); } @@ -624,16 +610,9 @@ protected function canShare(IShare $share) { } } - /** - * Share a path - * - * @param IShare $share - * @return IShare The share object - * @throws \Exception - * - * TODO: handle link share permissions or check them - */ - public function createShare(IShare $share) { + #[Override] + public function createShare(IShare $share): IShare { + // TODO: handle link share permissions or check them $this->canShare($share); $this->generalCreateChecks($share); @@ -766,15 +745,8 @@ public function createShare(IShare $share) { return $share; } - /** - * Update a share - * - * @param IShare $share - * @return IShare The share object - * @throws \InvalidArgumentException - * @throws HintException - */ - public function updateShare(IShare $share, bool $onlyValid = true) { + #[Override] + public function updateShare(IShare $share, bool $onlyValid = true): IShare { $expirationDateUpdated = false; $this->canShare($share); @@ -915,15 +887,7 @@ public function updateShare(IShare $share, bool $onlyValid = true) { return $share; } - /** - * Accept a share. - * - * @param IShare $share - * @param string $recipientId - * @return IShare The share object - * @throws \InvalidArgumentException Thrown if the provider does not implement `IShareProviderSupportsAccept` - * @since 9.0.0 - */ + #[Override] public function acceptShare(IShare $share, string $recipientId): IShare { [$providerId,] = $this->splitFullId($share->getFullId()); $provider = $this->factory->getProvider($providerId); @@ -947,9 +911,9 @@ public function acceptShare(IShare $share, string $recipientId): IShare { * @param IShare $share the share to update its password. * @param IShare $originalShare the original share to compare its * password with. - * @return boolean whether the password was updated or not. + * @return bool whether the password was updated or not. */ - private function updateSharePasswordIfNeeded(IShare $share, IShare $originalShare) { + private function updateSharePasswordIfNeeded(IShare $share, IShare $originalShare): bool { $passwordsAreDifferent = ($share->getPassword() !== $originalShare->getPassword()) && (($share->getPassword() !== null && $originalShare->getPassword() === null) || ($share->getPassword() === null && $originalShare->getPassword() !== null) @@ -1009,9 +973,9 @@ private function setSharePasswordExpirationTime(IShare $share): void { * Delete all the children of this share * * @param IShare $share - * @return IShare[] List of deleted shares + * @return list List of deleted shares */ - protected function deleteChildren(IShare $share) { + protected function deleteChildren(IShare $share): array { $deletedShares = []; $provider = $this->factory->getProviderForType($share->getShareType()); @@ -1118,14 +1082,8 @@ protected function promoteReshares(IShare $share): void { } } - /** - * Delete a share - * - * @param IShare $share - * @throws ShareNotFound - * @throws \InvalidArgumentException - */ - public function deleteShare(IShare $share) { + #[Override] + public function deleteShare(IShare $share): void { try { $share->getFullId(); } catch (\UnexpectedValueException $e) { @@ -1147,17 +1105,8 @@ public function deleteShare(IShare $share) { $this->promoteReshares($share); } - - /** - * Unshare a file as the recipient. - * This can be different from a regular delete for example when one of - * the users in a groups deletes that share. But the provider should - * handle this. - * - * @param IShare $share - * @param string $recipientId - */ - public function deleteFromSelf(IShare $share, $recipientId) { + #[Override] + public function deleteFromSelf(IShare $share, string $recipientId): void { [$providerId,] = $this->splitFullId($share->getFullId()); $provider = $this->factory->getProvider($providerId); @@ -1166,6 +1115,7 @@ public function deleteFromSelf(IShare $share, $recipientId) { $this->dispatchEvent($event, 'leave share'); } + #[Override] public function restoreShare(IShare $share, string $recipientId): IShare { [$providerId,] = $this->splitFullId($share->getFullId()); $provider = $this->factory->getProvider($providerId); @@ -1173,10 +1123,8 @@ public function restoreShare(IShare $share, string $recipientId): IShare { return $provider->restore($share, $recipientId); } - /** - * @inheritdoc - */ - public function moveShare(IShare $share, $recipientId) { + #[Override] + public function moveShare(IShare $share, string $recipientId): IShare { if ($share->getShareType() === IShare::TYPE_LINK || $share->getShareType() === IShare::TYPE_EMAIL) { throw new \InvalidArgumentException($this->l->t('Cannot change target of link share')); @@ -1203,7 +1151,8 @@ public function moveShare(IShare $share, $recipientId) { return $provider->move($share, $recipientId); } - public function getSharesInFolder($userId, Folder $node, $reshares = false, $shallow = true) { + #[Override] + public function getSharesInFolder($userId, Folder $node, bool $reshares = false, bool $shallow = true): array { $providers = $this->factory->getAllProviders(); if (!$shallow) { throw new \Exception('non-shallow getSharesInFolder is no longer supported'); @@ -1233,10 +1182,8 @@ public function getSharesInFolder($userId, Folder $node, $reshares = false, $sha return $shares; } - /** - * @inheritdoc - */ - public function getSharesBy($userId, $shareType, $path = null, $reshares = false, $limit = 50, $offset = 0, bool $onlyValid = true) { + #[Override] + public function getSharesBy(string $userId, int $shareType, ?Node $path = null, bool $reshares = false, int $limit = 50, int $offset = 0, bool $onlyValid = true): array { if ($path !== null && !($path instanceof \OCP\Files\File) && !($path instanceof \OCP\Files\Folder)) { @@ -1317,10 +1264,8 @@ public function getSharesBy($userId, $shareType, $path = null, $reshares = false return $shares; } - /** - * @inheritdoc - */ - public function getSharedWith($userId, $shareType, $node = null, $limit = 50, $offset = 0) { + #[Override] + public function getSharedWith(string $userId, int $shareType, ?Node $node = null, int $limit = 50, int $offset = 0): array { try { $provider = $this->factory->getProviderForType($shareType); } catch (ProviderException $e) { @@ -1341,29 +1286,17 @@ public function getSharedWith($userId, $shareType, $node = null, $limit = 50, $o return $shares; } - /** - * @inheritdoc - */ - public function getDeletedSharedWith($userId, $shareType, $node = null, $limit = 50, $offset = 0) { + #[Override] + public function getDeletedSharedWith(string $userId, int $shareType, ?Node $node = null, int $limit = 50, int $offset = 0): array { $shares = $this->getSharedWith($userId, $shareType, $node, $limit, $offset); - // Only get deleted shares - $shares = array_filter($shares, function (IShare $share) { - return $share->getPermissions() === 0; - }); - - // Only get shares where the owner still exists - $shares = array_filter($shares, function (IShare $share) { - return $this->userManager->userExists($share->getShareOwner()); - }); - - return $shares; + // Only get shares deleted shares and where the owner still exists + return array_filter($shares, fn (IShare $share): bool => $share->getPermissions() === 0 + && $this->userManager->userExists($share->getShareOwner())); } - /** - * @inheritdoc - */ - public function getShareById($id, $recipient = null, bool $onlyValid = true) { + #[Override] + public function getShareById($id, $recipient = null, bool $onlyValid = true): IShare { if ($id === null) { throw new ShareNotFound(); } @@ -1385,29 +1318,9 @@ public function getShareById($id, $recipient = null, bool $onlyValid = true) { return $share; } - /** - * Get all the shares for a given path - * - * @param \OCP\Files\Node $path - * @param int $page - * @param int $perPage - * - * @return Share[] - */ - public function getSharesByPath(\OCP\Files\Node $path, $page = 0, $perPage = 50) { - return []; - } - - /** - * Get the share by token possible with password - * - * @param string $token - * @return IShare - * - * @throws ShareNotFound - */ - public function getShareByToken($token) { - // tokens cannot be valid local user names + #[Override] + public function getShareByToken(string $token): IShare { + // tokens cannot be valid local usernames if ($this->userManager->userExists($token)) { throw new ShareNotFound(); } @@ -1417,8 +1330,7 @@ public function getShareByToken($token) { $provider = $this->factory->getProviderForType(IShare::TYPE_LINK); $share = $provider->getShareByToken($token); } - } catch (ProviderException $e) { - } catch (ShareNotFound $e) { + } catch (ProviderException|ShareNotFound) { } @@ -1427,8 +1339,7 @@ public function getShareByToken($token) { try { $provider = $this->factory->getProviderForType(IShare::TYPE_REMOTE); $share = $provider->getShareByToken($token); - } catch (ProviderException $e) { - } catch (ShareNotFound $e) { + } catch (ProviderException|ShareNotFound) { } } @@ -1437,8 +1348,7 @@ public function getShareByToken($token) { try { $provider = $this->factory->getProviderForType(IShare::TYPE_EMAIL); $share = $provider->getShareByToken($token); - } catch (ProviderException $e) { - } catch (ShareNotFound $e) { + } catch (ProviderException|ShareNotFound) { } } @@ -1446,8 +1356,7 @@ public function getShareByToken($token) { try { $provider = $this->factory->getProviderForType(IShare::TYPE_CIRCLE); $share = $provider->getShareByToken($token); - } catch (ProviderException $e) { - } catch (ShareNotFound $e) { + } catch (ProviderException|ShareNotFound) { } } @@ -1455,8 +1364,7 @@ public function getShareByToken($token) { try { $provider = $this->factory->getProviderForType(IShare::TYPE_ROOM); $share = $provider->getShareByToken($token); - } catch (ProviderException $e) { - } catch (ShareNotFound $e) { + } catch (ProviderException|ShareNotFound) { } } @@ -1509,14 +1417,8 @@ protected function checkShare(IShare $share): void { } } - /** - * Verify the password of a public share - * - * @param IShare $share - * @param ?string $password - * @return bool - */ - public function checkPassword(IShare $share, $password) { + #[Override] + public function checkPassword(IShare $share, ?string $password): bool { // if there is no password on the share object / passsword is null, there is nothing to check if ($password === null || $share->getPassword() === null) { @@ -1543,10 +1445,8 @@ public function checkPassword(IShare $share, $password) { return true; } - /** - * @inheritdoc - */ - public function userDeleted($uid) { + #[Override] + public function userDeleted(string $uid): void { $types = [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_LINK, IShare::TYPE_REMOTE, IShare::TYPE_EMAIL]; foreach ($types as $type) { @@ -1559,10 +1459,8 @@ public function userDeleted($uid) { } } - /** - * @inheritdoc - */ - public function groupDeleted($gid) { + #[Override] + public function groupDeleted(string $gid): void { foreach ([IShare::TYPE_GROUP, IShare::TYPE_REMOTE_GROUP] as $type) { try { $provider = $this->factory->getProviderForType($type); @@ -1586,10 +1484,8 @@ public function groupDeleted($gid) { $this->config->setAppValue('core', 'shareapi_exclude_groups_list', json_encode($excludedGroups)); } - /** - * @inheritdoc - */ - public function userDeletedFromGroup($uid, $gid) { + #[Override] + public function userDeletedFromGroup(string $uid, string $gid): void { foreach ([IShare::TYPE_GROUP, IShare::TYPE_REMOTE_GROUP] as $type) { try { $provider = $this->factory->getProviderForType($type); @@ -1601,7 +1497,7 @@ public function userDeletedFromGroup($uid, $gid) { } #[\Override] - public function getAccessList(\OCP\Files\Node $path, $recursive = true, $currentAccess = false) { + public function getAccessList(\OCP\Files\Node $path, $recursive = true, $currentAccess = false): array { $owner = $path->getOwner(); if ($owner === null) { @@ -1685,31 +1581,18 @@ public function getAccessList(\OCP\Files\Node $path, $recursive = true, $current return $al; } - /** - * Create a new share - * - * @return IShare - */ - public function newShare() { + #[Override] + public function newShare(): IShare { return new \OC\Share20\Share($this->rootFolder, $this->userManager); } - /** - * Is the share API enabled - * - * @return bool - */ - public function shareApiEnabled() { + #[Override] + public function shareApiEnabled(): bool { return $this->config->getAppValue('core', 'shareapi_enabled', 'yes') === 'yes'; } - /** - * Is public link sharing enabled - * - * @param ?IUser $user User to check against group exclusions, defaults to current session user - * @return bool - */ - public function shareApiAllowLinks(?IUser $user = null) { + #[Override] + public function shareApiAllowLinks(?IUser $user = null): bool { if ($this->config->getAppValue('core', 'shareapi_allow_links', 'yes') !== 'yes') { return false; } @@ -1736,13 +1619,8 @@ protected function userCanCreateLinkShares(IUser $user): bool { return $this->shareApiAllowLinks($user); } - /** - * Is password on public link requires - * - * @param bool Check group membership exclusion - * @return bool - */ - public function shareApiLinkEnforcePassword(bool $checkGroupMembership = true) { + #[Override] + public function shareApiLinkEnforcePassword(bool $checkGroupMembership = true): bool { $excludedGroups = $this->config->getAppValue('core', 'shareapi_enforce_links_password_excluded_groups', ''); if ($excludedGroups !== '' && $checkGroupMembership) { $excludedGroups = json_decode($excludedGroups); @@ -1757,117 +1635,66 @@ public function shareApiLinkEnforcePassword(bool $checkGroupMembership = true) { return $this->appConfig->getValueBool('core', ConfigLexicon::SHARE_LINK_PASSWORD_ENFORCED); } - /** - * Is default link expire date enabled - * - * @return bool - */ - public function shareApiLinkDefaultExpireDate() { + #[Override] + public function shareApiLinkDefaultExpireDate(): bool { return $this->appConfig->getValueBool('core', ConfigLexicon::SHARE_LINK_EXPIRE_DATE_DEFAULT); } - /** - * Is default link expire date enforced - * - * @return bool - */ - public function shareApiLinkDefaultExpireDateEnforced() { + #[Override] + public function shareApiLinkDefaultExpireDateEnforced(): bool { return $this->shareApiLinkDefaultExpireDate() && $this->appConfig->getValueBool('core', ConfigLexicon::SHARE_LINK_EXPIRE_DATE_ENFORCED); } - - /** - * Number of default link expire days - * - * @return int - */ - public function shareApiLinkDefaultExpireDays() { + #[Override] + public function shareApiLinkDefaultExpireDays(): int { return (int)$this->config->getAppValue('core', 'shareapi_expire_after_n_days', '7'); } - /** - * Is default internal expire date enabled - * - * @return bool - */ + #[Override] public function shareApiInternalDefaultExpireDate(): bool { return $this->config->getAppValue('core', 'shareapi_default_internal_expire_date', 'no') === 'yes'; } - /** - * Is default remote expire date enabled - * - * @return bool - */ + #[Override] public function shareApiRemoteDefaultExpireDate(): bool { return $this->config->getAppValue('core', 'shareapi_default_remote_expire_date', 'no') === 'yes'; } - /** - * Is default expire date enforced - * - * @return bool - */ + #[Override] public function shareApiInternalDefaultExpireDateEnforced(): bool { return $this->shareApiInternalDefaultExpireDate() && $this->config->getAppValue('core', 'shareapi_enforce_internal_expire_date', 'no') === 'yes'; } - /** - * Is default expire date enforced for remote shares - * - * @return bool - */ + #[Override] public function shareApiRemoteDefaultExpireDateEnforced(): bool { return $this->shareApiRemoteDefaultExpireDate() && $this->config->getAppValue('core', 'shareapi_enforce_remote_expire_date', 'no') === 'yes'; } - /** - * Number of default expire days - * - * @return int - */ + #[Override] public function shareApiInternalDefaultExpireDays(): int { return (int)$this->config->getAppValue('core', 'shareapi_internal_expire_after_n_days', '7'); } - /** - * Number of default expire days for remote shares - * - * @return int - */ + #[Override] public function shareApiRemoteDefaultExpireDays(): int { return (int)$this->config->getAppValue('core', 'shareapi_remote_expire_after_n_days', '7'); } - /** - * Allow public upload on link shares - * - * @return bool - */ - public function shareApiLinkAllowPublicUpload() { + #[Override] + public function shareApiLinkAllowPublicUpload(): bool { return $this->config->getAppValue('core', 'shareapi_allow_public_upload', 'yes') === 'yes'; } - /** - * check if user can only share with group members - * - * @return bool - */ - public function shareWithGroupMembersOnly() { + #[Override] + public function shareWithGroupMembersOnly(): bool { return $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes'; } - /** - * If shareWithGroupMembersOnly is enabled, return an optional - * list of groups that must be excluded from the principle of - * belonging to the same group. - * - * @return array - */ - public function shareWithGroupMembersOnlyExcludeGroupsList() { + #[Override] + public function shareWithGroupMembersOnlyExcludeGroupsList(): array { if (!$this->shareWithGroupMembersOnly()) { return []; } @@ -1875,53 +1702,59 @@ public function shareWithGroupMembersOnlyExcludeGroupsList() { return json_decode($excludeGroups, true) ?? []; } - /** - * Check if users can share with groups - * - * @return bool - */ - public function allowGroupSharing() { + #[Override] + public function allowGroupSharing(): bool { return $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes') === 'yes'; } + #[Override] public function allowEnumeration(): bool { return $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes'; } + #[Override] public function limitEnumerationToGroups(): bool { return $this->allowEnumeration() && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes'; } + #[Override] public function limitEnumerationToPhone(): bool { return $this->allowEnumeration() && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes'; } + #[Override] public function allowEnumerationFullMatch(): bool { return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes'; } + #[Override] public function matchEmail(): bool { return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_email', 'yes') === 'yes'; } + #[Override] public function matchUserId(): bool { return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes') === 'yes'; } + #[Override] public function ignoreSecondDisplayName(): bool { return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_dn', 'no') === 'yes'; } + #[Override] public function allowCustomTokens(): bool { return $this->appConfig->getValueBool('core', ConfigLexicon::SHARE_CUSTOM_TOKEN); } + #[Override] public function allowViewWithoutDownload(): bool { return $this->appConfig->getValueBool('core', 'shareapi_allow_view_without_download', true); } + #[Override] public function currentUserCanEnumerateTargetUser(?IUser $currentUser, IUser $targetUser): bool { if ($this->allowEnumerationFullMatch()) { return true; @@ -1958,31 +1791,23 @@ public function currentUserCanEnumerateTargetUser(?IUser $currentUser, IUser $ta return false; } - /** - * Check if sharing is disabled for the current user - */ + #[Override] public function sharingDisabledForUser(?string $userId): bool { return $this->shareDisableChecker->sharingDisabledForUser($userId); } - /** - * @inheritdoc - */ - public function outgoingServer2ServerSharesAllowed() { + #[Override] + public function outgoingServer2ServerSharesAllowed(): bool { return $this->config->getAppValue('files_sharing', 'outgoing_server2server_share_enabled', 'yes') === 'yes'; } - /** - * @inheritdoc - */ - public function outgoingServer2ServerGroupSharesAllowed() { + #[Override] + public function outgoingServer2ServerGroupSharesAllowed(): bool { return $this->config->getAppValue('files_sharing', 'outgoing_server2server_group_share_enabled', 'no') === 'yes'; } - /** - * @inheritdoc - */ - public function shareProviderExists($shareType) { + #[Override] + public function shareProviderExists(int $shareType): bool { try { $this->factory->getProviderForType($shareType); } catch (ProviderException $e) { @@ -1996,6 +1821,7 @@ public function registerShareProvider(string $shareProviderClass): void { $this->factory->registerProvider($shareProviderClass); } + #[Override] public function getAllShares(): iterable { $providers = $this->factory->getAllProviders(); @@ -2004,9 +1830,10 @@ public function getAllShares(): iterable { } } + #[Override] public function generateToken(): string { // Initial token length - $tokenLength = \OC\Share\Helper::getTokenLength(); + $tokenLength = Helper::getTokenLength(); do { $tokenExists = false; diff --git a/lib/private/legacy/OC_Helper.php b/lib/private/legacy/OC_Helper.php index 84ef0aee565e5..65f4300f850ec 100644 --- a/lib/private/legacy/OC_Helper.php +++ b/lib/private/legacy/OC_Helper.php @@ -105,65 +105,6 @@ public static function canExecute($name, $path = false) { return false; } - /** - * Adds a suffix to the name in case the file exists - * - * @param string $path - * @param string $filename - * @return string - */ - public static function buildNotExistingFileName($path, $filename) { - $view = \OC\Files\Filesystem::getView(); - return self::buildNotExistingFileNameForView($path, $filename, $view); - } - - /** - * Adds a suffix to the name in case the file exists - * - * @param string $path - * @param string $filename - * @return string - */ - public static function buildNotExistingFileNameForView($path, $filename, \OC\Files\View $view) { - if ($path === '/') { - $path = ''; - } - if ($pos = strrpos($filename, '.')) { - $name = substr($filename, 0, $pos); - $ext = substr($filename, $pos); - } else { - $name = $filename; - $ext = ''; - } - - $newpath = $path . '/' . $filename; - if ($view->file_exists($newpath)) { - if (preg_match_all('/\((\d+)\)/', $name, $matches, PREG_OFFSET_CAPTURE)) { - //Replace the last "(number)" with "(number+1)" - $last_match = count($matches[0]) - 1; - $counter = $matches[1][$last_match][0] + 1; - $offset = $matches[0][$last_match][1]; - $match_length = strlen($matches[0][$last_match][0]); - } else { - $counter = 2; - $match_length = 0; - $offset = false; - } - do { - if ($offset) { - //Replace the last "(number)" with "(number+1)" - $newname = substr_replace($name, '(' . $counter . ')', $offset, $match_length); - } else { - $newname = $name . ' (' . $counter . ')'; - } - $newpath = $path . '/' . $newname . $ext; - $counter++; - } while ($view->file_exists($newpath)); - } - - return $newpath; - } - /** * Checks if a function is available * diff --git a/lib/public/Activity/IEvent.php b/lib/public/Activity/IEvent.php index 6014b75123c75..898eebac4b582 100644 --- a/lib/public/Activity/IEvent.php +++ b/lib/public/Activity/IEvent.php @@ -211,14 +211,14 @@ public function getRichMessageParameters(): array; * Set the object of the activity * * @param string $objectType - * @param int $objectId + * @param string|int $objectId * @param string $objectName * @return IEvent * @throws InvalidValueException if the object is invalid * @since 8.2.0 * @since 30.0.0 throws {@see InvalidValueException} instead of \InvalidArgumentException */ - public function setObject(string $objectType, int $objectId, string $objectName = ''): self; + public function setObject(string $objectType, string|int $objectId, string $objectName = ''): self; /** * Set the link of the activity @@ -292,10 +292,10 @@ public function getMessageParameters(): array; public function getObjectType(): string; /** - * @return int + * @return string|int * @since 8.2.0 */ - public function getObjectId(): int; + public function getObjectId(): string|int; /** * @return string diff --git a/lib/public/Federation/ICloudFederationProvider.php b/lib/public/Federation/ICloudFederationProvider.php index b30041f81d614..b92404866cb5f 100644 --- a/lib/public/Federation/ICloudFederationProvider.php +++ b/lib/public/Federation/ICloudFederationProvider.php @@ -58,7 +58,7 @@ public function shareReceived(ICloudFederationShare $share); * * @since 14.0.0 */ - public function notificationReceived($notificationType, $providerId, array $notification); + public function notificationReceived(string $notificationType, string $providerId, array $notification); /** * get the supported share types, e.g. "user", "group", etc. diff --git a/lib/public/Files.php b/lib/public/Files.php index bad2aa7b6cd32..77657c41a912a 100644 --- a/lib/public/Files.php +++ b/lib/public/Files.php @@ -126,16 +126,4 @@ public static function streamCopy($source, $target, ?bool $includeResult = null) } return $includeResult ? [$count, $result] : $count; } - - /** - * Adds a suffix to the name in case the file exists - * @param string $path - * @param string $filename - * @return string - * @since 5.0.0 - * @deprecated 14.0.0 use getNonExistingName of the OCP\Files\Folder object - */ - public static function buildNotExistingFileName($path, $filename) { - return \OC_Helper::buildNotExistingFileName($path, $filename); - } } diff --git a/lib/public/Files/Folder.php b/lib/public/Files/Folder.php index 163d362ecb474..ba23b872abd40 100644 --- a/lib/public/Files/Folder.php +++ b/lib/public/Files/Folder.php @@ -194,12 +194,12 @@ public function isCreatable(); /** * Add a suffix to the name in case the file exists * - * @param string $name + * @param string $filename * @return string * @throws NotPermittedException * @since 8.1.0 */ - public function getNonExistingName($name); + public function getNonExistingName($filename); /** * @param int $limit diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index 6e6ddf2644a3d..40c11c90be0f0 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -7,6 +7,7 @@ */ namespace OCP\Share; +use OCP\AppFramework\Attribute\Consumable; use OCP\Files\Folder; use OCP\Files\Node; @@ -18,22 +19,17 @@ /** * This interface allows to manage sharing files between users and groups. * - * This interface must not be implemented in your application but - * instead should be used as a service and injected in your code with - * dependency injection. - * * @since 9.0.0 */ +#[Consumable(since: '9.0.0')] interface IManager { /** * Create a Share * - * @param IShare $share - * @return IShare The share object * @throws \Exception * @since 9.0.0 */ - public function createShare(IShare $share); + public function createShare(IShare $share): IShare; /** * Update a share. @@ -41,20 +37,15 @@ public function createShare(IShare $share); * The share can't be removed this way (permission 0): use deleteShare * The state can't be changed this way: use acceptShare * - * @param IShare $share * @param bool $onlyValid Only updates valid shares, invalid shares will be deleted automatically and are not updated - * @return IShare The share object * @throws \InvalidArgumentException * @since 9.0.0 */ - public function updateShare(IShare $share, bool $onlyValid = true); + public function updateShare(IShare $share, bool $onlyValid = true): IShare; /** * Accept a share. * - * @param IShare $share - * @param string $recipientId - * @return IShare The share object * @throws \InvalidArgumentException * @since 18.0.0 */ @@ -63,12 +54,11 @@ public function acceptShare(IShare $share, string $recipientId): IShare; /** * Delete a share * - * @param IShare $share * @throws ShareNotFound * @throws \InvalidArgumentException * @since 9.0.0 */ - public function deleteShare(IShare $share); + public function deleteShare(IShare $share): void; /** * Unshare a file as the recipient. @@ -76,11 +66,9 @@ public function deleteShare(IShare $share); * the users in a groups deletes that share. But the provider should * handle this. * - * @param IShare $share - * @param string $recipientId * @since 9.0.0 */ - public function deleteFromSelf(IShare $share, $recipientId); + public function deleteFromSelf(IShare $share, string $recipientId): void; /** * Restore the share when it has been deleted @@ -100,13 +88,10 @@ public function restoreShare(IShare $share, string $recipientId): IShare; * Move the share as a recipient of the share. * This is updating the share target. So where the recipient has the share mounted. * - * @param IShare $share - * @param string $recipientId - * @return IShare * @throws \InvalidArgumentException If $share is a link share or the $recipient does not match * @since 9.0.0 */ - public function moveShare(IShare $share, $recipientId); + public function moveShare(IShare $share, string $recipientId): IShare; /** * Get all shares shared by (initiated) by the provided user in a folder. @@ -115,16 +100,16 @@ public function moveShare(IShare $share, $recipientId); * @param Folder $node * @param bool $reshares * @param bool $shallow Whether the method should stop at the first level, or look into sub-folders. - * @return IShare[][] [$fileId => IShare[], ...] + * @return array> [$fileId => IShare[], ...] * @since 11.0.0 */ - public function getSharesInFolder($userId, Folder $node, $reshares = false, $shallow = true); + public function getSharesInFolder(string $userId, Folder $node, bool $reshares = false, bool $shallow = true): array; /** * Get shares shared by (initiated) by the provided user. * * @param string $userId - * @param int $shareType + * @param IShare::TYPE_* $shareType * @param Node|null $path * @param bool $reshares * @param int $limit The maximum number of returned results, -1 for all results @@ -133,35 +118,32 @@ public function getSharesInFolder($userId, Folder $node, $reshares = false, $sha * @return IShare[] * @since 9.0.0 */ - public function getSharesBy($userId, $shareType, $path = null, $reshares = false, $limit = 50, $offset = 0, bool $onlyValid = true); + public function getSharesBy(string $userId, int $shareType, ?Node $path = null, bool $reshares = false, int $limit = 50, int $offset = 0, bool $onlyValid = true): array; /** * Get shares shared with $user. * Filter by $node if provided * * @param string $userId - * @param int $shareType + * @param IShare::TYPE_* $shareType * @param Node|null $node * @param int $limit The maximum number of shares returned, -1 for all * @param int $offset * @return IShare[] * @since 9.0.0 */ - public function getSharedWith($userId, $shareType, $node = null, $limit = 50, $offset = 0); + public function getSharedWith(string $userId, int $shareType, ?Node $node = null, int $limit = 50, int $offset = 0): array; /** * Get deleted shares shared with $user. * Filter by $node if provided * - * @param string $userId - * @param int $shareType - * @param Node|null $node + * @param IShare::TYPE_* $shareType * @param int $limit The maximum number of shares returned, -1 for all - * @param int $offset * @return IShare[] * @since 14.0.0 */ - public function getDeletedSharedWith($userId, $shareType, $node = null, $limit = 50, $offset = 0); + public function getDeletedSharedWith(string $userId, int $shareType, ?Node $node = null, int $limit = 50, int $offset = 0): array; /** * Retrieve a share by the share id. @@ -169,34 +151,27 @@ public function getDeletedSharedWith($userId, $shareType, $node = null, $limit = * This makes sure that if a user has moved/deleted a group share this * is reflected. * - * @param string $id * @param string|null $recipient userID of the recipient * @param bool $onlyValid Only returns valid shares, invalid shares will be deleted automatically and are not returned - * @return IShare * @throws ShareNotFound * @since 9.0.0 */ - public function getShareById($id, $recipient = null, bool $onlyValid = true); + public function getShareById(string $id, ?string $recipient = null, bool $onlyValid = true): IShare; /** * Get the share by token possible with password * - * @param string $token - * @return IShare * @throws ShareNotFound * @since 9.0.0 */ - public function getShareByToken($token); + public function getShareByToken(string $token): IShare; /** * Verify the password of a public share * - * @param IShare $share - * @param ?string $password - * @return bool * @since 9.0.0 */ - public function checkPassword(IShare $share, $password); + public function checkPassword(IShare $share, ?string $password): bool; /** * The user with UID is deleted. @@ -204,29 +179,25 @@ public function checkPassword(IShare $share, $password); * as shares owned by this user. * Shares only initiated by this user are fine. * - * @param string $uid * @since 9.1.0 */ - public function userDeleted($uid); + public function userDeleted(string $uid): void; /** * The group with $gid is deleted * We need to clear up all shares to this group * - * @param string $gid * @since 9.1.0 */ - public function groupDeleted($gid); + public function groupDeleted(string $gid): void; /** * The user $uid is deleted from the group $gid * All user specific group shares have to be removed * - * @param string $uid - * @param string $gid * @since 9.1.0 */ - public function userDeletedFromGroup($uid, $gid); + public function userDeletedFromGroup(string $uid, string $gid): void; /** * Get access list to a path. This means @@ -270,7 +241,6 @@ public function userDeletedFromGroup($uid, $gid); * * This is required for encryption/activity * - * @param \OCP\Files\Node $path * @param bool $recursive Should we check all parent folders as well * @param bool $currentAccess Should the user have currently access to the file * @return ($currentAccess is true @@ -283,24 +253,22 @@ public function userDeletedFromGroup($uid, $gid); * : array{users?: list, remote?: bool, public?: bool, mail?: list}) * @since 12.0.0 */ - public function getAccessList(\OCP\Files\Node $path, $recursive = true, $currentAccess = false); + public function getAccessList(Node $path, bool $recursive = true, bool $currentAccess = false): array; /** * Instantiates a new share object. This is to be passed to * createShare. * - * @return IShare * @since 9.0.0 */ - public function newShare(); + public function newShare(): IShare; /** * Is the share API enabled * - * @return bool * @since 9.0.0 */ - public function shareApiEnabled(); + public function shareApiEnabled(): bool; /** * Is public link sharing enabled @@ -310,46 +278,41 @@ public function shareApiEnabled(); * @since 9.0.0 * @since 33.0.0 Added optional $user parameter */ - public function shareApiAllowLinks(?IUser $user = null); + public function shareApiAllowLinks(): bool; /** * Is password on public link required * * @param bool $checkGroupMembership Check group membership exclusion - * @return bool * @since 9.0.0 * @since 24.0.0 Added optional $checkGroupMembership parameter */ - public function shareApiLinkEnforcePassword(bool $checkGroupMembership = true); + public function shareApiLinkEnforcePassword(bool $checkGroupMembership = true): bool; /** * Is default expire date enabled * - * @return bool * @since 9.0.0 */ - public function shareApiLinkDefaultExpireDate(); + public function shareApiLinkDefaultExpireDate(): bool; /** * Is default expire date enforced *` - * @return bool * @since 9.0.0 */ - public function shareApiLinkDefaultExpireDateEnforced(); + public function shareApiLinkDefaultExpireDateEnforced(): bool; /** * Number of default expire days * - * @return int * @since 9.0.0 */ - public function shareApiLinkDefaultExpireDays(); + public function shareApiLinkDefaultExpireDays(): int; /** * Is default internal expire date enabled * - * @return bool * @since 22.0.0 */ public function shareApiInternalDefaultExpireDate(): bool; @@ -357,7 +320,6 @@ public function shareApiInternalDefaultExpireDate(): bool; /** * Is default remote expire date enabled * - * @return bool * @since 22.0.0 */ public function shareApiRemoteDefaultExpireDate(): bool; @@ -365,7 +327,6 @@ public function shareApiRemoteDefaultExpireDate(): bool; /** * Is default expire date enforced * - * @return bool * @since 22.0.0 */ public function shareApiInternalDefaultExpireDateEnforced(): bool; @@ -373,7 +334,6 @@ public function shareApiInternalDefaultExpireDateEnforced(): bool; /** * Is default expire date enforced for remote shares * - * @return bool * @since 22.0.0 */ public function shareApiRemoteDefaultExpireDateEnforced(): bool; @@ -381,7 +341,6 @@ public function shareApiRemoteDefaultExpireDateEnforced(): bool; /** * Number of default expire days * - * @return int * @since 22.0.0 */ public function shareApiInternalDefaultExpireDays(): int; @@ -389,7 +348,6 @@ public function shareApiInternalDefaultExpireDays(): int; /** * Number of default expire days for remote shares * - * @return int * @since 22.0.0 */ public function shareApiRemoteDefaultExpireDays(): int; @@ -397,17 +355,16 @@ public function shareApiRemoteDefaultExpireDays(): int; /** * Allow public upload on link shares * - * @return bool * @since 9.0.0 */ - public function shareApiLinkAllowPublicUpload(); + public function shareApiLinkAllowPublicUpload(): bool; /** - * check if user can only share with group members - * @return bool + * Check if user can only share with group members. + * * @since 9.0.0 */ - public function shareWithGroupMembersOnly(); + public function shareWithGroupMembersOnly(): bool; /** * If shareWithGroupMembersOnly is enabled, return an optional @@ -416,19 +373,18 @@ public function shareWithGroupMembersOnly(); * @return array * @since 27.0.0 */ - public function shareWithGroupMembersOnlyExcludeGroupsList(); + public function shareWithGroupMembersOnlyExcludeGroupsList(): array; /** * Check if users can share with groups - * @return bool + * * @since 9.0.1 */ - public function allowGroupSharing(); + public function allowGroupSharing(): bool; /** * Check if user enumeration is allowed * - * @return bool * @since 19.0.0 */ public function allowEnumeration(): bool; @@ -436,7 +392,6 @@ public function allowEnumeration(): bool; /** * Check if user enumeration is limited to the users groups * - * @return bool * @since 19.0.0 */ public function limitEnumerationToGroups(): bool; @@ -444,7 +399,6 @@ public function limitEnumerationToGroups(): bool; /** * Check if user enumeration is limited to the phonebook matches * - * @return bool * @since 21.0.1 */ public function limitEnumerationToPhone(): bool; @@ -453,7 +407,6 @@ public function limitEnumerationToPhone(): bool; * Check if user enumeration is allowed to return also on full match * and ignore limitations to phonebook or groups. * - * @return bool * @since 21.0.1 */ public function allowEnumerationFullMatch(): bool; @@ -462,7 +415,6 @@ public function allowEnumerationFullMatch(): bool; * When `allowEnumerationFullMatch` is enabled and `matchEmail` is set, * then also return results for full email matches. * - * @return bool * @since 25.0.0 */ public function matchEmail(): bool; @@ -471,7 +423,6 @@ public function matchEmail(): bool; * When `allowEnumerationFullMatch` is enabled and `matchUserId` is set, * then also return results for full user id matches. * - * @return bool * @since 33.0.0 */ public function matchUserId(): bool; @@ -480,7 +431,6 @@ public function matchUserId(): bool; * When `allowEnumerationFullMatch` is enabled and `ignoreSecondDisplayName` is set, * then the search should ignore matches on the second displayname and only use the first. * - * @return bool * @since 25.0.0 */ public function ignoreSecondDisplayName(): bool; @@ -504,9 +454,6 @@ public function allowViewWithoutDownload(): bool; /** * Check if the current user can enumerate the target user * - * @param IUser|null $currentUser - * @param IUser $targetUser - * @return bool * @since 23.0.0 */ public function currentUserCanEnumerateTargetUser(?IUser $currentUser, IUser $targetUser): bool; @@ -520,26 +467,23 @@ public function sharingDisabledForUser(?string $userId): bool; /** * Check if outgoing server2server shares are allowed - * @return bool * @since 9.0.0 */ - public function outgoingServer2ServerSharesAllowed(); + public function outgoingServer2ServerSharesAllowed(): bool; /** * Check if outgoing server2server shares are allowed - * @return bool * @since 14.0.0 */ - public function outgoingServer2ServerGroupSharesAllowed(); + public function outgoingServer2ServerGroupSharesAllowed(): bool; /** * Check if a given share provider exists - * @param int $shareType - * @return bool + * @param IShare::TYPE_* $shareType * @since 11.0.0 */ - public function shareProviderExists($shareType); + public function shareProviderExists(int $shareType): bool; /** * @param string $shareProviderClass diff --git a/lib/public/Share/IShareProvider.php b/lib/public/Share/IShareProvider.php index 23187ca833ef4..c76040fccc5a2 100644 --- a/lib/public/Share/IShareProvider.php +++ b/lib/public/Share/IShareProvider.php @@ -95,7 +95,7 @@ public function move(\OCP\Share\IShare $share, $recipient); * @param Folder $node * @param bool $reshares Also get the shares where $user is the owner instead of just the shares where $user is the initiator * @param bool $shallow Whether the method should stop at the first level, or look into sub-folders. - * @return \OCP\Share\IShare[][] + * @return array> * @since 11.0.0 */ public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = true); @@ -117,7 +117,7 @@ public function getSharesBy($userId, $shareType, $node, $reshares, $limit, $offs /** * Get share by id * - * @param int $id + * @param string $id * @param string|null $recipientId * @return \OCP\Share\IShare * @throws ShareNotFound @@ -155,7 +155,7 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset); * @throws ShareNotFound * @since 9.0.0 */ - public function getShareByToken($token); + public function getShareByToken(string $token); /** * A user is deleted from the system @@ -204,7 +204,7 @@ public function getAccessList($nodes, $currentAccess); * Get all the shares in this provider returned as iterable to reduce memory * overhead * - * @return iterable + * @return iterable * @since 18.0.0 */ public function getAllShares(): iterable; diff --git a/openapi.json b/openapi.json index 24f56c7828870..7aff72d29a187 100644 --- a/openapi.json +++ b/openapi.json @@ -2515,7 +2515,8 @@ ], "properties": { "accepted": { - "type": "boolean" + "type": "integer", + "format": "int64" }, "file_id": { "type": "integer", @@ -2523,8 +2524,7 @@ "nullable": true }, "id": { - "type": "integer", - "format": "int64" + "type": "string" }, "mimetype": { "type": "string", @@ -2545,8 +2545,7 @@ "type": "string" }, "parent": { - "type": "integer", - "format": "int64", + "type": "string", "nullable": true }, "permissions": { @@ -18049,8 +18048,7 @@ "description": "ID of the user that receives the share" }, "remoteId": { - "type": "integer", - "format": "int64", + "type": "string", "nullable": true, "default": null, "description": "ID of the remote" @@ -18170,8 +18168,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -18262,7 +18259,7 @@ "/ocs/v2.php/cloud/shares/{id}/permissions": { "post": { "operationId": "federatedfilesharing-request_handler-update-permissions", - "summary": "update share information to keep federated re-shares in sync", + "summary": "Update share information to keep federated re-shares in sync.", "tags": [ "federatedfilesharing/request_handler" ], @@ -18307,8 +18304,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -18423,8 +18419,7 @@ "description": "ID of the remote share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -18521,8 +18516,7 @@ "description": "ID of the remote share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -18609,8 +18603,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -18697,8 +18690,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -25527,7 +25519,7 @@ "/ocs/v2.php/apps/files_sharing/api/v1/remote_shares/pending/{id}": { "post": { "operationId": "files_sharing-remote-accept-share", - "summary": "Accept a remote share", + "summary": "Accept a remote share.", "tags": [ "files_sharing/remote" ], @@ -25546,8 +25538,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -25650,7 +25641,7 @@ }, "delete": { "operationId": "files_sharing-remote-decline-share", - "summary": "Decline a remote share", + "summary": "Decline a remote share.", "tags": [ "files_sharing/remote" ], @@ -25669,8 +25660,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -25794,8 +25784,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -25919,8 +25908,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 9801f05d6108e..ad60cb535558c 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -1121,4 +1121,102 @@ public function testGetOrCreateFolder(int $case): void { $result = $node->getOrCreateFolder($folderName); $this->assertEquals($child, $result); } + + private function callBuildNotExistingFileNameForView(string $path, string $name, View&MockObject $view): string { + $rootFolder = $this->createMock(IRootFolder::class); + $folder = new Folder($rootFolder, $view, $path); + return $path . (str_ends_with('/', $path) ? '' : '/') . $folder->getNonExistingName($name); + } + + public function testBuildNotExistingFileNameForView(): void { + $viewMock = $this->createMock(View::class); + $this->assertEquals('/filename', $this->callBuildNotExistingFileNameForView('/', 'filename', $viewMock)); + $this->assertEquals('dir/filename.ext', $this->callBuildNotExistingFileNameForView('dir', 'filename.ext', $viewMock)); + + $viewMock = $this->createMock(View::class); + $viewMock->expects($this->exactly(2)) + ->method('file_exists') + ->willReturnMap([ + // Conflict on filename.ext + ['dir/filename.ext', true], + ['dir/filename (2).ext', false], + ]); + $this->assertEquals('dir/filename (2).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename.ext', $viewMock)); + + $viewMock = $this->createMock(View::class); + $viewMock->expects($this->exactly(3)) + ->method('file_exists') + ->willReturnMap([ + // Conflict on filename.ext + ['dir/filename.ext', true], + ['dir/filename (2).ext', true], + ['dir/filename (3).ext', false], + ]); + $this->assertEquals('dir/filename (3).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename.ext', $viewMock)); + + $viewMock = $this->createMock(View::class); + $viewMock->expects($this->exactly(2)) + ->method('file_exists') + ->willReturnMap([ + ['dir/filename (1).ext', true], + ['dir/filename (2).ext', false], + ]); + $this->assertEquals('dir/filename (2).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename (1).ext', $viewMock)); + + $viewMock = $this->createMock(View::class); + $viewMock->expects($this->exactly(2)) + ->method('file_exists') + ->willReturnMap([ + ['dir/filename (2).ext', true], + ['dir/filename (3).ext', false], + ]); + $this->assertEquals('dir/filename (3).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename (2).ext', $viewMock)); + + $viewMock = $this->createMock(View::class); + $viewMock->expects($this->exactly(3)) + ->method('file_exists') + ->willReturnMap([ + ['dir/filename (2).ext', true], + ['dir/filename (3).ext', true], + ['dir/filename (4).ext', false], + ]); + $this->assertEquals('dir/filename (4).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename (2).ext', $viewMock)); + + $viewMock = $this->createMock(View::class); + $viewMock->expects($this->exactly(2)) + ->method('file_exists') + ->willReturnMap([ + ['dir/filename(1).ext', true], + ['dir/filename(2).ext', false], + ]); + $this->assertEquals('dir/filename(2).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename(1).ext', $viewMock)); + + $viewMock = $this->createMock(View::class); + $viewMock->expects($this->exactly(2)) + ->method('file_exists') + ->willReturnMap([ + ['dir/filename(1) (1).ext', true], + ['dir/filename(1) (2).ext', false], + ]); + $this->assertEquals('dir/filename(1) (2).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename(1) (1).ext', $viewMock)); + + $viewMock = $this->createMock(View::class); + $viewMock->expects($this->exactly(3)) + ->method('file_exists') + ->willReturnMap([ + ['dir/filename(1) (1).ext', true], + ['dir/filename(1) (2).ext', true], + ['dir/filename(1) (3).ext', false], + ]); + $this->assertEquals('dir/filename(1) (3).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename(1) (1).ext', $viewMock)); + + $viewMock = $this->createMock(View::class); + $viewMock->expects($this->exactly(2)) + ->method('file_exists') + ->willReturnMap([ + ['dir/filename(1) (2) (3).ext', true], + ['dir/filename(1) (2) (4).ext', false], + ]); + $this->assertEquals('dir/filename(1) (2) (4).ext', $this->callBuildNotExistingFileNameForView('dir', 'filename(1) (2) (3).ext', $viewMock)); + } } diff --git a/tests/lib/LegacyHelperTest.php b/tests/lib/LegacyHelperTest.php deleted file mode 100644 index 0123cab2e39cd..0000000000000 --- a/tests/lib/LegacyHelperTest.php +++ /dev/null @@ -1,106 +0,0 @@ -createMock(View::class); - $this->assertEquals('/filename', OC_Helper::buildNotExistingFileNameForView('/', 'filename', $viewMock)); - $this->assertEquals('dir/filename.ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename.ext', $viewMock)); - - $viewMock = $this->createMock(View::class); - $viewMock->expects($this->exactly(2)) - ->method('file_exists') - ->willReturnMap([ - // Conflict on filename.ext - ['dir/filename.ext', true], - ['dir/filename (2).ext', false], - ]); - $this->assertEquals('dir/filename (2).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename.ext', $viewMock)); - - $viewMock = $this->createMock(View::class); - $viewMock->expects($this->exactly(3)) - ->method('file_exists') - ->willReturnMap([ - // Conflict on filename.ext - ['dir/filename.ext', true], - ['dir/filename (2).ext', true], - ['dir/filename (3).ext', false], - ]); - $this->assertEquals('dir/filename (3).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename.ext', $viewMock)); - - $viewMock = $this->createMock(View::class); - $viewMock->expects($this->exactly(2)) - ->method('file_exists') - ->willReturnMap([ - ['dir/filename (1).ext', true], - ['dir/filename (2).ext', false], - ]); - $this->assertEquals('dir/filename (2).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename (1).ext', $viewMock)); - - $viewMock = $this->createMock(View::class); - $viewMock->expects($this->exactly(2)) - ->method('file_exists') - ->willReturnMap([ - ['dir/filename (2).ext', true], - ['dir/filename (3).ext', false], - ]); - $this->assertEquals('dir/filename (3).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename (2).ext', $viewMock)); - - $viewMock = $this->createMock(View::class); - $viewMock->expects($this->exactly(3)) - ->method('file_exists') - ->willReturnMap([ - ['dir/filename (2).ext', true], - ['dir/filename (3).ext', true], - ['dir/filename (4).ext', false], - ]); - $this->assertEquals('dir/filename (4).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename (2).ext', $viewMock)); - - $viewMock = $this->createMock(View::class); - $viewMock->expects($this->exactly(2)) - ->method('file_exists') - ->willReturnMap([ - ['dir/filename(1).ext', true], - ['dir/filename(2).ext', false], - ]); - $this->assertEquals('dir/filename(2).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename(1).ext', $viewMock)); - - $viewMock = $this->createMock(View::class); - $viewMock->expects($this->exactly(2)) - ->method('file_exists') - ->willReturnMap([ - ['dir/filename(1) (1).ext', true], - ['dir/filename(1) (2).ext', false], - ]); - $this->assertEquals('dir/filename(1) (2).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename(1) (1).ext', $viewMock)); - - $viewMock = $this->createMock(View::class); - $viewMock->expects($this->exactly(3)) - ->method('file_exists') - ->willReturnMap([ - ['dir/filename(1) (1).ext', true], - ['dir/filename(1) (2).ext', true], - ['dir/filename(1) (3).ext', false], - ]); - $this->assertEquals('dir/filename(1) (3).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename(1) (1).ext', $viewMock)); - - $viewMock = $this->createMock(View::class); - $viewMock->expects($this->exactly(2)) - ->method('file_exists') - ->willReturnMap([ - ['dir/filename(1) (2) (3).ext', true], - ['dir/filename(1) (2) (4).ext', false], - ]); - $this->assertEquals('dir/filename(1) (2) (4).ext', OC_Helper::buildNotExistingFileNameForView('dir', 'filename(1) (2) (3).ext', $viewMock)); - } -} diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index a7b93d35821b4..df950b699cd0b 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -36,12 +36,10 @@ use OCP\IGroupManager; use OCP\IL10N; use OCP\IServerContainer; -use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; use OCP\L10N\IFactory; -use OCP\Mail\IMailer; use OCP\Security\Events\ValidatePasswordPolicyEvent; use OCP\Security\IHasher; use OCP\Security\ISecureRandom; @@ -59,6 +57,7 @@ use OCP\Share\IShareProvider; use OCP\Share\IShareProviderSupportsAllSharesInFolder; use OCP\Util; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockBuilder; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -77,51 +76,26 @@ public function listener() { */ #[\PHPUnit\Framework\Attributes\Group('DB')] class ManagerTest extends \Test\TestCase { - /** @var Manager */ - protected $manager; - /** @var LoggerInterface|MockObject */ - protected $logger; - /** @var IConfig|MockObject */ - protected $config; - /** @var ISecureRandom|MockObject */ - protected $secureRandom; - /** @var IHasher|MockObject */ - protected $hasher; - /** @var IShareProvider|MockObject */ - protected $defaultProvider; - /** @var IMountManager|MockObject */ - protected $mountManager; - /** @var IGroupManager|MockObject */ - protected $groupManager; - /** @var IL10N|MockObject */ - protected $l; - /** @var IFactory|MockObject */ - protected $l10nFactory; - /** @var DummyFactory */ - protected $factory; - /** @var IUserManager|MockObject */ - protected $userManager; - /** @var IRootFolder | MockObject */ - protected $rootFolder; - /** @var IEventDispatcher|MockObject */ - protected $dispatcher; - /** @var IMailer|MockObject */ - protected $mailer; - /** @var IURLGenerator|MockObject */ - protected $urlGenerator; - /** @var \OC_Defaults|MockObject */ - protected $defaults; - /** @var IUserSession|MockObject */ - protected $userSession; - /** @var KnownUserService|MockObject */ - protected $knownUserService; - /** @var ShareDisableChecker|MockObject */ - protected $shareDisabledChecker; + protected Manager $manager; + protected LoggerInterface&MockObject $logger; + protected IConfig&MockObject $config; + protected ISecureRandom&MockObject $secureRandom; + protected IHasher&MockObject $hasher; + protected IShareProvider&MockObject $defaultProvider; + protected IMountManager&MockObject $mountManager; + protected IGroupManager&MockObject $groupManager; + protected IL10N&MockObject $l; + protected IFactory&MockObject $l10nFactory; + protected DummyFactory $factory; + protected IUserManager&MockObject $userManager; + protected IRootFolder&MockObject $rootFolder; + protected IEventDispatcher&MockObject $dispatcher; + protected IUserSession&MockObject $userSession; + protected KnownUserService&MockObject $knownUserService; + protected ShareDisableChecker $shareDisabledChecker; private DateTimeZone $timezone; - /** @var IDateTimeZone|MockObject */ - protected $dateTimeZone; - /** @var IAppConfig|MockObject */ - protected $appConfig; + protected IDateTimeZone&MockObject $dateTimeZone; + protected IAppConfig&MockObject $appConfig; protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); @@ -132,9 +106,6 @@ protected function setUp(): void { $this->groupManager = $this->createMock(IGroupManager::class); $this->userManager = $this->createMock(IUserManager::class); $this->rootFolder = $this->createMock(IRootFolder::class); - $this->mailer = $this->createMock(IMailer::class); - $this->urlGenerator = $this->createMock(IURLGenerator::class); - $this->defaults = $this->createMock(\OC_Defaults::class); $this->dispatcher = $this->createMock(IEventDispatcher::class); $this->userSession = $this->createMock(IUserSession::class); $this->knownUserService = $this->createMock(KnownUserService::class); @@ -179,9 +150,6 @@ private function createManager(IProviderFactory $factory): Manager { $factory, $this->userManager, $this->rootFolder, - $this->mailer, - $this->urlGenerator, - $this->defaults, $this->dispatcher, $this->userSession, $this->knownUserService, @@ -192,9 +160,9 @@ private function createManager(IProviderFactory $factory): Manager { } /** - * @return MockBuilder + * @return MockBuilder */ - private function createManagerMock() { + private function createManagerMock(): MockBuilder { return $this->getMockBuilder(Manager::class) ->setConstructorArgs([ $this->logger, @@ -207,9 +175,6 @@ private function createManagerMock() { $this->factory, $this->userManager, $this->rootFolder, - $this->mailer, - $this->urlGenerator, - $this->defaults, $this->dispatcher, $this->userSession, $this->knownUserService, @@ -245,7 +210,7 @@ public static function dataTestDelete(): array { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('dataTestDelete')] + #[DataProvider('dataTestDelete')] public function testDelete($shareType, $sharedWith): void { $manager = $this->createManagerMock() ->onlyMethods(['getShareById', 'deleteChildren', 'promoteReshares']) @@ -518,7 +483,7 @@ public function testPromoteReshareFile(): void { }); $manager->method('generalCreateChecks')->willThrowException(new GenericShareException()); - $manager->expects($this->exactly(1))->method('updateShare')->with($reShare); + $manager->expects($this->exactly(1))->method('updateShare')->with($reShare)->willReturn($reShare); self::invokePrivate($manager, 'promoteReshares', [$share]); } @@ -574,9 +539,10 @@ public function testPromoteReshare(): void { ]; $manager->expects($this->exactly(2)) ->method('updateShare') - ->willReturnCallback(function ($share) use (&$calls): void { + ->willReturnCallback(function ($share) use (&$calls): IShare { $expected = array_shift($calls); $this->assertEquals($expected, $share); + return $expected; }); self::invokePrivate($manager, 'promoteReshares', [$share]); @@ -602,7 +568,7 @@ public function testPromoteReshareWhenUserHasAnotherShare(): void { $reShare->method('getNode')->willReturn($folder); $this->defaultProvider->method('getSharesBy')->willReturn([$reShare]); - $manager->method('generalCreateChecks')->willReturn(true); + $manager->method('generalCreateChecks'); /* No share is promoted because generalCreateChecks does not throw */ $manager->expects($this->never())->method('updateShare'); @@ -668,9 +634,10 @@ public function testPromoteReshareOfUsersInGroupShare(): void { ]; $manager->expects($this->exactly(2)) ->method('updateShare') - ->willReturnCallback(function ($share) use (&$calls): void { + ->willReturnCallback(function ($share) use (&$calls): IShare { $expected = array_shift($calls); $this->assertEquals($expected, $share); + return $expected; }); self::invokePrivate($manager, 'promoteReshares', [$share]); @@ -1056,7 +1023,7 @@ private function createNodeMock(string $class, array $methods, string $storageTy return $mock; } - #[\PHPUnit\Framework\Attributes\DataProvider('dataGeneralChecks')] + #[DataProvider('dataGeneralChecks')] public function testGeneralChecks(array $shareParams, ?string $exceptionMessage, bool $exception): void { if ($shareParams[2] !== null) { $shareParams[2] = $this->createNodeMock(...$shareParams[2]); @@ -1107,8 +1074,6 @@ public function testGeneralCheckShareRoot(): void { $this->expectException(\InvalidArgumentException::class); $this->expectExceptionMessage('You cannot share your root folder'); - $thrown = null; - $this->userManager->method('userExists')->willReturnMap([ ['user0', true], ['user1', true], @@ -1132,7 +1097,7 @@ public static function validateExpirationDateInternalProvider() { return [[IShare::TYPE_USER], [IShare::TYPE_REMOTE], [IShare::TYPE_REMOTE_GROUP]]; } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalInPast($shareType): void { $this->expectException(GenericShareException::class); $this->expectExceptionMessage('Expiration date is in the past'); @@ -1148,7 +1113,7 @@ public function testValidateExpirationDateInternalInPast($shareType): void { self::invokePrivate($this->manager, 'validateExpirationDateInternal', [$share]); } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalEnforceButNotSet($shareType): void { $this->expectException(\InvalidArgumentException::class); $this->expectExceptionMessage('Expiration date is enforced'); @@ -1173,7 +1138,7 @@ public function testValidateExpirationDateInternalEnforceButNotSet($shareType): self::invokePrivate($this->manager, 'validateExpirationDateInternal', [$share]); } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalEnforceButNotEnabledAndNotSet($shareType): void { $share = $this->manager->newShare(); $share->setProviderId('foo')->setId('bar'); @@ -1196,7 +1161,7 @@ public function testValidateExpirationDateInternalEnforceButNotEnabledAndNotSet( $this->assertNull($share->getExpirationDate()); } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalEnforceButNotSetNewShare($shareType): void { $share = $this->manager->newShare(); $share->setShareType($shareType); @@ -1229,7 +1194,7 @@ public function testValidateExpirationDateInternalEnforceButNotSetNewShare($shar $this->assertEquals($expected, $share->getExpirationDate()); } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalEnforceRelaxedDefaultButNotSetNewShare($shareType): void { $share = $this->manager->newShare(); $share->setShareType($shareType); @@ -1262,7 +1227,7 @@ public function testValidateExpirationDateInternalEnforceRelaxedDefaultButNotSet $this->assertEquals($expected, $share->getExpirationDate()); } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalEnforceTooFarIntoFuture($shareType): void { $this->expectException(GenericShareException::class); $this->expectExceptionMessage('Cannot set expiration date more than 3 days in the future'); @@ -1293,7 +1258,7 @@ public function testValidateExpirationDateInternalEnforceTooFarIntoFuture($share self::invokePrivate($this->manager, 'validateExpirationDateInternal', [$share]); } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalEnforceValid($shareType): void { $future = new \DateTime('now', $this->dateTimeZone->getTimeZone()); $future->add(new \DateInterval('P2D')); @@ -1333,7 +1298,7 @@ public function testValidateExpirationDateInternalEnforceValid($shareType): void $this->assertEquals($expected, $share->getExpirationDate()); } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalNoDefault($shareType): void { $date = new \DateTime('now', $this->dateTimeZone->getTimeZone()); $date->add(new \DateInterval('P5D')); @@ -1357,7 +1322,7 @@ public function testValidateExpirationDateInternalNoDefault($shareType): void { $this->assertEquals($expected, $share->getExpirationDate()); } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalNoDateNoDefault($shareType): void { $hookListener = $this->createMock(DummyShareManagerListener::class); Util::connectHook('\OC\Share', 'verifyExpirationDate', $hookListener, 'listener'); @@ -1374,7 +1339,7 @@ public function testValidateExpirationDateInternalNoDateNoDefault($shareType): v $this->assertNull($share->getExpirationDate()); } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalNoDateDefault($shareType): void { $share = $this->manager->newShare(); $share->setShareType($shareType); @@ -1411,7 +1376,7 @@ public function testValidateExpirationDateInternalNoDateDefault($shareType): voi $this->assertEquals($expected, $share->getExpirationDate()); } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalDefault($shareType): void { $future = new \DateTime('now', $this->timezone); $future->add(new \DateInterval('P5D')); @@ -1451,7 +1416,7 @@ public function testValidateExpirationDateInternalDefault($shareType): void { $this->assertEquals($expected, $share->getExpirationDate()); } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalHookModification($shareType): void { $nextWeek = new \DateTime('now', $this->timezone); $nextWeek->add(new \DateInterval('P7D')); @@ -1475,7 +1440,7 @@ public function testValidateExpirationDateInternalHookModification($shareType): $this->assertEquals($save, $share->getExpirationDate()); } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalHookException($shareType): void { $this->expectException(\Exception::class); $this->expectExceptionMessage('Invalid date!'); @@ -1498,7 +1463,7 @@ public function testValidateExpirationDateInternalHookException($shareType): voi self::invokePrivate($this->manager, 'validateExpirationDateInternal', [$share]); } - #[\PHPUnit\Framework\Attributes\DataProvider('validateExpirationDateInternalProvider')] + #[DataProvider('validateExpirationDateInternalProvider')] public function testValidateExpirationDateInternalExistingShareNoDefault($shareType): void { $share = $this->manager->newShare(); $share->setShareType($shareType); @@ -2426,7 +2391,7 @@ public static function dataIsSharingDisabledForUser() { * @param string[] $groupIds * @param bool $expected */ - #[\PHPUnit\Framework\Attributes\DataProvider('dataIsSharingDisabledForUser')] + #[DataProvider('dataIsSharingDisabledForUser')] public function testIsSharingDisabledForUser($excludeGroups, $groupList, $setList, $groupIds, $expected): void { $user = $this->createMock(IUser::class); @@ -2476,7 +2441,7 @@ public static function dataCanShare() { * @param string $sharingEnabled * @param bool $disabledForUser */ - #[\PHPUnit\Framework\Attributes\DataProvider('dataCanShare')] + #[DataProvider('dataCanShare')] public function testCanShare($expected, $sharingEnabled, $disabledForUser): void { $this->config->method('getAppValue') ->willReturnMap([ @@ -2530,8 +2495,7 @@ public function testCreateShareUser(): void { $manager->expects($this->once()) ->method('canShare') - ->with($share) - ->willReturn(true); + ->with($share); $manager->expects($this->once()) ->method('generalCreateChecks') ->with($share); @@ -2585,8 +2549,7 @@ public function testCreateShareGroup(): void { $manager->expects($this->once()) ->method('canShare') - ->with($share) - ->willReturn(true); + ->with($share); $manager->expects($this->once()) ->method('generalCreateChecks') ->with($share); @@ -2650,8 +2613,7 @@ public function testCreateShareLink(): void { $manager->expects($this->once()) ->method('canShare') - ->with($share) - ->willReturn(true); + ->with($share); $manager->expects($this->once()) ->method('generalCreateChecks') ->with($share); @@ -2756,8 +2718,7 @@ public function testCreateShareMail(): void { $manager->expects($this->once()) ->method('canShare') - ->with($share) - ->willReturn(true); + ->with($share); $manager->expects($this->once()) ->method('generalCreateChecks') ->with($share); @@ -2854,8 +2815,7 @@ public function testCreateShareHookError(): void { $manager->expects($this->once()) ->method('canShare') - ->with($share) - ->willReturn(true); + ->with($share); $manager->expects($this->once()) ->method('generalCreateChecks') ->with($share); @@ -2933,8 +2893,7 @@ public function testCreateShareOfIncomingFederatedShare(): void { $manager->expects($this->once()) ->method('canShare') - ->with($share) - ->willReturn(true); + ->with($share); $manager->expects($this->once()) ->method('generalCreateChecks') ->with($share); @@ -3510,7 +3469,7 @@ public function testUpdateShareCantChangeShareType(): void { $originalShare = $this->manager->newShare(); $originalShare->setShareType(IShare::TYPE_GROUP); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $share = $this->manager->newShare(); @@ -3539,7 +3498,7 @@ public function testUpdateShareCantChangeRecipientForGroupShare(): void { $originalShare->setShareType(IShare::TYPE_GROUP) ->setSharedWith('origGroup'); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $share = $this->manager->newShare(); @@ -3567,7 +3526,7 @@ public function testUpdateShareCantShareWithOwner(): void { $originalShare->setShareType(IShare::TYPE_USER) ->setSharedWith('sharedWith'); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $share = $this->manager->newShare(); @@ -3602,7 +3561,7 @@ public function testUpdateShareUser(): void { $node->method('getId')->willReturn(100); $node->method('getPath')->willReturn('/newUser/files/myPath'); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $share = $this->manager->newShare(); @@ -3662,7 +3621,7 @@ public function testUpdateShareGroup(): void { ->setSharedWith('origUser') ->setPermissions(31); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $node = $this->createMock(File::class); @@ -3728,7 +3687,7 @@ public function testUpdateShareLink(): void { ->setNode($file) ->setPermissions(15); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $manager->expects($this->once())->method('validateExpirationDateLink')->with($share); $manager->expects($this->once())->method('verifyPassword')->with('password'); @@ -3810,7 +3769,7 @@ public function testUpdateShareLinkEnableSendPasswordByTalkWithNoPassword(): voi ->setNode($file) ->setPermissions(15); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $manager->expects($this->once())->method('generalCreateChecks')->with($share); $manager->expects($this->once())->method('linkCreateChecks')->with($share); @@ -3875,7 +3834,7 @@ public function testUpdateShareMail(): void { ->setNode($file) ->setPermissions(Constants::PERMISSION_ALL); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $manager->expects($this->once())->method('generalCreateChecks')->with($share); $manager->expects($this->once())->method('verifyPassword')->with('password'); @@ -3958,7 +3917,7 @@ public function testUpdateShareMailEnableSendPasswordByTalk(): void { ->setNode($file) ->setPermissions(Constants::PERMISSION_ALL); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $manager->expects($this->once())->method('generalCreateChecks')->with($share); $manager->expects($this->once())->method('verifyPassword')->with('password'); @@ -4041,7 +4000,7 @@ public function testUpdateShareMailEnableSendPasswordByTalkWithDifferentPassword ->setNode($file) ->setPermissions(Constants::PERMISSION_ALL); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $manager->expects($this->once())->method('generalCreateChecks')->with($share); $manager->expects($this->once())->method('verifyPassword')->with('password'); @@ -4132,7 +4091,7 @@ public function testUpdateShareMailEnableSendPasswordByTalkWithNoPassword(): voi ->setNode($file) ->setPermissions(Constants::PERMISSION_ALL); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $manager->expects($this->once())->method('generalCreateChecks')->with($share); $manager->expects($this->never())->method('verifyPassword'); @@ -4205,7 +4164,7 @@ public function testUpdateShareMailEnableSendPasswordByTalkRemovingPassword(): v ->setNode($file) ->setPermissions(Constants::PERMISSION_ALL); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $manager->expects($this->once())->method('generalCreateChecks')->with($share); $manager->expects($this->once())->method('verifyPassword'); @@ -4278,7 +4237,7 @@ public function testUpdateShareMailEnableSendPasswordByTalkRemovingPasswordWithE ->setNode($file) ->setPermissions(Constants::PERMISSION_ALL); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $manager->expects($this->once())->method('generalCreateChecks')->with($share); $manager->expects($this->once())->method('verifyPassword'); @@ -4351,7 +4310,7 @@ public function testUpdateShareMailEnableSendPasswordByTalkWithPreviousPassword( ->setNode($file) ->setPermissions(Constants::PERMISSION_ALL); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $manager->expects($this->once())->method('generalCreateChecks')->with($share); $manager->expects($this->never())->method('verifyPassword'); @@ -4425,7 +4384,7 @@ public function testUpdateShareMailDisableSendPasswordByTalkWithPreviousPassword ->setNode($file) ->setPermissions(Constants::PERMISSION_ALL); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $manager->expects($this->once())->method('generalCreateChecks')->with($share); $manager->expects($this->never())->method('verifyPassword'); @@ -4499,7 +4458,7 @@ public function testUpdateShareMailDisableSendPasswordByTalkWithoutChangingPassw ->setNode($file) ->setPermissions(Constants::PERMISSION_ALL); - $manager->expects($this->once())->method('canShare')->willReturn(true); + $manager->expects($this->once())->method('canShare'); $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare); $manager->expects($this->once())->method('generalCreateChecks')->with($share); $manager->expects($this->never())->method('verifyPassword'); @@ -4538,9 +4497,7 @@ public function testMoveShareLink(): void { $share = $this->manager->newShare(); $share->setShareType(IShare::TYPE_LINK); - $recipient = $this->createMock(IUser::class); - - $this->manager->moveShare($share, $recipient); + $this->manager->moveShare($share, 'recipient'); } @@ -4628,7 +4585,7 @@ public function testMoveShareGroup(): void { $this->addToAssertionCount(1); } - #[\PHPUnit\Framework\Attributes\DataProvider('dataTestShareProviderExists')] + #[DataProvider('dataTestShareProviderExists')] public function testShareProviderExists($shareType, $expected): void { $factory = $this->getMockBuilder('OCP\Share\IProviderFactory')->getMock(); $factory->expects($this->any())->method('getProviderForType') @@ -5001,7 +4958,7 @@ public static function dataCurrentUserCanEnumerateTargetUser(): array { /** * @param bool $expected */ - #[\PHPUnit\Framework\Attributes\DataProvider('dataCurrentUserCanEnumerateTargetUser')] + #[DataProvider('dataCurrentUserCanEnumerateTargetUser')] public function testCurrentUserCanEnumerateTargetUser(bool $currentUserIsGuest, bool $allowEnumerationFullMatch, bool $allowEnumeration, bool $limitEnumerationToPhone, bool $limitEnumerationToGroups, bool $isKnownToUser, bool $haveCommonGroup, bool $expected): void { /** @var IManager|MockObject $manager */ $manager = $this->createManagerMock()