fix ldap issues due to new security components (#2689)

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:

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;

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);
 

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

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\Ldap\LdapUserProvider;
-use App\Security\DoctrineUserProvider;
 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 = $this->getUserProvider($user);
-        $userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($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 = $this->getUserProvider($user);
-        $userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($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 = $this->getUserProvider($user);
-        $userProvider->expects($this->once())->method('loadUserByUsername')->willReturn($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();

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 ');
+        $now = $sut->createDateTime();
-        $financial = $sut->createStartOfFinancialYear('2020-07-23 15:30:00');
+        $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);