From 0cd2358f3d17e66ac99cd61392f63396d853d690 Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 9 Mar 2026 10:54:42 -0400 Subject: [PATCH 1/5] refactor(user_ldap): add dn2ocname type hints and align docblock Signed-off-by: Josh --- apps/user_ldap/lib/Access.php | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 3e4efc5b8dbdb..585f82d2ba497 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -471,18 +471,28 @@ public function dn2username($fdn) { } /** - * returns an internal Nextcloud name for the given LDAP DN, false on DN outside of search DN + * Resolves an LDAP DN to an internal Nextcloud user/group ID, optionally creating a new mapping. * - * @param string $fdn the dn of the user object - * @param string|null $ldapName optional, the display name of the object - * @param bool $isUser optional, whether it is a user object (otherwise group assumed) - * @param bool|null $newlyMapped - * @param array|null $record - * @param bool $autoMapping Should the group be mapped if not yet mapped - * @return false|string with with the name to use in Nextcloud - * @throws \Exception + * Order: existing DN mapping, UUID mapping (updates DN), then optional auto-mapping (with collision fallback). + * + * @param string $fdn Full LDAP DN to resolve. + * @param string|null $ldapName Optional group display-name candidate (used only if $isUser is false). + * @param bool $isUser Whether the entry is a user (true) or a group (false). + * @param bool|null &$newlyMapped Tracks whether a new mapping gets created in this call. + * @param-out bool $newlyMapped True iff this call created a new mapping; false otherwise. + * @param array|null $record Optional pre-fetched LDAP record; if null, attributes are read from LDAP. + * @param bool $autoMapping If false, only existing mappings are resolved and no new mapping is created. + * @return string|false Internal Nextcloud name, or false if resolution/mapping fails. + * @throws ServerNotAvailableException */ - public function dn2ocname($fdn, $ldapName = null, $isUser = true, &$newlyMapped = null, ?array $record = null, bool $autoMapping = true) { + public function dn2ocname( + string $fdn, + ?string = $ldapName = null, + bool $isUser = true, + ?bool &$newlyMapped = null, + ?array $record = null, + bool $autoMapping = true, + ): string|false { static $intermediates = []; if (isset($intermediates[($isUser ? 'user-' : 'group-') . $fdn])) { return false; // is a known intermediate From 64eb783ca4e028b7f9d228e0a6ac9c8c80e97c2a Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 9 Mar 2026 11:40:33 -0400 Subject: [PATCH 2/5] refactor(user_ldap): make dn2ocname ldapCacheTTL restore exception-safe with finally Even without the robustness improvement, eliminates duplicate lines/logic. Also made conditional easier to read and clarified comments for this code block. Signed-off-by: Josh --- apps/user_ldap/lib/Access.php | 58 +++++++++++++++++------------------ 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 585f82d2ba497..af2f4128ce040 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -596,40 +596,40 @@ public function dn2ocname( $intName = $this->sanitizeGroupIDCandidate($ldapName); } - //a new user/group! Add it only if it doesn't conflict with other backend's users or existing groups - //disabling Cache is required to avoid that the new user is cached as not-existing in fooExists check - //NOTE: mind, disabling cache affects only this instance! Using it - // outside of core user management will still cache the user as non-existing. + // Map a new user/group only if the candidate ID does not collide with existing users/groups. + + // Disable LDAP cache for this connection instance so conflict-detection existence + // checks are evaluated fresh and, more importantly, their results are not cached before mapping. $originalTTL = $this->connection->ldapCacheTTL; $this->connection->setConfiguration(['ldapCacheTTL' => 0]); - if ($intName !== '' - && (($isUser && !$this->ncUserManager->userExists($intName)) - || (!$isUser && !Server::get(IGroupManager::class)->groupExists($intName)) - ) - ) { - $this->connection->setConfiguration(['ldapCacheTTL' => $originalTTL]); - $newlyMapped = $this->mapAndAnnounceIfApplicable($mapper, $fdn, $intName, $uuid, $isUser); - if ($newlyMapped) { - $this->logger->debug('Mapped {fdn} as {name}', ['fdn' => $fdn,'name' => $intName]); - return $intName; + + try { + if ($intName !== '') { + $noUserConflict = $isUser && !$this->ncUserManager->userExists($intName); + $noGroupConflict = !$isUser && !Server::get(IGroupManager::class)->groupExists($intName); + + if ($noUserConflict || $noGroupConflict) { + $newlyMapped = $this->mapAndAnnounceIfApplicable($mapper, $fdn, $intName, $uuid, $isUser); + if ($newlyMapped) { + $this->logger->debug('Mapped {fdn} as {name}', ['fdn' => $fdn, 'name' => $intName]); + return $intName; + } + } } - } - $this->connection->setConfiguration(['ldapCacheTTL' => $originalTTL]); - $altName = $this->createAltInternalOwnCloudName($intName, $isUser); - if (is_string($altName)) { - if ($this->mapAndAnnounceIfApplicable($mapper, $fdn, $altName, $uuid, $isUser)) { - $this->logger->warning( - 'Mapped {fdn} as {altName} because of a name collision on {intName}.', - [ - 'fdn' => $fdn, - 'altName' => $altName, - 'intName' => $intName, - ] - ); - $newlyMapped = true; - return $altName; + $altName = $this->createAltInternalOwnCloudName($intName, $isUser); + if (is_string($altName)) { + if ($this->mapAndAnnounceIfApplicable($mapper, $fdn, $altName, $uuid, $isUser)) { + $this->logger->warning( + 'Mapped {fdn} as {altName} because of a name collision on {intName}.', + ['fdn' => $fdn, 'altName' => $altName, 'intName' => $intName] + ); + $newlyMapped = true; + return $altName; + } } + } finally { + $this->connection->setConfiguration(['ldapCacheTTL' => $originalTTL]); } //if everything else did not help.. From dbe26e15c2e65f2f7c585fa8cc20efad4c652053 Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 9 Mar 2026 11:56:40 -0400 Subject: [PATCH 3/5] refactor(user_ldap): clarify intent of "intermediates" in dn2ocname New comment Code clarity Signed-off-by: Josh --- apps/user_ldap/lib/Access.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index af2f4128ce040..02751d9206d8d 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -493,9 +493,12 @@ public function dn2ocname( ?array $record = null, bool $autoMapping = true, ): string|false { + // Use a static cache to track DNs already identified as intermediates + // within this request, avoiding redundant processing for the same DN. static $intermediates = []; - if (isset($intermediates[($isUser ? 'user-' : 'group-') . $fdn])) { - return false; // is a known intermediate + $key = ($isUser ? 'user-' : 'group-') . $fdn; + if (isset($intermediates[$key])) { + return false; // already known intermediate } $newlyMapped = false; From d581e71c2cdcef6c91b97630e53e2886a9b5a865 Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 9 Mar 2026 12:30:48 -0400 Subject: [PATCH 4/5] chore: fixup typo Access refactor Signed-off-by: Josh --- apps/user_ldap/lib/Access.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 02751d9206d8d..675d21dcfdab6 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -487,7 +487,7 @@ public function dn2username($fdn) { */ public function dn2ocname( string $fdn, - ?string = $ldapName = null, + ?string $ldapName = null, bool $isUser = true, ?bool &$newlyMapped = null, ?array $record = null, From 745c48b2c9b99bad5b04dc146cf16faee33ae2f2 Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 9 Mar 2026 13:39:35 -0400 Subject: [PATCH 5/5] chore: fixup Access.php so php-cs is happy Signed-off-by: Josh --- apps/user_ldap/lib/Access.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 675d21dcfdab6..c55a02da8ca22 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -600,7 +600,7 @@ public function dn2ocname( } // Map a new user/group only if the candidate ID does not collide with existing users/groups. - + // Disable LDAP cache for this connection instance so conflict-detection existence // checks are evaluated fresh and, more importantly, their results are not cached before mapping. $originalTTL = $this->connection->ldapCacheTTL;