Merge pull request #2845 from nextcloud/deleteuser-gethomeearly

Get user home folder before deletion

2 files changed, 39 insertions, 1 deletions

diff --git a/lib/private/User/User.php b/lib/private/User/User.php
index 3cc6dc3b7ed..4b37efe0705 100644
--- a/lib/private/User/User.php
+++ b/lib/private/User/User.php
@@ -197,6 +197,8 @@ class User implements IUser {
 		if ($this->emitter) {
 			$this->emitter->emit('\OC\User', 'preDelete', array($this));
 		}
+		// get the home now because it won't return it after user deletion
+		$homePath = $this->getHome();
 		$result = $this->backend->deleteUser($this->uid);
 		if ($result) {
 
@@ -210,7 +212,11 @@ class User implements IUser {
 			\OC::$server->getConfig()->deleteAllUserValues($this->uid);
 
 			// Delete user files in /data/
-			\OC_Helper::rmdirr($this->getHome());
+			if ($homePath !== false) {
+				// FIXME: this operates directly on FS, should use View instead...
+				// also this is not testable/mockable...
+				\OC_Helper::rmdirr($homePath);
+			}
 
 			// Delete the users entry in the storage table
 			Storage::remove('home::' . $this->uid);
diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php
index 734e86c187b..edb8ac4224e 100644
--- a/tests/lib/User/UserTest.php
+++ b/tests/lib/User/UserTest.php
@@ -200,6 +200,38 @@ class UserTest extends TestCase {
 		$this->assertTrue($user->delete());
 	}
 
+	public function testDeleteWithDifferentHome() {
+		/**
+		 * @var Backend | \PHPUnit_Framework_MockObject_MockObject $backend
+		 */
+		$backend = $this->createMock(Dummy::class);
+
+		$backend->expects($this->at(0))
+			->method('implementsActions')
+			->will($this->returnCallback(function ($actions) {
+				if ($actions === Backend::GET_HOME) {
+					return true;
+				} else {
+					return false;
+				}
+			}));
+
+		// important: getHome MUST be called before deleteUser because
+		// once the user is deleted, getHome implementations might not
+		// return anything
+		$backend->expects($this->at(1))
+			->method('getHome')
+			->with($this->equalTo('foo'))
+			->will($this->returnValue('/home/foo'));
+
+		$backend->expects($this->at(2))
+			->method('deleteUser')
+			->with($this->equalTo('foo'));
+
+		$user = new User('foo', $backend);
+		$this->assertTrue($user->delete());
+	}
+
 	public function testGetHome() {
 		/**
 		 * @var Backend | \PHPUnit_Framework_MockObject_MockObject $backend