From 0c2d94621da109615ddc178c076a6ffb4f888385 Mon Sep 17 00:00:00 2001 From: Joseph Bielawski Date: Sat, 26 Aug 2023 18:11:16 +0200 Subject: [PATCH 1/3] Improve compatibility with Symfony 6 --- src/Security/Core/User/OAuthUser.php | 8 ++----- tests/App/AppKernel.php | 27 +++++++--------------- tests/Security/Core/User/OAuthUserTest.php | 5 ---- 3 files changed, 10 insertions(+), 30 deletions(-) diff --git a/src/Security/Core/User/OAuthUser.php b/src/Security/Core/User/OAuthUser.php index 31665d960..cbae2e20d 100644 --- a/src/Security/Core/User/OAuthUser.php +++ b/src/Security/Core/User/OAuthUser.php @@ -53,16 +53,12 @@ public function getUsername(): string return $this->getUserIdentifier(); } - public function eraseCredentials(): bool + public function eraseCredentials(): void { - return true; } 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/tests/App/AppKernel.php b/tests/App/AppKernel.php index 536450386..7354406e9 100644 --- a/tests/App/AppKernel.php +++ b/tests/App/AppKernel.php @@ -22,8 +22,6 @@ use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Kernel; -use Symfony\Component\Security\Core\Security; -use Symfony\Component\Security\Http\Controller\UserValueResolver; final class AppKernel extends Kernel { @@ -58,24 +56,15 @@ public function prepareContainer(ContainerBuilder $container): void { parent::prepareContainer($container); - // With Symfony 5.3+, session settings were changed - if (Kernel::VERSION_ID >= 50300) { - $container->prependExtensionConfig('framework', [ - 'session' => [ - 'enabled' => true, - 'storage_factory_id' => 'session.storage.factory.mock_file', - ], - ]); - } else { - $container->prependExtensionConfig('framework', [ - 'session' => [ - 'enabled' => true, - 'storage_id' => 'session.storage.mock_file', - ], - ]); - } + $container->prependExtensionConfig('framework', [ + 'session' => [ + 'enabled' => true, + 'storage_factory_id' => 'session.storage.factory.mock_file', + ], + ]); - if (method_exists(Security::class, 'getUser') && !class_exists(UserValueResolver::class)) { + // Symfony 5.4, requires additional configuration + if (Kernel::VERSION_ID >= 50100 && Kernel::VERSION_ID < 59999) { $container->loadFromExtension('security', [ 'firewalls' => [ 'login_area' => [ diff --git a/tests/Security/Core/User/OAuthUserTest.php b/tests/Security/Core/User/OAuthUserTest.php index fb0ddabcf..d46a9ee11 100644 --- a/tests/Security/Core/User/OAuthUserTest.php +++ b/tests/Security/Core/User/OAuthUserTest.php @@ -46,11 +46,6 @@ public function testGetUsername(): void $this->assertEquals('other', $user->getUsername()); } - public function testEraseCredentials(): void - { - $this->assertTrue($this->user->eraseCredentials()); - } - public function testEquals(): void { $otherUser = new OAuthUser('other'); From 6e3dfd0b287b5169c3dcce57aaed6a1cc6a230f4 Mon Sep 17 00:00:00 2001 From: Joseph Bielawski Date: Sat, 26 Aug 2023 18:26:56 +0200 Subject: [PATCH 2/3] Improve compatibility with Symfony 6.2+ --- tests/Controller/LoginControllerTest.php | 16 +++++++++++----- .../Controller/LoginControllerTest.php | 18 +++++++----------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/tests/Controller/LoginControllerTest.php b/tests/Controller/LoginControllerTest.php index 6cde6fc8c..082ea1dbe 100644 --- a/tests/Controller/LoginControllerTest.php +++ b/tests/Controller/LoginControllerTest.php @@ -15,6 +15,7 @@ use HWI\Bundle\OAuthBundle\Security\Core\Exception\AccountNotLinkedException; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpFoundation\ParameterBag; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; @@ -23,7 +24,7 @@ use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException; use Symfony\Component\Security\Core\Exception\AuthenticationException; -use Symfony\Component\Security\Core\Security; +use Symfony\Component\Security\Core\Security as DeprecatedSecurity; use Symfony\Component\Security\Http\Authentication\AuthenticationUtils; use Twig\Environment; @@ -119,7 +120,7 @@ public function testRegistrationRedirect(): void { $this->request->attributes = new ParameterBag( [ - Security::AUTHENTICATION_ERROR => new AccountNotLinkedException(), + $this->getSecurityErrorKey() => new AccountNotLinkedException(), ] ); @@ -144,7 +145,7 @@ public function testRequestError(): void $this->request->attributes = new ParameterBag( [ - Security::AUTHENTICATION_ERROR => $authenticationException, + $this->getSecurityErrorKey() => $authenticationException, ] ); @@ -164,7 +165,7 @@ public function testSessionError(): void $this->session->expects($this->once()) ->method('has') - ->with(Security::AUTHENTICATION_ERROR) + ->with($this->getSecurityErrorKey()) ->willReturn(true) ; @@ -172,7 +173,7 @@ public function testSessionError(): void $this->session->expects($this->once()) ->method('get') - ->with(Security::AUTHENTICATION_ERROR) + ->with($this->getSecurityErrorKey()) ->willReturn($authenticationException) ; @@ -207,4 +208,9 @@ private function createController(): LoginController 'IS_AUTHENTICATED_REMEMBERED' ); } + + private function getSecurityErrorKey(): string + { + return class_exists(Security::class) ? Security::AUTHENTICATION_ERROR : DeprecatedSecurity::AUTHENTICATION_ERROR; + } } diff --git a/tests/Functional/Controller/LoginControllerTest.php b/tests/Functional/Controller/LoginControllerTest.php index 87e788e22..577d85694 100644 --- a/tests/Functional/Controller/LoginControllerTest.php +++ b/tests/Functional/Controller/LoginControllerTest.php @@ -16,10 +16,10 @@ use HWI\Bundle\OAuthBundle\Security\Core\Exception\AccountNotLinkedException; use HWI\Bundle\OAuthBundle\Tests\Functional\AuthenticationHelperTrait; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpClient\MockHttpClient; -use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Core\Exception\UserNotFoundException; -use Symfony\Component\Security\Core\Security; +use Symfony\Component\Security\Core\Security as DeprecatedSecurity; use Symfony\Contracts\HttpClient\HttpClientInterface; final class LoginControllerTest extends WebTestCase @@ -48,7 +48,7 @@ public function testRedirectingToRegistrationFormWithError(): void $client = self::createClient(); $session = $this->getSession($client); - $session->set(Security::AUTHENTICATION_ERROR, new AccountNotLinkedException()); + $session->set($this->getSecurityErrorKey(), new AccountNotLinkedException()); $this->saveSession($client, $session); @@ -67,10 +67,10 @@ public function testLoginPageWithError(): void $client = self::createClient(); $client->getContainer()->set('hwi_oauth.http_client', $httpClient); - $exception = $this->createUserNotFoundException(); + $exception = new UserNotFoundException(); $session = $this->getSession($client); - $session->set(Security::AUTHENTICATION_ERROR, $exception); + $session->set($this->getSecurityErrorKey(), $exception); $this->logIn($client, $session); @@ -82,12 +82,8 @@ public function testLoginPageWithError(): void $this->assertSame($exception->getMessageKey(), $crawler->filter('span')->text(), $response->getContent()); } - private function createUserNotFoundException() + private function getSecurityErrorKey(): string { - if (class_exists(UserNotFoundException::class)) { - return new UserNotFoundException(); - } - - return new UsernameNotFoundException(); + return class_exists(Security::class) ? Security::AUTHENTICATION_ERROR : DeprecatedSecurity::AUTHENTICATION_ERROR; } } From 62945aa7521e32d0d0fb55d31672734b9166ab40 Mon Sep 17 00:00:00 2001 From: Joseph Bielawski Date: Sat, 26 Aug 2023 18:39:27 +0200 Subject: [PATCH 3/3] Remove compatibility code for Symfony <5.4 --- tests/App/AppKernel.php | 14 ------ .../Provider/OAuthProviderTest.php | 10 ----- .../Core/User/OAuthUserProviderTest.php | 44 ++++++++----------- .../Authenticator/OAuthAuthenticatorTest.php | 11 ----- 4 files changed, 18 insertions(+), 61 deletions(-) diff --git a/tests/App/AppKernel.php b/tests/App/AppKernel.php index 7354406e9..830521209 100644 --- a/tests/App/AppKernel.php +++ b/tests/App/AppKernel.php @@ -62,20 +62,6 @@ public function prepareContainer(ContainerBuilder $container): void 'storage_factory_id' => 'session.storage.factory.mock_file', ], ]); - - // Symfony 5.4, requires additional configuration - if (Kernel::VERSION_ID >= 50100 && Kernel::VERSION_ID < 59999) { - $container->loadFromExtension('security', [ - 'firewalls' => [ - 'login_area' => [ - 'logout_on_user_change' => true, - ], - 'main' => [ - 'logout_on_user_change' => true, - ], - ], - ]); - } } public function getCacheDir(): string diff --git a/tests/Security/Core/Authentication/Provider/OAuthProviderTest.php b/tests/Security/Core/Authentication/Provider/OAuthProviderTest.php index f4b8fbbdb..8058a3ad7 100644 --- a/tests/Security/Core/Authentication/Provider/OAuthProviderTest.php +++ b/tests/Security/Core/Authentication/Provider/OAuthProviderTest.php @@ -108,11 +108,6 @@ public function testAuthenticatesToken(): void $this->assertNotNull($token->getUser()); $this->assertInstanceOf(OAuthToken::class, $token); - // @deprecated since Symfony 5.4 - if (method_exists($token, 'isAuthenticated')) { - $this->assertTrue($token->isAuthenticated()); - } - $this->assertEquals($expectedToken, $token->getRawToken()); $this->assertEquals($expectedToken['access_token'], $token->getAccessToken()); $this->assertEquals($expectedToken['refresh_token'], $token->getRefreshToken()); @@ -255,11 +250,6 @@ public function testTokenRefreshesWhenExpired(bool $authenticated, bool $userKno $this->assertInstanceOf(OAuthToken::class, $token); - // @deprecated since Symfony 5.4 - if (method_exists($token, 'isAuthenticated')) { - $this->assertTrue($token->isAuthenticated()); - } - $this->assertEquals($refreshedToken, $token->getRawToken()); $this->assertEquals($refreshedToken['access_token'], $token->getAccessToken()); $this->assertEquals($refreshedToken['refresh_token'], $token->getRefreshToken()); diff --git a/tests/Security/Core/User/OAuthUserProviderTest.php b/tests/Security/Core/User/OAuthUserProviderTest.php index aa08511b7..69279f4d1 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 { @@ -28,20 +27,23 @@ protected function setUp(): void $this->provider = new OAuthUserProvider(); } + /** + * @group legacy + */ + public function testLoadUserByUsername(): void + { + $user = $this->provider->loadUserByUsername('asm89'); + + $this->assertInstanceOf(OAuthUser::class, $user); + $this->assertEquals('asm89', $user->getUserIdentifier()); + } + 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 +56,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); @@ -89,11 +86,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/Http/Authenticator/OAuthAuthenticatorTest.php b/tests/Security/Http/Authenticator/OAuthAuthenticatorTest.php index fd0623c18..dbf75a769 100644 --- a/tests/Security/Http/Authenticator/OAuthAuthenticatorTest.php +++ b/tests/Security/Http/Authenticator/OAuthAuthenticatorTest.php @@ -28,7 +28,6 @@ use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\User\InMemoryUser; -use Symfony\Component\Security\Core\User\User; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface; use Symfony\Component\Security\Http\Authentication\AuthenticationSuccessHandlerInterface; @@ -168,11 +167,6 @@ public function testAuthenticate(): void $this->assertEquals($resourceOwnerName, $token->getResourceOwnerName()); $this->assertEquals($user, $token->getUser()); $this->assertEquals('refresh_token', $token->getRefreshToken()); - - // required for compatibility with Symfony 5.4 - if (method_exists($token, 'isAuthenticated')) { - $this->assertTrue($token->isAuthenticated()); - } } public function testOnAuthenticationSuccess(): void @@ -418,11 +412,6 @@ private function getUserResponseMock(): UserResponseInterface private function createUser(): UserInterface { - if (class_exists(User::class)) { - // @phpstan-ignore-next-line Symfony < 5.4 BC layer - return new User('asm89', 'foo', ['ROLE_USER']); - } - return new InMemoryUser('asm89', 'foo', ['ROLE_USER']); }