LP#
1844121: prevent staff login by expired barcode
open-ils.auth.authenticate.init accepts an identifier as its
sole parameter, then determines whether it looks like a
username or barcode and retrieves the patron's password
salt as the seed accordingly.
open-ils.auth.authenticate.complete can accept the identifier
via the 'identifier', 'username', or 'barcode' keys, but the
key used does not need to match how .init found the patron.
As a consequence, the .init/.complete dance can retrieve the
patron by barcode but handle the barcode value as if it were
a username, thereby bypassing the check of whether the barcode
was inactive. In particular, the AngularJS staff client login
process does this, meaning that staff members can log in to the
staff client via the AngularJS form using an expired barcode.
This is not good. The OPAC explicitly blocks logging in using an
inactive barcode because it checks the identifier type and sets
the key passed to .complete accordingly. The Angular staff login
page also prevents logging in using an inactive barcode because
(a) it uses open-ils.auth.login, which doesn't have the same
problem and (b) it forces the identifier to be marked as a user
name regardless.
NOTE: this means that the Angular staff login form prevents staff
from logging in via barcode, which potentially is a regression as
compared to the AngularJS side (or, alternatively, is providing
additional necessary strictness).
This patch avoids the problem by having .complete inspect the
cached seed created by .init to determine how the user was ultimately
found.
Some alternative approaches that were rejected include:
[1] Having AngularJS just mirror Angular. Problem: if some staff
users are used to using their barcode to log in, doing
this would cause an immediate problem. I note that because
the staff interface URL is commonly expressed as
https://library.example/eg/staff, is currently far more common
for the staff interface to be logged into via the AngularJS
form rather than the Angular one.
[2] Having AngularJS use open-ils.auth.login, but make it and
Angular use 'identifier' as the key rather than 'username'.
Problem: while this would have the desired effect if you
only use native authentication, if you're using open-ils.auth_proxy,
it won't work - open-ils.auth_proxy.login doesn't recognize an
'identifier' parameter. While that could be changed, it
is more invasive.
To test
-------
[1] Set up a staff user that has a username, an active barcode,
and an inactive barcode.
[2] Log in to the AngularJS staff interface (/eg/staff) using
the username, the active barcode, and the inactive one.
[3] Note that you are permitted to log in with all three identifiers.
[4] Apply the patch and repeat step 2.
[5] This time, logging in using the inactive barcode should
fail.
[6] Verify that other login types continue to work as expected:
- Angular staff login form
- OPAC
- SIP2 terminal login
- SIP2 user authentication
- operator change (Angular and AngularJS)
- Web-based self-check
[7] Extra credit: test logging in via open-ils.auth_proxy with
it falling back to native authentication.
Signed-off-by: Galen Charlton <gmc@equinoxOLI.org>
Signed-off-by: Shula Link <slink@gchrl.org>
Signed-off-by: Mike Rylander <mrylander@gmail.com>