Skip to content

Commit af25b5a

Browse files
nickvergessennickvergessen
committed
feat(log): Allow to combine log.conditions to only log (app&user)
Signed-off-by: Joas Schilling <coding@schilljs.com>
1 parent 7bc4ccb commit af25b5a

5 files changed

Lines changed: 173 additions & 26 deletions

File tree

‎config/config.sample.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
66
* SPDX-License-Identifier: AGPL-3.0-only
77
*/
8-
8+
99
/**
1010
* This configuration file is only provided to document the different
1111
* configuration options and their usage.
@@ -1048,13 +1048,25 @@
10481048
* this condition is met
10491049
* - ``apps``: if the log message is invoked by one of the specified apps,
10501050
* this condition is met
1051+
* - ``matches``: if all the conditions inside a group match,
1052+
* this condition is met. This allows to log only entries to an app
1053+
* by a few users.
10511054
*
10521055
* Defaults to an empty array.
10531056
*/
10541057
'log.condition' => [
10551058
'shared_secret' => '57b58edb6637fe3059b3595cf9c41b9',
10561059
'users' => ['sample-user'],
10571060
'apps' => ['files'],
1061+
'matches' => [
1062+
[
1063+
'shared_secret' => '57b58edb6637fe3059b3595cf9c41b9',
1064+
'users' => ['sample-user'],
1065+
'apps' => ['files'],
1066+
'loglevel' => 1,
1067+
'message' => 'contains substring'
1068+
],
1069+
],
10581070
],
10591071

10601072
/**

‎lib/private/Diagnostics/EventLogger.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public function isLoggingActivated(): bool {
6666
return true;
6767
}
6868

69-
$isDebugLevel = $this->internalLogger->getLogLevel([]) === Log::DEBUG;
69+
$isDebugLevel = $this->internalLogger->getLogLevel([], '') === Log::DEBUG;
7070
return $systemValue && $isDebugLevel;
7171
}
7272

‎lib/private/Log.php‎

Lines changed: 68 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
use OC\Log\ExceptionSerializer;
4343
use OCP\EventDispatcher\IEventDispatcher;
4444
use OCP\ILogger;
45+
use OCP\IRequest;
4546
use OCP\IUserSession;
4647
use OCP\Log\BeforeMessageLoggedEvent;
4748
use OCP\Log\IDataLogger;
@@ -181,7 +182,7 @@ public function debug(string $message, array $context = []): void {
181182
* @param array $context
182183
*/
183184
public function log(int $level, string $message, array $context = []): void {
184-
$minLevel = $this->getLogLevel($context);
185+
$minLevel = $this->getLogLevel($context, $message);
185186
if ($level < $minLevel
186187
&& (($this->crashReporters?->hasReporters() ?? false) === false)
187188
&& (($this->eventDispatcher?->hasListeners(BeforeMessageLoggedEvent::class) ?? false) === false)) {
@@ -222,9 +223,25 @@ public function log(int $level, string $message, array $context = []): void {
222223
}
223224
}
224225

225-
public function getLogLevel($context): int {
226+
public function getLogLevel(array $context, string $message): int {
227+
/**
228+
* @psalm-var array{
229+
* shared_secret?: string,
230+
* users?: string[],
231+
* apps?: string[],
232+
* matches?: array<array-key, array{
233+
* shared_secret?: string,
234+
* users?: string[],
235+
* apps?: string[],
236+
* message?: string,
237+
* loglevel: 0|1|2|3|4,
238+
* }>
239+
* } $logCondition
240+
*/
226241
$logCondition = $this->config->getValue('log.condition', []);
227242

243+
$userId = false;
244+
228245
/**
229246
* check for a special log condition - this enables an increased log on
230247
* a per request/user base
@@ -234,21 +251,8 @@ public function getLogLevel($context): int {
234251
$this->logConditionSatisfied = false;
235252
if (!empty($logCondition)) {
236253
// check for secret token in the request
237-
if (isset($logCondition['shared_secret'])) {
238-
$request = \OC::$server->getRequest();
239-
240-
if ($request->getMethod() === 'PUT' &&
241-
!str_contains($request->getHeader('Content-Type'), 'application/x-www-form-urlencoded') &&
242-
!str_contains($request->getHeader('Content-Type'), 'application/json')) {
243-
$logSecretRequest = '';
244-
} else {
245-
$logSecretRequest = $request->getParam('log_secret', '');
246-
}
247-
248-
// if token is found in the request change set the log condition to satisfied
249-
if ($request && hash_equals($logCondition['shared_secret'], $logSecretRequest)) {
250-
$this->logConditionSatisfied = true;
251-
}
254+
if (isset($logCondition['shared_secret']) && $this->checkLogSecret($logCondition['shared_secret'])) {
255+
$this->logConditionSatisfied = true;
252256
}
253257

254258
// check for user
@@ -261,6 +265,8 @@ public function getLogLevel($context): int {
261265
} elseif (in_array($user->getUID(), $logCondition['users'], true)) {
262266
// if the user matches set the log condition to satisfied
263267
$this->logConditionSatisfied = true;
268+
} else {
269+
$userId = $user->getUID();
264270
}
265271
}
266272
}
@@ -271,6 +277,11 @@ public function getLogLevel($context): int {
271277
return ILogger::DEBUG;
272278
}
273279

280+
if ($userId === false && isset($logCondition['matches'])) {
281+
$user = \OCP\Server::get(IUserSession::class)->getUser();
282+
$userId = $user === null ? false : $user->getUID();
283+
}
284+
274285
if (isset($context['app'])) {
275286
/**
276287
* check log condition based on the context of each log message
@@ -281,16 +292,49 @@ public function getLogLevel($context): int {
281292
}
282293
}
283294

284-
$configLogLevel = $this->config->getValue('loglevel', ILogger::WARN);
285-
if (is_numeric($configLogLevel)) {
286-
return min((int)$configLogLevel, ILogger::FATAL);
295+
if (isset($logCondition['matches'])) {
296+
$configLogLevel = $this->config->getValue('loglevel', ILogger::WARN);
297+
if (is_numeric($configLogLevel)) {
298+
return min((int)$configLogLevel, ILogger::FATAL);
299+
}
300+
301+
// Invalid configuration, warn the user and fall back to default level of WARN
302+
error_log('Nextcloud configuration: "loglevel" is not a valid integer');
303+
return ILogger::WARN;
304+
}
305+
306+
foreach ($logCondition['matches'] as $option) {
307+
if (
308+
(!isset($option['shared_secret']) || $this->checkLogSecret($option['shared_secret']))
309+
&& (!isset($option['users']) || in_array($userId, $option['users'], true))
310+
&& (!isset($option['apps']) || (isset($context['app']) && in_array($context['app'], $option['apps'], true)))
311+
&& (!isset($option['message']) || str_contains($message, $option['message']))
312+
) {
313+
if (!isset($option['apps']) && !isset($option['loglevel']) && !isset($option['message'])) {
314+
/* Only user and/or secret are listed as conditions, we can cache the result for the rest of the request */
315+
$this->logConditionSatisfied = true;
316+
return ILogger::DEBUG;
317+
}
318+
return $option['loglevel'] ?? ILogger::DEBUG;
319+
}
287320
}
288321

289-
// Invalid configuration, warn the user and fall back to default level of WARN
290-
error_log('Nextcloud configuration: "loglevel" is not a valid integer');
291322
return ILogger::WARN;
292323
}
293324

325+
protected function checkLogSecret(string $conditionSecret): bool {
326+
$request = \OCP\Server::get(IRequest::class);
327+
328+
if ($request->getMethod() === 'PUT' &&
329+
!str_contains($request->getHeader('Content-Type'), 'application/x-www-form-urlencoded') &&
330+
!str_contains($request->getHeader('Content-Type'), 'application/json')) {
331+
return hash_equals($conditionSecret, '');
332+
}
333+
334+
// if token is found in the request change set the log condition to satisfied
335+
return hash_equals($conditionSecret, $request->getParam('log_secret', ''));
336+
}
337+
294338
/**
295339
* Logs an exception very detailed
296340
*
@@ -303,7 +347,7 @@ public function logException(Throwable $exception, array $context = []): void {
303347
$app = $context['app'] ?? 'no app in context';
304348
$level = $context['level'] ?? ILogger::ERROR;
305349

306-
$minLevel = $this->getLogLevel($context);
350+
$minLevel = $this->getLogLevel($context, $context['message'] ?? $exception->getMessage());
307351
if ($level < $minLevel
308352
&& (($this->crashReporters?->hasReporters() ?? false) === false)
309353
&& (($this->eventDispatcher?->hasListeners(BeforeMessageLoggedEvent::class) ?? false) === false)) {
@@ -348,7 +392,7 @@ public function logData(string $message, array $data, array $context = []): void
348392
$app = $context['app'] ?? 'no app in context';
349393
$level = $context['level'] ?? ILogger::ERROR;
350394

351-
$minLevel = $this->getLogLevel($context);
395+
$minLevel = $this->getLogLevel($context, $message);
352396

353397
array_walk($context, [$this->normalizer, 'format']);
354398

‎lib/private/SystemConfig.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ class SystemConfig {
6767
'github.client_secret' => true,
6868
'log.condition' => [
6969
'shared_secret' => true,
70+
'matches' => true,
7071
],
7172
'license-key' => true,
7273
'redis' => [

‎tests/lib/LoggerTest.php‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
use OC\Log;
1111
use OC\SystemConfig;
1212
use OCP\ILogger;
13+
use OCP\IUser;
14+
use OCP\IUserSession;
1315
use OCP\Log\IWriter;
1416
use OCP\Support\CrashReport\IRegistry;
1517
use PHPUnit\Framework\MockObject\MockObject;
@@ -64,6 +66,94 @@ public function testAppCondition() {
6466
$this->assertEquals($expected, $this->getLogs());
6567
}
6668

69+
public function dataMatchesCondition(): array {
70+
return [
71+
[
72+
'user0',
73+
[
74+
'apps' => ['app2'],
75+
],
76+
[
77+
'1 Info of app2',
78+
],
79+
],
80+
[
81+
'user2',
82+
[
83+
'users' => ['user1', 'user2'],
84+
'apps' => ['app1'],
85+
],
86+
[
87+
'1 Info of app1',
88+
],
89+
],
90+
[
91+
'user3',
92+
[
93+
'users' => ['user3'],
94+
],
95+
[
96+
'1 Info without app',
97+
'1 Info of app1',
98+
'1 Info of app2',
99+
'0 Debug of app3',
100+
],
101+
],
102+
[
103+
'user4',
104+
[
105+
'users' => ['user4'],
106+
'apps' => ['app3'],
107+
'loglevel' => 0,
108+
],
109+
[
110+
'0 Debug of app3',
111+
],
112+
],
113+
[
114+
'user4',
115+
[
116+
'message' => ' of ',
117+
],
118+
[
119+
'1 Info of app1',
120+
'1 Info of app2',
121+
'0 Debug of app3',
122+
],
123+
],
124+
];
125+
}
126+
127+
/**
128+
* @dataProvider dataMatchesCondition
129+
*/
130+
public function testMatchesCondition(string $userId, array $conditions, array $expectedLogs): void {
131+
$this->config->expects($this->any())
132+
->method('getValue')
133+
->willReturnMap([
134+
['loglevel', ILogger::WARN, ILogger::WARN],
135+
['log.condition', [], ['matches' => [
136+
$conditions,
137+
]]],
138+
]);
139+
$logger = $this->logger;
140+
141+
$user = $this->createMock(IUser::class);
142+
$user->method('getUID')
143+
->willReturn($userId);
144+
$userSession = $this->createMock(IUserSession::class);
145+
$userSession->method('getUser')
146+
->willReturn($user);
147+
$this->overwriteService(IUserSession::class, $userSession);
148+
149+
$logger->info('Info without app');
150+
$logger->info('Info of app1', ['app' => 'app1']);
151+
$logger->info('Info of app2', ['app' => 'app2']);
152+
$logger->debug('Debug of app3', ['app' => 'app3']);
153+
154+
$this->assertEquals($expectedLogs, $this->getLogs());
155+
}
156+
67157
public function testLoggingWithDataArray(): void {
68158
$writerMock = $this->createMock(IWriter::class);
69159
$logFile = new Log($writerMock, $this->config);

0 commit comments

Comments
 (0)