From b852ccde4c31a1c63fa1727bb82827021ba76a5f Mon Sep 17 00:00:00 2001
From: Joseph Bielawski <stloyd@gmail.com>
Date: Thu, 9 Dec 2021 19:46:36 +0100
Subject: [PATCH] Bugfix | Fixed issue when `connect` configuration is not set
 but `ConnectController` was used

---
 Controller/ConnectController.php              | 17 +++++-----
 DependencyInjection/HWIOAuthExtension.php     | 31 +++++--------------
 Resources/config/controller.xml               |  7 +++--
 .../AbstractConnectControllerTest.php         |  9 +++---
 .../RedirectToServiceControllerTest.php       | 10 +++---
 .../HWIOAuthExtensionTest.php                 |  4 +--
 6 files changed, 30 insertions(+), 48 deletions(-)

diff --git a/Controller/ConnectController.php b/Controller/ConnectController.php
index 0765cb939..8218748f1 100644
--- a/Controller/ConnectController.php
+++ b/Controller/ConnectController.php
@@ -56,14 +56,13 @@ final class ConnectController
     private RequestStack $requestStack;
     private EventDispatcherInterface $dispatcher;
     private TokenStorageInterface $tokenStorage;
-    private AccountConnectorInterface $accountConnector;
     private UserCheckerInterface $userChecker;
-    private RegistrationFormHandlerInterface $formHandler;
     private AuthorizationCheckerInterface $authorizationChecker;
     private FormFactoryInterface $formFactory;
     private Environment $twig;
     private RouterInterface $router;
-    private bool $enableConnect;
+    private ?AccountConnectorInterface $accountConnector;
+    private ?RegistrationFormHandlerInterface $formHandler;
     private string $grantRule;
     private bool $failedUseReferer;
     private string $failedAuthPath;
@@ -80,25 +79,23 @@ public function __construct(
         RequestStack $requestStack,
         EventDispatcherInterface $dispatcher,
         TokenStorageInterface $tokenStorage,
-        AccountConnectorInterface $accountConnector,
         UserCheckerInterface $userChecker,
-        RegistrationFormHandlerInterface $formHandler,
         AuthorizationCheckerInterface $authorizationChecker,
         FormFactoryInterface $formFactory,
         Environment $twig,
         RouterInterface $router,
-        bool $enableConnect,
         string $grantRule,
         bool $failedUseReferer,
         string $failedAuthPath,
         bool $enableConnectConfirmation,
         array $firewallNames,
-        string $registrationForm
+        string $registrationForm,
+        ?AccountConnectorInterface $accountConnector,
+        ?RegistrationFormHandlerInterface $formHandler
     ) {
         $this->oauthUtils = $oauthUtils;
         $this->resourceOwnerMapLocator = $resourceOwnerMapLocator;
         $this->requestStack = $requestStack;
-        $this->enableConnect = $enableConnect;
         $this->grantRule = $grantRule;
         $this->failedUseReferer = $failedUseReferer;
         $this->failedAuthPath = $failedAuthPath;
@@ -128,7 +125,7 @@ public function __construct(
      */
     public function registrationAction(Request $request, string $key): Response
     {
-        if (!$this->enableConnect) {
+        if (!$this->accountConnector || !$this->formHandler) {
             throw new NotFoundHttpException();
         }
 
@@ -213,7 +210,7 @@ public function registrationAction(Request $request, string $key): Response
      */
     public function connectServiceAction(Request $request, string $service): Response
     {
-        if (!$this->enableConnect) {
+        if (!$this->accountConnector || !$this->formHandler) {
             throw new NotFoundHttpException();
         }
 
diff --git a/DependencyInjection/HWIOAuthExtension.php b/DependencyInjection/HWIOAuthExtension.php
index df9e95ba1..43badd5b9 100644
--- a/DependencyInjection/HWIOAuthExtension.php
+++ b/DependencyInjection/HWIOAuthExtension.php
@@ -95,8 +95,6 @@ public function load(array $configs, ContainerBuilder $container)
         }
 
         $this->createConnectIntegration($container, $config);
-
-        $container->setAlias('hwi_oauth.user_checker', new Alias('security.user_checker', true));
     }
 
     /**
@@ -161,31 +159,16 @@ public function getAlias(): string
      */
     private function createConnectIntegration(ContainerBuilder $container, array $config): void
     {
-        $container->setParameter('hwi_oauth.connect.confirmation', false);
-        $container->setParameter('hwi_oauth.connect.registration_form', null);
-
-        if (!isset($config['connect'])) {
-            $container->setParameter('hwi_oauth.connect', false);
+        $container->setParameter('hwi_oauth.connect', isset($config['connect']));
+        $container->setParameter('hwi_oauth.connect.confirmation', $config['connect']['confirmation'] ?? false);
+        $container->setParameter('hwi_oauth.connect.registration_form', $config['connect']['registration_form'] ?? null);
 
-            return;
+        if (isset($config['connect']['account_connector'])) {
+            $container->setAlias('hwi_oauth.account.connector', new Alias($config['connect']['account_connector'], true));
         }
 
-        $container->setParameter('hwi_oauth.connect', true);
-
-        foreach ($config['connect'] as $key => $serviceId) {
-            if ('confirmation' === $key) {
-                $container->setParameter('hwi_oauth.connect.confirmation', $config['connect']['confirmation']);
-
-                continue;
-            }
-
-            if ('registration_form' === $key) {
-                $container->setParameter('hwi_oauth.connect.registration_form', $config['connect']['registration_form']);
-
-                continue;
-            }
-
-            $container->setAlias('hwi_oauth.'.str_replace('_', '.', $key), new Alias($serviceId, true));
+        if (isset($config['connect']['registration_form_handler'])) {
+            $container->setAlias('hwi_oauth.registration.form.handler', new Alias($config['connect']['registration_form_handler'], true));
         }
     }
 }
diff --git a/Resources/config/controller.xml b/Resources/config/controller.xml
index 71f9d504e..da6758a29 100644
--- a/Resources/config/controller.xml
+++ b/Resources/config/controller.xml
@@ -5,26 +5,27 @@
            xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
 
     <services>
+        <service id="hwi_oauth.user_checker" alias="security.user_checker" public="true" />
+
         <service id="HWI\Bundle\OAuthBundle\Controller\ConnectController" public="true">
             <argument key="$oauthUtils" type="service" id="hwi_oauth.security.oauth_utils" />
             <argument key="$resourceOwnerMapLocator" type="service" id="hwi_oauth.resource_ownermap_locator" />
             <argument key="$requestStack" type="service" id="request_stack" />
             <argument key="$dispatcher" type="service" id="event_dispatcher" />
             <argument key="$tokenStorage" type="service" id="security.token_storage" />
-            <argument key="$accountConnector" type="service" id="hwi_oauth.account.connector" />
-            <argument key="$formHandler" type="service" id="hwi_oauth.registration.form.handler" />
             <argument key="$userChecker" type="service" id="hwi_oauth.user_checker" />
             <argument key="$authorizationChecker" type="service" id="security.authorization_checker" />
             <argument key="$formFactory" type="service" id="form.factory" />
             <argument key="$twig" type="service" id="twig" />
             <argument key="$router" type="service" id="router" />
-            <argument key="$enableConnect">%hwi_oauth.connect%</argument>
             <argument key="$grantRule">%hwi_oauth.grant_rule%</argument>
             <argument key="$failedUseReferer">%hwi_oauth.failed_use_referer%</argument>
             <argument key="$failedAuthPath">%hwi_oauth.failed_auth_path%</argument>
             <argument key="$enableConnectConfirmation">%hwi_oauth.connect.confirmation%</argument>
             <argument key="$firewallNames">%hwi_oauth.firewall_names%</argument>
             <argument key="$registrationForm">%hwi_oauth.connect.registration_form%</argument>
+            <argument key="$accountConnector" type="service" id="hwi_oauth.account.connector" on-invalid="null" />
+            <argument key="$formHandler" type="service" id="hwi_oauth.registration.form.handler" on-invalid="null" />
         </service>
 
         <service id="HWI\Bundle\OAuthBundle\Controller\LoginController" public="true">
diff --git a/Tests/Controller/AbstractConnectControllerTest.php b/Tests/Controller/AbstractConnectControllerTest.php
index 0002ccd17..e71abcd57 100644
--- a/Tests/Controller/AbstractConnectControllerTest.php
+++ b/Tests/Controller/AbstractConnectControllerTest.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /*
  * This file is part of the HWIOAuthBundle package.
  *
@@ -178,20 +180,19 @@ protected function createConnectController(
             $this->createMock(RequestStack::class),
             $this->eventDispatcher,
             $this->tokenStorage,
-            $this->accountConnector,
             $this->userChecker,
-            $this->registrationFormHandler,
             $this->authorizationChecker,
             $this->formFactory,
             $this->twig,
             $this->router,
-            $connectEnabled,
             'IS_AUTHENTICATED_REMEMBERED',
             true,
             'fake_route',
             $confirmConnect,
             $firewallNames,
-            RegistrationFormType::class
+            RegistrationFormType::class,
+            $connectEnabled ? $this->accountConnector : null,
+            $connectEnabled ? $this->registrationFormHandler : null
         );
     }
 }
diff --git a/Tests/Controller/RedirectToServiceControllerTest.php b/Tests/Controller/RedirectToServiceControllerTest.php
index acbe33d06..673a8960e 100644
--- a/Tests/Controller/RedirectToServiceControllerTest.php
+++ b/Tests/Controller/RedirectToServiceControllerTest.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /*
  * This file is part of the HWIOAuthBundle package.
  *
@@ -25,7 +27,7 @@
 use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
 use Symfony\Component\Security\Http\HttpUtils;
 
-class RedirectToServiceControllerTest extends TestCase
+final class RedirectToServiceControllerTest extends TestCase
 {
     /**
      * @var MockObject&SessionInterface
@@ -47,13 +49,13 @@ protected function setUp(): void
         $this->request->setSession($this->session);
     }
 
-    public function test()
+    public function testTargetUrlIsCorrect(): void
     {
         $controller = $this->createController();
 
         $response = $controller->redirectToServiceAction($this->request, 'facebook');
 
-        $this->assertEquals('https://domain.com/oauth/v2/auth', $response->getTargetUrl());
+        $this->assertSame('https://domain.com/oauth/v2/auth', $response->getTargetUrl());
     }
 
     public function testTargetPathParameter(): void
@@ -91,7 +93,7 @@ public function testFailedUseReferer(): void
             ->with('_security.default.failed_target_path', 'https://google.com')
         ;
 
-        $controller = $this->createController(true, false);
+        $controller = $this->createController(true);
         $controller->redirectToServiceAction($this->request, 'facebook');
     }
 
diff --git a/Tests/DependencyInjection/HWIOAuthExtensionTest.php b/Tests/DependencyInjection/HWIOAuthExtensionTest.php
index f1f0cb5e5..7553c5978 100644
--- a/Tests/DependencyInjection/HWIOAuthExtensionTest.php
+++ b/Tests/DependencyInjection/HWIOAuthExtensionTest.php
@@ -360,7 +360,6 @@ public function testConfigurationLoadDefaults(): void
         $this->assertParameter('hwi_oauth_connect', 'hwi_oauth.failed_auth_path');
         $this->assertParameter(['any_name' => 'any_name', 'some_service' => 'some_service'], 'hwi_oauth.resource_owners');
 
-        $this->assertParameter(false, 'hwi_oauth.connect');
         $this->assertParameter(false, 'hwi_oauth.connect.confirmation');
 
         $this->assertAlias('security.user_checker', 'hwi_oauth.user_checker');
@@ -637,9 +636,8 @@ protected function getFullConfig(): array
 
 templating_engine: "php"
 EOF;
-        $parser = new Parser();
 
-        return $parser->parse($yaml);
+        return (new Parser())->parse($yaml);
     }
 
     private function assertAlias(string $value, string $key): void