From: Bill Erickson <berickxx@gmail.com>
Date: Mon, 11 Jan 2016 16:02:23 +0000 (-0500)
Subject: LP#1468422 Report inactive card on password OK
X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=be3e4e339a8de50edc112a267bff2f11d35de2ba;p=evergreen%2Ftadl.git

LP#1468422 Report inactive card on password OK

Prevent leaking information from authentication by only reporting that a
card is inactive if the caller provided the correct credentials.  This
is consistent with how the code handles inactive patrons.

To avoid a lot of code duplication and to reduce the potential for
leaking memory (C code, amiright?), this commit includes a number of
changes to avoid exiting the API function early and saving the memory
cleanup routines until the end of the API call.

Signed-off-by: Bill Erickson <berickxx@gmail.com>
Signed-off-by: Dan Wells <dbw2@calvin.edu>
---

diff --git a/Open-ILS/src/c-apps/oils_auth.c b/Open-ILS/src/c-apps/oils_auth.c
index 5486f3e53a..cc42f47597 100644
--- a/Open-ILS/src/c-apps/oils_auth.c
+++ b/Open-ILS/src/c-apps/oils_auth.c
@@ -647,6 +647,7 @@ int oilsAuthComplete( osrfMethodContext* ctx ) {
         jsonNewNumberObject(oilsFMGetObjectId(userObj)));
     jsonObjectSetKey(params,"org_unit", jsonNewNumberObject(orgloc));
     jsonObjectSetKey(params, "login_type", jsonNewObject(type));
+    if (barcode) jsonObjectSetKey(params, "barcode", jsonNewObject(barcode));
 
     jsonObject* authEvt = oilsUtilsQuickReq( // freed after password test
         "open-ils.auth_internal",
@@ -661,78 +662,93 @@ int oilsAuthComplete( osrfMethodContext* ctx ) {
         return -1;
     }
 
-    const char* evtCode = 
+    const char* authEvtCode = 
         jsonObjectGetString(jsonObjectGetKey(authEvt, "textcode"));
 
-    // For security/privacy sake, only report that a patron is 
-    // inactive if the correct password is provided below.
-    int user_inactive = !strcmp(evtCode, "PATRON_INACTIVE");
+    if (!strcmp(authEvtCode, OILS_EVENT_AUTH_FAILED)) {
+        // Received the generic login failure event.
 
-    if (strcmp(evtCode, "SUCCESS") && !user_inactive) { // validate failed
         osrfLogInfo(OSRF_LOG_MARK,  
             "failed login: username=%s, barcode=%s, workstation=%s",
             uname, (barcode ? barcode : "(none)"), ws);
-        osrfAppRespondComplete(ctx, authEvt);
-        jsonObjectFree(authEvt);
-        jsonObjectFree(userObj);
-        if(freeable_uname) free(freeable_uname);
-        return 0;           // No such user
+
+        response = oilsNewEvent(
+            __FILE__, harmless_line_number, OILS_EVENT_AUTH_FAILED);
     }
 
-	// Such a user exists and isn't barred or deleted.
-	// Now see if he or she has the right credentials.
-	int passOK = oilsAuthVerifyPassword(
-        ctx, user_id, identifier, password, nonce);
+    int passOK = 0;
+    
+    if (!response) {
+        // User exists and is not barred, etc.  Test the password.
 
-	if( passOK < 0 ) {
-        jsonObjectFree(authEvt);
-		jsonObjectFree(userObj);
-        if(freeable_uname) free(freeable_uname);
-		return passOK;
-	}
+        passOK = oilsAuthVerifyPassword(
+            ctx, user_id, identifier, password, nonce);
 
-    if (passOK && user_inactive) {
-        // Patron is inactive but provided the correct password.
-        // Return the original PATRON_INACTIVE event.
-        osrfAppRespondComplete(ctx, authEvt);
-        jsonObjectFree(authEvt);
-        jsonObjectFree(userObj);
-        if(freeable_uname) free(freeable_uname);
-        return 0;
+        if (!passOK) {
+            // Password check failed. Return generic login failure.
+
+            response = oilsNewEvent(
+                __FILE__, harmless_line_number, OILS_EVENT_AUTH_FAILED);
+
+            osrfLogInfo(OSRF_LOG_MARK,  
+                "failed login: username=%s, barcode=%s, workstation=%s",
+                    uname, (barcode ? barcode : "(none)"), ws );
+        }
+    }
+
+
+    // Below here, we know the password check succeeded if no response
+    // object is present.
+
+    if (!response && (
+        !strcmp(authEvtCode, "PATRON_INACTIVE") ||
+        !strcmp(authEvtCode, "PATRON_CARD_INACTIVE"))) {
+        // Patron and/or card is inactive but the correct password 
+        // was provided.  Alert the caller to the inactive-ness.
+        response = oilsNewEvent2(
+            OSRF_LOG_MARK, authEvtCode,
+            jsonObjectGetKey(authEvt, "payload")   // cloned within Event
+        );
     }
 
-    jsonObjectFree(authEvt); // we're all done with this now.
+    if (!response && strcmp(authEvtCode, OILS_EVENT_SUCCESS)) {
+        // Validate API returned an unexpected non-success event.
+        // To be safe, treat this as a generic login failure.
 
-	if( passOK ) { // login successful  
-        
-		char* ewhat = "login";
+        response = oilsNewEvent(
+            __FILE__, harmless_line_number, OILS_EVENT_AUTH_FAILED);
+    }
 
-		if (0 == strcmp(ctx->method->name, "open-ils.auth.authenticate.verify")) {
-			response = oilsNewEvent( OSRF_LOG_MARK, OILS_EVENT_SUCCESS );
-			ewhat = "verify";
+    if (!response) {
+        // password OK and no other events have prevented login completion.
 
-		} else {
-			response = oilsAuthHandleLoginOK( userObj, uname, type, orgloc, workstation );
-		}
+        char* ewhat = "login";
 
-		oilsUtilsTrackUserActivity(
-			oilsFMGetObjectId(userObj), 
-			ewho, ewhat, 
-			osrfAppSessionGetIngress()
-		);
+        if (0 == strcmp(ctx->method->name, "open-ils.auth.authenticate.verify")) {
+            response = oilsNewEvent( OSRF_LOG_MARK, OILS_EVENT_SUCCESS );
+            ewhat = "verify";
 
-	} else {
-		response = oilsNewEvent( __FILE__, harmless_line_number, OILS_EVENT_AUTH_FAILED );
-		osrfLogInfo(OSRF_LOG_MARK,  "failed login: username=%s, barcode=%s, workstation=%s",
-				uname, (barcode ? barcode : "(none)"), ws );
-	}
+        } else {
+            response = oilsAuthHandleLoginOK(
+                userObj, uname, type, orgloc, workstation);
+        }
+
+        oilsUtilsTrackUserActivity(
+            oilsFMGetObjectId(userObj), 
+            ewho, ewhat, 
+            osrfAppSessionGetIngress()
+        );
+    }
 
-	jsonObjectFree(userObj);
-	osrfAppRespondComplete( ctx, oilsEventToJSON(response) );
-	oilsEventFree(response);
+    // reply
+    osrfAppRespondComplete(ctx, oilsEventToJSON(response));
 
-	if(freeable_uname)
-		free(freeable_uname);
+    // clean up
+    oilsEventFree(response);
+    jsonObjectFree(userObj);
+    jsonObjectFree(authEvt);
+    if(freeable_uname)
+        free(freeable_uname);
 
 	return 0;
 }