Skip to content

Commit

Permalink
fix(files_sharing,files): Do not validate shares before adjusting the…
Browse files Browse the repository at this point in the history 
… owner

Signed-off-by: provokateurin <kate@provokateurin.de>
  • Loading branch information
@provokateurin
provokateurin committed Nov 25, 2024
1 parent c2ca99e commit 715e714
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 22 deletions.
4 changes: 2 additions & 2 deletions apps/files/lib/Service/OwnershipTransferService.php
View file
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
View file
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
View file
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
View file
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

0 comments on commit 715e714

Please sign in to comment.