diff --git a/CHANGELOG.md b/CHANGELOG.md index 49955f6c..b2dff994 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,8 @@ ## Unreleased -- Added missing `capture-soft-fails` config schema option (#417) +- Add missing `capture-soft-fails` option to the XSD schema for the XML config (#417) +- Fix regression that send PII even when the `send_default_pii` option is off (#425) ## 4.0.0 (2021-01-19) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index f1ea7310..cde27129 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -82,7 +82,7 @@ parameters: - message: "#^Instantiated class Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\GetResponseEvent not found\\.$#" - count: 6 + count: 8 path: tests/EventListener/RequestListenerTest.php - diff --git a/src/EventListener/RequestListener.php b/src/EventListener/RequestListener.php index 13cb397c..23058e2c 100644 --- a/src/EventListener/RequestListener.php +++ b/src/EventListener/RequestListener.php @@ -51,6 +51,12 @@ public function handleKernelRequestEvent(RequestListenerRequestEvent $event): vo return; } + $client = $this->hub->getClient(); + + if (null === $client || !$client->getOptions()->shouldSendDefaultPii()) { + return; + } + $token = null; $userData = UserDataBag::createFromUserIpAddress($event->getRequest()->getClientIp()); diff --git a/tests/EventListener/RequestListenerTest.php b/tests/EventListener/RequestListenerTest.php index 3d71b4f7..e2d67b97 100644 --- a/tests/EventListener/RequestListenerTest.php +++ b/tests/EventListener/RequestListenerTest.php @@ -6,7 +6,9 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Sentry\ClientInterface; use Sentry\Event; +use Sentry\Options; use Sentry\SentryBundle\EventListener\RequestListener; use Sentry\State\HubInterface; use Sentry\State\Scope; @@ -53,15 +55,19 @@ protected function setUp(): void * * @param GetResponseEvent|RequestEvent $requestEvent */ - public function testHandleKernelRequestEvent($requestEvent, ?TokenInterface $token, UserDataBag $expectedUser): void + public function testHandleKernelRequestEvent($requestEvent, ?ClientInterface $client, ?TokenInterface $token, ?UserDataBag $expectedUser): void { $scope = new Scope(); - $this->tokenStorage->expects($this->once()) + $this->hub->expects($this->any()) + ->method('getClient') + ->willReturn($client); + + $this->tokenStorage->expects($this->any()) ->method('getToken') ->willReturn($token); - $this->hub->expects($this->once()) + $this->hub->expects($this->any()) ->method('configureScope') ->willReturnCallback(static function (callable $callback) use ($scope): void { $callback($scope); @@ -83,12 +89,35 @@ public function handleKernelRequestEventForSymfonyVersionLowerThan43DataProvider return; } + yield 'event.requestType != MASTER_REQUEST' => [ + new GetResponseEvent( + $this->createMock(HttpKernelInterface::class), + new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), + HttpKernelInterface::SUB_REQUEST + ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), + null, + null, + ]; + + yield 'options.send_default_pii = FALSE' => [ + new GetResponseEvent( + $this->createMock(HttpKernelInterface::class), + new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), + HttpKernelInterface::MASTER_REQUEST + ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => false])), + null, + null, + ]; + yield 'token IS NULL' => [ new GetResponseEvent( $this->createMock(HttpKernelInterface::class), new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), HttpKernelInterface::MASTER_REQUEST ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), null, UserDataBag::createFromUserIpAddress('127.0.0.1'), ]; @@ -99,6 +128,7 @@ public function handleKernelRequestEventForSymfonyVersionLowerThan43DataProvider new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), HttpKernelInterface::MASTER_REQUEST ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), new class() extends AbstractToken { public function __construct() { @@ -121,6 +151,7 @@ public function getCredentials() new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), HttpKernelInterface::MASTER_REQUEST ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), new class() extends AbstractToken { public function __construct() { @@ -143,6 +174,7 @@ public function getCredentials() new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), HttpKernelInterface::MASTER_REQUEST ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), new class() extends AbstractToken { public function __construct() { @@ -166,6 +198,7 @@ public function getCredentials() new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), HttpKernelInterface::MASTER_REQUEST ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), new class() extends AbstractToken { public function __construct() { @@ -213,6 +246,7 @@ public function getCredentials() new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), HttpKernelInterface::MASTER_REQUEST ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), new class() extends AbstractToken { public function __construct() { @@ -245,12 +279,35 @@ public function handleKernelRequestEventForSymfonyVersionAtLeast43DataProvider() return; } + yield 'event.requestType != MASTER_REQUEST' => [ + new RequestEvent( + $this->createMock(HttpKernelInterface::class), + new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), + HttpKernelInterface::SUB_REQUEST + ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), + null, + null, + ]; + + yield 'options.send_default_pii = FALSE' => [ + new RequestEvent( + $this->createMock(HttpKernelInterface::class), + new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), + HttpKernelInterface::MASTER_REQUEST + ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => false])), + null, + null, + ]; + yield 'token IS NULL' => [ new RequestEvent( $this->createMock(HttpKernelInterface::class), new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), HttpKernelInterface::MASTER_REQUEST ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), null, UserDataBag::createFromUserIpAddress('127.0.0.1'), ]; @@ -261,6 +318,7 @@ public function handleKernelRequestEventForSymfonyVersionAtLeast43DataProvider() new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), HttpKernelInterface::MASTER_REQUEST ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), new class() extends AbstractToken { public function __construct() { @@ -283,6 +341,7 @@ public function getCredentials() new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), HttpKernelInterface::MASTER_REQUEST ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), new class() extends AbstractToken { public function __construct() { @@ -305,6 +364,7 @@ public function getCredentials() new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), HttpKernelInterface::MASTER_REQUEST ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), new class() extends AbstractToken { public function __construct() { @@ -328,6 +388,7 @@ public function getCredentials() new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), HttpKernelInterface::MASTER_REQUEST ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), new class() extends AbstractToken { public function __construct() { @@ -375,6 +436,7 @@ public function getCredentials() new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']), HttpKernelInterface::MASTER_REQUEST ), + $this->getMockedClientWithOptions(new Options(['send_default_pii' => true])), new class() extends AbstractToken { public function __construct() { @@ -511,4 +573,14 @@ static function () { ], ]; } + + private function getMockedClientWithOptions(Options $options): ClientInterface + { + $client = $this->createMock(ClientInterface::class); + $client->expects($this->any()) + ->method('getOptions') + ->willReturn($options); + + return $client; + } }