From c74678a1d239aaf91906039ccb3db940df4a0b41 Mon Sep 17 00:00:00 2001 From: Alex Arnaud Date: Wed, 24 Feb 2016 10:13:40 +0100 Subject: [PATCH] Bug 15889: LDAP authentication: Only update mapped attributes Test plan: - Update your configuration file to use LDAP authentication and enable update (1) option, - login with an existing user with extended attrbitutes that are not in LDAP mapping, - check that all attributes are still here. Signed-off-by: Chris Signed-off-by: Philippe Blouin Signed-off-by: Jesse Weaver --- C4/Auth_with_ldap.pm | 18 +++++-------- t/db_dependent/Auth_with_ldap.t | 48 +++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm index 4a9c302417..14db437c72 100644 --- a/C4/Auth_with_ldap.pm +++ b/C4/Auth_with_ldap.pm @@ -206,25 +206,19 @@ sub checkpw_ldap { return 0; # B2, D2 } if (C4::Context->preference('ExtendedPatronAttributes') && $borrowernumber && ($config{update} ||$config{replicate})) { - my @extended_patron_attributes; foreach my $attribute_type ( C4::Members::AttributeTypes::GetAttributeTypes() ) { my $code = $attribute_type->{code}; - if ( exists($borrower{$code}) && $borrower{$code} !~ m/^\s*$/ ) { # skip empty values - push @extended_patron_attributes, { code => $code, value => $borrower{$code} }; + unless (exists($borrower{$code}) && $borrower{$code} !~ m/^\s*$/ ) { + next; } - } - #Check before add - my @unique_attr; - foreach my $attr ( @extended_patron_attributes ) { - if (C4::Members::Attributes::CheckUniqueness($attr->{code}, $attr->{value}, $borrowernumber)) { - push @unique_attr, $attr; + if (C4::Members::Attributes::CheckUniqueness($code, $borrower{$code}, $borrowernumber)) { + C4::Members::Attributes::UpdateBorrowerAttribute($borrowernumber, {code => $code, value => $borrower{$code}}); } else { - warn "ERROR_extended_unique_id_failed $attr->{code} $attr->{value}"; + warn "ERROR_extended_unique_id_failed $code $borrower{$code}"; } } - C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, \@unique_attr, 'no_branch_limit'); } -return(1, $cardnumber, $userid); + return(1, $cardnumber, $userid); } # Pass LDAP entry object and local cardnumber (userid). diff --git a/t/db_dependent/Auth_with_ldap.t b/t/db_dependent/Auth_with_ldap.t index 4982248c8e..9d68055aa3 100755 --- a/t/db_dependent/Auth_with_ldap.t +++ b/t/db_dependent/Auth_with_ldap.t @@ -20,6 +20,7 @@ use Modern::Perl; use Test::More tests => 4; use Test::MockModule; use Test::MockObject; +use t::lib::TestBuilder; use Test::Warn; use C4::Context; @@ -29,6 +30,8 @@ my $dbh = C4::Context->dbh; $dbh->{ AutoCommit } = 0; $dbh->{ RaiseError } = 1; +my $builder = t::lib::TestBuilder->new(); + # Variables controlling LDAP server config my $update = 0; my $replicate = 0; @@ -61,6 +64,32 @@ $ldap->mock( 'new', sub { } }); +my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; +my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; +my $attrType = $builder->build({ + source => 'BorrowerAttributeType', + value => { + category_code => $categorycode + } +}); + +my $borrower = $builder->build({ + source => 'Borrower', + value => { + userid => 'hola', + branchcode => $branchcode, + categorycode => $categorycode + } +}); + +$builder->build({ + source => 'BorrowerAttribute', + value => { + borrowernumber => $borrower->{borrowernumber}, + code => $attrType->{code}, + attribute => 'FOO' + } +}); # C4::Auth_with_ldap needs several stuff set first ^^^ use_ok( 'C4::Auth_with_ldap' ); @@ -84,7 +113,7 @@ subtest "checkpw_ldap tests" => sub { subtest "auth_by_bind = 1 tests" => sub { - plan tests => 7; + plan tests => 8; $auth_by_bind = 1; @@ -105,8 +134,23 @@ subtest "checkpw_ldap tests" => sub { $anonymous_bind = 1; $desired_bind_result = 'success'; $desired_search_result = 'success'; - $desired_count_result = 0; # user auth problem + $desired_count_result = 1; $non_anonymous_bind_result = 'success'; + $update = 1; + reload_ldap_module(); + + my $auth = new Test::MockModule('C4::Auth_with_ldap'); + $auth->mock( 'update_local', sub { + return $borrower->{cardnumber}; + }); + + C4::Auth_with_ldap::checkpw_ldap( $dbh, 'hola', password => 'hey'); + ok(@{ C4::Members::Attributes::GetBorrowerAttributes($borrower->{borrowernumber}) }, 'Extended attributes are not deleted'); + $auth->unmock('update_local'); + + $update = 0; + $desired_count_result = 0; # user auth problem + C4::Members::DelMember($borrower->{borrowernumber}); reload_ldap_module(); is ( C4::Auth_with_ldap::checkpw_ldap( $dbh, 'hola', password => 'hey' ), 0, "checkpw_ldap returns 0 if user lookup returns 0"); -- 2.39.5