diff --git includes/common.inc includes/common.inc
index 7758686..9028f37 100644
--- includes/common.inc
+++ includes/common.inc
@@ -1276,39 +1276,70 @@ function valid_url($url, $absolute = FALSE) {
*/
/**
- * Register an event for the current visitor (hostname/IP) to the flood control mechanism.
+ * Register an event for the current visitor to the flood control mechanism.
*
* @param $name
* The name of an event.
+ * @param $identifier
+ * Optional identifier (defaults to the current user's IP address).
*/
-function flood_register_event($name) {
+function flood_register_event($name, $identifier = NULL) {
+ if (!isset($identifier)) {
+ $identifier = ip_address();
+ }
db_insert('flood')
->fields(array(
'event' => $name,
- 'hostname' => ip_address(),
+ 'identifier' => $identifier,
'timestamp' => REQUEST_TIME,
))
->execute();
}
/**
- * Check if the current visitor (hostname/IP) is allowed to proceed with the specified event.
+ * Make the flood control mechanism forget about an event for the current visitor.
+ *
+ * @param $name
+ * The name of an event.
+ * @param $identifier
+ * Optional identifier (defaults to the current user's IP address).
+ */
+function flood_clear_event($name, $identifier = NULL) {
+ if (!isset($identifier)) {
+ $identifier = ip_address();
+ }
+ db_delete('flood')
+ ->condition('event', $name)
+ ->condition('identifier', $identifier)
+ ->execute();
+}
+
+/**
+ * Check if the current visitor is allowed to proceed with the specified event.
*
* The user is allowed to proceed if he did not trigger the specified event more
- * than $threshold times per hour.
+ * than $threshold times in the specified window.
*
* @param $name
* The name of the event.
* @param $threshold
* The maximum number of the specified event per hour (per visitor).
+ * @param $window
+ * Optional number of seconds over which to look for events. Defaults to
+ * 3600 (1 hour).
+ * @param $identifier
+ * Optional identifier (defaults to the current user's IP address).
* @return
* True if the user did not exceed the hourly threshold. False otherwise.
*/
-function flood_is_allowed($name, $threshold) {
- $number = db_query("SELECT COUNT(*) FROM {flood} WHERE event = :event AND hostname = :hostname AND timestamp > :timestamp", array(
+function flood_is_allowed($name, $threshold, $window = 3600, $identifier = NULL) {
+ if (!isset($identifier)) {
+ $identifier = ip_address();
+ }
+ $number = db_query("SELECT COUNT(*) FROM {flood} WHERE event = :event AND identifier = :identifier AND timestamp > :timestamp", array(
':event' => $name,
- ':hostname' => ip_address(),
- ':timestamp' => REQUEST_TIME - 3600))
+ ':identifier' => $identifier,
+ ':timestamp' => REQUEST_TIME - $window))
->fetchField();
return ($number < $threshold);
}
diff --git modules/system/system.install modules/system/system.install
index d9611ab..508257b 100644
--- modules/system/system.install
+++ modules/system/system.install
@@ -805,8 +805,8 @@ function system_schema() {
'not null' => TRUE,
'default' => '',
),
- 'hostname' => array(
- 'description' => 'Hostname of the visitor.',
+ 'identifier' => array(
+ 'description' => 'Identifier of the visitor, such as IP address or hostname.',
'type' => 'varchar',
'length' => 128,
'not null' => TRUE,
@@ -821,7 +821,7 @@ function system_schema() {
),
'primary key' => array('fid'),
'indexes' => array(
- 'allow' => array('event', 'hostname', 'timestamp'),
+ 'allow' => array('event', 'identifier', 'timestamp'),
),
);
@@ -2266,6 +2266,17 @@ function system_update_7030() {
}
/**
+* Alter field hostnme to identifier in the {flood} table.
+ */
+function system_update_7031() {
+ $ret = array();
+ db_change_field($ret, 'flood', 'hostname', 'identifier', array('type' => 'varchar', 'length' => 128, 'not null' => TRUE, 'default' => ''));
+ db_drop_index($ret, 'flood', 'allow');
+ db_add_index($ret, 'flood', 'allow', array('event', 'identifier', 'timestamp'));
+ return $ret;
+}
+
+/**
* @} End of "defgroup updates-6.x-to-7.x"
* The next series of updates should start at 8000.
*/
diff --git modules/user/user.module modules/user/user.module
index 668b508..e9c5a99 100644
--- modules/user/user.module
+++ modules/user/user.module
@@ -1618,39 +1618,92 @@ function user_login_name_validate($form, &$form_state) {
* is set to the matching user ID.
*/
function user_login_authenticate_validate($form, &$form_state) {
- user_authenticate($form_state);
+ $password = trim($form_state['values']['pass']);
+ // Name and pass keys are required.
+ if (!empty($form_state['values']['name']) && !empty($password)) {
+ // Don't allow any login from that IP if the limit has been reached.
+ // Default is 50 failed attempts allowed in one hour.
+ if (!flood_is_allowed('failed_login_attempt_ip', variable_get('user_failed_login_ip_limit', 50), variable_get('user_failed_login_ip_window', 3600))) {
+ $form_state['flood_control_uid'] = 0;
+ return;
+ }
+ $account = db_query("SELECT * FROM {users} WHERE name = :name AND status = 1", array(':name' => $form_state['values']['name']))->fetchObject();
+ if ($account) {
+ $form_state['flood_register_event_uid'] = $account->uid;
+
+ // Don't allow login if the limit for this user has been reached. Specify
+ // the uid as identifier to limit all attempts regardless of origin IP.
+ // Default is to allow 5 failed attempts every 6 hours.
+ if (!flood_is_allowed('failed_login_attempt_uid' , variable_get('user_failed_login_per_user_limit', 5), variable_get('user_failed_login_per_user_window', 21600), $account->uid)) {
+ $form_state['flood_control_uid'] = $account->uid;
+ return;
+ }
+ }
+ // We are not limited by flood control, so try to authenticate.
+ // Set $form_state['uid'] as a flag for user_login_final_validate().
+ $form_state['uid'] = user_authenticate($form_state['values']['name'], $password);
+ }
}
/**
- * A validate handler on the login form. Should be the last validator. Sets an
- * error if user has not been authenticated yet.
+ * A validate handler on the login form. Should be the last validator.
+ *
+ * Sets a form error if user has not been authenticated, or if too many
+ * logins have been attempted.
*/
function user_login_final_validate($form, &$form_state) {
if (empty($form_state['uid'])) {
- form_set_error('name', t('Sorry, unrecognized username or password. Have you forgotten your password?', array('@password' => url('user/password'))));
- watchdog('user', 'Login attempt failed for %user.', array('%user' => $form_state['values']['name']));
+ // Register a global failed login event.
+ flood_register_event('failed_login_attempt_ip');
+
+ if (isset($form_state['flood_register_event_uid'])) {
+ // Register a per-user failed login event. Specify uid as identifier so we
+ // limit all attempts regardless of origin IP.
+ flood_register_event('failed_login_attempt_uid', $form_state['flood_register_event_uid']);
+ }
+
+ if (isset($form_state['flood_control_uid'])) {
+ if ($form_state['flood_control_uid'] == 0) {
+ // We didn't find a uid, so limit is IP-based.
+ form_set_error('name', t('Sorry, too many failed login attempts from your IP address. This IP address is temporarily blocked. Please try again later, or request a new password.', array('@url' => url('user/password'))));
+ }
+ else {
+ form_set_error('name', format_plural(variable_get('user_failed_login_per_user_limit', 5), 'Sorry, there has been more than one failed login attempt for this account. It is temporarily blocked. Please try again later, or request a new password.', 'Sorry, there have been more than @count failed login attempts for this account. It is temporarily blocked. Please try again later, or request a new password.', array('@url' => url('user/password'))));
+ }
+ }
+ else {
+ form_set_error('name', t('Sorry, unrecognized username or password. Have you forgotten your password?', array('@password' => url('user/password'))));
+ watchdog('user', 'Login attempt failed for %user.', array('%user' => $form_state['values']['name']));
+ }
+ }
+ else {
+ // Clear past failures for this uid so as not to block a user who might
+ // log in and out more than once in an hour.
+ flood_clear_event('failed_login_attempt_uid', $form_state['uid']);
}
}
/**
- * Try to log in the user locally. $form_state['uid'] is set to
- * a user ID if successful.
+ * Try to validate the user's login credentials locally.
*
- * @param $form_state
- * Form submission state with at least 'name' and 'pass' keys.
+ * @param $name
+ * User name to authenticate.
+ * @param $password
+ * A plain-text password, such as trimmed text from form values.
+ * @return
+ * The user's uid on success, or FALSE on failure to authenticate.
*/
-function user_authenticate(&$form_state) {
- $password = trim($form_state['values']['pass']);
+function user_authenticate($name, $password) {
+ $uid = FALSE;
// Name and pass keys are required.
- if (!empty($form_state['values']['name']) && !empty($password)) {
- $account = db_query("SELECT * FROM {users} WHERE name = :name AND status = 1", array(':name' => $form_state['values']['name']))->fetchObject();
+ if (!empty($name) && !empty($password)) {
+ $account = db_query("SELECT * FROM {users} WHERE name = :name AND status = 1", array(':name' => $name))->fetchObject();
if ($account) {
// Allow alternate password hashing schemes.
require_once DRUPAL_ROOT . '/' . variable_get('password_inc', 'includes/password.inc');
if (user_check_password($password, $account)) {
-
- // Successful authentication. Set a flag for user_login_final_validate().
- $form_state['uid'] = $account->uid;
+ // Successful authentication..
+ $uid = $account->uid;
// Update user to new password scheme if needed.
if (user_needs_new_hash($account)) {
@@ -1665,6 +1718,7 @@ function user_authenticate(&$form_state) {
}
}
}
+ return $uid;
}
/**
diff --git modules/user/user.test modules/user/user.test
index c05c38e..e020647 100644
--- modules/user/user.test
+++ modules/user/user.test
@@ -159,6 +159,113 @@ class UserValidationTestCase extends DrupalWebTestCase {
}
}
+class UserLoginTestCase extends DrupalWebTestCase {
+ public static function getInfo() {
+ return array(
+ 'name' => 'User login',
+ 'description' => 'Ensure that login works as expected.',
+ 'group' => 'User',
+ );
+ }
+
+ /**
+ * Test the global login flood control.
+ */
+ function testGlobalLoginFloodControl() {
+ // Set the global login limit.
+ variable_set('user_failed_login_ip_limit', 10);
+ // Get the per-user limit out of the picture.
+ variable_set('user_failed_login_per_user_limit', 4000);
+
+ $user1 = $this->drupalCreateUser(array());
+ $incorrect_user1 = clone $user1;
+ $incorrect_user1->pass_raw = $this->randomName();
+
+ // Try 2 failed logins.
+ for ($i = 0; $i < 2; $i++) {
+ $this->assertFailedLogin($incorrect_user1);
+ }
+
+ // Try one successful login, it will not reset overall flood control mechanism.
+ $this->drupalLogin($user1);
+ $this->drupalLogout();
+
+ // Try 8 more failed logins, they should not trigger the flood control
+ // mechanism.
+ for ($i = 0; $i < 8; $i++) {
+ $this->assertFailedLogin($incorrect_user1);
+ }
+
+ // The next login trial should result in an IP-based flood error message.
+ $this->assertFailedLogin($incorrect_user1, 0);
+
+ // A login with the correct password should also result in a flood error
+ // message.
+ $this->assertFailedLogin($user1, 0);
+ }
+
+ /**
+ * Test the per-user login flood control.
+ */
+ function testPerUserLoginFloodControl() {
+ // Get the global limit out of the picture.
+ variable_set('user_failed_login_ip_limit', 4000);
+ // Set the per-user login limit.
+ variable_set('user_failed_login_per_user_limit', 3);
+
+ $user1 = $this->drupalCreateUser(array());
+ $incorrect_user1 = clone $user1;
+ $incorrect_user1->pass_raw = $this->randomName();
+
+ $user2 = $this->drupalCreateUser(array());
+ $incorrect_user2 = clone $user2;
+ $incorrect_user2->pass_raw = $this->randomName();
+
+ // Try 3 failed logins for user 1.
+ for ($i = 0; $i < 3; $i++) {
+ $this->assertFailedLogin($incorrect_user1);
+ }
+
+ // Try one successful attempt for user 2, it should not trigger any
+ // flood control.
+ $this->drupalLogin($user2);
+ $this->drupalLogout();
+
+ // Try one more attempt for user 1, it should be rejected, even if the
+ // correct password has been used.
+ $this->assertFailedLogin($user1, $user1->uid);
+ }
+
+ /**
+ * Make an unsuccessful login attempt.
+ *
+ * @param $user
+ * The user.
+ * @param $flood_uid
+ * Whether to expect or not that the flood control mechanism will be
+ * triggered.
+ */
+ function assertFailedLogin($user, $flood_uid = NULL) {
+ $edit = array(
+ 'name' => $user->name,
+ 'pass' => $user->pass_raw,
+ );
+ $this->drupalPost('user', $edit, t('Log in'));
+ if (isset($flood_uid)) {
+ if ($flood_uid == 0) {
+ // We didn't find a uid, so limit is IP-based.
+ $this->assertRaw(t('Sorry, too many failed login attempts from your IP address. This IP address is temporarily blocked. Please try again later, or request a new password.', array('@url' => url('user/password'))));
+ }
+ else {
+ $this->assertRaw(format_plural(variable_get('user_failed_login_per_user_limit', 5), 'Sorry, there has been more than one failed login attempt for this account. It is temporarily blocked. Please try again later, or request a new password.', 'Sorry, there have been more than @count failed login attempts for this account. It is temporarily blocked. Please try again later, or request a new password.', array('@url' => url('user/password'))));
+ }
+ }
+ else {
+ $this->assertText(t('Sorry, unrecognized username or password. Have you forgotten your password?'));
+ }
+ }
+}
+
class UserCancelTestCase extends DrupalWebTestCase {
public static function getInfo() {
return array(