From a479b5762eac7122ad0af2a600422d431ee36a11 Mon Sep 17 00:00:00 2001 From: Christiaan Goossens Date: Fri, 13 Jul 2018 11:47:32 +0200 Subject: [PATCH 1/4] Fix implicit grant scopes --- src/Grant/ImplicitGrant.php | 17 +++++++++-------- tests/Grant/ImplicitGrantTest.php | 27 +++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index 9810c30a..9c053311 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -154,13 +154,6 @@ class ImplicitGrant extends AbstractAuthorizeGrant $redirectUri ); - // Finalize the requested scopes - $finalizedScopes = $this->scopeRepository->finalizeScopes( - $scopes, - $this->getIdentifier(), - $client - ); - $stateParameter = $this->getQueryStringParameter('state', $request); $authorizationRequest = new AuthorizationRequest(); @@ -172,7 +165,7 @@ class ImplicitGrant extends AbstractAuthorizeGrant $authorizationRequest->setState($stateParameter); } - $authorizationRequest->setScopes($finalizedScopes); + $authorizationRequest->setScopes($scopes); return $authorizationRequest; } @@ -194,6 +187,14 @@ class ImplicitGrant extends AbstractAuthorizeGrant // The user approved the client, redirect them back with an access token if ($authorizationRequest->isAuthorizationApproved() === true) { + // Finalize the requested scopes + $finalizedScopes = $this->scopeRepository->finalizeScopes( + $authorizationRequest->getScopes(), + $this->getIdentifier(), + $authorizationRequest->getClient(), + $authorizationRequest->getUser()->getIdentifier() + ); + $accessToken = $this->issueAccessToken( $this->accessTokenTTL, $authorizationRequest->getClient(), diff --git a/tests/Grant/ImplicitGrantTest.php b/tests/Grant/ImplicitGrantTest.php index 0080548f..257ea16d 100644 --- a/tests/Grant/ImplicitGrantTest.php +++ b/tests/Grant/ImplicitGrantTest.php @@ -94,7 +94,6 @@ class ImplicitGrantTest extends TestCase $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); $scopeEntity = new ScopeEntity(); $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scopeEntity); - $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new ImplicitGrant(new \DateInterval('PT10M')); $grant->setClientRepository($clientRepositoryMock); @@ -129,7 +128,6 @@ class ImplicitGrantTest extends TestCase $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); $scopeEntity = new ScopeEntity(); $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scopeEntity); - $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); $grant = new ImplicitGrant(new \DateInterval('PT10M')); $grant->setClientRepository($clientRepositoryMock); @@ -286,9 +284,14 @@ class ImplicitGrantTest extends TestCase $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity()); $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); + $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeEntity = new ScopeEntity(); + $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); + $grant = new ImplicitGrant(new \DateInterval('PT10M')); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $grant->setAccessTokenRepository($accessTokenRepositoryMock); + $grant->setScopeRepository($scopeRepositoryMock); $this->assertInstanceOf(RedirectResponse::class, $grant->completeAuthorizationRequest($authRequest)); } @@ -309,9 +312,14 @@ class ImplicitGrantTest extends TestCase $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity()); $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); + $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeEntity = new ScopeEntity(); + $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); + $grant = new ImplicitGrant(new \DateInterval('PT10M')); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $grant->setAccessTokenRepository($accessTokenRepositoryMock); + $grant->setScopeRepository($scopeRepositoryMock); $grant->completeAuthorizationRequest($authRequest); } @@ -330,9 +338,14 @@ class ImplicitGrantTest extends TestCase $accessTokenRepositoryMock->expects($this->at(0))->method('persistNewAccessToken')->willThrowException(UniqueTokenIdentifierConstraintViolationException::create()); $accessTokenRepositoryMock->expects($this->at(1))->method('persistNewAccessToken')->willReturnSelf(); + $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeEntity = new ScopeEntity(); + $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); + $grant = new ImplicitGrant(new \DateInterval('PT10M')); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $grant->setAccessTokenRepository($accessTokenRepositoryMock); + $grant->setScopeRepository($scopeRepositoryMock); $this->assertInstanceOf(RedirectResponse::class, $grant->completeAuthorizationRequest($authRequest)); } @@ -354,9 +367,14 @@ class ImplicitGrantTest extends TestCase $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity()); $accessTokenRepositoryMock->method('persistNewAccessToken')->willThrowException(OAuthServerException::serverError('something bad happened')); + $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeEntity = new ScopeEntity(); + $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); + $grant = new ImplicitGrant(new \DateInterval('PT10M')); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $grant->setAccessTokenRepository($accessTokenRepositoryMock); + $grant->setScopeRepository($scopeRepositoryMock); $grant->completeAuthorizationRequest($authRequest); } @@ -378,9 +396,14 @@ class ImplicitGrantTest extends TestCase $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity()); $accessTokenRepositoryMock->method('persistNewAccessToken')->willThrowException(UniqueTokenIdentifierConstraintViolationException::create()); + $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeEntity = new ScopeEntity(); + $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); + $grant = new ImplicitGrant(new \DateInterval('PT10M')); $grant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); $grant->setAccessTokenRepository($accessTokenRepositoryMock); + $grant->setScopeRepository($scopeRepositoryMock); $grant->completeAuthorizationRequest($authRequest); } From acf16e924a09ea3d2c86137569cfcf3d59214794 Mon Sep 17 00:00:00 2001 From: Christiaan Goossens Date: Fri, 13 Jul 2018 13:11:18 +0200 Subject: [PATCH 2/4] Actually use finalizedScopes in access token --- src/Grant/ImplicitGrant.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index 9c053311..1890a6ba 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -199,7 +199,7 @@ class ImplicitGrant extends AbstractAuthorizeGrant $this->accessTokenTTL, $authorizationRequest->getClient(), $authorizationRequest->getUser()->getIdentifier(), - $authorizationRequest->getScopes() + $finalizedScopes ); $response = new RedirectResponse(); From 6bc6ac09d207998626d7335897465b0708e1fa0a Mon Sep 17 00:00:00 2001 From: sephster Date: Sun, 23 Sep 2018 18:30:14 +0100 Subject: [PATCH 3/4] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e9668d0..7dbca52e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Changed +- Moved `finalizeScopes()` call from `validateAuthorizationRequest` method to the `completeAuthorizationRequest` method so it is called just before the access token is issued (PR #923) + ## [7.2.0] - released 2018-06-23 ### Changed From 71c605117a7b2cfc168c572e2bf633837859b00a Mon Sep 17 00:00:00 2001 From: sephster Date: Sun, 23 Sep 2018 18:31:26 +0100 Subject: [PATCH 4/4] Add missing word --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dbca52e..370ab559 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] ### Changed -- Moved `finalizeScopes()` call from `validateAuthorizationRequest` method to the `completeAuthorizationRequest` method so it is called just before the access token is issued (PR #923) +- Moved the `finalizeScopes()` call from `validateAuthorizationRequest` method to the `completeAuthorizationRequest` method so it is called just before the access token is issued (PR #923) ## [7.2.0] - released 2018-06-23