https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29930
Tomás Cohen Arazi <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] --- Comment #11 from Tomás Cohen Arazi <[email protected]> --- Hi all, I'm not *that* familiar with the LDAP code, but I reverse engineered how it works (on its interaction with Net::LDAP) for writing those mocked tests, so I could help. At first sight, it doesn't look like a harmless tiny change. That 15 year old line seems to deal with how the `cardnumber` is set. So tests cannot be skipped. Looking at the code, what we have is: 1. LDAP login 2. Login ok => Locally query the patron on the DB. At this point, the user-entered identifier is compared to `borrowers.userid` and then `borrowers.cardnumber`. At this point we have the Koha-defined `cardnumber` (nullable) and the `userid` (nullable on the DB, but auto-generated if NULL, by Koha::Patron->store). 3. In either case, %borrower is populated with the mapped data from the LDAP response, and using the user-introduced identifier as cardnumber... => SEEMS incorrect I agree at this point we actually know the local userid and cardnumber and the current code is naive in terms of how data is cared about. My understanding is that the patch is correct, meaning that if there's a mapping for the cardnumber, the loop in ldap_entry_2_hash() would overwrite it anyway. So we are just avoiding the cardnumber being set incorrectly when it is not mapped. Will try to provide a test for this. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
