Skip to content
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

Jump to bottom

Fix federated group shares when no longer found in remote server #44587

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
+982 −28
Draft

Fix federated group shares when no longer found in remote server #44587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ccbe85b
refactor: Switch local and remote server roles
danxuliu Mar 21, 2024
3d9242c
test: Add integration test for federated share with unavailable server
danxuliu Mar 21, 2024
7437fd6
test: Add integration tests for federated shares with temporary down …
danxuliu Mar 21, 2024
2b1cc55
test: Add integration tests for federated shares from temporary down …
danxuliu Mar 21, 2024
b15de31
fix: Clear remote shares between scenarios
danxuliu Mar 27, 2024
19f7134
test: Add integration tests for deleting federated group shares from …
danxuliu Mar 22, 2024
514ce86
test: Add integration tests for deleting federated group shares with …
danxuliu Mar 22, 2024
86947cb
fix: Fix federated group shares when no longer found in remote server
danxuliu Mar 22, 2024
ad659f3
refactor: Add another user and group to the test case
danxuliu Mar 31, 2024
74e8524
refactor: Make test group share parameters configurable
danxuliu Mar 31, 2024
3749b31
refactor: Make external share manager for test group share configurable
danxuliu Mar 31, 2024
718560a
test: Add unit tests for removing federated group shares
danxuliu Mar 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 29 additions & 3 deletions apps/files_sharing/lib/External/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,18 @@ public function setMountPoint($source, $target) {
return $result;
}

public function removeShare($mountPoint): bool {
/**
* Remove a share based on its mountpoint.
*
* User shares are always fully removed; group shares by default are just
* marked again as pending for the current user, unless explicitly forced
* to be fully removed (parent group share and all its sub-shares).
*
* @param bool $force True to fully removed a group share, false to mark it
* as pending for the current user
* @return bool True if the share could be removed, false otherwise
*/
public function removeShare(string $mountPoint, bool $force = false): bool {
try {
$mountPointObj = $this->mountManager->find($mountPoint);
} catch (NotFoundException $e) {
Expand All @@ -617,7 +628,7 @@ public function removeShare($mountPoint): bool {

try {
$getShare = $this->connection->prepare('
SELECT `remote`, `share_token`, `remote_id`, `share_type`, `id`
SELECT `remote`, `share_token`, `remote_id`, `share_type`, `id`, `parent`
FROM `*PREFIX*share_external`
WHERE `mountpoint_hash` = ? AND `user` = ?');
$result = $getShare->execute([$hash, $this->uid]);
Expand All @@ -638,7 +649,22 @@ public function removeShare($mountPoint): bool {
$deleteResult = $query->execute([(int)$share['id']]);
$deleteResult->closeCursor();
} elseif ($share !== false && (int)$share['share_type'] === IShare::TYPE_GROUP) {
$this->updateAccepted((int)$share['id'], false);
if ($force) {
$qb = $this->connection->getQueryBuilder();
// delete group share entry and matching sub-entries
$qb->delete('share_external')
->where(
$qb->expr()->orX(
$qb->expr()->eq('id', $qb->createParameter('share_parent_id')),
$qb->expr()->eq('parent', $qb->createParameter('share_parent_id'))
)
);

$qb->setParameter('share_parent_id', $share['parent']);
$qb->executeStatement();
} else {
$this->updateAccepted((int)$share['id'], false);
}
}

$this->removeReShares($id);
Expand Down
4 changes: 2 additions & 2 deletions apps/files_sharing/lib/External/Storage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ public function checkStorageAvailability() {
// valid Nextcloud instance means that the public share no longer exists
// since this is permanent (re-sharing the file will create a new token)
// we remove the invalid storage
$this->manager->removeShare($this->mountPoint);
$this->manager->removeShare($this->mountPoint, true);
$this->manager->getMountManager()->removeMount($this->mountPoint);
throw new StorageInvalidException("Remote share not found", 0, $e);
} else {
Expand All @@ -248,7 +248,7 @@ public function checkStorageAvailability() {
}
} catch (ForbiddenException $e) {
// auth error, remove share for now (provide a dialog in the future)
$this->manager->removeShare($this->mountPoint);
$this->manager->removeShare($this->mountPoint, true);
$this->manager->getMountManager()->removeMount($this->mountPoint);
throw new StorageInvalidException("Auth error when getting remote share");
} catch (\GuzzleHttp\Exception\ConnectException $e) {
Expand Down