Error when trying to reset password of the account without email address

Hello @poker10
i have gone through the issue and were able to reproduce it
but i think it would be better if we "add a form validation message, to check existence of email for a valid username" instead of showing the existing generic message.
My solution is to add a validation UserPasswordForm.php

 if (empty($account->getEmail())) {
        $form_state->setErrorByName('name', $this->t('The account does not have a valid email address.'));
        return;
      }

I think it will be a good approach since it is breaking the drupal site instead of any security issues of what we see here
https://www.drupal.org/project/drupal/issues/1521996

Prevent Users Without Email Addresses from Using the Password Reset Form
You can implement a check in the password reset form handler (UserPasswordForm) to ensure that only users with a valid email address can submit the form.
Modify the submitForm() function in a custom module or use a hook, like hook_form_alter() or hook_form_user_pass_alter(), to check for the presence of an email address associated with the username.

function mymodule_form_user_pass_alter(&$form, \Drupal\Core\Form\FormStateInterface $form_state, $form_id) {
    $form['#validate'][] = 'mymodule_user_pass_validate';
}

function mymodule_user_pass_validate($form, \Drupal\Core\Form\FormStateInterface $form_state) {
    $username = $form_state->getValue('name');
    $account = user_load_by_name($username);

    if ($account && !$account->getEmail()) {
        $form_state->setErrorByName('name', t('The account does not have an email address and cannot reset the password.'));
    }
}

@gaurav gupta I was thinking about keeping the UI message the same, even if the username does not have an email associated, exactly because of this information disclosure issue: #1521996: Password reset form reveals whether an email or username is in use → . If we change the error message for this case, it will be possible to use this for username enumeration (especially on sites where email addresses are not required/used). I do not think that is a good idea.

I suggest to keep the behavior the same as now, so everytime the form is submitted, the same message is printed and only add another condition/or change existing conditions to handle the case a username does not have an email address.

@zartab farooquee We cannot block the form submit, since this issue is about anonymous users doing password resets. So we will know if the entered username has email address only after the form is submitted.

The similar issue is here, which is for logged in users: 🐛 Accounts without an email address will display a warning message when resetting their password on the edit page Active

Thanks!

checked the MR - https://git.drupalcode.org/project/drupal/-/merge_requests/10051 and it's worked for me.

Test steps-
- Create a user without email address
- Go to reset passwords page
- submit the form
- observe the error

Test status - Fail
Patch applied successfully.
Issue still persists.
Please refer the screenshots in the attachments.
before-3481894.png
after-3481894.png

MR does not match the proposed solution.

That set I don't think just putting an empty check is correct, where's the backtrace to make sure we aren't hiding a larger issue.

I am not sure what we need to add to the issue summary, but agree that the MR should be updated - at least we need another elseif in the existing condition, so that we do not end up with Password reset form was submitted with an unknown or inactive account: %name. when actually the account exists, it just does not have an email set. So I think we should move the check to a new elseif with a new separate watchdog message, so that is clear, that a form was submitted for an account without email address. The printed message seems to be correct then, and it is as proposed in IS.

Probably we should also add a simple test for this behavior as well? Something like - create user without email, log out and try to send the password for that username. I do not think we are hiding a larger issue here, simply the PhpMail does not expect to get a NULL email (which is a case, if a user account does not have an email set).

Thanks!

Added the tests and provided the suggested changes.
Please review.

Left comments on MR.

I think there is nothing special required for the use-case mentioned in the IS, so it seems like that if we fix this this directly in the _user_mail_notify() (see 🐛 _user_mail_notify() doesn't handle accounts without an email address Active ), no other changes will be needed here.