Skip to content

fix(files_external): Mark password fields for LoginCredentials and SessionCredentials as hidden and optional #52628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
provokateurin merged 2 commits into from
May 5, 2025
Merged

fix(files_external): Mark password fields for LoginCredentials and SessionCredentials as hidden and optional #52628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01d2a9f
fix(files_external): Convert VALUE_HIDDEN to FLAG_HIDDEN to allow com…
provokateurin May 5, 2025
db5f6a4
fix(files_external): Mark password fields for LoginCredentials and Se…
provokateurin May 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/files_external/lib/Command/Backends.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ private function serializeAuthBackend(\JsonSerializable $backend): array {
*/
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();
Expand Down
9 changes: 6 additions & 3 deletions apps/files_external/lib/Lib/Auth/OAuth1/OAuth1.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,17 @@ public function __construct(IL10N $l) {
->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')
;
Expand Down
6 changes: 4 additions & 2 deletions apps/files_external/lib/Lib/Auth/OAuth2/OAuth2.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ public function __construct(IL10N $l) {
->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')
;
Expand Down
4 changes: 3 additions & 1 deletion apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ public function __construct(
->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);
Expand Down
4 changes: 3 additions & 1 deletion apps/files_external/lib/Lib/Auth/Password/SessionCredentials.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ public function __construct(
->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),
]);
}

Expand Down
3 changes: 2 additions & 1 deletion apps/files_external/lib/Lib/Auth/PublicKey/RSA.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ public function __construct(
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')
;
Expand Down
4 changes: 0 additions & 4 deletions apps/files_external/lib/Lib/Backend/LegacyBackend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,6 @@ public function __construct($class, array $definition, Builtin $authMechanism) {
$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)
Expand Down
5 changes: 3 additions & 2 deletions apps/files_external/lib/Lib/Backend/SMB.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ public function __construct(IL10N $l, Password $legacyAuth) {
->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)
Expand Down
2 changes: 1 addition & 1 deletion apps/files_external/lib/Lib/DefinitionParameter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '';
Expand Down
8 changes: 4 additions & 4 deletions apps/files_external/src/settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

/**
Expand Down Expand Up @@ -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 + '" />')
}
Expand Down
58 changes: 0 additions & 58 deletions apps/files_external/templates/settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 $_ */
Expand Down Expand Up @@ -42,63 +41,6 @@
}
}

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">
Expand Down
4 changes: 0 additions & 4 deletions apps/files_external/tests/Backend/LegacyBackendTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ public function testConstructor(): void {
'textfield' => 'Text field',
'passwordfield' => '*Password field',
'checkbox' => '!Checkbox',
'hiddenfield' => '#Hidden field',
'optionaltext' => '&Optional text field',
'optionalpassword' => '&*Optional password field',
],
Expand Down Expand Up @@ -66,9 +65,6 @@ public function testConstructor(): void {
$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());
Expand Down
11 changes: 5 additions & 6 deletions apps/files_external/tests/DefinitionParameterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ public function testJsonSerialization(): void {
'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());
}
Expand All @@ -53,6 +53,7 @@ public function validateValueProvider() {
[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],
Expand All @@ -62,8 +63,6 @@ public function validateValueProvider() {

[Param::VALUE_PASSWORD, Param::FLAG_NONE, 'foobar', true],
[Param::VALUE_PASSWORD, Param::FLAG_NONE, '', false],

[Param::VALUE_HIDDEN, Param::FLAG_NONE, '', false]
];
}

Expand Down
4 changes: 2 additions & 2 deletions dist/files_external-settings.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/files_external-settings.js.map
View file
Open in desktop

Large diffs are not rendered by default.

Loading