Add reset password using old password to validate

[dominic-ks]

Originally requested here:

• https://wordpress.org/support/topic/add-ability-to-change-password/

Add a route to allow users to set a new password for their account by providing their current password.

[Spiker1992]

Hi @dominic-ks are you open to using composer? The reason why I am asking is that I have looked at your repo, because I am interested in helping you out with this task, and noticed that you don't have PPUnit tests?

[dominic-ks]

@Spiker1992 hello, yes am totally up for adding composer and PHP Unit, so by all means please do. And in fact, I've opened this to reference that and if you want to jump on it then that'd be grand!

• Add unit tests

[Spiker1992]

Hi @dominic-ks.

I've played around with the code base but quickly realized that there are questions that need to be answered.

1. Would this be a stand-alone endpoint that will require an email, an old password, and a new password, call to which will reset a password?

2. Do you have a preference for what to call this endpoint?

3. If the point (1) is yes, then it might be worth using the existing /set-password endpoint for both and applying a strategy which will decide if to use the existing method or the old password method. This was it's not a breaking change.

4. I have played around with the code a bit and thought that we can probably move validation and other checks out of the controller method to slim them up & to reuse the code.

I thought of something like below for controllers:

add_action('rest_api_init', function () {
  $route_namespace = apply_filters('bdpwr_route_namespace', 'bdpwr/v1');
  register_rest_route($route_namespace, '/reset-password', array(

    'methods' => 'POST',

    'callback' => function ($data) {
      try {
        ResetPasswordAction::handle($data['email']);
      } catch (Exception $e) {
        return WPErrorMessageFactory::handle($e, ErrorMessageRegistry::class);
      }

      return ResponseRepository::handle(200, 'A password reset email has been sent to your email address.');
    },

    'permission_callback' => function () {
      return true;
    },

  ));
});

The ResetPasswordAction class would handle validation and perform an action. This way we can unit/integration test classes themself. Let me know what you think

Below is the rest of the code, if I get a chance I will add a further description of decisions I've made:

A class to handle responses that we have in other routes:

class ResponseRepository
{
  public static function handle(string $status, string $message)
  {
    return array(
      'data' => array(
        'status' => $status,
      ),
      'message' => $message,
    );
  }
}

ErrorMessageRegistry with a list of all the error message that we have

class ErrorMessageRegistry
{
  public const ERROR_MESSAGES = [
    'no_email' => 'You must provide an email address.',
    'bad_email' => 'No user found with this email address.',
  ];
}

A factory class which will return relevant error message from the ErrorMessageRegistry class based on exception

class WPErrorMessageFactory
{
  public static function handle(Exception $e, string $registry)
  {
    $errorMessage = $e->getMessage();
    $messages = $registry::ERROR_MESSAGES;

    $messageExists = isset($messages[$errorMessage]);
    $message = $messageExists ? $messages[$errorMessage] : $errorMessage;
    $code = $messageExists ? $errorMessage : 'bad_request';
    $data =  array('status' => $e->getCode());

    return new WP_Error($code, $message, $data);
  }
}

Actual action class that performs the task of sending a reset code

class ResetPasswordAction
{
  public static function handle(array $data)
  {
    static::validateEmail($data);
    static::sendResetCode($data['email']);
  }

  protected static function validateEmail(array $data)
  {
    $email = $data['email'];

    if (empty($email) || $email === '') {
      throw new InvalidArgumentException('no_email', 400);
    }
  }

  protected static function sendResetCode(string $email)
  {
    $user = static::getUserByEmail($email);
    $user->send_reset_code();
  }

  protected static function getUserByEmail(string $email)
  {
    $userId = email_exists($email);

    if (!$userId) {
      throw new InvalidArgumentException('bad_email', 500);
    }

    return bdpwr_get_user($userId);
  }
}

[dominic-ks]

Hi @Spiker1992,

Thanks very much for looking at this. Yes I'm happy with the approach generally, re: the initial question about using the existing route or creating a new one, I'd probably go for creating a new one just to keep it cleaner and I think something like /update-password would make sense since that's what the user is doing.

If you wanted to get this into a pull request then I can have a look and test it out.

One thing I would request is just a minor one on formatting if we can try and keep to what's there already, e.g.

class Response_Repository { //underscore space class names & braces open on same line
  public static function handle( string $status , string $message ) { //spacing in function & method parameters
    return array(
      'data' => array(
        'status' => $status,
      ),
      'message' => $message,
    );
  }
}

And also array() syntax rather than []

[Spiker1992]

@dominic-ks Awesome. I will look into this sometime tomorrow.

Ok, I am not used to WP's formatting, but I will try to do my best in keeping everything consistent.

[Spiker1992]

@dominic-ks

If you wanted to get this into a pull request then I can have a look and test it out.

I've added a PR, let me know if it works.

I've tried looking for validation methods within WP, to see if there is anything smoother that we could use. This is because I personally don't like having validation within an Action class since that breaks DDD... but I couldn't find anything native. It might be worth investigating the usage of it in WP Plugins. We would need a custom exception unique to this package which set_error_handler will then capture and return WP_Error

I will try looking into unit tests, via your link in the task, and see where I get to.