Add a normalization step for the user-identifier in firewalls

Spomky · Spomky · commit 80eb8a34baa1 · 2025-01-18T08:40:42.000+01:00
diff --git a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
@@ -6,6 +6,7 @@ CHANGELOG
 
  * Add `Security::isGrantedForUser()` to test user authorization without relying on the session. For example, users not currently logged in, or while processing a message from a message queue
  * Add encryption support to `OidcTokenHandler` (JWE)
+ * 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
@@ -37,6 +37,7 @@ public function create(ContainerBuilder $container, string $id, array $config):
             ->replaceArgument(6, $config['filter'])
             ->replaceArgument(7, $config['password_attribute'])
             ->replaceArgument(8, $config['extra_fields'])
+            ->replaceArgument(9, $config['role_fetcher'] ? new Reference($config['role_fetcher']) : null)
         ;
     }
 
@@ -63,6 +64,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/Resources/config/security.php b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.php
@@ -268,6 +268,7 @@
                 abstract_arg('filter'),
                 abstract_arg('password_attribute'),
                 abstract_arg('extra_fields (email etc)'),
+                abstract_arg('role fetcher'),
             ])
 
         ->set('security.user.provider.chain', ChainUserProvider::class)
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
@@ -0,0 +1,26 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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
@@ -0,0 +1,27 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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
@@ -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 (!class_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
@@ -3,6 +3,7 @@ imports:
 services:
     Symfony\Component\Ldap\Ldap:
         arguments: ['@Symfony\Component\Ldap\Adapter\ExtLdap\Adapter']
+        tags: [ 'ldap' ]
 
     Symfony\Component\Ldap\Adapter\ExtLdap\Adapter:
         arguments:
@@ -21,7 +22,7 @@ security:
                 search_password: ''
                 default_roles: ROLE_USER
                 uid_key: uid
-                extra_fields: ['email']
+                #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
@@ -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
@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+7.3
+---
+
+ * Add ability to fetch roles
+
 7.2
 ---
 
diff --git a/src/Symfony/Component/Ldap/Security/AssignDefaultRoles.php b/src/Symfony/Component/Ldap/Security/AssignDefaultRoles.php
@@ -0,0 +1,33 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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
@@ -37,6 +37,7 @@ class LdapUserProvider implements UserProviderInterface, PasswordUpgraderInterfa
 {
     private string $uidKey;
     private string $defaultSearch;
+    private RoleFetcherInterface $roleFetcher;
 
     public function __construct(
         private LdapInterface $ldap,
@@ -48,12 +49,14 @@ public function __construct(
         ?string $filter = null,
         private ?string $passwordAttribute = null,
         private array $extraFields = [],
+        ?RoleFetcherInterface $roleFetcher = null,
     ) {
         $uidKey ??= 'sAMAccountName';
         $filter ??= '({uid_key}={user_identifier})';
 
         $this->uidKey = $uidKey;
         $this->defaultSearch = str_replace('{uid_key}', $uidKey, $filter);
+        $this->roleFetcher = $roleFetcher ?? new AssignDefaultRoles($defaultRoles);
     }
 
     public function loadUserByIdentifier(string $identifier): UserInterface
@@ -147,7 +150,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
@@ -0,0 +1,56 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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<string, string> $mapping
+     */
+    public function __construct(
+        private array $mapping,
+        private string $attributeName = 'ismemberof',
+        private string $groupNameRegex = '/^CN=(?P<group>[^,]+),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
@@ -0,0 +1,25 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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

Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,7 @@ public function create(ContainerBuilder $container, string $id, array $config):`
`37`	`37`	`->replaceArgument(6, $config['filter'])`
`38`	`38`	`->replaceArgument(7, $config['password_attribute'])`
`39`	`39`	`->replaceArgument(8, $config['extra_fields'])`
	`40`	`+ ->replaceArgument(9, $config['role_fetcher'] ? new Reference($config['role_fetcher']) : null)`
`40`	`41`	`;`
`41`	`42`	`}`
`42`	`43`
`@@ -63,6 +64,7 @@ public function addConfiguration(NodeDefinition $node): void`
`63`	`64`	`->requiresAtLeastOneElement()`
`64`	`65`	`->prototype('scalar')->end()`
`65`	`66`	`->end()`
	`67`	`+ ->scalarNode('role_fetcher')->defaultNull()->end()`
`66`	`68`	`->scalarNode('uid_key')->defaultValue('sAMAccountName')->end()`
`67`	`69`	`->scalarNode('filter')->defaultValue('({uid_key}={user_identifier})')->end()`
`68`	`70`	`->scalarNode('password_attribute')->defaultNull()->end()`
Original file line number	Diff line number	Diff 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 }`