From ea14abb3c908a3525d361623682afb4b50c1dd1a Mon Sep 17 00:00:00 2001 From: Stefan Brankovikj Date: Sat, 14 Mar 2020 19:38:25 +0100 Subject: [PATCH 1/2] Lazy connection before searching for user while trying to authenticate --- src/Resolvers/UserResolver.php | 10 +++++++++- tests/DatabaseProviderTest.php | 30 ++++++++++++++++++++++++++++++ tests/UserResolverTest.php | 19 ++++++++++++++++++- 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/Resolvers/UserResolver.php b/src/Resolvers/UserResolver.php index 0ceab59..ffc95af 100644 --- a/src/Resolvers/UserResolver.php +++ b/src/Resolvers/UserResolver.php @@ -201,7 +201,15 @@ protected function getPasswordFromCredentials($credentials) */ protected function getLdapAuthProvider(): ProviderInterface { - return $this->ldap->getProvider($this->connection ?? $this->getLdapAuthConnectionName()); + $provider = $this->ldap->getProvider($this->connection ?? $this->getLdapAuthConnectionName()); + + if (!$provider->getConnection()->isBound()) { + // We'll make sure we have a bound connection before + // allowing dynamic calls on the default provider. + $provider->connect(); + } + + return $provider; } /** diff --git a/tests/DatabaseProviderTest.php b/tests/DatabaseProviderTest.php index 4f78741..7ebb350 100644 --- a/tests/DatabaseProviderTest.php +++ b/tests/DatabaseProviderTest.php @@ -3,8 +3,13 @@ namespace Adldap\Laravel\Tests; use Adldap\AdldapInterface; +use Adldap\Connections\ConnectionInterface; +use Adldap\Connections\Provider; +use Adldap\Connections\ProviderInterface; use Adldap\Laravel\Commands\Import; use Adldap\Laravel\Facades\Resolver; +use Adldap\Laravel\Resolvers\ResolverInterface; +use Adldap\Laravel\Resolvers\UserResolver; use Adldap\Laravel\Tests\Handlers\LdapAttributeHandler; use Adldap\Laravel\Tests\Models\TestUser as EloquentUser; use Adldap\Laravel\Tests\Scopes\JohnDoeScope; @@ -13,6 +18,7 @@ use Illuminate\Support\Facades\App; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Hash; +use Mockery as m; class DatabaseProviderTest extends DatabaseTestCase { @@ -89,8 +95,20 @@ public function auth_fails_when_user_not_found() /** @test */ public function config_scopes_are_applied() { + $ldapMock = m::mock(AdldapInterface::class); + App::instance(AdldapInterface::class, $ldapMock); + /** @var Provider $provider */ + $provider = App::make(Provider::class); config(['ldap_auth.scopes' => [JohnDoeScope::class]]); + $providerMock = m::mock(ProviderInterface::class); + $connectionMock = m::mock(ConnectionInterface::class); + + $providerMock->shouldReceive('getConnection')->once()->andReturn($connectionMock); + $connectionMock->shouldReceive('isBound')->once()->andReturn(true); + $ldapMock->shouldReceive('getProvider')->once()->andReturn($providerMock); + $providerMock->shouldReceive('search')->once()->andReturn($provider->search()); + $expectedFilter = '(&(objectclass=\75\73\65\72)(objectcategory=\70\65\72\73\6f\6e)(!(objectclass=\63\6f\6e\74\61\63\74))(cn=\4a\6f\68\6e\20\44\6f\65))'; $this->assertEquals($expectedFilter, Resolver::query()->getQuery()); @@ -219,6 +237,10 @@ public function auth_attempts_fallback_using_config_option() /** @test */ public function auth_attempts_using_fallback_does_not_require_connection() { + $ldapMock = m::mock(AdldapInterface::class); + App::instance(AdldapInterface::class, $ldapMock); + /** @var Provider $provider */ + $provider = App::make(Provider::class); config(['ldap_auth.login_fallback' => true]); EloquentUser::create([ @@ -232,6 +254,14 @@ public function auth_attempts_using_fallback_does_not_require_connection() 'password' => 'Password123', ]; + $providerMock = m::mock(ProviderInterface::class); + $connectionMock = m::mock(ConnectionInterface::class); + + $providerMock->shouldReceive('getConnection')->times(3)->andReturn($connectionMock); + $connectionMock->shouldReceive('isBound')->times(3)->andReturn(true); + $ldapMock->shouldReceive('getProvider')->times(3)->andReturn($providerMock); + $providerMock->shouldReceive('search')->times(3)->andReturn($provider->search()); + $this->assertTrue(Auth::attempt($credentials)); $user = Auth::user(); diff --git a/tests/UserResolverTest.php b/tests/UserResolverTest.php index 7b588a6..495a503 100644 --- a/tests/UserResolverTest.php +++ b/tests/UserResolverTest.php @@ -3,6 +3,7 @@ namespace Adldap\Laravel\Tests; use Adldap\AdldapInterface; +use Adldap\Connections\ConnectionInterface; use Adldap\Connections\ProviderInterface; use Adldap\Laravel\Auth\NoDatabaseUserProvider; use Adldap\Laravel\Resolvers\UserResolver; @@ -85,6 +86,11 @@ public function scopes_are_applied_when_query_is_called() ->shouldReceive('users')->once()->andReturn($builder); $ad = m::mock(AdldapInterface::class); + $ldapConnection = m::mock(ConnectionInterface::class); + $ldapConnection->shouldReceive('isBound')->once()->andReturn(false); + + $provider->shouldReceive('getConnection')->once()->andReturn($ldapConnection); + $provider->shouldReceive('connect')->once(); $ad->shouldReceive('getProvider')->with('default')->andReturn($provider); @@ -99,8 +105,14 @@ public function connection_is_set_when_retrieving_provider() Config::shouldReceive('get')->once()->with('ldap_auth.connection', 'default')->andReturn('other-domain'); $ad = m::mock(AdldapInterface::class); + $provider = m::mock(ProviderInterface::class); - $ad->shouldReceive('getProvider')->andReturn(m::mock(ProviderInterface::class))->with('other-domain'); + $ad->shouldReceive('getProvider')->with('other-domain')->andReturn($provider); + $ldapConnection = m::mock(ConnectionInterface::class); + $ldapConnection->shouldReceive('isBound')->once()->andReturn(false); + + $provider->shouldReceive('getConnection')->once()->andReturn($ldapConnection); + $provider->shouldReceive('connect')->once(); $r = m::mock(UserResolver::class, [$ad])->makePartial(); @@ -130,6 +142,11 @@ public function by_credentials_retrieves_alternate_username_attribute_depending_ ->shouldReceive('users')->once()->andReturn($query); $ad = m::mock(AdldapInterface::class); + $ldapConnection = m::mock(ConnectionInterface::class); + $ldapConnection->shouldReceive('isBound')->once()->andReturn(false); + + $ldapProvider->shouldReceive('getConnection')->once()->andReturn($ldapConnection); + $ldapProvider->shouldReceive('connect')->once(); $ad->shouldReceive('getProvider')->once()->andReturn($ldapProvider); From 385f9bafcb925c4cba84ecfccc2d4fbb63b0c548 Mon Sep 17 00:00:00 2001 From: Stefan Brankovikj Date: Sat, 14 Mar 2020 20:03:59 +0100 Subject: [PATCH 2/2] Follow the code style checks --- src/Resolvers/UserResolver.php | 2 +- tests/DatabaseProviderTest.php | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Resolvers/UserResolver.php b/src/Resolvers/UserResolver.php index ffc95af..62cf768 100644 --- a/src/Resolvers/UserResolver.php +++ b/src/Resolvers/UserResolver.php @@ -203,7 +203,7 @@ protected function getLdapAuthProvider(): ProviderInterface { $provider = $this->ldap->getProvider($this->connection ?? $this->getLdapAuthConnectionName()); - if (!$provider->getConnection()->isBound()) { + if (! $provider->getConnection()->isBound()) { // We'll make sure we have a bound connection before // allowing dynamic calls on the default provider. $provider->connect(); diff --git a/tests/DatabaseProviderTest.php b/tests/DatabaseProviderTest.php index 7ebb350..e3ee184 100644 --- a/tests/DatabaseProviderTest.php +++ b/tests/DatabaseProviderTest.php @@ -8,8 +8,6 @@ use Adldap\Connections\ProviderInterface; use Adldap\Laravel\Commands\Import; use Adldap\Laravel\Facades\Resolver; -use Adldap\Laravel\Resolvers\ResolverInterface; -use Adldap\Laravel\Resolvers\UserResolver; use Adldap\Laravel\Tests\Handlers\LdapAttributeHandler; use Adldap\Laravel\Tests\Models\TestUser as EloquentUser; use Adldap\Laravel\Tests\Scopes\JohnDoeScope;