Repository: hwi/HWIOAuthBundle (php)

Issue:
The "session" service and "SessionInterface" alias are deprecated
Hi, I'm in the process of upgrading our app to Symfony 5.3, and got these deprecation notices:

> Since symfony/framework-bundle 5.3: The "session" service and "SessionInterface" alias are deprecated, use "$requestStack->getSession()" instead. It is being referenced by the "HWI\Bundle\OAuthBundle\Controller\LoginController" service.

> Since symfony/framework-bundle 5.3: The "session" service and "SessionInterface" alias are deprecated, use "$requestStack->getSession()" instead. It is being referenced by the "hwi_oauth.storage.session" service.

We're using the latest version, `1.3.0`.

Would you accept a PR? Or do you prefer to handle it? It should be a pretty straightforward fix.

Pull request:
Fix deprecation of SessionInterface as service
Fixes #1722.

Replaces #1723

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2950286..994522f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,8 +1,9 @@
 Changelog
 =========
 ## 1.4.0 (2021-xx-xx)
-* Dropped Symfony 5.0 support as it is EOL,
-* Dropped PHP 7.2 support as it is EOL,
+* BC Break: dropped Symfony 5.0 support as it is EOL,
+* BC Break: dropped PHP 7.2 support as it is EOL,
+* BC Break: changed `__construct()` argument for `OAuth/RequestDataStorage/SessionStorage`, from `SessionInterface $session` to `RequestStack $requestStack`,
 * Added: Symfony 5.1 Security system support,
 * Added: state support for service authentication URL's,
 * Added: ability to change the response after `HWIOAuthEvents::CONNECT_COMPLETED` is fired,
diff --git a/Controller/ConnectController.php b/Controller/ConnectController.php
index 56654eb..ab27fe5 100644
--- a/Controller/ConnectController.php
+++ b/Controller/ConnectController.php
@@ -24,6 +24,7 @@ use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\Form\Extension\Core\Type\FormType;
 use Symfony\Component\Form\FormInterface;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpFoundation\Session\SessionInterface;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
@@ -49,10 +50,19 @@ final class ConnectController extends AbstractController
      */
     private $resourceOwnerMapLocator;
 
-    public function __construct(OAuthUtils $oauthUtils, ResourceOwnerMapLocator $resourceOwnerMapLocator)
-    {
+    /**
+     * @var RequestStack
+     */
+    private $requestStack;
+
+    public function __construct(
+        OAuthUtils $oauthUtils,
+        ResourceOwnerMapLocator $resourceOwnerMapLocator,
+        RequestStack $requestStack
+    ) {
         $this->oauthUtils = $oauthUtils;
         $this->resourceOwnerMapLocator = $resourceOwnerMapLocator;
+        $this->requestStack = $requestStack;
     }
 
     /**
@@ -78,7 +88,7 @@ final class ConnectController extends AbstractController
         }
 
         $error = null;
-        $session = $request->hasSession() ? $request->getSession() : $this->get('session');
+        $session = $request->hasSession() ? $request->getSession() : $this->getSession();
         if ($session) {
             if (!$session->isStarted()) {
                 $session->start();
@@ -168,7 +178,7 @@ final class ConnectController extends AbstractController
         // Get the data from the resource owner
         $resourceOwner = $this->getResourceOwnerByName($service);
 
-        $session = $request->hasSession() ? $request->getSession() : $this->get('session');
+        $session = $request->hasSession() ? $request->getSession() : $this->getSession();
         if ($session && !$session->isStarted()) {
             $session->start();
         }
@@ -354,4 +364,17 @@ final class ConnectController extends AbstractController
     {
         $this->get('event_dispatcher')->dispatch($event, $eventName);
     }
+
+    private function getSession(): ?SessionInterface
+    {
+        if (method_exists($this->requestStack, 'getSession')) {
+            return $this->requestStack->getSession();
+        }
+
+        if ((null !== $request = $this->requestStack->getCurrentRequest()) && $request->hasSession()) {
+            return $request->getSession();
+        }
+
+        return null;
+    }
 }
diff --git a/Controller/LoginController.php b/Controller/LoginController.php
index 4d398ed..98fec16 100644
--- a/Controller/LoginController.php
+++ b/Controller/LoginController.php
@@ -15,6 +15,7 @@ use HWI\Bundle\OAuthBundle\Security\Core\Exception\AccountNotLinkedException;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpFoundation\Session\SessionInterface;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
@@ -54,22 +55,22 @@ final class LoginController extends AbstractController
     private $authorizationChecker;
 
     /**
-     * @var SessionInterface
+     * @var RequestStack
      */
-    private $session;
+    private $requestStack;
 
     public function __construct(
         AuthenticationUtils $authenticationUtils,
         RouterInterface $router,
         AuthorizationCheckerInterface $authorizationChecker,
-        SessionInterface $session,
+        RequestStack $requestStack,
         bool $connect,
         string $grantRule
     ) {
         $this->authenticationUtils = $authenticationUtils;
         $this->router = $router;
         $this->authorizationChecker = $authorizationChecker;
-        $this->session = $session;
+        $this->requestStack = $requestStack;
         $this->connect = $connect;
         $this->grantRule = $grantRule;
     }
@@ -93,7 +94,7 @@ final class LoginController extends AbstractController
         // if connecting is enabled and there is no user, redirect to the registration form
         if ($this->connect && !$hasUser && $error instanceof AccountNotLinkedException) {
             $key = time();
-            $session = $request->hasSession() ? $request->getSession() : $this->session;
+            $session = $request->hasSession() ? $request->getSession() : $this->getSession();
             if ($session) {
                 if (!$session->isStarted()) {
                     $session->start();
@@ -113,4 +114,17 @@ final class LoginController extends AbstractController
             'error' => $error,
         ]);
     }
+
+    private function getSession(): ?SessionInterface
+    {
+        if (method_exists($this->requestStack, 'getSession')) {
+            return $this->requestStack->getSession();
+        }
+
+        if ((null !== $request = $this->requestStack->getCurrentRequest()) && $request->hasSession()) {
+            return $request->getSession();
+        }
+
+        return null;
+    }
 }
diff --git a/OAuth/RequestDataStorage/SessionStorage.php b/OAuth/RequestDataStorage/SessionStorage.php
index 2d466b5..b961a36 100644
--- a/OAuth/RequestDataStorage/SessionStorage.php
+++ b/OAuth/RequestDataStorage/SessionStorage.php
@@ -13,6 +13,7 @@ namespace HWI\Bundle\OAuthBundle\OAuth\RequestDataStorage;
 
 use HWI\Bundle\OAuthBundle\OAuth\RequestDataStorageInterface;
 use HWI\Bundle\OAuthBundle\OAuth\ResourceOwnerInterface;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Session\SessionInterface;
 
 /**
@@ -21,17 +22,19 @@ use Symfony\Component\HttpFoundation\Session\SessionInterface;
  * @author Alexander <iam.asm89@gmail.com>
  * @author Francisco Facioni <fran6co@gmail.com>
  * @author Joseph Bielawski <stloyd@gmail.com>
+ *
+ * @final since 1.4
  */
 class SessionStorage implements RequestDataStorageInterface
 {
     /**
-     * @var SessionInterface
+     * @var RequestStack
      */
-    private $session;
+    private $requestStack;
 
-    public function __construct(SessionInterface $session)
+    public function __construct(RequestStack $requestStack)
     {
-        $this->session = $session;
+        $this->requestStack = $requestStack;
     }
 
     /**
@@ -40,13 +43,13 @@ class SessionStorage implements RequestDataStorageInterface
     public function fetch(ResourceOwnerInterface $resourceOwner, $key, $type = 'token')
     {
         $key = $this->generateKey($resourceOwner, $key, $type);
-        if (null === $data = $this->session->get($key)) {
+        if (null === $data = $this->getSession()->get($key)) {
             throw new \InvalidArgumentException('No data available in storage.');
         }
 
-        // request tokens are one time use only
-        if (\in_array($type, ['token', 'csrf_state'])) {
-            $this->session->remove($key);
+        // Request tokens are one time use only
+        if (\in_array($type, ['token', 'csrf_state'], true)) {
+            $this->getSession()->remove($key);
         }
 
         return $data;
@@ -67,7 +70,7 @@ class SessionStorage implements RequestDataStorageInterface
             $key = $this->generateKey($resourceOwner, $this->getStorageKey($value), $type);
         }
 
-        $this->session->set($key, $this->getStorageValue($value));
+        $this->getSession()->set($key, $this->getStorageValue($value));
     }
 
     /**
@@ -112,4 +115,17 @@ class SessionStorage implements RequestDataStorageInterface
 
         return (string) $storageKey;
     }
+
+    private function getSession(): SessionInterface
+    {
+        if (method_exists($this->requestStack, 'getSession')) {
+            return $this->requestStack->getSession();
+        }
+
+        if ((null !== $request = $this->requestStack->getCurrentRequest()) && $request->hasSession()) {
+            return $request->getSession();
+        }
+
+        throw new \LogicException('There is currently no session available.');
+    }
 }
diff --git a/Resources/config/controller.xml b/Resources/config/controller.xml
index 12bbc71..b6453aa 100644
--- a/Resources/config/controller.xml
+++ b/Resources/config/controller.xml
@@ -8,6 +8,7 @@
         <service id="HWI\Bundle\OAuthBundle\Controller\ConnectController" public="true">
             <argument type="service" id="hwi_oauth.security.oauth_utils" />
             <argument type="service" id="hwi_oauth.resource_ownermap_locator" />
+            <argument type="service" id="request_stack" />
 
             <call method="setContainer">
                 <argument type="service" id="service_container" />
@@ -20,7 +21,7 @@
             <argument type="service" id="security.authentication_utils" />
             <argument type="service" id="router" />
             <argument type="service" id="security.authorization_checker" />
-            <argument type="service" id="session" />
+            <argument type="service" id="request_stack" />
             <argument>%hwi_oauth.connect%</argument>
             <argument>%hwi_oauth.grant_rule%</argument>
 
diff --git a/Resources/config/oauth.xml b/Resources/config/oauth.xml
index 7932c6e..a2f01ff 100644
--- a/Resources/config/oauth.xml
+++ b/Resources/config/oauth.xml
@@ -106,7 +106,7 @@
 
         <!-- Session storage -->
         <service id="hwi_oauth.storage.session" class="HWI\Bundle\OAuthBundle\OAuth\RequestDataStorage\SessionStorage">
-            <argument type="service" id="session" />
+            <argument type="service" id="request_stack" />
         </service>
 
         <service id="hwi_oauth.security.oauth_utils" class="HWI\Bundle\OAuthBundle\Security\OAuthUtils">


Test patch:
diff --git a/Tests/Controller/AbstractConnectControllerTest.php b/Tests/Controller/AbstractConnectControllerTest.php
index 085340b..a9a6f96 100644
--- a/Tests/Controller/AbstractConnectControllerTest.php
+++ b/Tests/Controller/AbstractConnectControllerTest.php
@@ -28,6 +28,7 @@ use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBa
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\Form\FormFactoryInterface;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Session\SessionInterface;
 use Symfony\Component\Routing\RouterInterface;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
@@ -182,7 +183,7 @@ abstract class AbstractConnectControllerTest extends TestCase
         $this->request = Request::create('/');
         $this->request->setSession($this->session);
 
-        $this->controller = new ConnectController($this->oAuthUtils, $this->resourceOwnerMapLocator);
+        $this->controller = new ConnectController($this->oAuthUtils, $this->resourceOwnerMapLocator, $this->createMock(RequestStack::class));
         $this->controller->setContainer($this->container);
     }
 
diff --git a/Tests/Controller/LoginControllerTest.php b/Tests/Controller/LoginControllerTest.php
index bcad03e..6e33664 100644
--- a/Tests/Controller/LoginControllerTest.php
+++ b/Tests/Controller/LoginControllerTest.php
@@ -60,6 +60,11 @@ class LoginControllerTest extends TestCase
      */
     private $session;
 
+    /**
+     * @var MockObject|RequestStack
+     */
+    private $requestStack;
+
     /**
      * @var Request
      */
@@ -92,6 +97,9 @@ class LoginControllerTest extends TestCase
         $this->request = Request::create('/');
         $this->request->setSession($this->session);
 
+        $this->requestStack = $this->createMock(RequestStack::class);
+        $this->requestStack->push($this->request);
+
         $requestStack = new RequestStack();
         $requestStack->push($this->request);
         $this->authenticationUtils = new AuthenticationUtils($requestStack);
@@ -216,7 +224,7 @@ class LoginControllerTest extends TestCase
             $this->authenticationUtils,
             $this->router,
             $this->authorizationChecker,
-            $this->session,
+            $this->requestStack,
             $connect,
             $grantRule
         );
diff --git a/Tests/Functional/Controller/ConnectControllerTest.php b/Tests/Functional/Controller/ConnectControllerTest.php
index 76dba18..355a62d 100644
--- a/Tests/Functional/Controller/ConnectControllerTest.php
+++ b/Tests/Functional/Controller/ConnectControllerTest.php
@@ -110,7 +110,7 @@ final class ConnectControllerTest extends WebTestCase
 
         $this->createDatabase($client);
 
-        $session = $client->getContainer()->get('session');
+        $session = $client->getContainer()->get('request_stack')->getSession();
         $key = 1;
         $session->set('_hwi_oauth.connect_confirmation.'.$key, ['access_token' => 'valid-access-token']);
         $this->logIn($client, $session);
diff --git a/Tests/Functional/Controller/LoginControllerTest.php b/Tests/Functional/Controller/LoginControllerTest.php
index ebe1e7f..277cdcc 100644
--- a/Tests/Functional/Controller/LoginControllerTest.php
+++ b/Tests/Functional/Controller/LoginControllerTest.php
@@ -69,7 +69,7 @@ final class LoginControllerTest extends WebTestCase
         $client = static::createClient();
         $httpClient = $this->prophesize(ClientInterface::class);
         $client->getContainer()->set(ClientInterface::class, $httpClient->reveal());
-        $session = $client->getContainer()->get('session');
+        $session = $client->getContainer()->get('request_stack')->getSession();
 
         $this->logIn($client, $session);
         $exception = new UsernameNotFoundException();
diff --git a/Tests/OAuth/RequestDataStorage/SessionStorageTest.php b/Tests/OAuth/RequestDataStorage/SessionStorageTest.php
index e61d878..bcfb50d 100644
--- a/Tests/OAuth/RequestDataStorage/SessionStorageTest.php
+++ b/Tests/OAuth/RequestDataStorage/SessionStorageTest.php
@@ -15,6 +15,8 @@ use HWI\Bundle\OAuthBundle\OAuth\RequestDataStorage\SessionStorage;
 use HWI\Bundle\OAuthBundle\OAuth\ResourceOwnerInterface;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Session\SessionInterface;
 
 class SessionStorageTest extends TestCase
@@ -41,7 +43,18 @@ class SessionStorageTest extends TestCase
         $this->resourceOwner->method('getName')->willReturn('resource_owner_name');
         $this->resourceOwner->method('getOption')->with('client_id')->willReturn('client_id');
 
-        $this->storage = new SessionStorage($this->session);
+        $request = $this->createMock(Request::class);
+        $request->method('hasSession')->willReturn(true);
+        $request->method('getSession')->willReturn($this->session);
+
+        $requestStack = $this->createMock(RequestStack::class);
+        $requestStack->method('getCurrentRequest')->willReturn($request);
+
+        if (method_exists(RequestStack::class, 'getSession')) {
+            $requestStack->method('getSession')->willReturn($this->session);
+        }
+
+        $this->storage = new SessionStorage($requestStack);
     }
 
     public function testSaveTokenWithoutOAuthTokenPassedThrowsException(): void