From a7a6f8a6788019183508afc89a597d12926242ef Mon Sep 17 00:00:00 2001 From: Amaury Leroux de Lens Date: Fri, 4 Dec 2015 13:21:25 +0100 Subject: [PATCH 1/2] [Security] Add Guard authenticator method This method will be called before starting an authentication against a guard authhenticator. The authentication will be tried only if the supports method returned This improves understanding of code, increase consistency and removes responsability for method To decide if the current request should be supported or not. --- UPGRADE-3.4.md | 3 + UPGRADE-4.0.md | 3 + .../Guard/AbstractGuardAuthenticator.php | 19 +- .../Security/Guard/AuthenticatorInterface.php | 170 +++++++++++++++++ .../Firewall/GuardAuthenticationListener.php | 54 ++++-- .../Guard/GuardAuthenticatorHandler.php | 30 +-- .../Guard/GuardAuthenticatorInterface.php | 5 +- .../Provider/GuardAuthenticationProvider.php | 18 +- .../GuardAuthenticationListenerTest.php | 176 +++++++++++++++++- .../Tests/GuardAuthenticatorHandlerTest.php | 2 +- .../Token/PreAuthenticationGuardToken.php | 2 +- 11 files changed, 435 insertions(+), 47 deletions(-) create mode 100644 src/Symfony/Component/Security/Guard/AuthenticatorInterface.php diff --git a/UPGRADE-3.4.md b/UPGRADE-3.4.md index e5b6434c37f4e..3996618fc18ff 100644 --- a/UPGRADE-3.4.md +++ b/UPGRADE-3.4.md @@ -302,6 +302,9 @@ SecurityBundle * Using voters that do not implement the `VoterInterface`is now deprecated in the `AccessDecisionManager` and this functionality will be removed in 4.0. + * Using guard authenticator that implement the `GuardAuthenticatorInterface` is now + deprecated, this will be removed in 4.0. `AuthenticatorInterface` must be used now. + * `FirewallContext::getListeners()` now returns `\Traversable|array` * `InitAclCommand::__construct()` now takes an instance of diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index 2d2e04899c18b..a9a382b8e9d14 100644 --- a/UPGRADE-4.0.md +++ b/UPGRADE-4.0.md @@ -659,6 +659,9 @@ Security * The `RoleInterface` has been removed. Extend the `Symfony\Component\Security\Core\Role\Role` class instead. + * The `GuardAuthenticatorInterface` has been removed. Implement + `Symfony\Component\Security\Guard\AuthenticatorInterface` class instead. + * The `LogoutUrlGenerator::registerListener()` method expects a 6th `string $context = null` argument. * The `AccessDecisionManager::setVoters()` method has been removed. Pass the diff --git a/src/Symfony/Component/Security/Guard/AbstractGuardAuthenticator.php b/src/Symfony/Component/Security/Guard/AbstractGuardAuthenticator.php index 609d772e194b4..97c9c4a0922d9 100644 --- a/src/Symfony/Component/Security/Guard/AbstractGuardAuthenticator.php +++ b/src/Symfony/Component/Security/Guard/AbstractGuardAuthenticator.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Security\Guard; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Guard\Token\PostAuthenticationGuardToken; @@ -19,8 +20,24 @@ * * @author Ryan Weaver */ -abstract class AbstractGuardAuthenticator implements GuardAuthenticatorInterface +abstract class AbstractGuardAuthenticator implements AuthenticatorInterface { + /** + * Default implementation of the AuthenticatorInterface::supports method + * As we still have the deprecated GuardAuthenticatorInterface, this method must be implemented here + * Once GuardAuthenticatorInterface will be removed, this method should be removed too. + * + * @param Request $request + * + * @return bool + */ + public function supports(Request $request) + { + @trigger_error('The Symfony\Component\Security\Guard\AbstractGuardAuthenticator::supports default implementation is used. This is provided for backward compatibility on GuardAuthenticationInterface that is deprecated since version 3.1 and will be removed in 4.0. Provide your own implementation of the supports method instead.', E_USER_DEPRECATED); + + return true; + } + /** * Shortcut to create a PostAuthenticationGuardToken for you, if you don't really * care about which authenticated token you're using. diff --git a/src/Symfony/Component/Security/Guard/AuthenticatorInterface.php b/src/Symfony/Component/Security/Guard/AuthenticatorInterface.php new file mode 100644 index 0000000000000..a1497b0eef4f9 --- /dev/null +++ b/src/Symfony/Component/Security/Guard/AuthenticatorInterface.php @@ -0,0 +1,170 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Guard; + +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\Core\User\UserInterface; +use Symfony\Component\Security\Core\User\UserProviderInterface; +use Symfony\Component\Security\Guard\Token\GuardTokenInterface; +use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface; + +/** + * The interface for all "guard" authenticators. + * + * The methods on this interface are called throughout the guard authentication + * process to give you the power to control most parts of the process from + * one location. + * + * @author Ryan Weaver + * @author Amaury Leroux de Lens + */ +interface AuthenticatorInterface extends AuthenticationEntryPointInterface +{ + /** + * Does the authenticator support the given Request ? + * + * If this returns true, authentication will continue (e.g. getCredentials() will be called). + * If false, this authenticator is done. The next (if any) authenticators will be called and + * may authenticate the user, or leave the user as anonymous. + * + * @param Request $request + * + * @return bool + */ + public function supports(Request $request); + + /** + * Get the authentication credentials from the request and return them + * as any type (e.g. an associate array). If you return null, authentication + * will be skipped. + * + * Whatever value you return here will be passed to getUser() and checkCredentials() + * + * For example, for a form login, you might: + * + * return array( + * 'username' => $request->request->get('_username'), + * 'password' => $request->request->get('_password'), + * ); + * + * Or for an API token that's on a header, you might use: + * + * return array('api_key' => $request->headers->get('X-API-TOKEN')); + * + * @param Request $request + * + * @return mixed|null + */ + public function getCredentials(Request $request); + + /** + * Return a UserInterface object based on the credentials. + * + * The *credentials* are the return value from getCredentials() + * + * You may throw an AuthenticationException if you wish. If you return + * null, then a UsernameNotFoundException is thrown for you. + * + * @param mixed $credentials + * @param UserProviderInterface $userProvider + * + * @throws AuthenticationException + * + * @return UserInterface|null + */ + public function getUser($credentials, UserProviderInterface $userProvider); + + /** + * Returns true if the credentials are valid. + * + * If any value other than true is returned, authentication will + * fail. You may also throw an AuthenticationException if you wish + * to cause authentication to fail. + * + * The *credentials* are the return value from getCredentials() + * + * @param mixed $credentials + * @param UserInterface $user + * + * @return bool + * + * @throws AuthenticationException + */ + public function checkCredentials($credentials, UserInterface $user); + + /** + * Creates an authenticated token for the given user. + * + * If you don't care about which token class is used or don't really + * understand what a "token" is, you can skip this method by extending + * the AbstractGuardAuthenticator class from your authenticator. + * + * @see AbstractGuardAuthenticator + * + * @param UserInterface $user + * @param string $providerKey The provider (i.e. firewall) key + * + * @return GuardTokenInterface + */ + public function createAuthenticatedToken(UserInterface $user, $providerKey); + + /** + * Called when authentication executed, but failed (e.g. wrong username password). + * + * This should return the Response sent back to the user, like a + * RedirectResponse to the login page or a 403 response. + * + * If you return null, the request will continue, but the user will + * not be authenticated. This is probably not what you want to do. + * + * @param Request $request + * @param AuthenticationException $exception + * + * @return Response|null + */ + public function onAuthenticationFailure(Request $request, AuthenticationException $exception); + + /** + * Called when authentication executed and was successful! + * + * This should return the Response sent back to the user, like a + * RedirectResponse to the last page they visited. + * + * If you return null, the current request will continue, and the user + * will be authenticated. This makes sense, for example, with an API. + * + * @param Request $request + * @param TokenInterface $token + * @param string $providerKey The provider (i.e. firewall) key + * + * @return Response|null + */ + public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey); + + /** + * Does this method support remember me cookies? + * + * Remember me cookie will be set if *all* of the following are met: + * A) This method returns true + * B) The remember_me key under your firewall is configured + * C) The "remember me" functionality is activated. This is usually + * done by having a _remember_me checkbox in your form, but + * can be configured by the "always_remember_me" and "remember_me_parameter" + * parameters under the "remember_me" firewall key + * + * @return bool + */ + public function supportsRememberMe(); +} diff --git a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php index 41a37e6fef55b..dd5ae2c16b9f4 100644 --- a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php +++ b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php @@ -18,6 +18,7 @@ use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; use Symfony\Component\Security\Guard\GuardAuthenticatorInterface; +use Symfony\Component\Security\Guard\AuthenticatorInterface; use Psr\Log\LoggerInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; @@ -28,6 +29,7 @@ * Authentication listener for the "guard" system. * * @author Ryan Weaver + * @author Amaury Leroux de Lens */ class GuardAuthenticationListener implements ListenerInterface { @@ -39,11 +41,11 @@ class GuardAuthenticationListener implements ListenerInterface private $rememberMeServices; /** - * @param GuardAuthenticatorHandler $guardHandler The Guard handler - * @param AuthenticationManagerInterface $authenticationManager An AuthenticationManagerInterface instance - * @param string $providerKey The provider (i.e. firewall) key - * @param iterable|GuardAuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationProvider - * @param LoggerInterface $logger A LoggerInterface instance + * @param GuardAuthenticatorHandler $guardHandler The Guard handler + * @param AuthenticationManagerInterface $authenticationManager An AuthenticationManagerInterface instance + * @param string $providerKey The provider (i.e. firewall) key + * @param iterable|AuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationProvider + * @param LoggerInterface $logger A LoggerInterface instance */ public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, $providerKey, $guardAuthenticators, LoggerInterface $logger = null) { @@ -92,7 +94,7 @@ public function handle(GetResponseEvent $event) } } - private function executeGuardAuthenticator($uniqueGuardKey, GuardAuthenticatorInterface $guardAuthenticator, GetResponseEvent $event) + private function executeGuardAuthenticator($uniqueGuardKey, AuthenticatorInterface $guardAuthenticator, GetResponseEvent $event) { $request = $event->getRequest(); try { @@ -100,12 +102,38 @@ private function executeGuardAuthenticator($uniqueGuardKey, GuardAuthenticatorIn $this->logger->debug('Calling getCredentials() on guard configurator.', array('firewall_key' => $this->providerKey, 'authenticator' => get_class($guardAuthenticator))); } + // abort the execution of the authenticator if it doesn't support the request. + if ($guardAuthenticator instanceof GuardAuthenticatorInterface) { + // it's a GuardAuthenticatorInterface + // we support the previous behaviour to avoid BC break. + $credentialsCanBeNull = true; + @trigger_error('The Symfony\Component\Security\Guard\GuardAuthenticatorInterface interface is deprecated since version 3.1 and will be removed in 4.0. Use Symfony\Component\Security\Guard\Authenticator\GuardAuthenticatorInterface instead.', E_USER_DEPRECATED); + } else { + if (true !== $guardAuthenticator->supports($request)) { + return; + } + // as there was a support for given request, + // authenticator is expected to give not-null credentials. + $credentialsCanBeNull = false; + } + // allow the authenticator to fetch authentication info from the request $credentials = $guardAuthenticator->getCredentials($request); - // allow null to be returned to skip authentication if (null === $credentials) { - return; + // if GuardAuthenticatorInterface is used + // allow null to skip authentication. + if ($credentialsCanBeNull) { + return; + } + + // otherwise something went wrong and the authentication must fail + throw new \UnexpectedValueException(sprintf( + 'You must return some credentials from %s:getCredentials(). + To skip authentication, return false from %s::supports().', + get_class($guardAuthenticator), + get_class($guardAuthenticator) + )); } // create a token with the unique key, so that the provider knows which authenticator to use @@ -172,12 +200,12 @@ public function setRememberMeServices(RememberMeServicesInterface $rememberMeSer * Checks to see if remember me is supported in the authenticator and * on the firewall. If it is, the RememberMeServicesInterface is notified. * - * @param GuardAuthenticatorInterface $guardAuthenticator - * @param Request $request - * @param TokenInterface $token - * @param Response $response + * @param AuthenticatorInterface $guardAuthenticator + * @param Request $request + * @param TokenInterface $token + * @param Response $response */ - private function triggerRememberMe(GuardAuthenticatorInterface $guardAuthenticator, Request $request, TokenInterface $token, Response $response = null) + private function triggerRememberMe(AuthenticatorInterface $guardAuthenticator, Request $request, TokenInterface $token, Response $response = null) { if (null === $this->rememberMeServices) { if (null !== $this->logger) { diff --git a/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php b/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php index 5e1351dcc25ee..c8fb6e394e992 100644 --- a/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php +++ b/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php @@ -61,14 +61,14 @@ public function authenticateWithToken(TokenInterface $token, Request $request) /** * Returns the "on success" response for the given GuardAuthenticator. * - * @param TokenInterface $token - * @param Request $request - * @param GuardAuthenticatorInterface $guardAuthenticator - * @param string $providerKey The provider (i.e. firewall) key + * @param TokenInterface $token + * @param Request $request + * @param AuthenticatorInterface $guardAuthenticator + * @param string $providerKey The provider (i.e. firewall) key * * @return null|Response */ - public function handleAuthenticationSuccess(TokenInterface $token, Request $request, GuardAuthenticatorInterface $guardAuthenticator, $providerKey) + public function handleAuthenticationSuccess(TokenInterface $token, Request $request, AuthenticatorInterface $guardAuthenticator, $providerKey) { $response = $guardAuthenticator->onAuthenticationSuccess($request, $token, $providerKey); @@ -88,14 +88,14 @@ public function handleAuthenticationSuccess(TokenInterface $token, Request $requ * Convenience method for authenticating the user and returning the * Response *if any* for success. * - * @param UserInterface $user - * @param Request $request - * @param GuardAuthenticatorInterface $authenticator - * @param string $providerKey The provider (i.e. firewall) key + * @param UserInterface $user + * @param Request $request + * @param AuthenticatorInterface $authenticator + * @param string $providerKey The provider (i.e. firewall) key * * @return Response|null */ - public function authenticateUserAndHandleSuccess(UserInterface $user, Request $request, GuardAuthenticatorInterface $authenticator, $providerKey) + public function authenticateUserAndHandleSuccess(UserInterface $user, Request $request, AuthenticatorInterface $authenticator, $providerKey) { // create an authenticated token for the User $token = $authenticator->createAuthenticatedToken($user, $providerKey); @@ -110,14 +110,14 @@ public function authenticateUserAndHandleSuccess(UserInterface $user, Request $r * Handles an authentication failure and returns the Response for the * GuardAuthenticator. * - * @param AuthenticationException $authenticationException - * @param Request $request - * @param GuardAuthenticatorInterface $guardAuthenticator - * @param string $providerKey The key of the firewall + * @param AuthenticationException $authenticationException + * @param Request $request + * @param AuthenticatorInterface $guardAuthenticator + * @param string $providerKey The key of the firewall * * @return null|Response */ - public function handleAuthenticationFailure(AuthenticationException $authenticationException, Request $request, GuardAuthenticatorInterface $guardAuthenticator, $providerKey) + public function handleAuthenticationFailure(AuthenticationException $authenticationException, Request $request, AuthenticatorInterface $guardAuthenticator, $providerKey) { $token = $this->tokenStorage->getToken(); if ($token instanceof PostAuthenticationGuardToken && $providerKey === $token->getProviderKey()) { diff --git a/src/Symfony/Component/Security/Guard/GuardAuthenticatorInterface.php b/src/Symfony/Component/Security/Guard/GuardAuthenticatorInterface.php index 307d70f9e973e..2078ad237b9e2 100644 --- a/src/Symfony/Component/Security/Guard/GuardAuthenticatorInterface.php +++ b/src/Symfony/Component/Security/Guard/GuardAuthenticatorInterface.php @@ -18,7 +18,6 @@ use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; use Symfony\Component\Security\Guard\Token\GuardTokenInterface; -use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface; /** * The interface for all "guard" authenticators. @@ -28,8 +27,10 @@ * one location. * * @author Ryan Weaver + * + * @deprecated Symfony\Component\Security\Guard\AuthenticatorInterface must be used instead */ -interface GuardAuthenticatorInterface extends AuthenticationEntryPointInterface +interface GuardAuthenticatorInterface extends AuthenticatorInterface { /** * Get the authentication credentials from the request and return them diff --git a/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php b/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php index 90b9580a3a876..c93776a34857d 100644 --- a/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php +++ b/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php @@ -14,7 +14,7 @@ use Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface; use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; -use Symfony\Component\Security\Guard\GuardAuthenticatorInterface; +use Symfony\Component\Security\Guard\AuthenticatorInterface; use Symfony\Component\Security\Guard\Token\GuardTokenInterface; use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken; use Symfony\Component\Security\Core\User\UserCheckerInterface; @@ -32,7 +32,7 @@ class GuardAuthenticationProvider implements AuthenticationProviderInterface { /** - * @var GuardAuthenticatorInterface[] + * @var AuthenticatorInterface[] */ private $guardAuthenticators; private $userProvider; @@ -40,10 +40,10 @@ class GuardAuthenticationProvider implements AuthenticationProviderInterface private $userChecker; /** - * @param iterable|GuardAuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationListener - * @param UserProviderInterface $userProvider The user provider - * @param string $providerKey The provider (i.e. firewall) key - * @param UserCheckerInterface $userChecker + * @param iterable|AuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationListener + * @param UserProviderInterface $userProvider The user provider + * @param string $providerKey The provider (i.e. firewall) key + * @param UserCheckerInterface $userChecker */ public function __construct($guardAuthenticators, UserProviderInterface $userProvider, $providerKey, UserCheckerInterface $userChecker) { @@ -56,7 +56,7 @@ public function __construct($guardAuthenticators, UserProviderInterface $userPro /** * Finds the correct authenticator for the token and calls it. * - * @param GuardTokenInterface $token + * @param TokenInterface|GuardTokenInterface $token * * @return TokenInterface */ @@ -101,7 +101,7 @@ public function authenticate(TokenInterface $token) // instances that will be checked if you have multiple firewalls. } - private function authenticateViaGuard(GuardAuthenticatorInterface $guardAuthenticator, PreAuthenticationGuardToken $token) + private function authenticateViaGuard(AuthenticatorInterface $guardAuthenticator, PreAuthenticationGuardToken $token) { // get the user from the GuardAuthenticator $user = $guardAuthenticator->getUser($token->getCredentials(), $this->userProvider); @@ -142,6 +142,6 @@ private function authenticateViaGuard(GuardAuthenticatorInterface $guardAuthenti public function supports(TokenInterface $token) { - return $token instanceof GuardTokenInterface; + return $token instanceof TokenInterface; } } diff --git a/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php b/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php index 943ca49247574..60f703904cffd 100644 --- a/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php +++ b/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php @@ -20,6 +20,7 @@ /** * @author Ryan Weaver + * @author Amaury Leroux de Lens */ class GuardAuthenticationListenerTest extends TestCase { @@ -32,11 +33,16 @@ class GuardAuthenticationListenerTest extends TestCase public function testHandleSuccess() { - $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock(); + $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); $authenticateToken = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock(); $providerKey = 'my_firewall'; $credentials = array('username' => 'weaverryan', 'password' => 'all_your_base'); + + $authenticator + ->expects($this->once()) + ->method('supports') + ->willReturn(true); $authenticator ->expects($this->once()) ->method('getCredentials') @@ -82,10 +88,14 @@ public function testHandleSuccess() public function testHandleSuccessStopsAfterResponseIsSet() { - $authenticator1 = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock(); - $authenticator2 = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock(); + $authenticator1 = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); + $authenticator2 = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); // mock the first authenticator to fail, and set a Response + $authenticator1 + ->expects($this->once()) + ->method('supports') + ->willReturn(true); $authenticator1 ->expects($this->once()) ->method('getCredentials') @@ -94,6 +104,7 @@ public function testHandleSuccessStopsAfterResponseIsSet() ->expects($this->once()) ->method('handleAuthenticationFailure') ->willReturn(new Response()); + // the second authenticator should *never* be called $authenticator2 ->expects($this->never()) @@ -112,10 +123,15 @@ public function testHandleSuccessStopsAfterResponseIsSet() public function testHandleSuccessWithRememberMe() { - $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock(); + $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); $authenticateToken = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock(); $providerKey = 'my_firewall_with_rememberme'; + $authenticator + ->expects($this->once()) + ->method('supports') + ->with($this->equalTo($this->request)) + ->willReturn(true); $authenticator ->expects($this->once()) ->method('getCredentials') @@ -155,10 +171,14 @@ public function testHandleSuccessWithRememberMe() public function testHandleCatchesAuthenticationException() { - $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock(); + $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); $providerKey = 'my_firewall2'; $authException = new AuthenticationException('Get outta here crazy user with a bad password!'); + $authenticator + ->expects($this->once()) + ->method('supports') + ->willReturn(true); $authenticator ->expects($this->once()) ->method('getCredentials') @@ -185,6 +205,96 @@ public function testHandleCatchesAuthenticationException() $listener->handle($this->event); } + /** + * @group legacy + */ + public function testLegacyInterfaceNullCredentials() + { + $authenticatorA = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock(); + $providerKey = 'my_firewall3'; + + $authenticatorA + ->expects($this->once()) + ->method('getCredentials') + ->will($this->returnValue(null)); + + // this is not called + $this->authenticationManager + ->expects($this->never()) + ->method('authenticate'); + + $this->guardAuthenticatorHandler + ->expects($this->never()) + ->method('handleAuthenticationSuccess'); + + $listener = new GuardAuthenticationListener( + $this->guardAuthenticatorHandler, + $this->authenticationManager, + $providerKey, + array($authenticatorA), + $this->logger + ); + + $listener->handle($this->event); + } + + /** + * @group legacy + */ + public function testLegacyInterfaceKeepsWorking() + { + $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock(); + $authenticateToken = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock(); + $providerKey = 'my_firewall'; + + $credentials = array('username' => 'weaverryan', 'password' => 'all_your_base'); + + $authenticator + ->expects($this->once()) + ->method('getCredentials') + ->with($this->equalTo($this->request)) + ->will($this->returnValue($credentials)); + + // a clone of the token that should be created internally + $uniqueGuardKey = 'my_firewall_0'; + $nonAuthedToken = new PreAuthenticationGuardToken($credentials, $uniqueGuardKey); + + $this->authenticationManager + ->expects($this->once()) + ->method('authenticate') + ->with($this->equalTo($nonAuthedToken)) + ->will($this->returnValue($authenticateToken)); + + $this->guardAuthenticatorHandler + ->expects($this->once()) + ->method('authenticateWithToken') + ->with($authenticateToken, $this->request); + + $this->guardAuthenticatorHandler + ->expects($this->once()) + ->method('handleAuthenticationSuccess') + ->with($authenticateToken, $this->request, $authenticator, $providerKey); + + $listener = new GuardAuthenticationListener( + $this->guardAuthenticatorHandler, + $this->authenticationManager, + $providerKey, + array($authenticator), + $this->logger + ); + + $listener->setRememberMeServices($this->rememberMeServices); + // should never be called - our handleAuthenticationSuccess() does not return a Response + $this->rememberMeServices + ->expects($this->never()) + ->method('loginSuccess'); + + $listener->handle($this->event); + } + + /** + * @group legacy + */ public function testReturnNullToSkipAuth() { $authenticatorA = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock(); @@ -220,6 +330,62 @@ public function testReturnNullToSkipAuth() $listener->handle($this->event); } + public function testSupportsReturnFalseSkipAuth() + { + $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); + $providerKey = 'my_firewall4'; + + $authenticator + ->expects($this->once()) + ->method('supports') + ->will($this->returnValue(false)); + + // this is not called + $authenticator + ->expects($this->never()) + ->method('getCredentials'); + + $listener = new GuardAuthenticationListener( + $this->guardAuthenticatorHandler, + $this->authenticationManager, + $providerKey, + array($authenticator), + $this->logger + ); + + $listener->handle($this->event); + } + + /** + * @expectedException \UnexpectedValueException + */ + public function testSupportsReturnTrueRaiseMissingCredentialsException() + { + $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); + $providerKey = 'my_firewall4'; + + $authenticator + ->expects($this->once()) + ->method('supports') + ->will($this->returnValue(true)); + + // this will raise exception + $authenticator + ->expects($this->once()) + ->method('getCredentials') + ->will($this->returnValue(null)); + + $listener = new GuardAuthenticationListener( + $this->guardAuthenticatorHandler, + $this->authenticationManager, + $providerKey, + array($authenticator), + $this->logger + ); + + $listener->handle($this->event); + } + protected function setUp() { $this->authenticationManager = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationProviderManager') diff --git a/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php b/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php index b46bc4a78d4f1..29199d820eb11 100644 --- a/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php +++ b/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php @@ -128,7 +128,7 @@ protected function setUp() $this->dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')->getMock(); $this->token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock(); $this->request = new Request(array(), array(), array(), array(), array(), array()); - $this->guardAuthenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock(); + $this->guardAuthenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); } protected function tearDown() diff --git a/src/Symfony/Component/Security/Guard/Token/PreAuthenticationGuardToken.php b/src/Symfony/Component/Security/Guard/Token/PreAuthenticationGuardToken.php index abbe985c9e0b2..e1b39417d78e2 100644 --- a/src/Symfony/Component/Security/Guard/Token/PreAuthenticationGuardToken.php +++ b/src/Symfony/Component/Security/Guard/Token/PreAuthenticationGuardToken.php @@ -29,7 +29,7 @@ class PreAuthenticationGuardToken extends AbstractToken implements GuardTokenInt /** * @param mixed $credentials - * @param string $guardProviderKey Unique key that bind this token to a specific GuardAuthenticatorInterface + * @param string $guardProviderKey Unique key that bind this token to a specific AuthenticatorInterface */ public function __construct($credentials, $guardProviderKey) { From 78eecba780a059034028660ebb1ea81dcb8a511f Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Thu, 5 Oct 2017 09:59:11 +0200 Subject: [PATCH 2/2] Fix BC layer --- UPGRADE-3.4.md | 6 +- UPGRADE-4.0.md | 6 +- src/Symfony/Component/Security/CHANGELOG.md | 1 + .../Guard/AbstractGuardAuthenticator.php | 10 +- .../Security/Guard/AuthenticatorInterface.php | 121 +----------------- .../Firewall/GuardAuthenticationListener.php | 31 ++--- .../Guard/GuardAuthenticatorHandler.php | 8 +- .../Guard/GuardAuthenticatorInterface.php | 5 +- .../Provider/GuardAuthenticationProvider.php | 6 +- .../GuardAuthenticationListenerTest.php | 30 +++-- .../Tests/GuardAuthenticatorHandlerTest.php | 3 +- .../GuardAuthenticationProviderTest.php | 65 ++++++++++ 12 files changed, 122 insertions(+), 170 deletions(-) diff --git a/UPGRADE-3.4.md b/UPGRADE-3.4.md index 3996618fc18ff..c410d5c4cc44d 100644 --- a/UPGRADE-3.4.md +++ b/UPGRADE-3.4.md @@ -295,6 +295,9 @@ Security * Deprecated the HTTP digest authentication: `NonceExpiredException`, `DigestAuthenticationListener` and `DigestAuthenticationEntryPoint` will be removed in 4.0. Use another authentication system like `http_basic` instead. + + * The `GuardAuthenticatorInterface` has been deprecated and will be removed in 4.0. + Use `AuthenticatorInterface` instead. SecurityBundle -------------- @@ -302,9 +305,6 @@ SecurityBundle * Using voters that do not implement the `VoterInterface`is now deprecated in the `AccessDecisionManager` and this functionality will be removed in 4.0. - * Using guard authenticator that implement the `GuardAuthenticatorInterface` is now - deprecated, this will be removed in 4.0. `AuthenticatorInterface` must be used now. - * `FirewallContext::getListeners()` now returns `\Traversable|array` * `InitAclCommand::__construct()` now takes an instance of diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index a9a382b8e9d14..bd645e3444987 100644 --- a/UPGRADE-4.0.md +++ b/UPGRADE-4.0.md @@ -659,9 +659,6 @@ Security * The `RoleInterface` has been removed. Extend the `Symfony\Component\Security\Core\Role\Role` class instead. - * The `GuardAuthenticatorInterface` has been removed. Implement - `Symfony\Component\Security\Guard\AuthenticatorInterface` class instead. - * The `LogoutUrlGenerator::registerListener()` method expects a 6th `string $context = null` argument. * The `AccessDecisionManager::setVoters()` method has been removed. Pass the @@ -676,6 +673,9 @@ Security `DigestAuthenticationListener` and `DigestAuthenticationEntryPoint` classes have been removed. Use another authentication system like `http_basic` instead. + * The `GuardAuthenticatorInterface` interface has been removed. + Use `AuthenticatorInterface` instead. + SecurityBundle -------------- diff --git a/src/Symfony/Component/Security/CHANGELOG.md b/src/Symfony/Component/Security/CHANGELOG.md index 25e89572a557a..bd1b7b59eb793 100644 --- a/src/Symfony/Component/Security/CHANGELOG.md +++ b/src/Symfony/Component/Security/CHANGELOG.md @@ -15,6 +15,7 @@ CHANGELOG requests. * deprecated HTTP digest authentication * Added a new password encoder for the Argon2i hashing algorithm + * deprecated `GuardAuthenticatorInterface` in favor of `AuthenticatorInterface` 3.3.0 ----- diff --git a/src/Symfony/Component/Security/Guard/AbstractGuardAuthenticator.php b/src/Symfony/Component/Security/Guard/AbstractGuardAuthenticator.php index 97c9c4a0922d9..5eceb8470a567 100644 --- a/src/Symfony/Component/Security/Guard/AbstractGuardAuthenticator.php +++ b/src/Symfony/Component/Security/Guard/AbstractGuardAuthenticator.php @@ -23,17 +23,13 @@ abstract class AbstractGuardAuthenticator implements AuthenticatorInterface { /** - * Default implementation of the AuthenticatorInterface::supports method - * As we still have the deprecated GuardAuthenticatorInterface, this method must be implemented here - * Once GuardAuthenticatorInterface will be removed, this method should be removed too. + * {@inheritdoc} * - * @param Request $request - * - * @return bool + * @deprecated since version 3.4, to be removed in 4.0 */ public function supports(Request $request) { - @trigger_error('The Symfony\Component\Security\Guard\AbstractGuardAuthenticator::supports default implementation is used. This is provided for backward compatibility on GuardAuthenticationInterface that is deprecated since version 3.1 and will be removed in 4.0. Provide your own implementation of the supports method instead.', E_USER_DEPRECATED); + @trigger_error(sprintf('The "%s()" method is deprecated since version 3.4 and will be removed in 4.0. Implement the "%s::supports()" method in class "%s" instead.', __METHOD__, AuthenticatorInterface::class, get_class($this)), E_USER_DEPRECATED); return true; } diff --git a/src/Symfony/Component/Security/Guard/AuthenticatorInterface.php b/src/Symfony/Component/Security/Guard/AuthenticatorInterface.php index a1497b0eef4f9..f68b8d6f43b08 100644 --- a/src/Symfony/Component/Security/Guard/AuthenticatorInterface.php +++ b/src/Symfony/Component/Security/Guard/AuthenticatorInterface.php @@ -12,13 +12,6 @@ namespace Symfony\Component\Security\Guard; 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\Core\User\UserInterface; -use Symfony\Component\Security\Core\User\UserProviderInterface; -use Symfony\Component\Security\Guard\Token\GuardTokenInterface; -use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface; /** * The interface for all "guard" authenticators. @@ -30,14 +23,12 @@ * @author Ryan Weaver * @author Amaury Leroux de Lens */ -interface AuthenticatorInterface extends AuthenticationEntryPointInterface +interface AuthenticatorInterface extends GuardAuthenticatorInterface { /** - * Does the authenticator support the given Request ? + * Does the authenticator support the given Request? * - * If this returns true, authentication will continue (e.g. getCredentials() will be called). - * If false, this authenticator is done. The next (if any) authenticators will be called and - * may authenticate the user, or leave the user as anonymous. + * If this returns false, the authenticator will be skipped. * * @param Request $request * @@ -47,8 +38,7 @@ public function supports(Request $request); /** * Get the authentication credentials from the request and return them - * as any type (e.g. an associate array). If you return null, authentication - * will be skipped. + * as any type (e.g. an associate array). * * Whatever value you return here will be passed to getUser() and checkCredentials() * @@ -65,106 +55,9 @@ public function supports(Request $request); * * @param Request $request * - * @return mixed|null - */ - public function getCredentials(Request $request); - - /** - * Return a UserInterface object based on the credentials. - * - * The *credentials* are the return value from getCredentials() - * - * You may throw an AuthenticationException if you wish. If you return - * null, then a UsernameNotFoundException is thrown for you. + * @return mixed Any non-null value * - * @param mixed $credentials - * @param UserProviderInterface $userProvider - * - * @throws AuthenticationException - * - * @return UserInterface|null + * @throws \UnexpectedValueException If null is returned */ - public function getUser($credentials, UserProviderInterface $userProvider); - - /** - * Returns true if the credentials are valid. - * - * If any value other than true is returned, authentication will - * fail. You may also throw an AuthenticationException if you wish - * to cause authentication to fail. - * - * The *credentials* are the return value from getCredentials() - * - * @param mixed $credentials - * @param UserInterface $user - * - * @return bool - * - * @throws AuthenticationException - */ - public function checkCredentials($credentials, UserInterface $user); - - /** - * Creates an authenticated token for the given user. - * - * If you don't care about which token class is used or don't really - * understand what a "token" is, you can skip this method by extending - * the AbstractGuardAuthenticator class from your authenticator. - * - * @see AbstractGuardAuthenticator - * - * @param UserInterface $user - * @param string $providerKey The provider (i.e. firewall) key - * - * @return GuardTokenInterface - */ - public function createAuthenticatedToken(UserInterface $user, $providerKey); - - /** - * Called when authentication executed, but failed (e.g. wrong username password). - * - * This should return the Response sent back to the user, like a - * RedirectResponse to the login page or a 403 response. - * - * If you return null, the request will continue, but the user will - * not be authenticated. This is probably not what you want to do. - * - * @param Request $request - * @param AuthenticationException $exception - * - * @return Response|null - */ - public function onAuthenticationFailure(Request $request, AuthenticationException $exception); - - /** - * Called when authentication executed and was successful! - * - * This should return the Response sent back to the user, like a - * RedirectResponse to the last page they visited. - * - * If you return null, the current request will continue, and the user - * will be authenticated. This makes sense, for example, with an API. - * - * @param Request $request - * @param TokenInterface $token - * @param string $providerKey The provider (i.e. firewall) key - * - * @return Response|null - */ - public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey); - - /** - * Does this method support remember me cookies? - * - * Remember me cookie will be set if *all* of the following are met: - * A) This method returns true - * B) The remember_me key under your firewall is configured - * C) The "remember me" functionality is activated. This is usually - * done by having a _remember_me checkbox in your form, but - * can be configured by the "always_remember_me" and "remember_me_parameter" - * parameters under the "remember_me" firewall key - * - * @return bool - */ - public function supportsRememberMe(); + public function getCredentials(Request $request); } diff --git a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php index dd5ae2c16b9f4..4ec0b8f32630d 100644 --- a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php +++ b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php @@ -15,9 +15,9 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\Security\Guard\GuardAuthenticatorHandler; +use Symfony\Component\Security\Guard\GuardAuthenticatorInterface; use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; -use Symfony\Component\Security\Guard\GuardAuthenticatorInterface; use Symfony\Component\Security\Guard\AuthenticatorInterface; use Psr\Log\LoggerInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; @@ -94,7 +94,7 @@ public function handle(GetResponseEvent $event) } } - private function executeGuardAuthenticator($uniqueGuardKey, AuthenticatorInterface $guardAuthenticator, GetResponseEvent $event) + private function executeGuardAuthenticator($uniqueGuardKey, GuardAuthenticatorInterface $guardAuthenticator, GetResponseEvent $event) { $request = $event->getRequest(); try { @@ -102,38 +102,29 @@ private function executeGuardAuthenticator($uniqueGuardKey, AuthenticatorInterfa $this->logger->debug('Calling getCredentials() on guard configurator.', array('firewall_key' => $this->providerKey, 'authenticator' => get_class($guardAuthenticator))); } - // abort the execution of the authenticator if it doesn't support the request. - if ($guardAuthenticator instanceof GuardAuthenticatorInterface) { - // it's a GuardAuthenticatorInterface - // we support the previous behaviour to avoid BC break. - $credentialsCanBeNull = true; - @trigger_error('The Symfony\Component\Security\Guard\GuardAuthenticatorInterface interface is deprecated since version 3.1 and will be removed in 4.0. Use Symfony\Component\Security\Guard\Authenticator\GuardAuthenticatorInterface instead.', E_USER_DEPRECATED); - } else { - if (true !== $guardAuthenticator->supports($request)) { + // abort the execution of the authenticator if it doesn't support the request + if ($guardAuthenticator instanceof AuthenticatorInterface) { + if (!$guardAuthenticator->supports($request)) { return; } // as there was a support for given request, // authenticator is expected to give not-null credentials. $credentialsCanBeNull = false; + } else { + // deprecated since version 3.4, to be removed in 4.0 + $credentialsCanBeNull = true; } // allow the authenticator to fetch authentication info from the request $credentials = $guardAuthenticator->getCredentials($request); if (null === $credentials) { - // if GuardAuthenticatorInterface is used - // allow null to skip authentication. + // deprecated since version 3.4, to be removed in 4.0 if ($credentialsCanBeNull) { return; } - // otherwise something went wrong and the authentication must fail - throw new \UnexpectedValueException(sprintf( - 'You must return some credentials from %s:getCredentials(). - To skip authentication, return false from %s::supports().', - get_class($guardAuthenticator), - get_class($guardAuthenticator) - )); + throw new \UnexpectedValueException(sprintf('The return value of "%s::getCredentials()" must not be null. Return false from "%s::supports()" instead.', get_class($guardAuthenticator), get_class($guardAuthenticator))); } // create a token with the unique key, so that the provider knows which authenticator to use @@ -205,7 +196,7 @@ public function setRememberMeServices(RememberMeServicesInterface $rememberMeSer * @param TokenInterface $token * @param Response $response */ - private function triggerRememberMe(AuthenticatorInterface $guardAuthenticator, Request $request, TokenInterface $token, Response $response = null) + private function triggerRememberMe(GuardAuthenticatorInterface $guardAuthenticator, Request $request, TokenInterface $token, Response $response = null) { if (null === $this->rememberMeServices) { if (null !== $this->logger) { diff --git a/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php b/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php index c8fb6e394e992..abe263d888d5f 100644 --- a/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php +++ b/src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php @@ -29,6 +29,8 @@ * can be called directly (e.g. for manual authentication) or overridden. * * @author Ryan Weaver + * + * @final since version 3.4 */ class GuardAuthenticatorHandler { @@ -68,7 +70,7 @@ public function authenticateWithToken(TokenInterface $token, Request $request) * * @return null|Response */ - public function handleAuthenticationSuccess(TokenInterface $token, Request $request, AuthenticatorInterface $guardAuthenticator, $providerKey) + public function handleAuthenticationSuccess(TokenInterface $token, Request $request, GuardAuthenticatorInterface $guardAuthenticator, $providerKey) { $response = $guardAuthenticator->onAuthenticationSuccess($request, $token, $providerKey); @@ -95,7 +97,7 @@ public function handleAuthenticationSuccess(TokenInterface $token, Request $requ * * @return Response|null */ - public function authenticateUserAndHandleSuccess(UserInterface $user, Request $request, AuthenticatorInterface $authenticator, $providerKey) + public function authenticateUserAndHandleSuccess(UserInterface $user, Request $request, GuardAuthenticatorInterface $authenticator, $providerKey) { // create an authenticated token for the User $token = $authenticator->createAuthenticatedToken($user, $providerKey); @@ -117,7 +119,7 @@ public function authenticateUserAndHandleSuccess(UserInterface $user, Request $r * * @return null|Response */ - public function handleAuthenticationFailure(AuthenticationException $authenticationException, Request $request, AuthenticatorInterface $guardAuthenticator, $providerKey) + public function handleAuthenticationFailure(AuthenticationException $authenticationException, Request $request, GuardAuthenticatorInterface $guardAuthenticator, $providerKey) { $token = $this->tokenStorage->getToken(); if ($token instanceof PostAuthenticationGuardToken && $providerKey === $token->getProviderKey()) { diff --git a/src/Symfony/Component/Security/Guard/GuardAuthenticatorInterface.php b/src/Symfony/Component/Security/Guard/GuardAuthenticatorInterface.php index 2078ad237b9e2..0d11573c03b11 100644 --- a/src/Symfony/Component/Security/Guard/GuardAuthenticatorInterface.php +++ b/src/Symfony/Component/Security/Guard/GuardAuthenticatorInterface.php @@ -18,6 +18,7 @@ use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; use Symfony\Component\Security\Guard\Token\GuardTokenInterface; +use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface; /** * The interface for all "guard" authenticators. @@ -28,9 +29,9 @@ * * @author Ryan Weaver * - * @deprecated Symfony\Component\Security\Guard\AuthenticatorInterface must be used instead + * @deprecated since version 3.4, to be removed in 4.0. Use AuthenticatorInterface instead */ -interface GuardAuthenticatorInterface extends AuthenticatorInterface +interface GuardAuthenticatorInterface extends AuthenticationEntryPointInterface { /** * Get the authentication credentials from the request and return them diff --git a/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php b/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php index c93776a34857d..2f2678035fbcf 100644 --- a/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php +++ b/src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php @@ -56,7 +56,7 @@ public function __construct($guardAuthenticators, UserProviderInterface $userPro /** * Finds the correct authenticator for the token and calls it. * - * @param TokenInterface|GuardTokenInterface $token + * @param GuardTokenInterface $token * * @return TokenInterface */ @@ -101,7 +101,7 @@ public function authenticate(TokenInterface $token) // instances that will be checked if you have multiple firewalls. } - private function authenticateViaGuard(AuthenticatorInterface $guardAuthenticator, PreAuthenticationGuardToken $token) + private function authenticateViaGuard($guardAuthenticator, PreAuthenticationGuardToken $token) { // get the user from the GuardAuthenticator $user = $guardAuthenticator->getUser($token->getCredentials(), $this->userProvider); @@ -142,6 +142,6 @@ private function authenticateViaGuard(AuthenticatorInterface $guardAuthenticator public function supports(TokenInterface $token) { - return $token instanceof TokenInterface; + return $token instanceof GuardTokenInterface; } } diff --git a/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php b/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php index 60f703904cffd..5af9f130f8645 100644 --- a/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php +++ b/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php @@ -14,7 +14,10 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Guard\AuthenticatorInterface; use Symfony\Component\Security\Guard\Firewall\GuardAuthenticationListener; +use Symfony\Component\Security\Guard\GuardAuthenticatorInterface; use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken; use Symfony\Component\Security\Core\Exception\AuthenticationException; @@ -33,8 +36,8 @@ class GuardAuthenticationListenerTest extends TestCase public function testHandleSuccess() { - $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); - $authenticateToken = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock(); + $authenticator = $this->getMockBuilder(AuthenticatorInterface::class)->getMock(); + $authenticateToken = $this->getMockBuilder(TokenInterface::class)->getMock(); $providerKey = 'my_firewall'; $credentials = array('username' => 'weaverryan', 'password' => 'all_your_base'); @@ -88,8 +91,8 @@ public function testHandleSuccess() public function testHandleSuccessStopsAfterResponseIsSet() { - $authenticator1 = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); - $authenticator2 = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); + $authenticator1 = $this->getMockBuilder(AuthenticatorInterface::class)->getMock(); + $authenticator2 = $this->getMockBuilder(AuthenticatorInterface::class)->getMock(); // mock the first authenticator to fail, and set a Response $authenticator1 @@ -104,7 +107,6 @@ public function testHandleSuccessStopsAfterResponseIsSet() ->expects($this->once()) ->method('handleAuthenticationFailure') ->willReturn(new Response()); - // the second authenticator should *never* be called $authenticator2 ->expects($this->never()) @@ -123,8 +125,8 @@ public function testHandleSuccessStopsAfterResponseIsSet() public function testHandleSuccessWithRememberMe() { - $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); - $authenticateToken = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock(); + $authenticator = $this->getMockBuilder(AuthenticatorInterface::class)->getMock(); + $authenticateToken = $this->getMockBuilder(TokenInterface::class)->getMock(); $providerKey = 'my_firewall_with_rememberme'; $authenticator @@ -171,7 +173,7 @@ public function testHandleSuccessWithRememberMe() public function testHandleCatchesAuthenticationException() { - $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); + $authenticator = $this->getMockBuilder(AuthenticatorInterface::class)->getMock(); $providerKey = 'my_firewall2'; $authException = new AuthenticationException('Get outta here crazy user with a bad password!'); @@ -210,7 +212,7 @@ public function testHandleCatchesAuthenticationException() */ public function testLegacyInterfaceNullCredentials() { - $authenticatorA = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock(); + $authenticatorA = $this->getMockBuilder(GuardAuthenticatorInterface::class)->getMock(); $providerKey = 'my_firewall3'; $authenticatorA @@ -243,8 +245,8 @@ public function testLegacyInterfaceNullCredentials() */ public function testLegacyInterfaceKeepsWorking() { - $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock(); - $authenticateToken = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock(); + $authenticator = $this->getMockBuilder(GuardAuthenticatorInterface::class)->getMock(); + $authenticateToken = $this->getMockBuilder(TokenInterface::class)->getMock(); $providerKey = 'my_firewall'; $credentials = array('username' => 'weaverryan', 'password' => 'all_your_base'); @@ -332,7 +334,7 @@ public function testReturnNullToSkipAuth() public function testSupportsReturnFalseSkipAuth() { - $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); + $authenticator = $this->getMockBuilder(AuthenticatorInterface::class)->getMock(); $providerKey = 'my_firewall4'; $authenticator @@ -359,9 +361,9 @@ public function testSupportsReturnFalseSkipAuth() /** * @expectedException \UnexpectedValueException */ - public function testSupportsReturnTrueRaiseMissingCredentialsException() + public function testReturnNullFromGetCredentials() { - $authenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); + $authenticator = $this->getMockBuilder(AuthenticatorInterface::class)->getMock(); $providerKey = 'my_firewall4'; $authenticator diff --git a/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php b/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php index 29199d820eb11..c67f38e9ef91c 100644 --- a/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php +++ b/src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php @@ -14,6 +14,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Security\Guard\AuthenticatorInterface; use Symfony\Component\Security\Guard\GuardAuthenticatorHandler; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Http\Event\InteractiveLoginEvent; @@ -128,7 +129,7 @@ protected function setUp() $this->dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')->getMock(); $this->token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock(); $this->request = new Request(array(), array(), array(), array(), array(), array()); - $this->guardAuthenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\AuthenticatorInterface')->getMock(); + $this->guardAuthenticator = $this->getMockBuilder(AuthenticatorInterface::class)->getMock(); } protected function tearDown() diff --git a/src/Symfony/Component/Security/Guard/Tests/Provider/GuardAuthenticationProviderTest.php b/src/Symfony/Component/Security/Guard/Tests/Provider/GuardAuthenticationProviderTest.php index ed3920533fe92..9d8301fce8011 100644 --- a/src/Symfony/Component/Security/Guard/Tests/Provider/GuardAuthenticationProviderTest.php +++ b/src/Symfony/Component/Security/Guard/Tests/Provider/GuardAuthenticationProviderTest.php @@ -12,6 +12,9 @@ namespace Symfony\Component\Security\Guard\Tests\Provider; use PHPUnit\Framework\TestCase; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\User\UserInterface; +use Symfony\Component\Security\Guard\AuthenticatorInterface; use Symfony\Component\Security\Guard\Provider\GuardAuthenticationProvider; use Symfony\Component\Security\Guard\Token\PostAuthenticationGuardToken; @@ -28,6 +31,68 @@ public function testAuthenticate() { $providerKey = 'my_cool_firewall'; + $authenticatorA = $this->getMockBuilder(AuthenticatorInterface::class)->getMock(); + $authenticatorB = $this->getMockBuilder(AuthenticatorInterface::class)->getMock(); + $authenticatorC = $this->getMockBuilder(AuthenticatorInterface::class)->getMock(); + $authenticators = array($authenticatorA, $authenticatorB, $authenticatorC); + + // called 2 times - for authenticator A and B (stops on B because of match) + $this->preAuthenticationToken->expects($this->exactly(2)) + ->method('getGuardProviderKey') + // it will return the "1" index, which will match authenticatorB + ->will($this->returnValue('my_cool_firewall_1')); + + $enteredCredentials = array( + 'username' => '_weaverryan_test_user', + 'password' => 'guard_auth_ftw', + ); + $this->preAuthenticationToken->expects($this->atLeastOnce()) + ->method('getCredentials') + ->will($this->returnValue($enteredCredentials)); + + // authenticators A and C are never called + $authenticatorA->expects($this->never()) + ->method('getUser'); + $authenticatorC->expects($this->never()) + ->method('getUser'); + + $mockedUser = $this->getMockBuilder(UserInterface::class)->getMock(); + $authenticatorB->expects($this->once()) + ->method('getUser') + ->with($enteredCredentials, $this->userProvider) + ->will($this->returnValue($mockedUser)); + // checkCredentials is called + $authenticatorB->expects($this->once()) + ->method('checkCredentials') + ->with($enteredCredentials, $mockedUser) + // authentication works! + ->will($this->returnValue(true)); + $authedToken = $this->getMockBuilder(TokenInterface::class)->getMock(); + $authenticatorB->expects($this->once()) + ->method('createAuthenticatedToken') + ->with($mockedUser, $providerKey) + ->will($this->returnValue($authedToken)); + + // user checker should be called + $this->userChecker->expects($this->once()) + ->method('checkPreAuth') + ->with($mockedUser); + $this->userChecker->expects($this->once()) + ->method('checkPostAuth') + ->with($mockedUser); + + $provider = new GuardAuthenticationProvider($authenticators, $this->userProvider, $providerKey, $this->userChecker); + $actualAuthedToken = $provider->authenticate($this->preAuthenticationToken); + $this->assertSame($authedToken, $actualAuthedToken); + } + + /** + * @group legacy + */ + public function testLegacyAuthenticate() + { + $providerKey = 'my_cool_firewall'; + $authenticatorA = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock(); $authenticatorB = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock(); $authenticatorC = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock();