Skip to content

Commit

Permalink
Merge pull request #51204 from nextcloud/backport/51000/stable29
Browse files Browse the repository at this point in the history
[stable29] fix(FederatedShareProvider): Delete external shares when groups are deleted or users removed from a group
  • Loading branch information
sorbaugh authored Mar 4, 2025
2 parents 7a4db4b + dcf7162 commit 04ae493
Show file tree
Hide file tree
Showing 9 changed files with 160 additions and 59 deletions.
60 changes: 45 additions & 15 deletions apps/federatedfilesharing/lib/FederatedShareProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -911,30 +911,60 @@ public function userDeleted($uid, $shareType) {
//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();
}

/**
Expand Down
23 changes: 23 additions & 0 deletions build/integration/sharing_features/sharing-v1-part2.feature
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,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
Expand Down
12 changes: 8 additions & 4 deletions lib/base.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,12 @@
*/

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;
Expand All @@ -79,6 +83,7 @@
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;
Expand Down Expand Up @@ -968,12 +973,11 @@ private static function registerRenderReferenceEventListener() {
*/
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);
}
}

Expand Down
3 changes: 2 additions & 1 deletion lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1806,7 +1806,7 @@
'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',
Expand All @@ -1815,6 +1815,7 @@
'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',
Expand Down
3 changes: 2 additions & 1 deletion lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1839,7 +1839,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',
Expand All @@ -1848,6 +1848,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',
Expand Down
32 changes: 32 additions & 0 deletions lib/private/Share20/GroupDeletedListener.php
Original file line number Diff line number Diff line change
@@ -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());
}
}
34 changes: 0 additions & 34 deletions lib/private/Share20/Hooks.php

This file was deleted.

20 changes: 16 additions & 4 deletions lib/private/Share20/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -1665,8 +1665,14 @@ public function userDeleted($uid) {
* @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 === '') {
Expand All @@ -1686,8 +1692,14 @@ public function groupDeleted($gid) {
* @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);
}
}

/**
Expand Down
32 changes: 32 additions & 0 deletions lib/private/Share20/UserDeletedListener.php
Original file line number Diff line number Diff line change
@@ -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());
}
}

0 comments on commit 04ae493

Please sign in to comment.