From 6464ada6c6801ab9a10d7a64154212ba30428ab9 Mon Sep 17 00:00:00 2001 From: Nick Clemens 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 Signed-off-by: Marcel de Rooy Signed-off-by: Martin Renvoize --- 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 | 39 +++++++++++++++++++ tools/import_borrowers.pl | 1 + 5 files changed, 63 insertions(+), 3 deletions(-) diff --git a/Koha/Patrons/Import.pm b/Koha/Patrons/Import.pm index 8ced08cba4..ed1e68deaf 100644 --- a/Koha/Patrons/Import.pm +++ b/Koha/Patrons/Import.pm @@ -71,6 +71,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'); @@ -255,8 +256,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} ); @@ -301,6 +302,9 @@ sub import_patrons { ); } } + if ($overwrite_passwords){ + $patron->set_password({ password => $borrower{password} }); + } if ($extended) { if ($ext_preserve) { $patron_attributes = $patron->extended_attributes->merge_with( $patron_attributes ); 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 @@
  • +
      +
    • + + +
      • @@ -276,7 +282,7 @@
    • - '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.
    • @@ -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); + }); [% 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 784d929b76..26937b9d46 100644 --- a/t/db_dependent/Koha/Patrons/Import.t +++ b/t/db_dependent/Koha/Patrons/Import.t @@ -434,6 +434,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 67b6c48042..b10e023191 100755 --- a/tools/import_borrowers.pl +++ b/tools/import_borrowers.pl @@ -128,6 +128,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.39.5