From dd9c79f8b088a8809a67e9aeff6a440ea2666a86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petar=20Obradovi=C4=87?= Date: Sun, 3 Feb 2019 12:53:33 +0100 Subject: [PATCH 1/2] Implement client scope inheritance and restriction --- Event/ScopeResolveEvent.php | 3 - League/Repository/ScopeRepository.php | 35 +++++++++- Model/Client.php | 9 --- Tests/Fixtures/FixtureFactory.php | 44 +++++++++---- Tests/Integration/AuthorizationServerTest.php | 66 ++++++++++++++++++- Tests/TestHelper.php | 2 +- docs/basic-setup.md | 2 - docs/controlling-token-scopes.md | 27 +------- 8 files changed, 134 insertions(+), 54 deletions(-) diff --git a/Event/ScopeResolveEvent.php b/Event/ScopeResolveEvent.php index 1a006b8c..c4df7446 100644 --- a/Event/ScopeResolveEvent.php +++ b/Event/ScopeResolveEvent.php @@ -45,9 +45,6 @@ public function getScopes(): array return $this->scopes; } - /** - * @param Scope[] $scopes - */ public function setScopes(Scope ...$scopes): self { $this->scopes = (array) $scopes; diff --git a/League/Repository/ScopeRepository.php b/League/Repository/ScopeRepository.php index 31412e0b..fb67b63d 100644 --- a/League/Repository/ScopeRepository.php +++ b/League/Repository/ScopeRepository.php @@ -3,13 +3,16 @@ namespace Trikoder\Bundle\OAuth2Bundle\League\Repository; use League\OAuth2\Server\Entities\ClientEntityInterface; +use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Trikoder\Bundle\OAuth2Bundle\Converter\ScopeConverter; use Trikoder\Bundle\OAuth2Bundle\Event\ScopeResolveEvent; use Trikoder\Bundle\OAuth2Bundle\Manager\ClientManagerInterface; use Trikoder\Bundle\OAuth2Bundle\Manager\ScopeManagerInterface; +use Trikoder\Bundle\OAuth2Bundle\Model\Client as ClientModel; use Trikoder\Bundle\OAuth2Bundle\Model\Grant as GrantModel; +use Trikoder\Bundle\OAuth2Bundle\Model\Scope as ScopeModel; use Trikoder\Bundle\OAuth2Bundle\OAuth2Events; final class ScopeRepository implements ScopeRepositoryInterface @@ -71,10 +74,12 @@ public function finalizeScopes( ) { $client = $this->clientManager->find($clientEntity->getIdentifier()); + $scopes = $this->setupScopes($client, $this->scopeConverter->toDomainArray($scopes)); + $event = $this->eventDispatcher->dispatch( OAuth2Events::SCOPE_RESOLVE, new ScopeResolveEvent( - $this->scopeConverter->toDomainArray($scopes), + $scopes, new GrantModel($grantType), $client, $userIdentifier @@ -83,4 +88,32 @@ public function finalizeScopes( return $this->scopeConverter->toLeagueArray($event->getScopes()); } + + /** + * @return ScopeModel[] + */ + private function setupScopes(ClientModel $client, array $requestedScopes): array + { + $clientScopes = $client->getScopes(); + + if (empty($clientScopes)) { + return $requestedScopes; + } + + if (empty($requestedScopes)) { + return $clientScopes; + } + + $finalizedScopes = []; + + foreach ($requestedScopes as $requestedScope) { + if (!\in_array($requestedScope, $clientScopes, true)) { + throw OAuthServerException::invalidScope((string) $requestedScope); + } + + $finalizedScopes[] = $requestedScope; + } + + return $finalizedScopes; + } } diff --git a/Model/Client.php b/Model/Client.php index 1dc4af20..bc177d8b 100644 --- a/Model/Client.php +++ b/Model/Client.php @@ -63,9 +63,6 @@ public function getRedirectUris(): array return $this->redirectUris; } - /** - * @param RedirectUri[] $redirectUris - */ public function setRedirectUris(RedirectUri ...$redirectUris): self { $this->redirectUris = (array) $redirectUris; @@ -81,9 +78,6 @@ public function getGrants(): array return $this->grants; } - /** - * @param Grant[] $grants - */ public function setGrants(Grant ...$grants): self { $this->grants = (array) $grants; @@ -99,9 +93,6 @@ public function getScopes(): array return $this->scopes; } - /** - * @param Scope[] $scopes - */ public function setScopes(Scope ...$scopes): self { $this->scopes = (array) $scopes; diff --git a/Tests/Fixtures/FixtureFactory.php b/Tests/Fixtures/FixtureFactory.php index 7954b2d9..26346544 100644 --- a/Tests/Fixtures/FixtureFactory.php +++ b/Tests/Fixtures/FixtureFactory.php @@ -13,6 +13,12 @@ use Trikoder\Bundle\OAuth2Bundle\Model\RefreshToken; use Trikoder\Bundle\OAuth2Bundle\Model\Scope; +/** + * Development hints: + * + * You can easily generate token identifiers using the following command: + * --- dev/bin/php -r "echo bin2hex(random_bytes(40)) . PHP_EOL;" + */ final class FixtureFactory { public const FIXTURE_ACCESS_TOKEN_USER_BOUND = '96fb0ff864bf242425bfa7b9b6f47294fda556bf5eef78f753f61c2b827125d37d5d5735bcaed5b8'; @@ -27,17 +33,27 @@ final class FixtureFactory public const FIXTURE_REFRESH_TOKEN_DIFFERENT_CLIENT = '73b1618470fdccf1c96eda132f8a19d6da43c31e2efd19daeab2c98c0ac36bf95b3ea72fdc8d6752'; public const FIXTURE_REFRESH_TOKEN_EXPIRED = '3b3db453a137debb7b5f445c971bef18bb4f045d272a66a27054a0713096d2a8377679d204495c88'; public const FIXTURE_REFRESH_TOKEN_REVOKED = '63641841630c2e4d747e0f9ebe12ee04424e322874b8e68ef69fd58f1899ef70beb09733e23928a6'; + public const FIXTURE_REFRESH_TOKEN_WITH_SCOPES = 'e47d593ed661840b3633e4577c3261ef57ba225be193b190deb69ee9afefdc19f54f890fbdda59f5'; public const FIXTURE_CLIENT_FIRST = 'foo'; public const FIXTURE_CLIENT_SECOND = 'bar'; public const FIXTURE_CLIENT_INACTIVE = 'baz_inactive'; - public const FIXTURE_CLIENT_RESTRICTED_GRANTS = 'qux_restricted'; + public const FIXTURE_CLIENT_RESTRICTED_GRANTS = 'qux_restricted_grants'; + public const FIXTURE_CLIENT_RESTRICTED_SCOPES = 'quux_restricted_scopes'; public const FIXTURE_SCOPE_FIRST = 'fancy'; public const FIXTURE_SCOPE_SECOND = 'rock'; public const FIXTURE_USER = 'user'; + public static function createUser(array $roles = []): User + { + $user = new User(); + $user['roles'] = $roles; + + return $user; + } + public static function initializeFixtures( ScopeManagerInterface $scopeManager, ClientManagerInterface $clientManager, @@ -64,7 +80,7 @@ public static function initializeFixtures( /** * @return AccessToken[] */ - public static function createAccessTokens(ScopeManagerInterface $scopeManager, ClientManagerInterface $clientManager): array + private static function createAccessTokens(ScopeManagerInterface $scopeManager, ClientManagerInterface $clientManager): array { $accessTokens = []; @@ -131,7 +147,7 @@ public static function createAccessTokens(ScopeManagerInterface $scopeManager, C /** * @return RefreshToken[] */ - public static function createRefreshTokens(AccessTokenManagerInterface $accessTokenManager): array + private static function createRefreshTokens(AccessTokenManagerInterface $accessTokenManager): array { $refreshTokens = []; @@ -160,13 +176,19 @@ public static function createRefreshTokens(AccessTokenManagerInterface $accessTo )) ->revoke(); + $refreshTokens[] = new RefreshToken( + self::FIXTURE_REFRESH_TOKEN_WITH_SCOPES, + new DateTime('+1 month'), + $accessTokenManager->find(self::FIXTURE_ACCESS_TOKEN_USER_BOUND_WITH_SCOPES) + ); + return $refreshTokens; } /** * @return Client[] */ - public static function createClients(): array + private static function createClients(): array { $clients = []; @@ -180,26 +202,22 @@ public static function createClients(): array $clients[] = (new Client(self::FIXTURE_CLIENT_RESTRICTED_GRANTS, 'wicked')) ->setGrants(new Grant('password')); + $clients[] = (new Client(self::FIXTURE_CLIENT_RESTRICTED_SCOPES, 'beer')) + ->setScopes(new Scope(self::FIXTURE_SCOPE_SECOND)); + return $clients; } /** * @return Scope[] */ - public static function createScopes(): array + private static function createScopes(): array { $scopes = []; $scopes[] = new Scope(self::FIXTURE_SCOPE_FIRST); + $scopes[] = new Scope(self::FIXTURE_SCOPE_SECOND); return $scopes; } - - public static function createUser(array $roles = []): User - { - $user = new User(); - $user['roles'] = $roles; - - return $user; - } } diff --git a/Tests/Integration/AuthorizationServerTest.php b/Tests/Integration/AuthorizationServerTest.php index bf02ef33..4ddebce9 100644 --- a/Tests/Integration/AuthorizationServerTest.php +++ b/Tests/Integration/AuthorizationServerTest.php @@ -92,7 +92,7 @@ public function testInactiveClient(): void public function testRestrictedGrantClient(): void { - $request = $this->createAuthorizationRequest('qux_restricted:wicked', [ + $request = $this->createAuthorizationRequest('qux_restricted_grants:wicked', [ 'grant_type' => 'client_credentials', ]); @@ -103,6 +103,21 @@ public function testRestrictedGrantClient(): void $this->assertSame('Client authentication failed', $response['message']); } + public function testRestrictedScopeClient(): void + { + $request = $this->createAuthorizationRequest('quux_restricted_scopes:beer', [ + 'grant_type' => 'client_credentials', + 'scope' => 'fancy rock', + ]); + + $response = $this->handleAuthorizationRequest($request); + + // Response assertions. + $this->assertSame('invalid_scope', $response['error']); + $this->assertSame('The requested scope is invalid, unknown, or malformed', $response['message']); + $this->assertSame('Check the `fancy` scope', $response['hint']); + } + public function testInvalidGrantType(): void { $request = $this->createAuthorizationRequest('foo:secret', [ @@ -187,6 +202,37 @@ public function testValidClientCredentialsGrantWithScope(): void ); } + public function testValidClientCredentialsGrantWithInheritedScope(): void + { + $request = $this->createAuthorizationRequest('quux_restricted_scopes:beer', [ + 'grant_type' => 'client_credentials', + ]); + + timecop_freeze(new DateTime()); + + $response = $this->handleAuthorizationRequest($request); + + timecop_return(); + + $accessToken = $this->getAccessToken($response['access_token']); + + // Response assertions. + $this->assertSame('Bearer', $response['token_type']); + $this->assertSame(3600, $response['expires_in']); + $this->assertInstanceOf(AccessToken::class, $accessToken); + + // Make sure the access token is issued for the given client ID. + $this->assertSame('quux_restricted_scopes', $accessToken->getClient()->getIdentifier()); + + // The access token should have the requested scope. + $this->assertEquals( + [ + $this->scopeManager->find(FixtureFactory::FIXTURE_SCOPE_SECOND), + ], + $accessToken->getScopes() + ); + } + public function testValidPasswordGrant(): void { $this->eventDispatcher->addListener('trikoder.oauth2.user_resolve', function (UserResolveEvent $event) { @@ -321,6 +367,24 @@ public function testDifferentClientRefreshGrant(): void $this->assertSame('Token is not linked to client', $response['hint']); } + public function testDifferentScopeRefreshGrant(): void + { + $existingRefreshToken = $this->refreshTokenManager->find(FixtureFactory::FIXTURE_REFRESH_TOKEN_WITH_SCOPES); + + $request = $this->createAuthorizationRequest('foo:secret', [ + 'grant_type' => 'refresh_token', + 'scope' => 'rock', + 'refresh_token' => TestHelper::generateEncryptedPayload($existingRefreshToken), + ]); + + $response = $this->handleAuthorizationRequest($request); + + // Response assertions. + $this->assertSame('invalid_scope', $response['error']); + $this->assertSame('The requested scope is invalid, unknown, or malformed', $response['message']); + $this->assertSame('Check the `rock` scope', $response['hint']); + } + public function testExpiredRefreshGrant(): void { $existingRefreshToken = $this->refreshTokenManager->find(FixtureFactory::FIXTURE_REFRESH_TOKEN_EXPIRED); diff --git a/Tests/TestHelper.php b/Tests/TestHelper.php index bdd2cd01..e66fd91f 100644 --- a/Tests/TestHelper.php +++ b/Tests/TestHelper.php @@ -26,7 +26,7 @@ public static function generateEncryptedPayload(RefreshTokenModel $refreshToken) 'client_id' => $refreshToken->getAccessToken()->getClient()->getIdentifier(), 'refresh_token_id' => $refreshToken->getIdentifier(), 'access_token_id' => $refreshToken->getAccessToken()->getIdentifier(), - 'scopes' => $refreshToken->getAccessToken()->getScopes(), + 'scopes' => array_map('strval', $refreshToken->getAccessToken()->getScopes()), 'user_id' => $refreshToken->getAccessToken()->getUserIdentifier(), 'expire_time' => $refreshToken->getExpiry()->getTimestamp(), ]); diff --git a/docs/basic-setup.md b/docs/basic-setup.md index 2efc16a3..a41ea15b 100644 --- a/docs/basic-setup.md +++ b/docs/basic-setup.md @@ -60,8 +60,6 @@ $ bin/console trikoder:oauth2:update-client --grant-type client_credentials --gr $ bin/console trikoder:oauth2:update-client --scope create --scope read foo ``` -> **NOTE:** You will have to setup an [event listener](controlling-token-scopes.md#listener) which will assign the client scopes to the issued access token. - ### Delete a client For now, clients deletion have to be managed manually using SQL queries. diff --git a/docs/controlling-token-scopes.md b/docs/controlling-token-scopes.md index 946777eb..d4e6274a 100644 --- a/docs/controlling-token-scopes.md +++ b/docs/controlling-token-scopes.md @@ -4,8 +4,6 @@ It's possible to alter issued access token's scopes by subscribing to the `triko ## Example -The following event listener will make sure the client will only be able to select scopes which they have been granted access to. - ### Listener ```php getClient()->getScopes(); - - if (empty($clientScopes)) { - // If the client doesn't have any scopes defined, that means he can access everything! - return; - } - $requestedScopes = $event->getScopes(); - if (empty($requestedScopes)) { - // If the client didn't request any scopes, inherit them from the client. - $requestedScopes = $clientScopes; - } - - $finalizedScopes = array_intersect($clientScopes, $requestedScopes); - - if (empty($finalizedScopes)) { - // If the filtered scopes end up being empty, fallback to using client scopes. - $finalizedScopes = $clientScopes; - } + // Make adjustments to the client's requested scopes... + ... - $event->setScopes(...$finalizedScopes); + $event->setScopes(...$requestedScopes); } } ``` From 50dce9fdcc427db3255782cf292997abc9877898 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petar=20Obradovi=C4=87?= Date: Wed, 20 Feb 2019 08:22:55 +0100 Subject: [PATCH 2/2] Minor code style adjustments --- Converter/ScopeConverter.php | 4 ++++ Event/ScopeResolveEvent.php | 2 +- Event/UserResolveEvent.php | 6 ------ League/Repository/ScopeRepository.php | 2 ++ Manager/InMemory/AccessTokenManager.php | 6 ++++++ Model/AccessToken.php | 4 ++-- Model/Client.php | 6 +++--- 7 files changed, 18 insertions(+), 12 deletions(-) diff --git a/Converter/ScopeConverter.php b/Converter/ScopeConverter.php index 9218a186..b19fc9bd 100644 --- a/Converter/ScopeConverter.php +++ b/Converter/ScopeConverter.php @@ -13,6 +13,8 @@ public function toDomain(ScopeEntity $scope): ScopeModel } /** + * @param ScopeEntity[] $scopes + * * @return ScopeModel[] */ public function toDomainArray(array $scopes): array @@ -31,6 +33,8 @@ public function toLeague(ScopeModel $scope): ScopeEntity } /** + * @param ScopeModel[] $scopes + * * @return ScopeEntity[] */ public function toLeagueArray(array $scopes): array diff --git a/Event/ScopeResolveEvent.php b/Event/ScopeResolveEvent.php index c4df7446..71442ccd 100644 --- a/Event/ScopeResolveEvent.php +++ b/Event/ScopeResolveEvent.php @@ -47,7 +47,7 @@ public function getScopes(): array public function setScopes(Scope ...$scopes): self { - $this->scopes = (array) $scopes; + $this->scopes = $scopes; return $this; } diff --git a/Event/UserResolveEvent.php b/Event/UserResolveEvent.php index b8be8b99..039b993f 100644 --- a/Event/UserResolveEvent.php +++ b/Event/UserResolveEvent.php @@ -62,17 +62,11 @@ public function getClient(): Client return $this->client; } - /** - * @return UserInterface - */ public function getUser(): ?UserInterface { return $this->user; } - /** - * @param UserInterface $user - */ public function setUser(?UserInterface $user): self { $this->user = $user; diff --git a/League/Repository/ScopeRepository.php b/League/Repository/ScopeRepository.php index fb67b63d..c67c836f 100644 --- a/League/Repository/ScopeRepository.php +++ b/League/Repository/ScopeRepository.php @@ -90,6 +90,8 @@ public function finalizeScopes( } /** + * @param ScopeModel[] $requestedScopes + * * @return ScopeModel[] */ private function setupScopes(ClientModel $client, array $requestedScopes): array diff --git a/Manager/InMemory/AccessTokenManager.php b/Manager/InMemory/AccessTokenManager.php index 9a6df467..2a887aa5 100644 --- a/Manager/InMemory/AccessTokenManager.php +++ b/Manager/InMemory/AccessTokenManager.php @@ -12,11 +12,17 @@ final class AccessTokenManager implements AccessTokenManagerInterface */ private $accessTokens = []; + /** + * {@inheritdoc} + */ public function find(string $identifier): ?AccessToken { return $this->accessTokens[$identifier] ?? null; } + /** + * {@inheritdoc} + */ public function save(AccessToken $accessToken): void { $this->accessTokens[$accessToken->getIdentifier()] = $accessToken; diff --git a/Model/AccessToken.php b/Model/AccessToken.php index 4a022697..c7fd6f7d 100644 --- a/Model/AccessToken.php +++ b/Model/AccessToken.php @@ -41,8 +41,8 @@ public function __construct( DateTime $expiry, Client $client, ?string $userIdentifier, - array $scopes) - { + array $scopes + ) { $this->identifier = $identifier; $this->expiry = $expiry; $this->client = $client; diff --git a/Model/Client.php b/Model/Client.php index bc177d8b..c5cc3028 100644 --- a/Model/Client.php +++ b/Model/Client.php @@ -65,7 +65,7 @@ public function getRedirectUris(): array public function setRedirectUris(RedirectUri ...$redirectUris): self { - $this->redirectUris = (array) $redirectUris; + $this->redirectUris = $redirectUris; return $this; } @@ -80,7 +80,7 @@ public function getGrants(): array public function setGrants(Grant ...$grants): self { - $this->grants = (array) $grants; + $this->grants = $grants; return $this; } @@ -95,7 +95,7 @@ public function getScopes(): array public function setScopes(Scope ...$scopes): self { - $this->scopes = (array) $scopes; + $this->scopes = $scopes; return $this; }