From 63f9f22fa37b14171c6f92d24f99ccf01ae7af00 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Thu, 8 Jun 2017 22:24:49 +0200 Subject: [PATCH 1/3] Log an error level message when user auth fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user login using the form we know log an error level information with information about the user: - username used - IP - User agent For example: > Authentication failure for user "eza", from IP "127.0.0.1", with UA: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36". It’ll allows server admin using fail2ban to configure it to block these people if they generate too much failure authentication. --- app/config/security.yml | 1 + .../UserBundle/Resources/config/services.yml | 8 +++ .../CustomAuthenticationFailureHandler.php | 62 +++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 src/Wallabag/UserBundle/Security/CustomAuthenticationFailureHandler.php diff --git a/app/config/security.yml b/app/config/security.yml index ffb1d356f..171a69e2e 100644 --- a/app/config/security.yml +++ b/app/config/security.yml @@ -41,6 +41,7 @@ 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/Resources/config/services.yml b/src/Wallabag/UserBundle/Resources/config/services.yml index 72f6f12c1..6ab463e36 100644 --- a/src/Wallabag/UserBundle/Resources/config/services.yml +++ b/src/Wallabag/UserBundle/Resources/config/services.yml @@ -35,3 +35,11 @@ services: - "%wallabag_core.list_mode%" tags: - { name: kernel.event_subscriber } + + wallabag_user.security.custom_auth_failure_handler: + class: Wallabag\UserBundle\Security\CustomAuthenticationFailureHandler + arguments: + - "@http_kernel" + - "@security.http_utils" + - { } + - "@logger" diff --git a/src/Wallabag/UserBundle/Security/CustomAuthenticationFailureHandler.php b/src/Wallabag/UserBundle/Security/CustomAuthenticationFailureHandler.php new file mode 100644 index 000000000..93e2d17b7 --- /dev/null +++ b/src/Wallabag/UserBundle/Security/CustomAuthenticationFailureHandler.php @@ -0,0 +1,62 @@ +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').'".'); + } +} From fa1c9d7cc7f3c4d2f9167a5b62bbc8cd1f9df59b Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Thu, 8 Jun 2017 22:52:26 +0200 Subject: [PATCH 2/3] CS --- .../Security/CustomAuthenticationFailureHandler.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Wallabag/UserBundle/Security/CustomAuthenticationFailureHandler.php b/src/Wallabag/UserBundle/Security/CustomAuthenticationFailureHandler.php index 93e2d17b7..2d4ea0ea7 100644 --- a/src/Wallabag/UserBundle/Security/CustomAuthenticationFailureHandler.php +++ b/src/Wallabag/UserBundle/Security/CustomAuthenticationFailureHandler.php @@ -51,9 +51,9 @@ class CustomAuthenticationFailureHandler extends DefaultAuthenticationFailureHan } /** - * Log error information about fialure + * Log error information about fialure. * - * @param Request $request + * @param Request $request */ private function logError(Request $request) { From f81a34e37929a822755d120215d2f18f042ff713 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Fri, 9 Jun 2017 09:45:43 +0200 Subject: [PATCH 3/3] Use a listener to catch auth failure --- app/config/security.yml | 1 - .../AuthenticationFailureListener.php | 40 +++++++++++ .../UserBundle/Resources/config/services.yml | 10 +-- .../CustomAuthenticationFailureHandler.php | 62 ----------------- .../AuthenticationFailureListenerTest.php | 66 +++++++++++++++++++ 5 files changed, 111 insertions(+), 68 deletions(-) create mode 100644 src/Wallabag/UserBundle/EventListener/AuthenticationFailureListener.php delete mode 100644 src/Wallabag/UserBundle/Security/CustomAuthenticationFailureHandler.php create mode 100644 tests/Wallabag/UserBundle/EventListener/AuthenticationFailureListenerTest.php diff --git a/app/config/security.yml b/app/config/security.yml index 171a69e2e..ffb1d356f 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 000000000..10f132333 --- /dev/null +++ b/src/Wallabag/UserBundle/EventListener/AuthenticationFailureListener.php @@ -0,0 +1,40 @@ +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 6ab463e36..f2cd6e015 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 2d4ea0ea7..000000000 --- a/src/Wallabag/UserBundle/Security/CustomAuthenticationFailureHandler.php +++ /dev/null @@ -1,62 +0,0 @@ -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 000000000..6191ea131 --- /dev/null +++ b/tests/Wallabag/UserBundle/EventListener/AuthenticationFailureListenerTest.php @@ -0,0 +1,66 @@ +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']); + } +}