Merge pull request #19915 from nextcloud/external-storage-password-placeholders

Use placeholder values for password fields in external storage webui

8 files changed, 65 insertions, 37 deletions

diff --git a/apps/files_external/lib/Controller/GlobalStoragesController.php b/apps/files_external/lib/Controller/GlobalStoragesController.php
index cf9eb718d5d..c8fc4b00ef8 100644
--- a/apps/files_external/lib/Controller/GlobalStoragesController.php
+++ b/apps/files_external/lib/Controller/GlobalStoragesController.php
@@ -113,7 +113,7 @@ class GlobalStoragesController extends StoragesController {
 		$this->updateStorageStatus($newStorage);
 
 		return new DataResponse(
-			$newStorage,
+			$this->formatStorageForUI($newStorage),
 			Http::STATUS_CREATED
 		);
 	}
@@ -180,7 +180,7 @@ class GlobalStoragesController extends StoragesController {
 		$this->updateStorageStatus($storage, $testOnly);
 
 		return new DataResponse(
-			$storage,
+			$this->formatStorageForUI($storage),
 			Http::STATUS_OK
 		);
 
diff --git a/apps/files_external/lib/Controller/StoragesController.php b/apps/files_external/lib/Controller/StoragesController.php
index 6b3bb5d6a53..8b1b0cb7a6c 100644
--- a/apps/files_external/lib/Controller/StoragesController.php
+++ b/apps/files_external/lib/Controller/StoragesController.php
@@ -31,6 +31,7 @@ namespace OCA\Files_External\Controller;
 
 use OCA\Files_External\Lib\Auth\AuthMechanism;
 use OCA\Files_External\Lib\Backend\Backend;
+use OCA\Files_External\Lib\DefinitionParameter;
 use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException;
 use OCA\Files_External\Lib\StorageConfig;
 use OCA\Files_External\NotFoundException;
@@ -146,9 +147,9 @@ abstract class StoragesController extends Controller {
 		$mountPoint = $storage->getMountPoint();
 		if ($mountPoint === '') {
 			return new DataResponse(
-				array(
-					'message' => (string)$this->l10n->t('Invalid mount point')
-				),
+				[
+					'message' => (string)$this->l10n->t('Invalid mount point'),
+				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
 			);
 		}
@@ -156,9 +157,9 @@ abstract class StoragesController extends Controller {
 		if ($storage->getBackendOption('objectstore')) {
 			// objectstore must not be sent from client side
 			return new DataResponse(
-				array(
-					'message' => (string)$this->l10n->t('Objectstore forbidden')
-				),
+				[
+					'message' => (string)$this->l10n->t('Objectstore forbidden'),
+				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
 			);
 		}
@@ -170,11 +171,11 @@ abstract class StoragesController extends Controller {
 		if ($backend->checkDependencies()) {
 			// invalid backend
 			return new DataResponse(
-				array(
+				[
 					'message' => (string)$this->l10n->t('Invalid storage backend "%s"', [
-						$backend->getIdentifier()
-					])
-				),
+						$backend->getIdentifier(),
+					]),
+				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
 			);
 		}
@@ -182,22 +183,22 @@ abstract class StoragesController extends Controller {
 		if (!$backend->isVisibleFor($this->service->getVisibilityType())) {
 			// not permitted to use backend
 			return new DataResponse(
-				array(
+				[
 					'message' => (string)$this->l10n->t('Not permitted to use backend "%s"', [
-						$backend->getIdentifier()
-					])
-				),
+						$backend->getIdentifier(),
+					]),
+				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
 			);
 		}
 		if (!$authMechanism->isVisibleFor($this->service->getVisibilityType())) {
 			// not permitted to use auth mechanism
 			return new DataResponse(
-				array(
+				[
 					'message' => (string)$this->l10n->t('Not permitted to use authentication mechanism "%s"', [
-						$authMechanism->getIdentifier()
-					])
-				),
+						$authMechanism->getIdentifier(),
+					]),
+				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
 			);
 		}
@@ -205,9 +206,9 @@ abstract class StoragesController extends Controller {
 		if (!$backend->validateStorage($storage)) {
 			// unsatisfied parameters
 			return new DataResponse(
-				array(
-					'message' => (string)$this->l10n->t('Unsatisfied backend parameters')
-				),
+				[
+					'message' => (string)$this->l10n->t('Unsatisfied backend parameters'),
+				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
 			);
 		}
@@ -215,7 +216,7 @@ abstract class StoragesController extends Controller {
 			// unsatisfied parameters
 			return new DataResponse(
 				[
-					'message' => (string)$this->l10n->t('Unsatisfied authentication mechanism parameters')
+					'message' => (string)$this->l10n->t('Unsatisfied authentication mechanism parameters'),
 				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
 			);
@@ -272,7 +273,7 @@ abstract class StoragesController extends Controller {
 			// FIXME: convert storage exceptions to StorageNotAvailableException
 			$storage->setStatus(
 				StorageNotAvailableException::STATUS_ERROR,
-				get_class($e).': '.$e->getMessage()
+				get_class($e) . ': ' . $e->getMessage()
 			);
 		}
 	}
@@ -283,7 +284,7 @@ abstract class StoragesController extends Controller {
 	 * @return DataResponse
 	 */
 	public function index() {
-		$storages = $this->service->getStorages();
+		$storages = $this->formatStoragesForUI($this->service->getStorages());
 
 		return new DataResponse(
 			$storages,
@@ -291,6 +292,29 @@ abstract class StoragesController extends Controller {
 		);
 	}
 
+	protected function formatStoragesForUI(array $storages): array {
+		return array_map(function ($storage) {
+			return $this->formatStorageForUI($storage);
+		}, $storages);
+	}
+
+	protected function formatStorageForUI(StorageConfig $storage): StorageConfig {
+		/** @var DefinitionParameter[] $parameters */
+		$parameters = array_merge($storage->getBackend()->getParameters(), $storage->getAuthMechanism()->getParameters());
+
+		$options = $storage->getBackendOptions();
+		foreach ($options as $key => $value) {
+			foreach ($parameters as $parameter) {
+				if ($parameter->getName() === $key && $parameter->getType() === DefinitionParameter::VALUE_PASSWORD) {
+					$storage->setBackendOption($key, DefinitionParameter::UNMODIFIED_PLACEHOLDER);
+					break;
+				}
+			}
+		}
+
+		return $storage;
+	}
+
 	/**
 	 * Get an external storage entry.
 	 *
@@ -307,14 +331,14 @@ abstract class StoragesController extends Controller {
 		} catch (NotFoundException $e) {
 			return new DataResponse(
 				[
-					'message' => (string)$this->l10n->t('Storage with ID "%d" not found', array($id))
+					'message' => (string)$this->l10n->t('Storage with ID "%d" not found', [$id]),
 				],
 				Http::STATUS_NOT_FOUND
 			);
 		}
 
 		return new DataResponse(
-			$storage,
+			$this->formatStorageForUI($storage),
 			Http::STATUS_OK
 		);
 	}
@@ -332,7 +356,7 @@ abstract class StoragesController extends Controller {
 		} catch (NotFoundException $e) {
 			return new DataResponse(
 				[
-					'message' => (string)$this->l10n->t('Storage with ID "%d" not found', array($id))
+					'message' => (string)$this->l10n->t('Storage with ID "%d" not found', [$id]),
 				],
 				Http::STATUS_NOT_FOUND
 			);
diff --git a/apps/files_external/lib/Controller/UserGlobalStoragesController.php b/apps/files_external/lib/Controller/UserGlobalStoragesController.php
index b564adc5811..8e0dac5a898 100644
--- a/apps/files_external/lib/Controller/UserGlobalStoragesController.php
+++ b/apps/files_external/lib/Controller/UserGlobalStoragesController.php
@@ -85,7 +85,7 @@ class UserGlobalStoragesController extends StoragesController {
 	 * @NoAdminRequired
 	 */
 	public function index() {
-		$storages = $this->service->getUniqueStorages();
+		$storages = $this->formatStoragesForUI($this->service->getUniqueStorages());
 
 		// remove configuration data, this must be kept private
 		foreach ($storages as $storage) {
@@ -133,7 +133,7 @@ class UserGlobalStoragesController extends StoragesController {
 		$this->sanitizeStorage($storage);
 
 		return new DataResponse(
-			$storage,
+			$this->formatStorageForUI($storage),
 			Http::STATUS_OK
 		);
 	}
@@ -182,7 +182,7 @@ class UserGlobalStoragesController extends StoragesController {
 		$this->sanitizeStorage($storage);
 
 		return new DataResponse(
-			$storage,
+			$this->formatStorageForUI($storage),
 			Http::STATUS_OK
 		);
 
diff --git a/apps/files_external/lib/Controller/UserStoragesController.php b/apps/files_external/lib/Controller/UserStoragesController.php
index 6f55af96d1d..46285c4758a 100644
--- a/apps/files_external/lib/Controller/UserStoragesController.php
+++ b/apps/files_external/lib/Controller/UserStoragesController.php
@@ -148,7 +148,7 @@ class UserStoragesController extends StoragesController {
 		$this->updateStorageStatus($newStorage);
 
 		return new DataResponse(
-			$newStorage,
+			$this->formatStorageForUI($newStorage),
 			Http::STATUS_CREATED
 		);
 	}
@@ -208,7 +208,7 @@ class UserStoragesController extends StoragesController {
 		$this->updateStorageStatus($storage, $testOnly);
 
 		return new DataResponse(
-			$storage,
+			$this->formatStorageForUI($storage),
 			Http::STATUS_OK
 		);
 
diff --git a/apps/files_external/lib/Lib/Auth/AuthMechanism.php b/apps/files_external/lib/Lib/Auth/AuthMechanism.php
index 891719eebb1..cd8f8242e30 100644
--- a/apps/files_external/lib/Lib/Auth/AuthMechanism.php
+++ b/apps/files_external/lib/Lib/Auth/AuthMechanism.php
@@ -51,7 +51,6 @@ use OCA\Files_External\Lib\VisibilityTrait;
  *      Object can affect storage mounting
  */
 class AuthMechanism implements \JsonSerializable {
-
 	/** Standard authentication schemes */
 	const SCHEME_NULL = 'null';
 	const SCHEME_BUILTIN = 'builtin';
diff --git a/apps/files_external/lib/Lib/DefinitionParameter.php b/apps/files_external/lib/Lib/DefinitionParameter.php
index e1f8ed733a1..7250a77e6c1 100644
--- a/apps/files_external/lib/Lib/DefinitionParameter.php
+++ b/apps/files_external/lib/Lib/DefinitionParameter.php
@@ -27,6 +27,9 @@ namespace OCA\Files_External\Lib;
  * Parameter for an external storage definition
  */
 class DefinitionParameter implements \JsonSerializable {
+	// placeholder value for password fields, when the client updates a storage configuration
+	// placeholder values are ignored and the field is left unmodified
+	const UNMODIFIED_PLACEHOLDER = '__unmodified__';
 
 	/** Value constants */
 	const VALUE_TEXT = 0;
diff --git a/apps/files_external/lib/Lib/FrontendDefinitionTrait.php b/apps/files_external/lib/Lib/FrontendDefinitionTrait.php
index c1f6e5ce1f3..58e2d5ffdcf 100644
--- a/apps/files_external/lib/Lib/FrontendDefinitionTrait.php
+++ b/apps/files_external/lib/Lib/FrontendDefinitionTrait.php
@@ -154,5 +154,4 @@ trait FrontendDefinitionTrait {
 		}
 		return true;
 	}
-
 }
diff --git a/apps/files_external/lib/Service/StoragesService.php b/apps/files_external/lib/Service/StoragesService.php
index 61c011a1517..60ccec13206 100644
--- a/apps/files_external/lib/Service/StoragesService.php
+++ b/apps/files_external/lib/Service/StoragesService.php
@@ -36,6 +36,7 @@ use OCA\Files_External\Lib\Auth\AuthMechanism;
 use OCA\Files_External\Lib\Auth\InvalidAuth;
 use OCA\Files_External\Lib\Backend\Backend;
 use OCA\Files_External\Lib\Backend\InvalidBackend;
+use OCA\Files_External\Lib\DefinitionParameter;
 use OCA\Files_External\Lib\StorageConfig;
 use OCA\Files_External\NotFoundException;
 use OCP\Files\Config\IUserMountCache;
@@ -427,7 +428,9 @@ abstract class StoragesService {
 		$changedOptions = array_diff_assoc($updatedStorage->getMountOptions(), $oldStorage->getMountOptions());
 
 		foreach ($changedConfig as $key => $value) {
-			$this->dbConfig->setConfig($id, $key, $value);
+			if ($value !== DefinitionParameter::UNMODIFIED_PLACEHOLDER) {
+				$this->dbConfig->setConfig($id, $key, $value);
+			}
 		}
 		foreach ($changedOptions as $key => $value) {
 			$this->dbConfig->setOption($id, $key, $value);