From e42710e8458ec5dd375076f8626d83dec3748e0f Mon Sep 17 00:00:00 2001 From: Giedo Terol Date: Tue, 13 Nov 2018 11:48:30 +0100 Subject: [PATCH 1/4] Adjust composer.json to allow symfony/cache 4.x --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 08722f0..4ab10c5 100644 --- a/composer.json +++ b/composer.json @@ -42,7 +42,7 @@ "symfony/yaml": ">=2.8.18", "symfony/finder": ">=2.8.18", "symfony/property-access": ">=2.8.18", - "symfony/cache": "^3.3.0", + "symfony/cache": ">=3.3.0", "doctrine/collections": "^1.3", "kleijnweb/php-api-descriptions": "dev-master as 1.0.0-alpha5", "kleijnweb/php-api-routing-bundle": "dev-master as 1.0.0-alpha2", From 59424e04eb231149c45f4d03bd8698e4eacd5cb7 Mon Sep 17 00:00:00 2001 From: Giedo Terol Date: Tue, 13 Nov 2018 14:09:21 +0100 Subject: [PATCH 2/4] Fix compatibility with Symfony 4.x --- .../Response/Error/HttpError.php | 75 ++++++++++--------- src/Security/RequestAuthorizationListener.php | 10 ++- 2 files changed, 49 insertions(+), 36 deletions(-) diff --git a/src/EventListener/Response/Error/HttpError.php b/src/EventListener/Response/Error/HttpError.php index 3c781e5..d07d92b 100644 --- a/src/EventListener/Response/Error/HttpError.php +++ b/src/EventListener/Response/Error/HttpError.php @@ -12,9 +12,7 @@ use Psr\Log\LogLevel; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; -use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException; -use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; +use Symfony\Component\HttpKernel\Exception\HttpException; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\Exception\AuthenticationException; @@ -48,6 +46,13 @@ class HttpError */ private $exception; + const HTTP_CODE_SEVERITY = [ + Response::HTTP_UNAUTHORIZED => LogLevel::WARNING, + Response::HTTP_FORBIDDEN => LogLevel::WARNING, + Response::HTTP_NOT_FOUND => LogLevel::INFO, + Response::HTTP_METHOD_NOT_ALLOWED => LogLevel::WARNING, + ]; + /** * HttpError constructor. * @@ -70,40 +75,42 @@ public function __construct(Request $request, \Exception $exception, LogRefBuild return; } - if ($exception instanceof NotFoundHttpException) { - $this->statusCode = Response::HTTP_NOT_FOUND; - $this->severity = LogLevel::INFO; - } else { - if ($exception instanceof MethodNotAllowedHttpException) { - $this->statusCode = Response::HTTP_METHOD_NOT_ALLOWED; - $this->severity = LogLevel::WARNING; - } elseif ($exception instanceof AuthenticationException) { - $this->statusCode = Response::HTTP_UNAUTHORIZED; - $this->severity = LogLevel::WARNING; - } elseif ($exception instanceof AccessDeniedException || $exception instanceof AccessDeniedHttpException) { - $this->statusCode = Response::HTTP_FORBIDDEN; - $this->severity = LogLevel::WARNING; + if ($exception instanceof HttpException) { + $this->statusCode = $exception->getStatusCode(); + } else if ($exception instanceof AuthenticationException) { + $this->statusCode = Response::HTTP_UNAUTHORIZED; + } elseif ($exception instanceof AccessDeniedException) { + $this->statusCode = Response::HTTP_FORBIDDEN; + } + + if ($this->statusCode && isset(self::HTTP_CODE_SEVERITY[$this->statusCode])) { + $this->severity = self::HTTP_CODE_SEVERITY[$this->statusCode]; + } + + if (!$this->severity) { + $guessedStatusCode = null; + if (strlen((string)$code) !== 3) { + $guessedStatusCode = Response::HTTP_INTERNAL_SERVER_ERROR; + $this->severity = LogLevel::CRITICAL; } else { - if (strlen((string)$code) !== 3) { - $this->statusCode = Response::HTTP_INTERNAL_SERVER_ERROR; - $this->severity = LogLevel::CRITICAL; - } else { - $class = (int)substr((string)$code, 0, 1); - switch ($class) { - case 4: - $this->statusCode = Response::HTTP_BAD_REQUEST; - $this->severity = LogLevel::NOTICE; - break; - case 5: - $this->statusCode = Response::HTTP_INTERNAL_SERVER_ERROR; - $this->severity = LogLevel::ERROR; - break; - default: - $this->statusCode = Response::HTTP_INTERNAL_SERVER_ERROR; - $this->severity = LogLevel::CRITICAL; - } + $class = (int)substr((string)$code, 0, 1); + switch ($class) { + case 4: + $guessedStatusCode = Response::HTTP_BAD_REQUEST; + $this->severity = LogLevel::NOTICE; + break; + case 5: + $guessedStatusCode = Response::HTTP_INTERNAL_SERVER_ERROR; + $this->severity = LogLevel::ERROR; + break; + default: + $guessedStatusCode = Response::HTTP_INTERNAL_SERVER_ERROR; + $this->severity = LogLevel::CRITICAL; } } + if (!$this->statusCode) { + $this->statusCode = $guessedStatusCode; + } } $this->message = Response::$statusTexts[$this->statusCode]; } diff --git a/src/Security/RequestAuthorizationListener.php b/src/Security/RequestAuthorizationListener.php index fb48a0c..b3aba9c 100644 --- a/src/Security/RequestAuthorizationListener.php +++ b/src/Security/RequestAuthorizationListener.php @@ -11,6 +11,8 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; use Symfony\Component\Security\Core\Exception\AccessDeniedException; +use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException; +use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Http\Firewall\ListenerInterface; /** @@ -44,8 +46,12 @@ public function handle(GetResponseEvent $event) return; } - if (!$this->authorizationChecker->isGranted(self::ATTRIBUTE, $event->getRequest())) { - throw new AccessDeniedException(); + try { + if (!$this->authorizationChecker->isGranted(self::ATTRIBUTE, $event->getRequest())) { + throw new AccessDeniedException(); + } + } catch (AuthenticationCredentialsNotFoundException $e) { + throw new AuthenticationException(); } } } From b69599696f2946af2f448cf33d1db96f824ed3ad Mon Sep 17 00:00:00 2001 From: Giedo Terol Date: Tue, 13 Nov 2018 14:13:22 +0100 Subject: [PATCH 3/4] Satisfy scrutinizer --- src/EventListener/Response/Error/HttpError.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/EventListener/Response/Error/HttpError.php b/src/EventListener/Response/Error/HttpError.php index d07d92b..9a89c94 100644 --- a/src/EventListener/Response/Error/HttpError.php +++ b/src/EventListener/Response/Error/HttpError.php @@ -88,7 +88,6 @@ public function __construct(Request $request, \Exception $exception, LogRefBuild } if (!$this->severity) { - $guessedStatusCode = null; if (strlen((string)$code) !== 3) { $guessedStatusCode = Response::HTTP_INTERNAL_SERVER_ERROR; $this->severity = LogLevel::CRITICAL; From a027e6392a574e87febdc451e4c0746100b8ff0e Mon Sep 17 00:00:00 2001 From: Giedo Terol Date: Tue, 13 Nov 2018 14:52:17 +0100 Subject: [PATCH 4/4] Fix coverage decrease and satisfy PHPCS --- .../Response/Error/HttpError.php | 2 +- .../Response/Error/HttpErrorTest.php | 30 +++++++++++++------ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/EventListener/Response/Error/HttpError.php b/src/EventListener/Response/Error/HttpError.php index 9a89c94..dc887f3 100644 --- a/src/EventListener/Response/Error/HttpError.php +++ b/src/EventListener/Response/Error/HttpError.php @@ -77,7 +77,7 @@ public function __construct(Request $request, \Exception $exception, LogRefBuild if ($exception instanceof HttpException) { $this->statusCode = $exception->getStatusCode(); - } else if ($exception instanceof AuthenticationException) { + } elseif ($exception instanceof AuthenticationException) { $this->statusCode = Response::HTTP_UNAUTHORIZED; } elseif ($exception instanceof AccessDeniedException) { $this->statusCode = Response::HTTP_FORBIDDEN; diff --git a/tests/unit/EventListener/Response/Error/HttpErrorTest.php b/tests/unit/EventListener/Response/Error/HttpErrorTest.php index afe7461..7808e92 100644 --- a/tests/unit/EventListener/Response/Error/HttpErrorTest.php +++ b/tests/unit/EventListener/Response/Error/HttpErrorTest.php @@ -14,6 +14,7 @@ use Psr\Log\LogLevel; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException; +use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\Exception\AuthenticationException; /** @@ -38,8 +39,8 @@ public function willClassifyExceptionsWith4xxCodesAsBadRequestNotices() { for ($i = 0; $i < 99; $i++) { $error = new HttpError(new Request(), new \Exception('mimimimimimi', 400 + $i), $this->logRefBuilder); - $this->assertSame($error->getSeverity(), LogLevel::NOTICE); - $this->assertSame($error->getStatusCode(), 400); + $this->assertSame(LogLevel::NOTICE, $error->getSeverity()); + $this->assertSame(400, $error->getStatusCode()); $this->assertStringStartsWith('Bad Request', $error->getMessage()); } } @@ -51,8 +52,8 @@ public function willClassifyExceptionsWith5xxCodesAsRuntimeErrors() { for ($i = 0; $i < 99; $i++) { $error = new HttpError(new Request(), new \Exception('mimimimimimi', 500 + $i), $this->logRefBuilder); - $this->assertSame($error->getSeverity(), LogLevel::ERROR); - $this->assertSame($error->getStatusCode(), 500); + $this->assertSame(LogLevel::ERROR, $error->getSeverity()); + $this->assertSame(500, $error->getStatusCode()); $this->assertStringStartsWith('Internal Server Error', $error->getMessage()); } } @@ -65,8 +66,8 @@ public function willClassifyExceptionsWithUnexpectedCodesAsCriticalErrors() $sample = [4096, 777, 22, 5, 0]; foreach ($sample as $code) { $error = new HttpError(new Request(), new \Exception('mimimimimimi', $code), $this->logRefBuilder); - $this->assertSame($error->getSeverity(), LogLevel::CRITICAL); - $this->assertSame($error->getStatusCode(), 500); + $this->assertSame(LogLevel::CRITICAL, $error->getSeverity()); + $this->assertSame(500, $error->getStatusCode()); $this->assertStringStartsWith('Internal Server Error', $error->getMessage()); } } @@ -77,7 +78,7 @@ public function willClassifyExceptionsWithUnexpectedCodesAsCriticalErrors() public function willClassifyMethodNotAllowedHttpExceptionAsWarningsAndReturn405Status() { $error = new HttpError(new Request(), new MethodNotAllowedHttpException(['GET']), $this->logRefBuilder); - $this->assertSame($error->getSeverity(), LogLevel::WARNING); + $this->assertSame(LogLevel::WARNING, $error->getSeverity()); $this->assertSame($error->getStatusCode(), 405); $this->assertStringStartsWith('Method Not Allowed', $error->getMessage()); } @@ -88,8 +89,19 @@ public function willClassifyMethodNotAllowedHttpExceptionAsWarningsAndReturn405S public function willClassifyAuthenticationExceptionAsWarningsAndReturn401Status() { $error = new HttpError(new Request(), new AuthenticationException(), $this->logRefBuilder); - $this->assertSame($error->getSeverity(), LogLevel::WARNING); - $this->assertSame($error->getStatusCode(), 401); + $this->assertSame(LogLevel::WARNING, $error->getSeverity()); + $this->assertSame(401, $error->getStatusCode()); $this->assertStringStartsWith('Unauthorized', $error->getMessage()); } + + /** + * @test + */ + public function willClassifyAccessDeniedExceptionAsWarningsAndReturn403Status() + { + $error = new HttpError(new Request(), new AccessDeniedException(), $this->logRefBuilder); + $this->assertSame(LogLevel::WARNING, $error->getSeverity()); + $this->assertSame(403, $error->getStatusCode()); + $this->assertStringStartsWith('Forbidden', $error->getMessage()); + } }