From 44a9649ab44713ccc8b31be58b93d5ec26d5d6ef Mon Sep 17 00:00:00 2001 From: Jonathan Lelievre Date: Fri, 23 Feb 2024 00:12:25 +0100 Subject: [PATCH] Force checking protected endpoints in all tests, protected endpoints are also checked with no scopes, deletion tests have been improved with a failing delete to make sure the entity is really not deleted, and client is always created on the fly to prevent Response caching issues --- .../ApiPlatform/ApiClientEndpointTest.php | 98 ++++++++++--------- tests/Integration/ApiPlatform/ApiTestCase.php | 53 ++++++++-- .../ApiPlatform/CustomerGroupApiTest.php | 32 +----- .../ApiPlatform/GetHookStatusTest.php | 13 +++ tests/Integration/ApiPlatform/GetHookTest.php | 8 ++ .../ApiPlatform/ProductEndpointTest.php | 55 +++++------ .../ProductMultiShopEndpointTest.php | 20 ++-- 7 files changed, 161 insertions(+), 118 deletions(-) diff --git a/tests/Integration/ApiPlatform/ApiClientEndpointTest.php b/tests/Integration/ApiPlatform/ApiClientEndpointTest.php index 6064581..bb75b4d 100644 --- a/tests/Integration/ApiPlatform/ApiClientEndpointTest.php +++ b/tests/Integration/ApiPlatform/ApiClientEndpointTest.php @@ -36,25 +36,6 @@ public static function setUpBeforeClass(): void self::createApiClient(['api_client_write', 'api_client_read']); } - /** - * @dataProvider getProtectedEndpoints - * - * @param string $method - * @param string $uri - */ - public function testProtectedEndpoints(string $method, string $uri, string $contentType = 'application/json'): void - { - $options['headers']['content-type'] = $contentType; - // Check that endpoints are not accessible without a proper Bearer token - $client = static::createClient([], $options); - $response = $client->request($method, $uri); - self::assertResponseStatusCodeSame(401); - - $content = $response->getContent(false); - $this->assertNotEmpty($content); - $this->assertEquals('No Authorization header provided', $content); - } - public function getProtectedEndpoints(): iterable { yield 'get endpoint' => [ @@ -68,22 +49,20 @@ public function getProtectedEndpoints(): iterable ]; yield 'update endpoint' => [ - 'PUT', + 'PATCH', '/api/api-client/1', ]; yield 'delete endpoint' => [ 'DELETE', '/api/api-client/1', - 'application/merge-patch+json', ]; } public function testAddApiClient(): int { $bearerToken = $this->getBearerToken(['api_client_write']); - $client = static::createClient(); - $response = $client->request('POST', '/api/api-client', [ + $response = static::createClient()->request('POST', '/api/api-client', [ 'auth_bearer' => $bearerToken, 'json' => [ 'clientId' => 'client_id_test', @@ -125,8 +104,7 @@ public function testAddApiClient(): int public function testGetApiClient(int $apiClientId): int { $bearerToken = $this->getBearerToken(['api_client_read']); - $client = static::createClient(); - $response = $client->request('GET', '/api/api-client/' . $apiClientId, [ + $response = static::createClient()->request('GET', '/api/api-client/' . $apiClientId, [ 'auth_bearer' => $bearerToken, ]); self::assertResponseStatusCodeSame(200); @@ -162,10 +140,9 @@ public function testGetApiClient(int $apiClientId): int public function testUpdateApiClient(int $apiClientId): int { $bearerToken = $this->getBearerToken(['api_client_write']); - $client = static::createClient(); - // Update product with partial data, even multilang fields can be updated language by language - $response = $client->request('PUT', '/api/api-client/' . $apiClientId, [ + // Update API client + $response = static::createClient()->request('PATCH', '/api/api-client/' . $apiClientId, [ 'auth_bearer' => $bearerToken, 'json' => [ 'clientId' => 'client_id_test_updated', @@ -200,6 +177,35 @@ public function testUpdateApiClient(int $apiClientId): int $decodedResponse ); + // Update partially API client + $response = static::createClient()->request('PATCH', '/api/api-client/' . $apiClientId, [ + 'auth_bearer' => $bearerToken, + 'json' => [ + 'description' => 'Client description test partially updated', + 'lifetime' => 900, + ], + ]); + self::assertResponseStatusCodeSame(200); + + $decodedResponse = json_decode($response->getContent(), true); + $this->assertNotFalse($decodedResponse); + // Returned data has modified fields, the others haven't changed + $this->assertEquals( + [ + 'apiClientId' => $apiClientId, + 'clientId' => 'client_id_test_updated', + 'clientName' => 'Client name test updated', + 'description' => 'Client description test partially updated', + 'enabled' => false, + 'lifetime' => 900, + 'scopes' => [ + 'api_client_write', + 'hook_read', + ], + ], + $decodedResponse + ); + return $apiClientId; } @@ -213,8 +219,7 @@ public function testUpdateApiClient(int $apiClientId): int public function testGetUpdatedApiClient(int $apiClientId): int { $bearerToken = $this->getBearerToken(['api_client_read']); - $client = static::createClient(); - $response = $client->request('GET', '/api/api-client/' . $apiClientId, [ + $response = static::createClient()->request('GET', '/api/api-client/' . $apiClientId, [ 'auth_bearer' => $bearerToken, ]); self::assertResponseStatusCodeSame(200); @@ -226,9 +231,9 @@ public function testGetUpdatedApiClient(int $apiClientId): int 'apiClientId' => $apiClientId, 'clientId' => 'client_id_test_updated', 'clientName' => 'Client name test updated', - 'description' => 'Client description test updated', + 'description' => 'Client description test partially updated', 'enabled' => false, - 'lifetime' => 1800, + 'lifetime' => 900, 'scopes' => [ 'api_client_write', 'hook_read', @@ -247,34 +252,39 @@ public function testGetUpdatedApiClient(int $apiClientId): int */ public function testDeleteApiClient(int $apiClientId): void { - $bearerToken = $this->getBearerToken(['api_client_read', 'api_client_write']); - $client = static::createClient(); - // Delete API client without token - $client->request('DELETE', '/api/api-client/' . $apiClientId); + static::createClient()->request('DELETE', '/api/api-client/' . $apiClientId); self::assertResponseStatusCodeSame(401); // Delete API client without token - $client->request('DELETE', '/api/api-client/' . $apiClientId, [ + static::createClient()->request('DELETE', '/api/api-client/' . $apiClientId, [ 'auth_bearer' => 'toto', ]); self::assertResponseStatusCodeSame(401); + // Try to delete with a token with only read scope + $readBearerToken = $this->getBearerToken(['api_client_read']); + $response = static::createClient()->request('DELETE', '/api/api-client/' . $apiClientId, [ + 'auth_bearer' => $readBearerToken, + ]); + $this->assertEquals(403, $response->getStatusCode()); + self::assertResponseStatusCodeSame(403); + // Check that API client was not deleted - $client->request('GET', '/api/api-client/' . $apiClientId, [ - 'auth_bearer' => $bearerToken, + static::createClient()->request('GET', '/api/api-client/' . $apiClientId, [ + 'auth_bearer' => $readBearerToken, ]); self::assertResponseStatusCodeSame(200); // Delete API client with valid token - $response = $client->request('DELETE', '/api/api-client/' . $apiClientId, [ - 'auth_bearer' => $bearerToken, + $writeBearerToken = $this->getBearerToken(['api_client_write']); + $response = static::createClient()->request('DELETE', '/api/api-client/' . $apiClientId, [ + 'auth_bearer' => $writeBearerToken, ]); self::assertResponseStatusCodeSame(204); $this->assertEmpty($response->getContent()); - $client = static::createClient(); - $client->request('GET', '/api/api-client/' . $apiClientId, [ - 'auth_bearer' => $bearerToken, + static::createClient()->request('GET', '/api/api-client/' . $apiClientId, [ + 'auth_bearer' => $readBearerToken, ]); self::assertResponseStatusCodeSame(404); } diff --git a/tests/Integration/ApiPlatform/ApiTestCase.php b/tests/Integration/ApiPlatform/ApiTestCase.php index 4728be4..f3e03b4 100644 --- a/tests/Integration/ApiPlatform/ApiTestCase.php +++ b/tests/Integration/ApiPlatform/ApiTestCase.php @@ -56,6 +56,50 @@ public static function tearDownAfterClass(): void self::$clientSecret = null; } + /** + * @dataProvider getProtectedEndpoints + * + * @param string $method + * @param string $uri + */ + public function testProtectedEndpoints(string $method, string $uri, string $contentType = 'application/json'): void + { + $options['headers']['content-type'] = $contentType; + // Check that endpoints are not accessible without a proper Bearer token + $response = static::createClient([], $options)->request($method, $uri); + self::assertResponseStatusCodeSame(401); + + $content = $response->getContent(false); + $this->assertNotEmpty($content); + $this->assertEquals('No Authorization header provided', $content); + + // Test same endpoint with a token but without scopes + $emptyBearerToken = $this->getBearerToken(); + static::createClient([], $options)->request($method, $uri, ['auth_bearer' => $emptyBearerToken]); + self::assertResponseStatusCodeSame(403); + } + + /** + * You must provide a list of protected endpoints that will we automatically checked, + * the test will check that the endpoints are not accessible when no token is specified + * AND that they are not accessible when the no particular scope is specified. + * + * You should use yield return like this: + * + * yield 'get endpoint' => [ + * 'GET', + * '/api/product/1', + * ]; + * + * Since all Api Platform resources should likely have some protected endpoints this provider + * method was made abstract to force its implementation. In the unlikely event you need to use + * this class on a resouce with absolutely no protected endpoints you can still implement this + * method and return new \EmptyIterator(); + * + * @return iterable + */ + abstract public function getProtectedEndpoints(): iterable; + protected static function createClient(array $kernelOptions = [], array $defaultOptions = []): Client { if (!isset($defaultOptions['headers']['accept'])) { @@ -74,7 +118,6 @@ protected function getBearerToken(array $scopes = []): string if (null === self::$clientSecret) { self::createApiClient($scopes); } - $client = static::createClient(); $parameters = ['parameters' => [ 'client_id' => static::CLIENT_ID, 'client_secret' => static::$clientSecret, @@ -87,14 +130,13 @@ protected function getBearerToken(array $scopes = []): string 'content-type' => 'application/x-www-form-urlencoded', ], ]; - $response = $client->request('POST', '/api/oauth2/token', $options); + $response = static::createClient()->request('POST', '/api/oauth2/token', $options); return json_decode($response->getContent())->access_token; } protected static function createApiClient(array $scopes = [], int $lifetime = 10000): void { - $client = static::createClient(); $command = new AddApiClientCommand( static::CLIENT_NAME, static::CLIENT_ID, @@ -104,7 +146,7 @@ protected static function createApiClient(array $scopes = [], int $lifetime = 10 $scopes ); - $container = $client->getContainer(); + $container = static::createClient()->getContainer(); $commandBus = $container->get('prestashop.core.command_bus'); $createdApiClient = $commandBus->handle($command); @@ -113,7 +155,6 @@ protected static function createApiClient(array $scopes = [], int $lifetime = 10 protected static function addLanguageByLocale(string $locale): int { - $client = static::createClient(); $isoCode = substr($locale, 0, strpos($locale, '-')); // Copy resource assets into tmp folder to mimic an upload file path @@ -140,7 +181,7 @@ protected static function addLanguageByLocale(string $locale): int [1] ); - $container = $client->getContainer(); + $container = static::createClient()->getContainer(); $commandBus = $container->get('prestashop.core.command_bus'); return $commandBus->handle($command)->getValue(); diff --git a/tests/Integration/ApiPlatform/CustomerGroupApiTest.php b/tests/Integration/ApiPlatform/CustomerGroupApiTest.php index 848792c..723a5f9 100644 --- a/tests/Integration/ApiPlatform/CustomerGroupApiTest.php +++ b/tests/Integration/ApiPlatform/CustomerGroupApiTest.php @@ -47,23 +47,6 @@ public static function tearDownAfterClass(): void DatabaseDump::restoreTables(['group', 'group_lang', 'group_reduction', 'group_shop', 'category_group']); } - /** - * @dataProvider getProtectedEndpoints - * - * @param string $method - * @param string $uri - */ - public function testProtectedEndpoints(string $method, string $uri): void - { - $client = static::createClient(); - $response = $client->request($method, $uri); - self::assertResponseStatusCodeSame(401); - - $content = $response->getContent(false); - $this->assertNotEmpty($content); - $this->assertEquals('No Authorization header provided', $content); - } - public function getProtectedEndpoints(): iterable { yield 'get endpoint' => [ @@ -92,8 +75,7 @@ public function testAddCustomerGroup(): int $numberOfGroups = count(\Group::getGroups(\Context::getContext()->language->id)); $bearerToken = $this->getBearerToken(['customer_group_write']); - $client = static::createClient(); - $response = $client->request('POST', '/api/customers/group', [ + $response = static::createClient()->request('POST', '/api/customers/group', [ 'auth_bearer' => $bearerToken, 'json' => [ 'localizedNames' => [ @@ -141,9 +123,8 @@ public function testUpdateCustomerGroup(int $customerGroupId): int $numberOfGroups = count(\Group::getGroups(\Context::getContext()->language->id)); $bearerToken = $this->getBearerToken(['customer_group_write']); - $client = static::createClient(); // Update customer group with partial data - $response = $client->request('PUT', '/api/customers/group/' . $customerGroupId, [ + $response = static::createClient()->request('PUT', '/api/customers/group/' . $customerGroupId, [ 'auth_bearer' => $bearerToken, 'json' => [ 'localizedNames' => [ @@ -187,8 +168,7 @@ public function testUpdateCustomerGroup(int $customerGroupId): int public function testGetCustomerGroup(int $customerGroupId): int { $bearerToken = $this->getBearerToken(['customer_group_read']); - $client = static::createClient(); - $response = $client->request('GET', '/api/customers/group/' . $customerGroupId, [ + $response = static::createClient()->request('GET', '/api/customers/group/' . $customerGroupId, [ 'auth_bearer' => $bearerToken, ]); self::assertResponseStatusCodeSame(200); @@ -223,16 +203,14 @@ public function testGetCustomerGroup(int $customerGroupId): int public function testDeleteCustomerGroup(int $customerGroupId): void { $bearerToken = $this->getBearerToken(['customer_group_read', 'customer_group_write']); - $client = static::createClient(); // Update customer group with partial data - $response = $client->request('DELETE', '/api/customers/group/' . $customerGroupId, [ + $response = static::createClient()->request('DELETE', '/api/customers/group/' . $customerGroupId, [ 'auth_bearer' => $bearerToken, ]); self::assertResponseStatusCodeSame(204); $this->assertEmpty($response->getContent()); - $client = static::createClient(); - $client->request('GET', '/api/customers/group/' . $customerGroupId, [ + static::createClient()->request('GET', '/api/customers/group/' . $customerGroupId, [ 'auth_bearer' => $bearerToken, ]); self::assertResponseStatusCodeSame(404); diff --git a/tests/Integration/ApiPlatform/GetHookStatusTest.php b/tests/Integration/ApiPlatform/GetHookStatusTest.php index 3f53338..c04b845 100644 --- a/tests/Integration/ApiPlatform/GetHookStatusTest.php +++ b/tests/Integration/ApiPlatform/GetHookStatusTest.php @@ -44,6 +44,19 @@ public static function tearDownAfterClass(): void DatabaseDump::restoreTables(['hook']); } + public function getProtectedEndpoints(): iterable + { + yield 'get endpoint' => [ + 'GET', + '/api/hook-status/1', + ]; + + yield 'put endpoint' => [ + 'PUT', + '/api/hook-status', + ]; + } + public function testGetHookStatus(): void { $inactiveHook = new \Hook(); diff --git a/tests/Integration/ApiPlatform/GetHookTest.php b/tests/Integration/ApiPlatform/GetHookTest.php index e6c109f..9b23c6b 100644 --- a/tests/Integration/ApiPlatform/GetHookTest.php +++ b/tests/Integration/ApiPlatform/GetHookTest.php @@ -45,6 +45,14 @@ public static function tearDownAfterClass(): void DatabaseDump::restoreTables(['hook']); } + public function getProtectedEndpoints(): iterable + { + yield 'get endpoint' => [ + 'GET', + '/api/hooks/1', + ]; + } + public function testGetHook(): void { $hook = new \Hook(); diff --git a/tests/Integration/ApiPlatform/ProductEndpointTest.php b/tests/Integration/ApiPlatform/ProductEndpointTest.php index fcf45f9..effa9de 100644 --- a/tests/Integration/ApiPlatform/ProductEndpointTest.php +++ b/tests/Integration/ApiPlatform/ProductEndpointTest.php @@ -61,25 +61,6 @@ public static function tearDownAfterClass(): void (new ResourceResetter())->resetTestModules(); } - /** - * @dataProvider getProtectedEndpoints - * - * @param string $method - * @param string $uri - */ - public function testProtectedEndpoints(string $method, string $uri, string $contentType = 'application/json'): void - { - $options['headers']['content-type'] = $contentType; - // Check that endpoints are not accessible without a proper Bearer token - $client = static::createClient([], $options); - $response = $client->request($method, $uri); - self::assertResponseStatusCodeSame(401); - - $content = $response->getContent(false); - $this->assertNotEmpty($content); - $this->assertEquals('No Authorization header provided', $content); - } - public function getProtectedEndpoints(): iterable { yield 'get endpoint' => [ @@ -108,8 +89,7 @@ public function testAddProduct(): int { $productsNumber = $this->getProductsNumber(); $bearerToken = $this->getBearerToken(['product_write']); - $client = static::createClient(); - $response = $client->request('POST', '/api/product', [ + $response = static::createClient()->request('POST', '/api/product', [ 'auth_bearer' => $bearerToken, 'json' => [ 'type' => ProductType::TYPE_STANDARD, @@ -158,10 +138,9 @@ public function testPartialUpdateProduct(int $productId): int { $productsNumber = $this->getProductsNumber(); $bearerToken = $this->getBearerToken(['product_write']); - $client = static::createClient(); // Update product with partial data, even multilang fields can be updated language by language - $response = $client->request('PATCH', '/api/product/' . $productId, [ + $response = static::createClient()->request('PATCH', '/api/product/' . $productId, [ 'auth_bearer' => $bearerToken, 'headers' => [ 'content-type' => 'application/merge-patch+json', @@ -201,7 +180,7 @@ public function testPartialUpdateProduct(int $productId): int ); // Update product with partial data, only name default language the other names are not impacted - $response = $client->request('PATCH', '/api/product/' . $productId, [ + $response = static::createClient()->request('PATCH', '/api/product/' . $productId, [ 'auth_bearer' => $bearerToken, 'headers' => [ 'content-type' => 'application/merge-patch+json', @@ -244,8 +223,7 @@ public function testPartialUpdateProduct(int $productId): int public function testGetProduct(int $productId): int { $bearerToken = $this->getBearerToken(['product_read']); - $client = static::createClient(); - $response = $client->request('GET', '/api/product/' . $productId, [ + $response = static::createClient()->request('GET', '/api/product/' . $productId, [ 'auth_bearer' => $bearerToken, ]); self::assertResponseStatusCodeSame(200); @@ -281,11 +259,22 @@ public function testGetProduct(int $productId): int public function testDeleteProduct(int $productId): void { $productsNumber = $this->getProductsNumber(); - $bearerToken = $this->getBearerToken(['product_read', 'product_write']); - $client = static::createClient(); - // Delete product - $response = $client->request('DELETE', '/api/product/' . $productId, [ - 'auth_bearer' => $bearerToken, + $readBearerToken = $this->getBearerToken(['product_read']); + // Delete product with token without write permission + static::createClient()->request('DELETE', '/api/product/' . $productId, [ + 'auth_bearer' => $readBearerToken, + ]); + self::assertResponseStatusCodeSame(403); + // The product should still exists + static::createClient()->request('GET', '/api/product/' . $productId, [ + 'auth_bearer' => $readBearerToken, + ]); + self::assertResponseStatusCodeSame(200); + + // Delete product with proper token + $writeBearerToken = $this->getBearerToken(['product_write']); + $response = static::createClient()->request('DELETE', '/api/product/' . $productId, [ + 'auth_bearer' => $writeBearerToken, ]); self::assertResponseStatusCodeSame(204); $this->assertEmpty($response->getContent()); @@ -293,8 +282,8 @@ public function testDeleteProduct(int $productId): void // One less products $this->assertEquals($productsNumber - 1, $this->getProductsNumber()); - $client = static::createClient(); - $client->request('GET', '/api/product/' . $productId, [ + $bearerToken = $this->getBearerToken(['product_read', 'product_write']); + static::createClient()->request('GET', '/api/product/' . $productId, [ 'auth_bearer' => $bearerToken, ]); self::assertResponseStatusCodeSame(404); diff --git a/tests/Integration/ApiPlatform/ProductMultiShopEndpointTest.php b/tests/Integration/ApiPlatform/ProductMultiShopEndpointTest.php index f935f1e..ee4aab2 100644 --- a/tests/Integration/ApiPlatform/ProductMultiShopEndpointTest.php +++ b/tests/Integration/ApiPlatform/ProductMultiShopEndpointTest.php @@ -85,6 +85,14 @@ public static function setUpBeforeClass(): void ]; } + public function getProtectedEndpoints(): iterable + { + yield 'get endpoint' => [ + 'GET', + '/api/product/1', + ]; + } + public static function tearDownAfterClass(): void { parent::tearDownAfterClass(); @@ -99,8 +107,7 @@ public static function tearDownAfterClass(): void public function testShopContextIsRequired(): void { $bearerToken = $this->getBearerToken(['product_write']); - $client = static::createClient(); - $response = $client->request('POST', '/api/product', [ + $response = static::createClient()->request('POST', '/api/product', [ 'auth_bearer' => $bearerToken, 'json' => [ 'type' => ProductType::TYPE_STANDARD, @@ -118,8 +125,7 @@ public function testShopContextIsRequired(): void public function testCreateProductForFirstShop(): int { $bearerToken = $this->getBearerToken(['product_write']); - $client = static::createClient(); - $response = $client->request('POST', '/api/product', [ + $response = static::createClient()->request('POST', '/api/product', [ 'auth_bearer' => $bearerToken, 'json' => [ 'type' => ProductType::TYPE_STANDARD, @@ -155,8 +161,7 @@ public function testCreateProductForFirstShop(): int public function testGetProductForFirstShopIsSuccessful(int $productId): int { $bearerToken = $this->getBearerToken(['product_read']); - $client = static::createClient(); - $response = $client->request('GET', '/api/product/' . $productId, [ + $response = static::createClient()->request('GET', '/api/product/' . $productId, [ 'auth_bearer' => $bearerToken, 'extra' => [ 'parameters' => [ @@ -180,8 +185,7 @@ public function testGetProductForFirstShopIsSuccessful(int $productId): int public function testGetProductForSecondShopIsFailing(int $productId): int { $bearerToken = $this->getBearerToken(['product_read']); - $client = static::createClient(); - $response = $client->request('GET', '/api/product/' . $productId, [ + $response = static::createClient()->request('GET', '/api/product/' . $productId, [ 'auth_bearer' => $bearerToken, 'extra' => [ 'parameters' => [