Merge pull request #51205 from nextcloud/backport/51000/stable30

[stable30] fix(FederatedShareProvider): Delete external shares when groups are deleted or users removed from a group

apps/federatedfilesharing/lib/FederatedShareProvider.php

@@ -880,30 +880,60 @@ class FederatedShareProvider implements IShareProvider {
 		//TODO: probably a good idea to send unshare info to remote servers
 
 		$qb = $this->dbConnection->getQueryBuilder();
-
 		$qb->delete('share')
 			->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_REMOTE)))
 			->andWhere($qb->expr()->eq('uid_owner', $qb->createNamedParameter($uid)))
-			->execute();
+			->executeStatement();
+
+		$qb = $this->dbConnection->getQueryBuilder();
+		$qb->delete('share_external')
+			->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_GROUP)))
+			->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($uid)))
+			->executeStatement();
 	}
 
-	/**
-	 * This provider does not handle groups
-	 *
-	 * @param string $gid
-	 */
 	public function groupDeleted($gid) {
-		// We don't handle groups here
+		$qb = $this->dbConnection->getQueryBuilder();
+		$qb->select('id')
+			->from('share_external')
+			->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_GROUP)))
+			// This is not a typo, the group ID is really stored in the 'user' column
+			->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($gid)));
+		$cursor = $qb->executeQuery();
+		$parentShareIds = $cursor->fetchAll(\PDO::FETCH_COLUMN);
+		$cursor->closeCursor();
+		if ($parentShareIds === []) {
+			return;
+		}
+
+		$qb = $this->dbConnection->getQueryBuilder();
+		$parentShareIdsParam = $qb->createNamedParameter($parentShareIds, IQueryBuilder::PARAM_INT_ARRAY);
+		$qb->delete('share_external')
+			->where($qb->expr()->in('id', $parentShareIdsParam))
+			->orWhere($qb->expr()->in('parent', $parentShareIdsParam))
+			->executeStatement();
 	}
 
-	/**
-	 * This provider does not handle groups
-	 *
-	 * @param string $uid
-	 * @param string $gid
-	 */
 	public function userDeletedFromGroup($uid, $gid) {
-		// We don't handle groups here
+		$qb = $this->dbConnection->getQueryBuilder();
+		$qb->select('id')
+			->from('share_external')
+			->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_GROUP)))
+			// This is not a typo, the group ID is really stored in the 'user' column
+			->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($gid)));
+		$cursor = $qb->executeQuery();
+		$parentShareIds = $cursor->fetchAll(\PDO::FETCH_COLUMN);
+		$cursor->closeCursor();
+		if ($parentShareIds === []) {
+			return;
+		}
+
+		$qb = $this->dbConnection->getQueryBuilder();
+		$parentShareIdsParam = $qb->createNamedParameter($parentShareIds, IQueryBuilder::PARAM_INT_ARRAY);
+		$qb->delete('share_external')
+			->where($qb->expr()->in('parent', $parentShareIdsParam))
+			->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($uid)))
+			->executeStatement();
 	}
 
 	/**

build/integration/sharing_features/sharing-v1-part2.feature

@@ -543,6 +543,29 @@ Feature: sharing
     And the HTTP status code should be "200"
     And last share_id is included in the answer
 
+  Scenario: Group shares are deleted when the group is deleted
+    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"
+    When sending "GET" to "/apps/files_sharing/api/v1/shares?shared_with_me=true"
+    Then the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share_id is included in the answer
+    When group "group0" does not exist
+    Then sending "GET" to "/apps/files_sharing/api/v1/shares?shared_with_me=true"
+    And the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share_id is not included in the answer
+    When group "group0" exists
+    Then sending "GET" to "/apps/files_sharing/api/v1/shares?shared_with_me=true"
+    And the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And last share_id is not included in the answer
+
   Scenario: User is not allowed to reshare file
   As an "admin"
     Given user "user0" exists

lib/base.php

@@ -7,8 +7,12 @@ declare(strict_types=1);
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 use OC\Encryption\HookManager;
+use OC\Share20\GroupDeletedListener;
 use OC\Share20\Hooks;
+use OC\Share20\UserDeletedListener;
+use OC\Share20\UserRemovedListener;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Group\Events\GroupDeletedEvent;
 use OCP\Group\Events\UserRemovedEvent;
 use OCP\ILogger;
 use OCP\IRequest;
@@ -18,6 +22,7 @@ use OCP\Security\Bruteforce\IThrottler;
 use OCP\Server;
 use OCP\Share;
 use OCP\User\Events\UserChangedEvent;
+use OCP\User\Events\UserDeletedEvent;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\Routing\Exception\MethodNotAllowedException;
 use function OCP\Log\logger;
@@ -911,12 +916,11 @@ class OC {
 	 */
 	public static function registerShareHooks(\OC\SystemConfig $systemConfig): void {
 		if ($systemConfig->getValue('installed')) {
-			OC_Hook::connect('OC_User', 'post_deleteUser', Hooks::class, 'post_deleteUser');
-			OC_Hook::connect('OC_User', 'post_deleteGroup', Hooks::class, 'post_deleteGroup');
 
-			/** @var IEventDispatcher $dispatcher */
 			$dispatcher = Server::get(IEventDispatcher::class);
-			$dispatcher->addServiceListener(UserRemovedEvent::class, \OC\Share20\UserRemovedListener::class);
+			$dispatcher->addServiceListener(UserRemovedEvent::class, UserRemovedListener::class);
+			$dispatcher->addServiceListener(GroupDeletedEvent::class, GroupDeletedListener::class);
+			$dispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedListener::class);
 		}
 	}
 

lib/composer/composer/autoload_classmap.php

@@ -1918,7 +1918,7 @@ return array(
     'OC\\Share20\\Exception\\BackendError' => $baseDir . '/lib/private/Share20/Exception/BackendError.php',
     'OC\\Share20\\Exception\\InvalidShare' => $baseDir . '/lib/private/Share20/Exception/InvalidShare.php',
     'OC\\Share20\\Exception\\ProviderException' => $baseDir . '/lib/private/Share20/Exception/ProviderException.php',
-    'OC\\Share20\\Hooks' => $baseDir . '/lib/private/Share20/Hooks.php',
+    'OC\\Share20\\GroupDeletedListener' => $baseDir . '/lib/private/Share20/GroupDeletedListener.php',
     'OC\\Share20\\LegacyHooks' => $baseDir . '/lib/private/Share20/LegacyHooks.php',
     'OC\\Share20\\Manager' => $baseDir . '/lib/private/Share20/Manager.php',
     'OC\\Share20\\ProviderFactory' => $baseDir . '/lib/private/Share20/ProviderFactory.php',
@@ -1927,6 +1927,7 @@ return array(
     'OC\\Share20\\ShareAttributes' => $baseDir . '/lib/private/Share20/ShareAttributes.php',
     'OC\\Share20\\ShareDisableChecker' => $baseDir . '/lib/private/Share20/ShareDisableChecker.php',
     'OC\\Share20\\ShareHelper' => $baseDir . '/lib/private/Share20/ShareHelper.php',
+    'OC\\Share20\\UserDeletedListener' => $baseDir . '/lib/private/Share20/UserDeletedListener.php',
     'OC\\Share20\\UserRemovedListener' => $baseDir . '/lib/private/Share20/UserRemovedListener.php',
     'OC\\Share\\Constants' => $baseDir . '/lib/private/Share/Constants.php',
     'OC\\Share\\Helper' => $baseDir . '/lib/private/Share/Helper.php',

lib/composer/composer/autoload_static.php

@@ -1951,7 +1951,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Share20\\Exception\\BackendError' => __DIR__ . '/../../..' . '/lib/private/Share20/Exception/BackendError.php',
         'OC\\Share20\\Exception\\InvalidShare' => __DIR__ . '/../../..' . '/lib/private/Share20/Exception/InvalidShare.php',
         'OC\\Share20\\Exception\\ProviderException' => __DIR__ . '/../../..' . '/lib/private/Share20/Exception/ProviderException.php',
-        'OC\\Share20\\Hooks' => __DIR__ . '/../../..' . '/lib/private/Share20/Hooks.php',
+        'OC\\Share20\\GroupDeletedListener' => __DIR__ . '/../../..' . '/lib/private/Share20/GroupDeletedListener.php',
         'OC\\Share20\\LegacyHooks' => __DIR__ . '/../../..' . '/lib/private/Share20/LegacyHooks.php',
         'OC\\Share20\\Manager' => __DIR__ . '/../../..' . '/lib/private/Share20/Manager.php',
         'OC\\Share20\\ProviderFactory' => __DIR__ . '/../../..' . '/lib/private/Share20/ProviderFactory.php',
@@ -1960,6 +1960,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Share20\\ShareAttributes' => __DIR__ . '/../../..' . '/lib/private/Share20/ShareAttributes.php',
         'OC\\Share20\\ShareDisableChecker' => __DIR__ . '/../../..' . '/lib/private/Share20/ShareDisableChecker.php',
         'OC\\Share20\\ShareHelper' => __DIR__ . '/../../..' . '/lib/private/Share20/ShareHelper.php',
+        'OC\\Share20\\UserDeletedListener' => __DIR__ . '/../../..' . '/lib/private/Share20/UserDeletedListener.php',
         'OC\\Share20\\UserRemovedListener' => __DIR__ . '/../../..' . '/lib/private/Share20/UserRemovedListener.php',
         'OC\\Share\\Constants' => __DIR__ . '/../../..' . '/lib/private/Share/Constants.php',
         'OC\\Share\\Helper' => __DIR__ . '/../../..' . '/lib/private/Share/Helper.php',

lib/private/Share20/GroupDeletedListener.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OC\Share20;
+
+use OCP\EventDispatcher\Event;
+use OCP\EventDispatcher\IEventListener;
+use OCP\Group\Events\GroupDeletedEvent;
+use OCP\Share\IManager;
+
+/**
+ * @template-implements IEventListener<GroupDeletedEvent>
+ */
+class GroupDeletedListener implements IEventListener {
+	public function __construct(
+		protected IManager $shareManager,
+	) {
+	}
+
+	public function handle(Event $event): void {
+		if (!$event instanceof GroupDeletedEvent) {
+			return;
+		}
+
+		$this->shareManager->groupDeleted($event->getGroup()->getGID());
+	}
+}

lib/private/Share20/Hooks.php

@@ -1,20 +0,0 @@
-<?php
-
-/**
- * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
- * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-namespace OC\Share20;
-
-use OCP\Share\IManager as IShareManager;
-
-class Hooks {
-	public static function post_deleteUser($arguments) {
-		\OC::$server->get(IShareManager::class)->userDeleted($arguments['uid']);
-	}
-
-	public static function post_deleteGroup($arguments) {
-		\OC::$server->get(IShareManager::class)->groupDeleted($arguments['gid']);
-	}
-}

lib/private/Share20/Manager.php

@@ -1536,8 +1536,14 @@ class Manager implements IManager {
 	 * @inheritdoc
 	 */
 	public function groupDeleted($gid) {
-		$provider = $this->factory->getProviderForType(IShare::TYPE_GROUP);
-		$provider->groupDeleted($gid);
+		foreach ([IShare::TYPE_GROUP, IShare::TYPE_REMOTE_GROUP] as $type) {
+			try {
+				$provider = $this->factory->getProviderForType($type);
+			} catch (ProviderException $e) {
+				continue;
+			}
+			$provider->groupDeleted($gid);
+		}
 
 		$excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', '');
 		if ($excludedGroups === '') {
@@ -1557,8 +1563,14 @@ class Manager implements IManager {
 	 * @inheritdoc
 	 */
 	public function userDeletedFromGroup($uid, $gid) {
-		$provider = $this->factory->getProviderForType(IShare::TYPE_GROUP);
-		$provider->userDeletedFromGroup($uid, $gid);
+		foreach ([IShare::TYPE_GROUP, IShare::TYPE_REMOTE_GROUP] as $type) {
+			try {
+				$provider = $this->factory->getProviderForType($type);
+			} catch (ProviderException $e) {
+				continue;
+			}
+			$provider->userDeletedFromGroup($uid, $gid);
+		}
 	}
 
 	/**

lib/private/Share20/UserDeletedListener.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OC\Share20;
+
+use OCP\EventDispatcher\Event;
+use OCP\EventDispatcher\IEventListener;
+use OCP\Share\IManager;
+use OCP\User\Events\UserDeletedEvent;
+
+/**
+ * @template-implements IEventListener<UserDeletedEvent>
+ */
+class UserDeletedListener implements IEventListener {
+	public function __construct(
+		protected IManager $shareManager,
+	) {
+	}
+
+	public function handle(Event $event): void {
+		if (!$event instanceof UserDeletedEvent) {
+			return;
+		}
+
+		$this->shareManager->userDeleted($event->getUser()->getUID());
+	}
+}