Validate permissions for created admin storages, auth mechanism

Backend and auth mechanism permissions are checked on storage creation,
both for personal storages and for admin storages

5 files changed, 63 insertions, 34 deletions

diff --git a/apps/files_external/controller/globalstoragescontroller.php b/apps/files_external/controller/globalstoragescontroller.php
index 756a34fc5d4..32408420039 100644
--- a/apps/files_external/controller/globalstoragescontroller.php
+++ b/apps/files_external/controller/globalstoragescontroller.php
@@ -32,6 +32,7 @@ use \OCP\AppFramework\Http;
 use \OCA\Files_external\Service\GlobalStoragesService;
 use \OCA\Files_external\NotFoundException;
 use \OCA\Files_external\Lib\StorageConfig;
+use \OCA\Files_External\Service\BackendService;
 
 /**
  * Global storages controller
@@ -178,4 +179,14 @@ class GlobalStoragesController extends StoragesController {
 
 	}
 
+	/**
+	 * Get the user type for this controller, used in validation
+	 *
+	 * @return string BackendService::USER_* constants
+	 */
+	protected function getUserType() {
+		return BackendService::USER_ADMIN;
+	}
+
+
 }
diff --git a/apps/files_external/controller/storagescontroller.php b/apps/files_external/controller/storagescontroller.php
index 613f22c0331..d99b8b5f2c5 100644
--- a/apps/files_external/controller/storagescontroller.php
+++ b/apps/files_external/controller/storagescontroller.php
@@ -36,6 +36,7 @@ use \OCA\Files_External\Lib\Backend\Backend;
 use \OCA\Files_External\Lib\Auth\AuthMechanism;
 use \OCP\Files\StorageNotAvailableException;
 use \OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException;
+use \OCA\Files_External\Service\BackendService;
 
 /**
  * Base class for storages controllers
@@ -157,12 +158,36 @@ abstract class StoragesController extends Controller {
 			return new DataResponse(
 				array(
 					'message' => (string)$this->l10n->t('Invalid storage backend "%s"', [
-						$storage->getBackend()->getIdentifier()
+						$backend->getIdentifier()
 					])
 				),
 				Http::STATUS_UNPROCESSABLE_ENTITY
 			);
 		}
+
+		if (!$backend->isPermitted($this->getUserType(), BackendService::PERMISSION_CREATE)) {
+			// not permitted to use backend
+			return new DataResponse(
+				array(
+					'message' => (string)$this->l10n->t('Not permitted to use backend "%s"', [
+						$backend->getIdentifier()
+					])
+				),
+				Http::STATUS_UNPROCESSABLE_ENTITY
+			);
+		}
+		if (!$authMechanism->isPermitted($this->getUserType(), BackendService::PERMISSION_CREATE)) {
+			// not permitted to use auth mechanism
+			return new DataResponse(
+				array(
+					'message' => (string)$this->l10n->t('Not permitted to use authentication mechanism "%s"', [
+						$authMechanism->getIdentifier()
+					])
+				),
+				Http::STATUS_UNPROCESSABLE_ENTITY
+			);
+		}
+
 		if (!$backend->validateStorage($storage)) {
 			// unsatisfied parameters
 			return new DataResponse(
@@ -186,6 +211,13 @@ abstract class StoragesController extends Controller {
 	}
 
 	/**
+	 * Get the user type for this controller, used in validation
+	 *
+	 * @return string BackendService::USER_* constants
+	 */
+	abstract protected function getUserType();
+
+	/**
 	 * Check whether the given storage is available / valid.
 	 *
 	 * Note that this operation can be time consuming depending
diff --git a/apps/files_external/controller/userstoragescontroller.php b/apps/files_external/controller/userstoragescontroller.php
index 9baac3a8031..585ff8eeb00 100644
--- a/apps/files_external/controller/userstoragescontroller.php
+++ b/apps/files_external/controller/userstoragescontroller.php
@@ -62,38 +62,6 @@ class UserStoragesController extends StoragesController {
 	}
 
 	/**
-	 * Validate storage config
-	 *
-	 * @param StorageConfig $storage storage config
-	 *
-	 * @return DataResponse|null returns response in case of validation error
-	 */
-	protected function validate(StorageConfig $storage) {
-		$result = parent::validate($storage);
-
-		if ($result !== null) {
-			return $result;
-		}
-
-		// Verify that the mount point applies for the current user
-		// Prevent non-admin users from mounting local storage and other disabled backends
-		/** @var Backend */
-		$backend = $storage->getBackend();
-		if (!$backend->isPermitted(BackendService::USER_PERSONAL, BackendService::PERMISSION_MOUNT)) {
-			return new DataResponse(
-				array(
-					'message' => (string)$this->l10n->t('Admin-only storage backend "%s"', [
-						$storage->getBackend()->getIdentifier()
-					])
-				),
-				Http::STATUS_UNPROCESSABLE_ENTITY
-			);
-		}
-
-		return null;
-	}
-
-	/**
 	 * Return storage
 	 *
 	 * @NoAdminRequired
@@ -218,4 +186,14 @@ class UserStoragesController extends StoragesController {
 	public function destroy($id) {
 		return parent::destroy($id);
 	}
+
+	/**
+	 * Get the user type for this controller, used in validation
+	 *
+	 * @return string BackendService::USER_* constants
+	 */
+	protected function getUserType() {
+		return BackendService::USER_PERSONAL;
+	}
+
 }
diff --git a/apps/files_external/tests/controller/storagescontrollertest.php b/apps/files_external/tests/controller/storagescontrollertest.php
index 5a2cff99244..c43761f3bcb 100644
--- a/apps/files_external/tests/controller/storagescontrollertest.php
+++ b/apps/files_external/tests/controller/storagescontrollertest.php
@@ -75,6 +75,8 @@ abstract class StoragesControllerTest extends \Test\TestCase {
 		$authMech = $this->getAuthMechMock();
 		$authMech->method('validateStorage')
 			->willReturn(true);
+		$authMech->method('isPermitted')
+			->willReturn(true);
 		$backend = $this->getBackendMock();
 		$backend->method('validateStorage')
 			->willReturn(true);
@@ -114,6 +116,8 @@ abstract class StoragesControllerTest extends \Test\TestCase {
 		$authMech = $this->getAuthMechMock();
 		$authMech->method('validateStorage')
 			->willReturn(true);
+		$authMech->method('isPermitted')
+			->willReturn(true);
 		$backend = $this->getBackendMock();
 		$backend->method('validateStorage')
 			->willReturn(true);
@@ -245,6 +249,8 @@ abstract class StoragesControllerTest extends \Test\TestCase {
 		$authMech = $this->getAuthMechMock();
 		$authMech->method('validateStorage')
 			->willReturn(true);
+		$authMech->method('isPermitted')
+			->willReturn(true);
 		$backend = $this->getBackendMock();
 		$backend->method('validateStorage')
 			->willReturn(true);
@@ -338,6 +344,8 @@ abstract class StoragesControllerTest extends \Test\TestCase {
 		$authMech = $this->getAuthMechMock();
 		$authMech->method('validateStorage')
 			->will($this->returnValue($authMechValidate));
+		$authMech->method('isPermitted')
+			->willReturn(true);
 
 		$storageConfig = new StorageConfig();
 		$storageConfig->setMountPoint('mount');
diff --git a/apps/files_external/tests/controller/userstoragescontrollertest.php b/apps/files_external/tests/controller/userstoragescontrollertest.php
index b9668064e33..720e59cff93 100644
--- a/apps/files_external/tests/controller/userstoragescontrollertest.php
+++ b/apps/files_external/tests/controller/userstoragescontrollertest.php
@@ -51,7 +51,7 @@ class UserStoragesControllerTest extends StoragesControllerTest {
 	public function testAddOrUpdateStorageDisallowedBackend() {
 		$backend = $this->getBackendMock();
 		$backend->method('isPermitted')
-			->with(BackendService::USER_PERSONAL, BackendService::PERMISSION_MOUNT)
+			->with(BackendService::USER_PERSONAL, BackendService::PERMISSION_CREATE)
 			->willReturn(false);
 		$authMech = $this->getAuthMechMock();