Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update RequestListener.php #265

Merged
merged 8 commits into from Oct 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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)

Expand Down
15 changes: 4 additions & 11 deletions src/EventListener/RequestListener.php
Expand Up @@ -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;

/**
Expand All @@ -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;
}

/**
Expand All @@ -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());
}
Expand Down
1 change: 0 additions & 1 deletion src/Resources/config/services.xml
Expand Up @@ -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%" />
Expand Down
86 changes: 9 additions & 77 deletions test/EventListener/RequestListenerTest.php
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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());
Expand All @@ -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()
Expand All @@ -126,8 +119,7 @@ public function testOnKernelRequestUserDataIsNotSetIfSendPiiIsDisabled(): void

$listener = new RequestListener(
$this->currentHub->reveal(),
$tokenStorage->reveal(),
$authorizationChecker->reveal()
$tokenStorage->reveal()
);

$listener->onKernelRequest($event->reveal());
Expand All @@ -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()
Expand All @@ -151,8 +142,7 @@ public function testOnKernelRequestUserDataIsNotSetIfNoClientIsPresent(): void

$listener = new RequestListener(
$this->currentHub->reveal(),
$tokenStorage->reveal(),
$authorizationChecker->reveal()
$tokenStorage->reveal()
);

$listener->onKernelRequest($event->reveal());
Expand All @@ -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
);

Expand All @@ -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);

Expand All @@ -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());
Expand All @@ -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);
Expand All @@ -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());
Expand All @@ -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);

Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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()
Expand All @@ -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());
Expand Down