Username is validated against email access rules (and vice versa) for new password requests

logintoboggan has nothing to do with the request new password feature.

Confirmed. We have tight access rules on e-mail adresses on our site and when onetry ies to get a new password by entering the username this is refused because the username is checked against the e-mail rules.

This bug has been introduced in version 1.745.2.35 (tagged since DRUPAL-5-11) of /drupal/modules/user/user.module.
In user_pass_validate():

  // Blocked accounts cannot request a new password,
  // check provided username and email against access rules.
  if (drupal_is_denied('user', $name) || drupal_is_denied('mail', $name)) {
    form_set_error('name', t('%name is not allowed to request a new password.', array('%name' => $name)));
  }

There is a logical error in the if condition.

Please feel free to submit a patch. You can get the revision history of user.module at http://cvs.drupal.org/viewvc.py/drupal/drupal/modules/user/user.module?v...

Regards,
hajo

New code is

  $is_denied = FALSE;

  if ($account = user_load(array('mail' => $name, 'status' => 1))) {
    if (drupal_is_denied('mail', $name)) {
      $is_denied = TRUE;
    }
  }
  elseif ($account = user_load(array('name' => $name, 'status' => 1))) {
    if (drupal_is_denied('user', $name)) {
      $is_denied = TRUE;
    }
  }

  if ($is_denied) {
  	form_set_error('name', t('%name is not allowed to request a new password.', array('%name' => $name)));
  }

This way drupal_is_denied is only called with parameters matching the way the account was loaded.

The bug is present in Drupal 5 and 6 in the latest version. In Drupal 5 it is in file user.module, in Drupal 6 it is in user.pages.inc. The bug is not present in Drupal 7.

D7 is still missing parts of SA-2008-060. Bumping to 7 so that we can fix that too.

The correct logic should be:

  // Try to load by email.
  $account = user_load(array('mail' => $name, 'status' => 1));
  if (!$account) {
    // No success, try to load by name.
    $account = user_load(array('name' => $name, 'status' => 1));
  }
  if ($account) {
    // Blocked accounts cannot request a new password,
    // check provided username and email against access rules.
    if (drupal_is_denied('user', $account->name) || drupal_is_denied('mail', $account->mail)) {
      form_set_error('name', t('%name is not allowed to request a new password.', array('%name' => $name)));
    }
  }
  if (isset($account->uid)) {
    form_set_value(array('#parents' => array('account')), $account, $form_state);
  }
  else {
    form_set_error('name', t('Sorry, %name is not recognized as a user name or an e-mail address.', array('%name' => $name)));
  }

We have a problem with this in few different live 6.x sites. IF SA-2008-060 is not an issue in 6.x any longer is there any chance this patch would get included in 6.12?

If D7 is still missing parts of SA-2008-060 then this is critical for 7.x...

Any updates?

Where can one define access rules in D7? (admin/user/rules/list doesn't work for me)

It doesn't exist in D7. This probably needs to move to the not-yet-functioning-needs-another-co-maintainer replacement http://drupal.org/project/user_restrictions

Because the access rules functionality was taken out of D7, I submit a patch against D6, namely version 1.11.2.2 of modules/user/user.pages.inc.
Wrong file: Please see file from comment #12.

Wrong patch-file in comment #11

The patch in #12 is a good minimal patch to fix the bug

It just moves the test to after the account user_load is performed and then ensures that the username is validated against the username rules and the email is validated against the email rules

I've tested the patch and it works for me.

thanks hajo

Patch at #11 is basically the same as at duplicate issue #386138-6: Can't request new password using email address.

However the one at #11 is not in the correct format - diff needs to be run from the Drupal root, not the user module's folder. Also one of the blank lines next to the removed block of code should also be removed.

patch from drupal root attached

This should be committed to next release, very annoying indeed.

Patch works flawlessly on 6.16 BTW. Tx.

worked well, code seems logical

This patch fixed this issue on my site as well. Thank you seanburlington.

credit where it's due - the patch belongs to hajo - I just reviewed it and tweaked the format

It'd be good to get it into the next release :-)

Actually, the code is written by Damien Tournoud since my first proposal wasn't correct. As you wrote: credit where it's due ;-)

Thank you Damien!

Great, committed, thanks.

the patch is not correct.

users who are BLOCKED are excluded from the search due to restriction in the rule:
$account = user_load(array('name' => $name, 'status' => 1));

status-> 1

the correct search should be:
$account = user_load(array('name' => $name));

That's the way it is meant to be, isn't it? See the comment "// Blocked accounts cannot request a new password" in Damiens code (Comment No. 5).

Post 5 solved the issue for me and seems to be secure. Will this be put into the source for version 6?

It is, at least in version 6.20.