fix deprecations (move same controller) (#2440)

config/routes/annotations.yaml

@@ -11,3 +11,8 @@ app.api:
     resource: '../src/API/'
     type: rest
     prefix: /api
+
+security:
+    resource: '../../src/Controller/Auth/'
+    type: annotation
+    prefix: /auth

config/services.yaml

@@ -20,7 +20,7 @@ services:
     # this creates a service per class whose id is the fully-qualified class name
     App\:
         resource: '../src/*'
-        exclude: '../src/{DependencyInjection,Entity,Event,Migrations,Tests,Kernel.php,Constants.php}'
+        exclude: '../src/{DependencyInjection,Entity,Event,Migrations,Model,Tests,Kernel.php,Constants.php}'
 
     # controllers are imported separately to make sure services can be injected
     # as action arguments even if you don't extend any base controller class

src/Configuration/SystemConfiguration.php

@@ -23,6 +23,13 @@ class SystemConfiguration implements SystemBundleConfiguration
         return $repository->getConfiguration();
     }
 
+    // ========== SAML configurations ==========
+
+    public function isSamlActive(): bool
+    {
+        return (bool) $this->find('saml.activate');
+    }
+
     // ========== Calendar configurations ==========
 
     public function getCalendarBusinessDays(): array

src/Controller/Auth/SamlController.php

@@ -7,8 +7,9 @@
  * file that was distributed with this source code.
  */
 
-namespace App\Saml\Controller;
+namespace App\Controller\Auth;
 
+use App\Configuration\SystemConfiguration;
 use App\Saml\SamlAuth;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\HttpFoundation\Request;
@@ -21,14 +22,13 @@ use Symfony\Component\Security\Core\Security;
  */
 final class SamlController extends AbstractController
 {
-    /**
-     * @var SamlAuth
-     */
     private $oneLoginAuth;
+    private $systemConfiguration;
 
-    public function __construct(SamlAuth $oneLoginAuth)
+    public function __construct(SamlAuth $oneLoginAuth, SystemConfiguration $systemConfiguration)
     {
         $this->oneLoginAuth = $oneLoginAuth;
+        $this->systemConfiguration = $systemConfiguration;
     }
 
     /**
@@ -36,6 +36,10 @@ final class SamlController extends AbstractController
      */
     public function loginAction(Request $request)
     {
+        if (!$this->systemConfiguration->isSamlActive()) {
+            throw $this->createNotFoundException('SAML deactivated');
+        }
+
         $session = $request->getSession();
         $authErrorKey = Security::AUTHENTICATION_ERROR;
 
@@ -60,6 +64,10 @@ final class SamlController extends AbstractController
      */
     public function metadataAction()
     {
+        if (!$this->systemConfiguration->isSamlActive()) {
+            throw $this->createNotFoundException('SAML deactivated');
+        }
+
         $metadata = $this->oneLoginAuth->getSettings()->getSPMetadata();
 
         $response = new Response($metadata);
@@ -73,6 +81,10 @@ final class SamlController extends AbstractController
      */
     public function assertionConsumerServiceAction()
     {
+        if (!$this->systemConfiguration->isSamlActive()) {
+            throw $this->createNotFoundException('SAML deactivated');
+        }
+
         throw new \RuntimeException('You must configure the check path in your firewall.');
     }
 
@@ -81,6 +93,10 @@ final class SamlController extends AbstractController
      */
     public function logoutAction()
     {
+        if (!$this->systemConfiguration->isSamlActive()) {
+            throw $this->createNotFoundException('SAML deactivated');
+        }
+
         throw new \RuntimeException('You must configure the logout path in your firewall.');
     }
 }

src/Kernel.php

@@ -115,15 +115,13 @@ class Kernel extends BaseKernel
                     yield new $class();
                 }
             }
-
+        } else {
-            return;
+            // ... or we load them dynamically from the plugins directory
-        }
+            foreach ($this->getBundleDirectories() as $bundleDir) {
-
+                $bundleName = $bundleDir->getRelativePathname();
-        // ... or we load them dynamically from the plugins directory
+                $pluginClass = 'KimaiPlugin\\' . $bundleName . '\\' . $bundleName;
-        foreach ($this->getBundleDirectories() as $bundleDir) {
+                yield new $pluginClass();
-            $bundleName = $bundleDir->getRelativePathname();
+            }
-            $pluginClass = 'KimaiPlugin\\' . $bundleName . '\\' . $bundleName;
-            yield new $pluginClass();
         }
     }
 
@@ -209,7 +207,6 @@ class Kernel extends BaseKernel
 
         // some routes are based on app configs and will be imported manually
         $this->configureFosUserRoutes($routes);
-        $this->configureSamlRoutes($routes);
 
         // load bundle specific route files
         if (is_dir($confDir . '/routes/')) {
@@ -251,15 +248,4 @@ class Kernel extends BaseKernel
             );
         }
     }
-
-    protected function configureSamlRoutes(RouteCollectionBuilder $routes)
-    {
-        $saml = $this->getContainer()->getParameter('kimai.saml');
-
-        if (!$saml['activate']) {
-            return;
-        }
-
-        $routes->import('../src/Saml/Controller/SamlController.php', '/auth', 'annotation');
-    }
 }

tests/Configuration/SystemConfigurationTest.php

@@ -91,7 +91,11 @@ class SystemConfigurationTest extends TestCase
                     ]
                 ],
                 'weekends' => true,
-            ]
+            ],
+            'saml' => [
+                'activate' => false,
+                'title' => 'Fantastic OAuth login'
+            ],
         ];
     }
 
@@ -135,14 +139,17 @@ class SystemConfigurationTest extends TestCase
         $this->assertEquals('RUB', $sut->find('defaults.customer.currency'));
         $this->assertTrue($sut->find('timesheet.rules.allow_future_times'));
         $this->assertEquals(7, $sut->find('timesheet.active_entries.hard_limit'));
+        $this->assertFalse($sut->isSamlActive());
     }
 
     public function testDefaultWithMixedConfigs()
     {
         $sut = $this->getSut($this->getDefaultSettings(), [
             (new Configuration())->setName('timesheet.rules.allow_future_times')->setValue(''),
+            (new Configuration())->setName('saml.activate')->setValue(true),
         ]);
         $this->assertFalse($sut->find('timesheet.rules.allow_future_times'));
+        $this->assertTrue($sut->isSamlActive());
     }
 
     public function testUnknownConfigs()

tests/Controller/Auth/SamlControllerTest.php

@@ -7,15 +7,18 @@
  * file that was distributed with this source code.
  */
 
-namespace App\Tests\Saml\Controller;
+namespace App\Tests\Controller\Auth;
 
-use App\Saml\Controller\SamlController;
+use App\Configuration\SystemConfiguration;
+use App\Controller\Auth\SamlController;
+use App\Tests\Configuration\TestConfigLoader;
 use App\Tests\Mocks\Saml\SamlAuthFactory;
 use PHPUnit\Framework\TestCase;
 use PHPUnit\Util\Xml;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpFoundation\Session\SessionInterface;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\Security\Core\Security;
 
 /**
@@ -23,18 +26,44 @@ use Symfony\Component\Security\Core\Security;
  */
 class SamlControllerTest extends TestCase
 {
+    /**
+     * @param array $settings
+     * @param array $loaderSettings
+     * @return SystemConfiguration
+     */
+    protected function getSystemConfigurationMock(array $settings, array $loaderSettings = [])
+    {
+        $loader = new TestConfigLoader($loaderSettings);
+
+        return new SystemConfiguration($loader, $settings);
+    }
+
+    protected function getDefaultSettings(bool $activated = true)
+    {
+        return [
+            'saml' => [
+                'activate' => $activated,
+            ]
+        ];
+    }
+
     protected function getAuth()
     {
         return (new SamlAuthFactory($this))->create();
     }
 
+    protected function getSystemConfiguration(bool $activated = true)
+    {
+        return $this->getSystemConfigurationMock($this->getDefaultSettings($activated), []);
+    }
+
     public function testAssertionConsumerServiceAction()
     {
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionMessage('You must configure the check path in your firewall.');
 
         $oauth = $this->getAuth();
-        $sut = new SamlController($oauth);
+        $sut = new SamlController($oauth, $this->getSystemConfiguration());
         $sut->assertionConsumerServiceAction();
     }
 
@@ -44,7 +73,7 @@ class SamlControllerTest extends TestCase
         $this->expectExceptionMessage('You must configure the logout path in your firewall.');
 
         $oauth = $this->getAuth();
-        $sut = new SamlController($oauth);
+        $sut = new SamlController($oauth, $this->getSystemConfiguration());
         $sut->logoutAction();
     }
 
@@ -75,7 +104,7 @@ class SamlControllerTest extends TestCase
 EOD;
 
         $oauth = $this->getAuth();
-        $sut = new SamlController($oauth);
+        $sut = new SamlController($oauth, $this->getSystemConfiguration());
         $result = $sut->metadataAction();
 
         self::assertInstanceOf(Response::class, $result);
@@ -98,7 +127,43 @@ EOD;
         $request->attributes->set(Security::AUTHENTICATION_ERROR, new \Exception('My test error'));
 
         $oauth = $this->getAuth();
-        $sut = new SamlController($oauth);
+        $sut = new SamlController($oauth, $this->getSystemConfiguration());
         $sut->loginAction($request);
     }
+
+    public function testLoginActionThrowsExceptionOnDisabledSaml()
+    {
+        $this->expectException(NotFoundHttpException::class);
+        $this->expectExceptionMessage('SAML deactivated');
+
+        $sut = new SamlController($this->getAuth(), $this->getSystemConfiguration(false));
+        $sut->loginAction(new Request());
+    }
+
+    public function testMetadataActionThrowsExceptionOnDisabledSaml()
+    {
+        $this->expectException(NotFoundHttpException::class);
+        $this->expectExceptionMessage('SAML deactivated');
+
+        $sut = new SamlController($this->getAuth(), $this->getSystemConfiguration(false));
+        $sut->metadataAction();
+    }
+
+    public function testLogoutActionThrowsExceptionOnDisabledSaml()
+    {
+        $this->expectException(NotFoundHttpException::class);
+        $this->expectExceptionMessage('SAML deactivated');
+
+        $sut = new SamlController($this->getAuth(), $this->getSystemConfiguration(false));
+        $sut->logoutAction();
+    }
+
+    public function testAcsActionThrowsExceptionOnDisabledSaml()
+    {
+        $this->expectException(NotFoundHttpException::class);
+        $this->expectExceptionMessage('SAML deactivated');
+
+        $sut = new SamlController($this->getAuth(), $this->getSystemConfiguration(false));
+        $sut->assertionConsumerServiceAction();
+    }
 }

tests/EventSubscriber/Actions/TimesheetSubscriberTest.php

@@ -12,6 +12,8 @@ namespace App\Tests\EventSubscriber\Actions;
 use App\EventSubscriber\Actions\TimesheetSubscriber;
 
 /**
+ * @covers \App\EventSubscriber\Actions\AbstractActionsSubscriber
+ * @covers \App\EventSubscriber\Actions\AbstractTimesheetSubscriber
  * @covers \App\EventSubscriber\Actions\TimesheetSubscriber
  */
 class TimesheetSubscriberTest extends AbstractActionsSubscriberTest

tests/EventSubscriber/Actions/TimesheetTeamMultiUpdateSubscriberTest.php

@@ -12,6 +12,8 @@ namespace App\Tests\EventSubscriber\Actions;
 use App\EventSubscriber\Actions\TimesheetTeamMultiUpdateSubscriber;
 
 /**
+ * @covers \App\EventSubscriber\Actions\AbstractActionsSubscriber
+ * @covers \App\EventSubscriber\Actions\TimesheetTeamMultiUpdateSubscriber
  * @covers \App\EventSubscriber\Actions\TimesheetTeamMultiUpdateSubscriber
  */
 class TimesheetTeamMultiUpdateSubscriberTest extends AbstractActionsSubscriberTest

tests/EventSubscriber/Actions/TimesheetsSubscriberTest.php

@@ -12,6 +12,8 @@ namespace App\Tests\EventSubscriber\Actions;
 use App\EventSubscriber\Actions\TimesheetsSubscriber;
 
 /**
+ * @covers \App\EventSubscriber\Actions\AbstractActionsSubscriber
+ * @covers \App\EventSubscriber\Actions\AbstractTimesheetsSubscriber
  * @covers \App\EventSubscriber\Actions\TimesheetsSubscriber
  */
 class TimesheetsSubscriberTest extends AbstractActionsSubscriberTest

tests/EventSubscriber/Actions/TimesheetsTeamSubscriberTest.php

@@ -12,6 +12,8 @@ namespace App\Tests\EventSubscriber\Actions;
 use App\EventSubscriber\Actions\TimesheetsTeamSubscriber;
 
 /**
+ * @covers \App\EventSubscriber\Actions\AbstractActionsSubscriber
+ * @covers \App\EventSubscriber\Actions\AbstractTimesheetsSubscriber
  * @covers \App\EventSubscriber\Actions\TimesheetsTeamSubscriber
  */
 class TimesheetsTeamSubscriberTest extends AbstractActionsSubscriberTest