From f3584b89a7ee96ac996955b4f53d6be74ef6b579 Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Wed, 15 Oct 2014 11:31:01 -0400
Subject: [PATCH] Fixed bug: ChoiceAuth fails if password confirmation wrong. -
This commit addresses a limitation in the original solution to VUFIND-1011. -
A new validateCredentials() method has been added to differentiate password
checks from login attempts.
---
.../VuFind/src/VuFind/Auth/AbstractBase.php | 29 ++++++++++++++---
module/VuFind/src/VuFind/Auth/ChoiceAuth.php | 32 ++++++++++++++++---
module/VuFind/src/VuFind/Auth/Manager.php | 15 +++++++++
.../Controller/MyResearchController.php | 19 +++++------
4 files changed, 76 insertions(+), 19 deletions(-)
diff --git a/module/VuFind/src/VuFind/Auth/AbstractBase.php b/module/VuFind/src/VuFind/Auth/AbstractBase.php
index 828b9b5cc4e..22471d119dc 100644
--- a/module/VuFind/src/VuFind/Auth/AbstractBase.php
+++ b/module/VuFind/src/VuFind/Auth/AbstractBase.php
@@ -27,7 +27,7 @@
* @link http://www.vufind.org Main Page
*/
namespace VuFind\Auth;
-use VuFind\Exception\Auth as AuthException;
+use VuFind\Db\Row\User, VuFind\Exception\Auth as AuthException;
/**
* Abstract authentication base class
@@ -112,10 +112,31 @@ abstract class AbstractBase implements \VuFind\Db\Table\DbTableAwareInterface
* account credentials.
*
* @throws AuthException
- * @return \VuFind\Db\Row\User Object representing logged-in user.
+ * @return User Object representing logged-in user.
*/
abstract public function authenticate($request);
+ /**
+ * Validate the credentials in the provided request, but do not change the state
+ * of the current logged-in user. Return true for valid credentials, false
+ * otherwise.
+ *
+ * @param \Zend\Http\PhpEnvironment\Request $request Request object containing
+ * account credentials.
+ *
+ * @throws AuthException
+ * @return bool
+ */
+ public function validateCredentials($request)
+ {
+ try {
+ $user = $this->authenticate($request);
+ } catch (AuthException $e) {
+ return false;
+ }
+ return isset($user) && $user instanceof User;
+ }
+
/**
* Has the user's login expired?
*
@@ -134,7 +155,7 @@ abstract class AbstractBase implements \VuFind\Db\Table\DbTableAwareInterface
* new account details.
*
* @throws AuthException
- * @return \VuFind\Db\Row\User New user row.
+ * @return User New user row.
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function create($request)
@@ -151,7 +172,7 @@ abstract class AbstractBase implements \VuFind\Db\Table\DbTableAwareInterface
* new account details.
*
* @throws AuthException
- * @return \VuFind\Db\Row\User New user row.
+ * @return User New user row.
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function updatePassword($request)
diff --git a/module/VuFind/src/VuFind/Auth/ChoiceAuth.php b/module/VuFind/src/VuFind/Auth/ChoiceAuth.php
index da179177f1f..e2517cb45a3 100644
--- a/module/VuFind/src/VuFind/Auth/ChoiceAuth.php
+++ b/module/VuFind/src/VuFind/Auth/ChoiceAuth.php
@@ -26,7 +26,7 @@
* @link http://vufind.org/wiki/vufind2:authentication_handlers Wiki
*/
namespace VuFind\Auth;
-use VuFind\Exception\Auth as AuthException;
+use VuFind\Db\Row\User, VuFind\Exception\Auth as AuthException;
use Zend\Http\PhpEnvironment\Request;
/**
@@ -126,7 +126,7 @@ class ChoiceAuth extends AbstractBase
* @param Request $request Request object containing account credentials.
*
* @throws AuthException
- * @return \VuFind\Db\Row\User Object representing logged-in user.
+ * @return User Object representing logged-in user.
*/
public function authenticate($request)
{
@@ -139,7 +139,7 @@ class ChoiceAuth extends AbstractBase
* @param Request $request Request object containing new account details.
*
* @throws AuthException
- * @return \VuFind\Db\Row\User New user row.
+ * @return User New user row.
*/
public function create($request)
{
@@ -244,7 +244,7 @@ class ChoiceAuth extends AbstractBase
* @param Request $request Request object containing password change details.
*
* @throws AuthException
- * @return \VuFind\Db\Row\User New user row.
+ * @return User New user row.
*/
public function updatePassword($request)
{
@@ -333,4 +333,28 @@ class ChoiceAuth extends AbstractBase
}
}
}
+
+ /**
+ * Validate the credentials in the provided request, but do not change the state
+ * of the current logged-in user. Return true for valid credentials, false
+ * otherwise.
+ *
+ * @param \Zend\Http\PhpEnvironment\Request $request Request object containing
+ * account credentials.
+ *
+ * @throws AuthException
+ * @return bool
+ */
+ public function validateCredentials($request)
+ {
+ try {
+ // In this instance we are checking credentials but do not wish to
+ // change the state of the current object. Thus, we use proxyAuthMethod()
+ // here instead of proxyUserLoad().
+ $user = $this->proxyAuthMethod('authenticate', func_get_args());
+ } catch (AuthException $e) {
+ return false;
+ }
+ return isset($user) && $user instanceof User;
+ }
}
diff --git a/module/VuFind/src/VuFind/Auth/Manager.php b/module/VuFind/src/VuFind/Auth/Manager.php
index 8117f579df9..ad4e7112dd3 100644
--- a/module/VuFind/src/VuFind/Auth/Manager.php
+++ b/module/VuFind/src/VuFind/Auth/Manager.php
@@ -502,4 +502,19 @@ class Manager
);
}
}
+ /**
+ * Validate the credentials in the provided request, but do not change the state
+ * of the current logged-in user. Return true for valid credentials, false
+ * otherwise.
+ *
+ * @param \Zend\Http\PhpEnvironment\Request $request Request object containing
+ * account credentials.
+ *
+ * @throws AuthException
+ * @return bool
+ */
+ public function validateCredentials($request)
+ {
+ return $this->getAuth()->validateCredentials($request);
+ }
}
diff --git a/module/VuFind/src/VuFind/Controller/MyResearchController.php b/module/VuFind/src/VuFind/Controller/MyResearchController.php
index dbf51132e20..f71351de5d2 100644
--- a/module/VuFind/src/VuFind/Controller/MyResearchController.php
+++ b/module/VuFind/src/VuFind/Controller/MyResearchController.php
@@ -1291,18 +1291,15 @@ class MyResearchController extends AbstractBase
// Verify old password if we're logged in
if ($this->getUser()) {
if (isset($post->oldpwd)) {
- try {
- // Reassign oldpwd to password in the request so login works
- $temp_password = $post->password;
- $post->password = $post->oldpwd;
- $this->getAuthManager()->login($request);
- $post->password = $temp_password;
- } catch(AuthException $e) {
- $this->flashMessenger()->setNamespace('error')
- ->addMessage($e->getMessage());
- return $view;
- }
+ // Reassign oldpwd to password in the request so login works
+ $tempPassword = $post->password;
+ $post->password = $post->oldpwd;
+ $valid = $this->getAuthManager()->validateCredentials($request);
+ $post->password = $tempPassword;
} else {
+ $valid = false;
+ }
+ if (!$valid) {
$this->flashMessenger()->setNamespace('error')
->addMessage('authentication_error_invalid');
$view->verifyold = true;
--
GitLab