Bugzilla – Attachment 48650 Details for
Bug 15889
Login with LDAP deletes extended attributes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 15889 - LDAP authentication: Only update mapped attributes.
Bug-15889---LDAP-authentication-Only-update-mapped.patch (text/plain), 5.19 KB, created by
Chris Cormack
on 2016-03-03 19:38:17 UTC
(
hide
)
Description:
Bug 15889 - LDAP authentication: Only update mapped attributes.
Filename:
MIME Type:
Creator:
Chris Cormack
Created:
2016-03-03 19:38:17 UTC
Size:
5.19 KB
patch
obsolete
>From ffa73801204165ba601e0dc69f596c5c57683d8b Mon Sep 17 00:00:00 2001 >From: Alex Arnaud <alex.arnaud@biblibre.com> >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 > (<update>1</update>) 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 <chrisc@catalyst.net.nz> >--- > C4/Auth_with_ldap.pm | 19 ++++++---------- > t/db_dependent/Auth_with_ldap.t | 48 +++++++++++++++++++++++++++++++++++++-- > 2 files changed, 53 insertions(+), 14 deletions(-) > >diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm >index 58484a2..3fba519 100644 >--- a/C4/Auth_with_ldap.pm >+++ b/C4/Auth_with_ldap.pm >@@ -207,25 +207,20 @@ 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); > } >-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 4982248..9d68055 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"); >-- >1.7.10.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 15889
:
48327
|
48650
|
49389