From d45da42965627f432badecd99d038afff5f514f8 Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Mon, 19 Aug 2019 16:16:30 +0000
Subject: [PATCH] Bug 23473: Allow overwrite of passwords during import

To test:
 1 - Have some patrons in your system
 2 - Export some of their info via reports
    SELECT cardnumber, userid, surname, firstname, password, branchcode, categorycode
 3 - Edit the file from above, changing all the password lines
 4 - Import the file with overwrite
 5 - Confirm passwords have not changed (run the report again and confirm the hashes are the same)
 6 - Apply patch
 7 - Restart all the things
 8 - Check the new box on import screen to overwrite passwrods
 9 - Import file again
10 - Confirm passwords have changed
11 - Signin using new password to verify the hash is the password as supplied
12 - Repeat via commandline import supplying --overwrite_passwords option
13 - Verify works as expected
14 - Prove -v t/db_dependent/Koha/Patrons/Import.t

Sponsored-by: ByWater Solutions

Signed-off-by: Ron Marion <ron.marion@goddard.edu>
---
 Koha/Patrons/Import.pm                             |  8 +++--
 .../prog/en/modules/tools/import_borrowers.tt      | 15 +++++++-
 misc/import_patrons.pl                             |  3 ++
 t/db_dependent/Koha/Patrons/Import.t               | 41 +++++++++++++++++++++-
 tools/import_borrowers.pl                          |  1 +
 5 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/Koha/Patrons/Import.pm b/Koha/Patrons/Import.pm
index 3575b8a4e2..ea8f615c31 100644
--- a/Koha/Patrons/Import.pm
+++ b/Koha/Patrons/Import.pm
@@ -72,6 +72,7 @@ sub import_patrons {
     my $defaults             = $params->{defaults};
     my $ext_preserve         = $params->{preserve_extended_attributes};
     my $overwrite_cardnumber = $params->{overwrite_cardnumber};
+    my $overwrite_passwords  = $params->{overwrite_passwords};
     my $extended             = C4::Context->preference('ExtendedPatronAttributes');
     my $set_messaging_prefs  = C4::Context->preference('EnhancedMessagingPreferences');
 
@@ -241,8 +242,8 @@ sub import_patrons {
                 # use values from extant patron unless our csv file includes this column or we provided a default.
                 # FIXME : You cannot update a field with a  perl-evaluated false value using the defaults.
 
-                # The password is always encrypted, skip it!
-                next if $col eq 'password';
+                # The password is always encrypted, skip it unless we are forcing overwrite!
+                next if $col eq 'password' && !$overwrite_passwords;
 
                 unless ( exists( $csvkeycol{$col} ) || $defaults->{$col} ) {
                     $borrower{$col} = $member->{$col} if ( $member->{$col} );
@@ -287,6 +288,9 @@ sub import_patrons {
                     );
                 }
             }
+            if ($overwrite_passwords){
+                $patron->set_password({ password => $borrower{password} });
+            }
             if ($extended) {
                 if ($ext_preserve) {
                     my $old_attributes = GetBorrowerAttributes($borrowernumber);
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 712dfe4927..52a07d9c57 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
@@ -220,6 +220,12 @@
 
                 <li class="radio">
                     <input type="radio" id="overwrite_cardnumberyes" name="overwrite_cardnumber" value="1" /><label for="overwrite_cardnumberyes">Overwrite the existing one with this</label>
+                    <ul>
+                        <li>
+                            <input type="checkbox" id="overwrite_passwords" name="overwrite_passwords" disabled="true"/>
+                            <label for="overwrite_passwords">Replace patron passwords with those in the file</label>
+                        </li
+                    </ul>
                 </li>
             </ol>
         </fieldset>
@@ -276,7 +282,7 @@
         </li>
 
         <li>
-            'password' should be stored in plaintext, and will be converted to a Bcrypt hash (if your passwords are already encrypted, talk to your system administrator about options).
+            'password' should be stored in plaintext, and will be converted to a Bcrypt hash (if your passwords are already encrypted, talk to your system administrator about options). Passwords will not be updated on overwrite unless replace passwords option is checked.
         </li>
 
         <li>
@@ -313,6 +319,13 @@ you can supply dates in ISO format (e.g., '2010-10-28').
                 $(".expand_defaults").toggle();
             });
         });
+
+        $("#overwrite_cardnumberno").click(function(){
+            $("#overwrite_passwords").prop('checked',false).prop('disabled',true);
+        });
+        $("#overwrite_cardnumberyes").click(function(){
+            $("#overwrite_passwords").prop('disabled',false);
+        });
     </script>
 [% END %]
 [% INCLUDE 'intranet-bottom.inc' %]
diff --git a/misc/import_patrons.pl b/misc/import_patrons.pl
index 205174e64d..b341308c68 100755
--- a/misc/import_patrons.pl
+++ b/misc/import_patrons.pl
@@ -29,6 +29,7 @@ my $Import = Koha::Patrons::Import->new();
 my $csv_file;
 my $matchpoint;
 my $overwrite_cardnumber;
+my $overwrite_passwords;
 my %defaults;
 my $ext_preserve = 0;
 my $confirm;
@@ -41,6 +42,7 @@ GetOptions(
     'm|matchpoint=s'                => \$matchpoint,
     'd|default=s'                   => \%defaults,
     'o|overwrite'                   => \$overwrite_cardnumber,
+    'op|overwrite_passwords'        => \$overwrite_passwords,
     'p|preserve-extended-attributes' => \$ext_preserve,
     'v|verbose+'                    => \$verbose,
     'h|help|?'                      => \$help,
@@ -61,6 +63,7 @@ my $return = $Import->import_patrons(
         defaults                     => \%defaults,
         matchpoint                   => $matchpoint,
         overwrite_cardnumber         => $overwrite_cardnumber,
+        overwrite_passwords          => $overwrite_passwords,
         preserve_extended_attributes => $ext_preserve,
     }
 );
diff --git a/t/db_dependent/Koha/Patrons/Import.t b/t/db_dependent/Koha/Patrons/Import.t
index b41a2654a0..17e52cbb9b 100644
--- a/t/db_dependent/Koha/Patrons/Import.t
+++ b/t/db_dependent/Koha/Patrons/Import.t
@@ -18,7 +18,7 @@
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 use Modern::Perl;
-use Test::More tests => 157;
+use Test::More tests => 158;
 use Test::Warn;
 
 # To be replaced by t::lib::Mock
@@ -428,6 +428,45 @@ subtest 'test_import_with_cardnumber_0' => sub {
 
 };
 
+subtest 'test_import_with_password_overwrite' => sub {
+    plan tests => 4;
+
+    #Remove possible existing user to avoid clashes
+    my $ernest = Koha::Patrons->find({ userid => 'ErnestP' });
+    $ernest->delete if $ernest;
+
+    #Setup our info
+    my $branchcode = $builder->build({ source => "Branch"})->{branchcode};
+    my $categorycode = $builder->build({ source => "Category"})->{categorycode};
+    my $csv_headers  = 'surname,userid,branchcode,categorycode,password';
+    my $csv_password = "Worrell,ErnestP,$branchcode,$categorycode,Ernest";
+    my $csv_password_change = "Worrell,ErnestP,$branchcode,$categorycode,Vern";
+    my $defaults = { cardnumber => "" }; #currently all the defaults come as "" if not filled
+
+    #Make the test files for importing
+    my $filename_1 = make_csv($temp_dir, $csv_headers, $csv_password);
+    open(my $handle_1, "<", $filename_1) or die "cannot open < $filename_1: $!";
+    my $params_1 = { file => $handle_1, matchpoint => 'userid', overwrite_passwords => 1, overwrite_cardnumber => 1};
+    my $filename_2 = make_csv($temp_dir, $csv_headers, $csv_password_change);
+    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 $result = $patrons_import->import_patrons($params_1, $defaults);
+    like($result->{feedback}->[1]->{value}, qr/^Worrell \/ \d+/, 'First borrower imported as expected');
+    $ernest = Koha::Patrons->find({ userid => 'ErnestP' });
+    isnt($ernest->password,'Ernest',"New patron is imported, password is encrypted");
+
+    #Save info to double check
+    my $orig_pass = $ernest->password;
+
+    $result = $patrons_import->import_patrons($params_2, $defaults);
+    $ernest = Koha::Patrons->find({ userid => 'ErnestP' });
+    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");
+
+};
+
 
 subtest 'test_prepare_columns' => sub {
     plan tests => 16;
diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl
index 4da2cd0d03..4324b3a71b 100755
--- a/tools/import_borrowers.pl
+++ b/tools/import_borrowers.pl
@@ -127,6 +127,7 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
             defaults                     => \%defaults,
             matchpoint                   => $matchpoint,
             overwrite_cardnumber         => $input->param('overwrite_cardnumber'),
+            overwrite_passwords          => $input->param('overwrite_passwords'),
             preserve_extended_attributes => $input->param('ext_preserve') || 0,
         }
     );
-- 
2.11.0