From 872ab062a36f6b962136fa127f705994e2400d75 Mon Sep 17 00:00:00 2001 From: scottmk 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.43.2