Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added valid_password method in ValidationRules #227

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/Authentication/AuthenticatorInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@ public function check(): bool;
*/
public function validate(array $credentials, bool $returnUser=false);

/**
* Validates the user password
*
* @param User $user
* @param string $password
*
* @return bool
*/
public function validate_password(User $user, string $password) : bool;

/**
* Returns the User instance for the current logged in user.
*
Expand Down
23 changes: 20 additions & 3 deletions src/Authentication/LocalAuthenticator.php
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,7 @@ public function validate(array $credentials, bool $returnUser=false)
}

// Now, try matching the passwords.
$result = password_verify(base64_encode(
hash('sha384', $password, true)
), $user->password_hash);
$result = $this->validate_password($user, $password);

if (! $result)
{
Expand All @@ -186,4 +184,23 @@ public function validate(array $credentials, bool $returnUser=false)
: true;
}

/**
* Validates the user password
*
* @param User $user
* @param string $password
*
* @return bool
*/
public function validate_password(User $user, string $password) : bool
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically all method names are camel-cased unless used for validation rules.

Suggested change
public function validate_password(User $user, string $password) : bool
public function validatePassword(User $user, string $password) : bool

{
// Can't validate without a password.
if (empty($credentials['password']) || count($credentials) < 2)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a few problems here:

  1. The $credentials array doesn't exist in this method
  2. I prefer to return early instead of wrapping code in an if statement like this.
  3. Even if all of the other stuff was good, the current implementation doesn't always return a boolean like the method signature states.

{
return password_verify(base64_encode(
hash('sha384', $password, true)
), $user->password_hash);
}
}

}
21 changes: 21 additions & 0 deletions src/Authentication/Passwords/ValidationRules.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,27 @@ public function strong_password(string $value, string &$error1 = null, array $da
return $result;
}

/**
* A validation helper method to check if the passed
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* A validation helper method to check if the passed
* A validation helper method to check if the

* current user's password is valid
*
* @param string $password
*
* @return bool
*/
public function valid_password(string $password)
{
helper('auth');
$user = user();

if (empty($user)) {
return false;
}

$authenticate = \Config\Services::authentication();
return $authenticate->validate_password($user, $password);
}

/**
* Builds a new user instance from the global request.
*
Expand Down
5 changes: 5 additions & 0 deletions src/Language/en/Validation.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?php

return [
'valid_password' => 'The {field} is not valid.',
];