summaryrefslogtreecommitdiffstats
path: root/apps
diff options
context:
space:
mode:
authorJoas Schilling <213943+nickvergessen@users.noreply.github.com>2018-04-24 11:57:52 +0200
committerGitHub <noreply@github.com>2018-04-24 11:57:52 +0200
commit09398397b35e8cf0d97da5fac6595c892f248af3 (patch)
tree93cfaa07060c9f0743699b2a9f45b08a3bc9c404 /apps
parent1ceb081c9be32fd0bb6167171f7437852ab17207 (diff)
parent4d5a2cce8df12537e3006e8e5976eb8dd24783a7 (diff)
downloadnextcloud-server-09398397b35e8cf0d97da5fac6595c892f248af3.tar.gz
nextcloud-server-09398397b35e8cf0d97da5fac6595c892f248af3.zip
Merge pull request #9282 from nextcloud/bugfix/9279/scrit_type_share_api
Make the ShareAPIController strict
Diffstat (limited to 'apps')
-rw-r--r--apps/files_sharing/lib/Controller/ShareAPIController.php81
-rw-r--r--apps/files_sharing/tests/Controller/ShareAPIControllerTest.php28
2 files changed, 55 insertions, 54 deletions
diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php
index 595e21825f3..30a8ded2a4a 100644
--- a/apps/files_sharing/lib/Controller/ShareAPIController.php
+++ b/apps/files_sharing/lib/Controller/ShareAPIController.php
@@ -1,4 +1,5 @@
<?php
+declare(strict_types=1);
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
@@ -36,6 +37,7 @@ use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\AppFramework\OCS\OCSNotFoundException;
use OCP\AppFramework\OCSController;
use OCP\Constants;
+use OCP\Files\Folder;
use OCP\Files\Node;
use OCP\Files\NotFoundException;
use OCP\IConfig;
@@ -51,6 +53,7 @@ use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\Exceptions\GenericShareException;
use OCP\Lock\ILockingProvider;
use OCP\Share\IShare;
+use OCA\Files_Sharing\External\Storage;
/**
* Class Share20OCS
@@ -65,8 +68,6 @@ class ShareAPIController extends OCSController {
private $groupManager;
/** @var IUserManager */
private $userManager;
- /** @var IRequest */
- protected $request;
/** @var IRootFolder */
private $rootFolder;
/** @var IURLGenerator */
@@ -95,14 +96,14 @@ class ShareAPIController extends OCSController {
* @param IConfig $config
*/
public function __construct(
- $appName,
+ string $appName,
IRequest $request,
IManager $shareManager,
IGroupManager $groupManager,
IUserManager $userManager,
IRootFolder $rootFolder,
IURLGenerator $urlGenerator,
- $userId,
+ string $userId,
IL10N $l10n,
IConfig $config
) {
@@ -127,7 +128,7 @@ class ShareAPIController extends OCSController {
* @return array
* @throws NotFoundException In case the node can't be resolved.
*/
- protected function formatShare(\OCP\Share\IShare $share, Node $recipientNode = null) {
+ protected function formatShare(\OCP\Share\IShare $share, Node $recipientNode = null): array {
$sharedBy = $this->userManager->get($share->getSharedBy());
$shareOwner = $this->userManager->get($share->getShareOwner());
@@ -233,7 +234,7 @@ class ShareAPIController extends OCSController {
* @param string $property
* @return string
*/
- private function getDisplayNameFromAddressBook($query, $property) {
+ private function getDisplayNameFromAddressBook(string $query, string $property): string {
// FIXME: If we inject the contacts manager it gets initialized bofore any address books are registered
$result = \OC::$server->getContactsManager()->search($query, [$property]);
foreach ($result as $r) {
@@ -256,7 +257,7 @@ class ShareAPIController extends OCSController {
* @return DataResponse
* @throws OCSNotFoundException
*/
- public function getShare($id) {
+ public function getShare(string $id): DataResponse {
try {
$share = $this->getShareById($id);
} catch (ShareNotFound $e) {
@@ -284,7 +285,7 @@ class ShareAPIController extends OCSController {
* @return DataResponse
* @throws OCSNotFoundException
*/
- public function deleteShare($id) {
+ public function deleteShare(string $id): DataResponse {
try {
$share = $this->getShareById($id);
} catch (ShareNotFound $e) {
@@ -332,14 +333,14 @@ class ShareAPIController extends OCSController {
* @suppress PhanUndeclaredClassMethod
*/
public function createShare(
- $path = null,
- $permissions = null,
- $shareType = -1,
- $shareWith = null,
- $publicUpload = 'false',
- $password = '',
- $expireDate = ''
- ) {
+ string $path = null,
+ int $permissions = null,
+ int $shareType = -1,
+ string $shareWith = null,
+ string $publicUpload = 'false',
+ string $password = '',
+ string $expireDate = ''
+ ): DataResponse {
$share = $this->shareManager->newShare();
if ($permissions === null) {
@@ -384,7 +385,7 @@ class ShareAPIController extends OCSController {
* We check the permissions via webdav. But the permissions of the mount point
* do not equal the share permissions. Here we fix that for federated mounts.
*/
- if ($path->getStorage()->instanceOfStorage('OCA\Files_Sharing\External\Storage')) {
+ if ($path->getStorage()->instanceOfStorage(Storage::class)) {
$permissions &= ~($permissions & ~$path->getPermissions());
}
@@ -510,7 +511,7 @@ class ShareAPIController extends OCSController {
* @param boolean $includeTags
* @return DataResponse
*/
- private function getSharedWithMe($node = null, $includeTags) {
+ private function getSharedWithMe($node = null, bool $includeTags): DataResponse {
$userShares = $this->shareManager->getSharedWith($this->currentUser, \OCP\Share::SHARE_TYPE_USER, $node, -1, 0);
$groupShares = $this->shareManager->getSharedWith($this->currentUser, \OCP\Share::SHARE_TYPE_GROUP, $node, -1, 0);
@@ -545,7 +546,7 @@ class ShareAPIController extends OCSController {
* @return DataResponse
* @throws OCSBadRequestException
*/
- private function getSharesInDir($folder) {
+ private function getSharesInDir(Node $folder): DataResponse {
if (!($folder instanceof \OCP\Files\Folder)) {
throw new OCSBadRequestException($this->l->t('Not a directory'));
}
@@ -597,12 +598,12 @@ class ShareAPIController extends OCSController {
* @throws OCSNotFoundException
*/
public function getShares(
- $shared_with_me = 'false',
- $reshares = 'false',
- $subfiles = 'false',
- $path = null,
- $include_tags = 'false'
- ) {
+ string $shared_with_me = 'false',
+ string $reshares = 'false',
+ string $subfiles = 'false',
+ string $path = null,
+ string $include_tags = 'false'
+ ): DataResponse {
if ($path !== null) {
$userFolder = $this->rootFolder->getUserFolder($this->currentUser);
@@ -616,6 +617,8 @@ class ShareAPIController extends OCSController {
}
}
+ $include_tags = $include_tags === 'true';
+
if ($shared_with_me === 'true') {
$result = $this->getSharedWithMe($path, $include_tags);
return $result;
@@ -673,7 +676,7 @@ class ShareAPIController extends OCSController {
/**
* @NoAdminRequired
*
- * @param int $id
+ * @param string $id
* @param int $permissions
* @param string $password
* @param string $publicUpload
@@ -684,12 +687,12 @@ class ShareAPIController extends OCSController {
* @throws OCSForbiddenException
*/
public function updateShare(
- $id,
- $permissions = null,
- $password = null,
- $publicUpload = null,
- $expireDate = null
- ) {
+ string $id,
+ int $permissions = null,
+ string $password = null,
+ string $publicUpload = null,
+ string $expireDate = null
+ ): DataResponse {
try {
$share = $this->getShareById($id);
} catch (ShareNotFound $e) {
@@ -730,7 +733,7 @@ class ShareAPIController extends OCSController {
Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE, // correct
Constants::PERMISSION_CREATE, // hidden file list
Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE, // allow to edit single files
- ])
+ ], true)
) {
throw new OCSBadRequestException($this->l->t('Can\'t change permissions for public share links'));
}
@@ -830,11 +833,7 @@ class ShareAPIController extends OCSController {
return new DataResponse($this->formatShare($share));
}
- /**
- * @param \OCP\Share\IShare $share
- * @return bool
- */
- protected function canAccessShare(\OCP\Share\IShare $share, $checkGroups = true) {
+ protected function canAccessShare(\OCP\Share\IShare $share, bool $checkGroups = true): bool {
// A file with permissions 0 can't be accessed by us. So Don't show it
if ($share->getPermissions() === 0) {
return false;
@@ -880,7 +879,7 @@ class ShareAPIController extends OCSController {
* @throws \Exception
* @return \DateTime
*/
- private function parseDate($expireDate) {
+ private function parseDate(string $expireDate): \DateTime {
try {
$date = new \DateTime($expireDate);
} catch (\Exception $e) {
@@ -904,7 +903,7 @@ class ShareAPIController extends OCSController {
* @return \OCP\Share\IShare
* @throws ShareNotFound
*/
- private function getShareById($id) {
+ private function getShareById(string $id): IShare {
$share = null;
// First check if it is an internal share.
@@ -946,6 +945,7 @@ class ShareAPIController extends OCSController {
* Lock a Node
*
* @param \OCP\Files\Node $node
+ * @throws LockedException
*/
private function lock(\OCP\Files\Node $node) {
$node->lock(ILockingProvider::LOCK_SHARED);
@@ -954,6 +954,7 @@ class ShareAPIController extends OCSController {
/**
* Cleanup the remaining locks
+ * @throws @LockedException
*/
public function cleanup() {
if ($this->lockedNode !== null) {
diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
index 4d944bfd58f..89a21d7d1e4 100644
--- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
+++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
@@ -756,7 +756,7 @@ class ShareAPIControllerTest extends TestCase {
}))
->will($this->returnArgument(0));
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_USER, 'validUser');
$this->assertInstanceOf(get_class($expected), $result);
@@ -863,7 +863,7 @@ class ShareAPIControllerTest extends TestCase {
}))
->will($this->returnArgument(0));
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_GROUP, 'validGroup');
$this->assertInstanceOf(get_class($expected), $result);
@@ -998,7 +998,7 @@ class ShareAPIControllerTest extends TestCase {
})
)->will($this->returnArgument(0));
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'true', '', '');
$this->assertInstanceOf(get_class($expected), $result);
@@ -1032,7 +1032,7 @@ class ShareAPIControllerTest extends TestCase {
})
)->will($this->returnArgument(0));
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', 'password', '');
$this->assertInstanceOf(get_class($expected), $result);
@@ -1079,7 +1079,7 @@ class ShareAPIControllerTest extends TestCase {
})
)->will($this->returnArgument(0));
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, \OCP\Share::SHARE_TYPE_LINK, null, 'false', '', '2000-01-01');
$this->assertInstanceOf(get_class($expected), $result);
@@ -1254,7 +1254,7 @@ class ShareAPIControllerTest extends TestCase {
$this->shareManager->method('getSharedWith')
->willReturn([]);
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->updateShare(42, null, '', 'false', '');
$this->assertInstanceOf(get_class($expected), $result);
@@ -1289,7 +1289,7 @@ class ShareAPIControllerTest extends TestCase {
$this->shareManager->method('getSharedWith')
->willReturn([]);
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->updateShare(42, null, 'password', 'true', '2000-01-01');
$this->assertInstanceOf(get_class($expected), $result);
@@ -1323,7 +1323,7 @@ class ShareAPIControllerTest extends TestCase {
})
)->will($this->returnArgument(0));
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->updateShare(42, $permissions, $password, $publicUpload, $expireDate);
$this->assertInstanceOf(get_class($expected), $result);
@@ -1440,7 +1440,7 @@ class ShareAPIControllerTest extends TestCase {
})
)->will($this->returnArgument(0));
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->updateShare(42, null, 'newpassword', null, null);
$this->assertInstanceOf(get_class($expected), $result);
@@ -1477,7 +1477,7 @@ class ShareAPIControllerTest extends TestCase {
})
)->will($this->returnArgument(0));
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->updateShare(42, null, null, null, '2010-12-23');
$this->assertInstanceOf(get_class($expected), $result);
@@ -1514,7 +1514,7 @@ class ShareAPIControllerTest extends TestCase {
$this->shareManager->method('getSharedWith')
->willReturn([]);
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->updateShare(42, null, null, 'true', null);
$this->assertInstanceOf(get_class($expected), $result);
@@ -1550,7 +1550,7 @@ class ShareAPIControllerTest extends TestCase {
$this->shareManager->method('getSharedWith')->willReturn([]);
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->updateShare(42, 7, null, null, null);
$this->assertInstanceOf(get_class($expected), $result);
@@ -1586,7 +1586,7 @@ class ShareAPIControllerTest extends TestCase {
$this->shareManager->method('getSharedWith')->willReturn([]);
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->updateShare(42, 31, null, null, null);
$this->assertInstanceOf(get_class($expected), $result);
@@ -1615,7 +1615,7 @@ class ShareAPIControllerTest extends TestCase {
$this->shareManager->method('getSharedWith')->willReturn([]);
- $expected = new DataResponse(null);
+ $expected = new DataResponse([]);
$result = $ocs->updateShare(42, 31, null, null, null);
$this->assertInstanceOf(get_class($expected), $result);