From 70c4620cbeecf2117fdc409bbd596a484edeb975 Mon Sep 17 00:00:00 2001 From: Joseph Bielawski Date: Sun, 20 Aug 2023 19:01:48 +0200 Subject: [PATCH] Remove BC layer for Symfony < 5.4 --- .../RefreshOAuthTokenCompilerPass.php | 57 ------------------- .../Factory/OAuthAuthenticatorFactory.php | 16 +----- .../Security/Factory/OAuthFactory.php | 7 +-- src/HWIOAuthBundle.php | 17 +----- .../Response/SensioConnectUserResponse.php | 2 +- src/Security/Core/User/EntityUserProvider.php | 38 +++---------- src/Security/Core/User/OAuthUser.php | 5 +- src/Security/Core/User/OAuthUserProvider.php | 5 +- .../Passport/SelfValidatedOAuthPassport.php | 2 +- .../RefreshAccessTokenListenerOld.php | 39 ------------- .../HWIOAuthExtensionTest.php | 6 +- tests/OAuth/Response/PathUserResponseTest.php | 4 +- .../Core/User/EntityUserProviderTest.php | 9 --- .../Core/User/OAuthUserProviderTest.php | 34 +++-------- tests/Security/Core/User/OAuthUserTest.php | 4 +- 15 files changed, 31 insertions(+), 214 deletions(-) delete mode 100644 src/DependencyInjection/CompilerPass/RefreshOAuthTokenCompilerPass.php delete mode 100644 src/Security/Http/Firewall/RefreshAccessTokenListenerOld.php diff --git a/src/DependencyInjection/CompilerPass/RefreshOAuthTokenCompilerPass.php b/src/DependencyInjection/CompilerPass/RefreshOAuthTokenCompilerPass.php deleted file mode 100644 index cd61c61ec..000000000 --- a/src/DependencyInjection/CompilerPass/RefreshOAuthTokenCompilerPass.php +++ /dev/null @@ -1,57 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace HWI\Bundle\OAuthBundle\DependencyInjection\CompilerPass; - -use Symfony\Component\DependencyInjection\Argument\IteratorArgument; -use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; -use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\Reference; - -/** - * @deprecated For Symfony 4.4 only - * - * Adds already registered by OAuthFactory RefreshAccessTokenListenerOld to security.firewall.map.context. - * It's taking control immediately after token was passed from session to token storage. - */ -final class RefreshOAuthTokenCompilerPass implements CompilerPassInterface -{ - public function process(ContainerBuilder $container): void - { - foreach ($container->getDefinitions() as $listenerId => $listenerDef) { - if (!str_starts_with($listenerId, 'hwi_oauth.context_listener.token_refresher.')) { - continue; - } - - // Cut 'hwi_oauth.context_listener.token_refresher.' - $firewallName = substr($listenerId, 43); - $firewallMapContextId = 'security.firewall.map.context.'.$firewallName; - - if (!$container->has($firewallMapContextId)) { - continue; - } - - $firewallMapContextDef = $container->getDefinition($firewallMapContextId); - /* @var IteratorArgument $listenersIter */ - $listenerIter = $firewallMapContextDef->getArgument(0); - - $listenerRefs = $listenerIter->getValues(); - // add listener after security.context_listener.X - foreach ($listenerRefs as $pos => $posValue) { - if (str_starts_with($posValue, 'security.context_listener.')) { - array_splice($listenerRefs, $pos + 1, 0, [new Reference($listenerId)]); - break; - } - } - $listenerIter->setValues($listenerRefs); - } - } -} diff --git a/src/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php b/src/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php index 5feeff307..e1e645932 100644 --- a/src/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php +++ b/src/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php @@ -13,7 +13,6 @@ use HWI\Bundle\OAuthBundle\Security\Http\Authenticator\OAuthAuthenticator; use HWI\Bundle\OAuthBundle\Security\Http\Firewall\RefreshAccessTokenListener; -use HWI\Bundle\OAuthBundle\Security\Http\Firewall\RefreshAccessTokenListenerOld; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AbstractFactory; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AuthenticatorFactoryInterface; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FirewallListenerFactoryInterface; @@ -96,18 +95,9 @@ public function createListeners(ContainerBuilder $container, string $firewallNam $listenerDef = $container->setDefinition($listenerId, new ChildDefinition('hwi_oauth.context_listener.abstract_token_refresher')); $listenerDef->addMethodCall('setResourceOwnerMap', [$this->createResourceOwnerMapReference($firewallName)]); - - if ($container->hasDefinition($authenticatorId)) { - // new auth manager - $listenerDef - ->setClass(RefreshAccessTokenListener::class) - ->replaceArgument(0, new Reference($authenticatorId)); - } else { - // old auth manager - $listenerDef - ->setClass(RefreshAccessTokenListenerOld::class) - ->replaceArgument(0, new Reference($providerId)); - } + $listenerDef + ->setClass(RefreshAccessTokenListener::class) + ->replaceArgument(0, new Reference($authenticatorId)); return [$listenerId]; } diff --git a/src/DependencyInjection/Security/Factory/OAuthFactory.php b/src/DependencyInjection/Security/Factory/OAuthFactory.php index 970cadc40..c076f644d 100644 --- a/src/DependencyInjection/Security/Factory/OAuthFactory.php +++ b/src/DependencyInjection/Security/Factory/OAuthFactory.php @@ -12,7 +12,7 @@ namespace HWI\Bundle\OAuthBundle\DependencyInjection\Security\Factory; use HWI\Bundle\OAuthBundle\Security\Core\Authentication\Provider\OAuthProvider; -use HWI\Bundle\OAuthBundle\Security\Http\Firewall\RefreshAccessTokenListenerOld; +use HWI\Bundle\OAuthBundle\Security\Http\Firewall\RefreshAccessTokenListener; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AbstractFactory; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\NodeDefinition; @@ -110,8 +110,6 @@ protected function createAuthProvider(ContainerBuilder $container, $id, $config, ->addArgument(new Reference('security.token_storage')) ; - $this->createRefreshTokenListeners($container, $providerId, $id); - $this->firewallNames[$id] = true; return $providerId; @@ -186,8 +184,7 @@ protected function createRefreshTokenListeners(ContainerBuilder $container, stri $listenerDef = $container->setDefinition($listenerId, new ChildDefinition('hwi_oauth.context_listener.abstract_token_refresher')); $listenerDef - ->addMethodCall('setResourceOwnerMap', [$this->createResourceOwnerMapReference($id)]) - ->setClass(RefreshAccessTokenListenerOld::class) + ->setClass(RefreshAccessTokenListener::class) ->replaceArgument(0, new Reference($providerId)); } diff --git a/src/HWIOAuthBundle.php b/src/HWIOAuthBundle.php index 238ffb3c9..62d37f85a 100644 --- a/src/HWIOAuthBundle.php +++ b/src/HWIOAuthBundle.php @@ -12,15 +12,12 @@ namespace HWI\Bundle\OAuthBundle; use HWI\Bundle\OAuthBundle\DependencyInjection\CompilerPass\EnableRefreshOAuthTokenListenerCompilerPass; -use HWI\Bundle\OAuthBundle\DependencyInjection\CompilerPass\RefreshOAuthTokenCompilerPass; use HWI\Bundle\OAuthBundle\DependencyInjection\CompilerPass\ResourceOwnerCompilerPass; use HWI\Bundle\OAuthBundle\DependencyInjection\Security\Factory\OAuthAuthenticatorFactory; -use HWI\Bundle\OAuthBundle\DependencyInjection\Security\Factory\OAuthFactory; use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Extension\ExtensionInterface; use Symfony\Component\HttpKernel\Bundle\Bundle; -use Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface; /** * @author Geoffrey Bachelet @@ -37,19 +34,7 @@ public function build(ContainerBuilder $container) /** @var SecurityExtension $extension */ $extension = $container->getExtension('security'); - - $firewallNames = $this->extension->getFirewallNames(); - - if (method_exists($extension, 'addAuthenticatorFactory')) { - $extension->addAuthenticatorFactory(new OAuthAuthenticatorFactory($firewallNames)); - } elseif (interface_exists(AuthenticationProviderInterface::class)) { - // @phpstan-ignore-next-line Symfony 4.4 BC layer - $extension->addSecurityListenerFactory(new OAuthFactory($firewallNames)); - $container->addCompilerPass(new RefreshOAuthTokenCompilerPass()); - } else { - // @phpstan-ignore-next-line Symfony < 5.4 BC layer - $extension->addSecurityListenerFactory(new OAuthAuthenticatorFactory($firewallNames)); - } + $extension->addAuthenticatorFactory(new OAuthAuthenticatorFactory($this->extension->getFirewallNames())); $container->addCompilerPass(new ResourceOwnerCompilerPass()); $container->addCompilerPass(new EnableRefreshOAuthTokenListenerCompilerPass()); diff --git a/src/OAuth/Response/SensioConnectUserResponse.php b/src/OAuth/Response/SensioConnectUserResponse.php index 7a248e6a6..574c14311 100644 --- a/src/OAuth/Response/SensioConnectUserResponse.php +++ b/src/OAuth/Response/SensioConnectUserResponse.php @@ -33,7 +33,7 @@ final class SensioConnectUserResponse extends AbstractUserResponse */ public function getUserIdentifier(): string { - /** @var \DOMAttr $attribute */ + /** @var \DOMAttr|null $attribute */ $attribute = $this->data->attributes->getNamedItem('id'); if (null === $attribute->value) { throw new \InvalidArgumentException('User identifier was not found in response.'); diff --git a/src/Security/Core/User/EntityUserProvider.php b/src/Security/Core/User/EntityUserProvider.php index 314ddd710..6b574d73e 100644 --- a/src/Security/Core/User/EntityUserProvider.php +++ b/src/Security/Core/User/EntityUserProvider.php @@ -17,7 +17,6 @@ use HWI\Bundle\OAuthBundle\OAuth\Response\UserResponseInterface; use Symfony\Component\PropertyAccess\PropertyAccess; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; -use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Core\Exception\UserNotFoundException; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; @@ -57,10 +56,7 @@ public function loadUserByIdentifier(string $identifier): UserInterface $user = $this->findUser(['username' => $identifier]); if (!$user) { - $exception = new UserNotFoundException(sprintf("User '%s' not found.", $identifier)); - $exception->setUserIdentifier($identifier); - - throw $exception; + throw $this->createUserNotFoundException($identifier, sprintf("User '%s' not found.", $username)); } return $user; @@ -75,12 +71,7 @@ public function loadUserByIdentifier(string $identifier): UserInterface */ public function loadUserByUsername($username) { - $user = $this->findUser(['username' => $username]); - if (!$user) { - throw $this->createUserNotFoundException($username, sprintf("User '%s' not found.", $username)); - } - - return $user; + return $this->loadUserByIdentifier($username); } public function loadUserByOAuthUserResponse(UserResponseInterface $response): ?UserInterface @@ -109,11 +100,10 @@ public function refreshUser(UserInterface $user): ?UserInterface $userId = $accessor->getValue($user, $identifier); - // @phpstan-ignore-next-line Symfony <5.4 BC layer - $username = method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername(); + $userIdentifier = $user->getUserIdentifier(); if (null === $user = $this->findUser([$identifier => $userId])) { - throw $this->createUserNotFoundException($username, sprintf('User with ID "%d" could not be reloaded.', $userId)); + throw $this->createUserNotFoundException($userIdentifier, sprintf('User with ID "%d" could not be reloaded.', $userId)); } return $user; @@ -133,24 +123,10 @@ private function findUser(array $criteria): ?UserInterface return $this->repository->findOneBy($criteria); } - /** - * @return UsernameNotFoundException|UserNotFoundException - */ - private function createUserNotFoundException(string $username, string $message) + private function createUserNotFoundException(string $identifier, string $message): UserNotFoundException { - if (class_exists(UserNotFoundException::class)) { - $exception = new UserNotFoundException($message); - $exception->setUserIdentifier($username); - } else { - if (!class_exists(UsernameNotFoundException::class)) { - throw new \RuntimeException('Unsupported Symfony version used!'); - } - - $exception = new UsernameNotFoundException($message); - if (method_exists($exception, 'setUsername')) { - $exception->setUsername($username); // @phpstan-ignore-this-line Symfony <5.4 BC layer - } - } + $exception = new UserNotFoundException($message); + $exception->setUserIdentifier($identifier); return $exception; } diff --git a/src/Security/Core/User/OAuthUser.php b/src/Security/Core/User/OAuthUser.php index 31665d960..d6a9feed3 100644 --- a/src/Security/Core/User/OAuthUser.php +++ b/src/Security/Core/User/OAuthUser.php @@ -60,9 +60,6 @@ public function eraseCredentials(): bool public function equals(UserInterface $user): bool { - // @phpstan-ignore-next-line Symfony <5.4 BC layer - $username = method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername(); - - return $username === $this->username; + return $user->getUserIdentifier() === $this->username; } } diff --git a/src/Security/Core/User/OAuthUserProvider.php b/src/Security/Core/User/OAuthUserProvider.php index 5067f7302..6d27f6fc2 100644 --- a/src/Security/Core/User/OAuthUserProvider.php +++ b/src/Security/Core/User/OAuthUserProvider.php @@ -49,10 +49,7 @@ public function refreshUser(UserInterface $user): UserInterface throw new UnsupportedUserException(sprintf('Unsupported user class "%s"', \get_class($user))); } - // @phpstan-ignore-next-line Symfony <5.4 BC layer - $username = method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername(); - - return $this->loadUserByUsername($username); + return $this->loadUserByUsername($user->getUserIdentifier()); } public function supportsClass($class): bool diff --git a/src/Security/Http/Authenticator/Passport/SelfValidatedOAuthPassport.php b/src/Security/Http/Authenticator/Passport/SelfValidatedOAuthPassport.php index 3d60e4f77..11bdefe21 100644 --- a/src/Security/Http/Authenticator/Passport/SelfValidatedOAuthPassport.php +++ b/src/Security/Http/Authenticator/Passport/SelfValidatedOAuthPassport.php @@ -36,7 +36,7 @@ public function __construct(OAuthToken $token, array $badges = []) $userBadge = class_exists(UserBadge::class) ? new UserBadge( - method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername(), + $user->getUserIdentifier(), static function () use ($user) { return $user; } ) : $user; diff --git a/src/Security/Http/Firewall/RefreshAccessTokenListenerOld.php b/src/Security/Http/Firewall/RefreshAccessTokenListenerOld.php deleted file mode 100644 index e8d49da0d..000000000 --- a/src/Security/Http/Firewall/RefreshAccessTokenListenerOld.php +++ /dev/null @@ -1,39 +0,0 @@ - - * - * 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\Firewall; - -use HWI\Bundle\OAuthBundle\Security\Core\Authentication\Token\OAuthToken; -use Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface; - -class RefreshAccessTokenListenerOld extends RefreshAccessTokenListener -{ - private AuthenticationProviderInterface $oAuthProvider; - - public function __construct( - AuthenticationProviderInterface $oAuthProvider - ) { - $this->oAuthProvider = $oAuthProvider; - } - - /** - * @template T of OAuthToken - * - * @param T $token - * - * @return T - */ - protected function refreshToken(OAuthToken $token): OAuthToken - { - // @phpstan-ignore-next-line returns TokenInterface instead of OAuthToken - return $this->oAuthProvider->authenticate($token); - } -} diff --git a/tests/DependencyInjection/HWIOAuthExtensionTest.php b/tests/DependencyInjection/HWIOAuthExtensionTest.php index f28c99966..0744d1613 100644 --- a/tests/DependencyInjection/HWIOAuthExtensionTest.php +++ b/tests/DependencyInjection/HWIOAuthExtensionTest.php @@ -282,7 +282,7 @@ public function testConfigurationPassValidOAuth2WithPaths(): void } /** - * @legacy + * @group legacy */ public function testConfigurationPassValidOAuth1WithClass(): void { @@ -312,7 +312,7 @@ public function testConfigurationPassValidOAuth1WithClass(): void } /** - * @legacy + * @group legacy */ public function testConfigurationPassValidOAuth2WithClassOnly(): void { @@ -333,7 +333,7 @@ public function testConfigurationPassValidOAuth2WithClassOnly(): void } /** - * @legacy + * @group legacy */ public function testConfigurationPassValidOAuth2WithPathsAndClass(): void { diff --git a/tests/OAuth/Response/PathUserResponseTest.php b/tests/OAuth/Response/PathUserResponseTest.php index ba4192c9c..e9e1352cc 100644 --- a/tests/OAuth/Response/PathUserResponseTest.php +++ b/tests/OAuth/Response/PathUserResponseTest.php @@ -95,7 +95,7 @@ public function testGetUserIdentifier(): void $this->responseObject->setPaths($paths); $this->responseObject->setData(json_encode(['id' => 666])); - $this->assertEquals('666', $this->responseObject->getUserIdentifier()); + $this->assertEquals('666', $this->responseObject->getUsername()); } public function testGetUserIdentifierWithoutResponseThrowsException(): void @@ -126,7 +126,7 @@ public function testGetUsername(): void public function testGetUsernameWithoutResponseReturnsNull(): void { $this->responseObject->setPaths(['identifier' => 'id']); - $this->assertNull($this->responseObject->getUsername()); + $this->assertNull($this->responseObject->getUserIdentifier()); } public function testGetNickname(): void diff --git a/tests/Security/Core/User/EntityUserProviderTest.php b/tests/Security/Core/User/EntityUserProviderTest.php index e39f829c8..ece018106 100644 --- a/tests/Security/Core/User/EntityUserProviderTest.php +++ b/tests/Security/Core/User/EntityUserProviderTest.php @@ -49,15 +49,6 @@ public function testLoadUserByIdentifierThrowsExceptionWhenUserIsNotFound(): voi $provider->loadUserByIdentifier('asm89'); } - public function testLoadUserByUsernameThrowsExceptionWhenUserIsNull(): void - { - $this->assertUserNotFoundException(); - $this->expectExceptionMessage("User 'asm89' not found."); - - $provider = $this->createEntityUserProvider(); - $provider->loadUserByUsername('asm89'); - } - public function testLoadUserByOAuthUserResponseThrowsExceptionWhenNoPropertyIsConfigured(): void { $this->expectException(\RuntimeException::class); diff --git a/tests/Security/Core/User/OAuthUserProviderTest.php b/tests/Security/Core/User/OAuthUserProviderTest.php index aa08511b7..02df5f869 100644 --- a/tests/Security/Core/User/OAuthUserProviderTest.php +++ b/tests/Security/Core/User/OAuthUserProviderTest.php @@ -17,7 +17,6 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; use Symfony\Component\Security\Core\User\InMemoryUser; -use Symfony\Component\Security\Core\User\User; final class OAuthUserProviderTest extends TestCase { @@ -30,18 +29,10 @@ protected function setUp(): void public function testLoadUserByUsernameOrIdentifier(): void { - if (method_exists($this->provider, 'loadUserByUsername')) { - $user = $this->provider->loadUserByUsername('asm89'); - - $this->assertInstanceOf(OAuthUser::class, $user); - // @phpstan-ignore-next-line Symfony < 5.4 BC layer - $this->assertEquals('asm89', $user->getUsername()); - } else { - $user = $this->provider->loadUserByIdentifier('asm89'); - - $this->assertInstanceOf(OAuthUser::class, $user); - $this->assertEquals('asm89', $user->getUserIdentifier()); - } + $user = $this->provider->loadUserByIdentifier('asm89'); + + $this->assertInstanceOf(OAuthUser::class, $user); + $this->assertEquals('asm89', $user->getUserIdentifier()); } public function testRefreshUser(): void @@ -54,13 +45,8 @@ public function testRefreshUser(): void public function testRefreshUserUnsupportedClass(): void { - if (class_exists(User::class)) { - $user = new User('asm89', 'foo'); - $message = 'Unsupported user class "Symfony\\Component\\Security\\Core\\User\\User"'; - } else { - $user = new InMemoryUser('asm89', 'foo'); - $message = 'Unsupported user class "Symfony\\Component\\Security\\Core\\User\\InMemoryUser"'; - } + $user = new InMemoryUser('asm89', 'foo'); + $message = 'Unsupported user class "Symfony\\Component\\Security\\Core\\User\\InMemoryUser"'; $this->expectException(UnsupportedUserException::class); $this->expectExceptionMessage($message); @@ -88,12 +74,6 @@ public function testLoadUserByOAuthUserResponse(): void $user = $this->provider->loadUserByOAuthUserResponse($responseMock); $this->assertInstanceOf(OAuthUser::class, $user); - - if (method_exists($this->provider, 'loadUserByUsername')) { - // @phpstan-ignore-next-line Symfony < 5.4 BC layer - $this->assertEquals('asm89', $user->getUsername()); - } else { - $this->assertEquals('asm89', $user->getUserIdentifier()); - } + $this->assertEquals('asm89', $user->getUserIdentifier()); } } diff --git a/tests/Security/Core/User/OAuthUserTest.php b/tests/Security/Core/User/OAuthUserTest.php index fb0ddabcf..a701fc770 100644 --- a/tests/Security/Core/User/OAuthUserTest.php +++ b/tests/Security/Core/User/OAuthUserTest.php @@ -40,10 +40,10 @@ public function testGetSalt(): void public function testGetUsername(): void { - $this->assertEquals('asm89', $this->user->getUsername()); + $this->assertEquals('asm89', $this->user->getUserIdentifier()); $user = new OAuthUser('other'); - $this->assertEquals('other', $user->getUsername()); + $this->assertEquals('other', $user->getUserIdentifier()); } public function testEraseCredentials(): void