mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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>
This commit is contained in:
Daniel Calviño Sánchez 2020-05-28 20:27:33 +02:00
parent a2b0a7c30e
commit e59e283cc2
2 changed files with 104 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
apps/sharebymail/lib/ShareByMailProvider.php
View File

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

98
apps/sharebymail/tests/ShareByMailProviderTest.php
View File

@ -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() { public function testCreateSendPasswordByMailWithEnforcedPasswordProtection() {
$share = $this->getMockBuilder(IShare::class)->getMock(); $share = $this->getMockBuilder(IShare::class)->getMock();
$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); $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'); $instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject');
$share->expects($this->any())->method('getNode')->willReturn($node); $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. // 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('enforcePasswordProtection')->willReturn(true);
$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->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() { public function testCreateSendPasswordByTalkWithEnforcedPasswordProtection() {
$share = $this->getMockBuilder(IShare::class)->getMock(); $share = $this->getMockBuilder(IShare::class)->getMock();
$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com'); $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'); $instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject');
$share->expects($this->any())->method('getNode')->willReturn($node); $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. // 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('enforcePasswordProtection')->willReturn(true);
$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true); $this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);