From edbcd49f13be8c390d9815c6c371e214d49375bb Mon Sep 17 00:00:00 2001 From: nfebe Date: Tue, 28 Jan 2025 23:34:45 +0100 Subject: [PATCH 1/2] fix: Send password to owner when password sending fails, regardless of enforcement Previously, the `sendPasswordToOwner` method would only trigger if password enforcement was enabled and sending the password to the recipient failed. This behavior was inconsistent with the intended functionality, as the owner should receive the password whenever sending to the recipient fails, regardless of whether password enforcement is enabled. This change ensures that the owner is notified of the password whenever sending it to the recipient fails, improving consistency and user experience. Signed-off-by: nfebe --- apps/sharebymail/lib/ShareByMailProvider.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index c443459a7127b..b61fca47203a9 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -272,10 +272,9 @@ public function sendMailNotification(IShare $share): bool { // Lastly, if the mail to recipient failed, we send the password to the owner as a fallback. // If a password expires, the recipient will still be able to request a new one via talk. $passwordExpire = $this->config->getSystemValue('sharing.enable_mail_link_password_expiration', false); - $passwordEnforced = $this->shareManager->shareApiLinkEnforcePassword(); if ($passwordExpire === false || $share->getSendPasswordByTalk()) { $send = $this->sendPassword($share, $share->getPassword(), $validEmails); - if ($passwordEnforced && $send === false) { + if ($send === false) { $this->sendPasswordToOwner($share, $share->getPassword()); } } From d652cd1f35c516608a387c36d0c78c0c02bb5269 Mon Sep 17 00:00:00 2001 From: nfebe Date: Wed, 29 Jan 2025 18:21:42 +0100 Subject: [PATCH 2/2] WIP test(share_by_mail): Send password to owner if `send_by_mail` is disabled Signed-off-by: nfebe --- .../tests/ShareByMailProviderTest.php | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/apps/sharebymail/tests/ShareByMailProviderTest.php b/apps/sharebymail/tests/ShareByMailProviderTest.php index 68e7e8691fa22..b863b2193acf4 100644 --- a/apps/sharebymail/tests/ShareByMailProviderTest.php +++ b/apps/sharebymail/tests/ShareByMailProviderTest.php @@ -310,6 +310,8 @@ public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswo ->with('sharing.enable_mail_link_password_expiration') ->willReturn(true); + $this->settingsManager->expects($this->once())->method('sendPasswordByMail')->willReturn(true); + // No password has been set and no password sent via talk has been requested, // but password has been enforced for the whole instance and will be generated. $instance->expects($this->once())->method('sendEmail')->with($share, ['receiver@example.com']); @@ -320,6 +322,51 @@ public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswo $instance->sendMailNotification($share); } + + public function testCreateSendPasswordToOwnerWhenSendPasswordByMailIsDisabled(): void { + $expectedShare = $this->createMock(IShare::class); + $node = $this->getMockBuilder(File::class)->getMock(); + $node->method('getName')->willReturn('filename'); + + $share = $this->getMockBuilder(IShare::class)->getMock(); + $share->method('getSharedWith')->willReturn('receiver@example.com'); + $share->method('getSendPasswordByTalk')->willReturn(false); + $share->method('getSharedBy')->willReturn('owner'); + $share->method('getNode')->willReturn($node); + $share->method('getId')->willReturn(42); + $share->method('getNote')->willReturn(''); + $share->method('getToken')->willReturn('token'); + $share->method('getPassword')->willReturn('password'); + + $this->mailer->method('validateMailAddress')->willReturn(true); + $this->hasher->expects($this->once())->method('hash')->with('password')->willReturn('passwordHashed'); + $share->expects($this->once())->method('setPassword')->with('passwordHashed'); + + $instance = $this->getInstance([ + 'getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', + 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity', + 'sendEmail', 'sendPassword', 'sendPasswordToOwner', + ]); + + $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', 'password' => 'password']); + $instance->expects($this->once())->method('createShareObject')->with(['rawShare', 'password' => 'password'])->willReturn($expectedShare); + + $this->shareManager->method('shareApiLinkEnforcePassword')->willReturn(false); + $this->config->expects($this->once())->method('getSystemValue')->with('sharing.enable_mail_link_password_expiration')->willReturn(true); + $this->settingsManager->expects($this->once())->method('sendPasswordByMail')->willReturn(false); + + $instance->expects($this->once())->method('sendPasswordToOwner')->with($share); + $instance->expects($this->once())->method('sendEmail')->with($share, ['receiver@example.com']); + $instance->expects($this->never())->method('sendPassword'); + + $this->assertSame($expectedShare, $instance->create($share)); + $instance->sendMailNotification($share); + } + + public function testCreateSendPasswordByMailWithEnforcedPasswordProtectionWithPermanentPassword(): void { $expectedShare = $this->createMock(IShare::class);