From f073a2014d71ced5964fda02ae95086a9f0abed9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Thu, 24 Sep 2026 15:36:52 +0200 Subject: [PATCH] fix: keep the log processors working on a stateless firewall --- src/Logging/InjectUserProcessor.php | 28 ++++++-- src/Logging/UserDataProcessor.php | 21 ++++-- tests/Logging/InjectUserProcessorTest.php | 83 +++++++++++++++++++++++ tests/Logging/UserDataProcessorTest.php | 41 +++++++++++ 4 files changed, 163 insertions(+), 10 deletions(-) create mode 100644 tests/Logging/InjectUserProcessorTest.php create mode 100644 tests/Logging/UserDataProcessorTest.php diff --git a/src/Logging/InjectUserProcessor.php b/src/Logging/InjectUserProcessor.php index 966a26a..1c387c7 100644 --- a/src/Logging/InjectUserProcessor.php +++ b/src/Logging/InjectUserProcessor.php @@ -8,8 +8,15 @@ use Symfony\Bundle\SecurityBundle\Security; class InjectUserProcessor { - public function __construct(private readonly Security $security) - { + /** + * The SAPI is injected rather than read from PHP_SAPI inside the processor so a test can + * exercise the web path: under PHPUnit php_sapi_name() is always "cli", which made the + * branch below unreachable from every functional test. + */ + public function __construct( + private readonly Security $security, + private readonly string $sapi = \PHP_SAPI, + ) { } public function __invoke(LogRecord $record): LogRecord @@ -18,10 +25,9 @@ class InjectUserProcessor return $record; } - /** @var User $user */ $user = $this->security->getUser(); - if (null === $user || 'cli' === php_sapi_name()) { + if (null === $user || 'cli' === $this->sapi) { $record->extra['user'] = [ 'id' => null, 'username' => 'system', @@ -31,6 +37,20 @@ class InjectUserProcessor return $record; } + // Not every firewall authenticates a row in the user table: the stateless "api" firewall + // resolves its shared key to an in-memory user, which has an identifier and roles and + // nothing else. Reaching for getId() or getEmail() there is a 500 on a request that had + // already stored its data. + if (false === $user instanceof User) { + $record->extra['user'] = [ + 'id' => null, + 'username' => $user->getUserIdentifier(), + 'roles' => $user->getRoles(), + ]; + + return $record; + } + $record->extra['user'] = [ 'id' => $user->getId(), 'email' => $user->getEmail(), diff --git a/src/Logging/UserDataProcessor.php b/src/Logging/UserDataProcessor.php index 55277ee..74dc67c 100644 --- a/src/Logging/UserDataProcessor.php +++ b/src/Logging/UserDataProcessor.php @@ -2,7 +2,7 @@ namespace App\Logging; -use App\Entity\Core\User; +use App\Entity\User; use Monolog\Attribute\AsMonologProcessor; use Monolog\LogRecord; use Symfony\Bundle\SecurityBundle\Security; @@ -11,8 +11,15 @@ use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; #[AsMonologProcessor] class UserDataProcessor { - public function __construct(private readonly Security $security) - { + /** + * The SAPI is injected rather than read from PHP_SAPI inside the processor so a test can + * exercise the web path: under PHPUnit php_sapi_name() is always "cli", which made + * everything below the check unreachable from every functional test. + */ + public function __construct( + private readonly Security $security, + private readonly string $sapi = \PHP_SAPI, + ) { } public function __invoke(LogRecord $record): LogRecord @@ -21,7 +28,7 @@ class UserDataProcessor return $record; } - if ('cli' === php_sapi_name()) { + if ('cli' === $this->sapi) { $record->extra['user'] = [ 'id' => null, 'username' => 'SYSTEM', @@ -31,7 +38,6 @@ class UserDataProcessor return $record; } - /** @var User $user */ $user = $this->security->getUser(); if (null === $user) { @@ -57,7 +63,10 @@ class UserDataProcessor $username = $user->getUserIdentifier(); } $record->extra['user'] = [ - 'id' => $user->getId(), + // Only a user table row has an id: the stateless "api" firewall resolves its shared + // key to an in-memory user, and calling getId() on that is a 500 on a request that + // had already stored its data. + 'id' => $user instanceof User ? $user->getId() : null, 'username' => $username, 'roles' => $user->getRoles(), ]; diff --git a/tests/Logging/InjectUserProcessorTest.php b/tests/Logging/InjectUserProcessorTest.php new file mode 100644 index 0000000..7e5332e --- /dev/null +++ b/tests/Logging/InjectUserProcessorTest.php @@ -0,0 +1,83 @@ +process(new InMemoryUser('api', null, ['ROLE_API'])); + + $this->assertSame( + ['id' => null, 'username' => 'api', 'roles' => ['ROLE_API']], + $record->extra['user'] + ); + } + + public function testAnAnonymousRequestIsLoggedAsSystem(): void + { + $record = $this->process(null); + + $this->assertSame('system', $record->extra['user']['username']); + } + + public function testTheConsoleIsLoggedAsSystemEvenWithAUser(): void + { + $record = $this->process(new InMemoryUser('api', null, ['ROLE_API']), sapi: 'cli'); + + $this->assertSame('system', $record->extra['user']['username']); + } + + public function testAnAlreadyResolvedUserIsLeftAlone(): void + { + $security = $this->createMock(Security::class); + $security->expects($this->never())->method('getUser'); + + $processor = new InjectUserProcessor($security, 'fpm-fcgi'); + $record = $processor(self::record(['user' => ['username' => 'kept']])); + + $this->assertSame(['username' => 'kept'], $record->extra['user']); + } + + private function process(?InMemoryUser $user, string $sapi = 'fpm-fcgi'): LogRecord + { + $security = $this->createMock(Security::class); + $security->method('getUser')->willReturn($user); + + $processor = new InjectUserProcessor($security, $sapi); + + return $processor(self::record()); + } + + /** + * @param array $extra + */ + private static function record(array $extra = []): LogRecord + { + return new LogRecord( + new \DateTimeImmutable(), + 'api', + Level::Info, + 'Application form submission received', + [], + $extra + ); + } +} diff --git a/tests/Logging/UserDataProcessorTest.php b/tests/Logging/UserDataProcessorTest.php new file mode 100644 index 0000000..6376b38 --- /dev/null +++ b/tests/Logging/UserDataProcessorTest.php @@ -0,0 +1,41 @@ +createMock(Security::class); + $security->method('getUser')->willReturn(new InMemoryUser('api', null, ['ROLE_API'])); + $security->method('getToken')->willReturn(null); + + $processor = new UserDataProcessor($security, 'fpm-fcgi'); + + $record = $processor(new LogRecord( + new \DateTimeImmutable(), + 'api', + Level::Info, + 'Application form submission received' + )); + + $this->assertSame( + ['id' => null, 'username' => 'api', 'roles' => ['ROLE_API']], + $record->extra['user'] + ); + } +}