From 509c9fe2e8f782ebf7e1b728017c241d8e37313b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 26 Nov 2019 21:29:25 +0100 Subject: [PATCH] Improved AuthenticationMiddleware API tests --- .../RequestToHttpAuthPlugin.php | 5 +- ...teShortUrlContentNegotiationMiddleware.php | 6 --- .../Action/CreateShortUrlActionTest.php | 3 +- .../Action/DeleteShortUrlActionTest.php | 5 +- .../Action/EditShortUrlActionTagsTest.php | 5 +- .../Action/EditShortUrlActionTest.php | 5 +- .../test-api/Action/GetVisitsActionTest.php | 3 +- .../Action/ResolveShortUrlActionTest.php | 3 +- .../test-api/Action/UpdateTagActionTest.php | 5 +- .../Middleware/AuthenticationTest.php | 50 +++++++++++++++++-- 10 files changed, 57 insertions(+), 33 deletions(-) diff --git a/module/Rest/src/Authentication/RequestToHttpAuthPlugin.php b/module/Rest/src/Authentication/RequestToHttpAuthPlugin.php index f0cc0fe4..9e3e28e0 100644 --- a/module/Rest/src/Authentication/RequestToHttpAuthPlugin.php +++ b/module/Rest/src/Authentication/RequestToHttpAuthPlugin.php @@ -36,10 +36,7 @@ class RequestToHttpAuthPlugin implements RequestToHttpAuthPluginInterface public function fromRequest(ServerRequestInterface $request): Plugin\AuthenticationPluginInterface { if (! $this->hasAnySupportedHeader($request)) { - throw NoAuthenticationException::fromExpectedTypes([ - Plugin\ApiKeyHeaderPlugin::HEADER_NAME, - Plugin\AuthorizationHeaderPlugin::HEADER_NAME, - ]); + throw NoAuthenticationException::fromExpectedTypes(self::SUPPORTED_AUTH_HEADERS); } return $this->authPluginManager->get($this->getFirstAvailableHeader($request)); diff --git a/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php b/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php index 64da53d5..98b64401 100644 --- a/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php +++ b/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php @@ -21,12 +21,6 @@ class CreateShortUrlContentNegotiationMiddleware implements MiddlewareInterface private const PLAIN_TEXT = 'text'; private const JSON = 'json'; - /** - * Process an incoming server request and return a response, optionally delegating - * response creation to a handler. - * @throws \RuntimeException - * @throws \InvalidArgumentException - */ public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { $response = $handler->handle($request); diff --git a/module/Rest/test-api/Action/CreateShortUrlActionTest.php b/module/Rest/test-api/Action/CreateShortUrlActionTest.php index 61c5c7a0..758bb4ac 100644 --- a/module/Rest/test-api/Action/CreateShortUrlActionTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlActionTest.php @@ -6,7 +6,6 @@ namespace ShlinkioApiTest\Shlink\Rest\Action; use Cake\Chronos\Chronos; use GuzzleHttp\RequestOptions; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use function Functional\map; @@ -44,7 +43,7 @@ class CreateShortUrlActionTest extends ApiTestCase [$statusCode, $payload] = $this->createShortUrl(['customSlug' => $slug, 'domain' => $domain]); $this->assertEquals(self::STATUS_BAD_REQUEST, $statusCode); - $this->assertEquals(RestUtils::INVALID_SLUG_ERROR, $payload['error']); + $this->assertEquals('INVALID_SLUG', $payload['error']); } /** @test */ diff --git a/module/Rest/test-api/Action/DeleteShortUrlActionTest.php b/module/Rest/test-api/Action/DeleteShortUrlActionTest.php index 042b8e90..c6cf443e 100644 --- a/module/Rest/test-api/Action/DeleteShortUrlActionTest.php +++ b/module/Rest/test-api/Action/DeleteShortUrlActionTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; class DeleteShortUrlActionTest extends ApiTestCase @@ -16,7 +15,7 @@ class DeleteShortUrlActionTest extends ApiTestCase ['error' => $error] = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_SHORTCODE_ERROR, $error); + $this->assertEquals('INVALID_SHORTCODE', $error); } /** @test */ @@ -31,6 +30,6 @@ class DeleteShortUrlActionTest extends ApiTestCase ['error' => $error] = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_UNPROCESSABLE_ENTITY, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_SHORTCODE_DELETION_ERROR, $error); + $this->assertEquals('INVALID_SHORTCODE_DELETION', $error); } } diff --git a/module/Rest/test-api/Action/EditShortUrlActionTagsTest.php b/module/Rest/test-api/Action/EditShortUrlActionTagsTest.php index d033fcee..e6cae9b6 100644 --- a/module/Rest/test-api/Action/EditShortUrlActionTagsTest.php +++ b/module/Rest/test-api/Action/EditShortUrlActionTagsTest.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; use GuzzleHttp\RequestOptions; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; class EditShortUrlActionTagsTest extends ApiTestCase @@ -17,7 +16,7 @@ class EditShortUrlActionTagsTest extends ApiTestCase ['error' => $error] = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_ARGUMENT_ERROR, $error); + $this->assertEquals('INVALID_ARGUMENT', $error); } /** @test */ @@ -29,6 +28,6 @@ class EditShortUrlActionTagsTest extends ApiTestCase ['error' => $error] = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_SHORTCODE_ERROR, $error); + $this->assertEquals('INVALID_SHORTCODE', $error); } } diff --git a/module/Rest/test-api/Action/EditShortUrlActionTest.php b/module/Rest/test-api/Action/EditShortUrlActionTest.php index 65f2fd1d..739ec191 100644 --- a/module/Rest/test-api/Action/EditShortUrlActionTest.php +++ b/module/Rest/test-api/Action/EditShortUrlActionTest.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; use GuzzleHttp\RequestOptions; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; class EditShortUrlActionTest extends ApiTestCase @@ -17,7 +16,7 @@ class EditShortUrlActionTest extends ApiTestCase ['error' => $error] = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_SHORTCODE_ERROR, $error); + $this->assertEquals('INVALID_SHORTCODE', $error); } /** @test */ @@ -29,6 +28,6 @@ class EditShortUrlActionTest extends ApiTestCase ['error' => $error] = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_ARGUMENT_ERROR, $error); + $this->assertEquals('INVALID_ARGUMENT', $error); } } diff --git a/module/Rest/test-api/Action/GetVisitsActionTest.php b/module/Rest/test-api/Action/GetVisitsActionTest.php index d2c5f6eb..f9c6f404 100644 --- a/module/Rest/test-api/Action/GetVisitsActionTest.php +++ b/module/Rest/test-api/Action/GetVisitsActionTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; class GetVisitsActionTest extends ApiTestCase @@ -16,6 +15,6 @@ class GetVisitsActionTest extends ApiTestCase ['error' => $error] = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_SHORTCODE_ERROR, $error); + $this->assertEquals('INVALID_SHORTCODE', $error); } } diff --git a/module/Rest/test-api/Action/ResolveShortUrlActionTest.php b/module/Rest/test-api/Action/ResolveShortUrlActionTest.php index 4065517e..46f07d53 100644 --- a/module/Rest/test-api/Action/ResolveShortUrlActionTest.php +++ b/module/Rest/test-api/Action/ResolveShortUrlActionTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; class ResolveShortUrlActionTest extends ApiTestCase @@ -16,6 +15,6 @@ class ResolveShortUrlActionTest extends ApiTestCase ['error' => $error] = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_SHORTCODE_ERROR, $error); + $this->assertEquals('INVALID_SHORTCODE', $error); } } diff --git a/module/Rest/test-api/Action/UpdateTagActionTest.php b/module/Rest/test-api/Action/UpdateTagActionTest.php index 0f7c0400..12f37296 100644 --- a/module/Rest/test-api/Action/UpdateTagActionTest.php +++ b/module/Rest/test-api/Action/UpdateTagActionTest.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; use GuzzleHttp\RequestOptions; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; class UpdateTagActionTest extends ApiTestCase @@ -20,7 +19,7 @@ class UpdateTagActionTest extends ApiTestCase ['error' => $error] = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_ARGUMENT_ERROR, $error); + $this->assertEquals('INVALID_ARGUMENT', $error); } public function provideInvalidBody(): iterable @@ -40,6 +39,6 @@ class UpdateTagActionTest extends ApiTestCase ['error' => $error] = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); - $this->assertEquals(RestUtils::NOT_FOUND_ERROR, $error); + $this->assertEquals('TAG_NOT_FOUND', $error); } } diff --git a/module/Rest/test-api/Middleware/AuthenticationTest.php b/module/Rest/test-api/Middleware/AuthenticationTest.php index 02836c85..b918a369 100644 --- a/module/Rest/test-api/Middleware/AuthenticationTest.php +++ b/module/Rest/test-api/Middleware/AuthenticationTest.php @@ -4,9 +4,8 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Middleware; -use Shlinkio\Shlink\Rest\Authentication\Plugin\ApiKeyHeaderPlugin; +use Shlinkio\Shlink\Rest\Authentication\Plugin; use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPlugin; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use function implode; @@ -21,7 +20,7 @@ class AuthenticationTest extends ApiTestCase ['error' => $error, 'message' => $message] = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_UNAUTHORIZED, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_AUTHORIZATION_ERROR, $error); + $this->assertEquals('INVALID_AUTHORIZATION', $error); $this->assertEquals( sprintf( 'Expected one of the following authentication headers, but none were provided, ["%s"]', @@ -39,13 +38,13 @@ class AuthenticationTest extends ApiTestCase { $resp = $this->callApi(self::METHOD_GET, '/short-codes', [ 'headers' => [ - ApiKeyHeaderPlugin::HEADER_NAME => $apiKey, + Plugin\ApiKeyHeaderPlugin::HEADER_NAME => $apiKey, ], ]); ['error' => $error, 'message' => $message] = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_UNAUTHORIZED, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_API_KEY_ERROR, $error); + $this->assertEquals('INVALID_API_KEY', $error); $this->assertEquals('Provided API key does not exist or is invalid.', $message); } @@ -55,4 +54,45 @@ class AuthenticationTest extends ApiTestCase yield 'key which is expired' => ['expired_api_key']; yield 'key which is disabled' => ['disabled_api_key']; } + + /** + * @test + * @dataProvider provideInvalidAuthorizations + */ + public function authorizationErrorIsReturnedIfInvalidDataIsProvided( + string $authValue, + string $expectedMessage, + string $expectedError + ): void { + $resp = $this->callApi(self::METHOD_GET, '/short-codes', [ + 'headers' => [ + Plugin\AuthorizationHeaderPlugin::HEADER_NAME => $authValue, + ], + ]); + ['error' => $error, 'message' => $message] = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_UNAUTHORIZED, $resp->getStatusCode()); + $this->assertEquals($expectedError, $error); + $this->assertEquals($expectedMessage, $message); + } + + public function provideInvalidAuthorizations(): iterable + { + yield 'no type' => [ + 'invalid', + 'You need to provide the Bearer type in the Authorization header.', + 'INVALID_AUTHORIZATION', + ]; + yield 'invalid type' => [ + 'Basic invalid', + 'Provided authorization type Basic is not supported. Use Bearer instead.', + 'INVALID_AUTHORIZATION', + ]; + yield 'invalid JWT' => [ + 'Bearer invalid', + 'Missing or invalid auth token provided. Perform a new authentication request and send provided ' + . 'token on every new request on the Authorization header', + 'INVALID_AUTH_TOKEN', + ]; + } }