Merge pull request #21143 from nextcloud/fix-password-changes-in-link-and-mail-shares

Fix password changes in link and mail shares

.drone.yml

@@ -1359,6 +1359,37 @@ trigger:
     - pull_request
     - push
 
+---
+kind: pipeline
+name: integration-sharing-v1-video-verification
+
+steps:
+- name: submodules
+  image: docker:git
+  commands:
+    - git submodule update --init
+- name: install-talk
+  image: docker:git
+  commands:
+    # JavaScript files are not used in integration tests so it is not needed to
+    # build them.
+    - git clone https://github.com/nextcloud/spreed apps/spreed
+- name: integration-sharing-v1-video-verification
+  image: nextcloudci/integration-php7.3:integration-php7.3-2
+  commands:
+    - bash tests/drone-run-integration-tests.sh || exit 0
+    - ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
+    - cd build/integration
+    - ./run.sh sharing_features/sharing-v1-video-verification.feature
+
+trigger:
+  branch:
+    - master
+    - stable*
+  event:
+    - pull_request
+    - push
+
 ---
 kind: pipeline
 name: integration-setup-features

apps/sharebymail/lib/ShareByMailProvider.php

@@ -187,12 +187,16 @@ class ShareByMailProvider implements IShareProvider {
 
 		// if the admin enforces a password for all mail shares we create a
 		// random password and send it to the recipient
-		$password = '';
+		$password = $share->getPassword() ?: '';
 		$passwordEnforced = $this->settingsManager->enforcePasswordProtection();
-		if ($passwordEnforced) {
+		if ($passwordEnforced && empty($password)) {
 			$password = $this->autoGeneratePassword($share);
 		}
 
+		if (!empty($password)) {
+			$share->setPassword($this->hasher->hash($password));
+		}
+
 		$shareId = $this->createMailShare($share);
 		$send = $this->sendPassword($share, $password);
 		if ($passwordEnforced && $send === false) {
@@ -233,8 +237,6 @@ class ShareByMailProvider implements IShareProvider {
 
 		$password = $this->secureRandom->generate($passwordLength, $passwordCharset);
 
-		$share->setPassword($this->hasher->hash($password));
-
 		return $password;
 	}
 

apps/sharebymail/tests/ShareByMailProviderTest.php

@@ -240,6 +240,51 @@ class ShareByMailProviderTest extends TestCase {
 		);
 	}
 
+	public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswordProtection() {
+		$share = $this->getMockBuilder(IShare::class)->getMock();
+		$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
+		$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
+		$share->expects($this->any())->method('getSharedBy')->willReturn('owner');
+
+		$node = $this->getMockBuilder(File::class)->getMock();
+		$node->expects($this->any())->method('getName')->willReturn('filename');
+
+		$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity']);
+
+		$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
+		$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
+		$instance->expects($this->once())->method('createShareActivity')->with($share);
+		$instance->expects($this->once())->method('getRawShare')->with(42)->willReturn('rawShare');
+		$instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject');
+		$share->expects($this->any())->method('getNode')->willReturn($node);
+
+		$share->expects($this->once())->method('getPassword')->willReturn('password');
+		$this->hasher->expects($this->once())->method('hash')->with('password')->willReturn('passwordHashed');
+		$share->expects($this->once())->method('setPassword')->with('passwordHashed');
+
+		// The given password (but not the autogenerated password) should be
+		// mailed to the receiver of the share.
+		$this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(false);
+		$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
+		$instance->expects($this->never())->method('autoGeneratePassword');
+
+		$message = $this->createMock(IMessage::class);
+		$message->expects($this->once())->method('setTo')->with(['receiver@example.com']);
+		$this->mailer->expects($this->once())->method('createMessage')->willReturn($message);
+		$this->mailer->expects($this->once())->method('createEMailTemplate')->with('sharebymail.RecipientPasswordNotification', [
+			'filename' => 'filename',
+			'password' => 'password',
+			'initiator' => 'owner',
+			'initiatorEmail' => null,
+			'shareWith' => 'receiver@example.com',
+		]);
+		$this->mailer->expects($this->once())->method('send');
+
+		$this->assertSame('shareObject',
+			$instance->create($share)
+		);
+	}
+
 	public function testCreateSendPasswordByMailWithEnforcedPasswordProtection() {
 		$share = $this->getMockBuilder(IShare::class)->getMock();
 		$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
@@ -258,14 +303,70 @@ class ShareByMailProviderTest extends TestCase {
 		$instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject');
 		$share->expects($this->any())->method('getNode')->willReturn($node);
 
+		$share->expects($this->once())->method('getPassword')->willReturn(null);
+		$this->hasher->expects($this->once())->method('hash')->with('autogeneratedPassword')->willReturn('autogeneratedPasswordHashed');
+		$share->expects($this->once())->method('setPassword')->with('autogeneratedPasswordHashed');
+
 		// The autogenerated password should be mailed to the receiver of the share.
 		$this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(true);
 		$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
-		$instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('password');
+		$instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('autogeneratedPassword');
 
 		$message = $this->createMock(IMessage::class);
 		$message->expects($this->once())->method('setTo')->with(['receiver@example.com']);
 		$this->mailer->expects($this->once())->method('createMessage')->willReturn($message);
+		$this->mailer->expects($this->once())->method('createEMailTemplate')->with('sharebymail.RecipientPasswordNotification', [
+			'filename' => 'filename',
+			'password' => 'autogeneratedPassword',
+			'initiator' => 'owner',
+			'initiatorEmail' => null,
+			'shareWith' => 'receiver@example.com',
+		]);
+		$this->mailer->expects($this->once())->method('send');
+
+		$this->assertSame('shareObject',
+			$instance->create($share)
+		);
+	}
+
+	public function testCreateSendPasswordByMailWithPasswordAndWithEnforcedPasswordProtection() {
+		$share = $this->getMockBuilder(IShare::class)->getMock();
+		$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
+		$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
+		$share->expects($this->any())->method('getSharedBy')->willReturn('owner');
+
+		$node = $this->getMockBuilder(File::class)->getMock();
+		$node->expects($this->any())->method('getName')->willReturn('filename');
+
+		$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity']);
+
+		$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
+		$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
+		$instance->expects($this->once())->method('createShareActivity')->with($share);
+		$instance->expects($this->once())->method('getRawShare')->with(42)->willReturn('rawShare');
+		$instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject');
+		$share->expects($this->any())->method('getNode')->willReturn($node);
+
+		$share->expects($this->once())->method('getPassword')->willReturn('password');
+		$this->hasher->expects($this->once())->method('hash')->with('password')->willReturn('passwordHashed');
+		$share->expects($this->once())->method('setPassword')->with('passwordHashed');
+
+		// The given password (but not the autogenerated password) should be
+		// mailed to the receiver of the share.
+		$this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(true);
+		$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
+		$instance->expects($this->never())->method('autoGeneratePassword');
+
+		$message = $this->createMock(IMessage::class);
+		$message->expects($this->once())->method('setTo')->with(['receiver@example.com']);
+		$this->mailer->expects($this->once())->method('createMessage')->willReturn($message);
+		$this->mailer->expects($this->once())->method('createEMailTemplate')->with('sharebymail.RecipientPasswordNotification', [
+			'filename' => 'filename',
+			'password' => 'password',
+			'initiator' => 'owner',
+			'initiatorEmail' => null,
+			'shareWith' => 'receiver@example.com',
+		]);
 		$this->mailer->expects($this->once())->method('send');
 
 		$this->assertSame('shareObject',
@@ -291,14 +392,25 @@ class ShareByMailProviderTest extends TestCase {
 		$instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject');
 		$share->expects($this->any())->method('getNode')->willReturn($node);
 
+		$share->expects($this->once())->method('getPassword')->willReturn(null);
+		$this->hasher->expects($this->once())->method('hash')->with('autogeneratedPassword')->willReturn('autogeneratedPasswordHashed');
+		$share->expects($this->once())->method('setPassword')->with('autogeneratedPasswordHashed');
+
 		// The autogenerated password should be mailed to the owner of the share.
 		$this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(true);
 		$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
-		$instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('password');
+		$instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('autogeneratedPassword');
 
 		$message = $this->createMock(IMessage::class);
 		$message->expects($this->once())->method('setTo')->with(['owner@example.com' => 'Owner display name']);
 		$this->mailer->expects($this->once())->method('createMessage')->willReturn($message);
+		$this->mailer->expects($this->once())->method('createEMailTemplate')->with('sharebymail.OwnerPasswordNotification', [
+			'filename' => 'filename',
+			'password' => 'autogeneratedPassword',
+			'initiator' => 'Owner display name',
+			'initiatorEmail' => 'owner@example.com',
+			'shareWith' => 'receiver@example.com',
+		]);
 		$this->mailer->expects($this->once())->method('send');
 
 		$user = $this->createMock(IUser::class);
@@ -527,6 +639,13 @@ class ShareByMailProviderTest extends TestCase {
 		$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn($newSendPasswordByTalk);
 
 		if ($sendMail) {
+			$this->mailer->expects($this->once())->method('createEMailTemplate')->with('sharebymail.RecipientPasswordNotification', [
+				'filename' => 'filename',
+				'password' => $plainTextPassword,
+				'initiator' => null,
+				'initiatorEmail' => null,
+				'shareWith' => 'receiver@example.com',
+			]);
 			$this->mailer->expects($this->once())->method('send');
 		} else {
 			$this->mailer->expects($this->never())->method('send');

build/integration/config/behat.yml

@@ -65,6 +65,7 @@ default:
               - admin
               - admin
             regular_user_password: 123456
+        - TalkContext
     setup:
       paths:
         - "%paths.base%/../setup_features"

build/integration/features/bootstrap/BasicStructure.php

@@ -45,6 +45,7 @@ require __DIR__ . '/../../vendor/autoload.php';
 trait BasicStructure {
 	use Auth;
 	use Download;
+	use Mail;
 	use Trashbin;
 
 	/** @var string */

build/integration/features/bootstrap/FakeSMTPHelper.php

@@ -0,0 +1,159 @@
+<?php
+
+// Code below modified from https://github.com/axllent/fake-smtp/blob/f0856f8a0df6f4ca5a573cf31428c09ebc5b9ea3/fakeSMTP.php,
+// which is under the MIT license (https://github.com/axllent/fake-smtp/blob/f0856f8a0df6f4ca5a573cf31428c09ebc5b9ea3/LICENSE)
+
+/**
+ * fakeSMTP - A PHP / inetd fake smtp server.
+ * Allows client<->server interaction
+ * The comunication is based upon the SMPT standards defined in http://www.lesnikowski.com/mail/Rfc/rfc2821.txt
+ */
+
+class fakeSMTP {
+	public $logFile = false;
+	public $serverHello = 'fakeSMTP ESMTP PHP Mail Server Ready';
+
+	public function __construct($fd) {
+		$this->mail = [];
+		$this->mail['ipaddress'] = false;
+		$this->mail['emailSender'] = '';
+		$this->mail['emailRecipients'] = [];
+		$this->mail['emailSubject'] = false;
+		$this->mail['rawEmail'] = false;
+		$this->mail['emailHeaders'] = false;
+		$this->mail['emailBody'] = false;
+
+		$this->fd = $fd;
+	}
+
+	public function receive() {
+		$hasValidFrom = false;
+		$hasValidTo = false;
+		$receivingData = false;
+		$header = true;
+		$this->reply('220 '.$this->serverHello);
+		$this->mail['ipaddress'] = $this->detectIP();
+		while ($data = fgets($this->fd)) {
+			$data = preg_replace('@\r\n@', "\n", $data);
+
+			if (!$receivingData) {
+				$this->log($data);
+			}
+
+			if (!$receivingData && preg_match('/^MAIL FROM:\s?<(.*)>/i', $data, $match)) {
+				if (preg_match('/(.*)@\[.*\]/i', $match[1]) || $match[1] != '' || $this->validateEmail($match[1])) {
+					$this->mail['emailSender'] = $match[1];
+					$this->reply('250 2.1.0 Ok');
+					$hasValidFrom = true;
+				} else {
+					$this->reply('551 5.1.7 Bad sender address syntax');
+				}
+			} elseif (!$receivingData && preg_match('/^RCPT TO:\s?<(.*)>/i', $data, $match)) {
+				if (!$hasValidFrom) {
+					$this->reply('503 5.5.1 Error: need MAIL command');
+				} else {
+					if (preg_match('/postmaster@\[.*\]/i', $match[1]) || $this->validateEmail($match[1])) {
+						array_push($this->mail['emailRecipients'], $match[1]);
+						$this->reply('250 2.1.5 Ok');
+						$hasValidTo = true;
+					} else {
+						$this->reply('501 5.1.3 Bad recipient address syntax '.$match[1]);
+					}
+				}
+			} elseif (!$receivingData && preg_match('/^RSET$/i', trim($data))) {
+				$this->reply('250 2.0.0 Ok');
+				$hasValidFrom = false;
+				$hasValidTo = false;
+			} elseif (!$receivingData && preg_match('/^NOOP$/i', trim($data))) {
+				$this->reply('250 2.0.0 Ok');
+			} elseif (!$receivingData && preg_match('/^VRFY (.*)/i', trim($data), $match)) {
+				$this->reply('250 2.0.0 '.$match[1]);
+			} elseif (!$receivingData && preg_match('/^DATA/i', trim($data))) {
+				if (!$hasValidTo) {
+					$this->reply('503 5.5.1 Error: need RCPT command');
+				} else {
+					$this->reply('354 Ok Send data ending with <CRLF>.<CRLF>');
+					$receivingData = true;
+				}
+			} elseif (!$receivingData && preg_match('/^(HELO|EHLO)/i', $data)) {
+				$this->reply('250 HELO '.$this->mail['ipaddress']);
+			} elseif (!$receivingData && preg_match('/^QUIT/i', trim($data))) {
+				break;
+			} elseif (!$receivingData) {
+				//~ $this->reply('250 Ok');
+				$this->reply('502 5.5.2 Error: command not recognized');
+			} elseif ($receivingData && $data == ".\n") {
+				/* Email Received, now let's look at it */
+				$receivingData = false;
+				$this->reply('250 2.0.0 Ok: queued as '.$this->generateRandom(10));
+				$splitmail = explode("\n\n", $this->mail['rawEmail'], 2);
+				if (count($splitmail) == 2) {
+					$this->mail['emailHeaders'] = $splitmail[0];
+					$this->mail['emailBody'] = $splitmail[1];
+					$headers = preg_replace("/ \s+/", ' ', preg_replace("/\n\s/", ' ', $this->mail['emailHeaders']));
+					$headerlines = explode("\n", $headers);
+					for ($i=0; $i<count($headerlines); $i++) {
+						if (preg_match('/^Subject: (.*)/i', $headerlines[$i], $matches)) {
+							$this->mail['emailSubject'] = trim($matches[1]);
+						}
+					}
+				} else {
+					$this->mail['emailBody'] = $splitmail[0];
+				}
+				set_time_limit(5); // Just run the exit to prevent open threads / abuse
+			} elseif ($receivingData) {
+				$this->mail['rawEmail'] .= $data;
+			}
+		}
+		/* Say good bye */
+		$this->reply('221 2.0.0 Bye '.$this->mail['ipaddress']);
+
+		fclose($this->fd);
+	}
+
+	public function log($s) {
+		if ($this->logFile) {
+			file_put_contents($this->logFile, trim($s)."\n", FILE_APPEND);
+		}
+	}
+
+	private function reply($s) {
+		$this->log("REPLY:$s");
+		fwrite($this->fd, $s . "\r\n");
+	}
+
+	private function detectIP() {
+		$raw = explode(':', stream_socket_get_name($this->fd, true));
+		return $raw[0];
+	}
+
+	private function validateEmail($email) {
+		return preg_match('/^[_a-z0-9-+]+(\.[_a-z0-9-+]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,4})$/', strtolower($email));
+	}
+
+	private function generateRandom($length=8) {
+		$password = '';
+		$possible = '2346789BCDFGHJKLMNPQRTVWXYZ';
+		$maxlength = strlen($possible);
+		$i = 0;
+		for ($i=0; $i < $length; $i++) {
+			$char = substr($possible, mt_rand(0, $maxlength-1), 1);
+			if (!strstr($password, $char)) {
+				$password .= $char;
+			}
+		}
+		return $password;
+	}
+}
+
+$socket = stream_socket_server('tcp://127.0.0.1:2525', $errno, $errstr);
+if (!$socket) {
+	exit();
+}
+
+while ($fd = stream_socket_accept($socket)) {
+	$fakeSMTP = new fakeSMTP($fd);
+	$fakeSMTP->receive();
+}
+
+fclose($socket);

build/integration/features/bootstrap/Mail.php

@@ -0,0 +1,57 @@
+<?php
+/**
+ * @copyright Copyright (c) 2020, Daniel Calviño Sánchez (danxuliu@gmail.com)
+ *
+ * @author Daniel Calviño Sánchez <danxuliu@gmail.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+trait Mail {
+
+	// CommandLine trait is expected to be used in the class that uses this
+	// trait.
+
+	/**
+	 * @var string
+	 */
+	private $fakeSmtpServerPid;
+
+	/**
+	 * @AfterScenario
+	 */
+	public function killDummyMailServer() {
+		if (!$this->fakeSmtpServerPid) {
+			return;
+		}
+
+		exec("kill " . $this->fakeSmtpServerPid);
+
+		$this->invokingTheCommand('config:system:delete mail_smtpport');
+	}
+
+	/**
+	 * @Given /^dummy mail server is listening$/
+	 */
+	public function dummyMailServerIsListening() {
+		// Default smtpport (25) is restricted for regular users, so the
+		// FakeSMTP uses 2525 instead.
+		$this->invokingTheCommand('config:system:set mail_smtpport --value=2525 --type integer');
+
+		$this->fakeSmtpServerPid = exec("php features/bootstrap/FakeSMTPHelper.php >/dev/null 2>&1 & echo $!");
+	}
+}

build/integration/features/bootstrap/Sharing.php

@@ -151,11 +151,9 @@ trait Sharing {
 	}
 
 	/**
-	 * @Then /^Public shared file "([^"]*)" can be downloaded$/
+	 * @Then /^last link share can be downloaded$/
 	 */
-	public function checkPublicSharedFile($filename) {
-		$client = new Client();
-		$options = [];
+	public function lastLinkShareCanBeDownloaded() {
 		if (count($this->lastShareData->data->element) > 0) {
 			$url = $this->lastShareData->data[0]->url;
 		} else {
@@ -166,10 +164,23 @@ trait Sharing {
 	}
 
 	/**
-	 * @Then /^Public shared file "([^"]*)" with password "([^"]*)" can be downloaded$/
+	 * @Then /^last share can be downloaded$/
 	 */
-	public function checkPublicSharedFileWithPassword($filename, $password) {
-		$options = [];
+	public function lastShareCanBeDownloaded() {
+		if (count($this->lastShareData->data->element) > 0) {
+			$token = $this->lastShareData->data[0]->token;
+		} else {
+			$token = $this->lastShareData->data->token;
+		}
+
+		$fullUrl = substr($this->baseUrl, 0, -4) . "index.php/s/" . $token . "/download";
+		$this->checkDownload($fullUrl, null, 'text/plain');
+	}
+
+	/**
+	 * @Then /^last share with password "([^"]*)" can be downloaded$/
+	 */
+	public function lastShareWithPasswordCanBeDownloaded($password) {
 		if (count($this->lastShareData->data->element) > 0) {
 			$token = $this->lastShareData->data[0]->token;
 		} else {

build/integration/features/bootstrap/SharingContext.php

@@ -33,7 +33,9 @@ require __DIR__ . '/../../vendor/autoload.php';
 class SharingContext implements Context, SnippetAcceptingContext {
 	use Sharing;
 	use AppConfiguration;
+	use CommandLine;
 
 	protected function resetAppConfigs() {
+		$this->modifyServerConfig('sharebymail', 'enforcePasswordProtection', 'no');
 	}
 }

build/integration/features/bootstrap/TalkContext.php

@@ -0,0 +1,72 @@
+<?php
+/**
+ * @copyright Copyright (c) 2020, Daniel Calviño Sánchez (danxuliu@gmail.com)
+ *
+ * @author Daniel Calviño Sánchez <danxuliu@gmail.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+use Behat\Behat\Context\Context;
+
+class TalkContext implements Context {
+
+	/**
+	 * @BeforeFeature @Talk
+	 * @BeforeScenario @Talk
+	 */
+	public static function skipTestsIfTalkIsNotInstalled() {
+		if (!TalkContext::isTalkInstalled()) {
+			throw new Exception('Talk needs to be installed to run features or scenarios tagged with @Talk');
+		}
+	}
+
+	/**
+	 * @AfterScenario @Talk
+	 */
+	public static function disableTalk() {
+		TalkContext::runOcc(['app:disable', 'spreed']);
+	}
+
+	private static function isTalkInstalled(): bool {
+		$appList = TalkContext::runOcc(['app:list']);
+
+		return strpos($appList, 'spreed') !== false;
+	}
+
+	private static function runOcc(array $args): string {
+		// Based on "runOcc" from CommandLine trait (which can not be used due
+		// to not being static and being already used in other sibling
+		// contexts).
+		$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);
+
+		return $lastStdOut;
+	}
+}

build/integration/sharing_features/sharing-v1-video-verification.feature

@@ -0,0 +1,504 @@
+@Talk
+Feature: sharing
+  Background:
+    Given using api version "1"
+    Given using old dav path
+    Given invoking occ with "app:enable spreed"
+
+  Scenario: Creating a link share with send password by Talk
+    Given user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+      | password | secret |
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Enabling send password by Talk in a link share
+    Given user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Enabling send password by Talk with different password in a link share
+    Given user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+      | password | secret |
+    And Updating last share with
+      | password | another secret |
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "another secret" can be downloaded
+
+  Scenario: Enabling send password by Talk with different password set after creation in a link share
+    Given user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+    And Updating last share with
+      | password | secret |
+    And Updating last share with
+      | password | another secret |
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "another secret" can be downloaded
+
+  Scenario: Enabling send password by Talk with same password in a link share
+    Given user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+      | password | secret |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Enabling send password by Talk with same password set after creation in a link share
+    Given user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+    And Updating last share with
+      | password | secret |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Enabling send password by Talk without updating password in a link share
+    Given user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+      | password | secret |
+    And Updating last share with
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Enabling send password by Talk without updating password set after creation in a link share
+    Given user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+    And Updating last share with
+      | password | secret |
+    And Updating last share with
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Enabling send password by Talk with no password in a link share
+    Given user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+    And Updating last share with
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "200"
+    And last share can be downloaded
+
+  Scenario: Enabling send password by Talk with no password removed after creation in a link share
+    Given user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+      | password | secret |
+    And Updating last share with
+      | password | |
+    And Updating last share with
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "200"
+    And last share can be downloaded
+
+  Scenario: Disabling send password by Talk without setting new password in a link share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+      | password | secret |
+      | sendPasswordByTalk | true |
+    And Updating last share with
+      | sendPasswordByTalk | false |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Disabling send password by Talk without setting new password set after creation in a link share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | true |
+    And Updating last share with
+      | sendPasswordByTalk | false |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Disabling send password by Talk setting same password in a link share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+      | password | secret |
+      | sendPasswordByTalk | true |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | false |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Disabling send password by Talk setting same password set after creation in a link share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | true |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | false |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Disabling send password by Talk setting new password in a link share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+      | password | secret |
+      | sendPasswordByTalk | true |
+    And Updating last share with
+      | password | another secret |
+      | sendPasswordByTalk | false |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "another secret" can be downloaded
+
+  Scenario: Disabling send password by Talk setting new password set after creation in a link share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 3 |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | true |
+    And Updating last share with
+      | password | another secret |
+      | sendPasswordByTalk | false |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "another secret" can be downloaded
+
+
+
+
+
+  Scenario: Creating a mail share with send password by Talk
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+      | password | secret |
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Enabling send password by Talk in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Enabling send password by Talk with different password in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+      | password | secret |
+    And Updating last share with
+      | password | another secret |
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "another secret" can be downloaded
+
+  Scenario: Enabling send password by Talk with different password set after creation in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+    And Updating last share with
+      | password | secret |
+    And Updating last share with
+      | password | another secret |
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "another secret" can be downloaded
+
+  Scenario: Enabling send password by Talk with same password in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+      | password | secret |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Enabling send password by Talk with same password set after creation in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+    And Updating last share with
+      | password | secret |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Enabling send password by Talk without updating password in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+      | password | secret |
+    And Updating last share with
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Enabling send password by Talk without updating password set after creation in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+    And Updating last share with
+      | password | secret |
+    And Updating last share with
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Enabling send password by Talk with no password in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+    And Updating last share with
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "200"
+    And last share can be downloaded
+
+  Scenario: Enabling send password by Talk with no password removed after creation in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+      | password | secret |
+    And Updating last share with
+      | password | |
+    And Updating last share with
+      | sendPasswordByTalk | true |
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "200"
+    And last share can be downloaded
+
+  Scenario: Disabling send password by Talk without setting new password in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+      | password | secret |
+      | sendPasswordByTalk | true |
+    And Updating last share with
+      | sendPasswordByTalk | false |
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Disabling send password by Talk without setting new password set after creation in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | true |
+    And Updating last share with
+      | sendPasswordByTalk | false |
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Disabling send password by Talk setting same password in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+      | password | secret |
+      | sendPasswordByTalk | true |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | false |
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Disabling send password by Talk setting same password set after creation in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | true |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | false |
+    Then the OCS status code should be "400"
+    And the HTTP status code should be "200"
+    And last share with password "secret" can be downloaded
+
+  Scenario: Disabling send password by Talk setting new password in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+      | password | secret |
+      | sendPasswordByTalk | true |
+    And Updating last share with
+      | password | another secret |
+      | sendPasswordByTalk | false |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "another secret" can be downloaded
+
+  Scenario: Disabling send password by Talk setting new password set after creation in a mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dummy@test.com |
+    And Updating last share with
+      | password | secret |
+      | sendPasswordByTalk | true |
+    And Updating last share with
+      | password | another secret |
+      | sendPasswordByTalk | false |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "another secret" can be downloaded

build/integration/sharing_features/sharing-v1.feature

@@ -56,6 +56,92 @@ Feature: sharing
     Then the OCS status code should be "403"
     And the HTTP status code should be "401"
 
+  Scenario: Creating a new mail share
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dumy@test.com |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share can be downloaded
+
+  Scenario: Creating a new mail share with password
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dumy@test.com |
+      | password | publicpw |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "publicpw" can be downloaded
+
+  Scenario: Creating a new mail share with password when password protection is enforced
+    Given dummy mail server is listening
+    And As an "admin"
+    And parameter "enforcePasswordProtection" of app "sharebymail" is set to "yes"
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dumy@test.com |
+      | password | publicpw |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "publicpw" can be downloaded
+
+  Scenario: Creating a new mail share and setting a password
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dumy@test.com |
+    And Updating last share with
+      | password | publicpw |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "publicpw" can be downloaded
+
+  Scenario: Creating a new mail share and setting a password twice
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dumy@test.com |
+    And Updating last share with
+      | password | publicpw |
+    And Updating last share with
+      | password | another publicpw |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "another publicpw" can be downloaded
+
+  Scenario: Creating a new mail share and setting the same password twice
+    Given dummy mail server is listening
+    And user "user0" exists
+    And As an "user0"
+    When creating a share with
+      | path | welcome.txt |
+      | shareType | 4 |
+      | shareWith | dumy@test.com |
+    And Updating last share with
+      | password | publicpw |
+    And Updating last share with
+      | password | publicpw |
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share with password "publicpw" can be downloaded
+
   Scenario: Creating a new public share
     Given user "user0" exists
     And As an "user0"
@@ -64,7 +150,7 @@ Feature: sharing
       | shareType | 3 |
     Then the OCS status code should be "100"
     And the HTTP status code should be "200"
-    And Public shared file "welcome.txt" can be downloaded
+    And last link share can be downloaded
 
   Scenario: Creating a new public share with password
     Given user "user0" exists
@@ -75,7 +161,7 @@ Feature: sharing
       | password | publicpw |
     Then the OCS status code should be "100"
     And the HTTP status code should be "200"
-    And Public shared file "welcome.txt" with password "publicpw" can be downloaded
+    And last share with password "publicpw" can be downloaded
 
   Scenario: Creating a new public share of a folder
    Given user "user0" exists
@@ -108,7 +194,7 @@ Feature: sharing
       | expireDate | +3 days |
     Then the OCS status code should be "100"
     And the HTTP status code should be "200"
-    And Public shared file "welcome.txt" with password "publicpw" can be downloaded
+    And last share with password "publicpw" can be downloaded
 
   Scenario: Creating a new public share, updating its expiration date and getting its info
     Given user "user0" exists

lib/private/Share20/Manager.php

@@ -968,8 +968,14 @@ class Manager implements IManager {
 		} elseif ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) {
 			$this->linkCreateChecks($share);
 
+			$plainTextPassword = $share->getPassword();
+
 			$this->updateSharePasswordIfNeeded($share, $originalShare);
 
+			if (empty($plainTextPassword) && $share->getSendPasswordByTalk()) {
+				throw new \InvalidArgumentException('Can’t enable sending the password by Talk with an empty password');
+			}
+
 			if ($share->getExpirationDate() != $originalShare->getExpirationDate()) {
 				//Verify the expiration date
 				$this->validateExpirationDate($share);
@@ -977,11 +983,9 @@ class Manager implements IManager {
 			}
 		} elseif ($share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL) {
 			// The new password is not set again if it is the same as the old
-			// one, unless when switching from sending by Talk to sending by
-			// mail.
+			// one.
 			$plainTextPassword = $share->getPassword();
-			if (!empty($plainTextPassword) && !$this->updateSharePasswordIfNeeded($share, $originalShare) &&
-					!($originalShare->getSendPasswordByTalk() && !$share->getSendPasswordByTalk())) {
+			if (!empty($plainTextPassword) && !$this->updateSharePasswordIfNeeded($share, $originalShare)) {
 				$plainTextPassword = null;
 			}
 			if (empty($plainTextPassword) && !$originalShare->getSendPasswordByTalk() && $share->getSendPasswordByTalk()) {
@@ -989,6 +993,8 @@ class Manager implements IManager {
 				// would already have access to the share without having to call
 				// the sharer to verify her identity
 				throw new \InvalidArgumentException('Can’t enable sending the password by Talk without setting a new password');
+			} elseif (empty($plainTextPassword) && $originalShare->getSendPasswordByTalk() && !$share->getSendPasswordByTalk()) {
+				throw new \InvalidArgumentException('Can’t disable sending the password by Talk without setting a new password');
 			}
 		}
 
@@ -1075,8 +1081,14 @@ class Manager implements IManager {
 	 * @return boolean whether the password was updated or not.
 	 */
 	private function updateSharePasswordIfNeeded(\OCP\Share\IShare $share, \OCP\Share\IShare $originalShare) {
+		$passwordsAreDifferent = ($share->getPassword() !== $originalShare->getPassword()) &&
+									(($share->getPassword() !== null && $originalShare->getPassword() === null) ||
+									 ($share->getPassword() === null && $originalShare->getPassword() !== null) ||
+									 ($share->getPassword() !== null && $originalShare->getPassword() !== null &&
+										!$this->hasher->verify($share->getPassword(), $originalShare->getPassword())));
+
 		// Password updated.
-		if ($share->getPassword() !== $originalShare->getPassword()) {
+		if ($passwordsAreDifferent) {
 			//Verify the password
 			$this->verifyPassword($share->getPassword());
 
@@ -1086,6 +1098,10 @@ class Manager implements IManager {
 
 				return true;
 			}
+		} else {
+			// Reset the password to the original one, as it is either the same
+			// as the "new" password or a hashed version of it.
+			$share->setPassword($originalShare->getPassword());
 		}
 
 		return false;

tests/lib/Share20/ManagerTest.php

@@ -2741,6 +2741,75 @@ class ManagerTest extends \Test\TestCase {
 		$manager->updateShare($share);
 	}
 
+	public function testUpdateShareLinkEnableSendPasswordByTalkWithNoPassword() {
+		$this->expectException(\InvalidArgumentException::class);
+		$this->expectExceptionMessage('Can’t enable sending the password by Talk with an empty password');
+
+		$manager = $this->createManagerMock()
+			->setMethods([
+				'canShare',
+				'getShareById',
+				'generalCreateChecks',
+				'linkCreateChecks',
+				'pathCreateChecks',
+				'verifyPassword',
+				'validateExpirationDate',
+			])
+			->getMock();
+
+		$originalShare = $this->manager->newShare();
+		$originalShare->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setPermissions(15);
+
+		$tomorrow = new \DateTime();
+		$tomorrow->setTime(0,0,0);
+		$tomorrow->add(new \DateInterval('P1D'));
+
+		$file = $this->createMock(File::class);
+		$file->method('getId')->willReturn(100);
+
+		$share = $this->manager->newShare();
+		$share->setProviderId('foo')
+			->setId('42')
+			->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setToken('token')
+			->setSharedBy('owner')
+			->setShareOwner('owner')
+			->setPassword(null)
+			->setSendPasswordByTalk(true)
+			->setExpirationDate($tomorrow)
+			->setNode($file)
+			->setPermissions(15);
+
+		$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->once())->method('linkCreateChecks')->with($share);
+		$manager->expects($this->never())->method('verifyPassword');
+		$manager->expects($this->never())->method('pathCreateChecks');
+		$manager->expects($this->never())->method('validateExpirationDate');
+
+		$this->hasher->expects($this->never())
+			->method('hash');
+
+		$this->defaultProvider->expects($this->never())
+			->method('update');
+
+		$hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post');
+		$hookListner->expects($this->never())->method('post');
+
+		$hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post');
+		$hookListner2->expects($this->never())->method('post');
+
+		$hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post');
+		$hookListner3->expects($this->never())->method('post');
+
+		$manager->updateShare($share);
+	}
+
 	public function testUpdateShareMail() {
 		$manager = $this->createManagerMock()
 			->setMethods([
@@ -2894,6 +2963,88 @@ class ManagerTest extends \Test\TestCase {
 		$manager->updateShare($share);
 	}
 
+	public function testUpdateShareMailEnableSendPasswordByTalkWithDifferentPassword() {
+		$manager = $this->createManagerMock()
+			->setMethods([
+				'canShare',
+				'getShareById',
+				'generalCreateChecks',
+				'verifyPassword',
+				'pathCreateChecks',
+				'linkCreateChecks',
+				'validateExpirationDate',
+			])
+			->getMock();
+
+		$originalShare = $this->manager->newShare();
+		$originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL)
+			->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setPassword('anotherPasswordHash')
+			->setSendPasswordByTalk(false);
+
+		$tomorrow = new \DateTime();
+		$tomorrow->setTime(0,0,0);
+		$tomorrow->add(new \DateInterval('P1D'));
+
+		$file = $this->createMock(File::class);
+		$file->method('getId')->willReturn(100);
+
+		$share = $this->manager->newShare();
+		$share->setProviderId('foo')
+			->setId('42')
+			->setShareType(\OCP\Share::SHARE_TYPE_EMAIL)
+			->setToken('token')
+			->setSharedBy('owner')
+			->setShareOwner('owner')
+			->setPassword('password')
+			->setSendPasswordByTalk(true)
+			->setExpirationDate($tomorrow)
+			->setNode($file)
+			->setPermissions(\OCP\Constants::PERMISSION_ALL);
+
+		$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->once())->method('verifyPassword')->with('password');
+		$manager->expects($this->once())->method('pathCreateChecks')->with($file);
+		$manager->expects($this->never())->method('linkCreateChecks');
+		$manager->expects($this->never())->method('validateExpirationDate');
+
+		$this->hasher->expects($this->once())
+			->method('verify')
+			->with('password', 'anotherPasswordHash')
+			->willReturn(false);
+
+		$this->hasher->expects($this->once())
+			->method('hash')
+			->with('password')
+			->willReturn('hashed');
+
+		$this->defaultProvider->expects($this->once())
+			->method('update')
+			->with($share, 'password')
+			->willReturn($share);
+
+		$hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post');
+		$hookListner->expects($this->never())->method('post');
+
+		$hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post');
+		$hookListner2->expects($this->once())->method('post')->with([
+			'itemType' => 'file',
+			'itemSource' => 100,
+			'uidOwner' => 'owner',
+			'token' => 'token',
+			'disabled' => false,
+		]);
+
+		$hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post');
+		$hookListner3->expects($this->never())->method('post');
+
+		$manager->updateShare($share);
+	}
 
 	public function testUpdateShareMailEnableSendPasswordByTalkWithNoPassword() {
 		$this->expectException(\InvalidArgumentException::class);
@@ -2986,7 +3137,7 @@ class ManagerTest extends \Test\TestCase {
 		$originalShare = $this->manager->newShare();
 		$originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL)
 			->setPermissions(\OCP\Constants::PERMISSION_ALL)
-			->setPassword('password')
+			->setPassword('passwordHash')
 			->setSendPasswordByTalk(false);
 
 		$tomorrow = new \DateTime();
@@ -3058,7 +3209,7 @@ class ManagerTest extends \Test\TestCase {
 		$originalShare = $this->manager->newShare();
 		$originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL)
 			->setPermissions(\OCP\Constants::PERMISSION_ALL)
-			->setPassword('password')
+			->setPassword('passwordHash')
 			->setSendPasswordByTalk(false);
 
 		$tomorrow = new \DateTime();
@@ -3130,7 +3281,7 @@ class ManagerTest extends \Test\TestCase {
 		$originalShare = $this->manager->newShare();
 		$originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL)
 			->setPermissions(\OCP\Constants::PERMISSION_ALL)
-			->setPassword('password')
+			->setPassword('passwordHash')
 			->setSendPasswordByTalk(false);
 
 		$tomorrow = new \DateTime();
@@ -3161,6 +3312,11 @@ class ManagerTest extends \Test\TestCase {
 		$manager->expects($this->never())->method('linkCreateChecks');
 		$manager->expects($this->never())->method('validateExpirationDate');
 
+		$this->hasher->expects($this->once())
+			->method('verify')
+			->with('password', 'passwordHash')
+			->willReturn(true);
+
 		$this->hasher->expects($this->never())
 			->method('hash');
 
@@ -3183,6 +3339,9 @@ class ManagerTest extends \Test\TestCase {
 	}
 
 	public function testUpdateShareMailDisableSendPasswordByTalkWithPreviousPassword() {
+		$this->expectException(\InvalidArgumentException::class);
+		$this->expectExceptionMessage('Can’t disable sending the password by Talk without setting a new password');
+
 		$manager = $this->createManagerMock()
 			->setMethods([
 				'canShare',
@@ -3198,7 +3357,7 @@ class ManagerTest extends \Test\TestCase {
 		$originalShare = $this->manager->newShare();
 		$originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL)
 			->setPermissions(\OCP\Constants::PERMISSION_ALL)
-			->setPassword('password')
+			->setPassword('passwordHash')
 			->setSendPasswordByTalk(true);
 
 		$tomorrow = new \DateTime();
@@ -3224,18 +3383,21 @@ 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->once())->method('pathCreateChecks')->with($file);
 		$manager->expects($this->never())->method('verifyPassword');
+		$manager->expects($this->never())->method('pathCreateChecks');
 		$manager->expects($this->never())->method('linkCreateChecks');
 		$manager->expects($this->never())->method('validateExpirationDate');
 
+		$this->hasher->expects($this->once())
+			->method('verify')
+			->with('password', 'passwordHash')
+			->willReturn(true);
+
 		$this->hasher->expects($this->never())
 			->method('hash');
 
-		$this->defaultProvider->expects($this->once())
-			->method('update')
-			->with($share, 'password')
-			->willReturn($share);
+		$this->defaultProvider->expects($this->never())
+			->method('update');
 
 		$hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
 		\OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post');
@@ -3252,6 +3414,79 @@ class ManagerTest extends \Test\TestCase {
 		$manager->updateShare($share);
 	}
 
+	public function testUpdateShareMailDisableSendPasswordByTalkWithoutChangingPassword() {
+		$this->expectException(\InvalidArgumentException::class);
+		$this->expectExceptionMessage('Can’t disable sending the password by Talk without setting a new password');
+
+		$manager = $this->createManagerMock()
+			->setMethods([
+				'canShare',
+				'getShareById',
+				'generalCreateChecks',
+				'verifyPassword',
+				'pathCreateChecks',
+				'linkCreateChecks',
+				'validateExpirationDate',
+			])
+			->getMock();
+
+		$originalShare = $this->manager->newShare();
+		$originalShare->setShareType(\OCP\Share::SHARE_TYPE_EMAIL)
+			->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setPassword('passwordHash')
+			->setSendPasswordByTalk(true);
+
+		$tomorrow = new \DateTime();
+		$tomorrow->setTime(0,0,0);
+		$tomorrow->add(new \DateInterval('P1D'));
+
+		$file = $this->createMock(File::class);
+		$file->method('getId')->willReturn(100);
+
+		$share = $this->manager->newShare();
+		$share->setProviderId('foo')
+			->setId('42')
+			->setShareType(\OCP\Share::SHARE_TYPE_EMAIL)
+			->setToken('token')
+			->setSharedBy('owner')
+			->setShareOwner('owner')
+			->setPassword('passwordHash')
+			->setSendPasswordByTalk(false)
+			->setExpirationDate($tomorrow)
+			->setNode($file)
+			->setPermissions(\OCP\Constants::PERMISSION_ALL);
+
+		$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->never())->method('pathCreateChecks');
+		$manager->expects($this->never())->method('linkCreateChecks');
+		$manager->expects($this->never())->method('validateExpirationDate');
+
+		$this->hasher->expects($this->never())
+			->method('verify');
+
+		$this->hasher->expects($this->never())
+			->method('hash');
+
+		$this->defaultProvider->expects($this->never())
+			->method('update');
+
+		$hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_set_expiration_date', $hookListner, 'post');
+		$hookListner->expects($this->never())->method('post');
+
+		$hookListner2 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_update_password', $hookListner2, 'post');
+		$hookListner2->expects($this->never())->method('post');
+
+		$hookListner3 = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_update_permissions', $hookListner3, 'post');
+		$hookListner3->expects($this->never())->method('post');
+
+		$manager->updateShare($share);
+	}
 
 	public function testMoveShareLink() {
 		$this->expectException(\InvalidArgumentException::class);