diff --git a/Model/Report.php b/Model/Report.php index d27da98..f5dd080 100644 --- a/Model/Report.php +++ b/Model/Report.php @@ -31,6 +31,12 @@ class Report /** @var string[] */ private $filteredHeaders; + /** @var string[] */ + private $pendingPayloads = []; + + /** @var bool */ + private $shutdownRegistered = false; + public function __construct( Config $config, CurlFactory $curlFactory, @@ -80,41 +86,94 @@ private function getProductVersion(): string ); } + private function buildPayload(RequestInterface $request, array $rules): string + { + return $this->serializer->serialize([ + 'type' => 'report', + 'timestamp' => time(), + 'rules' => $rules, + 'version' => $this->getPackageVersion(), + 'product_version' => $this->getProductVersion(), + 'request' => [ + 'method' => $request->getMethod(), + 'uri' => $request->getRequestUri(), + 'body' => $request->getContent(), + 'ips' => $this->ip->collectRequestIPs(), + 'headers' => $this->getRequestHeaders($request), + 'scheme' => $request->getScheme(), + 'params' => $request->getParams(), + 'files' => $request->getFiles(), + ] + ]); + } + + private function postPayload(string $data) + { + $curl = $this->curlFactory->create(); + $curl->setCredentials($this->config->getLicenseKey(), $this->config->getLicenseKey()); + $curl->setTimeout(5); + $curl->addHeader('Expect', ''); // prevents curl from expecting 100-continue + $curl->addHeader('Content-Type', 'application/json'); + $curl->post($this->config->getReportUrl(), $data); + + if (!in_array($curl->getStatus(), [200, 429])) { + throw new \RuntimeException(sprintf("Invalid status code: %d", $curl->getStatus())); + } + } + + private function logFailure(\Exception $e) + { + $this->logger->error(sprintf("Failed to send report: %s", $e->getMessage())); + } + public function sendReport(RequestInterface $request, array $rules) { if (!$this->config->isReportEnabled()) { return; } try { - $curl = $this->curlFactory->create(); - $curl->setCredentials($this->config->getLicenseKey(), $this->config->getLicenseKey()); - $curl->setTimeout(5); - $curl->addHeader('Expect', ''); // prevents curl from expecting 100-continue - $curl->addHeader('Content-Type', 'application/json'); - $data = $this->serializer->serialize([ - 'type' => 'report', - 'timestamp' => time(), - 'rules' => $rules, - 'version' => $this->getPackageVersion(), - 'product_version' => $this->getProductVersion(), - 'request' => [ - 'method' => $request->getMethod(), - 'uri' => $request->getRequestUri(), - 'body' => $request->getContent(), - 'ips' => $this->ip->collectRequestIPs(), - 'headers' => $this->getRequestHeaders($request), - 'scheme' => $request->getScheme(), - 'params' => $request->getParams(), - 'files' => $request->getFiles(), - ] - ]); - $curl->post($this->config->getReportUrl(), $data); - - if (!in_array($curl->getStatus(), [200, 429])) { - throw new \RuntimeException(sprintf("Invalid status code: %d", $curl->getStatus())); - } + $this->postPayload($this->buildPayload($request, $rules)); + } catch (\Exception $e) { + $this->logFailure($e); + } + } + + public function sendReportDeferred(RequestInterface $request, array $rules) + { + if (!$this->config->isReportEnabled()) { + return; + } + try { + // Built now: at shutdown the request body stream, the config cache and the version cache + // backend may already be closed. + $this->pendingPayloads[] = $this->buildPayload($request, $rules); } catch (\Exception $e) { - $this->logger->error(sprintf("Failed to send report: %s", $e->getMessage())); + $this->logFailure($e); + return; + } + if (!$this->shutdownRegistered) { + $this->shutdownRegistered = true; + register_shutdown_function([$this, 'flushDeferredReports']); + } + } + + public function flushDeferredReports() + { + $payloads = $this->pendingPayloads; + $this->pendingPayloads = []; + if (empty($payloads)) { + return; + } + // Only PHP-FPM exposes this; on CLI and mod_php the reports are sent as before. + if (function_exists('fastcgi_finish_request')) { + fastcgi_finish_request(); + } + foreach ($payloads as $payload) { + try { + $this->postPayload($payload); + } catch (\Exception $e) { + $this->logFailure($e); + } } } diff --git a/Plugin/Shield.php b/Plugin/Shield.php index 6f18e57..412a15e 100644 --- a/Plugin/Shield.php +++ b/Plugin/Shield.php @@ -86,7 +86,7 @@ public function aroundDispatch(FrontControllerInterface $subject, callable $proc return $proceed($request); } - $this->report->sendReport($request, $matchedRules); + $this->report->sendReportDeferred($request, $matchedRules); foreach ($matchedRules as $rule) { if ($rule->action === 'block') { diff --git a/Test/Model/ReportTest.php b/Test/Model/ReportTest.php new file mode 100644 index 0000000..4ccf3f6 --- /dev/null +++ b/Test/Model/ReportTest.php @@ -0,0 +1,94 @@ +createMock(Config::class); + $config->method('isReportEnabled')->willReturn($reportEnabled); + $config->method('getLicenseKey')->willReturn('key'); + $config->method('getReportUrl')->willReturn('https://shield.example.com/report'); + + $curl = $this->createMock(Curl::class); + $curl->method('getStatus')->willReturn(200); + $curl->method('post')->willReturnCallback(function ($url, $data) { + $this->posts[] = [$url, $data]; + }); + + $curlFactory = $this->getMockBuilder(CurlFactory::class) + ->disableOriginalConstructor() + ->disableAutoload() + ->setMethods(['create']) + ->getMock(); + $curlFactory->method('create')->willReturn($curl); + + return new Report( + $config, + $curlFactory, + $this->createMock(Logger::class), + new Serializer(), + new IP(), + $this->createMock(ProductMetadataInterface::class) + ); + } + + public function testDeferredReportIsNotPostedImmediately() + { + $report = $this->buildReport(); + $report->sendReportDeferred(new RequestStub(), []); + + $this->assertSame([], $this->posts); + + $report->flushDeferredReports(); + $this->assertCount(1, $this->posts); + } + + public function testFlushPostsEveryDeferredReport() + { + $report = $this->buildReport(); + $report->sendReportDeferred(new RequestStub('', 'POST', '/first'), []); + $report->sendReportDeferred(new RequestStub('', 'POST', '/second'), []); + $report->flushDeferredReports(); + + $this->assertCount(2, $this->posts); + $this->assertSame('https://shield.example.com/report', $this->posts[0][0]); + $this->assertStringContainsString('"uri":"\/first"', $this->posts[0][1]); + $this->assertStringContainsString('"uri":"\/second"', $this->posts[1][1]); + $this->assertStringContainsString('"type":"report"', $this->posts[0][1]); + } + + public function testFlushIsIdempotent() + { + $report = $this->buildReport(); + $report->sendReportDeferred(new RequestStub(), []); + $report->flushDeferredReports(); + $report->flushDeferredReports(); + + $this->assertCount(1, $this->posts); + } + + public function testNothingIsQueuedWhenReportingDisabled() + { + $report = $this->buildReport(false); + $report->sendReportDeferred(new RequestStub(), []); + $report->flushDeferredReports(); + + $this->assertSame([], $this->posts); + } +} diff --git a/Test/Plugin/ShieldTest.php b/Test/Plugin/ShieldTest.php index 989d71b..7e5c44a 100644 --- a/Test/Plugin/ShieldTest.php +++ b/Test/Plugin/ShieldTest.php @@ -25,13 +25,17 @@ public function create() namespace Sansec\Shield\Test\Plugin { use Magento\Framework\App\FrontControllerInterface; + use Magento\Framework\App\Response\Http as HttpResponse; use Magento\Framework\App\Response\HttpFactory as HttpResponseFactory; + use Magento\Framework\View\Element\Template; use Magento\Framework\View\Element\TemplateFactory; use PHPUnit\Framework\MockObject\Rule\InvocationOrder; use PHPUnit\Framework\TestCase; + use Psr\Log\LoggerInterface as Logger; use Sansec\Shield\Model\Config; use Sansec\Shield\Model\IP; use Sansec\Shield\Model\Report; + use Sansec\Shield\Model\Rule; use Sansec\Shield\Model\Waf; use Sansec\Shield\Plugin\Shield; use Sansec\Shield\Test\RequestStub; @@ -51,25 +55,47 @@ protected function tearDown(): void $_SERVER = $this->serverBackup; } - private function buildPlugin(array $whitelistedIps, InvocationOrder $expectedWafCalls): Shield - { + private function buildPlugin( + array $whitelistedIps, + InvocationOrder $expectedWafCalls, + array $matchedRules = [], + ?Report $report = null + ): Shield { $config = $this->createMock(Config::class); $config->method('isEnabled')->willReturn(true); $config->method('getWhitelistedIps')->willReturn($whitelistedIps); $waf = $this->createMock(Waf::class); - $waf->expects($expectedWafCalls)->method('matchRequest')->willReturn([]); + $waf->expects($expectedWafCalls)->method('matchRequest')->willReturn($matchedRules); return new Shield( $config, $waf, - $this->createMock(Report::class), + $report ?: $this->createMock(Report::class), new IP(), - $this->createMock(HttpResponseFactory::class), - $this->createMock(TemplateFactory::class) + $this->buildResponseFactory(), + $this->buildTemplateFactory() ); } + private function buildResponseFactory(): HttpResponseFactory + { + $factory = $this->createMock(HttpResponseFactory::class); + $factory->method('create')->willReturn($this->createMock(HttpResponse::class)); + return $factory; + } + + private function buildTemplateFactory(): TemplateFactory + { + $template = $this->createMock(Template::class); + $template->method('setTemplate')->willReturnSelf(); + $template->method('toHtml')->willReturn(''); + + $factory = $this->createMock(TemplateFactory::class); + $factory->method('create')->willReturn($template); + return $factory; + } + private function dispatch(Shield $plugin): bool { $proceedCalled = false; @@ -105,5 +131,35 @@ public function testEmptyWhitelistDoesNotBypassWaf() $plugin = $this->buildPlugin([], $this->once()); $this->dispatch($plugin); } + + public function testMatchedRuleDefersTheReport() + { + $_SERVER['REMOTE_ADDR'] = '203.0.113.42'; + + $rule = new Rule(new IP(), $this->createMock(Logger::class), 'report'); + $report = $this->createMock(Report::class); + $report->expects($this->never())->method('sendReport'); + $report->expects($this->once())->method('sendReportDeferred')->with( + $this->isInstanceOf(RequestStub::class), + [$rule] + ); + + $plugin = $this->buildPlugin([], $this->once(), [$rule], $report); + $this->assertTrue($this->dispatch($plugin)); + } + + public function testBlockingRuleDefersTheReportAndSkipsDispatch() + { + $_SERVER['REMOTE_ADDR'] = '203.0.113.42'; + + $rule = new Rule(new IP(), $this->createMock(Logger::class), 'block'); + $report = $this->createMock(Report::class); + $report->expects($this->never())->method('sendReport'); + $report->expects($this->once())->method('sendReportDeferred'); + $report->expects($this->once())->method('logBlockedRequest'); + + $plugin = $this->buildPlugin([], $this->once(), [$rule], $report); + $this->assertFalse($this->dispatch($plugin)); + } } } diff --git a/Test/RequestStub.php b/Test/RequestStub.php index 3420eca..085c878 100644 --- a/Test/RequestStub.php +++ b/Test/RequestStub.php @@ -112,4 +112,19 @@ public function getPost() { return $this->post; } + + public function getHeaders() + { + return new \Laminas\Stdlib\Parameters($this->headers); + } + + public function getFiles() + { + return new \Laminas\Stdlib\Parameters([]); + } + + public function getScheme() + { + return 'https'; + } }