Do not use a CSRF token for the referer ID

contao · Jul 4, 2019 · 8e23cd6 · 8e23cd6
1 parent 6adc8e9
commit 8e23cd6
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 61 deletions.
diff --git a/src/EventListener/RefererIdListener.php b/src/EventListener/RefererIdListener.php
@@ -14,29 +14,28 @@
 
 use Contao\CoreBundle\Routing\ScopeMatcher;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
-use Symfony\Component\Security\Csrf\CsrfToken;
-use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
+use Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface;
 
 class RefererIdListener
 {
     /**
-     * @var CsrfTokenManagerInterface
+     * @var TokenGeneratorInterface
      */
-    private $tokenManager;
+    private $tokenGenerator;
 
     /**
      * @var ScopeMatcher
      */
     private $scopeMatcher;
 
     /**
-     * @var CsrfToken
+     * @var string
      */
     private $token;
 
-    public function __construct(CsrfTokenManagerInterface $tokenManager, ScopeMatcher $scopeMatcher)
+    public function __construct(TokenGeneratorInterface $tokenGenerator, ScopeMatcher $scopeMatcher)
     {
-        $this->tokenManager = $tokenManager;
+        $this->tokenGenerator = $tokenGenerator;
         $this->scopeMatcher = $scopeMatcher;
     }
 
@@ -52,9 +51,9 @@ public function onKernelRequest(GetResponseEvent $event): void
         $request = $event->getRequest();
 
         if (null === $this->token) {
-            $this->token = $this->tokenManager->refreshToken('contao_referer_id');
+            $this->token = $this->tokenGenerator->generateToken();
         }
 
-        $request->attributes->set('_contao_referer_id', $this->token->getValue());
+        $request->attributes->set('_contao_referer_id', $this->token);
     }
 }
diff --git a/src/Resources/config/listener.yml b/src/Resources/config/listener.yml
@@ -133,7 +133,7 @@ services:
     contao.listener.referer_id:
         class: Contao\CoreBundle\EventListener\RefererIdListener
         arguments:
-            - '@contao.referer_id.manager'
+            - '@contao.token_generator'
             - '@contao.routing.scope_matcher'
         tags:
             # The priority must be lower than the one of the Symfony route listener (defaults to 32)

diff --git a/src/Resources/config/services.yml b/src/Resources/config/services.yml
@@ -320,15 +320,6 @@ services:
         tags:
             - { name: contao.picker_provider }
 
-    contao.referer_id.manager:
-        class: Symfony\Component\Security\Csrf\CsrfTokenManager
-        arguments:
-            - '@contao.referer_id.token_generator'
-            - '@security.csrf.token_storage'
-
-    contao.referer_id.token_generator:
-        class: Contao\CoreBundle\Referer\TokenGenerator
-
     contao.repository.remember_me:
         class: Contao\CoreBundle\Repository\RememberMeRepository
         arguments:
@@ -614,6 +605,11 @@ services:
             - '@translator'
         public: true
 
+    contao.token_generator:
+        class: Symfony\Component\Security\Csrf\TokenGenerator\UriSafeTokenGenerator
+        arguments:
+            - 48
+
     contao.translation.translator:
         class: Contao\CoreBundle\Translation\Translator
         decorates: translator

diff --git a/tests/DependencyInjection/ContaoCoreExtensionTest.php b/tests/DependencyInjection/ContaoCoreExtensionTest.php
@@ -74,7 +74,6 @@
 use Contao\CoreBundle\Picker\FilePickerProvider;
 use Contao\CoreBundle\Picker\PagePickerProvider;
 use Contao\CoreBundle\Picker\PickerBuilder;
-use Contao\CoreBundle\Referer\TokenGenerator;
 use Contao\CoreBundle\Repository\RememberMeRepository;
 use Contao\CoreBundle\Routing\Enhancer\InputEnhancer;
 use Contao\CoreBundle\Routing\FrontendLoader;
@@ -127,6 +126,7 @@
 use Symfony\Component\HttpKernel\EventListener\LocaleListener as BaseLocaleListener;
 use Symfony\Component\HttpKernel\EventListener\RouterListener;
 use Symfony\Component\Security\Csrf\CsrfTokenManager;
+use Symfony\Component\Security\Csrf\TokenGenerator\UriSafeTokenGenerator;
 use Symfony\Component\Security\Http\Firewall;
 
 class ContaoCoreExtensionTest extends TestCase
@@ -523,7 +523,7 @@ public function testRegistersTheRefererIdListener(): void
 
         $this->assertSame(RefererIdListener::class, $definition->getClass());
         $this->assertTrue($definition->isPrivate());
-        $this->assertSame('contao.referer_id.manager', (string) $definition->getArgument(0));
+        $this->assertSame('contao.token_generator', (string) $definition->getArgument(0));
         $this->assertSame('contao.routing.scope_matcher', (string) $definition->getArgument(1));
 
         $tags = $definition->getTags();
@@ -1217,28 +1217,6 @@ public function testRegistersTheArticlePickerProvider(): void
         $this->assertArrayHasKey('contao.picker_provider', $tags);
     }
 
-    public function testRegistersTheRefererIdManager(): void
-    {
-        $this->assertTrue($this->container->has('contao.referer_id.manager'));
-
-        $definition = $this->container->getDefinition('contao.referer_id.manager');
-
-        $this->assertSame(CsrfTokenManager::class, $definition->getClass());
-        $this->assertTrue($definition->isPrivate());
-        $this->assertSame('contao.referer_id.token_generator', (string) $definition->getArgument(0));
-        $this->assertSame('security.csrf.token_storage', (string) $definition->getArgument(1));
-    }
-
-    public function testRegistersTheRefererIdTokenGenerator(): void
-    {
-        $this->assertTrue($this->container->has('contao.referer_id.token_generator'));
-
-        $definition = $this->container->getDefinition('contao.referer_id.token_generator');
-
-        $this->assertSame(TokenGenerator::class, $definition->getClass());
-        $this->assertTrue($definition->isPrivate());
-    }
-
     public function testRegistersTheRememberMeRepository(): void
     {
         $this->assertTrue($this->container->has('contao.repository.remember_me'));
@@ -1693,6 +1671,16 @@ public function testRegistersTheSecurityUserChecker(): void
         $this->assertSame('logger', (string) $definition->getArgument(1));
     }
 
+    public function testRegistersTheTokenGenerator(): void
+    {
+        $this->assertTrue($this->container->has('contao.token_generator'));
+
+        $definition = $this->container->getDefinition('contao.token_generator');
+
+        $this->assertSame(UriSafeTokenGenerator::class, $definition->getClass());
+        $this->assertTrue($definition->isPrivate());
+    }
+
     public function testRegistersTheContaoBackendSession(): void
     {
         $this->assertTrue($this->container->has('contao.session.contao_backend'));

diff --git a/tests/EventListener/RefererIdListenerTest.php b/tests/EventListener/RefererIdListenerTest.php
@@ -20,8 +20,7 @@
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 use Symfony\Component\HttpKernel\KernelInterface;
-use Symfony\Component\Security\Csrf\CsrfToken;
-use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
+use Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface;
 
 class RefererIdListenerTest extends TestCase
 {
@@ -33,7 +32,7 @@ public function testAddsTheTokenToTheRequest(): void
         $kernel = $this->createMock(KernelInterface::class);
         $event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
 
-        $listener = new RefererIdListener($this->mockTokenManager(), $this->mockScopeMatcher());
+        $listener = new RefererIdListener($this->mockTokenGenerator(), $this->mockScopeMatcher());
         $listener->onKernelRequest($event);
 
         $this->assertTrue($request->attributes->has('_contao_referer_id'));
@@ -48,7 +47,7 @@ public function testDoesNotAddTheTokenInFrontEndScope(): void
         $kernel = $this->createMock(KernelInterface::class);
         $event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
 
-        $listener = new RefererIdListener($this->mockTokenManager(), $this->mockScopeMatcher());
+        $listener = new RefererIdListener($this->mockTokenGenerator(), $this->mockScopeMatcher());
         $listener->onKernelRequest($event);
 
         $this->assertFalse($request->attributes->has('_contao_referer_id'));
@@ -62,7 +61,7 @@ public function testDoesNotAddTheTokenToASubrequest(): void
         $kernel = $this->createMock(KernelInterface::class);
         $event = new GetResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST);
 
-        $listener = new RefererIdListener($this->mockTokenManager(), $this->mockScopeMatcher());
+        $listener = new RefererIdListener($this->mockTokenGenerator(), $this->mockScopeMatcher());
         $listener->onKernelRequest($event);
 
         $this->assertFalse($request->attributes->has('_contao_referer_id'));
@@ -76,7 +75,7 @@ public function testAddsTheSameTokenToSubsequestRequests(): void
         $kernel = $this->createMock(KernelInterface::class);
         $event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
 
-        $listener = new RefererIdListener($this->mockTokenManager(), $this->mockScopeMatcher());
+        $listener = new RefererIdListener($this->mockTokenGenerator(), $this->mockScopeMatcher());
         $listener->onKernelRequest($event);
 
         $this->assertTrue($request->attributes->has('_contao_referer_id'));
@@ -89,21 +88,16 @@ public function testAddsTheSameTokenToSubsequestRequests(): void
     }
 
     /**
-     * @return CsrfTokenManagerInterface&MockObject
+     * @return TokenGeneratorInterface&MockObject
      */
-    private function mockTokenManager(): CsrfTokenManagerInterface
+    private function mockTokenGenerator(): TokenGeneratorInterface
     {
-        $tokenManager = $this->createMock(CsrfTokenManagerInterface::class);
-        $tokenManager
-            ->method('getToken')
-            ->willReturn(new CsrfToken('_csrf', 'testValue'))
+        $tokenGenerator = $this->createMock(TokenGeneratorInterface::class);
+        $tokenGenerator
+            ->method('generateToken')
+            ->willReturn('testValue')
         ;
 
-        $tokenManager
-            ->method('refreshToken')
-            ->willReturnOnConsecutiveCalls(new CsrfToken('_csrf', 'testValue'), new CsrfToken('_csrf', 'foo'))
-        ;
-
-        return $tokenManager;
+        return $tokenGenerator;
     }
 }