From 5b8510be56f80496c26dda39795dc915fd31adbf Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Tue, 27 Jul 2021 19:31:49 +0200 Subject: [PATCH] fix ldap issues due to new security components (#2689) --- config/packages/security.yaml | 2 +- src/Ldap/FormLoginLdapFactory.php | 2 ++ src/Ldap/LdapAuthenticationProvider.php | 4 +-- src/Security/DoctrineUserProvider.php | 5 +-- tests/Ldap/LdapAuthenticationProviderTest.php | 34 +++++++++++-------- tests/Timesheet/DateTimeFactoryTest.php | 12 +++++-- 6 files changed, 36 insertions(+), 23 deletions(-) diff --git a/config/packages/security.yaml b/config/packages/security.yaml index 85e929a1..a192b691 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -5,7 +5,7 @@ security: providers: chain_provider: chain: - providers: [kimai_ldap,kimai_internal] + providers: [kimai_internal,kimai_ldap] kimai_ldap: id: App\Ldap\LdapUserProvider kimai_internal: diff --git a/src/Ldap/FormLoginLdapFactory.php b/src/Ldap/FormLoginLdapFactory.php index 1d1aa8a8..29d80bd2 100644 --- a/src/Ldap/FormLoginLdapFactory.php +++ b/src/Ldap/FormLoginLdapFactory.php @@ -13,6 +13,7 @@ use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SecurityF use Symfony\Component\Config\Definition\Builder\NodeDefinition; use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Reference; /** * Inspired by https://github.com/Maks3w/FR3DLdapBundle @ MIT License @@ -48,6 +49,7 @@ class FormLoginLdapFactory implements SecurityFactoryInterface $container ->setDefinition($providerId, new ChildDefinition(LdapAuthenticationProvider::class)) ->replaceArgument(1, $id) + ->replaceArgument(2, new Reference($userProviderId)) ; return $providerId; diff --git a/src/Ldap/LdapAuthenticationProvider.php b/src/Ldap/LdapAuthenticationProvider.php index 1efc908c..fdd31a98 100644 --- a/src/Ldap/LdapAuthenticationProvider.php +++ b/src/Ldap/LdapAuthenticationProvider.php @@ -11,7 +11,6 @@ namespace App\Ldap; use App\Configuration\LdapConfiguration; use App\Entity\User; -use App\Security\DoctrineUserProvider; use Symfony\Component\Security\Core\Authentication\Provider\UserAuthenticationProvider; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; @@ -20,6 +19,7 @@ use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Core\User\UserCheckerInterface; use Symfony\Component\Security\Core\User\UserInterface; +use Symfony\Component\Security\Core\User\UserProviderInterface; /** * Inspired by https://github.com/Maks3w/FR3DLdapBundle @ MIT License @@ -30,7 +30,7 @@ class LdapAuthenticationProvider extends UserAuthenticationProvider private $ldapManager; private $config; - public function __construct(UserCheckerInterface $userChecker, $providerKey, DoctrineUserProvider $userProvider, LdapManager $ldapManager, LdapConfiguration $config, $hideUserNotFoundExceptions = true) + public function __construct(UserCheckerInterface $userChecker, $providerKey, UserProviderInterface $userProvider, LdapManager $ldapManager, LdapConfiguration $config, $hideUserNotFoundExceptions = true) { parent::__construct($userChecker, $providerKey, $hideUserNotFoundExceptions); diff --git a/src/Security/DoctrineUserProvider.php b/src/Security/DoctrineUserProvider.php index deec079c..8646eb58 100644 --- a/src/Security/DoctrineUserProvider.php +++ b/src/Security/DoctrineUserProvider.php @@ -18,10 +18,7 @@ use Symfony\Component\Security\Core\User\PasswordUpgraderInterface; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; -/** - * @final - */ -class DoctrineUserProvider implements UserProviderInterface, PasswordUpgraderInterface +final class DoctrineUserProvider implements UserProviderInterface, PasswordUpgraderInterface { /** * @var UserRepository diff --git a/tests/Ldap/LdapAuthenticationProviderTest.php b/tests/Ldap/LdapAuthenticationProviderTest.php index 87e60c72..043cfe28 100644 --- a/tests/Ldap/LdapAuthenticationProviderTest.php +++ b/tests/Ldap/LdapAuthenticationProviderTest.php @@ -14,8 +14,7 @@ use App\Configuration\SystemConfiguration; use App\Entity\User; use App\Ldap\LdapAuthenticationProvider; use App\Ldap\LdapManager; -use App\Repository\UserRepository; -use App\Security\DoctrineUserProvider; +use App\Ldap\LdapUserProvider; use App\Tests\Configuration\TestConfigLoader; use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; @@ -23,15 +22,25 @@ use Symfony\Component\Security\Core\Exception\AuthenticationServiceException; use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Core\User\UserChecker; +use Symfony\Component\Security\Core\User\UserProviderInterface; /** * @covers \App\Ldap\LdapAuthenticationProvider */ class LdapAuthenticationProviderTest extends TestCase { - private function getUserProvider(): DoctrineUserProvider + public const USER_PROVIDER_CLASS = LdapUserProvider::class; + + private function getUserProvider(?User $user = null): UserProviderInterface { - return new DoctrineUserProvider($this->createMock(UserRepository::class)); + if ($user === null) { + return new LdapUserProvider($this->createMock(LdapManager::class)); + } + + $userProvider = $this->getMockBuilder(self::USER_PROVIDER_CLASS)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); + $userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($user); + + return $userProvider; } private function getConfiguration(bool $active = true): LdapConfiguration @@ -98,8 +107,7 @@ class LdapAuthenticationProviderTest extends TestCase $user = (new User())->setUsername('foo')->setEnabled(true); $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); $config = $this->getConfiguration(true); - $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); - $userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($user); + $userProvider = $this->getUserProvider($user); $providerKey = 'secured_area'; $userChecker = new UserChecker(); @@ -118,8 +126,7 @@ class LdapAuthenticationProviderTest extends TestCase $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->onlyMethods(['bind'])->getMock(); $manager->expects($this->once())->method('bind')->willReturn(false); $config = $this->getConfiguration(true); - $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); - $userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($user); + $userProvider = $this->getUserProvider($user); $providerKey = 'secured_area'; $userChecker = new UserChecker(); @@ -138,7 +145,7 @@ class LdapAuthenticationProviderTest extends TestCase $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->onlyMethods(['bind'])->getMock(); $manager->expects($this->once())->method('bind')->willReturn(false); $config = $this->getConfiguration(true); - $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); + $userProvider = $this->getMockBuilder(self::USER_PROVIDER_CLASS)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); $userProvider->expects($this->never())->method('loadUserByUsername'); $providerKey = 'secured_area'; $userChecker = new UserChecker(); @@ -159,8 +166,7 @@ class LdapAuthenticationProviderTest extends TestCase self::assertSame($updateUser, $user); }); $config = $this->getConfiguration(true); - $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); - $userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($user); + $userProvider = $this->getUserProvider($user); $providerKey = 'secured_area'; $userChecker = new UserChecker(); @@ -181,7 +187,7 @@ class LdapAuthenticationProviderTest extends TestCase self::assertSame($updateUser, $user); }); $config = $this->getConfiguration(true); - $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); + $userProvider = $this->getMockBuilder(self::USER_PROVIDER_CLASS)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); $userProvider->expects($this->never())->method('loadUserByUsername'); $providerKey = 'secured_area'; $userChecker = new UserChecker(); @@ -200,7 +206,7 @@ class LdapAuthenticationProviderTest extends TestCase $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); $config = $this->getConfiguration(true); - $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); + $userProvider = $this->getMockBuilder(self::USER_PROVIDER_CLASS)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); $userProvider->expects($this->once())->method('loadUserByUsername')->willThrowException(new UsernameNotFoundException('blub foo bar')); $providerKey = 'secured_area'; $userChecker = new UserChecker(); @@ -219,7 +225,7 @@ class LdapAuthenticationProviderTest extends TestCase $manager = $this->getMockBuilder(LdapManager::class)->disableOriginalConstructor()->getMock(); $config = $this->getConfiguration(true); - $userProvider = $this->getMockBuilder(DoctrineUserProvider::class)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); + $userProvider = $this->getMockBuilder(self::USER_PROVIDER_CLASS)->disableOriginalConstructor()->onlyMethods(['loadUserByUsername'])->getMock(); $userProvider->expects($this->once())->method('loadUserByUsername')->willThrowException(new \Exception('server away', 1234)); $providerKey = 'secured_area'; $userChecker = new UserChecker(); diff --git a/tests/Timesheet/DateTimeFactoryTest.php b/tests/Timesheet/DateTimeFactoryTest.php index 74e81cde..53f240c7 100644 --- a/tests/Timesheet/DateTimeFactoryTest.php +++ b/tests/Timesheet/DateTimeFactoryTest.php @@ -203,8 +203,16 @@ class DateTimeFactoryTest extends TestCase { $sut = $this->createDateTimeFactory(self::TEST_TIMEZONE); - $expected = $sut->createDateTime('2021-07-22 23:59:59 '); - $financial = $sut->createStartOfFinancialYear('2020-07-23 15:30:00'); + $now = $sut->createDateTime(); + $expected = $sut->createDateTime(); + $expected->setDate((int) $expected->format('Y'), 7, 22); + $expected->setTime(23, 59, 59); + + if ($now > $expected) { + $expected->modify('+1 year'); + } + + $financial = $sut->createStartOfFinancialYear('2018-07-23 15:30:00'); $end = $sut->createEndOfFinancialYear($financial); self::assertEquals($expected, $end);