Skip to content

Commit c1a964d

Browse files
l-vol-vo
committed
Allow to use ldap in a chain provider
1 parent b39fcef commit c1a964d

File tree

8 files changed

+266
-21
lines changed

8 files changed

+266
-21
lines changed

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
use Symfony\Component\Ldap\LdapInterface;
1818
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
1919
use Symfony\Component\Security\Core\Exception\LogicException;
20-
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
2120
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
2221
use Symfony\Component\Security\Http\Authenticator\Passport\UserPassportInterface;
2322
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
@@ -46,9 +45,6 @@ public function onCheckPassport(CheckPassportEvent $event)
4645

4746
/** @var LdapBadge $ldapBadge */
4847
$ldapBadge = $passport->getBadge(LdapBadge::class);
49-
if ($ldapBadge->isResolved()) {
50-
return;
51-
}
5248

5349
if (!$passport instanceof UserPassportInterface || !$passport->hasBadge(PasswordCredentials::class)) {
5450
throw new \LogicException(sprintf('LDAP authentication requires a passport containing a user and password credentials, authenticator "%s" does not fulfill these requirements.', \get_class($event->getAuthenticator())));
@@ -70,8 +66,8 @@ public function onCheckPassport(CheckPassportEvent $event)
7066
}
7167

7268
$user = $passport->getUser();
73-
if (!$user instanceof PasswordAuthenticatedUserInterface) {
74-
trigger_deprecation('symfony/ldap', '5.3', 'Not implementing the "%s" interface in class "%s" while using password-based authenticators is deprecated.', PasswordAuthenticatedUserInterface::class, get_debug_type($user));
69+
if (!$user instanceof LdapUser) {
70+
return;
7571
}
7672

7773
/** @var LdapInterface $ldap */
@@ -104,7 +100,6 @@ public function onCheckPassport(CheckPassportEvent $event)
104100
}
105101

106102
$passwordCredentials->markResolved();
107-
$ldapBadge->markResolved();
108103
}
109104

110105
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,20 +24,24 @@
2424
*/
2525
class LdapBadge implements BadgeInterface
2626
{
27-
private $resolved = false;
27+
private $resolved = true;
2828
private $ldapServiceId;
2929
private $dnString;
3030
private $searchDn;
3131
private $searchPassword;
3232
private $queryString;
3333

34-
public function __construct(string $ldapServiceId, string $dnString = '{username}', string $searchDn = '', string $searchPassword = '', string $queryString = null)
34+
public function __construct(string $ldapServiceId, string $dnString = '{user_identifier}', string $searchDn = '', string $searchPassword = '', string $queryString = null, $resolved = false)
3535
{
3636
$this->ldapServiceId = $ldapServiceId;
3737
$this->dnString = $dnString;
3838
$this->searchDn = $searchDn;
3939
$this->searchPassword = $searchPassword;
4040
$this->queryString = $queryString;
41+
$this->resolved = $resolved;
42+
if (false === $this->resolved) {
43+
trigger_deprecation('symfony/ldap', '5.4', 'Passing false as resolved initial value is deprecated, use true instead.');
44+
}
4145
}
4246

4347
public function getLdapServiceId(): string
@@ -67,6 +71,8 @@ public function getQueryString(): ?string
6771

6872
public function markResolved(): void
6973
{
74+
trigger_deprecation('symfony/ldap', '5.4', 'Calling %s is deprecated.', __METHOD__);
75+
7076
$this->resolved = true;
7177
}
7278

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

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use Symfony\Component\Ldap\LdapInterface;
2323
use Symfony\Component\Ldap\Security\CheckLdapCredentialsListener;
2424
use Symfony\Component\Ldap\Security\LdapBadge;
25+
use Symfony\Component\Ldap\Security\LdapUser;
2526
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
2627
use Symfony\Component\Security\Core\Exception\AuthenticationException;
2728
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
@@ -69,9 +70,10 @@ public static function provideShouldNotCheckPassport()
6970
yield [new TestAuthenticator(), new Passport(new UserBadge('test'), new PasswordCredentials('s3cret'))];
7071

7172
// ldap already resolved
72-
$badge = new LdapBadge('app.ldap');
73-
$badge->markResolved();
74-
yield [new TestAuthenticator(), new Passport(new UserBadge('test'), new PasswordCredentials('s3cret'), [$badge])];
73+
$ldapBadge = new LdapBadge('app.ldap', '{user_identifier}', '', '', null, true);
74+
$userBadge = new UserBadge('test');
75+
$userBadge->setUserLoader(function () { return new InMemoryUser('test', 'pass', ['ROLE_USER']); });
76+
yield [new TestAuthenticator(), new Passport($userBadge, new PasswordCredentials('s3cret'), [$ldapBadge])];
7577
}
7678

7779
public function testPasswordCredentialsAlreadyResolvedThrowsException()
@@ -81,7 +83,7 @@ public function testPasswordCredentialsAlreadyResolvedThrowsException()
8183

8284
$badge = new PasswordCredentials('s3cret');
8385
$badge->markResolved();
84-
$passport = new Passport(new UserBadge('test'), $badge, [new LdapBadge('app.ldap')]);
86+
$passport = new Passport(new UserBadge('test'), $badge, [new LdapBadge('app.ldap', '{user_identifier}', '', '', null, true)]);
8587

8688
$listener = $this->createListener();
8789
$listener->onCheckPassport(new CheckPassportEvent(new TestAuthenticator(), $passport));
@@ -93,7 +95,7 @@ public function testInvalidLdapServiceId()
9395
$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?');
9496

9597
$listener = $this->createListener();
96-
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('not_existing_ldap_service')));
98+
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('not_existing_ldap_service', '{user_identifier}', '', '', null, true)));
9799
}
98100

99101
/**
@@ -115,7 +117,10 @@ public static function provideWrongPassportData()
115117
}
116118

117119
// no password credentials
118-
yield [new SelfValidatingPassport(new UserBadge('test'), [new LdapBadge('app.ldap')])];
120+
yield [new SelfValidatingPassport(
121+
new UserBadge('test'),
122+
[new LdapBadge('app.ldap', '{user_identifier}', '', '', null, true)]
123+
)];
119124
}
120125

121126
/**
@@ -129,7 +134,7 @@ public function testLegacyWrongPassport()
129134
// no user passport
130135
$passport = $this->createMock(PassportInterface::class);
131136
$passport->expects($this->any())->method('hasBadge')->with(LdapBadge::class)->willReturn(true);
132-
$passport->expects($this->any())->method('getBadge')->with(LdapBadge::class)->willReturn(new LdapBadge('app.ldap'));
137+
$passport->expects($this->any())->method('getBadge')->with(LdapBadge::class)->willReturn(new LdapBadge('app.ldap', '{user_identifier}', '', '', null, true));
133138

134139
$listener = $this->createListener();
135140
$listener->onCheckPassport(new CheckPassportEvent(new TestAuthenticator(), $passport));
@@ -156,6 +161,9 @@ public function testBindFailureShouldThrowAnException()
156161
$listener->onCheckPassport($this->createEvent());
157162
}
158163

164+
/**
165+
* @group legacy
166+
*/
159167
public function testQueryForDn()
160168
{
161169
$collection = new class([new Entry('')]) extends \ArrayObject implements CollectionInterface {
@@ -183,7 +191,7 @@ public function toArray(): array
183191
$this->ldap->expects($this->once())->method('query')->with('{username}', 'wouter_test')->willReturn($query);
184192

185193
$listener = $this->createListener();
186-
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('app.ldap', '{username}', 'elsa', 'test1234A$', '{username}_test')));
194+
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('app.ldap', '{username}', 'elsa', 'test1234A$', '{username}_test', true)));
187195
}
188196

189197
public function testEmptyQueryResultShouldThrowAnException()
@@ -211,14 +219,16 @@ public function testEmptyQueryResultShouldThrowAnException()
211219
$this->ldap->expects($this->once())->method('query')->willReturn($query);
212220

213221
$listener = $this->createListener();
214-
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('app.ldap', '{username}', 'elsa', 'test1234A$', '{username}_test')));
222+
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('app.ldap', '{username}', 'elsa', 'test1234A$', '{username}_test', true)));
215223
}
216224

217225
private function createEvent($password = 's3cr3t', $ldapBadge = null)
218226
{
227+
$ldapUser = new LdapUser(new Entry('cn=Wouter,dc=example,dc=com'), 'Wouter', null, ['ROLE_USER']);
228+
219229
return new CheckPassportEvent(
220230
new TestAuthenticator(),
221-
new Passport(new UserBadge('Wouter', function () { return new InMemoryUser('Wouter', null, ['ROLE_USER']); }), new PasswordCredentials($password), [$ldapBadge ?? new LdapBadge('app.ldap')])
231+
new Passport(new UserBadge('Wouter', function () use ($ldapUser) { return $ldapUser; }), new PasswordCredentials($password), [$ldapBadge ?? new LdapBadge('app.ldap', '{user_identifier}', '', '', null, true)])
222232
);
223233
}
224234

‎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 5.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 5.4: Calling Symfony\Component\Ldap\Security\LdapBadge::markResolved is deprecated.');
38+
39+
$sut = new LdapBadge('foo', '{user_identifier}', '', '', null, true);
40+
$sut->markResolved();
41+
}
42+
}

‎src/Symfony/Component/Ldap/composer.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
"ext-ldap": "*"
2424
},
2525
"require-dev": {
26-
"symfony/security-core": "^5.3|^6.0"
26+
"symfony/security-core": "^5.3|^6.0",
27+
"symfony/security-http": "^5.4"
2728
},
2829
"conflict": {
2930
"symfony/options-resolver": "<4.4",

0 commit comments

Comments
 (0)