Skip to content

fix(files_sharing): Hide own reshares #53304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sorbaugh merged 3 commits into from
Jul 1, 2025
Merged

fix(files_sharing): Hide own reshares #53304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
edf5493
fix(files): Limit transferring incoming shares to the selected path
provokateurin Jun 23, 2025
2e5ccc7
fix(files): Always transfer incoming shares when doing ownership tran…
provokateurin Jun 3, 2025
8e580f8
fix(files_sharing): Hide own reshares
provokateurin Jun 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 1 addition & 23 deletions apps/files/lib/Command/TransferOwnership.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ protected function configure(): void {
'transfer-incoming-shares',
null,
InputOption::VALUE_OPTIONAL,
'transfer incoming user file shares to destination user. Usage: --transfer-incoming-shares=1 (value required)',
'Incoming shares are always transferred now, so this option does not affect the ownership transfer anymore',
'2'
)->addOption(
'include-external-storage',
Expand Down Expand Up @@ -129,35 +129,13 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}

try {
$includeIncomingArgument = $input->getOption('transfer-incoming-shares');

switch ($includeIncomingArgument) {
case '0':
$includeIncoming = false;
break;
case '1':
$includeIncoming = true;
break;
case '2':
$includeIncoming = $this->config->getSystemValue('transferIncomingShares', false);
if (gettype($includeIncoming) !== 'boolean') {
$output->writeln("<error> config.php: 'transfer-incoming-shares': wrong usage. Transfer aborted.</error>");
return self::FAILURE;
}
break;
default:
$output->writeln('<error>Option --transfer-incoming-shares: wrong usage. Transfer aborted.</error>');
return self::FAILURE;
}

$this->transferService->transfer(
$sourceUserObject,
$destinationUserObject,
$path,
$output,
$input->getOption('move') === true,
false,
$includeIncoming,
$includeExternalStorage,
);
} catch (TransferOwnershipException $e) {
Expand Down
77 changes: 41 additions & 36 deletions apps/files/lib/Service/OwnershipTransferService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
namespace OCA\Files\Service;

use Closure;
use Exception;
use OC\Files\Filesystem;
use OC\Files\View;
use OC\User\NoUserException;
Expand All @@ -19,6 +20,7 @@
use OCP\Encryption\IManager as IEncryptionManager;
use OCP\Files\Config\IHomeMountProvider;
use OCP\Files\Config\IUserMountCache;
use OCP\Files\File;
use OCP\Files\FileInfo;
use OCP\Files\InvalidPathException;
use OCP\Files\IRootFolder;
Expand Down Expand Up @@ -70,7 +72,6 @@ public function transfer(
?OutputInterface $output = null,
bool $move = false,
bool $firstLogin = false,
bool $transferIncomingShares = false,
bool $includeExternalStorage = false,
): void {
$output = $output ?? new NullOutput();
Expand Down Expand Up @@ -157,29 +158,26 @@ public function transfer(
$sizeDifference = $sourceSize - $view->getFileInfo($finalTarget)->getSize();

// transfer the incoming shares
if ($transferIncomingShares === true) {
$sourceShares = $this->collectIncomingShares(
$sourceUid,
$output,
$view
);
$destinationShares = $this->collectIncomingShares(
$destinationUid,
$output,
$view,
true
);
$this->transferIncomingShares(
$sourceUid,
$destinationUid,
$sourceShares,
$destinationShares,
$output,
$path,
$finalTarget,
$move
);
}
$sourceShares = $this->collectIncomingShares(
$sourceUid,
$output,
$sourcePath,
);
$destinationShares = $this->collectIncomingShares(
$destinationUid,
$output,
null,
);
$this->transferIncomingShares(
$sourceUid,
$destinationUid,
$sourceShares,
$destinationShares,
$output,
$path,
$finalTarget,
$move
);

$destinationPath = $finalTarget . '/' . $path;
// restore the shares
Expand Down Expand Up @@ -344,7 +342,7 @@ private function collectUsersShares(
return mb_strpos(
Filesystem::normalizePath($relativePath . '/', false),
$normalizedPath . '/') === 0;
} catch (\Exception $e) {
} catch (Exception $e) {
return false;
}
});
Expand Down Expand Up @@ -372,14 +370,16 @@ private function collectUsersShares(
}, $shares)));
}

private function collectIncomingShares(string $sourceUid,
private function collectIncomingShares(
string $sourceUid,
OutputInterface $output,
View $view,
bool $addKeys = false): array {
?string $path,
): array {
$output->writeln("Collecting all incoming share information for files and folders of $sourceUid ...");

$shares = [];
$progress = new ProgressBar($output);
$normalizedPath = Filesystem::normalizePath($path);

$offset = 0;
while (true) {
Expand All @@ -388,14 +388,19 @@ private function collectIncomingShares(string $sourceUid,
if (empty($sharePage)) {
break;
}
if ($addKeys) {
foreach ($sharePage as $singleShare) {
$shares[$singleShare->getNodeId()] = $singleShare;
}
} else {
foreach ($sharePage as $singleShare) {
$shares[] = $singleShare;
}

if ($path !== null && $path !== "$sourceUid/files") {
$sharePage = array_filter($sharePage, static function (IShare $share) use ($sourceUid, $normalizedPath) {
try {
return str_starts_with(Filesystem::normalizePath($sourceUid . '/files' . $share->getTarget() . '/', false), $normalizedPath . '/');
} catch (Exception) {
return false;
}
});
}

foreach ($sharePage as $share) {
$shares[$share->getNodeId()] = $share;
}

$offset += 50;
Expand Down
2 changes: 1 addition & 1 deletion apps/files_sharing/lib/MountProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public function getMountsForUser(IUser $user, IStorageFactory $loader) {

// filter out excluded shares and group shares that includes self
$shares = array_filter($shares, function (IShare $share) use ($user) {
return $share->getPermissions() > 0 && $share->getShareOwner() !== $user->getUID();
return $share->getPermissions() > 0 && $share->getShareOwner() !== $user->getUID() && $share->getSharedBy() !== $user->getUID();
});

$superShares = $this->buildSuperShares($shares, $user);
Expand Down
13 changes: 0 additions & 13 deletions build/integration/features/bootstrap/CommandLineContext.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,19 +108,6 @@ public function transferringOwnershipPath($path, $user1, $user2) {
}
}

/**
* @When /^transferring ownership of path "([^"]+)" from "([^"]+)" to "([^"]+)" with received shares$/
*/
public function transferringOwnershipPathWithIncomingShares($path, $user1, $user2) {
$path = '--path=' . $path;
if ($this->runOcc(['files:transfer-ownership', $path, $user1, $user2, '--transfer-incoming-shares=1']) === 0) {
$this->lastTransferPath = $this->findLastTransferFolderForUser($user1, $user2);
} else {
// failure
$this->lastTransferPath = null;
}
}

/**
* @When /^using received transfer folder of "([^"]+)" as dav path$/
*/
Expand Down
51 changes: 15 additions & 36 deletions build/integration/files_features/transfer-ownership.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,10 @@ Feature: transfer-ownership
And As an "user2"
Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is"
And using old dav path
And as "user0" the folder "/test" exists
And as "user0" the folder "/test" does not exist
And using received transfer folder of "user1" as dav path
And as "user1" the folder "/test" does not exist
And As an "user0"
And as "user1" the folder "/test" exists
And As an "user1"
And Getting info of last share
And the OCS status code should be "100"
And Share fields of last share match with
Expand All @@ -210,13 +210,12 @@ Feature: transfer-ownership
And user "user1" accepts last share
When transferring ownership from "user0" to "user1"
And the command was successful
And As an "user1"
Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is"
And using old dav path
And as "user0" the folder "/test" exists
Then as "user0" the folder "/test" does not exist
When As an "user1"
And using received transfer folder of "user1" as dav path
And as "user1" the folder "/test" does not exist
And As an "user1"
Then as "user1" the folder "/test" exists
And Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is"
And Getting info of last share
And the OCS status code should be "100"
And Share fields of last share match with
Expand All @@ -242,18 +241,18 @@ Feature: transfer-ownership
And As an "user2"
Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is"
And using old dav path
And as "user0" the folder "/test" exists
And as "user0" the folder "/test" does not exist
And using received transfer folder of "user1" as dav path
And as "user1" the folder "/test" does not exist
And As an "user0"
And as "user1" the folder "/test" exists
And As an "user1"
And Getting info of last share
And the OCS status code should be "100"
And Share fields of last share match with
| uid_owner | user0 |
| uid_file_owner | user3 |
| share_with | group1 |

Scenario: transferring ownership does not transfer received shares
Scenario: transferring ownership transfers received shares
Given user "user0" exists
And user "user1" exists
And user "user2" exists
Expand All @@ -264,16 +263,16 @@ Feature: transfer-ownership
And the command was successful
And As an "user1"
And using received transfer folder of "user1" as dav path
Then as "user1" the folder "/test" does not exist
Then as "user1" the folder "/test" exists
And using old dav path
And as "user0" the folder "/test" exists
And as "user0" the folder "/test" does not exist
And As an "user2"
And Getting info of last share
And the OCS status code should be "100"
And Share fields of last share match with
| uid_owner | user2 |
| uid_file_owner | user2 |
| share_with | user0 |
| share_with | user1 |

@local_storage
Scenario: transferring ownership does not transfer external storage
Expand Down Expand Up @@ -516,26 +515,6 @@ Feature: transfer-ownership
Then the command failed with exit code 1
And the command error output contains the text "Moving a storage (user0/files/test) into another storage (user1) is not allowed"

Scenario: transferring ownership does not transfer received shares
Given user "user0" exists
And user "user1" exists
And user "user2" exists
And User "user2" created a folder "/test"
And User "user0" created a folder "/sub"
And folder "/test" of user "user2" is shared with user "user0" with permissions 31
And user "user0" accepts last share
And User "user0" moved folder "/test" to "/sub/test"
When transferring ownership of path "sub" from "user0" to "user1"
And the command was successful
And As an "user1"
And using received transfer folder of "user1" as dav path
Then as "user1" the folder "/sub" exists
And as "user1" the folder "/sub/test" does not exist
And using old dav path
And as "user0" the folder "/sub" does not exist
And Getting info of last share
And the OCS status code should be "404"

Scenario: transferring ownership transfers received shares into subdir when requested
Given user "user0" exists
And user "user1" exists
Expand All @@ -548,7 +527,7 @@ Feature: transfer-ownership
And User "user0" moved folder "/transfer-share" to "/sub/transfer-share"
And folder "/do-not-transfer" of user "user2" is shared with user "user0" with permissions 31
And user "user0" accepts last share
When transferring ownership of path "sub" from "user0" to "user1" with received shares
When transferring ownership of path "sub" from "user0" to "user1"
And the command was successful
And As an "user1"
And using received transfer folder of "user1" as dav path
Expand Down
Loading