Skip to content

Commit

Permalink
Merge pull request #46859 from nextcloud/fix-status-check-and-saving-…
Browse files Browse the repository at this point in the history
…of-external-storages
  • Loading branch information
skjnldsv authored Sep 4, 2024
2 parents ab511e8 + fa0862c commit f1706df
Show file tree
Hide file tree
Showing 12 changed files with 214 additions and 16 deletions.
2 changes: 1 addition & 1 deletion apps/files_external/css/settings.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion apps/files_external/css/settings.css.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions apps/files_external/css/settings.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
40 changes: 29 additions & 11 deletions apps/files_external/js/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
});
}
Expand All @@ -1254,19 +1265,20 @@ 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)) {
callback(storage);
}
}
},
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);
}
}
});
Expand All @@ -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);
}
});
},
Expand All @@ -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');
Expand All @@ -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);
},

/**
Expand Down
7 changes: 7 additions & 0 deletions apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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;
Expand Down Expand Up @@ -41,6 +42,12 @@ public function saveBackendOptions(IUser $user, $id, $backendOptions) {
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'],
Expand Down
5 changes: 5 additions & 0 deletions apps/files_external/lib/Lib/Auth/Password/UserProvided.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ private function getCredentialsIdentifier($storageId) {
}

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
Expand Down
2 changes: 1 addition & 1 deletion apps/files_external/templates/settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')); ?>">
Expand Down
2 changes: 1 addition & 1 deletion build/integration/features/bootstrap/BasicStructure.php
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ public function sendingAToWithRequesttoken($method, $url, $body = null) {
$fd = $body->getRowsHash();
$options['form_params'] = $fd;
} elseif ($body) {
$options = array_merge($options, $body);
$options = array_merge_recursive($options, $body);
}

$client = new Client();
Expand Down
103 changes: 103 additions & 0 deletions build/integration/features/bootstrap/ExternalStorage.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
1 change: 1 addition & 0 deletions build/integration/features/bootstrap/FeatureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
class FeatureContext implements Context, SnippetAcceptingContext {
use ContactsMenu;
use ExternalStorage;
use Search;
use WebDav;
use Trashbin;
Expand Down
Loading

0 comments on commit f1706df

Please sign in to comment.