Skip to content

Commit 3bc3d03

Browse files
sorbaughsorbaugh
authored
Merge pull request #51000 from nextcloud/fix/federatedfilesharing/group-cleanup
fix(FederatedShareProvider): Delete external shares when groups are deleted or users removed from a group
2 parents a9a839c + f5b5aec commit 3bc3d03

File tree

9 files changed

+159
-44
lines changed

9 files changed

+159
-44
lines changed

‎apps/federatedfilesharing/lib/FederatedShareProvider.php‎

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -882,30 +882,60 @@ public function userDeleted($uid, $shareType) {
882882
//TODO: probably a good idea to send unshare info to remote servers
883883

884884
$qb = $this->dbConnection->getQueryBuilder();
885-
886885
$qb->delete('share')
887886
->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_REMOTE)))
888887
->andWhere($qb->expr()->eq('uid_owner', $qb->createNamedParameter($uid)))
889888
->executeStatement();
889+
890+
$qb = $this->dbConnection->getQueryBuilder();
891+
$qb->delete('share_external')
892+
->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_GROUP)))
893+
->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($uid)))
894+
->executeStatement();
890895
}
891896

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

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

911941
/**

‎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 OCP\Util;
2227
use Psr\Log\LoggerInterface;
2328
use Symfony\Component\Routing\Exception\MethodNotAllowedException;
@@ -945,12 +950,11 @@ private static function registerRenderReferenceEventListener() {
945950
*/
946951
public static function registerShareHooks(\OC\SystemConfig $systemConfig): void {
947952
if ($systemConfig->getValue('installed')) {
948-
OC_Hook::connect('OC_User', 'post_deleteUser', Hooks::class, 'post_deleteUser');
949-
OC_Hook::connect('OC_User', 'post_deleteGroup', Hooks::class, 'post_deleteGroup');
950953

951-
/** @var IEventDispatcher $dispatcher */
952954
$dispatcher = Server::get(IEventDispatcher::class);
953-
$dispatcher->addServiceListener(UserRemovedEvent::class, \OC\Share20\UserRemovedListener::class);
955+
$dispatcher->addServiceListener(UserRemovedEvent::class, UserRemovedListener::class);
956+
$dispatcher->addServiceListener(GroupDeletedEvent::class, GroupDeletedListener::class);
957+
$dispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedListener::class);
954958
}
955959
}
956960

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1992,7 +1992,7 @@
19921992
'OC\\Share20\\Exception\\BackendError' => $baseDir . '/lib/private/Share20/Exception/BackendError.php',
19931993
'OC\\Share20\\Exception\\InvalidShare' => $baseDir . '/lib/private/Share20/Exception/InvalidShare.php',
19941994
'OC\\Share20\\Exception\\ProviderException' => $baseDir . '/lib/private/Share20/Exception/ProviderException.php',
1995-
'OC\\Share20\\Hooks' => $baseDir . '/lib/private/Share20/Hooks.php',
1995+
'OC\\Share20\\GroupDeletedListener' => $baseDir . '/lib/private/Share20/GroupDeletedListener.php',
19961996
'OC\\Share20\\LegacyHooks' => $baseDir . '/lib/private/Share20/LegacyHooks.php',
19971997
'OC\\Share20\\Manager' => $baseDir . '/lib/private/Share20/Manager.php',
19981998
'OC\\Share20\\ProviderFactory' => $baseDir . '/lib/private/Share20/ProviderFactory.php',
@@ -2001,6 +2001,7 @@
20012001
'OC\\Share20\\ShareAttributes' => $baseDir . '/lib/private/Share20/ShareAttributes.php',
20022002
'OC\\Share20\\ShareDisableChecker' => $baseDir . '/lib/private/Share20/ShareDisableChecker.php',
20032003
'OC\\Share20\\ShareHelper' => $baseDir . '/lib/private/Share20/ShareHelper.php',
2004+
'OC\\Share20\\UserDeletedListener' => $baseDir . '/lib/private/Share20/UserDeletedListener.php',
20042005
'OC\\Share20\\UserRemovedListener' => $baseDir . '/lib/private/Share20/UserRemovedListener.php',
20052006
'OC\\Share\\Constants' => $baseDir . '/lib/private/Share/Constants.php',
20062007
'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
@@ -2041,7 +2041,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
20412041
'OC\\Share20\\Exception\\BackendError' => __DIR__ . '/../../..' . '/lib/private/Share20/Exception/BackendError.php',
20422042
'OC\\Share20\\Exception\\InvalidShare' => __DIR__ . '/../../..' . '/lib/private/Share20/Exception/InvalidShare.php',
20432043
'OC\\Share20\\Exception\\ProviderException' => __DIR__ . '/../../..' . '/lib/private/Share20/Exception/ProviderException.php',
2044-
'OC\\Share20\\Hooks' => __DIR__ . '/../../..' . '/lib/private/Share20/Hooks.php',
2044+
'OC\\Share20\\GroupDeletedListener' => __DIR__ . '/../../..' . '/lib/private/Share20/GroupDeletedListener.php',
20452045
'OC\\Share20\\LegacyHooks' => __DIR__ . '/../../..' . '/lib/private/Share20/LegacyHooks.php',
20462046
'OC\\Share20\\Manager' => __DIR__ . '/../../..' . '/lib/private/Share20/Manager.php',
20472047
'OC\\Share20\\ProviderFactory' => __DIR__ . '/../../..' . '/lib/private/Share20/ProviderFactory.php',
@@ -2050,6 +2050,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
20502050
'OC\\Share20\\ShareAttributes' => __DIR__ . '/../../..' . '/lib/private/Share20/ShareAttributes.php',
20512051
'OC\\Share20\\ShareDisableChecker' => __DIR__ . '/../../..' . '/lib/private/Share20/ShareDisableChecker.php',
20522052
'OC\\Share20\\ShareHelper' => __DIR__ . '/../../..' . '/lib/private/Share20/ShareHelper.php',
2053+
'OC\\Share20\\UserDeletedListener' => __DIR__ . '/../../..' . '/lib/private/Share20/UserDeletedListener.php',
20532054
'OC\\Share20\\UserRemovedListener' => __DIR__ . '/../../..' . '/lib/private/Share20/UserRemovedListener.php',
20542055
'OC\\Share\\Constants' => __DIR__ . '/../../..' . '/lib/private/Share/Constants.php',
20552056
'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
@@ -1540,8 +1540,14 @@ public function userDeleted($uid) {
15401540
* @inheritdoc
15411541
*/
15421542
public function groupDeleted($gid) {
1543-
$provider = $this->factory->getProviderForType(IShare::TYPE_GROUP);
1544-
$provider->groupDeleted($gid);
1543+
foreach ([IShare::TYPE_GROUP, IShare::TYPE_REMOTE_GROUP] as $type) {
1544+
try {
1545+
$provider = $this->factory->getProviderForType($type);
1546+
} catch (ProviderException $e) {
1547+
continue;
1548+
}
1549+
$provider->groupDeleted($gid);
1550+
}
15451551

15461552
$excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', '');
15471553
if ($excludedGroups === '') {
@@ -1561,8 +1567,14 @@ public function groupDeleted($gid) {
15611567
* @inheritdoc
15621568
*/
15631569
public function userDeletedFromGroup($uid, $gid) {
1564-
$provider = $this->factory->getProviderForType(IShare::TYPE_GROUP);
1565-
$provider->userDeletedFromGroup($uid, $gid);
1570+
foreach ([IShare::TYPE_GROUP, IShare::TYPE_REMOTE_GROUP] as $type) {
1571+
try {
1572+
$provider = $this->factory->getProviderForType($type);
1573+
} catch (ProviderException $e) {
1574+
continue;
1575+
}
1576+
$provider->userDeletedFromGroup($uid, $gid);
1577+
}
15661578
}
15671579

15681580
/**

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