Merge pull request #265 from cklm/patch-1

Update RequestListener.php
getsentry · Oct 28, 2019 · d521013 · d521013
2 parents 58050af + 1156946
commit d521013
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 89 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## Unreleased
  - Fix handling of command with no name on `ConsoleListener` (#261)
+ - Remove check by AuthorizationChecker in  `RequestListener` (#264)
+ - Fixed undefined variable in `RequestListener` (#263)
 
 ## 3.2.0 (2019-10-04)
 

diff --git a/src/EventListener/RequestListener.php b/src/EventListener/RequestListener.php
@@ -8,8 +8,6 @@
 use Symfony\Component\HttpKernel\Event\FilterControllerEvent;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
-use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
-use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;
 use Symfony\Component\Security\Core\User\UserInterface;
 
 /**
@@ -24,23 +22,17 @@ final class RequestListener
     /** @var TokenStorageInterface|null */
     private $tokenStorage;
 
-    /** @var AuthorizationCheckerInterface|null */
-    private $authorizationChecker;
-
     /**
      * RequestListener constructor.
      * @param HubInterface $hub
      * @param TokenStorageInterface|null $tokenStorage
-     * @param AuthorizationCheckerInterface|null $authorizationChecker
      */
     public function __construct(
         HubInterface $hub,
-        ?TokenStorageInterface $tokenStorage,
-        ?AuthorizationCheckerInterface $authorizationChecker
+        ?TokenStorageInterface $tokenStorage
     ) {
         $this->hub = $hub; // not used, needed to trigger instantiation
         $this->tokenStorage = $tokenStorage;
-        $this->authorizationChecker = $authorizationChecker;
     }
 
     /**
@@ -65,11 +57,12 @@ public function onKernelRequest(GetResponseEvent $event): void
             $token = $this->tokenStorage->getToken();
         }
 
+        $userData = [];
+
         if (
             null !== $token
-            && null !== $this->authorizationChecker
             && $token->isAuthenticated()
-            && $this->authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)
+            && $token->getUser()
         ) {
             $userData = $this->getUserData($token->getUser());
         }

diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml
@@ -40,7 +40,6 @@
         <service id="Sentry\SentryBundle\EventListener\RequestListener" class="Sentry\SentryBundle\EventListener\RequestListener" public="false">
             <argument type="service" id="Sentry\State\HubInterface" />
             <argument type="service" id="security.token_storage" on-invalid="ignore" />
-            <argument type="service" id="security.authorization_checker" on-invalid="ignore" />
 
             <tag name="kernel.event_listener" event="kernel.request" method="onKernelRequest" priority="%sentry.listener_priorities.request%" />
             <tag name="kernel.event_listener" event="kernel.controller" method="onKernelController" priority="%sentry.listener_priorities.request%" />

diff --git a/test/EventListener/RequestListenerTest.php b/test/EventListener/RequestListenerTest.php
@@ -16,8 +16,6 @@
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
-use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
-use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;
 use Symfony\Component\Security\Core\User\UserInterface;
 
 class RequestListenerTest extends TestCase
@@ -59,7 +57,6 @@ protected function setUp()
     public function testOnKernelRequestUserDataIsSetToScope($user): void
     {
         $tokenStorage = $this->prophesize(TokenStorageInterface::class);
-        $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class);
         $event = $this->prophesize(GetResponseEvent::class);
         $request = $this->prophesize(Request::class);
         $token = $this->prophesize(TokenInterface::class);
@@ -72,8 +69,6 @@ public function testOnKernelRequestUserDataIsSetToScope($user): void
 
         $token->isAuthenticated()
             ->willReturn(true);
-        $authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)
-            ->willReturn(true);
 
         $token->getUser()
             ->willReturn($user);
@@ -85,8 +80,7 @@ public function testOnKernelRequestUserDataIsSetToScope($user): void
 
         $listener = new RequestListener(
             $this->currentHub->reveal(),
-            $tokenStorage->reveal(),
-            $authorizationChecker->reveal()
+            $tokenStorage->reveal()
         );
 
         $listener->onKernelRequest($event->reveal());
@@ -113,7 +107,6 @@ public function userDataProvider(): \Generator
     public function testOnKernelRequestUserDataIsNotSetIfSendPiiIsDisabled(): void
     {
         $tokenStorage = $this->prophesize(TokenStorageInterface::class);
-        $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class);
         $event = $this->prophesize(GetResponseEvent::class);
 
         $event->isMasterRequest()
@@ -126,8 +119,7 @@ public function testOnKernelRequestUserDataIsNotSetIfSendPiiIsDisabled(): void
 
         $listener = new RequestListener(
             $this->currentHub->reveal(),
-            $tokenStorage->reveal(),
-            $authorizationChecker->reveal()
+            $tokenStorage->reveal()
         );
 
         $listener->onKernelRequest($event->reveal());
@@ -138,7 +130,6 @@ public function testOnKernelRequestUserDataIsNotSetIfSendPiiIsDisabled(): void
     public function testOnKernelRequestUserDataIsNotSetIfNoClientIsPresent(): void
     {
         $tokenStorage = $this->prophesize(TokenStorageInterface::class);
-        $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class);
         $event = $this->prophesize(GetResponseEvent::class);
 
         $event->isMasterRequest()
@@ -151,8 +142,7 @@ public function testOnKernelRequestUserDataIsNotSetIfNoClientIsPresent(): void
 
         $listener = new RequestListener(
             $this->currentHub->reveal(),
-            $tokenStorage->reveal(),
-            $authorizationChecker->reveal()
+            $tokenStorage->reveal()
         );
 
         $listener->onKernelRequest($event->reveal());
@@ -162,55 +152,19 @@ public function testOnKernelRequestUserDataIsNotSetIfNoClientIsPresent(): void
 
     public function testOnKernelRequestUsernameIsNotSetIfTokenStorageIsAbsent(): void
     {
-        $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class);
-        $event = $this->prophesize(GetResponseEvent::class);
-        $request = $this->prophesize(Request::class);
-
-        $event->isMasterRequest()
-            ->willReturn(true);
-
-        $authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)
-            ->shouldNotBeCalled();
-
-        $event->getRequest()
-            ->willReturn($request->reveal());
-        $request->getClientIp()
-            ->willReturn('1.2.3.4');
-
-        $listener = new RequestListener(
-            $this->currentHub->reveal(),
-            null,
-            $authorizationChecker->reveal()
-        );
-
-        $listener->onKernelRequest($event->reveal());
-
-        $expectedUserData = [
-            'ip_address' => '1.2.3.4',
-        ];
-        $this->assertEquals($expectedUserData, $this->getUserContext($this->currentScope));
-    }
-
-    public function testOnKernelRequestUsernameIsNotSetIfAuthorizationCheckerIsAbsent(): void
-    {
-        $tokenStorage = $this->prophesize(TokenStorageInterface::class);
         $event = $this->prophesize(GetResponseEvent::class);
         $request = $this->prophesize(Request::class);
 
         $event->isMasterRequest()
             ->willReturn(true);
 
-        $tokenStorage->getToken()
-            ->willReturn($this->prophesize(TokenInterface::class)->reveal());
-
         $event->getRequest()
             ->willReturn($request->reveal());
         $request->getClientIp()
             ->willReturn('1.2.3.4');
 
         $listener = new RequestListener(
             $this->currentHub->reveal(),
-            $tokenStorage->reveal(),
             null
         );
 
@@ -225,7 +179,6 @@ public function testOnKernelRequestUsernameIsNotSetIfAuthorizationCheckerIsAbsen
     public function testOnKernelRequestUsernameIsNotSetIfTokenIsAbsent(): void
     {
         $tokenStorage = $this->prophesize(TokenStorageInterface::class);
-        $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class);
         $event = $this->prophesize(GetResponseEvent::class);
         $request = $this->prophesize(Request::class);
 
@@ -235,18 +188,14 @@ public function testOnKernelRequestUsernameIsNotSetIfTokenIsAbsent(): void
         $tokenStorage->getToken()
             ->willReturn(null);
 
-        $authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)
-            ->shouldNotBeCalled();
-
         $event->getRequest()
             ->willReturn($request->reveal());
         $request->getClientIp()
             ->willReturn('1.2.3.4');
 
         $listener = new RequestListener(
             $this->currentHub->reveal(),
-            $tokenStorage->reveal(),
-            $authorizationChecker->reveal()
+            $tokenStorage->reveal()
         );
 
         $listener->onKernelRequest($event->reveal());
@@ -263,7 +212,6 @@ public function testOnKernelRequestUsernameIsNotSetIfTokenIsAbsent(): void
     public function testOnKernelRequestUsernameIsNotSetIfTokenIsNotAuthenticated(): void
     {
         $tokenStorage = $this->prophesize(TokenStorageInterface::class);
-        $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class);
         $token = $this->prophesize(TokenInterface::class);
         $event = $this->prophesize(GetResponseEvent::class);
         $request = $this->prophesize(Request::class);
@@ -277,18 +225,14 @@ public function testOnKernelRequestUsernameIsNotSetIfTokenIsNotAuthenticated():
         $token->isAuthenticated()
             ->willReturn(false);
 
-        $authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)
-            ->shouldNotBeCalled();
-
         $event->getRequest()
             ->willReturn($request->reveal());
         $request->getClientIp()
             ->willReturn('1.2.3.4');
 
         $listener = new RequestListener(
             $this->currentHub->reveal(),
-            $tokenStorage->reveal(),
-            $authorizationChecker->reveal()
+            $tokenStorage->reveal()
         );
 
         $listener->onKernelRequest($event->reveal());
@@ -302,7 +246,6 @@ public function testOnKernelRequestUsernameIsNotSetIfTokenIsNotAuthenticated():
     public function testOnKernelRequestUsernameIsNotSetIfUserIsNotRemembered(): void
     {
         $tokenStorage = $this->prophesize(TokenStorageInterface::class);
-        $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class);
         $event = $this->prophesize(GetResponseEvent::class);
         $request = $this->prophesize(Request::class);
 
@@ -312,18 +255,14 @@ public function testOnKernelRequestUsernameIsNotSetIfUserIsNotRemembered(): void
         $tokenStorage->getToken()
             ->willReturn(null);
 
-        $authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)
-            ->willReturn(false);
-
         $event->getRequest()
             ->willReturn($request->reveal());
         $request->getClientIp()
             ->willReturn('1.2.3.4');
 
         $listener = new RequestListener(
             $this->currentHub->reveal(),
-            $tokenStorage->reveal(),
-            $authorizationChecker->reveal()
+            $tokenStorage->reveal()
         );
 
         $listener->onKernelRequest($event->reveal());
@@ -347,8 +286,7 @@ public function testOnKernelControllerAddsRouteTag(): void
 
         $listener = new RequestListener(
             $this->currentHub->reveal(),
-            $this->prophesize(TokenStorageInterface::class)->reveal(),
-            $this->prophesize(AuthorizationCheckerInterface::class)->reveal()
+            $this->prophesize(TokenStorageInterface::class)->reveal()
         );
 
         $listener->onKernelController($event->reveal());
@@ -371,8 +309,7 @@ public function testOnKernelControllerRouteTagIsNotSetIfRequestDoesNotHaveARoute
 
         $listener = new RequestListener(
             $this->currentHub->reveal(),
-            $this->prophesize(TokenStorageInterface::class)->reveal(),
-            $this->prophesize(AuthorizationCheckerInterface::class)->reveal()
+            $this->prophesize(TokenStorageInterface::class)->reveal()
         );
 
         $listener->onKernelController($event->reveal());
@@ -384,7 +321,6 @@ public function testOnKernelRequestUserDataAndTagsAreNotSetInSubRequest(): void
             ->shouldNotBeCalled();
 
         $tokenStorage = $this->prophesize(TokenStorageInterface::class);
-        $authorizationChecker = $this->prophesize(AuthorizationCheckerInterface::class);
         $event = $this->prophesize(GetResponseEvent::class);
 
         $event->isMasterRequest()
@@ -393,13 +329,9 @@ public function testOnKernelRequestUserDataAndTagsAreNotSetInSubRequest(): void
         $tokenStorage->getToken()
             ->shouldNotBeCalled();
 
-        $authorizationChecker->isGranted(AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED)
-            ->shouldNotBeCalled();
-
         $listener = new RequestListener(
             $this->currentHub->reveal(),
-            $tokenStorage->reveal(),
-            $authorizationChecker->reveal()
+            $tokenStorage->reveal()
         );
 
         $listener->onKernelRequest($event->reveal());