diff --git a/apps/admin_audit/lib/Actions/Action.php b/apps/admin_audit/lib/Actions/Action.php index acd415d82ea9e..e94b27e63e5ed 100644 --- a/apps/admin_audit/lib/Actions/Action.php +++ b/apps/admin_audit/lib/Actions/Action.php @@ -2,65 +2,98 @@ declare(strict_types=1); /** - * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors + * SPDX-FileCopyrightText: 2016-2026 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ namespace OCA\AdminAudit\Actions; use OCA\AdminAudit\IAuditLogger; +/** + * Base class for audit logging actions + * + * Provides structured logging for admin audit events with type-safe parameter handling + */ class Action { - public function __construct( - private IAuditLogger $logger, + private readonly IAuditLogger $logger, ) { } /** - * Log a single action with a log level of info + * Log a single action with a log level of info. * - * @param string $text - * @param array $params - * @param array $elements - * @param bool $obfuscateParameters + * Example usage: + * $this->log( + * 'User "%s" added to group "%s"', + * ['user' => 'alice', 'group' => 'admins'], + * ['user', 'group'] + * ); + * + * @param string $messageTemplate Format string for vsprintf (e.g., "User %s deleted file %s") + * @param array $data Associative array of data values + * @param array $requiredKeys Array of keys that must exist in $data; order must match format placeholders + * @param bool $excludeSensitiveData If true, omit parameter details from error logs */ - public function log(string $text, - array $params, - array $elements, - bool $obfuscateParameters = false): void { - foreach ($elements as $element) { - if (!isset($params[$element])) { - if ($obfuscateParameters) { - $this->logger->critical( - '$params["' . $element . '"] was missing.', - ['app' => 'admin_audit'] - ); - } else { - $this->logger->critical( - '$params["' . $element . '"] was missing. Transferred value: {params}', - ['app' => 'admin_audit', 'params' => $params] - ); - } - return; + public function log( + string $messageTemplate, + array $data, + array $requiredKeys, + bool $excludeSensitiveData = false + ): void { + $missingKeys = []; + foreach ($requiredKeys as $key) { + if (!isset($data[$key])) { + $missingKeys[] = $key; } } - $replaceArray = []; - foreach ($elements as $element) { - if ($params[$element] instanceof \DateTime) { - $params[$element] = $params[$element]->format('Y-m-d H:i:s'); + if (!empty($missingKeys)) { + $context = ['app' => 'admin_audit', 'missing_keys' => $missingKeys]; + + if (!$excludeSensitiveData) { + $context['provided_keys'] = array_keys($data); } - $replaceArray[] = $params[$element]; + + $this->logger->critical( + 'Required audit parameters missing: {missing_keys}', + $context + ); + return; } - $this->logger->info( - vsprintf( - $text, - $replaceArray - ), - [ - 'app' => 'admin_audit' - ] - ); + $replacementValues = []; + foreach ($requiredKeys as $key) { + $value = $data[$key]; + + // Handle different types safely + if ($value instanceof \DateTime) { + $replacementValues[] = $value->format('Y-m-d H:i:s'); + } elseif (is_bool($value)) { + $replacementValues[] = $value ? 'true' : 'false'; + } elseif (is_scalar($value)) { + $replacementValues[] = (string)$value; + } elseif ($value === null) { + $replacementValues[] = 'null'; + } else { + $replacementValues[] = json_encode($value, JSON_UNESCAPED_SLASHES) ?: gettype($value); + } + } + + try { + $message = vsprintf($messageTemplate, $replacementValues); + $this->logger->info($message, ['app' => 'admin_audit']); + } catch (\ValueError $e) { + // vsprintf throws ValueError in PHP 8+ when format/argument mismatch occurs + $this->logger->critical( + 'Audit log format string mismatch: {error}', + [ + 'app' => 'admin_audit', + 'error' => $e->getMessage(), + 'format' => $messageTemplate, + 'element_count' => count($requiredKeys) + ] + ); + } } } diff --git a/apps/admin_audit/tests/Actions/ActionTest.php b/apps/admin_audit/tests/Actions/ActionTest.php new file mode 100644 index 0000000000000..640c7088d8ec2 --- /dev/null +++ b/apps/admin_audit/tests/Actions/ActionTest.php @@ -0,0 +1,113 @@ +logger = $this->createMock(IAuditLogger:: class); + $this->action = new Action($this->logger); + } + + public function testLogWithBooleanTrue(): void { + $this->logger->expects($this->once()) + ->method('info') + ->with('Setting enabled: true', ['app' => 'admin_audit']); + + $this->action->log( + 'Setting enabled: %s', + ['enabled' => true], + ['enabled'] + ); + } + + public function testLogWithBooleanFalse(): void { + $this->logger->expects($this->once()) + ->method('info') + ->with('Setting enabled: false', ['app' => 'admin_audit']); + + $this->action->log( + 'Setting enabled: %s', + ['enabled' => false], + ['enabled'] + ); + } + + public function testLogWithNull(): void { + $this->logger->expects($this->once()) + ->method('info') + ->with('Value is: null', ['app' => 'admin_audit']); + + $this->action->log( + 'Value is: %s', + ['value' => null], + ['value'] + ); + } + + public function testLogWithMissingKey(): void { + $this->logger->expects($this->once()) + ->method('critical') + ->with( + 'Required audit parameters missing: {missing_keys}', + $this->callback(function ($context) { + return $context['app'] === 'admin_audit' + && $context['missing_keys'] === ['missing_key'] + && isset($context['provided_keys']); + }) + ); + + $this->action->log( + 'Value: %s', + ['other_key' => 'value'], + ['missing_key'] + ); + } + + public function testLogWithDateTimeValue(): void { + $date = new \DateTime('2026-01-02 15:30:45'); + + $this->logger->expects($this->once()) + ->method('info') + ->with('Date: 2026-01-02 15:30:45', ['app' => 'admin_audit']); + + $this->action->log( + 'Date: %s', + ['date' => $date], + ['date'] + ); + } + + public function testLogWithFormatMismatch(): void { + $this->logger->expects($this->once()) + ->method('critical') + ->with( + 'Audit log format string mismatch: {error}', + $this->callback(function ($context) { + return $context['app'] === 'admin_audit' + && isset($context['error']) + && $context['format'] === 'Too many: %s %s %s'; + }) + ); + + $this->action->log( + 'Too many: %s %s %s', + ['one' => '1', 'two' => '2'], + ['one', 'two'] // Only 2 values for 3 placeholders + ); + } +}