Prevent overwriting failure_path in AuthenticationFailureHandler

CHANGELOG.md

@@ -1,5 +1,8 @@
 Changelog
 =========
+## 2.2.1 (2024-03-xx)
+* Bugfix: Prevent overwriting `failure_path` in `AuthenticationFailureHandler` when connect functionality is not enabled
+
 ## 2.2.0 (2024-02-28)
 * BC Break: Dropped support for PHP 7.4 & 8.0,
 * Added: Telegram resource owner,

src/Resources/config/oauth.php

@@ -69,6 +69,7 @@
         ->args([
             service('request_stack'),
             service('router'),
+            service('security.http_utils'),
             '%hwi_oauth.connect%',
         ]);
 };

src/Security/Http/Authentication/AuthenticationFailureHandler.php

@@ -14,20 +14,20 @@
 namespace HWI\Bundle\OAuthBundle\Security\Http\Authentication;
 
 use HWI\Bundle\OAuthBundle\Security\Core\Exception\AccountNotLinkedException;
-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\RouterInterface;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
 use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface;
+use Symfony\Component\Security\Http\HttpUtils;
 use Symfony\Component\Security\Http\ParameterBagUtils;
 
 final class AuthenticationFailureHandler implements AuthenticationFailureHandlerInterface
 {
     private array $defaultOptions = [
-        'failure_path' => 'hwi_oauth_connect_registration',
+        'failure_path' => null,
         'failure_forward' => false,
         'login_path' => '/login',
         'failure_path_parameter' => '_failure_path',
@@ -36,8 +36,11 @@ final class AuthenticationFailureHandler implements AuthenticationFailureHandler
     public function __construct(
         private readonly RequestStack $requestStack,
         private readonly RouterInterface $router,
+        private readonly HttpUtils $httpUtils,
         private readonly bool $connect,
+        array $options = [],
     ) {
+        $this->setOptions($options);
     }
 
     public function setOptions(array $options): void
@@ -58,7 +61,7 @@ public function onAuthenticationFailure(Request $request, AuthenticationExceptio
         $options['failure_path'] ??= $options['login_path'];
 
         $error = $exception->getPrevious();
-
+        $key = null;
         if ($this->connect && $error instanceof AccountNotLinkedException) {
             $key = time();
             $session = $request->hasSession() ? $request->getSession() : $this->getSession();
@@ -69,29 +72,17 @@ public function onAuthenticationFailure(Request $request, AuthenticationExceptio
 
                 $session->set('_hwi_oauth.registration_error.'.$key, $error);
             }
-
-            if ('/' === $options['failure_path'][0]) {
-                $failurePath = $request->getUriForPath($options['failure_path'][0]);
-            } else {
-                $failurePath = $this->router->generate($options['failure_path'], ['key' => $key]);
-            }
-
-            return new RedirectResponse($failurePath);
-        }
-
-        if ($error instanceof AuthenticationException) {
-            $error = $error->getMessageKey();
-        } else {
-            $error = $exception->getMessageKey();
         }
 
-        if ('/' === $options['login_path'][0]) {
-            $loginPath = $request->getUriForPath($options['login_path'][0]);
+        if (null !== $key) {
+            $failurePath = $this->router->generate($options['failure_path'], ['key' => $key]);
+        } elseif ('/' === $options['failure_path'][0]) {
+            $failurePath = $options['failure_path'];
         } else {
-            $loginPath = $this->router->generate($options['login_path'], ['error' => $error]);
+            $failurePath = $this->router->generate($options['failure_path']);
         }
 
-        return new RedirectResponse($loginPath);
+        return $this->httpUtils->createRedirectResponse($request, $failurePath);
     }
 
     private function getSession(): ?SessionInterface

src/Security/Http/Authenticator/OAuthAuthenticator.php

@@ -128,6 +128,7 @@ public function authenticate(Request $request): Passport
 
         $userBadge = class_exists(UserBadge::class)
             ? new UserBadge(
+                /* @phpstan-ignore-next-line */
                 $user ? method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername() : null,
                 static function () use ($user) { return $user; }
             )

tests/Security/Http/Authentication/AuthenticationFailureHandlerTest.php

@@ -21,8 +21,10 @@
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpFoundation\Session\SessionInterface;
+use Symfony\Component\Routing\Matcher\UrlMatcherInterface;
 use Symfony\Component\Routing\RouterInterface;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
+use Symfony\Component\Security\Http\HttpUtils;
 
 final class AuthenticationFailureHandlerTest extends TestCase
 {
@@ -35,6 +37,7 @@ public function testRedirectsToLoginPathWhenNotSet()
         $handler = new AuthenticationFailureHandler(
             $requestStack,
             $router = $this->createMock(RouterInterface::class),
+            new HttpUtils($router, $this->createMock(UrlMatcherInterface::class)),
             false
         );
 
@@ -56,13 +59,14 @@ public function testRedirectsToLoginRoute()
         $handler = new AuthenticationFailureHandler(
             $requestStack,
             $router = $this->createMock(RouterInterface::class),
+            new HttpUtils($router, $this->createMock(UrlMatcherInterface::class)),
             false,
         );
         $handler->setOptions(['login_path' => 'route_name']);
 
         $router->expects($this->once())
             ->method('generate')
-            ->with('route_name', ['error' => 'An authentication exception occurred.'], 1)
+            ->with('route_name', [], 1)
             ->willReturn('/path');
 
         $this->assertInstanceOf(
@@ -80,6 +84,7 @@ public function testDoesNothingWhenConnectIsDisabled()
         $handler = new AuthenticationFailureHandler(
             $requestStack,
             $router = $this->createMock(RouterInterface::class),
+            new HttpUtils($router, $this->createMock(UrlMatcherInterface::class)),
             false
         );
 
@@ -101,6 +106,7 @@ public function testDoesNothingWhenExceptionIsNotOAuthOne()
         $handler = new AuthenticationFailureHandler(
             $requestStack,
             $router = $this->createMock(RouterInterface::class),
+            new HttpUtils($router, $this->createMock(UrlMatcherInterface::class)),
             true
         );
 
@@ -124,7 +130,9 @@ public function testRedirectsToRegistrationPage()
         $handler = new AuthenticationFailureHandler(
             $requestStack,
             $router = $this->createMock(RouterInterface::class),
-            true
+            new HttpUtils($router, $this->createMock(UrlMatcherInterface::class)),
+            true,
+            ['failure_path' => 'hwi_oauth_connect_registration']
         );
 
         $router->expects($this->once())