Fix EZP-24476: Refactor CSRF token generation in Security login form

> https://jira.ez.no/browse/EZP-24476
ezpublishlegacy · Jun 30, 2015 · e3b4af0 · e3b4af0
1 parent 06c571d
commit e3b4af0
Show file tree

Hide file tree

Showing 8 changed files with 52 additions and 53 deletions.
diff --git a/doc/bc/changes-6.0.md b/doc/bc/changes-6.0.md
@@ -145,6 +145,11 @@ Changes affecting version compatibility with former or future versions.
   Before, it contained the physical path to the file, e.g. `var/site/storage/original/...`. Since this path isn't
   allowed to pass through the rewrite rules for security, it was not usable.
   This also affects REST, that will now expose a valid HTTP download URL.
+
+* `csrf_token` variable is not passed to login template any more. Use `csrf_token()` Twig function to generate it instead.
+  ```jinja
+  <input type="hidden" name="_csrf_token" value="{{ csrf_token("authenticate") }}" />
+  ```
 
 ## Deprecations
 

diff --git a/eZ/Bundle/EzPublishCoreBundle/Resources/config/security.yml b/eZ/Bundle/EzPublishCoreBundle/Resources/config/security.yml
@@ -33,7 +33,7 @@ services:
 
     ezpublish.security.controller:
         class: %ezpublish.security.controller.class%
-        arguments: [@templating, @ezpublish.config.resolver, @?form.csrf_provider]
+        arguments: [@templating, @ezpublish.config.resolver, @security.authentication_utils]
 
     ezpublish.security.login_listener:
         class: %ezpublish.security.login_listener.class%

diff --git a/eZ/Bundle/EzPublishCoreBundle/Resources/views/Security/login.html.twig b/eZ/Bundle/EzPublishCoreBundle/Resources/views/Security/login.html.twig
@@ -22,7 +22,7 @@
                         <input type="password" id="password" class="form-control" name="_password" required="required" placeholder="{{ 'Enter password'|trans }}" />
                     </div>
 
-                    <input type="hidden" name="_csrf_token" value="{{ csrf_token }}" />
+                    <input type="hidden" name="_csrf_token" value="{{ csrf_token("authenticate") }}" />
 
                     {#
                         If you want to control the URL the user

diff --git a/eZ/Bundle/EzPublishRestBundle/EventListener/CsrfListener.php b/eZ/Bundle/EzPublishRestBundle/EventListener/CsrfListener.php
@@ -13,9 +13,10 @@
 use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
-use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface;
 use eZ\Publish\Core\Base\Exceptions\UnauthorizedException;
 use eZ\Bundle\EzPublishRestBundle\RestEvents;
+use Symfony\Component\Security\Csrf\CsrfToken;
+use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
 
 class CsrfListener implements EventSubscriberInterface
 {
@@ -25,9 +26,9 @@ class CsrfListener implements EventSubscriberInterface
     const CSRF_TOKEN_HEADER = "X-CSRF-Token";
 
     /**
-     * @var \Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface
+     * @var null|CsrfTokenManagerInterface
      */
-    private $csrfProvider;
+    private $csrfTokenManager;
 
     /**
      * @var \Symfony\Component\EventDispatcher\EventDispatcherInterface
@@ -51,19 +52,19 @@ class CsrfListener implements EventSubscriberInterface
      * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $eventDispatcher
      * @param bool $csrfEnabled
      * @param string $csrfTokenIntention
-     * @param null|\Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface $csrfProvider
+     * @param null|CsrfTokenManagerInterface $csrfTokenManager
      */
     public function __construct(
         EventDispatcherInterface $eventDispatcher,
         $csrfEnabled,
         $csrfTokenIntention,
-        CsrfProviderInterface $csrfProvider = null
+        CsrfTokenManagerInterface $csrfTokenManager = null
     )
     {
         $this->eventDispatcher = $eventDispatcher;
         $this->csrfEnabled = $csrfEnabled;
         $this->csrfTokenIntention = $csrfTokenIntention;
-        $this->csrfProvider = $csrfProvider;
+        $this->csrfTokenManager = $csrfTokenManager;
     }
 
     /**
@@ -138,8 +139,7 @@ protected function isMethodSafe( $method )
      */
     protected function isLoginRequest( $route )
     {
-        // TODO: add CSRF token to protect against force-login attacks
-        return $route == "ezpublish_rest_createSession";
+        return $route === "ezpublish_rest_createSession";
     }
 
     /**
@@ -154,9 +154,11 @@ protected function checkCsrfToken( Request $request )
             return false;
         }
 
-        return $this->csrfProvider->isCsrfTokenValid(
-            $this->csrfTokenIntention,
-            $request->headers->get( self::CSRF_TOKEN_HEADER )
+        return $this->csrfTokenManager->isTokenValid(
+            new CsrfToken(
+                $this->csrfTokenIntention,
+                $request->headers->get( self::CSRF_TOKEN_HEADER )
+            )
         );
     }
 }
diff --git a/eZ/Bundle/EzPublishRestBundle/Resources/config/services.yml b/eZ/Bundle/EzPublishRestBundle/Resources/config/services.yml
@@ -230,7 +230,7 @@ services:
             - @event_dispatcher
             - %form.type_extension.csrf.enabled%
             - %ezpublish_rest.csrf_token_intention%
-            - @?form.csrf_provider
+            - @?security.csrf.token_manager
         tags:
             - { name: kernel.event_subscriber }
 

diff --git a/eZ/Bundle/EzPublishRestBundle/Tests/EventListener/CsrfListenerTest.php b/eZ/Bundle/EzPublishRestBundle/Tests/EventListener/CsrfListenerTest.php
@@ -21,6 +21,7 @@
 use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface;
 use eZ\Bundle\EzPublishRestBundle\EventListener\CsrfListener;
 use Symfony\Component\HttpKernel\KernelEvents;
+use Symfony\Component\Security\Csrf\CsrfToken;
 
 class CsrfListenerTest extends EventListenerTest
 {
@@ -162,19 +163,23 @@ public function testValidToken()
      */
     protected function getCsrfProviderMock()
     {
-        $provider = $this->getMock(
-            'Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface'
-        );
+        $provider = $this->getMock( '\Symfony\Component\Security\Csrf\CsrfTokenManagerInterface' );
         $provider->expects( $this->any() )
-            ->method( 'isCsrfTokenValid' )
+            ->method( 'isTokenValid' )
             ->will(
-                $this->returnValueMap(
-                    array(
-                        array( self::INTENTION, self::VALID_TOKEN, true ),
-                        array( self::INTENTION, self::INVALID_TOKEN, false )
-                    )
+                $this->returnCallback(
+                    function ( CsrfToken $token )
+                    {
+                        if ( $token == new CsrfToken( self::INTENTION, self::VALID_TOKEN ) )
+                        {
+                            return true;
+                        }
+
+                        return false;
+                    }
                 )
             );
+
         return $provider;
     }
 

diff --git a/eZ/Publish/Core/MVC/Symfony/Controller/SecurityController.php b/eZ/Publish/Core/MVC/Symfony/Controller/SecurityController.php
@@ -10,10 +10,8 @@
 namespace eZ\Publish\Core\MVC\Symfony\Controller;
 
 use eZ\Publish\Core\MVC\ConfigResolverInterface;
-use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface;
-use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
-use Symfony\Component\Security\Core\Security;
+use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
 use Symfony\Component\Templating\EngineInterface;
 
 class SecurityController
@@ -29,39 +27,25 @@ class SecurityController
     protected $configResolver;
 
     /**
-     * @var \Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface|null
+     * @var \Symfony\Component\Security\Http\Authentication\AuthenticationUtils
      */
-    protected $csrfProvider;
+    protected $authenticationUtils;
 
-    public function __construct( EngineInterface $templateEngine, ConfigResolverInterface $configResolver, CsrfProviderInterface $csrfProvider = null )
+    public function __construct( EngineInterface $templateEngine, ConfigResolverInterface $configResolver, AuthenticationUtils $authenticationUtils )
     {
         $this->templateEngine = $templateEngine;
         $this->configResolver = $configResolver;
-        $this->csrfProvider = $csrfProvider;
+        $this->authenticationUtils = $authenticationUtils;
     }
 
-    public function loginAction( Request $request )
+    public function loginAction()
     {
-        $session = $request->getSession();
-
-        if ( $request->attributes->has( Security::AUTHENTICATION_ERROR ) )
-        {
-            $error = $request->attributes->get( Security::AUTHENTICATION_ERROR );
-        }
-        else
-        {
-            $error = $session->get( Security::AUTHENTICATION_ERROR );
-            $session->remove( Security::AUTHENTICATION_ERROR );
-        }
-
-        $csrfToken = isset( $this->csrfProvider ) ? $this->csrfProvider->generateCsrfToken( 'authenticate' ) : null;
         return new Response(
             $this->templateEngine->render(
                 $this->configResolver->getParameter( 'security.login_template' ),
                 array(
-                    'last_username' => $session->get( Security::LAST_USERNAME ),
-                    'error' => $error,
-                    'csrf_token' => $csrfToken,
+                    'last_username' => $this->authenticationUtils->getLastUsername(),
+                    'error' => $this->authenticationUtils->getLastAuthenticationError(),
                     'layout' => $this->configResolver->getParameter( 'security.base_layout' ),
                 )
             )

diff --git a/eZ/Publish/Core/REST/Server/Controller/User.php b/eZ/Publish/Core/REST/Server/Controller/User.php
@@ -36,6 +36,7 @@
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
+use Symfony\Component\Security\Csrf\CsrfToken;
 
 /**
  * User controller
@@ -995,17 +996,19 @@ public function createSession( Request $request )
         try
         {
             $csrfToken = '';
-            $csrfProvider = $this->container->get( 'form.csrf_provider', ContainerInterface::NULL_ON_INVALID_REFERENCE );
+            $csrfTokenManager = $this->container->get( 'security.csrf.token_manager', ContainerInterface::NULL_ON_INVALID_REFERENCE );
             $session = $request->getSession();
             if ( $session->isStarted() )
             {
-                if ( $csrfProvider )
+                if ( $csrfTokenManager )
                 {
                     $csrfToken = $request->headers->get( 'X-CSRF-Token' );
                     if (
-                        !$csrfProvider->isCsrfTokenValid(
-                            $this->container->getParameter( 'ezpublish_rest.csrf_token_intention' ),
-                            $csrfToken
+                        !$csrfTokenManager->isTokenValid(
+                            new CsrfToken(
+                                $this->container->getParameter( 'ezpublish_rest.csrf_token_intention' ),
+                                $csrfToken
+                            )
                         )
                     )
                     {
@@ -1020,7 +1023,7 @@ public function createSession( Request $request )
             // This will seamlessly start the session.
             if ( !$csrfToken )
             {
-                $csrfToken = $csrfProvider->generateCsrfToken(
+                $csrfToken = $csrfTokenManager->generateCsrfToken(
                     $this->container->getParameter( 'ezpublish_rest.csrf_token_intention' )
                 );
             }