Skip to content

feat(user_ldap): add function to find a user in ldap #56721

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

Draft
thpiron wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

feat(user_ldap): add function to find a user in ldap #56721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2f25d0b
feat(user_ldap): add function to find a user in ldap
thpiron Nov 26, 2025
c2d6df2
fix: apply linter
thpiron Dec 13, 2025
bdeb14b
fix: remove useless doctstring
thpiron Dec 13, 2025
fe25e82
fix: use a specific exception defined in public API
thpiron Dec 13, 2025
375a69d
fix: add since version
thpiron Dec 13, 2025
b430068
fix: go through all ldap backends to check user
thpiron Dec 13, 2025
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
5 changes: 5 additions & 0 deletions apps/user_ldap/lib/LDAPProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

use OCA\User_LDAP\User\DeletedUsersIndex;
use OCP\IServerContainer;
use OCP\IUser;
use OCP\LDAP\IDeletionFlagSupport;
use OCP\LDAP\ILDAPProvider;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -327,4 +328,8 @@ public function getMultiValueUserAttribute(string $uid, string $attribute): arra
$connection->writeToCache($key, $values);
return $values;
}

public function findOneUser(string $filter, string $attribute, string $searchTerm): ?IUser {
return $this->userBackend->getUserFromCustomAttribute($filter, $attribute, $searchTerm);
}
}
38 changes: 38 additions & 0 deletions apps/user_ldap/lib/User_LDAP.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
use OCA\User_LDAP\User\DeletedUsersIndex;
use OCA\User_LDAP\User\OfflineUser;
use OCA\User_LDAP\User\User;
use OCP\IUser;
use OCP\IUserBackend;
use OCP\IUserManager;
use OCP\LDAP\MultipleUsersReturnedException;
use OCP\Notification\IManager as INotificationManager;
use OCP\User\Backend\ICountMappedUsersBackend;
use OCP\User\Backend\ILimitAwareCountUsersBackend;
Expand All @@ -29,6 +32,7 @@ public function __construct(
protected UserPluginManager $userPluginManager,
protected LoggerInterface $logger,
protected DeletedUsersIndex $deletedUsersIndex,
protected IUserManager $userManager,
) {
parent::__construct($access);
}
Expand Down Expand Up @@ -643,4 +647,38 @@ public function setUserEnabled(string $uid, bool $enabled, callable $queryDataba
public function getDisabledUserList(?int $limit = null, int $offset = 0, string $search = ''): array {
throw new \Exception('This is implemented directly in User_Proxy');
}

/**
* Fetches one user from LDAP based on a filter or a custom attribute and search term.
*
* If no custom filter is provided or the filter is empty, it creates a simple equality filter with the given attribute.
* If a custom filter is provided, it uses that filter directly and the attribute and search term are ignored.
*
* @throws MultipleUsersReturnedException if multiple users have been found (search query should not allow this)
*
* @param string $filter The LDAP filter to use. If null or empty string, a default filter is constructed.
* @param string $attribute The LDAP attribute name to search against (e.g., 'mail', 'cn', 'uid').
* @param string $searchTerm The search term to match against the attribute. Will be escaped for LDAP filter safety.
*
* @return IUser |null Returns an IUser if found in LDAP using the configured LDAP filter, or null if no user is found.
*/
public function getUserFromCustomAttribute(string $filter, string $attribute, string $searchTerm): ?IUser {
if ($filter === null || $filter === '') {
Copy link
Member

@blizzz blizzz Dec 12, 2025

Choose a reason for hiding this comment

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

$filter cannot be null, as the signature demands a string.

Copy link
Author

@thpiron thpiron Dec 13, 2025

Choose a reason for hiding this comment

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

Noted, I'll change this when reworking the interface function(s), keeping this as unresolved for now.

$searchTerm = $this->access->escapeFilterPart($searchTerm);
$filter = "($attribute=$searchTerm)";
Copy link
Member

@blizzz blizzz Dec 12, 2025

Choose a reason for hiding this comment

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

The filter should always be an and-combination of the user list filter, otherwise it might pull in users without access.

(Technically the login filter would be more correct semantically, but it might not resolve on such setups by configuration to avoid direct login.)

Copy link
Author

@thpiron thpiron Dec 13, 2025

Choose a reason for hiding this comment

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

Noted, I'll change this when reworking the interface function(s), keeping this as unresolved for now.

}
$records = $this->access->searchUsers($filter, ['dn']);
if (count($records) === 1) {
$ldapUser = $this->access->userManager->get($records[0]['dn'][0]);
$user = $this->userManager->get($ldapUser->getUsername());
return $user;
} elseif (count($records) > 1) {
$this->logger->error(
'Multiple users found for filter: ' . $filter,
['app' => 'user_ldap']
);
throw new MultipleUsersReturnedException();
}
return null;
}
}
20 changes: 20 additions & 0 deletions apps/user_ldap/lib/User_Proxy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,17 @@
use OCA\User_LDAP\User\DeletedUsersIndex;
use OCA\User_LDAP\User\OfflineUser;
use OCA\User_LDAP\User\User;
use OCP\IUser;
use OCP\IUserBackend;
use OCP\IUserManager;
use OCP\Notification\IManager as INotificationManager;
use OCP\User\Backend\ICountMappedUsersBackend;
use OCP\User\Backend\ILimitAwareCountUsersBackend;
use OCP\User\Backend\IProvideEnabledStateBackend;
use OCP\UserInterface;
use Psr\Log\LoggerInterface;
use OCP\LDAP\MultipleUsersReturnedException;


/**
* @template-extends Proxy<User_LDAP>
Expand All @@ -30,6 +34,7 @@ public function __construct(
private UserPluginManager $userPluginManager,
private LoggerInterface $logger,
private DeletedUsersIndex $deletedUsersIndex,
private IUserManager $userManager,
) {
parent::__construct($helper, $ldap, $accessFactory);
}
Expand All @@ -41,6 +46,7 @@ protected function newInstance(string $configPrefix): User_LDAP {
$this->userPluginManager,
$this->logger,
$this->deletedUsersIndex,
$this->userManager,
);
}

Expand Down Expand Up @@ -431,4 +437,18 @@ public function getDisabledUserList(?int $limit = null, int $offset = 0, string
)
);
}

public function getUserFromCustomAttribute(string $filter, string $attribute, string $searchTerm): ?IUser {
$this->setup();
$user = null;
foreach ($this->backends as $backend) {
$fetchUser = $backend->getUserFromCustomAttribute($filter, $attribute, $searchTerm);
// if we found a different user, no need to continue
if ($user !== null && $fetchUser !== null && $fetchUser->getUID() !== $user->getUID()) {
throw new MultipleUsersReturnedException('Multiple users found for custom attribute search');
}
$user = $fetchUser; // may be null
}
return $user;
}
}
11 changes: 11 additions & 0 deletions lib/public/LDAP/ILDAPProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
*/
namespace OCP\LDAP;

use OCP\IUser;

/**
* Interface ILDAPProvider
*
Expand Down Expand Up @@ -151,4 +153,13 @@ public function getUserAttribute(string $uid, string $attribute): ?string;
* @since 22.0.0
*/
public function getMultiValueUserAttribute(string $uid, string $attribute): array;

/**
* Search for a single user in ldap
*
* @return IUser|null Returns a IUser if found in ldap using the configured ldap filter
* @throws MultipleUsersReturnedException if multiple users has been found (search query should not allow this)
* @since 33.0.0
*/
Copy link
Member

@blizzz blizzz Dec 12, 2025

Choose a reason for hiding this comment

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

On the API, we note down @since annotations

Suggested change
*/
* @since 33.0.0
*/

Copy link
Author

@thpiron thpiron Dec 13, 2025

Choose a reason for hiding this comment

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

375a69d

public function findOneUser(string $filter, string $attribute, string $searchTerm): ?IUser;
Copy link
Member

@blizzz blizzz Dec 12, 2025

Choose a reason for hiding this comment

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

The signature requires all parameters be set, contrary to the spec (it could have been written more precise, i take that) and I think contrary to your suggestion.

Two ways how to solve it:

  1. Use nullable and optional parameters: ?string $filter = null, ?string $attribute = null, ?string $searchTerm = null. The downside is that the usage does not become clear by the parameters and it is easy to use the methods wrongly.
  2. Split it into two methods: findOneUserByLdapFilter(string $filter): ?IUser and findOneUserByAttributeValue(string $attribute, string $searchTerm): ?IUser.

I prefer the latter.

We can also go a step back and agree on one solution, either the filter (which actually is a filter part, could pronounce this as well and clarify that this is and-connected to the user list filter, as per my comment further above), or the attribute-value pair.

Due to

Other login backends that may rely on LDAP, like the SAML/SSO or OIDC backend, will get an additional configuration option to specify an attribute to which the received user id is being compared.

leaving out the filter (I am also a bit nervous about that: to easy to make a mistake) and only accepting the attribute name and the search value. What do you think?

Copy link
Author

@thpiron thpiron Dec 13, 2025

Choose a reason for hiding this comment

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

I'm not happy with this function signature either, splitting it in to function is indeed better I think.

For most cases I think that only the attribute/searchTerm part would be enough, but maybe an having an "expert" config with the filter could be useful in complex configurations.
This would also be consistent with the way user_ldap config works, where it is almost always possible to edit the filter manually.

Maybe adding in the findOneUserByLdapFilter docstring that it should be used carefully by the other apps (as an advanced setting)?

}
14 changes: 14 additions & 0 deletions lib/public/LDAP/MultipleUsersReturnedException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCP\LDAP;

/**
* Exception for a ldap search that unexpectedly returns multiple users.
* @since 33.0.0
*/
class MultipleUsersReturnedException extends \Exception {
}