Merge pull request #46859 from nextcloud/fix-status-check-and-saving-of-external-storages

This commit is contained in:
John Molakvoæ 2024-09-04 11:33:30 +02:00 committed by GitHub
commit f1706df367
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 214 additions and 16 deletions

View file

@ -1,4 +1,4 @@
/*!
* SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/#files_external{margin-bottom:0px}#externalStorage{margin:15px 0 20px 0}#externalStorage tr.externalStorageLoading>td{text-align:center}#externalStorage td>input:not(.applicableToAllUsers),#externalStorage td>select{width:100%}#externalStorage .popovermenu li>.menuitem{width:fit-content !important}#externalStorage td.status{display:table-cell;vertical-align:middle}#externalStorage td.status>span{display:inline-block;height:28px;width:28px;vertical-align:text-bottom;border-radius:50%;cursor:pointer}#externalStorage td.mountPoint,#externalStorage td.backend,#externalStorage td.authentication,#externalStorage td.configuration{min-width:160px;width:15%}#externalStorage td>img{padding-top:7px;opacity:.5}#externalStorage td>img:hover{padding-top:7px;cursor:pointer;opacity:1}#addMountPoint>td{border:none}#addMountPoint>td.applicable{visibility:hidden}#addMountPoint>td.hidden{visibility:hidden}#externalStorage td{height:50px}#externalStorage td.mountOptionsToggle,#externalStorage td.remove,#externalStorage td.save{position:relative;padding:0 !important;width:44px}#externalStorage td.mountOptionsToggle [class^=icon-],#externalStorage td.mountOptionsToggle [class*=" icon-"],#externalStorage td.remove [class^=icon-],#externalStorage td.remove [class*=" icon-"],#externalStorage td.save [class^=icon-],#externalStorage td.save [class*=" icon-"]{width:44px;height:44px;margin:3px;opacity:.5;padding:14px;vertical-align:text-bottom;cursor:pointer}#externalStorage td.mountOptionsToggle [class^=icon-]:hover,#externalStorage td.mountOptionsToggle [class*=" icon-"]:hover,#externalStorage td.remove [class^=icon-]:hover,#externalStorage td.remove [class*=" icon-"]:hover,#externalStorage td.save [class^=icon-]:hover,#externalStorage td.save [class*=" icon-"]:hover{opacity:1}#selectBackend{margin-inline-start:-10px;width:150px}#externalStorage td.configuration,#externalStorage td.backend{white-space:normal}#externalStorage td.configuration>*{white-space:nowrap}#externalStorage td.configuration input.added{margin-inline-end:6px}#externalStorage label>input[type=checkbox]{margin-inline-end:3px}#externalStorage td.configuration label{width:100%;display:inline-flex;align-items:center}#externalStorage td.configuration input.disabled-success{background-color:rgba(134,255,110,.9)}#externalStorage td.applicable label{display:inline-flex;align-items:center}#externalStorage td.applicable div.chzn-container{position:relative;top:3px}#externalStorage .select2-container.applicableUsers{width:100% !important}#userMountingBackends{padding-inline-start:25px}.files-external-select2 .select2-results .select2-result-label{height:32px;padding:3px}.files-external-select2 .select2-results .select2-result-label>span{display:block;position:relative}.files-external-select2 .select2-results .select2-result-label .avatardiv{display:inline-block}.files-external-select2 .select2-results .select2-result-label .avatardiv+span{position:absolute;top:5px;margin-inline-start:10px}.files-external-select2 .select2-results .select2-result-label .avatardiv[data-type=group]+span{vertical-align:top;top:6px;position:absolute;max-width:80%;inset-inline-start:30px;text-overflow:ellipsis;white-space:nowrap;overflow:hidden}#externalStorage .select2-container .select2-search-choice{display:flex}#externalStorage .select2-container .select2-search-choice .select2-search-choice-close{display:block;inset-inline-start:auto;position:relative;width:20px}#externalStorage .mountOptionsToggle .dropdown{width:auto}.nav-icon-external-storage{background-image:var(--icon-external-dark)}.global_credentials__personal{margin:10px auto;text-align:center;width:min(400px,100vw)}/*# sourceMappingURL=settings.css.map */
*/#files_external{margin-bottom:0px}#externalStorage{margin:15px 0 20px 0}#externalStorage tr.externalStorageLoading>td{text-align:center}#externalStorage td>input:not(.applicableToAllUsers),#externalStorage td>select{width:100%}#externalStorage .popovermenu li>.menuitem{width:fit-content !important}#externalStorage td.status{display:table-cell;vertical-align:middle;width:43px}#externalStorage td.status>span{display:inline-block;height:28px;width:28px;vertical-align:text-bottom;border-radius:50%;cursor:pointer}#externalStorage td.mountPoint,#externalStorage td.backend,#externalStorage td.authentication,#externalStorage td.configuration{min-width:160px;width:15%}#externalStorage td>img{padding-top:7px;opacity:.5}#externalStorage td>img:hover{padding-top:7px;cursor:pointer;opacity:1}#addMountPoint>td{border:none}#addMountPoint>td.applicable{visibility:hidden}#addMountPoint>td.hidden{visibility:hidden}#externalStorage td{height:50px}#externalStorage td.mountOptionsToggle,#externalStorage td.remove,#externalStorage td.save{position:relative;padding:0 !important;width:44px}#externalStorage td.mountOptionsToggle [class^=icon-],#externalStorage td.mountOptionsToggle [class*=" icon-"],#externalStorage td.remove [class^=icon-],#externalStorage td.remove [class*=" icon-"],#externalStorage td.save [class^=icon-],#externalStorage td.save [class*=" icon-"]{width:44px;height:44px;margin:3px;opacity:.5;padding:14px;vertical-align:text-bottom;cursor:pointer}#externalStorage td.mountOptionsToggle [class^=icon-]:hover,#externalStorage td.mountOptionsToggle [class*=" icon-"]:hover,#externalStorage td.remove [class^=icon-]:hover,#externalStorage td.remove [class*=" icon-"]:hover,#externalStorage td.save [class^=icon-]:hover,#externalStorage td.save [class*=" icon-"]:hover{opacity:1}#selectBackend{margin-inline-start:-10px;width:150px}#externalStorage td.configuration,#externalStorage td.backend{white-space:normal}#externalStorage td.configuration>*{white-space:nowrap}#externalStorage td.configuration input.added{margin-inline-end:6px}#externalStorage label>input[type=checkbox]{margin-inline-end:3px}#externalStorage td.configuration label{width:100%;display:inline-flex;align-items:center}#externalStorage td.configuration input.disabled-success{background-color:rgba(134,255,110,.9)}#externalStorage td.applicable label{display:inline-flex;align-items:center}#externalStorage td.applicable div.chzn-container{position:relative;top:3px}#externalStorage .select2-container.applicableUsers{width:100% !important}#userMountingBackends{padding-inline-start:25px}.files-external-select2 .select2-results .select2-result-label{height:32px;padding:3px}.files-external-select2 .select2-results .select2-result-label>span{display:block;position:relative}.files-external-select2 .select2-results .select2-result-label .avatardiv{display:inline-block}.files-external-select2 .select2-results .select2-result-label .avatardiv+span{position:absolute;top:5px;margin-inline-start:10px}.files-external-select2 .select2-results .select2-result-label .avatardiv[data-type=group]+span{vertical-align:top;top:6px;position:absolute;max-width:80%;inset-inline-start:30px;text-overflow:ellipsis;white-space:nowrap;overflow:hidden}#externalStorage .select2-container .select2-search-choice{display:flex}#externalStorage .select2-container .select2-search-choice .select2-search-choice-close{display:block;inset-inline-start:auto;position:relative;width:20px}#externalStorage .mountOptionsToggle .dropdown{width:auto}.nav-icon-external-storage{background-image:var(--icon-external-dark)}.global_credentials__personal{margin:10px auto;text-align:center;width:min(400px,100vw)}/*# sourceMappingURL=settings.css.map */

View file

@ -1 +1 @@
{"version":3,"sourceRoot":"","sources":["settings.scss"],"names":[],"mappings":"AAAA;AAAA;AAAA;AAAA,GAIA,gBACC,kBAGD,iBACC,qBAEA,8CACC,kBAKD,gFACC,WAIF,2CACI,6BAGJ,2BAEC,mBACA,sBAGD,gCACC,qBACA,YACA,WACA,2BACA,kBACA,eAGA,gIACC,gBACA,UAGF,mDACA,uEACA,8BACA,+CACA,2CAEA,oBACC,YACA,2FAGC,kBACA,qBACA,WACA,yRAEC,WACA,YACA,WACA,WACA,aACA,2BACA,eACA,6TACC,UAMJ,eACC,0BACA,YAGD,8DAEC,mBAED,oCACC,mBAGD,8CACC,sBAGD,4CACC,sBAGD,wCACC,WACA,oBACA,mBAGD,yDACC,sCAGD,qCACC,oBACA,mBAGD,kDACC,kBACA,QAGD,oDACC,sBAGD,sBACC,0BAGD,+DACC,YACA,YAED,oEACC,cACA,kBAED,0EACC,qBAED,+EACC,kBACA,QACA,yBAED,gGACC,mBACA,QACA,kBACA,cACA,wBACA,uBACA,mBACA,gBAGD,2DACC,aACA,wFACC,cACA,wBACA,kBACA,WAIF,+CACC,WAGD,2BACC,2CAGD,8BACI,iBACA,kBACA","file":"settings.css"}
{"version":3,"sourceRoot":"","sources":["settings.scss"],"names":[],"mappings":"AAAA;AAAA;AAAA;AAAA,GAIA,gBACC,kBAGD,iBACC,qBAEA,8CACC,kBAKD,gFACC,WAIF,2CACI,6BAGJ,2BAEC,mBACA,sBAEA,WAGD,gCACC,qBACA,YACA,WACA,2BACA,kBACA,eAGA,gIACC,gBACA,UAGF,mDACA,uEACA,8BACA,+CACA,2CAEA,oBACC,YACA,2FAGC,kBACA,qBACA,WACA,yRAEC,WACA,YACA,WACA,WACA,aACA,2BACA,eACA,6TACC,UAMJ,eACC,0BACA,YAGD,8DAEC,mBAED,oCACC,mBAGD,8CACC,sBAGD,4CACC,sBAGD,wCACC,WACA,oBACA,mBAGD,yDACC,sCAGD,qCACC,oBACA,mBAGD,kDACC,kBACA,QAGD,oDACC,sBAGD,sBACC,0BAGD,+DACC,YACA,YAED,oEACC,cACA,kBAED,0EACC,qBAED,+EACC,kBACA,QACA,yBAED,gGACC,mBACA,QACA,kBACA,cACA,wBACA,uBACA,mBACA,gBAGD,2DACC,aACA,wFACC,cACA,wBACA,kBACA,WAIF,+CACC,WAGD,2BACC,2CAGD,8BACI,iBACA,kBACA","file":"settings.css"}

View file

@ -28,6 +28,8 @@
/* overwrite conflicting core styles */
display: table-cell;
vertical-align: middle;
/* ensure width does not change even if internal span is not displayed */
width: 43px;
}
#externalStorage td.status > span {

View file

@ -766,6 +766,8 @@ MountConfigListView.prototype = _.extend({
storageConfig.backend = $target.val();
$tr.find('.mountPoint input').val('');
$tr.find('.selectBackend').prop('selectedIndex', 0)
var onCompletion = jQuery.Deferred();
$tr = this.newStorage(storageConfig, onCompletion);
$tr.find('.applicableToAllUsers').prop('checked', false).trigger('change');
@ -874,7 +876,7 @@ MountConfigListView.prototype = _.extend({
$tr.find('.applicable,.mountOptionsToggle').empty();
$tr.find('.save').empty();
if (backend.invalid) {
this.updateStatus($tr, false, 'Unknown backend: ' + backend.name);
this.updateStatus($tr, false, t('files_external', 'Unknown backend: {backendName}', {backendName: backend.name}));
}
return $tr;
}
@ -977,9 +979,10 @@ MountConfigListView.prototype = _.extend({
data: {'testOnly' : true},
contentType: 'application/json',
success: function(result) {
result = Object.values(result);
var onCompletion = jQuery.Deferred();
var $rows = $();
Object.values(result).forEach(function(storageParams) {
result.forEach(function(storageParams) {
var storageConfig;
var isUserGlobal = storageParams.type === 'system' && self._isPersonal;
storageParams.mountPoint = storageParams.mountPoint.substr(1); // trim leading slash
@ -1008,6 +1011,13 @@ MountConfigListView.prototype = _.extend({
// userglobal storages do not expose configuration data
$tr.find('.configuration').text(t('files_external', 'Admin defined'));
}
// don't recheck config automatically when there are a large number of storages
if (result.length < 20) {
self.recheckStorageConfig($tr);
} else {
self.updateStatus($tr, StorageConfig.Status.INDETERMINATE, t('files_external', 'Automatic status checking is disabled due to the large number of configured storages, click to check status'));
}
$rows = $rows.add($tr);
});
initApplicableUsersMultiselect(self.$el.find('.applicableUsers'), this._userListLimit);
@ -1226,8 +1236,9 @@ MountConfigListView.prototype = _.extend({
success: function () {
$tr.remove();
},
error: function () {
self.updateStatus($tr, StorageConfig.Status.ERROR);
error: function (result) {
const statusMessage = (result && result.responseJSON) ? result.responseJSON.message : undefined;
self.updateStatus($tr, StorageConfig.Status.ERROR, statusMessage);
}
});
}
@ -1254,7 +1265,7 @@ MountConfigListView.prototype = _.extend({
if (concurrentTimer === undefined
|| $tr.data('save-timer') === concurrentTimer
) {
self.updateStatus($tr, result.status);
self.updateStatus($tr, result.status, result.statusMessage);
$tr.data('id', result.id);
if (_.isFunction(callback)) {
@ -1262,11 +1273,12 @@ MountConfigListView.prototype = _.extend({
}
}
},
error: function() {
error: function(result) {
if (concurrentTimer === undefined
|| $tr.data('save-timer') === concurrentTimer
) {
self.updateStatus($tr, StorageConfig.Status.ERROR);
const statusMessage = (result && result.responseJSON) ? result.responseJSON.message : undefined;
self.updateStatus($tr, StorageConfig.Status.ERROR, statusMessage);
}
}
});
@ -1290,8 +1302,9 @@ MountConfigListView.prototype = _.extend({
success: function(result) {
self.updateStatus($tr, result.status, result.statusMessage);
},
error: function() {
self.updateStatus($tr, StorageConfig.Status.ERROR);
error: function(result) {
const statusMessage = (result && result.responseJSON) ? result.responseJSON.message : undefined;
self.updateStatus($tr, StorageConfig.Status.ERROR, statusMessage);
}
});
},
@ -1308,6 +1321,7 @@ MountConfigListView.prototype = _.extend({
switch (status) {
case null:
// remove status
$statusSpan.hide();
break;
case StorageConfig.Status.IN_PROGRESS:
$statusSpan.attr('class', 'icon-loading-small');
@ -1321,9 +1335,13 @@ MountConfigListView.prototype = _.extend({
default:
$statusSpan.attr('class', 'error icon-error-white');
}
if (typeof message === 'string') {
$statusSpan.attr('title', message);
if (status !== null) {
$statusSpan.show();
}
if (typeof message !== 'string') {
message = t('files_external', 'Click to recheck the configuration');
}
$statusSpan.attr('title', message);
},
/**

View file

@ -9,6 +9,7 @@ declare(strict_types=1);
namespace OCA\Files_External\Lib\Auth\Password;
use OCA\Files_External\Lib\Auth\AuthMechanism;
use OCA\Files_External\Lib\DefinitionParameter;
use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException;
use OCA\Files_External\Lib\StorageConfig;
use OCA\Files_External\Service\BackendService;
@ -41,6 +42,12 @@ class UserGlobalAuth extends AuthMechanism {
if (!isset($backendOptions['user']) && !isset($backendOptions['password'])) {
return;
}
if ($backendOptions['password'] === DefinitionParameter::UNMODIFIED_PLACEHOLDER) {
$oldCredentials = $this->credentialsManager->retrieve($user->getUID(), self::CREDENTIALS_IDENTIFIER);
$backendOptions['password'] = $oldCredentials['password'];
}
// make sure we're not setting any unexpected keys
$credentials = [
'user' => $backendOptions['user'],

View file

@ -47,6 +47,11 @@ class UserProvided extends AuthMechanism implements IUserProvided {
}
public function saveBackendOptions(IUser $user, $mountId, array $options) {
if ($options['password'] === DefinitionParameter::UNMODIFIED_PLACEHOLDER) {
$oldCredentials = $this->credentialsManager->retrieve($user->getUID(), $this->getCredentialsIdentifier($mountId));
$options['password'] = $oldCredentials['password'];
}
$this->credentialsManager->store($user->getUID(), $this->getCredentialsIdentifier($mountId), [
'user' => $options['user'], // explicitly copy the fields we want instead of just passing the entire $options array
'password' => $options['password'] // this way we prevent users from being able to modify any other field

View file

@ -146,7 +146,7 @@ function writeParameterInput($parameter, $options, $classes = []) {
<?php endif; ?>
>
<td class="status">
<span data-placement="right" title="<?php p($l->t('Click to recheck the configuration')); ?>"></span>
<span data-placement="right" title="<?php p($l->t('Click to recheck the configuration')); ?>" style="display: none;"></span>
</td>
<td class="mountPoint"><input type="text" name="mountPoint" value=""
placeholder="<?php p($l->t('Folder name')); ?>">

View file

@ -327,7 +327,7 @@ trait BasicStructure {
$fd = $body->getRowsHash();
$options['form_params'] = $fd;
} elseif ($body) {
$options = array_merge($options, $body);
$options = array_merge_recursive($options, $body);
}
$client = new Client();

View file

@ -0,0 +1,103 @@
<?php
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
use Behat\Gherkin\Node\TableNode;
use PHPUnit\Framework\Assert;
require __DIR__ . '/../../vendor/autoload.php';
trait ExternalStorage {
private array $storageIds = [];
private array $lastExternalStorageData;
/**
* @AfterScenario
**/
public function deleteCreatedStorages(): void {
foreach ($this->storageIds as $storageId) {
$this->deleteStorage($storageId);
}
$this->storageIds = [];
}
private function deleteStorage(string $storageId): void {
// Based on "runOcc" from CommandLine trait
$args = ['files_external:delete', '--yes', $storageId];
$args = array_map(function ($arg) {
return escapeshellarg($arg);
}, $args);
$args[] = '--no-ansi --no-warnings';
$args = implode(' ', $args);
$descriptor = [
0 => ['pipe', 'r'],
1 => ['pipe', 'w'],
2 => ['pipe', 'w'],
];
$process = proc_open('php console.php ' . $args, $descriptor, $pipes, $ocPath = '../..');
$lastStdOut = stream_get_contents($pipes[1]);
proc_close($process);
}
/**
* @When logged in user creates external global storage
*
* @param TableNode $fields
*/
public function loggedInUserCreatesExternalGlobalStorage(TableNode $fields): void {
$this->sendJsonWithRequestToken('POST', '/index.php/apps/files_external/globalstorages', $fields);
$this->theHTTPStatusCodeShouldBe('201');
$this->lastExternalStorageData = json_decode($this->response->getBody(), $asAssociativeArray = true);
$this->storageIds[] = $this->lastExternalStorageData['id'];
}
/**
* @When logged in user updates last external userglobal storage
*
* @param TableNode $fields
*/
public function loggedInUserUpdatesLastExternalUserglobalStorage(TableNode $fields): void {
$this->sendJsonWithRequestToken('PUT', '/index.php/apps/files_external/userglobalstorages/' . $this->lastExternalStorageData['id'], $fields);
$this->theHTTPStatusCodeShouldBe('200');
$this->lastExternalStorageData = json_decode($this->response->getBody(), $asAssociativeArray = true);
}
/**
* @Then fields of last external storage match with
*
* @param TableNode $fields
*/
public function fieldsOfLastExternalStorageMatchWith(TableNode $fields): void {
foreach ($fields->getRowsHash() as $expectedField => $expectedValue) {
if (!array_key_exists($expectedField, $this->lastExternalStorageData)) {
Assert::fail("$expectedField was not found in response");
}
Assert::assertEquals($expectedValue, $this->lastExternalStorageData[$expectedField], "Field '$expectedField' does not match ({$this->lastExternalStorageData[$expectedField]})");
}
}
private function sendJsonWithRequestToken(string $method, string $url, TableNode $fields): void {
$isFirstField = true;
$fieldsAsJsonString = '{';
foreach ($fields->getRowsHash() as $key => $value) {
$fieldsAsJsonString .= ($isFirstField ? '' : ',') . '"' . $key . '":' . $value;
$isFirstField = false;
}
$fieldsAsJsonString .= '}';
$body = [
'headers' => [
'Content-Type' => 'application/json',
],
'body' => $fieldsAsJsonString,
];
$this->sendingAToWithRequesttoken($method, $url, $body);
}
}

View file

@ -15,6 +15,7 @@ require __DIR__ . '/../../vendor/autoload.php';
*/
class FeatureContext implements Context, SnippetAcceptingContext {
use ContactsMenu;
use ExternalStorage;
use Search;
use WebDav;
use Trashbin;

View file

@ -63,3 +63,65 @@ Feature: external-storage
Then as "user1" the file "/local_storage/foo2/textfile0.txt" does not exist
And as "user0" the file "/local_storage/foo2/textfile0.txt" does not exist
And as "user1" the file "/local.txt" exists
Scenario: Save an external storage with password provided by user
Given Logging in using web as "admin"
And logged in user creates external global storage
| mountPoint | "ExternalStorageTest" |
| backend | "owncloud" |
| authMechanism | "password::userprovided" |
| backendOptions | {"host":"http://localhost:8080","secure":false} |
And fields of last external storage match with
| status | 2 |
When logged in user updates last external userglobal storage
| backendOptions | {"user":"admin","password":"admin"} |
Then fields of last external storage match with
| status | 0 |
Scenario: Save an external storage again with an unmodified password provided by user
Given Logging in using web as "admin"
And logged in user creates external global storage
| mountPoint | "ExternalStorageTest" |
| backend | "owncloud" |
| authMechanism | "password::userprovided" |
| backendOptions | {"host":"http://localhost:8080","secure":false} |
And fields of last external storage match with
| status | 2 |
And logged in user updates last external userglobal storage
| backendOptions | {"user":"admin","password":"admin"} |
When logged in user updates last external userglobal storage
| backendOptions | {"user":"admin","password":"__unmodified__"} |
Then fields of last external storage match with
| status | 0 |
Scenario: Save an external storage with global credentials provided by user
Given Logging in using web as "admin"
And logged in user creates external global storage
| mountPoint | "ExternalStorageTest" |
| backend | "owncloud" |
| authMechanism | "password::global::user" |
| backendOptions | {"host":"http://localhost:8080","secure":false} |
And fields of last external storage match with
| status | 2 |
When logged in user updates last external userglobal storage
| backendOptions | {"user":"admin","password":"admin"} |
Then fields of last external storage match with
| status | 0 |
Scenario: Save an external storage again with unmodified global credentials provided by user
Given Logging in using web as "admin"
And logged in user creates external global storage
| mountPoint | "ExternalStorageTest" |
| backend | "owncloud" |
| authMechanism | "password::global::user" |
| backendOptions | {"host":"http://localhost:8080","secure":false} |
And fields of last external storage match with
| status | 2 |
And logged in user updates last external userglobal storage
| backendOptions | {"user":"admin","password":"admin"} |
When logged in user updates last external userglobal storage
| backendOptions | {"user":"admin","password":"__unmodified__"} |
Then fields of last external storage match with
| status | 0 |

View file

@ -41,7 +41,7 @@ echo $PORT
echo "" > phpserver.log
php -S localhost:$PORT -t ../.. &> phpserver.log &
PHP_CLI_SERVER_WORKERS=2 php -S localhost:$PORT -t ../.. &> phpserver.log &
PHPPID=$!
echo $PHPPID