From e72c2cffbccef5c88d290fe5391f02b7bcc6e884 Mon Sep 17 00:00:00 2001 From: Westin Shafer Date: Mon, 26 Feb 2018 17:42:55 -0700 Subject: [PATCH 1/7] Remove dependancy on Zend\Expressive\Authentication\ResponsePrototypeTrait and use factoryFactories instead --- src/OAuth2Adapter.php | 12 +++--- src/OAuth2AdapterFactory.php | 6 +-- src/OAuth2Middleware.php | 30 ++++++++------ src/OAuth2MiddlewareFactory.php | 6 +-- test/OAuth2AdapterFactoryTest.php | 5 ++- test/OAuth2AdapterTest.php | 22 ++++++++--- test/OAuth2MiddlewareFactoryTest.php | 4 +- test/OAuth2MiddlewareTest.php | 32 +++++++++++---- test/Pdo/OAuth2PdoMiddlewareTest.php | 59 ++++++++++++++++++++++++---- 9 files changed, 128 insertions(+), 48 deletions(-) diff --git a/src/OAuth2Adapter.php b/src/OAuth2Adapter.php index 32e1047..d54dc96 100644 --- a/src/OAuth2Adapter.php +++ b/src/OAuth2Adapter.php @@ -28,9 +28,9 @@ class OAuth2Adapter implements AuthenticationInterface protected $resourceServer; /** - * @var ResponseInterface + * @var callable */ - protected $responsePrototype; + protected $responseFactory; /** * Constructor @@ -38,10 +38,12 @@ class OAuth2Adapter implements AuthenticationInterface * @param ResourceServer $resourceServer * @param ResponseInterface $responsePrototype */ - public function __construct(ResourceServer $resourceServer, ResponseInterface $responsePrototype) + public function __construct(ResourceServer $resourceServer, callable $responseFactory) { $this->resourceServer = $resourceServer; - $this->responsePrototype = $responsePrototype; + $this->responseFactory = function () use ($responseFactory) : ResponseInterface { + return $responseFactory(); + }; } /** @@ -66,7 +68,7 @@ public function authenticate(ServerRequestInterface $request) : ?UserInterface */ public function unauthorizedResponse(ServerRequestInterface $request) : ResponseInterface { - return $this->responsePrototype + return ($this->responseFactory)() ->withHeader( 'WWW-Authenticate', 'Bearer token-example' diff --git a/src/OAuth2AdapterFactory.php b/src/OAuth2AdapterFactory.php index e64ac96..733cb4d 100644 --- a/src/OAuth2AdapterFactory.php +++ b/src/OAuth2AdapterFactory.php @@ -12,13 +12,11 @@ use League\OAuth2\Server\ResourceServer; use Psr\Container\ContainerInterface; +use Psr\Http\Message\ResponseInterface; use Zend\Expressive\Authentication\OAuth2\Exception; -use Zend\Expressive\Authentication\ResponsePrototypeTrait; class OAuth2AdapterFactory { - use ResponsePrototypeTrait; - public function __invoke(ContainerInterface $container) : OAuth2Adapter { $resourceServer = $container->has(ResourceServer::class) @@ -33,7 +31,7 @@ public function __invoke(ContainerInterface $container) : OAuth2Adapter return new OAuth2Adapter( $resourceServer, - $this->getResponsePrototype($container) + $container->get(ResponseInterface::class) ); } } diff --git a/src/OAuth2Middleware.php b/src/OAuth2Middleware.php index 346ad58..d7ddd40 100644 --- a/src/OAuth2Middleware.php +++ b/src/OAuth2Middleware.php @@ -26,9 +26,9 @@ class OAuth2Middleware implements MiddlewareInterface protected $server; /** - * @var ResponseInterface + * @var callable */ - protected $responsePrototype; + protected $responseFactory; /** * Constructor @@ -36,10 +36,12 @@ class OAuth2Middleware implements MiddlewareInterface * @param AuthorizationServer $server * @param ResponseInterface $responsePrototype */ - public function __construct(AuthorizationServer $server, ResponseInterface $responsePrototype) + public function __construct(AuthorizationServer $server, callable $responseFactory) { $this->server = $server; - $this->responsePrototype = $responsePrototype; + $this->responseFactory = function () use ($responseFactory) : ResponseInterface { + return $responseFactory(); + }; } /** @@ -54,7 +56,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface case 'POST': return $this->accessTokenRequest($request); } - return $this->responsePrototype->withStatus(501); // Method not implemented + return ($this->responseFactory)()->withStatus(501); // Method not implemented } /** @@ -69,6 +71,9 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface */ protected function authorizationRequest(ServerRequestInterface $request) : ResponseInterface { + // Create a new response for the request + $response = ($this->responseFactory)(); + try { // Validate the HTTP request and return an AuthorizationRequest object. $authRequest = $this->server->validateAuthorizationRequest($request); @@ -87,12 +92,12 @@ protected function authorizationRequest(ServerRequestInterface $request) : Respo $authRequest->setAuthorizationApproved(true); // Return the HTTP redirect response - return $this->server->completeAuthorizationRequest($authRequest, $this->responsePrototype); + return $this->server->completeAuthorizationRequest($authRequest, $response); } catch (OAuthServerException $exception) { - return $exception->generateHttpResponse($this->responsePrototype); + return $exception->generateHttpResponse($response); } catch (\Exception $exception) { return (new OAuthServerException($exception->getMessage(), 0, 'unknown_error', 500)) - ->generateHttpResponse($this->responsePrototype); + ->generateHttpResponse($response); } } @@ -109,13 +114,16 @@ protected function authorizationRequest(ServerRequestInterface $request) : Respo */ protected function accessTokenRequest(ServerRequestInterface $request) : ResponseInterface { + // Create a new response for the request + $response = ($this->responseFactory)(); + try { - return $this->server->respondToAccessTokenRequest($request, $this->responsePrototype); + return $this->server->respondToAccessTokenRequest($request, $response); } catch (OAuthServerException $exception) { - return $exception->generateHttpResponse($this->responsePrototype); + return $exception->generateHttpResponse($response); } catch (\Exception $exception) { return (new OAuthServerException($exception->getMessage(), 0, 'unknown_error', 500)) - ->generateHttpResponse($this->responsePrototype); + ->generateHttpResponse($response); } } } diff --git a/src/OAuth2MiddlewareFactory.php b/src/OAuth2MiddlewareFactory.php index 643f3d4..0a980ae 100644 --- a/src/OAuth2MiddlewareFactory.php +++ b/src/OAuth2MiddlewareFactory.php @@ -12,12 +12,10 @@ use League\OAuth2\Server\AuthorizationServer; use Psr\Container\ContainerInterface; -use Zend\Expressive\Authentication\ResponsePrototypeTrait; +use Psr\Http\Message\ResponseInterface; class OAuth2MiddlewareFactory { - use ResponsePrototypeTrait; - public function __invoke(ContainerInterface $container) : OAuth2Middleware { $authServer = $container->has(AuthorizationServer::class) @@ -33,7 +31,7 @@ public function __invoke(ContainerInterface $container) : OAuth2Middleware return new OAuth2Middleware( $authServer, - $this->getResponsePrototype($container) + $container->get(ResponseInterface::class) ); } } diff --git a/test/OAuth2AdapterFactoryTest.php b/test/OAuth2AdapterFactoryTest.php index 3df5cb3..a0603bb 100644 --- a/test/OAuth2AdapterFactoryTest.php +++ b/test/OAuth2AdapterFactoryTest.php @@ -10,6 +10,7 @@ namespace ZendTest\Expressive\Authentication\OAuth2; +use function foo\func; use League\OAuth2\Server\ResourceServer; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; @@ -52,8 +53,8 @@ public function testInvokeWithResourceServerEmptyResponse() ->willReturn($this->resourceServer->reveal()); $this->container - ->has(ResponseInterface::class) - ->willReturn(false); + ->get(ResponseInterface::class) + ->willReturn(function () {}); $factory = new OAuth2AdapterFactory(); $adapter = $factory($this->container->reveal()); diff --git a/test/OAuth2AdapterTest.php b/test/OAuth2AdapterTest.php index 72eea1b..645a45e 100644 --- a/test/OAuth2AdapterTest.php +++ b/test/OAuth2AdapterTest.php @@ -30,9 +30,13 @@ public function setUp() public function testConstructor() { + $factory = function () { + return; + }; + $adapter = new OAuth2Adapter( $this->resourceServer->reveal(), - $this->response->reveal() + $factory ); $this->assertInstanceOf(OAuth2Adapter::class, $adapter); $this->assertInstanceOf(AuthenticationInterface::class, $adapter); @@ -50,7 +54,9 @@ public function testOAuthServerExceptionRaisedDuringAuthenticateResultsInInvalid $adapter = new OAuth2Adapter( $this->resourceServer->reveal(), - $this->response->reveal() + function () { + return $this->response->reveal(); + } ); $this->assertNull($adapter->authenticate($request->reveal())); @@ -67,7 +73,9 @@ public function testAuthenticateReturnsNullIfResourceServerDoesNotProduceAUserId $adapter = new OAuth2Adapter( $this->resourceServer->reveal(), - $this->response->reveal() + function () { + return $this->response->reveal(); + } ); $this->assertNull($adapter->authenticate($request->reveal())); @@ -84,7 +92,9 @@ public function testAuthenticateReturnsAUserIfTheResourceServerProducesAUserId() $adapter = new OAuth2Adapter( $this->resourceServer->reveal(), - $this->response->reveal() + function () { + return $this->response->reveal(); + } ); $user = $adapter->authenticate($request->reveal()); @@ -107,7 +117,9 @@ public function testUnauthorizedResponseProducesAResponseWithAWwwAuthenticateHea $adapter = new OAuth2Adapter( $this->resourceServer->reveal(), - $this->response->reveal() + function () { + return $this->response->reveal(); + } ); $this->assertSame( diff --git a/test/OAuth2MiddlewareFactoryTest.php b/test/OAuth2MiddlewareFactoryTest.php index 1a80c7f..59e6881 100644 --- a/test/OAuth2MiddlewareFactoryTest.php +++ b/test/OAuth2MiddlewareFactoryTest.php @@ -53,8 +53,8 @@ public function testInvokeWithAuthServerWithoutResponseInterface() ->get(AuthorizationServer::class) ->willReturn($this->authServer->reveal()); $this->container - ->has(ResponseInterface::class) - ->willReturn(false); + ->get(ResponseInterface::class) + ->willReturn(function () {}); $factory = new OAuth2MiddlewareFactory(); $middleware = $factory($this->container->reveal()); diff --git a/test/OAuth2MiddlewareTest.php b/test/OAuth2MiddlewareTest.php index 5dfe8f5..0512748 100644 --- a/test/OAuth2MiddlewareTest.php +++ b/test/OAuth2MiddlewareTest.php @@ -38,7 +38,9 @@ public function testConstructor() { $middleware = new OAuth2Middleware( $this->authServer->reveal(), - $this->response->reveal() + function () { + return $this->response->reveal(); + } ); $this->assertInstanceOf(OAuth2Middleware::class, $middleware); $this->assertInstanceOf(MiddlewareInterface::class, $middleware); @@ -69,7 +71,9 @@ public function testProcessWithGet() $middleware = new OAuth2Middleware( $this->authServer->reveal(), - $this->response->reveal() + function () { + return $this->response->reveal(); + } ); $response = $middleware->process( $this->serverRequest->reveal(), @@ -92,7 +96,9 @@ public function testProcessWithPost() $middleware = new OAuth2Middleware( $this->authServer->reveal(), - $this->response->reveal() + function () { + return $this->response->reveal(); + } ); $response = $middleware->process( $this->serverRequest->reveal(), @@ -117,7 +123,9 @@ public function testAuthorizationRequestRaisingOAuthServerExceptionGeneratesResp $middleware = new OAuth2Middleware( $this->authServer->reveal(), - $this->response->reveal() + function () { + return $this->response->reveal(); + } ); $this->serverRequest->getMethod()->willReturn('GET'); @@ -157,7 +165,9 @@ public function testAuthorizationRequestRaisingUnknownExceptionGeneratesResponse $middleware = new OAuth2Middleware( $this->authServer->reveal(), - $this->response->reveal() + function () { + return $this->response->reveal(); + } ); $this->serverRequest->getMethod()->willReturn('GET'); @@ -177,7 +187,9 @@ public function testReturns501ResponseForInvalidMethods() $middleware = new OAuth2Middleware( $this->authServer->reveal(), - $this->response->reveal() + function () { + return $this->response->reveal(); + } ); $response = $middleware->process( @@ -206,7 +218,9 @@ public function testPostRequestResultingInOAuthServerExceptionUsesExceptionToGen $middleware = new OAuth2Middleware( $this->authServer->reveal(), - $this->response->reveal() + function () { + return $this->response->reveal(); + } ); $response = $middleware->process( @@ -247,7 +261,9 @@ public function testPostRequestResultingInGenericExceptionCastsExceptionToOauthS $middleware = new OAuth2Middleware( $this->authServer->reveal(), - $this->response->reveal() + function () { + return $this->response->reveal(); + } ); $response = $middleware->process( diff --git a/test/Pdo/OAuth2PdoMiddlewareTest.php b/test/Pdo/OAuth2PdoMiddlewareTest.php index ac2829b..ce2bc21 100644 --- a/test/Pdo/OAuth2PdoMiddlewareTest.php +++ b/test/Pdo/OAuth2PdoMiddlewareTest.php @@ -92,7 +92,12 @@ public function setUp() public function testConstructor() { - $authMiddleware = new OAuth2Middleware($this->authServer, $this->response); + $authMiddleware = new OAuth2Middleware( + $this->authServer, + function() { + return $this->response; + } + ); $this->assertInstanceOf(OAuth2Middleware::class, $authMiddleware); } @@ -123,7 +128,14 @@ public function testProcessClientCredentialGrant() $params, [ 'Content-Type' => 'application/x-www-form-urlencoded' ] ); - $authMiddleware = new OAuth2Middleware($this->authServer, $this->response); + + $authMiddleware = new OAuth2Middleware( + $this->authServer, + function() { + return $this->response; + } + ); + $response = $authMiddleware->process($request, $this->handler->reveal()); $this->assertEquals(200, $response->getStatusCode()); @@ -166,7 +178,14 @@ public function testProcessPasswordGrant() $params, [ 'Content-Type' => 'application/x-www-form-urlencoded' ] ); - $authMiddleware = new OAuth2Middleware($this->authServer, $this->response); + + $authMiddleware = new OAuth2Middleware( + $this->authServer, + function() { + return $this->response; + } + ); + $response = $authMiddleware->process($request, $this->handler->reveal()); $this->assertEquals(200, $response->getStatusCode()); @@ -214,7 +233,13 @@ public function testProcessGetAuthorizationCode() $params ); - $authMiddleware = new OAuth2Middleware($this->authServer, $this->response); + $authMiddleware = new OAuth2Middleware( + $this->authServer, + function() { + return $this->response; + } + ); + $response = $authMiddleware->process($request, $this->handler->reveal()); $this->assertEquals(302, $response->getStatusCode()); @@ -265,7 +290,14 @@ public function testProcessFromAuthorizationCode(string $code) $params, [ 'Content-Type' => 'application/x-www-form-urlencoded' ] ); - $authMiddleware = new OAuth2Middleware($this->authServer, $this->response); + + $authMiddleware = new OAuth2Middleware( + $this->authServer, + function() { + return $this->response; + } + ); + $response = $authMiddleware->process($request, $this->handler->reveal()); $this->assertEquals(200, $response->getStatusCode()); @@ -307,7 +339,14 @@ public function testProcessImplicitGrant() [], $params ); - $authMiddleware = new OAuth2Middleware($this->authServer, $this->response); + + $authMiddleware = new OAuth2Middleware( + $this->authServer, + function() { + return $this->response; + } + ); + $response = $authMiddleware->process($request, $this->handler->reveal()); $this->assertEquals(302, $response->getStatusCode()); @@ -356,7 +395,13 @@ public function testProcessRefreshTokenGrant(string $refreshToken) [ 'Content-Type' => 'application/x-www-form-urlencoded' ] ); - $authMiddleware = new OAuth2Middleware($this->authServer, $this->response); + $authMiddleware = new OAuth2Middleware( + $this->authServer, + function() { + return $this->response; + } + ); + $response = $authMiddleware->process($request, $this->handler->reveal()); $this->assertEquals(200, $response->getStatusCode()); From ad822a02df961867a1aaaa678a6219003ef0ddc9 Mon Sep 17 00:00:00 2001 From: Westin Shafer Date: Mon, 26 Feb 2018 17:58:55 -0700 Subject: [PATCH 2/7] Fix Styles --- test/OAuth2AdapterFactoryTest.php | 3 ++- test/OAuth2MiddlewareFactoryTest.php | 3 ++- test/Pdo/OAuth2PdoMiddlewareTest.php | 14 +++++++------- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/test/OAuth2AdapterFactoryTest.php b/test/OAuth2AdapterFactoryTest.php index a0603bb..728b1ca 100644 --- a/test/OAuth2AdapterFactoryTest.php +++ b/test/OAuth2AdapterFactoryTest.php @@ -54,7 +54,8 @@ public function testInvokeWithResourceServerEmptyResponse() $this->container ->get(ResponseInterface::class) - ->willReturn(function () {}); + ->willReturn(function () { + }); $factory = new OAuth2AdapterFactory(); $adapter = $factory($this->container->reveal()); diff --git a/test/OAuth2MiddlewareFactoryTest.php b/test/OAuth2MiddlewareFactoryTest.php index 59e6881..4ae51bb 100644 --- a/test/OAuth2MiddlewareFactoryTest.php +++ b/test/OAuth2MiddlewareFactoryTest.php @@ -54,7 +54,8 @@ public function testInvokeWithAuthServerWithoutResponseInterface() ->willReturn($this->authServer->reveal()); $this->container ->get(ResponseInterface::class) - ->willReturn(function () {}); + ->willReturn(function () { + }); $factory = new OAuth2MiddlewareFactory(); $middleware = $factory($this->container->reveal()); diff --git a/test/Pdo/OAuth2PdoMiddlewareTest.php b/test/Pdo/OAuth2PdoMiddlewareTest.php index ce2bc21..6ac5006 100644 --- a/test/Pdo/OAuth2PdoMiddlewareTest.php +++ b/test/Pdo/OAuth2PdoMiddlewareTest.php @@ -94,7 +94,7 @@ public function testConstructor() { $authMiddleware = new OAuth2Middleware( $this->authServer, - function() { + function () { return $this->response; } ); @@ -131,7 +131,7 @@ public function testProcessClientCredentialGrant() $authMiddleware = new OAuth2Middleware( $this->authServer, - function() { + function () { return $this->response; } ); @@ -181,7 +181,7 @@ public function testProcessPasswordGrant() $authMiddleware = new OAuth2Middleware( $this->authServer, - function() { + function () { return $this->response; } ); @@ -235,7 +235,7 @@ public function testProcessGetAuthorizationCode() $authMiddleware = new OAuth2Middleware( $this->authServer, - function() { + function () { return $this->response; } ); @@ -293,7 +293,7 @@ public function testProcessFromAuthorizationCode(string $code) $authMiddleware = new OAuth2Middleware( $this->authServer, - function() { + function () { return $this->response; } ); @@ -342,7 +342,7 @@ public function testProcessImplicitGrant() $authMiddleware = new OAuth2Middleware( $this->authServer, - function() { + function () { return $this->response; } ); @@ -397,7 +397,7 @@ public function testProcessRefreshTokenGrant(string $refreshToken) $authMiddleware = new OAuth2Middleware( $this->authServer, - function() { + function () { return $this->response; } ); From f997322a6b8a89855fe81a064a8a751e7494e099 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Tue, 27 Feb 2018 09:17:36 -0600 Subject: [PATCH 3/7] Removes redundant docblocks --- src/OAuth2Adapter.php | 6 ------ src/OAuth2Middleware.php | 6 ------ 2 files changed, 12 deletions(-) diff --git a/src/OAuth2Adapter.php b/src/OAuth2Adapter.php index d54dc96..f1278f1 100644 --- a/src/OAuth2Adapter.php +++ b/src/OAuth2Adapter.php @@ -32,12 +32,6 @@ class OAuth2Adapter implements AuthenticationInterface */ protected $responseFactory; - /** - * Constructor - * - * @param ResourceServer $resourceServer - * @param ResponseInterface $responsePrototype - */ public function __construct(ResourceServer $resourceServer, callable $responseFactory) { $this->resourceServer = $resourceServer; diff --git a/src/OAuth2Middleware.php b/src/OAuth2Middleware.php index d7ddd40..e2219a6 100644 --- a/src/OAuth2Middleware.php +++ b/src/OAuth2Middleware.php @@ -30,12 +30,6 @@ class OAuth2Middleware implements MiddlewareInterface */ protected $responseFactory; - /** - * Constructor - * - * @param AuthorizationServer $server - * @param ResponseInterface $responsePrototype - */ public function __construct(AuthorizationServer $server, callable $responseFactory) { $this->server = $server; From 87acb14d9a8b3d98af7431874537b0f72c217358 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Tue, 27 Feb 2018 09:18:49 -0600 Subject: [PATCH 4/7] Remove unused import --- test/OAuth2AdapterFactoryTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/test/OAuth2AdapterFactoryTest.php b/test/OAuth2AdapterFactoryTest.php index 728b1ca..ebb7544 100644 --- a/test/OAuth2AdapterFactoryTest.php +++ b/test/OAuth2AdapterFactoryTest.php @@ -10,7 +10,6 @@ namespace ZendTest\Expressive\Authentication\OAuth2; -use function foo\func; use League\OAuth2\Server\ResourceServer; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; From d5d8617138a8e21118507ae6802b34870d43e596 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Tue, 27 Feb 2018 09:32:47 -0600 Subject: [PATCH 5/7] Refactors tests - Adds property declarations, with annotations detailing types. - Use a `$responseFactory` property instead of creating a factory each time the SUT is instantiated. --- test/OAuth2AdapterFactoryTest.php | 26 ++++++++--- test/OAuth2AdapterTest.php | 38 ++++++++-------- test/OAuth2MiddlewareTest.php | 64 ++++++++++++++------------- test/Pdo/OAuth2PdoMiddlewareTest.php | 65 +++++++++++++++++++--------- 4 files changed, 118 insertions(+), 75 deletions(-) diff --git a/test/OAuth2AdapterFactoryTest.php b/test/OAuth2AdapterFactoryTest.php index ebb7544..b06e8ca 100644 --- a/test/OAuth2AdapterFactoryTest.php +++ b/test/OAuth2AdapterFactoryTest.php @@ -12,6 +12,7 @@ use League\OAuth2\Server\ResourceServer; use PHPUnit\Framework\TestCase; +use Prophecy\Prophecy\ObjectProphecy; use Psr\Container\ContainerInterface; use Psr\Http\Message\ResponseInterface; use Zend\Expressive\Authentication\AuthenticationInterface; @@ -20,11 +21,26 @@ class OAuth2AdapterFactoryTest extends TestCase { + /** @var ContainerInterface|ObjectProphecy */ + private $container; + + /** @var ResourceServer|ObjectProphecy */ + private $resourceServer; + + /** @var ResponseInterface|ObjectProphecy */ + private $response; + + /** @var callable */ + private $responseFactory; + public function setUp() { - $this->container = $this->prophesize(ContainerInterface::class); - $this->resourceServer = $this->prophesize(ResourceServer::class); - $this->response = $this->prophesize(ResponseInterface::class); + $this->container = $this->prophesize(ContainerInterface::class); + $this->resourceServer = $this->prophesize(ResourceServer::class); + $this->response = $this->prophesize(ResponseInterface::class); + $this->responseFactory = function () { + return $this->response->reveal(); + }; } public function testConstructor() @@ -77,9 +93,7 @@ public function testInvokeResourceServerAndResponse() ->willReturn(true); $this->container ->get(ResponseInterface::class) - ->willReturn(function () { - return $this->response->reveal(); - }); + ->willReturn($this->responseFactory); $factory = new OAuth2AdapterFactory(); $adapter = $factory($this->container->reveal()); diff --git a/test/OAuth2AdapterTest.php b/test/OAuth2AdapterTest.php index 645a45e..bed3ce8 100644 --- a/test/OAuth2AdapterTest.php +++ b/test/OAuth2AdapterTest.php @@ -22,21 +22,29 @@ class OAuth2AdapterTest extends TestCase { + /** @var ResourceServer|ObjectProphecy */ + private $resourceServer; + + /** @var ResponseInterface|ObjectProphecy */ + private $response; + + /** @var callable */ + private $responseFactory; + public function setUp() { - $this->resourceServer = $this->prophesize(ResourceServer::class); - $this->response = $this->prophesize(ResponseInterface::class); + $this->resourceServer = $this->prophesize(ResourceServer::class); + $this->response = $this->prophesize(ResponseInterface::class); + $this->responseFactory = function () { + return $this->response->reveal(); + }; } public function testConstructor() { - $factory = function () { - return; - }; - $adapter = new OAuth2Adapter( $this->resourceServer->reveal(), - $factory + $this->responseFactory ); $this->assertInstanceOf(OAuth2Adapter::class, $adapter); $this->assertInstanceOf(AuthenticationInterface::class, $adapter); @@ -54,9 +62,7 @@ public function testOAuthServerExceptionRaisedDuringAuthenticateResultsInInvalid $adapter = new OAuth2Adapter( $this->resourceServer->reveal(), - function () { - return $this->response->reveal(); - } + $this->responseFactory ); $this->assertNull($adapter->authenticate($request->reveal())); @@ -73,9 +79,7 @@ public function testAuthenticateReturnsNullIfResourceServerDoesNotProduceAUserId $adapter = new OAuth2Adapter( $this->resourceServer->reveal(), - function () { - return $this->response->reveal(); - } + $this->responseFactory ); $this->assertNull($adapter->authenticate($request->reveal())); @@ -92,9 +96,7 @@ public function testAuthenticateReturnsAUserIfTheResourceServerProducesAUserId() $adapter = new OAuth2Adapter( $this->resourceServer->reveal(), - function () { - return $this->response->reveal(); - } + $this->responseFactory ); $user = $adapter->authenticate($request->reveal()); @@ -117,9 +119,7 @@ public function testUnauthorizedResponseProducesAResponseWithAWwwAuthenticateHea $adapter = new OAuth2Adapter( $this->resourceServer->reveal(), - function () { - return $this->response->reveal(); - } + $this->responseFactory ); $this->assertSame( diff --git a/test/OAuth2MiddlewareTest.php b/test/OAuth2MiddlewareTest.php index 0512748..b84a213 100644 --- a/test/OAuth2MiddlewareTest.php +++ b/test/OAuth2MiddlewareTest.php @@ -15,6 +15,7 @@ use League\OAuth2\Server\RequestTypes\AuthorizationRequest; use PHPUnit\Framework\TestCase; use Prophecy\Argument; +use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\StreamInterface; @@ -25,22 +26,41 @@ class OAuth2MiddlewareTest extends TestCase { + /** @var AuthorizationRequest|ObjectProphecy */ + private $authRequest; + + /** @var AuthorizationServer|ObjectProphecy */ + private $authServer; + + /** @var RequestHandlerInterface|ObjectProphecy */ + private $handler; + + /** @var ResponseInterface|ObjectProphecy */ + private $response; + + /** @var callable */ + private $responseFactory; + + /** @var ServerRequestInterface|ObjectProphecy */ + private $serverRequest; + public function setUp() { - $this->authServer = $this->prophesize(AuthorizationServer::class); - $this->response = $this->prophesize(ResponseInterface::class); - $this->serverRequest = $this->prophesize(ServerRequestInterface::class); - $this->authRequest = $this->prophesize(AuthorizationRequest::class); - $this->handler = $this->prophesize(RequestHandlerInterface::class); + $this->authServer = $this->prophesize(AuthorizationServer::class); + $this->response = $this->prophesize(ResponseInterface::class); + $this->serverRequest = $this->prophesize(ServerRequestInterface::class); + $this->authRequest = $this->prophesize(AuthorizationRequest::class); + $this->handler = $this->prophesize(RequestHandlerInterface::class); + $this->responseFactory = function () { + return $this->response->reveal(); + }; } public function testConstructor() { $middleware = new OAuth2Middleware( $this->authServer->reveal(), - function () { - return $this->response->reveal(); - } + $this->responseFactory ); $this->assertInstanceOf(OAuth2Middleware::class, $middleware); $this->assertInstanceOf(MiddlewareInterface::class, $middleware); @@ -71,9 +91,7 @@ public function testProcessWithGet() $middleware = new OAuth2Middleware( $this->authServer->reveal(), - function () { - return $this->response->reveal(); - } + $this->responseFactory ); $response = $middleware->process( $this->serverRequest->reveal(), @@ -96,9 +114,7 @@ public function testProcessWithPost() $middleware = new OAuth2Middleware( $this->authServer->reveal(), - function () { - return $this->response->reveal(); - } + $this->responseFactory ); $response = $middleware->process( $this->serverRequest->reveal(), @@ -123,9 +139,7 @@ public function testAuthorizationRequestRaisingOAuthServerExceptionGeneratesResp $middleware = new OAuth2Middleware( $this->authServer->reveal(), - function () { - return $this->response->reveal(); - } + $this->responseFactory ); $this->serverRequest->getMethod()->willReturn('GET'); @@ -165,9 +179,7 @@ public function testAuthorizationRequestRaisingUnknownExceptionGeneratesResponse $middleware = new OAuth2Middleware( $this->authServer->reveal(), - function () { - return $this->response->reveal(); - } + $this->responseFactory ); $this->serverRequest->getMethod()->willReturn('GET'); @@ -187,9 +199,7 @@ public function testReturns501ResponseForInvalidMethods() $middleware = new OAuth2Middleware( $this->authServer->reveal(), - function () { - return $this->response->reveal(); - } + $this->responseFactory ); $response = $middleware->process( @@ -218,9 +228,7 @@ public function testPostRequestResultingInOAuthServerExceptionUsesExceptionToGen $middleware = new OAuth2Middleware( $this->authServer->reveal(), - function () { - return $this->response->reveal(); - } + $this->responseFactory ); $response = $middleware->process( @@ -261,9 +269,7 @@ public function testPostRequestResultingInGenericExceptionCastsExceptionToOauthS $middleware = new OAuth2Middleware( $this->authServer->reveal(), - function () { - return $this->response->reveal(); - } + $this->responseFactory ); $response = $middleware->process( diff --git a/test/Pdo/OAuth2PdoMiddlewareTest.php b/test/Pdo/OAuth2PdoMiddlewareTest.php index 6ac5006..344c78b 100644 --- a/test/Pdo/OAuth2PdoMiddlewareTest.php +++ b/test/Pdo/OAuth2PdoMiddlewareTest.php @@ -19,6 +19,7 @@ use League\OAuth2\Server\Grant\RefreshTokenGrant; use PDO; use PHPUnit\Framework\TestCase; +use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequest; @@ -40,6 +41,39 @@ class OAuth2PdoMiddlewareTest extends TestCase const PRIVATE_KEY = __DIR__ .'/../TestAsset/private.key'; const ENCRYPTION_KEY = 'T2x2+1OGrEzfS+01OUmwhOcJiGmE58UD1fllNn6CGcQ='; + /** @var AccessTokenRepository */ + private $accessTokenRepository; + + /** @var AuthCodeRepository */ + private $authCodeRepository; + + /** @var AuthServer */ + private $authServer; + + /** @var ClientRepository */ + private $clientRepository; + + /** @var RequestHandlerInterface|ObjectProphecy */ + private $handler; + + /** @var PdoService */ + private $pdoService; + + /** @var RefreshTokenRepository */ + private $refreshTokenRepository; + + /** @var Response */ + private $response; + + /** @var callable */ + private $responseFactory; + + /** @var ScopeRepository */ + private $scopeRepository; + + /** @var UserRepository */ + private $userRepository; + public static function setUpBeforeClass() { self::tearDownAfterClass(); @@ -88,15 +122,16 @@ public function setUp() ); $this->handler = $this->prophesize(RequestHandlerInterface::class); + $this->responseFactory = function () { + return $this->response; + }; } public function testConstructor() { $authMiddleware = new OAuth2Middleware( $this->authServer, - function () { - return $this->response; - } + $this->responseFactory ); $this->assertInstanceOf(OAuth2Middleware::class, $authMiddleware); } @@ -131,9 +166,7 @@ public function testProcessClientCredentialGrant() $authMiddleware = new OAuth2Middleware( $this->authServer, - function () { - return $this->response; - } + $this->responseFactory ); $response = $authMiddleware->process($request, $this->handler->reveal()); @@ -181,9 +214,7 @@ public function testProcessPasswordGrant() $authMiddleware = new OAuth2Middleware( $this->authServer, - function () { - return $this->response; - } + $this->responseFactory ); $response = $authMiddleware->process($request, $this->handler->reveal()); @@ -235,9 +266,7 @@ public function testProcessGetAuthorizationCode() $authMiddleware = new OAuth2Middleware( $this->authServer, - function () { - return $this->response; - } + $this->responseFactory ); $response = $authMiddleware->process($request, $this->handler->reveal()); @@ -293,9 +322,7 @@ public function testProcessFromAuthorizationCode(string $code) $authMiddleware = new OAuth2Middleware( $this->authServer, - function () { - return $this->response; - } + $this->responseFactory ); $response = $authMiddleware->process($request, $this->handler->reveal()); @@ -342,9 +369,7 @@ public function testProcessImplicitGrant() $authMiddleware = new OAuth2Middleware( $this->authServer, - function () { - return $this->response; - } + $this->responseFactory ); $response = $authMiddleware->process($request, $this->handler->reveal()); @@ -397,9 +422,7 @@ public function testProcessRefreshTokenGrant(string $refreshToken) $authMiddleware = new OAuth2Middleware( $this->authServer, - function () { - return $this->response; - } + $this->responseFactory ); $response = $authMiddleware->process($request, $this->handler->reveal()); From f9ce613482a2dbf64bb841c7619b3d33f39890b4 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Tue, 27 Feb 2018 09:36:21 -0600 Subject: [PATCH 6/7] Adds CHANGELOG entries for #17 --- CHANGELOG.md | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4aa27bb..ea094f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,40 @@ All notable changes to this project will be documented in this file, in reverse chronological order by release. +## 1.0.0alpha3 - 2018-02-27 + +### Added + +- Nothing. + +### Changed + +- [#17](https://github.com/zendframework/zend-expressive-authentication-oauth2/pull/17) + changes the constructor of each of the `Zend\Expressive\Authentication\OAuth2\OAuth2Adapter` + and `Zend\Expressive\Authentication\OAuth2\OAuth2Middleware` classes to accept + a callable `$responseFactory` instead of a `Psr\Http\Message\ResponseInterface` + response prototype. The `$responseFactory` should produce a + `ResponseInterface` implementation when invoked. + +- [#17](https://github.com/zendframework/zend-expressive-authentication-oauth2/pull/17) + updates the `OAuth2AdapterFactory` and `OAuth2MiddlewareFactory` classes to no + longer use `Zend\Expressive\Authentication\ResponsePrototypeTrait`, and + instead always depend on the `Psr\Http\Message\ResponseInterface` service to + correctly return a PHP callable capable of producing a `ResponseInterface` + instance. + +### Deprecated + +- Nothing. + +### Removed + +- Nothing. + +### Fixed + +- Nothing. + ## 1.0.0alpha2 - 2018-02-26 ### Added From 6caabba652b960e6b77fb70495a524775d8af671 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Tue, 27 Feb 2018 09:42:16 -0600 Subject: [PATCH 7/7] Refactors factory tests Ensures that the factories are testing expected behavior: - TypeError should be raised for a non-callable response factory. - TypeError should be raised for a response instance returned instead of a response factory. --- test/OAuth2AdapterFactoryTest.php | 31 +++++++++++++++++---- test/OAuth2MiddlewareFactoryTest.php | 41 +++++++++++++++++++++++----- 2 files changed, 59 insertions(+), 13 deletions(-) diff --git a/test/OAuth2AdapterFactoryTest.php b/test/OAuth2AdapterFactoryTest.php index b06e8ca..aa08059 100644 --- a/test/OAuth2AdapterFactoryTest.php +++ b/test/OAuth2AdapterFactoryTest.php @@ -15,6 +15,8 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Container\ContainerInterface; use Psr\Http\Message\ResponseInterface; +use stdClass; +use TypeError; use Zend\Expressive\Authentication\AuthenticationInterface; use Zend\Expressive\Authentication\OAuth2\OAuth2Adapter; use Zend\Expressive\Authentication\OAuth2\OAuth2AdapterFactory; @@ -58,7 +60,7 @@ public function testInvokeWithEmptyContainer() $oauth2Adapter = $factory($this->container->reveal()); } - public function testInvokeWithResourceServerEmptyResponse() + public function testFactoryRaisesTypeErrorForNonCallableResponseFactory() { $this->container ->has(ResourceServer::class) @@ -69,17 +71,34 @@ public function testInvokeWithResourceServerEmptyResponse() $this->container ->get(ResponseInterface::class) - ->willReturn(function () { - }); + ->willReturn(new stdClass()); $factory = new OAuth2AdapterFactory(); + + $this->expectException(TypeError::class); $adapter = $factory($this->container->reveal()); + } - $this->assertInstanceOf(OAuth2Adapter::class, $adapter); - $this->assertInstanceOf(AuthenticationInterface::class, $adapter); + public function testFactoryRaisesTypeErrorWhenResponseServiceProvidesResponseInstance() + { + $this->container + ->has(ResourceServer::class) + ->willReturn(true); + $this->container + ->get(ResourceServer::class) + ->willReturn($this->resourceServer->reveal()); + + $this->container + ->get(ResponseInterface::class) + ->will([$this->response, 'reveal']); + + $factory = new OAuth2AdapterFactory(); + + $this->expectException(TypeError::class); + $adapter = $factory($this->container->reveal()); } - public function testInvokeResourceServerAndResponse() + public function testFactoryReturnsInstanceWhenAppropriateDependenciesArePresentInContainer() { $this->container ->has(ResourceServer::class) diff --git a/test/OAuth2MiddlewareFactoryTest.php b/test/OAuth2MiddlewareFactoryTest.php index 4ae51bb..021938a 100644 --- a/test/OAuth2MiddlewareFactoryTest.php +++ b/test/OAuth2MiddlewareFactoryTest.php @@ -12,8 +12,11 @@ use League\OAuth2\Server\AuthorizationServer; use PHPUnit\Framework\TestCase; +use Prophecy\Prophecy\ObjectProphecy; use Psr\Container\ContainerInterface; use Psr\Http\Message\ResponseInterface; +use stdClass; +use TypeError; use Zend\Expressive\Authentication\OAuth2\Exception\InvalidConfigException; use Zend\Expressive\Authentication\OAuth2\OAuth2Middleware; use Zend\Expressive\Authentication\OAuth2\OAuth2MiddlewareFactory; @@ -23,6 +26,15 @@ */ class OAuth2MiddlewareFactoryTest extends TestCase { + /** @var AuthorizationServer|ObjectProphecy */ + private $authServer; + + /** @var AuthServer|ObjectProphecy */ + private $container; + + /** @var ResponseInterface|ObjectProphecy */ + private $response; + public function setUp() { $this->container = $this->prophesize(ContainerInterface::class); @@ -44,7 +56,7 @@ public function testInvokeWithEmptyContainer() $middleware = $factory($this->container->reveal()); } - public function testInvokeWithAuthServerWithoutResponseInterface() + public function testFactoryRaisesTypeErrorForNonCallableResponseFactory() { $this->container ->has(AuthorizationServer::class) @@ -54,21 +66,36 @@ public function testInvokeWithAuthServerWithoutResponseInterface() ->willReturn($this->authServer->reveal()); $this->container ->get(ResponseInterface::class) - ->willReturn(function () { - }); + ->willReturn(new stdClass()); $factory = new OAuth2MiddlewareFactory(); - $middleware = $factory($this->container->reveal()); - $this->assertInstanceOf(OAuth2Middleware::class, $middleware); + + $this->expectException(TypeError::class); + $factory($this->container->reveal()); } - public function testInvokeWithAuthServerWithResponseInterface() + public function testFactoryRaisesTypeErrorWhenResponseServiceProvidesResponseInstance() { $this->container ->has(AuthorizationServer::class) ->willReturn(true); $this->container - ->has(ResponseInterface::class) + ->get(AuthorizationServer::class) + ->willReturn($this->authServer->reveal()); + $this->container + ->get(ResponseInterface::class) + ->will([$this->response, 'reveal']); + + $factory = new OAuth2MiddlewareFactory(); + + $this->expectException(TypeError::class); + $factory($this->container->reveal()); + } + + public function testFactoryReturnsInstanceWhenAppropriateDependenciesArePresentInContainer() + { + $this->container + ->has(AuthorizationServer::class) ->willReturn(true); $this->container ->get(AuthorizationServer::class)