dre1080 / warden

More than just a user database auth package for FuelPHP
http://dre1080.github.com/warden
MIT License
46 stars 11 forks source link

Should reset_password_by_token() return exception in case of missing/invalid reset token? #38

Closed PrimozRome closed 11 years ago

PrimozRome commented 11 years ago

Currently function returns NULL if the input reset token does not exist. I am not right on this one, but as I see it I think it would be better that function would throw exception in case of non-existing/invalid reset token. Like for expired_token... Something like this:

public static function reset_password_by_token($reset_password_token, $new_password)
{
    $recoverable = static::find('first', array(
       'where' => array(
           'reset_password_token' => $reset_password_token
        )
    ));

   if ($recoverable) {
      if ($recoverable->is_reset_password_period_valid()) {
         $recoverable->reset_password($new_password);
      } else {
         throw new Failure('expired_token', array('name' => 'Reset password'));
      }
   }
   else {
    throw new Failure('invalid_token', array('name' => 'Reset password'));
   }

   return $recoverable;
}
dre1080 commented 11 years ago

Ahh yes, it should actually be throwing a Failure exception to make it have the same behavior. This should also be the same for confirm_by_token.

PrimozRome commented 11 years ago

Great thanks for the fix.