Skip to content

Commit ae7f9cb

Browse files
come-nccome-nc
authored
Merge pull request #43025 from nextcloud/bugfix/error-on-reshare-after-transfer-ownership
fix: update re-share if shared-by user has been revoked
2 parents d948fcb + 2ca5191 commit ae7f9cb

File tree

6 files changed

+208
-0
lines changed

6 files changed

+208
-0
lines changed

‎apps/files_sharing/appinfo/info.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ Turning the feature off removes shared files and folders on the server for all s
4949
<command>OCA\Files_Sharing\Command\CleanupRemoteStorages</command>
5050
<command>OCA\Files_Sharing\Command\ExiprationNotification</command>
5151
<command>OCA\Files_Sharing\Command\DeleteOrphanShares</command>
52+
<command>OCA\Files_Sharing\Command\FixShareOwners</command>
5253
</commands>
5354

5455
<settings>

‎apps/files_sharing/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
'OCA\\Files_Sharing\\Command\\CleanupRemoteStorages' => $baseDir . '/../lib/Command/CleanupRemoteStorages.php',
2828
'OCA\\Files_Sharing\\Command\\DeleteOrphanShares' => $baseDir . '/../lib/Command/DeleteOrphanShares.php',
2929
'OCA\\Files_Sharing\\Command\\ExiprationNotification' => $baseDir . '/../lib/Command/ExiprationNotification.php',
30+
'OCA\\Files_Sharing\\Command\\FixShareOwners' => $baseDir . '/../lib/Command/FixShareOwners.php',
3031
'OCA\\Files_Sharing\\Controller\\AcceptController' => $baseDir . '/../lib/Controller/AcceptController.php',
3132
'OCA\\Files_Sharing\\Controller\\DeletedShareAPIController' => $baseDir . '/../lib/Controller/DeletedShareAPIController.php',
3233
'OCA\\Files_Sharing\\Controller\\ExternalSharesController' => $baseDir . '/../lib/Controller/ExternalSharesController.php',

‎apps/files_sharing/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ class ComposerStaticInitFiles_Sharing
4242
'OCA\\Files_Sharing\\Command\\CleanupRemoteStorages' => __DIR__ . '/..' . '/../lib/Command/CleanupRemoteStorages.php',
4343
'OCA\\Files_Sharing\\Command\\DeleteOrphanShares' => __DIR__ . '/..' . '/../lib/Command/DeleteOrphanShares.php',
4444
'OCA\\Files_Sharing\\Command\\ExiprationNotification' => __DIR__ . '/..' . '/../lib/Command/ExiprationNotification.php',
45+
'OCA\\Files_Sharing\\Command\\FixShareOwners' => __DIR__ . '/..' . '/../lib/Command/FixShareOwners.php',
4546
'OCA\\Files_Sharing\\Controller\\AcceptController' => __DIR__ . '/..' . '/../lib/Controller/AcceptController.php',
4647
'OCA\\Files_Sharing\\Controller\\DeletedShareAPIController' => __DIR__ . '/..' . '/../lib/Controller/DeletedShareAPIController.php',
4748
'OCA\\Files_Sharing\\Controller\\ExternalSharesController' => __DIR__ . '/..' . '/../lib/Controller/ExternalSharesController.php',

‎apps/files_sharing/lib/Command/FixShareOwners.php‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\Files_Sharing\Command;
10+
11+
use OC\Core\Command\Base;
12+
use OCA\Files_Sharing\OrphanHelper;
13+
use Symfony\Component\Console\Input\InputInterface;
14+
use Symfony\Component\Console\Input\InputOption;
15+
use Symfony\Component\Console\Output\OutputInterface;
16+
17+
class FixShareOwners extends Base {
18+
public function __construct(
19+
private readonly OrphanHelper $orphanHelper,
20+
) {
21+
parent::__construct();
22+
}
23+
24+
protected function configure(): void {
25+
$this
26+
->setName('sharing:fix-share-owners')
27+
->setDescription('Fix owner of broken shares after transfer ownership on old versions')
28+
->addOption(
29+
'dry-run',
30+
null,
31+
InputOption::VALUE_NONE,
32+
'only show which shares would be updated'
33+
);
34+
}
35+
36+
public function execute(InputInterface $input, OutputInterface $output): int {
37+
$shares = $this->orphanHelper->getAllShares();
38+
$dryRun = $input->getOption('dry-run');
39+
$count = 0;
40+
41+
foreach ($shares as $share) {
42+
if ($this->orphanHelper->isShareValid($share['owner'], $share['fileid']) || !$this->orphanHelper->fileExists($share['fileid'])) {
43+
continue;
44+
}
45+
46+
$owner = $this->orphanHelper->findOwner($share['fileid']);
47+
48+
if ($owner !== null) {
49+
if ($dryRun) {
50+
$output->writeln("Share with id <info>{$share['id']}</info> (target: <info>{$share['target']}</info>) can be updated to owner <info>$owner</info>");
51+
} else {
52+
$this->orphanHelper->updateShareOwner($share['id'], $owner);
53+
$output->writeln("Share with id <info>{$share['id']}</info> (target: <info>{$share['target']}</info>) updated to owner <info>$owner</info>");
54+
}
55+
$count++;
56+
}
57+
}
58+
59+
if ($count === 0) {
60+
$output->writeln('No broken shares detected');
61+
}
62+
63+
return static::SUCCESS;
64+
}
65+
}

‎apps/files_sharing/lib/OrphanHelper.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,15 @@
1010

1111
use OC\User\NoUserException;
1212
use OCP\DB\QueryBuilder\IQueryBuilder;
13+
use OCP\Files\Config\IUserMountCache;
1314
use OCP\Files\IRootFolder;
1415
use OCP\IDBConnection;
1516

1617
class OrphanHelper {
1718
public function __construct(
1819
private IDBConnection $connection,
1920
private IRootFolder $rootFolder,
21+
private IUserMountCache $userMountCache,
2022
) {
2123
}
2224

@@ -68,4 +70,26 @@ public function getAllShares() {
6870
];
6971
}
7072
}
73+
74+
public function findOwner(int $fileId): ?string {
75+
$mounts = $this->userMountCache->getMountsForFileId($fileId);
76+
if (!$mounts) {
77+
return null;
78+
}
79+
foreach ($mounts as $mount) {
80+
$userHomeMountPoint = '/' . $mount->getUser()->getUID() . '/';
81+
if ($mount->getMountPoint() === $userHomeMountPoint) {
82+
return $mount->getUser()->getUID();
83+
}
84+
}
85+
return null;
86+
}
87+
88+
public function updateShareOwner(int $shareId, string $owner): void {
89+
$query = $this->connection->getQueryBuilder();
90+
$query->update('share')
91+
->set('uid_owner', $query->createNamedParameter($owner))
92+
->where($query->expr()->eq('id', $query->createNamedParameter($shareId, IQueryBuilder::PARAM_INT)));
93+
$query->executeStatement();
94+
}
7195
}

‎apps/files_sharing/tests/Command/FixShareOwnersTest.php‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
<?php
2+
/**
3+
* SPDX-FileCopyrightText: 2017-2024 Nextcloud GmbH and Nextcloud contributors
4+
* SPDX-License-Identifier: AGPL-3.0-only
5+
*/
6+
namespace OCA\Files_Sharing\Tests\Command;
7+
8+
use OCA\Files_Sharing\Command\FixShareOwners;
9+
use OCA\Files_Sharing\OrphanHelper;
10+
use Symfony\Component\Console\Input\InputInterface;
11+
use Symfony\Component\Console\Output\OutputInterface;
12+
use Test\TestCase;
13+
14+
/**
15+
* Class FixShareOwnersTest
16+
*
17+
* @package OCA\Files_Sharing\Tests\Command
18+
*/
19+
class FixShareOwnersTest extends TestCase {
20+
/**
21+
* @var FixShareOwners
22+
*/
23+
private $command;
24+
25+
/**
26+
* @var OrphanHelper|\PHPUnit\Framework\MockObject\MockObject
27+
*/
28+
private $orphanHelper;
29+
30+
protected function setUp(): void {
31+
parent::setUp();
32+
33+
$this->orphanHelper = $this->createMock(OrphanHelper::class);
34+
$this->command = new FixShareOwners($this->orphanHelper);
35+
}
36+
37+
public function testExecuteNoSharesDetected() {
38+
$this->orphanHelper->expects($this->once())
39+
->method('getAllShares')
40+
->willReturn([
41+
['id' => 1, 'owner' => 'user1', 'fileid' => 1, 'target' => 'target1'],
42+
['id' => 2, 'owner' => 'user2', 'fileid' => 2, 'target' => 'target2'],
43+
]);
44+
$this->orphanHelper->expects($this->exactly(2))
45+
->method('isShareValid')
46+
->willReturn(true);
47+
48+
$input = $this->createMock(InputInterface::class);
49+
$output = $this->createMock(OutputInterface::class);
50+
51+
$output->expects($this->once())
52+
->method('writeln')
53+
->with('No broken shares detected');
54+
$this->command->execute($input, $output);
55+
}
56+
57+
public function testExecuteSharesDetected() {
58+
$this->orphanHelper->expects($this->once())
59+
->method('getAllShares')
60+
->willReturn([
61+
['id' => 1, 'owner' => 'user1', 'fileid' => 1, 'target' => 'target1'],
62+
['id' => 2, 'owner' => 'user2', 'fileid' => 2, 'target' => 'target2'],
63+
]);
64+
$this->orphanHelper->expects($this->exactly(2))
65+
->method('isShareValid')
66+
->willReturnOnConsecutiveCalls(true, false);
67+
$this->orphanHelper->expects($this->once())
68+
->method('fileExists')
69+
->willReturn(true);
70+
$this->orphanHelper->expects($this->once())
71+
->method('findOwner')
72+
->willReturn('newOwner');
73+
$this->orphanHelper->expects($this->once())
74+
->method('updateShareOwner');
75+
76+
$input = $this->createMock(InputInterface::class);
77+
$output = $this->createMock(OutputInterface::class);
78+
79+
$output->expects($this->once())
80+
->method('writeln')
81+
->with('Share with id <info>2</info> (target: <info>target2</info>) updated to owner <info>newOwner</info>');
82+
$this->command->execute($input, $output);
83+
}
84+
85+
public function testExecuteSharesDetectedDryRun() {
86+
$this->orphanHelper->expects($this->once())
87+
->method('getAllShares')
88+
->willReturn([
89+
['id' => 1, 'owner' => 'user1', 'fileid' => 1, 'target' => 'target1'],
90+
['id' => 2, 'owner' => 'user2', 'fileid' => 2, 'target' => 'target2'],
91+
]);
92+
$this->orphanHelper->expects($this->exactly(2))
93+
->method('isShareValid')
94+
->willReturnOnConsecutiveCalls(true, false);
95+
$this->orphanHelper->expects($this->once())
96+
->method('fileExists')
97+
->willReturn(true);
98+
$this->orphanHelper->expects($this->once())
99+
->method('findOwner')
100+
->willReturn('newOwner');
101+
$this->orphanHelper->expects($this->never())
102+
->method('updateShareOwner');
103+
104+
$input = $this->createMock(InputInterface::class);
105+
$output = $this->createMock(OutputInterface::class);
106+
107+
$output->expects($this->once())
108+
->method('writeln')
109+
->with('Share with id <info>2</info> (target: <info>target2</info>) can be updated to owner <info>newOwner</info>');
110+
$input->expects($this->once())
111+
->method('getOption')
112+
->with('dry-run')
113+
->willReturn(true);
114+
$this->command->execute($input, $output);
115+
}
116+
}

0 commit comments

Comments
 (0)