Skip to content

Commit 235b7d7

Browse files
provokateurinprovokateurin
authored
Merge pull request #49073 from nextcloud/feat/files_sharing/co-owner
2 parents 5088b91 + 7c3a78a commit 235b7d7

22 files changed

+427
-57
lines changed

‎apps/files/lib/Service/OwnershipTransferService.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ private function collectUsersShares(
310310
foreach ($supportedShareTypes as $shareType) {
311311
$offset = 0;
312312
while (true) {
313-
$sharePage = $this->shareManager->getSharesBy($sourceUid, $shareType, null, true, 50, $offset);
313+
$sharePage = $this->shareManager->getSharesBy($sourceUid, $shareType, null, true, 50, $offset, onlyValid: false);
314314
$progress->advance(count($sharePage));
315315
if (empty($sharePage)) {
316316
break;
@@ -464,7 +464,7 @@ private function restoreShares(
464464
}
465465
$share->setNodeId($newNodeId);
466466

467-
$this->shareManager->updateShare($share);
467+
$this->shareManager->updateShare($share, onlyValid: false);
468468
}
469469
}
470470
} catch (NotFoundException $e) {

‎apps/files_sharing/lib/Controller/ShareAPIController.php‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
use OCP\Files\Folder;
3636
use OCP\Files\InvalidPathException;
3737
use OCP\Files\IRootFolder;
38+
use OCP\Files\Mount\IShareOwnerlessMount;
3839
use OCP\Files\Node;
3940
use OCP\Files\NotFoundException;
4041
use OCP\HintException;
@@ -1235,18 +1236,6 @@ public function updateShare(
12351236
if ($share->getShareType() === IShare::TYPE_LINK
12361237
|| $share->getShareType() === IShare::TYPE_EMAIL) {
12371238

1238-
/**
1239-
* We do not allow editing link shares that the current user
1240-
* doesn't own. This is confusing and lead to errors when
1241-
* someone else edit a password or expiration date without
1242-
* the share owner knowing about it.
1243-
* We only allow deletion
1244-
*/
1245-
1246-
if ($share->getSharedBy() !== $this->userId) {
1247-
throw new OCSForbiddenException($this->l->t('You are not allowed to edit link shares that you don\'t own'));
1248-
}
1249-
12501239
// Update hide download state
12511240
$attributes = $share->getAttributes() ?? $share->newAttributes();
12521241
if ($hideDownload === 'true') {
@@ -1553,6 +1542,12 @@ protected function canEditShare(IShare $share): bool {
15531542
return true;
15541543
}
15551544

1545+
$userFolder = $this->rootFolder->getUserFolder($this->userId);
1546+
$file = $userFolder->getFirstNodeById($share->getNodeId());
1547+
if ($file?->getMountPoint() instanceof IShareOwnerlessMount && $this->shareProviderResharingRights($this->userId, $share, $file)) {
1548+
return true;
1549+
}
1550+
15561551
//! we do NOT support some kind of `admin` in groups.
15571552
//! You cannot edit shares shared to a group you're
15581553
//! a member of if you're not the share owner or the file owner!
@@ -1588,6 +1583,12 @@ protected function canDeleteShare(IShare $share): bool {
15881583
return true;
15891584
}
15901585

1586+
$userFolder = $this->rootFolder->getUserFolder($this->userId);
1587+
$file = $userFolder->getFirstNodeById($share->getNodeId());
1588+
if ($file?->getMountPoint() instanceof IShareOwnerlessMount && $this->shareProviderResharingRights($this->userId, $share, $file)) {
1589+
return true;
1590+
}
1591+
15911592
return false;
15921593
}
15931594

‎apps/files_sharing/lib/Updater.php‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,24 @@ private static function moveShareInOrOutOfShare($path): void {
5050

5151
$shareManager = \OC::$server->getShareManager();
5252

53+
// We intentionally include invalid shares, as they have been automatically invalidated due to the node no longer
54+
// being accessible for the user. Only in this case where we adjust the share after it was moved we want to ignore
55+
// this to be able to still adjust it.
56+
5357
// FIXME: should CIRCLES be included here ??
54-
$shares = $shareManager->getSharesBy($user->getUID(), IShare::TYPE_USER, $src, false, -1);
55-
$shares = array_merge($shares, $shareManager->getSharesBy($user->getUID(), IShare::TYPE_GROUP, $src, false, -1));
56-
$shares = array_merge($shares, $shareManager->getSharesBy($user->getUID(), IShare::TYPE_ROOM, $src, false, -1));
58+
$shares = $shareManager->getSharesBy($user->getUID(), IShare::TYPE_USER, $src, false, -1, onlyValid: false);
59+
$shares = array_merge($shares, $shareManager->getSharesBy($user->getUID(), IShare::TYPE_GROUP, $src, false, -1, onlyValid: false));
60+
$shares = array_merge($shares, $shareManager->getSharesBy($user->getUID(), IShare::TYPE_ROOM, $src, false, -1, onlyValid: false));
5761

5862
if ($src instanceof Folder) {
5963
$cacheAccess = Server::get(FileAccess::class);
6064

6165
$sourceStorageId = $src->getStorage()->getCache()->getNumericStorageId();
6266
$sourceInternalPath = $src->getInternalPath();
6367
$subShares = array_merge(
64-
$shareManager->getSharesBy($user->getUID(), IShare::TYPE_USER),
65-
$shareManager->getSharesBy($user->getUID(), IShare::TYPE_GROUP),
66-
$shareManager->getSharesBy($user->getUID(), IShare::TYPE_ROOM),
68+
$shareManager->getSharesBy($user->getUID(), IShare::TYPE_USER, onlyValid: false),
69+
$shareManager->getSharesBy($user->getUID(), IShare::TYPE_GROUP, onlyValid: false),
70+
$shareManager->getSharesBy($user->getUID(), IShare::TYPE_ROOM, onlyValid: false),
6771
);
6872
$shareSourceIds = array_map(fn (IShare $share) => $share->getNodeId(), $subShares);
6973
$shareSources = $cacheAccess->getByFileIdsInStorage($shareSourceIds, $sourceStorageId);
@@ -111,7 +115,7 @@ private static function moveShareInOrOutOfShare($path): void {
111115

112116
$share->setShareOwner($newOwner);
113117
$share->setPermissions($newPermissions);
114-
$shareManager->updateShare($share);
118+
$shareManager->updateShare($share, onlyValid: false);
115119
}
116120
}
117121

‎apps/files_sharing/src/components/SharingEntry.vue‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@ export default {
7878
} else if (this.share.type === this.SHARE_TYPES.SHARE_TYPE_GUEST) {
7979
title += ` (${t('files_sharing', 'guest')})`
8080
}
81+
if (!this.isShareOwner && this.share.ownerDisplayName) {
82+
title += ' ' + t('files_sharing', 'by {initiator}', {
83+
initiator: this.share.ownerDisplayName,
84+
})
85+
}
8186
return title
8287
},
8388
tooltip() {

‎apps/files_sharing/tests/Controller/ShareAPIControllerTest.php‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use OCP\Files\Folder;
1919
use OCP\Files\IRootFolder;
2020
use OCP\Files\Mount\IMountPoint;
21+
use OCP\Files\Mount\IShareOwnerlessMount;
2122
use OCP\Files\NotFoundException;
2223
use OCP\Files\Storage\IStorage;
2324
use OCP\IConfig;
@@ -232,10 +233,20 @@ public function testDeleteShareLocked(): void {
232233
$this->expectExceptionMessage('Could not delete share');
233234

234235
$node = $this->getMockBuilder(File::class)->getMock();
236+
$node->method('getId')->willReturn(1);
235237

236238
$share = $this->newShare();
237239
$share->setNode($node);
238240

241+
$userFolder = $this->getMockBuilder(Folder::class)->getMock();
242+
$this->rootFolder->method('getUserFolder')
243+
->with($this->currentUser)
244+
->willReturn($userFolder);
245+
246+
$userFolder->method('getFirstNodeById')
247+
->with($share->getNodeId())
248+
->willReturn($node);
249+
239250
$this->shareManager
240251
->expects($this->once())
241252
->method('getShareById')
@@ -476,6 +487,62 @@ public function testDeleteSharedWithGroupIDontBelongTo(): void {
476487
$this->ocs->deleteShare(42);
477488
}
478489

490+
public function testDeleteShareOwnerless(): void {
491+
$ocs = $this->mockFormatShare();
492+
493+
$mount = $this->createMock(IShareOwnerlessMount::class);
494+
495+
$file = $this->createMock(File::class);
496+
$file
497+
->expects($this->exactly(2))
498+
->method('getPermissions')
499+
->willReturn(Constants::PERMISSION_SHARE);
500+
$file
501+
->expects($this->once())
502+
->method('getMountPoint')
503+
->willReturn($mount);
504+
505+
$userFolder = $this->createMock(Folder::class);
506+
$userFolder
507+
->expects($this->exactly(2))
508+
->method('getFirstNodeById')
509+
->with(2)
510+
->willReturn($file);
511+
512+
$this->rootFolder
513+
->method('getUserFolder')
514+
->with($this->currentUser)
515+
->willReturn($userFolder);
516+
517+
$share = $this->createMock(IShare::class);
518+
$share
519+
->expects($this->once())
520+
->method('getNode')
521+
->willReturn($file);
522+
$share
523+
->expects($this->exactly(2))
524+
->method('getNodeId')
525+
->willReturn(2);
526+
$share
527+
->expects($this->exactly(2))
528+
->method('getPermissions')
529+
->willReturn(Constants::PERMISSION_SHARE);
530+
531+
$this->shareManager
532+
->expects($this->once())
533+
->method('getShareById')
534+
->with('ocinternal:1', $this->currentUser)
535+
->willReturn($share);
536+
537+
$this->shareManager
538+
->expects($this->once())
539+
->method('deleteShare')
540+
->with($share);
541+
542+
$result = $ocs->deleteShare(1);
543+
$this->assertInstanceOf(DataResponse::class, $result);
544+
}
545+
479546
/*
480547
* FIXME: Enable once we have a federated Share Provider
481548
@@ -3748,6 +3815,63 @@ public function testUpdateShareCanIncreasePermissionsIfOwner(): void {
37483815
$this->assertInstanceOf(DataResponse::class, $result);
37493816
}
37503817

3818+
public function testUpdateShareOwnerless(): void {
3819+
$ocs = $this->mockFormatShare();
3820+
3821+
$mount = $this->createMock(IShareOwnerlessMount::class);
3822+
3823+
$file = $this->createMock(File::class);
3824+
$file
3825+
->expects($this->exactly(2))
3826+
->method('getPermissions')
3827+
->willReturn(Constants::PERMISSION_SHARE);
3828+
$file
3829+
->expects($this->once())
3830+
->method('getMountPoint')
3831+
->willReturn($mount);
3832+
3833+
$userFolder = $this->createMock(Folder::class);
3834+
$userFolder
3835+
->expects($this->exactly(2))
3836+
->method('getFirstNodeById')
3837+
->with(2)
3838+
->willReturn($file);
3839+
3840+
$this->rootFolder
3841+
->method('getUserFolder')
3842+
->with($this->currentUser)
3843+
->willReturn($userFolder);
3844+
3845+
$share = $this->createMock(IShare::class);
3846+
$share
3847+
->expects($this->once())
3848+
->method('getNode')
3849+
->willReturn($file);
3850+
$share
3851+
->expects($this->exactly(2))
3852+
->method('getNodeId')
3853+
->willReturn(2);
3854+
$share
3855+
->expects($this->exactly(2))
3856+
->method('getPermissions')
3857+
->willReturn(Constants::PERMISSION_SHARE);
3858+
3859+
$this->shareManager
3860+
->expects($this->once())
3861+
->method('getShareById')
3862+
->with('ocinternal:1', $this->currentUser)
3863+
->willReturn($share);
3864+
3865+
$this->shareManager
3866+
->expects($this->once())
3867+
->method('updateShare')
3868+
->with($share)
3869+
->willReturn($share);
3870+
3871+
$result = $ocs->updateShare(1, Constants::PERMISSION_ALL);
3872+
$this->assertInstanceOf(DataResponse::class, $result);
3873+
}
3874+
37513875
public function dataFormatShare() {
37523876
$file = $this->getMockBuilder(File::class)->getMock();
37533877
$folder = $this->getMockBuilder(Folder::class)->getMock();

‎dist/146-146.js‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/4242-4242.js.license renamed to dist/146-146.js.license

File renamed without changes.

‎dist/146-146.js.map‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/146-146.js.map.license‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
146-146.js.license

‎dist/4242-4242.js‎

Lines changed: 0 additions & 2 deletions
This file was deleted.

0 commit comments

Comments
 (0)