Merge pull request nextcloud#16101 from nextcloud/fix/ldap/invaliate-…

…user-after-delete-by-plugin invalidates user when plugin reported deletion success
small1 · Jun 27, 2019 · 690406d · 690406d
2 parents 0d3f9be + d0f31c5
commit 690406d
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 7 deletions.
diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php
@@ -1134,8 +1134,17 @@ public function createGroup($gid) {
 		if ($this->groupPluginManager->implementsActions(GroupInterface::CREATE_GROUP)) {
 			if ($dn = $this->groupPluginManager->createGroup($gid)) {
 				//updates group mapping
-				$this->access->dn2ocname($dn, $gid, false);
-				$this->access->connection->writeToCache("groupExists".$gid, true);
+				$uuid = $this->access->getUUID($dn, false);
+				if(is_string($uuid)) {
+					$this->access->mapAndAnnounceIfApplicable(
+						$this->access->getGroupMapper(),
+						$dn,
+						$gid,
+						$uuid,
+						false
+					);
+					$this->access->connection->writeToCache("groupExists" . $gid, true);
+				}
 			}
 			return $dn != null;
 		}

diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php
@@ -382,18 +382,21 @@ public function userExists($uid) {
 	*/
 	public function deleteUser($uid) {
 		if ($this->userPluginManager->canDeleteUser()) {
-			return $this->userPluginManager->deleteUser($uid);
+			$status = $this->userPluginManager->deleteUser($uid);
+			if($status === false) {
+				return false;
+			}
 		}
 
 		$marked = $this->ocConfig->getUserValue($uid, 'user_ldap', 'isDeleted', 0);
 		if((int)$marked === 0) {
 			\OC::$server->getLogger()->notice(
 				'User '.$uid . ' is not marked as deleted, not cleaning up.',
-				array('app' => 'user_ldap'));
+				['app' => 'user_ldap']);
 			return false;
 		}
 		\OC::$server->getLogger()->info('Cleaning up after user ' . $uid,
-			array('app' => 'user_ldap'));
+			['app' => 'user_ldap']);
 
 		$this->access->getUserMapper()->unmap($uid); // we don't emit unassign signals here, since it is implicit to delete signals fired from core
 		$this->access->userManager->invalidate($uid);

diff --git a/apps/user_ldap/tests/User_LDAPTest.php b/apps/user_ldap/tests/User_LDAPTest.php
@@ -343,9 +343,27 @@ public function testDeleteUserWithPlugin() {
 		$this->pluginManager->expects($this->once())
 			->method('deleteUser')
 			->with('uid')
-			->willReturn('result');
+			->willReturn(true);
+
+		$this->config->expects($this->once())
+			->method('getUserValue')
+			->with('uid', 'user_ldap', 'isDeleted', 0)
+			->willReturn(1);
+
+		$mapper = $this->createMock(UserMapping::class);
+		$mapper->expects($this->once())
+			->method('unmap')
+			->with('uid');
+
+		$this->access->expects($this->atLeastOnce())
+			->method('getUserMapper')
+			->willReturn($mapper);
+
+		$this->userManager->expects($this->once())
+			->method('invalidate')
+			->with('uid');
 
-		$this->assertEquals($this->backend->deleteUser('uid'),'result');
+		$this->assertEquals(true, $this->backend->deleteUser('uid'));
 	}
 
 	/**