Retrieve user IP from request context (#131)

This will fix ivalid client IP retrieval when app is behind a reverse proxy as we retrieve IP through framework logic.

Author: eliecharra

src/EventListener/ExceptionListener.php

@@ -9,6 +9,8 @@
 use Symfony\Component\Console\Event\ConsoleEvent;
 use Symfony\Component\Console\Event\ConsoleExceptionEvent;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
@@ -35,6 +37,9 @@ class ExceptionListener implements SentryExceptionListenerInterface
     /** @var EventDispatcherInterface */
     protected $eventDispatcher;
 
+    /** @var RequestStack */
+    private $requestStack;
+
     /** @var string[] */
     protected $skipCapture;
 
@@ -49,12 +54,14 @@ class ExceptionListener implements SentryExceptionListenerInterface
     public function __construct(
         \Raven_Client $client,
         EventDispatcherInterface $dispatcher,
+        RequestStack $requestStack,
         array $skipCapture,
         TokenStorageInterface $tokenStorage = null,
         AuthorizationCheckerInterface $authorizationChecker = null
     ) {
         $this->client = $client;
         $this->eventDispatcher = $dispatcher;
+        $this->requestStack = $requestStack;
         $this->skipCapture = $skipCapture;
         $this->tokenStorage = $tokenStorage;
         $this->authorizationChecker = $authorizationChecker;
@@ -179,20 +186,27 @@ protected function shouldExceptionCaptureBeSkipped(\Throwable $exception): bool
      */
     private function setUserValue($user)
     {
+        $data = [];
+
+        $request = $this->requestStack->getCurrentRequest();
+        if ($request instanceof Request) {
+            $data['ip_address'] = $request->getClientIp();
+        }
+
         if ($user instanceof UserInterface) {
-            $this->client->set_user_data($user->getUsername());
+            $this->client->set_user_data($user->getUsername(), null, $data);
 
             return;
         }
 
         if (is_string($user)) {
-            $this->client->set_user_data($user);
+            $this->client->set_user_data($user, null, $data);
 
             return;
         }
 
         if (is_object($user) && method_exists($user, '__toString')) {
-            $this->client->set_user_data((string)$user);
+            $this->client->set_user_data((string)$user, null, $data);
         }
     }
 }

src/Resources/config/services.yml

@@ -11,6 +11,7 @@ services:
     arguments:
       - '@sentry.client'
       - '@event_dispatcher'
+      - '@request_stack'
       - '%sentry.skip_capture%'
       - '@?security.token_storage'
       - '@?security.authorization_checker'

test/DependencyInjection/SentryExtensionTest.php

@@ -12,6 +12,7 @@
 use Symfony\Component\DependencyInjection\Container;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
 
 class SentryExtensionTest extends TestCase
@@ -376,6 +377,10 @@ private function getContainer(array $configuration = []): Container
         $mockEventDispatcher = $this
             ->createMock(EventDispatcherInterface::class);
 
+        $mockRequestStack = $this
+            ->createMock(RequestStack::class);
+
+        $containerBuilder->set('request_stack', $mockRequestStack);
         $containerBuilder->set('event_dispatcher', $mockEventDispatcher);
         $containerBuilder->setAlias(self::LISTENER_TEST_PUBLIC_ALIAS, new Alias('sentry.exception_listener', true));
 

test/EventListener/ExceptionListenerTest.php

@@ -16,6 +16,8 @@
 use Symfony\Component\DependencyInjection\Alias;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
 use Symfony\Component\HttpKernel\Exception\HttpException;
@@ -40,17 +42,21 @@ class ExceptionListenerTest extends TestCase
 
     private $mockEventDispatcher;
 
+    private $mockRequestStack;
+
     public function setUp()
     {
         $this->mockTokenStorage = $this->createMock(TokenStorageInterface::class);
         $this->mockAuthorizationChecker = $this->createMock(AuthorizationCheckerInterface::class);
         $this->mockSentryClient = $this->createMock(\Raven_Client::class);
         $this->mockEventDispatcher = $this->createMock(EventDispatcherInterface::class);
+        $this->mockRequestStack = $this->createMock(RequestStack::class);
 
         $containerBuilder = new ContainerBuilder();
         $containerBuilder->setParameter('kernel.root_dir', 'kernel/root');
         $containerBuilder->setParameter('kernel.environment', 'test');
 
+        $containerBuilder->set('request_stack', $this->mockRequestStack);
         $containerBuilder->set('security.token_storage', $this->mockTokenStorage);
         $containerBuilder->set('security.authorization_checker', $this->mockAuthorizationChecker);
         $containerBuilder->set('sentry.client', $this->mockSentryClient);
@@ -384,6 +390,54 @@ public function test_that_username_is_set_from_user_interface_if_token_present_a
         $listener->onKernelRequest($mockEvent);
     }
 
+    public function test_that_ip_is_set_from_request_stack()
+    {
+        $mockToken = $this->createMock(TokenInterface::class);
+
+        $mockToken
+            ->method('getUser')
+            ->willReturn('some_user');
+
+        $mockToken
+            ->method('isAuthenticated')
+            ->willReturn(true);
+
+        $mockEvent = $this->createMock(GetResponseEvent::class);
+
+        $mockRequest = $this->createMock(Request::class);
+
+        $mockRequest
+            ->method('getClientIp')
+            ->willReturn('1.2.3.4');
+
+        $this->mockRequestStack
+            ->method('getCurrentRequest')
+            ->willReturn($mockRequest);
+
+        $mockEvent
+            ->expects($this->once())
+            ->method('getRequestType')
+            ->willReturn(HttpKernelInterface::MASTER_REQUEST);
+
+        $this->mockAuthorizationChecker
+            ->method('isGranted')
+            ->with($this->identicalTo(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED))
+            ->willReturn(true);
+
+        $this->mockTokenStorage
+            ->method('getToken')
+            ->willReturn($mockToken);
+
+        $this->mockSentryClient
+            ->expects($this->once())
+            ->method('set_user_data')
+            ->with($this->identicalTo('some_user'), null, ['ip_address' => '1.2.3.4']);
+
+        $this->containerBuilder->compile();
+        $listener = $this->getListener();
+        $listener->onKernelRequest($mockEvent);
+    }
+
     public function test_regression_with_unauthenticated_user_token_PR_78()
     {
         $mockToken = $this->createMock(TokenInterface::class);