fix: update re-share if shared-by user has been revoked
- Resolves: #42407
Summary
TODO
- [ ] ...
Checklist
- Code is properly formatted
- Sign-off message is added to all commits
- [ ] Tests (unit, integration, api and/or acceptance) are included
- [ ] Screenshots before/after for front-end changes
- [ ] Documentation (manuals or wiki) has been updated or is not required
- [ ] Backports requested where applicable (ex: critical bugfixes)
Another effected case by this issue:
- UserA share Folder1 to UserB
- UserB share Folder1 to UserC
- UserA revoke UserB
- UserA try to update sharing permission of UserC -> Error: Could not get proper share mount for <NODE ID>. Failing since else the next calls are called with null
@artonge I've updated as you requested, please check again.
Can you add comments to clarify what each
ifgroup does?I am also wondering if we could find this shares with the following query:
SELECT f.fileid, f.path, f.storage, s.id as share_id, s.uid_owner as share_owner, s.uid_initiator as share_initiator, s.share_with as share_recipient FROM oc_filecache f JOIN oc_share s ON f.fileid = s.file_source AND s.uid_initiator NOT IN ( SELECT user_id FROM oc_mounts m WHERE f.storage = m.storage_id )If so, then we could have a background job to remove them every hour or so like https://github.com/nextcloud/server/blob/master/apps/files_sharing/lib/DeleteOrphanedSharesJob.php, which might be easier than the current solution.
@artonge It only works if the shared-by user refreshes his files list after his share has revoked. So, I don't think this query would do the job.
Tests are failing
@come-nc Could you please review again? All checks have passed now.
@luka-nextcloud After talking with Robin, actually the ownership transfer is supposed to update the owner of the share so there should be no problem in the first place?
@come-nc Does it mean that these changes are enough for this issue?
@come-nc Does it mean that these changes are enough for this issue?
No, the second diff looks like it will cause trouble by arbitrary changing sharedBy of shares.
I lost track of what you are trying to fix, can you explain how to reproduce your problem? Why do you remove the exception handling removing invalid shares in OwnershipTransferService?
@come-nc
- Reproduce steps:
- UserA share Folder1 to UserB
- UserB share Folder1 to UserC
- UserA deleted sharing to UserB
- UserA transfer Folder1 to UserD
-> UserC lost access to Folder1. When UserC open sharing tab of Folder1, the error message
Error: Could not get proper share mount for . Failing since else the next calls are called with nullreturned.
- Issue occurred because the sharing of UserB to UserC was not deleted or updated after UserA deleted sharing to UserB. Then, when UserA transfering Folder1 to UserD, that re-share record cannot be transferred because it throws
GenericShareExceptionerror.
@come-nc
- Reproduce steps:
- UserA share Folder1 to UserB
- UserB share Folder1 to UserC
- UserA deleted sharing to UserB
- UserA transfer Folder1 to UserD -> UserC lost access to Folder1. When UserC open sharing tab of Folder1, the error message
Error: Could not get proper share mount for . Failing since else the next calls are called with nullreturned.
- Issue occurred because the sharing of UserB to UserC was not deleted or updated after UserA deleted sharing to UserB. Then, when UserA transfering Folder1 to UserD, that re-share record cannot be transferred because it throws
GenericShareExceptionerror.
UserC already loses access to Folder1 after step 3, right? At that point the sharing tab shows no error?
@come-nc
UserC already loses access to Folder1 after step 3, right?
No, UserC didn't lose access to Folder1 after step3. This is the issue we are trying to fix.
At that point the sharing tab shows no error?
No error.
@come-nc Original problem: Re-share was not deleted when parent share deleted. Then, it caused error while transferring ownership.
Reproduce steps: Mentioned above.
Solution: Delete the re-share and its children when parent share is deleted.

