Bug 28822 - SIP logins add to login attempts?
Summary: SIP logins add to login attempts?
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-08-06 09:25 UTC by Marcel de Rooy
Modified: 2021-10-18 17:16 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2021-08-06 09:25:50 UTC
From Agnes Rivers-Moore on bug 21190:

Ah - there might be a problem with Koha counting SIP logins as 'failed' attempts. I could not see it before, but thanks to this enhancement I do see that the total of login attempts goes up by one when I log in to Overdrive Libby, and it is not reset to zero again - even though the login is successful.  We had found more patrons complained of being locked out of the OPAC when they had not had 8 failed login attempts. If SIP login adds one every time, and does not clear back to zero, people using SIP products can come back to the Koha OPAC and find themselves locked out. I am on Koha 20.11
It may be this needs to be a separate new bug - anyone interested to describe what's not working as expected with SIP logins? (and possibly other kinds of external login too...)
Comment 1 Marcel de Rooy 2021-08-06 09:42:06 UTC
ILS/Patron.pm:sub check_password {
ILS/Patron.pod: $str = $patron->check_password($password);
ILS/Patron.pod:=head2 C<$bool = $patron-E<gt>check_password($password);>
Sip/MsgType.pm:            $password_rc = $patron->check_password($patron_pwd);
Sip/MsgType.pm:                $resp .= add_field( FID_VALID_PATRON_PWD, sipbool( $patron->check_password( $fields->{ (FID_PATRON_PWD) } ) ), $server );
Sip/MsgType.pm:                $password_rc = $patron->check_password($patron_pwd);
Sip/MsgType.pm:        if ( !defined($patron_pwd) || $patron->check_password($patron_pwd) ) {
Sip/MsgType.pm:            $resp .= add_field( FID_VALID_PATRON_PWD, sipbool( $patron->check_password($patron_pwd) ), $server );

build patron status
handle checkout
handle_patron_info
handle_patron_enable CALLED twice
Comment 2 Marcel de Rooy 2021-08-06 09:56:10 UTC
Kyle or Nick,
Does 27600 address this? That one refers to renewal.
I have the idea that this one might be more?
Does it depend on Overdrive ?
Comment 3 Nick Clemens 2021-08-06 09:59:59 UTC
(In reply to Marcel de Rooy from comment #2)
> Kyle or Nick,
> Does 27600 address this? That one refers to renewal.
> I have the idea that this one might be more?
> Does it depend on Overdrive ?

It does not address this, just a related issue with the call to check_password
Comment 4 Agnes Rivers-Moore 2021-09-27 23:33:43 UTC
I think this may apply to all external online services that check with Koha through SIP for patron status. Probably does not apply to self-serve checkout through SIP as I think people log out again after checking out, but I have not tested and am not sure.  Sad to see this considered LOW importance, as it is a major failure for the many patrons we have who use our online services (ebooks, video, magazines), then frequently and repeatedly have to ask staff to reset their password through no fault of theirs. Please reconsider.
Comment 5 Katrin Fischer 2021-09-28 06:19:38 UTC
Hi Agnes, we don't use the first importance field actively - low is just the default and you can ignore it. (I think we cannot hide it :( ) The second category 'normal' is what we are actually using.
Comment 6 Katrin Fischer 2021-09-28 06:23:14 UTC
I am updating to 'major' to get more attention on this. FailedLoginAttempts should be a recommended security measure to avoid brute force attacks on patron accounts and if I understand it correctly, this bug will prevent libraries with self checks (some require login) etc. from using it.
Comment 7 Jonathan Druart 2021-10-13 13:11:12 UTC
I cannot recreate using sip_cli_emulator with a checkout.

Marcel, did you manage to recreate the problem?
Comment 8 Marcel de Rooy 2021-10-13 13:16:09 UTC
(In reply to Jonathan Druart from comment #7)
> I cannot recreate using sip_cli_emulator with a checkout.
> 
> Marcel, did you manage to recreate the problem?

No. Will try later this week.
Comment 9 Agnes Rivers-Moore 2021-10-13 14:43:06 UTC
We are seeing this often but it's not from a checkout - it is from logging in to a remote 3rd party service that uses SIP to check the borrower status. In my example - Overdrive - the Koha member logs in to access their account in Overdrive, Overdrive queries Koha with SIP to check the member patron record is not expired or suspended. There is no checkout process associated with Koha, but somehow that gets recorded as a login attempt (either an unsuccessful one, or one not cancelled by a log out). Overdrive checkout then proceeds in the Overdrive member account. The same may be happening with other third party SIP connections that check status. To recreate - set up Overdrive to confirm members status using SIP. Create a Koha account. Create an Overdrive account using the member barcode. Log in to Libby using the patron barcode. In Koha, check the current logins = 1. Log out of Libby, login count in Koha remains at 1. Log in to Libby - check login count increases to 2. I hope I have explained clearly. 
We do want the SIP logins to be registered in Koha - there was a bug/enhancement about that as there was no record of 'last seen' in Koha from those external SIP transactions. That was worked on, so that the last seen would update for external SIP logins.
Comment 10 Marcel de Rooy 2021-10-18 12:39:27 UTC
(In reply to Agnes Rivers-Moore from comment #9)
> We are seeing this often but it's not from a checkout - it is from logging
> in to a remote 3rd party service that uses SIP to check the borrower status.
> In my example - Overdrive - the Koha member logs in to access their account
> in Overdrive, Overdrive queries Koha with SIP to check the member patron
> record is not expired or suspended. There is no checkout process associated
> with Koha, but somehow that gets recorded as a login attempt (either an
> unsuccessful one, or one not cancelled by a log out). Overdrive checkout
> then proceeds in the Overdrive member account. The same may be happening
> with other third party SIP connections that check status. To recreate - set
> up Overdrive to confirm members status using SIP. Create a Koha account.
> Create an Overdrive account using the member barcode. Log in to Libby using
> the patron barcode. In Koha, check the current logins = 1. Log out of Libby,
> login count in Koha remains at 1. Log in to Libby - check login count
> increases to 2. I hope I have explained clearly. 
> We do want the SIP logins to be registered in Koha - there was a
> bug/enhancement about that as there was no record of 'last seen' in Koha
> from those external SIP transactions. That was worked on, so that the last
> seen would update for external SIP logins.

Looks like you need someone who knows these third party tools.
What I do see in the Koha code, is that patron_status and patron_info requests when accompanied by a patron password will lead to a verification that might increase login_attempts when it is wrong. Note that patron_info does not need it to provide output.
In other words: Does Overdrive perhaps send a empty password string for a patron info request?
Comment 11 Nick Clemens 2021-10-18 12:57:09 UTC
(In reply to Marcel de Rooy from comment #10)
> Looks like you need someone who knows these third party tools.
> What I do see in the Koha code, is that patron_status and patron_info
> requests when accompanied by a patron password will lead to a verification
> that might increase login_attempts when it is wrong. Note that patron_info
> does not need it to provide output.
> In other words: Does Overdrive perhaps send a empty password string for a
> patron info request?

It can, this is configured per site with OverDrive

There is a SIP configuration setting: allow_empty_passwords

If that is set the login_attempts must not be incremented on an empty password
Comment 12 Jonathan Druart 2021-10-18 13:08:09 UTC
(In reply to Nick Clemens from comment #11)
> There is a SIP configuration setting: allow_empty_passwords

Good catch Nick. So this is certainly a duplicate of bug 21997.
Comment 13 Marcel de Rooy 2021-10-18 13:13:56 UTC
Does the patron_info sub actually need this SIPconfig setting? Or should we just not verify an empty string here?
Comment 14 Agnes Rivers-Moore 2021-10-18 14:22:38 UTC
(In reply to Jonathan Druart from comment #12)
> (In reply to Nick Clemens from comment #11)
> > There is a SIP configuration setting: allow_empty_passwords
> 
> Good catch Nick. So this is certainly a duplicate of bug 21997.

Well, we are on 20.11.09 - so if it was fixed it broke again, or this is not exactly the same issue.
Comment 15 Jonathan Druart 2021-10-18 14:27:25 UTC
(In reply to Agnes Rivers-Moore from comment #14)
> (In reply to Jonathan Druart from comment #12)
> > (In reply to Nick Clemens from comment #11)
> > > There is a SIP configuration setting: allow_empty_passwords
> > 
> > Good catch Nick. So this is certainly a duplicate of bug 21997.
> 
> Well, we are on 20.11.09 - so if it was fixed it broke again, or this is not
> exactly the same issue.

Do you have allow_empty_passwords turned on in the config?
Comment 16 Agnes Rivers-Moore 2021-10-18 14:35:03 UTC
(In reply to Jonathan Druart from comment #12)
> (In reply to Nick Clemens from comment #11)
> > There is a SIP configuration setting: allow_empty_passwords
> 
> Good catch Nick. So this is certainly a duplicate of bug 21997.

Well, we are on 20.11.09 - so if it was fixed it broke again, or this is not exactly the same issue. (In reply to Jonathan Druart from comment #15)
> (In reply to Agnes Rivers-Moore from comment #14)
> > (In reply to Jonathan Druart from comment #12)
> > > (In reply to Nick Clemens from comment #11)
> > > > There is a SIP configuration setting: allow_empty_passwords
> > > 
> > > Good catch Nick. So this is certainly a duplicate of bug 21997.
> > 
> > Well, we are on 20.11.09 - so if it was fixed it broke again, or this is not
> > exactly the same issue.
> 
> Do you have allow_empty_passwords turned on in the config?

In Administration - Enhanced Content I see a pref for Overdrive specifically that says A password is NOT REQUIRED for user access to OverDrive.
In config - I do not know, is that on the back end? I can ask Equinox. Would it not take the setting from the Administration Pref?
Comment 17 Nick Clemens 2021-10-18 14:52:15 UTC
(In reply to Agnes Rivers-Moore from comment #16)
> In Administration - Enhanced Content I see a pref for Overdrive specifically
> that says A password is NOT REQUIRED for user access to OverDrive.
> In config - I do not know, is that on the back end? I can ask Equinox. Would
> it not take the setting from the Administration Pref?

That setting is related to OverDrive checkouts - it does not change what overdrive sends, but simply whether Koha asks the user for their password when logging them in to Koha - so it is a setting you determine with OverDrive when setting up access
Comment 18 Nick Clemens 2021-10-18 15:12:38 UTC
I can confirm, setting:
allow_empty_passwords="1"
in SIP config will eliminate the issue

You can test  on kohadevbox using telnet:
apt install telnet
telnet localhost 6001
9300CNterm1|COterm1|CPCPL|
6300120211018    150348          AOCPL|AA23529000035676|ACterm1|AD|


Without allow_empty_passwords an empty string is checked and login_attempts incremeneted
With it, login attempts are not

You can see in the response: 
|AFGreetings from Koha.  -- Invalid password|
if the setting is not enabled

Agnes: You should either ask Equinox to change the setting for you, or ask OverDrive to send/require passwords if you would rather go that way.

Otherwise, I believe this is working correctly
Comment 19 Agnes Rivers-Moore 2021-10-18 17:13:50 UTC
Thank you Nick. Just curious what the function of the Admin Preference is if it does not relate to the SIP setup?
Comment 20 Agnes Rivers-Moore 2021-10-18 17:16:16 UTC
Ooops, didn't see your earlier explanation .I get it now - and we don't have Overdrive checkout turnedon so I guess for us it has no effect.