From 1ca45b697ec4b4d8cf5e80034ac04090860d6716 Mon Sep 17 00:00:00 2001 From: Rob Taylor Date: Fri, 3 Aug 2018 11:49:54 +0100 Subject: [PATCH 1/3] Include scope in response if given scopes don't match requested scopes --- src/AuthorizationServer.php | 2 +- src/ResponseTypes/BearerTokenResponse.php | 16 ++++- src/ResponseTypes/RedirectResponse.php | 3 +- src/ResponseTypes/ResponseTypeInterface.php | 3 +- .../ResponseTypes/BearerResponseTypeTest.php | 59 +++++++++++++++++-- tests/Stubs/StubResponseType.php | 2 +- 6 files changed, 75 insertions(+), 10 deletions(-) diff --git a/src/AuthorizationServer.php b/src/AuthorizationServer.php index f1e96146b..f4748f7ca 100644 --- a/src/AuthorizationServer.php +++ b/src/AuthorizationServer.php @@ -190,7 +190,7 @@ public function respondToAccessTokenRequest(ServerRequestInterface $request, Res ); if ($tokenResponse instanceof ResponseTypeInterface) { - return $tokenResponse->generateHttpResponse($response); + return $tokenResponse->generateHttpResponse($response, $request); } } diff --git a/src/ResponseTypes/BearerTokenResponse.php b/src/ResponseTypes/BearerTokenResponse.php index a57573a05..cfc37c5c6 100644 --- a/src/ResponseTypes/BearerTokenResponse.php +++ b/src/ResponseTypes/BearerTokenResponse.php @@ -14,24 +14,38 @@ use League\OAuth2\Server\Entities\AccessTokenEntityInterface; use League\OAuth2\Server\Entities\RefreshTokenEntityInterface; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ServerRequestInterface; class BearerTokenResponse extends AbstractResponseType { /** * {@inheritdoc} */ - public function generateHttpResponse(ResponseInterface $response) + public function generateHttpResponse(ResponseInterface $response, ServerRequestInterface $request = null) { $expireDateTime = $this->accessToken->getExpiryDateTime()->getTimestamp(); $jwtAccessToken = $this->accessToken->convertToJWT($this->privateKey); + $requestParameters = (array) $request->getParsedBody(); + $requestedScopes = $requestParameters['scope'] ?? []; + $responseParams = [ 'token_type' => 'Bearer', 'expires_in' => $expireDateTime - (new \DateTime())->getTimestamp(), 'access_token' => (string) $jwtAccessToken, ]; + $givenScopes = array_map(function($scope) { + return $scope->getIdentifier(); + }, $this->accessToken->getScopes()); + + if (!empty(array_diff($requestedScopes, $givenScopes)) + || !empty(array_diff($givenScopes, $requestedScopes)) + ) { + $responseParams['scope'] = implode(' ', $givenScopes); + } + if ($this->refreshToken instanceof RefreshTokenEntityInterface) { $refreshToken = $this->encrypt( json_encode( diff --git a/src/ResponseTypes/RedirectResponse.php b/src/ResponseTypes/RedirectResponse.php index e4639148d..ecdf29178 100644 --- a/src/ResponseTypes/RedirectResponse.php +++ b/src/ResponseTypes/RedirectResponse.php @@ -12,6 +12,7 @@ namespace League\OAuth2\Server\ResponseTypes; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ServerRequestInterface; class RedirectResponse extends AbstractResponseType { @@ -33,7 +34,7 @@ public function setRedirectUri($redirectUri) * * @return ResponseInterface */ - public function generateHttpResponse(ResponseInterface $response) + public function generateHttpResponse(ResponseInterface $response, ServerRequestInterface $request = null) { return $response->withStatus(302)->withHeader('Location', $this->redirectUri); } diff --git a/src/ResponseTypes/ResponseTypeInterface.php b/src/ResponseTypes/ResponseTypeInterface.php index 5eddd6079..f00a37e31 100644 --- a/src/ResponseTypes/ResponseTypeInterface.php +++ b/src/ResponseTypes/ResponseTypeInterface.php @@ -15,6 +15,7 @@ use League\OAuth2\Server\Entities\AccessTokenEntityInterface; use League\OAuth2\Server\Entities\RefreshTokenEntityInterface; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ServerRequestInterface; interface ResponseTypeInterface { @@ -33,7 +34,7 @@ public function setRefreshToken(RefreshTokenEntityInterface $refreshToken); * * @return ResponseInterface */ - public function generateHttpResponse(ResponseInterface $response); + public function generateHttpResponse(ResponseInterface $response, ServerRequestInterface $request = null); /** * Set the encryption key diff --git a/tests/ResponseTypes/BearerResponseTypeTest.php b/tests/ResponseTypes/BearerResponseTypeTest.php index 31245b077..9038efee5 100644 --- a/tests/ResponseTypes/BearerResponseTypeTest.php +++ b/tests/ResponseTypes/BearerResponseTypeTest.php @@ -44,7 +44,7 @@ public function testGenerateHttpResponse() $responseType->setAccessToken($accessToken); $responseType->setRefreshToken($refreshToken); - $response = $responseType->generateHttpResponse(new Response()); + $response = $responseType->generateHttpResponse(new Response(), new ServerRequest()); $this->assertInstanceOf(ResponseInterface::class, $response); $this->assertEquals(200, $response->getStatusCode()); @@ -86,7 +86,7 @@ public function testGenerateHttpResponseWithExtraParams() $responseType->setAccessToken($accessToken); $responseType->setRefreshToken($refreshToken); - $response = $responseType->generateHttpResponse(new Response()); + $response = $responseType->generateHttpResponse(new Response(), new ServerRequest()); $this->assertInstanceOf(ResponseInterface::class, $response); $this->assertEquals(200, $response->getStatusCode()); @@ -100,9 +100,58 @@ public function testGenerateHttpResponseWithExtraParams() $this->assertObjectHasAttribute('expires_in', $json); $this->assertObjectHasAttribute('access_token', $json); $this->assertObjectHasAttribute('refresh_token', $json); + $this->assertObjectHasAttribute('scope', $json); $this->assertObjectHasAttribute('foo', $json); $this->assertAttributeEquals('bar', 'foo', $json); + $this->assertAttributeEquals('basic', 'scope', $json); + } + + public function testGenerateHttpResponseWithInvalidScope() + { + $responseType = new BearerTokenResponse(); + $responseType->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); + $responseType->setEncryptionKey(base64_encode(random_bytes(36))); + + $client = new ClientEntity(); + $client->setIdentifier('clientName'); + + $scope = new ScopeEntity(); + $scope->setIdentifier('basic'); + + $accessToken = new AccessTokenEntity(); + $accessToken->setIdentifier('abcdef'); + $accessToken->setExpiryDateTime((new \DateTime())->add(new \DateInterval('PT1H'))); + $accessToken->setClient($client); + $accessToken->addScope($scope); + + $refreshToken = new RefreshTokenEntity(); + $refreshToken->setIdentifier('abcdef'); + $refreshToken->setAccessToken($accessToken); + $refreshToken->setExpiryDateTime((new \DateTime())->add(new \DateInterval('PT1H'))); + + $responseType->setAccessToken($accessToken); + $responseType->setRefreshToken($refreshToken); + + $request = new ServerRequest(['scope' => 'invalid']); + + $response = $responseType->generateHttpResponse(new Response(), $request); + + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals('no-cache', $response->getHeader('pragma')[0]); + $this->assertEquals('no-store', $response->getHeader('cache-control')[0]); + $this->assertEquals('application/json; charset=UTF-8', $response->getHeader('content-type')[0]); + + $response->getBody()->rewind(); + $json = json_decode($response->getBody()->getContents()); + $this->assertAttributeEquals('Bearer', 'token_type', $json); + $this->assertObjectHasAttribute('expires_in', $json); + $this->assertObjectHasAttribute('access_token', $json); + $this->assertObjectHasAttribute('refresh_token', $json); + $this->assertObjectHasAttribute('scope', $json); + + $this->assertAttributeEquals('basic', 'scope', $json); } public function testDetermineAccessTokenInHeaderValidToken() @@ -128,7 +177,7 @@ public function testDetermineAccessTokenInHeaderValidToken() $responseType->setAccessToken($accessToken); $responseType->setRefreshToken($refreshToken); - $response = $responseType->generateHttpResponse(new Response()); + $response = $responseType->generateHttpResponse(new Response(), new ServerRequest()); $json = json_decode((string) $response->getBody()); $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); @@ -173,7 +222,7 @@ public function testDetermineAccessTokenInHeaderInvalidJWT() $responseType->setAccessToken($accessToken); $responseType->setRefreshToken($refreshToken); - $response = $responseType->generateHttpResponse(new Response()); + $response = $responseType->generateHttpResponse(new Response(), new ServerRequest()); $json = json_decode((string) $response->getBody()); $authorizationValidator = new BearerTokenValidator($accessTokenRepositoryMock); @@ -215,7 +264,7 @@ public function testDetermineAccessTokenInHeaderRevokedToken() $responseType->setAccessToken($accessToken); $responseType->setRefreshToken($refreshToken); - $response = $responseType->generateHttpResponse(new Response()); + $response = $responseType->generateHttpResponse(new Response(), new ServerRequest()); $json = json_decode((string) $response->getBody()); $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); diff --git a/tests/Stubs/StubResponseType.php b/tests/Stubs/StubResponseType.php index ac8679d22..37e4d7dd3 100644 --- a/tests/Stubs/StubResponseType.php +++ b/tests/Stubs/StubResponseType.php @@ -63,7 +63,7 @@ public function validateAccessToken(ServerRequestInterface $request) * * @return ResponseInterface */ - public function generateHttpResponse(ResponseInterface $response) + public function generateHttpResponse(ResponseInterface $response, ServerRequestInterface $request = null) { return new Response(); } From 41ba7bdce6fd391d18781d175f8fdfd035569edd Mon Sep 17 00:00:00 2001 From: Rob Taylor Date: Fri, 3 Aug 2018 12:13:52 +0100 Subject: [PATCH 2/3] Fix StyleCI issues --- src/ResponseTypes/BearerTokenResponse.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ResponseTypes/BearerTokenResponse.php b/src/ResponseTypes/BearerTokenResponse.php index cfc37c5c6..4676f1a11 100644 --- a/src/ResponseTypes/BearerTokenResponse.php +++ b/src/ResponseTypes/BearerTokenResponse.php @@ -36,7 +36,7 @@ public function generateHttpResponse(ResponseInterface $response, ServerRequestI 'access_token' => (string) $jwtAccessToken, ]; - $givenScopes = array_map(function($scope) { + $givenScopes = array_map(function ($scope) { return $scope->getIdentifier(); }, $this->accessToken->getScopes()); From e6e2f2d08f06917ee8867b25155dae532634c7b1 Mon Sep 17 00:00:00 2001 From: Rob Taylor Date: Fri, 3 Aug 2018 12:27:48 +0100 Subject: [PATCH 3/3] Don't fail if request parameter isn't given to 'generateHttpResponse' --- src/ResponseTypes/BearerTokenResponse.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ResponseTypes/BearerTokenResponse.php b/src/ResponseTypes/BearerTokenResponse.php index 4676f1a11..c2eb2eedd 100644 --- a/src/ResponseTypes/BearerTokenResponse.php +++ b/src/ResponseTypes/BearerTokenResponse.php @@ -27,7 +27,10 @@ public function generateHttpResponse(ResponseInterface $response, ServerRequestI $jwtAccessToken = $this->accessToken->convertToJWT($this->privateKey); - $requestParameters = (array) $request->getParsedBody(); + if ($request) { + $requestParameters = (array) $request->getParsedBody(); + } + $requestedScopes = $requestParameters['scope'] ?? []; $responseParams = [