From b183e4acebdd1fe0be0be48c9bdb597281167565 Mon Sep 17 00:00:00 2001 From: Florent Morselli Date: Fri, 7 Feb 2025 21:05:10 +0100 Subject: [PATCH] [Security] Ability to add roles in form_login_ldap by ldap group This update allows LDAP to fetch roles for a given user entry by using the new RoleFetcherInterface. The LdapUserProvider class has been adjusted to use this new functionality. --- .../Bundle/SecurityBundle/CHANGELOG.md | 1 + .../Security/UserProvider/LdapFactory.php | 3 +- .../Controller/TestController.php | 26 ++++++++ .../Security/Ldap/DummyRoleFetcher.php | 27 ++++++++ .../Tests/Functional/JsonLoginLdapTest.php | 48 ++++++++++++++ .../Functional/app/JsonLoginLdap/config.yml | 6 +- .../Functional/app/JsonLoginLdap/routing.yml | 3 + src/Symfony/Component/Ldap/CHANGELOG.md | 2 + .../Ldap/Security/AssignDefaultRoles.php | 33 ++++++++++ .../Ldap/Security/LdapUserProvider.php | 8 ++- .../Component/Ldap/Security/MemberOfRoles.php | 56 +++++++++++++++++ .../Ldap/Security/RoleFetcherInterface.php | 25 ++++++++ .../Tests/Security/LdapUserProviderTest.php | 62 +++++++++++++++++++ 13 files changed, 295 insertions(+), 5 deletions(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLdapLoginBundle/Controller/TestController.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLdapLoginBundle/Security/Ldap/DummyRoleFetcher.php create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLoginLdap/routing.yml create mode 100644 src/Symfony/Component/Ldap/Security/AssignDefaultRoles.php create mode 100644 src/Symfony/Component/Ldap/Security/MemberOfRoles.php create mode 100644 src/Symfony/Component/Ldap/Security/RoleFetcherInterface.php diff --git a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md index d9339926d61df..574a8910fcd9c 100644 --- a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md @@ -8,6 +8,7 @@ CHANGELOG * Add encryption support to `OidcTokenHandler` (JWE) * Add `expose_security_errors` config option to display `AccountStatusException` * Deprecate the `security.hide_user_not_found` config option in favor of `security.expose_security_errors` + * Add ability to fetch LDAP roles 7.2 --- diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/UserProvider/LdapFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/UserProvider/LdapFactory.php index b8d442fd99251..1efa2c642fdbc 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/UserProvider/LdapFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/UserProvider/LdapFactory.php @@ -32,7 +32,7 @@ public function create(ContainerBuilder $container, string $id, array $config): ->replaceArgument(1, $config['base_dn']) ->replaceArgument(2, $config['search_dn']) ->replaceArgument(3, $config['search_password']) - ->replaceArgument(4, $config['default_roles']) + ->replaceArgument(4, $config['role_fetcher'] ? new Reference($config['role_fetcher']) : $config['default_roles']) ->replaceArgument(5, $config['uid_key']) ->replaceArgument(6, $config['filter']) ->replaceArgument(7, $config['password_attribute']) @@ -63,6 +63,7 @@ public function addConfiguration(NodeDefinition $node): void ->requiresAtLeastOneElement() ->prototype('scalar')->end() ->end() + ->scalarNode('role_fetcher')->defaultNull()->end() ->scalarNode('uid_key')->defaultValue('sAMAccountName')->end() ->scalarNode('filter')->defaultValue('({uid_key}={user_identifier})')->end() ->scalarNode('password_attribute')->defaultNull()->end() diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLdapLoginBundle/Controller/TestController.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLdapLoginBundle/Controller/TestController.php new file mode 100644 index 0000000000000..3bf5e6e43dd85 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLdapLoginBundle/Controller/TestController.php @@ -0,0 +1,26 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLdapLoginBundle\Controller; + +use Symfony\Component\HttpFoundation\JsonResponse; +use Symfony\Component\Security\Core\User\UserInterface; + +class TestController +{ + public function loginCheckAction(UserInterface $user) + { + return new JsonResponse([ + 'message' => \sprintf('Welcome @%s!', $user->getUserIdentifier()), + 'roles' => $user->getRoles(), + ]); + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLdapLoginBundle/Security/Ldap/DummyRoleFetcher.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLdapLoginBundle/Security/Ldap/DummyRoleFetcher.php new file mode 100644 index 0000000000000..417c8afe8061c --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/JsonLdapLoginBundle/Security/Ldap/DummyRoleFetcher.php @@ -0,0 +1,27 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLdapLoginBundle\Security\Ldap; + +use Symfony\Component\Ldap\Entry; +use Symfony\Component\Ldap\Security\RoleFetcherInterface; + +class DummyRoleFetcher implements RoleFetcherInterface +{ + public function fetchRoles(Entry $entry): array + { + if ($entry->getAttribute('uid') === ['spomky']) { + return ['ROLE_SUPER_ADMIN', 'ROLE_USER']; + } + + return ['ROLE_LDAP_USER_42', 'ROLE_USER']; + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/JsonLoginLdapTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/JsonLoginLdapTest.php index 583e153695fed..f11908299834f 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/JsonLoginLdapTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/JsonLoginLdapTest.php @@ -11,7 +11,14 @@ namespace Symfony\Bundle\SecurityBundle\Tests\Functional; +use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpKernel\Kernel; +use Symfony\Component\Ldap\Adapter\AdapterInterface; +use Symfony\Component\Ldap\Adapter\CollectionInterface; +use Symfony\Component\Ldap\Adapter\ConnectionInterface; +use Symfony\Component\Ldap\Adapter\ExtLdap\Adapter; +use Symfony\Component\Ldap\Adapter\QueryInterface; +use Symfony\Component\Ldap\Entry; class JsonLoginLdapTest extends AbstractWebTestCase { @@ -22,4 +29,45 @@ public function testKernelBoot() $this->assertInstanceOf(Kernel::class, $kernel); } + + public function testDefaultJsonLdapLoginSuccess() + { + if (!interface_exists(\Symfony\Component\Ldap\Security\RoleFetcherInterface::class)) { + $this->markTestSkipped('The "LDAP" component does not support LDAP roles.'); + } + // Given + $client = $this->createClient(['test_case' => 'JsonLoginLdap', 'root_config' => 'config.yml', 'debug' => true]); + $container = $client->getContainer(); + $connectionMock = $this->createMock(ConnectionInterface::class); + $collection = new class([new Entry('', ['uid' => ['spomky']])]) extends \ArrayObject implements CollectionInterface { + public function toArray(): array + { + return $this->getArrayCopy(); + } + }; + $queryMock = $this->createMock(QueryInterface::class); + $queryMock + ->method('execute') + ->willReturn($collection) + ; + $ldapAdapterMock = $this->createMock(AdapterInterface::class); + $ldapAdapterMock + ->method('getConnection') + ->willReturn($connectionMock) + ; + $ldapAdapterMock + ->method('createQuery') + ->willReturn($queryMock) + ; + $container->set(Adapter::class, $ldapAdapterMock); + + // When + $client->request('POST', '/login', [], [], ['CONTENT_TYPE' => 'application/json'], '{"user": {"login": "spomky", "password": "foo"}}'); + $response = $client->getResponse(); + + // Then + $this->assertInstanceOf(JsonResponse::class, $response); + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame(['message' => 'Welcome @spomky!', 'roles' => ['ROLE_SUPER_ADMIN', 'ROLE_USER']], json_decode($response->getContent(), true)); + } } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLoginLdap/config.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLoginLdap/config.yml index 71e107b126e54..c75c1a79673d1 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLoginLdap/config.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLoginLdap/config.yml @@ -3,6 +3,9 @@ imports: services: Symfony\Component\Ldap\Ldap: arguments: ['@Symfony\Component\Ldap\Adapter\ExtLdap\Adapter'] + tags: [ 'ldap' ] + dummy_role_fetcher: + class: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLdapLoginBundle\Security\Ldap\DummyRoleFetcher Symfony\Component\Ldap\Adapter\ExtLdap\Adapter: arguments: @@ -19,9 +22,8 @@ security: base_dn: 'dc=onfroy,dc=net' search_dn: '' search_password: '' - default_roles: ROLE_USER + role_fetcher: dummy_role_fetcher uid_key: uid - extra_fields: ['email'] firewalls: main: diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLoginLdap/routing.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLoginLdap/routing.yml new file mode 100644 index 0000000000000..bbec958cd8dae --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/JsonLoginLdap/routing.yml @@ -0,0 +1,3 @@ +login_check: + path: /login + defaults: { _controller: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\JsonLdapLoginBundle\Controller\TestController::loginCheckAction } diff --git a/src/Symfony/Component/Ldap/CHANGELOG.md b/src/Symfony/Component/Ldap/CHANGELOG.md index ff9cedd42d8c3..9f1bb9f16b1fc 100644 --- a/src/Symfony/Component/Ldap/CHANGELOG.md +++ b/src/Symfony/Component/Ldap/CHANGELOG.md @@ -5,6 +5,8 @@ CHANGELOG --- * Deprecate `LdapUser::eraseCredentials()` in favor of `__serialize()` + * Add `RoleFetcherInterface` to allow roles fetching at user loading + * Add ability to fetch LDAP roles 7.2 --- diff --git a/src/Symfony/Component/Ldap/Security/AssignDefaultRoles.php b/src/Symfony/Component/Ldap/Security/AssignDefaultRoles.php new file mode 100644 index 0000000000000..351b52d4600b2 --- /dev/null +++ b/src/Symfony/Component/Ldap/Security/AssignDefaultRoles.php @@ -0,0 +1,33 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Ldap\Security; + +use Symfony\Component\Ldap\Entry; + +final readonly class AssignDefaultRoles implements RoleFetcherInterface +{ + /** + * @param string[] $roles + */ + public function __construct( + private array $roles, + ) { + } + + /** + * @return string[] + */ + public function fetchRoles(Entry $entry): array + { + return $this->roles; + } +} diff --git a/src/Symfony/Component/Ldap/Security/LdapUserProvider.php b/src/Symfony/Component/Ldap/Security/LdapUserProvider.php index 211f2ac64f0be..5c0f4785437e7 100644 --- a/src/Symfony/Component/Ldap/Security/LdapUserProvider.php +++ b/src/Symfony/Component/Ldap/Security/LdapUserProvider.php @@ -37,13 +37,14 @@ class LdapUserProvider implements UserProviderInterface, PasswordUpgraderInterfa { private string $uidKey; private string $defaultSearch; + private RoleFetcherInterface $roleFetcher; public function __construct( private LdapInterface $ldap, private string $baseDn, private ?string $searchDn = null, #[\SensitiveParameter] private ?string $searchPassword = null, - private array $defaultRoles = [], + array|RoleFetcherInterface $defaultRoles = [], ?string $uidKey = null, ?string $filter = null, private ?string $passwordAttribute = null, @@ -54,6 +55,7 @@ public function __construct( $this->uidKey = $uidKey; $this->defaultSearch = str_replace('{uid_key}', $uidKey, $filter); + $this->roleFetcher = \is_array($defaultRoles) ? new AssignDefaultRoles($defaultRoles) : $defaultRoles; } public function loadUserByIdentifier(string $identifier): UserInterface @@ -147,7 +149,9 @@ protected function loadUser(string $identifier, Entry $entry): UserInterface $extraFields[$field] = $this->getAttributeValue($entry, $field); } - return new LdapUser($entry, $identifier, $password, $this->defaultRoles, $extraFields); + $roles = $this->roleFetcher->fetchRoles($entry); + + return new LdapUser($entry, $identifier, $password, $roles, $extraFields); } private function getAttributeValue(Entry $entry, string $attribute): mixed diff --git a/src/Symfony/Component/Ldap/Security/MemberOfRoles.php b/src/Symfony/Component/Ldap/Security/MemberOfRoles.php new file mode 100644 index 0000000000000..166274a379ff0 --- /dev/null +++ b/src/Symfony/Component/Ldap/Security/MemberOfRoles.php @@ -0,0 +1,56 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Ldap\Security; + +use Symfony\Component\Ldap\Entry; + +final readonly class MemberOfRoles implements RoleFetcherInterface +{ + /** + * @param array $mapping + */ + public function __construct( + private array $mapping, + private string $attributeName = 'ismemberof', + private string $groupNameRegex = '/^CN=(?P[^,]+),ou.*$/i', + ) { + } + + /** + * @return string[] + */ + public function fetchRoles(Entry $entry): array + { + if (!$entry->hasAttribute($this->attributeName)) { + return []; + } + + $roles = []; + foreach ($entry->getAttribute($this->attributeName) as $group) { + $groupName = $this->getGroupName($group); + if (\array_key_exists($groupName, $this->mapping)) { + $roles[] = $this->mapping[$groupName]; + } + } + + return array_unique($roles); + } + + private function getGroupName(string $group): string + { + if (preg_match($this->groupNameRegex, $group, $matches)) { + return $matches['group']; + } + + return $group; + } +} diff --git a/src/Symfony/Component/Ldap/Security/RoleFetcherInterface.php b/src/Symfony/Component/Ldap/Security/RoleFetcherInterface.php new file mode 100644 index 0000000000000..29bf3b9d55588 --- /dev/null +++ b/src/Symfony/Component/Ldap/Security/RoleFetcherInterface.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Ldap\Security; + +use Symfony\Component\Ldap\Entry; + +/** + * Fetches LDAP roles for a given entry. + */ +interface RoleFetcherInterface +{ + /** + * @return string[] The list of roles + */ + public function fetchRoles(Entry $entry): array; +} diff --git a/src/Symfony/Component/Ldap/Tests/Security/LdapUserProviderTest.php b/src/Symfony/Component/Ldap/Tests/Security/LdapUserProviderTest.php index 90955357e3136..0e35c329e1de0 100644 --- a/src/Symfony/Component/Ldap/Tests/Security/LdapUserProviderTest.php +++ b/src/Symfony/Component/Ldap/Tests/Security/LdapUserProviderTest.php @@ -19,6 +19,8 @@ use Symfony\Component\Ldap\LdapInterface; use Symfony\Component\Ldap\Security\LdapUser; use Symfony\Component\Ldap\Security\LdapUserProvider; +use Symfony\Component\Ldap\Security\MemberOfRoles; +use Symfony\Component\Ldap\Security\RoleFetcherInterface; use Symfony\Component\Security\Core\Exception\InvalidArgumentException; use Symfony\Component\Security\Core\Exception\UserNotFoundException; @@ -388,4 +390,64 @@ public function testRefreshUserShouldReturnUserWithSameProperties() $this->assertEquals($user, $provider->refreshUser($user)); } + + public function testLoadUserWithCorrectRoles() + { + // Given + $result = $this->createMock(CollectionInterface::class); + $query = $this->createMock(QueryInterface::class); + $query + ->method('execute') + ->willReturn($result) + ; + $ldap = $this->createMock(LdapInterface::class); + $result + ->method('offsetGet') + ->with(0) + ->willReturn(new Entry('foo', ['sAMAccountName' => ['foo']])) + ; + $result + ->method('count') + ->willReturn(1) + ; + $ldap + ->method('escape') + ->willReturn('foo') + ; + $ldap + ->method('query') + ->willReturn($query) + ; + $roleFetcher = $this->createMock(RoleFetcherInterface::class); + $roleFetcher + ->method('fetchRoles') + ->willReturn(['ROLE_FOO', 'ROLE_BAR']) + ; + + $provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com', defaultRoles: $roleFetcher); + + // When + $user = $provider->loadUserByIdentifier('foo'); + + // Then + $this->assertInstanceOf(LdapUser::class, $user); + $this->assertSame(['ROLE_FOO', 'ROLE_BAR'], $user->getRoles()); + } + + public function testMemberOfRoleFetch() + { + // Given + $roleFetcher = new MemberOfRoles( + ['Staff' => 'ROLE_STAFF', 'Admin' => 'ROLE_ADMIN'], + 'memberOf' + ); + + $entry = new Entry('uid=elliot.alderson,ou=staff,ou=people,dc=example,dc=com', ['memberOf' => ['cn=Staff,ou=Groups,dc=example,dc=com', 'cn=Admin,ou=Groups,dc=example,dc=com']]); + + // When + $roles = $roleFetcher->fetchRoles($entry); + + // Then + $this->assertSame(['ROLE_STAFF', 'ROLE_ADMIN'], $roles); + } }