Performed some LDAP service/test cleanup

diff --git a/app/Auth/Access/LdapService.php b/app/Auth/Access/LdapService.php
index c48a72f9803122b40a189d2240a54a14d5ba2a09..9ffbbfbb75b57862c5543c4e4002013bd6485d28 100644 (file)
--- a/app/Auth/Access/LdapService.php
+++ b/app/Auth/Access/LdapService.php
@@ -88,14 +88,32 @@ class LdapService
             return null;
         }
 
             return null;
         }
 

+        $userCn = $this->getUserResponseProperty($user, 'cn', null);

         return [
         return [

-            'uid'   => (isset($user['uid'])) ? $user['uid'][0] : $user['dn'],
-            'name'  => (isset($user[$displayNameAttr])) ? (is_array($user[$displayNameAttr]) ? $user[$displayNameAttr][0] : $user[$displayNameAttr]) : $user['cn'][0],
+            'uid'   => $this->getUserResponseProperty($user, 'uid', $user['dn']),
+            'name' => $this->getUserResponseProperty($user, $displayNameAttr, $userCn),

             'dn'    => $user['dn'],
             'dn'    => $user['dn'],

-            'email' => (isset($user[$emailAttr])) ? (is_array($user[$emailAttr]) ? $user[$emailAttr][0] : $user[$emailAttr]) : null
+            'email' => $this->getUserResponseProperty($user, $emailAttr, null),

         ];
     }
 
         ];
     }
 

+    /**
+     * Get a property from an LDAP user response fetch.
+     * Handles properties potentially being part of an array.
+     * @param array $userDetails
+     * @param string $propertyKey
+     * @param $defaultValue
+     * @return mixed
+     */
+    protected function getUserResponseProperty(array $userDetails, string $propertyKey, $defaultValue)
+    {
+        if (isset($userDetails[$propertyKey])) {
+            return (is_array($userDetails[$propertyKey]) ? $userDetails[$propertyKey][0] : $userDetails[$propertyKey]);
+        }
+
+        return $defaultValue;
+    }
+

     /**
      * @param Authenticatable $user
      * @param string          $username
     /**
      * @param Authenticatable $user
      * @param string          $username

diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php
index 54cf84266e2109838982ccccf4d4967649290252..5ccb1415e6fed57e2bced5e33e6d04b966cadf2a 100644 (file)
--- a/tests/Auth/LdapTest.php
+++ b/tests/Auth/LdapTest.php
@@ -46,6 +46,15 @@ class LdapTest extends BrowserKitTest
         });
     }
 
         });
     }
 

+    protected function mockUserLogin()
+    {
+        return $this->visit('/login')
+            ->see('Username')
+            ->type($this->mockUser->name, '#username')
+            ->type($this->mockUser->password, '#password')
+            ->press('Log In');
+    }
+

     public function test_login()
     {
         $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId);
     public function test_login()
     {
         $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId);


@@ -61,11 +70,7 @@ class LdapTest extends BrowserKitTest
         $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true);
         $this->mockEscapes(4);
 
         $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true);
         $this->mockEscapes(4);
 

-        $this->visit('/login')
-            ->see('Username')
-            ->type($this->mockUser->name, '#username')
-            ->type($this->mockUser->password, '#password')
-            ->press('Log In')
+        $this->mockUserLogin()

             ->seePageIs('/login')->see('Please enter an email to use for this account.');
 
         $this->type($this->mockUser->email, '#email')
             ->seePageIs('/login')->see('Please enter an email to use for this account.');
 
         $this->type($this->mockUser->email, '#email')


@@ -91,11 +96,7 @@ class LdapTest extends BrowserKitTest
         $this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true);
         $this->mockEscapes(2);
 
         $this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true);
         $this->mockEscapes(2);
 

-        $this->visit('/login')
-            ->see('Username')
-            ->type($this->mockUser->name, '#username')
-            ->type($this->mockUser->password, '#password')
-            ->press('Log In')
+        $this->mockUserLogin()

             ->seePageIs('/')
             ->see($this->mockUser->name)
             ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $ldapDn]);
             ->seePageIs('/')
             ->see($this->mockUser->name)
             ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $ldapDn]);


@@ -116,11 +117,7 @@ class LdapTest extends BrowserKitTest
         $this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true, true, false);
         $this->mockEscapes(2);
 
         $this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true, true, false);
         $this->mockEscapes(2);
 

-        $this->visit('/login')
-            ->see('Username')
-            ->type($this->mockUser->name, '#username')
-            ->type($this->mockUser->password, '#password')
-            ->press('Log In')
+        $this->mockUserLogin()

             ->seePageIs('/login')->see('These credentials do not match our records.')
             ->dontSeeInDatabase('users', ['external_auth_id' => $this->mockUser->name]);
     }
             ->seePageIs('/login')->see('These credentials do not match our records.')
             ->dontSeeInDatabase('users', ['external_auth_id' => $this->mockUser->name]);
     }


@@ -197,12 +194,7 @@ class LdapTest extends BrowserKitTest
         $this->mockEscapes(5);
         $this->mockExplodes(6);
 
         $this->mockEscapes(5);
         $this->mockExplodes(6);
 

-        $this->visit('/login')
-            ->see('Username')
-            ->type($this->mockUser->name, '#username')
-            ->type($this->mockUser->password, '#password')
-            ->press('Log In')
-            ->seePageIs('/');
+        $this->mockUserLogin()->seePageIs('/');

 
         $user = User::where('email', $this->mockUser->email)->first();
         $this->seeInDatabase('role_user', [
 
         $user = User::where('email', $this->mockUser->email)->first();
         $this->seeInDatabase('role_user', [


@@ -250,12 +242,7 @@ class LdapTest extends BrowserKitTest
         $this->mockEscapes(4);
         $this->mockExplodes(2);
 
         $this->mockEscapes(4);
         $this->mockExplodes(2);
 

-        $this->visit('/login')
-            ->see('Username')
-            ->type($this->mockUser->name, '#username')
-            ->type($this->mockUser->password, '#password')
-            ->press('Log In')
-            ->seePageIs('/');
+        $this->mockUserLogin()->seePageIs('/');

 
         $user = User::where('email', $this->mockUser->email)->first();
         $this->seeInDatabase('role_user', [
 
         $user = User::where('email', $this->mockUser->email)->first();
         $this->seeInDatabase('role_user', [


@@ -304,12 +291,7 @@ class LdapTest extends BrowserKitTest
         $this->mockEscapes(4);
         $this->mockExplodes(2);
 
         $this->mockEscapes(4);
         $this->mockExplodes(2);
 

-        $this->visit('/login')
-            ->see('Username')
-            ->type($this->mockUser->name, '#username')
-            ->type($this->mockUser->password, '#password')
-            ->press('Log In')
-            ->seePageIs('/');
+        $this->mockUserLogin()->seePageIs('/');

 
         $user = User::where('email', $this->mockUser->email)->first();
         $this->seeInDatabase('role_user', [
 
         $user = User::where('email', $this->mockUser->email)->first();
         $this->seeInDatabase('role_user', [


@@ -355,12 +337,7 @@ class LdapTest extends BrowserKitTest
         $this->mockEscapes(5);
         $this->mockExplodes(6);
 
         $this->mockEscapes(5);
         $this->mockExplodes(6);
 

-        $this->visit('/login')
-            ->see('Username')
-            ->type($this->mockUser->name, '#username')
-            ->type($this->mockUser->password, '#password')
-            ->press('Log In')
-            ->seePageIs('/');
+        $this->mockUserLogin()->seePageIs('/');

 
         $user = User::where('email', $this->mockUser->email)->first();
         $this->seeInDatabase('role_user', [
 
         $user = User::where('email', $this->mockUser->email)->first();
         $this->seeInDatabase('role_user', [


@@ -375,7 +352,6 @@ class LdapTest extends BrowserKitTest
 
     public function test_login_uses_specified_display_name_attribute()
     {
 
     public function test_login_uses_specified_display_name_attribute()
     {

-        $originalAttribute = config('services.ldap.display_name_attribute');

         app('config')->set([
             'services.ldap.display_name_attribute' => 'displayName'
         ]);
         app('config')->set([
             'services.ldap.display_name_attribute' => 'displayName'
         ]);


@@ -394,11 +370,7 @@ class LdapTest extends BrowserKitTest
         $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true);
         $this->mockEscapes(4);
 
         $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true);
         $this->mockEscapes(4);
 

-        $this->visit('/login')
-            ->see('Username')
-            ->type($this->mockUser->name, '#username')
-            ->type($this->mockUser->password, '#password')
-            ->press('Log In')
+        $this->mockUserLogin()

             ->seePageIs('/login')->see('Please enter an email to use for this account.');
 
         $this->type($this->mockUser->email, '#email')
             ->seePageIs('/login')->see('Please enter an email to use for this account.');
 
         $this->type($this->mockUser->email, '#email')


@@ -406,15 +378,10 @@ class LdapTest extends BrowserKitTest
             ->seePageIs('/')
             ->see('displayNameAttribute')
             ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, 'name' => 'displayNameAttribute']);
             ->seePageIs('/')
             ->see('displayNameAttribute')
             ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, 'name' => 'displayNameAttribute']);

-        
-        app('config')->set([
-            'services.ldap.display_name_attribute' => $originalAttribute
-        ]);

     }
 
     public function test_login_uses_default_display_name_attribute_if_specified_not_present()
     {
     }
 
     public function test_login_uses_default_display_name_attribute_if_specified_not_present()
     {

-        $originalAttribute = config('services.ldap.display_name_attribute');

         app('config')->set([
             'services.ldap.display_name_attribute' => 'displayName'
         ]);
         app('config')->set([
             'services.ldap.display_name_attribute' => 'displayName'
         ]);


@@ -432,11 +399,7 @@ class LdapTest extends BrowserKitTest
         $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true);
         $this->mockEscapes(4);
 
         $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true);
         $this->mockEscapes(4);
 

-        $this->visit('/login')
-            ->see('Username')
-            ->type($this->mockUser->name, '#username')
-            ->type($this->mockUser->password, '#password')
-            ->press('Log In')
+        $this->mockUserLogin()

             ->seePageIs('/login')->see('Please enter an email to use for this account.');
 
         $this->type($this->mockUser->email, '#email')
             ->seePageIs('/login')->see('Please enter an email to use for this account.');
 
         $this->type($this->mockUser->email, '#email')


@@ -444,10 +407,6 @@ class LdapTest extends BrowserKitTest
             ->seePageIs('/')
             ->see($this->mockUser->name)
             ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, 'name' => $this->mockUser->name]);
             ->seePageIs('/')
             ->see($this->mockUser->name)
             ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, 'name' => $this->mockUser->name]);

-    
-        app('config')->set([
-            'services.ldap.display_name_attribute' => $originalAttribute
-        ]);

     }
 
 }
     }
 
 }