diff --git a/src/Auth.php b/src/Auth.php index bb9f59032..55b079651 100644 --- a/src/Auth.php +++ b/src/Auth.php @@ -239,7 +239,7 @@ final class Auth $user = $this->getUser(); if (!($user instanceof User)) { - throw new NotLoggedInException(); + throw NotLoggedInException::create(); } if ($user->verifyTwoFactor($otp)) { diff --git a/src/Controller/Api/Internal/ListenerAuthAction.php b/src/Controller/Api/Internal/ListenerAuthAction.php index d0ae4779a..c482af8ff 100644 --- a/src/Controller/Api/Internal/ListenerAuthAction.php +++ b/src/Controller/Api/Internal/ListenerAuthAction.php @@ -42,7 +42,7 @@ final class ListenerAuthAction implements SingleActionInterface ] ); - throw new PermissionDeniedException(); + throw PermissionDeniedException::create(); } } diff --git a/src/Controller/Api/Stations/RemotesController.php b/src/Controller/Api/Stations/RemotesController.php index 9678e586d..77cd66bb5 100644 --- a/src/Controller/Api/Stations/RemotesController.php +++ b/src/Controller/Api/Stations/RemotesController.php @@ -218,7 +218,7 @@ final class RemotesController extends AbstractStationApiCrudController $record = parent::getRecord($request, $params); if ($record instanceof StationRemote && !$record->isEditable()) { - throw new PermissionDeniedException('This record cannot be edited.'); + throw PermissionDeniedException::create(); } return $record; diff --git a/src/Controller/Frontend/SetupController.php b/src/Controller/Frontend/SetupController.php index 1ede68c60..2e3e7b0a5 100644 --- a/src/Controller/Frontend/SetupController.php +++ b/src/Controller/Frontend/SetupController.php @@ -220,7 +220,7 @@ final class SetupController // If past "register" step, require login. $auth = $request->getAuth(); if (!$auth->isLoggedIn()) { - throw new NotLoggedInException(); + throw NotLoggedInException::create(); } // Step 2: Set up Station diff --git a/src/Exception/BootstrapException.php b/src/Exception/BootstrapException.php index 3bf12243f..56dc4a4d3 100644 --- a/src/Exception/BootstrapException.php +++ b/src/Exception/BootstrapException.php @@ -12,7 +12,7 @@ final class BootstrapException extends Exception { public function __construct( string $message = '', - int $code = 0, + int $code = 500, Throwable $previous = null, Level $loggerLevel = Level::Alert ) { diff --git a/src/Exception/CannotCompleteActionException.php b/src/Exception/CannotCompleteActionException.php index 83451fd64..211f86163 100644 --- a/src/Exception/CannotCompleteActionException.php +++ b/src/Exception/CannotCompleteActionException.php @@ -12,7 +12,7 @@ final class CannotCompleteActionException extends Exception { public function __construct( string $message = 'Cannot complete action.', - int $code = 0, + int $code = 500, Throwable $previous = null, Level $loggerLevel = Level::Info ) { diff --git a/src/Exception/CannotProcessMediaException.php b/src/Exception/CannotProcessMediaException.php index 79b34d278..44080df2b 100644 --- a/src/Exception/CannotProcessMediaException.php +++ b/src/Exception/CannotProcessMediaException.php @@ -14,7 +14,7 @@ final class CannotProcessMediaException extends Exception public function __construct( string $message = 'Cannot process media file.', - int $code = 0, + int $code = 500, Throwable $previous = null, Level $loggerLevel = Level::Warning ) { diff --git a/src/Exception/CsrfValidationException.php b/src/Exception/CsrfValidationException.php index 264df7e8e..0e6d94a0f 100644 --- a/src/Exception/CsrfValidationException.php +++ b/src/Exception/CsrfValidationException.php @@ -12,7 +12,7 @@ final class CsrfValidationException extends Exception { public function __construct( string $message = 'CSRF Validation Error', - int $code = 0, + int $code = 500, Throwable $previous = null, Level $loggerLevel = Level::Info ) { diff --git a/src/Exception/InvalidRequestAttribute.php b/src/Exception/InvalidRequestAttribute.php index f6deea5b9..ae8e77150 100644 --- a/src/Exception/InvalidRequestAttribute.php +++ b/src/Exception/InvalidRequestAttribute.php @@ -12,7 +12,7 @@ final class InvalidRequestAttribute extends Exception { public function __construct( string $message = 'Invalid request attribute.', - int $code = 0, + int $code = 500, Throwable $previous = null, Level $loggerLevel = Level::Info ) { diff --git a/src/Exception/NoFileUploadedException.php b/src/Exception/NoFileUploadedException.php index 347ae1dce..f5c91fb1f 100644 --- a/src/Exception/NoFileUploadedException.php +++ b/src/Exception/NoFileUploadedException.php @@ -12,7 +12,7 @@ final class NoFileUploadedException extends Exception { public function __construct( string $message = 'No file was uploaded.', - int $code = 0, + int $code = 400, Throwable $previous = null, Level $loggerLevel = Level::Info ) { diff --git a/src/Exception/NotLoggedInException.php b/src/Exception/NotLoggedInException.php index a71892a5f..f5abc9871 100644 --- a/src/Exception/NotLoggedInException.php +++ b/src/Exception/NotLoggedInException.php @@ -12,10 +12,17 @@ final class NotLoggedInException extends Exception { public function __construct( string $message = 'Not logged in.', - int $code = 0, + int $code = 403, Throwable $previous = null, Level $loggerLevel = Level::Debug ) { parent::__construct($message, $code, $previous, $loggerLevel); } + + public static function create(): self + { + return new self( + __('You must be logged in to access this page.') + ); + } } diff --git a/src/Exception/PermissionDeniedException.php b/src/Exception/PermissionDeniedException.php index 240e891d2..a106aeb4f 100644 --- a/src/Exception/PermissionDeniedException.php +++ b/src/Exception/PermissionDeniedException.php @@ -12,10 +12,17 @@ final class PermissionDeniedException extends Exception { public function __construct( string $message = 'Permission denied.', - int $code = 0, + int $code = 403, Throwable $previous = null, Level $loggerLevel = Level::Info ) { parent::__construct($message, $code, $previous, $loggerLevel); } + + public static function create(): self + { + return new self( + __('You do not have permission to access this portion of the site.') + ); + } } diff --git a/src/Exception/RateLimitExceededException.php b/src/Exception/RateLimitExceededException.php index b1485b6ac..60cf8ccd8 100644 --- a/src/Exception/RateLimitExceededException.php +++ b/src/Exception/RateLimitExceededException.php @@ -12,7 +12,7 @@ final class RateLimitExceededException extends Exception { public function __construct( string $message = 'You have exceeded the rate limit for this application.', - int $code = 0, + int $code = 429, Throwable $previous = null, Level $loggerLevel = Level::Info ) { diff --git a/src/Exception/StationUnsupportedException.php b/src/Exception/StationUnsupportedException.php index de03c40c1..c6eb03134 100644 --- a/src/Exception/StationUnsupportedException.php +++ b/src/Exception/StationUnsupportedException.php @@ -12,7 +12,7 @@ final class StationUnsupportedException extends Exception { public function __construct( string $message = 'This feature is not currently supported on this station.', - int $code = 0, + int $code = 500, Throwable $previous = null, Level $loggerLevel = Level::Info ) { diff --git a/src/Exception/StorageLocationFullException.php b/src/Exception/StorageLocationFullException.php index 9b3f1f2cf..ff8d50d68 100644 --- a/src/Exception/StorageLocationFullException.php +++ b/src/Exception/StorageLocationFullException.php @@ -12,7 +12,7 @@ final class StorageLocationFullException extends Exception { public function __construct( string $message = 'Storage location is full.', - int $code = 0, + int $code = 500, Throwable $previous = null, Level $loggerLevel = Level::Info ) { diff --git a/src/Exception/Supervisor/AlreadyRunningException.php b/src/Exception/Supervisor/AlreadyRunningException.php index 18997b761..7b8dc1104 100644 --- a/src/Exception/Supervisor/AlreadyRunningException.php +++ b/src/Exception/Supervisor/AlreadyRunningException.php @@ -12,7 +12,7 @@ final class AlreadyRunningException extends SupervisorException { public function __construct( string $message = 'Process was already running.', - int $code = 0, + int $code = 500, Throwable $previous = null, Level $loggerLevel = Level::Info ) { diff --git a/src/Exception/Supervisor/NotRunningException.php b/src/Exception/Supervisor/NotRunningException.php index 913a08a8b..517491bd2 100644 --- a/src/Exception/Supervisor/NotRunningException.php +++ b/src/Exception/Supervisor/NotRunningException.php @@ -12,7 +12,7 @@ final class NotRunningException extends SupervisorException { public function __construct( string $message = 'Process was not running yet.', - int $code = 0, + int $code = 500, Throwable $previous = null, Level $loggerLevel = Level::Info ) { diff --git a/src/Exception/ValidationException.php b/src/Exception/ValidationException.php index 8e2045aaf..578786ab0 100644 --- a/src/Exception/ValidationException.php +++ b/src/Exception/ValidationException.php @@ -15,7 +15,7 @@ final class ValidationException extends Exception public function __construct( string $message = 'Validation error.', - int $code = 0, + int $code = 400, Throwable $previous = null, Level $loggerLevel = Level::Info ) { diff --git a/src/Http/ErrorHandler.php b/src/Http/ErrorHandler.php index 3d6573ea2..338c8ee7b 100644 --- a/src/Http/ErrorHandler.php +++ b/src/Http/ErrorHandler.php @@ -68,6 +68,19 @@ final class ErrorHandler extends SlimErrorHandler return parent::__invoke($request, $exception, $displayErrorDetails, $logErrors, $logErrorDetails); } + protected function determineStatusCode(): int + { + if ($this->method === 'OPTIONS') { + return 200; + } + + if ($this->exception instanceof Exception || $this->exception instanceof HttpException) { + return $this->exception->getCode(); + } + + return 500; + } + private function shouldReturnJson(ServerRequestInterface $req): bool { $xhr = $req->getHeaderLine('X-Requested-With') === 'XMLHttpRequest'; @@ -120,12 +133,13 @@ final class ErrorHandler extends SlimErrorHandler return parent::respond(); } + /** @var Response $response */ + $response = $this->responseFactory->createResponse($this->statusCode); + // Special handling for cURL requests. $ua = $this->request->getHeaderLine('User-Agent'); if (false !== stripos($ua, 'curl') || false !== stripos($ua, 'Liquidsoap')) { - $response = $this->responseFactory->createResponse($this->statusCode); - $response->getBody()->write( sprintf( 'Error: %s on %s L%s', @@ -138,42 +152,12 @@ final class ErrorHandler extends SlimErrorHandler return $response; } - if ($this->exception instanceof HttpException) { - /** @var Response $response */ - $response = $this->responseFactory->createResponse($this->exception->getCode()); - - if ($this->returnJson) { - $apiResponse = Error::fromException($this->exception, $this->showDetailed); - return $response->withJson($apiResponse); - } - - $view = $this->view->withRequest($this->request); - - try { - return $view->renderToResponse( - $response, - 'system/error_http', - [ - 'exception' => $this->exception, - ] - ); - } catch (Throwable) { - return parent::respond(); - } + if ($this->returnJson) { + $apiResponse = Error::fromException($this->exception, $this->showDetailed); + return $response->withJson($apiResponse); } if ($this->exception instanceof NotLoggedInException) { - /** @var Response $response */ - $response = $this->responseFactory->createResponse(403); - - if ($this->returnJson) { - $error = Error::fromException($this->exception); - $error->code = 403; - $error->message = __('You must be logged in to access this page.'); - - return $response->withJson($error); - } - // Redirect to login page for not-logged-in users. $sessionPersistence = $this->injectSession->getSessionPersistence($this->request); @@ -181,54 +165,33 @@ final class ErrorHandler extends SlimErrorHandler $session = $sessionPersistence->initializeSessionFromRequest($this->request); $flash = new Flash($session); - $flash->error(__('You must be logged in to access this page.')); + $flash->error($this->exception->getMessage()); // Set referrer for login redirection. $session->set('login_referrer', $this->request->getUri()->getPath()); + /** @var Response $response */ $response = $sessionPersistence->persistSession($session, $response); - /** @var Response $response */ return $response->withRedirect($this->router->named('account:login')); } if ($this->exception instanceof PermissionDeniedException) { - /** @var Response $response */ - $response = $this->responseFactory->createResponse(403); - - if ($this->returnJson) { - $error = Error::fromException($this->exception); - $error->code = 403; - $error->message = __('You do not have permission to access this portion of the site.'); - - return $response->withJson($error); - } - $sessionPersistence = $this->injectSession->getSessionPersistence($this->request); /** @var Session $session */ $session = $sessionPersistence->initializeSessionFromRequest($this->request); $flash = new Flash($session); - $flash->error( - __('You do not have permission to access this portion of the site.'), - ); + $flash->error($this->exception->getMessage()); + /** @var Response $response */ $response = $sessionPersistence->persistSession($session, $response); // Bounce back to homepage for permission-denied users. - /** @var Response $response */ return $response->withRedirect($this->router->named('home')); } - /** @var Response $response */ - $response = $this->responseFactory->createResponse(500); - - if ($this->returnJson) { - $apiResponse = Error::fromException($this->exception, $this->showDetailed); - return $response->withJson($apiResponse); - } - if ($this->showDetailed && class_exists(Run::class)) { // Register error-handler. $handler = new PrettyPageHandler(); @@ -246,12 +209,14 @@ final class ErrorHandler extends SlimErrorHandler return $response->write($run->handleException($this->exception)); } - $view = $this->view->withRequest($this->request); - try { + $view = $this->view->withRequest($this->request); + return $view->renderToResponse( $response, - 'system/error_general', + ($this->exception instanceof HttpException) + ? 'system/error_http' + : 'system/error_general', [ 'exception' => $this->exception, ] diff --git a/src/Middleware/Permissions.php b/src/Middleware/Permissions.php index 5fa27c852..13a4673bf 100644 --- a/src/Middleware/Permissions.php +++ b/src/Middleware/Permissions.php @@ -33,12 +33,12 @@ final class Permissions extends AbstractMiddleware try { $user = $request->getUser(); } catch (Exception) { - throw new PermissionDeniedException(); + throw PermissionDeniedException::create(); } $acl = $request->getAcl(); if (!$acl->userAllowed($user, $this->action, $stationId)) { - throw new PermissionDeniedException(); + throw PermissionDeniedException::create(); } return $handler->handle($request); diff --git a/src/Middleware/RequireLogin.php b/src/Middleware/RequireLogin.php index b7b5af4da..00b63dc32 100644 --- a/src/Middleware/RequireLogin.php +++ b/src/Middleware/RequireLogin.php @@ -20,7 +20,7 @@ final class RequireLogin extends AbstractMiddleware try { $request->getUser(); } catch (Exception) { - throw new NotLoggedInException(); + throw NotLoggedInException::create(); } return $handler->handle($request); diff --git a/tests/Functional/Frontend_PublicCest.php b/tests/Functional/Frontend_PublicCest.php index 48888985a..6eae43ebd 100644 --- a/tests/Functional/Frontend_PublicCest.php +++ b/tests/Functional/Frontend_PublicCest.php @@ -22,7 +22,7 @@ class Frontend_PublicCest extends CestAbstract $this->em->flush(); $I->amOnPage('/public/' . $testStation->getId()); - $I->seeResponseCodeIs(500); + $I->seeResponseCodeIs(404); // Enable public pages $testStation = $this->getTestStation();