From 6bd49e256b22fd21c5a0f815f54ea6c145345f39 Mon Sep 17 00:00:00 2001
From: Vincent Petry <vincent@nextcloud.com>
Date: Mon, 19 Apr 2021 15:25:26 +0200
Subject: [PATCH] Fix empty password check for mail shares

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
---
 build/psalm-baseline.xml          | 1 +
 lib/private/Share20/Manager.php   | 9 +++++++--
 tests/lib/Share20/ManagerTest.php | 4 ++--
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
index a82e1b81c0..dc2726c7c3 100644
--- a/build/psalm-baseline.xml
+++ b/build/psalm-baseline.xml
@@ -5427,6 +5427,7 @@
     </UndefinedInterfaceMethod>
   </file>
   <file src="lib/private/Share20/Manager.php">
+    <NullArgument occurrences="1"/>
     <InvalidArgument occurrences="7">
       <code>'OCP\Share::preShare'</code>
       <code>'OCP\Share::postShare'</code>
diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index c25a98a91f..5c1f61e040 100644
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -1007,7 +1007,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()) {
@@ -1115,9 +1116,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 12425683e7..2c8d8563a6 100644
--- a/tests/lib/Share20/ManagerTest.php
+++ b/tests/lib/Share20/ManagerTest.php
@@ -3511,7 +3511,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');
@@ -3583,7 +3583,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');