Merge pull request #53357 from nextcloud/backport/53250/stable31

[stable31] fix(user_ldap): Harmonize parameter obfuscation and serialization accross logging methods

Author: AndyScherzinger

apps/user_ldap/lib/LDAP.php

@@ -11,13 +11,15 @@
 use OCA\User_LDAP\DataCollector\LdapDataCollector;
 use OCA\User_LDAP\Exceptions\ConstraintViolationException;
 use OCP\IConfig;
+use OCP\ILogger;
 use OCP\Profiler\IProfiler;
 use OCP\Server;
 use Psr\Log\LoggerInterface;
 
 class LDAP implements ILDAPWrapper {
 	protected array $curArgs = [];
 	protected LoggerInterface $logger;
+	protected IConfig $config;
 
 	private ?LdapDataCollector $dataCollector = null;
 
@@ -32,6 +34,7 @@ public function __construct(
 		}
 
 		$this->logger = Server::get(LoggerInterface::class);
+		$this->config = Server::get(IConfig::class);
 	}
 
 	/**
@@ -291,6 +294,21 @@ protected function invokeLDAPMethod(string $func, ...$arguments) {
 		return null;
 	}
 
+	/**
+	 * Turn resources into string, and removes potentially problematic cookie string to avoid breaking logfiles
+	 */
+	private function sanitizeFunctionParameters(array $args): array {
+		return array_map(function ($item) {
+			if ($this->isResource($item)) {
+				return '(resource)';
+			}
+			if (isset($item[0]['value']['cookie']) && $item[0]['value']['cookie'] !== '') {
+				$item[0]['value']['cookie'] = '*opaque cookie*';
+			}
+			return $item;
+		}, $args);
+	}
+
 	private function preFunctionCall(string $functionName, array $args): void {
 		$this->curArgs = $args;
 		if (strcasecmp($functionName, 'ldap_bind') === 0 || strcasecmp($functionName, 'ldap_exop_passwd') === 0) {
@@ -301,32 +319,24 @@ private function preFunctionCall(string $functionName, array $args): void {
 			$args[2] = IConfig::SENSITIVE_VALUE;
 		}
 
-		$this->logger->debug('Calling LDAP function {func} with parameters {args}', [
-			'app' => 'user_ldap',
-			'func' => $functionName,
-			'args' => json_encode($args),
-		]);
+		if ($this->config->getSystemValue('loglevel') === ILogger::DEBUG) {
+			/* Only running this if debug loglevel is on, to avoid processing parameters on production */
+			$this->logger->debug('Calling LDAP function {func} with parameters {args}', [
+				'app' => 'user_ldap',
+				'func' => $functionName,
+				'args' => $this->sanitizeFunctionParameters($args),
+			]);
+		}
 
 		if ($this->dataCollector !== null) {
-			$args = array_map(function ($item) {
-				if ($this->isResource($item)) {
-					return '(resource)';
-				}
-				if (isset($item[0]['value']['cookie']) && $item[0]['value']['cookie'] !== '') {
-					$item[0]['value']['cookie'] = '*opaque cookie*';
-				}
-				return $item;
-			}, $this->curArgs);
-
 			$backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS);
-			$this->dataCollector->startLdapRequest($functionName, $args, $backtrace);
+			$this->dataCollector->startLdapRequest($functionName, $this->sanitizeFunctionParameters($args), $backtrace);
 		}
 
 		if ($this->logFile !== '' && is_writable(dirname($this->logFile)) && (!file_exists($this->logFile) || is_writable($this->logFile))) {
-			$args = array_map(fn ($item) => (!$this->isResource($item) ? $item : '(resource)'), $this->curArgs);
 			file_put_contents(
 				$this->logFile,
-				$functionName . '::' . json_encode($args) . "\n",
+				$functionName . '::' . json_encode($this->sanitizeFunctionParameters($args)) . "\n",
 				FILE_APPEND
 			);
 		}