fix: Fix unmodified placeholder replacing the actual value when updating

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 <danxuliu@gmail.com>

diff --git a/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php b/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php
index 6312a3d136e78b727ec835d0cb5643e922a99c80..bf43ceebde178577234a0e6a913bd21465eb75a1 100644 (file)
--- a/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php
+++ b/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php
@@ -29,6 +29,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;
@@ -61,6 +62,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'],

diff --git a/apps/files_external/lib/Lib/Auth/Password/UserProvided.php b/apps/files_external/lib/Lib/Auth/Password/UserProvided.php
index 0c8140e3c141c4ecc08442ef673c512c544ce7f1..3c4163926b69d64c4327d1d94cbda786bfc9d78f 100644 (file)
--- a/apps/files_external/lib/Lib/Auth/Password/UserProvided.php
+++ b/apps/files_external/lib/Lib/Auth/Password/UserProvided.php
@@ -65,6 +65,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

diff --git a/build/integration/features/external-storage.feature b/build/integration/features/external-storage.feature
index d62c7a4f5c712f86cf663ec8d58fe6f5ca80a63b..2b724be44b03c155950bc05dfc00dac7db5b752c 100644 (file)
--- a/build/integration/features/external-storage.feature
+++ b/build/integration/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 |