From 166a3f6ace5ad56473b00cf0cd1c499a654a9bd2 Mon Sep 17 00:00:00 2001
From: catch <catch@35733.no-reply.drupal.org>
Date: Sat, 29 Mar 2014 15:08:21 +0100
Subject: [PATCH] Issue #2160021 by juampy, damiankloip, klausi, vijaycs85:
Basic auth has no login flood control.
---
core/core.services.yml | 4 +-
core/lib/Drupal/Core/Session/AccountProxy.php | 16 +----
.../basic_auth/basic_auth.services.yml | 2 +-
.../Authentication/Provider/BasicAuth.php | 70 +++++++++++++++++--
.../Tests/Authentication/BasicAuthTest.php | 64 ++++++++++++++++-
5 files changed, 132 insertions(+), 24 deletions(-)
diff --git a/core/core.services.yml b/core/core.services.yml
index 51ce4c4ca271..18e37f98ebea 100644
--- a/core/core.services.yml
+++ b/core/core.services.yml
@@ -715,9 +715,7 @@ services:
arguments: ['@authentication']
current_user:
class: Drupal\Core\Session\AccountProxy
- arguments: ['@authentication']
- calls:
- - [setRequest, ['@?request=']]
+ arguments: ['@authentication', '@request']
asset.css.collection_renderer:
class: Drupal\Core\Asset\CssCollectionRenderer
arguments: [ '@state' ]
diff --git a/core/lib/Drupal/Core/Session/AccountProxy.php b/core/lib/Drupal/Core/Session/AccountProxy.php
index c3f10adda895..9f002180bcbc 100644
--- a/core/lib/Drupal/Core/Session/AccountProxy.php
+++ b/core/lib/Drupal/Core/Session/AccountProxy.php
@@ -49,22 +49,12 @@ class AccountProxy implements AccountProxyInterface {
*
* @param \Drupal\Core\Authentication\AuthenticationManagerInterface $authentication_manager
* The authentication manager.
- */
- public function __construct(AuthenticationManagerInterface $authentication_manager) {
- $this->authenticationManager = $authentication_manager;
- }
-
- /**
- * Sets the current request.
- *
* @param \Symfony\Component\HttpFoundation\Request $request
- * The current request.
+ * The request object used for authenticating.
*/
- public function setRequest(Request $request) {
+ public function __construct(AuthenticationManagerInterface $authentication_manager, Request $request) {
+ $this->authenticationManager = $authentication_manager;
$this->request = $request;
- // Reset the current user to ensure that new calls will return the correct
- // user based on the request.
- $this->account = NULL;
}
/**
diff --git a/core/modules/basic_auth/basic_auth.services.yml b/core/modules/basic_auth/basic_auth.services.yml
index 4c7a4493d2a9..a0d436866a66 100644
--- a/core/modules/basic_auth/basic_auth.services.yml
+++ b/core/modules/basic_auth/basic_auth.services.yml
@@ -1,6 +1,6 @@
services:
authentication.basic_auth:
class: Drupal\basic_auth\Authentication\Provider\BasicAuth
- arguments: ['@config.factory', '@user.auth']
+ arguments: ['@config.factory', '@user.auth', '@flood', '@entity.manager']
tags:
- { name: authentication_provider, priority: 100 }
diff --git a/core/modules/basic_auth/lib/Drupal/basic_auth/Authentication/Provider/BasicAuth.php b/core/modules/basic_auth/lib/Drupal/basic_auth/Authentication/Provider/BasicAuth.php
index 579d8a0236f1..660fff16c568 100644
--- a/core/modules/basic_auth/lib/Drupal/basic_auth/Authentication/Provider/BasicAuth.php
+++ b/core/modules/basic_auth/lib/Drupal/basic_auth/Authentication/Provider/BasicAuth.php
@@ -9,8 +9,9 @@
use \Drupal\Component\Utility\String;
use Drupal\Core\Authentication\AuthenticationProviderInterface;
-use Drupal\Core\Config\Config;
use Drupal\Core\Config\ConfigFactoryInterface;
+use Drupal\Core\Entity\EntityManagerInterface;
+use Drupal\Core\Flood\FloodInterface;
use Drupal\user\UserAuthInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
@@ -36,15 +37,37 @@ class BasicAuth implements AuthenticationProviderInterface {
*/
protected $userAuth;
+ /**
+ * The flood service.
+ *
+ * @var \Drupal\Core\Flood\FloodInterface
+ */
+ protected $flood;
+
+ /**
+ * The user storage.
+ *
+ * @var \Drupal\user\UserStorageInterface
+ */
+ protected $userStorage;
+
/**
* Constructs a HTTP basic authentication provider object.
*
* @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
* The config factory.
+ * @param \Drupal\user\UserAuthInterface $user_auth
+ * The user authentication service.
+ * @param \Drupal\Core\Flood\FloodInterface $flood
+ * The flood service.
+ * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
+ * The entity manager service.
*/
- public function __construct(ConfigFactoryInterface $config_factory, UserAuthInterface $user_auth) {
+ public function __construct(ConfigFactoryInterface $config_factory, UserAuthInterface $user_auth, FloodInterface $flood, EntityManagerInterface $entity_manager) {
$this->configFactory = $config_factory;
$this->userAuth = $user_auth;
+ $this->flood = $flood;
+ $this->userStorage = $entity_manager->getStorage('user');
}
/**
@@ -60,12 +83,48 @@ public function applies(Request $request) {
* {@inheritdoc}
*/
public function authenticate(Request $request) {
+ $flood_config = $this->configFactory->get('user.flood');
$username = $request->headers->get('PHP_AUTH_USER');
$password = $request->headers->get('PHP_AUTH_PW');
- $uid = $this->userAuth->authenticate($username, $password);
- if ($uid) {
- return user_load($uid);
+ // Flood protection: this is very similar to the user login form code.
+ // @see \Drupal\user\Form\UserLoginForm::validateAuthentication()
+ // Do not allow any login from the current user's IP if the limit has been
+ // reached. Default is 50 failed attempts allowed in one hour. This is
+ // independent of the per-user limit to catch attempts from one IP to log
+ // in to many different user accounts. We have a reasonably high limit
+ // since there may be only one apparent IP for all users at an institution.
+ if ($this->flood->isAllowed('basic_auth.failed_login_ip', $flood_config->get('ip_limit'), $flood_config->get('ip_window'))) {
+ $accounts = $this->userStorage->loadByProperties(array('name' => $username, 'status' => 1));
+ $account = reset($accounts);
+ if ($account) {
+ if ($flood_config->get('uid_only')) {
+ // Register flood events based on the uid only, so they apply for any
+ // IP address. This is the most secure option.
+ $identifier = $account->id();
+ }
+ else {
+ // The default identifier is a combination of uid and IP address. This
+ // is less secure but more resistant to denial-of-service attacks that
+ // could lock out all users with public user names.
+ $identifier = $account->id() . '-' . $request->getClientIP();
+ }
+ // Don't allow login if the limit for this user has been reached.
+ // Default is to allow 5 failed attempts every 6 hours.
+ if ($this->flood->isAllowed('basic_auth.failed_login_user', $flood_config->get('user_limit'), $flood_config->get('user_window'), $identifier)) {
+ $uid = $this->userAuth->authenticate($username, $password);
+ if ($uid) {
+ $this->flood->clear('basic_auth.failed_login_user', $identifier);
+ return $this->userStorage->load($uid);
+ }
+ else {
+ // Register a per-user failed login event.
+ $this->flood->register('basic_auth.failed_login_user', $flood_config->get('user_window'), $identifier);
+ }
+ }
+ }
}
+ // Always register an IP-based failed login event.
+ $this->flood->register('basic_auth.failed_login_ip', $flood_config->get('ip_window'));
return NULL;
}
@@ -92,4 +151,5 @@ public function handleException(GetResponseForExceptionEvent $event) {
}
return FALSE;
}
+
}
diff --git a/core/modules/basic_auth/lib/Drupal/basic_auth/Tests/Authentication/BasicAuthTest.php b/core/modules/basic_auth/lib/Drupal/basic_auth/Tests/Authentication/BasicAuthTest.php
index b8f580556e1a..134ddb2a7aa4 100644
--- a/core/modules/basic_auth/lib/Drupal/basic_auth/Tests/Authentication/BasicAuthTest.php
+++ b/core/modules/basic_auth/lib/Drupal/basic_auth/Tests/Authentication/BasicAuthTest.php
@@ -7,9 +7,7 @@
namespace Drupal\basic_auth\Tests\Authentication;
-use Drupal\Core\Authentication\Provider\BasicAuth;
use Drupal\simpletest\WebTestBase;
-use Symfony\Component\HttpFoundation\Request;
/**
* Test for http basic authentication.
@@ -61,6 +59,67 @@ public function testBasicAuth() {
$this->curlClose();
}
+ /**
+ * Test the global login flood control.
+ */
+ function testGlobalLoginFloodControl() {
+ \Drupal::config('user.flood')
+ ->set('ip_limit', 2)
+ // Set a high per-user limit out so that it is not relevant in the test.
+ ->set('user_limit', 4000)
+ ->save();
+
+ $user = $this->drupalCreateUser(array());
+ $incorrect_user = clone $user;
+ $incorrect_user->pass_raw .= 'incorrect';
+
+ // Try 2 failed logins.
+ for ($i = 0; $i < 2; $i++) {
+ $this->basicAuthGet('router_test/test11', $incorrect_user->getUsername(), $incorrect_user->pass_raw);
+ }
+
+ // IP limit has reached to its limit. Even valid user credentials will fail.
+ $this->basicAuthGet('router_test/test11', $user->getUsername(), $user->pass_raw);
+ $this->assertResponse('403', 'Access is blocked because of IP based flood prevention.');
+ }
+
+ /**
+ * Test the per-user login flood control.
+ */
+ function testPerUserLoginFloodControl() {
+ \Drupal::config('user.flood')
+ // Set a high global limit out so that it is not relevant in the test.
+ ->set('ip_limit', 4000)
+ ->set('user_limit', 2)
+ ->save();
+
+ $user = $this->drupalCreateUser(array());
+ $incorrect_user = clone $user;
+ $incorrect_user->pass_raw .= 'incorrect';
+ $user2 = $this->drupalCreateUser(array());
+
+ // Try a failed login.
+ $this->basicAuthGet('router_test/test11', $incorrect_user->getUsername(), $incorrect_user->pass_raw);
+
+ // A successful login will reset the per-user flood control count.
+ $this->basicAuthGet('router_test/test11', $user->getUsername(), $user->pass_raw);
+ $this->assertResponse('200', 'Per user flood prevention gets reset on a successful login.');
+
+ // Try 2 failed logins for a user. They will trigger flood control.
+ for ($i = 0; $i < 2; $i++) {
+ $this->basicAuthGet('router_test/test11', $incorrect_user->getUsername(), $incorrect_user->pass_raw);
+ }
+
+ // Now the user account is blocked.
+ $this->basicAuthGet('router_test/test11', $user->getUsername(), $user->pass_raw);
+ $this->assertResponse('403', 'The user account is blocked due to per user flood prevention.');
+
+ // Try one successful attempt for a different user, it should not trigger
+ // any flood control.
+ $this->basicAuthGet('router_test/test11', $user2->getUsername(), $user2->pass_raw);
+ $this->assertResponse('200', 'Per user flood prevention does not block access for other users.');
+ }
+
/**
* Does HTTP basic auth request.
*
@@ -93,4 +152,5 @@ protected function basicAuthGet($path, $username, $password) {
return $out;
}
+
}
--
GitLab