Skip to content

Commit 59c05fc

Browse files
provokateurinprovokateurin
authored
Merge pull request #61938 from nextcloud/refactor/interaction/allow-multiple-resources-and-receivers
refactor(Interaction): Allow multiple resources and receivers in a single event
2 parents 8648e4e + 4125bec commit 59c05fc

9 files changed

Lines changed: 118 additions & 98 deletions

File tree

‎apps/files/lib/Listener/RestrictInteractionListener.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,10 @@ public function __construct(
3636
*/
3737
#[\Override]
3838
public function handle(Event $event): void {
39-
if ($event->resource instanceof NodeResource && ($event->resource->getNodePermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ) {
40-
throw new InteractionRestrictedException('No read permission on the node.', $this->l10n->t('No read permission on file.'));
39+
foreach ($event->resources as $resource) {
40+
if ($resource instanceof NodeResource && ($resource->getNodePermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ) {
41+
throw new InteractionRestrictedException('No read permission on the node.', $this->l10n->t('No read permission on file.'));
42+
}
4143
}
4244
}
4345
}

‎apps/files/tests/Listener/RestrictInteractionListenerTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public function testNodeResourceShareActionMissingReadPermission(): void {
5353
$folderNode->getStorage()->getCache()->update($folderNode->getId(), ['permissions' => Constants::PERMISSION_ALL & ~Constants::PERMISSION_READ]);
5454

5555
foreach ([$fileNode, $folderNode] as $node) {
56-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, new NodeResource($node->getId(), $this->user->getUID(), $node), null, null);
56+
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [new NodeResource($node->getId(), $this->user->getUID(), $node)], null, []);
5757
$this->assertEquals('No read permission on file.', $event->isInteractionRestricted());
5858
}
5959
}

‎apps/files_sharing/lib/Listener/RestrictInteractionListener.php‎

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -45,43 +45,47 @@ public function __construct(
4545
*/
4646
#[\Override]
4747
public function handle(Event $event): void {
48-
if ($event->resource instanceof NodeResource && $event->action instanceof ShareAction) {
49-
if (!$event->resource->getNode()->isShareable()) {
50-
throw new InteractionRestrictedException('Node is not shareable.', $this->l10n->t('You are not allowed to share "%s".', [$event->resource->getNode()->getName()]));
51-
}
48+
foreach ($event->resources as $resource) {
49+
if ($resource instanceof NodeResource && $event->action instanceof ShareAction) {
50+
if (!$resource->getNode()->isShareable()) {
51+
throw new InteractionRestrictedException('Node is not shareable.', $this->l10n->t('You are not allowed to share "%s".', [$resource->getNode()->getName()]));
52+
}
5253

53-
$userFolder = $this->rootFolder->getUserFolder($event->userId);
54-
if ($event->resource->nodeId === $userFolder->getId()) {
55-
throw new InteractionRestrictedException('Cannot share home folder node.', $this->l10n->t('You cannot share your home folder.'));
56-
}
54+
$userFolder = $this->rootFolder->getUserFolder($event->userId);
55+
if ($resource->nodeId === $userFolder->getId()) {
56+
throw new InteractionRestrictedException('Cannot share home folder node.', $this->l10n->t('You cannot share your home folder.'));
57+
}
5758

58-
if ($event->action->filesSharingPermissions !== null) {
59-
if ($event->resource->getNode() instanceof File) {
60-
if (($event->action->filesSharingPermissions & Constants::PERMISSION_DELETE) === Constants::PERMISSION_DELETE) {
61-
throw new InteractionRestrictedException('Cannot share file node with delete permission.', $this->l10n->t('File cannot be shared with delete permission.'));
62-
}
59+
if ($event->action->filesSharingPermissions !== null) {
60+
if ($resource->getNode() instanceof File) {
61+
if (($event->action->filesSharingPermissions & Constants::PERMISSION_DELETE) === Constants::PERMISSION_DELETE) {
62+
throw new InteractionRestrictedException('Cannot share file node with delete permission.', $this->l10n->t('File cannot be shared with delete permission.'));
63+
}
6364

64-
if (($event->action->filesSharingPermissions & Constants::PERMISSION_CREATE) === Constants::PERMISSION_CREATE) {
65-
throw new InteractionRestrictedException('Cannot share file node with create permission.', $this->l10n->t('File cannot be shared with create permission.'));
65+
if (($event->action->filesSharingPermissions & Constants::PERMISSION_CREATE) === Constants::PERMISSION_CREATE) {
66+
throw new InteractionRestrictedException('Cannot share file node with create permission.', $this->l10n->t('File cannot be shared with create permission.'));
67+
}
6668
}
67-
}
6869

69-
if (!$event->receiver instanceof LinkReceiver
70-
&& !$event->receiver instanceof EmailReceiver
71-
&& ($event->action->filesSharingPermissions & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ) {
72-
throw new InteractionRestrictedException('No read permission on the share.', $this->l10n->t('File share needs at least read permission.'));
73-
}
70+
foreach ($event->receivers as $receiver) {
71+
if (!$receiver instanceof LinkReceiver
72+
&& !$receiver instanceof EmailReceiver
73+
&& ($event->action->filesSharingPermissions & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ) {
74+
throw new InteractionRestrictedException('No read permission on the share.', $this->l10n->t('File share needs at least read permission.'));
75+
}
7476

75-
if (($event->receiver instanceof LinkReceiver || $event->receiver instanceof EmailReceiver)
76-
&& $event->resource->getNode() instanceof Folder
77-
&& ($event->action->filesSharingPermissions & (Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE)) !== 0
78-
&& !$this->manager->shareApiLinkAllowPublicUpload()) {
79-
throw new InteractionRestrictedException('Public upload is not allowed.', $this->l10n->t('Public upload is not allowed.'));
80-
}
77+
if (($receiver instanceof LinkReceiver || $receiver instanceof EmailReceiver)
78+
&& $resource->getNode() instanceof Folder
79+
&& ($event->action->filesSharingPermissions & (Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE)) !== 0
80+
&& !$this->manager->shareApiLinkAllowPublicUpload()) {
81+
throw new InteractionRestrictedException('Public upload is not allowed.', $this->l10n->t('Public upload is not allowed.'));
82+
}
83+
}
8184

82-
if (($event->action->filesSharingPermissions & ~$event->resource->getNodePermissions()) !== 0) {
83-
$path = $userFolder->getRelativePath($event->resource->getNode()->getPath());
84-
throw new InteractionRestrictedException('Cannot share node with more permissions than the node already has.', $this->l10n->t('You cannot share "%s" with more permission than you have yourself.', [$path]));
85+
if (($event->action->filesSharingPermissions & ~$resource->getNodePermissions()) !== 0) {
86+
$path = $userFolder->getRelativePath($resource->getNode()->getPath());
87+
throw new InteractionRestrictedException('Cannot share node with more permissions than the node already has.', $this->l10n->t('You cannot share "%s" with more permission than you have yourself.', [$path]));
88+
}
8589
}
8690
}
8791
}

‎apps/files_sharing/tests/Listener/RestrictInteractionListenerTest.php‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,15 @@ public function testNodeResourceShareActionMissingSharePermission(): void {
6868
$this->assertNotNull($folderNode);
6969

7070
foreach ([$fileNode, $folderNode] as $node) {
71-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, new NodeResource($node->getId(), $this->user->getUID(), $node), new ShareAction(), null);
71+
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [new NodeResource($node->getId(), $this->user->getUID(), $node)], new ShareAction(), []);
7272
$this->assertEquals('You are not allowed to share "' . $node->getName() . '".', $event->isInteractionRestricted());
7373
}
7474
}
7575

7676
public function testNodeResourceShareActionNotHomeFolder(): void {
7777
$userFolder = Server::get(IRootFolder::class)->getUserFolder($this->user->getUID());
7878

79-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, new NodeResource($userFolder->getId(), $this->user->getUID(), $userFolder), new ShareAction(), null);
79+
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [new NodeResource($userFolder->getId(), $this->user->getUID(), $userFolder)], new ShareAction(), []);
8080
$this->assertEquals('You cannot share your home folder.', $event->isInteractionRestricted());
8181
}
8282

@@ -90,7 +90,7 @@ public function testNodeResourceShareActionIncreasePermission(): void {
9090
$folderNode->getStorage()->getCache()->update($folderNode->getId(), ['permissions' => Constants::PERMISSION_READ | Constants::PERMISSION_SHARE]);
9191

9292
foreach ([$fileNode, $folderNode] as $node) {
93-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, new NodeResource($node->getId(), $this->user->getUID(), $node), new ShareAction(Constants::PERMISSION_READ | Constants::PERMISSION_SHARE | Constants::PERMISSION_UPDATE), null);
93+
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [new NodeResource($node->getId(), $this->user->getUID(), $node)], new ShareAction(Constants::PERMISSION_READ | Constants::PERMISSION_SHARE | Constants::PERMISSION_UPDATE), []);
9494
$this->assertEquals('You cannot share "/' . $node->getName() . '" with more permission than you have yourself.', $event->isInteractionRestricted());
9595
}
9696
}
@@ -101,7 +101,7 @@ public function testNodeResourceShareActionIncreasePermissionFileDelete(): void
101101
$node = $userFolder->newFile('foo.txt', 'bar');
102102
$node->getStorage()->getCache()->update($node->getId(), ['permissions' => Constants::PERMISSION_READ | Constants::PERMISSION_SHARE]);
103103

104-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, new NodeResource($node->getId(), $this->user->getUID(), $node), new ShareAction(Constants::PERMISSION_READ | Constants::PERMISSION_SHARE | Constants::PERMISSION_DELETE), null);
104+
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [new NodeResource($node->getId(), $this->user->getUID(), $node)], new ShareAction(Constants::PERMISSION_READ | Constants::PERMISSION_SHARE | Constants::PERMISSION_DELETE), []);
105105
$this->assertEquals('File cannot be shared with delete permission.', $event->isInteractionRestricted());
106106
}
107107

@@ -111,7 +111,7 @@ public function testNodeResourceShareActionIncreasePermissionFileCreate(): void
111111
$node = $userFolder->newFile('foo.txt', 'bar');
112112
$node->getStorage()->getCache()->update($node->getId(), ['permissions' => Constants::PERMISSION_READ | Constants::PERMISSION_SHARE]);
113113

114-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, new NodeResource($node->getId(), $this->user->getUID(), $node), new ShareAction(Constants::PERMISSION_READ | Constants::PERMISSION_SHARE | Constants::PERMISSION_CREATE), null);
114+
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [new NodeResource($node->getId(), $this->user->getUID(), $node)], new ShareAction(Constants::PERMISSION_READ | Constants::PERMISSION_SHARE | Constants::PERMISSION_CREATE), []);
115115
$this->assertEquals('File cannot be shared with create permission.', $event->isInteractionRestricted());
116116
}
117117

@@ -121,7 +121,7 @@ public function testNodeResourceShareActionFileHasDeletePermission(): void {
121121
$node = $userFolder->newFile('foo.txt', 'bar');
122122
$node->getStorage()->getCache()->update($node->getId(), ['permissions' => Constants::PERMISSION_ALL]);
123123

124-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, new NodeResource($node->getId(), $this->user->getUID(), $node), new ShareAction(Constants::PERMISSION_DELETE), null);
124+
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [new NodeResource($node->getId(), $this->user->getUID(), $node)], new ShareAction(Constants::PERMISSION_DELETE), []);
125125
$this->assertEquals('File cannot be shared with delete permission.', $event->isInteractionRestricted());
126126
}
127127

@@ -131,7 +131,7 @@ public function testNodeResourceShareActionFileHasCreatePermission(): void {
131131
$node = $userFolder->newFile('foo.txt', 'bar');
132132
$node->getStorage()->getCache()->update($node->getId(), ['permissions' => Constants::PERMISSION_ALL]);
133133

134-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, new NodeResource($node->getId(), $this->user->getUID(), $node), new ShareAction(Constants::PERMISSION_CREATE), null);
134+
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [new NodeResource($node->getId(), $this->user->getUID(), $node)], new ShareAction(Constants::PERMISSION_CREATE), []);
135135
$this->assertEquals('File cannot be shared with create permission.', $event->isInteractionRestricted());
136136
}
137137

@@ -157,7 +157,7 @@ public function testNodeResourceShareActionNoLinkEmailReceiverMissingReadPermiss
157157
new RoomReceiver(''),
158158
new UserReceiver(''),
159159
] as $receiver) {
160-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, $resource, new ShareAction(Constants::PERMISSION_ALL & ~Constants::PERMISSION_READ), $receiver);
160+
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [$resource], new ShareAction(Constants::PERMISSION_ALL & ~Constants::PERMISSION_READ), [$receiver]);
161161
$this->assertEquals('File share needs at least read permission.', $event->isInteractionRestricted());
162162
}
163163

@@ -185,7 +185,7 @@ public function testNodeResourceShareActionLinkEmailReceiverPublicUploadDisabled
185185
Constants::PERMISSION_UPDATE,
186186
Constants::PERMISSION_DELETE,
187187
] as $permissions) {
188-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, $resource, new ShareAction($permissions), $receiver);
188+
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [$resource], new ShareAction($permissions), [$receiver]);
189189
$this->assertEquals('Public upload is not allowed.', $event->isInteractionRestricted());
190190
}
191191
}

‎core/Listener/RestrictInteractionListener.php‎

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -54,39 +54,41 @@ public function handle(Event $event): void {
5454
throw new InteractionRestrictedException('Sharing is not allowed for the user.', $this->l10n->t('Sharing is not allowed for you.'));
5555
}
5656

57-
if ($this->manager->shareWithGroupMembersOnly()) {
58-
if ($event->receiver instanceof UserReceiver) {
59-
$groups = array_intersect(
60-
$this->groupManager->getUserGroupIds($event->getUser()),
61-
$this->groupManager->getUserGroupIds($event->receiver->getUser()),
62-
);
63-
64-
$groups = array_diff($groups, $this->manager->shareWithGroupMembersOnlyExcludeGroupsList());
57+
foreach ($event->receivers as $receiver) {
58+
if ($this->manager->shareWithGroupMembersOnly()) {
59+
if ($receiver instanceof UserReceiver) {
60+
$groups = array_intersect(
61+
$this->groupManager->getUserGroupIds($event->getUser()),
62+
$this->groupManager->getUserGroupIds($receiver->getUser()),
63+
);
64+
65+
$groups = array_diff($groups, $this->manager->shareWithGroupMembersOnlyExcludeGroupsList());
66+
67+
if ($groups === []) {
68+
throw new InteractionRestrictedException('Sharing is only allowed with group members.', $this->l10n->t('Sharing is only allowed with group members.'));
69+
}
70+
}
6571

66-
if ($groups === []) {
67-
throw new InteractionRestrictedException('Sharing is only allowed with group members.', $this->l10n->t('Sharing is only allowed with group members.'));
72+
if ($receiver instanceof GroupReceiver && (!$receiver->getGroup()->inGroup($event->getUser()) || in_array($receiver->getGroup()->getGID(), $this->manager->shareWithGroupMembersOnlyExcludeGroupsList(), true))) {
73+
throw new InteractionRestrictedException('Sharing is only allowed to the groups the user is a member of.', $this->l10n->t('Sharing is only allowed within your own groups.'));
6874
}
6975
}
7076

71-
if ($event->receiver instanceof GroupReceiver && (!$event->receiver->getGroup()->inGroup($event->getUser()) || in_array($event->receiver->getGroup()->getGID(), $this->manager->shareWithGroupMembersOnlyExcludeGroupsList(), true))) {
72-
throw new InteractionRestrictedException('Sharing is only allowed to the groups the user is a member of.', $this->l10n->t('Sharing is only allowed within your own groups.'));
77+
if ($receiver instanceof GroupReceiver && !$this->manager->allowGroupSharing()) {
78+
throw new InteractionRestrictedException('Group sharing is not allowed.', $this->l10n->t('Group sharing is not allowed.'));
7379
}
74-
}
75-
76-
if ($event->receiver instanceof GroupReceiver && !$this->manager->allowGroupSharing()) {
77-
throw new InteractionRestrictedException('Group sharing is not allowed.', $this->l10n->t('Group sharing is not allowed.'));
78-
}
7980

80-
if (($event->receiver instanceof LinkReceiver || $event->receiver instanceof EmailReceiver) && !$this->manager->shareApiAllowLinks($event->getUser())) {
81-
throw new InteractionRestrictedException('Public link sharing is not allowed.', $this->l10n->t('Public link sharing is not allowed.'));
82-
}
81+
if (($receiver instanceof LinkReceiver || $receiver instanceof EmailReceiver) && !$this->manager->shareApiAllowLinks($event->getUser())) {
82+
throw new InteractionRestrictedException('Public link sharing is not allowed.', $this->l10n->t('Public link sharing is not allowed.'));
83+
}
8384

84-
if ($event->receiver instanceof RemoteUserReceiver && !$this->manager->outgoingServer2ServerSharesAllowed()) {
85-
throw new InteractionRestrictedException('Sharing to remote users is not allowed.', $this->l10n->t('Sharing to remote users is not allowed.'));
86-
}
85+
if ($receiver instanceof RemoteUserReceiver && !$this->manager->outgoingServer2ServerSharesAllowed()) {
86+
throw new InteractionRestrictedException('Sharing to remote users is not allowed.', $this->l10n->t('Sharing to remote users is not allowed.'));
87+
}
8788

88-
if ($event->receiver instanceof RemoteGroupReceiver && !$this->manager->outgoingServer2ServerGroupSharesAllowed()) {
89-
throw new InteractionRestrictedException('Sharing to remote groups is not allowed.', $this->l10n->t('Sharing to remote groups is not allowed.'));
89+
if ($receiver instanceof RemoteGroupReceiver && !$this->manager->outgoingServer2ServerGroupSharesAllowed()) {
90+
throw new InteractionRestrictedException('Sharing to remote groups is not allowed.', $this->l10n->t('Sharing to remote groups is not allowed.'));
91+
}
9092
}
9193
}
9294
}

‎lib/private/Share20/Manager.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ protected function generalChecks(IShare $share): void {
261261

262262
foreach ($users as $user) {
263263
$resource = new NodeResource($share->getNodeId(), $user->getUID());
264-
$event = new RestrictInteractionEvent($user->getUID(), $user, $resource, $action, $receiver);
264+
$event = new RestrictInteractionEvent($user->getUID(), $user, [$resource], $action, [$receiver]);
265265
try {
266266
$isRestricted = $event->isInteractionRestricted();
267267
if ($isRestricted !== false) {

0 commit comments

Comments
 (0)