Skip to content

[stable34] refactor(Interaction): Allow multiple resources and receivers in a single event #61956

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

Draft
backportbot wants to merge 1 commit into
base: stable34
Choose a base branch
from
+104 −77
Draft

[stable34] refactor(Interaction): Allow multiple resources and receivers in a single event #61956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions apps/files/lib/Listener/RestrictInteractionListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ public function __construct(
*/
#[\Override]
public function handle(Event $event): void {
if ($event->resource instanceof NodeResource && ($event->resource->getNodePermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ) {
throw new InteractionRestrictedException('No read permission on the node.', $this->l10n->t('No read permission on file.'));
foreach ($event->resources as $resource) {
if ($resource instanceof NodeResource && ($resource->getNodePermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ) {
throw new InteractionRestrictedException('No read permission on the node.', $this->l10n->t('No read permission on file.'));
}
}
}
}
2 changes: 1 addition & 1 deletion apps/files/tests/Listener/RestrictInteractionListenerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function testNodeResourceShareActionMissingReadPermission(): void {
$folderNode->getStorage()->getCache()->update($folderNode->getId(), ['permissions' => Constants::PERMISSION_ALL & ~Constants::PERMISSION_READ]);

foreach ([$fileNode, $folderNode] as $node) {
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, new NodeResource($node->getId(), $this->user->getUID(), $node), null, null);
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [new NodeResource($node->getId(), $this->user->getUID(), $node)], null, []);
$this->assertEquals('No read permission on file.', $event->isInteractionRestricted());
}
}
Expand Down
33 changes: 22 additions & 11 deletions apps/files_sharing/lib/Listener/RestrictInteractionListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,16 @@ public function __construct(
*/
#[\Override]
public function handle(Event $event): void {
if ($event->resource instanceof NodeResource && $event->action instanceof ShareAction) {
if (!$event->resource->getNode()->isShareable()) {
throw new InteractionRestrictedException('Node is not shareable.', $this->l10n->t('You are not allowed to share "%s".', [$event->resource->getNode()->getName()]));
}
foreach ($event->resources as $resource) {
if ($resource instanceof NodeResource && $event->action instanceof ShareAction) {
if (!$resource->getNode()->isShareable()) {
throw new InteractionRestrictedException('Node is not shareable.', $this->l10n->t('You are not allowed to share "%s".', [$resource->getNode()->getName()]));
}

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

if ($event->action->filesSharingPermissions !== null) {
if (($event->action->filesSharingPermissions & ~$event->resource->getNodePermissions()) !== 0) {
Expand All @@ -66,10 +67,20 @@ public function handle(Event $event): void {
throw new InteractionRestrictedException('Cannot share file node with delete permission.', $this->l10n->t('File cannot be shared with delete permission.'));
}

if (($event->action->filesSharingPermissions & Constants::PERMISSION_CREATE) === Constants::PERMISSION_CREATE) {
throw new InteractionRestrictedException('Cannot share file node with create permission.', $this->l10n->t('File cannot be shared with create permission.'));
foreach ($event->receivers as $receiver) {
if (!$receiver instanceof LinkReceiver
&& !$receiver instanceof EmailReceiver
&& ($event->action->filesSharingPermissions & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ) {
throw new InteractionRestrictedException('No read permission on the share.', $this->l10n->t('File share needs at least read permission.'));
}

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

if (!$event->receiver instanceof LinkReceiver
&& !$event->receiver instanceof EmailReceiver
Expand Down
14 changes: 7 additions & 7 deletions apps/files_sharing/tests/Listener/RestrictInteractionListenerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,15 @@ public function testNodeResourceShareActionMissingSharePermission(): void {
$this->assertNotNull($folderNode);

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

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

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

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

foreach ([$fileNode, $folderNode] as $node) {
$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);
$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), []);
$this->assertEquals('You cannot share "/' . $node->getName() . '" with more permission than you have yourself.', $event->isInteractionRestricted());
}
}
Expand All @@ -101,7 +101,7 @@ public function testNodeResourceShareActionFileHasDeletePermission(): void {
$node = $userFolder->newFile('foo.txt', 'bar');
$node->getStorage()->getCache()->update($node->getId(), ['permissions' => Constants::PERMISSION_ALL]);

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

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

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

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

Expand Down Expand Up @@ -165,7 +165,7 @@ public function testNodeResourceShareActionLinkEmailReceiverPublicUploadDisabled
Constants::PERMISSION_UPDATE,
Constants::PERMISSION_DELETE,
] as $permissions) {
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, $resource, new ShareAction($permissions), $receiver);
$event = new RestrictInteractionEvent($this->user->getUID(), $this->user, [$resource], new ShareAction($permissions), [$receiver]);
$this->assertEquals('Public upload is not allowed.', $event->isInteractionRestricted());
}
}
Expand Down
52 changes: 27 additions & 25 deletions core/Listener/RestrictInteractionListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,39 +54,41 @@ public function handle(Event $event): void {
throw new InteractionRestrictedException('Sharing is not allowed for the user.', $this->l10n->t('Sharing is not allowed for you.'));
}

if ($this->manager->shareWithGroupMembersOnly()) {
if ($event->receiver instanceof UserReceiver) {
$groups = array_intersect(
$this->groupManager->getUserGroupIds($event->getUser()),
$this->groupManager->getUserGroupIds($event->receiver->getUser()),
);

$groups = array_diff($groups, $this->manager->shareWithGroupMembersOnlyExcludeGroupsList());
foreach ($event->receivers as $receiver) {
if ($this->manager->shareWithGroupMembersOnly()) {
if ($receiver instanceof UserReceiver) {
$groups = array_intersect(
$this->groupManager->getUserGroupIds($event->getUser()),
$this->groupManager->getUserGroupIds($receiver->getUser()),
);

$groups = array_diff($groups, $this->manager->shareWithGroupMembersOnlyExcludeGroupsList());

if ($groups === []) {
throw new InteractionRestrictedException('Sharing is only allowed with group members.', $this->l10n->t('Sharing is only allowed with group members.'));
}
}

if ($groups === []) {
throw new InteractionRestrictedException('Sharing is only allowed with group members.', $this->l10n->t('Sharing is only allowed with group members.'));
if ($receiver instanceof GroupReceiver && (!$receiver->getGroup()->inGroup($event->getUser()) || in_array($receiver->getGroup()->getGID(), $this->manager->shareWithGroupMembersOnlyExcludeGroupsList(), true))) {
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.'));
}
}

if ($event->receiver instanceof GroupReceiver && (!$event->receiver->getGroup()->inGroup($event->getUser()) || in_array($event->receiver->getGroup()->getGID(), $this->manager->shareWithGroupMembersOnlyExcludeGroupsList(), true))) {
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.'));
if ($receiver instanceof GroupReceiver && !$this->manager->allowGroupSharing()) {
throw new InteractionRestrictedException('Group sharing is not allowed.', $this->l10n->t('Group sharing is not allowed.'));
}
}

if ($event->receiver instanceof GroupReceiver && !$this->manager->allowGroupSharing()) {
throw new InteractionRestrictedException('Group sharing is not allowed.', $this->l10n->t('Group sharing is not allowed.'));
}

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

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

if ($event->receiver instanceof RemoteGroupReceiver && !$this->manager->outgoingServer2ServerGroupSharesAllowed()) {
throw new InteractionRestrictedException('Sharing to remote groups is not allowed.', $this->l10n->t('Sharing to remote groups is not allowed.'));
if ($receiver instanceof RemoteGroupReceiver && !$this->manager->outgoingServer2ServerGroupSharesAllowed()) {
throw new InteractionRestrictedException('Sharing to remote groups is not allowed.', $this->l10n->t('Sharing to remote groups is not allowed.'));
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/private/Share20/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ protected function generalChecks(IShare $share): void {

foreach ($users as $user) {
$resource = new NodeResource($share->getNodeId(), $user->getUID());
$event = new RestrictInteractionEvent($user->getUID(), $user, $resource, $action, $receiver);
$event = new RestrictInteractionEvent($user->getUID(), $user, [$resource], $action, [$receiver]);
try {
$isRestricted = $event->isInteractionRestricted();
if ($isRestricted !== false) {
Expand Down
18 changes: 10 additions & 8 deletions lib/public/Interaction/RestrictInteractionEvent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
use RuntimeException;

/**
* 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.
* If the resource has a distinction between an owner and a initiator, the event must be emitted for both.
* 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.
* If the resources have a distinction between an owner and a initiator, the event must be emitted for each separately.
* Emitters may omit one or multiple properties, if they are not known.
* Emitters must call {@see isInteractionRestricted} instead of dispatching the event manually, to ensure exception handling and audit logging is done correctly.
* Listeners may ignore any of the properties and only check the ones that are relevant to them.
Expand All @@ -36,9 +36,11 @@ final class RestrictInteractionEvent extends Event {
public function __construct(
public readonly string $userId,
private ?IUser $user,
public readonly ?InteractionResource $resource,
/** @var list<InteractionResource> $resources */
public readonly array $resources,
public readonly ?InteractionAction $action,
public readonly ?InteractionReceiver $receiver,
/** @var list<InteractionReceiver> $receivers */
public readonly array $receivers,
) {
parent::__construct();
}
Expand Down Expand Up @@ -69,22 +71,22 @@ public function isInteractionRestricted(): false|string {
$params = [
$this->action instanceof InteractionAction ? $this->action::class : '?',
$this->userId,
$this->resource?->getID() ?? '?',
$this->receiver?->getID() ?? '?',
$this->resources === [] ? '?' : implode(', ', array_map(static fn (InteractionResource $resource): string => $resource->getID(), $this->resources)),
$this->receivers === [] ? '?' : implode(', ', array_map(static fn (InteractionReceiver $receiver): string => $receiver->getID() ?? $receiver::class, $this->receivers)),
];

try {
$eventDispatcher->dispatchTyped($this);

$eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent(
'Interaction "%s" from user "%s" on "%s" to "%s" is allowed.',
'Interaction %s from user %s on %s to %s is allowed.',
$params,
));

return false;
} catch (InteractionRestrictedException $interactionRestrictedException) {
$eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent(
'Interaction "%s" from user "%s" on "%s" to "%s" is restricted: ' . $interactionRestrictedException->getMessage(),
'Interaction %s from user %s on %s to %s is restricted: ' . $interactionRestrictedException->getMessage(),
$params,
));

Expand Down
Loading