From 5ed377e0035098828187748c8cba0e3c60f9f817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 30 Jul 2024 03:05:27 +0200 Subject: [PATCH] fix: Fix unmodified placeholder replacing the actual value when updating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When updating global storages and user storages a property is not updated by "StoragesService::updateStorage()" if the value matches the unmodified placeholder. However, userglobal storages are not updated through the "StoragesService"; as only the authentication mechanism is updated it is directly done with "saveBackendOptions()" in "IUserProvided" or "UserGlobalAuth". Due to this the unmodified placeholder value needs to be explicitly checked in those cases and replaced by the actual value (note that in this case it is not possible to just skip updating a specific property). Signed-off-by: Daniel Calviño Sánchez --- .../lib/Lib/Auth/Password/UserGlobalAuth.php | 7 ++++ .../lib/Lib/Auth/Password/UserProvided.php | 5 +++ .../files_features/external-storage.feature | 32 +++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php b/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php index d43c4327b6ba5..d91ebba274428 100644 --- a/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php +++ b/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php @@ -29,6 +29,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; @@ -61,6 +62,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'], diff --git a/apps/files_external/lib/Lib/Auth/Password/UserProvided.php b/apps/files_external/lib/Lib/Auth/Password/UserProvided.php index 7fcbdb383368e..61aa208e78d51 100644 --- a/apps/files_external/lib/Lib/Auth/Password/UserProvided.php +++ b/apps/files_external/lib/Lib/Auth/Password/UserProvided.php @@ -65,6 +65,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 diff --git a/build/integration/files_features/external-storage.feature b/build/integration/files_features/external-storage.feature index d62c7a4f5c712..2b724be44b03c 100644 --- a/build/integration/files_features/external-storage.feature +++ b/build/integration/files_features/external-storage.feature @@ -77,6 +77,22 @@ Feature: external-storage 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 @@ -90,3 +106,19 @@ Feature: external-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 |