From 9bd18ee04154cff821a09d8966df36b2ef193fc3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 25 Mar 2017 09:37:13 +0100 Subject: [PATCH] Migrated CheckAuthenticationMiddleware to psr-15 middleware --- .../CheckAuthenticationMiddleware.php | 45 ++++------- .../CheckAuthenticationMiddlewareTest.php | 78 +++++++++---------- 2 files changed, 53 insertions(+), 70 deletions(-) diff --git a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php index 0e7c1cdb..0304a408 100644 --- a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php @@ -2,6 +2,9 @@ namespace Shlinkio\Shlink\Rest\Middleware; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; +use Fig\Http\Message\StatusCodeInterface; +use Interop\Http\ServerMiddleware\DelegateInterface; +use Interop\Http\ServerMiddleware\MiddlewareInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; @@ -14,9 +17,8 @@ use Zend\Diactoros\Response\JsonResponse; use Zend\Expressive\Router\RouteResult; use Zend\I18n\Translator\TranslatorInterface; use Zend\Stdlib\ErrorHandler; -use Zend\Stratigility\MiddlewareInterface; -class CheckAuthenticationMiddleware implements MiddlewareInterface +class CheckAuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterface { const AUTHORIZATION_HEADER = 'Authorization'; @@ -52,31 +54,15 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface } /** - * Process an incoming request and/or response. - * - * Accepts a server-side request and a response instance, and does - * something with them. - * - * If the response is not complete and/or further processing would not - * interfere with the work done in the middleware, or if the middleware - * wants to delegate to another process, it can use the `$out` callable - * if present. - * - * If the middleware does not return a value, execution of the current - * request is considered complete, and the response instance provided will - * be considered the response to return. - * - * Alternately, the middleware may return a response instance. - * - * Often, middleware will `return $out();`, with the assumption that a - * later middleware will return a response. + * Process an incoming server request and return a response, optionally delegating + * to the next middleware component to create the response. * * @param Request $request - * @param Response $response - * @param null|callable $out - * @return null|Response + * @param DelegateInterface $delegate + * + * @return Response */ - public function __invoke(Request $request, Response $response, callable $out = null) + public function process(Request $request, DelegateInterface $delegate) { // If current route is the authenticate route or an OPTIONS request, continue to the next middleware /** @var RouteResult $routeResult */ @@ -86,7 +72,7 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface || $routeResult->getMatchedRouteName() === 'rest-authenticate' || $request->getMethod() === 'OPTIONS' ) { - return $out($request, $response); + return $delegate->process($request); } // Check that the auth header was provided, and that it belongs to a non-expired token @@ -103,7 +89,7 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface 'message' => sprintf($this->translator->translate( 'You need to provide the Bearer type in the %s header.' ), self::AUTHORIZATION_HEADER), - ], 401); + ], self::STATUS_UNAUTHORIZED); } // Make sure the authorization type is Bearer @@ -114,7 +100,7 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface 'message' => sprintf($this->translator->translate( 'Provided authorization type %s is not supported. Use Bearer instead.' ), $authType), - ], 401); + ], self::STATUS_UNAUTHORIZED); } try { @@ -126,8 +112,7 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface // Update the token expiration and continue to next middleware $jwt = $this->jwtService->refresh($jwt); - /** @var Response $response */ - $response = $out($request, $response); + $response = $delegate->process($request); // Return the response with the updated token on it return $response->withHeader(self::AUTHORIZATION_HEADER, 'Bearer ' . $jwt); @@ -150,6 +135,6 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface ), self::AUTHORIZATION_HEADER ), - ], 401); + ], self::STATUS_UNAUTHORIZED); } } diff --git a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php index 4f472726..65523f62 100644 --- a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php @@ -1,10 +1,13 @@ assertFalse($isCalled); - $this->middleware->__invoke($request, $response, function ($req, $resp) use (&$isCalled) { - $isCalled = true; - }); - $this->assertTrue($isCalled); + $delegate = $this->prophesize(DelegateInterface::class); + /** @var MethodProphecy $process */ + $process = $delegate->process($request)->willReturn(new Response()); + + $this->middleware->process($request, $delegate->reveal()); + $process->shouldHaveBeenCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRouteFailure(['GET']) ); - $response = new Response(); - $isCalled = false; - $this->assertFalse($isCalled); - $this->middleware->__invoke($request, $response, function ($req, $resp) use (&$isCalled) { - $isCalled = true; - }); - $this->assertTrue($isCalled); + $delegate = $this->prophesize(DelegateInterface::class); + /** @var MethodProphecy $process */ + $process = $delegate->process($request)->willReturn(new Response()); + $this->middleware->process($request, $delegate->reveal()); + $process->shouldHaveBeenCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRoute(new Route('foo', '', Route::HTTP_METHOD_ANY, 'rest-authenticate'), []) ); - $response = new Response(); - $isCalled = false; - $this->assertFalse($isCalled); - $this->middleware->__invoke($request, $response, function ($req, $resp) use (&$isCalled) { - $isCalled = true; - }); - $this->assertTrue($isCalled); + $delegate = $this->prophesize(DelegateInterface::class); + /** @var MethodProphecy $process */ + $process = $delegate->process($request)->willReturn(new Response()); + $this->middleware->process($request, $delegate->reveal()); + $process->shouldHaveBeenCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRoute(new Route('bar', 'foo'), []) )->withMethod('OPTIONS'); - $response = new Response(); - $isCalled = false; - $this->assertFalse($isCalled); - $this->middleware->__invoke($request, $response, function ($req, $resp) use (&$isCalled) { - $isCalled = true; - }); - $this->assertTrue($isCalled); + $delegate = $this->prophesize(DelegateInterface::class); + /** @var MethodProphecy $process */ + $process = $delegate->process($request)->willReturn(new Response()); + $this->middleware->process($request, $delegate->reveal()); + $process->shouldHaveBeenCalledTimes(1); } /** @@ -88,7 +84,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase RouteResult::class, RouteResult::fromRoute(new Route('bar', 'foo'), []) ); - $response = $this->middleware->__invoke($request, new Response()); + $response = $this->middleware->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(401, $response->getStatusCode()); } @@ -103,7 +99,8 @@ class CheckAuthenticationMiddlewareTest extends TestCase RouteResult::fromRoute(new Route('bar', 'foo'), []) )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, $authToken); - $response = $this->middleware->__invoke($request, new Response()); + $response = $this->middleware->process($request, TestUtils::createDelegateMock()->reveal()); + $this->assertEquals(401, $response->getStatusCode()); $this->assertTrue(strpos($response->getBody()->getContents(), 'You need to provide the Bearer type') > 0); } @@ -119,7 +116,8 @@ class CheckAuthenticationMiddlewareTest extends TestCase RouteResult::fromRoute(new Route('bar', 'foo'), []) )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, 'Basic ' . $authToken); - $response = $this->middleware->__invoke($request, new Response()); + $response = $this->middleware->process($request, TestUtils::createDelegateMock()->reveal()); + $this->assertEquals(401, $response->getStatusCode()); $this->assertTrue( strpos($response->getBody()->getContents(), 'Provided authorization type Basic is not supported') > 0 @@ -138,14 +136,14 @@ class CheckAuthenticationMiddlewareTest extends TestCase )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, 'Bearer ' . $authToken); $this->jwtService->verify($authToken)->willReturn(false)->shouldBeCalledTimes(1); - $response = $this->middleware->__invoke($request, new Response()); + $response = $this->middleware->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(401, $response->getStatusCode()); } /** * @test */ - public function provideCorrectTokenUpdatesExpirationAndFallbacksToNextMiddleware() + public function provideCorrectTokenUpdatesExpirationAndFallsBackToNextMiddleware() { $authToken = 'ABC-abc'; $request = ServerRequestFactory::fromGlobals()->withAttribute( @@ -155,12 +153,12 @@ class CheckAuthenticationMiddlewareTest extends TestCase $this->jwtService->verify($authToken)->willReturn(true)->shouldBeCalledTimes(1); $this->jwtService->refresh($authToken)->willReturn($authToken)->shouldBeCalledTimes(1); - $isCalled = false; - $this->assertFalse($isCalled); - $this->middleware->__invoke($request, new Response(), function ($req, $resp) use (&$isCalled) { - $isCalled = true; - return $resp; - }); - $this->assertTrue($isCalled); + $delegate = $this->prophesize(DelegateInterface::class); + /** @var MethodProphecy $process */ + $process = $delegate->process($request)->willReturn(new Response()); + $resp = $this->middleware->process($request, $delegate->reveal()); + + $process->shouldHaveBeenCalledTimes(1); + $this->assertArrayHasKey(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, $resp->getHeaders()); } }