Fix creating a mail share with a password

When a mail share was created with a password the given password was not
hashed, so it was not possible to open the share with that password.
Moreover, if passwords were enforced the given password was ignored and
a new one was set (although in this case it was hashed so it worked as
expected). Now the given password is properly hashed and not overriden.

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

diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php
index 4292ac9bf181bf510912e678c7b46f83ba235d4c..3a1a069172cdb54abb26b814696dd2299ff6ba8b 100644 (file)
--- a/apps/sharebymail/lib/ShareByMailProvider.php
+++ b/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;
        }
 

diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php
index 431ea5ca8cfba6d1660a550ccde2a2369f9d7923..2dc8ff9c953d61c3ab15a50d1fc09c4fc58e736b 100644 (file)
--- a/apps/sharebymail/tests/ShareByMailProviderTest.php
+++ b/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,6 +303,10 @@ 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);
@@ -280,6 +329,51 @@ class ShareByMailProviderTest extends TestCase {
                );
        }
 
+       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',
+                       $instance->create($share)
+               );
+       }
+
        public function testCreateSendPasswordByTalkWithEnforcedPasswordProtection() {
                $share = $this->getMockBuilder(IShare::class)->getMock();
                $share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
@@ -298,6 +392,10 @@ 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);