From 710a69b4b5c81262da3c2f4efff10a87356598d0 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 12 May 2024 16:33:34 +0200 Subject: [PATCH] feat(log): Allow to combine log.conditions to only log (app&user) Signed-off-by: Joas Schilling --- config/config.sample.php | 12 ++++ lib/private/Diagnostics/EventLogger.php | 2 +- lib/private/Log.php | 92 ++++++++++++++++++------- lib/private/SystemConfig.php | 1 + tests/lib/LoggerTest.php | 90 ++++++++++++++++++++++++ 5 files changed, 172 insertions(+), 25 deletions(-) diff --git a/config/config.sample.php b/config/config.sample.php index ba0ea6d410136..6e1128c29521a 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -1057,6 +1057,9 @@ * this condition is met * - ``apps``: if the log message is invoked by one of the specified apps, * this condition is met + * - ``matches``: if all the conditions inside a group match, + * this condition is met. This allows to log only entries to an app + * by a few users. * * Defaults to an empty array. */ @@ -1064,6 +1067,15 @@ 'shared_secret' => '57b58edb6637fe3059b3595cf9c41b9', 'users' => ['sample-user'], 'apps' => ['files'], + 'matches' => [ + [ + 'shared_secret' => '57b58edb6637fe3059b3595cf9c41b9', + 'users' => ['sample-user'], + 'apps' => ['files'], + 'loglevel' => 1, + 'message' => 'contains substring' + ], + ], ], /** diff --git a/lib/private/Diagnostics/EventLogger.php b/lib/private/Diagnostics/EventLogger.php index 11c59b9227adb..40cbd3e9e5d58 100644 --- a/lib/private/Diagnostics/EventLogger.php +++ b/lib/private/Diagnostics/EventLogger.php @@ -48,7 +48,7 @@ public function isLoggingActivated(): bool { return true; } - $isDebugLevel = $this->internalLogger->getLogLevel([]) === Log::DEBUG; + $isDebugLevel = $this->internalLogger->getLogLevel([], '') === Log::DEBUG; return $systemValue && $isDebugLevel; } diff --git a/lib/private/Log.php b/lib/private/Log.php index c7684a1aefd18..4fce04367079f 100644 --- a/lib/private/Log.php +++ b/lib/private/Log.php @@ -14,6 +14,7 @@ use OC\Log\ExceptionSerializer; use OCP\EventDispatcher\IEventDispatcher; use OCP\ILogger; +use OCP\IRequest; use OCP\IUserSession; use OCP\Log\BeforeMessageLoggedEvent; use OCP\Log\IDataLogger; @@ -153,7 +154,7 @@ public function debug(string $message, array $context = []): void { * @param array $context */ public function log(int $level, string $message, array $context = []): void { - $minLevel = $this->getLogLevel($context); + $minLevel = $this->getLogLevel($context, $message); if ($level < $minLevel && (($this->crashReporters?->hasReporters() ?? false) === false) && (($this->eventDispatcher?->hasListeners(BeforeMessageLoggedEvent::class) ?? false) === false)) { @@ -194,9 +195,25 @@ public function log(int $level, string $message, array $context = []): void { } } - public function getLogLevel($context): int { + public function getLogLevel(array $context, string $message): int { + /** + * @psalm-var array{ + * shared_secret?: string, + * users?: string[], + * apps?: string[], + * matches?: array + * } $logCondition + */ $logCondition = $this->config->getValue('log.condition', []); + $userId = false; + /** * check for a special log condition - this enables an increased log on * a per request/user base @@ -206,21 +223,8 @@ public function getLogLevel($context): int { $this->logConditionSatisfied = false; if (!empty($logCondition)) { // check for secret token in the request - if (isset($logCondition['shared_secret'])) { - $request = \OC::$server->getRequest(); - - if ($request->getMethod() === 'PUT' && - !str_contains($request->getHeader('Content-Type'), 'application/x-www-form-urlencoded') && - !str_contains($request->getHeader('Content-Type'), 'application/json')) { - $logSecretRequest = ''; - } else { - $logSecretRequest = $request->getParam('log_secret', ''); - } - - // if token is found in the request change set the log condition to satisfied - if ($request && hash_equals($logCondition['shared_secret'], $logSecretRequest)) { - $this->logConditionSatisfied = true; - } + if (isset($logCondition['shared_secret']) && $this->checkLogSecret($logCondition['shared_secret'])) { + $this->logConditionSatisfied = true; } // check for user @@ -233,6 +237,8 @@ public function getLogLevel($context): int { } elseif (in_array($user->getUID(), $logCondition['users'], true)) { // if the user matches set the log condition to satisfied $this->logConditionSatisfied = true; + } else { + $userId = $user->getUID(); } } } @@ -243,6 +249,11 @@ public function getLogLevel($context): int { return ILogger::DEBUG; } + if ($userId === false && isset($logCondition['matches'])) { + $user = \OCP\Server::get(IUserSession::class)->getUser(); + $userId = $user === null ? false : $user->getUID(); + } + if (isset($context['app'])) { /** * check log condition based on the context of each log message @@ -253,16 +264,49 @@ public function getLogLevel($context): int { } } - $configLogLevel = $this->config->getValue('loglevel', ILogger::WARN); - if (is_numeric($configLogLevel)) { - return min((int)$configLogLevel, ILogger::FATAL); + if (!isset($logCondition['matches'])) { + $configLogLevel = $this->config->getValue('loglevel', ILogger::WARN); + if (is_numeric($configLogLevel)) { + return min((int)$configLogLevel, ILogger::FATAL); + } + + // Invalid configuration, warn the user and fall back to default level of WARN + error_log('Nextcloud configuration: "loglevel" is not a valid integer'); + return ILogger::WARN; + } + + foreach ($logCondition['matches'] as $option) { + if ( + (!isset($option['shared_secret']) || $this->checkLogSecret($option['shared_secret'])) + && (!isset($option['users']) || in_array($userId, $option['users'], true)) + && (!isset($option['apps']) || (isset($context['app']) && in_array($context['app'], $option['apps'], true))) + && (!isset($option['message']) || str_contains($message, $option['message'])) + ) { + if (!isset($option['apps']) && !isset($option['loglevel']) && !isset($option['message'])) { + /* Only user and/or secret are listed as conditions, we can cache the result for the rest of the request */ + $this->logConditionSatisfied = true; + return ILogger::DEBUG; + } + return $option['loglevel'] ?? ILogger::DEBUG; + } } - // Invalid configuration, warn the user and fall back to default level of WARN - error_log('Nextcloud configuration: "loglevel" is not a valid integer'); return ILogger::WARN; } + protected function checkLogSecret(string $conditionSecret): bool { + $request = \OCP\Server::get(IRequest::class); + + if ($request->getMethod() === 'PUT' && + !str_contains($request->getHeader('Content-Type'), 'application/x-www-form-urlencoded') && + !str_contains($request->getHeader('Content-Type'), 'application/json')) { + return hash_equals($conditionSecret, ''); + } + + // if token is found in the request change set the log condition to satisfied + return hash_equals($conditionSecret, $request->getParam('log_secret', '')); + } + /** * Logs an exception very detailed * @@ -275,7 +319,7 @@ public function logException(Throwable $exception, array $context = []): void { $app = $context['app'] ?? 'no app in context'; $level = $context['level'] ?? ILogger::ERROR; - $minLevel = $this->getLogLevel($context); + $minLevel = $this->getLogLevel($context, $context['message'] ?? $exception->getMessage()); if ($level < $minLevel && (($this->crashReporters?->hasReporters() ?? false) === false) && (($this->eventDispatcher?->hasListeners(BeforeMessageLoggedEvent::class) ?? false) === false)) { @@ -320,7 +364,7 @@ public function logData(string $message, array $data, array $context = []): void $app = $context['app'] ?? 'no app in context'; $level = $context['level'] ?? ILogger::ERROR; - $minLevel = $this->getLogLevel($context); + $minLevel = $this->getLogLevel($context, $message); array_walk($context, [$this->normalizer, 'format']); diff --git a/lib/private/SystemConfig.php b/lib/private/SystemConfig.php index 310433474ec7a..f817e327b194b 100644 --- a/lib/private/SystemConfig.php +++ b/lib/private/SystemConfig.php @@ -50,6 +50,7 @@ class SystemConfig { 'github.client_secret' => true, 'log.condition' => [ 'shared_secret' => true, + 'matches' => true, ], 'license-key' => true, 'redis' => [ diff --git a/tests/lib/LoggerTest.php b/tests/lib/LoggerTest.php index e3a51d9380455..6343ce6206501 100644 --- a/tests/lib/LoggerTest.php +++ b/tests/lib/LoggerTest.php @@ -10,6 +10,8 @@ use OC\Log; use OC\SystemConfig; use OCP\ILogger; +use OCP\IUser; +use OCP\IUserSession; use OCP\Log\IWriter; use OCP\Support\CrashReport\IRegistry; use PHPUnit\Framework\MockObject\MockObject; @@ -73,6 +75,94 @@ public function testAppCondition() { $this->assertEquals($expected, $this->getLogs()); } + public function dataMatchesCondition(): array { + return [ + [ + 'user0', + [ + 'apps' => ['app2'], + ], + [ + '1 Info of app2', + ], + ], + [ + 'user2', + [ + 'users' => ['user1', 'user2'], + 'apps' => ['app1'], + ], + [ + '1 Info of app1', + ], + ], + [ + 'user3', + [ + 'users' => ['user3'], + ], + [ + '1 Info without app', + '1 Info of app1', + '1 Info of app2', + '0 Debug of app3', + ], + ], + [ + 'user4', + [ + 'users' => ['user4'], + 'apps' => ['app3'], + 'loglevel' => 0, + ], + [ + '0 Debug of app3', + ], + ], + [ + 'user4', + [ + 'message' => ' of ', + ], + [ + '1 Info of app1', + '1 Info of app2', + '0 Debug of app3', + ], + ], + ]; + } + + /** + * @dataProvider dataMatchesCondition + */ + public function testMatchesCondition(string $userId, array $conditions, array $expectedLogs): void { + $this->config->expects($this->any()) + ->method('getValue') + ->willReturnMap([ + ['loglevel', ILogger::WARN, ILogger::WARN], + ['log.condition', [], ['matches' => [ + $conditions, + ]]], + ]); + $logger = $this->logger; + + $user = $this->createMock(IUser::class); + $user->method('getUID') + ->willReturn($userId); + $userSession = $this->createMock(IUserSession::class); + $userSession->method('getUser') + ->willReturn($user); + $this->overwriteService(IUserSession::class, $userSession); + + $logger->info('Info without app'); + $logger->info('Info of app1', ['app' => 'app1']); + $logger->info('Info of app2', ['app' => 'app2']); + $logger->debug('Debug of app3', ['app' => 'app3']); + + $this->assertEquals($expectedLogs, $this->getLogs()); + } + public function testLoggingWithDataArray(): void { $this->mockDefaultLogLevel(); $writerMock = $this->createMock(IWriter::class);