Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Custom password checker for database adapter #3371

Closed
wants to merge 2 commits into from

5 participants

@albulescu
$auth = new AuthenticationService();
$authAdapter = new DbTable( $this->services->get('db') );

$authAdapter->setPasswordChecker(function($dbpass, $userpass){
    $bcrypt = new Bcrypt();
    return $bcrypt->verify($userpass, $dbpass);
});

$authAdapter->setTableName("users");
$authAdapter->setIdentityColumn($this->emailAuth ? "email":"username");
$authAdapter->setCredentialColumn("password");
$authAdapter->setIdentity($username);
$authAdapter->setCredential($password);
$auth->setAdapter($authAdapter);
$result = $auth->authenticate();

PS: REMOVE NEGATION FROM CALLBACK CALL :D

albulescu added some commits
@albulescu albulescu Custom password checker for database adapter
$auth = new AuthenticationService();
$authAdapter = new DbTable( $this->services->get('db') );

$authAdapter->setPasswordChecker(function($dbpass, $userpass){
	$bcrypt = new Bcrypt();
	return $bcrypt->verify($userpass, $dbpass);
});

$authAdapter->setTableName("users");
$authAdapter->setIdentityColumn($this->emailAuth ? "email":"username");
$authAdapter->setCredentialColumn("password");
$authAdapter->setIdentity($username);
$authAdapter->setCredential($password);
$auth->setAdapter($authAdapter);
$result = $auth->authenticate();
9026954
@albulescu albulescu Remove negation from if clause 476d6af
@weierophinney

This definitely looks interesting. However:

  • I need tests! :-)
  • Please run your code through php-cs-fixer to fix common CS issues.

Thanks!

@albulescu
@neeckeloo

In my opinion, you could allow to use any PHP callable and not only closures.

@BinaryKitten

@neeckeloo Substitute "could" for "should".

I am also worried about adding unique features to the DbTable Authentication adapter as it increases divergence

Finally the issue i see with this code is that it's still making the credential check in the db as well as in code, i would like to see that negated (no point doing the credential check if the system is going to check it in code)

@youngguns-nl

Finally the issue i see with this code is that it's still making the credential check in the db as well as in code, i would like to see that negated (no point doing the credential check if the system is going to check it in code)

+1 besides that, checking the credentials in the databases is very hard (impossible without the usage of stored procedures) if you use key derivation functions like pbkdf2 to secure your password storage. I vote for fetching the credential value from the database and check it in code by calling a PHP callable.

@BinaryKitten BinaryKitten referenced this pull request from a commit in BinaryKitten/zf2
Kathryn Reeve Adding in Callback for #3371 be3e2c4
@weierophinney weierophinney commented on the diff
library/Zend/Authentication/Adapter/DbTable.php
@@ -459,6 +486,13 @@ protected function _authenticateValidateResultSet(array $resultIdentities)
*/
protected function _authenticateValidateResult($resultIdentity)
{
+ if($this->passwordChecker != null) {
+ $callback = $this->passwordChecker;
+ if ($callback($resultIdentity[$this->credentialColumn], $this->credential)) {
@weierophinney Owner

This should use call_user_func(), as not all Callable items may be called using function call syntax (e.g., static and instance methods).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney

@albulescu While it's a good idea, we ask that contributors follow our contributing guidelines, which includes following coding standards and writing unit tests. If you don't want to do that, I'll close the issue; as it is, it looks like @youngguns-nl and @BinaryKitten are likely working on a robust version of this.

@weierophinney

Closing in favor of #3490.

@youngguns-nl youngguns-nl referenced this pull request from a commit in youngguns-nl/zf2
Kathryn Reeve Adding in Callback for #3371 d7391a8
@ghost Unknown referenced this pull request from a commit
Kathryn Reeve Adding in Callback for #3371 17d545c
@ghost Unknown referenced this pull request from a commit
Kathryn Reeve Adding in Callback for #3371 7af8942
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-authentication
Kathryn Reeve Adding in Callback for zendframework/zf2#3371 fa42950
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-authentication
Kathryn Reeve Adding in Callback for zendframework/zf2#3371 8843414
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 7, 2013
  1. @albulescu

    Custom password checker for database adapter

    albulescu authored
    $auth = new AuthenticationService();
    $authAdapter = new DbTable( $this->services->get('db') );
    
    $authAdapter->setPasswordChecker(function($dbpass, $userpass){
    	$bcrypt = new Bcrypt();
    	return $bcrypt->verify($userpass, $dbpass);
    });
    
    $authAdapter->setTableName("users");
    $authAdapter->setIdentityColumn($this->emailAuth ? "email":"username");
    $authAdapter->setCredentialColumn("password");
    $authAdapter->setIdentity($username);
    $authAdapter->setCredential($password);
    $auth->setAdapter($authAdapter);
    $result = $auth->authenticate();
  2. @albulescu
This page is out of date. Refresh to see the latest.
Showing with 34 additions and 0 deletions.
  1. +34 −0 library/Zend/Authentication/Adapter/DbTable.php
View
34 library/Zend/Authentication/Adapter/DbTable.php
@@ -103,6 +103,12 @@ class DbTable implements AdapterInterface
protected $ambiguityIdentity = false;
/**
+ * Closure for custom password checker
+ * @val Closure
+ */
+ protected $passwordChecker = null;
+
+ /**
* __construct() - Sets configuration options
*
* @param DbAdapter $zendDb
@@ -303,6 +309,27 @@ public function getResultRowObject($returnColumns = null, $omitColumns = null)
return $returnObject;
}
+
+ /**
+ * Set the password checker callback
+ * @param Closure $callback
+ */
+ public function setPasswordChecker( $callback ) {
+
+ if(!is_callable($callback)) {
+ throw new \InvalidArgumentException("Password checker must be callable");
+ }
+
+ $this->passwordChecker = $callback;
+ }
+
+ /**
+ * Get the password cehcker closure
+ */
+ public function getPasswordChecker() {
+ return $this->passwordChecker;
+ }
+
/**
* This method is called to attempt an authentication. Previous to this
* call, this adapter would have already been configured with all
@@ -459,6 +486,13 @@ protected function _authenticateValidateResultSet(array $resultIdentities)
*/
protected function _authenticateValidateResult($resultIdentity)
{
+ if($this->passwordChecker != null) {
+ $callback = $this->passwordChecker;
+ if ($callback($resultIdentity[$this->credentialColumn], $this->credential)) {
@weierophinney Owner

This should use call_user_func(), as not all Callable items may be called using function call syntax (e.g., static and instance methods).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $resultIdentity['zend_auth_credential_match'] = '1';
+ }
+ }
+
if ($resultIdentity['zend_auth_credential_match'] != '1') {
$this->authenticateResultInfo['code'] = AuthenticationResult::FAILURE_CREDENTIAL_INVALID;
$this->authenticateResultInfo['messages'][] = 'Supplied credential is invalid.';
Something went wrong with that request. Please try again.