From 9a6134710d6651d0594c1b2eaaec59024bb37aea Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Wed, 8 Jan 2020 14:24:04 +0000
Subject: [PATCH] Bug 23473: Don't allow setting blank passwords via import

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 Koha/Patrons/Import.pm                            |  2 +-
 .../prog/en/modules/tools/import_borrowers.tt     |  2 +-
 t/db_dependent/Koha/Patrons/Import.t              | 15 ++++++++++++++-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/Koha/Patrons/Import.pm b/Koha/Patrons/Import.pm
index 06a411ede2..6523e8d936 100644
--- a/Koha/Patrons/Import.pm
+++ b/Koha/Patrons/Import.pm
@@ -303,7 +303,7 @@ sub import_patrons {
                     );
                 }
             }
-            if ($overwrite_passwords){
+            if ($overwrite_passwords && defined $borrower{password} && $borrower{password} ne ''){
                 try {
                     $patron->set_password({ password => $borrower{password} });
                 }
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt
index 70fe71712b..915525de75 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt
@@ -248,7 +248,7 @@
                     <ul>
                         <li>
                             <input class="overwrite_passwords" type="checkbox" id="overwrite_passwords" name="overwrite_passwords" disabled/>
-                            <label class="overwrite_passwords" for="overwrite_passwords">Replace patron passwords with those in the file</label>
+                            <label class="overwrite_passwords" for="overwrite_passwords">Replace patron passwords with those in the file (blank passwords will be ignored)</label>
                         </li
                     </ul>
                 </li>
diff --git a/t/db_dependent/Koha/Patrons/Import.t b/t/db_dependent/Koha/Patrons/Import.t
index ed62974a7a..acfce22a2c 100644
--- a/t/db_dependent/Koha/Patrons/Import.t
+++ b/t/db_dependent/Koha/Patrons/Import.t
@@ -435,7 +435,7 @@ subtest 'test_import_with_cardnumber_0' => sub {
 };
 
 subtest 'test_import_with_password_overwrite' => sub {
-    plan tests => 4;
+    plan tests => 6;
 
     #Remove possible existing user to avoid clashes
     my $ernest = Koha::Patrons->find({ userid => 'ErnestP' });
@@ -447,6 +447,7 @@ subtest 'test_import_with_password_overwrite' => sub {
     my $csv_headers  = 'surname,userid,branchcode,categorycode,password';
     my $csv_password = "Worrell,ErnestP,$branchcode,$categorycode,Ernest1";
     my $csv_password_change = "Worrell,ErnestP,$branchcode,$categorycode,Vern1";
+    my $csv_blank_password = "Worel,ErnestP,$branchcode,$categorycode,";
     my $defaults = { cardnumber => "" }; #currently all the defaults come as "" if not filled
 
     #Make the test files for importing
@@ -457,6 +458,10 @@ subtest 'test_import_with_password_overwrite' => sub {
     open(my $handle_2, "<", $filename_2) or die "cannot open < $filename_2: $!";
     my $params_2 = { file => $handle_2, matchpoint => 'userid', overwrite_passwords => 1, overwrite_cardnumber => 1};
 
+    my $filename_3 = make_csv($temp_dir, $csv_headers, $csv_blank_password);
+    open(my $handle_3, "<", $filename_3) or die "cannot open < $filename_3: $!";
+    my $params_3 = { file => $handle_3, matchpoint => 'userid', overwrite_passwords => 1, overwrite_cardnumber => 1};
+
 
     my $result = $patrons_import->import_patrons($params_1, $defaults);
     like($result->{feedback}->[1]->{value}, qr/^Worrell \/ \d+/, 'First borrower imported as expected');
@@ -471,6 +476,14 @@ subtest 'test_import_with_password_overwrite' => sub {
     isnt($ernest->password,$orig_pass,"New patron is overwritten, password is overwritten");
     isnt($ernest->password,'Vern',"Password is overwritten and is encrypted from value provided");
 
+    #Save info to check not changed
+    $orig_pass = $ernest->password;
+
+    $result = $patrons_import->import_patrons($params_3, $defaults);
+    $ernest = Koha::Patrons->find({ userid => 'ErnestP' });
+    is($ernest->surname,'Worel',"Patron is overwritten, surname changed");
+    is($ernest->password,$orig_pass,"Patron was overwritten but password is not overwritten if blank");
+
 };
 
 
-- 
2.20.1