Skip to content

Commit 17f9f29

Browse files
sorbaughsorbaugh
authored
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
2 parents 745b72b + 46cec25 commit 17f9f29

9 files changed

Lines changed: 160 additions & 45 deletions

File tree

‎apps/federatedfilesharing/lib/FederatedShareProvider.php‎

Lines changed: 45 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -880,30 +880,60 @@ public function userDeleted($uid, $shareType) {
880880
//TODO: probably a good idea to send unshare info to remote servers
881881

882882
$qb = $this->dbConnection->getQueryBuilder();
883-
884883
$qb->delete('share')
885884
->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_REMOTE)))
886885
->andWhere($qb->expr()->eq('uid_owner', $qb->createNamedParameter($uid)))
887-
->execute();
886+
->executeStatement();
887+
888+
$qb = $this->dbConnection->getQueryBuilder();
889+
$qb->delete('share_external')
890+
->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_GROUP)))
891+
->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($uid)))
892+
->executeStatement();
888893
}
889894

890-
/**
891-
* This provider does not handle groups
892-
*
893-
* @param string $gid
894-
*/
895895
public function groupDeleted($gid) {
896-
// We don't handle groups here
896+
$qb = $this->dbConnection->getQueryBuilder();
897+
$qb->select('id')
898+
->from('share_external')
899+
->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_GROUP)))
900+
// This is not a typo, the group ID is really stored in the 'user' column
901+
->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($gid)));
902+
$cursor = $qb->executeQuery();
903+
$parentShareIds = $cursor->fetchAll(\PDO::FETCH_COLUMN);
904+
$cursor->closeCursor();
905+
if ($parentShareIds === []) {
906+
return;
907+
}
908+
909+
$qb = $this->dbConnection->getQueryBuilder();
910+
$parentShareIdsParam = $qb->createNamedParameter($parentShareIds, IQueryBuilder::PARAM_INT_ARRAY);
911+
$qb->delete('share_external')
912+
->where($qb->expr()->in('id', $parentShareIdsParam))
913+
->orWhere($qb->expr()->in('parent', $parentShareIdsParam))
914+
->executeStatement();
897915
}
898916

899-
/**
900-
* This provider does not handle groups
901-
*
902-
* @param string $uid
903-
* @param string $gid
904-
*/
905917
public function userDeletedFromGroup($uid, $gid) {
906-
// We don't handle groups here
918+
$qb = $this->dbConnection->getQueryBuilder();
919+
$qb->select('id')
920+
->from('share_external')
921+
->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_GROUP)))
922+
// This is not a typo, the group ID is really stored in the 'user' column
923+
->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($gid)));
924+
$cursor = $qb->executeQuery();
925+
$parentShareIds = $cursor->fetchAll(\PDO::FETCH_COLUMN);
926+
$cursor->closeCursor();
927+
if ($parentShareIds === []) {
928+
return;
929+
}
930+
931+
$qb = $this->dbConnection->getQueryBuilder();
932+
$parentShareIdsParam = $qb->createNamedParameter($parentShareIds, IQueryBuilder::PARAM_INT_ARRAY);
933+
$qb->delete('share_external')
934+
->where($qb->expr()->in('parent', $parentShareIdsParam))
935+
->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($uid)))
936+
->executeStatement();
907937
}
908938

909939
/**

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,29 @@ Feature: sharing
543543
And the HTTP status code should be "200"
544544
And last share_id is included in the answer
545545

546+
Scenario: Group shares are deleted when the group is deleted
547+
Given As an "admin"
548+
And user "user0" exists
549+
And user "user1" exists
550+
And group "group0" exists
551+
And user "user0" belongs to group "group0"
552+
And file "textfile0.txt" of user "user1" is shared with group "group0"
553+
And As an "user0"
554+
When sending "GET" to "/apps/files_sharing/api/v1/shares?shared_with_me=true"
555+
Then the OCS status code should be "100"
556+
And the HTTP status code should be "200"
557+
And last share_id is included in the answer
558+
When group "group0" does not exist
559+
Then sending "GET" to "/apps/files_sharing/api/v1/shares?shared_with_me=true"
560+
And the OCS status code should be "100"
561+
And the HTTP status code should be "200"
562+
And last share_id is not included in the answer
563+
When group "group0" exists
564+
Then sending "GET" to "/apps/files_sharing/api/v1/shares?shared_with_me=true"
565+
And the OCS status code should be "100"
566+
And the HTTP status code should be "200"
567+
And last share_id is not included in the answer
568+
546569
Scenario: User is not allowed to reshare file
547570
As an "admin"
548571
Given user "user0" exists

‎lib/base.php‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,12 @@
77
* SPDX-License-Identifier: AGPL-3.0-only
88
*/
99
use OC\Encryption\HookManager;
10+
use OC\Share20\GroupDeletedListener;
1011
use OC\Share20\Hooks;
12+
use OC\Share20\UserDeletedListener;
13+
use OC\Share20\UserRemovedListener;
1114
use OCP\EventDispatcher\IEventDispatcher;
15+
use OCP\Group\Events\GroupDeletedEvent;
1216
use OCP\Group\Events\UserRemovedEvent;
1317
use OCP\ILogger;
1418
use OCP\IRequest;
@@ -18,6 +22,7 @@
1822
use OCP\Server;
1923
use OCP\Share;
2024
use OCP\User\Events\UserChangedEvent;
25+
use OCP\User\Events\UserDeletedEvent;
2126
use Psr\Log\LoggerInterface;
2227
use Symfony\Component\Routing\Exception\MethodNotAllowedException;
2328
use function OCP\Log\logger;
@@ -911,12 +916,11 @@ private static function registerRenderReferenceEventListener() {
911916
*/
912917
public static function registerShareHooks(\OC\SystemConfig $systemConfig): void {
913918
if ($systemConfig->getValue('installed')) {
914-
OC_Hook::connect('OC_User', 'post_deleteUser', Hooks::class, 'post_deleteUser');
915-
OC_Hook::connect('OC_User', 'post_deleteGroup', Hooks::class, 'post_deleteGroup');
916919

917-
/** @var IEventDispatcher $dispatcher */
918920
$dispatcher = Server::get(IEventDispatcher::class);
919-
$dispatcher->addServiceListener(UserRemovedEvent::class, \OC\Share20\UserRemovedListener::class);
921+
$dispatcher->addServiceListener(UserRemovedEvent::class, UserRemovedListener::class);
922+
$dispatcher->addServiceListener(GroupDeletedEvent::class, GroupDeletedListener::class);
923+
$dispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedListener::class);
920924
}
921925
}
922926

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1918,7 +1918,7 @@
19181918
'OC\\Share20\\Exception\\BackendError' => $baseDir . '/lib/private/Share20/Exception/BackendError.php',
19191919
'OC\\Share20\\Exception\\InvalidShare' => $baseDir . '/lib/private/Share20/Exception/InvalidShare.php',
19201920
'OC\\Share20\\Exception\\ProviderException' => $baseDir . '/lib/private/Share20/Exception/ProviderException.php',
1921-
'OC\\Share20\\Hooks' => $baseDir . '/lib/private/Share20/Hooks.php',
1921+
'OC\\Share20\\GroupDeletedListener' => $baseDir . '/lib/private/Share20/GroupDeletedListener.php',
19221922
'OC\\Share20\\LegacyHooks' => $baseDir . '/lib/private/Share20/LegacyHooks.php',
19231923
'OC\\Share20\\Manager' => $baseDir . '/lib/private/Share20/Manager.php',
19241924
'OC\\Share20\\ProviderFactory' => $baseDir . '/lib/private/Share20/ProviderFactory.php',
@@ -1927,6 +1927,7 @@
19271927
'OC\\Share20\\ShareAttributes' => $baseDir . '/lib/private/Share20/ShareAttributes.php',
19281928
'OC\\Share20\\ShareDisableChecker' => $baseDir . '/lib/private/Share20/ShareDisableChecker.php',
19291929
'OC\\Share20\\ShareHelper' => $baseDir . '/lib/private/Share20/ShareHelper.php',
1930+
'OC\\Share20\\UserDeletedListener' => $baseDir . '/lib/private/Share20/UserDeletedListener.php',
19301931
'OC\\Share20\\UserRemovedListener' => $baseDir . '/lib/private/Share20/UserRemovedListener.php',
19311932
'OC\\Share\\Constants' => $baseDir . '/lib/private/Share/Constants.php',
19321933
'OC\\Share\\Helper' => $baseDir . '/lib/private/Share/Helper.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1951,7 +1951,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
19511951
'OC\\Share20\\Exception\\BackendError' => __DIR__ . '/../../..' . '/lib/private/Share20/Exception/BackendError.php',
19521952
'OC\\Share20\\Exception\\InvalidShare' => __DIR__ . '/../../..' . '/lib/private/Share20/Exception/InvalidShare.php',
19531953
'OC\\Share20\\Exception\\ProviderException' => __DIR__ . '/../../..' . '/lib/private/Share20/Exception/ProviderException.php',
1954-
'OC\\Share20\\Hooks' => __DIR__ . '/../../..' . '/lib/private/Share20/Hooks.php',
1954+
'OC\\Share20\\GroupDeletedListener' => __DIR__ . '/../../..' . '/lib/private/Share20/GroupDeletedListener.php',
19551955
'OC\\Share20\\LegacyHooks' => __DIR__ . '/../../..' . '/lib/private/Share20/LegacyHooks.php',
19561956
'OC\\Share20\\Manager' => __DIR__ . '/../../..' . '/lib/private/Share20/Manager.php',
19571957
'OC\\Share20\\ProviderFactory' => __DIR__ . '/../../..' . '/lib/private/Share20/ProviderFactory.php',
@@ -1960,6 +1960,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
19601960
'OC\\Share20\\ShareAttributes' => __DIR__ . '/../../..' . '/lib/private/Share20/ShareAttributes.php',
19611961
'OC\\Share20\\ShareDisableChecker' => __DIR__ . '/../../..' . '/lib/private/Share20/ShareDisableChecker.php',
19621962
'OC\\Share20\\ShareHelper' => __DIR__ . '/../../..' . '/lib/private/Share20/ShareHelper.php',
1963+
'OC\\Share20\\UserDeletedListener' => __DIR__ . '/../../..' . '/lib/private/Share20/UserDeletedListener.php',
19631964
'OC\\Share20\\UserRemovedListener' => __DIR__ . '/../../..' . '/lib/private/Share20/UserRemovedListener.php',
19641965
'OC\\Share\\Constants' => __DIR__ . '/../../..' . '/lib/private/Share/Constants.php',
19651966
'OC\\Share\\Helper' => __DIR__ . '/../../..' . '/lib/private/Share/Helper.php',

‎lib/private/Share20/GroupDeletedListener.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OC\Share20;
10+
11+
use OCP\EventDispatcher\Event;
12+
use OCP\EventDispatcher\IEventListener;
13+
use OCP\Group\Events\GroupDeletedEvent;
14+
use OCP\Share\IManager;
15+
16+
/**
17+
* @template-implements IEventListener<GroupDeletedEvent>
18+
*/
19+
class GroupDeletedListener implements IEventListener {
20+
public function __construct(
21+
protected IManager $shareManager,
22+
) {
23+
}
24+
25+
public function handle(Event $event): void {
26+
if (!$event instanceof GroupDeletedEvent) {
27+
return;
28+
}
29+
30+
$this->shareManager->groupDeleted($event->getGroup()->getGID());
31+
}
32+
}

‎lib/private/Share20/Hooks.php‎

Lines changed: 0 additions & 20 deletions
This file was deleted.

‎lib/private/Share20/Manager.php‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1536,8 +1536,14 @@ public function userDeleted($uid) {
15361536
* @inheritdoc
15371537
*/
15381538
public function groupDeleted($gid) {
1539-
$provider = $this->factory->getProviderForType(IShare::TYPE_GROUP);
1540-
$provider->groupDeleted($gid);
1539+
foreach ([IShare::TYPE_GROUP, IShare::TYPE_REMOTE_GROUP] as $type) {
1540+
try {
1541+
$provider = $this->factory->getProviderForType($type);
1542+
} catch (ProviderException $e) {
1543+
continue;
1544+
}
1545+
$provider->groupDeleted($gid);
1546+
}
15411547

15421548
$excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', '');
15431549
if ($excludedGroups === '') {
@@ -1557,8 +1563,14 @@ public function groupDeleted($gid) {
15571563
* @inheritdoc
15581564
*/
15591565
public function userDeletedFromGroup($uid, $gid) {
1560-
$provider = $this->factory->getProviderForType(IShare::TYPE_GROUP);
1561-
$provider->userDeletedFromGroup($uid, $gid);
1566+
foreach ([IShare::TYPE_GROUP, IShare::TYPE_REMOTE_GROUP] as $type) {
1567+
try {
1568+
$provider = $this->factory->getProviderForType($type);
1569+
} catch (ProviderException $e) {
1570+
continue;
1571+
}
1572+
$provider->userDeletedFromGroup($uid, $gid);
1573+
}
15621574
}
15631575

15641576
/**

‎lib/private/Share20/UserDeletedListener.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OC\Share20;
10+
11+
use OCP\EventDispatcher\Event;
12+
use OCP\EventDispatcher\IEventListener;
13+
use OCP\Share\IManager;
14+
use OCP\User\Events\UserDeletedEvent;
15+
16+
/**
17+
* @template-implements IEventListener<UserDeletedEvent>
18+
*/
19+
class UserDeletedListener implements IEventListener {
20+
public function __construct(
21+
protected IManager $shareManager,
22+
) {
23+
}
24+
25+
public function handle(Event $event): void {
26+
if (!$event instanceof UserDeletedEvent) {
27+
return;
28+
}
29+
30+
$this->shareManager->userDeleted($event->getUser()->getUID());
31+
}
32+
}

0 commit comments

Comments
 (0)