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; } diff --git a/Authentication/AuthenticatorManager.php b/Authentication/AuthenticatorManager.php index 4ab19b1d..5a60b9e2 100644 --- a/Authentication/AuthenticatorManager.php +++ b/Authentication/AuthenticatorManager.php @@ -83,7 +83,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 @@ -165,6 +165,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 @@ -213,7 +214,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; } @@ -223,7 +224,7 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req return null; } - private function handleAuthenticationSuccess(TokenInterface $authenticatedToken, Passport $passport, Request $request, AuthenticatorInterface $authenticator): ?Response + private function handleAuthenticationSuccess(TokenInterface $authenticatedToken, Passport $passport, Request $request, AuthenticatorInterface $authenticator, ?TokenInterface $previousToken): ?Response { $this->tokenStorage->setToken($authenticatedToken); @@ -233,7 +234,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/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)); } diff --git a/Authenticator/Debug/TraceableAuthenticator.php b/Authenticator/Debug/TraceableAuthenticator.php index 1ac91ce3..7131ea0b 100644 --- a/Authenticator/Debug/TraceableAuthenticator.php +++ b/Authenticator/Debug/TraceableAuthenticator.php @@ -92,9 +92,6 @@ public function isInteractive(): bool return $this->authenticator instanceof InteractiveAuthenticatorInterface && $this->authenticator->isInteractive(); } - /** - * @internal - */ public function getAuthenticator(): AuthenticatorInterface { return $this->authenticator; diff --git a/Event/LoginSuccessEvent.php b/Event/LoginSuccessEvent.php index 581ec9c7..4af1ea44 100644 --- a/Event/LoginSuccessEvent.php +++ b/Event/LoginSuccessEvent.php @@ -34,15 +34,17 @@ class LoginSuccessEvent extends Event private AuthenticatorInterface $authenticator; private Passport $passport; private TokenInterface $authenticatedToken; + private ?TokenInterface $previousToken; private Request $request; private ?Response $response; private string $firewallName; - public function __construct(AuthenticatorInterface $authenticator, Passport $passport, TokenInterface $authenticatedToken, Request $request, ?Response $response, string $firewallName) + public function __construct(AuthenticatorInterface $authenticator, Passport $passport, TokenInterface $authenticatedToken, Request $request, ?Response $response, string $firewallName, TokenInterface $previousToken = null) { $this->authenticator = $authenticator; $this->passport = $passport; $this->authenticatedToken = $authenticatedToken; + $this->previousToken = $previousToken; $this->request = $request; $this->response = $response; $this->firewallName = $firewallName; @@ -68,6 +70,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 2de7887c..47a10bdd 100644 --- a/EventListener/SessionStrategyListener.php +++ b/EventListener/SessionStrategyListener.php @@ -43,6 +43,15 @@ public function onSuccessfulLogin(LoginSuccessEvent $event): void return; } + if ($previousToken = $event->getPreviousToken()) { + $user = $token->getUserIdentifier(); + $previousUser = $previousToken->getUserIdentifier(); + + if ('' !== ($user ?? '') && $user === $previousUser) { + return; + } + } + $this->sessionAuthenticationStrategy->onAuthentication($request, $token); } 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; } 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 diff --git a/Tests/Authentication/AuthenticatorManagerTest.php b/Tests/Authentication/AuthenticatorManagerTest.php index a7f1facd..03117470 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]; } @@ -118,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]; @@ -188,7 +189,7 @@ public function testEraseCredentials($eraseCredentials) $manager->authenticateRequest($this->request); } - public function provideEraseCredentialsData() + public static function provideEraseCredentialsData() { yield [true]; 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/Authentication/DefaultAuthenticationFailureHandlerTest.php b/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php index 9b53b8a7..e1f35a12 100644 --- a/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php +++ b/Tests/Authentication/DefaultAuthenticationFailureHandlerTest.php @@ -47,7 +47,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() @@ -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/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], diff --git a/Tests/Authenticator/AbstractLoginFormAuthenticatorTest.php b/Tests/Authenticator/AbstractLoginFormAuthenticatorTest.php index 2c9a3903..c8c90b7f 100644 --- a/Tests/Authenticator/AbstractLoginFormAuthenticatorTest.php +++ b/Tests/Authenticator/AbstractLoginFormAuthenticatorTest.php @@ -31,7 +31,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/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/FormLoginAuthenticatorTest.php b/Tests/Authenticator/FormLoginAuthenticatorTest.php index ddc7b325..f81ec2c4 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', UserBadge::MAX_USERNAME_LENGTH + 1), false]; yield [str_repeat('x', UserBadge::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 ad3f7535..5350dd4a 100644 --- a/Tests/Authenticator/JsonLoginAuthenticatorTest.php +++ b/Tests/Authenticator/JsonLoginAuthenticatorTest.php @@ -48,7 +48,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"}')]; @@ -67,7 +67,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]; @@ -107,7 +107,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.']; @@ -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/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/EventListener/CheckCredentialsListenerTest.php b/Tests/EventListener/CheckCredentialsListenerTest.php index bab2ad0f..ea5b3dd9 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]; @@ -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/EventListener/CheckRememberMeConditionsListenerTest.php b/Tests/EventListener/CheckRememberMeConditionsListenerTest.php index b4653646..2aded29b 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/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/EventListener/PasswordMigratingListenerTest.php b/Tests/EventListener/PasswordMigratingListenerTest.php index e66b9bb6..b041f761 100644 --- a/Tests/EventListener/PasswordMigratingListenerTest.php +++ b/Tests/EventListener/PasswordMigratingListenerTest.php @@ -15,19 +15,19 @@ 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\Authentication\Token\NullToken; 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\Passport; use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport; use Symfony\Component\Security\Http\Event\LoginSuccessEvent; use Symfony\Component\Security\Http\EventListener\PasswordMigratingListener; +use Symfony\Component\Security\Http\Tests\Fixtures\DummyAuthenticator; class PasswordMigratingListenerTest extends TestCase { @@ -57,18 +57,18 @@ 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 new DummyTestPasswordAuthenticatedUser(); })))]; // 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())]))]; } public function testUpgradeWithUpgrader() { - $passwordUpgrader = $this->createPasswordUpgrader(); + $passwordUpgrader = $this->getMockForAbstractClass(TestMigratingUserProvider::class); $passwordUpgrader->expects($this->once()) ->method('upgradePassword') ->with($this->user, 'new-hash') @@ -105,14 +105,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(Passport $passport) + private static function createEvent(Passport $passport) { - return new LoginSuccessEvent($this->createMock(AuthenticatorInterface::class), $passport, $this->createMock(TokenInterface::class), new Request(), null, 'main'); + return new LoginSuccessEvent(new DummyAuthenticator(), $passport, new NullToken(), new Request(), null, 'main'); } } @@ -123,9 +123,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): UserInterface + { + } + + public function supportsClass(string $class): bool + { + } + + public function loadUserByUsername(string $username): UserInterface + { + } +} + 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/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/EventListener/UserProviderListenerTest.php b/Tests/EventListener/UserProviderListenerTest.php index 85c2d22c..a8840351 100644 --- a/Tests/EventListener/UserProviderListenerTest.php +++ b/Tests/EventListener/UserProviderListenerTest.php @@ -53,7 +53,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 1c02e929..283a9f73 100644 --- a/Tests/Firewall/ContextListenerTest.php +++ b/Tests/Firewall/ContextListenerTest.php @@ -156,7 +156,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'], @@ -307,7 +307,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); diff --git a/Tests/Firewall/ExceptionListenerTest.php b/Tests/Firewall/ExceptionListenerTest.php index ee382ed1..6e4d862d 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)], @@ -192,7 +192,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 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/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/DummySupportsAuthenticator.php b/Tests/Fixtures/DummySupportsAuthenticator.php new file mode 100644 index 00000000..8e7d394a --- /dev/null +++ b/Tests/Fixtures/DummySupportsAuthenticator.php @@ -0,0 +1,29 @@ + + * + * 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; + +class DummySupportsAuthenticator extends DummyAuthenticator +{ + private $supports; + + public function __construct(?bool $supports) + { + $this->supports = $supports; + } + + public function supports(Request $request): ?bool + { + return $this->supports; + } +} 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], diff --git a/Tests/LoginLink/LoginLinkHandlerTest.php b/Tests/LoginLink/LoginLinkHandlerTest.php index 8ff7047f..09080540 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,16 +81,29 @@ 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); $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'),