[Security] Move default authentication success handling strategy to seperate class [Security] Update configuration for changes regarding default success handler [Security] Fix + add AbstractFactory test

4 files changed, 90 insertions, 47 deletions

diff --git a/Http/Authentication/AuthenticationSuccessHandlerInterface.php b/Http/Authentication/AuthenticationSuccessHandlerInterface.php
index 8917f3e..99b1ae6 100644
--- a/Http/Authentication/AuthenticationSuccessHandlerInterface.php
+++ b/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
      */
     function onAuthenticationSuccess(Request $request, TokenInterface $token);
 }
diff --git a/Http/Authentication/DefaultAuthenticationSuccessHandler.php b/Http/Authentication/DefaultAuthenticationSuccessHandler.php
new file mode 100644
index 0000000..5d414c6
--- /dev/null
+++ b/Http/Authentication/DefaultAuthenticationSuccessHandler.php
@@ -0,0 +1,84 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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 Alexander <iam.asm89@gmail.com>
+ */
+class DefaultAuthenticationSuccessHandler implements AuthenticationSuccessHandlerInterface
+{
+    /**
+     * Constructor.
+     *
+     * @param HttpUtils $httpUtils HttpUtils
+     * @param array     $options   Options for processing a successful authentication attempt.
+     */
+    public function __construct(HttpUtils $httpUtils, array $options)
+    {
+        $this->httpUtils = $httpUtils;
+
+        $this->options = array_merge(array(
+            'always_use_default_target_path' => false,
+            'default_target_path'            => '/',
+            '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.target_path')) {
+            $session->remove('_security.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/Http/Firewall/AbstractAuthenticationListener.php b/Http/Firewall/AbstractAuthenticationListener.php
index 1caaf0a..1e27f89 100644
--- a/Http/Firewall/AbstractAuthenticationListener.php
+++ b/Http/Firewall/AbstractAuthenticationListener.php
@@ -77,7 +77,7 @@ abstract class AbstractAuthenticationListener implements ListenerInterface
      * @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, array $options = array(), AuthenticationFailureHandlerInterface $failureHandler = null, LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null)
     {
         if (empty($providerKey)) {
             throw new \InvalidArgumentException('$providerKey must not be empty.');
@@ -92,10 +92,6 @@ abstract class AbstractAuthenticationListener implements ListenerInterface
         $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);
@@ -238,13 +234,7 @@ 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 (null !== $this->rememberMeServices) {
             $this->rememberMeServices->loginSuccess($request, $response, $token);
@@ -252,35 +242,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/Http/Firewall/UsernamePasswordFormAuthenticationListener.php b/Http/Firewall/UsernamePasswordFormAuthenticationListener.php
index 35b0b11..c0fb79e 100644
--- a/Http/Firewall/UsernamePasswordFormAuthenticationListener.php
+++ b/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 = null, array $options = array(), AuthenticationFailureHandlerInterface $failureHandler = null, 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, array_merge(array(
             'username_parameter' => '_username',
             'password_parameter' => '_password',
             'csrf_parameter'     => '_csrf_token',
             'intention'          => 'authenticate',
             'post_only'          => true,
-        ), $options), $successHandler, $failureHandler, $logger, $dispatcher);
+        ), $options), $failureHandler, $logger, $dispatcher);
 
         $this->csrfProvider = $csrfProvider;
     }