Merge pull request #42405 from nextcloud/fix/42374/ldap-remembered-gr…

…oups-format fix(LDAP): ensure stored groups are formatted as simple list
nextcloud · Dec 22, 2023 · 728dfa6 · 728dfa6
2 parents 81ed6d6 + ee096f8
commit 728dfa6
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 2 deletions.
diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php
@@ -685,7 +685,7 @@ private function isUserOnLDAP(string $uid): bool {
 
 	protected function getCachedGroupsForUserId(string $uid): array {
 		$groupStr = $this->config->getUserValue($uid, 'user_ldap', 'cached-group-memberships-' . $this->access->connection->getConfigPrefix(), '[]');
-		return json_decode($groupStr) ?? [];
+		return json_decode($groupStr, true) ?? [];
 	}
 
 	/**
@@ -838,7 +838,7 @@ public function getUserGroups($uid): array {
 			return $groups;
 		}
 
-		$groups = array_unique($groups, SORT_LOCALE_STRING);
+		$groups = array_values(array_unique($groups, SORT_LOCALE_STRING));
 		$this->access->connection->writeToCache($cacheKey, $groups);
 
 		$groupStr = \json_encode($groups);

diff --git a/apps/user_ldap/tests/Group_LDAPTest.php b/apps/user_ldap/tests/Group_LDAPTest.php
@@ -901,6 +901,33 @@ public function testGetUserGroupsOfflineUser() {
 		$this->assertTrue(in_array('groupF', $returnedGroups));
 	}
 
+	/**
+	 * regression tests against a case where a json object was stored instead of expected list
+	 * @see https://github.com/nextcloud/server/issues/42374
+	 */
+	public function testGetUserGroupsOfflineUserUnexpectedJson() {
+		$this->enableGroups();
+
+		$offlineUser = $this->createMock(OfflineUser::class);
+
+		$this->config->expects($this->any())
+			->method('getUserValue')
+			->with('userX', 'user_ldap', 'cached-group-memberships-', $this->anything())
+			// results in a json object: {"0":"groupB","2":"groupF"}
+			->willReturn(\json_encode([0 => 'groupB', 2 => 'groupF']));
+
+		$this->access->userManager->expects($this->any())
+			->method('get')
+			->with('userX')
+			->willReturn($offlineUser);
+
+		$this->initBackend();
+		$returnedGroups = $this->groupBackend->getUserGroups('userX');
+		$this->assertCount(2, $returnedGroups);
+		$this->assertTrue(in_array('groupB', $returnedGroups));
+		$this->assertTrue(in_array('groupF', $returnedGroups));
+	}
+
 	public function testGetUserGroupsUnrecognizedOfflineUser() {
 		$this->enableGroups();
 		$dn = 'cn=userX,dc=foobar';