Skip to content

Commit 4ecc160

Browse files
fabpotfabpot
committed
feature symfony#52181 [Security] Ability to add roles in form_login_ldap by ldap group (Spomky)
This PR was merged into the 7.3 branch. Discussion ---------- [Security] Ability to add roles in `form_login_ldap` by ldap group | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix symfony#51225 | License | MIT This PR adds a way for setting roles in `form_login_ldap` based on LDAP configuration. ~Please note that it is based on SF6.4, but may be changed to 7.1 if already in feature freeze period.~ => Rebased for targeting SF 7.1 * [x] Feature * [x] Tests * [ ] Documentation Commits ------- b183e4a [Security] Ability to add roles in form_login_ldap by ldap group
2 parents 4d9b7be + b183e4a commit 4ecc160

File tree

13 files changed

+295
-5
lines changed

13 files changed

+295
-5
lines changed

‎src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ CHANGELOG
88
* Add encryption support to `OidcTokenHandler` (JWE)
99
* Add `expose_security_errors` config option to display `AccountStatusException`
1010
* Deprecate the `security.hide_user_not_found` config option in favor of `security.expose_security_errors`
11+
* Add ability to fetch LDAP roles
1112

1213
7.2
1314
---

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/UserProvider/LdapFactory.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public function create(ContainerBuilder $container, string $id, array $config):
3232
->replaceArgument(1, $config['base_dn'])
3333
->replaceArgument(2, $config['search_dn'])
3434
->replaceArgument(3, $config['search_password'])
35-
->replaceArgument(4, $config['default_roles'])
35+
->replaceArgument(4, $config['role_fetcher'] ? new Reference($config['role_fetcher']) : $config['default_roles'])
3636
->replaceArgument(5, $config['uid_key'])
3737
->replaceArgument(6, $config['filter'])
3838
->replaceArgument(7, $config['password_attribute'])
@@ -63,6 +63,7 @@ public function addConfiguration(NodeDefinition $node): void
6363
->requiresAtLeastOneElement()
6464
->prototype('scalar')->end()
6565
->end()
66+
->scalarNode('role_fetcher')->defaultNull()->end()
6667
->scalarNode('uid_key')->defaultValue('sAMAccountName')->end()
6768
->scalarNode('filter')->defaultValue('({uid_key}={user_identifier})')->end()
6869
->scalarNode('password_attribute')->defaultNull()->end()

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLdapLoginBundle/Controller/TestController.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
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 Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLdapLoginBundle\Controller;
13+
14+
use Symfony\Component\HttpFoundation\JsonResponse;
15+
use Symfony\Component\Security\Core\User\UserInterface;
16+
17+
class TestController
18+
{
19+
public function loginCheckAction(UserInterface $user)
20+
{
21+
return new JsonResponse([
22+
'message' => \sprintf('Welcome @%s!', $user->getUserIdentifier()),
23+
'roles' => $user->getRoles(),
24+
]);
25+
}
26+
}

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLdapLoginBundle/Security/Ldap/DummyRoleFetcher.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
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 Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLdapLoginBundle\Security\Ldap;
13+
14+
use Symfony\Component\Ldap\Entry;
15+
use Symfony\Component\Ldap\Security\RoleFetcherInterface;
16+
17+
class DummyRoleFetcher implements RoleFetcherInterface
18+
{
19+
public function fetchRoles(Entry $entry): array
20+
{
21+
if ($entry->getAttribute('uid') === ['spomky']) {
22+
return ['ROLE_SUPER_ADMIN', 'ROLE_USER'];
23+
}
24+
25+
return ['ROLE_LDAP_USER_42', 'ROLE_USER'];
26+
}
27+
}

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/JsonLoginLdapTest.php

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,14 @@
1111

1212
namespace Symfony\Bundle\SecurityBundle\Tests\Functional;
1313

14+
use Symfony\Component\HttpFoundation\JsonResponse;
1415
use Symfony\Component\HttpKernel\Kernel;
16+
use Symfony\Component\Ldap\Adapter\AdapterInterface;
17+
use Symfony\Component\Ldap\Adapter\CollectionInterface;
18+
use Symfony\Component\Ldap\Adapter\ConnectionInterface;
19+
use Symfony\Component\Ldap\Adapter\ExtLdap\Adapter;
20+
use Symfony\Component\Ldap\Adapter\QueryInterface;
21+
use Symfony\Component\Ldap\Entry;
1522

1623
class JsonLoginLdapTest extends AbstractWebTestCase
1724
{
@@ -22,4 +29,45 @@ public function testKernelBoot()
2229

2330
$this->assertInstanceOf(Kernel::class, $kernel);
2431
}
32+
33+
public function testDefaultJsonLdapLoginSuccess()
34+
{
35+
if (!interface_exists(\Symfony\Component\Ldap\Security\RoleFetcherInterface::class)) {
36+
$this->markTestSkipped('The "LDAP" component does not support LDAP roles.');
37+
}
38+
// Given
39+
$client = $this->createClient(['test_case' => 'JsonLoginLdap', 'root_config' => 'config.yml', 'debug' => true]);
40+
$container = $client->getContainer();
41+
$connectionMock = $this->createMock(ConnectionInterface::class);
42+
$collection = new class([new Entry('', ['uid' => ['spomky']])]) extends \ArrayObject implements CollectionInterface {
43+
public function toArray(): array
44+
{
45+
return $this->getArrayCopy();
46+
}
47+
};
48+
$queryMock = $this->createMock(QueryInterface::class);
49+
$queryMock
50+
->method('execute')
51+
->willReturn($collection)
52+
;
53+
$ldapAdapterMock = $this->createMock(AdapterInterface::class);
54+
$ldapAdapterMock
55+
->method('getConnection')
56+
->willReturn($connectionMock)
57+
;
58+
$ldapAdapterMock
59+
->method('createQuery')
60+
->willReturn($queryMock)
61+
;
62+
$container->set(Adapter::class, $ldapAdapterMock);
63+
64+
// When
65+
$client->request('POST', '/login', [], [], ['CONTENT_TYPE' => 'application/json'], '{"user": {"login": "spomky", "password": "foo"}}');
66+
$response = $client->getResponse();
67+
68+
// Then
69+
$this->assertInstanceOf(JsonResponse::class, $response);
70+
$this->assertSame(200, $response->getStatusCode());
71+
$this->assertSame(['message' => 'Welcome @spomky!', 'roles' => ['ROLE_SUPER_ADMIN', 'ROLE_USER']], json_decode($response->getContent(), true));
72+
}
2573
}

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLoginLdap/config.yml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ imports:
33
services:
44
Symfony\Component\Ldap\Ldap:
55
arguments: ['@Symfony\Component\Ldap\Adapter\ExtLdap\Adapter']
6+
tags: [ 'ldap' ]
7+
dummy_role_fetcher:
8+
class: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLdapLoginBundle\Security\Ldap\DummyRoleFetcher
69

710
Symfony\Component\Ldap\Adapter\ExtLdap\Adapter:
811
arguments:
@@ -19,9 +22,8 @@ security:
1922
base_dn: 'dc=onfroy,dc=net'
2023
search_dn: ''
2124
search_password: ''
22-
default_roles: ROLE_USER
25+
role_fetcher: dummy_role_fetcher
2326
uid_key: uid
24-
extra_fields: ['email']
2527

2628
firewalls:
2729
main:

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLoginLdap/routing.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
login_check:
2+
path: /login
3+
defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLdapLoginBundle\Controller\TestController::loginCheckAction }

‎src/Symfony/Component/Ldap/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ CHANGELOG
55
---
66

77
* Deprecate `LdapUser::eraseCredentials()` in favor of `__serialize()`
8+
* Add `RoleFetcherInterface` to allow roles fetching at user loading
9+
* Add ability to fetch LDAP roles
810

911
7.2
1012
---

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
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 Symfony\Component\Ldap\Security;
13+
14+
use Symfony\Component\Ldap\Entry;
15+
16+
final readonly class AssignDefaultRoles implements RoleFetcherInterface
17+
{
18+
/**
19+
* @param string[] $roles
20+
*/
21+
public function __construct(
22+
private array $roles,
23+
) {
24+
}
25+
26+
/**
27+
* @return string[]
28+
*/
29+
public function fetchRoles(Entry $entry): array
30+
{
31+
return $this->roles;
32+
}
33+
}

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,14 @@ class LdapUserProvider implements UserProviderInterface, PasswordUpgraderInterfa
3737
{
3838
private string $uidKey;
3939
private string $defaultSearch;
40+
private RoleFetcherInterface $roleFetcher;
4041

4142
public function __construct(
4243
private LdapInterface $ldap,
4344
private string $baseDn,
4445
private ?string $searchDn = null,
4546
#[\SensitiveParameter] private ?string $searchPassword = null,
46-
private array $defaultRoles = [],
47+
array|RoleFetcherInterface $defaultRoles = [],
4748
?string $uidKey = null,
4849
?string $filter = null,
4950
private ?string $passwordAttribute = null,
@@ -54,6 +55,7 @@ public function __construct(
5455

5556
$this->uidKey = $uidKey;
5657
$this->defaultSearch = str_replace('{uid_key}', $uidKey, $filter);
58+
$this->roleFetcher = \is_array($defaultRoles) ? new AssignDefaultRoles($defaultRoles) : $defaultRoles;
5759
}
5860

5961
public function loadUserByIdentifier(string $identifier): UserInterface
@@ -147,7 +149,9 @@ protected function loadUser(string $identifier, Entry $entry): UserInterface
147149
$extraFields[$field] = $this->getAttributeValue($entry, $field);
148150
}
149151

150-
return new LdapUser($entry, $identifier, $password, $this->defaultRoles, $extraFields);
152+
$roles = $this->roleFetcher->fetchRoles($entry);
153+
154+
return new LdapUser($entry, $identifier, $password, $roles, $extraFields);
151155
}
152156

153157
private function getAttributeValue(Entry $entry, string $attribute): mixed

0 commit comments

Comments
 (0)