Perform visibility checks on storages

StoragesService::getStorages() will check the visibility of the backend
and auth mechanism for the storage, and if either are not visible to the
user (aka disabled by admin) then the storage will be filtered out. The
original method StoragesService::getAllStorages() still exists in case
such storages need to be detected, but its use is discouraged.

apps/files_external/controller/globalstoragescontroller.php

@@ -179,14 +179,5 @@ class GlobalStoragesController extends StoragesController {
 
 	}
 
-	/**
-	 * Get the visibility type for this controller, used in validation
-	 *
-	 * @return string BackendService::VISIBILITY_* constants
-	 */
-	protected function getVisibilityType() {
-		return BackendService::VISIBILITY_ADMIN;
-	}
-
 
 }

apps/files_external/controller/storagescontroller.php

@@ -153,7 +153,7 @@ abstract class StoragesController extends Controller {
 		$backend = $storage->getBackend();
 		/** @var AuthMechanism */
 		$authMechanism = $storage->getAuthMechanism();
-		if (!$backend || $backend->checkDependencies()) {
+		if ($backend->checkDependencies()) {
 			// invalid backend
 			return new DataResponse(
 				array(
@@ -165,7 +165,7 @@ abstract class StoragesController extends Controller {
 			);
 		}
 
-		if (!$backend->isVisibleFor($this->getVisibilityType())) {
+		if (!$backend->isVisibleFor($this->service->getVisibilityType())) {
 			// not permitted to use backend
 			return new DataResponse(
 				array(
@@ -176,7 +176,7 @@ abstract class StoragesController extends Controller {
 				Http::STATUS_UNPROCESSABLE_ENTITY
 			);
 		}
-		if (!$authMechanism->isVisibleFor($this->getVisibilityType())) {
+		if (!$authMechanism->isVisibleFor($this->service->getVisibilityType())) {
 			// not permitted to use auth mechanism
 			return new DataResponse(
 				array(
@@ -210,13 +210,6 @@ abstract class StoragesController extends Controller {
 		return null;
 	}
 
-	/**
-	 * Get the visibility type for this controller, used in validation
-	 *
-	 * @return string BackendService::VISIBILITY_* constants
-	 */
-	abstract protected function getVisibilityType();
-
 	/**
 	 * Check whether the given storage is available / valid.
 	 *

apps/files_external/controller/userstoragescontroller.php

@@ -187,13 +187,4 @@ class UserStoragesController extends StoragesController {
 		return parent::destroy($id);
 	}
 
-	/**
-	 * Get the visibility type for this controller, used in validation
-	 *
-	 * @return string BackendService::VISIBILITY_* constants
-	 */
-	protected function getVisibilityType() {
-		return BackendService::VISIBILITY_PERSONAL;
-	}
-
 }

apps/files_external/lib/config.php

@@ -112,7 +112,7 @@ class OC_Mount_Config {
 	 * @param string $uid user
 	 * @return array of mount point string as key, mountpoint config as value
 	 *
-	 * @deprecated 8.2.0 use UserGlobalStoragesService::getAllStorages() and UserStoragesService::getAllStorages()
+	 * @deprecated 8.2.0 use UserGlobalStoragesService::getStorages() and UserStoragesService::getStorages()
 	 */
 	public static function getAbsoluteMountPoints($uid) {
 		$mountPoints = array();
@@ -124,7 +124,7 @@ class OC_Mount_Config {
 		$userGlobalStoragesService->setUser($user);
 		$userStoragesService->setUser($user);
 
-		foreach ($userGlobalStoragesService->getAllStorages() as $storage) {
+		foreach ($userGlobalStoragesService->getStorages() as $storage) {
 			$mountPoint = '/'.$uid.'/files'.$storage->getMountPoint();
 			$mountEntry = self::prepareMountPointEntry($storage, false);
 			foreach ($mountEntry['options'] as &$option) {
@@ -133,7 +133,7 @@ class OC_Mount_Config {
 			$mountPoints[$mountPoint] = $mountEntry;
 		}
 
-		foreach ($userStoragesService->getAllStorages() as $storage) {
+		foreach ($userStoragesService->getStorages() as $storage) {
 			$mountPoint = '/'.$uid.'/files'.$storage->getMountPoint();
 			$mountEntry = self::prepareMountPointEntry($storage, true);
 			foreach ($mountEntry['options'] as &$option) {
@@ -153,13 +153,13 @@ class OC_Mount_Config {
 	 *
 	 * @return array
 	 *
-	 * @deprecated 8.2.0 use GlobalStoragesService::getAllStorages()
+	 * @deprecated 8.2.0 use GlobalStoragesService::getStorages()
 	 */
 	public static function getSystemMountPoints() {
 		$mountPoints = [];
 		$service = self::$app->getContainer()->query('OCA\Files_External\Service\GlobalStoragesService');
 
-		foreach ($service->getAllStorages() as $storage) {
+		foreach ($service->getStorages() as $storage) {
 			$mountPoints[] = self::prepareMountPointEntry($storage, false);
 		}
 
@@ -171,13 +171,13 @@ class OC_Mount_Config {
 	 *
 	 * @return array
 	 *
-	 * @deprecated 8.2.0 use UserStoragesService::getAllStorages()
+	 * @deprecated 8.2.0 use UserStoragesService::getStorages()
 	 */
 	public static function getPersonalMountPoints() {
 		$mountPoints = [];
 		$service = self::$app->getContainer()->query('OCA\Files_External\Service\UserStoragesService');
 
-		foreach ($service->getAllStorages() as $storage) {
+		foreach ($service->getStorages() as $storage) {
 			$mountPoints[] = self::prepareMountPointEntry($storage, true);
 		}
 

apps/files_external/lib/config/configadapter.php

@@ -133,7 +133,7 @@ class ConfigAdapter implements IMountProvider {
 			$mounts[$storage->getMountPoint()] = $mount;
 		}
 
-		foreach ($this->userStoragesService->getAllStorages() as $storage) {
+		foreach ($this->userStoragesService->getStorages() as $storage) {
 			try {
 				$this->prepareStorageConfig($storage, $user);
 				$impl = $this->constructStorage($storage);

apps/files_external/personal.php

@@ -54,7 +54,7 @@ foreach ($authMechanisms as $authMechanism) {
 $tmpl = new OCP\Template('files_external', 'settings');
 $tmpl->assign('encryptionEnabled', \OC::$server->getEncryptionManager()->isEnabled());
 $tmpl->assign('isAdminPage', false);
-$tmpl->assign('storages', $userStoragesService->getAllStorages());
+$tmpl->assign('storages', $userStoragesService->getStorages());
 $tmpl->assign('dependencies', OC_Mount_Config::dependencyMessage($backendService->getBackends()));
 $tmpl->assign('backends', $backends);
 $tmpl->assign('authMechanisms', $authMechanisms);

apps/files_external/service/globalstoragesservice.php

@@ -210,4 +210,13 @@ class GlobalStoragesService extends StoragesService {
 			);
 		}
 	}
+
+	/**
+	 * Get the visibility type for this controller, used in validation
+	 *
+	 * @return string BackendService::VISIBILITY_* constants
+	 */
+	public function getVisibilityType() {
+		return BackendService::VISIBILITY_ADMIN;
+	}
 }

apps/files_external/service/storagesservice.php

@@ -29,6 +29,8 @@ 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;
 
 /**
  * Service class to manage external storages
@@ -331,7 +333,7 @@ abstract class StoragesService {
 	}
 
 	/**
-	 * Gets all storages
+	 * Gets all storages, valid or not
 	 *
 	 * @return array array of storage configs
 	 */
@@ -339,6 +341,47 @@ abstract class StoragesService {
 		return $this->readConfig();
 	}
 
+	/**
+	 * Gets all valid storages
+	 *
+	 * @return array
+	 */
+	public function getStorages() {
+		return array_filter($this->getAllStorages(), [$this, 'validateStorage']);
+	}
+
+	/**
+	 * Validate storage
+	 * FIXME: De-duplicate with StoragesController::validate()
+	 *
+	 * @param StorageConfig $storage
+	 * @return bool
+	 */
+	protected function validateStorage(StorageConfig $storage) {
+		/** @var Backend */
+		$backend = $storage->getBackend();
+		/** @var AuthMechanism */
+		$authMechanism = $storage->getAuthMechanism();
+
+		if (!$backend->isVisibleFor($this->getVisibilityType())) {
+			// not permitted to use backend
+			return false;
+		}
+		if (!$authMechanism->isVisibleFor($this->getVisibilityType())) {
+			// not permitted to use auth mechanism
+			return false;
+		}
+
+		return true;
+	}
+
+	/**
+	 * Get the visibility type for this controller, used in validation
+	 *
+	 * @return string BackendService::VISIBILITY_* constants
+	 */
+	abstract public function getVisibilityType();
+
 	/**
 	 * Add new storage to the configuration
 	 *

apps/files_external/service/userglobalstoragesservice.php

@@ -117,7 +117,7 @@ class UserGlobalStoragesService extends GlobalStoragesService {
 	 * @return StorageConfig[]
 	 */
 	public function getUniqueStorages() {
-		$storages = $this->getAllStorages();
+		$storages = $this->getStorages();
 
 		$storagesByMountpoint = [];
 		foreach ($storages as $storage) {

apps/files_external/service/userstoragesservice.php

@@ -171,4 +171,13 @@ class UserStoragesService extends StoragesService {
 			$this->triggerHooks($newStorage, Filesystem::signal_create_mount);
 		}
 	}
+
+	/**
+	 * Get the visibility type for this controller, used in validation
+	 *
+	 * @return string BackendService::VISIBILITY_* constants
+	 */
+	public function getVisibilityType() {
+		return BackendService::VISIBILITY_PERSONAL;
+	}
 }

apps/files_external/settings.php

@@ -65,7 +65,7 @@ $userBackends = array_filter($backendService->getAvailableBackends(), function($
 $tmpl = new OCP\Template('files_external', 'settings');
 $tmpl->assign('encryptionEnabled', \OC::$server->getEncryptionManager()->isEnabled());
 $tmpl->assign('isAdminPage', true);
-$tmpl->assign('storages', $globalStoragesService->getAllStorages());
+$tmpl->assign('storages', $globalStoragesService->getStorages());
 $tmpl->assign('backends', $backends);
 $tmpl->assign('authMechanisms', $authMechanisms);
 $tmpl->assign('userBackends', $userBackends);

apps/files_external/tests/controller/globalstoragescontrollertest.php

@@ -24,6 +24,7 @@ use \OCA\Files_external\Controller\GlobalStoragesController;
 use \OCA\Files_external\Service\GlobalStoragesService;
 use \OCP\AppFramework\Http;
 use \OCA\Files_external\NotFoundException;
+use \OCA\Files_External\Service\BackendService;
 
 class GlobalStoragesControllerTest extends StoragesControllerTest {
 	public function setUp() {
@@ -32,6 +33,9 @@ class GlobalStoragesControllerTest extends StoragesControllerTest {
 			->disableOriginalConstructor()
 			->getMock();
 
+		$this->service->method('getVisibilityType')
+			->willReturn(BackendService::VISIBILITY_ADMIN);
+
 		$this->controller = new GlobalStoragesController(
 			'files_external',
 			$this->getMock('\OCP\IRequest'),

apps/files_external/tests/controller/userstoragescontrollertest.php

@@ -40,6 +40,9 @@ class UserStoragesControllerTest extends StoragesControllerTest {
 			->disableOriginalConstructor()
 			->getMock();
 
+		$this->service->method('getVisibilityType')
+			->willReturn(BackendService::VISIBILITY_PERSONAL);
+
 		$this->controller = new UserStoragesController(
 			'files_external',
 			$this->getMock('\OCP\IRequest'),

apps/files_external/tests/service/userglobalstoragesservicetest.php

@@ -209,7 +209,11 @@ class UserGlobalStoragesServiceTest extends GlobalStoragesServiceTest {
 		$expectedPrecedence
 	) {
 		$backend = $this->backendService->getBackend('identifier:\OCA\Files_External\Lib\Backend\SMB');
+		$backend->method('isVisibleFor')
+			->willReturn(true);
 		$authMechanism = $this->backendService->getAuthMechanism('identifier:\Auth\Mechanism');
+		$authMechanism->method('isVisibleFor')
+			->willReturn(true);
 
 		$storage1 = new StorageConfig();
 		$storage1->setMountPoint('mountpoint');