LDAP: Updated default user filter placeholder format

To not conflict with env variables, and to align with placeholders used
for PDF gen command.
Added test to cover, including old format supported for
back-compatibility.
For #4967
This commit is contained in:
Dan Brown 2024-04-28 12:29:57 +01:00
parent e1149a27e9
commit 6b681961e5
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
4 changed files with 42 additions and 5 deletions

View File

@ -215,7 +215,7 @@ LDAP_SERVER=false
LDAP_BASE_DN=false
LDAP_DN=false
LDAP_PASS=false
LDAP_USER_FILTER=false
LDAP_USER_FILTER="(&(uid={user}))"
LDAP_VERSION=false
LDAP_START_TLS=false
LDAP_TLS_INSECURE=false

View File

@ -249,13 +249,18 @@ class LdapService
/**
* Build a filter string by injecting common variables.
* Both "${var}" and "{var}" style placeholders are supported.
* Dollar based are old format but supported for compatibility.
*/
protected function buildFilter(string $filterString, array $attrs): string
{
$newAttrs = [];
foreach ($attrs as $key => $attrText) {
$newKey = '${' . $key . '}';
$newAttrs[$newKey] = $this->ldap->escape($attrText);
$escapedText = $this->ldap->escape($attrText);
$oldVarKey = '${' . $key . '}';
$newVarKey = '{' . $key . '}';
$newAttrs[$oldVarKey] = $escapedText;
$newAttrs[$newVarKey] = $escapedText;
}
return strtr($filterString, $newAttrs);

View File

@ -123,7 +123,7 @@ return [
'dn' => env('LDAP_DN', false),
'pass' => env('LDAP_PASS', false),
'base_dn' => env('LDAP_BASE_DN', false),
'user_filter' => env('LDAP_USER_FILTER', '(&(uid=${user}))'),
'user_filter' => env('LDAP_USER_FILTER', '(&(uid={user}))'),
'version' => env('LDAP_VERSION', false),
'id_attribute' => env('LDAP_ID_ATTRIBUTE', 'uid'),
'email_attribute' => env('LDAP_EMAIL_ATTRIBUTE', 'mail'),

View File

@ -32,7 +32,7 @@ class LdapTest extends TestCase
'services.ldap.id_attribute' => 'uid',
'services.ldap.user_to_groups' => false,
'services.ldap.version' => '3',
'services.ldap.user_filter' => '(&(uid=${user}))',
'services.ldap.user_filter' => '(&(uid={user}))',
'services.ldap.follow_referrals' => false,
'services.ldap.tls_insecure' => false,
'services.ldap.thumbnail_attribute' => null,
@ -178,6 +178,38 @@ class LdapTest extends TestCase
$this->assertDatabaseHas('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => 'cooluser456']);
}
public function test_user_filter_default_placeholder_format()
{
config()->set('services.ldap.user_filter', '(&(uid={user}))');
$this->mockUser->name = 'barryldapuser';
$expectedFilter = '(&(uid=\62\61\72\72\79\6c\64\61\70\75\73\65\72))';
$this->commonLdapMocks(1, 1, 1, 1, 1);
$this->mockLdap->shouldReceive('searchAndGetEntries')
->once()
->with($this->resourceId, config('services.ldap.base_dn'), $expectedFilter, \Mockery::type('array'))
->andReturn(['count' => 0, 0 => []]);
$resp = $this->mockUserLogin();
$resp->assertRedirect('/login');
}
public function test_user_filter_old_placeholder_format()
{
config()->set('services.ldap.user_filter', '(&(username=${user}))');
$this->mockUser->name = 'barryldapuser';
$expectedFilter = '(&(username=\62\61\72\72\79\6c\64\61\70\75\73\65\72))';
$this->commonLdapMocks(1, 1, 1, 1, 1);
$this->mockLdap->shouldReceive('searchAndGetEntries')
->once()
->with($this->resourceId, config('services.ldap.base_dn'), $expectedFilter, \Mockery::type('array'))
->andReturn(['count' => 0, 0 => []]);
$resp = $this->mockUserLogin();
$resp->assertRedirect('/login');
}
public function test_initial_incorrect_credentials()
{
$this->commonLdapMocks(1, 1, 1, 0, 1);