Skip to content

Commit f431c14

Browse files
MorrisJobkeMorrisJobke
authored
Merge pull request #21151 from nextcloud/backport/21143/stable19
[stable19] Fix password changes in link and mail shares
2 parents bed5c21 + fcafad2 commit f431c14

File tree

14 files changed

+1326
-30
lines changed

14 files changed

+1326
-30
lines changed

‎.drone.yml‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1359,6 +1359,37 @@ trigger:
13591359
- pull_request
13601360
- push
13611361

1362+
---
1363+
kind: pipeline
1364+
name: integration-sharing-v1-video-verification
1365+
1366+
steps:
1367+
- name: submodules
1368+
image: docker:git
1369+
commands:
1370+
- git submodule update --init
1371+
- name: install-talk
1372+
image: docker:git
1373+
commands:
1374+
# JavaScript files are not used in integration tests so it is not needed to
1375+
# build them.
1376+
- git clone --branch stable19 --depth 1 https://github.com/nextcloud/spreed apps/spreed
1377+
- name: integration-sharing-v1-video-verification
1378+
image: nextcloudci/integration-php7.3:integration-php7.3-2
1379+
commands:
1380+
- bash tests/drone-run-integration-tests.sh || exit 0
1381+
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
1382+
- cd build/integration
1383+
- ./run.sh sharing_features/sharing-v1-video-verification.feature
1384+
1385+
trigger:
1386+
branch:
1387+
- master
1388+
- stable*
1389+
event:
1390+
- pull_request
1391+
- push
1392+
13621393
---
13631394
kind: pipeline
13641395
name: integration-setup-features

‎apps/sharebymail/lib/ShareByMailProvider.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,16 @@ public function create(IShare $share) {
187187

188188
// if the admin enforces a password for all mail shares we create a
189189
// random password and send it to the recipient
190-
$password = '';
190+
$password = $share->getPassword() ?: '';
191191
$passwordEnforced = $this->settingsManager->enforcePasswordProtection();
192-
if ($passwordEnforced) {
192+
if ($passwordEnforced && empty($password)) {
193193
$password = $this->autoGeneratePassword($share);
194194
}
195195

196+
if (!empty($password)) {
197+
$share->setPassword($this->hasher->hash($password));
198+
}
199+
196200
$shareId = $this->createMailShare($share);
197201
$send = $this->sendPassword($share, $password);
198202
if ($passwordEnforced && $send === false) {
@@ -233,8 +237,6 @@ protected function autoGeneratePassword($share) {
233237

234238
$password = $this->secureRandom->generate($passwordLength, $passwordCharset);
235239

236-
$share->setPassword($this->hasher->hash($password));
237-
238240
return $password;
239241
}
240242

‎apps/sharebymail/tests/ShareByMailProviderTest.php‎

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,51 @@ public function testCreateSendPasswordByMailWithoutEnforcedPasswordProtection()
240240
);
241241
}
242242

243+
public function testCreateSendPasswordByMailWithPasswordAndWithoutEnforcedPasswordProtection() {
244+
$share = $this->getMockBuilder(IShare::class)->getMock();
245+
$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
246+
$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
247+
$share->expects($this->any())->method('getSharedBy')->willReturn('owner');
248+
249+
$node = $this->getMockBuilder(File::class)->getMock();
250+
$node->expects($this->any())->method('getName')->willReturn('filename');
251+
252+
$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity']);
253+
254+
$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
255+
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
256+
$instance->expects($this->once())->method('createShareActivity')->with($share);
257+
$instance->expects($this->once())->method('getRawShare')->with(42)->willReturn('rawShare');
258+
$instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject');
259+
$share->expects($this->any())->method('getNode')->willReturn($node);
260+
261+
$share->expects($this->once())->method('getPassword')->willReturn('password');
262+
$this->hasher->expects($this->once())->method('hash')->with('password')->willReturn('passwordHashed');
263+
$share->expects($this->once())->method('setPassword')->with('passwordHashed');
264+
265+
// The given password (but not the autogenerated password) should be
266+
// mailed to the receiver of the share.
267+
$this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(false);
268+
$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
269+
$instance->expects($this->never())->method('autoGeneratePassword');
270+
271+
$message = $this->createMock(IMessage::class);
272+
$message->expects($this->once())->method('setTo')->with(['receiver@example.com']);
273+
$this->mailer->expects($this->once())->method('createMessage')->willReturn($message);
274+
$this->mailer->expects($this->once())->method('createEMailTemplate')->with('sharebymail.RecipientPasswordNotification', [
275+
'filename' => 'filename',
276+
'password' => 'password',
277+
'initiator' => 'owner',
278+
'initiatorEmail' => null,
279+
'shareWith' => 'receiver@example.com',
280+
]);
281+
$this->mailer->expects($this->once())->method('send');
282+
283+
$this->assertSame('shareObject',
284+
$instance->create($share)
285+
);
286+
}
287+
243288
public function testCreateSendPasswordByMailWithEnforcedPasswordProtection() {
244289
$share = $this->getMockBuilder(IShare::class)->getMock();
245290
$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
@@ -258,14 +303,70 @@ public function testCreateSendPasswordByMailWithEnforcedPasswordProtection() {
258303
$instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject');
259304
$share->expects($this->any())->method('getNode')->willReturn($node);
260305

306+
$share->expects($this->once())->method('getPassword')->willReturn(null);
307+
$this->hasher->expects($this->once())->method('hash')->with('autogeneratedPassword')->willReturn('autogeneratedPasswordHashed');
308+
$share->expects($this->once())->method('setPassword')->with('autogeneratedPasswordHashed');
309+
261310
// The autogenerated password should be mailed to the receiver of the share.
262311
$this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(true);
263312
$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
264-
$instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('password');
313+
$instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('autogeneratedPassword');
265314

266315
$message = $this->createMock(IMessage::class);
267316
$message->expects($this->once())->method('setTo')->with(['receiver@example.com']);
268317
$this->mailer->expects($this->once())->method('createMessage')->willReturn($message);
318+
$this->mailer->expects($this->once())->method('createEMailTemplate')->with('sharebymail.RecipientPasswordNotification', [
319+
'filename' => 'filename',
320+
'password' => 'autogeneratedPassword',
321+
'initiator' => 'owner',
322+
'initiatorEmail' => null,
323+
'shareWith' => 'receiver@example.com',
324+
]);
325+
$this->mailer->expects($this->once())->method('send');
326+
327+
$this->assertSame('shareObject',
328+
$instance->create($share)
329+
);
330+
}
331+
332+
public function testCreateSendPasswordByMailWithPasswordAndWithEnforcedPasswordProtection() {
333+
$share = $this->getMockBuilder(IShare::class)->getMock();
334+
$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
335+
$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
336+
$share->expects($this->any())->method('getSharedBy')->willReturn('owner');
337+
338+
$node = $this->getMockBuilder(File::class)->getMock();
339+
$node->expects($this->any())->method('getName')->willReturn('filename');
340+
341+
$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity']);
342+
343+
$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
344+
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
345+
$instance->expects($this->once())->method('createShareActivity')->with($share);
346+
$instance->expects($this->once())->method('getRawShare')->with(42)->willReturn('rawShare');
347+
$instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject');
348+
$share->expects($this->any())->method('getNode')->willReturn($node);
349+
350+
$share->expects($this->once())->method('getPassword')->willReturn('password');
351+
$this->hasher->expects($this->once())->method('hash')->with('password')->willReturn('passwordHashed');
352+
$share->expects($this->once())->method('setPassword')->with('passwordHashed');
353+
354+
// The given password (but not the autogenerated password) should be
355+
// mailed to the receiver of the share.
356+
$this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(true);
357+
$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
358+
$instance->expects($this->never())->method('autoGeneratePassword');
359+
360+
$message = $this->createMock(IMessage::class);
361+
$message->expects($this->once())->method('setTo')->with(['receiver@example.com']);
362+
$this->mailer->expects($this->once())->method('createMessage')->willReturn($message);
363+
$this->mailer->expects($this->once())->method('createEMailTemplate')->with('sharebymail.RecipientPasswordNotification', [
364+
'filename' => 'filename',
365+
'password' => 'password',
366+
'initiator' => 'owner',
367+
'initiatorEmail' => null,
368+
'shareWith' => 'receiver@example.com',
369+
]);
269370
$this->mailer->expects($this->once())->method('send');
270371

271372
$this->assertSame('shareObject',
@@ -291,14 +392,25 @@ public function testCreateSendPasswordByTalkWithEnforcedPasswordProtection() {
291392
$instance->expects($this->once())->method('createShareObject')->with('rawShare')->willReturn('shareObject');
292393
$share->expects($this->any())->method('getNode')->willReturn($node);
293394

395+
$share->expects($this->once())->method('getPassword')->willReturn(null);
396+
$this->hasher->expects($this->once())->method('hash')->with('autogeneratedPassword')->willReturn('autogeneratedPasswordHashed');
397+
$share->expects($this->once())->method('setPassword')->with('autogeneratedPasswordHashed');
398+
294399
// The autogenerated password should be mailed to the owner of the share.
295400
$this->settingsManager->expects($this->any())->method('enforcePasswordProtection')->willReturn(true);
296401
$this->settingsManager->expects($this->any())->method('sendPasswordByMail')->willReturn(true);
297-
$instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('password');
402+
$instance->expects($this->once())->method('autoGeneratePassword')->with($share)->willReturn('autogeneratedPassword');
298403

299404
$message = $this->createMock(IMessage::class);
300405
$message->expects($this->once())->method('setTo')->with(['owner@example.com' => 'Owner display name']);
301406
$this->mailer->expects($this->once())->method('createMessage')->willReturn($message);
407+
$this->mailer->expects($this->once())->method('createEMailTemplate')->with('sharebymail.OwnerPasswordNotification', [
408+
'filename' => 'filename',
409+
'password' => 'autogeneratedPassword',
410+
'initiator' => 'Owner display name',
411+
'initiatorEmail' => 'owner@example.com',
412+
'shareWith' => 'receiver@example.com',
413+
]);
302414
$this->mailer->expects($this->once())->method('send');
303415

304416
$user = $this->createMock(IUser::class);
@@ -527,6 +639,13 @@ public function testUpdateSendPassword($plainTextPassword, string $originalPassw
527639
$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn($newSendPasswordByTalk);
528640

529641
if ($sendMail) {
642+
$this->mailer->expects($this->once())->method('createEMailTemplate')->with('sharebymail.RecipientPasswordNotification', [
643+
'filename' => 'filename',
644+
'password' => $plainTextPassword,
645+
'initiator' => null,
646+
'initiatorEmail' => null,
647+
'shareWith' => 'receiver@example.com',
648+
]);
530649
$this->mailer->expects($this->once())->method('send');
531650
} else {
532651
$this->mailer->expects($this->never())->method('send');

‎build/integration/config/behat.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ default:
6565
- admin
6666
- admin
6767
regular_user_password: 123456
68+
- TalkContext
6869
setup:
6970
paths:
7071
- "%paths.base%/../setup_features"

‎build/integration/features/bootstrap/BasicStructure.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
trait BasicStructure {
4646
use Auth;
4747
use Download;
48+
use Mail;
4849
use Trashbin;
4950

5051
/** @var string */

0 commit comments

Comments
 (0)