LDAP: fixing issues for re-authenticating users (#2681)

This commit is contained in:
Kevin Papst
2021-07-23 16:12:38 +02:00
committed by GitHub
parent f1f60042d8
commit 6d5244aa5d
4 changed files with 23 additions and 26 deletions

View File

@@ -13,7 +13,6 @@ use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SecurityF
use Symfony\Component\Config\Definition\Builder\NodeDefinition; use Symfony\Component\Config\Definition\Builder\NodeDefinition;
use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;
/** /**
* Inspired by https://github.com/Maks3w/FR3DLdapBundle @ MIT License * Inspired by https://github.com/Maks3w/FR3DLdapBundle @ MIT License
@@ -49,7 +48,6 @@ class FormLoginLdapFactory implements SecurityFactoryInterface
$container $container
->setDefinition($providerId, new ChildDefinition(LdapAuthenticationProvider::class)) ->setDefinition($providerId, new ChildDefinition(LdapAuthenticationProvider::class))
->replaceArgument(1, $id) ->replaceArgument(1, $id)
->replaceArgument(2, new Reference($userProviderId))
; ;
return $providerId; return $providerId;

View File

@@ -11,6 +11,7 @@ namespace App\Ldap;
use App\Configuration\LdapConfiguration; use App\Configuration\LdapConfiguration;
use App\Entity\User; use App\Entity\User;
use App\Security\DoctrineUserProvider;
use Symfony\Component\Security\Core\Authentication\Provider\UserAuthenticationProvider; use Symfony\Component\Security\Core\Authentication\Provider\UserAuthenticationProvider;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
@@ -19,28 +20,17 @@ use Symfony\Component\Security\Core\Exception\BadCredentialsException;
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
use Symfony\Component\Security\Core\User\UserCheckerInterface; use Symfony\Component\Security\Core\User\UserCheckerInterface;
use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Core\User\UserProviderInterface;
/** /**
* Inspired by https://github.com/Maks3w/FR3DLdapBundle @ MIT License * Inspired by https://github.com/Maks3w/FR3DLdapBundle @ MIT License
*/ */
class LdapAuthenticationProvider extends UserAuthenticationProvider class LdapAuthenticationProvider extends UserAuthenticationProvider
{ {
/**
* @var UserProviderInterface
*/
private $userProvider; private $userProvider;
/**
* @var LdapManager
*/
private $ldapManager; private $ldapManager;
/**
* @var LdapConfiguration
*/
private $config; private $config;
public function __construct(UserCheckerInterface $userChecker, $providerKey, UserProviderInterface $userProvider, LdapManager $ldapManager, LdapConfiguration $config, $hideUserNotFoundExceptions = true) public function __construct(UserCheckerInterface $userChecker, $providerKey, DoctrineUserProvider $userProvider, LdapManager $ldapManager, LdapConfiguration $config, $hideUserNotFoundExceptions = true)
{ {
parent::__construct($userChecker, $providerKey, $hideUserNotFoundExceptions); parent::__construct($userChecker, $providerKey, $hideUserNotFoundExceptions);

View File

@@ -18,7 +18,10 @@ use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Core\User\UserProviderInterface; use Symfony\Component\Security\Core\User\UserProviderInterface;
final class DoctrineUserProvider implements UserProviderInterface, PasswordUpgraderInterface /**
* @final
*/
class DoctrineUserProvider implements UserProviderInterface, PasswordUpgraderInterface
{ {
/** /**
* @var UserRepository * @var UserRepository

View File

@@ -14,7 +14,8 @@ use App\Configuration\SystemConfiguration;
use App\Entity\User; use App\Entity\User;
use App\Ldap\LdapAuthenticationProvider; use App\Ldap\LdapAuthenticationProvider;
use App\Ldap\LdapManager; use App\Ldap\LdapManager;
use App\Ldap\LdapUserProvider; use App\Repository\UserRepository;
use App\Security\DoctrineUserProvider;
use App\Tests\Configuration\TestConfigLoader; use App\Tests\Configuration\TestConfigLoader;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
@@ -28,6 +29,11 @@ use Symfony\Component\Security\Core\User\UserChecker;
*/ */
class LdapAuthenticationProviderTest extends TestCase class LdapAuthenticationProviderTest extends TestCase
{ {
private function getUserProvider(): DoctrineUserProvider
{
return new DoctrineUserProvider($this->createMock(UserRepository::class));
}
private function getConfiguration(bool $active = true): LdapConfiguration private function getConfiguration(bool $active = true): LdapConfiguration
{ {
$systemConfig = new SystemConfiguration(new TestConfigLoader([]), ['ldap' => ['activate' => $active]]); $systemConfig = new SystemConfiguration(new TestConfigLoader([]), ['ldap' => ['activate' => $active]]);
@@ -40,7 +46,7 @@ class LdapAuthenticationProviderTest extends TestCase
{ {
$manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock();
$config = $this->getConfiguration(false); $config = $this->getConfiguration(false);
$userProvider = new LdapUserProvider($manager); $userProvider = $this->getUserProvider();
$providerKey = 'secured_area'; $providerKey = 'secured_area';
$userChecker = new UserChecker(); $userChecker = new UserChecker();
@@ -55,7 +61,7 @@ class LdapAuthenticationProviderTest extends TestCase
{ {
$manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock();
$config = $this->getConfiguration(true); $config = $this->getConfiguration(true);
$userProvider = new LdapUserProvider($manager); $userProvider = $this->getUserProvider();
$providerKey = 'secured_area'; $providerKey = 'secured_area';
$userChecker = new UserChecker(); $userChecker = new UserChecker();
@@ -73,7 +79,7 @@ class LdapAuthenticationProviderTest extends TestCase
$manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock();
$config = $this->getConfiguration(true); $config = $this->getConfiguration(true);
$userProvider = new LdapUserProvider($manager); $userProvider = $this->getUserProvider();
$providerKey = 'secured_area'; $providerKey = 'secured_area';
$userChecker = new UserChecker(); $userChecker = new UserChecker();
@@ -92,7 +98,7 @@ class LdapAuthenticationProviderTest extends TestCase
$user = (new User())->setUsername('foo')->setEnabled(true); $user = (new User())->setUsername('foo')->setEnabled(true);
$manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock();
$config = $this->getConfiguration(true); $config = $this->getConfiguration(true);
$userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock();
$userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($user); $userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($user);
$providerKey = 'secured_area'; $providerKey = 'secured_area';
$userChecker = new UserChecker(); $userChecker = new UserChecker();
@@ -112,7 +118,7 @@ class LdapAuthenticationProviderTest extends TestCase
$manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->onlyMethods(['bind'])->getMock(); $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->onlyMethods(['bind'])->getMock();
$manager->expects($this->once())->method('bind')->willReturn(false); $manager->expects($this->once())->method('bind')->willReturn(false);
$config = $this->getConfiguration(true); $config = $this->getConfiguration(true);
$userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock();
$userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($user); $userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($user);
$providerKey = 'secured_area'; $providerKey = 'secured_area';
$userChecker = new UserChecker(); $userChecker = new UserChecker();
@@ -132,7 +138,7 @@ class LdapAuthenticationProviderTest extends TestCase
$manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->onlyMethods(['bind'])->getMock(); $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->onlyMethods(['bind'])->getMock();
$manager->expects($this->once())->method('bind')->willReturn(false); $manager->expects($this->once())->method('bind')->willReturn(false);
$config = $this->getConfiguration(true); $config = $this->getConfiguration(true);
$userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock();
$userProvider->expects($this->never())->method('loadUserByUsername'); $userProvider->expects($this->never())->method('loadUserByUsername');
$providerKey = 'secured_area'; $providerKey = 'secured_area';
$userChecker = new UserChecker(); $userChecker = new UserChecker();
@@ -153,7 +159,7 @@ class LdapAuthenticationProviderTest extends TestCase
self::assertSame($updateUser, $user); self::assertSame($updateUser, $user);
}); });
$config = $this->getConfiguration(true); $config = $this->getConfiguration(true);
$userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock();
$userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($user); $userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($user);
$providerKey = 'secured_area'; $providerKey = 'secured_area';
$userChecker = new UserChecker(); $userChecker = new UserChecker();
@@ -175,7 +181,7 @@ class LdapAuthenticationProviderTest extends TestCase
self::assertSame($updateUser, $user); self::assertSame($updateUser, $user);
}); });
$config = $this->getConfiguration(true); $config = $this->getConfiguration(true);
$userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock();
$userProvider->expects($this->never())->method('loadUserByUsername'); $userProvider->expects($this->never())->method('loadUserByUsername');
$providerKey = 'secured_area'; $providerKey = 'secured_area';
$userChecker = new UserChecker(); $userChecker = new UserChecker();
@@ -194,7 +200,7 @@ class LdapAuthenticationProviderTest extends TestCase
$manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock();
$config = $this->getConfiguration(true); $config = $this->getConfiguration(true);
$userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock();
$userProvider->expects($this->once())->method('loadUserByUsername')->willThrowException(new UsernameNotFoundException('blub foo bar')); $userProvider->expects($this->once())->method('loadUserByUsername')->willThrowException(new UsernameNotFoundException('blub foo bar'));
$providerKey = 'secured_area'; $providerKey = 'secured_area';
$userChecker = new UserChecker(); $userChecker = new UserChecker();
@@ -213,7 +219,7 @@ class LdapAuthenticationProviderTest extends TestCase
$manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock();
$config = $this->getConfiguration(true); $config = $this->getConfiguration(true);
$userProvider = $this->getMockBuilder(LdapUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock();
$userProvider->expects($this->once())->method('loadUserByUsername')->willThrowException(new \Exception('server away', 1234)); $userProvider->expects($this->once())->method('loadUserByUsername')->willThrowException(new \Exception('server away', 1234));
$providerKey = 'secured_area'; $providerKey = 'secured_area';
$userChecker = new UserChecker(); $userChecker = new UserChecker();