Fix empty password check for mail shares

Signed-off-by: Vincent Petry <vincent@nextcloud.com>

diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
index 65a317495b30ae694bf868f98e68d2396d2a9208..868e84409aca5644a4d397bb00d4ad9947f99a01 100644 (file)
--- a/build/psalm-baseline.xml
+++ b/build/psalm-baseline.xml
@@ -4851,6 +4851,7 @@
     </UndefinedInterfaceMethod>
   </file>
   <file src="lib/private/Share20/Manager.php">
+    <NullArgument occurrences="1"/>
     <InvalidArgument occurrences="7">
       <code>$data</code>
       <code>'OCP\Share::postAcceptShare'</code>

diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index 9df544bb628adf0ac740fdf3ed65dfd6919e8608..1fb3bcdc36cb339d8789880f5f3efc59ce7f99c6 100644 (file)
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -1008,7 +1008,8 @@ class Manager implements IManager {
                        // The new password is not set again if it is the same as the old
                        // one.
                        $plainTextPassword = $share->getPassword();
-                       if (!empty($plainTextPassword) && !$this->updateSharePasswordIfNeeded($share, $originalShare)) {
+                       $updatedPassword = $this->updateSharePasswordIfNeeded($share, $originalShare);
+                       if (!empty($plainTextPassword) && !$updatedPassword) {
                                $plainTextPassword = null;
                        }
                        if (empty($plainTextPassword) && !$originalShare->getSendPasswordByTalk() && $share->getSendPasswordByTalk()) {
@@ -1116,9 +1117,13 @@ class Manager implements IManager {
                        $this->verifyPassword($share->getPassword());
 
                        // If a password is set. Hash it!
-                       if ($share->getPassword() !== null) {
+                       if (!empty($share->getPassword())) {
                                $share->setPassword($this->hasher->hash($share->getPassword()));
 
+                               return true;
+                       } else {
+                               // Empty string and null are seen as NOT password protected
+                               $share->setPassword(null);
                                return true;
                        }
                } else {

diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php
index c70887ec87dc83e7c8f657575db108d80b0152b4..33d960efb01da4d992f921e5b0d04f8370b3a0dd 100644 (file)
--- a/tests/lib/Share20/ManagerTest.php
+++ b/tests/lib/Share20/ManagerTest.php
@@ -3515,7 +3515,7 @@ class ManagerTest extends \Test\TestCase {
                $manager->expects($this->once())->method('canShare')->willReturn(true);
                $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare);
                $manager->expects($this->once())->method('generalCreateChecks')->with($share);
-               $manager->expects($this->never())->method('verifyPassword');
+               $manager->expects($this->once())->method('verifyPassword');
                $manager->expects($this->never())->method('pathCreateChecks');
                $manager->expects($this->never())->method('linkCreateChecks');
                $manager->expects($this->never())->method('validateExpirationDateLink');
@@ -3587,7 +3587,7 @@ class ManagerTest extends \Test\TestCase {
                $manager->expects($this->once())->method('canShare')->willReturn(true);
                $manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare);
                $manager->expects($this->once())->method('generalCreateChecks')->with($share);
-               $manager->expects($this->never())->method('verifyPassword');
+               $manager->expects($this->once())->method('verifyPassword');
                $manager->expects($this->never())->method('pathCreateChecks');
                $manager->expects($this->never())->method('linkCreateChecks');
                $manager->expects($this->never())->method('validateExpirationDateLink');