Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
fix(files_sharing,files): Do not validate shares before adjusting the…
… owner

Signed-off-by: provokateurin <[email protected]>
  • Loading branch information
provokateurin committed Nov 25, 2024
commit 715e7143f042f537ecbc2fcfd936e1996dec1ade
4 changes: 2 additions & 2 deletions apps/files/lib/Service/OwnershipTransferService.php
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ private function collectUsersShares(
foreach ($supportedShareTypes as $shareType) {
$offset = 0;
while (true) {
$sharePage = $this->shareManager->getSharesBy($sourceUid, $shareType, null, true, 50, $offset);
$sharePage = $this->shareManager->getSharesBy($sourceUid, $shareType, null, true, 50, $offset, onlyValid: false);
$progress->advance(count($sharePage));
if (empty($sharePage)) {
break;
Expand Down Expand Up @@ -464,7 +464,7 @@ private function restoreShares(
}
$share->setNodeId($newNodeId);

$this->shareManager->updateShare($share);
$this->shareManager->updateShare($share, onlyValid: false);
}
}
} catch (NotFoundException $e) {
Expand Down
18 changes: 11 additions & 7 deletions apps/files_sharing/lib/Updater.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,24 @@ private static function moveShareInOrOutOfShare($path): void {

$shareManager = \OC::$server->getShareManager();

// We intentionally include invalid shares, as they have been automatically invalidated due to the node no longer
// being accessible for the user. Only in this case where we adjust the share after it was moved we want to ignore
// this to be able to still adjust it.

// FIXME: should CIRCLES be included here ??
$shares = $shareManager->getSharesBy($user->getUID(), IShare::TYPE_USER, $src, false, -1);
$shares = array_merge($shares, $shareManager->getSharesBy($user->getUID(), IShare::TYPE_GROUP, $src, false, -1));
$shares = array_merge($shares, $shareManager->getSharesBy($user->getUID(), IShare::TYPE_ROOM, $src, false, -1));
$shares = $shareManager->getSharesBy($user->getUID(), IShare::TYPE_USER, $src, false, -1, onlyValid: false);
$shares = array_merge($shares, $shareManager->getSharesBy($user->getUID(), IShare::TYPE_GROUP, $src, false, -1, onlyValid: false));
$shares = array_merge($shares, $shareManager->getSharesBy($user->getUID(), IShare::TYPE_ROOM, $src, false, -1, onlyValid: false));

if ($src instanceof Folder) {
$cacheAccess = Server::get(FileAccess::class);

$sourceStorageId = $src->getStorage()->getCache()->getNumericStorageId();
$sourceInternalPath = $src->getInternalPath();
$subShares = array_merge(
$shareManager->getSharesBy($user->getUID(), IShare::TYPE_USER),
$shareManager->getSharesBy($user->getUID(), IShare::TYPE_GROUP),
$shareManager->getSharesBy($user->getUID(), IShare::TYPE_ROOM),
$shareManager->getSharesBy($user->getUID(), IShare::TYPE_USER, onlyValid: false),
$shareManager->getSharesBy($user->getUID(), IShare::TYPE_GROUP, onlyValid: false),
$shareManager->getSharesBy($user->getUID(), IShare::TYPE_ROOM, onlyValid: false),
);
$shareSourceIds = array_map(fn (IShare $share) => $share->getNodeId(), $subShares);
$shareSources = $cacheAccess->getByFileIdsInStorage($shareSourceIds, $sourceStorageId);
Expand Down Expand Up @@ -111,7 +115,7 @@ private static function moveShareInOrOutOfShare($path): void {

$share->setShareOwner($newOwner);
$share->setPermissions($newPermissions);
$shareManager->updateShare($share);
$shareManager->updateShare($share, onlyValid: false);
}
}

Expand Down
24 changes: 14 additions & 10 deletions lib/private/Share20/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -786,13 +786,13 @@ public function createShare(IShare $share) {
* @throws \InvalidArgumentException
* @throws GenericShareException
*/
public function updateShare(IShare $share) {
public function updateShare(IShare $share, bool $onlyValid = true) {
$expirationDateUpdated = false;

$this->canShare($share);

try {
$originalShare = $this->getShareById($share->getFullId());
$originalShare = $this->getShareById($share->getFullId(), onlyValid: $onlyValid);
} catch (\UnexpectedValueException $e) {
throw new \InvalidArgumentException($this->l->t('Share does not have a full ID'));
}
Expand Down Expand Up @@ -1241,7 +1241,7 @@ public function getSharesInFolder($userId, Folder $node, $reshares = false, $sha
/**
* @inheritdoc
*/
public function getSharesBy($userId, $shareType, $path = null, $reshares = false, $limit = 50, $offset = 0) {
public function getSharesBy($userId, $shareType, $path = null, $reshares = false, $limit = 50, $offset = 0, bool $onlyValid = true) {
if ($path !== null &&
!($path instanceof \OCP\Files\File) &&
!($path instanceof \OCP\Files\Folder)) {
Expand Down Expand Up @@ -1270,11 +1270,13 @@ public function getSharesBy($userId, $shareType, $path = null, $reshares = false
while (true) {
$added = 0;
foreach ($shares as $share) {
try {
$this->checkShare($share);
} catch (ShareNotFound $e) {
// Ignore since this basically means the share is deleted
continue;
if ($onlyValid) {
try {
$this->checkShare($share);
} catch (ShareNotFound $e) {
// Ignore since this basically means the share is deleted
continue;
}
}

$added++;
Expand Down Expand Up @@ -1366,7 +1368,7 @@ public function getDeletedSharedWith($userId, $shareType, $node = null, $limit =
/**
* @inheritdoc
*/
public function getShareById($id, $recipient = null) {
public function getShareById($id, $recipient = null, bool $onlyValid = true) {
if ($id === null) {
throw new ShareNotFound();
}
Expand All @@ -1381,7 +1383,9 @@ public function getShareById($id, $recipient = null) {

$share = $provider->getShareById($id, $recipient);

$this->checkShare($share);
if ($onlyValid) {
$this->checkShare($share);
}

return $share;
}
Expand Down
9 changes: 6 additions & 3 deletions lib/public/Share/IManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ public function createShare(IShare $share);
* The state can't be changed this way: use acceptShare
*
* @param IShare $share
* @param bool $onlyValid Only updates valid shares, invalid shares will be deleted automatically and are not updated
* @return IShare The share object
* @throws \InvalidArgumentException
* @since 9.0.0
*/
public function updateShare(IShare $share);
public function updateShare(IShare $share, bool $onlyValid = true);

/**
* Accept a share.
Expand Down Expand Up @@ -127,10 +128,11 @@ public function getSharesInFolder($userId, Folder $node, $reshares = false, $sha
* @param bool $reshares
* @param int $limit The maximum number of returned results, -1 for all results
* @param int $offset
* @param bool $onlyValid Only returns valid shares, invalid shares will be deleted automatically and are not returned
* @return IShare[]
* @since 9.0.0
*/
public function getSharesBy($userId, $shareType, $path = null, $reshares = false, $limit = 50, $offset = 0);
public function getSharesBy($userId, $shareType, $path = null, $reshares = false, $limit = 50, $offset = 0, bool $onlyValid = true);

/**
* Get shares shared with $user.
Expand Down Expand Up @@ -168,11 +170,12 @@ public function getDeletedSharedWith($userId, $shareType, $node = null, $limit =
*
* @param string $id
* @param string|null $recipient userID of the recipient
* @param bool $onlyValid Only returns valid shares, invalid shares will be deleted automatically and are not returned
* @return IShare
* @throws ShareNotFound
* @since 9.0.0
*/
public function getShareById($id, $recipient = null);
public function getShareById($id, $recipient = null, bool $onlyValid = true);

/**
* Get the share by token possible with password
Expand Down