From e2e18db460f05950c95ddd84db0370d0e321fd2c Mon Sep 17 00:00:00 2001 From: Igor Scheller Date: Mon, 10 Jul 2023 17:59:49 +0200 Subject: [PATCH] API: Moved json handling and route-api tagging to ApiRouteHandler --- config/app.php | 2 +- resources/api/openapi.yml | 25 ++- src/Helpers/Authenticator.php | 2 +- src/Middleware/ApiRouteHandler.php | 93 ++++++++++ src/Middleware/RouteDispatcher.php | 7 +- src/Middleware/SessionHandler.php | 18 +- .../SessionHandlerServiceProvider.php | 32 ---- tests/Unit/Controllers/ApiControllerTest.php | 4 +- tests/Unit/Helpers/AuthenticatorTest.php | 4 +- tests/Unit/Middleware/ApiRouteHandlerTest.php | 172 ++++++++++++++++++ .../SessionHandlerServiceProviderTest.php | 46 ----- tests/Unit/Middleware/SessionHandlerTest.php | 9 +- 12 files changed, 303 insertions(+), 111 deletions(-) create mode 100644 src/Middleware/ApiRouteHandler.php delete mode 100644 src/Middleware/SessionHandlerServiceProvider.php create mode 100644 tests/Unit/Middleware/ApiRouteHandlerTest.php delete mode 100644 tests/Unit/Middleware/SessionHandlerServiceProviderTest.php diff --git a/config/app.php b/config/app.php index ff3bed30..3cd08e40 100644 --- a/config/app.php +++ b/config/app.php @@ -28,7 +28,6 @@ return [ \Engelsystem\Renderer\TwigServiceProvider::class, \Engelsystem\Middleware\RouteDispatcherServiceProvider::class, \Engelsystem\Middleware\RequestHandlerServiceProvider::class, - \Engelsystem\Middleware\SessionHandlerServiceProvider::class, \Engelsystem\Http\Validation\ValidationServiceProvider::class, \Engelsystem\Http\RedirectServiceProvider::class, @@ -54,6 +53,7 @@ return [ // The application code \Engelsystem\Middleware\ErrorHandler::class, + \Engelsystem\Middleware\ApiRouteHandler::class, \Engelsystem\Middleware\VerifyCsrfToken::class, \Engelsystem\Middleware\RouteDispatcher::class, \Engelsystem\Middleware\SessionHandler::class, diff --git a/resources/api/openapi.yml b/resources/api/openapi.yml index 2da287f3..a67bb771 100644 --- a/resources/api/openapi.yml +++ b/resources/api/openapi.yml @@ -32,12 +32,31 @@ components: bearerFormat: API key from settings responses: - UnauthorizedError: + UnauthorizedError: # 401 description: Access token is missing or invalid - ForbiddenError: - description: The client is not allowed to acces + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + ForbiddenError: # 403 + description: The client is not allowed to access + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + NotImplementedError: # 501 + description: This endpoint or method is not implemented + content: + application/json: + schema: + $ref: '#/components/schemas/Error' schemas: + Error: + type: object + properties: + message: + type: string News: type: object properties: diff --git a/src/Helpers/Authenticator.php b/src/Helpers/Authenticator.php index 7fde7d4a..b5459dcc 100644 --- a/src/Helpers/Authenticator.php +++ b/src/Helpers/Authenticator.php @@ -42,7 +42,7 @@ class Authenticator } $this->user = $this->userFromSession(); - if (!$this->user && request()->getAttribute('route-api', false)) { + if (!$this->user && request()->getAttribute('route-api-accessible', false)) { $this->user = $this->userFromApi(); } diff --git a/src/Middleware/ApiRouteHandler.php b/src/Middleware/ApiRouteHandler.php new file mode 100644 index 00000000..5fe4f549 --- /dev/null +++ b/src/Middleware/ApiRouteHandler.php @@ -0,0 +1,93 @@ +getUri()))->getPath(); + if ($request instanceof Request) { + $path = $request->getPathInfo(); + } + + $path = urldecode($path); + $isApi = $this->apiPrefix && (Str::startsWith($path, $this->apiPrefix . '/') || $path == $this->apiPrefix); + $isApiAccessible = $isApi || $this->apiAccessiblePaths && in_array($path, $this->apiAccessiblePaths); + $request = $request + ->withAttribute('route-api', $isApi) + ->withAttribute('route-api-accessible', $isApiAccessible); + + return $isApi ? $this->processApi($request, $handler) : $handler->handle($request); + } + + /** + * Process the API request by ensuring that JSON is returned + */ + protected function processApi(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + try { + $response = $handler->handle($request); + } catch (ModelNotFoundException $e) { + $response = new Response('', 404); + $response->setContent($response->getReasonPhrase()); + } catch (HttpException $e) { + $response = new Response($e->getMessage(), $e->getStatusCode(), $e->getHeaders()); + $response->setContent($response->getContent() ?: $response->getReasonPhrase()); + } catch (Throwable $e) { + /** @var Handler $handler */ + $handler = app('error.handler'); + $handler->exceptionHandler($e, true); + $response = new Response('', 500); + $response->setContent($response->getReasonPhrase()); + } + + if (!Str::isJson((string) $response->getBody())) { + $content = (string) $response->getBody(); + $content = Stream::create(json_encode([ + 'message' => $content, + ])); + $response = $response + ->withHeader('content-type', 'application/json') + ->withBody($content); + } + + $eTag = md5((string) $response->getBody()); + $response->setEtag($eTag); + + return $response; + } +} diff --git a/src/Middleware/RouteDispatcher.php b/src/Middleware/RouteDispatcher.php index 99568385..dcc5d05c 100644 --- a/src/Middleware/RouteDispatcher.php +++ b/src/Middleware/RouteDispatcher.php @@ -14,18 +14,15 @@ use Psr\Http\Server\RequestHandlerInterface; class RouteDispatcher implements MiddlewareInterface { - protected ?MiddlewareInterface $notFound = null; - /** - * @param ResponseInterface $response Default response + * @param ResponseInterface $response Default response * @param MiddlewareInterface|null $notFound Handles any requests if the route can't be found */ public function __construct( protected FastRouteDispatcher $dispatcher, protected ResponseInterface $response, - MiddlewareInterface $notFound = null + protected ?MiddlewareInterface $notFound = null ) { - $this->notFound = $notFound; } /** diff --git a/src/Middleware/SessionHandler.php b/src/Middleware/SessionHandler.php index bf75c0e2..56274b25 100644 --- a/src/Middleware/SessionHandler.php +++ b/src/Middleware/SessionHandler.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Engelsystem\Middleware; -use Illuminate\Support\Str; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; @@ -14,26 +13,21 @@ use Symfony\Component\HttpFoundation\Session\Storage\SessionStorageInterface; class SessionHandler implements MiddlewareInterface { - public function __construct( - protected SessionStorageInterface $session, - protected array $paths = [], - protected ?string $apiPrefix = null - ) { + public function __construct(protected SessionStorageInterface $session) + { } public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - $requestPath = $request->getAttribute('route-request-path'); - $isApi = in_array($requestPath, $this->paths) - || ($this->apiPrefix && Str::startsWith($requestPath, $this->apiPrefix)); - $request = $request->withAttribute('route-api', $isApi); - $return = $handler->handle($request); $cookies = $request->getCookieParams(); if ( - $isApi + // Is api (accessible) path + $request->getAttribute('route-api-accessible') + // Uses native PHP session && $this->session instanceof NativeSessionStorage + // No session cookie was sent on request && !isset($cookies[$this->session->getName()]) ) { $this->destroyNative(); diff --git a/src/Middleware/SessionHandlerServiceProvider.php b/src/Middleware/SessionHandlerServiceProvider.php deleted file mode 100644 index 808fdc4c..00000000 --- a/src/Middleware/SessionHandlerServiceProvider.php +++ /dev/null @@ -1,32 +0,0 @@ -app - ->when(SessionHandler::class) - ->needs('$paths') - ->give(function () { - return [ - '/atom', - '/rss', - '/health', - '/ical', - '/metrics', - '/shifts-json-export', - '/stats', - ]; - }); - $this->app - ->when(SessionHandler::class) - ->needs('$apiPrefix') - ->give('/api'); - } -} diff --git a/tests/Unit/Controllers/ApiControllerTest.php b/tests/Unit/Controllers/ApiControllerTest.php index 72430bb1..4818bfe3 100644 --- a/tests/Unit/Controllers/ApiControllerTest.php +++ b/tests/Unit/Controllers/ApiControllerTest.php @@ -9,7 +9,7 @@ use Engelsystem\Http\Response; use Engelsystem\Models\News; use League\OpenAPIValidation\PSR7\OperationAddress as OpenApiAddress; use League\OpenAPIValidation\PSR7\ResponseValidator as OpenApiResponseValidator; -use League\OpenAPIValidation\PSR7\ValidatorBuilder; +use League\OpenAPIValidation\PSR7\ValidatorBuilder as OpenApiValidatorBuilder; class ApiControllerTest extends ControllerTest { @@ -106,7 +106,7 @@ class ApiControllerTest extends ControllerTest parent::setUp(); $openApiDefinition = $this->app->get('path.resources.api') . '/openapi.yml'; - $this->validator = (new ValidatorBuilder()) + $this->validator = (new OpenApiValidatorBuilder()) ->fromYamlFile($openApiDefinition) ->getResponseValidator(); } diff --git a/tests/Unit/Helpers/AuthenticatorTest.php b/tests/Unit/Helpers/AuthenticatorTest.php index 973e12f5..0369ebc0 100644 --- a/tests/Unit/Helpers/AuthenticatorTest.php +++ b/tests/Unit/Helpers/AuthenticatorTest.php @@ -90,7 +90,7 @@ class AuthenticatorTest extends ServiceProviderTest $session = new Session(new MockArraySessionStorage()); $request = $request->withHeader('Authorization', 'Bearer F00Bar'); - $request = $request->withAttribute('route-api', true); + $request = $request->withAttribute('route-api-accessible', true); $this->app->instance('request', $request); User::factory()->create(['api_key' => 'F00Bar']); @@ -160,7 +160,7 @@ class AuthenticatorTest extends ServiceProviderTest $this->initDatabase(); $request = new Request(); - $request = $request->withAttribute('route-api', true); + $request = $request->withAttribute('route-api-accessible', true); $session = new Session(new MockArraySessionStorage()); $this->app->instance('request', $request); diff --git a/tests/Unit/Middleware/ApiRouteHandlerTest.php b/tests/Unit/Middleware/ApiRouteHandlerTest.php new file mode 100644 index 00000000..5cb35718 --- /dev/null +++ b/tests/Unit/Middleware/ApiRouteHandlerTest.php @@ -0,0 +1,172 @@ +provideIsApi(), + ['/metrics', true, false], + ['/metrics/test', false, false], + ['/health', true, false], + ]; + } + + /** + * @covers \Engelsystem\Middleware\ApiRouteHandler::process + * @covers \Engelsystem\Middleware\ApiRouteHandler::processApi + * @covers \Engelsystem\Middleware\ApiRouteHandler::__construct + * @dataProvider provideIsApi + */ + public function testProcessIsApi(string $uri, bool $isApi): void + { + $request = Request::create($uri); + /** @var RequestHandlerInterface|MockObject $handler */ + $handler = $this->getMockForAbstractClass(RequestHandlerInterface::class); + $response = new Response('response content'); + + $handler->expects($this->once()) + ->method('handle') + ->willReturnCallback(function (ServerRequestInterface $request) use ($response, $isApi) { + $this->assertEquals($isApi, $request->getAttribute('route-api')); + return $response; + }); + + $middleware = new ApiRouteHandler(); + $apiResponse = $middleware->process($request, $handler); + + if ($isApi) { + $this->assertEquals('application/json', $apiResponse->getHeaderLine('content-type')); + $this->assertEquals('{"message":"response content"}', (string) $apiResponse->getBody()); + $this->assertNotEmpty($apiResponse->getHeaderLine('Etag')); + } else { + $this->assertEquals($response, $apiResponse); + } + } + + /** + * @covers \Engelsystem\Middleware\ApiRouteHandler::process + * @dataProvider provideIsApiAccessiblePath + */ + public function testProcessIsApiAccessiblePath(string $uri, bool $isApiAccessible, bool $isOnlyApi = true): void + { + $request = Request::create($uri); + /** @var RequestHandlerInterface|MockObject $handler */ + $handler = $this->getMockForAbstractClass(RequestHandlerInterface::class); + $response = new Response('response content'); + + $handler->expects($this->once()) + ->method('handle') + ->willReturnCallback(function (ServerRequestInterface $request) use ($response, $isApiAccessible) { + $this->assertEquals($isApiAccessible, $request->getAttribute('route-api-accessible')); + return $response; + }); + + $middleware = new ApiRouteHandler(); + $apiResponse = $middleware->process($request, $handler); + + if (!$isOnlyApi) { + $this->assertEquals($response, $apiResponse); + } + } + + /** + * @covers \Engelsystem\Middleware\ApiRouteHandler::processApi + */ + public function testProcessApiModelNotFoundException(): void + { + $request = Request::create('/api/test'); + /** @var RequestHandlerInterface|MockObject $handler */ + $handler = $this->getMockForAbstractClass(RequestHandlerInterface::class); + + $handler->expects($this->once()) + ->method('handle') + ->willReturnCallback(function (): void { + throw new ModelNotFoundException(User::class); + }); + + $middleware = new ApiRouteHandler(); + $response = $middleware->process($request, $handler); + + $this->assertEquals(404, $response->getStatusCode()); + $this->assertEquals('{"message":"Not Found"}', (string) $response->getBody()); + } + + /** + * @covers \Engelsystem\Middleware\ApiRouteHandler::processApi + */ + public function testProcessApiHttpException(): void + { + $request = Request::create('/api/test'); + /** @var RequestHandlerInterface|MockObject $handler */ + $handler = $this->getMockForAbstractClass(RequestHandlerInterface::class); + + $handler->expects($this->once()) + ->method('handle') + ->willReturnCallback(function (): void { + throw new HttpNotFound(); + }); + + $middleware = new ApiRouteHandler(); + $response = $middleware->process($request, $handler); + + $this->assertEquals(404, $response->getStatusCode()); + $this->assertEquals('{"message":"Not Found"}', (string) $response->getBody()); + } + + /** + * @covers \Engelsystem\Middleware\ApiRouteHandler::processApi + */ + public function testProcessGenericException(): void + { + $e = new Exception(); + $request = Request::create('/api/test'); + /** @var RequestHandlerInterface|MockObject $handler */ + $handler = $this->getMockForAbstractClass(RequestHandlerInterface::class); + $errorHandler = $this->createMock(Handler::class); + $this->setExpects($errorHandler, 'exceptionHandler', [$e, true], '', $this->once()); + $this->app->instance('error.handler', $errorHandler); + + $handler->expects($this->once()) + ->method('handle') + ->willReturnCallback(function () use ($e): void { + throw $e; + }); + + $middleware = new ApiRouteHandler(); + $response = $middleware->process($request, $handler); + + $this->assertEquals(500, $response->getStatusCode()); + $this->assertEquals('{"message":"Internal Server Error"}', (string) $response->getBody()); + } +} diff --git a/tests/Unit/Middleware/SessionHandlerServiceProviderTest.php b/tests/Unit/Middleware/SessionHandlerServiceProviderTest.php deleted file mode 100644 index 61e1ed7a..00000000 --- a/tests/Unit/Middleware/SessionHandlerServiceProviderTest.php +++ /dev/null @@ -1,46 +0,0 @@ -createMock(ContextualBindingBuilder::class); - $app = $this->getApp(['when']); - - $app->expects($this->once()) - ->method('when') - ->with(SessionHandler::class) - ->willReturn($bindingBuilder); - - $bindingBuilder->expects($this->once()) - ->method('needs') - ->with('$paths') - ->willReturn($bindingBuilder); - - $bindingBuilder->expects($this->once()) - ->method('give') - ->willReturnCallback(function (callable $callable): void { - $paths = $callable(); - - $this->assertIsArray($paths); - $this->assertTrue(in_array('/metrics', $paths)); - }); - - $serviceProvider = new SessionHandlerServiceProvider($app); - $serviceProvider->register(); - } -} diff --git a/tests/Unit/Middleware/SessionHandlerTest.php b/tests/Unit/Middleware/SessionHandlerTest.php index 51b3ea52..d24168fc 100644 --- a/tests/Unit/Middleware/SessionHandlerTest.php +++ b/tests/Unit/Middleware/SessionHandlerTest.php @@ -40,13 +40,8 @@ class SessionHandlerTest extends TestCase $request->expects($this->exactly(2)) ->method('getAttribute') - ->with('route-request-path') - ->willReturnOnConsecutiveCalls('/foo', '/lorem'); - - $request->expects($this->exactly(2)) - ->method('withAttribute') - ->withConsecutive(['route-api', true], ['route-api', false]) - ->willReturn($request); + ->with('route-api-accessible') + ->willReturnOnConsecutiveCalls(true, false); $sessionStorage->expects($this->once()) ->method('getName')