Merge pull request #21468 from owncloud/getmount-id-filter

Filter getStorage to make sure the user has access to it

7 files changed, 102 insertions, 4 deletions

diff --git a/apps/files_external/lib/storageconfig.php b/apps/files_external/lib/storageconfig.php
index 49a40a9a5d7..b369a45f407 100644
--- a/apps/files_external/lib/storageconfig.php
+++ b/apps/files_external/lib/storageconfig.php
@@ -29,6 +29,8 @@ use \OCA\Files_External\Lib\Auth\AuthMechanism;
  * External storage configuration
  */
 class StorageConfig implements \JsonSerializable {
+	const MOUNT_TYPE_ADMIN = 1;
+	const MOUNT_TYPE_PERSONAl = 2;
 
 	/**
 	 * Storage config id
@@ -108,6 +110,13 @@ class StorageConfig implements \JsonSerializable {
 	private $mountOptions = [];
 
 	/**
+	 * Whether it's a personal or admin mount
+	 *
+	 * @var int
+	 */
+	private $type;
+
+	/**
 	 * Creates a storage config
 	 *
 	 * @param int|null $id config id or null for a new config
@@ -350,6 +359,20 @@ class StorageConfig implements \JsonSerializable {
 	}
 
 	/**
+	 * @return int self::MOUNT_TYPE_ADMIN or self::MOUNT_TYPE_PERSONAl
+	 */
+	public function getType() {
+		return $this->type;
+	}
+
+	/**
+	 * @param int $type self::MOUNT_TYPE_ADMIN or self::MOUNT_TYPE_PERSONAl
+	 */
+	public function setType($type) {
+		$this->type = $type;
+	}
+
+	/**
 	 * Serialize config to JSON
 	 *
 	 * @return array
diff --git a/apps/files_external/service/globalstoragesservice.php b/apps/files_external/service/globalstoragesservice.php
index 2d25288e7bc..c524020e025 100644
--- a/apps/files_external/service/globalstoragesservice.php
+++ b/apps/files_external/service/globalstoragesservice.php
@@ -157,4 +157,8 @@ class GlobalStoragesService extends StoragesService {
 	public function getVisibilityType() {
 		return BackendService::VISIBILITY_ADMIN;
 	}
+
+	protected function isApplicable(StorageConfig $config) {
+		return true;
+	}
 }
diff --git a/apps/files_external/service/storagesservice.php b/apps/files_external/service/storagesservice.php
index dd28c415cea..26ff956d27e 100644
--- a/apps/files_external/service/storagesservice.php
+++ b/apps/files_external/service/storagesservice.php
@@ -23,12 +23,9 @@
 
 namespace OCA\Files_external\Service;
 
-use \OCP\IUserSession;
 use \OC\Files\Filesystem;
-
 use \OCA\Files_external\Lib\StorageConfig;
 use \OCA\Files_external\NotFoundException;
-use \OCA\Files_External\Service\BackendService;
 use \OCA\Files_External\Lib\Backend\Backend;
 use \OCA\Files_External\Lib\Auth\AuthMechanism;
 use \OCP\Files\StorageNotAvailableException;
@@ -85,6 +82,7 @@ abstract class StoragesService {
 				array_values($applicableGroups),
 				$mount['priority']
 			);
+			$config->setType($mount['type']);
 			$config->setId((int)$mount['mount_id']);
 			return $config;
 		} catch (\UnexpectedValueException $e) {
@@ -132,10 +130,23 @@ abstract class StoragesService {
 			throw new NotFoundException('Storage with id "' . $id . '" not found');
 		}
 
-		return $this->getStorageConfigFromDBMount($mount);
+		$config = $this->getStorageConfigFromDBMount($mount);
+		if ($this->isApplicable($config)) {
+			return $config;
+		} else {
+			throw new NotFoundException('Storage with id "' . $id . '" not found');
+		}
 	}
 
 	/**
+	 * Check whether this storage service should provide access to a storage
+	 *
+	 * @param StorageConfig $config
+	 * @return bool
+	 */
+	abstract protected function isApplicable(StorageConfig $config);
+
+	/**
 	 * Gets all storages, valid or not
 	 *
 	 * @return StorageConfig[] array of storage configs
diff --git a/apps/files_external/service/userglobalstoragesservice.php b/apps/files_external/service/userglobalstoragesservice.php
index e58815f8a79..50973883563 100644
--- a/apps/files_external/service/userglobalstoragesservice.php
+++ b/apps/files_external/service/userglobalstoragesservice.php
@@ -152,4 +152,22 @@ class UserGlobalStoragesService extends GlobalStoragesService {
 		return 0;
 	}
 
+	protected function isApplicable(StorageConfig $config) {
+		$applicableUsers = $config->getApplicableUsers();
+		$applicableGroups = $config->getApplicableGroups();
+
+		if (count($applicableUsers) === 0 && count($applicableGroups) === 0) {
+			return true;
+		}
+		if (in_array($this->getUser()->getUID(), $applicableUsers, true)) {
+			return true;
+		}
+		$groupIds = $this->groupManager->getUserGroupIds($this->getUser());
+		foreach ($groupIds as $groupId) {
+			if (in_array($groupId, $applicableGroups, true)) {
+				return true;
+			}
+		}
+		return false;
+	}
 }
diff --git a/apps/files_external/service/userstoragesservice.php b/apps/files_external/service/userstoragesservice.php
index 9b622f9b470..19981dd0137 100644
--- a/apps/files_external/service/userstoragesservice.php
+++ b/apps/files_external/service/userstoragesservice.php
@@ -130,4 +130,8 @@ class UserStoragesService extends StoragesService {
 	public function getVisibilityType() {
 		return BackendService::VISIBILITY_PERSONAL;
 	}
+
+	protected function isApplicable(StorageConfig $config) {
+		return ($config->getApplicableUsers() === [$this->getUser()->getUID()]) && $config->getType() === StorageConfig::MOUNT_TYPE_PERSONAl;
+	}
 }
diff --git a/apps/files_external/tests/service/userglobalstoragesservicetest.php b/apps/files_external/tests/service/userglobalstoragesservicetest.php
index 8057762cb28..d4c48b3f691 100644
--- a/apps/files_external/tests/service/userglobalstoragesservicetest.php
+++ b/apps/files_external/tests/service/userglobalstoragesservicetest.php
@@ -21,6 +21,7 @@
  */
 namespace OCA\Files_External\Tests\Service;
 
+use OCA\Files_external\NotFoundException;
 use OCA\Files_external\Service\StoragesService;
 use \OCA\Files_External\Service\UserGlobalStoragesService;
 use \OCP\IGroupManager;
@@ -140,6 +141,13 @@ class UserGlobalStoragesServiceTest extends GlobalStoragesServiceTest {
 			$this->assertEquals('/mountpoint', $retrievedStorage->getMountPoint());
 		} else {
 			$this->assertEquals(0, count($storages));
+
+			try {
+				$this->service->getStorage($newStorage->getId());
+				$this->fail('Failed asserting that storage can\'t be accessed by id');
+			} catch (NotFoundException $e) {
+
+			}
 		}
 
 	}
diff --git a/apps/files_external/tests/service/userstoragesservicetest.php b/apps/files_external/tests/service/userstoragesservicetest.php
index 6b6e6313e77..a2d3819427c 100644
--- a/apps/files_external/tests/service/userstoragesservicetest.php
+++ b/apps/files_external/tests/service/userstoragesservicetest.php
@@ -23,6 +23,8 @@ namespace OCA\Files_external\Tests\Service;
 
 use \OC\Files\Filesystem;
 
+use OCA\Files_external\Service\GlobalStoragesService;
+use OCA\Files_external\Service\StoragesService;
 use \OCA\Files_external\Service\UserStoragesService;
 use \OCA\Files_external\NotFoundException;
 use \OCA\Files_external\Lib\StorageConfig;
@@ -38,9 +40,16 @@ class UserStoragesServiceTest extends StoragesServiceTest {
 
 	private $userId;
 
+	/**
+	 * @var StoragesService
+	 */
+	protected $globalStoragesService;
+
 	public function setUp() {
 		parent::setUp();
 
+		$this->globalStoragesService = new GlobalStoragesService($this->backendService, $this->dbConfig);
+
 		$this->userId = $this->getUniqueID('user_');
 		$this->createUser($this->userId, $this->userId);
 		$this->user = \OC::$server->getUserManager()->get($this->userId);
@@ -174,4 +183,25 @@ class UserStoragesServiceTest extends StoragesServiceTest {
 			$this->userId
 		);
 	}
+
+	/**
+	 * @expectedException \OCA\Files_external\NotFoundException
+	 */
+	public function testGetAdminStorage() {
+		$backend = $this->backendService->getBackend('identifier:\OCA\Files_External\Lib\Backend\SMB');
+		$authMechanism = $this->backendService->getAuthMechanism('identifier:\Auth\Mechanism');
+
+		$storage = new StorageConfig();
+		$storage->setMountPoint('mountpoint');
+		$storage->setBackend($backend);
+		$storage->setAuthMechanism($authMechanism);
+		$storage->setBackendOptions(['password' => 'testPassword']);
+		$storage->setApplicableUsers([$this->userId]);
+
+		$newStorage = $this->globalStoragesService->addStorage($storage);
+
+		$this->assertInstanceOf('\OCA\Files_external\Lib\StorageConfig', $this->globalStoragesService->getStorage($newStorage->getId()));
+
+		$this->service->getStorage($newStorage->getId());
+	}
 }