[stable17] Fix password changes in link and mail shares

.drone.yml

@@ -1262,6 +1262,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 --branch stable17 --depth 1 https://github.com/nextcloud/spreed apps/spreed
+- name: integration-sharing-v1-video-verification
+  image: nextcloudci/integration-php7.1:1
+  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 features/sharing-v1-video-verification.feature
+
+trigger:
+  branch:
+    - master
+    - stable*
+  event:
+    - pull_request
+    - push
+
 ---
 kind: pipeline
 name: integration-checksums-v1

apps/files_sharing/lib/Controller/ShareAPIController.php

@@ -535,6 +535,11 @@ public function createShare(
 			}
 			$share->setSharedWith($shareWith);
 
+			// Set password
+			if ($password !== '') {
+				$share->setPassword($password);
+			}
+
 			if ($sendPasswordByTalk === 'true') {
 				if (!$this->appManager->isEnabledForUser('spreed')) {
 					throw new OCSForbiddenException($this->l->t('Sharing %s sending the password by Nextcloud Talk failed because Nextcloud Talk is not enabled', [$path->getPath()]));

apps/sharebymail/lib/ShareByMailProvider.php

@@ -173,12 +173,16 @@ public function create(IShare $share) {
 
 		// 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) {
@@ -220,8 +224,6 @@ protected function autoGeneratePassword($share) {
 
 		$password = $this->secureRandom->generate($passwordLength, $passwordCharset);
 
-		$share->setPassword($this->hasher->hash($password));
-
 		return $password;
 	}
 

apps/sharebymail/tests/ShareByMailProviderTest.php

@@ -235,6 +235,51 @@ public function testCreateSendPasswordByMailWithoutEnforcedPasswordProtection()
 		);
 	}
 
+	public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswordProtection() {
+		$share = $this->getMockBuilder(IShare::class)->getMock();
+		$share->expects($this->any())->method('getSharedWith')->willReturn('[email protected]');
+		$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(['[email protected]']);
+		$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' => '[email protected]',
+		]);
+		$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('[email protected]');
@@ -253,14 +298,70 @@ public function testCreateSendPasswordByMailWithEnforcedPasswordProtection() {
 		$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(['[email protected]']);
 		$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' => '[email protected]',
+		]);
+		$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('[email protected]');
+		$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(['[email protected]']);
+		$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' => '[email protected]',
+		]);
 		$this->mailer->expects($this->once())->method('send');
 
 		$this->assertSame('shareObject',
@@ -286,14 +387,25 @@ public function testCreateSendPasswordByTalkWithEnforcedPasswordProtection() {
 		$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(['[email protected]' => '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' => '[email protected]',
+			'shareWith' => '[email protected]',
+		]);
 		$this->mailer->expects($this->once())->method('send');
 
 		$user = $this->createMock(IUser::class);
@@ -523,6 +635,13 @@ public function testUpdateSendPassword($plainTextPassword, string $originalPassw
 		$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' => '[email protected]',
+			]);
 			$this->mailer->expects($this->once())->method('send');
 		} else {
 			$this->mailer->expects($this->never())->method('send');

build/integration/config/behat.yml

@@ -25,6 +25,7 @@ default:
         - CommandLineContext:
             baseUrl: http://localhost:8080
             ocPath: ../../
+        - TalkContext
     federation:
       paths:
         - "%paths.base%/../federation_features"