From 132046a88a780244ad01f0eedf82ccacd6024822 Mon Sep 17 00:00:00 2001 From: Will Taylor-Jackson Date: Thu, 31 May 2018 10:35:26 +0100 Subject: [PATCH 1/4] feat: tests for restricting client grant types --- tests/ClientRepositoryTest.php | 44 ++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 tests/ClientRepositoryTest.php diff --git a/tests/ClientRepositoryTest.php b/tests/ClientRepositoryTest.php new file mode 100644 index 000000000..7647213a7 --- /dev/null +++ b/tests/ClientRepositoryTest.php @@ -0,0 +1,44 @@ + 5, + 'grant_types' => ['password'], + 'name' => 'Password only client', + 'password_client' => true, + 'secret' => 'secret', + ]); + + $clientModelRepository = Mockery::mock(Laravel\Passport\ClientRepository::class); + $clientModelRepository->shouldReceive('findActive')->with('passwordOnlyClient')->andReturn($passwordOnlyClient); + + $this->clientRepository = new Laravel\Passport\Bridge\ClientRepository($clientModelRepository); + } + + public function tearDown() + { + Mockery::close(); + } + + public function test_password_only_client_is_permitted() + { + // client restricts the grant types - password grant must be accepted + $client = $this->clientRepository->getClientEntity('passwordOnlyClient', 'password', 'secret'); + $this->assertEquals('passwordOnlyClient', $client->getIdentifier()); + } + + public function test_password_only_client_is_prevented() + { + // client restricts the grant types - client_credentials grant must not be accepted + $client = $this->clientRepository->getClientEntity('passwordOnlyClient', 'client_credentials', 'secret'); + $this->assertEquals(null, $client); + } +} From c68bca06cff15aa707fcb2beb79ceeef1ca3124b Mon Sep 17 00:00:00 2001 From: Will Taylor-Jackson Date: Thu, 31 May 2018 10:36:21 +0100 Subject: [PATCH 2/4] feat: allow grant types to be restricted for a client --- src/Bridge/ClientRepository.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Bridge/ClientRepository.php b/src/Bridge/ClientRepository.php index b82bc1f30..c1ddb5cc4 100644 --- a/src/Bridge/ClientRepository.php +++ b/src/Bridge/ClientRepository.php @@ -64,6 +64,10 @@ public function getClientEntity($clientIdentifier, $grantType = null, */ protected function handlesGrant($record, $grantType) { + if (is_array($record->grant_types) && !in_array($grantType, $record->grant_types)) { + return false; + } + switch ($grantType) { case 'authorization_code': return ! $record->firstParty(); From a327ee1b2e7af7c48b57063f939acfdddf548c6c Mon Sep 17 00:00:00 2001 From: Will Taylor-Jackson Date: Thu, 31 May 2018 12:38:03 +0100 Subject: [PATCH 3/4] fix: update broken tests, move to correct class --- tests/BridgeClientRepositoryTest.php | 51 ++++++++++++++++++++-------- tests/ClientRepositoryTest.php | 44 ------------------------ 2 files changed, 37 insertions(+), 58 deletions(-) delete mode 100644 tests/ClientRepositoryTest.php diff --git a/tests/BridgeClientRepositoryTest.php b/tests/BridgeClientRepositoryTest.php index 59853f79b..905d03167 100644 --- a/tests/BridgeClientRepositoryTest.php +++ b/tests/BridgeClientRepositoryTest.php @@ -5,6 +5,15 @@ class BridgeClientRepositoryTest extends TestCase { + public function setUp() + { + $clientModelRepository = Mockery::mock(Laravel\Passport\ClientRepository::class); + $clientModelRepository->shouldReceive('findActive')->with(1)->andReturn(new BridgeClientRepositoryTestClientStub); + + $this->clientModelRepository = $clientModelRepository; + $this->repository = new Laravel\Passport\Bridge\ClientRepository($clientModelRepository); + } + public function tearDown() { Mockery::close(); @@ -12,28 +21,40 @@ public function tearDown() public function test_can_get_client_for_auth_code_grant() { - $clients = Mockery::mock('Laravel\Passport\ClientRepository'); - $client = new BridgeClientRepositoryTestClientStub; - $clients->shouldReceive('findActive')->with(1)->andReturn($client); - $repository = new ClientRepository($clients); - - $client = $repository->getClientEntity(1, 'authorization_code', 'secret', true); + $client = $this->repository->getClientEntity(1, 'authorization_code', 'secret', true); $this->assertInstanceOf('Laravel\Passport\Bridge\Client', $client); - $this->assertNull($repository->getClientEntity(1, 'authorization_code', 'wrong-secret', true)); - $this->assertNull($repository->getClientEntity(1, 'client_credentials', 'wrong-secret', true)); + $this->assertNull($this->repository->getClientEntity(1, 'authorization_code', 'wrong-secret', true)); + $this->assertNull($this->repository->getClientEntity(1, 'client_credentials', 'wrong-secret', true)); } public function test_can_get_client_for_client_credentials_grant() { - $clients = Mockery::mock('Laravel\Passport\ClientRepository'); - $client = new BridgeClientRepositoryTestClientStub; + $client = $this->clientModelRepository->findActive(1); $client->personal_access_client = true; - $clients->shouldReceive('findActive')->with(1)->andReturn($client); - $repository = new ClientRepository($clients); - $this->assertInstanceOf('Laravel\Passport\Bridge\Client', $repository->getClientEntity(1, 'client_credentials', 'secret', true)); - $this->assertNull($repository->getClientEntity(1, 'authorization_code', 'secret', true)); + $this->assertInstanceOf('Laravel\Passport\Bridge\Client', $this->repository->getClientEntity(1, 'client_credentials', 'secret', true)); + $this->assertNull($this->repository->getClientEntity(1, 'authorization_code', 'secret', true)); + } + + public function test_password_only_client_is_permitted() + { + $client = $this->clientModelRepository->findActive(1); + $client->password_client = true; + $client->grants = ['password']; + + $client = $this->repository->getClientEntity(1, 'password', 'secret'); + $this->assertEquals('Client', $client->getName()); + } + + public function test_password_only_client_is_prevented() + { + $client = $this->clientModelRepository->findActive(1); + $client->password_client = true; + $client->grant_types = ['password']; + + $client = $this->repository->getClientEntity(1, 'client_credentials', 'secret'); + $this->assertNull($client); } } @@ -44,6 +65,8 @@ class BridgeClientRepositoryTestClientStub public $secret = 'secret'; public $personal_access_client = false; public $password_client = false; + public $grant_types; + public function firstParty() { return $this->personal_access_client || $this->password_client; diff --git a/tests/ClientRepositoryTest.php b/tests/ClientRepositoryTest.php deleted file mode 100644 index 7647213a7..000000000 --- a/tests/ClientRepositoryTest.php +++ /dev/null @@ -1,44 +0,0 @@ - 5, - 'grant_types' => ['password'], - 'name' => 'Password only client', - 'password_client' => true, - 'secret' => 'secret', - ]); - - $clientModelRepository = Mockery::mock(Laravel\Passport\ClientRepository::class); - $clientModelRepository->shouldReceive('findActive')->with('passwordOnlyClient')->andReturn($passwordOnlyClient); - - $this->clientRepository = new Laravel\Passport\Bridge\ClientRepository($clientModelRepository); - } - - public function tearDown() - { - Mockery::close(); - } - - public function test_password_only_client_is_permitted() - { - // client restricts the grant types - password grant must be accepted - $client = $this->clientRepository->getClientEntity('passwordOnlyClient', 'password', 'secret'); - $this->assertEquals('passwordOnlyClient', $client->getIdentifier()); - } - - public function test_password_only_client_is_prevented() - { - // client restricts the grant types - client_credentials grant must not be accepted - $client = $this->clientRepository->getClientEntity('passwordOnlyClient', 'client_credentials', 'secret'); - $this->assertEquals(null, $client); - } -} From 03e89f720f7f72730bc662e4256e46b7c781bd53 Mon Sep 17 00:00:00 2001 From: Will Taylor-Jackson Date: Thu, 31 May 2018 21:58:22 +0100 Subject: [PATCH 4/4] feat: tests for grant types restriction --- tests/BridgeClientRepositoryTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/BridgeClientRepositoryTest.php b/tests/BridgeClientRepositoryTest.php index 681c0dbce..0e8eb9cfc 100644 --- a/tests/BridgeClientRepositoryTest.php +++ b/tests/BridgeClientRepositoryTest.php @@ -88,6 +88,22 @@ public function test_client_credentials_grant_is_prevented() $this->assertNull($this->repository->getClientEntity(1, 'client_credentials', 'secret')); } + + public function test_grant_types_allows_request() + { + $client = $this->clientModelRepository->findActive(1); + $client->grant_types = ['client_credentials']; + + $this->assertInstanceOf('Laravel\Passport\Bridge\Client', $this->repository->getClientEntity(1, 'client_credentials', 'secret')); + } + + 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')); + } } class BridgeClientRepositoryTestClientStub