fix: update shares on group delete

Signed-off-by: Robin Appelman <robin@icewind.nl>

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);
 

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<UserAddedEvent|UserRemovedEvent|ShareCreatedEvent|ShareTransferredEvent|BeforeShareDeletedEvent|UserShareAccessUpdatedEvent>
+ * @psalm-type GroupEvents = UserAddedEvent|UserRemovedEvent|GroupDeletedEvent|BeforeGroupDeletedEvent
+ * @template-implements IEventListener<GroupEvents|ShareCreatedEvent|ShareTransferredEvent|BeforeShareDeletedEvent|UserShareAccessUpdatedEvent>
  */
 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());
 		}

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);
+	}
 }

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();
 		}

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