From 97e3026790d953d7a67fe487e30775cd995e93df Mon Sep 17 00:00:00 2001 From: Matt Allan Date: Wed, 17 Jul 2019 14:38:54 -0400 Subject: [PATCH 1/2] Upgrade to league/oauth2-server 8.0 --- composer.json | 2 +- src/Bridge/AccessToken.php | 6 ++- src/Bridge/AccessTokenRepository.php | 2 +- src/Bridge/Client.php | 4 +- src/Bridge/ClientRepository.php | 31 +++++++-------- tests/BridgeAccessTokenRepositoryTest.php | 24 ++++++++++-- tests/BridgeClientRepositoryTest.php | 47 +++++++++++++---------- 7 files changed, 73 insertions(+), 43 deletions(-) diff --git a/composer.json b/composer.json index 4bd34b523..8b20138b4 100644 --- a/composer.json +++ b/composer.json @@ -26,7 +26,7 @@ "illuminate/encryption": "~5.8.0|~5.9.0", "illuminate/http": "~5.8.0|~5.9.0", "illuminate/support": "~5.8.0|~5.9.0", - "league/oauth2-server": "^7.0", + "league/oauth2-server": "^8.0", "phpseclib/phpseclib": "^2.0", "symfony/psr-http-message-bridge": "^1.0", "zendframework/zend-diactoros": "^2.0" diff --git a/src/Bridge/AccessToken.php b/src/Bridge/AccessToken.php index 3adcbd049..649081ae4 100644 --- a/src/Bridge/AccessToken.php +++ b/src/Bridge/AccessToken.php @@ -3,6 +3,7 @@ namespace Laravel\Passport\Bridge; use League\OAuth2\Server\Entities\Traits\EntityTrait; +use League\OAuth2\Server\Entities\ClientEntityInterface; use League\OAuth2\Server\Entities\Traits\AccessTokenTrait; use League\OAuth2\Server\Entities\Traits\TokenEntityTrait; use League\OAuth2\Server\Entities\AccessTokenEntityInterface; @@ -16,14 +17,17 @@ class AccessToken implements AccessTokenEntityInterface * * @param string $userIdentifier * @param array $scopes + * @param ClientEntityInterface $client * @return void */ - public function __construct($userIdentifier, array $scopes = []) + public function __construct($userIdentifier, array $scopes, ClientEntityInterface $client) { $this->setUserIdentifier($userIdentifier); foreach ($scopes as $scope) { $this->addScope($scope); } + + $this->setClient($client); } } diff --git a/src/Bridge/AccessTokenRepository.php b/src/Bridge/AccessTokenRepository.php index 7b6770e8d..dc3d982ce 100644 --- a/src/Bridge/AccessTokenRepository.php +++ b/src/Bridge/AccessTokenRepository.php @@ -45,7 +45,7 @@ public function __construct(TokenRepository $tokenRepository, Dispatcher $events */ public function getNewToken(ClientEntityInterface $clientEntity, array $scopes, $userIdentifier = null) { - return new AccessToken($userIdentifier, $scopes); + return new AccessToken($userIdentifier, $scopes, $clientEntity); } /** diff --git a/src/Bridge/Client.php b/src/Bridge/Client.php index 1c4551d4f..04e2cc96e 100644 --- a/src/Bridge/Client.php +++ b/src/Bridge/Client.php @@ -16,13 +16,15 @@ class Client implements ClientEntityInterface * @param string $identifier * @param string $name * @param string $redirectUri + * @param bool $isConfidential * @return void */ - public function __construct($identifier, $name, $redirectUri) + public function __construct($identifier, $name, $redirectUri, $isConfidential = false) { $this->setIdentifier((string) $identifier); $this->name = $name; $this->redirectUri = explode(',', $redirectUri); + $this->isConfidential = $isConfidential; } } diff --git a/src/Bridge/ClientRepository.php b/src/Bridge/ClientRepository.php index 076933fe4..9e89ab153 100644 --- a/src/Bridge/ClientRepository.php +++ b/src/Bridge/ClientRepository.php @@ -28,31 +28,32 @@ public function __construct(ClientModelRepository $clients) /** * {@inheritdoc} */ - public function getClientEntity($clientIdentifier, $grantType = null, - $clientSecret = null, $mustValidateSecret = true) + public function getClientEntity($clientIdentifier) { - // First, we will verify that the client exists and is authorized to create personal - // access tokens. Generally personal access tokens are only generated by the user - // from the main interface. We'll only let certain clients generate the tokens. $record = $this->clients->findActive($clientIdentifier); - if (! $record || ! $this->handlesGrant($record, $grantType)) { + if (! $record) { return; } - // Once we have an existing client record we will create this actual client instance - // and verify the secret if necessary. If the secret is valid we will be ready to - // return this client instance back out to the consuming methods and finish up. - $client = new Client( - $clientIdentifier, $record->name, $record->redirect + return new Client( + $clientIdentifier, $record->name, $record->redirect, ! is_null($record->secret) ); + } - if ($mustValidateSecret && - ! hash_equals($record->secret, (string) $clientSecret)) { - return; + public function validateClient($clientIdentifier, $clientSecret, $grantType) + { + // First, we will verify that the client exists and is authorized to create personal + // access tokens. Generally personal access tokens are only generated by the user + // from the main interface. We'll only let certain clients generate the tokens. + $record = $this->clients->findActive($clientIdentifier); + + if (! $record || ! $this->handlesGrant($record, $grantType)) { + return false; } - return $client; + // Once we have an existing client record we will verify the secret. + return hash_equals($record->secret, (string) $clientSecret); } /** diff --git a/tests/BridgeAccessTokenRepositoryTest.php b/tests/BridgeAccessTokenRepositoryTest.php index 18dc26d30..b573b0e16 100644 --- a/tests/BridgeAccessTokenRepositoryTest.php +++ b/tests/BridgeAccessTokenRepositoryTest.php @@ -3,7 +3,7 @@ namespace Laravel\Passport\Tests; use Mockery as m; -use Carbon\Carbon; +use Carbon\CarbonImmutable; use PHPUnit\Framework\TestCase; use Laravel\Passport\Bridge\Scope; use Laravel\Passport\Bridge\Client; @@ -21,7 +21,7 @@ public function tearDown() public function test_access_tokens_can_be_persisted() { - $expiration = Carbon::now(); + $expiration = CarbonImmutable::now(); $tokenRepository = m::mock(TokenRepository::class); $events = m::mock(Dispatcher::class); @@ -39,13 +39,29 @@ public function test_access_tokens_can_be_persisted() $events->shouldReceive('dispatch')->once(); - $accessToken = new AccessToken(2, [new Scope('scopes')]); + $accessToken = new AccessToken(2, [new Scope('scopes')], new Client('client-id', 'name', 'redirect')); $accessToken->setIdentifier(1); $accessToken->setExpiryDateTime($expiration); - $accessToken->setClient(new Client('client-id', 'name', 'redirect')); $repository = new AccessTokenRepository($tokenRepository, $events); $repository->persistNewAccessToken($accessToken); } + + public function test_can_get_new_access_token() + { + $tokenRepository = m::mock(TokenRepository::class); + $events = m::mock(Dispatcher::class); + $repository = new AccessTokenRepository($tokenRepository, $events); + $client = new Client('client-id', 'name', 'redirect'); + $scopes = [new Scope('place-orders'), new Scope('check-status')]; + $userIdentifier = 123; + + $token = $repository->getNewToken($client, $scopes, $userIdentifier); + + $this->assertInstanceOf(AccessToken::class, $token); + $this->assertEquals($client, $token->getClient()); + $this->assertEquals($scopes, $token->getScopes()); + $this->assertEquals($userIdentifier, $token->getUserIdentifier()); + } } diff --git a/tests/BridgeClientRepositoryTest.php b/tests/BridgeClientRepositoryTest.php index 0a350a19a..b861a96c3 100644 --- a/tests/BridgeClientRepositoryTest.php +++ b/tests/BridgeClientRepositoryTest.php @@ -38,25 +38,32 @@ public function tearDown() unset($this->clientModelRepository, $this->repository); } - public function test_can_get_client_for_auth_code_grant() + public function test_can_get_client() { - $client = $this->repository->getClientEntity(1, 'authorization_code', 'secret', true); + $client = $this->repository->getClientEntity(1); $this->assertInstanceOf(Client::class, $client); - $this->assertNull($this->repository->getClientEntity(1, 'authorization_code', 'wrong-secret', true)); - $this->assertNull($this->repository->getClientEntity(1, 'client_credentials', 'wrong-secret', true)); + $this->assertEquals('1', $client->getIdentifier()); + $this->assertEquals('Client', $client->getName()); + $this->assertEquals(['http://localhost'], $client->getRedirectUri()); + $this->assertTrue($client->isConfidential()); } - public function test_can_get_client_for_client_credentials_grant() + public function test_can_validate_client_for_auth_code_grant() + { + $this->assertTrue($this->repository->validateClient(1, 'secret', 'authorization_code')); + $this->assertFalse($this->repository->validateClient(1, 'wrong-secret', 'authorization_code')); + $this->assertFalse($this->repository->validateClient(1, 'wrong-secret', 'client_credentials')); + } + + public function test_can_validate_client_for_client_credentials_grant() { $client = $this->clientModelRepository->findActive(1); $client->personal_access_client = true; - $this->assertInstanceOf( - Client::class, - $this->repository->getClientEntity(1, 'client_credentials', 'secret', true) - ); - $this->assertNull($this->repository->getClientEntity(1, 'authorization_code', 'secret', true)); + $this->assertTrue($this->repository->validateClient(1, 'secret', 'client_credentials')); + $this->assertFalse($this->repository->validateClient(1, 'wrong-secret', 'client_credentials')); + $this->assertFalse($this->repository->validateClient(1, 'secret', 'authorization_code')); } public function test_password_grant_is_permitted() @@ -64,17 +71,17 @@ public function test_password_grant_is_permitted() $client = $this->clientModelRepository->findActive(1); $client->password_client = true; - $this->assertInstanceOf(Client::class, $this->repository->getClientEntity(1, 'password', 'secret')); + $this->assertTrue($this->repository->validateClient(1, 'secret', 'password')); } public function test_password_grant_is_prevented() { - $this->assertNull($this->repository->getClientEntity(1, 'password', 'secret')); + $this->assertFalse($this->repository->validateClient(1, 'secret', 'password')); } public function test_authorization_code_grant_is_permitted() { - $this->assertInstanceOf(Client::class, $this->repository->getClientEntity(1, 'authorization_code', 'secret')); + $this->assertTrue($this->repository->validateClient(1, 'secret', 'authorization_code')); } public function test_authorization_code_grant_is_prevented() @@ -82,7 +89,7 @@ public function test_authorization_code_grant_is_prevented() $client = $this->clientModelRepository->findActive(1); $client->password_client = true; - $this->assertNull($this->repository->getClientEntity(1, 'authorization_code', 'secret')); + $this->assertFalse($this->repository->validateClient(1, 'secret', 'authorization_code')); } public function test_personal_access_grant_is_permitted() @@ -90,17 +97,17 @@ public function test_personal_access_grant_is_permitted() $client = $this->clientModelRepository->findActive(1); $client->personal_access_client = true; - $this->assertInstanceOf(Client::class, $this->repository->getClientEntity(1, 'personal_access', 'secret')); + $this->assertTrue($this->repository->validateClient(1, 'secret', 'personal_access')); } public function test_personal_access_grant_is_prevented() { - $this->assertNull($this->repository->getClientEntity(1, 'personal_access', 'secret')); + $this->assertFalse($this->repository->validateClient(1, 'secret', 'personal_access')); } public function test_client_credentials_grant_is_permitted() { - $this->assertInstanceOf(Client::class, $this->repository->getClientEntity(1, 'client_credentials', 'secret')); + $this->assertTrue($this->repository->validateClient(1, 'secret', 'client_credentials')); } public function test_client_credentials_grant_is_prevented() @@ -108,7 +115,7 @@ public function test_client_credentials_grant_is_prevented() $client = $this->clientModelRepository->findActive(1); $client->secret = null; - $this->assertNull($this->repository->getClientEntity(1, 'client_credentials', 'secret')); + $this->assertFalse($this->repository->validateClient(1, 'secret', 'client_credentials')); } public function test_grant_types_allows_request() @@ -116,7 +123,7 @@ public function test_grant_types_allows_request() $client = $this->clientModelRepository->findActive(1); $client->grant_types = ['client_credentials']; - $this->assertInstanceOf(Client::class, $this->repository->getClientEntity(1, 'client_credentials', 'secret')); + $this->assertTrue($this->repository->validateClient(1, 'secret', 'client_credentials')); } public function test_grant_types_disallows_request() @@ -124,7 +131,7 @@ public function test_grant_types_disallows_request() $client = $this->clientModelRepository->findActive(1); $client->grant_types = ['client_credentials']; - $this->assertNull($this->repository->getClientEntity(1, 'authorization_code', 'secret')); + $this->assertFalse($this->repository->validateClient(1, 'secret', 'authorization_code')); } } From c395a024bd9ac7e4c77bc44a9f9a6a501924f0fd Mon Sep 17 00:00:00 2001 From: Matt Allan Date: Thu, 18 Jul 2019 09:58:52 -0400 Subject: [PATCH 2/2] formatting --- src/Bridge/Client.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Bridge/Client.php b/src/Bridge/Client.php index 04e2cc96e..a294f37b4 100644 --- a/src/Bridge/Client.php +++ b/src/Bridge/Client.php @@ -16,7 +16,7 @@ class Client implements ClientEntityInterface * @param string $identifier * @param string $name * @param string $redirectUri - * @param bool $isConfidential + * @param bool $isConfidential * @return void */ public function __construct($identifier, $name, $redirectUri, $isConfidential = false) @@ -24,7 +24,7 @@ public function __construct($identifier, $name, $redirectUri, $isConfidential = $this->setIdentifier((string) $identifier); $this->name = $name; - $this->redirectUri = explode(',', $redirectUri); $this->isConfidential = $isConfidential; + $this->redirectUri = explode(',', $redirectUri); } }