Skip to content

Commit

Permalink
Merge pull request #29180 from nextcloud/bug/27798/fix-php-error-in-l…
Browse files Browse the repository at this point in the history 
…dap-access
  • Loading branch information
@skjnldsv
skjnldsv authored Oct 15, 2021
2 parents 1119197 + f9e6f2e commit 8df577b
Show file tree
Hide file tree
Showing 12 changed files with 207 additions and 116 deletions.
3 changes: 2 additions & 1 deletion apps/user_ldap/ajax/wizard.php
View file
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
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
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

0 comments on commit 8df577b

Please sign in to comment.