Previous to bug 14507, SIP2 only did internal authentication. A change to the way we check empty passwords has caused any empty password to send back a CQ of Y. Previous to that patch set, a CQ of Y would only be sent back of the patron password column was NULL. Now, an empty AD field *always* returns a CQ of Y.
Created attachment 51879 [details] [review] Bug 16610 - Regression in SIP2 user password handling Previous to bug 14507, SIP2 only did internal authentication. A change to the way we check empty passwords has caused any empty password to send back a CQ of Y. Previous to that patch set, a CQ of Y would only be sent back of the patron password column was NULL. Now, an empty AD field *always* returns a CQ of Y. Test Plan: 1) Send a patron information request with an empty AD field Note: You must send the AD field or you won't get back a CQ field 2) Note you get back a CQ of Y 3) Apply this patch 4) Repeat step 1 5) Note you now get back a CQ of N
Created attachment 51881 [details] [review] [SIGNED-OFF] Bug 16610 - Regression in SIP2 user password handling Previous to bug 14507, SIP2 only did internal authentication. A change to the way we check empty passwords has caused any empty password to send back a CQ of Y. Previous to that patch set, a CQ of Y would only be sent back of the patron password column was NULL. Now, an empty AD field *always* returns a CQ of Y. Test Plan: 1) Send a patron information request with an empty AD field Note: You must send the AD field or you won't get back a CQ field 2) Note you get back a CQ of Y 3) Apply this patch 4) Repeat step 1 5) Note you now get back a CQ of N Signed-off-by: Trent Roby <troby@bclib.info>
Created attachment 51946 [details] [review] Bug 16610: [QA Follow-up] Add a test To verify the original patch, this test shows that before applying it the Patron Info request did not return CQ==N for an empty password. Note that the Patron Status did btw. After applying the original patch, the test passes for patron info as well as status. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 51947 [details] [review] Bug 16610 - Regression in SIP2 user password handling Previous to bug 14507, SIP2 only did internal authentication. A change to the way we check empty passwords has caused any empty password to send back a CQ of Y. Previous to that patch set, a CQ of Y would only be sent back of the patron password column was NULL. Now, an empty AD field *always* returns a CQ of Y. Test Plan: 1) Send a patron information request with an empty AD field Note: You must send the AD field or you won't get back a CQ field 2) Note you get back a CQ of Y 3) Apply this patch 4) Repeat step 1 5) Note you now get back a CQ of N Signed-off-by: Trent Roby <troby@bclib.info> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to Master - Should be in the November 2016 release. Thanks!
Pushed in 16.05. Will be in 16.05.01.
Patch pushed to 3.22.x, will be in 3.22.8
> Previous to bug 14507, SIP2 only did internal authentication. A change > to the way we check empty passwords has caused any empty password to > send back a CQ of Y. Previous to that patch set, a CQ of Y would only be > sent back of the patron password column was NULL. Now, an empty AD field > *always* returns a CQ of Y. We recently began testing our selfcheck terminals with Koha. This causes an issue with our setup. The machines we use (Tor3 from Bibliotheca) has an option for disabling pincode/password entry (which is the situation we want). However, this does not remove the AD fields from the transaction, just sends an empty AD. I have no idea how common this behaviour is, but it seems to be similar to the issue mentioned in bug 15124. We also need the users to have a password field for web access, so we cannot leave that field empty. The combined consequence of this is that getting a CQN back makes it impossible to use the selfcheck system. Changing the way the selfcheck system creator implements their things is not really something we can do. This whole thing could be made into a syspref, allowing for empty AD fields. I'm not sure how to proceed with this, since this is a resolved bug. Should adding a syspref for this be done using a new bug entry?
Large numbers of units send empty password fields if they do not want the password to be checked. This patch causes them to fail
As this changes how the sipserver has worked in the past and in a way incompatible with many clients it should really be governed by a config optin in the sip configuration file.
I have added a bug 18755 to allow the old behaviour on setting an option in the config of the associated sip account