A one-call login API would be swell

Bug #1710949 reported by Bill Erickson
This bug affects 1 person
Affects Status Importance Assigned to Milestone

Bug Description

Evergreen 2.12 / Wishlist

The Evergreen login process currently requires 2 API calls, with an intermediate layer of MD5 password hashing. This process is unnecessarily complex. It complicates client code by requiring an MD5 library and raises the bar to entry for 3rd-party services needing to authenticate with Evergreen.

I propose a new API call open-ils.auth.login (thanks Jason Stephenson for name suggestion) which performs both parts of open-ils.auth.authenticate.init and open-ils.auth.authenticate.complete. It will use the same parameters as .complete, except that it will expect the un-hashed/bare password and will have no use for a 'nonce' value.

For example:

srfsh# request open-ils.auth open-ils.auth.login

We also have a chance to improve how we use the generic "identifier" parameter. The "identifier" is passed by the caller when the caller wants the server to determine if a value is a username or barcode. The existing .init API is limited by checking the barcode regex setting at the root org unit, since it does not support an "org" parameter. With the new API, a combination of "identifier" and "org" allows the server to reliably determine whether a value is a username or barcode, in the same way that (for example) the TPAC does. The barcode regex check would no longer need to happen in the UI code.

Code en route.

Revision history for this message
Galen Charlton (gmc) wrote :

Yeah, it's probably time for this.

One of the advantages of CHAP was its ability to work over an unencrypted transport. Of course, nowadays we have much more assurance that TLS will be in the mix, but it might be a niceness to take this as an opportunity to teach the gateway/translator that designated APIs should not be available if it thinks that the request is being served over HTTP (or if the admin hasn't flipped a setting to indicate that a particular Apache instance is sitting behind a proxy that's taking care of endpoint HTTPS).

Revision history for this message
Bill Erickson (berick) wrote :

Code w/ Perl live test pushed. Release notes pending.


This code has no effect on existing API's. They will continue to function as before.

From the commit:

Adds a new open-ils.auth API call 'open-ils.auth.login' which performs the combined steps of open-ils.auth.authenticate.init and open-ils.auth.authenticate.complete so the caller only need call one API to login.

API params are consistent with open-ils.auth.authenticate.complete with 2 notable exceptions. The API uses the bare password instead of the hashed password, so the caller need not perform the extra hashing steps. Also, no 'nonce' parameter is used as it's no longer needed, because there is no intermediate authentication cache object as with .init + .complete.

Using the generic "identifier" parameter in combination with the "org" parameter allows the API to reliably determine if a value is a username or barcode. This means by default users should use the "identifier" value (with an "org" value) unless they already know for certain a username or barcode is being used.

Response data is consistent with open-ils.auth.authenticate.complete.


srfsh# request open-ils.auth open-ils.auth.login {"identifier":"admin","password":"fakepassword", "org":4}

Other changes in the new code:

1. Once a caller has reached the maximum number of allowed login failures in a give time frame, no further attempts to track failures occurs, based on the idea that no additional cpu/network cycles should be wasted on a lost cause and counting past the max serves no functional purpose.

2. A failure count object is only added to memcache when failures occur, unlike open-ils.auth.authenticate.init which creates a failure tracking object for every login.

3. The code avoids use of the jsonParseFmt() and va_list_to_string() functions as these functions require extra data cleansing.

Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Galen.

I wouldn't want to give the impression that using .init and .complete unencrypted is OK, though. It scrambles the password, but not the auth tokens or sensitive data that usually accompany them. There really are very few scenarios where talking to the gateways unencrypted is OK. Perhaps I should open a ticket for requiring SSL to the gateways?

On a related note, I've pushed a commit to add open-ils.auth.login to the list of APIs whose params should be redacted in the logs.

Revision history for this message
Bill Erickson (berick) wrote :

Release notes pushed. Adding pullrequest.

tags: added: pullrequest
Changed in evergreen:
milestone: none → 3.0-alpha
assignee: Bill Erickson (berick) → nobody
Galen Charlton (gmc)
Changed in evergreen:
assignee: nobody → Galen Charlton (gmc)
Revision history for this message
Galen Charlton (gmc) wrote :

Looks good. Signed off with a follow-up that tests blocking after too many failed attempts. Branch is user/gmcharlt/lp1710949_signoff

tags: added: signedoff
Changed in evergreen:
assignee: Galen Charlton (gmc) → nobody
Changed in evergreen:
assignee: nobody → Mike Rylander (mrylander)
Revision history for this message
Mike Rylander (mrylander) wrote :

Final review, tests are happy. Merged for great justice (and non-SSL risk... ;-) )!

Changed in evergreen:
assignee: Mike Rylander (mrylander) → nobody
status: New → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers