From 6dbf2a4c76b25586ebfb2bd9dd1c0b6473eb8f49 Mon Sep 17 00:00:00 2001
From: Ere Maijala <ere.maijala@helsinki.fi>
Date: Fri, 2 Nov 2018 16:13:34 +0200
Subject: [PATCH] Library card fixes and improvements (#1257)

- Security: do not pass the user's existing password back into the template; instead, leave the password field blank and check credentials only when it is changed.
- Bug fix: when the user updates an active library card, update the user table as well as the library_cards table.
- Bug fix: when the active library card contains invalid credentials, allow the user to either log back in with good credentials or switch to a different card.
---
 languages/ar.ini                              |  1 +
 languages/bn.ini                              |  1 +
 languages/ca.ini                              |  1 +
 languages/cs.ini                              |  1 +
 languages/cy.ini                              |  1 +
 languages/de.ini                              |  1 +
 languages/el.ini                              |  1 +
 languages/en.ini                              |  1 +
 languages/es.ini                              |  1 +
 languages/eu.ini                              |  1 +
 languages/fi.ini                              |  1 +
 languages/fr.ini                              |  1 +
 languages/gl.ini                              |  1 +
 languages/it.ini                              |  1 +
 languages/ja.ini                              |  1 +
 languages/nl.ini                              |  1 +
 languages/pl.ini                              |  1 +
 languages/pt-br.ini                           |  1 +
 languages/pt.ini                              |  1 +
 languages/ru.ini                              |  1 +
 languages/sv.ini                              |  1 +
 languages/tr.ini                              |  1 +
 .../Controller/LibraryCardsController.php     | 44 ++++++++++++-------
 module/VuFind/src/VuFind/Db/Row/User.php      |  8 ++--
 .../templates/librarycards/editcard.phtml     |  4 +-
 .../templates/librarycards/selectcard.phtml   |  3 ++
 .../templates/myresearch/cataloglogin.phtml   |  3 ++
 27 files changed, 63 insertions(+), 21 deletions(-)

diff --git a/languages/ar.ini b/languages/ar.ini
index 0d234aab938..0f57a385331 100644
--- a/languages/ar.ini
+++ b/languages/ar.ini
@@ -584,6 +584,7 @@ Library Catalog Search = "بحث فهرس المكتبة"
 Library Catalog Search Result = "نتائح بحث فهرس المكتبة"
 Library Catalog Username = "اسم مستخدم فهرس المكتبة"
 Library Web Search = "بحث ويب المكتبة"
+library_card_edit_password_placeholder = "كلمة مرور جديدة"
 lightbox_error = "خطأ: لا يمكن تحميل المربع المنبثق"
 Limit To = "تحديد إلى"
 List = "قائمة"
diff --git a/languages/bn.ini b/languages/bn.ini
index 570949cdda6..731eb626f28 100644
--- a/languages/bn.ini
+++ b/languages/bn.ini
@@ -585,6 +585,7 @@ Library Catalog Search = "গ্রন্থতালিকা অনুসন
 Library Catalog Search Result = "গ্রন্থতালিকার অনুসন্ধান ফলাফল"
 Library Catalog Username = "গ্রন্থতালিকা ব্যবহারকারীর নাম"
 Library Web Search = "গ্রন্থতালিকারগ্রন্থাগার ওয়েব অনুসন্ধান"
+library_card_edit_password_placeholder = "নতুন পাসওইয়ার্ড"
 lightbox_error = "ত্রুটিঃ পপআপ বক্স লোড করা যাবে না"
 Limit To = "পরিধির মধ্যে"
 List = "তালিকা"
diff --git a/languages/ca.ini b/languages/ca.ini
index a838eda0f94..7012becac94 100644
--- a/languages/ca.ini
+++ b/languages/ca.ini
@@ -595,6 +595,7 @@ Library Catalog Search = "Cerca al catàleg de la bibliotec"
 Library Catalog Search Result = "Resultat de la cerca al Catàleg de la Biblioteca"
 Library Catalog Username = "Nom d’usuari del Catàleg de la Biblioteca"
 Library Web Search = "Cerca al web de la Biblioteca"
+library_card_edit_password_placeholder = "Nova contrasenya"
 lightbox_error = "Error: No es pot carregar caixa emergent"
 Limit To = "Limitar a"
 List = "Llistat"
diff --git a/languages/cs.ini b/languages/cs.ini
index e9cf6212802..f5b69068a3f 100644
--- a/languages/cs.ini
+++ b/languages/cs.ini
@@ -582,6 +582,7 @@ Library Catalog Search = "Vyhledávání v katalogu"
 Library Catalog Search Result = "Výsledky hledání v knihovním katalogu"
 Library Catalog Username = "Uživatelské jméno"
 Library Web Search = "Vyhledávání na webu knihovny"
+library_card_edit_password_placeholder = "Nové heslo"
 lightbox_error = "Chyba: nemohu otevřít vyskakovací okno"
 Limit To = "Omezit na"
 List = "Seznam"
diff --git a/languages/cy.ini b/languages/cy.ini
index d5df9bb43fb..1579c750569 100644
--- a/languages/cy.ini
+++ b/languages/cy.ini
@@ -584,6 +584,7 @@ Library Catalog Search = "Chwilio Catalog y Llyfrgell"
 Library Catalog Search Result = "Canlyniad Chwiliad Catalog y Llyfrgell"
 Library Catalog Username = "Enw Defnyddiwr Catalog y Llyfrgell"
 Library Web Search = "Chwilio Gwe'r Llyfrgell"
+library_card_edit_password_placeholder = "Cyfrinair Newydd"
 lightbox_error = "Gwall: Ni Ellir Llwytho'r Bocs Naidlen"
 Limit To = "Cyfyngu i"
 List = "Rhestr"
diff --git a/languages/de.ini b/languages/de.ini
index c397cf2320e..1bda176f242 100644
--- a/languages/de.ini
+++ b/languages/de.ini
@@ -584,6 +584,7 @@ Library Catalog Search = "Bibliothekskatalogsuche"
 Library Catalog Search Result = "Suchergebnis im Bibliothekskatalog"
 Library Catalog Username = "Benutzername Bibliothekskatalog"
 Library Web Search = "Bibiothek Internetsuche"
+library_card_edit_password_placeholder = "Neues Passwort"
 lightbox_error = "Fehler: Laden des Popup-Fensters fehlgeschlagen"
 Limit To = "Eingrenzen"
 List = "Liste"
diff --git a/languages/el.ini b/languages/el.ini
index 4171aa9048b..2f9645cf461 100644
--- a/languages/el.ini
+++ b/languages/el.ini
@@ -584,6 +584,7 @@ Library Catalog Search = "Αναζήτηση στον κατάλογο της β
 Library Catalog Search Result = "Αποτέλεσμα καταλόγου βιβλιοθήκης"
 Library Catalog Username = "Όνομα Χρήστη καταλόγου βιβλιοθήκης"
 Library Web Search = "Αναζήτηση ιστοσελίδων βιβλιοθήκης"
+library_card_edit_password_placeholder = "Νέος κωδικός"
 lightbox_error = "Σφάλμα. Αδυναμία φόρτωσης αναδυόμενου παραθύρου"
 Limit To = "Περιορισμός σε"
 List = "Λίστα"
diff --git a/languages/en.ini b/languages/en.ini
index 71a2cd7dada..12c7436fab9 100644
--- a/languages/en.ini
+++ b/languages/en.ini
@@ -588,6 +588,7 @@ Library Catalog Search = "Library Catalog Search"
 Library Catalog Search Result = "Library Catalog Search Result"
 Library Catalog Username = "Library Catalog Username"
 Library Web Search = "Library Web Search"
+library_card_edit_password_placeholder = "New Password"
 lightbox_error = "Error: Cannot Load Popup Box"
 Limit To = "Limit To"
 List = "List"
diff --git a/languages/es.ini b/languages/es.ini
index 0f2309e4c1c..4004981e1dd 100644
--- a/languages/es.ini
+++ b/languages/es.ini
@@ -584,6 +584,7 @@ Library Catalog Search = "Búsqueda en el catálogo"
 Library Catalog Search Result = "Resultados de Búsqueda del Catálogo"
 Library Catalog Username = "Nombre de usuario del Catálog"
 Library Web Search = "Búsqueda en internet de la Biblioteca"
+library_card_edit_password_placeholder = "Nueva Contraseña"
 lightbox_error = "Error: No se puede cargar el cuadro de diálogo emergente"
 Limit To = "Limitar"
 List = "Lista"
diff --git a/languages/eu.ini b/languages/eu.ini
index 629f2fab457..e1bbe5b32a6 100644
--- a/languages/eu.ini
+++ b/languages/eu.ini
@@ -1204,6 +1204,7 @@ Library Catalog Search = "Bilatu Liburutegiko Katalogoan"
 Library Catalog Search Result = "Katalogoan egindako bilaketaren emaitzak"
 Library Catalog Username = "Erabiltzailea"
 Library Web Search = "Liburutegiaren web bilaketa"
+library_card_edit_password_placeholder = "Pasahitza berria"
 lightbox_error = "Errorea: ezin da kargatu"
 Limit To = "Bilaketa zehaztu"
 List = "Zerrenda"
diff --git a/languages/fi.ini b/languages/fi.ini
index 8edbd131d4e..358cbaaebfe 100644
--- a/languages/fi.ini
+++ b/languages/fi.ini
@@ -589,6 +589,7 @@ Library Catalog Search = "Haku kirjastoluettelosta"
 Library Catalog Search Result = "Hakutulokset"
 Library Catalog Username = "Kirjastokortin tunnus"
 Library Web Search = "Web-haku"
+library_card_edit_password_placeholder = "Uusi salasana"
 lightbox_error = "Virhe: Ponnahdusikkunan lataaminen epäonnistui."
 Limit To = "Rajaukset"
 List = "Lista"
diff --git a/languages/fr.ini b/languages/fr.ini
index 548303f33b7..03fba1b5a1a 100644
--- a/languages/fr.ini
+++ b/languages/fr.ini
@@ -583,6 +583,7 @@ Library Catalog Search = "Recherche dans le catalogue de la bibliothèque"
 Library Catalog Search Result = "Résultat de la recherche dans le catalogue de la bibliothèque"
 Library Catalog Username = "Nom d'utilisateur du catalogue de la bibliothèque"
 Library Web Search = "Recherche sur le site de la bibliothèque"
+library_card_edit_password_placeholder = "Nouveau mot de passe"
 lightbox_error = "Erreur: impossible d'ouvrir la pop-up"
 Limit To = "Limiter à"
 List = "Liste"
diff --git a/languages/gl.ini b/languages/gl.ini
index 3d49a0d6700..e4432c01a07 100644
--- a/languages/gl.ini
+++ b/languages/gl.ini
@@ -536,6 +536,7 @@ Library Catalog Search = "Procura no catálogo"
 Library Catalog Search Result = "Resultados de Procura do Catálogo"
 Library Catalog Username = "Nome de usuario do Catálogo"
 Library Web Search = "Procura na internet da Biblioteca"
+library_card_edit_password_placeholder = "Nova contrasinal"
 lightbox_error = "Erro: Non se pode cargar o cadro de diálogo emerxente"
 Limit To = "Limitar"
 List = "Lista"
diff --git a/languages/it.ini b/languages/it.ini
index 858d5aa36e2..0e118db4cfd 100644
--- a/languages/it.ini
+++ b/languages/it.ini
@@ -583,6 +583,7 @@ Library Catalog Search = "Cerca nel catalogo di biblioteca"
 Library Catalog Search Result = "Risultati della ricerca del catalogo della biblioteca"
 Library Catalog Username = "Username del catologo della biblioteca"
 Library Web Search = "Library Web Search"
+library_card_edit_password_placeholder = "Nuova Password"
 lightbox_error = "Errore: i Popup non funzionano"
 Limit To = "Limita a"
 List = "Lista"
diff --git a/languages/ja.ini b/languages/ja.ini
index 2d0c1c622ab..efc53827be6 100644
--- a/languages/ja.ini
+++ b/languages/ja.ini
@@ -584,6 +584,7 @@ Library Catalog Search = "図書館目録検索"
 Library Catalog Search Result = "図書館目録システムの検索結果"
 Library Catalog Username = "ユーザ名"
 Library Web Search = "図書館のWeb検索"
+library_card_edit_password_placeholder = "新パスワード"
 lightbox_error = "エラー: ポップアップボックスをロードできません"
 Limit To = "絞込み"
 List = "リスト"
diff --git a/languages/nl.ini b/languages/nl.ini
index e84ba354b51..2781409954a 100644
--- a/languages/nl.ini
+++ b/languages/nl.ini
@@ -584,6 +584,7 @@ Library Catalog Search = "Zoekopdracht catalogus"
 Library Catalog Search Result = "Zoekresultaat van de catalogus"
 Library Catalog Username = "Gebruikersnaam voor catalogus"
 Library Web Search = "Doorzoek de website van de bibliotheek"
+library_card_edit_password_placeholder = "Nieuw wachtwoord"
 lightbox_error = "Foutmelding: Pop-upvenster kan niet worden geladen"
 Limit To = "Beperken tot"
 List = "Lijst"
diff --git a/languages/pl.ini b/languages/pl.ini
index 05345b1da77..994196aaacf 100644
--- a/languages/pl.ini
+++ b/languages/pl.ini
@@ -645,6 +645,7 @@ Library Catalog Search = "Wyszukiwanie"
 Library Catalog Search Result = "Rezultaty"
 Library Catalog Username = "Nazwa użytkownika"
 Library Web Search = "Wyszukiwanie w internecie"
+library_card_edit_password_placeholder = "Nowe hasło"
 lightbox_error = "Błąd: Nie udało się załadować okienka pop-up."
 Limit To = "Ogranicz do"
 List = "Lista"
diff --git a/languages/pt-br.ini b/languages/pt-br.ini
index bb5331ce7d1..ee2f9651050 100644
--- a/languages/pt-br.ini
+++ b/languages/pt-br.ini
@@ -583,6 +583,7 @@ Library Catalog Search = "Busca no Catálogo da Biblioteca"
 Library Catalog Search Result = "Resultado da busca no Catálogo da Biblioteca"
 Library Catalog Username = "Login do Usuário no Catálogo da Biblioteca"
 Library Web Search = "Buscar na Web da Biblioteca"
+library_card_edit_password_placeholder = "Nova Senha"
 lightbox_error = "Erro: janela de pop-ups bloqueada"
 Limit To = "Limitar a"
 List = "Lista"
diff --git a/languages/pt.ini b/languages/pt.ini
index 5a4573dd36e..aee60b592d0 100644
--- a/languages/pt.ini
+++ b/languages/pt.ini
@@ -514,6 +514,7 @@ Library Catalog Search = "Pesquisa no Catálogo da Biblioteca"
 Library Catalog Search Result = "Resultado da pesquisa no Catálogo da Biblioteca"
 Library Catalog Username = "Login de Utilizador no Catálogo da Biblioteca"
 Library Web Search = "Pesquisar na Web da Biblioteca"
+library_card_edit_password_placeholder = "Nova Senha"
 lightbox_error = "Erro: janela de pop-ups bloqueada"
 Limit To = "Limitar a"
 List = "Lista"
diff --git a/languages/ru.ini b/languages/ru.ini
index 509f8576e74..99a02ab7aa3 100644
--- a/languages/ru.ini
+++ b/languages/ru.ini
@@ -562,6 +562,7 @@ Library Catalog Search = "Поиска в библиотечном катало
 Library Catalog Search Result = "Результат поиска по каталогу библиотеки"
 Library Catalog Username = "Имя пользователя по каталогу библиотеки"
 Library Web Search = "Web-поиск библиотеки"
+library_card_edit_password_placeholder = "Новый пароль"
 lightbox_error = "Ошибка: невозможно загрузить всплывающее окно."
 Limit To = "Ограничить:"
 List = "Список"
diff --git a/languages/sv.ini b/languages/sv.ini
index 3ddde6970a7..75980fe4a83 100644
--- a/languages/sv.ini
+++ b/languages/sv.ini
@@ -584,6 +584,7 @@ Library Catalog Search = "Library Catalog Search"
 Library Catalog Search Result = "Sökresultat"
 Library Catalog Username = "Användarnamn i bibliotekssystemet"
 Library Web Search = "Webbsökning"
+library_card_edit_password_placeholder = "Nytt lösenord"
 lightbox_error = "Fel: kunde inte öppna ett pop-up-fönster."
 Limit To = "Begränsa till"
 List = "Lista"
diff --git a/languages/tr.ini b/languages/tr.ini
index 281acabadf8..998c7469a7e 100644
--- a/languages/tr.ini
+++ b/languages/tr.ini
@@ -594,6 +594,7 @@ Library Catalog Search = "Kütüphane Katalog Taraması"
 Library Catalog Search Result = "Kütüphane Katalog Tarama Sonucu"
 Library Catalog Username = "Kütüphane Katalog Kullanıcı Adı"
 Library Web Search = "Kütüphane Web Taraması"
+library_card_edit_password_placeholder = "Yeni Åžifre"
 lightbox_error = "Lightbox Hatası"
 Limit To = "Aşağıdaki seçenekler ile taramanızı sınırlandırabilirsiniz."
 List = "Liste"
diff --git a/module/VuFind/src/VuFind/Controller/LibraryCardsController.php b/module/VuFind/src/VuFind/Controller/LibraryCardsController.php
index f700a488a7b..f99eb510cff 100644
--- a/module/VuFind/src/VuFind/Controller/LibraryCardsController.php
+++ b/module/VuFind/src/VuFind/Controller/LibraryCardsController.php
@@ -29,6 +29,8 @@
  */
 namespace VuFind\Controller;
 
+use VuFind\Exception\ILS as ILSException;
+
 /**
  * Controller for the library card functionality.
  *
@@ -131,7 +133,6 @@ class LibraryCardsController extends AbstractBase
                 'cardName' => $cardName,
                 'target' => $target ? $target : $defaultTarget,
                 'username' => $username,
-                'password' => $card->cat_password,
                 'targets' => $targets,
                 'defaultTarget' => $defaultTarget
             ]
@@ -193,13 +194,19 @@ class LibraryCardsController extends AbstractBase
         $user->activateLibraryCard($cardID);
 
         // Connect to the ILS and check that the credentials are correct:
-        $catalog = $this->getILS();
-        $patron = $catalog->patronLogin(
-            $user->cat_username, $user->getCatPassword()
-        );
-        if (!$patron) {
+        try {
+            $catalog = $this->getILS();
+            $patron = $catalog->patronLogin(
+                $user->cat_username,
+                $user->getCatPassword()
+            );
+            if (!$patron) {
+                $this->flashMessenger()
+                    ->addMessage('authentication_error_invalid', 'error');
+            }
+        } catch (ILSException $e) {
             $this->flashMessenger()
-                ->addMessage('authentication_error_invalid', 'error');
+                ->addMessage('authentication_error_technical', 'error');
         }
 
         $this->setFollowupUrlToReferer();
@@ -224,8 +231,9 @@ class LibraryCardsController extends AbstractBase
         $target = $this->params()->fromPost('target', '');
         $username = $this->params()->fromPost('username', '');
         $password = $this->params()->fromPost('password', '');
+        $id = $this->params()->fromRoute('id', $this->params()->fromQuery('id'));
 
-        if (!$username || !$password) {
+        if (!$username) {
             $this->flashMessenger()
                 ->addMessage('authentication_error_blank', 'error');
             return false;
@@ -235,16 +243,20 @@ class LibraryCardsController extends AbstractBase
             $username = "$target.$username";
         }
 
-        // Connect to the ILS and check that the credentials are correct:
-        $catalog = $this->getILS();
-        $patron = $catalog->patronLogin($username, $password);
-        if (!$patron) {
-            $this->flashMessenger()
-                ->addMessage('authentication_error_invalid', 'error');
-            return false;
+        // Check the credentials if the username is changed or a new password is
+        // entered:
+        $card = $user->getLibraryCard($id == 'NEW' ? null : $id);
+        if ($card->cat_username !== $username || trim($password)) {
+            // Connect to the ILS and check that the credentials are correct:
+            $catalog = $this->getILS();
+            $patron = $catalog->patronLogin($username, $password);
+            if (!$patron) {
+                $this->flashMessenger()
+                    ->addMessage('authentication_error_invalid', 'error');
+                return false;
+            }
         }
 
-        $id = $this->params()->fromRoute('id', $this->params()->fromQuery('id'));
         try {
             $user->saveLibraryCard(
                 $id == 'NEW' ? null : $id, $cardName, $username, $password
diff --git a/module/VuFind/src/VuFind/Db/Row/User.php b/module/VuFind/src/VuFind/Db/Row/User.php
index a0de0e5de28..2b3205e0478 100644
--- a/module/VuFind/src/VuFind/Db/Row/User.php
+++ b/module/VuFind/src/VuFind/Db/Row/User.php
@@ -567,9 +567,11 @@ class User extends RowGateway implements \VuFind\Db\Table\DbTableAwareInterface,
 
         $row->save();
 
-        // If this is the first library card or no credentials are currently set,
-        // activate the card now
-        if ($this->getLibraryCards()->count() == 1 || empty($this->cat_username)) {
+        // If this is the first or active library card, or no credentials are
+        // currently set, activate the card now
+        if ($this->getLibraryCards()->count() == 1 || empty($this->cat_username)
+            || $this->cat_username === $row->cat_username
+        ) {
             $this->activateLibraryCard($row->id);
         }
 
diff --git a/themes/bootstrap3/templates/librarycards/editcard.phtml b/themes/bootstrap3/templates/librarycards/editcard.phtml
index 1361cd6947b..1e241c594c5 100644
--- a/themes/bootstrap3/templates/librarycards/editcard.phtml
+++ b/themes/bootstrap3/templates/librarycards/editcard.phtml
@@ -13,7 +13,7 @@
 
 <h2><?=$this->transEsc($pageTitle); ?></h2>
 
-<form class="form-edit-card" method="post" name="<?=empty($this->card->id) ? 'newCardForm' : 'editCardForm'?>">
+<form class="form-edit-card" method="post" name="<?=empty($this->card->id) ? 'newCardForm' : 'editCardForm'?>" autocomplete="off">
   <input type="hidden" name="id" value="<?=empty($this->card->id) ? 'NEW' : $this->card->id ?>"/>
   <div class="form-group">
     <label class="control-label" for="card_name"><?=$this->transEsc('Library Card Name'); ?>:</label>
@@ -35,7 +35,7 @@
   </div>
   <div class="form-group">
     <label class="control-label" for="login_password"><?=$this->transEsc('Password')?>:</label>
-    <input id="login_password" type="password" name="password" value="<?=$this->escapeHtmlAttr($this->password)?>" class="form-control"/>
+    <input id="login_password" type="password" name="password" value="" placeholder="<?=!empty($this->card->id) ? $this->escapeHtmlAttr($this->translate('library_card_edit_password_placeholder')) : ''?>" class="form-control"/>
   </div>
   <div class="form-group">
     <input class="btn btn-primary" type="submit" name="submit" value="<?=$this->transEsc('Save') ?>"/>
diff --git a/themes/bootstrap3/templates/librarycards/selectcard.phtml b/themes/bootstrap3/templates/librarycards/selectcard.phtml
index ea295d90c99..b2fa24eca4d 100644
--- a/themes/bootstrap3/templates/librarycards/selectcard.phtml
+++ b/themes/bootstrap3/templates/librarycards/selectcard.phtml
@@ -3,6 +3,9 @@
     <form class="form-inline" action="<?=$this->url('librarycards-selectcard')?>" method="get">
       <label for="library_card"><?=$this->transEsc('Library Card')?></label>
       <select id="library_card" name="cardID" class="jumpMenu form-control">
+        <?php if (null === $this->user->cat_username): ?>
+          <option value="" selected="selected">-</option>
+        <?php endif; ?>
         <?php foreach ($cards as $card): ?>
           <?php
             $target = '';
diff --git a/themes/bootstrap3/templates/myresearch/cataloglogin.phtml b/themes/bootstrap3/templates/myresearch/cataloglogin.phtml
index 27ad44901d2..0cb7afbe7fa 100644
--- a/themes/bootstrap3/templates/myresearch/cataloglogin.phtml
+++ b/themes/bootstrap3/templates/myresearch/cataloglogin.phtml
@@ -13,6 +13,9 @@
 <?php else: ?>
   <h3><?=$this->transEsc('Library Catalog Profile')?></h3>
   <?=$this->flashmessages()?>
+  <p>
+    <?=$this->context($this)->renderInContext('librarycards/selectcard.phtml', ['user' => $this->auth()->isLoggedIn()]); ?>
+  </p>
   <p><?=$this->transEsc('cat_establish_account')?></p>
   <form method="post" action="<?=$this->serverUrl(true)?>" class="form-catalog-login">
     <?php if ($this->targets !== null): ?>
-- 
GitLab