Skip to content

[stable33] fix: Send password to owner when password sending fails, regardless of enforcement #64423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
backportbot wants to merge 2 commits into
base: stable33
Choose a base branch
from
+75 −17
Open

[stable33] fix: Send password to owner when password sending fails, regardless of enforcement #64423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a79af7c
fix: Send password to owner when password sending fails, regardless o…
nfebe Jan 28, 2025
0216a38
Fix: password notification to share owner behavior
GhassenKefi Sep 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 27 additions & 10 deletions apps/sharebymail/lib/ShareByMailProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,17 +269,19 @@ public function sendMailNotification(IShare $share): bool {

// If we have a password set, we send it to the recipient
if ($share->getPassword() !== null) {
// If share-by-talk password is enabled, we do not send the notification
// to the recipient. They will have to request it to the owner after opening the link.
// Secondly, if the password expiration is disabled, we send the notification to the recipient
// 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()) {
// If sending the password by mail is disabled, we send the password to the owner.
// If share-by-talk password is enabled, we do not send the password to the recipient.
// They can request it from the owner after opening the link.
// Otherwise, we send the password to the recipient.
// If sending the password to the recipient fails, we send the password to the owner as a fallback.
// Password expiration does not affect this flow: the password is either sent to the recipient
// or, if sending fails, sent to the owner as a fallback.
if ($this->settingsManager->sendPasswordByMail() === false || $share->getSendPasswordByTalk()) {
$this->trySendPasswordToOwner($share);
} else {
$send = $this->sendPassword($share, $share->getPassword(), $validEmails);
if ($passwordEnforced && $send === false) {
$this->sendPasswordToOwner($share, $share->getPassword());
if ($send === false) {
$this->trySendPasswordToOwner($share);
}
}
}
Expand Down Expand Up @@ -308,6 +310,21 @@ public function sendMailNotification(IShare $share): bool {
return false;
}

/**
* Notifying the owner of the password must not abort an otherwise
* successful share creation, e.g. when the owner has no email address set.
*/
private function trySendPasswordToOwner(IShare $share): void {
try {
$this->sendPasswordToOwner($share, $share->getPassword());
} catch (\Exception $e) {
$this->logger->error('Failed to send password to the owner of the share.', [
'app' => 'sharebymail',
'exception' => $e,
]);
}
}

/**
* @param IShare $share The share to send the email for
* @param array $emails The email addresses to send the email to
Expand Down
55 changes: 48 additions & 7 deletions apps/sharebymail/tests/ShareByMailProviderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,12 +258,12 @@ public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswo
// The given password (but not the autogenerated password) should not be
// mailed to the receiver of the share because permanent passwords are not enforced.
$this->shareManager->expects($this->any())->method('shareApiLinkEnforcePassword')->willReturn(false);
$this->config->expects($this->once())->method('getSystemValue')->with('sharing.enable_mail_link_password_expiration')->willReturn(false);
$this->settingsManager->expects($this->once())->method('sendPasswordByMail')->willReturn(true);
$instance->expects($this->never())->method('autoGeneratePassword');

// A password is set but no password sent via talk has been requested
$instance->expects($this->once())->method('sendEmail')->with($share, ['receiver@example.com']);
$instance->expects($this->once())->method('sendPassword')->with($share, 'password');
$instance->expects($this->once())->method('sendPassword')->with($share, 'password')->willReturn(true);
$instance->expects($this->never())->method('sendPasswordToOwner');

$this->assertSame($expectedShare, $instance->create($share));
Expand Down Expand Up @@ -306,20 +306,61 @@ public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswo
// aside from the main email notification.
$this->shareManager->expects($this->any())->method('shareApiLinkEnforcePassword')->willReturn(false);
$instance->expects($this->never())->method('autoGeneratePassword');
$this->config->expects($this->once())->method('getSystemValue')
->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']);
$instance->expects($this->never())->method('sendPassword');
$instance->expects($this->once())->method('sendPassword')->with($share, 'password')->willReturn(true);
$instance->expects($this->never())->method('sendPasswordToOwner');

$this->assertSame($expectedShare, $instance->create($share));
$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->settingsManager->expects($this->once())->method('sendPasswordByMail')->willReturn(false);

$instance->expects($this->once())->method('sendPasswordToOwner')->with($share, 'password');
$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);

Expand Down Expand Up @@ -523,7 +564,7 @@ public function testCreateSendPasswordByTalkWithEnforcedPasswordProtectionWithPe
$instance->expects($this->once())->method('getRawShare')->with(42)->willReturn(['rawShare', 'password' => 'autogeneratedPassword']);
$instance->expects($this->once())->method('createShareObject')->with(['rawShare', 'password' => 'autogeneratedPassword'])->willReturn($expectedShare);

$share->expects($this->exactly(4))->method('getPassword')->willReturnOnConsecutiveCalls(null, 'autogeneratedPassword', 'autogeneratedPassword', 'autogeneratedPassword');
$share->expects($this->exactly(3))->method('getPassword')->willReturnOnConsecutiveCalls(null, 'autogeneratedPassword', 'autogeneratedPassword');
$this->hasher->expects($this->once())->method('hash')->with('autogeneratedPassword')->willReturn('autogeneratedPasswordHashed');
$share->expects($this->once())->method('setPassword')->with('autogeneratedPasswordHashed');

Expand Down
Loading