From 203b63a9ce99eb3618132d2e3585edb2fd607246 Mon Sep 17 00:00:00 2001 From: Johannes Przymusinski Date: Tue, 7 Nov 2023 20:02:37 +0100 Subject: [PATCH] =?UTF-8?q?[TASK]=20100%=20test=20coverage=20=F0=9F=8E=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Controller/Mamo/MetricsController.php | 32 +++--- tests/Controller/MetricsControllerTest.php | 104 +++++++++++++++++- tests/MobiMamoConnectorTest.php | 15 +++ .../StorefrontControllerTestBehaviour.php | 47 ++++++++ tests/Utility/VersionUtilityTest.php | 5 + 5 files changed, 180 insertions(+), 23 deletions(-) create mode 100644 tests/MobiMamoConnectorTest.php create mode 100644 tests/Support/StorefrontControllerTestBehaviour.php diff --git a/src/Controller/Mamo/MetricsController.php b/src/Controller/Mamo/MetricsController.php index e657b00..4c48290 100644 --- a/src/Controller/Mamo/MetricsController.php +++ b/src/Controller/Mamo/MetricsController.php @@ -37,6 +37,7 @@ public function __construct( public function indexAction(Request $request): Response { $this->verifyRequest($request); + $secret = $this->getConfigurationSecret(); $registry = new CollectorRegistry(new InMemory()); $registry->getOrRegisterGauge('mamo', 'shopware6_platform', 'Shopware 6 Platform Version', ['latestVersion', 'currentVersion']) @@ -71,15 +72,6 @@ public function indexAction(Request $request): Response ]; if (! $request->query->has('unsecure')) { - $secret = $this->systemConfigService->get(MobiMamoConnector::CONFIG_KEY_SECRET); - if (! is_string($secret)) { - // Can only happen, when we change our config template or Shopware itself screws up. - $this->logger->error('Configuration Secret is not a string.', [ - 'receivedType' => gettype($secret), - ]); - throw new HttpException(500); - } - $headers['HMAC'] = hash_hmac('sha256', $result, $secret); } @@ -92,10 +84,7 @@ public function indexAction(Request $request): Response */ private function verifyRequest(Request $request): void { - $secret = $this->systemConfigService->get(MobiMamoConnector::CONFIG_KEY_SECRET); - if (! is_string($secret)) { - throw new HttpException(500, 'Configuration Secret is not a string.'); - } + $secret = $this->getConfigurationSecret(); // Handle legacy request with the secret in the query parameter. if ($request->query->has('unsecure')) { @@ -114,7 +103,7 @@ private function validateHmacRequest(Request $request, string $secret): void throw new HttpException(401); } - $body = file_get_contents('php://input'); + $body = $request->getContent(); if (! $body) { $this->logger->info('Request body is missing.'); throw new HttpException(400); @@ -127,6 +116,16 @@ private function validateHmacRequest(Request $request, string $secret): void } private function validateSecretRequest(Request $request, string $secret): void + { + $secret = $this->getConfigurationSecret(); + + if (! $this->requestAuthorizationService->isAuthorized($request, $secret)) { + $this->logger->info('Request is not authorized to access the metrics endpoint.'); + throw new HttpException(403); + } + } + + private function getConfigurationSecret(): string { $secret = $this->systemConfigService->get(MobiMamoConnector::CONFIG_KEY_SECRET); if (! is_string($secret)) { @@ -137,9 +136,6 @@ private function validateSecretRequest(Request $request, string $secret): void throw new HttpException(500); } - if (! $this->requestAuthorizationService->isAuthorized($request, $secret)) { - $this->logger->info('Request is not authorized to access the metrics endpoint.'); - throw new HttpException(403); - } + return $secret; } } diff --git a/tests/Controller/MetricsControllerTest.php b/tests/Controller/MetricsControllerTest.php index cf092c8..0ca77b4 100644 --- a/tests/Controller/MetricsControllerTest.php +++ b/tests/Controller/MetricsControllerTest.php @@ -5,22 +5,24 @@ namespace MobilisticsGmbH\MamoConnector\Tests\Controller; use MobilisticsGmbH\MamoConnector\MobiMamoConnector; +use MobilisticsGmbH\MamoConnector\Tests\Support\StorefrontControllerTestBehaviour; use PHPUnit\Framework\TestCase; use Shopware\Core\Framework\Test\TestCaseBase\IntegrationTestBehaviour; use Shopware\Core\System\SystemConfig\SystemConfigService; -use Shopware\Storefront\Test\Controller\StorefrontControllerTestBehaviour; class MetricsControllerTest extends TestCase { use IntegrationTestBehaviour; use StorefrontControllerTestBehaviour; + private const TESTING_SECRET = "testing-secret"; + public function testMetricsAction(): void { /** @var SystemConfigService $systemConfigService */ $systemConfigService = $this->getContainer()->get(SystemConfigService::class); - $systemConfigService->set(MobiMamoConnector::CONFIG_KEY_SECRET, 'testing-secret'); + $systemConfigService->set(MobiMamoConnector::CONFIG_KEY_SECRET, self::TESTING_SECRET); $metrics = $this->request('GET', 'mamo-connector/metrics?unsecure&secret=testing-secret', []); @@ -31,17 +33,109 @@ public function testMetricsAction(): void static::assertStringContainsString('mamo_shopware6_platform', $content); } - public function testFailWhenNoSecretProvided(): void + public function testFailWithInvalidLegacySecret(): void + { + /** @var SystemConfigService $systemConfigService */ + $systemConfigService = $this->getContainer()->get(SystemConfigService::class); + + $systemConfigService->set(MobiMamoConnector::CONFIG_KEY_SECRET, self::TESTING_SECRET); + + $metrics = $this->request('GET', 'mamo-connector/metrics?unsecure', []); + $content = $metrics->getContent(); + + static::assertNotFalse($content); + static::assertEquals(403, $metrics->getStatusCode()); + } + + public function testFailWithInvalidHmacHeader(): void { /** @var SystemConfigService $systemConfigService */ $systemConfigService = $this->getContainer()->get(SystemConfigService::class); - $systemConfigService->set(MobiMamoConnector::CONFIG_KEY_SECRET, 'testing-secret'); + $systemConfigService->set(MobiMamoConnector::CONFIG_KEY_SECRET, self::TESTING_SECRET); - $metrics = $this->request('GET', 'mamo-connector/metrics', []); + $metrics = $this->request('GET', 'mamo-connector/metrics', [ + "header" => [ + "Hmac" => "dummy" + ] + ]); $content = $metrics->getContent(); static::assertNotFalse($content); static::assertEquals(401, $metrics->getStatusCode()); } + + public function testValidRequestWithHmac(): void + { + /** @var SystemConfigService $systemConfigService */ + $systemConfigService = $this->getContainer()->get(SystemConfigService::class); + + $systemConfigService->set(MobiMamoConnector::CONFIG_KEY_SECRET, self::TESTING_SECRET); + + $content = '{"validateTime":' . mktime(0) . '}'; + + $metrics = $this->request('GET', 'mamo-connector/metrics', [], [], [ + "HTTP_Hmac" => hash_hmac('sha256', '{"validateTime":' . mktime(0) . '}', self::TESTING_SECRET), + ], $content); + $content = $metrics->getContent(); + + static::assertNotFalse($content); + static::assertEquals(200, $metrics->getStatusCode()); + static::assertStringContainsString('mamo_shopware6_platform', $content); + } + + public function testHmacMissingBody(): void + { + /** @var SystemConfigService $systemConfigService */ + $systemConfigService = $this->getContainer()->get(SystemConfigService::class); + + $systemConfigService->set(MobiMamoConnector::CONFIG_KEY_SECRET, self::TESTING_SECRET); + + $content = '{"validateTime":' . mktime(0) . '}'; + + $metrics = $this->request('GET', 'mamo-connector/metrics', [], [], [ + "HTTP_Hmac" => hash_hmac('sha256', '{"validateTime":' . mktime(0) . '}', self::TESTING_SECRET), + ], ""); // <- missing request body + $content = $metrics->getContent(); + + static::assertNotFalse($content); + static::assertEquals(400, $metrics->getStatusCode()); + } + + public function testHmacMismatch(): void + { + /** @var SystemConfigService $systemConfigService */ + $systemConfigService = $this->getContainer()->get(SystemConfigService::class); + + $systemConfigService->set(MobiMamoConnector::CONFIG_KEY_SECRET, self::TESTING_SECRET); + + $content = '{"validateTime":' . mktime(0) . '}'; + + $metrics = $this->request('GET', 'mamo-connector/metrics', [], [], [ + "HTTP_Hmac" => hash_hmac('sha256', '{"validateTime":' . mktime(0) . '}', self::TESTING_SECRET), + ], "hmac-mismatch"); // <- missing request body + $content = $metrics->getContent(); + + static::assertNotFalse($content); + static::assertEquals(403, $metrics->getStatusCode()); + } + + /** + * NOTE: This can only happen, when we change our config template or Shopware itself screws up. + */ + public function testConfigurationSecretIsNotAString(): void + { + /** @var SystemConfigService $systemConfigService */ + $systemConfigService = $this->getContainer()->get(SystemConfigService::class); + + // Screw up the configuration + $systemConfigService->set(MobiMamoConnector::CONFIG_KEY_SECRET, false); + + // Regular controller access + $metrics = $this->request('GET', 'mamo-connector/metrics?unsecure', []); + $content = $metrics->getContent(); + + static::assertNotFalse($content); + static::assertEquals(500, $metrics->getStatusCode()); + } } diff --git a/tests/MobiMamoConnectorTest.php b/tests/MobiMamoConnectorTest.php new file mode 100644 index 0000000..04fda08 --- /dev/null +++ b/tests/MobiMamoConnectorTest.php @@ -0,0 +1,15 @@ +executeComposerCommands()); + } +} diff --git a/tests/Support/StorefrontControllerTestBehaviour.php b/tests/Support/StorefrontControllerTestBehaviour.php new file mode 100644 index 0000000..37fc34c --- /dev/null +++ b/tests/Support/StorefrontControllerTestBehaviour.php @@ -0,0 +1,47 @@ + $data + */ + public function request(string $method, string $path, array $data, array $files = [], array $server = [], string $content = null, bool $changeHistory = true): Response + { + $browser = KernelLifecycleManager::createBrowser($this->getKernel()); + $browser->request($method, EnvironmentHelper::getVariable('APP_URL') . '/' . $path, $data, $files, $server, $content, $changeHistory); + + return $browser->getResponse(); + } + + /** + * @param array $data + * + * @return array + */ + public function tokenize(string $route, array $data): array + { + $requestStack = new RequestStack(); + $request = new Request(); + /** @var Session $session */ + $session = $this->getSession(); + $request->setSession($session); + $requestStack->push($request); + + return $data; + } + + abstract protected static function getKernel(): KernelInterface; + + abstract protected static function getContainer(): ContainerInterface; +} diff --git a/tests/Utility/VersionUtilityTest.php b/tests/Utility/VersionUtilityTest.php index a56d1d4..bdf6b0b 100644 --- a/tests/Utility/VersionUtilityTest.php +++ b/tests/Utility/VersionUtilityTest.php @@ -34,4 +34,9 @@ public function testFourPartVersionNumberWithLastNumberZero(): void $this->assertEquals(6004020000, VersionUtility::convertVersionToInteger('6.4.20.0')); $this->assertEquals('6.4.20.0', VersionUtility::convertIntegerToVersionNumber('6004020000', 4)); } + + public function testMissingVersionNumber(): void + { + $this->assertEquals(1000, VersionUtility::convertVersionToInteger('1.')); + } }