IBX-8290: Reworked REST authentication to comply with the new Symfony…

… authenticator mechanism under separate firewall (#98) * IBX-8290: Reworked REST authentication to comply with the new Symfony authenticator mechanism under separate firewall * improved UnauthorizedException throwing, introduced dedicated exception * cr remarks * cr remark vol.3 * adjusted unauthorized exception message to avoid exposing sensitive data * removed dev dependency
ibexa · Jun 20, 2024 · 450c2f1 · 450c2f1
1 parent ff31789
commit 450c2f1
Show file tree

Hide file tree

Showing 36 changed files with 845 additions and 3,488 deletions.
diff --git a/phpstan-baseline-8.0.neon b/phpstan-baseline-8.0.neon
@@ -1,10 +1,5 @@
 parameters:
 	ignoreErrors:
-		-
-			message: "#^Parameter \\#1 \\$filename of function file_put_contents expects string, string\\|false given\\.$#"
-			count: 1
-			path: src/lib/FieldTypeProcessor/BinaryInputProcessor.php
-
 		-
 			message: "#^Access to an undefined property DOMNode\\:\\:\\$data\\.$#"
 			count: 2
@@ -60,22 +55,7 @@ parameters:
 			count: 1
 			path: src/lib/Server/Controller/Role.php
 
-		-
-			message: "#^Parameter \\#2 \\$error_level of function trigger_error expects int, string given\\.$#"
-			count: 4
-			path: src/lib/Server/Controller/User.php
-
-		-
-			message: "#^Parameter \\#1 \\$message of method Psr\\\\Log\\\\LoggerInterface\\:\\:error\\(\\) expects string\\|Stringable, Symfony\\\\Component\\\\Security\\\\Core\\\\User\\\\UserInterface\\|null given\\.$#"
-			count: 1
-			path: src/lib/Server/Security/RestAuthenticator.php
-
 		-
 			message: "#^Parameter \\#1 \\$string of function base64_encode expects string, string\\|false given\\.$#"
 			count: 1
 			path: tests/bundle/Functional/BinaryContentTest.php
-
-		-
-			message: "#^Parameter \\#1 \\$directory of function mkdir expects string, string\\|false given\\.$#"
-			count: 1
-			path: tests/lib/FieldTypeProcessor/BinaryInputProcessorTest.php
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
diff --git a/src/bundle/DependencyInjection/Security/RestSessionBasedFactory.php b/src/bundle/DependencyInjection/Security/RestSessionBasedFactory.php
diff --git a/src/bundle/EventListener/CsrfListener.php b/src/bundle/EventListener/CsrfListener.php
@@ -4,11 +4,12 @@
  * @copyright Copyright (C) Ibexa AS. All rights reserved.
  * @license For full copyright and license information view LICENSE file distributed with this source code.
  */
+declare(strict_types=1);
 
 namespace Ibexa\Bundle\Rest\EventListener;
 
 use Ibexa\Bundle\Rest\RestEvents;
-use Ibexa\Core\Base\Exceptions\UnauthorizedException;
+use Ibexa\Contracts\Rest\Exceptions\UnauthorizedException;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpFoundation\Request;
@@ -17,58 +18,41 @@
 use Symfony\Component\Security\Csrf\CsrfToken;
 use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
 
-class CsrfListener implements EventSubscriberInterface
+/**
+ * @internal
+ */
+final class CsrfListener implements EventSubscriberInterface
 {
     /**
      * Name of the HTTP header containing CSRF token.
      */
-    public const CSRF_TOKEN_HEADER = 'X-CSRF-Token';
+    public const string CSRF_TOKEN_HEADER = 'X-CSRF-Token';
 
-    /**
-     * @var \Symfony\Component\Security\Csrf\CsrfTokenManagerInterface|null
-     */
-    private $csrfTokenManager;
+    private ?CsrfTokenManagerInterface $csrfTokenManager;
 
-    /**
-     * @var \Symfony\Component\EventDispatcher\EventDispatcherInterface
-     */
-    private $eventDispatcher;
+    private EventDispatcherInterface $eventDispatcher;
 
-    /**
-     * @var bool
-     */
-    private $csrfEnabled;
+    private bool $csrfEnabled;
 
-    /**
-     * @var bool
-     */
-    private $csrfTokenIntention;
+    private string $csrfTokenIntention;
 
     /**
      * Note that CSRF provider needs to be optional as it will not be available
      * when CSRF protection is disabled.
-     *
-     * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $eventDispatcher
-     * @param bool $csrfEnabled
-     * @param string $csrfTokenIntention
-     * @param \Symfony\Component\Security\Csrf\CsrfTokenManagerInterface|null $csrfTokenManager
      */
     public function __construct(
         EventDispatcherInterface $eventDispatcher,
-        $csrfEnabled,
-        $csrfTokenIntention,
-        CsrfTokenManagerInterface $csrfTokenManager = null
+        bool $csrfEnabled,
+        string $csrfTokenIntention,
+        ?CsrfTokenManagerInterface $csrfTokenManager = null
     ) {
         $this->eventDispatcher = $eventDispatcher;
         $this->csrfEnabled = $csrfEnabled;
         $this->csrfTokenIntention = $csrfTokenIntention;
         $this->csrfTokenManager = $csrfTokenManager;
     }
 
-    /**
-     * @return array
-     */
-    public static function getSubscribedEvents()
+    public static function getSubscribedEvents(): array
     {
         return [
             KernelEvents::REQUEST => 'onKernelRequest',
@@ -78,13 +62,12 @@ public static function getSubscribedEvents()
     /**
      * This method validates CSRF token if CSRF protection is enabled.
      *
-     * @param \Symfony\Component\HttpKernel\Event\RequestEvent $event
-     *
-     * @throws \Ibexa\Core\Base\Exceptions\UnauthorizedException
+     * @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException
      */
-    public function onKernelRequest(RequestEvent $event)
+    public function onKernelRequest(RequestEvent $event): void
     {
-        if (!$event->getRequest()->attributes->get('is_rest_request')) {
+        $request = $event->getRequest();
+        if (!$request->attributes->get('is_rest_request')) {
             return;
         }
 
@@ -93,85 +76,41 @@ public function onKernelRequest(RequestEvent $event)
         }
 
         // skip CSRF validation if no session is running
-        if (!$event->getRequest()->getSession()->isStarted()) {
-            return;
-        }
-
-        if ($this->isMethodSafe($event->getRequest()->getMethod())) {
+        if (!$request->getSession()->isStarted()) {
             return;
         }
 
-        if ($this->isSessionRoute($event->getRequest()->get('_route'))) {
+        if ($this->isMethodSafe($request->getMethod())) {
             return;
         }
 
-        if (!$event->getRequest()->attributes->getBoolean('csrf_protection', true)) {
+        if (!$request->attributes->getBoolean('csrf_protection', true)) {
             return;
         }
 
-        if (!$this->checkCsrfToken($event->getRequest())) {
-            throw new UnauthorizedException(
-                'Missing or invalid CSRF token',
-                $event->getRequest()->getMethod() . ' ' . $event->getRequest()->getPathInfo()
-            );
+        if (!$this->checkCsrfToken($request)) {
+            throw new UnauthorizedException('Missing or invalid CSRF token');
         }
 
         // Dispatching event so that CSRF token intention can be injected into Legacy Stack
         $this->eventDispatcher->dispatch($event, RestEvents::REST_CSRF_TOKEN_VALIDATED);
     }
 
-    /**
-     * @param string $method
-     *
-     * @return bool
-     */
-    protected function isMethodSafe($method)
+    private function isMethodSafe(string $method): bool
     {
         return in_array($method, ['GET', 'HEAD', 'OPTIONS']);
     }
 
-    /**
-     * @param string $route
-     *
-     * @return bool
-     *
-     * @deprecated Deprecated since 6.5. Use isSessionRoute() instead.
-     */
-    protected function isLoginRequest($route)
-    {
-        return $route === 'ibexa.rest.create_session';
-    }
-
-    /**
-     * Tests if a given $route is a session management one.
-     *
-     * @param string $route
-     *
-     * @return bool
-     *
-     * @deprecated since Ibexa DXP 3.3.7. Add csrf_protection: false attribute to route definition instead.
-     */
-    protected function isSessionRoute($route)
-    {
-        return in_array(
-            $route,
-            ['ibexa.rest.create_session', 'ibexa.rest.refresh_session', 'ibexa.rest.delete_session']
-        );
-    }
-
-    /**
-     * Checks the validity of the request's csrf token header.
-     *
-     * @param \Symfony\Component\HttpFoundation\Request $request
-     *
-     * @return bool true/false if the token is valid/invalid, false if none was found in the request's headers
-     */
-    protected function checkCsrfToken(Request $request)
+    private function checkCsrfToken(Request $request): bool
     {
         if (!$request->headers->has(self::CSRF_TOKEN_HEADER)) {
             return false;
         }
 
+        if ($this->csrfTokenManager === null) {
+            return false;
+        }
+
         return $this->csrfTokenManager->isTokenValid(
             new CsrfToken(
                 $this->csrfTokenIntention,

diff --git a/src/bundle/EventListener/RequestListener.php b/src/bundle/EventListener/RequestListener.php
@@ -7,10 +7,8 @@
 
 namespace Ibexa\Bundle\Rest\EventListener;
 
-use Ibexa\Bundle\Rest\UriParser\UriParser;
 use Ibexa\Contracts\Rest\UriParser\UriParserInterface;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
-use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
 
@@ -21,24 +19,15 @@
  *
  * Flags a REST request as such using the is_rest_request attribute.
  */
-class RequestListener implements EventSubscriberInterface
+final class RequestListener implements EventSubscriberInterface
 {
-    /**
-     * @deprecated rely on \Ibexa\Contracts\Rest\UriParser\UriParserInterface::isRestRequest instead.
-     * @see \Ibexa\Contracts\Rest\UriParser\UriParserInterface::isRestRequest()
-     */
-    public const REST_PREFIX_PATTERN = UriParser::DEFAULT_REST_PREFIX_PATTERN;
-
     private UriParserInterface $uriParser;
 
     public function __construct(UriParserInterface $uriParser)
     {
         $this->uriParser = $uriParser;
     }
 
-    /**
-     * @return array
-     */
     public static function getSubscribedEvents(): array
     {
         return [
@@ -57,17 +46,4 @@ public function onKernelRequest(RequestEvent $event): void
             $this->uriParser->isRestRequest($event->getRequest())
         );
     }
-
-    /**
-     * @param \Symfony\Component\HttpFoundation\Request $request
-     *
-     * @return bool
-     *
-     * @deprecated use \Ibexa\Contracts\Rest\UriParser\UriParserInterface::isRestRequest instead
-     * @see \Ibexa\Contracts\Rest\UriParser\UriParserInterface::isRestRequest()
-     */
-    protected function hasRestPrefix(Request $request)
-    {
-        return preg_match(self::REST_PREFIX_PATTERN, $request->getPathInfo());
-    }
 }