diff --git a/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php b/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php index 7fdd718cbfe..2fc262a1e67 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\Federation\ICloudIdManager; 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 @@ class RequestHandlerController extends OCSController { 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 @@ class RequestHandlerController extends OCSController { * @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 @@ class RequestHandlerController extends OCSController { ?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 @@ class RequestHandlerController extends OCSController { /** * 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 @@ class RequestHandlerController extends OCSController { */ #[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 @@ class RequestHandlerController extends OCSController { /** * 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 @@ class RequestHandlerController extends OCSController { */ #[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 @@ class RequestHandlerController extends OCSController { /** * 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 @@ class RequestHandlerController extends OCSController { /** * 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 @@ class RequestHandlerController extends OCSController { 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 @@ class RequestHandlerController extends OCSController { /** * 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 @@ class RequestHandlerController extends OCSController { } /** - * 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 @@ class RequestHandlerController extends OCSController { * @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 e2708ddd4c7..ca5c0f98d32 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -27,6 +27,7 @@ use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IShare; use OCP\Share\IShareProvider; use OCP\Share\IShareProviderSupportsAllSharesInFolder; +use Override; use Psr\Log\LoggerInterface; /** @@ -140,7 +141,7 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl if ($remoteShare) { try { $ownerCloudId = $this->cloudIdManager->getCloudId($remoteShare['owner'], $remoteShare['remote']); - $shareId = $this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate); + $shareId = (string)$this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate); $share->setId($shareId); [$token, $remoteId] = $this->askOwnerToReShare($shareWith, $share, $shareId); // remote share was create successfully if we get a valid token as return @@ -168,16 +169,14 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl } /** - * 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( + $shareId = (string)$this->addShareToDB( $share->getNodeId(), $share->getNodeType(), $share->getSharedWith(), @@ -321,12 +320,9 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl } /** - * 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 +344,12 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl } /** - * 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 +362,9 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl /** - * 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 +373,9 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl } /** - * 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 +388,8 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl } /** - * 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 +499,9 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl } /** - * 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,13 +733,8 @@ class FederatedShareProvider implements IShareProvider, IShareProviderSupportsAl return $shares; } - /** - * Get a share by token - * - * @param string $token - * @throws ShareNotFound - */ - public function getShareByToken($token): IShare { + #[Override] + public function getShareByToken(string $token): IShare { $qb = $this->dbConnection->getQueryBuilder(); $cursor = $qb->select('*') diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index 6fb8a60a1e4..502021c67ee 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -618,7 +618,7 @@ class CloudFederationProviderFiles implements ISignedCloudFederationProvider { $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); diff --git a/apps/federatedfilesharing/openapi.json b/apps/federatedfilesharing/openapi.json index 411ff856b18..bd172c574cd 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 e293f440fe3..ca09587cbe8 100644 --- a/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php +++ b/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php @@ -8,10 +8,8 @@ declare(strict_types=1); */ 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\ICloudFederationShare; 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 @@ class RequestHandlerControllerTest extends \Test\TestCase { $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 @@ class RequestHandlerControllerTest extends \Test\TestCase { $this->request, $this->federatedShareProvider, $this->connection, - $this->shareManager, - $this->notifications, - $this->addressHandler, - $this->userManager, $this->cloudIdManager, $this->logger, $this->cloudFederationFactory, @@ -106,7 +90,7 @@ class RequestHandlerControllerTest extends \Test\TestCase { $this->user2, 'name', '', - 1, + '1', $this->ownerCloudId, $this->owner, $this->user1CloudId, @@ -125,13 +109,13 @@ class RequestHandlerControllerTest extends \Test\TestCase { $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 @@ class RequestHandlerControllerTest extends \Test\TestCase { public function testAcceptShare(): void { - $id = 42; + $id = '42'; $notification = [ 'sharedSecret' => 'token', diff --git a/apps/files_sharing/lib/Controller/ShareInfoController.php b/apps/files_sharing/lib/Controller/ShareInfoController.php index 27cef7bff52..b7e79aec830 100644 --- a/apps/files_sharing/lib/Controller/ShareInfoController.php +++ b/apps/files_sharing/lib/Controller/ShareInfoController.php @@ -64,7 +64,7 @@ class ShareInfoController extends ApiController { try { $share = $this->shareManager->getShareByToken($t); } catch (ShareNotFound $e) { - $response = new JSONResponse(["message" => "Not found " . $t . " "], Http::STATUS_NOT_FOUND); + $response = new JSONResponse([], Http::STATUS_NOT_FOUND); $response->throttle(['token' => $t]); return $response; } diff --git a/apps/files_sharing/lib/Controller/ShareesAPIController.php b/apps/files_sharing/lib/Controller/ShareesAPIController.php index 8d88af0c19f..6f2185d87e9 100644 --- a/apps/files_sharing/lib/Controller/ShareesAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareesAPIController.php @@ -39,14 +39,11 @@ use function usort; */ 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 index 94dbc0abbf7..0c5bff9f9ee 100644 --- a/apps/files_sharing/lib/External/ExternalShare.php +++ b/apps/files_sharing/lib/External/ExternalShare.php @@ -106,7 +106,7 @@ class ExternalShare extends Entity implements \JsonSerializable { 'owner' => $this->getOwner(), 'user' => $this->getUser(), 'mountpoint' => $this->getMountpoint(), - 'accepted' => $this->getAccepted(), + 'accepted' => $this->getAccepted() === IShare::STATUS_ACCEPTED, // Added later on 'file_id' => null, diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index c2b26ccbff0..22adac48d1e 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -38,9 +38,7 @@ use OCP\Snowflake\IGenerator; use Psr\Log\LoggerInterface; class Manager { - public const STORAGE = '\OCA\Files_Sharing\External\Storage'; - - private ?IUser $user = null; + private ?IUser $user; public function __construct( private IDBConnection $connection, @@ -410,7 +408,7 @@ class Manager { $mountPoint = '/' . $user->getUID() . '/files' . $data['mountpoint']; $data['mountpoint'] = $mountPoint; $data['certificateManager'] = $this->certificateManager; - return new Mount(self::STORAGE, $mountPoint, $data, $this, $this->storageLoader); + return new Mount(Storage::class, $mountPoint, $data, $this, $this->storageLoader); } protected function mountShare(array $data, ?IUser $user = null): Mount { diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index 509b23f68b4..e8de4bc63b5 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -192,7 +192,7 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, * @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 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, } /** - * 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/tests/CapabilitiesTest.php b/apps/files_sharing/tests/CapabilitiesTest.php index 914be0702c3..e45e676be21 100644 --- a/apps/files_sharing/tests/CapabilitiesTest.php +++ b/apps/files_sharing/tests/CapabilitiesTest.php @@ -90,9 +90,6 @@ class CapabilitiesTest extends \Test\TestCase { $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/build/psalm-baseline.xml b/build/psalm-baseline.xml index ebeea6d8467..470c0b22122 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1261,17 +1261,6 @@ )]]> - - - - - - - - - - - @@ -1282,13 +1271,6 @@ - - - - - - - diff --git a/lib/private/Activity/Event.php b/lib/private/Activity/Event.php index 5ca326fbdd6..6a2bc5dba77 100644 --- a/lib/private/Activity/Event.php +++ b/lib/private/Activity/Event.php @@ -325,6 +325,9 @@ class Event implements IEvent { 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; diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 2722ba59e62..010765af3c4 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -10,6 +10,7 @@ namespace OC\Share20; 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\IConfig; 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\IShareProvider; use OCP\Share\IShareProviderSupportsAccept; use OCP\Share\IShareProviderSupportsAllSharesInFolder; use OCP\Share\IShareProviderWithNotification; +use Override; use Psr\Log\LoggerInterface; /** @@ -77,9 +77,6 @@ class Manager implements IManager { 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 @@ class Manager implements IManager { /** * 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 @@ class Manager implements IManager { * * @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 @@ class Manager implements IManager { * @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 @@ class Manager implements IManager { * @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 @@ class Manager implements IManager { /** * 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 @@ class Manager implements IManager { /** * 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 @@ class Manager implements IManager { /** * 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 @@ class Manager implements IManager { * 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 @@ class Manager implements IManager { } } - /** - * @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 @@ class Manager implements IManager { /** * 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 @@ class Manager implements IManager { } } - /** - * 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 @@ class Manager implements IManager { 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 @@ class Manager implements IManager { 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 @@ class Manager implements IManager { * @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 @@ class Manager implements IManager { * 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 @@ class Manager implements IManager { } } - /** - * 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 @@ class Manager implements IManager { $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 @@ class Manager implements IManager { $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 @@ class Manager implements IManager { 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 @@ class Manager implements IManager { 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 @@ class Manager implements IManager { 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 @@ class Manager implements IManager { 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 @@ class Manager implements IManager { 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,28 +1318,8 @@ class Manager implements IManager { 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): IShare { + #[Override] + public function getShareByToken(string $token): IShare { // tokens cannot be valid local usernames if ($this->userManager->userExists($token)) { throw new ShareNotFound(); @@ -1507,14 +1420,8 @@ class Manager implements IManager { } } - /** - * 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) { @@ -1541,10 +1448,8 @@ class Manager implements IManager { 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) { @@ -1557,10 +1462,8 @@ class Manager implements IManager { } } - /** - * @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); @@ -1584,10 +1487,8 @@ class Manager implements IManager { $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); @@ -1599,7 +1500,7 @@ class Manager implements IManager { } #[\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) { @@ -1683,31 +1584,18 @@ class Manager implements IManager { 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; } @@ -1734,13 +1622,8 @@ class Manager implements IManager { 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); @@ -1755,117 +1638,66 @@ class Manager implements IManager { 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 []; } @@ -1873,53 +1705,59 @@ class Manager implements IManager { 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_user_id', '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; @@ -1956,31 +1794,23 @@ class Manager implements IManager { 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) { @@ -1994,6 +1824,7 @@ class Manager implements IManager { $this->factory->registerProvider($shareProviderClass); } + #[Override] public function getAllShares(): iterable { $providers = $this->factory->getAllProviders(); @@ -2002,9 +1833,10 @@ class Manager implements IManager { } } + #[Override] public function generateToken(): string { // Initial token length - $tokenLength = \OC\Share\Helper::getTokenLength(); + $tokenLength = Helper::getTokenLength(); do { $tokenExists = false; diff --git a/lib/public/Federation/ICloudFederationProvider.php b/lib/public/Federation/ICloudFederationProvider.php index b30041f81d6..b92404866cb 100644 --- a/lib/public/Federation/ICloudFederationProvider.php +++ b/lib/public/Federation/ICloudFederationProvider.php @@ -58,7 +58,7 @@ interface ICloudFederationProvider { * * @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/Share/IManager.php b/lib/public/Share/IManager.php index 6e6ddf2644a..40c11c90be0 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 @@ use OCP\Share\Exceptions\ShareTokenException; /** * 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 @@ interface IManager { * 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 @@ interface IManager { /** * 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 @@ interface IManager { * 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 @@ interface IManager { * 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 @@ interface IManager { * @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 @@ interface IManager { * @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 @@ interface IManager { * 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 @@ interface IManager { * 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 @@ interface IManager { * * 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 @@ interface IManager { * : 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 @@ interface IManager { * @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 @@ interface IManager { /** * Is default remote expire date enabled * - * @return bool * @since 22.0.0 */ public function shareApiRemoteDefaultExpireDate(): bool; @@ -365,7 +327,6 @@ interface IManager { /** * Is default expire date enforced * - * @return bool * @since 22.0.0 */ public function shareApiInternalDefaultExpireDateEnforced(): bool; @@ -373,7 +334,6 @@ interface IManager { /** * Is default expire date enforced for remote shares * - * @return bool * @since 22.0.0 */ public function shareApiRemoteDefaultExpireDateEnforced(): bool; @@ -381,7 +341,6 @@ interface IManager { /** * Number of default expire days * - * @return int * @since 22.0.0 */ public function shareApiInternalDefaultExpireDays(): int; @@ -389,7 +348,6 @@ interface IManager { /** * Number of default expire days for remote shares * - * @return int * @since 22.0.0 */ public function shareApiRemoteDefaultExpireDays(): int; @@ -397,17 +355,16 @@ interface IManager { /** * 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 @@ interface IManager { * @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 @@ interface IManager { /** * Check if user enumeration is limited to the users groups * - * @return bool * @since 19.0.0 */ public function limitEnumerationToGroups(): bool; @@ -444,7 +399,6 @@ interface IManager { /** * Check if user enumeration is limited to the phonebook matches * - * @return bool * @since 21.0.1 */ public function limitEnumerationToPhone(): bool; @@ -453,7 +407,6 @@ interface IManager { * 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 @@ interface IManager { * 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 @@ interface IManager { * 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 @@ interface IManager { * 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 @@ interface IManager { /** * 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 @@ interface IManager { /** * 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 23187ca833e..e4e12046052 100644 --- a/lib/public/Share/IShareProvider.php +++ b/lib/public/Share/IShareProvider.php @@ -95,7 +95,7 @@ interface IShareProvider { * @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); @@ -155,7 +155,7 @@ interface IShareProvider { * @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 @@ interface IShareProvider { * 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 1a4f3fd4be5..94b9dfd8569 100644 --- a/openapi.json +++ b/openapi.json @@ -18047,8 +18047,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" @@ -18168,8 +18167,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -18260,7 +18258,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" ], @@ -18305,8 +18303,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -18421,8 +18418,7 @@ "description": "ID of the remote share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -18519,8 +18515,7 @@ "description": "ID of the remote share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -18607,8 +18602,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -18695,8 +18689,7 @@ "description": "ID of the share", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index a7b93d35821..df950b699cd 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -36,12 +36,10 @@ use OCP\IGroup; 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\IShare; 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 @@ class DummyShareManagerListener { */ #[\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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $factory, $this->userManager, $this->rootFolder, - $this->mailer, - $this->urlGenerator, - $this->defaults, $this->dispatcher, $this->userSession, $this->knownUserService, @@ -192,9 +160,9 @@ class ManagerTest extends \Test\TestCase { } /** - * @return MockBuilder + * @return MockBuilder */ - private function createManagerMock() { + private function createManagerMock(): MockBuilder { return $this->getMockBuilder(Manager::class) ->setConstructorArgs([ $this->logger, @@ -207,9 +175,6 @@ class ManagerTest extends \Test\TestCase { $this->factory, $this->userManager, $this->rootFolder, - $this->mailer, - $this->urlGenerator, - $this->defaults, $this->dispatcher, $this->userSession, $this->knownUserService, @@ -245,7 +210,7 @@ class ManagerTest extends \Test\TestCase { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('dataTestDelete')] + #[DataProvider('dataTestDelete')] public function testDelete($shareType, $sharedWith): void { $manager = $this->createManagerMock() ->onlyMethods(['getShareById', 'deleteChildren', 'promoteReshares']) @@ -518,7 +483,7 @@ class ManagerTest extends \Test\TestCase { }); $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 @@ class ManagerTest extends \Test\TestCase { ]; $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { ]; $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 @@ class ManagerTest extends \Test\TestCase { 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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { 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 @@ class ManagerTest extends \Test\TestCase { 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 @@ class ManagerTest extends \Test\TestCase { 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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { 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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { 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 @@ class ManagerTest extends \Test\TestCase { * @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 @@ class ManagerTest extends \Test\TestCase { * @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 @@ class ManagerTest extends \Test\TestCase { $manager->expects($this->once()) ->method('canShare') - ->with($share) - ->willReturn(true); + ->with($share); $manager->expects($this->once()) ->method('generalCreateChecks') ->with($share); @@ -2585,8 +2549,7 @@ class ManagerTest extends \Test\TestCase { $manager->expects($this->once()) ->method('canShare') - ->with($share) - ->willReturn(true); + ->with($share); $manager->expects($this->once()) ->method('generalCreateChecks') ->with($share); @@ -2650,8 +2613,7 @@ class ManagerTest extends \Test\TestCase { $manager->expects($this->once()) ->method('canShare') - ->with($share) - ->willReturn(true); + ->with($share); $manager->expects($this->once()) ->method('generalCreateChecks') ->with($share); @@ -2756,8 +2718,7 @@ class ManagerTest extends \Test\TestCase { $manager->expects($this->once()) ->method('canShare') - ->with($share) - ->willReturn(true); + ->with($share); $manager->expects($this->once()) ->method('generalCreateChecks') ->with($share); @@ -2854,8 +2815,7 @@ class ManagerTest extends \Test\TestCase { $manager->expects($this->once()) ->method('canShare') - ->with($share) - ->willReturn(true); + ->with($share); $manager->expects($this->once()) ->method('generalCreateChecks') ->with($share); @@ -2933,8 +2893,7 @@ class ManagerTest extends \Test\TestCase { $manager->expects($this->once()) ->method('canShare') - ->with($share) - ->willReturn(true); + ->with($share); $manager->expects($this->once()) ->method('generalCreateChecks') ->with($share); @@ -3510,7 +3469,7 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { ->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 @@ class ManagerTest extends \Test\TestCase { ->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 @@ class ManagerTest extends \Test\TestCase { ->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 @@ class ManagerTest extends \Test\TestCase { ->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 @@ class ManagerTest extends \Test\TestCase { ->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 @@ class ManagerTest extends \Test\TestCase { ->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 @@ class ManagerTest extends \Test\TestCase { ->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 @@ class ManagerTest extends \Test\TestCase { ->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 @@ class ManagerTest extends \Test\TestCase { ->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 @@ class ManagerTest extends \Test\TestCase { ->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 @@ class ManagerTest extends \Test\TestCase { ->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 @@ class ManagerTest extends \Test\TestCase { ->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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { /** * @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()