diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AbstractFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AbstractFactory.php index c74fb1348a..87fbb30d1f 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AbstractFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AbstractFactory.php @@ -28,14 +28,21 @@ abstract class AbstractFactory implements SecurityFactoryInterface { protected $options = array( 'check_path' => '/login_check', - 'login_path' => '/login', 'use_forward' => false, + ); + + protected $defaultSuccessHandlerOptions = array( 'always_use_default_target_path' => false, 'default_target_path' => '/', + 'login_path' => '/login', 'target_path_parameter' => '_target_path', 'use_referer' => false, + ); + + protected $defaultFailureHandlerOptions = array( 'failure_path' => null, 'failure_forward' => false, + 'login_path' => '/login', ); public function create(ContainerBuilder $container, $id, $config, $userProviderId, $defaultEntryPointId) @@ -71,7 +78,7 @@ abstract class AbstractFactory implements SecurityFactoryInterface ->scalarNode('failure_handler')->end() ; - foreach ($this->options as $name => $default) { + foreach (array_merge($this->options, $this->defaultSuccessHandlerOptions, $this->defaultFailureHandlerOptions) as $name => $default) { if (is_bool($default)) { $builder->booleanNode($name)->defaultValue($default); } else { @@ -149,21 +156,42 @@ abstract class AbstractFactory implements SecurityFactoryInterface $listenerId = $this->getListenerId(); $listener = new DefinitionDecorator($listenerId); $listener->replaceArgument(4, $id); - $listener->replaceArgument(5, array_intersect_key($config, $this->options)); - - // success handler - if (isset($config['success_handler'])) { - $listener->replaceArgument(6, new Reference($config['success_handler'])); - } - - // failure handler - if (isset($config['failure_handler'])) { - $listener->replaceArgument(7, new Reference($config['failure_handler'])); - } + $listener->replaceArgument(5, new Reference($this->createAuthenticationSuccessHandler($container, $id, $config))); + $listener->replaceArgument(6, new Reference($this->createAuthenticationFailureHandler($container, $id, $config))); + $listener->replaceArgument(7, array_intersect_key($config, $this->options)); $listenerId .= '.'.$id; $container->setDefinition($listenerId, $listener); return $listenerId; } + + protected function createAuthenticationSuccessHandler($container, $id, $config) + { + if (isset($config['success_handler'])) { + return $config['success_handler']; + } + + $successHandlerId = 'security.authentication.success_handler.'.$id; + + $successHandler = $container->setDefinition($successHandlerId, new DefinitionDecorator('security.authentication.success_handler')); + $successHandler->replaceArgument(1, $id); + $successHandler->replaceArgument(2, array_intersect_key($config, $this->defaultSuccessHandlerOptions)); + + return $successHandlerId; + } + + protected function createAuthenticationFailureHandler($container, $id, $config) + { + if (isset($config['failure_handler'])) { + return $config['failure_handler']; + } + + $id = 'security.authentication.failure_handler.'.$id; + + $failureHandler = $container->setDefinition($id, new DefinitionDecorator('security.authentication.failure_handler')); + $failureHandler->replaceArgument(2, array_intersect_key($config, $this->defaultFailureHandlerOptions)); + + return $id; + } } diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml index b6cd02007a..6c39faceff 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml @@ -37,6 +37,9 @@ Symfony\Component\Security\Core\Authentication\Provider\PreAuthenticatedAuthenticationProvider Symfony\Component\Security\Core\Authentication\Provider\AnonymousAuthenticationProvider + + Symfony\Component\Security\Http\Authentication\DefaultAuthenticationSuccessHandler + Symfony\Component\Security\Http\Authentication\DefaultAuthenticationFailureHandler @@ -98,13 +101,26 @@ + + - - + + + + + + + + + + + + + getFactory(); + list($container, + $authProviderId, + $listenerId, + $entryPointId + ) = $this->callFactory('foo', array('use_forward' => true, 'failure_path' => '/foo', 'success_handler' => 'qux', 'failure_handler' => 'bar', 'remember_me' => true), 'user_provider', 'entry_point'); + + // auth provider + $this->assertEquals('auth_provider', $authProviderId); + + // listener + $this->assertEquals('abstract_listener.foo', $listenerId); + $this->assertTrue($container->hasDefinition('abstract_listener.foo')); + $definition = $container->getDefinition('abstract_listener.foo'); + $this->assertEquals(array( + 'index_4' => 'foo', + 'index_5' => new Reference('qux'), + 'index_6' => new Reference('bar'), + 'index_7' => array( + 'use_forward' => true, + ), + ), $definition->getArguments()); + + // entry point + $this->assertEquals('entry_point', $entryPointId, '->create() does not change the default entry point.'); + } + + public function testDefaultFailureHandler() + { + list($container, + $authProviderId, + $listenerId, + $entryPointId + ) = $this->callFactory('foo', array('remember_me' => true), 'user_provider', 'entry_point'); + + $definition = $container->getDefinition('abstract_listener.foo'); + $arguments = $definition->getArguments(); + $this->assertEquals(new Reference('security.authentication.failure_handler.foo'), $arguments['index_6']); + } + + public function testDefaultSuccessHandler() + { + list($container, + $authProviderId, + $listenerId, + $entryPointId + ) = $this->callFactory('foo', array('remember_me' => true), 'user_provider', 'entry_point'); + + $definition = $container->getDefinition('abstract_listener.foo'); + $arguments = $definition->getArguments(); + $this->assertEquals(new Reference('security.authentication.success_handler.foo'), $arguments['index_5']); + } + + protected function callFactory($id, $config, $userProviderId, $defaultEntryPointId) + { + $factory = $this->getMockForAbstractClass('Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AbstractFactory', array()); $factory ->expects($this->once()) @@ -37,30 +91,8 @@ class AbstractFactoryTest extends \PHPUnit_Framework_TestCase list($authProviderId, $listenerId, $entryPointId - ) = $factory->create($container, 'foo', array('use_forward' => true, 'failure_path' => '/foo', 'success_handler' => 'foo', 'remember_me' => true), 'user_provider', 'entry_point'); + ) = $factory->create($container, $id, $config, $userProviderId, $defaultEntryPointId); - // auth provider - $this->assertEquals('auth_provider', $authProviderId); - - // listener - $this->assertEquals('abstract_listener.foo', $listenerId); - $this->assertTrue($container->hasDefinition('abstract_listener.foo')); - $definition = $container->getDefinition('abstract_listener.foo'); - $this->assertEquals(array( - 'index_4' => 'foo', - 'index_5' => array( - 'use_forward' => true, - 'failure_path' => '/foo', - ), - 'index_6' => new Reference('foo'), - ), $definition->getArguments()); - - // entry point - $this->assertEquals('entry_point', $entryPointId, '->create() does not change the default entry point.'); - } - - protected function getFactory() - { - return $this->getMockForAbstractClass('Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AbstractFactory', array()); + return array($container, $authProviderId, $listenerId, $entryPointId); } } diff --git a/src/Symfony/Component/Security/CHANGELOG.md b/src/Symfony/Component/Security/CHANGELOG.md index 2389ac3f50..797462b4e8 100644 --- a/src/Symfony/Component/Security/CHANGELOG.md +++ b/src/Symfony/Component/Security/CHANGELOG.md @@ -20,3 +20,6 @@ CHANGELOG * `ObjectIdentity::fromDomainObject`, `UserSecurityIdentity::fromAccount` and `UserSecurityIdentity::fromToken` now return correct identities for proxies objects (e.g. Doctrine proxies) + * [BC BREAK] moved the default authentication success and failure handling to + seperate classes. The order of arguments in the constructor of the + `AbstractAuthenticationListener` has changed. diff --git a/src/Symfony/Component/Security/Http/Authentication/AuthenticationFailureHandlerInterface.php b/src/Symfony/Component/Security/Http/Authentication/AuthenticationFailureHandlerInterface.php index 5bb00deec7..8dbd29a7f6 100644 --- a/src/Symfony/Component/Security/Http/Authentication/AuthenticationFailureHandlerInterface.php +++ b/src/Symfony/Component/Security/Http/Authentication/AuthenticationFailureHandlerInterface.php @@ -33,7 +33,7 @@ interface AuthenticationFailureHandlerInterface * @param Request $request * @param AuthenticationException $exception * - * @return Response|null the response to return + * @return Response The response to return, never null */ public function onAuthenticationFailure(Request $request, AuthenticationException $exception); } diff --git a/src/Symfony/Component/Security/Http/Authentication/AuthenticationSuccessHandlerInterface.php b/src/Symfony/Component/Security/Http/Authentication/AuthenticationSuccessHandlerInterface.php index bf03cd62ec..5c08e73cce 100644 --- a/src/Symfony/Component/Security/Http/Authentication/AuthenticationSuccessHandlerInterface.php +++ b/src/Symfony/Component/Security/Http/Authentication/AuthenticationSuccessHandlerInterface.php @@ -33,7 +33,7 @@ interface AuthenticationSuccessHandlerInterface * @param Request $request * @param TokenInterface $token * - * @return Response|null the response to return + * @return Response never null */ public function onAuthenticationSuccess(Request $request, TokenInterface $token); } diff --git a/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationFailureHandler.php b/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationFailureHandler.php new file mode 100644 index 0000000000..61d77a8ba8 --- /dev/null +++ b/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationFailureHandler.php @@ -0,0 +1,87 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Authentication; + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\HttpKernelInterface; +use Symfony\Component\HttpKernel\Log\LoggerInterface; +use Symfony\Component\Security\Core\Exception\AuthenticationException; +use Symfony\Component\Security\Core\SecurityContextInterface; +use Symfony\Component\Security\Http\HttpUtils; + +/** + * Class with the default authentication failure handling logic. + * + * Can be optionally be extended from by the developer to alter the behaviour + * while keeping the default behaviour. + * + * @author Fabien Potencier + * @author Johannes M. Schmitt + * @author Alexander + */ +class DefaultAuthenticationFailureHandler implements AuthenticationFailureHandlerInterface +{ + protected $httpKernel; + protected $httpUtils; + protected $logger; + protected $options; + + /** + * Constructor. + * + * @param HttpKernelInterface $httpKernel + * @param HttpUtils $httpUtils + * @param array $options Options for processing a failed authentication attempt. + * @param LoggerInterface $logger Optional logger + */ + public function __construct(HttpKernelInterface $httpKernel, HttpUtils $httpUtils, array $options, LoggerInterface $logger = null) + { + $this->httpKernel = $httpKernel; + $this->httpUtils = $httpUtils; + $this->logger = $logger; + + $this->options = array_merge(array( + 'failure_path' => null, + 'failure_forward' => false, + 'login_path' => '/login', + ), $options); + } + + /** + * {@inheritDoc} + */ + public function onAuthenticationFailure(Request $request, AuthenticationException $exception) + { + if (null === $this->options['failure_path']) { + $this->options['failure_path'] = $this->options['login_path']; + } + + if ($this->options['failure_forward']) { + if (null !== $this->logger) { + $this->logger->debug(sprintf('Forwarding to %s', $this->options['failure_path'])); + } + + $subRequest = $this->httpUtils->createRequest($request, $this->options['failure_path']); + $subRequest->attributes->set(SecurityContextInterface::AUTHENTICATION_ERROR, $exception); + + return $this->httpKernel->handle($subRequest, HttpKernelInterface::SUB_REQUEST); + } + + if (null !== $this->logger) { + $this->logger->debug(sprintf('Redirecting to %s', $this->options['failure_path'])); + } + + $request->getSession()->set(SecurityContextInterface::AUTHENTICATION_ERROR, $exception); + + return $this->httpUtils->createRedirectResponse($request, $this->options['failure_path']); + } +} diff --git a/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationSuccessHandler.php b/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationSuccessHandler.php new file mode 100644 index 0000000000..deb7d4574a --- /dev/null +++ b/src/Symfony/Component/Security/Http/Authentication/DefaultAuthenticationSuccessHandler.php @@ -0,0 +1,93 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Authentication; + +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Security\Http\HttpUtils; + +/** + * Class with the default authentication success handling logic. + * + * Can be optionally be extended from by the developer to alter the behaviour + * while keeping the default behaviour. + * + * @author Fabien Potencier + * @author Johannes M. Schmitt + * @author Alexander + */ +class DefaultAuthenticationSuccessHandler implements AuthenticationSuccessHandlerInterface +{ + protected $httpUtils; + protected $options; + protected $providerKey; + + /** + * Constructor. + * + * @param HttpUtils $httpUtils + * @param string $providerKey + * @param array $options Options for processing a successful authentication attempt. + */ + public function __construct(HttpUtils $httpUtils, $providerKey, array $options) + { + $this->httpUtils = $httpUtils; + $this->providerKey = $providerKey; + + $this->options = array_merge(array( + 'always_use_default_target_path' => false, + 'default_target_path' => '/', + 'login_path' => '/login', + 'target_path_parameter' => '_target_path', + 'use_referer' => false, + ), $options); + } + + /** + * {@inheritDoc} + */ + public function onAuthenticationSuccess(Request $request, TokenInterface $token) + { + return $this->httpUtils->createRedirectResponse($request, $this->determineTargetUrl($request)); + } + + /** + * Builds the target URL according to the defined options. + * + * @param Request $request + * + * @return string + */ + protected function determineTargetUrl(Request $request) + { + if ($this->options['always_use_default_target_path']) { + return $this->options['default_target_path']; + } + + if ($targetUrl = $request->get($this->options['target_path_parameter'], null, true)) { + return $targetUrl; + } + + $session = $request->getSession(); + if ($targetUrl = $session->get('_security.'.$this->providerKey.'.target_path')) { + $session->remove('_security.'.$this->providerKey.'.target_path'); + + return $targetUrl; + } + + if ($this->options['use_referer'] && ($targetUrl = $request->headers->get('Referer')) && $targetUrl !== $request->getUriForPath($this->options['login_path'])) { + return $targetUrl; + } + + return $this->options['default_target_path']; + } +} diff --git a/src/Symfony/Component/Security/Http/Firewall/AbstractAuthenticationListener.php b/src/Symfony/Component/Security/Http/Firewall/AbstractAuthenticationListener.php index c9fa328d26..f752e84321 100644 --- a/src/Symfony/Component/Security/Http/Firewall/AbstractAuthenticationListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/AbstractAuthenticationListener.php @@ -70,14 +70,14 @@ abstract class AbstractAuthenticationListener implements ListenerInterface * @param SessionAuthenticationStrategyInterface $sessionStrategy * @param HttpUtils $httpUtils An HttpUtilsInterface instance * @param string $providerKey + * @param AuthenticationSuccessHandlerInterface $successHandler + * @param AuthenticationFailureHandlerInterface $failureHandler * @param array $options An array of options for the processing of a * successful, or failed authentication attempt - * @param AuthenticationSuccessHandlerInterface $successHandler - * @param AuthenticationFailureHandlerInterface $failureHandler - * @param LoggerInterface $logger A LoggerInterface instance - * @param EventDispatcherInterface $dispatcher An EventDispatcherInterface instance + * @param LoggerInterface $logger A LoggerInterface instance + * @param EventDispatcherInterface $dispatcher An EventDispatcherInterface instance */ - public function __construct(SecurityContextInterface $securityContext, AuthenticationManagerInterface $authenticationManager, SessionAuthenticationStrategyInterface $sessionStrategy, HttpUtils $httpUtils, $providerKey, array $options = array(), AuthenticationSuccessHandlerInterface $successHandler = null, AuthenticationFailureHandlerInterface $failureHandler = null, LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null) + public function __construct(SecurityContextInterface $securityContext, AuthenticationManagerInterface $authenticationManager, SessionAuthenticationStrategyInterface $sessionStrategy, HttpUtils $httpUtils, $providerKey, AuthenticationSuccessHandlerInterface $successHandler, AuthenticationFailureHandlerInterface $failureHandler, array $options = array(), LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null) { if (empty($providerKey)) { throw new \InvalidArgumentException('$providerKey must not be empty.'); @@ -91,13 +91,6 @@ abstract class AbstractAuthenticationListener implements ListenerInterface $this->failureHandler = $failureHandler; $this->options = array_merge(array( 'check_path' => '/login_check', - 'login_path' => '/login', - 'always_use_default_target_path' => false, - 'default_target_path' => '/', - 'target_path_parameter' => '_target_path', - 'use_referer' => false, - 'failure_path' => null, - 'failure_forward' => false, ), $options); $this->logger = $logger; $this->dispatcher = $dispatcher; @@ -191,34 +184,13 @@ abstract class AbstractAuthenticationListener implements ListenerInterface $this->securityContext->setToken(null); - if (null !== $this->failureHandler) { - if (null !== $response = $this->failureHandler->onAuthenticationFailure($request, $failed)) { - return $response; - } + $response = $this->failureHandler->onAuthenticationFailure($request, $failed); + + if (!$response instanceof Response) { + throw new \RuntimeException('Authentication Failure Handler did not return a Response.'); } - if (null === $this->options['failure_path']) { - $this->options['failure_path'] = $this->options['login_path']; - } - - if ($this->options['failure_forward']) { - if (null !== $this->logger) { - $this->logger->debug(sprintf('Forwarding to %s', $this->options['failure_path'])); - } - - $subRequest = $this->httpUtils->createRequest($request, $this->options['failure_path']); - $subRequest->attributes->set(SecurityContextInterface::AUTHENTICATION_ERROR, $failed); - - return $event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST); - } - - if (null !== $this->logger) { - $this->logger->debug(sprintf('Redirecting to %s', $this->options['failure_path'])); - } - - $request->getSession()->set(SecurityContextInterface::AUTHENTICATION_ERROR, $failed); - - return $this->httpUtils->createRedirectResponse($request, $this->options['failure_path']); + return $response; } private function onSuccess(GetResponseEvent $event, Request $request, TokenInterface $token) @@ -238,12 +210,10 @@ abstract class AbstractAuthenticationListener implements ListenerInterface $this->dispatcher->dispatch(SecurityEvents::INTERACTIVE_LOGIN, $loginEvent); } - $response = null; - if (null !== $this->successHandler) { - $response = $this->successHandler->onAuthenticationSuccess($request, $token); - } - if (null === $response) { - $response = $this->httpUtils->createRedirectResponse($request, $this->determineTargetUrl($request)); + $response = $this->successHandler->onAuthenticationSuccess($request, $token); + + if (!$response instanceof Response) { + throw new \RuntimeException('Authentication Success Handler did not return a Response.'); } if (null !== $this->rememberMeServices) { @@ -252,35 +222,4 @@ abstract class AbstractAuthenticationListener implements ListenerInterface return $response; } - - /** - * Builds the target URL according to the defined options. - * - * @param Request $request - * - * @return string - */ - private function determineTargetUrl(Request $request) - { - if ($this->options['always_use_default_target_path']) { - return $this->options['default_target_path']; - } - - if ($targetUrl = $request->get($this->options['target_path_parameter'], null, true)) { - return $targetUrl; - } - - $session = $request->getSession(); - if ($targetUrl = $session->get('_security.' . $this->providerKey . '.target_path')) { - $session->remove('_security.' . $this->providerKey . '.target_path'); - - return $targetUrl; - } - - if ($this->options['use_referer'] && ($targetUrl = $request->headers->get('Referer')) && $targetUrl !== $request->getUriForPath($this->options['login_path'])) { - return $targetUrl; - } - - return $this->options['default_target_path']; - } } diff --git a/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php b/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php index 35b0b11c35..22330a8c43 100644 --- a/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php @@ -37,15 +37,15 @@ class UsernamePasswordFormAuthenticationListener extends AbstractAuthenticationL /** * {@inheritdoc} */ - public function __construct(SecurityContextInterface $securityContext, AuthenticationManagerInterface $authenticationManager, SessionAuthenticationStrategyInterface $sessionStrategy, HttpUtils $httpUtils, $providerKey, array $options = array(), AuthenticationSuccessHandlerInterface $successHandler = null, AuthenticationFailureHandlerInterface $failureHandler = null, LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null, CsrfProviderInterface $csrfProvider = null) + public function __construct(SecurityContextInterface $securityContext, AuthenticationManagerInterface $authenticationManager, SessionAuthenticationStrategyInterface $sessionStrategy, HttpUtils $httpUtils, $providerKey, AuthenticationSuccessHandlerInterface $successHandler, AuthenticationFailureHandlerInterface $failureHandler, array $options = array(), LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null, CsrfProviderInterface $csrfProvider = null) { - parent::__construct($securityContext, $authenticationManager, $sessionStrategy, $httpUtils, $providerKey, array_merge(array( + parent::__construct($securityContext, $authenticationManager, $sessionStrategy, $httpUtils, $providerKey, $successHandler, $failureHandler, array_merge(array( 'username_parameter' => '_username', 'password_parameter' => '_password', 'csrf_parameter' => '_csrf_token', 'intention' => 'authenticate', 'post_only' => true, - ), $options), $successHandler, $failureHandler, $logger, $dispatcher); + ), $options), $logger, $dispatcher); $this->csrfProvider = $csrfProvider; }