Skip to content

Commit

Permalink
Issue backdrop#227: Check usernames that are email addresses more rig…
Browse files Browse the repository at this point in the history
…idly, only allow if matches email.
  • Loading branch information
Jen Lampton committed Jan 7, 2015
1 parent 547d683 commit 80f2ee4
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 6 deletions.
1 change: 1 addition & 0 deletions core/modules/system/config/system.core.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
"user_admin_role": "",
"user_cancel_method": "user_cancel_block",
"user_email_verification": 1,
"user_email_match": 1,
"user_mail_status_activated_notify": 1,
"user_mail_status_blocked_notify": 0,
"user_mail_status_canceled_notify": 0,
Expand Down
88 changes: 87 additions & 1 deletion core/modules/user/tests/user.test
Original file line number Diff line number Diff line change
Expand Up @@ -1634,6 +1634,9 @@ class UserSignatureTestCase extends BackdropWebTestCase {
* Test that a user, having editing their own account, can still log in.
*/
class UserEditedOwnAccountTestCase extends BackdropWebTestCase {
/**
* Tests a user editing their own account.
*/
function testUserEditedOwnAccount() {
// Change account setting 'Who can register accounts?' to Administrators
// only.
Expand All @@ -1654,6 +1657,29 @@ class UserEditedOwnAccountTestCase extends BackdropWebTestCase {
// Set the new name on the user account and attempt to log back in.
$account->name = $edit['name'];
$this->backdropLogin($account);

// Attempt to change username to an email other than my own.
$edit['name'] = $this->randomName() . '@example.com';
$this->backdropPost('user/' . $account->uid . '/edit', $edit, t('Save'));
$this->assertText(t('An email address was provided as a username, but does not match the account email address.', array('%name' => $edit['name'])), 'Error message found when an email username does not match user email.');
$this->assertNoText(t('The changes have been saved.'), 'The user account was not saved.');

// Lookup user by name to make sure we didn't actually change the name.
$changed = user_load_by_name($edit['name']);
$this->assertFalse($changed, 'Username was not changed to email address other than my own.');

// Change username to my email address.
$edit['name'] = $account->mail;
$this->backdropPost('user/' . $account->uid . '/edit', $edit, t('Save'));
$this->assertText(t('The changes have been saved.'), 'The user account was saved.');

// Test that 'verify_email_match' turned off allows emails that don't match.
config_set('system.core', 'user_email_match', FALSE);

// Change username to random, non-matching email address.
$edit['name'] = $this->randomName() . '@example.com';
$this->backdropPost('user/' . $account->uid . '/edit', $edit, t('Save'));
$this->assertText(t('The changes have been saved.'), 'The user account was saved.');
}
}

Expand Down Expand Up @@ -2151,7 +2177,6 @@ class UserRegistrationTestCase extends UserLoginTestCase {
$this->assertEqual($new_user->init, $mail, 'Correct init field.');
}


/**
* Test that login credentials work.
*/
Expand Down Expand Up @@ -2262,4 +2287,65 @@ class UserRegistrationTestCase extends UserLoginTestCase {
$this->assertEqual($new_user->test_user_field[LANGUAGE_NONE][2]['value'], $value + 2, format_string('@js : The field value was correclty saved.', array('@js' => $js)));
}
}

/**
* Tests new users username matches their email if username is an email.
*/
function testRegistrationEmailAsUsername() {
// Don't require email verification.
// Allow registration by site visitors without administrator approval.
config('system.core')
->set('user_email_verification', FALSE)
->set('user_register', USER_REGISTER_VISITORS)
->save();

$mail = $this->randomName() . '@example.com';
$different = $this->randomName() . $mail;

// Set up edit array.
$edit = array();
$edit['mail'] = $mail;
$edit['name'] = $different;
$edit['pass[pass1]'] = $edit['pass[pass2]'] = $this->randomName();

// Attempt to create an account using an email that doesn't match the name.
$this->backdropPost('user/register', $edit, t('Create new account'));
$this->assertText(t('An email address was provided as a username, but does not match the account email address.'), 'Email username does not match user email - error message found.');
$this->assertNoText(t('Registration successful. You are now logged in.'), 'The user was not created and logged in.');

// Attempt to create new account using matching email address.
$edit['name'] = $edit['mail'] = $this->randomName() . '@example.com';

$this->backdropPost('user/register', $edit, t('Create new account'));
$this->assertText(t('Registration successful. You are now logged in.'), 'The user was created and logged in with matching email.');

$new_user = user_load_by_name($edit['name']);
$this->assertTrue(($new_user->name === $edit['name']) && ($new_user->mail === $edit['mail']), 'Created user with matching username and email address.');
}

/**
* Tests new users username not matching their email if username is an email.
*/
function testRegistrationEmailAsUsernameDisabled() {
// Test that 'user_email_match' turned off allows emails that don't match.
config('system.core')
->set('user_email_match', FALSE)
->set('user_email_verification', FALSE)
->set('user_register', USER_REGISTER_VISITORS)
->save();

$mail = $this->randomName() . '@example.com';
$different = $this->randomName() . $mail;

$edit = array();
$edit['mail'] = $mail;
$edit['name'] = $different;
$edit['pass[pass1]'] = $edit['pass[pass2]'] = $this->randomName();

// Attempt to create an account using an email that doesn't match the name.
// This should be OK, as 'vuser_email_match' is disabled.
$this->backdropPost('user/register', $edit, t('Create new account'));
$this->assertNoText(t('An email address was provided as a username, but does not match the account email address.'), 'Email username does not match user email - error message found.');
$this->assertText(t('Registration successful. You are now logged in.'), 'The user was not created and logged in.');
}
}
6 changes: 6 additions & 0 deletions core/modules/user/user.admin.inc
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ function user_admin_settings($form, &$form_state) {
'#default_value' => $config->get('user_email_verification'),
'#description' => t('New users will be required to validate their e-mail address prior to logging into the site, and will be assigned a system-generated password. With this setting disabled, users will be logged in immediately upon registering, and may select their own passwords during registration.')
);
$form['registration_cancellation']['user_email_match'] = array(
'#type' => 'checkbox',
'#title' => t('When an email address is used as username, require a matching email address.'),
'#default_value' => $config->get('user_email_match'),
);
module_load_include('inc', 'user', 'user.pages');
$form['registration_cancellation']['user_cancel_method'] = array(
'#type' => 'item',
Expand Down Expand Up @@ -539,6 +544,7 @@ function user_admin_settings_submit($form, &$form_state) {
->set('user_login_method', $form_state['values']['user_login_method'])
->set('user_register', $form_state['values']['user_register'])
->set('user_email_verification', $form_state['values']['user_email_verification'])
->set('user_email_match', $form_state['values']['user_email_match'])
->set('user_signatures', $form_state['values']['user_signatures'])
->set('user_mail_status_activated_notify', $form_state['values']['user_mail_status_activated_notify'])
->set('user_mail_status_blocked_notify', $form_state['values']['user_mail_status_blocked_notify'])
Expand Down
8 changes: 8 additions & 0 deletions core/modules/user/user.entity.inc
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,14 @@ class UserStorageController extends EntityDatabaseStorageController {
}
}

if (valid_email_address($entity->name)) {
// Check to see if matching is required.
$user_email_match = config_get('system.core', 'user_email_match');
if ($user_email_match && ($entity->name != $entity->mail)) {
throw new EntityMalformedException('The username and email are both addresses that do not match.');
}
}

if (!empty($entity->picture_upload)) {
$entity->picture = $entity->picture_upload;
}
Expand Down
18 changes: 13 additions & 5 deletions core/modules/user/user.module
Original file line number Diff line number Diff line change
Expand Up @@ -854,9 +854,12 @@ function user_validate_current_pass(&$form, &$form_state) {
*/
function user_account_form_validate($form, &$form_state) {
$account = $form['#user'];
$mail = $form_state['values']['mail'];

// Validate new or changing username.
if (isset($form_state['values']['name'])) {
if ($error = user_validate_name($form_state['values']['name'])) {
$name = $form_state['values']['name'];
if ($error = user_validate_name($name)) {
form_set_error('name', $error);
}
// Cast the user ID as an integer. It might have been set to NULL, which
Expand All @@ -865,19 +868,24 @@ function user_account_form_validate($form, &$form_state) {
$name_taken = (bool) db_select('users')
->fields('users', array('uid'))
->condition('uid', (int) $account->uid, '<>')
->condition('name', db_like($form_state['values']['name']), 'LIKE')
->condition('name', db_like($name), 'LIKE')
->range(0, 1)
->execute()
->fetchField();

if ($name_taken) {
form_set_error('name', t('The name %name is already taken.', array('%name' => $form_state['values']['name'])));
form_set_error('name', t('The name %name is already taken.', array('%name' => $name)));
}
// Check whether the user name provided is an email address. If so, make
// sure it matches the mail value.
if (config('system.core')->get('user_email_match') && (valid_email_address($name))) {
if ($name !== $mail) {
form_set_error('name', t('An email address was provided as a username, but does not match the account email address.'));
}
}
}
}

$mail = $form_state['values']['mail'];

if (!empty($mail)) {
$mail_taken = (bool) db_select('users')
->fields('users', array('uid'))
Expand Down

0 comments on commit 80f2ee4

Please sign in to comment.