LDAP: Updated tests for recursive group changes

Author: ssddanbrown

app/Access/LdapService.php

@@ -334,7 +334,7 @@ public function getUserGroups(string $userName): array
             ]);
         }
 
-        return $allGroups;
+        return $formattedGroups;
     }
 
     protected function extractGroupNamesFromLdapGroupDns(array $groupDNs): array

tests/Auth/LdapTest.php

@@ -76,14 +76,21 @@ protected function mockUserLogin(?string $email = null): TestResponse
     /**
      * Set LDAP method mocks for things we commonly call without altering.
      */
-    protected function commonLdapMocks(int $connects = 1, int $versions = 1, int $options = 2, int $binds = 4, int $escapes = 2, int $explodes = 0)
+    protected function commonLdapMocks(int $connects = 1, int $versions = 1, int $options = 2, int $binds = 4, int $escapes = 2, int $explodes = 0, int $groups = 0)
     {
         $this->mockLdap->shouldReceive('connect')->times($connects)->andReturn($this->resourceId);
         $this->mockLdap->shouldReceive('setVersion')->times($versions);
         $this->mockLdap->shouldReceive('setOption')->times($options);
         $this->mockLdap->shouldReceive('bind')->times($binds)->andReturn(true);
         $this->mockEscapes($escapes);
         $this->mockExplodes($explodes);
+        $this->mockGroupLookups($groups);
+    }
+
+    protected function mockGroupLookups(int $times = 1): void
+    {
+        $this->mockLdap->shouldReceive('read')->times($times)->andReturn(['count' => 0]);
+        $this->mockLdap->shouldReceive('getEntries')->times($times)->andReturn(['count' => 0]);
     }
 
     public function test_login()
@@ -307,8 +314,8 @@ public function test_login_maps_roles_and_retains_existing_roles()
             'services.ldap.remove_from_groups' => false,
         ]);
 
-        $this->commonLdapMocks(1, 1, 4, 5, 4, 6);
-        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4)
+        $this->commonLdapMocks(1, 1, 4, 5, 2, 2, 2);
+        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
                 'uid'      => [$this->mockUser->name],
@@ -352,8 +359,8 @@ public function test_login_maps_roles_and_removes_old_roles_if_set()
             'services.ldap.remove_from_groups' => true,
         ]);
 
-        $this->commonLdapMocks(1, 1, 3, 4, 3, 2);
-        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(3)
+        $this->commonLdapMocks(1, 1, 3, 4, 2, 1, 1);
+        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
                 'uid'      => [$this->mockUser->name],
@@ -394,22 +401,26 @@ public function test_dump_user_groups_shows_group_related_details_as_json()
             'dn'       => 'dc=test,' . config('services.ldap.base_dn'),
             'mail'     => [$this->mockUser->email],
         ]];
-        $this->commonLdapMocks(1, 1, 4, 5, 4, 2);
-        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4)
+        $this->commonLdapMocks(1, 1, 4, 5, 2, 2, 0);
+        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn($userResp, ['count' => 1,
-                0                           => [
-                    'dn'       => 'dc=test,' . config('services.ldap.base_dn'),
+                0 => [
+                    'dn' => 'dc=test,' . config('services.ldap.base_dn'),
                     'memberof' => [
                         'count' => 1,
-                        0       => 'cn=ldaptester,ou=groups,dc=example,dc=com',
+                        0 => 'cn=ldaptester,ou=groups,dc=example,dc=com',
                     ],
                 ],
-            ], [
+            ]);
+
+        $this->mockLdap->shouldReceive('read')->times(2);
+        $this->mockLdap->shouldReceive('getEntries')->times(2)
+            ->andReturn([
                 'count' => 1,
-                0       => [
-                    'dn'       => 'cn=ldaptester,ou=groups,dc=example,dc=com',
-                    'memberof' => [
+                0 => [
+                    'dn'        => 'cn=ldaptester,ou=groups,dc=example,dc=com',
+                    'memberof'  => [
                         'count' => 1,
                         0       => 'cn=monsters,ou=groups,dc=example,dc=com',
                     ],
@@ -426,9 +437,13 @@ public function test_dump_user_groups_shows_group_related_details_as_json()
                 ],
             ],
             'parsed_direct_user_groups' => [
-                'ldaptester',
+                'cn=ldaptester,ou=groups,dc=example,dc=com',
             ],
             'parsed_recursive_user_groups' => [
+                'cn=ldaptester,ou=groups,dc=example,dc=com',
+                'cn=monsters,ou=groups,dc=example,dc=com',
+            ],
+            'parsed_resulting_group_names' => [
                 'ldaptester',
                 'monsters',
             ],
@@ -458,15 +473,18 @@ public function test_recursive_group_search_queries_via_full_dn()
                       ],
         ];
 
-        $this->commonLdapMocks(1, 1, 3, 4, 3, 1);
+        $this->commonLdapMocks(1, 1, 3, 4, 2, 1);
 
         $escapedName = ldap_escape($this->mockUser->name);
         $this->mockLdap->shouldReceive('searchAndGetEntries')->twice()
             ->with($this->resourceId, config('services.ldap.base_dn'), "(&(uid={$escapedName}))", \Mockery::type('array'))
             ->andReturn($userResp, $groupResp);
 
-        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
-            ->with($this->resourceId, config('services.ldap.base_dn'), $groupResp[0]['dn'], ['memberof'])
+        $this->mockLdap->shouldReceive('read')->times(1)
+            ->with($this->resourceId, 'cn=ldaptester,ou=groups,dc=example,dc=com', '(objectClass=*)', ['memberof'])
+            ->andReturn(['count' => 0]);
+        $this->mockLdap->shouldReceive('getEntries')->times(1)
+            ->with($this->resourceId, ['count' => 0])
             ->andReturn(['count' => 0]);
 
         $resp = $this->mockUserLogin();
@@ -491,8 +509,8 @@ public function test_login_maps_roles_using_external_auth_ids_if_set()
             'services.ldap.remove_from_groups' => true,
         ]);
 
-        $this->commonLdapMocks(1, 1, 3, 4, 3, 2);
-        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(3)
+        $this->commonLdapMocks(1, 1, 3, 4, 2, 1, 1);
+        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
                 'uid'      => [$this->mockUser->name],
@@ -532,8 +550,8 @@ public function test_login_group_mapping_does_not_conflict_with_default_role()
             'services.ldap.remove_from_groups' => true,
         ]);
 
-        $this->commonLdapMocks(1, 1, 4, 5, 4, 6);
-        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4)
+        $this->commonLdapMocks(1, 1, 4, 5, 2, 2, 2);
+        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)
             ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
             ->andReturn(['count' => 1, 0 => [
                 'uid'      => [$this->mockUser->name],
@@ -772,9 +790,9 @@ public function test_login_with_email_confirmation_required_maps_groups_but_show
             'services.ldap.remove_from_groups' => true,
         ]);
 
-        $this->commonLdapMocks(1, 1, 6, 8, 6, 4);
+        $this->commonLdapMocks(1, 1, 6, 8, 4, 2, 2);
         $this->mockLdap->shouldReceive('searchAndGetEntries')
-            ->times(6)
+            ->times(4)
             ->andReturn(['count' => 1, 0 => [
                 'uid'      => [$user->name],
                 'cn'       => [$user->name],