From d0d90f197ae09fcbded08e203eadc0c7c97a76e6 Mon Sep 17 00:00:00 2001 From: Ere Maijala <ere.maijala@helsinki.fi> Date: Wed, 22 Jun 2016 09:57:00 -0400 Subject: [PATCH] Use stored credentials when checking certain capabilities. - Since the user had to log in with the ILS to get to the point of changing their password when using this auth method, we can safely rely on the database copy of the username rather than going through the expensive process of communicating with the ILS an extra time. - Since we are not refreshing the credentials, there is the (fairly small) possibility that the stored password will be outdated. This should not matter for the task of checking the ILS capability, since it is the username that is needed for identifying the user's home ILS in the MultiBackend driver (and in other contexts, this data is not currently used). - This commit also includes a minor comment improvement and a code simplification. --- module/VuFind/src/VuFind/Auth/ILS.php | 4 +-- .../src/VuFind/Auth/ILSAuthenticator.php | 26 ++++++++++++++++--- .../src/VuFind/ILS/Driver/MultiBackend.php | 2 +- .../ILS/Driver/MultiBackendTest.php | 5 ++++ 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/module/VuFind/src/VuFind/Auth/ILS.php b/module/VuFind/src/VuFind/Auth/ILS.php index 6c90a281d65..cec73bffa18 100644 --- a/module/VuFind/src/VuFind/Auth/ILS.php +++ b/module/VuFind/src/VuFind/Auth/ILS.php @@ -140,7 +140,7 @@ class ILS extends AbstractBase try { return false !== $this->getCatalog()->checkFunction( 'changePassword', - ['patron' => $this->getLoggedInPatron()] + ['patron' => $this->authenticator->getStoredCatalogCredentials()] ); } catch (ILSException $e) { return false; @@ -273,7 +273,7 @@ class ILS extends AbstractBase * * @throws AuthException * - * @return array Patron + * @return array|null Patron or null if no credentials exist */ protected function getLoggedInPatron() { diff --git a/module/VuFind/src/VuFind/Auth/ILSAuthenticator.php b/module/VuFind/src/VuFind/Auth/ILSAuthenticator.php index ef26a071d35..a6109d1cede 100644 --- a/module/VuFind/src/VuFind/Auth/ILSAuthenticator.php +++ b/module/VuFind/src/VuFind/Auth/ILSAuthenticator.php @@ -72,6 +72,28 @@ class ILSAuthenticator $this->catalog = $catalog; } + /** + * Get stored catalog credentials for the current user. + * + * Returns associative array of cat_username and cat_password if they are + * available, false otherwise. This method does not verify that the credentials + * are valid. + * + * @return array|bool + */ + public function getStoredCatalogCredentials() + { + // Fail if no username is found, but allow a missing password (not every ILS + // requires a password to connect). + if (($user = $this->auth->isLoggedIn()) && !empty($user->cat_username)) { + return [ + 'cat_username' => $user->cat_username, + 'cat_password' => $user->cat_password + ]; + } + return false; + } + /** * Log the current user into the catalog using stored credentials; if this * fails, clear the user's stored credentials so they can enter new, corrected @@ -85,9 +107,7 @@ class ILSAuthenticator { // Fail if no username is found, but allow a missing password (not every ILS // requires a password to connect). - if (($user = $this->auth->isLoggedIn()) - && isset($user->cat_username) && !empty($user->cat_username) - ) { + if (($user = $this->auth->isLoggedIn()) && !empty($user->cat_username)) { // Do we have a previously cached ILS account? if (isset($this->ilsAccount[$user->cat_username])) { return $this->ilsAccount[$user->cat_username]; diff --git a/module/VuFind/src/VuFind/ILS/Driver/MultiBackend.php b/module/VuFind/src/VuFind/ILS/Driver/MultiBackend.php index 5dd56520f21..de118570516 100644 --- a/module/VuFind/src/VuFind/ILS/Driver/MultiBackend.php +++ b/module/VuFind/src/VuFind/ILS/Driver/MultiBackend.php @@ -1178,7 +1178,7 @@ class MultiBackend extends AbstractBase } if (!$source) { try { - $patron = $this->ilsAuth->storedCatalogLogin(); + $patron = $this->ilsAuth->getStoredCatalogCredentials(); if ($patron && isset($patron['cat_username'])) { $source = $this->getSource($patron['cat_username']); } diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/ILS/Driver/MultiBackendTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/ILS/Driver/MultiBackendTest.php index ff03cb55c52..d76649dc19a 100644 --- a/module/VuFind/tests/unit-tests/src/VuFindTest/ILS/Driver/MultiBackendTest.php +++ b/module/VuFind/tests/unit-tests/src/VuFindTest/ILS/Driver/MultiBackendTest.php @@ -2321,6 +2321,11 @@ class MultiBackendTest extends \VuFindTest\Unit\TestCase ->will( $this->returnValue($this->getPatron('username', $userSource)) ); + $mockAuth->expects($this->any()) + ->method('getStoredCatalogCredentials') + ->will( + $this->returnValue($this->getPatron('username', $userSource)) + ); } return $mockAuth; } -- GitLab