Merge pull request #52649 from nextcloud/backport/52628/stable26stable26

[stable26] fix(files_external): Mark password fields for LoginCredentials and SessionCredentials as hidden and optional

14 files changed, 33 insertions, 96 deletions

diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js
index d3e9fd44c0d..e22f6a91496 100644
--- a/apps/files_external/js/settings.js
+++ b/apps/files_external/js/settings.js
@@ -603,14 +603,14 @@ var MountConfigListView = function($el, options) {
 
 MountConfigListView.ParameterFlags = {
 	OPTIONAL: 1,
-	USER_PROVIDED: 2
+	USER_PROVIDED: 2,
+	HIDDEN: 4,
 };
 
 MountConfigListView.ParameterTypes = {
 	TEXT: 0,
 	BOOLEAN: 1,
 	PASSWORD: 2,
-	HIDDEN: 3
 };
 
 /**
@@ -1090,13 +1090,13 @@ MountConfigListView.prototype = _.extend({
 		var newElement;
 
 		var trimmedPlaceholder = placeholder.value;
-		if (placeholder.type === MountConfigListView.ParameterTypes.PASSWORD) {
+		if (hasFlag(MountConfigListView.ParameterFlags.HIDDEN)) {
+			newElement = $('<input type="hidden" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" />');
+		} else if (placeholder.type === MountConfigListView.ParameterTypes.PASSWORD) {
 			newElement = $('<input type="password" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" placeholder="'+ trimmedPlaceholder+'" />');
 		} else if (placeholder.type === MountConfigListView.ParameterTypes.BOOLEAN) {
 			var checkboxId = _.uniqueId('checkbox_');
 			newElement = $('<div><label><input type="checkbox" id="'+checkboxId+'" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" />'+ trimmedPlaceholder+'</label></div>');
-		} else if (placeholder.type === MountConfigListView.ParameterTypes.HIDDEN) {
-			newElement = $('<input type="hidden" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" />');
 		} else {
 			newElement = $('<input type="text" class="'+classes.join(' ')+'" data-parameter="'+parameter+'" placeholder="'+ trimmedPlaceholder+'" />');
 		}
diff --git a/apps/files_external/lib/Command/Backends.php b/apps/files_external/lib/Command/Backends.php
index faf6209bd74..1c0852659dd 100644
--- a/apps/files_external/lib/Command/Backends.php
+++ b/apps/files_external/lib/Command/Backends.php
@@ -114,7 +114,7 @@ class Backends extends Base {
 	 */
 	private function formatConfiguration(array $parameters) {
 		$configuration = array_filter($parameters, function (DefinitionParameter $parameter) {
-			return $parameter->getType() !== DefinitionParameter::VALUE_HIDDEN;
+			return $parameter->isFlagSet(DefinitionParameter::FLAG_HIDDEN);
 		});
 		return array_map(function (DefinitionParameter $parameter) {
 			return $parameter->getTypeName();
diff --git a/apps/files_external/lib/Lib/Auth/OAuth1/OAuth1.php b/apps/files_external/lib/Lib/Auth/OAuth1/OAuth1.php
index 44cdcf79523..83b2d31dad6 100644
--- a/apps/files_external/lib/Lib/Auth/OAuth1/OAuth1.php
+++ b/apps/files_external/lib/Lib/Auth/OAuth1/OAuth1.php
@@ -38,14 +38,17 @@ class OAuth1 extends AuthMechanism {
 			->setText($l->t('OAuth1'))
 			->addParameters([
 				(new DefinitionParameter('configured', 'configured'))
-					->setType(DefinitionParameter::VALUE_HIDDEN),
+					->setType(DefinitionParameter::VALUE_TEXT)
+					->setFlag(DefinitionParameter::FLAG_HIDDEN),
 				new DefinitionParameter('app_key', $l->t('App key')),
 				(new DefinitionParameter('app_secret', $l->t('App secret')))
 					->setType(DefinitionParameter::VALUE_PASSWORD),
 				(new DefinitionParameter('token', 'token'))
-					->setType(DefinitionParameter::VALUE_HIDDEN),
+					->setType(DefinitionParameter::VALUE_PASSWORD)
+					->setFlag(DefinitionParameter::FLAG_HIDDEN),
 				(new DefinitionParameter('token_secret', 'token_secret'))
-					->setType(DefinitionParameter::VALUE_HIDDEN),
+					->setType(DefinitionParameter::VALUE_PASSWORD)
+					->setFlag(DefinitionParameter::FLAG_HIDDEN),
 			])
 			->addCustomJs('oauth1')
 		;
diff --git a/apps/files_external/lib/Lib/Auth/OAuth2/OAuth2.php b/apps/files_external/lib/Lib/Auth/OAuth2/OAuth2.php
index 4dc2b23035a..bb8243552e5 100644
--- a/apps/files_external/lib/Lib/Auth/OAuth2/OAuth2.php
+++ b/apps/files_external/lib/Lib/Auth/OAuth2/OAuth2.php
@@ -38,12 +38,14 @@ class OAuth2 extends AuthMechanism {
 			->setText($l->t('OAuth2'))
 			->addParameters([
 				(new DefinitionParameter('configured', 'configured'))
-					->setType(DefinitionParameter::VALUE_HIDDEN),
+					->setType(DefinitionParameter::VALUE_TEXT)
+					->setFlag(DefinitionParameter::FLAG_HIDDEN),
 				new DefinitionParameter('client_id', $l->t('Client ID')),
 				(new DefinitionParameter('client_secret', $l->t('Client secret')))
 					->setType(DefinitionParameter::VALUE_PASSWORD),
 				(new DefinitionParameter('token', 'token'))
-					->setType(DefinitionParameter::VALUE_HIDDEN),
+					->setType(DefinitionParameter::VALUE_PASSWORD)
+					->setFlag(DefinitionParameter::FLAG_HIDDEN),
 			])
 			->addCustomJs('oauth2')
 		;
diff --git a/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php b/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php
index 4a8264f82f8..4a26d865fc7 100644
--- a/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php
+++ b/apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php
@@ -81,7 +81,9 @@ class LoginCredentials extends AuthMechanism {
 			->setText($l->t('Log-in credentials, save in database'))
 			->addParameters([
 				(new DefinitionParameter('password', $l->t('Password')))
-					->setType(DefinitionParameter::VALUE_PASSWORD),
+					->setType(DefinitionParameter::VALUE_PASSWORD)
+					->setFlag(DefinitionParameter::FLAG_HIDDEN)
+					->setFlag(DefinitionParameter::FLAG_OPTIONAL),
 			]);
 
 		$eventDispatcher->addServiceListener(UserLoggedInEvent::class, StorePasswordListener::class);
diff --git a/apps/files_external/lib/Lib/Auth/Password/SessionCredentials.php b/apps/files_external/lib/Lib/Auth/Password/SessionCredentials.php
index ea14a282e3d..cbc75cc7fe8 100644
--- a/apps/files_external/lib/Lib/Auth/Password/SessionCredentials.php
+++ b/apps/files_external/lib/Lib/Auth/Password/SessionCredentials.php
@@ -51,7 +51,9 @@ class SessionCredentials extends AuthMechanism {
 			->setText($l->t('Log-in credentials, save in session'))
 			->addParameters([
 				(new DefinitionParameter('password', $l->t('Password')))
-					->setType(DefinitionParameter::VALUE_PASSWORD),
+					->setType(DefinitionParameter::VALUE_PASSWORD)
+					->setFlag(DefinitionParameter::FLAG_HIDDEN)
+					->setFlag(DefinitionParameter::FLAG_OPTIONAL),
 			]);
 	}
 
diff --git a/apps/files_external/lib/Lib/Auth/PublicKey/RSA.php b/apps/files_external/lib/Lib/Auth/PublicKey/RSA.php
index d45db03b2da..12417b1bc85 100644
--- a/apps/files_external/lib/Lib/Auth/PublicKey/RSA.php
+++ b/apps/files_external/lib/Lib/Auth/PublicKey/RSA.php
@@ -50,7 +50,8 @@ class RSA extends AuthMechanism {
 				new DefinitionParameter('user', $l->t('Username')),
 				new DefinitionParameter('public_key', $l->t('Public key')),
 				(new DefinitionParameter('private_key', 'private_key'))
-					->setType(DefinitionParameter::VALUE_HIDDEN),
+					->setType(DefinitionParameter::VALUE_PASSWORD)
+					->setFlag(DefinitionParameter::FLAG_HIDDEN),
 			])
 			->addCustomJs('public_key')
 		;
diff --git a/apps/files_external/lib/Lib/Backend/LegacyBackend.php b/apps/files_external/lib/Lib/Backend/LegacyBackend.php
index d00c5b0e944..fdb1dd07f28 100644
--- a/apps/files_external/lib/Lib/Backend/LegacyBackend.php
+++ b/apps/files_external/lib/Lib/Backend/LegacyBackend.php
@@ -69,10 +69,6 @@ class LegacyBackend extends Backend {
 				$type = DefinitionParameter::VALUE_PASSWORD;
 				$placeholder = substr($placeholder, 1);
 				break;
-			case '#':
-				$type = DefinitionParameter::VALUE_HIDDEN;
-				$placeholder = substr($placeholder, 1);
-				break;
 			}
 			$this->addParameter((new DefinitionParameter($name, $placeholder))
 				->setType($type)
diff --git a/apps/files_external/lib/Lib/Backend/SMB.php b/apps/files_external/lib/Lib/Backend/SMB.php
index 6c879676088..59f9d738719 100644
--- a/apps/files_external/lib/Lib/Backend/SMB.php
+++ b/apps/files_external/lib/Lib/Backend/SMB.php
@@ -69,8 +69,9 @@ class SMB extends Backend {
 					->setFlag(DefinitionParameter::FLAG_OPTIONAL)
 					->setTooltip($l->t("Check the ACL's of each file or folder inside a directory to filter out items where the user has no read permissions, comes with a performance penalty")),
 				(new DefinitionParameter('timeout', $l->t('Timeout')))
-					->setType(DefinitionParameter::VALUE_HIDDEN)
-					->setFlag(DefinitionParameter::FLAG_OPTIONAL),
+					->setType(DefinitionParameter::VALUE_TEXT)
+					->setFlag(DefinitionParameter::FLAG_OPTIONAL)
+					->setFlag(DefinitionParameter::FLAG_HIDDEN),
 			])
 			->addAuthScheme(AuthMechanism::SCHEME_PASSWORD)
 			->addAuthScheme(AuthMechanism::SCHEME_SMB)
diff --git a/apps/files_external/lib/Lib/DefinitionParameter.php b/apps/files_external/lib/Lib/DefinitionParameter.php
index ce0a37b4e9b..22488b9226c 100644
--- a/apps/files_external/lib/Lib/DefinitionParameter.php
+++ b/apps/files_external/lib/Lib/DefinitionParameter.php
@@ -35,12 +35,12 @@ class DefinitionParameter implements \JsonSerializable {
 	public const VALUE_TEXT = 0;
 	public const VALUE_BOOLEAN = 1;
 	public const VALUE_PASSWORD = 2;
-	public const VALUE_HIDDEN = 3;
 
 	/** Flag constants */
 	public const FLAG_NONE = 0;
 	public const FLAG_OPTIONAL = 1;
 	public const FLAG_USER_PROVIDED = 2;
+	public const FLAG_HIDDEN = 4;
 
 	/** @var string name of parameter */
 	private string $name;
diff --git a/apps/files_external/templates/settings.php b/apps/files_external/templates/settings.php
index d715e5946f1..61a149cec65 100644
--- a/apps/files_external/templates/settings.php
+++ b/apps/files_external/templates/settings.php
@@ -1,7 +1,6 @@
 <?php
 use \OCA\Files_External\Lib\Backend\Backend;
 use \OCA\Files_External\Lib\Auth\AuthMechanism;
-use \OCA\Files_External\Lib\DefinitionParameter;
 use \OCA\Files_External\Service\BackendService;
 
 /** @var array $_ */
@@ -38,63 +37,6 @@ $canCreateMounts = $_['visibilityType'] === BackendService::VISIBILITY_ADMIN ||
 		}
 	}
 
-	function writeParameterInput($parameter, $options, $classes = []) {
-		$value = '';
-		if (isset($options[$parameter->getName()])) {
-			$value = $options[$parameter->getName()];
-		}
-		$placeholder = $parameter->getText();
-		$is_optional = $parameter->isFlagSet(DefinitionParameter::FLAG_OPTIONAL);
-
-		switch ($parameter->getType()) {
-		case DefinitionParameter::VALUE_PASSWORD: ?>
-			<?php if ($is_optional) {
-			$classes[] = 'optional';
-		} ?>
-			<input type="password"
-				<?php if (!empty($classes)): ?> class="<?php p(implode(' ', $classes)); ?>"<?php endif; ?>
-				data-parameter="<?php p($parameter->getName()); ?>"
-				value="<?php p($value); ?>"
-				placeholder="<?php p($placeholder); ?>"
-			/>
-			<?php
-			break;
-		case DefinitionParameter::VALUE_BOOLEAN: ?>
-			<?php $checkboxId = uniqid("checkbox_"); ?>
-			<div>
-			<label>
-			<input type="checkbox"
-				id="<?php p($checkboxId); ?>"
-				<?php if (!empty($classes)): ?> class="checkbox <?php p(implode(' ', $classes)); ?>"<?php endif; ?>
-				data-parameter="<?php p($parameter->getName()); ?>"
-				<?php if ($value === true): ?> checked="checked"<?php endif; ?>
-			/>
-			<?php p($placeholder); ?>
-			</label>
-			</div>
-			<?php
-			break;
-		case DefinitionParameter::VALUE_HIDDEN: ?>
-			<input type="hidden"
-				<?php if (!empty($classes)): ?> class="<?php p(implode(' ', $classes)); ?>"<?php endif; ?>
-				data-parameter="<?php p($parameter->getName()); ?>"
-				value="<?php p($value); ?>"
-			/>
-			<?php
-			break;
-		default: ?>
-			<?php if ($is_optional) {
-			$classes[] = 'optional';
-		} ?>
-			<input type="text"
-				<?php if (!empty($classes)): ?> class="<?php p(implode(' ', $classes)); ?>"<?php endif; ?>
-				data-parameter="<?php p($parameter->getName()); ?>"
-				value="<?php p($value); ?>"
-				placeholder="<?php p($placeholder); ?>"
-			/>
-			<?php
-		}
-	}
 ?>
 
 <div class="emptyfilelist emptycontent hidden">
diff --git a/apps/files_external/tests/Backend/LegacyBackendTest.php b/apps/files_external/tests/Backend/LegacyBackendTest.php
index 2a69ab9e355..aeae18d384d 100644
--- a/apps/files_external/tests/Backend/LegacyBackendTest.php
+++ b/apps/files_external/tests/Backend/LegacyBackendTest.php
@@ -48,7 +48,6 @@ class LegacyBackendTest extends \Test\TestCase {
 				'textfield' => 'Text field',
 				'passwordfield' => '*Password field',
 				'checkbox' => '!Checkbox',
-				'hiddenfield' => '#Hidden field',
 				'optionaltext' => '&Optional text field',
 				'optionalpassword' => '&*Optional password field',
 			],
@@ -82,9 +81,6 @@ class LegacyBackendTest extends \Test\TestCase {
 		$this->assertEquals('Checkbox', $parameters['checkbox']->getText());
 		$this->assertEquals(DefinitionParameter::VALUE_BOOLEAN, $parameters['checkbox']->getType());
 		$this->assertEquals(DefinitionParameter::FLAG_NONE, $parameters['checkbox']->getFlags());
-		$this->assertEquals('Hidden field', $parameters['hiddenfield']->getText());
-		$this->assertEquals(DefinitionParameter::VALUE_HIDDEN, $parameters['hiddenfield']->getType());
-		$this->assertEquals(DefinitionParameter::FLAG_NONE, $parameters['hiddenfield']->getFlags());
 		$this->assertEquals('Optional text field', $parameters['optionaltext']->getText());
 		$this->assertEquals(DefinitionParameter::VALUE_TEXT, $parameters['optionaltext']->getType());
 		$this->assertEquals(DefinitionParameter::FLAG_OPTIONAL, $parameters['optionaltext']->getFlags());
diff --git a/apps/files_external/tests/DefinitionParameterTest.php b/apps/files_external/tests/DefinitionParameterTest.php
index 04d5f6762c5..4f988a10c49 100644
--- a/apps/files_external/tests/DefinitionParameterTest.php
+++ b/apps/files_external/tests/DefinitionParameterTest.php
@@ -55,12 +55,12 @@ class DefinitionParameterTest extends \Test\TestCase {
 			'tooltip' => '',
 		], $param->jsonSerialize());
 
-		$param->setType(Param::VALUE_HIDDEN);
-		$param->setFlags(Param::FLAG_NONE);
+		$param->setType(Param::VALUE_TEXT);
+		$param->setFlags(Param::FLAG_HIDDEN);
 		$this->assertEquals([
 			'value' => 'bar',
-			'flags' => Param::FLAG_NONE,
-			'type' => Param::VALUE_HIDDEN,
+			'flags' => Param::FLAG_HIDDEN,
+			'type' => Param::VALUE_TEXT,
 			'tooltip' => '',
 		], $param->jsonSerialize());
 	}
@@ -70,6 +70,7 @@ class DefinitionParameterTest extends \Test\TestCase {
 			[Param::VALUE_TEXT, Param::FLAG_NONE, 'abc', true],
 			[Param::VALUE_TEXT, Param::FLAG_NONE, '', false],
 			[Param::VALUE_TEXT, Param::FLAG_OPTIONAL, '', true],
+			[Param::VALUE_TEXT, Param::FLAG_HIDDEN, '', false],
 
 			[Param::VALUE_BOOLEAN, Param::FLAG_NONE, false, true],
 			[Param::VALUE_BOOLEAN, Param::FLAG_NONE, 123, false],
@@ -79,8 +80,6 @@ class DefinitionParameterTest extends \Test\TestCase {
 
 			[Param::VALUE_PASSWORD, Param::FLAG_NONE, 'foobar', true],
 			[Param::VALUE_PASSWORD, Param::FLAG_NONE, '', false],
-
-			[Param::VALUE_HIDDEN, Param::FLAG_NONE, '', false]
 		];
 	}
 
diff --git a/apps/files_external/tests/js/settingsSpec.js b/apps/files_external/tests/js/settingsSpec.js
index 4032f6f6a37..f97667e0376 100644
--- a/apps/files_external/tests/js/settingsSpec.js
+++ b/apps/files_external/tests/js/settingsSpec.js
@@ -130,10 +130,6 @@ describe('OCA.Files_External.Settings tests', function() {
 							'value': 'Boolean field',
 							'type': 1
 						},
-						'field_hidden': {
-							'value': 'Hidden field',
-							'type': 3
-						},
 						'field_text_optional': {
 							'value': 'Text field optional',
 							'flags': 1
@@ -361,7 +357,6 @@ describe('OCA.Files_External.Settings tests', function() {
 				});
 				_.each([
 					'field_bool',
-					'field_hidden',
 					'field_text_optional',
 					'field_password_optional'
 				], function(param) {
@@ -375,8 +370,6 @@ describe('OCA.Files_External.Settings tests', function() {
 				$tr.find('input[data-parameter=field_text]').val('foo');
 				$tr.find('input[data-parameter=field_password]').val('bar');
 				$tr.find('input[data-parameter=field_text_optional]').val('foobar');
-				// don't set field_password_optional
-				$tr.find('input[data-parameter=field_hidden]').val('baz');
 
 				var storage = view.getStorageConfig($tr);