wp-cli / entity-command

Manage WordPress comments, menus, options, posts, sites, terms, and users.
MIT License
100 stars 89 forks source link

The "wp user reset-password" command doesn't send the email with the password reset link #249

Closed amieiro closed 5 years ago

amieiro commented 5 years ago

I am checking the "wp user reset-password" command. The official description of the command is "Resets the password for one or more users".

First I check the users in my site:

$ wp user list 
+----+---------------------+---------------------+-------------------+---------------------+---------------+
| ID | user_login          | display_name        | user_email        | user_registered     | roles         |
+----+---------------------+---------------------+-------------------+---------------------+---------------+
| 1  | mysite1_manager     | mysite1_manager     | myemail@gmail.com | 2018-08-21 10:51:51 | administrator |
+----+---------------------+---------------------+-------------------+---------------------+---------------+

Then I try to reset the password for the user "mysite1_manager":

$ wp user reset-password mysite1_manager 

Reset password for mysite1_manager.
Success: Password reset.

Then I received an email with this text:

[My site 1] Notice of Password Change

Hi mysite1_manager,

This notice confirms that your password was changed on My site 1.

If you did not change your password, please contact the Site Administrator at
myemail@gmail.com

This email has been sent to myemail@gmail.com

Regards,
All at My site 1
https://www.mysite1.com

The password has been reset (I can't log in into my site), but I don't get a new password as the output of the command and I don't get a link in the email with a password reset link.

This site is configured in English. I have made the same test in another site configured in Spanish and I have received an email in Spanish, so I suppose that the WP-CLI command is executing some wrong WordPress core function that should send the reset link but instead it is changing the password.

[Mi sitio 2] Aviso de cambio de contraseña

Hola misitio2_manager,

Este aviso confirma que tu contraseña ha cambiado en Mi sitio 2.

Si no has cambiado la contraseña, contacta con el administrador del sitio en
myemail@gmail.com

Este correo ha sido enviado a myemail@gmail.com

Saludos,
El equipo de Mi sitio 2
https://www.misitio2.com

If the command works fine I expect:

My "wp cli info" is:

$ wp cli info
OS:     Linux 4.15.0-38-generic #41-Ubuntu SMP Wed Oct 10 10:59:38 UTC 2018 x86_64
Shell:  /bin/bash
PHP binary:     /usr/bin/php7.2
PHP version:    7.2.11-4+ubuntu18.04.1+deb.sury.org+1
php.ini used:   /etc/php/7.2/cli/php.ini
WP-CLI root dir:        phar://wp-cli.phar/vendor/wp-cli/wp-cli
WP-CLI vendor dir:      phar://wp-cli.phar/vendor
WP_CLI phar path:       /home/vagrant/Code/web/extension-command
WP-CLI packages dir:    /home/vagrant/.wp-cli/packages/
WP-CLI global config:
WP-CLI project config:  /home/vagrant/Code/web/extension-command/wp-cli.yml
WP-CLI version: 2.1.0
amieiro commented 5 years ago

I have updated the password of the user with the command

$ wp user update mysite1_manager --user_pass="my_new_password" 

and I have received the same email that when I reset the password with the command

$ wp user reset-password mysite1_manager 

so I think the reset password command could be using an incorrect WordPress core function.

amieiro commented 5 years ago

I think the problem is in the User_Command class (file https://github.com/wp-cli/entity-command/blob/master/src/User_Command.php)

The function

    /**
     * Resets the password for one or more users.
     *
     * ## OPTIONS
     *
     * <user>...
     * : one or more user logins or IDs.
     *
     * [--skip-email]
     * : Don't send an email notification to the affected user(s).
     *
     * ## EXAMPLES
     *
     *     # Reset the password for two users and send them the change email.
     *     $ wp user reset-password admin editor
     *     Reset password for admin.
     *     Reset password for editor.
     *     Success: Passwords reset.
     *
     * @subcommand reset-password
     */
    public function reset_password( $args, $assoc_args ) {
        $skip_email = Utils\get_flag_value( $assoc_args, 'skip-email' );
        if ( $skip_email ) {
            add_filter( 'send_password_change_email', '__return_false' );
        }
        $fetcher = new \WP_CLI\Fetchers\User;
        $users = $fetcher->get_many( $args );
        foreach( $users as $user ) {
            wp_update_user( array( 'ID' => $user->ID, 'user_pass' => wp_generate_password() ) );
            WP_CLI::log( "Reset password for {$user->user_login}." );
        }
        if ( $skip_email ) {
            remove_filter( 'send_password_change_email', '__return_false' );
        }
        WP_CLI::success( count( $users ) > 1 ? 'Passwords reset.' : 'Password reset.' );
    }

uses the wp_update_user() WordPress core function (https://codex.wordpress.org/Function_Reference/wp_update_user). This function updates the "user_pass" of each user.

I have checked the Codex (https://codex.wordpress.org/Function_Reference) and there is not any core function to send the reset email so I think that the correct function should be to use retrieve_password(), included in the wp-login.php file https://developer.wordpress.org/reference/files/wp-login.php/, that "Handles sending password retrieval email to user." https://developer.wordpress.org/reference/functions/retrieve_password/

I think the correct code could be something like this (I have not tested it, I have written it in a text editor).

    /**
     * Send the password reset email for one or more users.
     *
     * ## OPTIONS
     *
     * <user>...
     * : one or more user logins or IDs.
     *
     * ## EXAMPLES
     *
     *     # Send the password reset email for two users.
     *     $ wp user reset-password admin editor
     *     Reset password for admin.
     *     Reset password for editor.
     *     Success: Passwords reset.
     *
     * @subcommand reset-password
     */
    public function reset_password( $args ) {
        $fetcher = new \WP_CLI\Fetchers\User;
        $users = $fetcher->get_many( $args );
        $error_counter = 0;
        foreach( $users as $user ) {
            $_POST['user_login'] = $user->user_login;
            $error = retrieve_password();
            if ( is_wp_error( $error ) ) {
                WP_CLI::log( "Error sending the password reset email for {$user->user_login}: " . $error->get_error_message() );    
                $error_counter++;
            } else
            {
                WP_CLI::log( "Sent the password reset email for {$user->user_login}." );
            }
        }
        if (count( $users ) > $error_counter ) {
            WP_CLI::success( ( count( $users ) > ( $error_counter + 1 ) ) ? 'Reset emails sent.' : 'Reset email sent.' );
        }
        if ( $error_counter > 0) {
            $message = $error_counter . ( count( $users ) > 1 ? ' errors sending the password reset emails.' : ' error sending the password reset email.' );
            WP_CLI::error($message);
        }
    }
schlessera commented 5 years ago

Yes, that looks like a good approach.

However, it would be worthwhile to investigate first why and when it broke in the first place, to assert the assumption this was actually meant to send the "reset password" email. Because otherwise, we might be making a breaking change, when people use this function in their scripts to just receive a fresh random password directly from WP-CLI (which might have been how it worked).

amieiro commented 5 years ago

Yes, I think that the best would be investigating how people are using this command.

The command is not working, because it is changing the password and the user never gets the new password: it doesn't receive an email with the password nor the person that execute the command in the CLI receives the new password.

The command was added on Dec 17, 2017, by Daniel Bachhuber (@danielbachhuber) on this commit (v2.0.2) https://github.com/wp-cli/entity-command/commit/6db1bb7e74d61ad382bb6b05965f8c50ac4e9597 , so I think we should ask it to Daniel.

The usual "reset password" functionality in the web systems like wordpress.com or other SaaS is that it sends an email with a specially formed link that enables to the user to change his password (See https://en.support.wordpress.com/passwords/ or https://wordpress.com/wp-login.php?action=lostpassword).

danielbachhuber commented 5 years ago

The command isn't broken currently; the command was originally intended to "roll", or reset, the user's password (e.g. to handle a password breach), not send the user a new password.

I don't think it should be WP-CLI's responsibility to send the user a new password. WordPress has a well-defined model of presenting an <input> so the user can enter their own password.

schlessera commented 5 years ago

Alright, that makes sense, and I already suspected we might be missing something here.

I still think it might be useful to be able to trigger the "recover your password" email through scripting. Maybe we should wrap this into a new command wp user recover-password ?

amieiro commented 5 years ago

Ok, Daniel, it was my fault. Now I understand the original idea of the command.

Alain, do you want that I make a PR with this new probable command "wp user recover-password "? If this issue is finished, I think the best option is close it.

schlessera commented 5 years ago

@amieiro No fault in here, we're all just trying to figure out how best to get from here to there.

Yes, please create a PR for that if you want to work on such a command. I'm closing this issue for now.

LukasFritzeDev commented 5 years ago

@amieiro Has there been any progress for this new command? Can I help somewhere?

LukasFritzeDev commented 5 years ago

The sending of the retrieval email is done in wp-login.php in the function retrieve_password().

Since this file is not included during wp-cli processing (or is it?) we would need to reimplement at least some parts of this here … or touch the core to outsource this in a extra function, but I don’t think changing the core for this cli feature is what we should do. On the other having duplicate code is not a perfect solution either. Any opinion?

timnolte commented 2 years ago

I know this is an old thread but the WP-CLI documentation for reset-password is definitely not clear on the functionality of "rolling" the password in the event of a site breach. I too just tried using this assuming that I would get a password returned via the CLI or at the least a reset password email, which of course I received neither. I'm honestly not even clear on why you would want this command to send the user and email that their password has been changed as it would definitely cause more panic and chaos as the boilerplate email tells people that:

If you did not change your password, please contact the Site Administrator at...

I'm referring to the Documentation here: https://developer.wordpress.org/cli/commands/user/reset-password/

JiveDig commented 1 year ago

+1 on a way to bulk send the password reset email.

danielbachhuber commented 1 year ago

+1 on a way to bulk send the password reset email.

@JiveDig Can you share more detail about this? I think @timnolte's comment above is the best reason not to:

I'm honestly not even clear on why you would want this command to send the user and email that their password has been changed as it would definitely cause more panic and chaos as the boilerplate email tells people that:

If you did not change your password, please contact the Site Administrator at...

JiveDig commented 1 year ago

I guess you're right. Ultimately I'm looking for an easy/convenient way to let people know they need to reset their password. This may not be the best way.

  1. Suspicious activity on the site, or preventative measures want to be taken.
  2. CLI to bulk reset pw's (super fast)
  3. Let all users know they need to reset their passwords for security purposes. (slow and no great solution ATM).

It would be great if they could somehow see a notice that their password was reset when they go to log in next, but IDK if that's a WP core issue.

Right now there is confusion either way, without manually notifying users, which may not even be possible.

danielbachhuber commented 1 year ago

Ultimately I'm looking for an easy/convenient way to let people know they need to reset their password.

@JiveDig I'd suggest:

  1. Use WP-CLI to bulk reset user passwords.
  2. Use WP-CLI to generate a list of email addresses to email.
  3. Sending the email through whatever ESP you use for customer conversations.

I'm not sure how much I'd trust the server to send important account information.

JiveDig commented 1 year ago

Makes sense. Thanks @danielbachhuber

monsterbnd commented 1 year ago

In version 2.7.1, it still comes without a link to reset the password for the user. Has anyone correct this?