Skip to content

Also delete objectstore data on user deletion #15147

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

Closed
rullzer wants to merge 1 commit into from
Closed

Also delete objectstore data on user deletion #15147

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions lib/private/User/User.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

use OC\Accounts\AccountManager;
use OC\Files\Cache\Storage;
use OC\Files\Storage\Local;
use OC\Hooks\Emitter;
use OC_Helper;
use OCP\IAvatarManager;
Expand Down Expand Up @@ -203,6 +204,7 @@ public function delete() {
}
// get the home now because it won't return it after user deletion
$homePath = $this->getHome();
$userRoot = \OC::$server->getUserFolder($this->getUID());
$result = $this->backend->deleteUser($this->uid);
if ($result) {

Expand All @@ -221,6 +223,11 @@ public function delete() {
// Delete the user's keys in preferences
\OC::$server->getConfig()->deleteAllUserValues($this->uid);

// Delete all files
if (!$userRoot->getStorage()->instanceOfStorage(Local::class)) {
$userRoot->delete();
Copy link
Member

@MorrisJobke MorrisJobke Apr 18, 2019

Choose a reason for hiding this comment

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

Should we just do that for all OS and then drop the lines 231 to 236?

Copy link
Member

@MorrisJobke MorrisJobke Apr 18, 2019

Choose a reason for hiding this comment

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

or should we do this in a background job?

Copy link
Member Author

@rullzer rullzer Apr 18, 2019

Choose a reason for hiding this comment

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

Yes that was also my consideration.

So then deleting the user is:

  1. Disabling the user
  2. Setting a special flag
  3. Scheduling backgroundjob

Because we need to fetch the actual user to do the deletion. Else we can't fetch the storage etc.

Copy link
Member

@MorrisJobke MorrisJobke Apr 18, 2019

Choose a reason for hiding this comment

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

Makes sense 👍

Copy link
Member

@MorrisJobke MorrisJobke Apr 18, 2019

Choose a reason for hiding this comment

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

But this is something for master. This can go in like that for the back ports and then we add the background job for master.

}

// Delete user files in /data/
if ($homePath !== false) {
// FIXME: this operates directly on FS, should use View instead...
Expand Down