From: Jeremy Benoist <jeremy.benoist@gmail.com>
Date: Fri, 9 Jun 2017 07:45:43 +0000 (+0200)
Subject: Use a listener to catch auth failure
X-Git-Tag: 2.3.0~31^2~63^2
X-Git-Url: https://git.immae.eu/?p=github%2Fwallabag%2Fwallabag.git;a=commitdiff_plain;h=f81a34e37929a822755d120215d2f18f042ff713

Use a listener to catch auth failure
---

diff --git a/app/config/security.yml b/app/config/security.yml
index 171a69e2..ffb1d356 100644
--- a/app/config/security.yml
+++ b/app/config/security.yml
@@ -41,7 +41,6 @@ security:
             form_login:
                 provider: fos_userbundle
                 csrf_token_generator: security.csrf.token_manager
-                failure_handler: wallabag_user.security.custom_auth_failure_handler
 
             anonymous: true
             remember_me:
diff --git a/src/Wallabag/UserBundle/EventListener/AuthenticationFailureListener.php b/src/Wallabag/UserBundle/EventListener/AuthenticationFailureListener.php
new file mode 100644
index 00000000..10f13233
--- /dev/null
+++ b/src/Wallabag/UserBundle/EventListener/AuthenticationFailureListener.php
@@ -0,0 +1,40 @@
+<?php
+
+namespace Wallabag\UserBundle\EventListener;
+
+use Psr\Log\LoggerInterface;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\Security\Core\AuthenticationEvents;
+
+class AuthenticationFailureListener implements EventSubscriberInterface
+{
+    private $requestStack;
+    private $logger;
+
+    public function __construct(RequestStack $requestStack, LoggerInterface $logger)
+    {
+        $this->requestStack = $requestStack;
+        $this->logger = $logger;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public static function getSubscribedEvents()
+    {
+        return [
+            AuthenticationEvents::AUTHENTICATION_FAILURE => 'onAuthenticationFailure',
+        ];
+    }
+
+    /**
+     * On failure, add a custom error in log so server admin can configure fail2ban to block IP from people who try to login too much.
+     */
+    public function onAuthenticationFailure()
+    {
+        $request = $this->requestStack->getMasterRequest();
+
+        $this->logger->error('Authentication failure for user "'.$request->request->get('_username').'", from IP "'.$request->getClientIp().'", with UA: "'.$request->server->get('HTTP_USER_AGENT').'".');
+    }
+}
diff --git a/src/Wallabag/UserBundle/Resources/config/services.yml b/src/Wallabag/UserBundle/Resources/config/services.yml
index 6ab463e3..f2cd6e01 100644
--- a/src/Wallabag/UserBundle/Resources/config/services.yml
+++ b/src/Wallabag/UserBundle/Resources/config/services.yml
@@ -36,10 +36,10 @@ services:
         tags:
             - { name: kernel.event_subscriber }
 
-    wallabag_user.security.custom_auth_failure_handler:
-        class: Wallabag\UserBundle\Security\CustomAuthenticationFailureHandler
+    wallabag_user.listener.authentication_failure_event_listener:
+        class: Wallabag\UserBundle\EventListener\AuthenticationFailureListener
         arguments:
-            - "@http_kernel"
-            - "@security.http_utils"
-            - {  }
+            - "@request_stack"
             - "@logger"
+        tags:
+            - { name: kernel.event_listener, event: security.authentication.failure, method: onAuthenticationFailure }
diff --git a/src/Wallabag/UserBundle/Security/CustomAuthenticationFailureHandler.php b/src/Wallabag/UserBundle/Security/CustomAuthenticationFailureHandler.php
deleted file mode 100644
index 2d4ea0ea..00000000
--- a/src/Wallabag/UserBundle/Security/CustomAuthenticationFailureHandler.php
+++ /dev/null
@@ -1,62 +0,0 @@
-<?php
-
-namespace Wallabag\UserBundle\Security;
-
-use Symfony\Component\Security\Http\Authentication\DefaultAuthenticationFailureHandler;
-use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\Security\Core\Exception\AuthenticationException;
-use Symfony\Component\Security\Http\ParameterBagUtils;
-use Symfony\Component\HttpKernel\HttpKernelInterface;
-use Symfony\Component\Security\Core\Security;
-
-/**
- * This is a custom authentication failure.
- * It only aims to add a custom error in log so server admin can configure fail2ban to block IP from people who try to login too much.
- *
- * This only changing thing is the logError() addition
- */
-class CustomAuthenticationFailureHandler extends DefaultAuthenticationFailureHandler
-{
-    /**
-     * {@inheritdoc}
-     */
-    public function onAuthenticationFailure(Request $request, AuthenticationException $exception)
-    {
-        if ($failureUrl = ParameterBagUtils::getRequestParameterValue($request, $this->options['failure_path_parameter'])) {
-            $this->options['failure_path'] = $failureUrl;
-        }
-
-        if (null === $this->options['failure_path']) {
-            $this->options['failure_path'] = $this->options['login_path'];
-        }
-
-        if ($this->options['failure_forward']) {
-            $this->logger->debug('Authentication failure, forward triggered.', ['failure_path' => $this->options['failure_path']]);
-
-            $this->logError($request);
-
-            $subRequest = $this->httpUtils->createRequest($request, $this->options['failure_path']);
-            $subRequest->attributes->set(Security::AUTHENTICATION_ERROR, $exception);
-
-            return $this->httpKernel->handle($subRequest, HttpKernelInterface::SUB_REQUEST);
-        }
-
-        $this->logger->debug('Authentication failure, redirect triggered.', ['failure_path' => $this->options['failure_path']]);
-
-        $this->logError($request);
-
-        $request->getSession()->set(Security::AUTHENTICATION_ERROR, $exception);
-
-        return $this->httpUtils->createRedirectResponse($request, $this->options['failure_path']);
-    }
-
-    /**
-     * Log error information about fialure.
-     *
-     * @param Request $request
-     */
-    private function logError(Request $request)
-    {
-        $this->logger->error('Authentication failure for user "'.$request->request->get('_username').'", from IP "'.$request->getClientIp().'", with UA: "'.$request->server->get('HTTP_USER_AGENT').'".');
-    }
-}
diff --git a/tests/Wallabag/UserBundle/EventListener/AuthenticationFailureListenerTest.php b/tests/Wallabag/UserBundle/EventListener/AuthenticationFailureListenerTest.php
new file mode 100644
index 00000000..6191ea13
--- /dev/null
+++ b/tests/Wallabag/UserBundle/EventListener/AuthenticationFailureListenerTest.php
@@ -0,0 +1,66 @@
+<?php
+
+namespace Tests\Wallabag\UserBundle\EventListener;
+
+use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\HttpFoundation\Request;
+use Wallabag\UserBundle\EventListener\AuthenticationFailureListener;
+use Monolog\Logger;
+use Monolog\Handler\TestHandler;
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\Security\Core\AuthenticationEvents;
+use Symfony\Component\Security\Core\Event\AuthenticationFailureEvent;
+
+class AuthenticationFailureListenerTest extends \PHPUnit_Framework_TestCase
+{
+    private $requestStack;
+    private $logHandler;
+    private $listener;
+    private $dispatcher;
+
+    protected function setUp()
+    {
+        $request = Request::create('/');
+        $request->request->set('_username', 'admin');
+
+        $this->requestStack = new RequestStack();
+        $this->requestStack->push($request);
+
+        $this->logHandler = new TestHandler();
+        $logger = new Logger('test', [$this->logHandler]);
+
+        $this->listener = new AuthenticationFailureListener(
+            $this->requestStack,
+            $logger
+        );
+
+        $this->dispatcher = new EventDispatcher();
+        $this->dispatcher->addSubscriber($this->listener);
+    }
+
+    public function testOnAuthenticationFailure()
+    {
+        $token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $exception = $this->getMockBuilder('Symfony\Component\Security\Core\Exception\AuthenticationException')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $event = new AuthenticationFailureEvent(
+            $token,
+            $exception
+        );
+
+        $this->dispatcher->dispatch(
+            AuthenticationEvents::AUTHENTICATION_FAILURE,
+            $event
+        );
+
+        $records = $this->logHandler->getRecords();
+
+        $this->assertCount(1, $records);
+        $this->assertSame('Authentication failure for user "admin", from IP "127.0.0.1", with UA: "Symfony/3.X".', $records[0]['message']);
+    }
+}