IBX-8656: Skipped credentials check for SelfValidatingPassport (#411)

* IBX-8656: Skipped credentials check for `SelfValidatingPassport`

* removed duplicated authentication

Author: konradoboza

phpstan-baseline.neon

@@ -11480,11 +11480,6 @@ parameters:
 			count: 1
 			path: src/lib/MVC/Symfony/Event/ContentCacheClearEvent.php
 
-		-
-			message: "#^Property Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Event\\\\InteractiveLoginEvent\\:\\:\\$apiUser \\(Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\User\\\\User\\) in isset\\(\\) is not nullable\\.$#"
-			count: 1
-			path: src/lib/MVC/Symfony/Event/InteractiveLoginEvent.php
-
 		-
 			message: "#^Method Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Event\\\\PostSiteAccessMatchEvent\\:\\:__construct\\(\\) has parameter \\$requestType with no type specified\\.$#"
 			count: 1
@@ -45720,11 +45715,6 @@ parameters:
 			count: 1
 			path: tests/lib/MVC/Symfony/Event/ContentCacheClearEventTest.php
 
-		-
-			message: "#^Method Ibexa\\\\Tests\\\\Core\\\\MVC\\\\Symfony\\\\Event\\\\InteractiveLoginEventTest\\:\\:testGetSetAPIUser\\(\\) has no return type specified\\.$#"
-			count: 1
-			path: tests/lib/MVC/Symfony/Event/InteractiveLoginEventTest.php
-
 		-
 			message: "#^Method Ibexa\\\\Tests\\\\Core\\\\MVC\\\\Symfony\\\\Event\\\\RouteReferenceGenerationEventTest\\:\\:testConstruct\\(\\) has no return type specified\\.$#"
 			count: 1

src/lib/MVC/Symfony/Event/InteractiveLoginEvent.php

@@ -51,14 +51,6 @@ final class MVCEvents
      */
     public const CONFIG_SCOPE_RESTORE = 'ezpublish.config.scope_restore';
 
-    /**
-     * INTERACTIVE_LOGIN event occurs when a user has been authenticated by a foreign user provider.
-     * Listening to this event gives a chance to retrieve a valid API user to be injected in repository.
-     *
-     * The event listener method receives a {@see \Ibexa\Core\MVC\Symfony\Event\InteractiveLoginEvent} instance.
-     */
-    public const INTERACTIVE_LOGIN = 'ezpublish.security.interactive_login';
-
     /**
      * ROUTE_REFERENCE_GENERATION event occurs when a RouteReference is generated, and gives an opportunity to
      * alter the RouteReference, e.g. by adding parameters.

src/lib/MVC/Symfony/MVCEvents.php

@@ -19,6 +19,7 @@
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
+use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
 use Symfony\Component\Security\Http\Event\CheckPassportEvent;
 
 final class RepositoryUserAuthenticationSubscriber implements EventSubscriberInterface
@@ -52,7 +53,14 @@ public function validateRepositoryUser(CheckPassportEvent $event): void
             return;
         }
 
-        $badge = $event->getPassport()->getBadge(UserBadge::class);
+        $passport = $event->getPassport();
+        //validating password hash is not needed when SelfValidatingPassport is in use - this implementation is generally
+        //used when there are no credentials to be checked (e.g. API token authentication).
+        if ($passport instanceof SelfValidatingPassport) {
+            return;
+        }
+
+        $badge = $passport->getBadge(UserBadge::class);
         if (!$badge instanceof UserBadge) {
             return;
         }
@@ -68,8 +76,6 @@ public function validateRepositoryUser(CheckPassportEvent $event): void
                 $user->getAPIUser(),
                 $user->getPassword() ?? ''
             );
-
-            $event->getAuthenticator()->authenticate($request);
         } catch (UnsupportedPasswordHashType $exception) {
             $this->sleepUsingConstantTimer($startTime);
 

src/lib/MVC/Symfony/Security/Authentication/EventSubscriber/RepositoryUserAuthenticationSubscriber.php

@@ -24,6 +24,7 @@
 use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
 use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
 use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
+use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
 use Symfony\Component\Security\Http\Event\CheckPassportEvent;
 use Symfony\Component\Stopwatch\Stopwatch;
 
@@ -112,6 +113,21 @@ public function testAuthenticateConstantTimeDisabled(): void
         );
     }
 
+    public function testSkippingRepositoryUserValidationForSelfValidatingPassport(): void
+    {
+        $user = $this->createMock(User::class);
+        $user->expects(self::never())->method('getAPIUser');
+
+        $userService = $this->createMock(UserService::class);
+        $userService->expects(self::never())->method('checkUserCredentials');
+
+        $selfValidatingPassport = new SelfValidatingPassport(new UserBadge('foo'));
+
+        $this->getSubscriber($userService)->validateRepositoryUser(
+            $this->getCheckPassportEvent($user, $selfValidatingPassport)
+        );
+    }
+
     private function getSubscriber(
         ?UserService $userService = null,
         float $constantAuthTime = 1.0,
@@ -131,22 +147,26 @@ private function getSubscriber(
         );
     }
 
-    private function getCheckPassportEvent(?UserInterface $user = null): CheckPassportEvent
-    {
+    private function getCheckPassportEvent(
+        ?UserInterface $user = null,
+        ?Passport $passport = null,
+    ): CheckPassportEvent {
+        $authenticator = $this->createMock(AuthenticatorInterface::class);
         $user = $user ?? $this->createMock(User::class);
 
-        $userProvider = $this->createMock(User\APIUserProviderInterface::class);
-        $userProvider
-            ->expects(self::once())
-            ->method('loadUserByUsername')
-            ->willReturn($user);
+        if ($passport === null) {
+            $userProvider = $this->createMock(User\APIUserProviderInterface::class);
+            $userProvider
+                ->expects(self::once())
+                ->method('loadUserByUsername')
+                ->willReturn($user);
 
-        return new CheckPassportEvent(
-            $this->createMock(AuthenticatorInterface::class),
-            new Passport(
+            $passport = new Passport(
                 new UserBadge($user->getUsername(), [$userProvider, 'loadUserByUsername']),
                 new PasswordCredentials($user->getPassword() ?? '')
-            )
-        );
+            );
+        }
+
+        return new CheckPassportEvent($authenticator, $passport);
     }
 }