From 872ab062a36f6b962136fa127f705994e2400d75 Mon Sep 17 00:00:00 2001
From: scottmk <scottmk@dcc99617-32d9-48b4-a31d-7c20da2025e4>
Date: Thu, 18 Mar 2010 20:12:40 +0000
Subject: [PATCH] Performance tweak to oilsAuthComplete().
Formerly, if authenticating a barcode rather tnan a user name, we would
look up the same barcode twice in actor.card: first to get the user id
for a lookup in actor.usr, and again to determine whether the card was
active. Now we use a single lookup for both purposes. In other respects
the validation logic is unchanged.
The oilsAuthCheckCard() function, which performed the second lookup, has
been eliminated.
The oilsUtilsFetchUserByBarcode() function, in oils_utils.c, is no
longer called from anywhere and may be eliminated.
Also added a doxygen-style comment block to document oilsUtilsFetchUserByBarcode(),
and tweaked few comments in the body of the function.
M Open-ILS/src/c-apps/oils_auth.c
git-svn-id: svn://svn.open-ils.org/ILS/trunk@15916 dcc99617-32d9-48b4-a31d-7c20da2025e4
---
Open-ILS/src/c-apps/oils_auth.c | 92 +++++++++++++++++++++++++++--------------
1 file changed, 60 insertions(+), 32 deletions(-)
diff --git a/Open-ILS/src/c-apps/oils_auth.c b/Open-ILS/src/c-apps/oils_auth.c
index 27b290b10f..bc5a4407d4 100644
--- a/Open-ILS/src/c-apps/oils_auth.c
+++ b/Open-ILS/src/c-apps/oils_auth.c
@@ -390,31 +390,32 @@ static oilsEvent* oilsAuthVerifyWorkstation(
-/* see if the card used to login is marked as barred */
-static oilsEvent* oilsAuthCheckCard( const char* barcode ) {
- if(!barcode) return NULL;
- osrfLogDebug(OSRF_LOG_MARK, "Checking to see if barcode %s is active", barcode);
-
- jsonObject* params = jsonParseFmt("{\"barcode\":\"%s\"}", barcode);
- jsonObject* card = oilsUtilsQuickReq(
- "open-ils.cstore", "open-ils.cstore.direct.actor.card.search", params );
- jsonObjectFree(params);
-
- char* active = oilsFMGetString(card, "active");
- jsonObjectFree(card);
-
- oilsEvent* return_event = NULL;
- if( ! oilsUtilsIsDBTrue(active) ) {
- osrfLogInfo(OSRF_LOG_MARK, "barcode %s is not active, returning event", barcode);
- return_event = oilsNewEvent(OSRF_LOG_MARK, "PATRON_CARD_INACTIVE");
- }
+/**
+ @brief Implement the "complete" method.
+ @param ctx The method context.
+ @return -1 upon error; zero if successful, and if a STATUS message has been sent to the
+ client to indicate completion; a positive integer if successful but no such STATUS
+ message has been sent.
- free(active);
- return return_event;
-}
+ Method parameters:
+ - a hash with some combination of the following elements:
+ - "username"
+ - "barcode"
+ - "password" (hashed with the cached seed; not plaintext)
+ - "type"
+ - "org"
+ - "workstation"
+ The password is required. Either a username or a barcode must also be present.
+
+ Return to client: Intermediate authentication seed.
+ Validate the password, using the username if available, or the barcode if not. The
+ user must be active, and not barred from logging on. The barcode, if used for
+ authentication, must be active as well. The workstation, if specified, must be valid.
+ Upon deciding whether to allow the logon, return a corresponding event to the client.
+*/
int oilsAuthComplete( osrfMethodContext* ctx ) {
OSRF_METHOD_VERIFY_CONTEXT(ctx);
@@ -439,6 +440,7 @@ int oilsAuthComplete( osrfMethodContext* ctx ) {
oilsEvent* response = NULL;
jsonObject* userObj = NULL;
+ int card_active = 1; // boolean; assume active until proven otherwise
// Fetch a row from the actor.usr table, by username if available,
// or by barcode if not.
@@ -449,8 +451,32 @@ int oilsAuthComplete( osrfMethodContext* ctx ) {
userObj = NULL; // username not found
}
}
- else if(barcode)
- userObj = oilsUtilsFetchUserByBarcode( barcode );
+ else if(barcode) {
+ // Read from actor.card by barcode
+
+ osrfLogInfo( OSRF_LOG_MARK, "Fetching user by barcode %s", barcode );
+
+ jsonObject* params = jsonParseFmt("{\"barcode\":\"%s\"}", barcode);
+ jsonObject* card = oilsUtilsQuickReq(
+ "open-ils.cstore", "open-ils.cstore.direct.actor.card.search", params );
+ jsonObjectFree( params );
+
+ if( card ) {
+ // Determine whether the card is active
+ char* card_active_str = oilsFMGetString( card, "active" );
+ card_active = oilsUtilsIsDBTrue( card_active_str );
+ free( card_active_str );
+
+ // Look up the user who owns the card
+ char* userid = oilsFMGetString( card, "usr" );
+ jsonObjectFree( card );
+ params = jsonParseFmt( "[%s]", userid );
+ free( userid );
+ userObj = oilsUtilsQuickReq(
+ "open-ils.cstore", "open-ils.cstore.direct.actor.user.retrieve", params );
+ jsonObjectFree( params );
+ }
+ }
if(!userObj) {
response = oilsNewEvent( OSRF_LOG_MARK, OILS_EVENT_AUTH_FAILED );
@@ -489,23 +515,25 @@ int oilsAuthComplete( osrfMethodContext* ctx ) {
}
free(active);
- /* then see if the barcode they used is active */
- if( barcode && ctx && userObj && (response = oilsAuthCheckCard( barcode )) ) {
- osrfAppRespondComplete( ctx, oilsEventToJSON(response) );
- oilsEventFree(response);
- jsonObjectFree(userObj);
+ osrfLogInfo( OSRF_LOG_MARK, "Fetching card by barcode %s", barcode );
+
+ if( !card_active ) {
+ osrfLogInfo( OSRF_LOG_MARK, "barcode %s is not active, returning event", barcode );
+ response = oilsNewEvent( OSRF_LOG_MARK, "PATRON_CARD_INACTIVE" );
+ osrfAppRespondComplete( ctx, oilsEventToJSON( response ) );
+ oilsEventFree( response );
+ jsonObjectFree( userObj );
return 0;
}
- /* check to see if the user is even allowed to login */
+ // See if the user is even allowed to log in
if( oilsAuthCheckLoginPerm( ctx, userObj, type ) == -1 ) {
jsonObjectFree(userObj);
return 0;
}
-
- /* if a workstation is defined, flesh the user with the workstation info */
+ // If a workstation is defined, add the workstation info
if( workstation != NULL ) {
osrfLogDebug(OSRF_LOG_MARK, "Workstation is %s", workstation);
response = oilsAuthVerifyWorkstation( ctx, userObj, workstation );
@@ -517,7 +545,7 @@ int oilsAuthComplete( osrfMethodContext* ctx ) {
}
} else {
- /* otherwise, use the home org as the workstation org on the user */
+ // Otherwise, use the home org as the workstation org on the user
char* orgid = oilsFMGetString(userObj, "home_ou");
oilsFMSetString(userObj, "ws_ou", orgid);
free(orgid);
--
2.11.0