From ee25d94581f097147d153ad788553c93e101f121 Mon Sep 17 00:00:00 2001 From: Nicholas Narsing Date: Thu, 17 May 2018 22:49:17 -0400 Subject: [PATCH 1/5] Allow PSR-11 containers --- composer.json | 2 +- src/Authorization.php | 6 +++--- tests/AuthorizationTest.php | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/composer.json b/composer.json index 4700d48..968b8d1 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ "chadicus/slim-oauth2-http": "^3.1", "chadicus/psr-middleware": "^1.0", "psr/http-message": "^1.0", - "container-interop/container-interop": "^1.1" + "container-interop/container-interop": "^1.2" }, "require-dev": { "chadicus/coding-standard": "^1.3", diff --git a/src/Authorization.php b/src/Authorization.php index d1d404b..c7d8a05 100644 --- a/src/Authorization.php +++ b/src/Authorization.php @@ -7,7 +7,7 @@ use Chadicus\Psr\Middleware\MiddlewareInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; -use Interop\Container\ContainerInterface; +use Psr\Container\ContainerInterface; use OAuth2; /** @@ -54,9 +54,9 @@ class Authorization implements MiddlewareInterface public function __construct(OAuth2\Server $server, $container, array $scopes = []) { $this->server = $server; - if (!is_a($container, '\\ArrayAccess') && !is_a($container, '\\Interop\\Container\\ContainerInterface')) { + if (!is_a($container, '\\ArrayAccess') && !is_a($container, '\\Psr\\Container\\ContainerInterface')) { throw new \InvalidArgumentException( - '$container does not implement \\ArrayAccess or \\Interop\\Container\\ContainerInterface' + '$container does not implement \\ArrayAccess or \\Psr\\Container\\ContainerInterface' ); } diff --git a/tests/AuthorizationTest.php b/tests/AuthorizationTest.php index 4810715..b7c988f 100644 --- a/tests/AuthorizationTest.php +++ b/tests/AuthorizationTest.php @@ -466,7 +466,7 @@ public function invokeRetainsContentType() * @test * @covers ::__construct * @expectedException \InvalidArgumentException - * @expectedExceptionMessage $container does not implement \ArrayAccess or \Interop\Container\ContainerInterface + * @expectedExceptionMessage $container does not implement \ArrayAccess or \Psr\Container\ContainerInterface * * @return void */ From 09ab665bf89bb6e75b716f8458f7597810ff52d2 Mon Sep 17 00:00:00 2001 From: chadicus Date: Fri, 18 May 2018 09:25:04 -0400 Subject: [PATCH 2/5] Add psr/container dependency --- composer.json | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 968b8d1..39f214a 100644 --- a/composer.json +++ b/composer.json @@ -16,10 +16,11 @@ "require": { "php": "^5.6 || ^7.0", "bshaffer/oauth2-server-php": "^1.9", - "chadicus/slim-oauth2-http": "^3.1", "chadicus/psr-middleware": "^1.0", - "psr/http-message": "^1.0", - "container-interop/container-interop": "^1.2" + "chadicus/slim-oauth2-http": "^3.1", + "container-interop/container-interop": "^1.1", + "psr/container": "^1.0", + "psr/http-message": "^1.0" }, "require-dev": { "chadicus/coding-standard": "^1.3", From e0fd939b57e3090350eec43b946aefc27319f9ea Mon Sep 17 00:00:00 2001 From: chadicus Date: Fri, 18 May 2018 09:52:48 -0400 Subject: [PATCH 3/5] Ensure use of psr-11 is not backwards breaking. --- src/Authorization.php | 33 ++++++++++++++++++++++++++------- tests/AuthorizationTest.php | 2 +- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/Authorization.php b/src/Authorization.php index c7d8a05..92371ac 100644 --- a/src/Authorization.php +++ b/src/Authorization.php @@ -5,6 +5,7 @@ use Chadicus\Slim\OAuth2\Http\RequestBridge; use Chadicus\Slim\OAuth2\Http\ResponseBridge; use Chadicus\Psr\Middleware\MiddlewareInterface; +use Interop\Container\ContainerInterface as InteropContainerInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; use Psr\Container\ContainerInterface; @@ -54,13 +55,7 @@ class Authorization implements MiddlewareInterface public function __construct(OAuth2\Server $server, $container, array $scopes = []) { $this->server = $server; - if (!is_a($container, '\\ArrayAccess') && !is_a($container, '\\Psr\\Container\\ContainerInterface')) { - throw new \InvalidArgumentException( - '$container does not implement \\ArrayAccess or \\Psr\\Container\\ContainerInterface' - ); - } - - $this->container = $container; + $this->container = $this->validateContainer($container); $this->scopes = $this->formatScopes($scopes); } @@ -147,4 +142,28 @@ function (&$scope) { return $scopes; } + + private function validateContainer($container) + { + if (is_a($container, ArrayAccess::class)) { + return $container; + } + + if (is_a($container, ContainerInterface::class)) { + return $container; + } + + if (is_a($container, InteropContainerInterface::class)) { + return $container; + } + + throw new \InvalidArgumentException( + sprintf( + '$container does not implement %s, %s, or %s', + ArrayAccess::class, + ContainerInterface::class, + InteropContainerInterface::class + ) + ); + } } diff --git a/tests/AuthorizationTest.php b/tests/AuthorizationTest.php index b7c988f..978d284 100644 --- a/tests/AuthorizationTest.php +++ b/tests/AuthorizationTest.php @@ -466,7 +466,7 @@ public function invokeRetainsContentType() * @test * @covers ::__construct * @expectedException \InvalidArgumentException - * @expectedExceptionMessage $container does not implement \ArrayAccess or \Psr\Container\ContainerInterface + * @expectedExceptionMessage $container does not implement ArrayAccess, Psr\Container\ContainerInterface * * @return void */ From a593b06554833da2dcb93256f5f31263b8eacf89 Mon Sep 17 00:00:00 2001 From: chadicus Date: Mon, 21 May 2018 08:56:57 -0400 Subject: [PATCH 4/5] Reduce restrictions for container --- composer.json | 1 - src/Authorization.php | 36 +++++++++++------------------------- tests/AuthorizationTest.php | 19 ++++++++++++++++++- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/composer.json b/composer.json index 39f214a..eb41747 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,6 @@ "chadicus/psr-middleware": "^1.0", "chadicus/slim-oauth2-http": "^3.1", "container-interop/container-interop": "^1.1", - "psr/container": "^1.0", "psr/http-message": "^1.0" }, "require-dev": { diff --git a/src/Authorization.php b/src/Authorization.php index 92371ac..90c7e8a 100644 --- a/src/Authorization.php +++ b/src/Authorization.php @@ -5,10 +5,8 @@ use Chadicus\Slim\OAuth2\Http\RequestBridge; use Chadicus\Slim\OAuth2\Http\ResponseBridge; use Chadicus\Psr\Middleware\MiddlewareInterface; -use Interop\Container\ContainerInterface as InteropContainerInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; -use Psr\Container\ContainerInterface; use OAuth2; /** @@ -33,22 +31,21 @@ class Authorization implements MiddlewareInterface /** * Container for token. * - * @var ArrayAccess|ContainerInterface + * @var mixed */ private $container; /** * Create a new instance of the Authroization middleware. * - * @param OAuth2\Server $server The configured OAuth2 server. - * @param ArrayAccess|ContainerInterface $container A container object in which to store the token from the - * request. - * @param array $scopes Scopes required for authorization. $scopes can be given as an - * array of arrays. OR logic will use with each grouping. - * Example: - * Given ['superUser', ['basicUser', 'aPermission']], the request - * will be verified if the request token has 'superUser' scope - * OR 'basicUser' and 'aPermission' as its scope. + * @param OAuth2\Server $server The configured OAuth2 server. + * @param mixed $container A container object in which to store the token from the request. + * @param array $scopes Scopes required for authorization. $scopes can be given as an + * array of arrays. OR logic will use with each grouping. + * Example: + * Given ['superUser', ['basicUser', 'aPermission']], the request + * will be verified if the request token has 'superUser' scope + * OR 'basicUser' and 'aPermission' as its scope. * * @throws \InvalidArgumentException Thrown if $container is not an instance of ArrayAccess or ContainerInterface. */ @@ -149,21 +146,10 @@ private function validateContainer($container) return $container; } - if (is_a($container, ContainerInterface::class)) { + if (method_exists($container, 'set')) { return $container; } - if (is_a($container, InteropContainerInterface::class)) { - return $container; - } - - throw new \InvalidArgumentException( - sprintf( - '$container does not implement %s, %s, or %s', - ArrayAccess::class, - ContainerInterface::class, - InteropContainerInterface::class - ) - ); + throw new \InvalidArgumentException("\$container does not implement ArrayAccess or contain a 'set' method"); } } diff --git a/tests/AuthorizationTest.php b/tests/AuthorizationTest.php index 978d284..bae8589 100644 --- a/tests/AuthorizationTest.php +++ b/tests/AuthorizationTest.php @@ -466,7 +466,7 @@ public function invokeRetainsContentType() * @test * @covers ::__construct * @expectedException \InvalidArgumentException - * @expectedExceptionMessage $container does not implement ArrayAccess, Psr\Container\ContainerInterface + * @expectedExceptionMessage $container does not implement ArrayAccess or contain a 'set' method * * @return void */ @@ -476,6 +476,23 @@ public function constructWithInvalidContainer() new Authorization($oauth2ServerMock, new \StdClass()); } + /** + * Verify middleware cannot be constructed with a pure PSR-11 container. + * + * @test + * @covers ::__construct + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage $container does not implement ArrayAccess or contain a 'set' method + * + * @return void + */ + public function constructWithPSR11Container() + { + $container = $this->getMockBuilder('\\Psr\\Container\\ContainerInterface')->getMock(); + $oauth2ServerMock = $this->getMockBuilder('\\OAuth2\\Server')->disableOriginalConstructor()->getMock(); + new Authorization($oauth2ServerMock, $container); + } + /** * Verify middleware can use interop container. * From fb6be5c75c42507d5637f2c7e05bce528c01de07 Mon Sep 17 00:00:00 2001 From: chadicus Date: Sat, 15 Sep 2018 14:12:51 -0400 Subject: [PATCH 5/5] Add token to request as attribute --- src/Authorization.php | 10 +++- tests/AuthorizationTest.php | 111 ++++++++++++++++++------------------ 2 files changed, 63 insertions(+), 58 deletions(-) diff --git a/src/Authorization.php b/src/Authorization.php index 90c7e8a..5208097 100644 --- a/src/Authorization.php +++ b/src/Authorization.php @@ -14,6 +14,11 @@ */ class Authorization implements MiddlewareInterface { + /** + * @var string + */ + const TOKEN_ATTRIBUTE_KEY = 'oauth2-token'; + /** * OAuth2 Server * @@ -70,8 +75,9 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res $oauth2Request = RequestBridge::toOAuth2($request); foreach ($this->scopes as $scope) { if ($this->server->verifyResourceRequest($oauth2Request, null, $scope)) { - $this->setToken($this->server->getResourceController()->getToken()); - return $next($request, $response); + $token = $this->server->getResourceController()->getToken(); + $this->setToken($token); + return $next($request->withAttribute(self::TOKEN_ATTRIBUTE_KEY, $token), $response); } } diff --git a/tests/AuthorizationTest.php b/tests/AuthorizationTest.php index bae8589..345b80c 100644 --- a/tests/AuthorizationTest.php +++ b/tests/AuthorizationTest.php @@ -59,22 +59,22 @@ public function invoke() $middleware = new Authorization($server, $container); - $next = function ($request, $response) { + $expectedToken = [ + 'access_token' => 'atokenvalue', + 'client_id' => 'a client id', + 'user_id' => 'a user id', + 'expires' => 99999999900, + 'scope' => null, + ]; + $test = $this; + $next = function ($request, $response) use ($expectedToken, $test) { + $test->assertSame($expectedToken, $request->getAttribute(Authorization::TOKEN_ATTRIBUTE_KEY)); return $response; }; $middleware($request, new Response(), $next); - $this->assertSame( - [ - 'access_token' => 'atokenvalue', - 'client_id' => 'a client id', - 'user_id' => 'a user id', - 'expires' => 99999999900, - 'scope' => null, - ], - $container['token'] - ); + $this->assertSame($expectedToken, $container['token']); } /** @@ -171,23 +171,23 @@ public function withRequiredScope() $middleware = new Authorization($server, $container); - $next = function ($request, $response) { + $expectedToken = [ + 'access_token' => 'atokenvalue', + 'client_id' => 'a client id', + 'user_id' => 'a user id', + 'expires' => 99999999900, + 'scope' => 'allowFoo anotherScope', + ]; + $test = $this; + $next = function ($request, $response) use ($expectedToken, $test) { + $test->assertSame($expectedToken, $request->getAttribute(Authorization::TOKEN_ATTRIBUTE_KEY)); return $response; }; $response = $middleware->withRequiredScope(['allowFoo'])->__invoke($request, new Response(), $next); $this->assertSame(200, $response->getStatusCode()); - $this->assertSame( - [ - 'access_token' => 'atokenvalue', - 'client_id' => 'a client id', - 'user_id' => 'a user id', - 'expires' => 99999999900, - 'scope' => 'allowFoo anotherScope', - ], - $container['token'] - ); + $this->assertSame($expectedToken , $container['token']); } /** @@ -320,23 +320,22 @@ public function invokeWithEitherScope() $middleware = new Authorization($server, $container, ['superUser', ['basicUser', 'withPermission']]); - $next = function ($request, $response) { + $expectedToken = [ + 'access_token' => 'atokenvalue', + 'client_id' => 'a client id', + 'user_id' => 'a user id', + 'expires' => 99999999900, + 'scope' => 'basicUser withPermission anExtraScope', + ]; + $test = $this; + $next = function ($request, $response) use ($expectedToken, $test) { + $test->assertSame($expectedToken, $request->getAttribute(Authorization::TOKEN_ATTRIBUTE_KEY)); return $response; }; $response = $middleware($request, new Response(), $next); - $this->assertSame(200, $response->getStatusCode()); - $this->assertSame( - [ - 'access_token' => 'atokenvalue', - 'client_id' => 'a client id', - 'user_id' => 'a user id', - 'expires' => 99999999900, - 'scope' => 'basicUser withPermission anExtraScope', - ], - $container['token'] - ); + $this->assertSame($expectedToken, $container['token']); } /** @@ -380,22 +379,22 @@ public function invokeWithEmptyScope() $middleware = new Authorization($server, $container, []); - $next = function ($request, $response) { + $expectedToken = [ + 'access_token' => 'atokenvalue', + 'client_id' => 'a client id', + 'user_id' => 'a user id', + 'expires' => 99999999900, + 'scope' => null, + ]; + $test = $this; + $next = function ($request, $response) use ($expectedToken, $test) { + $test->assertSame($expectedToken, $request->getAttribute(Authorization::TOKEN_ATTRIBUTE_KEY)); return $response; }; $middleware($request, new Response(), $next); - $this->assertSame( - [ - 'access_token' => 'atokenvalue', - 'client_id' => 'a client id', - 'user_id' => 'a user id', - 'expires' => 99999999900, - 'scope' => null, - ], - $container['token'] - ); + $this->assertSame($expectedToken , $container['token']); } /** @@ -534,21 +533,21 @@ public function invokeWithInteropContainer() $middleware = new Authorization($server, $container); - $next = function ($request, $response) { + $expectedToken = [ + 'access_token' => 'atokenvalue', + 'client_id' => 'a client id', + 'user_id' => 'a user id', + 'expires' => 99999999900, + 'scope' => null, + ]; + $test = $this; + $next = function ($request, $response) use ($expectedToken, $test) { + $test->assertSame($expectedToken, $request->getAttribute(Authorization::TOKEN_ATTRIBUTE_KEY)); return $response; }; $middleware($request, new Response(), $next); - $this->assertSame( - [ - 'access_token' => 'atokenvalue', - 'client_id' => 'a client id', - 'user_id' => 'a user id', - 'expires' => 99999999900, - 'scope' => null, - ], - $container->get('token') - ); + $this->assertSame($expectedToken, $container->get('token')); } }