Use filter-based approach for LDAP project membership (#2384)

In response to user feedback about the new LDAP-based project membership
feature added in #2282, this PR
changes the project LDAP field to accept a filter instead of a group
name for consistency with our existing `LDAP_FILTERS_ON` environment
variable.

This PR also addresses an issue with `LDAP_FILTERS_ON` related to the
switch from Adldap2 in #2206 and reverts the environment variable rename
from `LDAP_HOSTS` to `LDAP_HOST` to maintain backwards compatibility
with existing systems.

app/Ldap/Rules/FilterRules.php

@@ -20,8 +20,14 @@ public function passes(LdapRecord $user, Eloquent $model = null): bool
             return true;
         }
 
-        return $user->groups()
-            ->recursive()
-            ->exists(\LdapRecord\Models\Entry::find($filter));
+        if (!($model instanceof \App\Models\User)) {
+            return false;
+        }
+
+        if (env('LDAP_PROVIDER', 'openldap') === 'activedirectory') {
+            return \LdapRecord\Models\ActiveDirectory\User::rawFilter($filter)->findByGuid($model->ldapguid) instanceof \LdapRecord\Models\ActiveDirectory\User;
+        } else {
+            return  \LdapRecord\Models\OpenLDAP\User::rawFilter($filter)->findByGuid($model->ldapguid) instanceof \LdapRecord\Models\OpenLDAP\User;
+        }
     }
 }

app/Utils/LdapUtils.php

@@ -12,24 +12,20 @@ class LdapUtils
 {
     public static function syncUser(User $user): void
     {
-        if ($user->ldapguid === null) {
-            $ldap_user = null;
-        } elseif (env('LDAP_PROVIDER', 'openldap') === 'activedirectory') {
-            $ldap_user = \LdapRecord\Models\ActiveDirectory\User::findByGuid($user->ldapguid);
-        } else {
-            $ldap_user = \LdapRecord\Models\OpenLDAP\User::findByGuid($user->ldapguid);
-        }
-
         $projects = Project::with('users')->get();
 
         foreach ($projects as $project) {
             if ($project->ldapfilter === null) {
                 continue;
             }
 
-            $matches_ldap_filter = $ldap_user !== null && $ldap_user->groups()
-                ->recursive()
-                ->exists(\LdapRecord\Models\Entry::find($project->ldapfilter));
+            if ($user->ldapguid === null) {
+                $matches_ldap_filter  = false;
+            } elseif (env('LDAP_PROVIDER', 'openldap') === 'activedirectory') {
+                $matches_ldap_filter = \LdapRecord\Models\ActiveDirectory\User::rawFilter($project->ldapfilter)->findByGuid($user->ldapguid) instanceof \LdapRecord\Models\ActiveDirectory\User;
+            } else {
+                $matches_ldap_filter = \LdapRecord\Models\OpenLDAP\User::rawFilter($project->ldapfilter)->findByGuid($user->ldapguid) instanceof \LdapRecord\Models\OpenLDAP\User;
+            }
 
             $relationship_already_exists = $project->users->contains($user);
 

config/ldap.php

@@ -29,7 +29,7 @@
     'connections' => [
 
         'default' => [
-            'hosts' => [env('LDAP_HOST', '127.0.0.1')],
+            'hosts' => [env('LDAP_HOSTS', '127.0.0.1')],
             'username' => env('LDAP_USERNAME', ''),
             'password' => env('LDAP_PASSWORD', ''),
             'port' => env('LDAP_PORT', 389),

docs/authentication.md

@@ -32,7 +32,7 @@ LDAP_USERNAME=cn=admin,dc=example,dc=org
 LDAP_PASSWORD=password
 CDASH_AUTHENTICATION_PROVIDER=ldap
 LDAP_PROVIDER=openldap
-LDAP_HOST=ldap
+LDAP_HOSTS=ldap
 LDAP_BASE_DN="dc=example,dc=org"
 LDAP_LOGGING=true
 LDAP_LOCATE_USERS_BY=mail
@@ -45,7 +45,7 @@ Here's a description of the `.env` variables involved in the LDAP authentication
 | LDAP_BASE_DN | The base distinguished name you'd like to perform query operations on. | dc=local,dc=com |
 | LDAP_BIND_USERS_BY | The LDAP users attribute used for authentication | distinguishedname |
 | LDAP_FILTERS_ON | Additional LDAP query filters to restrict authorized user list. For example, to restrict users to a specific Active Directory group: `cn=myRescrictedGroup,dc=example,dc=com` | false |
-| LDAP_HOST | The IP address or host name of your LDAP server. | 127.0.0.1 |
+| LDAP_HOSTS | The IP address or host name of your LDAP server. | 127.0.0.1 |
 | LDAP_LOCATE_USERS_BY | The LDAP users attribute used to locate your users. | mail |
 | LDAP_LOGGING | Whether or not to log LDAP activities. Useful for debugging. | true |
 | LDAP_USERNAME | Username for account that can query and run operations on your LDAP server(s). | '' |

phpstan-baseline.neon

@@ -2779,11 +2779,6 @@ parameters:
 			count: 1
 			path: app/Jobs/ProcessSubmission.php
 
-		-
-			message: "#^Call to an undefined method LdapRecord\\\\Models\\\\Model\\:\\:groups\\(\\)\\.$#"
-			count: 1
-			path: app/Ldap/Rules/FilterRules.php
-
 		-
 			message: "#^Method App\\\\Listeners\\\\ConfiguredSendEmailVerificationNotification\\:\\:handle\\(\\) has no return type specified\\.$#"
 			count: 1
@@ -3074,11 +3069,6 @@ parameters:
 			count: 1
 			path: app/Utils/AuthTokenUtil.php
 
-		-
-			message: "#^Call to an undefined method LdapRecord\\\\Models\\\\Model\\:\\:groups\\(\\)\\.$#"
-			count: 1
-			path: app/Utils/LdapUtils.php
-
 		-
 			message: """
 				#^Call to deprecated function pdo_real_escape_string\\(\\)\\:

resources/js/components/EditProject.vue

@@ -424,7 +424,7 @@
                   <td />
                   <td>
                     <div align="right">
-                      <strong>LDAP Group:</strong>
+                      <strong>LDAP Filter:</strong>
                     </div>
                   </td>
                   <td>

tests/Feature/LdapIntegration.php

@@ -17,9 +17,6 @@ class LdapIntegration extends TestCase
 {
     use CreatesProjects;
 
-    protected Group $group_1;
-    protected Group $group_2;
-
     /**
      * @var array<string,User>
      */
@@ -40,90 +37,71 @@ protected function setUp(): void
         putenv('LDAP_PASSWORD=password');
         putenv('CDASH_AUTHENTICATION_PROVIDER=ldap');
         putenv('LDAP_PROVIDER=openldap');
-        putenv('LDAP_HOST=ldap');
+        putenv('LDAP_HOSTS=ldap');
         putenv('LDAP_BASE_DN="dc=example,dc=org"');
         putenv('LDAP_LOGGING=true');
         putenv('LDAP_LOCATE_USERS_BY=uid');
 
         parent::setUp();
 
-        // Create two groups
-        $this->group_1 = Group::create([
-            'cn' => 'cdash_users_1_' . Str::uuid()->toString(),
-            'uniqueMember' => 'ou=users,dc=example,dc=org',
-        ]);
-
-        $this->group_2 = Group::create([
-            'cn' => 'cdash_users_2_' . Str::uuid()->toString(),
-            'uniqueMember' => 'ou=users,dc=example,dc=org',
-        ]);
-
         // Create a pair of users which are only in group 1
         $this->users['group_1_only_1'] = User::create([
-            'uid' => Str::uuid()->toString(),
+            'uid' => 'group_1_' . Str::uuid()->toString(),
             'cn' => Str::uuid()->toString(),
             'userpassword' => Str::uuid()->toString(),
             'objectclass' => 'inetOrgPerson',
             'sn' => Str::uuid()->toString(),
         ]);
-        $this->group_1->members()->attach($this->users['group_1_only_1']);
 
         $this->users['group_1_only_2'] = User::create([
-            'uid' => Str::uuid()->toString(),
+            'uid' => 'group_1_' . Str::uuid()->toString(),
             'cn' => Str::uuid()->toString(),
             'userpassword' => Str::uuid()->toString(),
             'objectclass' => 'inetOrgPerson',
             'sn' => Str::uuid()->toString(),
         ]);
-        $this->group_1->members()->attach($this->users['group_1_only_2']);
 
         // Create a pair of users which are only in group 2
         $this->users['group_2_only_1'] = User::create([
-            'uid' => Str::uuid()->toString(),
+            'uid' => 'group_2_' . Str::uuid()->toString(),
             'cn' => Str::uuid()->toString(),
             'userpassword' => Str::uuid()->toString(),
             'objectclass' => 'inetOrgPerson',
             'sn' => Str::uuid()->toString(),
         ]);
-        $this->group_2->members()->attach($this->users['group_2_only_1']);
 
         $this->users['group_2_only_2'] = User::create([
-            'uid' => Str::uuid()->toString(),
+            'uid' => 'group_2_' . Str::uuid()->toString(),
             'cn' => Str::uuid()->toString(),
             'userpassword' => Str::uuid()->toString(),
             'objectclass' => 'inetOrgPerson',
             'sn' => Str::uuid()->toString(),
         ]);
-        $this->group_2->members()->attach($this->users['group_2_only_2']);
 
         // Create a pair of users which are in both groups
         $this->users['groups_1_and_2_1'] = User::create([
-            'uid' => Str::uuid()->toString(),
+            'uid' => 'group_1_group_2_' . Str::uuid()->toString(),
             'cn' => Str::uuid()->toString(),
             'userpassword' => Str::uuid()->toString(),
             'objectclass' => 'inetOrgPerson',
             'sn' => Str::uuid()->toString(),
         ]);
-        $this->group_1->members()->attach($this->users['groups_1_and_2_1']);
-        $this->group_2->members()->attach($this->users['groups_1_and_2_1']);
 
         $this->users['groups_1_and_2_2'] = User::create([
-            'uid' => Str::uuid()->toString(),
+            'uid' => 'group_1_group_2_' . Str::uuid()->toString(),
             'cn' => Str::uuid()->toString(),
             'userpassword' => Str::uuid()->toString(),
             'objectclass' => 'inetOrgPerson',
             'sn' => Str::uuid()->toString(),
         ]);
-        $this->group_1->members()->attach($this->users['groups_1_and_2_2']);
-        $this->group_2->members()->attach($this->users['groups_1_and_2_2']);
 
         // Create a pair of projects which are restricted to specific LDAP groups
         $this->projects['only_group_1'] = Project::findOrFail((int) $this->makePrivateProject()->Id);
-        $this->projects['only_group_1']->ldapfilter = "cn={$this->group_1->getAttribute('cn')[0]},dc=example,dc=org";
+        $this->projects['only_group_1']->ldapfilter = "(uid=*group_1*)";
         $this->projects['only_group_1']->save();
 
         $this->projects['only_group_2'] = Project::findOrFail((int) $this->makePrivateProject()->Id);
-        $this->projects['only_group_2']->ldapfilter = "cn={$this->group_2->getAttribute('cn')[0]},dc=example,dc=org";
+        $this->projects['only_group_2']->ldapfilter = "(uid=*group_2*)";
         $this->projects['only_group_2']->save();
     }
 
@@ -139,14 +117,11 @@ protected function tearDown(): void
         putenv('LDAP_PASSWORD');
         putenv('CDASH_AUTHENTICATION_PROVIDER');
         putenv('LDAP_PROVIDER');
-        putenv('LDAP_HOST');
+        putenv('LDAP_HOSTS');
         putenv('LDAP_BASE_DN');
         putenv('LDAP_LOGGING');
         putenv('LDAP_LOCATE_USERS_BY');
 
-        $this->group_1->delete(true);
-        $this->group_2->delete(true);
-
         foreach ($this->users as $key => $user) {
             $user->delete(true);
         }
@@ -214,7 +189,7 @@ public function testLoginWithFilters(): void
         \App\Models\User::where(['email' => $user_all_groups->getAttribute('uid')[0]])->firstOrFail()->delete();
 
         // Restrict login to members of group 1 only
-        putenv("LDAP_FILTERS_ON=cn={$this->group_1->getAttribute('cn')[0]},dc=example,dc=org");
+        putenv("LDAP_FILTERS_ON=(uid=*group_1*)");
         $this->post('/login', [
             'email' => $user_group_1->getAttribute('uid')[0],
             'password' => $user_group_1->getAttribute('userpassword')[0],
@@ -236,6 +211,31 @@ public function testLoginWithFilters(): void
         self::assertCount(0, User::where(['email' => $user_group_2->getAttribute('uid')[0]])->get());
         \App\Models\User::where(['email' => $user_all_groups->getAttribute('uid')[0]])->firstOrFail()->delete();
 
+        // Restrict login to both groups
+        putenv("LDAP_FILTERS_ON=(|(uid=*group_1*)(uid=*group_2*))");
+
+        $this->post('/login', [
+            'email' => $user_group_1->getAttribute('uid')[0],
+            'password' => $user_group_1->getAttribute('userpassword')[0],
+        ])->assertRedirect('/');
+        $this->get('/logout')->assertRedirect('/');
+
+        $this->post('/login', [
+            'email' => $user_group_2->getAttribute('uid')[0],
+            'password' => $user_group_2->getAttribute('userpassword')[0],
+        ])->assertRedirect('/');
+        $this->get('/logout')->assertRedirect('/');
+
+        $this->post('/login', [
+            'email' => $user_all_groups->getAttribute('uid')[0],
+            'password' => $user_all_groups->getAttribute('userpassword')[0],
+        ])->assertRedirect('/');
+        $this->get('/logout')->assertRedirect('/');
+
+        \App\Models\User::where(['email' => $user_group_1->getAttribute('uid')[0]])->firstOrFail()->delete();
+        \App\Models\User::where(['email' => $user_group_2->getAttribute('uid')[0]])->firstOrFail()->delete();
+        \App\Models\User::where(['email' => $user_all_groups->getAttribute('uid')[0]])->firstOrFail()->delete();
+
         // "Delete" the env variable
         putenv('LDAP_FILTERS_ON');
     }
@@ -254,13 +254,13 @@ public function testArtisanSyncsProjectMembership(): void
         self::assertNotContains($user->email, $this->projects['only_group_2']->users()->pluck('email'));
 
         // Use a project which didn't have this user as a member when the initial login occurred
-        $this->projects['only_group_2']->ldapfilter = "cn={$this->group_1->getAttribute('cn')[0]},dc=example,dc=org";
+        $this->projects['only_group_2']->ldapfilter = "(uid=*group_1*)";
         $this->projects['only_group_2']->save();
         $this->artisan('ldap:sync_projects');
         self::assertContains($user->email, $this->projects['only_group_2']->users()->pluck('email'));
 
         // Change the group, and verify that the user was removed from the project
-        $this->projects['only_group_2']->ldapfilter = "cn={$this->group_2->getAttribute('cn')[0]},dc=example,dc=org";
+        $this->projects['only_group_2']->ldapfilter = "(uid=*group_2*)";
         $this->projects['only_group_2']->save();
         $this->artisan('ldap:sync_projects');
         self::assertNotContains($user->email, $this->projects['only_group_2']->users()->pluck('email'));
@@ -342,7 +342,7 @@ public function testGroupBasedProjectMembership(): void
         $this->assertCanAccessProject('groups_1_and_2_1', 'only_group_2');
 
         // Make sure the membership is removed when the LDAP group rule is changed
-        $this->projects['only_group_2']->ldapfilter = "cn={$this->group_1->getAttribute('cn')[0]},dc=example,dc=org";
+        $this->projects['only_group_2']->ldapfilter = "(uid=*group_1*)";
         $this->projects['only_group_2']->save();
 
         $this->artisan('ldap:sync_projects');
@@ -368,7 +368,7 @@ public function testSyncsGroupsUponLogin(): void
         // Basic sanity check...
         $this->assertCannotAccessProject('group_1_only_1', 'only_group_2');
 
-        $this->projects['only_group_2']->ldapfilter = "cn={$this->group_1->getAttribute('cn')[0]},dc=example,dc=org";
+        $this->projects['only_group_2']->ldapfilter = "(uid=*group_1*)";
         $this->projects['only_group_2']->save();
 
         // Still can't access the project because the link hasn't been created yet