Add forward compat layer for changed logout listener configuration in 2.x

mbabker · mbabker · commit c5d7967b6c86 · 2023-01-17T14:31:29.000-05:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,12 @@
 # Changelog
 
-## Unreleased
+## 1.2.0
+
+- [B/C Break] Removed the `Gesdinet\JWTRefreshTokenBundle\EventListener\LogoutEventListener` service definition; if needed, an abstract `gesdinet_jwt_refresh_token.security.listener.logout` definition replaces it and does not have a `kernel.event_listener` tag
+- [B/C Break] The `logout_firewall` config node default value is now null
+- Deprecated the `logout_firewall` config node, the `invalidate_token_on_logout` option should be set on the `refresh_jwt` authenticator
+
+## 1.1.0
 
 - [B/C Break] Changed the object mappings to mapped superclasses, this requires updating your app's configuration
 - Added support for checking the request path in the `refresh_jwt` authenticator
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -109,8 +109,9 @@ public function getConfigTreeBuilder(): TreeBuilder
                     ->end()
                 ->end()
                 ->scalarNode('logout_firewall')
-                    ->defaultValue('api')
-                    ->info('Name of the firewall that triggers the logout event to hook into (default: api)')
+                    ->setDeprecated(...$this->getDeprecationParameters('The "%node%" node is deprecated, enable the "invalidate_token_on_logout" option on the "refresh_jwt" firewall instead.', '1.2'))
+                    ->defaultNull()
+                    ->info('Name of the firewall that triggers the logout event to hook into')
                 ->end()
                 ->scalarNode('return_expiration')
                     ->defaultFalse()
diff --git a/DependencyInjection/GesdinetJWTRefreshTokenExtension.php b/DependencyInjection/GesdinetJWTRefreshTokenExtension.php
@@ -16,10 +16,13 @@
 use Gesdinet\JWTRefreshTokenBundle\Document\RefreshToken as RefreshTokenDocument;
 use Gesdinet\JWTRefreshTokenBundle\Entity\RefreshToken as RefreshTokenEntity;
 use Gesdinet\JWTRefreshTokenBundle\Request\Extractor\ExtractorInterface;
+use Symfony\Component\DependencyInjection\ChildDefinition;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\Config\FileLocator;
+use Symfony\Component\DependencyInjection\Parameter;
 use Symfony\Component\HttpKernel\DependencyInjection\Extension;
 use Symfony\Component\DependencyInjection\Loader;
+use Symfony\Component\Security\Http\Event\LogoutEvent;
 
 class GesdinetJWTRefreshTokenExtension extends Extension
 {
@@ -48,6 +51,12 @@ public function load(array $configs, ContainerBuilder $container): void
         $container->setParameter('gesdinet_jwt_refresh_token.return_expiration', $config['return_expiration']);
         $container->setParameter('gesdinet_jwt_refresh_token.return_expiration_parameter_name', $config['return_expiration_parameter_name']);
 
+        if ($config['logout_firewall']) {
+            $container->setDefinition('gesdinet_jwt_refresh_token.security.listener.logout.legacy_config', new ChildDefinition('gesdinet_jwt_refresh_token.security.listener.logout'))
+                ->addArgument(new Parameter('gesdinet_jwt_refresh_token.logout_firewall_context'))
+                ->addTag('kernel.event_listener', ['event' => LogoutEvent::class, 'method' => 'onLogout']);
+        }
+
         $refreshTokenClass = RefreshTokenEntity::class;
         $objectManager = 'doctrine.orm.entity_manager';
 
diff --git a/DependencyInjection/Security/Factory/RefreshTokenAuthenticatorFactory.php b/DependencyInjection/Security/Factory/RefreshTokenAuthenticatorFactory.php
@@ -17,6 +17,7 @@
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Parameter;
 use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\Security\Http\Event\LogoutEvent;
 
 final class RefreshTokenAuthenticatorFactory implements AuthenticatorFactoryInterface
 {
@@ -51,6 +52,10 @@ public function addConfiguration(NodeDefinition $builder): void
                 ->scalarNode('provider')->end()
                 ->scalarNode('success_handler')->end()
                 ->scalarNode('failure_handler')->end()
+                ->booleanNode('invalidate_token_on_logout')
+                    ->defaultFalse() // TODO - Enable by default in 2.0.
+                    ->info('When enabled, the refresh token will be invalided on logout.')
+                ->end()
                 /*
                 ->integerNode('ttl')
                     ->defaultNull()
@@ -87,6 +92,11 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal
             ->replaceArgument(5, new Reference($this->createAuthenticationFailureHandler($container, $firewallName, $config)))
             ->replaceArgument(6, $options);
 
+        if ($config['invalidate_token_on_logout']) {
+            $container->setDefinition('gesdinet_jwt_refresh_token.security.listener.logout.'.$firewallName, new ChildDefinition('gesdinet_jwt_refresh_token.security.listener.logout'))
+                ->addTag('kernel.event_listener', ['event' => LogoutEvent::class, 'method' => 'onLogout', 'dispatcher' => 'security.event_dispatcher.'.$firewallName]);
+        }
+
         return $authenticatorId;
     }
 
diff --git a/EventListener/LogoutEventListener.php b/EventListener/LogoutEventListener.php
@@ -22,14 +22,18 @@ class LogoutEventListener
     private ExtractorInterface $refreshTokenExtractor;
     private string $tokenParameterName;
     private array $cookieSettings;
-    private string $logout_firewall_context;
+
+    /**
+     * @deprecated to be removed in 2.0
+     */
+    private ?string $logoutFirewallContext;
 
     public function __construct(
         RefreshTokenManagerInterface $refreshTokenManager,
         ExtractorInterface $refreshTokenExtractor,
         string $tokenParameterName,
         array $cookieSettings,
-        string $logout_firewall_context
+        ?string $logoutFirewallContext = null
     ) {
         $this->refreshTokenManager = $refreshTokenManager;
         $this->refreshTokenExtractor = $refreshTokenExtractor;
@@ -43,15 +47,24 @@ public function __construct(
             'secure' => true,
             'remove_token_from_body' => true,
         ], $cookieSettings);
-        $this->logout_firewall_context = $logout_firewall_context;
+        $this->logoutFirewallContext = $logoutFirewallContext;
+
+        if (null !== $logoutFirewallContext) {
+            trigger_deprecation('gesdinet/jwt-refresh-token-bundle', '1.2', 'Passing the logout firewall context to "%s" is deprecated and will not be supported in 2.0.', self::class);
+        }
     }
 
     public function onLogout(LogoutEvent $event): void
     {
         $request = $event->getRequest();
-        $current_firewall_context = $request->attributes->get('_firewall_context');
 
-        if ($current_firewall_context !== $this->logout_firewall_context) {
+        /*
+         * This listener should only act in one of two conditions:
+         *
+         * 1) If the firewall context is not configured (this implies the listener is registered to the firewall specific event dispatcher)
+         * 2) (Deprecated) The request's firewall context matches the configured firewall context
+         */
+        if (null !== $this->logoutFirewallContext && $request->attributes->get('_firewall_context') !== $this->logoutFirewallContext) {
             return;
         }
 
diff --git a/Resources/config/services.php b/Resources/config/services.php
@@ -170,16 +170,13 @@
         ])
         ->tag('console.command');
 
-    $services->set(LogoutEventListener::class)
+    $services->set('gesdinet_jwt_refresh_token.security.listener.logout')
+        ->abstract()
+        ->class(LogoutEventListener::class)
         ->args([
             new Reference('gesdinet.jwtrefreshtoken.refresh_token_manager'),
             new Reference('gesdinet.jwtrefreshtoken.request.extractor.chain'),
             new Parameter('gesdinet_jwt_refresh_token.token_parameter_name'),
             new Parameter('gesdinet_jwt_refresh_token.cookie'),
-            new Parameter('gesdinet_jwt_refresh_token.logout_firewall_context'),
-        ])
-        ->tag('kernel.event_listener', [
-            'event' => 'Symfony\Component\Security\Http\Event\LogoutEvent',
-            'method' => 'onLogout',
         ]);
 };
diff --git a/Tests/Functional/DependencyInjection/GesdinetJWTRefreshTokenExtensionTest.php b/Tests/Functional/DependencyInjection/GesdinetJWTRefreshTokenExtensionTest.php
@@ -6,6 +6,7 @@
 use Gesdinet\JWTRefreshTokenBundle\Document\RefreshToken as RefreshTokenDocument;
 use Gesdinet\JWTRefreshTokenBundle\Entity\RefreshToken as RefreshTokenEntity;
 use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase;
+use Symfony\Component\Security\Http\Event\LogoutEvent;
 
 final class GesdinetJWTRefreshTokenExtensionTest extends AbstractExtensionTestCase
 {
@@ -40,10 +41,13 @@ public function test_container_is_loaded_with_default_configuration(): void
                 'remove_token_from_body' => true,
             ],
         );
+        $this->assertContainerBuilderHasParameter('gesdinet_jwt_refresh_token.logout_firewall_context', 'security.firewall.map.context.');
 
         $this->assertContainerBuilderHasParameter('gesdinet.jwtrefreshtoken.refresh_token.class', RefreshTokenEntity::class);
         $this->assertContainerBuilderHasParameter('gesdinet.jwtrefreshtoken.object_manager.id', 'doctrine.orm.entity_manager');
         $this->assertContainerBuilderHasParameter('gesdinet.jwtrefreshtoken.user_checker.id', 'security.user_checker');
+
+        $this->assertContainerBuilderNotHasService('gesdinet_jwt_refresh_token.security.listener.logout.legacy_config');
     }
 
     public function test_container_is_loaded_with_custom_configuration(): void
@@ -91,10 +95,13 @@ public function test_container_is_loaded_with_custom_configuration(): void
                 'remove_token_from_body' => true,
             ],
         );
+        $this->assertContainerBuilderHasParameter('gesdinet_jwt_refresh_token.logout_firewall_context', 'security.firewall.map.context.');
 
         $this->assertContainerBuilderHasParameter('gesdinet.jwtrefreshtoken.refresh_token.class', RefreshTokenDocument::class);
         $this->assertContainerBuilderHasParameter('gesdinet.jwtrefreshtoken.object_manager.id', 'doctrine_mongodb.odm.document_manager');
         $this->assertContainerBuilderHasParameter('gesdinet.jwtrefreshtoken.user_checker.id', 'my.user_checker');
+
+        $this->assertContainerBuilderNotHasService('gesdinet_jwt_refresh_token.security.listener.logout.legacy_config');
     }
 
     public function test_container_is_loaded_with_deprecated_parameters(): void
@@ -103,9 +110,13 @@ public function test_container_is_loaded_with_deprecated_parameters(): void
             'manager_type' => 'mongodb',
             'refresh_token_entity' => RefreshTokenDocument::class,
             'entity_manager' => 'doctrine_mongodb.odm.document_manager',
+            'logout_firewall' => 'api',
         ]);
 
+        $this->assertContainerBuilderHasParameter('gesdinet_jwt_refresh_token.logout_firewall_context', 'security.firewall.map.context.api');
         $this->assertContainerBuilderHasParameter('gesdinet.jwtrefreshtoken.refresh_token.class', RefreshTokenDocument::class);
         $this->assertContainerBuilderHasParameter('gesdinet.jwtrefreshtoken.object_manager.id', 'doctrine_mongodb.odm.document_manager');
+
+        $this->assertContainerBuilderHasServiceDefinitionWithTag('gesdinet_jwt_refresh_token.security.listener.logout.legacy_config', 'kernel.event_listener', ['event' => LogoutEvent::class, 'method' => 'onLogout']);
     }
 }
diff --git a/Tests/Functional/DependencyInjection/Security/Factory/RefreshTokenAuthenticatorFactoryTest.php b/Tests/Functional/DependencyInjection/Security/Factory/RefreshTokenAuthenticatorFactoryTest.php
@@ -7,6 +7,7 @@
 use Symfony\Component\DependencyInjection\ChildDefinition;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\Security\Http\Event\LogoutEvent;
 use Symfony\Component\Security\Http\RememberMe\RememberMeHandlerInterface;
 
 final class RefreshTokenAuthenticatorFactoryTest extends TestCase
@@ -39,13 +40,16 @@ public function test_authenticator_service_is_created_with_default_configuration
         $this->factory->createAuthenticator(
             $this->container,
             'test',
-            [],
+            [
+                'invalidate_token_on_logout' => false,
+            ],
             'app.user_provider'
         );
 
         $this->assertTrue($this->container->hasDefinition('security.authenticator.refresh_jwt.test'));
         $this->assertTrue($this->container->hasDefinition('security.authentication.success_handler.test.refresh_jwt'));
         $this->assertTrue($this->container->hasDefinition('security.authentication.failure_handler.test.refresh_jwt'));
+        $this->assertFalse($this->container->hasDefinition('gesdinet_jwt_refresh_token.security.listener.logout.test'));
 
         /** @var ChildDefinition $successHandler */
         $successHandler = $this->container->getDefinition('security.authentication.success_handler.test.refresh_jwt');
@@ -56,6 +60,35 @@ public function test_authenticator_service_is_created_with_default_configuration
         $this->assertSame('gesdinet.jwtrefreshtoken.security.authentication.failure_handler', $failureHandler->getParent());
     }
 
+    public function test_authenticator_service_is_created_and_logout_listener_registered_to_firewall_dispatcher(): void
+    {
+        $this->factory->createAuthenticator(
+            $this->container,
+            'test',
+            [
+                'invalidate_token_on_logout' => true,
+            ],
+            'app.user_provider'
+        );
+
+        $this->assertTrue($this->container->hasDefinition('security.authenticator.refresh_jwt.test'));
+        $this->assertTrue($this->container->hasDefinition('security.authentication.success_handler.test.refresh_jwt'));
+        $this->assertTrue($this->container->hasDefinition('security.authentication.failure_handler.test.refresh_jwt'));
+        $this->assertTrue($this->container->hasDefinition('gesdinet_jwt_refresh_token.security.listener.logout.test'));
+
+        /** @var ChildDefinition $successHandler */
+        $successHandler = $this->container->getDefinition('security.authentication.success_handler.test.refresh_jwt');
+        $this->assertSame('gesdinet.jwtrefreshtoken.security.authentication.success_handler', $successHandler->getParent());
+
+        /** @var ChildDefinition $failureHandler */
+        $failureHandler = $this->container->getDefinition('security.authentication.failure_handler.test.refresh_jwt');
+        $this->assertSame('gesdinet.jwtrefreshtoken.security.authentication.failure_handler', $failureHandler->getParent());
+
+        /** @var ChildDefinition $logoutListener */
+        $logoutListener = $this->container->getDefinition('gesdinet_jwt_refresh_token.security.listener.logout.test');
+        $this->assertSame(['event' => LogoutEvent::class, 'method' => 'onLogout', 'dispatcher' => 'security.event_dispatcher.test'], $logoutListener->getTags()['kernel.event_listener'][0]);
+    }
+
     public function test_authenticator_service_is_created_with_custom_handlers(): void
     {
         $this->factory->createAuthenticator(
@@ -64,13 +97,15 @@ public function test_authenticator_service_is_created_with_custom_handlers(): vo
             [
                 'success_handler' => 'app.security.authentication.success_handler',
                 'failure_handler' => 'app.security.authentication.failure_handler',
+                'invalidate_token_on_logout' => false,
             ],
             'app.user_provider'
         );
 
         $this->assertTrue($this->container->hasDefinition('security.authenticator.refresh_jwt.test'));
         $this->assertTrue($this->container->hasDefinition('security.authentication.success_handler.test.refresh_jwt'));
         $this->assertTrue($this->container->hasDefinition('security.authentication.failure_handler.test.refresh_jwt'));
+        $this->assertFalse($this->container->hasDefinition('gesdinet_jwt_refresh_token.security.listener.logout.test'));
 
         /** @var ChildDefinition $successHandler */
         $successHandler = $this->container->getDefinition('security.authentication.success_handler.test.refresh_jwt');
diff --git a/Tests/Unit/EventListener/LogoutEventListenerTest.php b/Tests/Unit/EventListener/LogoutEventListenerTest.php
