Skip to content

Commit 036bc51

Browse files
provokateurinprovokateurin
committed
refactor(Interaction): Allow multiple resources and receivers in a single event
Signed-off-by: provokateurin <kate@provokateurin.de>
1 parent c89eab5 commit 036bc51

9 files changed

Lines changed: 115 additions & 95 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: 33 additions & 29 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-
}
52-
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-
}
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+
}
5753

58-
if ($event->action->filesSharingPermissions !== null) {
59-
if (($event->action->filesSharingPermissions & ~$event->resource->getNodePermissions()) !== 0) {
60-
$path = $userFolder->getRelativePath($event->resource->getNode()->getPath());
61-
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]));
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.'));
6257
}
6358

64-
if ($event->resource->getNode() instanceof File) {
65-
if (($event->action->filesSharingPermissions & Constants::PERMISSION_DELETE) === Constants::PERMISSION_DELETE) {
66-
throw new InteractionRestrictedException('Cannot share file node with delete permission.', $this->l10n->t('File cannot be shared with delete permission.'));
59+
if ($event->action->filesSharingPermissions !== null) {
60+
if (($event->action->filesSharingPermissions & ~$resource->getNodePermissions()) !== 0) {
61+
$path = $userFolder->getRelativePath($resource->getNode()->getPath());
62+
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]));
6763
}
6864

69-
if (($event->action->filesSharingPermissions & Constants::PERMISSION_CREATE) === Constants::PERMISSION_CREATE) {
70-
throw new InteractionRestrictedException('Cannot share file node with create permission.', $this->l10n->t('File cannot be shared with create permission.'));
65+
if ($resource->getNode() instanceof File) {
66+
if (($event->action->filesSharingPermissions & Constants::PERMISSION_DELETE) === Constants::PERMISSION_DELETE) {
67+
throw new InteractionRestrictedException('Cannot share file node with delete permission.', $this->l10n->t('File cannot be shared with delete permission.'));
68+
}
69+
70+
if (($event->action->filesSharingPermissions & Constants::PERMISSION_CREATE) === Constants::PERMISSION_CREATE) {
71+
throw new InteractionRestrictedException('Cannot share file node with create permission.', $this->l10n->t('File cannot be shared with create permission.'));
72+
}
7173
}
72-
}
7374

74-
if (!$event->receiver instanceof LinkReceiver
75-
&& !$event->receiver instanceof EmailReceiver
76-
&& ($event->action->filesSharingPermissions & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ) {
77-
throw new InteractionRestrictedException('No read permission on the share.', $this->l10n->t('File share needs at least read permission.'));
78-
}
75+
foreach ($event->receivers as $receiver) {
76+
if (!$receiver instanceof LinkReceiver
77+
&& !$receiver instanceof EmailReceiver
78+
&& ($event->action->filesSharingPermissions & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ) {
79+
throw new InteractionRestrictedException('No read permission on the share.', $this->l10n->t('File share needs at least read permission.'));
80+
}
7981

80-
if (($event->receiver instanceof LinkReceiver || $event->receiver instanceof EmailReceiver)
81-
&& $event->resource->getNode() instanceof Folder
82-
&& ($event->action->filesSharingPermissions & (Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE)) !== 0
83-
&& !$this->manager->shareApiLinkAllowPublicUpload()) {
84-
throw new InteractionRestrictedException('Public upload is not allowed.', $this->l10n->t('Public upload is not allowed.'));
82+
if (($receiver instanceof LinkReceiver || $receiver instanceof EmailReceiver)
83+
&& $resource->getNode() instanceof Folder
84+
&& ($event->action->filesSharingPermissions & (Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE)) !== 0
85+
&& !$this->manager->shareApiLinkAllowPublicUpload()) {
86+
throw new InteractionRestrictedException('Public upload is not allowed.', $this->l10n->t('Public upload is not allowed.'));
87+
}
88+
}
8589
}
8690
}
8791
}

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

Lines changed: 7 additions & 7 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 testNodeResourceShareActionFileHasDeletePermission(): void {
101101
$node = $userFolder->newFile('foo.txt', 'bar');
102102
$node->getStorage()->getCache()->update($node->getId(), ['permissions' => Constants::PERMISSION_ALL]);
103103

104-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, new NodeResource($node->getId(), $this->user->getUID(), $node), new ShareAction(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_DELETE), []);
105105
$this->assertEquals('File cannot be shared with delete permission.', $event->isInteractionRestricted());
106106
}
107107

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

114-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, new NodeResource($node->getId(), $this->user->getUID(), $node), new ShareAction(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_CREATE), []);
115115
$this->assertEquals('File cannot be shared with create permission.', $event->isInteractionRestricted());
116116
}
117117

@@ -137,7 +137,7 @@ public function testNodeResourceShareActionNoLinkEmailReceiverMissingReadPermiss
137137
new RoomReceiver(''),
138138
new UserReceiver(''),
139139
] as $receiver) {
140-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, $resource, new ShareAction(Constants::PERMISSION_ALL & ~Constants::PERMISSION_READ), $receiver);
140+
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [$resource], new ShareAction(Constants::PERMISSION_ALL & ~Constants::PERMISSION_READ), [$receiver]);
141141
$this->assertEquals('File share needs at least read permission.', $event->isInteractionRestricted());
142142
}
143143

@@ -165,7 +165,7 @@ public function testNodeResourceShareActionLinkEmailReceiverPublicUploadDisabled
165165
Constants::PERMISSION_UPDATE,
166166
Constants::PERMISSION_DELETE,
167167
] as $permissions) {
168-
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, $resource, new ShareAction($permissions), $receiver);
168+
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [$resource], new ShareAction($permissions), [$receiver]);
169169
$this->assertEquals('Public upload is not allowed.', $event->isInteractionRestricted());
170170
}
171171
}

‎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) {

‎lib/public/Interaction/RestrictInteractionEvent.php‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
use RuntimeException;
2020

2121
/**
22-
* This event can be emitted to check if a user is allowed to perform an action, such that the receiver would become aware of the resource.
23-
* If the resource has a distinction between an owner and a initiator, the event must be emitted for both.
22+
* This event can be emitted to check if a user is allowed to perform an action, such that the receivers would become aware of the resources.
23+
* If the resources have a distinction between an owner and a initiator, the event must be emitted for each separately.
2424
* Emitters may omit one or multiple properties, if they are not known.
2525
* Emitters must call {@see isInteractionRestricted} instead of dispatching the event manually, to ensure exception handling and audit logging is done correctly.
2626
* Listeners may ignore any of the properties and only check the ones that are relevant to them.
@@ -36,9 +36,11 @@ final class RestrictInteractionEvent extends Event {
3636
public function __construct(
3737
public readonly string $userId,
3838
private ?IUser $user,
39-
public readonly ?InteractionResource $resource,
39+
/** @var list<InteractionResource> $resources */
40+
public readonly array $resources,
4041
public readonly ?InteractionAction $action,
41-
public readonly ?InteractionReceiver $receiver,
42+
/** @var list<InteractionReceiver> $receivers */
43+
public readonly array $receivers,
4244
) {
4345
parent::__construct();
4446
}
@@ -69,22 +71,22 @@ public function isInteractionRestricted(): false|string {
6971
$params = [
7072
$this->action instanceof InteractionAction ? $this->action::class : '?',
7173
$this->userId,
72-
$this->resource?->getID() ?? '?',
73-
$this->receiver?->getID() ?? '?',
74+
$this->resources === [] ? '?' : implode(', ', array_map(static fn (InteractionResource $resource): string => $resource->getID(), $this->resources)),
75+
$this->receivers === [] ? '?' : implode(', ', array_map(static fn (InteractionReceiver $receiver): string => $receiver->getID() ?? $receiver::class, $this->receivers)),
7476
];
7577

7678
try {
7779
$eventDispatcher->dispatchTyped($this);
7880

7981
$eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent(
80-
'Interaction "%s" from user "%s" on "%s" to "%s" is allowed.',
82+
'Interaction %s from user %s on %s to %s is allowed.',
8183
$params,
8284
));
8385

8486
return false;
8587
} catch (InteractionRestrictedException $interactionRestrictedException) {
8688
$eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent(
87-
'Interaction "%s" from user "%s" on "%s" to "%s" is restricted: ' . $interactionRestrictedException->getMessage(),
89+
'Interaction %s from user %s on %s to %s is restricted: ' . $interactionRestrictedException->getMessage(),
8890
$params,
8991
));
9092

0 commit comments

Comments
 (0)