diff options
author | John Molakvoæ <skjnldsv@users.noreply.github.com> | 2021-10-15 11:03:08 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-10-15 11:03:08 +0200 |
commit | 8df577ba0a706841399097301f9b802b7a24005d (patch) | |
tree | 20884d6742ec6a97b5885537f382eec5ce658378 | |
parent | 111919727687fe2f9481318ce5ea4c62fc9959c8 (diff) | |
parent | f9e6f2ea577b103b00be81bda3355a73245f1152 (diff) | |
download | nextcloud-server-8df577ba0a706841399097301f9b802b7a24005d.tar.gz nextcloud-server-8df577ba0a706841399097301f9b802b7a24005d.zip |
Merge pull request #29180 from nextcloud/bug/27798/fix-php-error-in-ldap-access
-rw-r--r-- | apps/user_ldap/ajax/wizard.php | 3 | ||||
-rw-r--r-- | apps/user_ldap/lib/Access.php | 64 | ||||
-rw-r--r-- | apps/user_ldap/lib/AccessFactory.php | 10 | ||||
-rw-r--r-- | apps/user_ldap/lib/Connection.php | 70 | ||||
-rw-r--r-- | apps/user_ldap/lib/Group_LDAP.php | 6 | ||||
-rw-r--r-- | apps/user_ldap/lib/Jobs/Sync.php | 12 | ||||
-rw-r--r-- | apps/user_ldap/lib/Jobs/UpdateGroups.php | 46 | ||||
-rw-r--r-- | apps/user_ldap/lib/Proxy.php | 5 | ||||
-rw-r--r-- | apps/user_ldap/lib/User_LDAP.php | 42 | ||||
-rw-r--r-- | apps/user_ldap/lib/Wizard.php | 50 | ||||
-rw-r--r-- | apps/user_ldap/tests/AccessTest.php | 13 | ||||
-rw-r--r-- | apps/user_ldap/tests/Integration/AbstractIntegrationTest.php | 2 |
12 files changed, 207 insertions, 116 deletions
diff --git a/apps/user_ldap/ajax/wizard.php b/apps/user_ldap/ajax/wizard.php index c65ae23cd3b..9df66f81b13 100644 --- a/apps/user_ldap/ajax/wizard.php +++ b/apps/user_ldap/ajax/wizard.php @@ -69,7 +69,8 @@ $access = new \OCA\User_LDAP\Access( $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); diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 14178023e12..7b7ae74d3f3 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -57,8 +57,8 @@ use OCA\User_LDAP\User\Manager; 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; @@ -95,6 +95,8 @@ class Access extends LDAPUtility { private $config; /** @var IUserManager */ private $ncUserManager; + /** @var LoggerInterface */ + private $logger; /** @var string */ private $lastCookie = ''; @@ -104,7 +106,8 @@ class Access extends LDAPUtility { Manager $userManager, Helper $helper, IConfig $config, - IUserManager $ncUserManager + IUserManager $ncUserManager, + LoggerInterface $logger ) { parent::__construct($ldap); $this->connection = $connection; @@ -113,6 +116,7 @@ class Access extends LDAPUtility { $this->helper = $helper; $this->config = $config; $this->ncUserManager = $ncUserManager; + $this->logger = $logger; } /** @@ -185,15 +189,16 @@ class Access extends LDAPUtility { */ 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 @@ -248,7 +253,7 @@ class Access extends LDAPUtility { } } 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; } @@ -279,13 +284,13 @@ class Access extends LDAPUtility { 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); @@ -371,7 +376,7 @@ class Access extends LDAPUtility { $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 { @@ -545,14 +550,14 @@ class Access extends LDAPUtility { } } 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]; @@ -562,6 +567,10 @@ class Access extends LDAPUtility { $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; @@ -569,9 +578,8 @@ class Access extends LDAPUtility { 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 @@ -611,7 +619,7 @@ class Access extends LDAPUtility { } //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; } @@ -933,7 +941,7 @@ class Access extends LDAPUtility { 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'] ); @@ -1112,13 +1120,13 @@ class Access extends LDAPUtility { * 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; } @@ -1152,7 +1160,7 @@ class Access extends LDAPUtility { 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; } @@ -1168,7 +1176,7 @@ class Access extends LDAPUtility { // 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; } @@ -1213,7 +1221,7 @@ class Access extends LDAPUtility { } } 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'] ); @@ -1249,7 +1257,7 @@ class Access extends LDAPUtility { ?int $offset = null, bool $skipHandling = false ) { - \OC::$server->getLogger()->debug('Count filter: {filter}', [ + $this->logger->debug('Count filter: {filter}', [ 'app' => 'user_ldap', 'filter' => $filter ]); @@ -1759,7 +1767,7 @@ class Access extends LDAPUtility { $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', @@ -1772,7 +1780,7 @@ class Access extends LDAPUtility { 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; } @@ -1867,7 +1875,7 @@ class Access extends LDAPUtility { * 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] @@ -2042,7 +2050,7 @@ class Access extends LDAPUtility { ): 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', @@ -2074,7 +2082,7 @@ class Access extends LDAPUtility { '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), diff --git a/apps/user_ldap/lib/AccessFactory.php b/apps/user_ldap/lib/AccessFactory.php index 42869ebb24c..71867bbb9a4 100644 --- a/apps/user_ldap/lib/AccessFactory.php +++ b/apps/user_ldap/lib/AccessFactory.php @@ -26,6 +26,7 @@ namespace OCA\User_LDAP; use OCA\User_LDAP\User\Manager; use OCP\IConfig; use OCP\IUserManager; +use Psr\Log\LoggerInterface; class AccessFactory { /** @var ILDAPWrapper */ @@ -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) { @@ -59,7 +64,8 @@ class AccessFactory { $this->userManager, $this->helper, $this->config, - $this->ncUserManager + $this->ncUserManager, + $this->logger ); } } diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php index d57ec9ee6f8..77ae34f9f6c 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -37,7 +37,7 @@ namespace OCA\User_LDAP; use OC\ServerNotAvailableException; -use OCP\ILogger; +use Psr\Log\LoggerInterface; /** * magic properties (incomplete) @@ -103,6 +103,9 @@ class Connection extends LDAPUtility { protected $bindResult = []; + /** @var LoggerInterface */ + protected $logger; + /** * Constructor * @param ILDAPWrapper $ldap @@ -122,6 +125,7 @@ class Connection extends LDAPUtility { $helper = new Helper(\OC::$server->getConfig(), \OC::$server->getDatabaseConnection()); $this->doNotValidate = !in_array($this->configPrefix, $helper->getServerConfigurationPrefixes()); + $this->logger = \OC::$server->get(LoggerInterface::class); } public function __destruct() { @@ -208,7 +212,10 @@ class Connection extends LDAPUtility { $this->establishConnection(); } if (is_null($this->ldapConnectionRes)) { - \OCP\Util::writeLog('user_ldap', 'No LDAP Connection to server ' . $this->configuration->ldapHost, ILogger::ERROR); + $this->logger->error( + 'No LDAP Connection to server ' . $this->configuration->ldapHost, + ['app' => 'user_ldap'] + ); throw new ServerNotAvailableException('Connection to LDAP server could not be established'); } return $this->ldapConnectionRes; @@ -378,10 +385,10 @@ class Connection extends LDAPUtility { && (!is_null($this->configID))) { $this->configuration->$effectiveSetting = 'auto'; $this->configuration->saveConfiguration(); - \OCP\Util::writeLog('user_ldap', - 'Illegal value for the '. - $effectiveSetting.', '.'reset to '. - 'autodetect.', ILogger::INFO); + $this->logger->info( + 'Illegal value for the '.$effectiveSetting.', reset to autodetect.', + ['app' => 'user_ldap'] + ); } } } @@ -404,10 +411,9 @@ class Connection extends LDAPUtility { if ((stripos($this->configuration->ldapHost, 'ldaps://') === 0) && $this->configuration->ldapTLS) { $this->configuration->ldapTLS = false; - \OCP\Util::writeLog( - 'user_ldap', + $this->logger->info( 'LDAPS (already using secure connection) and TLS do not work together. Switched off TLS.', - ILogger::INFO + ['app' => 'user_ldap'] ); } } @@ -447,10 +453,9 @@ class Connection extends LDAPUtility { break; } $configurationOK = false; - \OCP\Util::writeLog( - 'user_ldap', + $this->logger->warning( $errorStr.'No '.$subj.' given!', - ILogger::WARN + ['app' => 'user_ldap'] ); } } @@ -462,11 +467,11 @@ class Connection extends LDAPUtility { ($agent === '' && $pwd !== '') || ($agent !== '' && $pwd === '') ) { - \OCP\Util::writeLog( - 'user_ldap', + $this->logger->warning( $errorStr.'either no password is given for the user ' . 'agent or a password is given, but not an LDAP agent.', - ILogger::WARN); + ['app' => 'user_ldap'] + ); $configurationOK = false; } @@ -475,20 +480,18 @@ class Connection extends LDAPUtility { $baseGroups = $this->configuration->ldapBaseGroups; if (empty($base) && empty($baseUsers) && empty($baseGroups)) { - \OCP\Util::writeLog( - 'user_ldap', + $this->logger->warning( $errorStr.'Not a single Base DN given.', - ILogger::WARN + ['app' => 'user_ldap'] ); $configurationOK = false; } if (mb_strpos($this->configuration->ldapLoginFilter, '%uid', 0, 'UTF-8') === false) { - \OCP\Util::writeLog( - 'user_ldap', + $this->logger->warning( $errorStr.'login filter does not contain %uid place holder.', - ILogger::WARN + ['app' => 'user_ldap'] ); $configurationOK = false; } @@ -532,34 +535,32 @@ class Connection extends LDAPUtility { return false; } if (!$this->ignoreValidation && !$this->configured) { - \OCP\Util::writeLog( - 'user_ldap', + $this->logger->warning( 'Configuration is invalid, cannot connect', - ILogger::WARN + ['app' => 'user_ldap'] ); return false; } if (!$this->ldapConnectionRes) { if (!$this->ldap->areLDAPFunctionsAvailable()) { $phpLDAPinstalled = false; - \OCP\Util::writeLog( - 'user_ldap', + $this->logger->error( 'function ldap_connect is not available. Make sure that the PHP ldap module is installed.', - ILogger::ERROR + ['app' => 'user_ldap'] ); return false; } if ($this->configuration->turnOffCertCheck) { if (putenv('LDAPTLS_REQCERT=never')) { - \OCP\Util::writeLog('user_ldap', + $this->logger->debug( 'Turned off SSL certificate validation successfully.', - ILogger::DEBUG); + ['app' => 'user_ldap'] + ); } else { - \OCP\Util::writeLog( - 'user_ldap', + $this->logger->warning( 'Could not turn off SSL certificate validation.', - ILogger::WARN + ['app' => 'user_ldap'] ); } } @@ -669,9 +670,10 @@ class Connection extends LDAPUtility { if (!$ldapLogin) { $errno = $this->ldap->errno($cr); - \OCP\Util::writeLog('user_ldap', + $this->logger->warning( 'Bind failed: ' . $errno . ': ' . $this->ldap->error($cr), - ILogger::WARN); + ['app' => 'user_ldap'] + ); // Set to failure mode, if LDAP error code is not one of // - LDAP_SUCCESS (0) diff --git a/apps/user_ldap/lib/Group_LDAP.php b/apps/user_ldap/lib/Group_LDAP.php index 891995d885d..744f921c6dd 100644 --- a/apps/user_ldap/lib/Group_LDAP.php +++ b/apps/user_ldap/lib/Group_LDAP.php @@ -49,7 +49,7 @@ use OC\Cache\CappedMemoryCache; use OC\ServerNotAvailableException; use OCP\Group\Backend\IGetDisplayNameBackend; use OCP\GroupInterface; -use OCP\ILogger; +use Psr\Log\LoggerInterface; class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, IGetDisplayNameBackend { protected $enabled = false; @@ -62,7 +62,7 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I protected $cachedNestedGroups; /** @var GroupPluginManager */ protected $groupPluginManager; - /** @var ILogger */ + /** @var LoggerInterface */ protected $logger; /** @@ -82,7 +82,7 @@ class Group_LDAP extends BackendUtility implements GroupInterface, IGroupLDAP, I $this->cachedGroupsByMember = new CappedMemoryCache(); $this->cachedNestedGroups = new CappedMemoryCache(); $this->groupPluginManager = $groupPluginManager; - $this->logger = OC::$server->getLogger(); + $this->logger = OC::$server->get(LoggerInterface::class); $this->ldapGroupMemberAssocAttr = strtolower($gAssoc); } diff --git a/apps/user_ldap/lib/Jobs/Sync.php b/apps/user_ldap/lib/Jobs/Sync.php index ea5abbf080c..10036f8dc12 100644 --- a/apps/user_ldap/lib/Jobs/Sync.php +++ b/apps/user_ldap/lib/Jobs/Sync.php @@ -38,6 +38,7 @@ use OCP\IConfig; use OCP\IDBConnection; use OCP\IUserManager; use OCP\Notification\IManager; +use Psr\Log\LoggerInterface; class Sync extends TimedJob { public const MAX_INTERVAL = 12 * 60 * 60; // 12h @@ -58,6 +59,8 @@ class Sync extends TimedJob { protected $dbc; /** @var IUserManager */ protected $ncUserManager; + /** @var LoggerInterface */ + protected $logger; /** @var IManager */ protected $notificationManager; /** @var ConnectionFactory */ @@ -335,6 +338,12 @@ class Sync extends TimedJob { $this->ncUserManager = \OC::$server->getUserManager(); } + if (isset($argument['logger'])) { + $this->logger = $argument['logger']; + } else { + $this->logger = \OC::$server->get(LoggerInterface::class); + } + if (isset($argument['notificationManager'])) { $this->notificationManager = $argument['notificationManager']; } else { @@ -365,7 +374,8 @@ class Sync extends TimedJob { $this->userManager, $this->ldapHelper, $this->config, - $this->ncUserManager + $this->ncUserManager, + $this->logger ); } } diff --git a/apps/user_ldap/lib/Jobs/UpdateGroups.php b/apps/user_ldap/lib/Jobs/UpdateGroups.php index c2b43a3cf52..b42049eb3a8 100644 --- a/apps/user_ldap/lib/Jobs/UpdateGroups.php +++ b/apps/user_ldap/lib/Jobs/UpdateGroups.php @@ -35,7 +35,6 @@ use OCP\Group\Events\UserAddedEvent; use OCP\Group\Events\UserRemovedEvent; use OCP\IDBConnection; use OCP\IGroupManager; -use OCP\ILogger; use OCP\IUser; use OCP\IUserManager; use Psr\Log\LoggerInterface; @@ -89,15 +88,19 @@ class UpdateGroups extends TimedJob { } public function updateGroups() { - \OCP\Util::writeLog('user_ldap', 'Run background job "updateGroups"', ILogger::DEBUG); + $this->logger->debug( + 'Run background job "updateGroups"', + ['app' => 'user_ldap'] + ); $knownGroups = array_keys($this->getKnownGroups()); $actualGroups = $this->groupBackend->getGroups(); if (empty($actualGroups) && empty($knownGroups)) { - \OCP\Util::writeLog('user_ldap', + $this->logger->info( 'bgJ "updateGroups" – groups do not seem to be configured properly, aborting.', - ILogger::INFO); + ['app' => 'user_ldap'] + ); return; } @@ -105,7 +108,10 @@ class UpdateGroups extends TimedJob { $this->handleCreatedGroups(array_diff($actualGroups, $knownGroups)); $this->handleRemovedGroups(array_diff($knownGroups, $actualGroups)); - \OCP\Util::writeLog('user_ldap', 'bgJ "updateGroups" – Finished.', ILogger::DEBUG); + $this->logger->debug( + 'bgJ "updateGroups" – Finished.', + ['app' => 'user_ldap'] + ); } /** @@ -198,46 +204,56 @@ class UpdateGroups extends TimedJob { * @param string[] $createdGroups */ private function handleCreatedGroups($createdGroups) { - \OCP\Util::writeLog('user_ldap', 'bgJ "updateGroups" – dealing with created Groups.', ILogger::DEBUG); + $this->logger->debug( + 'bgJ "updateGroups" – dealing with created Groups.', + ['app' => 'user_ldap'] + ); $query = $this->dbc->getQueryBuilder(); $query->insert('ldap_group_members') ->setValue('owncloudname', $query->createParameter('owncloudname')) ->setValue('owncloudusers', $query->createParameter('owncloudusers')); foreach ($createdGroups as $createdGroup) { - \OCP\Util::writeLog('user_ldap', + $this->logger->info( 'bgJ "updateGroups" – new group "' . $createdGroup . '" found.', - ILogger::INFO); + ['app' => 'user_ldap'] + ); $users = serialize($this->groupBackend->usersInGroup($createdGroup)); $query->setParameter('owncloudname', $createdGroup) ->setParameter('owncloudusers', $users); $query->execute(); } - \OCP\Util::writeLog('user_ldap', + $this->logger->debug( 'bgJ "updateGroups" – FINISHED dealing with created Groups.', - ILogger::DEBUG); + ['app' => 'user_ldap'] + ); } /** * @param string[] $removedGroups */ private function handleRemovedGroups($removedGroups) { - \OCP\Util::writeLog('user_ldap', 'bgJ "updateGroups" – dealing with removed groups.', ILogger::DEBUG); + $this->logger->debug( + 'bgJ "updateGroups" – dealing with removed groups.', + ['app' => 'user_ldap'] + ); $query = $this->dbc->getQueryBuilder(); $query->delete('ldap_group_members') ->where($query->expr()->eq('owncloudname', $query->createParameter('owncloudname'))); foreach ($removedGroups as $removedGroup) { - \OCP\Util::writeLog('user_ldap', + $this->logger->info( 'bgJ "updateGroups" – group "' . $removedGroup . '" was removed.', - ILogger::INFO); + ['app' => 'user_ldap'] + ); $query->setParameter('owncloudname', $removedGroup); $query->execute(); } - \OCP\Util::writeLog('user_ldap', + $this->logger->debug( 'bgJ "updateGroups" – FINISHED dealing with removed groups.', - ILogger::DEBUG); + ['app' => 'user_ldap'] + ); } } diff --git a/apps/user_ldap/lib/Proxy.php b/apps/user_ldap/lib/Proxy.php index da069a4b6b4..8fb6a7f4c0f 100644 --- a/apps/user_ldap/lib/Proxy.php +++ b/apps/user_ldap/lib/Proxy.php @@ -36,6 +36,7 @@ use OCA\User_LDAP\Mapping\GroupMapping; use OCA\User_LDAP\Mapping\UserMapping; use OCA\User_LDAP\User\Manager; use OCP\Share\IManager; +use Psr\Log\LoggerInterface; abstract class Proxy { private static $accesses = []; @@ -70,6 +71,7 @@ abstract class Proxy { static $shareManager; static $coreUserManager; static $coreNotificationManager; + static $logger; if ($fs === null) { $ocConfig = \OC::$server->getConfig(); $fs = new FilesystemHelper(); @@ -81,12 +83,13 @@ abstract class Proxy { $coreUserManager = \OC::$server->getUserManager(); $coreNotificationManager = \OC::$server->getNotificationManager(); $shareManager = \OC::$server->get(IManager::class); + $logger = \OC::$server->get(LoggerInterface::class); } $userManager = new Manager($ocConfig, $fs, $log, $avatarM, new \OCP\Image(), $coreUserManager, $coreNotificationManager, $shareManager); $connector = new Connection($this->ldap, $configPrefix); - $access = new Access($connector, $this->ldap, $userManager, new Helper($ocConfig, \OC::$server->getDatabaseConnection()), $ocConfig, $coreUserManager); + $access = new Access($connector, $this->ldap, $userManager, new Helper($ocConfig, \OC::$server->getDatabaseConnection()), $ocConfig, $coreUserManager, $logger); $access->setUserMapper($userMap); $access->setGroupMapper($groupMap); self::$accesses[$configPrefix] = $access; diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php index 99d8e68b815..fdc7b0c3fbd 100644 --- a/apps/user_ldap/lib/User_LDAP.php +++ b/apps/user_ldap/lib/User_LDAP.php @@ -45,10 +45,9 @@ use OCA\User_LDAP\Exceptions\NotOnLDAP; use OCA\User_LDAP\User\OfflineUser; use OCA\User_LDAP\User\User; use OCP\IConfig; -use OCP\ILogger; use OCP\IUserSession; use OCP\Notification\IManager as INotificationManager; -use OCP\Util; +use Psr\Log\LoggerInterface; class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserInterface, IUserLDAP { /** @var \OCP\IConfig */ @@ -60,6 +59,9 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn /** @var UserPluginManager */ protected $userPluginManager; + /** @var LoggerInterface */ + protected $logger; + /** * @param Access $access * @param \OCP\IConfig $ocConfig @@ -71,6 +73,7 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn $this->ocConfig = $ocConfig; $this->notificationManager = $notificationManager; $this->userPluginManager = $userPluginManager; + $this->logger = \OC::$server->get(LoggerInterface::class); } /** @@ -172,17 +175,21 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn try { $ldapRecord = $this->getLDAPUserByLoginName($uid); } catch (NotOnLDAP $e) { - \OC::$server->getLogger()->logException($e, ['app' => 'user_ldap', 'level' => ILogger::DEBUG]); + $this->logger->debug( + $e->getMessage(), + ['app' => 'user_ldap', 'exception' => $e] + ); return false; } $dn = $ldapRecord['dn'][0]; $user = $this->access->userManager->get($dn); if (!$user instanceof User) { - Util::writeLog('user_ldap', + $this->logger->warning( 'LDAP Login: Could not get user object for DN ' . $dn . '. Maybe the LDAP entry has no set display name attribute?', - ILogger::WARN); + ['app' => 'user_ldap'] + ); return false; } if ($user->getUsername() !== false) { @@ -265,16 +272,20 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn $this->access->getFilterPartForUserSearch($search) ]); - Util::writeLog('user_ldap', + $this->logger->debug( 'getUsers: Options: search '.$search.' limit '.$limit.' offset '.$offset.' Filter: '.$filter, - ILogger::DEBUG); + ['app' => 'user_ldap'] + ); //do the search and translate results to Nextcloud names $ldap_users = $this->access->fetchListOfUsers( $filter, $this->access->userManager->getAttributes(true), $limit, $offset); $ldap_users = $this->access->nextcloudUserNames($ldap_users); - Util::writeLog('user_ldap', 'getUsers: '.count($ldap_users). ' Users found', ILogger::DEBUG); + $this->logger->debug( + 'getUsers: '.count($ldap_users). ' Users found', + ['app' => 'user_ldap'] + ); $this->access->connection->writeToCache($cachekey, $ldap_users); return $ldap_users; @@ -352,8 +363,10 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn $user = $this->access->userManager->get($uid); if (is_null($user)) { - Util::writeLog('user_ldap', 'No DN found for '.$uid.' on '. - $this->access->connection->ldapHost, ILogger::DEBUG); + $this->logger->debug( + 'No DN found for '.$uid.' on '.$this->access->connection->ldapHost, + ['app' => 'user_ldap'] + ); $this->access->connection->writeToCache('userExists'.$uid, false); return false; } @@ -378,12 +391,13 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn $marked = $this->ocConfig->getUserValue($uid, 'user_ldap', 'isDeleted', 0); if ((int)$marked === 0) { - \OC::$server->getLogger()->notice( + $this->logger->notice( 'User '.$uid . ' is not marked as deleted, not cleaning up.', - ['app' => 'user_ldap']); + ['app' => 'user_ldap'] + ); return false; } - \OC::$server->getLogger()->info('Cleaning up after user ' . $uid, + $this->logger->info('Cleaning up after user ' . $uid, ['app' => 'user_ldap']); $this->access->getUserMapper()->unmap($uid); // we don't emit unassign signals here, since it is implicit to delete signals fired from core @@ -618,7 +632,7 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn ); $this->access->cacheUserExists($username); } else { - \OC::$server->getLogger()->warning( + $this->logger->warning( 'Failed to map created LDAP user with userid {userid}, because UUID could not be determined', [ 'app' => 'user_ldap', diff --git a/apps/user_ldap/lib/Wizard.php b/apps/user_ldap/lib/Wizard.php index 9d69ef294c9..9b037b47134 100644 --- a/apps/user_ldap/lib/Wizard.php +++ b/apps/user_ldap/lib/Wizard.php @@ -38,7 +38,7 @@ namespace OCA\User_LDAP; use OC\ServerNotAvailableException; -use OCP\ILogger; +use Psr\Log\LoggerInterface; class Wizard extends LDAPUtility { /** @var \OCP\IL10N */ @@ -49,6 +49,9 @@ class Wizard extends LDAPUtility { protected $result; protected $resultCache = []; + /** @var LoggerInterface */ + protected $logger; + public const LRESULT_PROCESSED_OK = 2; public const LRESULT_PROCESSED_INVALID = 3; public const LRESULT_PROCESSED_SKIP = 4; @@ -76,6 +79,7 @@ class Wizard extends LDAPUtility { } $this->access = $access; $this->result = new WizardResult(); + $this->logger = \OC::$server->get(LoggerInterface::class); } public function __destruct() { @@ -299,9 +303,11 @@ class Wizard extends LDAPUtility { if ($winner !== '') { $this->applyFind('ldap_email_attr', $winner); if ($writeLog) { - \OCP\Util::writeLog('user_ldap', 'The mail attribute has ' . - 'automatically been reset, because the original value ' . - 'did not return any results.', ILogger::INFO); + $this->logger->info( + 'The mail attribute has automatically been reset, '. + 'because the original value did not return any results.', + ['app' => 'user_ldap'] + ); } } @@ -679,7 +685,10 @@ class Wizard extends LDAPUtility { foreach ($portSettings as $setting) { $p = $setting['port']; $t = $setting['tls']; - \OCP\Util::writeLog('user_ldap', 'Wiz: trying port '. $p . ', TLS '. $t, ILogger::DEBUG); + $this->logger->debug( + 'Wiz: trying port '. $p . ', TLS '. $t, + ['app' => 'user_ldap'] + ); //connectAndBind may throw Exception, it needs to be catched by the //callee of this method @@ -702,7 +711,10 @@ class Wizard extends LDAPUtility { 'ldapTLS' => (int)$t ]; $this->configuration->setConfiguration($config); - \OCP\Util::writeLog('user_ldap', 'Wiz: detected Port ' . $p, ILogger::DEBUG); + $this->logger->debug( + 'Wiz: detected Port ' . $p, + ['app' => 'user_ldap'] + ); $this->result->addChange('ldap_port', $p); return $this->result; } @@ -847,8 +859,10 @@ class Wizard extends LDAPUtility { if (!$this->ldap->isResource($rr)) { $errorNo = $this->ldap->errno($cr); $errorMsg = $this->ldap->error($cr); - \OCP\Util::writeLog('user_ldap', 'Wiz: Could not search base '.$base. - ' Error '.$errorNo.': '.$errorMsg, ILogger::INFO); + $this->logger->info( + 'Wiz: Could not search base '.$base.' Error '.$errorNo.': '.$errorMsg, + ['app' => 'user_ldap'] + ); return false; } $entries = $this->ldap->countEntries($cr, $rr); @@ -1024,7 +1038,10 @@ class Wizard extends LDAPUtility { break; } - \OCP\Util::writeLog('user_ldap', 'Wiz: Final filter '.$filter, ILogger::DEBUG); + $this->logger->debug( + 'Wiz: Final filter '.$filter, + ['app' => 'user_ldap'] + ); return $filter; } @@ -1044,7 +1061,10 @@ class Wizard extends LDAPUtility { if (!$hostInfo) { throw new \Exception(self::$l->t('Invalid Host')); } - \OCP\Util::writeLog('user_ldap', 'Wiz: Attempting to connect ', ILogger::DEBUG); + $this->logger->debug( + 'Wiz: Attempting to connect', + ['app' => 'user_ldap'] + ); $cr = $this->ldap->connect($host, $port); if (!is_resource($cr)) { throw new \Exception(self::$l->t('Invalid Host')); @@ -1063,7 +1083,10 @@ class Wizard extends LDAPUtility { } } - \OCP\Util::writeLog('user_ldap', 'Wiz: Attemping to Bind ', ILogger::DEBUG); + $this->logger->debug( + 'Wiz: Attemping to Bind', + ['app' => 'user_ldap'] + ); //interesting part: do the bind! $login = $this->ldap->bind($cr, $this->configuration->ldapAgentName, @@ -1078,7 +1101,10 @@ class Wizard extends LDAPUtility { if ($login === true) { $this->ldap->unbind($cr); - \OCP\Util::writeLog('user_ldap', 'Wiz: Bind successful to Port '. $port . ' TLS ' . (int)$tls, ILogger::DEBUG); + $this->logger->debug( + 'Wiz: Bind successful to Port '. $port . ' TLS ' . (int)$tls, + ['app' => 'user_ldap'] + ); return true; } diff --git a/apps/user_ldap/tests/AccessTest.php b/apps/user_ldap/tests/AccessTest.php index 92b337c2cb8..5945bbd2fde 100644 --- a/apps/user_ldap/tests/AccessTest.php +++ b/apps/user_ldap/tests/AccessTest.php @@ -51,6 +51,7 @@ use OCP\Image; use OCP\IUserManager; use OCP\Notification\IManager as INotificationManager; use OCP\Share\IManager; +use Psr\Log\LoggerInterface; use Test\TestCase; /** @@ -79,6 +80,8 @@ class AccessTest extends TestCase { private $config; /** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */ private $ncUserManager; + /** @var LoggerInterface|MockObject */ + private $logger; /** @var Access */ private $access; @@ -92,6 +95,7 @@ class AccessTest extends TestCase { $this->groupMapper = $this->createMock(GroupMapping::class); $this->ncUserManager = $this->createMock(IUserManager::class); $this->shareManager = $this->createMock(IManager::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->access = new Access( $this->connection, @@ -99,7 +103,8 @@ class AccessTest extends TestCase { $this->userManager, $this->helper, $this->config, - $this->ncUserManager + $this->ncUserManager, + $this->logger ); $this->access->setUserMapper($this->userMapper); $this->access->setGroupMapper($this->groupMapper); @@ -239,7 +244,7 @@ class AccessTest extends TestCase { [$lw, $con, $um, $helper] = $this->getConnectorAndLdapMock(); /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject $config */ $config = $this->createMock(IConfig::class); - $access = new Access($con, $lw, $um, $helper, $config, $this->ncUserManager); + $access = new Access($con, $lw, $um, $helper, $config, $this->ncUserManager, $this->logger); $lw->expects($this->exactly(1)) ->method('explodeDN') @@ -262,7 +267,7 @@ class AccessTest extends TestCase { /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject $config */ $config = $this->createMock(IConfig::class); $lw = new LDAP(); - $access = new Access($con, $lw, $um, $helper, $config, $this->ncUserManager); + $access = new Access($con, $lw, $um, $helper, $config, $this->ncUserManager, $this->logger); if (!function_exists('ldap_explode_dn')) { $this->markTestSkipped('LDAP Module not available'); @@ -443,7 +448,7 @@ class AccessTest extends TestCase { $attribute => ['count' => 1, $dnFromServer] ]); - $access = new Access($con, $lw, $um, $helper, $config, $this->ncUserManager); + $access = new Access($con, $lw, $um, $helper, $config, $this->ncUserManager, $this->logger); $values = $access->readAttribute('uid=whoever,dc=example,dc=org', $attribute); $this->assertSame($values[0], strtolower($dnFromServer)); } diff --git a/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php b/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php index c9d7d41d2b9..97177007332 100644 --- a/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php +++ b/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php @@ -143,7 +143,7 @@ abstract class AbstractIntegrationTest { * initializes the Access test instance */ protected function initAccess() { - $this->access = new Access($this->connection, $this->ldap, $this->userManager, $this->helper, \OC::$server->getConfig()); + $this->access = new Access($this->connection, $this->ldap, $this->userManager, $this->helper, \OC::$server->getConfig(), \OC::$server->getLogger()); } /** |