Skip to content

Avoid PHP errors when the LDAP attribute is not found #29180

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

Merged
skjnldsv merged 4 commits into from
Oct 15, 2021
Merged

Avoid PHP errors when the LDAP attribute is not found #29180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
437048e
Avoid PHP errors when the LDAP attribute is not found
come-nc Oct 12, 2021
51398d7
Use Psr\Log\LoggerInterface in OCA\User_LDAP\Access
come-nc Oct 12, 2021
d189a23
Fix two mistakes in previous migration to LoggerInterface in OCA\User…
come-nc Oct 14, 2021
f9e6f2e
Use Psr\Log\LoggerInterface where it can easily be used in user_ldap
come-nc Oct 14, 2021
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
3 changes: 2 additions & 1 deletion apps/user_ldap/ajax/wizard.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@
$userManager,
new \OCA\User_LDAP\Helper(\OC::$server->getConfig(), \OC::$server->getDatabaseConnection()),
\OC::$server->getConfig(),
\OC::$server->getUserManager()
\OC::$server->getUserManager(),
\OC::$server->get(\Psr\Log\LoggerInterface::class)
);

$wizard = new \OCA\User_LDAP\Wizard($configuration, $ldapWrapper, $access);
Expand Down
64 changes: 36 additions & 28 deletions apps/user_ldap/lib/Access.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@
use OCA\User_LDAP\User\OfflineUser;
use OCP\HintException;
use OCP\IConfig;
use OCP\ILogger;
use OCP\IUserManager;
use Psr\Log\LoggerInterface;
use function strlen;
use function substr;

Expand Down Expand Up @@ -95,6 +95,8 @@ class Access extends LDAPUtility {
private $config;
/** @var IUserManager */
private $ncUserManager;
/** @var LoggerInterface */
private $logger;
/** @var string */
private $lastCookie = '';

Expand All @@ -104,7 +106,8 @@ public function __construct(
Manager $userManager,
Helper $helper,
IConfig $config,
IUserManager $ncUserManager
IUserManager $ncUserManager,
LoggerInterface $logger
) {
parent::__construct($ldap);
$this->connection = $connection;
Expand All @@ -113,6 +116,7 @@ public function __construct(
$this->helper = $helper;
$this->config = $config;
$this->ncUserManager = $ncUserManager;
$this->logger = $logger;
}

/**
Expand Down Expand Up @@ -185,15 +189,16 @@ public function getConnection() {
*/
public function readAttribute($dn, $attr, $filter = 'objectClass=*') {
if (!$this->checkConnection()) {
\OCP\Util::writeLog('user_ldap',
$this->logger->warning(
'No LDAP Connector assigned, access impossible for readAttribute.',
ILogger::WARN);
['app' => 'user_ldap']
);
return false;
}
$cr = $this->connection->getConnectionResource();
if (!$this->ldap->isResource($cr)) {
//LDAP not available
\OCP\Util::writeLog('user_ldap', 'LDAP resource not available.', ILogger::DEBUG);
$this->logger->debug('LDAP resource not available.', ['app' => 'user_ldap']);
return false;
}
//Cancel possibly running Paged Results operation, otherwise we run in
Expand Down Expand Up @@ -248,7 +253,7 @@ public function readAttribute($dn, $attr, $filter = 'objectClass=*') {
}
} while ($isRangeRequest);

\OCP\Util::writeLog('user_ldap', 'Requested attribute ' . $attr . ' not found for ' . $dn, ILogger::DEBUG);
$this->logger->debug('Requested attribute ' . $attr . ' not found for ' . $dn, ['app' => 'user_ldap']);
return false;
}

Expand Down Expand Up @@ -279,13 +284,13 @@ public function executeRead($cr, $dn, $attribute, $filter, $maxResults) {
if (!$this->ldap->isResource($rr)) {
if ($attribute !== '') {
//do not throw this message on userExists check, irritates
\OCP\Util::writeLog('user_ldap', 'readAttribute failed for DN ' . $dn, ILogger::DEBUG);
$this->logger->debug('readAttribute failed for DN ' . $dn, ['app' => 'user_ldap']);
}
//in case an error occurs , e.g. object does not exist
return false;
}
if ($attribute === '' && ($filter === 'objectclass=*' || $this->invokeLDAPMethod('countEntries', $cr, $rr) === 1)) {
\OCP\Util::writeLog('user_ldap', 'readAttribute: ' . $dn . ' found', ILogger::DEBUG);
$this->logger->debug('readAttribute: ' . $dn . ' found', ['app' => 'user_ldap']);
return true;
}
$er = $this->invokeLDAPMethod('firstEntry', $cr, $rr);
Expand Down Expand Up @@ -371,7 +376,7 @@ public function setPassword($userDN, $password) {
$cr = $this->connection->getConnectionResource();
if (!$this->ldap->isResource($cr)) {
//LDAP not available
\OCP\Util::writeLog('user_ldap', 'LDAP resource not available.', ILogger::DEBUG);
$this->logger->debug('LDAP resource not available.', ['app' => 'user_ldap']);
return false;
}
try {
Expand Down Expand Up @@ -545,14 +550,14 @@ public function dn2ocname($fdn, $ldapName = null, $isUser = true, &$newlyMapped
}
} else {
//If the UUID can't be detected something is foul.
\OCP\Util::writeLog('user_ldap', 'Cannot determine UUID for ' . $fdn . '. Skipping.', ILogger::INFO);
$this->logger->debug('Cannot determine UUID for ' . $fdn . '. Skipping.', ['app' => 'user_ldap']);
return false;
}

if (is_null($ldapName)) {
$ldapName = $this->readAttribute($fdn, $nameAttribute, $filter);
if (!isset($ldapName[0]) && empty($ldapName[0])) {
\OCP\Util::writeLog('user_ldap', 'No or empty name for ' . $fdn . ' with filter ' . $filter . '.', ILogger::DEBUG);
if (!isset($ldapName[0]) || empty($ldapName[0])) {
$this->logger->debug('No or empty name for ' . $fdn . ' with filter ' . $filter . '.', ['app' => 'user_ldap']);
return false;
}
$ldapName = $ldapName[0];
Expand All @@ -562,16 +567,19 @@ public function dn2ocname($fdn, $ldapName = null, $isUser = true, &$newlyMapped
$usernameAttribute = (string)$this->connection->ldapExpertUsernameAttr;
if ($usernameAttribute !== '') {
$username = $this->readAttribute($fdn, $usernameAttribute);
if (!isset($username[0]) || empty($username[0])) {
$this->logger->debug('No or empty username (' . $usernameAttribute . ') for ' . $fdn . '.', ['app' => 'user_ldap']);
return false;
}
$username = $username[0];
} else {
$username = $uuid;
}
try {
$intName = $this->sanitizeUsername($username);
} catch (\InvalidArgumentException $e) {
\OC::$server->getLogger()->logException($e, [
'app' => 'user_ldap',
'level' => ILogger::WARN,
$this->logger->warning('Error sanitizing username: ' . $e->getMessage(), [
'exception' => $e,
]);
// we don't attempt to set a username here. We can go for
// for an alternative 4 digit random number as we would append
Expand Down Expand Up @@ -611,7 +619,7 @@ public function dn2ocname($fdn, $ldapName = null, $isUser = true, &$newlyMapped
}

//if everything else did not help..
\OCP\Util::writeLog('user_ldap', 'Could not create unique name for ' . $fdn . '.', ILogger::INFO);
$this->logger->info('Could not create unique name for ' . $fdn . '.', ['app' => 'user_ldap']);
return false;
}

Expand Down Expand Up @@ -933,7 +941,7 @@ public function batchApplyUserAttributes(array $ldapRecords) {
if ($user !== null) {
$user->processAttributes($userRecord);
} else {
\OC::$server->getLogger()->debug(
$this->logger->debug(
"The ldap user manager returned null for $ocName",
['app' => 'user_ldap']
);
Expand Down Expand Up @@ -1112,13 +1120,13 @@ private function invokeLDAPMethod() {
* Maybe implement exponential backoff?
* This was enough to get solr indexer working which has large delays between LDAP fetches.
*/
\OCP\Util::writeLog('user_ldap', "Connection lost on $command, attempting to reestablish.", ILogger::DEBUG);
$this->logger->debug("Connection lost on $command, attempting to reestablish.", ['app' => 'user_ldap']);
$this->connection->resetConnectionResource();
$cr = $this->connection->getConnectionResource();

if (!$this->ldap->isResource($cr)) {
// Seems like we didn't find any resource.
\OCP\Util::writeLog('user_ldap', "Could not $command, because resource is missing.", ILogger::DEBUG);
$this->logger->debug("Could not $command, because resource is missing.", ['app' => 'user_ldap']);
throw $e;
}

Expand Down Expand Up @@ -1152,7 +1160,7 @@ private function executeSearch(
if (!$this->ldap->isResource($cr)) {
// Seems like we didn't find any resource.
// Return an empty array just like before.
\OCP\Util::writeLog('user_ldap', 'Could not search, because resource is missing.', ILogger::DEBUG);
$this->logger->debug('Could not search, because resource is missing.', ['app' => 'user_ldap']);
return false;
}

Expand All @@ -1168,7 +1176,7 @@ private function executeSearch(
// cannot use $cr anymore, might have changed in the previous call!
$error = $this->ldap->errno($this->connection->getConnectionResource());
if (!$this->ldap->isResource($sr) || $error !== 0) {
\OCP\Util::writeLog('user_ldap', 'Attempt for Paging? ' . print_r($pagedSearchOK, true), ILogger::ERROR);
$this->logger->error('Attempt for Paging? ' . print_r($pagedSearchOK, true), ['app' => 'user_ldap']);
return false;
}

Expand Down Expand Up @@ -1213,7 +1221,7 @@ private function processPagedSearchStatus(
}
} else {
if (!is_null($limit) && (int)$this->connection->ldapPagingSize !== 0) {
\OC::$server->getLogger()->debug(
$this->logger->debug(
'Paged search was not available',
['app' => 'user_ldap']
);
Expand Down Expand Up @@ -1249,7 +1257,7 @@ private function count(
?int $offset = null,
bool $skipHandling = false
) {
\OC::$server->getLogger()->debug('Count filter: {filter}', [
$this->logger->debug('Count filter: {filter}', [
'app' => 'user_ldap',
'filter' => $filter
]);
Expand Down Expand Up @@ -1759,7 +1767,7 @@ private function detectUuidAttribute($dn, $isUser = true, $force = false, array

$value = $this->readAttribute($dn, $attribute);
if (is_array($value) && isset($value[0]) && !empty($value[0])) {
\OC::$server->getLogger()->debug(
$this->logger->debug(
'Setting {attribute} as {subject}',
[
'app' => 'user_ldap',
Expand All @@ -1772,7 +1780,7 @@ private function detectUuidAttribute($dn, $isUser = true, $force = false, array
return true;
}
}
\OC::$server->getLogger()->debug('Could not autodetect the UUID attribute', ['app' => 'user_ldap']);
$this->logger->debug('Could not autodetect the UUID attribute', ['app' => 'user_ldap']);

return false;
}
Expand Down Expand Up @@ -1867,7 +1875,7 @@ public function formatGuid2ForFilterUser($guid) {
* an exception here would kill the experience for a valid, acting
* user. Instead we write a log message.
*/
\OC::$server->getLogger()->info(
$this->logger->info(
'Passed string does not resemble a valid GUID. Known UUID ' .
'({uuid}) probably does not match UUID configuration.',
['app' => 'user_ldap', 'uuid' => $guid]
Expand Down Expand Up @@ -2042,7 +2050,7 @@ private function initPagedSearch(
): bool {
$pagedSearchOK = false;
if ($limit !== 0) {
\OC::$server->getLogger()->debug(
$this->logger->debug(
'initializing paged search for filter {filter}, base {base}, attr {attr}, limit {limit}, offset {offset}',
[
'app' => 'user_ldap',
Expand Down Expand Up @@ -2074,7 +2082,7 @@ private function initPagedSearch(
'controlPagedResult', $this->connection->getConnectionResource(), $limit, false
);
if ($pagedSearchOK) {
\OC::$server->getLogger()->debug('Ready for a paged search', ['app' => 'user_ldap']);
$this->logger->debug('Ready for a paged search', ['app' => 'user_ldap']);
}
/* ++ Fixing RHDS searches with pages with zero results ++
* We coudn't get paged searches working with our RHDS for login ($limit = 0),
Expand Down
10 changes: 8 additions & 2 deletions apps/user_ldap/lib/AccessFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use OCA\User_LDAP\User\Manager;
use OCP\IConfig;
use OCP\IUserManager;
use Psr\Log\LoggerInterface;

class AccessFactory {
/** @var ILDAPWrapper */
Expand All @@ -38,18 +39,22 @@ class AccessFactory {
protected $config;
/** @var IUserManager */
private $ncUserManager;
/** @var LoggerInterface */
private $logger;

public function __construct(
ILDAPWrapper $ldap,
Manager $userManager,
Helper $helper,
IConfig $config,
IUserManager $ncUserManager) {
IUserManager $ncUserManager,
LoggerInterface $logger) {
$this->ldap = $ldap;
$this->userManager = $userManager;
$this->helper = $helper;
$this->config = $config;
$this->ncUserManager = $ncUserManager;
$this->logger = $logger;
}

public function get(Connection $connection) {
Expand All @@ -59,7 +64,8 @@ public function get(Connection $connection) {
$this->userManager,
$this->helper,
$this->config,
$this->ncUserManager
$this->ncUserManager,
$this->logger
);
}
}
Loading