Retrieve user IP from request context (#132)

This will fix ivalid client IP retrieval when app is behind a reverse proxy as we retrieve IP through framework logic. Backport of #131 to 1.x

Author: eliecharra

src/EventListener/ExceptionListener.php

@@ -7,6 +7,8 @@
 use Symfony\Component\Console\Event\ConsoleCommandEvent;
 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;
@@ -33,6 +35,9 @@ class ExceptionListener implements SentryExceptionListenerInterface
     /** @var EventDispatcherInterface */
     protected $eventDispatcher;
 
+    /** @var RequestStack */
+    private $requestStack;
+
     /** @var string[] */
     protected $skipCapture;
 
@@ -47,12 +52,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;
@@ -158,20 +165,27 @@ protected function shouldExceptionCaptureBeSkipped(\Exception $exception)
      */
     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

@@ -10,6 +10,7 @@
 use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 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
@@ -373,6 +374,10 @@ private function getContainer(array $configuration = [])
         $mockEventDispatcher = $this
             ->createMock(EventDispatcherInterface::class);
 
+        $mockRequestStack = $this
+            ->createMock(RequestStack::class);
+
+        $containerBuilder->set('request_stack', $mockRequestStack);
         $containerBuilder->set('event_dispatcher', $mockEventDispatcher);
 
         $extension = new SentryExtension();

test/EventListener/ExceptionListenerTest.php

@@ -11,6 +11,8 @@
 use Symfony\Component\Console\Event\ConsoleExceptionEvent;
 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;
@@ -38,17 +40,21 @@ class ExceptionListenerTest extends TestCase
     /** @var EventDispatcherInterface|\PHPUnit_Framework_MockObject_MockObject */
     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);
@@ -424,6 +430,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->containerBuilder->get('sentry.exception_listener');
+        $listener->onKernelRequest($mockEvent);
+    }
+
     public function test_regression_with_unauthenticated_user_token_PR_78()
     {
         $mockToken = $this->createMock(TokenInterface::class);