From 7ef036c74f349f6fd2b891357394d70b0ed6a3c8 Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Sun, 15 Jan 2023 18:57:26 +0100 Subject: [PATCH 01/20] [Tests] New iteration of removing `$this` occurrences in future static data providers --- ...efaultAuthenticationSuccessHandlerTest.php | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/Tests/Authentication/DefaultAuthenticationSuccessHandlerTest.php b/Tests/Authentication/DefaultAuthenticationSuccessHandlerTest.php index 852fb4bc..2d63821b 100644 --- a/Tests/Authentication/DefaultAuthenticationSuccessHandlerTest.php +++ b/Tests/Authentication/DefaultAuthenticationSuccessHandlerTest.php @@ -38,7 +38,7 @@ public function testRequestRedirections(Request $request, $options, $redirectedU $this->assertSame('http://localhost'.$redirectedUrl, $handler->onAuthenticationSuccess($request, $token)->getTargetUrl()); } - public function getRequestRedirections() + public function testRequestRedirectionsWithTargetPathInSessions() { $session = $this->createMock(SessionInterface::class); $session->expects($this->once())->method('get')->with('_security.admin.target_path')->willReturn('/admin/dashboard'); @@ -46,6 +46,18 @@ public function getRequestRedirections() $requestWithSession = Request::create('/'); $requestWithSession->setSession($session); + $urlGenerator = $this->createMock(UrlGeneratorInterface::class); + $urlGenerator->expects($this->any())->method('generate')->willReturn('http://localhost/login'); + $httpUtils = new HttpUtils($urlGenerator); + $token = $this->createMock(TokenInterface::class); + $handler = new DefaultAuthenticationSuccessHandler($httpUtils); + $handler->setFirewallName('admin'); + + $this->assertSame('http://localhost/admin/dashboard', $handler->onAuthenticationSuccess($requestWithSession, $token)->getTargetUrl()); + } + + public static function getRequestRedirections() + { return [ 'default' => [ Request::create('/'), @@ -72,11 +84,6 @@ public function getRequestRedirections() ['target_path_parameter' => '_target_path[value]'], '/dashboard', ], - 'target path in session' => [ - $requestWithSession, - [], - '/admin/dashboard', - ], 'target path as referer' => [ Request::create('/', 'GET', [], [], [], ['HTTP_REFERER' => 'http://localhost/dashboard']), ['use_referer' => true], From 0436070c90e7c5197c564933ac1d52a17baa894a Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Tue, 24 Jan 2023 15:02:24 +0100 Subject: [PATCH 02/20] Update license years (last time) --- LICENSE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENSE b/LICENSE index 00837045..0138f8f0 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2004-2023 Fabien Potencier +Copyright (c) 2004-present Fabien Potencier Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal From a906fb4a4b38c994e9f2f4e81b134a6540ee338e Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Tue, 7 Feb 2023 21:09:05 +0100 Subject: [PATCH 03/20] [Tests] Migrate tests to static data providers --- .../PasswordMigratingListenerTest.php | 73 +++++++++++-- Tests/Fixtures/DummyAuthenticator.php | 53 +++++++++ Tests/Fixtures/DummyToken.php | 101 ++++++++++++++++++ 3 files changed, 217 insertions(+), 10 deletions(-) create mode 100644 Tests/Fixtures/DummyAuthenticator.php create mode 100644 Tests/Fixtures/DummyToken.php diff --git a/Tests/EventListener/PasswordMigratingListenerTest.php b/Tests/EventListener/PasswordMigratingListenerTest.php index 9f8e218e..afdfd86c 100644 --- a/Tests/EventListener/PasswordMigratingListenerTest.php +++ b/Tests/EventListener/PasswordMigratingListenerTest.php @@ -15,13 +15,11 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; use Symfony\Component\PasswordHasher\PasswordHasherInterface; -use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\PasswordUpgraderInterface; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; -use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\PasswordUpgradeBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface; @@ -29,6 +27,8 @@ use Symfony\Component\Security\Http\Authenticator\Passport\UserPassportInterface; use Symfony\Component\Security\Http\Event\LoginSuccessEvent; use Symfony\Component\Security\Http\EventListener\PasswordMigratingListener; +use Symfony\Component\Security\Http\Tests\Fixtures\DummyAuthenticator; +use Symfony\Component\Security\Http\Tests\Fixtures\DummyToken; class PasswordMigratingListenerTest extends TestCase { @@ -58,13 +58,13 @@ public function testUnsupportedEvents($event) $this->listener->onLoginSuccess($event); } - public function provideUnsupportedEvents() + public static function provideUnsupportedEvents() { // no password upgrade badge - yield [$this->createEvent(new SelfValidatingPassport(new UserBadge('test', function () { return $this->createMock(UserInterface::class); })))]; + yield [self::createEvent(new SelfValidatingPassport(new UserBadge('test', function () { return $this->createMock(UserInterface::class); })))]; // blank password - yield [$this->createEvent(new SelfValidatingPassport(new UserBadge('test', function () { return $this->createMock(TestPasswordAuthenticatedUser::class); }), [new PasswordUpgradeBadge('', $this->createPasswordUpgrader())]))]; + yield [self::createEvent(new SelfValidatingPassport(new UserBadge('test', function () { return new DummyTestPasswordAuthenticatedUser(); }), [new PasswordUpgradeBadge('', self::createPasswordUpgrader())]))]; } /** @@ -96,7 +96,7 @@ public function testUnsupportedPassport() public function testUpgradeWithUpgrader() { - $passwordUpgrader = $this->createPasswordUpgrader(); + $passwordUpgrader = $this->getMockForAbstractClass(TestMigratingUserProvider::class); $passwordUpgrader->expects($this->once()) ->method('upgradePassword') ->with($this->user, 'new-hash') @@ -133,14 +133,14 @@ public function testUserWithoutPassword() $this->listener->onLoginSuccess($event); } - private function createPasswordUpgrader() + private static function createPasswordUpgrader() { - return $this->getMockForAbstractClass(TestMigratingUserProvider::class); + return new DummyTestMigratingUserProvider(); } - private function createEvent(PassportInterface $passport) + private static function createEvent(PassportInterface $passport) { - return new LoginSuccessEvent($this->createMock(AuthenticatorInterface::class), $passport, $this->createMock(TokenInterface::class), new Request(), null, 'main'); + return new LoginSuccessEvent(new DummyAuthenticator(), $passport, new DummyToken(), new Request(), null, 'main'); } } @@ -151,9 +151,62 @@ abstract public function upgradePassword(PasswordAuthenticatedUserInterface $use abstract public function loadUserByIdentifier(string $identifier): UserInterface; } +class DummyTestMigratingUserProvider extends TestMigratingUserProvider +{ + public function upgradePassword(PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void + { + } + + public function loadUserByIdentifier(string $identifier): UserInterface + { + } + + public function refreshUser(UserInterface $user) + { + } + + public function supportsClass(string $class) + { + } + + public function loadUserByUsername(string $username) + { + } +} + abstract class TestPasswordAuthenticatedUser implements UserInterface, PasswordAuthenticatedUserInterface { abstract public function getPassword(): ?string; abstract public function getSalt(): ?string; } + +class DummyTestPasswordAuthenticatedUser extends TestPasswordAuthenticatedUser +{ + public function getPassword(): ?string + { + return null; + } + + public function getSalt(): ?string + { + return null; + } + + public function getRoles(): array + { + return []; + } + + public function eraseCredentials() + { + } + + public function getUsername() + { + } + + public function getUserIdentifier(): string + { + } +} diff --git a/Tests/Fixtures/DummyAuthenticator.php b/Tests/Fixtures/DummyAuthenticator.php new file mode 100644 index 00000000..0b221813 --- /dev/null +++ b/Tests/Fixtures/DummyAuthenticator.php @@ -0,0 +1,53 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Tests\Fixtures; + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Exception\AuthenticationException; +use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface; +use Symfony\Component\Security\Http\Authenticator\Passport\Passport; +use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface; + +/** + * @author Alexandre Daubois + */ +class DummyAuthenticator implements AuthenticatorInterface +{ + public function supports(Request $request): ?bool + { + return null; + } + + public function authenticate(Request $request): Passport + { + } + + public function createToken(Passport $passport, string $firewallName): TokenInterface + { + } + + public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response + { + return null; + } + + public function onAuthenticationFailure(Request $request, AuthenticationException $exception): ?Response + { + return null; + } + + public function createAuthenticatedToken(PassportInterface $passport, string $firewallName): TokenInterface + { + } +} diff --git a/Tests/Fixtures/DummyToken.php b/Tests/Fixtures/DummyToken.php new file mode 100644 index 00000000..0e921dbf --- /dev/null +++ b/Tests/Fixtures/DummyToken.php @@ -0,0 +1,101 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Tests\Fixtures; + +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\User\UserInterface; + +/** + * @author Alexandre Daubois + */ +class DummyToken implements TokenInterface +{ + public function serialize() + { + } + + public function unserialize($data) + { + } + + public function __toString(): string + { + } + + public function getRoleNames(): array + { + } + + public function getCredentials(): mixed + { + } + + public function getUser(): ?UserInterface + { + } + + public function setUser($user) + { + } + + public function isAuthenticated(): bool + { + } + + public function setAuthenticated(bool $isAuthenticated) + { + } + + public function eraseCredentials(): void + { + } + + public function getAttributes(): array + { + } + + public function setAttributes(array $attributes): void + { + } + + public function hasAttribute(string $name): bool + { + } + + public function getAttribute(string $name): mixed + { + } + + public function setAttribute(string $name, $value): void + { + } + + public function getUsername(): string + { + } + + public function getUserIdentifier(): string + { + } + + public function __serialize(): array + { + } + + public function __unserialize(array $data): void + { + } + + public function __call(string $name, array $arguments) + { + } +} From 139e88e13d253affa2a9e6d206a95e19d6fdbdbf Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 11 Feb 2023 11:47:45 +0100 Subject: [PATCH 04/20] Fix merge --- Tests/EventListener/PasswordMigratingListenerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/EventListener/PasswordMigratingListenerTest.php b/Tests/EventListener/PasswordMigratingListenerTest.php index afdfd86c..36fdfa5c 100644 --- a/Tests/EventListener/PasswordMigratingListenerTest.php +++ b/Tests/EventListener/PasswordMigratingListenerTest.php @@ -61,7 +61,7 @@ public function testUnsupportedEvents($event) public static function provideUnsupportedEvents() { // no password upgrade badge - yield [self::createEvent(new SelfValidatingPassport(new UserBadge('test', function () { return $this->createMock(UserInterface::class); })))]; + yield [self::createEvent(new SelfValidatingPassport(new UserBadge('test', function () { return new DummyTestPasswordAuthenticatedUser(); })))]; // blank password yield [self::createEvent(new SelfValidatingPassport(new UserBadge('test', function () { return new DummyTestPasswordAuthenticatedUser(); }), [new PasswordUpgradeBadge('', self::createPasswordUpgrader())]))]; From 2f8af972eeb99d8d42b313c475a26bad6bd66a66 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 11 Feb 2023 12:06:54 +0100 Subject: [PATCH 05/20] Fix merge --- Tests/EventListener/PasswordMigratingListenerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/EventListener/PasswordMigratingListenerTest.php b/Tests/EventListener/PasswordMigratingListenerTest.php index 202ce15a..cf5fc6af 100644 --- a/Tests/EventListener/PasswordMigratingListenerTest.php +++ b/Tests/EventListener/PasswordMigratingListenerTest.php @@ -110,7 +110,7 @@ private static function createPasswordUpgrader() return new DummyTestMigratingUserProvider(); } - private static function createEvent(PassportInterface $passport) + private static function createEvent(Passport $passport) { return new LoginSuccessEvent(new DummyAuthenticator(), $passport, new DummyToken(), new Request(), null, 'main'); } From 7dd57bc53dc219ebb3925f6dbd31877ea33679cd Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 11 Feb 2023 16:37:25 +0100 Subject: [PATCH 06/20] fix test --- Tests/EventListener/PasswordMigratingListenerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/EventListener/PasswordMigratingListenerTest.php b/Tests/EventListener/PasswordMigratingListenerTest.php index cf5fc6af..fb6406f3 100644 --- a/Tests/EventListener/PasswordMigratingListenerTest.php +++ b/Tests/EventListener/PasswordMigratingListenerTest.php @@ -133,7 +133,7 @@ public function loadUserByIdentifier(string $identifier): UserInterface { } - public function refreshUser(UserInterface $user) + public function refreshUser(UserInterface $user): UserInterface { } From e0fb245cdff3d162438f0ee44d10fabef00ca0ad Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 11 Feb 2023 16:42:41 +0100 Subject: [PATCH 07/20] fix test --- Tests/EventListener/PasswordMigratingListenerTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/EventListener/PasswordMigratingListenerTest.php b/Tests/EventListener/PasswordMigratingListenerTest.php index fb6406f3..5b81732c 100644 --- a/Tests/EventListener/PasswordMigratingListenerTest.php +++ b/Tests/EventListener/PasswordMigratingListenerTest.php @@ -137,11 +137,11 @@ public function refreshUser(UserInterface $user): UserInterface { } - public function supportsClass(string $class) + public function supportsClass(string $class): bool { } - public function loadUserByUsername(string $username) + public function loadUserByUsername(string $username): UserInterface { } } From 559bf2a0ac9fc167526fbfc88682bcee1d7eb5d0 Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Wed, 8 Feb 2023 13:54:12 +0100 Subject: [PATCH 08/20] [Tests] Migrate tests to static data providers --- .../AuthenticatorManagerTest.php | 20 +++++++---- Tests/Fixtures/DummySupportsAuthenticator.php | 35 +++++++++++++++++++ 2 files changed, 48 insertions(+), 7 deletions(-) create mode 100644 Tests/Fixtures/DummySupportsAuthenticator.php diff --git a/Tests/Authentication/AuthenticatorManagerTest.php b/Tests/Authentication/AuthenticatorManagerTest.php index 96dea273..01677154 100644 --- a/Tests/Authentication/AuthenticatorManagerTest.php +++ b/Tests/Authentication/AuthenticatorManagerTest.php @@ -33,6 +33,7 @@ use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport; use Symfony\Component\Security\Http\Event\AuthenticationTokenCreatedEvent; use Symfony\Component\Security\Http\Event\CheckPassportEvent; +use Symfony\Component\Security\Http\Tests\Fixtures\DummySupportsAuthenticator; class AuthenticatorManagerTest extends TestCase { @@ -64,15 +65,15 @@ public function testSupports($authenticators, $result) $this->assertEquals($result, $manager->supports($this->request)); } - public function provideSupportsData() + public static function provideSupportsData() { - yield [[$this->createAuthenticator(null), $this->createAuthenticator(null)], null]; - yield [[$this->createAuthenticator(null), $this->createAuthenticator(false)], null]; + yield [[self::createDummySupportsAuthenticator(null), self::createDummySupportsAuthenticator(null)], null]; + yield [[self::createDummySupportsAuthenticator(null), self::createDummySupportsAuthenticator(false)], null]; - yield [[$this->createAuthenticator(null), $this->createAuthenticator(true)], true]; - yield [[$this->createAuthenticator(true), $this->createAuthenticator(false)], true]; + yield [[self::createDummySupportsAuthenticator(null), self::createDummySupportsAuthenticator(true)], true]; + yield [[self::createDummySupportsAuthenticator(true), self::createDummySupportsAuthenticator(false)], true]; - yield [[$this->createAuthenticator(false), $this->createAuthenticator(false)], false]; + yield [[self::createDummySupportsAuthenticator(false), self::createDummySupportsAuthenticator(false)], false]; yield [[], false]; } @@ -351,7 +352,7 @@ public function log($level, $message, array $context = []): void $this->assertStringContainsString('Mock_TestInteractiveAuthenticator', $logger->logContexts[0]['authenticator']); } - private function createAuthenticator($supports = true) + private function createAuthenticator(?bool $supports = true) { $authenticator = $this->createMock(TestInteractiveAuthenticator::class); $authenticator->expects($this->any())->method('supports')->willReturn($supports); @@ -359,6 +360,11 @@ private function createAuthenticator($supports = true) return $authenticator; } + private static function createDummySupportsAuthenticator(?bool $supports = true) + { + return new DummySupportsAuthenticator($supports); + } + private function createManager($authenticators, $firewallName = 'main', $eraseCredentials = true, array $requiredBadges = [], LoggerInterface $logger = null) { return new AuthenticatorManager($authenticators, $this->tokenStorage, $this->eventDispatcher, $firewallName, $logger, $eraseCredentials, true, $requiredBadges); diff --git a/Tests/Fixtures/DummySupportsAuthenticator.php b/Tests/Fixtures/DummySupportsAuthenticator.php new file mode 100644 index 00000000..e2a037cc --- /dev/null +++ b/Tests/Fixtures/DummySupportsAuthenticator.php @@ -0,0 +1,35 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Tests\Fixtures; + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Exception\AuthenticationException; +use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface; +use Symfony\Component\Security\Http\Authenticator\Passport\Passport; +use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface; + +class DummySupportsAuthenticator extends DummyAuthenticator +{ + private $supports; + + public function __construct(?bool $supports) + { + $this->supports = $supports; + } + + public function supports(Request $request): ?bool + { + return $this->supports; + } +} From 42ae624fc518d8420f283e3e48a245681195cb74 Mon Sep 17 00:00:00 2001 From: Oskar Stark Date: Wed, 14 Dec 2022 15:42:16 +0100 Subject: [PATCH 09/20] Migrate to `static` data providers using `rector/rector` --- Tests/Authentication/AuthenticatorManagerTest.php | 4 ++-- Tests/Authenticator/AbstractLoginFormAuthenticatorTest.php | 2 +- Tests/Authenticator/FormLoginAuthenticatorTest.php | 6 +++--- Tests/Authenticator/HttpBasicAuthenticatorTest.php | 2 +- Tests/Authenticator/JsonLoginAuthenticatorTest.php | 6 +++--- Tests/Authenticator/LoginLinkAuthenticatorTest.php | 2 +- Tests/Authenticator/RememberMeAuthenticatorTest.php | 2 +- Tests/Authenticator/RemoteUserAuthenticatorTest.php | 2 +- Tests/EntryPoint/RetryAuthenticationEntryPointTest.php | 2 +- Tests/EventListener/CheckCredentialsListenerTest.php | 4 ++-- .../EventListener/CheckRememberMeConditionsListenerTest.php | 2 +- Tests/EventListener/UserProviderListenerTest.php | 2 +- Tests/Firewall/ContextListenerTest.php | 2 +- Tests/Firewall/ExceptionListenerTest.php | 4 ++-- Tests/Firewall/LogoutListenerTest.php | 2 +- .../UsernamePasswordFormAuthenticationListenerTest.php | 6 +++--- Tests/HttpUtilsTest.php | 4 ++-- Tests/LoginLink/LoginLinkHandlerTest.php | 2 +- Tests/RememberMe/AbstractRememberMeServicesTest.php | 4 ++-- Tests/RememberMe/TokenBasedRememberMeServicesTest.php | 2 +- 20 files changed, 31 insertions(+), 31 deletions(-) diff --git a/Tests/Authentication/AuthenticatorManagerTest.php b/Tests/Authentication/AuthenticatorManagerTest.php index 01677154..f1eddd09 100644 --- a/Tests/Authentication/AuthenticatorManagerTest.php +++ b/Tests/Authentication/AuthenticatorManagerTest.php @@ -119,7 +119,7 @@ public function testAuthenticateRequest($matchingAuthenticatorIndex) $this->assertTrue($listenerCalled, 'The CheckPassportEvent listener is not called'); } - public function provideMatchingAuthenticatorIndex() + public static function provideMatchingAuthenticatorIndex() { yield [0]; yield [1]; @@ -189,7 +189,7 @@ public function testEraseCredentials($eraseCredentials) $manager->authenticateRequest($this->request); } - public function provideEraseCredentialsData() + public static function provideEraseCredentialsData() { yield [true]; yield [false]; diff --git a/Tests/Authenticator/AbstractLoginFormAuthenticatorTest.php b/Tests/Authenticator/AbstractLoginFormAuthenticatorTest.php index 3a50c131..bbedd40b 100644 --- a/Tests/Authenticator/AbstractLoginFormAuthenticatorTest.php +++ b/Tests/Authenticator/AbstractLoginFormAuthenticatorTest.php @@ -30,7 +30,7 @@ public function testSupports(string $loginUrl, Request $request, bool $expected) $this->assertSame($expected, $authenticator->supports($request)); } - public function provideSupportsData(): iterable + public static function provideSupportsData(): iterable { yield [ '/login', diff --git a/Tests/Authenticator/FormLoginAuthenticatorTest.php b/Tests/Authenticator/FormLoginAuthenticatorTest.php index 3536b103..aa1ae8a9 100644 --- a/Tests/Authenticator/FormLoginAuthenticatorTest.php +++ b/Tests/Authenticator/FormLoginAuthenticatorTest.php @@ -60,7 +60,7 @@ public function testHandleWhenUsernameLength($username, $ok) $this->authenticator->authenticate($request); } - public function provideUsernamesForLength() + public static function provideUsernamesForLength() { yield [str_repeat('x', Security::MAX_USERNAME_LENGTH + 1), false]; yield [str_repeat('x', Security::MAX_USERNAME_LENGTH - 1), true]; @@ -126,7 +126,7 @@ public function testHandleNonStringUsernameWithToString($postOnly) $this->authenticator->authenticate($request); } - public function postOnlyDataProvider() + public static function postOnlyDataProvider() { yield [true]; yield [false]; @@ -171,7 +171,7 @@ public function testSupportsFormOnly(string $contentType, bool $shouldSupport) $this->assertSame($shouldSupport, $this->authenticator->supports($request)); } - public function provideContentTypes() + public static function provideContentTypes() { yield ['application/json', false]; yield ['application/x-www-form-urlencoded', true]; diff --git a/Tests/Authenticator/HttpBasicAuthenticatorTest.php b/Tests/Authenticator/HttpBasicAuthenticatorTest.php index 70f48b9b..b7b0cc01 100644 --- a/Tests/Authenticator/HttpBasicAuthenticatorTest.php +++ b/Tests/Authenticator/HttpBasicAuthenticatorTest.php @@ -67,7 +67,7 @@ public function testHttpBasicServerParametersMissing(array $serverParameters) $this->assertFalse($this->authenticator->supports($request)); } - public function provideMissingHttpBasicServerParameters() + public static function provideMissingHttpBasicServerParameters() { return [ [[]], diff --git a/Tests/Authenticator/JsonLoginAuthenticatorTest.php b/Tests/Authenticator/JsonLoginAuthenticatorTest.php index 47e02689..ae37976d 100644 --- a/Tests/Authenticator/JsonLoginAuthenticatorTest.php +++ b/Tests/Authenticator/JsonLoginAuthenticatorTest.php @@ -45,7 +45,7 @@ public function testSupport($request) $this->assertTrue($this->authenticator->supports($request)); } - public function provideSupportData() + public static function provideSupportData() { yield [new Request([], [], [], [], [], ['HTTP_CONTENT_TYPE' => 'application/json'], '{"username": "dunglas", "password": "foo"}')]; @@ -64,7 +64,7 @@ public function testSupportsWithCheckPath($request, $result) $this->assertSame($result, $this->authenticator->supports($request)); } - public function provideSupportsWithCheckPathData() + public static function provideSupportsWithCheckPathData() { yield [Request::create('/api/login', 'GET', [], [], [], ['HTTP_CONTENT_TYPE' => 'application/json']), true]; yield [Request::create('/login', 'GET', [], [], [], ['HTTP_CONTENT_TYPE' => 'application/json']), false]; @@ -104,7 +104,7 @@ public function testAuthenticateInvalid($request, $errorMessage, $exceptionType $this->authenticator->authenticate($request); } - public function provideInvalidAuthenticateData() + public static function provideInvalidAuthenticateData() { $request = new Request([], [], [], [], [], ['HTTP_CONTENT_TYPE' => 'application/json']); yield [$request, 'Invalid JSON.']; diff --git a/Tests/Authenticator/LoginLinkAuthenticatorTest.php b/Tests/Authenticator/LoginLinkAuthenticatorTest.php index 7e533398..fb704d98 100644 --- a/Tests/Authenticator/LoginLinkAuthenticatorTest.php +++ b/Tests/Authenticator/LoginLinkAuthenticatorTest.php @@ -50,7 +50,7 @@ public function testSupport(array $options, $request, bool $supported) $this->assertEquals($supported, $this->authenticator->supports($request)); } - public function provideSupportData() + public static function provideSupportData() { yield [['check_route' => '/validate_link'], Request::create('/validate_link?hash=abc123'), true]; yield [['check_route' => '/validate_link'], Request::create('/login?hash=abc123'), false]; diff --git a/Tests/Authenticator/RememberMeAuthenticatorTest.php b/Tests/Authenticator/RememberMeAuthenticatorTest.php index c7492a95..302def67 100644 --- a/Tests/Authenticator/RememberMeAuthenticatorTest.php +++ b/Tests/Authenticator/RememberMeAuthenticatorTest.php @@ -51,7 +51,7 @@ public function testSupports($request, $support) $this->assertSame($support, $this->authenticator->supports($request)); } - public function provideSupportsData() + public static function provideSupportsData() { yield [Request::create('/'), false]; diff --git a/Tests/Authenticator/RemoteUserAuthenticatorTest.php b/Tests/Authenticator/RemoteUserAuthenticatorTest.php index 89a5776d..5119f8ce 100644 --- a/Tests/Authenticator/RemoteUserAuthenticatorTest.php +++ b/Tests/Authenticator/RemoteUserAuthenticatorTest.php @@ -64,7 +64,7 @@ public function testAuthenticate(InMemoryUserProvider $userProvider, RemoteUserA $this->assertTrue($user->isEqualTo($passport->getUser())); } - public function provideAuthenticators() + public static function provideAuthenticators() { $userProvider = new InMemoryUserProvider(); yield [$userProvider, new RemoteUserAuthenticator($userProvider, new TokenStorage(), 'main'), 'REMOTE_USER']; diff --git a/Tests/EntryPoint/RetryAuthenticationEntryPointTest.php b/Tests/EntryPoint/RetryAuthenticationEntryPointTest.php index e9e5ddd5..9b9492d9 100644 --- a/Tests/EntryPoint/RetryAuthenticationEntryPointTest.php +++ b/Tests/EntryPoint/RetryAuthenticationEntryPointTest.php @@ -33,7 +33,7 @@ public function testStart($httpPort, $httpsPort, $request, $expectedUrl) $this->assertEquals($expectedUrl, $response->headers->get('Location')); } - public function dataForStart() + public static function dataForStart() { if (!class_exists(Request::class)) { return [[]]; diff --git a/Tests/EventListener/CheckCredentialsListenerTest.php b/Tests/EventListener/CheckCredentialsListenerTest.php index bab2ad0f..7135edbc 100644 --- a/Tests/EventListener/CheckCredentialsListenerTest.php +++ b/Tests/EventListener/CheckCredentialsListenerTest.php @@ -62,7 +62,7 @@ public function testPasswordAuthenticated($password, $passwordValid, $result) } } - public function providePasswords() + public static function providePasswords() { yield ['ThePa$$word', true, true]; yield ['Invalid', false, false]; @@ -100,7 +100,7 @@ public function testCustomAuthenticated($result) } } - public function provideCustomAuthenticatedResults() + public static function provideCustomAuthenticatedResults() { yield [true]; yield [false]; diff --git a/Tests/EventListener/CheckRememberMeConditionsListenerTest.php b/Tests/EventListener/CheckRememberMeConditionsListenerTest.php index adc4a512..a0e4904c 100644 --- a/Tests/EventListener/CheckRememberMeConditionsListenerTest.php +++ b/Tests/EventListener/CheckRememberMeConditionsListenerTest.php @@ -80,7 +80,7 @@ public function testSuccessfulLoginWithOptInRequestParameter($optInValue) $this->assertTrue($passport->getBadge(RememberMeBadge::class)->isEnabled()); } - public function provideRememberMeOptInValues() + public static function provideRememberMeOptInValues() { yield ['true']; yield ['1']; diff --git a/Tests/EventListener/UserProviderListenerTest.php b/Tests/EventListener/UserProviderListenerTest.php index 1f5a65ac..d81fc80c 100644 --- a/Tests/EventListener/UserProviderListenerTest.php +++ b/Tests/EventListener/UserProviderListenerTest.php @@ -59,7 +59,7 @@ public function testNotOverrideUserLoader($passport) $this->assertEquals($passport->hasBadge(UserBadge::class) ? $passport->getBadge(UserBadge::class) : null, $badgeBefore); } - public function provideCompletePassports() + public static function provideCompletePassports() { yield [new SelfValidatingPassport(new UserBadge('wouter', function () {}))]; } diff --git a/Tests/Firewall/ContextListenerTest.php b/Tests/Firewall/ContextListenerTest.php index 6c02847b..b98dc776 100644 --- a/Tests/Firewall/ContextListenerTest.php +++ b/Tests/Firewall/ContextListenerTest.php @@ -169,7 +169,7 @@ public function testInvalidTokenInSession($token) $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); } - public function provideInvalidToken() + public static function provideInvalidToken() { return [ ['foo'], diff --git a/Tests/Firewall/ExceptionListenerTest.php b/Tests/Firewall/ExceptionListenerTest.php index 487b0f7c..f05ce5f4 100644 --- a/Tests/Firewall/ExceptionListenerTest.php +++ b/Tests/Firewall/ExceptionListenerTest.php @@ -64,7 +64,7 @@ public function testAuthenticationExceptionWithEntryPoint(\Exception $exception) $this->assertSame($exception, $event->getThrowable()); } - public function getAuthenticationExceptionProvider() + public static function getAuthenticationExceptionProvider() { return [ [$e = new AuthenticationException(), new HttpException(Response::HTTP_UNAUTHORIZED, '', $e, [], 0)], @@ -183,7 +183,7 @@ public function testUnregister() $this->assertEmpty($dispatcher->getListeners()); } - public function getAccessDeniedExceptionProvider() + public static function getAccessDeniedExceptionProvider() { return [ [new AccessDeniedException()], diff --git a/Tests/Firewall/LogoutListenerTest.php b/Tests/Firewall/LogoutListenerTest.php index ec9165ff..57c23b5c 100644 --- a/Tests/Firewall/LogoutListenerTest.php +++ b/Tests/Firewall/LogoutListenerTest.php @@ -204,7 +204,7 @@ public function testLegacyLogoutHandlers() $this->assertSame($response, $event->getResponse()); } - public function provideInvalidCsrfTokens(): array + public static function provideInvalidCsrfTokens(): array { return [ ['invalid'], diff --git a/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php b/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php index 73d03e1a..063afc41 100644 --- a/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php +++ b/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php @@ -267,7 +267,7 @@ public function testInvalidCsrfToken($invalidToken) $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); } - public function postOnlyDataProvider(): array + public static function postOnlyDataProvider(): array { return [ [true], @@ -275,7 +275,7 @@ public function postOnlyDataProvider(): array ]; } - public function getUsernameForLength(): array + public static function getUsernameForLength(): array { return [ [str_repeat('x', Security::MAX_USERNAME_LENGTH + 1), false], @@ -283,7 +283,7 @@ public function getUsernameForLength(): array ]; } - public function provideInvalidCsrfTokens(): array + public static function provideInvalidCsrfTokens(): array { return [ ['invalid'], diff --git a/Tests/HttpUtilsTest.php b/Tests/HttpUtilsTest.php index 4d07f0a1..4a1ba23a 100644 --- a/Tests/HttpUtilsTest.php +++ b/Tests/HttpUtilsTest.php @@ -69,7 +69,7 @@ public function testCreateRedirectResponseWithBadRequestsDomain($url) $this->assertTrue($response->isRedirect('http://localhost/')); } - public function badRequestDomainUrls() + public static function badRequestDomainUrls() { return [ ['http://pirate.net/foo'], @@ -175,7 +175,7 @@ public function testCreateRequestPassesSecurityContextAttributesToTheNewRequest( $this->assertSame('foo', $subRequest->attributes->get($attribute)); } - public function provideSecurityContextAttributes() + public static function provideSecurityContextAttributes() { return [ [Security::AUTHENTICATION_ERROR], diff --git a/Tests/LoginLink/LoginLinkHandlerTest.php b/Tests/LoginLink/LoginLinkHandlerTest.php index f2d03eed..0d698362 100644 --- a/Tests/LoginLink/LoginLinkHandlerTest.php +++ b/Tests/LoginLink/LoginLinkHandlerTest.php @@ -89,7 +89,7 @@ public function testCreateLoginLink($user, array $extraProperties, Request $requ $this->assertSame('https://example.com/login/verify?user=weaverryan&hash=abchash&expires=1601235000', $loginLink->getUrl()); } - public function provideCreateLoginLinkData() + public static function provideCreateLoginLinkData() { yield [ new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash'), diff --git a/Tests/RememberMe/AbstractRememberMeServicesTest.php b/Tests/RememberMe/AbstractRememberMeServicesTest.php index 1cfec9bd..37e4d753 100644 --- a/Tests/RememberMe/AbstractRememberMeServicesTest.php +++ b/Tests/RememberMe/AbstractRememberMeServicesTest.php @@ -119,7 +119,7 @@ public function testLogout(array $options) $this->assertSame($options['httponly'], $cookie->isHttpOnly()); } - public function provideOptionsForLogout() + public static function provideOptionsForLogout() { return [ [['name' => 'foo', 'path' => '/', 'domain' => null, 'secure' => false, 'httponly' => true]], @@ -259,7 +259,7 @@ public function testLoginSuccessWhenRememberMeParameterIsPositive($value) $service->loginSuccess($request, $response, $token); } - public function getPositiveRememberMeParameterValues() + public static function getPositiveRememberMeParameterValues() { return [ ['true'], diff --git a/Tests/RememberMe/TokenBasedRememberMeServicesTest.php b/Tests/RememberMe/TokenBasedRememberMeServicesTest.php index ff774506..792afce1 100644 --- a/Tests/RememberMe/TokenBasedRememberMeServicesTest.php +++ b/Tests/RememberMe/TokenBasedRememberMeServicesTest.php @@ -107,7 +107,7 @@ public function testAutoLogin($username) $this->assertEquals('foosecret', $returnedToken->getSecret()); } - public function provideUsernamesForAutoLogin() + public static function provideUsernamesForAutoLogin() { return [ ['foouser', 'Simple username'], From fa788fff12a4d6b8741f591e8021dc8bf9f84b20 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 14 Feb 2023 09:53:37 +0100 Subject: [PATCH 10/20] Fix merge --- .../AccessToken/ChainedAccessTokenExtractorsTest.php | 4 ++-- .../FormEncodedBodyAccessTokenAuthenticatorTest.php | 2 +- .../AccessToken/HeaderAccessTokenAuthenticatorTest.php | 8 ++++---- .../AccessToken/QueryAccessTokenAuthenticatorTest.php | 2 +- Tests/Authenticator/JsonLoginAuthenticatorTest.php | 2 +- Tests/EventListener/IsGrantedAttributeListenerTest.php | 2 +- Tests/Firewall/LogoutListenerTest.php | 2 +- Tests/HttpUtilsTest.php | 4 ++-- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Tests/Authenticator/AccessToken/ChainedAccessTokenExtractorsTest.php b/Tests/Authenticator/AccessToken/ChainedAccessTokenExtractorsTest.php index 8dd3188e..1507e425 100644 --- a/Tests/Authenticator/AccessToken/ChainedAccessTokenExtractorsTest.php +++ b/Tests/Authenticator/AccessToken/ChainedAccessTokenExtractorsTest.php @@ -48,7 +48,7 @@ public function testSupport($request) $this->assertNull($this->authenticator->supports($request)); } - public function provideSupportData(): iterable + public static function provideSupportData(): iterable { yield [new Request([], [], [], [], [], ['HTTP_AUTHORIZATION' => 'Bearer VALID_ACCESS_TOKEN'])]; yield [new Request([], [], [], [], [], ['HTTP_AUTHORIZATION' => 'Bearer INVALID_ACCESS_TOKEN'])]; @@ -77,7 +77,7 @@ public function testAuthenticateInvalid($request, $errorMessage, $exceptionType $this->authenticator->authenticate($request); } - public function provideInvalidAuthenticateData(): iterable + public static function provideInvalidAuthenticateData(): iterable { $request = new Request(); yield [$request, 'Invalid credentials.', BadCredentialsException::class]; diff --git a/Tests/Authenticator/AccessToken/FormEncodedBodyAccessTokenAuthenticatorTest.php b/Tests/Authenticator/AccessToken/FormEncodedBodyAccessTokenAuthenticatorTest.php index b915a5d1..3299f017 100644 --- a/Tests/Authenticator/AccessToken/FormEncodedBodyAccessTokenAuthenticatorTest.php +++ b/Tests/Authenticator/AccessToken/FormEncodedBodyAccessTokenAuthenticatorTest.php @@ -92,7 +92,7 @@ public function testAuthenticateInvalid($request, $errorMessage, $exceptionType $this->authenticator->authenticate($request); } - public function provideInvalidAuthenticateData(): iterable + public static function provideInvalidAuthenticateData(): iterable { $request = new Request(); $request->setMethod(Request::METHOD_GET); diff --git a/Tests/Authenticator/AccessToken/HeaderAccessTokenAuthenticatorTest.php b/Tests/Authenticator/AccessToken/HeaderAccessTokenAuthenticatorTest.php index a4e7758b..de85e66f 100644 --- a/Tests/Authenticator/AccessToken/HeaderAccessTokenAuthenticatorTest.php +++ b/Tests/Authenticator/AccessToken/HeaderAccessTokenAuthenticatorTest.php @@ -45,7 +45,7 @@ public function testSupport($request) $this->assertNull($this->authenticator->supports($request)); } - public function provideSupportData(): iterable + public static function provideSupportData(): iterable { yield [new Request([], [], [], [], [], ['HTTP_AUTHORIZATION' => 'Bearer VALID_ACCESS_TOKEN'])]; yield [new Request([], [], [], [], [], ['HTTP_AUTHORIZATION' => 'Bearer INVALID_ACCESS_TOKEN'])]; @@ -61,7 +61,7 @@ public function testSupportsWithCustomTokenType($request, $result) $this->assertSame($result, $this->authenticator->supports($request)); } - public function provideSupportsWithCustomTokenTypeData(): iterable + public static function provideSupportsWithCustomTokenTypeData(): iterable { yield [new Request([], [], [], [], [], ['HTTP_AUTHORIZATION' => 'JWT VALID_ACCESS_TOKEN']), null]; yield [new Request([], [], [], [], [], ['HTTP_AUTHORIZATION' => 'JWT INVALID_ACCESS_TOKEN']), null]; @@ -79,7 +79,7 @@ public function testSupportsWithCustomHeaderParameter($request, $result) $this->assertSame($result, $this->authenticator->supports($request)); } - public function provideSupportsWithCustomHeaderParameter(): iterable + public static function provideSupportsWithCustomHeaderParameter(): iterable { yield [new Request([], [], [], [], [], ['HTTP_X_FOO' => 'Bearer VALID_ACCESS_TOKEN']), null]; yield [new Request([], [], [], [], [], ['HTTP_X_FOO' => 'Bearer INVALID_ACCESS_TOKEN']), null]; @@ -120,7 +120,7 @@ public function testAuthenticateInvalid($request, $errorMessage, $exceptionType $this->authenticator->authenticate($request); } - public function provideInvalidAuthenticateData(): iterable + public static function provideInvalidAuthenticateData(): iterable { $request = new Request(); yield [$request, 'Invalid credentials.', BadCredentialsException::class]; diff --git a/Tests/Authenticator/AccessToken/QueryAccessTokenAuthenticatorTest.php b/Tests/Authenticator/AccessToken/QueryAccessTokenAuthenticatorTest.php index 73f8392a..428b1fd0 100644 --- a/Tests/Authenticator/AccessToken/QueryAccessTokenAuthenticatorTest.php +++ b/Tests/Authenticator/AccessToken/QueryAccessTokenAuthenticatorTest.php @@ -88,7 +88,7 @@ public function testAuthenticateInvalid($request, $errorMessage, $exceptionType $this->authenticator->authenticate($request); } - public function provideInvalidAuthenticateData(): iterable + public static function provideInvalidAuthenticateData(): iterable { $request = new Request(); yield [$request, 'Invalid credentials.', BadCredentialsException::class]; diff --git a/Tests/Authenticator/JsonLoginAuthenticatorTest.php b/Tests/Authenticator/JsonLoginAuthenticatorTest.php index 89e73e0e..5350dd4a 100644 --- a/Tests/Authenticator/JsonLoginAuthenticatorTest.php +++ b/Tests/Authenticator/JsonLoginAuthenticatorTest.php @@ -142,7 +142,7 @@ public function testAuthenticationForEmptyCredentialDeprecation($request) $this->authenticator->authenticate($request); } - public function provideEmptyAuthenticateData() + public static function provideEmptyAuthenticateData() { $request = new Request([], [], [], [], [], ['HTTP_CONTENT_TYPE' => 'application/json'], '{"username": "", "password": "notempty"}'); yield [$request]; diff --git a/Tests/EventListener/IsGrantedAttributeListenerTest.php b/Tests/EventListener/IsGrantedAttributeListenerTest.php index 690660e7..ae00389d 100644 --- a/Tests/EventListener/IsGrantedAttributeListenerTest.php +++ b/Tests/EventListener/IsGrantedAttributeListenerTest.php @@ -249,7 +249,7 @@ public function testAccessDeniedMessages(string|Expression $attribute, string|ar } } - public function getAccessDeniedMessageTests() + public static function getAccessDeniedMessageTests() { yield ['ROLE_ADMIN', null, 'admin', 0, 'Access Denied by #[IsGranted("ROLE_ADMIN")] on controller']; yield ['ROLE_ADMIN', 'bar', 'withSubject', 2, 'Access Denied by #[IsGranted("ROLE_ADMIN", "arg2Name")] on controller']; diff --git a/Tests/Firewall/LogoutListenerTest.php b/Tests/Firewall/LogoutListenerTest.php index 581ce206..06139bcc 100644 --- a/Tests/Firewall/LogoutListenerTest.php +++ b/Tests/Firewall/LogoutListenerTest.php @@ -163,7 +163,7 @@ public function testCsrfValidationFails($invalidToken) $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); } - public function provideInvalidCsrfTokens(): array + public static function provideInvalidCsrfTokens(): array { return [ ['invalid'], diff --git a/Tests/HttpUtilsTest.php b/Tests/HttpUtilsTest.php index f47e27ba..e0131057 100644 --- a/Tests/HttpUtilsTest.php +++ b/Tests/HttpUtilsTest.php @@ -69,7 +69,7 @@ public function testCreateRedirectResponseWithBadRequestsDomain($url) $this->assertTrue($response->isRedirect('http://localhost/')); } - public function badRequestDomainUrls() + public static function badRequestDomainUrls() { return [ ['http://pirate.net/foo'], @@ -175,7 +175,7 @@ public function testCreateRequestPassesSecurityRequestAttributesToTheNewRequest( $this->assertSame('foo', $subRequest->attributes->get($attribute)); } - public function provideSecurityRequestAttributes() + public static function provideSecurityRequestAttributes() { return [ [SecurityRequestAttributes::AUTHENTICATION_ERROR], From 8071d6b92a6d885527cb66a2a97a5b67e8aeea7d Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 14 Feb 2023 15:10:14 +0100 Subject: [PATCH 11/20] Fix tests --- Authentication/AuthenticatorManager.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Authentication/AuthenticatorManager.php b/Authentication/AuthenticatorManager.php index 82c8cd2e..d41fb9c1 100644 --- a/Authentication/AuthenticatorManager.php +++ b/Authentication/AuthenticatorManager.php @@ -22,7 +22,7 @@ use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\Exception\CustomUserMessageAccountStatusException; -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\Http\Authenticator\AuthenticatorInterface; use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticator; @@ -268,7 +268,7 @@ private function handleAuthenticationFailure(AuthenticationException $authentica // Avoid leaking error details in case of invalid user (e.g. user not found or invalid account status) // to prevent user enumeration via response content comparison - if ($this->hideUserNotFoundExceptions && ($authenticationException instanceof UsernameNotFoundException || ($authenticationException instanceof AccountStatusException && !$authenticationException instanceof CustomUserMessageAccountStatusException))) { + if ($this->hideUserNotFoundExceptions && ($authenticationException instanceof UserNotFoundException || ($authenticationException instanceof AccountStatusException && !$authenticationException instanceof CustomUserMessageAccountStatusException))) { $authenticationException = new BadCredentialsException('Bad credentials.', 0, $authenticationException); } From 8a39d9746d7e23bd7778668d1443f8de2466f825 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 14 Feb 2023 15:21:46 +0100 Subject: [PATCH 12/20] [Security] fix compat with security-core v6 --- Authenticator/RememberMeAuthenticator.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Authenticator/RememberMeAuthenticator.php b/Authenticator/RememberMeAuthenticator.php index cca50c8b..a7d7f01a 100644 --- a/Authenticator/RememberMeAuthenticator.php +++ b/Authenticator/RememberMeAuthenticator.php @@ -20,7 +20,7 @@ use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Exception\CookieTheftException; 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\Http\Authenticator\Passport\Badge\UserBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Passport; use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface; @@ -119,7 +119,7 @@ public function onAuthenticationSuccess(Request $request, TokenInterface $token, public function onAuthenticationFailure(Request $request, AuthenticationException $exception): ?Response { if (null !== $this->logger) { - if ($exception instanceof UsernameNotFoundException) { + if ($exception instanceof UserNotFoundException) { $this->logger->info('User for remember-me cookie not found.', ['exception' => $exception]); } elseif ($exception instanceof UnsupportedUserException) { $this->logger->warning('User class for remember-me cookie not supported.', ['exception' => $exception]); From 77e01ce388e256bbf116aba2dbb42d3b489b919d Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 16 Feb 2023 10:33:00 +0100 Subject: [PATCH 13/20] CS fix --- Authenticator/AuthenticatorInterface.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Authenticator/AuthenticatorInterface.php b/Authenticator/AuthenticatorInterface.php index 4165d25a..b66874fa 100644 --- a/Authenticator/AuthenticatorInterface.php +++ b/Authenticator/AuthenticatorInterface.php @@ -51,9 +51,9 @@ public function supports(Request $request): ?bool; * You may throw any AuthenticationException in this method in case of error (e.g. * a UserNotFoundException when the user cannot be found). * - * @throws AuthenticationException - * * @return Passport + * + * @throws AuthenticationException */ public function authenticate(Request $request); /* : Passport; */ From 0570380d0864d3fa5f8c07b59ada16149bf0570a Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 24 Feb 2023 18:26:37 +0100 Subject: [PATCH 14/20] [Security] Migrate the session on login only when the user changes --- Authentication/AuthenticatorManager.php | 9 +- Event/LoginSuccessEvent.php | 9 +- EventListener/SessionStrategyListener.php | 10 ++ Firewall/AbstractAuthenticationListener.php | 18 +++- Firewall/AbstractPreAuthenticatedListener.php | 14 ++- Firewall/BasicAuthenticationListener.php | 14 ++- ...namePasswordJsonAuthenticationListener.php | 18 +++- .../PasswordMigratingListenerTest.php | 4 +- .../SessionStrategyListenerTest.php | 23 +++- Tests/Fixtures/DummySupportsAuthenticator.php | 6 -- Tests/Fixtures/DummyToken.php | 101 ------------------ 11 files changed, 101 insertions(+), 125 deletions(-) delete mode 100644 Tests/Fixtures/DummyToken.php diff --git a/Authentication/AuthenticatorManager.php b/Authentication/AuthenticatorManager.php index d41fb9c1..f78ce0b4 100644 --- a/Authentication/AuthenticatorManager.php +++ b/Authentication/AuthenticatorManager.php @@ -84,7 +84,7 @@ public function authenticateUser(UserInterface $user, AuthenticatorInterface $au $token = $this->eventDispatcher->dispatch(new AuthenticationTokenCreatedEvent($token, $passport))->getAuthenticatedToken(); // authenticate this in the system - return $this->handleAuthenticationSuccess($token, $passport, $request, $authenticator); + return $this->handleAuthenticationSuccess($token, $passport, $request, $authenticator, $this->tokenStorage->getToken()); } public function supports(Request $request): ?bool @@ -174,6 +174,7 @@ private function executeAuthenticators(array $authenticators, Request $request): private function executeAuthenticator(AuthenticatorInterface $authenticator, Request $request): ?Response { $passport = null; + $previousToken = $this->tokenStorage->getToken(); try { // get the passport from the Authenticator @@ -224,7 +225,7 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req } // success! (sets the token on the token storage, etc) - $response = $this->handleAuthenticationSuccess($authenticatedToken, $passport, $request, $authenticator); + $response = $this->handleAuthenticationSuccess($authenticatedToken, $passport, $request, $authenticator, $previousToken); if ($response instanceof Response) { return $response; } @@ -236,7 +237,7 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req return null; } - private function handleAuthenticationSuccess(TokenInterface $authenticatedToken, PassportInterface $passport, Request $request, AuthenticatorInterface $authenticator): ?Response + private function handleAuthenticationSuccess(TokenInterface $authenticatedToken, PassportInterface $passport, Request $request, AuthenticatorInterface $authenticator, ?TokenInterface $previousToken): ?Response { // @deprecated since Symfony 5.3 $user = $authenticatedToken->getUser(); @@ -252,7 +253,7 @@ private function handleAuthenticationSuccess(TokenInterface $authenticatedToken, $this->eventDispatcher->dispatch($loginEvent, SecurityEvents::INTERACTIVE_LOGIN); } - $this->eventDispatcher->dispatch($loginSuccessEvent = new LoginSuccessEvent($authenticator, $passport, $authenticatedToken, $request, $response, $this->firewallName)); + $this->eventDispatcher->dispatch($loginSuccessEvent = new LoginSuccessEvent($authenticator, $passport, $authenticatedToken, $request, $response, $this->firewallName, $previousToken)); return $loginSuccessEvent->getResponse(); } diff --git a/Event/LoginSuccessEvent.php b/Event/LoginSuccessEvent.php index d2272fe2..27a8621a 100644 --- a/Event/LoginSuccessEvent.php +++ b/Event/LoginSuccessEvent.php @@ -37,6 +37,7 @@ class LoginSuccessEvent extends Event private $authenticator; private $passport; private $authenticatedToken; + private $previousToken; private $request; private $response; private $firewallName; @@ -44,7 +45,7 @@ class LoginSuccessEvent extends Event /** * @param Passport $passport */ - public function __construct(AuthenticatorInterface $authenticator, PassportInterface $passport, TokenInterface $authenticatedToken, Request $request, ?Response $response, string $firewallName) + public function __construct(AuthenticatorInterface $authenticator, PassportInterface $passport, TokenInterface $authenticatedToken, Request $request, ?Response $response, string $firewallName, TokenInterface $previousToken = null) { if (!$passport instanceof Passport) { trigger_deprecation('symfony/security-http', '5.4', 'Not passing an instance of "%s" as "$passport" argument of "%s()" is deprecated, "%s" given.', Passport::class, __METHOD__, get_debug_type($passport)); @@ -53,6 +54,7 @@ public function __construct(AuthenticatorInterface $authenticator, PassportInter $this->authenticator = $authenticator; $this->passport = $passport; $this->authenticatedToken = $authenticatedToken; + $this->previousToken = $previousToken; $this->request = $request; $this->response = $response; $this->firewallName = $firewallName; @@ -83,6 +85,11 @@ public function getAuthenticatedToken(): TokenInterface return $this->authenticatedToken; } + public function getPreviousToken(): ?TokenInterface + { + return $this->previousToken; + } + public function getRequest(): Request { return $this->request; diff --git a/EventListener/SessionStrategyListener.php b/EventListener/SessionStrategyListener.php index b1ba2889..311a52ff 100644 --- a/EventListener/SessionStrategyListener.php +++ b/EventListener/SessionStrategyListener.php @@ -43,6 +43,16 @@ public function onSuccessfulLogin(LoginSuccessEvent $event): void return; } + if ($previousToken = $event->getPreviousToken()) { + // @deprecated since Symfony 5.3, change to $token->getUserIdentifier() in 6.0 + $user = method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername(); + $previousUser = method_exists($previousToken, 'getUserIdentifier') ? $previousToken->getUserIdentifier() : $previousToken->getUsername(); + + if ('' !== ($user ?? '') && $user === $previousUser) { + return; + } + } + $this->sessionAuthenticationStrategy->onAuthentication($request, $token); } diff --git a/Firewall/AbstractAuthenticationListener.php b/Firewall/AbstractAuthenticationListener.php index 33e2c08b..6ff49cb0 100644 --- a/Firewall/AbstractAuthenticationListener.php +++ b/Firewall/AbstractAuthenticationListener.php @@ -133,12 +133,14 @@ public function authenticate(RequestEvent $event) throw new SessionUnavailableException('Your session has timed out, or you have disabled cookies.'); } + $previousToken = $this->tokenStorage->getToken(); + if (null === $returnValue = $this->attemptAuthentication($request)) { return; } if ($returnValue instanceof TokenInterface) { - $this->sessionStrategy->onAuthentication($request, $returnValue); + $this->migrateSession($request, $returnValue, $previousToken); $response = $this->onSuccess($request, $returnValue); } elseif ($returnValue instanceof Response) { @@ -226,4 +228,18 @@ private function onSuccess(Request $request, TokenInterface $token): Response return $response; } + + private function migrateSession(Request $request, TokenInterface $token, ?TokenInterface $previousToken) + { + if ($previousToken) { + $user = method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername(); + $previousUser = method_exists($previousToken, 'getUserIdentifier') ? $previousToken->getUserIdentifier() : $previousToken->getUsername(); + + if ('' !== ($user ?? '') && $user === $previousUser) { + return; + } + } + + $this->sessionStrategy->onAuthentication($request, $token); + } } diff --git a/Firewall/AbstractPreAuthenticatedListener.php b/Firewall/AbstractPreAuthenticatedListener.php index b698e1e4..bc59e4e3 100644 --- a/Firewall/AbstractPreAuthenticatedListener.php +++ b/Firewall/AbstractPreAuthenticatedListener.php @@ -98,13 +98,14 @@ public function authenticate(RequestEvent $event) } try { + $previousToken = $token; $token = $this->authenticationManager->authenticate(new PreAuthenticatedToken($user, $credentials, $this->providerKey)); if (null !== $this->logger) { $this->logger->info('Pre-authentication successful.', ['token' => (string) $token]); } - $this->migrateSession($request, $token); + $this->migrateSession($request, $token, $previousToken); $this->tokenStorage->setToken($token); @@ -149,12 +150,21 @@ private function clearToken(AuthenticationException $exception) */ abstract protected function getPreAuthenticatedData(Request $request); - private function migrateSession(Request $request, TokenInterface $token) + private function migrateSession(Request $request, TokenInterface $token, ?TokenInterface $previousToken) { if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) { return; } + if ($previousToken) { + $user = method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername(); + $previousUser = method_exists($previousToken, 'getUserIdentifier') ? $previousToken->getUserIdentifier() : $previousToken->getUsername(); + + if ('' !== ($user ?? '') && $user === $previousUser) { + return; + } + } + $this->sessionStrategy->onAuthentication($request, $token); } } diff --git a/Firewall/BasicAuthenticationListener.php b/Firewall/BasicAuthenticationListener.php index 8113e9f1..cb02f012 100644 --- a/Firewall/BasicAuthenticationListener.php +++ b/Firewall/BasicAuthenticationListener.php @@ -88,9 +88,10 @@ public function authenticate(RequestEvent $event) } try { + $previousToken = $token; $token = $this->authenticationManager->authenticate(new UsernamePasswordToken($username, $request->headers->get('PHP_AUTH_PW'), $this->providerKey)); - $this->migrateSession($request, $token); + $this->migrateSession($request, $token, $previousToken); $this->tokenStorage->setToken($token); } catch (AuthenticationException $e) { @@ -121,12 +122,21 @@ public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyIn $this->sessionStrategy = $sessionStrategy; } - private function migrateSession(Request $request, TokenInterface $token) + private function migrateSession(Request $request, TokenInterface $token, ?TokenInterface $previousToken) { if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) { return; } + if ($previousToken) { + $user = method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername(); + $previousUser = method_exists($previousToken, 'getUserIdentifier') ? $previousToken->getUserIdentifier() : $previousToken->getUsername(); + + if ('' !== ($user ?? '') && $user === $previousUser) { + return; + } + } + $this->sessionStrategy->onAuthentication($request, $token); } } diff --git a/Firewall/UsernamePasswordJsonAuthenticationListener.php b/Firewall/UsernamePasswordJsonAuthenticationListener.php index 9679b33f..13025ce6 100644 --- a/Firewall/UsernamePasswordJsonAuthenticationListener.php +++ b/Firewall/UsernamePasswordJsonAuthenticationListener.php @@ -101,6 +101,7 @@ public function authenticate(RequestEvent $event) { $request = $event->getRequest(); $data = json_decode($request->getContent()); + $previousToken = $this->tokenStorage->getToken(); try { if (!$data instanceof \stdClass) { @@ -134,7 +135,7 @@ public function authenticate(RequestEvent $event) $token = new UsernamePasswordToken($username, $password, $this->providerKey); $authenticatedToken = $this->authenticationManager->authenticate($token); - $response = $this->onSuccess($request, $authenticatedToken); + $response = $this->onSuccess($request, $authenticatedToken, $previousToken); } catch (AuthenticationException $e) { $response = $this->onFailure($request, $e); } catch (BadRequestHttpException $e) { @@ -150,14 +151,14 @@ public function authenticate(RequestEvent $event) $event->setResponse($response); } - private function onSuccess(Request $request, TokenInterface $token): ?Response + private function onSuccess(Request $request, TokenInterface $token, ?TokenInterface $previousToken): ?Response { if (null !== $this->logger) { // @deprecated since Symfony 5.3, change to $token->getUserIdentifier() in 6.0 $this->logger->info('User has been authenticated successfully.', ['username' => method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername()]); } - $this->migrateSession($request, $token); + $this->migrateSession($request, $token, $previousToken); $this->tokenStorage->setToken($token); @@ -224,12 +225,21 @@ public function setTranslator(TranslatorInterface $translator) $this->translator = $translator; } - private function migrateSession(Request $request, TokenInterface $token) + private function migrateSession(Request $request, TokenInterface $token, ?TokenInterface $previousToken) { if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) { return; } + if ($previousToken) { + $user = method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername(); + $previousUser = method_exists($previousToken, 'getUserIdentifier') ? $previousToken->getUserIdentifier() : $previousToken->getUsername(); + + if ('' !== ($user ?? '') && $user === $previousUser) { + return; + } + } + $this->sessionStrategy->onAuthentication($request, $token); } } diff --git a/Tests/EventListener/PasswordMigratingListenerTest.php b/Tests/EventListener/PasswordMigratingListenerTest.php index 36fdfa5c..70725c20 100644 --- a/Tests/EventListener/PasswordMigratingListenerTest.php +++ b/Tests/EventListener/PasswordMigratingListenerTest.php @@ -15,6 +15,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface; use Symfony\Component\PasswordHasher\PasswordHasherInterface; +use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\PasswordUpgraderInterface; @@ -28,7 +29,6 @@ use Symfony\Component\Security\Http\Event\LoginSuccessEvent; use Symfony\Component\Security\Http\EventListener\PasswordMigratingListener; use Symfony\Component\Security\Http\Tests\Fixtures\DummyAuthenticator; -use Symfony\Component\Security\Http\Tests\Fixtures\DummyToken; class PasswordMigratingListenerTest extends TestCase { @@ -140,7 +140,7 @@ private static function createPasswordUpgrader() private static function createEvent(PassportInterface $passport) { - return new LoginSuccessEvent(new DummyAuthenticator(), $passport, new DummyToken(), new Request(), null, 'main'); + return new LoginSuccessEvent(new DummyAuthenticator(), $passport, new NullToken(), new Request(), null, 'main'); } } diff --git a/Tests/EventListener/SessionStrategyListenerTest.php b/Tests/EventListener/SessionStrategyListenerTest.php index a28b316f..51b8dc18 100644 --- a/Tests/EventListener/SessionStrategyListenerTest.php +++ b/Tests/EventListener/SessionStrategyListenerTest.php @@ -14,7 +14,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Session\SessionInterface; -use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; @@ -35,7 +35,7 @@ protected function setUp(): void $this->sessionAuthenticationStrategy = $this->createMock(SessionAuthenticationStrategyInterface::class); $this->listener = new SessionStrategyListener($this->sessionAuthenticationStrategy); $this->request = new Request(); - $this->token = $this->createMock(TokenInterface::class); + $this->token = $this->createMock(NullToken::class); } public function testRequestWithSession() @@ -62,6 +62,25 @@ public function testStatelessFirewalls() $listener->onSuccessfulLogin($this->createEvent('api_firewall')); } + public function testRequestWithSamePreviousUser() + { + $this->configurePreviousSession(); + $this->sessionAuthenticationStrategy->expects($this->never())->method('onAuthentication'); + + $token = $this->createMock(NullToken::class); + $token->expects($this->once()) + ->method('getUserIdentifier') + ->willReturn('test'); + $previousToken = $this->createMock(NullToken::class); + $previousToken->expects($this->once()) + ->method('getUserIdentifier') + ->willReturn('test'); + + $event = new LoginSuccessEvent($this->createMock(AuthenticatorInterface::class), new SelfValidatingPassport(new UserBadge('test', function () {})), $token, $this->request, null, 'main_firewall', $previousToken); + + $this->listener->onSuccessfulLogin($event); + } + private function createEvent($firewallName) { return new LoginSuccessEvent($this->createMock(AuthenticatorInterface::class), new SelfValidatingPassport(new UserBadge('test', function ($username) { return new InMemoryUser($username, null); })), $this->token, $this->request, null, $firewallName); diff --git a/Tests/Fixtures/DummySupportsAuthenticator.php b/Tests/Fixtures/DummySupportsAuthenticator.php index e2a037cc..8e7d394a 100644 --- a/Tests/Fixtures/DummySupportsAuthenticator.php +++ b/Tests/Fixtures/DummySupportsAuthenticator.php @@ -12,12 +12,6 @@ namespace Symfony\Component\Security\Http\Tests\Fixtures; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; -use Symfony\Component\Security\Core\Exception\AuthenticationException; -use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface; -use Symfony\Component\Security\Http\Authenticator\Passport\Passport; -use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface; class DummySupportsAuthenticator extends DummyAuthenticator { diff --git a/Tests/Fixtures/DummyToken.php b/Tests/Fixtures/DummyToken.php deleted file mode 100644 index 0e921dbf..00000000 --- a/Tests/Fixtures/DummyToken.php +++ /dev/null @@ -1,101 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Security\Http\Tests\Fixtures; - -use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; -use Symfony\Component\Security\Core\User\UserInterface; - -/** - * @author Alexandre Daubois - */ -class DummyToken implements TokenInterface -{ - public function serialize() - { - } - - public function unserialize($data) - { - } - - public function __toString(): string - { - } - - public function getRoleNames(): array - { - } - - public function getCredentials(): mixed - { - } - - public function getUser(): ?UserInterface - { - } - - public function setUser($user) - { - } - - public function isAuthenticated(): bool - { - } - - public function setAuthenticated(bool $isAuthenticated) - { - } - - public function eraseCredentials(): void - { - } - - public function getAttributes(): array - { - } - - public function setAttributes(array $attributes): void - { - } - - public function hasAttribute(string $name): bool - { - } - - public function getAttribute(string $name): mixed - { - } - - public function setAttribute(string $name, $value): void - { - } - - public function getUsername(): string - { - } - - public function getUserIdentifier(): string - { - } - - public function __serialize(): array - { - } - - public function __unserialize(array $data): void - { - } - - public function __call(string $name, array $arguments) - { - } -} From a167d712b22affcc97eb20cc4eebc44619e620b4 Mon Sep 17 00:00:00 2001 From: Florent Destremau Date: Mon, 27 Feb 2023 10:32:41 +0100 Subject: [PATCH 15/20] Removed @internal tag on TraceableAuthenticator::getAuthenticator() --- Authenticator/Debug/TraceableAuthenticator.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/Authenticator/Debug/TraceableAuthenticator.php b/Authenticator/Debug/TraceableAuthenticator.php index 4b77d9c4..40ee23a2 100644 --- a/Authenticator/Debug/TraceableAuthenticator.php +++ b/Authenticator/Debug/TraceableAuthenticator.php @@ -100,9 +100,6 @@ public function isInteractive(): bool return $this->authenticator instanceof InteractiveAuthenticatorInterface && $this->authenticator->isInteractive(); } - /** - * @internal - */ public function getAuthenticator(): AuthenticatorInterface { return $this->authenticator; From 6ab8f21209bc23d992f294be5341bdd646741776 Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Mon, 6 Mar 2023 21:48:01 +0100 Subject: [PATCH 16/20] [Tests] Replace `setMethods()` by `onlyMethods()` and `addMethods()` --- .../DefaultAuthenticationFailureHandlerTest.php | 2 +- Tests/EventListener/CheckCredentialsListenerTest.php | 4 ++-- Tests/Firewall/ContextListenerTest.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php b/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php index 46ac7243..31018aec 100644 --- a/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php +++ b/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php @@ -42,7 +42,7 @@ protected function setUp(): void $this->session = $this->createMock(SessionInterface::class); $this->request = $this->createMock(Request::class); $this->request->expects($this->any())->method('getSession')->willReturn($this->session); - $this->exception = $this->getMockBuilder(AuthenticationException::class)->setMethods(['getMessage'])->getMock(); + $this->exception = $this->getMockBuilder(AuthenticationException::class)->onlyMethods(['getMessage'])->getMock(); } public function testForward() diff --git a/Tests/EventListener/CheckCredentialsListenerTest.php b/Tests/EventListener/CheckCredentialsListenerTest.php index 7135edbc..ea5b3dd9 100644 --- a/Tests/EventListener/CheckCredentialsListenerTest.php +++ b/Tests/EventListener/CheckCredentialsListenerTest.php @@ -136,7 +136,7 @@ public function testAddsNoPasswordUpgradeBadgeIfItAlreadyExists() $this->hasherFactory->expects($this->any())->method('getPasswordHasher')->with($this->identicalTo($this->user))->willReturn($hasher); $passport = $this->getMockBuilder(Passport::class) - ->setMethods(['addBadge']) + ->onlyMethods(['addBadge']) ->setConstructorArgs([new UserBadge('wouter', function () { return $this->user; }), new PasswordCredentials('ThePa$$word'), [new PasswordUpgradeBadge('ThePa$$word')]]) ->getMock(); @@ -153,7 +153,7 @@ public function testAddsNoPasswordUpgradeBadgeIfPasswordIsInvalid() $this->hasherFactory->expects($this->any())->method('getPasswordHasher')->with($this->identicalTo($this->user))->willReturn($hasher); $passport = $this->getMockBuilder(Passport::class) - ->setMethods(['addBadge']) + ->onlyMethods(['addBadge']) ->setConstructorArgs([new UserBadge('wouter', function () { return $this->user; }), new PasswordCredentials('ThePa$$word'), [new PasswordUpgradeBadge('ThePa$$word')]]) ->getMock(); diff --git a/Tests/Firewall/ContextListenerTest.php b/Tests/Firewall/ContextListenerTest.php index b98dc776..58905eaf 100644 --- a/Tests/Firewall/ContextListenerTest.php +++ b/Tests/Firewall/ContextListenerTest.php @@ -366,7 +366,7 @@ public function testWithPreviousNotStartedSession() public function testSessionIsNotReported() { - $usageReporter = $this->getMockBuilder(\stdClass::class)->setMethods(['__invoke'])->getMock(); + $usageReporter = $this->getMockBuilder(\stdClass::class)->addMethods(['__invoke'])->getMock(); $usageReporter->expects($this->never())->method('__invoke'); $session = new Session(new MockArraySessionStorage(), null, null, $usageReporter); From 58de93d8d78dcd04821f4aa4255df37a4758ee15 Mon Sep 17 00:00:00 2001 From: AntoineDly Date: Wed, 8 Mar 2023 11:46:50 +0100 Subject: [PATCH 17/20] [Security] remove deprecated conditions in supports and authenticate methods from AccessListener class --- Firewall/AccessListener.php | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/Firewall/AccessListener.php b/Firewall/AccessListener.php index ccf3fde9..1194c45e 100644 --- a/Firewall/AccessListener.php +++ b/Firewall/AccessListener.php @@ -50,10 +50,7 @@ public function supports(Request $request): ?bool [$attributes] = $this->map->getPatterns($request); $request->attributes->set('_access_control_attributes', $attributes); - if ($attributes && ( - (\defined(AuthenticatedVoter::class.'::IS_AUTHENTICATED_ANONYMOUSLY') ? [AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY] !== $attributes : true) - && [AuthenticatedVoter::PUBLIC_ACCESS] !== $attributes - )) { + if ($attributes && [AuthenticatedVoter::PUBLIC_ACCESS] !== $attributes) { return true; } @@ -72,10 +69,9 @@ public function authenticate(RequestEvent $event) $attributes = $request->attributes->get('_access_control_attributes'); $request->attributes->remove('_access_control_attributes'); - if (!$attributes || (( - (\defined(AuthenticatedVoter::class.'::IS_AUTHENTICATED_ANONYMOUSLY') ? [AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY] === $attributes : false) - || [AuthenticatedVoter::PUBLIC_ACCESS] === $attributes - ) && $event instanceof LazyResponseEvent)) { + if (!$attributes || ( + [AuthenticatedVoter::PUBLIC_ACCESS] === $attributes && $event instanceof LazyResponseEvent + )) { return; } From 1ecf001b834ce25a21c37594883213b4a29f7eb0 Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Mon, 6 Mar 2023 20:42:33 +0100 Subject: [PATCH 18/20] [Tests] Remove occurrences of `withConsecutive()` --- .../DefaultAuthenticationFailureHandlerTest.php | 13 +++++++++---- .../PasswordMigratingListenerTest.php | 14 +++++++++++++- Tests/LoginLink/LoginLinkHandlerTest.php | 16 +++++++++++++++- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php b/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php index 46ac7243..bcd18740 100644 --- a/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php +++ b/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php @@ -197,10 +197,15 @@ public function testFailurePathFromRequestWithInvalidUrl() $this->logger->expects($this->exactly(2)) ->method('debug') - ->withConsecutive( - ['Ignoring query parameter "_my_failure_path": not a valid URL.'], - ['Authentication failure, redirect triggered.', ['failure_path' => '/login']] - ); + ->willReturnCallback(function (...$args) { + static $series = [ + ['Ignoring query parameter "_my_failure_path": not a valid URL.', []], + ['Authentication failure, redirect triggered.', ['failure_path' => '/login']], + ]; + + $expectedArgs = array_shift($series); + $this->assertSame($expectedArgs, $args); + }); $handler = new DefaultAuthenticationFailureHandler($this->httpKernel, $this->httpUtils, $options, $this->logger); diff --git a/Tests/EventListener/PasswordMigratingListenerTest.php b/Tests/EventListener/PasswordMigratingListenerTest.php index 70725c20..6fc48b7a 100644 --- a/Tests/EventListener/PasswordMigratingListenerTest.php +++ b/Tests/EventListener/PasswordMigratingListenerTest.php @@ -85,7 +85,19 @@ public function testUnsupportedPassport() // A custom Passport, without an UserBadge $passport = $this->createMock(UserPassportInterface::class); $passport->method('getUser')->willReturn($this->user); - $passport->method('hasBadge')->withConsecutive([PasswordUpgradeBadge::class], [UserBadge::class])->willReturnOnConsecutiveCalls(true, false); + $passport->method('hasBadge') + ->willReturnCallback(function (...$args) { + static $series = [ + [[PasswordUpgradeBadge::class], true], + [[UserBadge::class], false], + ]; + + [$expectedArgs, $return] = array_shift($series); + $this->assertSame($expectedArgs, $args); + + return $return; + }) + ; $passport->expects($this->once())->method('getBadge')->with(PasswordUpgradeBadge::class)->willReturn(new PasswordUpgradeBadge('pa$$word')); // We should never "getBadge" for "UserBadge::class" diff --git a/Tests/LoginLink/LoginLinkHandlerTest.php b/Tests/LoginLink/LoginLinkHandlerTest.php index 0d698362..6574a684 100644 --- a/Tests/LoginLink/LoginLinkHandlerTest.php +++ b/Tests/LoginLink/LoginLinkHandlerTest.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Security\Http\Tests\LoginLink; +use PHPUnit\Framework\Constraint\Constraint; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Cache\CacheItemPoolInterface; @@ -80,9 +81,22 @@ public function testCreateLoginLink($user, array $extraProperties, Request $requ ->method('getContext') ->willReturn($currentRequestContext = new RequestContext()); + $series = [ + $this->equalTo((new RequestContext())->fromRequest($request)->setParameter('_locale', $request->getLocale())), + $currentRequestContext, + ]; + $this->router->expects($this->exactly(2)) ->method('setContext') - ->withConsecutive([$this->equalTo((new RequestContext())->fromRequest($request)->setParameter('_locale', $request->getLocale()))], [$currentRequestContext]); + ->willReturnCallback(function (RequestContext $context) use (&$series) { + $expectedContext = array_shift($series); + + if ($expectedContext instanceof Constraint) { + $expectedContext->evaluate($context); + } else { + $this->assertSame($expectedContext, $context); + } + }); } $loginLink = $this->createLinker([], array_keys($extraProperties))->createLoginLink($user, $request); From 1310b9e1205f94a9a7ea3e42d0c8749cbaa5dd3b Mon Sep 17 00:00:00 2001 From: db306 Date: Tue, 22 Nov 2022 18:02:32 +0100 Subject: [PATCH 19/20] Make onAuthenticationSuccess Response optional --- Authentication/AuthenticationSuccessHandlerInterface.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Authentication/AuthenticationSuccessHandlerInterface.php b/Authentication/AuthenticationSuccessHandlerInterface.php index 690234e7..e440b36d 100644 --- a/Authentication/AuthenticationSuccessHandlerInterface.php +++ b/Authentication/AuthenticationSuccessHandlerInterface.php @@ -29,5 +29,5 @@ interface AuthenticationSuccessHandlerInterface /** * Usually called by AuthenticatorInterface::onAuthenticationSuccess() implementations. */ - public function onAuthenticationSuccess(Request $request, TokenInterface $token): Response; + public function onAuthenticationSuccess(Request $request, TokenInterface $token): ?Response; } From c468f059fac27680acf7e84cea07ba5ffff8942a Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Fri, 21 Apr 2023 13:56:14 +0200 Subject: [PATCH 20/20] Fix Psalm errors --- Authentication/CustomAuthenticationSuccessHandler.php | 2 +- Authentication/DefaultAuthenticationSuccessHandler.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Authentication/CustomAuthenticationSuccessHandler.php b/Authentication/CustomAuthenticationSuccessHandler.php index 5340b9ec..bfa3bdad 100644 --- a/Authentication/CustomAuthenticationSuccessHandler.php +++ b/Authentication/CustomAuthenticationSuccessHandler.php @@ -37,7 +37,7 @@ public function __construct(AuthenticationSuccessHandlerInterface $handler, arra } } - public function onAuthenticationSuccess(Request $request, TokenInterface $token): Response + public function onAuthenticationSuccess(Request $request, TokenInterface $token): ?Response { return $this->handler->onAuthenticationSuccess($request, $token); } diff --git a/Authentication/DefaultAuthenticationSuccessHandler.php b/Authentication/DefaultAuthenticationSuccessHandler.php index a6eeffcd..4fcf6baf 100644 --- a/Authentication/DefaultAuthenticationSuccessHandler.php +++ b/Authentication/DefaultAuthenticationSuccessHandler.php @@ -52,7 +52,7 @@ public function __construct(HttpUtils $httpUtils, array $options = [], LoggerInt $this->setOptions($options); } - public function onAuthenticationSuccess(Request $request, TokenInterface $token): Response + public function onAuthenticationSuccess(Request $request, TokenInterface $token): ?Response { return $this->httpUtils->createRedirectResponse($request, $this->determineTargetUrl($request)); }