diff --git a/module/Rest/config/auth.config.php b/module/Rest/config/auth.config.php index 7acb133e..9d0987e0 100644 --- a/module/Rest/config/auth.config.php +++ b/module/Rest/config/auth.config.php @@ -48,7 +48,6 @@ return [ Middleware\AuthenticationMiddleware::class => [ Authentication\RequestToHttpAuthPlugin::class, 'config.auth.routes_whitelist', - 'Logger_Shlink', ], ], diff --git a/module/Rest/src/Action/AuthenticateAction.php b/module/Rest/src/Action/AuthenticateAction.php index 1476f6de..bfaa745a 100644 --- a/module/Rest/src/Action/AuthenticateAction.php +++ b/module/Rest/src/Action/AuthenticateAction.php @@ -44,7 +44,7 @@ class AuthenticateAction extends AbstractRestAction $authData = $request->getParsedBody(); if (! isset($authData['apiKey'])) { return new JsonResponse([ - 'error' => RestUtils::INVALID_ARGUMENT_ERROR, + 'error' => 'INVALID_ARGUMENT', 'message' => 'You have to provide a valid API key under the "apiKey" param name.', ], self::STATUS_BAD_REQUEST); } @@ -53,7 +53,7 @@ class AuthenticateAction extends AbstractRestAction $apiKey = $this->apiKeyService->getByKey($authData['apiKey']); if ($apiKey === null || ! $apiKey->isValid()) { return new JsonResponse([ - 'error' => RestUtils::INVALID_API_KEY_ERROR, + 'error' => 'INVALID_API_KEY', 'message' => 'Provided API key does not exist or is invalid.', ], self::STATUS_UNAUTHORIZED); } diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php index 9d82c608..1ab26cbb 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php @@ -38,7 +38,7 @@ class EditShortUrlTagsAction extends AbstractRestAction if (! isset($bodyParams['tags'])) { return new JsonResponse([ - 'error' => RestUtils::INVALID_ARGUMENT_ERROR, + 'error' => 'INVALID_ARGUMENT', 'message' => 'A list of tags was not provided', ], self::STATUS_BAD_REQUEST); } diff --git a/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php b/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php index d5cd38f9..14735f9a 100644 --- a/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php +++ b/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php @@ -8,7 +8,6 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; -use Shlinkio\Shlink\Rest\Util\RestUtils; class ApiKeyHeaderPlugin implements AuthenticationPluginInterface { @@ -28,14 +27,9 @@ class ApiKeyHeaderPlugin implements AuthenticationPluginInterface public function verify(ServerRequestInterface $request): void { $apiKey = $request->getHeaderLine(self::HEADER_NAME); - if ($this->apiKeyService->check($apiKey)) { - return; + if (! $this->apiKeyService->check($apiKey)) { + throw VerifyAuthenticationException::forInvalidApiKey(); } - - throw VerifyAuthenticationException::withError( - RestUtils::INVALID_API_KEY_ERROR, - 'Provided API key does not exist or is invalid.' - ); } public function update(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface diff --git a/module/Rest/src/Authentication/Plugin/AuthorizationHeaderPlugin.php b/module/Rest/src/Authentication/Plugin/AuthorizationHeaderPlugin.php index d479d9d3..bf75d09b 100644 --- a/module/Rest/src/Authentication/Plugin/AuthorizationHeaderPlugin.php +++ b/module/Rest/src/Authentication/Plugin/AuthorizationHeaderPlugin.php @@ -8,7 +8,6 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Rest\Authentication\JWTServiceInterface; use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Throwable; use function count; @@ -38,19 +37,13 @@ class AuthorizationHeaderPlugin implements AuthenticationPluginInterface $authToken = $request->getHeaderLine(self::HEADER_NAME); $authTokenParts = explode(' ', $authToken); if (count($authTokenParts) === 1) { - throw VerifyAuthenticationException::withError( - RestUtils::INVALID_AUTHORIZATION_ERROR, - sprintf('You need to provide the Bearer type in the %s header.', self::HEADER_NAME) - ); + throw VerifyAuthenticationException::forMissingAuthType(); } // Make sure the authorization type is Bearer [$authType, $jwt] = $authTokenParts; if (strtolower($authType) !== 'bearer') { - throw VerifyAuthenticationException::withError( - RestUtils::INVALID_AUTHORIZATION_ERROR, - sprintf('Provided authorization type %s is not supported. Use Bearer instead.', $authType) - ); + throw VerifyAuthenticationException::forInvalidAuthType($authType); } try { @@ -58,21 +51,13 @@ class AuthorizationHeaderPlugin implements AuthenticationPluginInterface throw $this->createInvalidTokenError(); } } catch (Throwable $e) { - throw $this->createInvalidTokenError($e); + throw $this->createInvalidTokenError(); } } - private function createInvalidTokenError(?Throwable $prev = null): VerifyAuthenticationException + private function createInvalidTokenError(): VerifyAuthenticationException { - return VerifyAuthenticationException::withError( - RestUtils::INVALID_AUTH_TOKEN_ERROR, - sprintf( - 'Missing or invalid auth token provided. Perform a new authentication request and send provided ' - . 'token on every new request on the %s header', - self::HEADER_NAME - ), - $prev - ); + return VerifyAuthenticationException::forInvalidAuthToken(); } public function update(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface diff --git a/module/Rest/src/Exception/VerifyAuthenticationException.php b/module/Rest/src/Exception/VerifyAuthenticationException.php index 4121685e..8720f421 100644 --- a/module/Rest/src/Exception/VerifyAuthenticationException.php +++ b/module/Rest/src/Exception/VerifyAuthenticationException.php @@ -4,38 +4,81 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Exception; -use Throwable; +use Fig\Http\Message\StatusCodeInterface; +use Zend\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; +use Zend\ProblemDetails\Exception\ProblemDetailsExceptionInterface; use function sprintf; -class VerifyAuthenticationException extends RuntimeException +class VerifyAuthenticationException extends RuntimeException implements ProblemDetailsExceptionInterface { + use CommonProblemDetailsExceptionTrait; + /** @var string */ private $errorCode; /** @var string */ private $publicMessage; - public function __construct( - string $errorCode, - string $publicMessage, - string $message = '', - int $code = 0, - ?Throwable $previous = null - ) { - parent::__construct($message, $code, $previous); - $this->errorCode = $errorCode; - $this->publicMessage = $publicMessage; + public static function forInvalidApiKey(): self + { + $e = new self('Provided API key does not exist or is invalid.'); + + $e->publicMessage = $e->getMessage(); + $e->errorCode = 'INVALID_API_KEY'; + $e->detail = $e->getMessage(); + $e->title = 'Invalid API key'; + $e->type = 'INVALID_API_KEY'; + $e->status = StatusCodeInterface::STATUS_UNAUTHORIZED; + + return $e; } - public static function withError(string $errorCode, string $publicMessage, ?Throwable $prev = null): self + /** @deprecated */ + public static function forInvalidAuthToken(): self { - return new self( - $errorCode, - $publicMessage, - sprintf('Authentication verification failed with the public message "%s"', $publicMessage), - 0, - $prev + $e = new self( + 'Missing or invalid auth token provided. Perform a new authentication request and send provided ' + . 'token on every new request on the Authorization header' ); + + $e->publicMessage = $e->getMessage(); + $e->errorCode = 'INVALID_AUTH_TOKEN'; + $e->detail = $e->getMessage(); + $e->title = 'Invalid auth token'; + $e->type = 'INVALID_AUTH_TOKEN'; + $e->status = StatusCodeInterface::STATUS_UNAUTHORIZED; + + return $e; + } + + /** @deprecated */ + public static function forMissingAuthType(): self + { + $e = new self('You need to provide the Bearer type in the Authorization header.'); + + $e->publicMessage = $e->getMessage(); + $e->errorCode = 'INVALID_AUTHORIZATION'; + $e->detail = $e->getMessage(); + $e->title = 'Invalid authorization'; + $e->type = 'INVALID_AUTHORIZATION'; + $e->status = StatusCodeInterface::STATUS_UNAUTHORIZED; + + return $e; + } + + /** @deprecated */ + public static function forInvalidAuthType(string $providedType): self + { + $e = new self(sprintf('Provided authorization type %s is not supported. Use Bearer instead.', $providedType)); + + $e->publicMessage = $e->getMessage(); + $e->errorCode = 'INVALID_AUTHORIZATION'; + $e->detail = $e->getMessage(); + $e->title = 'Invalid authorization'; + $e->type = 'INVALID_AUTHORIZATION'; + $e->status = StatusCodeInterface::STATUS_UNAUTHORIZED; + + return $e; } public function getErrorCode(): string diff --git a/module/Rest/src/Middleware/AuthenticationMiddleware.php b/module/Rest/src/Middleware/AuthenticationMiddleware.php index 41dec7dc..4fd44bcf 100644 --- a/module/Rest/src/Middleware/AuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/AuthenticationMiddleware.php @@ -10,33 +10,22 @@ use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; -use Psr\Log\LoggerInterface; -use Psr\Log\NullLogger; use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPluginInterface; -use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; -use Shlinkio\Shlink\Rest\Util\RestUtils; -use Zend\Diactoros\Response\JsonResponse; use Zend\Expressive\Router\RouteResult; use function Functional\contains; class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterface, RequestMethodInterface { - /** @var LoggerInterface */ - private $logger; /** @var array */ private $routesWhitelist; /** @var RequestToHttpAuthPluginInterface */ private $requestToAuthPlugin; - public function __construct( - RequestToHttpAuthPluginInterface $requestToAuthPlugin, - array $routesWhitelist, - ?LoggerInterface $logger = null - ) { + public function __construct(RequestToHttpAuthPluginInterface $requestToAuthPlugin, array $routesWhitelist) + { $this->routesWhitelist = $routesWhitelist; $this->requestToAuthPlugin = $requestToAuthPlugin; - $this->logger = $logger ?: new NullLogger(); } public function process(Request $request, RequestHandlerInterface $handler): Response @@ -53,24 +42,9 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa } $plugin = $this->requestToAuthPlugin->fromRequest($request); + $plugin->verify($request); + $response = $handler->handle($request); - try { - $plugin->verify($request); - $response = $handler->handle($request); - return $plugin->update($request, $response); - } catch (VerifyAuthenticationException $e) { - $this->logger->warning('Authentication verification failed. {e}', ['e' => $e]); - return $this->createErrorResponse($e->getPublicMessage(), $e->getErrorCode()); - } - } - - private function createErrorResponse( - string $message, - string $errorCode = RestUtils::INVALID_AUTHORIZATION_ERROR - ): JsonResponse { - return new JsonResponse([ - 'error' => $errorCode, - 'message' => $message, - ], self::STATUS_UNAUTHORIZED); + return $plugin->update($request, $response); } } diff --git a/module/Rest/src/Util/RestUtils.php b/module/Rest/src/Util/RestUtils.php index 1dad3908..a6c349bf 100644 --- a/module/Rest/src/Util/RestUtils.php +++ b/module/Rest/src/Util/RestUtils.php @@ -4,54 +4,10 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Util; -use Shlinkio\Shlink\Common\Exception as Common; -use Shlinkio\Shlink\Core\Exception as Core; use Shlinkio\Shlink\Rest\Exception as Rest; -use Throwable; +/** @deprecated */ class RestUtils { - /** @deprecated */ - public const INVALID_SHORTCODE_ERROR = Core\ShortUrlNotFoundException::TYPE; - /** @deprecated */ - public const INVALID_SHORTCODE_DELETION_ERROR = Core\DeleteShortUrlException::TYPE; - /** @deprecated */ - public const INVALID_URL_ERROR = Core\InvalidUrlException::TYPE; - /** @deprecated */ - public const INVALID_ARGUMENT_ERROR = Core\ValidationException::TYPE; - /** @deprecated */ - public const INVALID_SLUG_ERROR = Core\NonUniqueSlugException::TYPE; - /** @deprecated */ - public const NOT_FOUND_ERROR = Core\TagNotFoundException::TYPE; - /** @deprecated */ - public const UNKNOWN_ERROR = 'UNKNOWN_ERROR'; - - public const INVALID_CREDENTIALS_ERROR = 'INVALID_CREDENTIALS'; - public const INVALID_AUTH_TOKEN_ERROR = 'INVALID_AUTH_TOKEN'; - /** @deprecated */ public const INVALID_AUTHORIZATION_ERROR = Rest\MissingAuthenticationException::TYPE; - public const INVALID_API_KEY_ERROR = 'INVALID_API_KEY'; - - /** @deprecated */ - public static function getRestErrorCodeFromException(Throwable $e): string - { - switch (true) { - case $e instanceof Core\ShortUrlNotFoundException: - return self::INVALID_SHORTCODE_ERROR; - case $e instanceof Core\InvalidUrlException: - return self::INVALID_URL_ERROR; - case $e instanceof Core\NonUniqueSlugException: - return self::INVALID_SLUG_ERROR; - case $e instanceof Common\InvalidArgumentException: - case $e instanceof Core\InvalidArgumentException: - case $e instanceof Core\ValidationException: - return self::INVALID_ARGUMENT_ERROR; - case $e instanceof Rest\AuthenticationException: - return self::INVALID_CREDENTIALS_ERROR; - case $e instanceof Core\DeleteShortUrlException: - return self::INVALID_SHORTCODE_DELETION_ERROR; - default: - return self::UNKNOWN_ERROR; - } - } } diff --git a/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php b/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php index d19fe0f6..9554c62d 100644 --- a/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php +++ b/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php @@ -13,41 +13,11 @@ use Throwable; use function array_map; use function random_int; use function range; -use function sprintf; class VerifyAuthenticationExceptionTest extends TestCase { use StringUtilsTrait; - /** - * @test - * @dataProvider provideExceptionData - */ - public function withErrorCreatesExpectedException(string $code, string $message, ?Throwable $prev): void - { - $e = VerifyAuthenticationException::withError($code, $message, $prev); - - $this->assertEquals(0, $e->getCode()); - $this->assertEquals( - sprintf('Authentication verification failed with the public message "%s"', $message), - $e->getMessage() - ); - $this->assertEquals($code, $e->getErrorCode()); - $this->assertEquals($message, $e->getPublicMessage()); - $this->assertEquals($prev, $e->getPrevious()); - } - - public function provideExceptionData(): iterable - { - return array_map(function () { - return [ - $this->generateRandomString(), - $this->generateRandomString(50), - random_int(0, 1) === 1 ? new Exception('Prev') : null, - ]; - }, range(1, 10)); - } - /** * @test * @dataProvider provideConstructorData diff --git a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php index 054db85e..eb150f57 100644 --- a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php @@ -138,7 +138,7 @@ class AuthenticationMiddlewareTest extends TestCase RouteResult::class, RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []) ); - $e = VerifyAuthenticationException::withError('the_error', 'the_message'); + $e = VerifyAuthenticationException::forInvalidApiKey(); $plugin = $this->prophesize(AuthenticationPluginInterface::class); $verify = $plugin->verify($request)->willThrow($e); diff --git a/module/Rest/test/Util/RestUtilsTest.php b/module/Rest/test/Util/RestUtilsTest.php deleted file mode 100644 index 6097cdc6..00000000 --- a/module/Rest/test/Util/RestUtilsTest.php +++ /dev/null @@ -1,41 +0,0 @@ -assertEquals( - RestUtils::INVALID_SHORTCODE_ERROR, - RestUtils::getRestErrorCodeFromException(new ShortUrlNotFoundException()) - ); - $this->assertEquals( - RestUtils::INVALID_URL_ERROR, - RestUtils::getRestErrorCodeFromException(new InvalidUrlException()) - ); - $this->assertEquals( - RestUtils::INVALID_ARGUMENT_ERROR, - RestUtils::getRestErrorCodeFromException(new InvalidArgumentException()) - ); - $this->assertEquals( - RestUtils::INVALID_CREDENTIALS_ERROR, - RestUtils::getRestErrorCodeFromException(new AuthenticationException()) - ); - $this->assertEquals( - RestUtils::UNKNOWN_ERROR, - RestUtils::getRestErrorCodeFromException(new WrongIpException()) - ); - } -}