Skip to content

Commit 4554527

Browse files
Handle invalid project LDAP filters (#3142)
#2985 failed to consider projects which set an invalid LDAP filter, leading all LDAP syncing to fail if any project has a bad LDAP filter. This PR handles the error and prints a warning.
1 parent 41793af commit 4554527

2 files changed

Lines changed: 28 additions & 2 deletions

File tree

app/Utils/LdapUtils.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use App\Models\User;
99
use Exception;
1010
use Illuminate\Support\Facades\Log;
11+
use LdapRecord\LdapRecordException;
1112

1213
class LdapUtils
1314
{
@@ -47,8 +48,14 @@ public static function syncUser(User $user): void
4748
continue;
4849
}
4950

50-
$matches_ldap_filter = $user->ldapguid !== null && $ldap_provider::rawFilter($project->ldapfilter)->findByGuid($user->ldapguid) !== null;
51-
$relationship_already_exists = $project->users->contains($user);
51+
try {
52+
$matches_ldap_filter = $user->ldapguid !== null && $ldap_provider::rawFilter($project->ldapfilter)->findByGuid($user->ldapguid) !== null;
53+
$relationship_already_exists = $project->users->contains($user);
54+
} catch (LdapRecordException) {
55+
// Prevent invalid filters from breaking other projects.
56+
Log::warning("Invalid LDAP filter '$project->ldapfilter' for project $project->name.");
57+
continue;
58+
}
5259

5360
if ($matches_ldap_filter && !$relationship_already_exists) {
5461
$project->users()->attach($user->id, ['role' => Project::PROJECT_USER]);

tests/Feature/LdapIntegration.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33
namespace Tests\Feature;
44

55
use App\Models\Project;
6+
use Illuminate\Support\Facades\Config;
67
use Illuminate\Support\Facades\Log;
78
use Illuminate\Support\Str;
89
use LdapRecord\LdapRecordException;
910
use LdapRecord\Models\ModelDoesNotExistException;
1011
use LdapRecord\Models\OpenLDAP\Group;
1112
use LdapRecord\Models\OpenLDAP\User;
13+
use Mockery;
1214
use Mockery\Exception\InvalidCountException;
1315
use Tests\TestCase;
1416
use Tests\Traits\CreatesProjects;
@@ -416,4 +418,21 @@ public function testSyncsGroupsUponLogin(): void
416418
$this->assertCanAccessProject('group_1_only_1', 'only_group_1');
417419
$this->assertCanAccessProject('group_1_only_1', 'only_group_2');
418420
}
421+
422+
public function testSyncCommandHandlesInvalidProjectFilters(): void
423+
{
424+
// We are testing that nothing fails...
425+
self::expectNotToPerformAssertions();
426+
427+
Log::shouldReceive('warning')
428+
->with("Invalid LDAP filter 'brokenldapfilter))' for project " . $this->projects['only_group_1']->name . '.');
429+
430+
// Allow other debug messages to be logged without causing a test failure.
431+
Log::shouldReceive('debug')->with(Mockery::any());
432+
433+
$this->projects['only_group_1']->ldapfilter = 'brokenldapfilter))';
434+
$this->projects['only_group_1']->save();
435+
436+
$this->artisan('ldap:sync_projects');
437+
}
419438
}

0 commit comments

Comments
 (0)