From f8420709460ff5bdc8101b628c3abaac87d6a160 Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Wed, 6 Jul 2022 18:28:58 +0200 Subject: [PATCH] Support logging the impersonator user, if any --- CHANGELOG.md | 5 +- phpstan-baseline.neon | 37 ++--- src/EventListener/RequestListener.php | 36 +++-- .../Fixtures/UserWithIdentifierStub.php | 12 +- tests/EventListener/RequestListenerTest.php | 153 +++++++++++++----- 5 files changed, 165 insertions(+), 78 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e77863f3..03403a8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,11 @@ ## Unreleased +- Support logging the impersonator user, if any (#647) + ## 4.3.0 (2022-05-30) -- Fix compatibility issue with Symfony >= 6.1.0 (#635) + +- Fix compatibility issue with Symfony `>= 6.1.0` (#635) - Add `TracingDriverConnectionInterface::getNativeConnection()` method to get the original driver connection (#597) - Add `options.http_timeout` and `options.http_connect_timeout` configuration options (#593) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index cda2318b..a708e56c 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -136,40 +136,20 @@ parameters: path: src/DependencyInjection/SentryExtension.php - - message: "#^Parameter \\#2 \\$value of method Symfony\\\\Component\\\\DependencyInjection\\\\Container\\:\\:setParameter\\(\\) expects array\\|bool\\|float\\|int\\|string\\|UnitEnum\\|null, mixed given\\.$#" + message: "#^Parameter \\#2 \\$value of method Symfony\\\\Component\\\\DependencyInjection\\\\Container\\:\\:setParameter\\(\\) expects array\\|bool\\|float\\|int\\|string\\|null, mixed given\\.$#" count: 2 path: src/DependencyInjection/SentryExtension.php - - - message: "#^Call to an undefined method Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\KernelEvent\\:\\:isMasterRequest\\(\\)\\.$#" - count: 1 - path: src/EventListener/AbstractTracingRequestListener.php - - message: "#^Class Sentry\\\\SentryBundle\\\\EventListener\\\\ConsoleCommandListener extends @final class Sentry\\\\SentryBundle\\\\EventListener\\\\ConsoleListener\\.$#" count: 1 path: src/EventListener/ConsoleCommandListener.php - - message: "#^Call to an undefined method Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\KernelEvent\\:\\:isMasterRequest\\(\\)\\.$#" - count: 1 - path: src/EventListener/RequestListener.php - - - - message: "#^Cannot call method getUser\\(\\) on Symfony\\\\Component\\\\Security\\\\Core\\\\Authentication\\\\Token\\\\TokenInterface\\|null\\.$#" - count: 1 - path: src/EventListener/RequestListener.php - - - - message: "#^Parameter \\#1 \\$user of method Sentry\\\\SentryBundle\\\\EventListener\\\\RequestListener\\:\\:getUsername\\(\\) expects object\\|string, Symfony\\\\Component\\\\Security\\\\Core\\\\User\\\\UserInterface\\|null given\\.$#" + message: "#^Method Symfony\\\\Component\\\\Security\\\\Core\\\\Authentication\\\\Token\\\\TokenInterface\\:\\:isAuthenticated\\(\\) invoked with 1 parameter, 0 required\\.$#" count: 1 path: src/EventListener/RequestListener.php - - - message: "#^Call to an undefined method Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\KernelEvent\\:\\:isMasterRequest\\(\\)\\.$#" - count: 1 - path: src/EventListener/SubRequestListener.php - - message: "#^Parameter \\#1 \\$driver of method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriverMiddleware\\:\\:wrap\\(\\) expects Doctrine\\\\DBAL\\\\Driver, mixed given\\.$#" count: 1 @@ -276,19 +256,19 @@ parameters: path: tests/End2End/TracingEnd2EndTest.php - - message: "#^Call to function method_exists\\(\\) with \\$this\\(Sentry\\\\SentryBundle\\\\Tests\\\\EventListener\\\\AuthenticatedTokenStub\\) and 'setAuthenticated' will always evaluate to false\\.$#" + message: "#^Parameter \\#1 \\$user of method Symfony\\\\Component\\\\Security\\\\Core\\\\Authentication\\\\Token\\\\AbstractToken\\:\\:setUser\\(\\) expects Symfony\\\\Component\\\\Security\\\\Core\\\\User\\\\UserInterface, string\\|Stringable\\|Symfony\\\\Component\\\\Security\\\\Core\\\\User\\\\UserInterface given\\.$#" count: 1 path: tests/EventListener/RequestListenerTest.php - - message: "#^Parameter \\#1 \\$user of method Symfony\\\\Component\\\\Security\\\\Core\\\\Authentication\\\\Token\\\\AbstractToken\\:\\:setUser\\(\\) expects Symfony\\\\Component\\\\Security\\\\Core\\\\User\\\\UserInterface, string\\|Stringable\\|Symfony\\\\Component\\\\Security\\\\Core\\\\User\\\\UserInterface given\\.$#" + message: "#^Parameter \\#3 \\$roles of class Symfony\\\\Component\\\\Security\\\\Core\\\\Authentication\\\\Token\\\\SwitchUserToken constructor expects array\\, string given\\.$#" count: 1 path: tests/EventListener/RequestListenerTest.php - - message: "#^Call to an undefined method Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\KernelEvent\\:\\:isMasterRequest\\(\\)\\.$#" + message: "#^Parameter \\#5 \\$originatedFromUri of class Symfony\\\\Component\\\\Security\\\\Core\\\\Authentication\\\\Token\\\\SwitchUserToken constructor expects string\\|null, Sentry\\\\SentryBundle\\\\Tests\\\\EventListener\\\\AuthenticatedTokenStub given\\.$#" count: 1 - path: tests/EventListener/SubRequestListenerTest.php + path: tests/EventListener/RequestListenerTest.php - message: "#^Call to an undefined method TCacheAdapter of Symfony\\\\Component\\\\Cache\\\\Adapter\\\\AdapterInterface\\:\\:delete\\(\\)\\.$#" @@ -310,6 +290,11 @@ parameters: count: 1 path: tests/Tracing/Cache/AbstractTraceableCacheAdapterTest.php + - + message: "#^Call to method PHPUnit\\\\Framework\\\\Assert\\:\\:assertSame\\(\\) with array\\{foo\\: Symfony\\\\Component\\\\Cache\\\\CacheItem\\} and Traversable\\ will always evaluate to false\\.$#" + count: 1 + path: tests/Tracing/Cache/AbstractTraceableCacheAdapterTest.php + - message: "#^Parameter \\#1 \\$decoratedAdapter of method Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Cache\\\\AbstractTraceableCacheAdapterTest\\\\:\\:createCacheAdapter\\(\\) expects TDecoratedCacheAdapter of Symfony\\\\Component\\\\Cache\\\\Adapter\\\\AdapterInterface, PHPUnit\\\\Framework\\\\MockObject\\\\MockObject&Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Cache\\\\CacheInterface given\\.$#" count: 2 diff --git a/src/EventListener/RequestListener.php b/src/EventListener/RequestListener.php index 2ab094d7..b566d615 100644 --- a/src/EventListener/RequestListener.php +++ b/src/EventListener/RequestListener.php @@ -10,6 +10,7 @@ use Symfony\Component\HttpKernel\Event\ControllerEvent; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\User\UserInterface; @@ -62,16 +63,11 @@ public function handleKernelRequestEvent(RequestEvent $event): void return; } - $token = null; $userData = new UserDataBag(); $userData->setIpAddress($event->getRequest()->getClientIp()); if (null !== $this->tokenStorage) { - $token = $this->tokenStorage->getToken(); - } - - if ($this->isTokenAuthenticated($token)) { - $userData->setUsername($this->getUsername($token->getUser())); + $this->setUserData($userData, $this->tokenStorage->getToken()); } $this->hub->configureScope(static function (Scope $scope) use ($userData): void { @@ -103,7 +99,7 @@ public function handleKernelControllerEvent(ControllerEvent $event): void } /** - * @param UserInterface|object|string $user + * @param UserInterface|object|string|null $user */ private function getUsername($user): ?string { @@ -128,12 +124,32 @@ private function getUsername($user): ?string return null; } - private function isTokenAuthenticated(?TokenInterface $token): bool + private function getImpersonatorUser(TokenInterface $token): ?string { - if (null === $token) { - return false; + if (!$token instanceof SwitchUserToken) { + return null; } + return $this->getUsername($token->getOriginalToken()->getUser()); + } + + private function setUserData(UserDataBag $userData, ?TokenInterface $token): void + { + if (null === $token || !$this->isTokenAuthenticated($token)) { + return; + } + + $userData->setUsername($this->getUsername($token->getUser())); + + $impersonatorUser = $this->getImpersonatorUser($token); + + if (null !== $impersonatorUser) { + $userData->setMetadata('impersonator_username', $impersonatorUser); + } + } + + private function isTokenAuthenticated(TokenInterface $token): bool + { if (method_exists($token, 'isAuthenticated') && !$token->isAuthenticated(false)) { return false; } diff --git a/tests/EventListener/Fixtures/UserWithIdentifierStub.php b/tests/EventListener/Fixtures/UserWithIdentifierStub.php index bdeb2aec..9b82b8ea 100644 --- a/tests/EventListener/Fixtures/UserWithIdentifierStub.php +++ b/tests/EventListener/Fixtures/UserWithIdentifierStub.php @@ -8,6 +8,16 @@ final class UserWithIdentifierStub implements UserInterface { + /** + * @var string + */ + private $username; + + public function __construct(string $username = 'foo_user') + { + $this->username = $username; + } + public function getUserIdentifier(): string { return $this->getUsername(); @@ -15,7 +25,7 @@ public function getUserIdentifier(): string public function getUsername(): string { - return 'foo_user'; + return $this->username; } public function getRoles(): array diff --git a/tests/EventListener/RequestListenerTest.php b/tests/EventListener/RequestListenerTest.php index 372cab8a..d6226ed7 100644 --- a/tests/EventListener/RequestListenerTest.php +++ b/tests/EventListener/RequestListenerTest.php @@ -22,6 +22,7 @@ use Symfony\Component\HttpKernel\Kernel; use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\User\UserInterface; @@ -51,6 +52,9 @@ protected function setUp(): void /** * @dataProvider handleKernelRequestEventDataProvider + * @dataProvider handleKernelRequestEventForSymfonyVersionLowerThan54DataProvider + * @dataProvider handleKernelRequestEventForSymfonyVersionGreaterThan54DataProvider + * @dataProvider handleKernelRequestEventForSymfonyVersionLowerThan60DataProvider */ public function testHandleKernelRequestEvent(RequestEvent $requestEvent, ?ClientInterface $client, ?TokenInterface $token, ?UserDataBag $expectedUser): void { @@ -138,46 +142,6 @@ public function handleKernelRequestEventDataProvider(): \Generator UserDataBag::createFromUserIpAddress('127.0.0.1'), ]; - if (version_compare(Kernel::VERSION, '6.0.0', '<')) { - yield 'token.authenticated = TRUE && token.user INSTANCEOF string' => [ - new RequestEvent( - $this->createMock(HttpKernelInterface::class), - new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), - HttpKernelInterface::MASTER_REQUEST - ), - $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), - new AuthenticatedTokenStub('foo_user'), - new UserDataBag(null, null, '127.0.0.1', 'foo_user'), - ]; - - yield 'token.authenticated = TRUE && token.user INSTANCEOF UserInterface && getUserIdentifier() method DOES NOT EXISTS' => [ - new RequestEvent( - $this->createMock(HttpKernelInterface::class), - new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), - HttpKernelInterface::MASTER_REQUEST - ), - $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), - new AuthenticatedTokenStub(new UserWithoutIdentifierStub()), - new UserDataBag(null, null, '127.0.0.1', 'foo_user'), - ]; - - yield 'token.authenticated = TRUE && token.user INSTANCEOF object && __toString() method EXISTS' => [ - new RequestEvent( - $this->createMock(HttpKernelInterface::class), - new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), - HttpKernelInterface::MASTER_REQUEST - ), - $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), - new AuthenticatedTokenStub(new class() implements \Stringable { - public function __toString(): string - { - return 'foo_user'; - } - }), - new UserDataBag(null, null, '127.0.0.1', 'foo_user'), - ]; - } - yield 'token.authenticated = TRUE && token.user INSTANCEOF UserInterface && getUserIdentifier() method EXISTS' => [ new RequestEvent( $this->createMock(HttpKernelInterface::class), @@ -201,6 +165,115 @@ public function __toString(): string ]; } + /** + * @return \Generator + */ + public function handleKernelRequestEventForSymfonyVersionLowerThan54DataProvider(): \Generator + { + if (version_compare(Kernel::VERSION, '5.4.0', '>=')) { + return; + } + + yield 'token.authenticated = TRUE && token INSTANCEOF SwitchUserToken' => [ + new RequestEvent( + $this->createMock(HttpKernelInterface::class), + new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), + HttpKernelInterface::MASTER_REQUEST + ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), + new SwitchUserToken( + new UserWithIdentifierStub(), + '', + 'user_provider', + ['ROLE_USER'], + new AuthenticatedTokenStub(new UserWithIdentifierStub('foo_user_impersonator')) + ), + UserDataBag::createFromArray([ + 'ip_address' => '127.0.0.1', + 'username' => 'foo_user', + 'impersonator_username' => 'foo_user_impersonator', + ]), + ]; + } + + /** + * @return \Generator + */ + public function handleKernelRequestEventForSymfonyVersionGreaterThan54DataProvider(): \Generator + { + if (version_compare(Kernel::VERSION, '5.4.0', '<')) { + return; + } + + yield 'token.authenticated = TRUE && token INSTANCEOF SwitchUserToken' => [ + new RequestEvent( + $this->createMock(HttpKernelInterface::class), + new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), + HttpKernelInterface::MASTER_REQUEST + ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), + new SwitchUserToken( + new UserWithIdentifierStub(), + 'main', + ['ROLE_USER'], + new AuthenticatedTokenStub(new UserWithIdentifierStub('foo_user_impersonator')) + ), + UserDataBag::createFromArray([ + 'ip_address' => '127.0.0.1', + 'username' => 'foo_user', + 'impersonator_username' => 'foo_user_impersonator', + ]), + ]; + } + + /** + * @return \Generator + */ + public function handleKernelRequestEventForSymfonyVersionLowerThan60DataProvider(): \Generator + { + if (version_compare(Kernel::VERSION, '6.0.0', '>=')) { + return; + } + + yield 'token.authenticated = TRUE && token.user INSTANCEOF string' => [ + new RequestEvent( + $this->createMock(HttpKernelInterface::class), + new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), + HttpKernelInterface::MASTER_REQUEST + ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), + new AuthenticatedTokenStub('foo_user'), + new UserDataBag(null, null, '127.0.0.1', 'foo_user'), + ]; + + yield 'token.authenticated = TRUE && token.user INSTANCEOF UserInterface && getUserIdentifier() method DOES NOT EXISTS' => [ + new RequestEvent( + $this->createMock(HttpKernelInterface::class), + new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), + HttpKernelInterface::MASTER_REQUEST + ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), + new AuthenticatedTokenStub(new UserWithoutIdentifierStub()), + new UserDataBag(null, null, '127.0.0.1', 'foo_user'), + ]; + + yield 'token.authenticated = TRUE && token.user INSTANCEOF object && __toString() method EXISTS' => [ + new RequestEvent( + $this->createMock(HttpKernelInterface::class), + new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), + HttpKernelInterface::MASTER_REQUEST + ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), + new AuthenticatedTokenStub(new class() implements \Stringable { + public function __toString(): string + { + return 'foo_user'; + } + }), + new UserDataBag(null, null, '127.0.0.1', 'foo_user'), + ]; + } + /** * @dataProvider handleKernelControllerEventDataProvider *