From b657f6711f8c2ebae6f00532b85180c13a9f2afa Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sun, 1 Dec 2019 09:33:36 +0100 Subject: [PATCH 01/23] Fix CS --- Tests/FirewallTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/FirewallTest.php b/Tests/FirewallTest.php index 774db612..3114f2da 100644 --- a/Tests/FirewallTest.php +++ b/Tests/FirewallTest.php @@ -12,7 +12,6 @@ namespace Symfony\Component\Security\Http\Tests; use PHPUnit\Framework\TestCase; -use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\Security\Http\Firewall; From 0e96123bd2f84b896825d4e6be9217088d158b8f Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Fri, 6 Dec 2019 14:04:56 +0100 Subject: [PATCH 02/23] gracefully handle missing event dispatchers --- Firewall/AbstractAuthenticationListener.php | 8 +++++++- Firewall/AbstractPreAuthenticatedListener.php | 7 ++++++- Firewall/ContextListener.php | 8 +++++++- Firewall/RememberMeListener.php | 8 +++++++- Firewall/SimplePreAuthenticationListener.php | 8 +++++++- Firewall/SwitchUserListener.php | 8 +++++++- Firewall/UsernamePasswordJsonAuthenticationListener.php | 8 +++++++- 7 files changed, 48 insertions(+), 7 deletions(-) diff --git a/Firewall/AbstractAuthenticationListener.php b/Firewall/AbstractAuthenticationListener.php index 6b358990..1929e4cf 100644 --- a/Firewall/AbstractAuthenticationListener.php +++ b/Firewall/AbstractAuthenticationListener.php @@ -93,7 +93,13 @@ public function __construct(TokenStorageInterface $tokenStorage, AuthenticationM 'require_previous_session' => true, ], $options); $this->logger = $logger; - $this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher); + + if (null !== $dispatcher && class_exists(LegacyEventDispatcherProxy::class)) { + $this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher); + } else { + $this->dispatcher = $dispatcher; + } + $this->httpUtils = $httpUtils; } diff --git a/Firewall/AbstractPreAuthenticatedListener.php b/Firewall/AbstractPreAuthenticatedListener.php index 500ae43e..6a3f1377 100644 --- a/Firewall/AbstractPreAuthenticatedListener.php +++ b/Firewall/AbstractPreAuthenticatedListener.php @@ -52,7 +52,12 @@ public function __construct(TokenStorageInterface $tokenStorage, AuthenticationM $this->authenticationManager = $authenticationManager; $this->providerKey = $providerKey; $this->logger = $logger; - $this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher); + + if (null !== $dispatcher && class_exists(LegacyEventDispatcherProxy::class)) { + $this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher); + } else { + $this->dispatcher = $dispatcher; + } } /** diff --git a/Firewall/ContextListener.php b/Firewall/ContextListener.php index 1059dd6c..26b6239a 100644 --- a/Firewall/ContextListener.php +++ b/Firewall/ContextListener.php @@ -66,7 +66,13 @@ public function __construct(TokenStorageInterface $tokenStorage, iterable $userP $this->userProviders = $userProviders; $this->sessionKey = '_security_'.$contextKey; $this->logger = $logger; - $this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher); + + if (null !== $dispatcher && class_exists(LegacyEventDispatcherProxy::class)) { + $this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher); + } else { + $this->dispatcher = $dispatcher; + } + $this->trustResolver = $trustResolver ?: new AuthenticationTrustResolver(AnonymousToken::class, RememberMeToken::class); } diff --git a/Firewall/RememberMeListener.php b/Firewall/RememberMeListener.php index ebc03db8..78841ecc 100644 --- a/Firewall/RememberMeListener.php +++ b/Firewall/RememberMeListener.php @@ -49,7 +49,13 @@ public function __construct(TokenStorageInterface $tokenStorage, RememberMeServi $this->rememberMeServices = $rememberMeServices; $this->authenticationManager = $authenticationManager; $this->logger = $logger; - $this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher); + + if (null !== $dispatcher && class_exists(LegacyEventDispatcherProxy::class)) { + $this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher); + } else { + $this->dispatcher = $dispatcher; + } + $this->catchExceptions = $catchExceptions; $this->sessionStrategy = null === $sessionStrategy ? new SessionAuthenticationStrategy(SessionAuthenticationStrategy::MIGRATE) : $sessionStrategy; } diff --git a/Firewall/SimplePreAuthenticationListener.php b/Firewall/SimplePreAuthenticationListener.php index 2c444e82..69f77326 100644 --- a/Firewall/SimplePreAuthenticationListener.php +++ b/Firewall/SimplePreAuthenticationListener.php @@ -65,7 +65,13 @@ public function __construct(TokenStorageInterface $tokenStorage, AuthenticationM $this->providerKey = $providerKey; $this->simpleAuthenticator = $simpleAuthenticator; $this->logger = $logger; - $this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher); + + if (null !== $dispatcher && class_exists(LegacyEventDispatcherProxy::class)) { + $this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher); + } else { + $this->dispatcher = $dispatcher; + } + $this->trustResolver = $trustResolver ?: new AuthenticationTrustResolver(AnonymousToken::class, RememberMeToken::class); } diff --git a/Firewall/SwitchUserListener.php b/Firewall/SwitchUserListener.php index 4b82fa1f..f605a279 100644 --- a/Firewall/SwitchUserListener.php +++ b/Firewall/SwitchUserListener.php @@ -70,7 +70,13 @@ public function __construct(TokenStorageInterface $tokenStorage, UserProviderInt $this->usernameParameter = $usernameParameter; $this->role = $role; $this->logger = $logger; - $this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher); + + if (null !== $dispatcher && class_exists(LegacyEventDispatcherProxy::class)) { + $this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher); + } else { + $this->dispatcher = $dispatcher; + } + $this->stateless = $stateless; } diff --git a/Firewall/UsernamePasswordJsonAuthenticationListener.php b/Firewall/UsernamePasswordJsonAuthenticationListener.php index 8606899a..7089c89d 100644 --- a/Firewall/UsernamePasswordJsonAuthenticationListener.php +++ b/Firewall/UsernamePasswordJsonAuthenticationListener.php @@ -69,7 +69,13 @@ public function __construct(TokenStorageInterface $tokenStorage, AuthenticationM $this->successHandler = $successHandler; $this->failureHandler = $failureHandler; $this->logger = $logger; - $this->eventDispatcher = LegacyEventDispatcherProxy::decorate($eventDispatcher); + + if (null !== $eventDispatcher && class_exists(LegacyEventDispatcherProxy::class)) { + $this->eventDispatcher = LegacyEventDispatcherProxy::decorate($eventDispatcher); + } else { + $this->eventDispatcher = $eventDispatcher; + } + $this->options = array_merge(['username_path' => 'username', 'password_path' => 'password'], $options); $this->propertyAccessor = $propertyAccessor ?: PropertyAccess::createPropertyAccessor(); } From 8ab510f214fd9c37769378b5036da58d444fe142 Mon Sep 17 00:00:00 2001 From: Diego Aguiar Date: Thu, 12 Dec 2019 11:50:33 -0700 Subject: [PATCH 03/23] [SECURITY] Revert "AbstractAuthenticationListener.php error instead info. Rebase of #28462" This reverts commit 867eb78cfea8d45334faddbbf1ad62b52fe5ed1a. --- Firewall/AbstractAuthenticationListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Firewall/AbstractAuthenticationListener.php b/Firewall/AbstractAuthenticationListener.php index 8b70a243..e400f4a7 100644 --- a/Firewall/AbstractAuthenticationListener.php +++ b/Firewall/AbstractAuthenticationListener.php @@ -184,7 +184,7 @@ abstract protected function attemptAuthentication(Request $request); private function onFailure(Request $request, AuthenticationException $failed): Response { if (null !== $this->logger) { - $this->logger->error('Authentication request failed.', ['exception' => $failed]); + $this->logger->info('Authentication request failed.', ['exception' => $failed]); } $token = $this->tokenStorage->getToken(); From f7d49a0c61e754e9ca9be4d05e6dd24eb82b2054 Mon Sep 17 00:00:00 2001 From: Lynn Date: Fri, 20 Dec 2019 16:07:22 +0100 Subject: [PATCH 04/23] Use supportsClass where possible --- Firewall/ContextListener.php | 7 ++++++- Tests/Firewall/ContextListenerTest.php | 28 ++++++++++++++++++-------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/Firewall/ContextListener.php b/Firewall/ContextListener.php index ea9f51f9..6a05ee51 100644 --- a/Firewall/ContextListener.php +++ b/Firewall/ContextListener.php @@ -168,12 +168,17 @@ protected function refreshUser(TokenInterface $token) $userNotFoundByProvider = false; $userDeauthenticated = false; + $userClass = \get_class($user); foreach ($this->userProviders as $provider) { if (!$provider instanceof UserProviderInterface) { throw new \InvalidArgumentException(sprintf('User provider "%s" must implement "%s".', \get_class($provider), UserProviderInterface::class)); } + if (!$provider->supportsClass($userClass)) { + continue; + } + try { $refreshedUser = $provider->refreshUser($user); $newToken = clone $token; @@ -233,7 +238,7 @@ protected function refreshUser(TokenInterface $token) return null; } - throw new \RuntimeException(sprintf('There is no user provider for user "%s".', \get_class($user))); + throw new \RuntimeException(sprintf('There is no user provider for user "%s".', $userClass)); } private function safelyUnserialize($serializedToken) diff --git a/Tests/Firewall/ContextListenerTest.php b/Tests/Firewall/ContextListenerTest.php index acab7087..74c45960 100644 --- a/Tests/Firewall/ContextListenerTest.php +++ b/Tests/Firewall/ContextListenerTest.php @@ -256,7 +256,7 @@ public function testIfTokenIsDeauthenticatedTriggersDeprecations() { $tokenStorage = new TokenStorage(); $refreshedUser = new User('foobar', 'baz'); - $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)]); + $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(true), new NotSupportingUserProvider(false), new SupportingUserProvider($refreshedUser)]); $this->assertSame($refreshedUser, $tokenStorage->getToken()->getUser()); } @@ -265,7 +265,7 @@ public function testIfTokenIsDeauthenticated() { $tokenStorage = new TokenStorage(); $refreshedUser = new User('foobar', 'baz'); - $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)], null, true); + $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(true), new NotSupportingUserProvider(false), new SupportingUserProvider($refreshedUser)], null, true); $this->assertNull($tokenStorage->getToken()); } @@ -287,7 +287,7 @@ public function testRememberMeGetsCanceledIfTokenIsDeauthenticated() $rememberMeServices = $this->createMock(RememberMeServicesInterface::class); $rememberMeServices->expects($this->once())->method('loginFail'); - $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)], null, true, $rememberMeServices); + $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(true), new NotSupportingUserProvider(false), new SupportingUserProvider($refreshedUser)], null, true, $rememberMeServices); $this->assertNull($tokenStorage->getToken()); } @@ -296,7 +296,7 @@ public function testTryAllUserProvidersUntilASupportingUserProviderIsFound() { $tokenStorage = new TokenStorage(); $refreshedUser = new User('foobar', 'baz'); - $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)], $refreshedUser); + $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(true), new NotSupportingUserProvider(false), new SupportingUserProvider($refreshedUser)], $refreshedUser); $this->assertSame($refreshedUser, $tokenStorage->getToken()->getUser()); } @@ -313,7 +313,7 @@ public function testNextSupportingUserProviderIsTriedIfPreviousSupportingUserPro public function testTokenIsSetToNullIfNoUserWasLoadedByTheRegisteredUserProviders() { $tokenStorage = new TokenStorage(); - $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(), new SupportingUserProvider()]); + $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(true), new NotSupportingUserProvider(false), new SupportingUserProvider()]); $this->assertNull($tokenStorage->getToken()); } @@ -321,14 +321,14 @@ public function testTokenIsSetToNullIfNoUserWasLoadedByTheRegisteredUserProvider public function testRuntimeExceptionIsThrownIfNoSupportingUserProviderWasRegistered() { $this->expectException('RuntimeException'); - $this->handleEventWithPreviousSession(new TokenStorage(), [new NotSupportingUserProvider(), new NotSupportingUserProvider()]); + $this->handleEventWithPreviousSession(new TokenStorage(), [new NotSupportingUserProvider(false), new NotSupportingUserProvider(true)]); } public function testAcceptsProvidersAsTraversable() { $tokenStorage = new TokenStorage(); $refreshedUser = new User('foobar', 'baz'); - $this->handleEventWithPreviousSession($tokenStorage, new \ArrayObject([new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)]), $refreshedUser); + $this->handleEventWithPreviousSession($tokenStorage, new \ArrayObject([new NotSupportingUserProvider(true), new NotSupportingUserProvider(false), new SupportingUserProvider($refreshedUser)]), $refreshedUser); $this->assertSame($refreshedUser, $tokenStorage->getToken()->getUser()); } @@ -383,6 +383,14 @@ private function handleEventWithPreviousSession(TokenStorageInterface $tokenStor class NotSupportingUserProvider implements UserProviderInterface { + /** @var bool */ + private $throwsUnsupportedException; + + public function __construct($throwsUnsupportedException) + { + $this->throwsUnsupportedException = $throwsUnsupportedException; + } + public function loadUserByUsername($username) { throw new UsernameNotFoundException(); @@ -390,7 +398,11 @@ public function loadUserByUsername($username) public function refreshUser(UserInterface $user) { - throw new UnsupportedUserException(); + if ($this->throwsUnsupportedException) { + throw new UnsupportedUserException(); + } + + return $user; } public function supportsClass($class) From 254cc7832f996de2570528b202f9217f4c03d696 Mon Sep 17 00:00:00 2001 From: Jan Rosier Date: Wed, 1 Jan 2020 12:03:25 +0100 Subject: [PATCH 05/23] Update year in license files --- LICENSE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENSE b/LICENSE index a677f437..9e936ec0 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2004-2019 Fabien Potencier +Copyright (c) 2004-2020 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 69c40d71cd0e56faa4378cc83449fd7042a63ba3 Mon Sep 17 00:00:00 2001 From: Shaharia Azam Date: Sat, 21 Dec 2019 04:13:14 +0600 Subject: [PATCH 06/23] Update links to documentation --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5be21118..dbac8c65 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ the Java Spring framework. Resources --------- - * [Documentation](https://symfony.com/doc/current/components/security/index.html) + * [Documentation](https://symfony.com/doc/current/components/security.html) * [Contributing](https://symfony.com/doc/current/contributing/index.html) * [Report issues](https://github.com/symfony/symfony/issues) and [send Pull Requests](https://github.com/symfony/symfony/pulls) From f8ea473688c4dd7adf5ba28a34d06a4b99df4861 Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Mon, 6 Jan 2020 22:25:08 +0100 Subject: [PATCH 07/23] [Security] Prevent canceled remember-me cookie from being accepted --- RememberMe/AbstractRememberMeServices.php | 4 ++++ Tests/RememberMe/AbstractRememberMeServicesTest.php | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/RememberMe/AbstractRememberMeServices.php b/RememberMe/AbstractRememberMeServices.php index 8dacdafb..bf69f301 100644 --- a/RememberMe/AbstractRememberMeServices.php +++ b/RememberMe/AbstractRememberMeServices.php @@ -99,6 +99,10 @@ public function getSecret() */ final public function autoLogin(Request $request) { + if (($cookie = $request->attributes->get(self::COOKIE_ATTR_NAME)) && null === $cookie->getValue()) { + return null; + } + if (null === $cookie = $request->cookies->get($this->options['name'])) { return null; } diff --git a/Tests/RememberMe/AbstractRememberMeServicesTest.php b/Tests/RememberMe/AbstractRememberMeServicesTest.php index 8dc2042f..cf70ed4c 100644 --- a/Tests/RememberMe/AbstractRememberMeServicesTest.php +++ b/Tests/RememberMe/AbstractRememberMeServicesTest.php @@ -39,6 +39,17 @@ public function testAutoLoginReturnsNullWhenNoCookie() $this->assertNull($service->autoLogin(new Request())); } + public function testAutoLoginReturnsNullAfterLoginFail() + { + $service = $this->getService(null, ['name' => 'foo', 'path' => null, 'domain' => null]); + + $request = new Request(); + $request->cookies->set('foo', 'foo'); + + $service->loginFail($request); + $this->assertNull($service->autoLogin($request)); + } + /** * @group legacy */ From eb3db77724194d2332a87483710be9015c48bca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Tue, 14 Jan 2020 22:28:32 +0100 Subject: [PATCH 08/23] Fix RememberMe with null password --- RememberMe/TokenBasedRememberMeServices.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/RememberMe/TokenBasedRememberMeServices.php b/RememberMe/TokenBasedRememberMeServices.php index afa12e4f..3df2ced6 100644 --- a/RememberMe/TokenBasedRememberMeServices.php +++ b/RememberMe/TokenBasedRememberMeServices.php @@ -89,10 +89,10 @@ protected function onLoginSuccess(Request $request, Response $response, TokenInt /** * Generates the cookie value. * - * @param string $class - * @param string $username The username - * @param int $expires The Unix timestamp when the cookie expires - * @param string $password The encoded password + * @param string $class + * @param string $username The username + * @param int $expires The Unix timestamp when the cookie expires + * @param string|null $password The encoded password * * @return string */ @@ -111,10 +111,10 @@ protected function generateCookieValue($class, $username, $expires, $password) /** * Generates a hash for the cookie to ensure it is not being tampered with. * - * @param string $class - * @param string $username The username - * @param int $expires The Unix timestamp when the cookie expires - * @param string $password The encoded password + * @param string $class + * @param string $username The username + * @param int $expires The Unix timestamp when the cookie expires + * @param string|null $password The encoded password * * @return string */ From 7b28a6f05238aab65f8fbdade42c7c806aacf064 Mon Sep 17 00:00:00 2001 From: Wouter de Jong Date: Sat, 25 Jan 2020 13:51:20 +0100 Subject: [PATCH 09/23] Improved error message when no supported user provider is found --- Firewall/ContextListener.php | 2 +- RememberMe/AbstractRememberMeServices.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Firewall/ContextListener.php b/Firewall/ContextListener.php index 6a05ee51..af912c44 100644 --- a/Firewall/ContextListener.php +++ b/Firewall/ContextListener.php @@ -238,7 +238,7 @@ protected function refreshUser(TokenInterface $token) return null; } - throw new \RuntimeException(sprintf('There is no user provider for user "%s".', $userClass)); + throw new \RuntimeException(sprintf('There is no user provider for user "%s". Shouldn\'t the "supportsClass()" method of your user provider return true for this classname?', $userClass)); } private function safelyUnserialize($serializedToken) diff --git a/RememberMe/AbstractRememberMeServices.php b/RememberMe/AbstractRememberMeServices.php index bf69f301..e47e1812 100644 --- a/RememberMe/AbstractRememberMeServices.php +++ b/RememberMe/AbstractRememberMeServices.php @@ -239,7 +239,7 @@ final protected function getUserProvider($class) } } - throw new UnsupportedUserException(sprintf('There is no user provider that supports class "%s".', $class)); + throw new UnsupportedUserException(sprintf('There is no user provider for user "%s". Shouldn\'t the "supportsClass()" method of your user provider return true for this classname?', $class)); } /** From 97edc98438873a03fe80ef5725b396104dbe44e0 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Tue, 4 Feb 2020 08:35:15 +0100 Subject: [PATCH 10/23] Add missing use statements --- Firewall.php | 1 + 1 file changed, 1 insertion(+) diff --git a/Firewall.php b/Firewall.php index ee769496..133c4486 100644 --- a/Firewall.php +++ b/Firewall.php @@ -17,6 +17,7 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\KernelEvents; +use Symfony\Component\Security\Http\Firewall\AbstractListener; use Symfony\Component\Security\Http\Firewall\AccessListener; use Symfony\Component\Security\Http\Firewall\LogoutListener; From ecdc0d1ddea712696d218563fd88d67d882bfd36 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Tue, 4 Feb 2020 09:03:00 +0100 Subject: [PATCH 11/23] Fix CS --- Logout/LogoutUrlGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Logout/LogoutUrlGenerator.php b/Logout/LogoutUrlGenerator.php index 71a071f3..b00d34f9 100644 --- a/Logout/LogoutUrlGenerator.php +++ b/Logout/LogoutUrlGenerator.php @@ -52,7 +52,7 @@ public function registerListener($key, $logoutPath, $csrfTokenId, $csrfParameter if (\func_num_args() >= 6) { $context = func_get_arg(5); } else { - if (__CLASS__ !== \get_class($this)) { + if (__CLASS__ !== static::class) { $r = new \ReflectionMethod($this, __FUNCTION__); if (__CLASS__ !== $r->getDeclaringClass()->getName()) { @trigger_error(sprintf('The "%s()" method will have a 6th `string $context = null` argument in version 4.0. Not defining it is deprecated since Symfony 3.3.', __METHOD__), E_USER_DEPRECATED); From e3f826de0f10024cdc56282c9e0eabbda332e05b Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Tue, 4 Feb 2020 10:29:10 +0100 Subject: [PATCH 12/23] Fix CS --- Tests/Firewall/ContextListenerTest.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Tests/Firewall/ContextListenerTest.php b/Tests/Firewall/ContextListenerTest.php index 092f3e7e..0d053190 100644 --- a/Tests/Firewall/ContextListenerTest.php +++ b/Tests/Firewall/ContextListenerTest.php @@ -352,8 +352,9 @@ protected function runSessionOnKernelResponse($newToken, $original = null) $session->set('_security_session', $original); } - $tokenStorage = new UsageTrackingTokenStorage(new TokenStorage(), new class([ - 'session' => function () use ($session) { return $session; } + $tokenStorage = new UsageTrackingTokenStorage(new TokenStorage(), new class(['session' => function () use ($session) { + return $session; + }, ]) implements ContainerInterface { use ServiceLocatorTrait; }); @@ -404,8 +405,9 @@ private function handleEventWithPreviousSession($userProviders, UserInterface $u if (method_exists(Request::class, 'getPreferredFormat')) { $usageIndex = $session->getUsageIndex(); - $tokenStorage = new UsageTrackingTokenStorage($tokenStorage, new class([ - 'session' => function () use ($session) { return $session; } + $tokenStorage = new UsageTrackingTokenStorage($tokenStorage, new class(['session' => function () use ($session) { + return $session; + }, ]) implements ContainerInterface { use ServiceLocatorTrait; }); From 20abb20f18690e11f36ee6942d415a8ad09bbacb Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Sun, 23 Feb 2020 15:53:58 +0100 Subject: [PATCH 13/23] [Security] Allow switching to another user when already switched --- Firewall/SwitchUserListener.php | 5 ++-- Tests/Firewall/SwitchUserListenerTest.php | 30 +++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/Firewall/SwitchUserListener.php b/Firewall/SwitchUserListener.php index 70492cc7..a5d077fb 100644 --- a/Firewall/SwitchUserListener.php +++ b/Firewall/SwitchUserListener.php @@ -134,7 +134,8 @@ private function attemptSwitchUser(Request $request, $username) return $token; } - throw new \LogicException(sprintf('You are already switched to "%s" user.', $token->getUsername())); + // User already switched, exit before seamlessly switching to another user + $token = $this->attemptExitUser($request); } if (false === $this->accessDecisionManager->decide($token, [$this->role])) { @@ -152,7 +153,7 @@ private function attemptSwitchUser(Request $request, $username) $this->userChecker->checkPostAuth($user); $roles = $user->getRoles(); - $roles[] = new SwitchUserRole('ROLE_PREVIOUS_ADMIN', $this->tokenStorage->getToken()); + $roles[] = new SwitchUserRole('ROLE_PREVIOUS_ADMIN', $token); $token = new UsernamePasswordToken($user, $user->getPassword(), $this->providerKey, $roles); diff --git a/Tests/Firewall/SwitchUserListenerTest.php b/Tests/Firewall/SwitchUserListenerTest.php index f4060f46..ab77180e 100644 --- a/Tests/Firewall/SwitchUserListenerTest.php +++ b/Tests/Firewall/SwitchUserListenerTest.php @@ -191,6 +191,36 @@ public function testSwitchUser() $this->assertInstanceOf('Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken', $this->tokenStorage->getToken()); } + public function testSwitchUserAlreadySwitched() + { + $originalToken = new UsernamePasswordToken('original', null, 'key', ['ROLE_FOO']); + $alreadySwitchedToken = new UsernamePasswordToken('switched_1', null, 'key', [new SwitchUserRole('ROLE_PREVIOUS_ADMIN', $originalToken)]); + + $tokenStorage = new TokenStorage(); + $tokenStorage->setToken($alreadySwitchedToken); + + $targetUser = new User('kuba', 'password', ['ROLE_FOO', 'ROLE_BAR']); + $this->request->query->set('_switch_user', 'kuba'); + + $this->accessDecisionManager->expects($this->once()) + ->method('decide')->with($originalToken, ['ROLE_ALLOWED_TO_SWITCH']) + ->willReturn(true); + $this->userProvider->expects($this->once()) + ->method('loadUserByUsername') + ->with('kuba') + ->willReturn($targetUser); + $this->userChecker->expects($this->once()) + ->method('checkPostAuth')->with($targetUser); + + $listener = new SwitchUserListener($tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager, null, '_switch_user', 'ROLE_ALLOWED_TO_SWITCH', null, false); + $listener->handle($this->event); + + $this->assertSame([], $this->request->query->all()); + $this->assertSame('', $this->request->server->get('QUERY_STRING')); + $this->assertSame('kuba', $tokenStorage->getToken()->getUsername()); + $this->assertSame($originalToken, $tokenStorage->getToken()->getRoles()[2]->getSource()); + } + public function testSwitchUserWorksWithFalsyUsernames() { $token = new UsernamePasswordToken('username', '', 'key', ['ROLE_FOO']); From b7df8ff958e4d56b88f76de40ec05a1c73d13f3e Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sun, 15 Mar 2020 10:01:00 +0100 Subject: [PATCH 14/23] Add missing dots at the end of exception messages --- Authentication/SimpleAuthenticationHandler.php | 4 ++-- Firewall/DigestAuthenticationListener.php | 6 +++--- Firewall/ExceptionListener.php | 2 +- Firewall/RemoteUserAuthenticationListener.php | 2 +- Firewall/SimpleFormAuthenticationListener.php | 2 +- Firewall/SimplePreAuthenticationListener.php | 4 ++-- Firewall/SwitchUserListener.php | 2 +- Firewall/X509AuthenticationListener.php | 2 +- RememberMe/AbstractRememberMeServices.php | 2 +- Session/SessionAuthenticationStrategy.php | 2 +- Tests/Firewall/ExceptionListenerTest.php | 2 +- 11 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Authentication/SimpleAuthenticationHandler.php b/Authentication/SimpleAuthenticationHandler.php index 1d03e23a..4456cff9 100644 --- a/Authentication/SimpleAuthenticationHandler.php +++ b/Authentication/SimpleAuthenticationHandler.php @@ -64,7 +64,7 @@ public function onAuthenticationSuccess(Request $request, TokenInterface $token) } if (null !== $response) { - throw new \UnexpectedValueException(sprintf('The %s::onAuthenticationSuccess method must return null to use the default success handler, or a Response object', \get_class($this->simpleAuthenticator))); + throw new \UnexpectedValueException(sprintf('The %s::onAuthenticationSuccess method must return null to use the default success handler, or a Response object.', \get_class($this->simpleAuthenticator))); } } @@ -91,7 +91,7 @@ public function onAuthenticationFailure(Request $request, AuthenticationExceptio } if (null !== $response) { - throw new \UnexpectedValueException(sprintf('The %s::onAuthenticationFailure method must return null to use the default failure handler, or a Response object', \get_class($this->simpleAuthenticator))); + throw new \UnexpectedValueException(sprintf('The %s::onAuthenticationFailure method must return null to use the default failure handler, or a Response object.', \get_class($this->simpleAuthenticator))); } } diff --git a/Firewall/DigestAuthenticationListener.php b/Firewall/DigestAuthenticationListener.php index 693dbcd5..2308896e 100644 --- a/Firewall/DigestAuthenticationListener.php +++ b/Firewall/DigestAuthenticationListener.php @@ -94,7 +94,7 @@ public function handle(GetResponseEvent $event) $user = $this->provider->loadUserByUsername($digestAuth->getUsername()); if (null === $user) { - throw new AuthenticationServiceException('Digest User provider returned null, which is an interface contract violation'); + throw new AuthenticationServiceException('Digest User provider returned null, which is an interface contract violation.'); } $serverDigestMd5 = $digestAuth->calculateServerDigest($user->getPassword(), $request->getMethod()); @@ -199,11 +199,11 @@ public function getUsername() public function validateAndDecode($entryPointKey, $expectedRealm) { if ($keys = array_diff(['username', 'realm', 'nonce', 'uri', 'response'], array_keys($this->elements))) { - throw new BadCredentialsException(sprintf('Missing mandatory digest value; received header "%s" (%s)', $this->header, implode(', ', $keys))); + throw new BadCredentialsException(sprintf('Missing mandatory digest value; received header "%s" (%s).', $this->header, implode(', ', $keys))); } if ('auth' === $this->elements['qop'] && !isset($this->elements['nc'], $this->elements['cnonce'])) { - throw new BadCredentialsException(sprintf('Missing mandatory digest value; received header "%s"', $this->header)); + throw new BadCredentialsException(sprintf('Missing mandatory digest value; received header "%s".', $this->header)); } if ($expectedRealm !== $this->elements['realm']) { diff --git a/Firewall/ExceptionListener.php b/Firewall/ExceptionListener.php index b1259d11..21e45516 100644 --- a/Firewall/ExceptionListener.php +++ b/Firewall/ExceptionListener.php @@ -212,7 +212,7 @@ private function startAuthentication(Request $request, AuthenticationException $ if (!$response instanceof Response) { $given = \is_object($response) ? \get_class($response) : \gettype($response); - throw new \LogicException(sprintf('The %s::start() method must return a Response object (%s returned)', \get_class($this->authenticationEntryPoint), $given)); + throw new \LogicException(sprintf('The %s::start() method must return a Response object (%s returned).', \get_class($this->authenticationEntryPoint), $given)); } return $response; diff --git a/Firewall/RemoteUserAuthenticationListener.php b/Firewall/RemoteUserAuthenticationListener.php index fbba95db..ea18b22d 100644 --- a/Firewall/RemoteUserAuthenticationListener.php +++ b/Firewall/RemoteUserAuthenticationListener.php @@ -41,7 +41,7 @@ public function __construct(TokenStorageInterface $tokenStorage, AuthenticationM protected function getPreAuthenticatedData(Request $request) { if (!$request->server->has($this->userKey)) { - throw new BadCredentialsException(sprintf('User key was not found: %s', $this->userKey)); + throw new BadCredentialsException(sprintf('User key was not found: %s.', $this->userKey)); } return [$request->server->get($this->userKey), null]; diff --git a/Firewall/SimpleFormAuthenticationListener.php b/Firewall/SimpleFormAuthenticationListener.php index b21a50d5..010f0574 100644 --- a/Firewall/SimpleFormAuthenticationListener.php +++ b/Firewall/SimpleFormAuthenticationListener.php @@ -54,7 +54,7 @@ class SimpleFormAuthenticationListener extends AbstractAuthenticationListener public function __construct(TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager, SessionAuthenticationStrategyInterface $sessionStrategy, HttpUtils $httpUtils, $providerKey, AuthenticationSuccessHandlerInterface $successHandler, AuthenticationFailureHandlerInterface $failureHandler, array $options = [], LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null, CsrfTokenManagerInterface $csrfTokenManager = null, SimpleFormAuthenticatorInterface $simpleAuthenticator = null) { if (!$simpleAuthenticator) { - throw new \InvalidArgumentException('Missing simple authenticator'); + throw new \InvalidArgumentException('Missing simple authenticator.'); } $this->simpleAuthenticator = $simpleAuthenticator; diff --git a/Firewall/SimplePreAuthenticationListener.php b/Firewall/SimplePreAuthenticationListener.php index 7e376f5e..d5163ae0 100644 --- a/Firewall/SimplePreAuthenticationListener.php +++ b/Firewall/SimplePreAuthenticationListener.php @@ -120,7 +120,7 @@ public function handle(GetResponseEvent $event) if ($response instanceof Response) { $event->setResponse($response); } elseif (null !== $response) { - throw new \UnexpectedValueException(sprintf('The %s::onAuthenticationFailure method must return null or a Response object', \get_class($this->simpleAuthenticator))); + throw new \UnexpectedValueException(sprintf('The %s::onAuthenticationFailure method must return null or a Response object.', \get_class($this->simpleAuthenticator))); } } @@ -132,7 +132,7 @@ public function handle(GetResponseEvent $event) if ($response instanceof Response) { $event->setResponse($response); } elseif (null !== $response) { - throw new \UnexpectedValueException(sprintf('The %s::onAuthenticationSuccess method must return null or a Response object', \get_class($this->simpleAuthenticator))); + throw new \UnexpectedValueException(sprintf('The %s::onAuthenticationSuccess method must return null or a Response object.', \get_class($this->simpleAuthenticator))); } } } diff --git a/Firewall/SwitchUserListener.php b/Firewall/SwitchUserListener.php index a5d077fb..7fe6b33f 100644 --- a/Firewall/SwitchUserListener.php +++ b/Firewall/SwitchUserListener.php @@ -100,7 +100,7 @@ public function handle(GetResponseEvent $event) try { $this->tokenStorage->setToken($this->attemptSwitchUser($request, $username)); } catch (AuthenticationException $e) { - throw new \LogicException(sprintf('Switch User failed: "%s"', $e->getMessage())); + throw new \LogicException(sprintf('Switch User failed: "%s".', $e->getMessage())); } } diff --git a/Firewall/X509AuthenticationListener.php b/Firewall/X509AuthenticationListener.php index e3cfb9d8..8adf5679 100644 --- a/Firewall/X509AuthenticationListener.php +++ b/Firewall/X509AuthenticationListener.php @@ -52,7 +52,7 @@ protected function getPreAuthenticatedData(Request $request) } if (null === $user) { - throw new BadCredentialsException(sprintf('SSL credentials not found: %s, %s', $this->userKey, $this->credentialKey)); + throw new BadCredentialsException(sprintf('SSL credentials not found: %s, %s.', $this->userKey, $this->credentialKey)); } return [$user, $request->server->get($this->credentialKey, '')]; diff --git a/RememberMe/AbstractRememberMeServices.php b/RememberMe/AbstractRememberMeServices.php index e47e1812..fba79fba 100644 --- a/RememberMe/AbstractRememberMeServices.php +++ b/RememberMe/AbstractRememberMeServices.php @@ -265,7 +265,7 @@ protected function encodeCookie(array $cookieParts) { foreach ($cookieParts as $cookiePart) { if (false !== strpos($cookiePart, self::COOKIE_DELIMITER)) { - throw new \InvalidArgumentException(sprintf('$cookieParts should not contain the cookie delimiter "%s"', self::COOKIE_DELIMITER)); + throw new \InvalidArgumentException(sprintf('$cookieParts should not contain the cookie delimiter "%s".', self::COOKIE_DELIMITER)); } } diff --git a/Session/SessionAuthenticationStrategy.php b/Session/SessionAuthenticationStrategy.php index a04a9afc..3a886afc 100644 --- a/Session/SessionAuthenticationStrategy.php +++ b/Session/SessionAuthenticationStrategy.php @@ -59,7 +59,7 @@ public function onAuthentication(Request $request, TokenInterface $token) return; default: - throw new \RuntimeException(sprintf('Invalid session authentication strategy "%s"', $this->strategy)); + throw new \RuntimeException(sprintf('Invalid session authentication strategy "%s".', $this->strategy)); } } } diff --git a/Tests/Firewall/ExceptionListenerTest.php b/Tests/Firewall/ExceptionListenerTest.php index aff73429..10c9b57b 100644 --- a/Tests/Firewall/ExceptionListenerTest.php +++ b/Tests/Firewall/ExceptionListenerTest.php @@ -86,7 +86,7 @@ public function testExceptionWhenEntryPointReturnsBadValue() $listener->onKernelException($event); // the exception has been replaced by our LogicException $this->assertInstanceOf('LogicException', $event->getException()); - $this->assertStringEndsWith('start() method must return a Response object (string returned)', $event->getException()->getMessage()); + $this->assertStringEndsWith('start() method must return a Response object (string returned).', $event->getException()->getMessage()); } /** From baab66bcd3cb305bbe7d2e3e3198d3d46f029d21 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 16 Mar 2020 08:32:23 +0100 Subject: [PATCH 15/23] Fix quotes in exception messages --- Authentication/SimpleAuthenticationHandler.php | 4 ++-- Firewall/ExceptionListener.php | 2 +- Firewall/RemoteUserAuthenticationListener.php | 2 +- Firewall/SimplePreAuthenticationListener.php | 4 ++-- Firewall/X509AuthenticationListener.php | 2 +- Tests/Authentication/SimpleAuthenticationHandlerTest.php | 4 ++-- Tests/Firewall/ExceptionListenerTest.php | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Authentication/SimpleAuthenticationHandler.php b/Authentication/SimpleAuthenticationHandler.php index 4456cff9..8e24ff74 100644 --- a/Authentication/SimpleAuthenticationHandler.php +++ b/Authentication/SimpleAuthenticationHandler.php @@ -64,7 +64,7 @@ public function onAuthenticationSuccess(Request $request, TokenInterface $token) } if (null !== $response) { - throw new \UnexpectedValueException(sprintf('The %s::onAuthenticationSuccess method must return null to use the default success handler, or a Response object.', \get_class($this->simpleAuthenticator))); + throw new \UnexpectedValueException(sprintf('The "%s::onAuthenticationSuccess()" method must return null to use the default success handler, or a Response object.', \get_class($this->simpleAuthenticator))); } } @@ -91,7 +91,7 @@ public function onAuthenticationFailure(Request $request, AuthenticationExceptio } if (null !== $response) { - throw new \UnexpectedValueException(sprintf('The %s::onAuthenticationFailure method must return null to use the default failure handler, or a Response object.', \get_class($this->simpleAuthenticator))); + throw new \UnexpectedValueException(sprintf('The "%s::onAuthenticationFailure()" method must return null to use the default failure handler, or a Response object.', \get_class($this->simpleAuthenticator))); } } diff --git a/Firewall/ExceptionListener.php b/Firewall/ExceptionListener.php index 21e45516..02329176 100644 --- a/Firewall/ExceptionListener.php +++ b/Firewall/ExceptionListener.php @@ -212,7 +212,7 @@ private function startAuthentication(Request $request, AuthenticationException $ if (!$response instanceof Response) { $given = \is_object($response) ? \get_class($response) : \gettype($response); - throw new \LogicException(sprintf('The %s::start() method must return a Response object (%s returned).', \get_class($this->authenticationEntryPoint), $given)); + throw new \LogicException(sprintf('The "%s::start()" method must return a Response object ("%s" returned).', \get_class($this->authenticationEntryPoint), $given)); } return $response; diff --git a/Firewall/RemoteUserAuthenticationListener.php b/Firewall/RemoteUserAuthenticationListener.php index ea18b22d..d456d852 100644 --- a/Firewall/RemoteUserAuthenticationListener.php +++ b/Firewall/RemoteUserAuthenticationListener.php @@ -41,7 +41,7 @@ public function __construct(TokenStorageInterface $tokenStorage, AuthenticationM protected function getPreAuthenticatedData(Request $request) { if (!$request->server->has($this->userKey)) { - throw new BadCredentialsException(sprintf('User key was not found: %s.', $this->userKey)); + throw new BadCredentialsException(sprintf('User key was not found: "%s".', $this->userKey)); } return [$request->server->get($this->userKey), null]; diff --git a/Firewall/SimplePreAuthenticationListener.php b/Firewall/SimplePreAuthenticationListener.php index d5163ae0..2b25333e 100644 --- a/Firewall/SimplePreAuthenticationListener.php +++ b/Firewall/SimplePreAuthenticationListener.php @@ -120,7 +120,7 @@ public function handle(GetResponseEvent $event) if ($response instanceof Response) { $event->setResponse($response); } elseif (null !== $response) { - throw new \UnexpectedValueException(sprintf('The %s::onAuthenticationFailure method must return null or a Response object.', \get_class($this->simpleAuthenticator))); + throw new \UnexpectedValueException(sprintf('The "%s::onAuthenticationFailure()" method must return null or a Response object.', \get_class($this->simpleAuthenticator))); } } @@ -132,7 +132,7 @@ public function handle(GetResponseEvent $event) if ($response instanceof Response) { $event->setResponse($response); } elseif (null !== $response) { - throw new \UnexpectedValueException(sprintf('The %s::onAuthenticationSuccess method must return null or a Response object.', \get_class($this->simpleAuthenticator))); + throw new \UnexpectedValueException(sprintf('The "%s::onAuthenticationSuccess()" method must return null or a Response object.', \get_class($this->simpleAuthenticator))); } } } diff --git a/Firewall/X509AuthenticationListener.php b/Firewall/X509AuthenticationListener.php index 8adf5679..8b9da1de 100644 --- a/Firewall/X509AuthenticationListener.php +++ b/Firewall/X509AuthenticationListener.php @@ -52,7 +52,7 @@ protected function getPreAuthenticatedData(Request $request) } if (null === $user) { - throw new BadCredentialsException(sprintf('SSL credentials not found: %s, %s.', $this->userKey, $this->credentialKey)); + throw new BadCredentialsException(sprintf('SSL credentials not found: "%s", "%s".', $this->userKey, $this->credentialKey)); } return [$user, $request->server->get($this->credentialKey, '')]; diff --git a/Tests/Authentication/SimpleAuthenticationHandlerTest.php b/Tests/Authentication/SimpleAuthenticationHandlerTest.php index cd28c52d..cbdfec13 100644 --- a/Tests/Authentication/SimpleAuthenticationHandlerTest.php +++ b/Tests/Authentication/SimpleAuthenticationHandlerTest.php @@ -81,7 +81,7 @@ public function testOnAuthenticationSuccessCallsSimpleAuthenticator() public function testOnAuthenticationSuccessThrowsAnExceptionIfNonResponseIsReturned() { $this->expectException('UnexpectedValueException'); - $this->expectExceptionMessage('onAuthenticationSuccess method must return null to use the default success handler, or a Response object'); + $this->expectExceptionMessage('onAuthenticationSuccess()" method must return null to use the default success handler, or a Response object'); $this->successHandler->expects($this->never()) ->method('onAuthenticationSuccess'); @@ -149,7 +149,7 @@ public function testOnAuthenticationFailureCallsSimpleAuthenticator() public function testOnAuthenticationFailureThrowsAnExceptionIfNonResponseIsReturned() { $this->expectException('UnexpectedValueException'); - $this->expectExceptionMessage('onAuthenticationFailure method must return null to use the default failure handler, or a Response object'); + $this->expectExceptionMessage('onAuthenticationFailure()" method must return null to use the default failure handler, or a Response object'); $this->failureHandler->expects($this->never()) ->method('onAuthenticationFailure'); diff --git a/Tests/Firewall/ExceptionListenerTest.php b/Tests/Firewall/ExceptionListenerTest.php index 10c9b57b..29899de1 100644 --- a/Tests/Firewall/ExceptionListenerTest.php +++ b/Tests/Firewall/ExceptionListenerTest.php @@ -86,7 +86,7 @@ public function testExceptionWhenEntryPointReturnsBadValue() $listener->onKernelException($event); // the exception has been replaced by our LogicException $this->assertInstanceOf('LogicException', $event->getException()); - $this->assertStringEndsWith('start() method must return a Response object (string returned).', $event->getException()->getMessage()); + $this->assertStringEndsWith('start()" method must return a Response object ("string" returned).', $event->getException()->getMessage()); } /** From 32596f3c90f0cb4f4cd027f1218fff7e08c2fc2e Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 24 Sep 2019 13:01:54 +0200 Subject: [PATCH 16/23] [DI] fix preloading script generation --- Firewall/AnonymousAuthenticationListener.php | 3 +++ Firewall/LegacyListenerTrait.php | 3 +++ 2 files changed, 6 insertions(+) diff --git a/Firewall/AnonymousAuthenticationListener.php b/Firewall/AnonymousAuthenticationListener.php index 0f1da391..999796d3 100644 --- a/Firewall/AnonymousAuthenticationListener.php +++ b/Firewall/AnonymousAuthenticationListener.php @@ -19,6 +19,9 @@ use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; +// Help opcache.preload discover always-needed symbols +class_exists(AnonymousToken::class); + /** * AnonymousAuthenticationListener automatically adds a Token if none is * already present. diff --git a/Firewall/LegacyListenerTrait.php b/Firewall/LegacyListenerTrait.php index 260cb680..6f2bc223 100644 --- a/Firewall/LegacyListenerTrait.php +++ b/Firewall/LegacyListenerTrait.php @@ -15,6 +15,9 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Event\RequestEvent; +// Help opcache.preload discover always-needed symbols +class_exists(RequestEvent::class); + /** * @deprecated * From ea48e2da7959835a4be4817566573d1aadd9c6fe Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 17 Mar 2020 19:02:13 +0100 Subject: [PATCH 17/23] [Security/Http] don't require the session to be started when tracking its id --- Firewall/ContextListener.php | 4 ++-- Tests/Firewall/ContextListenerTest.php | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/Firewall/ContextListener.php b/Firewall/ContextListener.php index 9243119a..1201e161 100644 --- a/Firewall/ContextListener.php +++ b/Firewall/ContextListener.php @@ -115,10 +115,10 @@ public function authenticate(RequestEvent $event) if (null !== $session) { $usageIndexValue = method_exists(Request::class, 'getPreferredFormat') && $session instanceof Session ? $usageIndexReference = &$session->getUsageIndex() : 0; - $sessionId = $session->getId(); + $sessionId = $request->cookies->get($session->getName()); $token = $session->get($this->sessionKey); - if ($this->sessionTrackerEnabler && $session->getId() === $sessionId) { + if ($this->sessionTrackerEnabler && \in_array($sessionId, [true, $session->getId()], true)) { $usageIndexReference = $usageIndexValue; } } diff --git a/Tests/Firewall/ContextListenerTest.php b/Tests/Firewall/ContextListenerTest.php index 0d053190..82a5f917 100644 --- a/Tests/Firewall/ContextListenerTest.php +++ b/Tests/Firewall/ContextListenerTest.php @@ -344,6 +344,26 @@ public function testDeauthenticatedEvent() $this->assertNull($tokenStorage->getToken()); } + /** + * @requires function \Symfony\Component\HttpFoundation\Request::getPreferredFormat + */ + public function testWithPreviousNotStartedSession() + { + $session = new Session(new MockArraySessionStorage()); + + $request = new Request(); + $request->setSession($session); + $request->cookies->set('MOCKSESSID', true); + + $usageIndex = $session->getUsageIndex(); + + $tokenStorage = new TokenStorage(); + $listener = new ContextListener($tokenStorage, [], 'context_key', null, null, null, [$tokenStorage, 'getToken']); + $listener(new RequestEvent($this->getMockBuilder(HttpKernelInterface::class)->getMock(), $request, HttpKernelInterface::MASTER_REQUEST)); + + $this->assertSame($usageIndex, $session->getUsageIndex()); + } + protected function runSessionOnKernelResponse($newToken, $original = null) { $session = new Session(new MockArraySessionStorage()); From d964f003ddd85bdff8739d463e2b97a90765697c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Tue, 17 Jul 2018 14:51:24 +0200 Subject: [PATCH 18/23] [Security] Remember me: allow to set the samesite cookie flag --- RememberMe/AbstractRememberMeServices.php | 3 ++- RememberMe/PersistentTokenBasedRememberMeServices.php | 8 ++++++-- RememberMe/TokenBasedRememberMeServices.php | 4 +++- .../PersistentTokenBasedRememberMeServicesTest.php | 4 +++- Tests/RememberMe/TokenBasedRememberMeServicesTest.php | 4 +++- 5 files changed, 17 insertions(+), 6 deletions(-) diff --git a/RememberMe/AbstractRememberMeServices.php b/RememberMe/AbstractRememberMeServices.php index fba79fba..53b63689 100644 --- a/RememberMe/AbstractRememberMeServices.php +++ b/RememberMe/AbstractRememberMeServices.php @@ -38,6 +38,7 @@ abstract class AbstractRememberMeServices implements RememberMeServicesInterface protected $options = [ 'secure' => false, 'httponly' => true, + 'samesite' => null, ]; private $providerKey; private $secret; @@ -281,7 +282,7 @@ protected function cancelCookie(Request $request) $this->logger->debug('Clearing remember-me cookie.', ['name' => $this->options['name']]); } - $request->attributes->set(self::COOKIE_ATTR_NAME, new Cookie($this->options['name'], null, 1, $this->options['path'], $this->options['domain'], $this->options['secure'], $this->options['httponly'])); + $request->attributes->set(self::COOKIE_ATTR_NAME, new Cookie($this->options['name'], null, 1, $this->options['path'], $this->options['domain'], $this->options['secure'], $this->options['httponly'], false, $this->options['samesite'])); } /** diff --git a/RememberMe/PersistentTokenBasedRememberMeServices.php b/RememberMe/PersistentTokenBasedRememberMeServices.php index 272a5cc2..94ec0eae 100644 --- a/RememberMe/PersistentTokenBasedRememberMeServices.php +++ b/RememberMe/PersistentTokenBasedRememberMeServices.php @@ -84,7 +84,9 @@ protected function processAutoLoginCookie(array $cookieParts, Request $request) $this->options['path'], $this->options['domain'], $this->options['secure'], - $this->options['httponly'] + $this->options['httponly'], + false, + $this->options['samesite'] ) ); @@ -117,7 +119,9 @@ protected function onLoginSuccess(Request $request, Response $response, TokenInt $this->options['path'], $this->options['domain'], $this->options['secure'], - $this->options['httponly'] + $this->options['httponly'], + false, + $this->options['samesite'] ) ); } diff --git a/RememberMe/TokenBasedRememberMeServices.php b/RememberMe/TokenBasedRememberMeServices.php index 3df2ced6..32e65f3c 100644 --- a/RememberMe/TokenBasedRememberMeServices.php +++ b/RememberMe/TokenBasedRememberMeServices.php @@ -81,7 +81,9 @@ protected function onLoginSuccess(Request $request, Response $response, TokenInt $this->options['path'], $this->options['domain'], $this->options['secure'], - $this->options['httponly'] + $this->options['httponly'], + false, + $this->options['samesite'] ) ); } diff --git a/Tests/RememberMe/PersistentTokenBasedRememberMeServicesTest.php b/Tests/RememberMe/PersistentTokenBasedRememberMeServicesTest.php index 599a7e81..7afa48ed 100644 --- a/Tests/RememberMe/PersistentTokenBasedRememberMeServicesTest.php +++ b/Tests/RememberMe/PersistentTokenBasedRememberMeServicesTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Http\Tests\RememberMe; use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Cookie; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\ResponseHeaderBag; @@ -268,7 +269,7 @@ public function testLoginFail() public function testLoginSuccessSetsCookieWhenLoggedInWithNonRememberMeTokenInterfaceImplementation() { - $service = $this->getService(null, ['name' => 'foo', 'domain' => 'myfoodomain.foo', 'path' => '/foo/path', 'secure' => true, 'httponly' => true, 'lifetime' => 3600, 'always_remember_me' => true]); + $service = $this->getService(null, ['name' => 'foo', 'domain' => 'myfoodomain.foo', 'path' => '/foo/path', 'secure' => true, 'httponly' => true, 'samesite' => Cookie::SAMESITE_STRICT, 'lifetime' => 3600, 'always_remember_me' => true]); $request = new Request(); $response = new Response(); @@ -305,6 +306,7 @@ public function testLoginSuccessSetsCookieWhenLoggedInWithNonRememberMeTokenInte $this->assertTrue($cookie->getExpiresTime() > time() + 3590 && $cookie->getExpiresTime() < time() + 3610); $this->assertEquals('myfoodomain.foo', $cookie->getDomain()); $this->assertEquals('/foo/path', $cookie->getPath()); + $this->assertSame(Cookie::SAMESITE_STRICT, $cookie->getSameSite()); } protected function encodeCookie(array $parts) diff --git a/Tests/RememberMe/TokenBasedRememberMeServicesTest.php b/Tests/RememberMe/TokenBasedRememberMeServicesTest.php index f24e4fff..4a34d614 100644 --- a/Tests/RememberMe/TokenBasedRememberMeServicesTest.php +++ b/Tests/RememberMe/TokenBasedRememberMeServicesTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Http\Tests\RememberMe; use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Cookie; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\ResponseHeaderBag; @@ -205,7 +206,7 @@ public function testLoginSuccessIgnoresTokensWhichDoNotContainAnUserInterfaceImp public function testLoginSuccess() { - $service = $this->getService(null, ['name' => 'foo', 'domain' => 'myfoodomain.foo', 'path' => '/foo/path', 'secure' => true, 'httponly' => true, 'lifetime' => 3600, 'always_remember_me' => true]); + $service = $this->getService(null, ['name' => 'foo', 'domain' => 'myfoodomain.foo', 'path' => '/foo/path', 'secure' => true, 'httponly' => true, 'samesite' => Cookie::SAMESITE_STRICT, 'lifetime' => 3600, 'always_remember_me' => true]); $request = new Request(); $response = new Response(); @@ -240,6 +241,7 @@ public function testLoginSuccess() $this->assertTrue($cookie->getExpiresTime() > time() + 3590 && $cookie->getExpiresTime() < time() + 3610); $this->assertEquals('myfoodomain.foo', $cookie->getDomain()); $this->assertEquals('/foo/path', $cookie->getPath()); + $this->assertSame(Cookie::SAMESITE_STRICT, $cookie->getSameSite()); } protected function getCookie($class, $username, $expires, $password) From 5c13e4916bd61033563eeb62d6b97567331e870f Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Wed, 25 Mar 2020 13:02:26 +0100 Subject: [PATCH 19/23] Fixed some typos --- Tests/Firewall/SwitchUserListenerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Firewall/SwitchUserListenerTest.php b/Tests/Firewall/SwitchUserListenerTest.php index ab77180e..e1533265 100644 --- a/Tests/Firewall/SwitchUserListenerTest.php +++ b/Tests/Firewall/SwitchUserListenerTest.php @@ -315,7 +315,7 @@ public function testSwitchUserWithReplacedToken() $this->assertSame($replacedToken, $this->tokenStorage->getToken()); } - public function testSwitchtUserThrowsAuthenticationExceptionIfNoCurrentToken() + public function testSwitchUserThrowsAuthenticationExceptionIfNoCurrentToken() { $this->expectException('Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException'); $this->tokenStorage->setToken(null); From 889356185703027502be9c9455eeb53949a24401 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Wed, 25 Mar 2020 22:25:16 +0100 Subject: [PATCH 20/23] add missing gitattributes for phpunit-bridge --- .gitattributes | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitattributes b/.gitattributes index ebb92870..84c7add0 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,3 +1,4 @@ /Tests export-ignore /phpunit.xml.dist export-ignore +/.gitattributes export-ignore /.gitignore export-ignore From 4238b11a24749105b097d943818dbdcb9b748486 Mon Sep 17 00:00:00 2001 From: Wouter de Jong Date: Sat, 28 Mar 2020 17:27:16 +0100 Subject: [PATCH 21/23] [Security/Http] Allow setting cookie security settings for delete_cookies --- Logout/CookieClearingLogoutHandler.php | 2 +- Tests/Logout/CookieClearingLogoutHandlerTest.php | 7 ++++++- composer.json | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Logout/CookieClearingLogoutHandler.php b/Logout/CookieClearingLogoutHandler.php index 2aa7c732..9367a62b 100644 --- a/Logout/CookieClearingLogoutHandler.php +++ b/Logout/CookieClearingLogoutHandler.php @@ -38,7 +38,7 @@ public function __construct(array $cookies) public function logout(Request $request, Response $response, TokenInterface $token) { foreach ($this->cookies as $cookieName => $cookieData) { - $response->headers->clearCookie($cookieName, $cookieData['path'], $cookieData['domain']); + $response->headers->clearCookie($cookieName, $cookieData['path'], $cookieData['domain'], isset($cookieData['secure']) ? $cookieData['secure'] : false, true, isset($cookieData['samesite']) ? $cookieData['samesite'] : null); } } } diff --git a/Tests/Logout/CookieClearingLogoutHandlerTest.php b/Tests/Logout/CookieClearingLogoutHandlerTest.php index 8dcc1033..f2407fcb 100644 --- a/Tests/Logout/CookieClearingLogoutHandlerTest.php +++ b/Tests/Logout/CookieClearingLogoutHandlerTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Http\Tests\Logout; use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Cookie; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\ResponseHeaderBag; @@ -25,7 +26,7 @@ public function testLogout() $response = new Response(); $token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock(); - $handler = new CookieClearingLogoutHandler(['foo' => ['path' => '/foo', 'domain' => 'foo.foo'], 'foo2' => ['path' => null, 'domain' => null]]); + $handler = new CookieClearingLogoutHandler(['foo' => ['path' => '/foo', 'domain' => 'foo.foo', 'secure' => true, 'samesite' => Cookie::SAMESITE_STRICT], 'foo2' => ['path' => null, 'domain' => null]]); $cookies = $response->headers->getCookies(); $this->assertCount(0, $cookies); @@ -39,12 +40,16 @@ public function testLogout() $this->assertEquals('foo', $cookie->getName()); $this->assertEquals('/foo', $cookie->getPath()); $this->assertEquals('foo.foo', $cookie->getDomain()); + $this->assertEquals(Cookie::SAMESITE_STRICT, $cookie->getSameSite()); + $this->assertTrue($cookie->isSecure()); $this->assertTrue($cookie->isCleared()); $cookie = $cookies['']['/']['foo2']; $this->assertStringStartsWith('foo2', $cookie->getName()); $this->assertEquals('/', $cookie->getPath()); $this->assertNull($cookie->getDomain()); + $this->assertNull($cookie->getSameSite()); + $this->assertFalse($cookie->isSecure()); $this->assertTrue($cookie->isCleared()); } } diff --git a/composer.json b/composer.json index 9ede332f..fcf018c5 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ "php": "^5.5.9|>=7.0.8", "symfony/security-core": "~3.2|~4.0", "symfony/event-dispatcher": "~2.8|~3.0|~4.0", - "symfony/http-foundation": "~2.8|~3.0|~4.0", + "symfony/http-foundation": "~3.4.39|^4.4.6", "symfony/http-kernel": "~3.3|~4.0", "symfony/polyfill-php56": "~1.0", "symfony/polyfill-php70": "~1.0", From 84657e5154b875b883f61a94abf557cc1e898f70 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 30 Mar 2020 13:26:49 +0200 Subject: [PATCH 22/23] Fix versions --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index fcf018c5..449e07ed 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ "php": "^5.5.9|>=7.0.8", "symfony/security-core": "~3.2|~4.0", "symfony/event-dispatcher": "~2.8|~3.0|~4.0", - "symfony/http-foundation": "~3.4.39|^4.4.6", + "symfony/http-foundation": "~3.4.40|^4.4.7", "symfony/http-kernel": "~3.3|~4.0", "symfony/polyfill-php56": "~1.0", "symfony/polyfill-php70": "~1.0", From b413064160255c31077bb082d25b7bd89275971b Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Fri, 6 Mar 2020 14:55:51 +0100 Subject: [PATCH 23/23] [Security] Fix access_control behavior with unanimous decision strategy --- Firewall/AccessListener.php | 10 +------ Tests/Firewall/AccessListenerTest.php | 41 +++++++++++++++++++++++++++ composer.json | 2 +- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/Firewall/AccessListener.php b/Firewall/AccessListener.php index 00673f60..b2944568 100644 --- a/Firewall/AccessListener.php +++ b/Firewall/AccessListener.php @@ -87,15 +87,7 @@ public function authenticate(RequestEvent $event) $this->tokenStorage->setToken($token); } - $granted = false; - foreach ($attributes as $key => $value) { - if ($this->accessDecisionManager->decide($token, [$key => $value], $request)) { - $granted = true; - break; - } - } - - if (!$granted) { + if (!$this->accessDecisionManager->decide($token, $attributes, $request, true)) { $exception = new AccessDeniedException(); $exception->setAttributes($attributes); $exception->setSubject($request); diff --git a/Tests/Firewall/AccessListenerTest.php b/Tests/Firewall/AccessListenerTest.php index 168e2564..75798d05 100644 --- a/Tests/Firewall/AccessListenerTest.php +++ b/Tests/Firewall/AccessListenerTest.php @@ -16,6 +16,7 @@ use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; +use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Http\AccessMapInterface; @@ -227,4 +228,44 @@ public function testHandleWhenTheSecurityTokenStorageHasNoToken() $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST)); } + + public function testHandleMWithultipleAttributesShouldBeHandledAsAnd() + { + $request = new Request(); + + $accessMap = $this->getMockBuilder('Symfony\Component\Security\Http\AccessMapInterface')->getMock(); + $accessMap + ->expects($this->any()) + ->method('getPatterns') + ->with($this->equalTo($request)) + ->willReturn([['foo' => 'bar', 'bar' => 'baz'], null]) + ; + + $authenticatedToken = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock(); + $authenticatedToken + ->expects($this->any()) + ->method('isAuthenticated') + ->willReturn(true) + ; + + $tokenStorage = new TokenStorage(); + $tokenStorage->setToken($authenticatedToken); + + $accessDecisionManager = $this->getMockBuilder('Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface')->getMock(); + $accessDecisionManager + ->expects($this->once()) + ->method('decide') + ->with($this->equalTo($authenticatedToken), $this->equalTo(['foo' => 'bar', 'bar' => 'baz']), $this->equalTo($request), true) + ->willReturn(true) + ; + + $listener = new AccessListener( + $tokenStorage, + $accessDecisionManager, + $accessMap, + $this->createMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface') + ); + + $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST)); + } } diff --git a/composer.json b/composer.json index 7c9cdb48..699ffcf7 100644 --- a/composer.json +++ b/composer.json @@ -17,7 +17,7 @@ ], "require": { "php": "^7.1.3", - "symfony/security-core": "^4.4", + "symfony/security-core": "^4.4.7", "symfony/http-foundation": "^3.4.40|^4.4.7|^5.0.7", "symfony/http-kernel": "^4.4", "symfony/property-access": "^3.4|^4.0|^5.0"