Skip to content

Commit

Permalink
fix(user_ldap): Avoid extra LDAP request when mapping a user for the …
Browse files Browse the repository at this point in the history 
…first time

Avoids using several LDAP searches to get UUID, display name and
 internal name, now gets all attributes at the same time.
Also avoids extra request to build an unused user object in userExists.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
  • Loading branch information
@come-nc
come-nc committed Jul 8, 2024
1 parent a434dfb commit e215f40
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 24 deletions.
116 changes: 95 additions & 21 deletions apps/user_ldap/lib/Access.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,56 @@ public function getConnection() {
return $this->connection;
}

/**
* Reads several attributes for an LDAP record identified by a DN and a filter
* No support for ranged attributes.
*
* @param string $dn the record in question
* @param array $attrs the attributes that shall be retrieved
* if empty, just check the record's existence
* @param string $filter
* @return array|false an array of values on success or an empty
* array if $attr is empty, false otherwise
* @throws ServerNotAvailableException
*/
public function readAttributes(string $dn, array $attrs, string $filter = 'objectClass=*'): array|false {
if (!$this->checkConnection()) {
$this->logger->warning(
'No LDAP Connector assigned, access impossible for readAttribute.',
['app' => 'user_ldap']
);
return false;
}
$cr = $this->connection->getConnectionResource();
$attrs = array_map(
fn (string $attr): string => mb_strtolower($attr, 'UTF-8'),
$attrs,
);

$values = [];
$record = $this->executeRead($dn, $attrs, $filter);
if (is_bool($record)) {
// when an exists request was run and it was successful, an empty
// array must be returned
return $record ? [] : false;
}

$result = [];
foreach ($attrs as $attr) {
$values = $this->extractAttributeValuesFromResult($record, $attr);
if (!empty($values)) {
$result[$attr] = $values;
}
}

if (!empty($result)) {
return $result;
}

$this->logger->debug('Requested attributes {attrs} not found for ' . $dn, ['app' => 'user_ldap', 'attrs' => $attrs]);
return false;
}

/**
* reads a given attribute for an LDAP record identified by a DN
*
Expand Down Expand Up @@ -191,9 +241,9 @@ public function readAttribute(string $dn, string $attr, string $filter = 'object
* returned data on a successful usual operation
* @throws ServerNotAvailableException
*/
public function executeRead(string $dn, string $attribute, string $filter) {
public function executeRead(string $dn, string|array $attribute, string $filter) {
$dn = $this->helper->DNasBaseParameter($dn);
$rr = @$this->invokeLDAPMethod('read', $dn, $filter, [$attribute]);
$rr = @$this->invokeLDAPMethod('read', $dn, $filter, (is_string($attribute) ? [$attribute] : $attribute));
if (!$this->ldap->isResource($rr)) {
if ($attribute !== '') {
//do not throw this message on userExists check, irritates
Expand Down Expand Up @@ -410,15 +460,15 @@ public function dn2groupname($fdn, $ldapName = null) {
* @return string|false with with the name to use in Nextcloud
* @throws \Exception
*/
public function dn2username($fdn, $ldapName = null) {
public function dn2username($fdn) {
//To avoid bypassing the base DN settings under certain circumstances
//with the group support, check whether the provided DN matches one of
//the given Bases
if (!$this->isDNPartOfBase($fdn, $this->connection->ldapBaseUsers)) {
return false;
}

return $this->dn2ocname($fdn, $ldapName, true);
return $this->dn2ocname($fdn, null, true);
}

/**
Expand All @@ -441,12 +491,8 @@ public function dn2ocname($fdn, $ldapName = null, $isUser = true, &$newlyMapped
$newlyMapped = false;
if ($isUser) {
$mapper = $this->getUserMapper();
$nameAttribute = $this->connection->ldapUserDisplayName;
$filter = $this->connection->ldapUserFilter;
} else {
$mapper = $this->getGroupMapper();
$nameAttribute = $this->connection->ldapGroupDisplayName;
$filter = $this->connection->ldapGroupFilter;
}

//let's try to retrieve the Nextcloud name from the mappings table
Expand All @@ -455,6 +501,36 @@ public function dn2ocname($fdn, $ldapName = null, $isUser = true, &$newlyMapped
return $ncName;
}

if ($isUser) {
$nameAttribute = $this->connection->ldapUserDisplayName;
$filter = $this->connection->ldapUserFilter;
$uuidAttr = 'ldapUuidUserAttribute';
$uuidOverride = $this->connection->ldapExpertUUIDUserAttr;
$usernameAttribute = (string)$this->connection->ldapExpertUsernameAttr;
$attributesToRead = [$nameAttribute,$usernameAttribute];
// TODO fetch also display name attributes and cache them if the user is mapped
} else {
$nameAttribute = $this->connection->ldapGroupDisplayName;
$filter = $this->connection->ldapGroupFilter;
$uuidAttr = 'ldapUuidGroupAttribute';
$uuidOverride = $this->connection->ldapExpertUUIDGroupAttr;
$attributesToRead = [$nameAttribute];
}

if ($this->detectUuidAttribute($fdn, $isUser, false, $record)) {
$attributesToRead[] = $this->connection->$uuidAttr;
}

if ($record === null) {
/* No record was passed, fetch it */
$record = $this->readAttributes($fdn, $attributesToRead, $filter);
if ($record === false) {
$this->logger->debug('Cannot read attributes for ' . $fdn . '. Skipping.', ['filter' => $filter]);
$intermediates[($isUser ? 'user-' : 'group-') . $fdn] = true;
return false;
}
}

//second try: get the UUID and check if it is known. Then, update the DN and return the name.
$uuid = $this->getUUID($fdn, $isUser, $record);
if (is_string($uuid)) {
Expand All @@ -469,20 +545,9 @@ public function dn2ocname($fdn, $ldapName = null, $isUser = true, &$newlyMapped
return false;
}

if (is_null($ldapName)) {
$ldapName = $this->readAttribute($fdn, $nameAttribute, $filter);
if (!isset($ldapName[0]) || empty($ldapName[0])) {
$this->logger->debug('No or empty name for ' . $fdn . ' with filter ' . $filter . '.', ['app' => 'user_ldap']);
$intermediates[($isUser ? 'user-' : 'group-') . $fdn] = true;
return false;
}
$ldapName = $ldapName[0];
}

if ($isUser) {
$usernameAttribute = (string)$this->connection->ldapExpertUsernameAttr;
if ($usernameAttribute !== '') {
$username = $this->readAttribute($fdn, $usernameAttribute);
$username = $record[$usernameAttribute];
if (!isset($username[0]) || empty($username[0])) {
$this->logger->debug('No or empty username (' . $usernameAttribute . ') for ' . $fdn . '.', ['app' => 'user_ldap']);
return false;
Expand All @@ -504,6 +569,15 @@ public function dn2ocname($fdn, $ldapName = null, $isUser = true, &$newlyMapped
return false;
}
} else {
if (is_null($ldapName)) {
$ldapName = $record[$nameAttribute];
if (!isset($ldapName[0]) || empty($ldapName[0])) {
$this->logger->debug('No or empty name for ' . $fdn . ' with filter ' . $filter . '.', ['app' => 'user_ldap']);
$intermediates['group-' . $fdn] = true;
return false;
}
$ldapName = $ldapName[0];
}
$intName = $this->sanitizeGroupIDCandidate($ldapName);
}

Expand All @@ -521,6 +595,7 @@ public function dn2ocname($fdn, $ldapName = null, $isUser = true, &$newlyMapped
$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;
}
}
Expand All @@ -535,7 +610,6 @@ public function dn2ocname($fdn, $ldapName = null, $isUser = true, &$newlyMapped
'fdn' => $fdn,
'altName' => $altName,
'intName' => $intName,
'app' => 'user_ldap',
]
);
$newlyMapped = true;
Expand Down
36 changes: 36 additions & 0 deletions apps/user_ldap/lib/User/Manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public function invalidate($uid) {
/**
* @brief checks whether the Access instance has been set
* @throws \Exception if Access has not been set
* @psalm-assert !null $this->access
* @return null
*/
private function checkAccess() {
Expand Down Expand Up @@ -237,4 +238,39 @@ public function get($id) {

return $this->createInstancyByUserName($id);
}

/**
* @brief Checks whether a User object by its DN or Nextcloud username exists
* @param string $id the DN or username of the user
* @throws \Exception when connection could not be established
*/
public function exists($id): bool {
$this->checkAccess();
$this->logger->debug('Checking if {id} exists', ['id' => $id]);
if (isset($this->usersByDN[$id])) {
return true;
} elseif (isset($this->usersByUid[$id])) {
return true;
}

if ($this->access->stringResemblesDN($id)) {
$this->logger->debug('{id} looks like a dn', ['id' => $id]);
$uid = $this->access->dn2username($id);
if ($uid !== false) {
return true;
}
}

// Most likely a uid. Check whether it is a deleted user
if ($this->isDeletedUser($id)) {
return true;
}
$this->logger->debug('username2dn({id})', ['id' => $id]);
$dn = $this->access->username2dn($id);
$this->logger->debug('end username2dn({id})', ['id' => $id]);
if ($dn !== false) {
return true;
}
return false;
}
}
5 changes: 2 additions & 3 deletions apps/user_ldap/lib/User_LDAP.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,9 @@ public function userExists($uid) {
if (!is_null($userExists)) {
return (bool)$userExists;
}
//getting dn, if false the user does not exist. If dn, he may be mapped only, requires more checking.
$user = $this->access->userManager->get($uid);
$userExists = $this->access->userManager->exists($uid);

if (is_null($user)) {
if (!$userExists) {
$this->logger->debug(
'No DN found for '.$uid.' on '.$this->access->connection->ldapHost,
['app' => 'user_ldap']
Expand Down

0 comments on commit e215f40

Please sign in to comment.