Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.x] Add forward compat layer for changed logout listener configuration #350

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[1.x] Add forward compat layer for changed logout listener configuration #350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,25 @@

## Unreleased

- [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.4.0

- Dropped support for Symfony 4.4

## 1.3.0

- Added support for partitioned cookies

## 1.2.0

- Added support for `LexikJWTAuthenticationBundle` 3.0
- Added support for Symfony 7.0

## 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
- Deprecated not configuring the request path to check in the `refresh_jwt` authenticator
Expand Down
5 changes: 3 additions & 2 deletions DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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('gesdinet/jwt-refresh-token-bundle', '1.5', 'The "%node%" node is deprecated, enable the "invalidate_token_on_logout" option on the "refresh_jwt" firewall instead.')
->defaultNull()
->info('Name of the firewall that triggers the logout event to hook into')
->end()
->scalarNode('return_expiration')
->defaultFalse()
Expand Down
11 changes: 10 additions & 1 deletion DependencyInjection/GesdinetJWTRefreshTokenExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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\ContainerBuilder;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Extension\Extension;
use Symfony\Component\DependencyInjection\Loader;
use Symfony\Component\DependencyInjection\Parameter;
use Symfony\Component\Security\Http\Event\LogoutEvent;

class GesdinetJWTRefreshTokenExtension extends Extension
{
Expand Down Expand Up @@ -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';

Expand Down
10 changes: 10 additions & 0 deletions DependencyInjection/Security/Factory/RefreshTokenAuthenticatorFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should set it to null here and run a deprecation if the boolean has not been defined in order to activate it by default in 2.0 without BC.

Isn't there a way to get the bundle configuration here or in the createAuthenticator in order to activate it by default when the 'logout_firewall' is null ? This would allow us to keep the default value on 'logout_firewall' without creating a BC and to launch a deprecation when it is defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should set it to null here and run a deprecation if the boolean has not been defined in order to activate it by default in 2.0 without BC.

I'd only do that if we needed the value to actually have a configuration (the same way the deprecation if you don't have the model class set in the bundle config works), because the 1.x branch supports both Symfony 4.4 and 5.4 we can't sanely keep the listener turned on by default for both workflows (which would result in the listener being registered on both the global event dispatcher and the firewall specific event dispatcher, and I have no idea how Symfony deals with merging and de-duping stuff for the firewall specific event dispatcher but I know it does move some stuff around).

Isn't there a way to get the bundle configuration here or in the createAuthenticator in order to activate it by default when the 'logout_firewall' is null ? This would allow us to keep the default value on 'logout_firewall' without creating a BC and to launch a deprecation when it is defined.

We can get to container parameters, but we can't get the processed $config array. For 1.x, I'm choosing to default to the listener being off by default with this backport and then restoring the current 1.1 behavior of it being active by default for 2.0 when we only have a single authentication flow to worry about.

As for the logout_firewall config node, that one's a bit of a pain point to work around. It defaulting to "api" really should've been something added to the Flex recipe for this bundle and not hardcoded into the bundle configuration. So we can't rely on checking if its value matches the current default of "api", we don't know if the user explicitly wanted that or if it was implicitly set to that by way of not configuring it. Changing its default to null gives us a better gauge of whether the node is explicitly configured or not and lets us do stuff with that information at compile time.

->info('When enabled, the refresh token will be invalided on logout.')
->end()
/*
->integerNode('ttl')
->defaultNull()
Expand Down Expand Up @@ -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;
}

Expand Down
23 changes: 18 additions & 5 deletions EventListener/LogoutEventListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -44,15 +48,24 @@ public function __construct(
'partitioned' => false,
'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.5', '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;
}

Expand Down
9 changes: 3 additions & 6 deletions Resources/config/services.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,16 +152,13 @@
])
->tag('console.command');

$services->set(LogoutEventListener::class)
$services->set('gesdinet_jwt_refresh_token.security.listener.logout')
->abstract()
->class(LogoutEventListener::class)
->args([
service('gesdinet.jwtrefreshtoken.refresh_token_manager'),
service('gesdinet.jwtrefreshtoken.request.extractor.chain'),
param('gesdinet_jwt_refresh_token.token_parameter_name'),
param('gesdinet_jwt_refresh_token.cookie'),
param('gesdinet_jwt_refresh_token.logout_firewall_context'),
])
->tag('kernel.event_listener', [
'event' => 'Symfony\Component\Security\Http\Event\LogoutEvent',
'method' => 'onLogout',
]);
};
11 changes: 11 additions & 0 deletions Tests/Functional/DependencyInjection/GesdinetJWTRefreshTokenExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -41,10 +42,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
Expand Down Expand Up @@ -94,10 +98,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
Expand All @@ -106,9 +113,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']);
}
}
37 changes: 36 additions & 1 deletion .../Functional/DependencyInjection/Security/Factory/RefreshTokenAuthenticatorFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

final class RefreshTokenAuthenticatorFactoryTest extends TestCase
{
Expand All @@ -31,13 +32,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');
Expand All @@ -48,6 +52,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(
Expand All @@ -56,13 +89,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');
Expand Down
Loading