Commit 166a3f6a authored by catch's avatar catch

Issue #2160021 by juampy, damiankloip, klausi, vijaycs85: Basic auth has no login flood control.

parent 0e6c631f
......@@ -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' ]
......
......@@ -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;
}
/**
......
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 }
......@@ -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;
}
}
......@@ -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;
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment