Skip to content

Commit 726db70

Browse files
l-vol-vo
committed
Allow to use ldap in a chain provider
1 parent 532dcda commit 726db70

File tree

8 files changed

+392
-23
lines changed

8 files changed

+392
-23
lines changed

‎src/Symfony/Component/Ldap/Attribute/WithLdapPassword.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
3+
namespace Symfony\Component\Ldap\Attribute;
4+
5+
#[\Attribute(\Attribute::TARGET_CLASS)]
6+
class WithLdapPassword
7+
{
8+
public function __construct(public readonly bool $enabled = true) {}
9+
}

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@
1313

1414
use Psr\Container\ContainerInterface;
1515
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
16+
use Symfony\Component\Ldap\Attribute\WithLdapPassword;
1617
use Symfony\Component\Ldap\Exception\InvalidCredentialsException;
1718
use Symfony\Component\Ldap\Exception\InvalidSearchCredentialsException;
1819
use Symfony\Component\Ldap\LdapInterface;
1920
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
2021
use Symfony\Component\Security\Core\Exception\LogicException;
22+
use Symfony\Component\Security\Core\User\UserInterface;
2123
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
2224
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
2325

@@ -48,9 +50,6 @@ public function onCheckPassport(CheckPassportEvent $event)
4850

4951
/** @var LdapBadge $ldapBadge */
5052
$ldapBadge = $passport->getBadge(LdapBadge::class);
51-
if ($ldapBadge->isResolved()) {
52-
return;
53-
}
5453

5554
if (!$passport->hasBadge(PasswordCredentials::class)) {
5655
throw new \LogicException(sprintf('LDAP authentication requires a passport containing password credentials, authenticator "%s" does not fulfill these requirements.', $event->getAuthenticator()::class));
@@ -72,6 +71,16 @@ public function onCheckPassport(CheckPassportEvent $event)
7271
}
7372

7473
$user = $passport->getUser();
74+
$nonLdapUserWithoutLdapPasswordAttribute = false;
75+
if (!$user instanceof LdapUser) {
76+
$reflectionClass = new \ReflectionClass($user);
77+
$attr = $reflectionClass->getAttributes(WithLdapPassword::class);
78+
79+
$nonLdapUserWithoutLdapPasswordAttribute = count($attr) === 0;
80+
if (!$nonLdapUserWithoutLdapPasswordAttribute && !$attr[0]->newInstance()->enabled) {
81+
return;
82+
}
83+
}
7584

7685
/** @var LdapInterface $ldap */
7786
$ldap = $this->ldapLocator->get($ldapBadge->getLdapServiceId());
@@ -104,8 +113,11 @@ public function onCheckPassport(CheckPassportEvent $event)
104113
throw new BadCredentialsException('The presented password is invalid.');
105114
}
106115

116+
if ($nonLdapUserWithoutLdapPasswordAttribute) {
117+
trigger_deprecation('symfony/ldap', '6.4', 'Authenticate a user that is not an instance of %s is deprecated and won\'t be the default behavior anymore in 7.0. Use the %s attribute to keep this behavior.', LdapUser::class, WithLdapPassword::class);
118+
}
119+
107120
$passwordCredentials->markResolved();
108-
$ldapBadge->markResolved();
109121
}
110122

111123
public static function getSubscribedEvents(): array

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public function supports(Request $request): ?bool
6060
public function authenticate(Request $request): Passport
6161
{
6262
$passport = $this->authenticator->authenticate($request);
63-
$passport->addBadge(new LdapBadge($this->ldapServiceId, $this->dnString, $this->searchDn, $this->searchPassword, $this->queryString));
63+
$passport->addBadge(new LdapBadge($this->ldapServiceId, $this->dnString, $this->searchDn, $this->searchPassword, $this->queryString, true));
6464

6565
return $passport;
6666
}

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,14 @@
2424
*/
2525
class LdapBadge implements BadgeInterface
2626
{
27-
private bool $resolved = false;
27+
private bool $resolved = true;
2828
private string $ldapServiceId;
2929
private string $dnString;
3030
private string $searchDn;
3131
private string $searchPassword;
3232
private ?string $queryString;
3333

34-
public function __construct(string $ldapServiceId, string $dnString = '{user_identifier}', string $searchDn = '', string $searchPassword = '', string $queryString = null)
34+
public function __construct(string $ldapServiceId, string $dnString = '{user_identifier}', string $searchDn = '', string $searchPassword = '', string $queryString = null, bool $resolved = false)
3535
{
3636
$this->ldapServiceId = $ldapServiceId;
3737
$dnString = str_replace('{username}', '{user_identifier}', $dnString, $replaceCount);
@@ -46,6 +46,10 @@ public function __construct(string $ldapServiceId, string $dnString = '{user_ide
4646
trigger_deprecation('symfony/ldap', '6.2', 'Using "{username}" parameter in LDAP configuration is deprecated, consider using "{user_identifier}" instead.');
4747
}
4848
$this->queryString = $queryString;
49+
$this->resolved = $resolved;
50+
if (false === $this->resolved) {
51+
trigger_deprecation('symfony/ldap', '6.4', 'Passing "false" as resolved initial value is deprecated, use "true" instead.');
52+
}
4953
}
5054

5155
public function getLdapServiceId(): string
@@ -75,6 +79,8 @@ public function getQueryString(): ?string
7579

7680
public function markResolved(): void
7781
{
82+
trigger_deprecation('symfony/ldap', '6.4', '%s is deprecated and will be removed in 7.0. %s is intended to bear LDAP information and doesn\'t need to be resolved anymore.', __METHOD__, __CLASS__);
83+
7884
$this->resolved = true;
7985
}
8086

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

Lines changed: 78 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,23 @@
1414
use PHPUnit\Framework\MockObject\MockObject;
1515
use PHPUnit\Framework\TestCase;
1616
use Psr\Container\ContainerInterface;
17+
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
1718
use Symfony\Component\HttpFoundation\Request;
1819
use Symfony\Component\HttpFoundation\Response;
1920
use Symfony\Component\Ldap\Adapter\CollectionInterface;
2021
use Symfony\Component\Ldap\Adapter\QueryInterface;
22+
use Symfony\Component\Ldap\Attribute\WithLdapPassword;
2123
use Symfony\Component\Ldap\Entry;
2224
use Symfony\Component\Ldap\Exception\InvalidCredentialsException;
2325
use Symfony\Component\Ldap\LdapInterface;
2426
use Symfony\Component\Ldap\Security\CheckLdapCredentialsListener;
2527
use Symfony\Component\Ldap\Security\LdapBadge;
28+
use Symfony\Component\Ldap\Security\LdapUser;
2629
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
2730
use Symfony\Component\Security\Core\Exception\AuthenticationException;
2831
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
2932
use Symfony\Component\Security\Core\User\InMemoryUser;
33+
use Symfony\Component\Security\Core\User\UserInterface;
3034
use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
3135
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
3236
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
@@ -38,6 +42,7 @@
3842

3943
class CheckLdapCredentialsListenerTest extends TestCase
4044
{
45+
use ExpectDeprecationTrait;
4146
private MockObject&LdapInterface $ldap;
4247

4348
protected function setUp(): void
@@ -46,25 +51,42 @@ protected function setUp(): void
4651
}
4752

4853
/**
49-
* @dataProvider provideShouldNotCheckPassport
54+
* @group legacy
55+
*
56+
* @dataProvider provideShouldCheckPassport
5057
*/
51-
public function testShouldNotCheckPassport($authenticator, $passport)
58+
public function testShouldCheckPassport(AuthenticatorInterface $authenticator, Passport $passport, bool $expectedBindCalled, bool $expectDeprecation)
5259
{
53-
$this->ldap->expects($this->never())->method('bind');
60+
if ($expectDeprecation) {
61+
$this->expectDeprecation('Since symfony/ldap 6.4: Authenticate a user that is not an instance of Symfony\Component\Ldap\Security\LdapUser is deprecated and won\'t be the default behavior anymore in 7.0. Use the Symfony\Component\Ldap\Attribute\WithLdapPassword attribute to keep this behavior.');
62+
}
63+
64+
if ($expectedBindCalled) {
65+
$this->ldap->expects($this->once())->method('bind');
66+
} else {
67+
$this->ldap->expects($this->never())->method('bind');
68+
}
5469

5570
$listener = $this->createListener();
5671
$listener->onCheckPassport(new CheckPassportEvent($authenticator, $passport));
5772
}
5873

59-
public static function provideShouldNotCheckPassport()
74+
public static function provideShouldCheckPassport()
6075
{
61-
// no LdapBadge
62-
yield [new TestAuthenticator(), new Passport(new UserBadge('test'), new PasswordCredentials('s3cret'))];
76+
yield 'no LdapBadge' => [new TestAuthenticator(), new Passport(new UserBadge('test'), new PasswordCredentials('s3cret')), false, false];
6377

64-
// ldap already resolved
65-
$badge = new LdapBadge('app.ldap');
66-
$badge->markResolved();
67-
yield [new TestAuthenticator(), new Passport(new UserBadge('test'), new PasswordCredentials('s3cret'), [$badge])];
78+
$ldapBadge = new LdapBadge('app.ldap', '{user_identifier}', '', '', null, true);
79+
$userBadge = new UserBadge('test');
80+
$userBadge->setUserLoader(function () { return new InMemoryUser('test', 'pass', ['ROLE_USER']); });
81+
yield 'non ldap user' => [new TestAuthenticator(), new Passport($userBadge, new PasswordCredentials('s3cret'), [$ldapBadge]), true, true];
82+
83+
$userBadge = new UserBadge('test');
84+
$userBadge->setUserLoader(function () { return new UserWithLdapPasswordEnabled('test', ['ROLE_USER']); });
85+
yield 'withLdapPassword enabled' => [new TestAuthenticator(), new Passport($userBadge, new PasswordCredentials('s3cret'), [$ldapBadge]), true, false];
86+
87+
$userBadge = new UserBadge('test');
88+
$userBadge->setUserLoader(function () { return new UserWithLdapPasswordDisabled('test', ['ROLE_USER']); });
89+
yield 'withLdapPassword disabled' => [new TestAuthenticator(), new Passport($userBadge, new PasswordCredentials('s3cret'), [$ldapBadge]), false, false];
6890
}
6991

7092
public function testPasswordCredentialsAlreadyResolvedThrowsException()
@@ -74,7 +96,7 @@ public function testPasswordCredentialsAlreadyResolvedThrowsException()
7496

7597
$badge = new PasswordCredentials('s3cret');
7698
$badge->markResolved();
77-
$passport = new Passport(new UserBadge('test'), $badge, [new LdapBadge('app.ldap')]);
99+
$passport = new Passport(new UserBadge('test'), $badge, [new LdapBadge('app.ldap', '{user_identifier}', '', '', null, true)]);
78100

79101
$listener = $this->createListener();
80102
$listener->onCheckPassport(new CheckPassportEvent(new TestAuthenticator(), $passport));
@@ -86,7 +108,7 @@ public function testInvalidLdapServiceId()
86108
$this->expectExceptionMessage('Cannot check credentials using the "not_existing_ldap_service" ldap service, as such service is not found. Did you maybe forget to add the "ldap" service tag to this service?');
87109

88110
$listener = $this->createListener();
89-
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('not_existing_ldap_service')));
111+
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('not_existing_ldap_service', '{user_identifier}', '', '', null, true)));
90112
}
91113

92114
/**
@@ -104,7 +126,10 @@ public function testWrongPassport($passport)
104126
public static function provideWrongPassportData()
105127
{
106128
// no password credentials
107-
yield [new SelfValidatingPassport(new UserBadge('test'), [new LdapBadge('app.ldap')])];
129+
yield [new SelfValidatingPassport(
130+
new UserBadge('test'),
131+
[new LdapBadge('app.ldap', '{user_identifier}', '', '', null, true)]
132+
)];
108133
}
109134

110135
public function testEmptyPasswordShouldThrowAnException()
@@ -171,6 +196,9 @@ public static function queryForDnProvider(): iterable
171196
yield ['{user_identifier}', '{user_identifier}_test'];
172197
}
173198

199+
/**
200+
* @group legacy
201+
*/
174202
public function testQueryForDn()
175203
{
176204
$collection = new class([new Entry('')]) extends \ArrayObject implements CollectionInterface {
@@ -198,7 +226,7 @@ public function toArray(): array
198226
$this->ldap->expects($this->once())->method('query')->with('{user_identifier}', 'wouter_test')->willReturn($query);
199227

200228
$listener = $this->createListener();
201-
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('app.ldap', '{user_identifier}', 'elsa', 'test1234A$', '{user_identifier}_test')));
229+
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('app.ldap', '{user_identifier}', 'elsa', 'test1234A$', '{user_identifier}_test', true)));
202230
}
203231

204232
public function testEmptyQueryResultShouldThrowAnException()
@@ -226,14 +254,21 @@ public function testEmptyQueryResultShouldThrowAnException()
226254
$this->ldap->expects($this->once())->method('query')->willReturn($query);
227255

228256
$listener = $this->createListener();
229-
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('app.ldap', '{user_identifier}', 'elsa', 'test1234A$', '{user_identifier}_test')));
257+
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('app.ldap', '{user_identifier}', 'elsa', 'test1234A$', '{user_identifier}_test', true)));
230258
}
231259

232260
private function createEvent($password = 's3cr3t', $ldapBadge = null)
233261
{
262+
$ldapUser = new LdapUser(new Entry('cn=Wouter,dc=example,dc=com'), 'Wouter', null, ['ROLE_USER']);
263+
264+
/*return new CheckPassportEvent(
265+
new TestAuthenticator(),
266+
new Passport(new UserBadge('Wouter', fn () => $ldapUser), new PasswordCredentials($password), [$ldapBadge ?? new LdapBadge('app.ldap', '{user_identifier}', '', '', null, true)])
267+
);*/
268+
234269
return new CheckPassportEvent(
235270
new TestAuthenticator(),
236-
new Passport(new UserBadge('Wouter', fn () => new InMemoryUser('Wouter', null, ['ROLE_USER'])), new PasswordCredentials($password), [$ldapBadge ?? new LdapBadge('app.ldap')])
271+
new Passport(new UserBadge('Wouter', fn () => new InMemoryUser('Wouter', null, ['ROLE_USER'])), new PasswordCredentials($password), [$ldapBadge ?? new LdapBadge('app.ldap', '{user_identifier}', '', '', null, true)])
237272
);
238273
}
239274

@@ -278,3 +313,30 @@ public function createToken(Passport $passport, string $firewallName): TokenInte
278313
}
279314
}
280315
}
316+
317+
class BaseUser implements UserInterface
318+
{
319+
public function __construct(private string $identifier, private array $roles)
320+
{
321+
}
322+
323+
public function getRoles(): array
324+
{
325+
return $this->roles;
326+
}
327+
328+
public function eraseCredentials(): void
329+
{
330+
}
331+
332+
public function getUserIdentifier(): string
333+
{
334+
return $this->identifier;
335+
}
336+
}
337+
338+
#[WithLdapPassword]
339+
class UserWithLdapPasswordEnabled extends BaseUser {}
340+
341+
#[WithLdapPassword(false)]
342+
class UserWithLdapPasswordDisabled extends BaseUser {}

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Security;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
16+
use Symfony\Component\Ldap\Security\LdapBadge;
17+
18+
final class LdapBadgeTest extends TestCase
19+
{
20+
use ExpectDeprecationTrait;
21+
22+
/**
23+
* @group legacy
24+
*/
25+
public function testDeprecationOnResolvedInitialValue()
26+
{
27+
$this->expectDeprecation('Since symfony/ldap 6.4: Passing "false" as resolved initial value is deprecated, use "true" instead.');
28+
29+
new LdapBadge('foo');
30+
}
31+
32+
/**
33+
* @group legacy
34+
*/
35+
public function testDeprecationOnMarkAsResolved()
36+
{
37+
$this->expectDeprecation('Since symfony/ldap 6.4: Symfony\Component\Ldap\Security\LdapBadge::markResolved is deprecated and will be removed in 7.0. Symfony\Component\Ldap\Security\LdapBadge is intended to bear LDAP information and doesn\'t need to be resolved anymore.');
38+
39+
$sut = new LdapBadge('foo', '{user_identifier}', '', '', null, true);
40+
$sut->markResolved();
41+
}
42+
}

0 commit comments

Comments
 (0)