Honour "sendPasswordByTalk" property in mail shares

When a password was set for a mail share an e-mail was sent to the
recipient with the password. Now the e-mail is no longer sent if the
password is meant to be sent by Talk.

However, before the e-mail was not sent when the share was updated but
the password was not changed. Now an e-mail is sent in that case too if
switching from a password sent by Talk to a password sent by mail.

On the other hand, when switching from a password sent by mail to a
password sent by Talk it is mandatory to change the password; otherwise
the recipient would already have access to the share without having to
call the sharer to verify her identity.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

2 files changed, 159 insertions, 2 deletions

diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php
index d26938a3242..6cc27957bbe 100644
--- a/apps/sharebymail/lib/ShareByMailProvider.php
+++ b/apps/sharebymail/lib/ShareByMailProvider.php
@@ -454,7 +454,7 @@ class ShareByMailProvider implements IShareProvider {
 		$initiator = $share->getSharedBy();
 		$shareWith = $share->getSharedWith();
 
-		if ($password === '' || $this->settingsManager->sendPasswordByMail() === false) {
+		if ($password === '' || $this->settingsManager->sendPasswordByMail() === false || $share->getSendPasswordByTalk()) {
 			return false;
 		}
 
@@ -707,7 +707,8 @@ class ShareByMailProvider implements IShareProvider {
 		// a real password was given
 		$validPassword = $plainTextPassword !== null && $plainTextPassword !== '';
 
-		if($validPassword && $originalShare->getPassword() !== $share->getPassword()) {
+		if($validPassword && ($originalShare->getPassword() !== $share->getPassword() ||
+								($originalShare->getSendPasswordByTalk() && !$share->getSendPasswordByTalk()))) {
 			$this->sendPassword($share, $plainTextPassword);
 		}
 		/*
diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php
index 48b5de62146..4942dac7022 100644
--- a/apps/sharebymail/tests/ShareByMailProviderTest.php
+++ b/apps/sharebymail/tests/ShareByMailProviderTest.php
@@ -38,6 +38,7 @@ use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Mail\IEMailTemplate;
 use OCP\Mail\IMailer;
+use OCP\Mail\IMessage;
 use OCP\Security\IHasher;
 use OCP\Security\ISecureRandom;
 use OCP\Share\IManager;
@@ -204,6 +205,107 @@ class ShareByMailProviderTest extends TestCase {
 		);
 	}
 
+	public function testCreateSendPasswordByMailWithoutEnforcedPasswordProtection() {
+		$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);
+
+		// The autogenerated password should not be mailed.
+		$this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(false);
+		$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
+		$instance->expects($this->never())->method('autoGeneratePassword');
+
+		$this->mailer->expects($this->never())->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');
+		$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);
+
+		// 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');
+
+		$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('send');
+
+		$this->assertSame('shareObject',
+			$instance->create($share)
+		);
+	}
+
+	public function testCreateSendPasswordByTalkWithEnforcedPasswordProtection() {
+		$share = $this->getMockBuilder(IShare::class)->getMock();
+		$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
+		$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(true);
+		$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);
+
+		// 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');
+
+		$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('send');
+
+		$user = $this->createMock(IUser::class);
+		$this->userManager->expects($this->once())->method('get')->with('owner')->willReturn($user);
+		$user->expects($this->once())->method('getDisplayName')->willReturn('Owner display name');
+		$user->expects($this->once())->method('getEMailAddress')->willReturn('owner@example.com');
+
+		$this->assertSame('shareObject',
+			$instance->create($share)
+		);
+	}
+
 	/**
 	 * @expectedException \Exception
 	 */
@@ -380,6 +482,60 @@ class ShareByMailProviderTest extends TestCase {
 		$this->assertSame($note, $result[0]['note']);
 	}
 
+	public function dataUpdateSendPassword() {
+		return [
+			['password', 'hashed', 'hashed new', false, false, true],
+			['', 'hashed', 'hashed new', false, false, false],
+			[null, 'hashed', 'hashed new', false, false, false],
+			['password', 'hashed', 'hashed', false, false, false],
+			['password', 'hashed', 'hashed new', false, true, false],
+			['password', 'hashed', 'hashed new', true, false, true],
+			['password', 'hashed', 'hashed', true, false, true],
+		];
+	}
+
+	/**
+	 * @dataProvider dataUpdateSendPassword
+	 *
+	 * @param string|null plainTextPassword
+	 * @param string originalPassword
+	 * @param string newPassword
+	 * @param string originalSendPasswordByTalk
+	 * @param string newSendPasswordByTalk
+	 * @param bool sendMail
+	 */
+	public function testUpdateSendPassword($plainTextPassword, string $originalPassword, string $newPassword, string $originalSendPasswordByTalk, string $newSendPasswordByTalk, bool $sendMail) {
+		$node = $this->getMockBuilder(File::class)->getMock();
+		$node->expects($this->any())->method('getName')->willReturn('filename');
+
+		$originalShare = $this->getMockBuilder(IShare::class)->getMock();
+		$originalShare->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
+		$originalShare->expects($this->any())->method('getNode')->willReturn($node);
+		$originalShare->expects($this->any())->method('getId')->willReturn(42);
+		$originalShare->expects($this->any())->method('getPassword')->willReturn($originalPassword);
+		$originalShare->expects($this->any())->method('getSendPasswordByTalk')->willReturn($originalSendPasswordByTalk);
+
+		$share = $this->getMockBuilder(IShare::class)->getMock();
+		$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
+		$share->expects($this->any())->method('getNode')->willReturn($node);
+		$share->expects($this->any())->method('getId')->willReturn(42);
+		$share->expects($this->any())->method('getPassword')->willReturn($newPassword);
+		$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn($newSendPasswordByTalk);
+
+		if ($sendMail) {
+			$this->mailer->expects($this->once())->method('send');
+		} else {
+			$this->mailer->expects($this->never())->method('send');
+		}
+
+		$instance = $this->getInstance(['getShareById', 'createPasswordSendActivity']);
+		$instance->expects($this->once())->method('getShareById')->willReturn($originalShare);
+
+		$this->assertSame($share,
+			$instance->update($share, $plainTextPassword)
+		);
+	}
+
 	public function testDelete() {
 		$instance = $this->getInstance(['removeShareFromTable']);
 		$this->share->expects($this->once())->method('getId')->willReturn(42);