From e63fe87b5cc8bd249b27721b40a57dfad9d2d66d Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 25 Feb 2026 19:51:27 +0100 Subject: [PATCH] fix: update shares on group delete Signed-off-by: Robin Appelman --- .../files_sharing/lib/AppInfo/Application.php | 3 ++ .../lib/Listener/SharesUpdatedListener.php | 15 ++++++- .../features/bootstrap/Sharing.php | 40 ++++++++++++++++--- .../integration/features/bootstrap/WebDav.php | 2 +- .../sharing_features/sharing-v1-part4.feature | 32 +++++++++++++++ 5 files changed, 85 insertions(+), 7 deletions(-) diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index 0cdb2e71f24..2fa251802fe 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -50,6 +50,7 @@ use OCP\Files\Events\BeforeDirectFileDownloadEvent; use OCP\Files\Events\BeforeZipCreatedEvent; use OCP\Files\Events\Node\BeforeNodeReadEvent; use OCP\Files\Events\UserHomeSetupEvent; +use OCP\Group\Events\BeforeGroupDeletedEvent; use OCP\Group\Events\GroupChangedEvent; use OCP\Group\Events\GroupDeletedEvent; use OCP\Group\Events\UserAddedEvent; @@ -124,6 +125,8 @@ class Application extends App implements IBootstrap { $context->registerEventListener(ShareTransferredEvent::class, SharesUpdatedListener::class); $context->registerEventListener(UserAddedEvent::class, SharesUpdatedListener::class); $context->registerEventListener(UserRemovedEvent::class, SharesUpdatedListener::class); + $context->registerEventListener(BeforeGroupDeletedEvent::class, SharesUpdatedListener::class); + $context->registerEventListener(GroupDeletedEvent::class, SharesUpdatedListener::class); $context->registerEventListener(UserShareAccessUpdatedEvent::class, SharesUpdatedListener::class); $context->registerEventListener(UserHomeSetupEvent::class, UserHomeSetupListener::class); diff --git a/apps/files_sharing/lib/Listener/SharesUpdatedListener.php b/apps/files_sharing/lib/Listener/SharesUpdatedListener.php index 1ab52a069ba..e17e392c906 100644 --- a/apps/files_sharing/lib/Listener/SharesUpdatedListener.php +++ b/apps/files_sharing/lib/Listener/SharesUpdatedListener.php @@ -15,6 +15,8 @@ use OCA\Files_Sharing\ShareRecipientUpdater; use OCP\Config\IUserConfig; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; +use OCP\Group\Events\BeforeGroupDeletedEvent; +use OCP\Group\Events\GroupDeletedEvent; use OCP\Group\Events\UserAddedEvent; use OCP\Group\Events\UserRemovedEvent; use OCP\IAppConfig; @@ -28,7 +30,8 @@ use Psr\Clock\ClockInterface; /** * Listen to various events that can change what shares a user has access to * - * @template-implements IEventListener + * @psalm-type GroupEvents = UserAddedEvent|UserRemovedEvent|GroupDeletedEvent|BeforeGroupDeletedEvent + * @template-implements IEventListener */ class SharesUpdatedListener implements IEventListener { /** @@ -58,6 +61,16 @@ class SharesUpdatedListener implements IEventListener { $this->updateOrMarkUser($user); } } + if ($event instanceof BeforeGroupDeletedEvent) { + // ensure the group users are loaded before the group is deleted + $event->getGroup()->getUsers(); + } + if ($event instanceof GroupDeletedEvent) { + // so we can iterate them after the group is deleted + foreach ($event->getGroup()->getUsers() as $user) { + $this->updateOrMarkUser($user); + } + } if ($event instanceof UserAddedEvent || $event instanceof UserRemovedEvent) { $this->updateOrMarkUser($event->getUser()); } diff --git a/build/integration/features/bootstrap/Sharing.php b/build/integration/features/bootstrap/Sharing.php index 72f6902af16..5fe98aff35d 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -5,6 +5,7 @@ * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-or-later */ + use Behat\Gherkin\Node\TableNode; use GuzzleHttp\Client; use PHPUnit\Framework\Assert; @@ -13,7 +14,6 @@ use Psr\Http\Message\ResponseInterface; require __DIR__ . '/autoload.php'; - trait Sharing { use Provisioning; @@ -566,17 +566,17 @@ trait Sharing { $expectedFields = array_merge($defaultExpectedFields, $body->getRowsHash()); if (!array_key_exists('uid_file_owner', $expectedFields) - && array_key_exists('uid_owner', $expectedFields)) { + && array_key_exists('uid_owner', $expectedFields)) { $expectedFields['uid_file_owner'] = $expectedFields['uid_owner']; } if (!array_key_exists('displayname_file_owner', $expectedFields) - && array_key_exists('displayname_owner', $expectedFields)) { + && array_key_exists('displayname_owner', $expectedFields)) { $expectedFields['displayname_file_owner'] = $expectedFields['displayname_owner']; } if (array_key_exists('share_type', $expectedFields) - && $expectedFields['share_type'] == 10 /* IShare::TYPE_ROOM */ - && array_key_exists('share_with', $expectedFields)) { + && $expectedFields['share_type'] == 10 /* IShare::TYPE_ROOM */ + && array_key_exists('share_with', $expectedFields)) { if ($expectedFields['share_with'] === 'private_conversation') { $expectedFields['share_with'] = 'REGEXP /^private_conversation_[0-9a-f]{6}$/'; } else { @@ -782,4 +782,34 @@ trait Sharing { } return $sharees; } + + /** + * @Then /^Share mounts for "([^"]*)" match$/ + */ + public function checkShareMounts(string $user, ?TableNode $body) { + if ($body instanceof TableNode) { + $fd = $body->getRows(); + + $expected = []; + foreach ($fd as $row) { + $expected[] = $row[0]; + } + $this->runOcc(['files:mount:list', '--output', 'json', '--cached-only', $user]); + $mounts = json_decode($this->lastStdOut, true)['cached']; + $shareMounts = array_filter($mounts, fn (array $data) => $data['provider'] === \OCA\Files_Sharing\MountProvider::class); + $actual = array_values(array_map(fn (array $data) => $data['mountpoint'], $shareMounts)); + Assert::assertEquals($expected, $actual); + } + } + + /** + * @Then /^Share mounts for "([^"]*)" are empty$/ + */ + public function checkShareMountsEmpty(string $user) { + $this->runOcc(['files:mount:list', '--output', 'json', '--cached-only', $user]); + $mounts = json_decode($this->lastStdOut, true)['cached']; + $shareMounts = array_filter($mounts, fn (array $data) => $data['provider'] === \OCA\Files_Sharing\MountProvider::class); + $actual = array_values(array_map(fn (array $data) => $data['mountpoint'], $shareMounts)); + Assert::assertEquals([], $actual); + } } diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index fb552ce785b..fb2e441d937 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -1011,7 +1011,7 @@ trait WebDav { */ public function connectingToDavEndpoint() { try { - $this->response = $this->makeDavRequest(null, 'PROPFIND', '', []); + $this->response = $this->makeDavRequest($this->currentUser, 'PROPFIND', '', []); } catch (\GuzzleHttp\Exception\ClientException $e) { $this->response = $e->getResponse(); } diff --git a/build/integration/sharing_features/sharing-v1-part4.feature b/build/integration/sharing_features/sharing-v1-part4.feature index 3b825aebd18..48f2e2e3b12 100644 --- a/build/integration/sharing_features/sharing-v1-part4.feature +++ b/build/integration/sharing_features/sharing-v1-part4.feature @@ -315,3 +315,35 @@ Scenario: Can copy file between shares if share permissions And the OCS status code should be "100" When User "user1" copies file "/share/test.txt" to "/re-share/movetest.txt" Then the HTTP status code should be "201" + +Scenario: Group deletes removes mount without marking + Given As an "admin" + And user "user0" exists + And user "user1" exists + And group "group0" exists + And user "user0" belongs to group "group0" + And file "textfile0.txt" of user "user1" is shared with group "group0" + And As an "user0" + Then Share mounts for "user0" match + | /user0/files/textfile0 (2).txt/ | + And group "group0" does not exist + Then Share mounts for "user0" are empty + +Scenario: Group deletes removes mount with marking + Given As an "admin" + And parameter "update_cutoff_time" of app "files_sharing" is set to "0" + And user "user0" exists + And user "user1" exists + And group "group0" exists + And user "user0" belongs to group "group0" + And file "textfile0.txt" of user "user1" is shared with group "group0" + And As an "user0" + Then Share mounts for "user0" are empty + When Connecting to dav endpoint + Then Share mounts for "user0" match + | /user0/files/textfile0 (2).txt/ | + And group "group0" does not exist + Then Share mounts for "user0" match + | /user0/files/textfile0 (2).txt/ | + When Connecting to dav endpoint + Then Share mounts for "user0" are empty