Replace custom authenticator passport with custom badge usage (#1978)

hwi · Feb 16, 2024 · 2b94931 · 2b94931
1 parent b54e42b
commit 2b94931
Show file tree

Hide file tree

Showing 13 changed files with 267 additions and 106 deletions.
diff --git a/src/Controller/RedirectToServiceController.php b/src/Controller/RedirectToServiceController.php
@@ -27,27 +27,14 @@
  */
 final class RedirectToServiceController
 {
-    private OAuthUtils $oauthUtils;
-    private DomainWhitelist $domainWhitelist;
-    private ResourceOwnerMapLocator $resourceOwnerMapLocator;
-    private ?string $targetPathParameter;
-    private bool $failedUseReferer;
-    private bool $useReferer;
-
     public function __construct(
-        OAuthUtils $oauthUtils,
-        DomainWhitelist $domainWhitelist,
-        ResourceOwnerMapLocator $resourceOwnerMapLocator,
-        ?string $targetPathParameter,
-        bool $failedUseReferer,
-        bool $useReferer
+        private readonly OAuthUtils $oauthUtils,
+        private readonly DomainWhitelist $domainWhitelist,
+        private readonly ResourceOwnerMapLocator $resourceOwnerMapLocator,
+        private readonly ?string $targetPathParameter,
+        private readonly bool $failedUseReferer,
+        private readonly bool $useReferer
     ) {
-        $this->oauthUtils = $oauthUtils;
-        $this->domainWhitelist = $domainWhitelist;
-        $this->resourceOwnerMapLocator = $resourceOwnerMapLocator;
-        $this->targetPathParameter = $targetPathParameter;
-        $this->failedUseReferer = $failedUseReferer;
-        $this->useReferer = $useReferer;
     }
 
     /**

diff --git a/src/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php b/src/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php
@@ -31,11 +31,8 @@
  */
 final class OAuthAuthenticatorFactory extends AbstractFactory implements AuthenticatorFactoryInterface, FirewallListenerFactoryInterface
 {
-    private \ArrayIterator $firewallNames;
-
-    public function __construct(\ArrayIterator $firewallNames)
+    public function __construct(private \ArrayIterator $firewallNames)
     {
-        $this->firewallNames = $firewallNames;
     }
 
     /**

diff --git a/src/Resources/config/oauth.php b/src/Resources/config/oauth.php
@@ -17,6 +17,7 @@
 use HWI\Bundle\OAuthBundle\Security\Core\Authentication\Provider\OAuthProvider;
 use HWI\Bundle\OAuthBundle\Security\Core\User\EntityUserProvider;
 use HWI\Bundle\OAuthBundle\Security\Core\User\OAuthUserProvider;
+use HWI\Bundle\OAuthBundle\Security\Http\Authentication\AuthenticationFailureHandler;
 use HWI\Bundle\OAuthBundle\Security\Http\EntryPoint\OAuthEntryPoint;
 use HWI\Bundle\OAuthBundle\Security\Http\Firewall\AbstractRefreshAccessTokenListener;
 use HWI\Bundle\OAuthBundle\Security\Http\Firewall\OAuthListener;
@@ -59,4 +60,7 @@
             '%hwi_oauth.connect%',
             '%hwi_oauth.grant_rule%',
         ]);
+
+    $services->set('hwi_oauth.authentication.failure_handler', AuthenticationFailureHandler::class)
+        ->arg('$connect', '%hwi_oauth.connect%');
 };
diff --git a/src/Security/Core/User/EntityUserProvider.php b/src/Security/Core/User/EntityUserProvider.php
@@ -15,6 +15,7 @@
 use Doctrine\Persistence\ObjectManager;
 use Doctrine\Persistence\ObjectRepository;
 use HWI\Bundle\OAuthBundle\OAuth\Response\UserResponseInterface;
+use HWI\Bundle\OAuthBundle\Security\Core\Exception\AccountNotLinkedException;
 use Symfony\Component\PropertyAccess\PropertyAccess;
 use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
 use Symfony\Component\Security\Core\Exception\UserNotFoundException;
@@ -125,7 +126,7 @@ private function findUser(array $criteria): ?UserInterface
 
     private function createUserNotFoundException(string $username, string $message): UserNotFoundException
     {
-        $exception = new UserNotFoundException($message);
+        $exception = new AccountNotLinkedException($message);
         $exception->setUserIdentifier($username);
 
         return $exception;

diff --git a/src/Security/Http/Authentication/AuthenticationFailureHandler.php b/src/Security/Http/Authentication/AuthenticationFailureHandler.php
@@ -0,0 +1,81 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the HWIOAuthBundle package.
+ *
+ * (c) Hardware Info <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+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\Generator\UrlGeneratorInterface;
+use Symfony\Component\Routing\RouterInterface;
+use Symfony\Component\Security\Core\Exception\AuthenticationException;
+use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface;
+use Twig\Environment;
+
+final class AuthenticationFailureHandler implements AuthenticationFailureHandlerInterface
+{
+    public function __construct(
+        private readonly RequestStack $requestStack,
+        private readonly RouterInterface $router,
+        private readonly Environment $twig,
+        private readonly bool $connect
+    ) {
+    }
+
+    public function onAuthenticationFailure(Request $request, AuthenticationException $exception): Response
+    {
+        $error = $exception->getPrevious();
+
+        if ($this->connect && $error instanceof AccountNotLinkedException) {
+            $key = time();
+            $session = $request->hasSession() ? $request->getSession() : $this->getSession();
+            if ($session) {
+                if (!$session->isStarted()) {
+                    $session->start();
+                }
+
+                $session->set('_hwi_oauth.registration_error.'.$key, $error);
+            }
+
+            return new RedirectResponse(
+                $this->router->generate('hwi_oauth_connect_registration', ['key' => $key], UrlGeneratorInterface::ABSOLUTE_PATH)
+            );
+        }
+
+        if ($error instanceof AuthenticationException) {
+            $error = $error->getMessageKey();
+        } else {
+            $error = $exception->getMessageKey();
+        }
+
+        return new Response(
+            $this->twig->render('@HWIOAuth/Connect/login.html.twig', ['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/src/Security/Http/Authenticator/OAuthAuthenticator.php b/src/Security/Http/Authenticator/OAuthAuthenticator.php
@@ -43,38 +43,19 @@
  */
 final class OAuthAuthenticator implements AuthenticatorInterface, AuthenticationEntryPointInterface, InteractiveAuthenticatorInterface
 {
-    private HttpUtils $httpUtils;
-    private OAuthAwareUserProviderInterface $userProvider;
-    private ResourceOwnerMapInterface $resourceOwnerMap;
-    private AuthenticationSuccessHandlerInterface $successHandler;
-    private AuthenticationFailureHandlerInterface $failureHandler;
-    private HttpKernelInterface $httpKernel;
-
     /**
-     * @var string[]
+     * @param string[] $checkPaths
      */
-    private array $checkPaths;
-
-    private array $options;
-
     public function __construct(
-        HttpUtils $httpUtils,
-        OAuthAwareUserProviderInterface $userProvider,
-        ResourceOwnerMapInterface $resourceOwnerMap,
-        array $checkPaths,
-        AuthenticationSuccessHandlerInterface $successHandler,
-        AuthenticationFailureHandlerInterface $failureHandler,
-        HttpKernelInterface $kernel,
-        array $options
+        private readonly HttpUtils $httpUtils,
+        private readonly OAuthAwareUserProviderInterface $userProvider,
+        private readonly ResourceOwnerMapInterface $resourceOwnerMap,
+        private readonly array $checkPaths,
+        private readonly AuthenticationSuccessHandlerInterface $successHandler,
+        private readonly AuthenticationFailureHandlerInterface $failureHandler,
+        private readonly HttpKernelInterface $kernel,
+        private readonly array $options
     ) {
-        $this->failureHandler = $failureHandler;
-        $this->successHandler = $successHandler;
-        $this->checkPaths = $checkPaths;
-        $this->resourceOwnerMap = $resourceOwnerMap;
-        $this->userProvider = $userProvider;
-        $this->httpUtils = $httpUtils;
-        $this->httpKernel = $kernel;
-        $this->options = $options;
     }
 
     public function supports(Request $request): bool
@@ -96,7 +77,7 @@ public function start(Request $request, ?AuthenticationException $authException
             $iterator = $request->query->getIterator();
             $subRequest->query->add(iterator_to_array($iterator));
 
-            $response = $this->httpKernel->handle($subRequest, HttpKernelInterface::SUB_REQUEST);
+            $response = $this->kernel->handle($subRequest, HttpKernelInterface::SUB_REQUEST);
             if (200 === $response->getStatusCode()) {
                 $response->headers->set('X-Status-Code', '401');
             }

diff --git a/src/Security/Http/EntryPoint/OAuthEntryPoint.php b/src/Security/Http/EntryPoint/OAuthEntryPoint.php
@@ -28,17 +28,12 @@
  */
 final class OAuthEntryPoint implements AuthenticationEntryPointInterface
 {
-    private HttpKernelInterface $httpKernel;
-    private HttpUtils $httpUtils;
-    private string $loginPath;
-    private bool $useForward;
-
-    public function __construct(HttpKernelInterface $kernel, HttpUtils $httpUtils, string $loginPath, bool $useForward = false)
-    {
-        $this->httpKernel = $kernel;
-        $this->httpUtils = $httpUtils;
-        $this->loginPath = $loginPath;
-        $this->useForward = $useForward;
+    public function __construct(
+        private readonly HttpKernelInterface $kernel,
+        private readonly HttpUtils $httpUtils,
+        private readonly string $loginPath,
+        private readonly bool $useForward = false
+    ) {
     }
 
     /**
@@ -53,7 +48,7 @@ public function start(Request $request, ?AuthenticationException $authException
             $iterator = $request->query->getIterator();
             $subRequest->query->add($iterator->getArrayCopy());
 
-            $response = $this->httpKernel->handle($subRequest, HttpKernelInterface::SUB_REQUEST);
+            $response = $this->kernel->handle($subRequest, HttpKernelInterface::SUB_REQUEST);
             if (200 === $response->getStatusCode()) {
                 $response->headers->set('X-Status-Code', '401');
             }

diff --git a/src/Security/Http/Firewall/RefreshAccessTokenListenerOld.php b/src/Security/Http/Firewall/RefreshAccessTokenListenerOld.php
@@ -16,9 +16,13 @@
 
 class RefreshAccessTokenListenerOld extends RefreshAccessTokenListener
 {
+    /**
+     * @phpstan-ignore-next-line
+     */
     private AuthenticationProviderInterface $oAuthProvider;
 
     public function __construct(
+        /* @phpstan-ignore-next-line */
         AuthenticationProviderInterface $oAuthProvider
     ) {
         $this->oAuthProvider = $oAuthProvider;

diff --git a/src/Security/Http/ResourceOwnerMap.php b/src/Security/Http/ResourceOwnerMap.php
@@ -26,25 +26,16 @@
  */
 final class ResourceOwnerMap implements ResourceOwnerMapInterface
 {
-    private HttpUtils $httpUtils;
-    private array $resourceOwners;
-    private array $possibleResourceOwners;
-    private ServiceLocator $locator;
-
     /**
      * @param array<string, string> $possibleResourceOwners array with possible resource owners names
      * @param array<string, string> $resourceOwners         array with configured resource owners
      */
     public function __construct(
-        HttpUtils $httpUtils,
-        array $possibleResourceOwners,
-        array $resourceOwners,
-        ServiceLocator $locator
+        private readonly HttpUtils $httpUtils,
+        private readonly array $possibleResourceOwners,
+        private readonly array $resourceOwners,
+        private readonly ServiceLocator $locator
     ) {
-        $this->httpUtils = $httpUtils;
-        $this->possibleResourceOwners = $possibleResourceOwners;
-        $this->resourceOwners = $resourceOwners;
-        $this->locator = $locator;
     }
 
     /**

diff --git a/src/Security/OAuthErrorHandler.php b/src/Security/OAuthErrorHandler.php
@@ -19,9 +19,9 @@ final class OAuthErrorHandler
     /**
      * "translated" OAuth errors to human readable format.
      *
-     * @var array
+     * @var array<string, string>
      */
-    private static $translatedOAuthErrors = [
+    private static array $translatedOAuthErrors = [
         'access_denied' => 'You have refused access for this site.',
         'authorization_expired' => 'Authorization expired.',
         'bad_verification_code' => 'Bad verification code.',
@@ -36,7 +36,7 @@ final class OAuthErrorHandler
     /**
      * @throws AuthenticationException
      */
-    public static function handleOAuthError(Request $request)
+    public static function handleOAuthError(Request $request): void
     {
         $error = null;
 
@@ -61,11 +61,7 @@ public static function handleOAuthError(Request $request)
         }
 
         if (null !== $error) {
-            if (isset(static::$translatedOAuthErrors[$error])) {
-                $error = static::$translatedOAuthErrors[$error];
-            } else {
-                $error = sprintf('Unknown OAuth error: "%s".', $error);
-            }
+            $error = self::$translatedOAuthErrors[$error] ?? sprintf('Unknown OAuth error: "%s".', $error);
 
             throw new AuthenticationException($error);
         }

diff --git a/src/Security/OAuthUtils.php b/src/Security/OAuthUtils.php
@@ -30,29 +30,18 @@ final class OAuthUtils
     public const SIGNATURE_METHOD_RSA = 'RSA-SHA1';
     public const SIGNATURE_METHOD_PLAINTEXT = 'PLAINTEXT';
 
-    private bool $connect;
-    private string $grantRule;
-    private HttpUtils $httpUtils;
-    private AuthorizationCheckerInterface $authorizationChecker;
-    private FirewallMap $firewallMap;
-
     /**
      * @var array<string, ResourceOwnerMapInterface>
      */
     private array $ownerMaps = [];
 
     public function __construct(
-        HttpUtils $httpUtils,
-        AuthorizationCheckerInterface $authorizationChecker,
-        FirewallMap $firewallMap,
-        bool $connect,
-        string $grantRule
+        private readonly HttpUtils $httpUtils,
+        private readonly AuthorizationCheckerInterface $authorizationChecker,
+        private readonly FirewallMap $firewallMap,
+        private readonly bool $connect,
+        private readonly string $grantRule
     ) {
-        $this->httpUtils = $httpUtils;
-        $this->authorizationChecker = $authorizationChecker;
-        $this->firewallMap = $firewallMap;
-        $this->connect = $connect;
-        $this->grantRule = $grantRule;
     }
 
     public function addResourceOwnerMap($firewallName, ResourceOwnerMapInterface $ownerMap): void
@@ -217,9 +206,7 @@ public static function signRequest(
                 $signature = false;
 
                 openssl_sign($baseString, $signature, $privateKey);
-                if (\PHP_VERSION_ID < 80000) {
-                    openssl_free_key($privateKey);
-                }
+
                 break;
 
             case self::SIGNATURE_METHOD_PLAINTEXT:
@@ -276,7 +263,7 @@ private function getResourceOwnerCheckPath(string $name, ?string $firewallName =
      * @param string|array<string, string>|null $queryParameter The query parameter to parse and add to the State
      * @param ResourceOwnerInterface            $resourceOwner  The resource owner holding the state to be added to
      */
-    private function addQueryParameterToState($queryParameter, ResourceOwnerInterface $resourceOwner): void
+    private function addQueryParameterToState(array|string|null $queryParameter, ResourceOwnerInterface $resourceOwner): void
     {
         if (null === $queryParameter) {
             return;