Skip to content

Commit

Permalink
Logging: fix log process to ensure we record page/method (#2702)
Browse files Browse the repository at this point in the history
Ensure we record page/method as early as possible, adding in user/session later.
fixes xibosignageltd/xibo-private#825
  • Loading branch information
dasgarner authored Aug 12, 2024
1 parent 6286621 commit 3333889
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 84 deletions.
39 changes: 0 additions & 39 deletions lib/Helper/AccessibleMonologWriter.php

This file was deleted.

54 changes: 54 additions & 0 deletions lib/Helper/RouteLogProcessor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php
/*
* Copyright (C) 2024 Xibo Signage Ltd
*
* Xibo - Digital Signage - https://xibosignage.com
*
* This file is part of Xibo.
*
* Xibo is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* any later version.
*
* Xibo is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with Xibo. If not, see <http://www.gnu.org/licenses/>.
*/


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;
}
}
23 changes: 7 additions & 16 deletions lib/Helper/LogProcessor.php → lib/Helper/UserLogProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
) {
Expand All @@ -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;
Expand All @@ -69,4 +60,4 @@ public function __invoke(array $record): array

return $record;
}
}
}
9 changes: 9 additions & 0 deletions lib/Middleware/ApiAuthorization.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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'));
}
}
11 changes: 10 additions & 1 deletion lib/Middleware/AuthenticationTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use Slim\Routing\RouteContext;
use Xibo\Entity\User;
use Xibo\Helper\HttpsDetect;
use Xibo\Helper\UserLogProcessor;

/**
* Trait AuthenticationTrait
Expand Down Expand Up @@ -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,
));
}

/**
Expand Down
36 changes: 14 additions & 22 deletions lib/Middleware/Log.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -44,42 +49,29 @@ 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);
}

/**
* @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);
));
}
}
6 changes: 3 additions & 3 deletions lib/Service/LogService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 5 additions & 1 deletion lib/Service/LogServiceInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -155,4 +159,4 @@ public static function resolveLogLevel($level);
* @param $level
*/
public function setLevel($level);
}
}
2 changes: 1 addition & 1 deletion web/api/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion web/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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));
Expand Down

0 comments on commit 3333889

Please sign in to comment.