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.

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

Encryption trash fixes #3425

Merged
merged 9 commits into from
Dec 6, 2024
Merged

Encryption trash fixes #3425

merged 9 commits into from
Dec 6, 2024

Conversation

icewind1991
Copy link
Member

@icewind1991 icewind1991 commented Nov 14, 2024

Currently, the groupfolders trashbin doesn't actually support encryption. Things are only somewhat working by chance.

This is because currently the trash/restore operations for groupfolders don't go through the encryption storage wrapper. This PR makes sure the encryption storage wrappers are in effect when trashing/restoring files.

  • fix file names generated when restore has a filename conflict missing a .
  • make "move to trash" and "restore" operations go trough the encryption wrapper
  • make the encryption wrapper apply to the trashbin so files don't decrypted when being moved to trash
  • don't break restoring files that were deleted before this fix

To test

  • setup an instance, create a groupfolder, setup encryption
  • Enable groupfolder encryption occ config:app:set groupfolders enable_encryption --value='true'
  • Upload some files to the groupfolder and delete them
  • Download the delete files: it will be still encrypted
  • Apply this PR
  • Upload and delete some more files
  • Download the newly deleted files from the trashbin, they should be decrypted
  • (optional) verify that the on-disk trashed files are encrypted
  • Restore all deleted files
  • Ensure all restored files can be read

@icewind1991 icewind1991 force-pushed the encryption-trash-fixes branch from 6778cd9 to 8c13d67 Compare November 28, 2024 13:59
@icewind1991 icewind1991 requested review from provokateurin, artonge, a team and skjnldsv and removed request for a team November 28, 2024 15:58
@icewind1991 icewind1991 added this to the Nextcloud 31 milestone Nov 28, 2024
@icewind1991 icewind1991 marked this pull request as ready for review November 28, 2024 16:06
lib/Trash/TrashBackend.php Outdated Show resolved Hide resolved
if ($groupFolder['folder_id'] === (int)$folderId) {
$trashRoot = $this->getTrashFolder((int)$folderId);
if ($groupFolder['folder_id'] === $folderId) {
$trashRoot = $this->rootFolder->get('/' . $user->getUID() . '/files_trashbin/groupfolders/' . $folderId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does that work with the user uid in the path for the groupfolder trash?
Is the trash for a groupfolders mounted in each of the users (having access) trashbin path? Where is that handled?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the trash is now mounted for each user for the groupfolder, see the changes to MountProvider

@icewind1991 icewind1991 force-pushed the encryption-trash-fixes branch from 3821522 to 7699828 Compare December 2, 2024 17:06
@icewind1991 icewind1991 force-pushed the encryption-trash-fixes branch from 7699828 to 630caec Compare December 6, 2024 14:33
@icewind1991
Copy link
Member Author

Force-merging as the failing cypress test is caused by an upstream regression

@icewind1991 icewind1991 merged commit b849818 into master Dec 6, 2024
44 of 46 checks passed
@icewind1991 icewind1991 deleted the encryption-trash-fixes branch December 6, 2024 16:54
@icewind1991
Copy link
Member Author

/backport to stable30

@icewind1991
Copy link
Member Author

29: #3484

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants