10000 bug #49187 [Ldap] Allow multiple values on `extra_fields` (mvhirsch) · symfony/symfony@69f113a · GitHub
[go: up one dir, main page]
Skip to content

Commit 69f113a

Browse files
nicolas-grekasnicolas-grekas
committed
bug #49187 [Ldap] Allow multiple values on extra_fields (mvhirsch)
This PR was merged into the 5.4 branch. Discussion ---------- [Ldap] Allow multiple values on `extra_fields` | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | ~ | License | MIT | Doc PR | Loading users with the `LdapUserProvider`, using `extra_fields` fails if I want to get `memberOf` attribute. This happens, if my user has multiple attributes set. After taking a look at `Ldap\Entry` (which supports multiple values per attribute). Looking at `LdapUserProvider::getAttributeValue()` it was very clear, that every attribute must be unique. The method `getAttributeValue()` has originated from `getPassword` dbf45e4, forcing the password attribute to be unique. Same goes for `uidKey` c91689b. Loading `extra_fields` should allow for multiple values per attribute (like `memberOf`). Did I break backward compatibility? No, I didn't. Simply because it was never usable as array while loading users via `LdapUserProvider`, instead an `InvalidArgumentException` was thrown. <!-- Replace this notice by a short README for your feature/bugfix. This will help reviewers and should be a good start for the documentation. Additionally (see https://symfony.com/releases): - Always add tests and ensure they pass. - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the latest branch. - For new features, provide some code snippets to help understand usage. - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry - Never break backward compatibility (see https://symfony.com/bc). --> Commits ------- e51b502 fixes retrieving multiple values for extra fields
2 parents 393c603 + e51b502 commit 69f113a

File tree

2 files changed

+49
-0
lines changed

2 files changed

+49
-0
lines changed

‎src/Symfony/Component/Ldap/Security/LdapUserProvider.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,9 @@ private function getAttributeValue(Entry $entry, string $attribute)
187187
}
188188

189189
$values = $entry->getAttribute($attribute);
190+
if (!\in_array($attribute, [$this->uidKey, $this->passwordAttribute])) {
191+
return $values;
192+
}
190193

191194
if (1 !== \count($values)) {
192195
throw new InvalidArgumentException(sprintf('Attribute "%s" has multiple values.', $attribute));

‎src/Symfony/Component/Ldap/Tests/Security/LdapUserProviderTest.php

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,52 @@ public function testLoadUserByUsernameIsSuccessfulWithPasswordAttribute()
333333
$this->assertInstanceOf(LdapUser::class, $provider->loadUserByIdentifier('foo'));
334334
}
335335

336+
public function testLoadUserByIdentifierIsSuccessfulWithMultipleExtraAttributes()
337+
{
338+
$result = $this->createMock(CollectionInterface::class);
339+
$query = $this->createMock(QueryInterface::class);
340+
$query
341+
->expects($this->once())
342+
->method('execute')
343+
->willReturn($result)
344+
;
345+
$ldap = $this->createMock(LdapInterface::class);
346+
$memberOf = [
347+
'cn=foo,ou=MyBusiness,dc=symfony,dc=com',
348+
'cn=bar,ou=MyBusiness,dc=symfony,dc=com',
349+
];
350+
$result
351+
->expects($this->once())
352+
->method('offsetGet')
353+
->with(0)
354+
->willReturn(new Entry('foo', [
355+
'sAMAccountName' => ['foo'],
356+
'userpassword' => ['bar'],
357+
'memberOf' => $memberOf,
358+
]))
359+
;
360+
$result
361+
->expects($this->once())
362+
->method('count')
363+
->willReturn(1)
364+
;
365+
$ldap
366+
->expects($this->once())
367+
->method('escape')
368+
->willReturn('foo')
369+
;
370+
$ldap
371+
->expects($this->once())
372+
->method('query')
373+
->willReturn($query)
374+
;
375+
376+
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com', null, null, [], 'sAMAccountName', '({uid_key}={user_identifier})', 'userpassword', ['memberOf']);
377+
$user = $provider->loadUserByIdentifier('foo');
378+
$this->assertInstanceOf(LdapUser::class, $user);
379+
$this->assertSame(['memberOf' => $memberOf], $user->getExtraFields());
380+
}
381+
336382
public function testRefreshUserShouldReturnUserWithSameProperties()
337383
{
338384
$ldap = $this->createMock(LdapInterface::class);

0 commit comments

Comments
 (0)
0