From 333388912adcfd209431e2e96b7084cbf1c29c83 Mon Sep 17 00:00:00 2001 From: Dan Garner Date: Mon, 12 Aug 2024 19:15:49 +0100 Subject: [PATCH] Logging: fix log process to ensure we record page/method (#2702) Ensure we record page/method as early as possible, adding in user/session later. fixes xibosignageltd/xibo-private#825 --- lib/Helper/AccessibleMonologWriter.php | 39 -------------- lib/Helper/RouteLogProcessor.php | 54 +++++++++++++++++++ ...{LogProcessor.php => UserLogProcessor.php} | 23 +++----- lib/Middleware/ApiAuthorization.php | 9 ++++ lib/Middleware/AuthenticationTrait.php | 11 +++- lib/Middleware/Log.php | 36 +++++-------- lib/Service/LogService.php | 6 +-- lib/Service/LogServiceInterface.php | 6 ++- web/api/index.php | 2 +- web/index.php | 2 +- 10 files changed, 104 insertions(+), 84 deletions(-) delete mode 100644 lib/Helper/AccessibleMonologWriter.php create mode 100644 lib/Helper/RouteLogProcessor.php rename lib/Helper/{LogProcessor.php => UserLogProcessor.php} (75%) diff --git a/lib/Helper/AccessibleMonologWriter.php b/lib/Helper/AccessibleMonologWriter.php deleted file mode 100644 index aa9786e8f2..0000000000 --- a/lib/Helper/AccessibleMonologWriter.php +++ /dev/null @@ -1,39 +0,0 @@ -resource; - } - - public function addHandler($handler) { - if (!$this->resource) - $this->settings['handlers'][] = $handler; - else - $this->getWriter()->pushHandler($handler); - } - - public function addProcessor($processor) { - if (!$this->resource) - $this->settings['processors'][] = $processor; - else - $this->getWriter()->pushProcessor($processor); - } -} \ No newline at end of file diff --git a/lib/Helper/RouteLogProcessor.php b/lib/Helper/RouteLogProcessor.php new file mode 100644 index 0000000000..cf2aeeea2a --- /dev/null +++ b/lib/Helper/RouteLogProcessor.php @@ -0,0 +1,54 @@ +. + */ + + +namespace Xibo\Helper; + +/** + * Class RouteLogProcessor + * a process to add route/method information to the log record + * @package Xibo\Helper + */ +class RouteLogProcessor +{ + /** + * Log Processor + * @param string $route + * @param string $method + */ + public function __construct( + private readonly string $route, + private readonly string $method + ) { + } + + /** + * @param array $record + * @return array + */ + public function __invoke(array $record): array + { + $record['extra']['method'] = $this->method; + $record['extra']['route'] = $this->route; + return $record; + } +} diff --git a/lib/Helper/LogProcessor.php b/lib/Helper/UserLogProcessor.php similarity index 75% rename from lib/Helper/LogProcessor.php rename to lib/Helper/UserLogProcessor.php index 97b3b3cf99..dee8b66b52 100644 --- a/lib/Helper/LogProcessor.php +++ b/lib/Helper/UserLogProcessor.php @@ -24,23 +24,19 @@ namespace Xibo\Helper; /** - * Class LogProcessor + * Class UserLogProcessor * @package Xibo\Helper */ -class LogProcessor +class UserLogProcessor { /** - * Log Processor - * @param string $route - * @param string $method - * @param int|null $userId + * UserLogProcessor + * @param int $userId * @param int|null $sessionHistoryId * @param int|null $requestId */ public function __construct( - private readonly string $route, - private readonly string $method, - private readonly ?int $userId, + private readonly int $userId, private readonly ?int $sessionHistoryId, private readonly ?int $requestId ) { @@ -52,12 +48,7 @@ public function __construct( */ public function __invoke(array $record): array { - $record['extra']['method'] = $this->method; - $record['extra']['route'] = $this->route; - - if ($this->userId != null) { - $record['extra']['userId'] = $this->userId; - } + $record['extra']['userId'] = $this->userId; if ($this->sessionHistoryId != null) { $record['extra']['sessionHistoryId'] = $this->sessionHistoryId; @@ -69,4 +60,4 @@ public function __invoke(array $record): array return $record; } -} \ No newline at end of file +} diff --git a/lib/Middleware/ApiAuthorization.php b/lib/Middleware/ApiAuthorization.php index 075c6ba839..0e9fd98607 100644 --- a/lib/Middleware/ApiAuthorization.php +++ b/lib/Middleware/ApiAuthorization.php @@ -32,6 +32,7 @@ use Slim\App as App; use Slim\Routing\RouteContext; use Xibo\Factory\ApplicationScopeFactory; +use Xibo\Helper\UserLogProcessor; use Xibo\OAuth\AccessTokenRepository; use Xibo\Support\Exception\AccessDeniedException; @@ -204,6 +205,14 @@ public function process(Request $request, RequestHandler $handler): Response $logger->setUserId($user->userId); $this->app->getContainer()->set('user', $user); $logger->setRequestId($requestId); + + // Add this request information to the logger. + $logger->getLoggerInterface()->pushProcessor(new UserLogProcessor( + $user->userId, + null, + $requestId, + )); + return $handler->handle($validatedRequest->withAttribute('name', 'API')); } } diff --git a/lib/Middleware/AuthenticationTrait.php b/lib/Middleware/AuthenticationTrait.php index dd3ac24c4f..72d2d1972c 100644 --- a/lib/Middleware/AuthenticationTrait.php +++ b/lib/Middleware/AuthenticationTrait.php @@ -29,6 +29,7 @@ use Slim\Routing\RouteContext; use Xibo\Entity\User; use Xibo\Helper\HttpsDetect; +use Xibo\Helper\UserLogProcessor; /** * Trait AuthenticationTrait @@ -143,7 +144,15 @@ protected function getUser($userId, $ip, $sessionHistoryId): User */ protected function setUserForRequest($user) { - $this->app->getContainer()->set('user', $user); + $container = $this->app->getContainer(); + $container->set('user', $user); + + // Add this users information to the logger + $this->getLog()->getLoggerInterface()->pushProcessor(new UserLogProcessor( + $user->userId, + $this->getLog()->getSessionHistoryId(), + null, + )); } /** diff --git a/lib/Middleware/Log.php b/lib/Middleware/Log.php index 5760260bc8..8c2462b586 100644 --- a/lib/Middleware/Log.php +++ b/lib/Middleware/Log.php @@ -28,13 +28,18 @@ use Psr\Http\Server\RequestHandlerInterface as RequestHandler; use Psr\Log\LoggerInterface; use Slim\App as App; -use Xibo\Helper\LogProcessor; +use Xibo\Helper\RouteLogProcessor; +/** + * Log Middleware + */ class Log implements Middleware { - /* @var App $app */ - private $app; + private App $app; + /** + * @param $app + */ public function __construct($app) { $this->app = $app; @@ -44,18 +49,14 @@ public function __construct($app) * @param Request $request * @param RequestHandler $handler * @return Response + * @throws \Psr\Container\ContainerExceptionInterface + * @throws \Psr\Container\NotFoundExceptionInterface */ public function process(Request $request, RequestHandler $handler): Response { $container = $this->app->getContainer(); - self::addLogProcessorToLogger( - $container->get('logger'), - $request, - $container->get('logService')->getUserId(), - $container->get('logService')->getSessionHistoryId(), - $container->get('logService')->getRequestId(), - ); + self::addLogProcessorToLogger($container->get('logger'), $request); return $handler->handle($request); } @@ -63,23 +64,14 @@ public function process(Request $request, RequestHandler $handler): Response /** * @param LoggerInterface $logger * @param \Psr\Http\Message\ServerRequestInterface $request - * @param ?int $userId - * @param ?int $sessionHistoryId */ public static function addLogProcessorToLogger( LoggerInterface $logger, Request $request, - ?int $userId = null, - ?int $sessionHistoryId = null, - ?int $requestId = null - ) { - $logHelper = new LogProcessor( + ): void { + $logger->pushProcessor(new RouteLogProcessor( $request->getUri()->getPath(), $request->getMethod(), - $userId, - $sessionHistoryId, - $requestId - ); - $logger->pushProcessor($logHelper); + )); } } diff --git a/lib/Service/LogService.php b/lib/Service/LogService.php index 4715edebaa..f11386900c 100644 --- a/lib/Service/LogService.php +++ b/lib/Service/LogService.php @@ -117,17 +117,17 @@ public function setRequestId($requestId) $this->requestId = $requestId; } - public function getUserId(): int + public function getUserId(): ?int { return $this->userId; } - public function getSessionHistoryId() + public function getSessionHistoryId(): ?int { return $this->sessionHistoryId; } - public function getRequestId() + public function getRequestId(): ?int { return $this->requestId; } diff --git a/lib/Service/LogServiceInterface.php b/lib/Service/LogServiceInterface.php index fc4143766b..6ecd6badd7 100644 --- a/lib/Service/LogServiceInterface.php +++ b/lib/Service/LogServiceInterface.php @@ -45,6 +45,10 @@ public function __construct($logger, $mode = 'production'); */ public function getLoggerInterface(): LoggerInterface; + public function getUserId(): ?int; + public function getSessionHistoryId(): ?int; + public function getRequestId(): ?int; + /** * Set the user Id * @param int $userId @@ -155,4 +159,4 @@ public static function resolveLogLevel($level); * @param $level */ public function setLevel($level); -} \ No newline at end of file +} diff --git a/web/api/index.php b/web/api/index.php index 8aaa330bee..1b2b009eb9 100644 --- a/web/api/index.php +++ b/web/api/index.php @@ -69,9 +69,9 @@ $app->add(new \Xibo\Middleware\ConnectorMiddleware($app)); $app->add(new \Xibo\Middleware\ListenersMiddleware($app)); -$app->add(new \Xibo\Middleware\Log($app)); $app->add(new \Xibo\Middleware\ApiAuthorization($app)); $app->add(new \Xibo\Middleware\State($app)); +$app->add(new \Xibo\Middleware\Log($app)); $app->add(new \Xibo\Middleware\Storage($app)); $app->add(new \Xibo\Middleware\Xmr($app)); $app->addRoutingMiddleware(); diff --git a/web/index.php b/web/index.php index 26e10012ad..2c7ca8e570 100644 --- a/web/index.php +++ b/web/index.php @@ -95,7 +95,6 @@ $app->add(new \Xibo\Middleware\Theme($app)); $app->add(new \Xibo\Middleware\CsrfGuard($app)); $app->add(new \Xibo\Middleware\Csp($container)); -$app->add(new \Xibo\Middleware\Log($app)); // Authentication $authentication = ($container->get('configService')->authentication != null) @@ -109,6 +108,7 @@ // TODO reconfigure this and enable //$app->add(new Xibo\Middleware\HttpCache()); $app->add(new \Xibo\Middleware\State($app)); +$app->add(new \Xibo\Middleware\Log($app)); $app->add(TwigMiddleware::createFromContainer($app)); $app->add(new \Xibo\Middleware\Storage($app)); $app->add(new \Xibo\Middleware\Xmr($app));