Use Psr\Log\LoggerInterface in OCA\User_LDAP\Access

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

6 files changed, 65 insertions, 37 deletions

diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php
index 370a4440501..8f39e22117b 100644
--- a/apps/user_ldap/lib/Access.php
+++ b/apps/user_ldap/lib/Access.php
@@ -58,8 +58,8 @@ use OCA\User_LDAP\Mapping\UserMapping;
 use OCA\User_LDAP\User\Manager;
 use OCA\User_LDAP\User\OfflineUser;
 use OCP\IConfig;
-use OCP\ILogger;
 use OCP\IUserManager;
+use Psr\Log\LoggerInterface;
 use function strlen;
 use function substr;
 
@@ -96,6 +96,8 @@ class Access extends LDAPUtility {
 	private $config;
 	/** @var IUserManager */
 	private $ncUserManager;
+	/** @var LoggerInterface */
+	private $logger;
 	/** @var string */
 	private $lastCookie = '';
 
@@ -105,7 +107,8 @@ class Access extends LDAPUtility {
 		Manager $userManager,
 		Helper $helper,
 		IConfig $config,
-		IUserManager $ncUserManager
+		IUserManager $ncUserManager,
+		LoggerInterface $logger
 	) {
 		parent::__construct($ldap);
 		$this->connection = $connection;
@@ -114,6 +117,7 @@ class Access extends LDAPUtility {
 		$this->helper = $helper;
 		$this->config = $config;
 		$this->ncUserManager = $ncUserManager;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -186,15 +190,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
@@ -249,7 +254,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;
 	}
 
@@ -280,13 +285,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);
@@ -372,7 +377,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 {
@@ -546,14 +551,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);
+				$this->logger->debug('No or empty name for ' . $fdn . ' with filter ' . $filter . '.', ['app' => 'user_ldap']);
 				return false;
 			}
 			$ldapName = $ldapName[0];
@@ -564,7 +569,7 @@ class Access extends LDAPUtility {
 			if ($usernameAttribute !== '') {
 				$username = $this->readAttribute($fdn, $usernameAttribute);
 				if (!isset($username[0]) || empty($username[0])) {
-					\OCP\Util::writeLog('user_ldap', 'No or empty username (' . $usernameAttribute . ') for ' . $fdn . '.', ILogger::DEBUG);
+					$this->logger->debug('No or empty username (' . $usernameAttribute . ') for ' . $fdn . '.', ['app' => 'user_ldap']);
 					return false;
 				}
 				$username = $username[0];
@@ -574,9 +579,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
@@ -616,7 +620,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;
 	}
 
@@ -938,7 +942,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']
 				);
@@ -1117,13 +1121,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;
 			}
 
@@ -1157,7 +1161,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;
 		}
 
@@ -1173,7 +1177,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;
 		}
 
@@ -1218,7 +1222,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']
 				);
@@ -1254,7 +1258,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
 		]);
@@ -1764,7 +1768,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',
@@ -1777,7 +1781,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;
 	}
@@ -1872,7 +1876,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]
@@ -2047,7 +2051,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',
@@ -2079,7 +2083,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 96f2655f046..7b67401efb4 100644
--- a/apps/user_ldap/lib/AccessFactory.php
+++ b/apps/user_ldap/lib/AccessFactory.php
@@ -27,6 +27,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 */
@@ -39,18 +40,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) {
@@ -60,7 +65,8 @@ class AccessFactory {
 			$this->userManager,
 			$this->helper,
 			$this->config,
-			$this->ncUserManager
+			$this->ncUserManager,
+			$this->logger
 		);
 	}
 }
diff --git a/apps/user_ldap/lib/Jobs/Sync.php b/apps/user_ldap/lib/Jobs/Sync.php
index fed8910b2e5..9f2ea722cee 100644
--- a/apps/user_ldap/lib/Jobs/Sync.php
+++ b/apps/user_ldap/lib/Jobs/Sync.php
@@ -39,6 +39,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
@@ -59,6 +60,8 @@ class Sync extends TimedJob {
 	protected $dbc;
 	/** @var  IUserManager */
 	protected $ncUserManager;
+	/** @var  LoggerInterface */
+	protected $logger;
 	/** @var  IManager */
 	protected $notificationManager;
 	/** @var ConnectionFactory */
@@ -336,6 +339,12 @@ class Sync extends TimedJob {
 			$this->ncUserManager = \OC::$server->getUserManager();
 		}
 
+		if (isset($argument['logger'])) {
+			$this->logger = $argument['logger'];
+		} else {
+			$this->logger = \OC::$server->getLogger();
+		}
+
 		if (isset($argument['notificationManager'])) {
 			$this->notificationManager = $argument['notificationManager'];
 		} else {
@@ -366,7 +375,8 @@ class Sync extends TimedJob {
 				$this->userManager,
 				$this->ldapHelper,
 				$this->config,
-				$this->ncUserManager
+				$this->ncUserManager,
+				$this->logger
 			);
 		}
 	}
diff --git a/apps/user_ldap/lib/Proxy.php b/apps/user_ldap/lib/Proxy.php
index e723c4e3abf..860033597f0 100644
--- a/apps/user_ldap/lib/Proxy.php
+++ b/apps/user_ldap/lib/Proxy.php
@@ -37,6 +37,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 = [];
@@ -71,6 +72,7 @@ abstract class Proxy {
 		static $shareManager;
 		static $coreUserManager;
 		static $coreNotificationManager;
+		static $logger;
 		if ($fs === null) {
 			$ocConfig = \OC::$server->getConfig();
 			$fs = new FilesystemHelper();
@@ -82,12 +84,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/tests/AccessTest.php b/apps/user_ldap/tests/AccessTest.php
index 8c401722592..4d4f92770f6 100644
--- a/apps/user_ldap/tests/AccessTest.php
+++ b/apps/user_ldap/tests/AccessTest.php
@@ -53,6 +53,7 @@ use OCP\Image;
 use OCP\IUserManager;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\Share\IManager;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 /**
@@ -81,6 +82,8 @@ class AccessTest extends TestCase {
 	private $config;
 	/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
 	private $ncUserManager;
+	/** @var LoggerInterface|MockObject */
+	private $logger;
 	/** @var Access */
 	private $access;
 
@@ -94,6 +97,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,
@@ -101,7 +105,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);
@@ -241,7 +246,7 @@ class AccessTest extends TestCase {
 		list($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')
@@ -264,7 +269,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');
@@ -445,7 +450,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 d0f2e8d4a95..453b79d0cfd 100644
--- a/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php
+++ b/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php
@@ -144,7 +144,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());
 	}
 
 	/**