[Koha-bugs] [Bug 29930] Auth with LDAP: Update tag leads to unwanted updates

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29930

Tomás Cohen Arazi <tomascohen at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tomascohen at gmail.com

--- Comment #11 from Tomás Cohen Arazi <tomascohen at gmail.com> ---
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.