From 88174f1cef5a01412b1ab57acc6672c507496697 Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Thu, 3 Mar 2016 08:53:04 -0500
Subject: [PATCH] Standardized behavior related to blank passwords.
---
module/VuFind/src/VuFind/Auth/CAS.php | 14 +++++++++++--
module/VuFind/src/VuFind/Auth/LDAP.php | 14 +++++++++++--
module/VuFind/src/VuFind/Auth/Shibboleth.php | 21 +++++++++-----------
3 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/module/VuFind/src/VuFind/Auth/CAS.php b/module/VuFind/src/VuFind/Auth/CAS.php
index b58fb4656d2..4fd2a760327 100644
--- a/module/VuFind/src/VuFind/Auth/CAS.php
+++ b/module/VuFind/src/VuFind/Auth/CAS.php
@@ -149,9 +149,19 @@ class CAS extends AbstractBase
}
}
- // Save credentials if applicable:
+ // Save credentials if applicable. Note that we want to allow empty
+ // passwords (see https://github.com/vufind-org/vufind/pull/532), but
+ // we also want to be careful not to replace a non-blank password with a
+ // blank one in case the auth mechanism fails to provide a password on
+ // an occasion after the user has manually stored one. (For discussion,
+ // see https://github.com/vufind-org/vufind/pull/612). Note that in the
+ // (unlikely) scenario that a password can actually change from non-blank
+ // to blank, additional work may need to be done here.
if (!empty($user->cat_username)) {
- $user->saveCredentials($user->cat_username, $catPassword);
+ $user->saveCredentials(
+ $user->cat_username,
+ empty($catPassword) ? $user->getCatPassword() : $catPassword
+ );
}
// Save and return the user object:
diff --git a/module/VuFind/src/VuFind/Auth/LDAP.php b/module/VuFind/src/VuFind/Auth/LDAP.php
index 53441e5af30..883383bb33a 100644
--- a/module/VuFind/src/VuFind/Auth/LDAP.php
+++ b/module/VuFind/src/VuFind/Auth/LDAP.php
@@ -289,9 +289,19 @@ class LDAP extends AbstractBase
}
}
- // Save credentials if applicable:
+ // Save credentials if applicable. Note that we want to allow empty
+ // passwords (see https://github.com/vufind-org/vufind/pull/532), but
+ // we also want to be careful not to replace a non-blank password with a
+ // blank one in case the auth mechanism fails to provide a password on
+ // an occasion after the user has manually stored one. (For discussion,
+ // see https://github.com/vufind-org/vufind/pull/612). Note that in the
+ // (unlikely) scenario that a password can actually change from non-blank
+ // to blank, additional work may need to be done here.
if (!empty($user->cat_username)) {
- $user->saveCredentials($user->cat_username, $catPassword);
+ $user->saveCredentials(
+ $user->cat_username,
+ empty($catPassword) ? $user->getCatPassword() : $catPassword
+ );
}
// Update the user in the database, then return it to the caller:
diff --git a/module/VuFind/src/VuFind/Auth/Shibboleth.php b/module/VuFind/src/VuFind/Auth/Shibboleth.php
index 83219f7adec..f747febcd20 100644
--- a/module/VuFind/src/VuFind/Auth/Shibboleth.php
+++ b/module/VuFind/src/VuFind/Auth/Shibboleth.php
@@ -121,21 +121,18 @@ class Shibboleth extends AbstractBase
}
}
- // Save credentials if applicable. Note that if $catPassword is empty,
- // we'll pass through the existing password already in the database;
- // otherwise, when users log out, their passwords may be cleared from
- // the database. We can't simply skip saving credentials when the password
- // is empty, because in some scenarios, an empty password is normal
- // (see https://github.com/vufind-org/vufind/pull/532 for details).
- // Note that this leaves an edge case where, if a user changes their
- // password from something to nothing, VuFind will not properly clear it
- // out. This seems unlikely, but if it is encountered, we may need to
- // add more logic here. See https://github.com/vufind-org/vufind/pull/612
- // for related discussion.
+ // Save credentials if applicable. Note that we want to allow empty
+ // passwords (see https://github.com/vufind-org/vufind/pull/532), but
+ // we also want to be careful not to replace a non-blank password with a
+ // blank one in case the auth mechanism fails to provide a password on
+ // an occasion after the user has manually stored one. (For discussion,
+ // see https://github.com/vufind-org/vufind/pull/612). Note that in the
+ // (unlikely) scenario that a password can actually change from non-blank
+ // to blank, additional work may need to be done here.
if (!empty($user->cat_username)) {
$user->saveCredentials(
$user->cat_username,
- empty($catPassword) ? $user->cat_password : $catPassword
+ empty($catPassword) ? $user->getCatPassword() : $catPassword
);
}
--
GitLab