Skip to content

Commit

Permalink
Retrieve user IP from request context (backport of #131)
Browse files Browse the repository at this point in the history
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 0.8.x

(cherry picked from commit b784b79)
  • Loading branch information
eliecharra authored and Jean85 committed Jun 1, 2018
1 parent 8dc0200 commit 6d57173
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 4 deletions.
20 changes: 17 additions & 3 deletions src/Sentry/SentryBundle/EventListener/ExceptionListener.php
Expand Up @@ -8,6 +8,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;
Expand All @@ -34,6 +36,9 @@ class ExceptionListener implements SentryExceptionListenerInterface
/** @var EventDispatcherInterface */
protected $eventDispatcher;

/** @var RequestStack */
private $requestStack;

/** @var string[] */
protected $skipCapture;

Expand All @@ -42,13 +47,15 @@ class ExceptionListener implements SentryExceptionListenerInterface
* @param TokenStorageInterface $tokenStorage
* @param AuthorizationCheckerInterface $authorizationChecker
* @param \Raven_Client $client
* @param RequestStack $requestStack
* @param array $skipCapture
* @param EventDispatcherInterface $dispatcher
*/
public function __construct(
TokenStorageInterface $tokenStorage = null,
AuthorizationCheckerInterface $authorizationChecker = null,
\Raven_Client $client = null,
RequestStack $requestStack,
array $skipCapture,
EventDispatcherInterface $dispatcher
) {
Expand Down Expand Up @@ -164,20 +171,27 @@ private function shouldExceptionCaptureBeSkipped(\Exception $exception)
*/
private function setUserValue($user)
{
$data = array();

$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($user->__toString());
$this->client->set_user_data((string)$user, null, $data);
}
}
}
1 change: 1 addition & 0 deletions src/Sentry/SentryBundle/Resources/config/services.yml
Expand Up @@ -11,6 +11,7 @@ services:
- '@?security.token_storage'
- '@?security.authorization_checker'
- '@sentry.client'
- '@request_stack'
- '%sentry.skip_capture%'
- '@event_dispatcher'
tags:
Expand Down
5 changes: 5 additions & 0 deletions test/DependencyInjection/SentryExtensionTest.php
Expand Up @@ -4,6 +4,7 @@

use Sentry\SentryBundle\DependencyInjection\SentryExtension;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\HttpFoundation\RequestStack;

class SentryExtensionTest extends \PHPUnit_Framework_TestCase
{
Expand Down Expand Up @@ -639,6 +640,10 @@ private function getContainer(array $options = array())
$mockEventDispatcher = $this
->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface');

$mockRequestStack = $this
->createMock('Symfony\Component\HttpFoundation\RequestStack');

$containerBuilder->set('request_stack', $mockRequestStack);
$containerBuilder->set('event_dispatcher', $mockEventDispatcher);

$extension = new SentryExtension();
Expand Down
62 changes: 61 additions & 1 deletion test/EventListener/ExceptionListenerTest.php
Expand Up @@ -2,9 +2,11 @@

namespace Sentry\SentryBundle\Test\EventListener;

use Sentry\SentryBundle\SentrySymfonyEvents;
use Sentry\SentryBundle\DependencyInjection\SentryExtension;
use Sentry\SentryBundle\SentrySymfonyEvents;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;

Expand All @@ -30,6 +32,11 @@ class ExceptionListenerTest extends \PHPUnit_Framework_TestCase
*/
private $mockAuthorizationChecker;

/** @var EventDispatcherInterface|\PHPUnit_Framework_MockObject_MockObject */
private $mockEventDispatcher;

private $mockRequestStack;

public function setUp()
{
parent::setUp();
Expand All @@ -52,10 +59,15 @@ public function setUp()
->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface')
;

$this->$mockRequestStack = $this
->getMock('Symfony\Component\HttpFoundation\RequestStack')
;

$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);
Expand Down Expand Up @@ -482,6 +494,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
Expand Down

0 comments on commit 6d57173

Please sign in to comment.