diff --git a/module/Core/src/Exception/DeleteShortUrlException.php b/module/Core/src/Exception/DeleteShortUrlException.php index 037fe942..984c2cbc 100644 --- a/module/Core/src/Exception/DeleteShortUrlException.php +++ b/module/Core/src/Exception/DeleteShortUrlException.php @@ -16,7 +16,7 @@ class DeleteShortUrlException extends DomainException implements ProblemDetailsE use CommonProblemDetailsExceptionTrait; private const TITLE = 'Cannot delete short URL'; - public const TYPE = 'INVALID_SHORTCODE_DELETION'; // FIXME Should be INVALID_SHORT_URL_DELETION + private const TYPE = 'INVALID_SHORTCODE_DELETION'; // FIXME Should be INVALID_SHORT_URL_DELETION /** @var int */ private $visitsThreshold; diff --git a/module/Core/src/Exception/InvalidUrlException.php b/module/Core/src/Exception/InvalidUrlException.php index 32758b4e..0b741910 100644 --- a/module/Core/src/Exception/InvalidUrlException.php +++ b/module/Core/src/Exception/InvalidUrlException.php @@ -16,7 +16,7 @@ class InvalidUrlException extends DomainException implements ProblemDetailsExcep use CommonProblemDetailsExceptionTrait; private const TITLE = 'Invalid URL'; - public const TYPE = 'INVALID_URL'; + private const TYPE = 'INVALID_URL'; public static function fromUrl(string $url, ?Throwable $previous = null): self { diff --git a/module/Core/src/Exception/NonUniqueSlugException.php b/module/Core/src/Exception/NonUniqueSlugException.php index 74e03f06..9ef7365d 100644 --- a/module/Core/src/Exception/NonUniqueSlugException.php +++ b/module/Core/src/Exception/NonUniqueSlugException.php @@ -15,7 +15,7 @@ class NonUniqueSlugException extends InvalidArgumentException implements Problem use CommonProblemDetailsExceptionTrait; private const TITLE = 'Invalid custom slug'; - public const TYPE = 'INVALID_SLUG'; + private const TYPE = 'INVALID_SLUG'; public static function fromSlug(string $slug, ?string $domain): self { diff --git a/module/Core/src/Exception/ShortUrlNotFoundException.php b/module/Core/src/Exception/ShortUrlNotFoundException.php index e07624c7..9617a486 100644 --- a/module/Core/src/Exception/ShortUrlNotFoundException.php +++ b/module/Core/src/Exception/ShortUrlNotFoundException.php @@ -15,7 +15,7 @@ class ShortUrlNotFoundException extends DomainException implements ProblemDetail use CommonProblemDetailsExceptionTrait; private const TITLE = 'Short URL not found'; - public const TYPE = 'INVALID_SHORTCODE'; + private const TYPE = 'INVALID_SHORTCODE'; public static function fromNotFoundShortCode(string $shortCode, ?string $domain = null): self { diff --git a/module/Core/src/Exception/TagNotFoundException.php b/module/Core/src/Exception/TagNotFoundException.php index 6ff36bb2..33cf1345 100644 --- a/module/Core/src/Exception/TagNotFoundException.php +++ b/module/Core/src/Exception/TagNotFoundException.php @@ -15,7 +15,7 @@ class TagNotFoundException extends DomainException implements ProblemDetailsExce use CommonProblemDetailsExceptionTrait; private const TITLE = 'Tag not found'; - public const TYPE = 'TAG_NOT_FOUND'; + private const TYPE = 'TAG_NOT_FOUND'; public static function fromTag(string $tag): self { diff --git a/module/Core/src/Exception/ValidationException.php b/module/Core/src/Exception/ValidationException.php index eea23a5a..c4ff2354 100644 --- a/module/Core/src/Exception/ValidationException.php +++ b/module/Core/src/Exception/ValidationException.php @@ -22,7 +22,7 @@ class ValidationException extends InvalidArgumentException implements ProblemDet use CommonProblemDetailsExceptionTrait; private const TITLE = 'Invalid data'; - public const TYPE = 'INVALID_ARGUMENT'; + private const TYPE = 'INVALID_ARGUMENT'; /** @var array */ private $invalidElements; diff --git a/module/Rest/src/Action/AuthenticateAction.php b/module/Rest/src/Action/AuthenticateAction.php index bfaa745a..b0919aae 100644 --- a/module/Rest/src/Action/AuthenticateAction.php +++ b/module/Rest/src/Action/AuthenticateAction.php @@ -10,7 +10,6 @@ use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Rest\Authentication\JWTServiceInterface; use Shlinkio\Shlink\Rest\Service\ApiKeyService; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; /** @deprecated */ diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php index 1ab26cbb..3f057e75 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php @@ -9,7 +9,6 @@ use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; class EditShortUrlTagsAction extends AbstractRestAction diff --git a/module/Rest/src/Exception/MissingAuthenticationException.php b/module/Rest/src/Exception/MissingAuthenticationException.php index 6ab55458..6ed76e2a 100644 --- a/module/Rest/src/Exception/MissingAuthenticationException.php +++ b/module/Rest/src/Exception/MissingAuthenticationException.php @@ -16,7 +16,7 @@ class MissingAuthenticationException extends RuntimeException implements Problem use CommonProblemDetailsExceptionTrait; private const TITLE = 'Invalid authorization'; - public const TYPE = 'INVALID_AUTHORIZATION'; + private const TYPE = 'INVALID_AUTHORIZATION'; public static function fromExpectedTypes(array $expectedTypes): self { diff --git a/module/Rest/src/Exception/VerifyAuthenticationException.php b/module/Rest/src/Exception/VerifyAuthenticationException.php index 8720f421..3b2a4115 100644 --- a/module/Rest/src/Exception/VerifyAuthenticationException.php +++ b/module/Rest/src/Exception/VerifyAuthenticationException.php @@ -14,17 +14,10 @@ class VerifyAuthenticationException extends RuntimeException implements ProblemD { use CommonProblemDetailsExceptionTrait; - /** @var string */ - private $errorCode; - /** @var string */ - private $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'; @@ -41,8 +34,6 @@ class VerifyAuthenticationException extends RuntimeException implements ProblemD . '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'; @@ -56,8 +47,6 @@ class VerifyAuthenticationException extends RuntimeException implements ProblemD { $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'; @@ -71,8 +60,6 @@ class VerifyAuthenticationException extends RuntimeException implements ProblemD { $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'; @@ -80,14 +67,4 @@ class VerifyAuthenticationException extends RuntimeException implements ProblemD return $e; } - - public function getErrorCode(): string - { - return $this->errorCode; - } - - public function getPublicMessage(): string - { - return $this->publicMessage; - } } diff --git a/module/Rest/src/Util/RestUtils.php b/module/Rest/src/Util/RestUtils.php deleted file mode 100644 index a6c349bf..00000000 --- a/module/Rest/src/Util/RestUtils.php +++ /dev/null @@ -1,13 +0,0 @@ -expectException(MissingAuthenticationException::class); $this->expectExceptionMessage(sprintf( - 'None of the valid authentication mechanisms where provided. Expected one of ["%s"]', + 'Expected one of the following authentication headers, but none were provided, ["%s"]', implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS) )); diff --git a/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php b/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php index 9554c62d..28563c5f 100644 --- a/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php +++ b/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php @@ -4,62 +4,16 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Exception; -use Exception; use PHPUnit\Framework\TestCase; -use Shlinkio\Shlink\Common\Util\StringUtilsTrait; use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; -use Throwable; - -use function array_map; -use function random_int; -use function range; class VerifyAuthenticationExceptionTest extends TestCase { - use StringUtilsTrait; - - /** - * @test - * @dataProvider provideConstructorData - */ - public function constructCreatesExpectedException( - string $errorCode, - string $publicMessage, - string $message, - int $code, - ?Throwable $prev - ): void { - $e = new VerifyAuthenticationException($errorCode, $publicMessage, $message, $code, $prev); - - $this->assertEquals($code, $e->getCode()); - $this->assertEquals($message, $e->getMessage()); - $this->assertEquals($errorCode, $e->getErrorCode()); - $this->assertEquals($publicMessage, $e->getPublicMessage()); - $this->assertEquals($prev, $e->getPrevious()); - } - - public function provideConstructorData(): iterable - { - return array_map(function (int $i) { - return [ - $this->generateRandomString(), - $this->generateRandomString(30), - $this->generateRandomString(50), - $i, - random_int(0, 1) === 1 ? new Exception('Prev') : null, - ]; - }, range(10, 20)); - } - /** @test */ - public function defaultConstructorValuesAreKept(): void + public function createsExpectedExceptionForInvalidApiKey(): void { - $e = new VerifyAuthenticationException('foo', 'bar'); + $e = VerifyAuthenticationException::forInvalidApiKey(); - $this->assertEquals(0, $e->getCode()); - $this->assertEquals('', $e->getMessage()); - $this->assertEquals('foo', $e->getErrorCode()); - $this->assertEquals('bar', $e->getPublicMessage()); - $this->assertNull($e->getPrevious()); + $this->assertEquals('Provided API key does not exist or is invalid.', $e->getMessage()); } } diff --git a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php index eb150f57..36dfaeee 100644 --- a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php @@ -4,12 +4,10 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Middleware; -use Exception; use Fig\Http\Message\RequestMethodInterface; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; -use Psr\Container\ContainerExceptionInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; @@ -17,20 +15,13 @@ use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Rest\Action\AuthenticateAction; use Shlinkio\Shlink\Rest\Authentication\Plugin\AuthenticationPluginInterface; -use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPlugin; use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPluginInterface; -use Shlinkio\Shlink\Rest\Exception\MissingAuthenticationException; -use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; -use Shlinkio\Shlink\Rest\Util\RestUtils; -use Throwable; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequest; use Zend\Expressive\Router\Route; use Zend\Expressive\Router\RouteResult; -use function implode; -use function sprintf; use function Zend\Stratigility\middleware; class AuthenticationMiddlewareTest extends TestCase @@ -93,72 +84,6 @@ class AuthenticationMiddlewareTest extends TestCase )->withMethod(RequestMethodInterface::METHOD_OPTIONS)]; } - /** - * @test - * @dataProvider provideExceptions - */ - public function errorIsReturnedWhenNoValidAuthIsProvided(Throwable $e): void - { - $request = (new ServerRequest())->withAttribute( - RouteResult::class, - RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []) - ); - $fromRequest = $this->requestToPlugin->fromRequest(Argument::any())->willThrow($e); - $logWarning = $this->logger->warning('Invalid or no authentication provided. {e}', ['e' => $e])->will( - function () { - } - ); - - /** @var Response\JsonResponse $response */ - $response = $this->middleware->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); - $payload = $response->getPayload(); - - $this->assertEquals(RestUtils::INVALID_AUTHORIZATION_ERROR, $payload['error']); - $this->assertEquals(sprintf( - 'Expected one of the following authentication headers, but none were provided, ["%s"]', - implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS) - ), $payload['message']); - $fromRequest->shouldHaveBeenCalledOnce(); - $logWarning->shouldHaveBeenCalledOnce(); - } - - public function provideExceptions(): iterable - { - $containerException = new class extends Exception implements ContainerExceptionInterface { - }; - - yield 'container exception' => [$containerException]; - yield 'authentication exception' => [MissingAuthenticationException::fromExpectedTypes([])]; - } - - /** @test */ - public function errorIsReturnedWhenVerificationFails(): void - { - $request = (new ServerRequest())->withAttribute( - RouteResult::class, - RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []) - ); - $e = VerifyAuthenticationException::forInvalidApiKey(); - $plugin = $this->prophesize(AuthenticationPluginInterface::class); - - $verify = $plugin->verify($request)->willThrow($e); - $fromRequest = $this->requestToPlugin->fromRequest(Argument::any())->willReturn($plugin->reveal()); - $logWarning = $this->logger->warning('Authentication verification failed. {e}', ['e' => $e])->will( - function () { - } - ); - - /** @var Response\JsonResponse $response */ - $response = $this->middleware->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); - $payload = $response->getPayload(); - - $this->assertEquals('the_error', $payload['error']); - $this->assertEquals('the_message', $payload['message']); - $verify->shouldHaveBeenCalledOnce(); - $fromRequest->shouldHaveBeenCalledOnce(); - $logWarning->shouldHaveBeenCalledOnce(); - } - /** @test */ public function updatedResponseIsReturnedWhenVerificationPasses(): void { diff --git a/phpstan.neon b/phpstan.neon index 7df6d88f..2d9d960d 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,6 +1,5 @@ parameters: ignoreErrors: - - '#is not subtype of Throwable#' - '#Undefined variable: \$metadata#' - '#AbstractQuery::setParameters()#' - '#mustRun()#'