Merge pull request #52628 from nextcloud/fix/files_external/hidden-password-fields

apps/files_external/lib/Command/Backends.php

@@ -94,7 +94,7 @@ class Backends extends Base {
 	 */
 	private function formatConfiguration(array $parameters): array {
 		$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();

apps/files_external/lib/Lib/Auth/OAuth1/OAuth1.php

@@ -21,14 +21,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')
 		;

apps/files_external/lib/Lib/Auth/OAuth2/OAuth2.php

@@ -21,12 +21,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')
 		;

apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php

@@ -43,7 +43,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);

apps/files_external/lib/Lib/Auth/Password/SessionCredentials.php

@@ -32,7 +32,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),
 			]);
 	}
 

apps/files_external/lib/Lib/Auth/PublicKey/RSA.php

@@ -31,7 +31,8 @@ class RSA extends AuthMechanism {
 				new DefinitionParameter('user', $l->t('Login')),
 				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')
 		;

apps/files_external/lib/Lib/Backend/LegacyBackend.php

@@ -52,10 +52,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)

apps/files_external/lib/Lib/Backend/SMB.php

@@ -49,8 +49,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 account 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)

apps/files_external/lib/Lib/DefinitionParameter.php

@@ -18,12 +18,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 human-readable parameter tooltip */
 	private string $tooltip = '';

apps/files_external/src/settings.js

@@ -647,13 +647,13 @@ const MountConfigListView = function($el, options) {
 MountConfigListView.ParameterFlags = {
 	OPTIONAL: 1,
 	USER_PROVIDED: 2,
+	HIDDEN: 4,
 }
 
 MountConfigListView.ParameterTypes = {
 	TEXT: 0,
 	BOOLEAN: 1,
 	PASSWORD: 2,
-	HIDDEN: 3,
 }
 
 /**
@@ -1142,13 +1142,13 @@ MountConfigListView.prototype = _.extend({
 		let newElement
 
 		const 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) {
 			const 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 + '" />')
 		}

apps/files_external/templates/settings.php

@@ -7,7 +7,6 @@
  */
 use OCA\Files_External\Lib\Auth\AuthMechanism;
 use OCA\Files_External\Lib\Backend\Backend;
-use OCA\Files_External\Lib\DefinitionParameter;
 use OCA\Files_External\Service\BackendService;
 
 /** @var array $_ */
@@ -42,63 +41,6 @@ foreach ($_['authMechanisms'] as $authMechanism) {
 	}
 }
 
-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">

apps/files_external/tests/Backend/LegacyBackendTest.php

@@ -32,7 +32,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',
 			],
@@ -66,9 +65,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());

apps/files_external/tests/DefinitionParameterTest.php

@@ -38,12 +38,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());
 	}
@@ -53,6 +53,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],
@@ -62,8 +63,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]
 		];
 	}