From 732b14aa25d7ba09763daf2734bdb18962aae15d Mon Sep 17 00:00:00 2001
From: Bill Erickson <berickxx@gmail.com>
Date: Tue, 26 Jan 2016 14:16:06 -0500
Subject: [PATCH] LP#1468422 Return vanilla login failure on nonexistent
 username/barcode

For backwards compatibility (and security), return the same login
failure for nonexistent usernames/barcodes as for bad passwords, etc.

Signed-off-by: Bill Erickson <berickxx@gmail.com>
Signed-off-by: Dan Wells <dbw2@calvin.edu>
---
 Open-ILS/src/c-apps/oils_auth.c | 70 +++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 30 deletions(-)

diff --git a/Open-ILS/src/c-apps/oils_auth.c b/Open-ILS/src/c-apps/oils_auth.c
index cc42f47597..22cf96e94e 100644
--- a/Open-ILS/src/c-apps/oils_auth.c
+++ b/Open-ILS/src/c-apps/oils_auth.c
@@ -195,7 +195,13 @@ static char* oilsAuthBuildInitCache(
     char* count_key = va_list_to_string(
         "%s%s%s", OILS_AUTH_CACHE_PRFX, ident, OILS_AUTH_COUNT_SFFX);
 
-    char* auth_seed = oilsAuthGetSalt(user_id);
+    char* auth_seed;
+    if (user_id == -1) {
+        // user does not exist.  Use a dummy seed
+        auth_seed = strdup("x");
+    } else {
+        auth_seed = oilsAuthGetSalt(user_id);
+    }
 
     jsonObject* seed_object = jsonParseFmt(
         "{\"%s\":\"%s\",\"user_id\":%d,\"seed\":\"%s\"}",
@@ -207,7 +213,12 @@ static char* oilsAuthBuildInitCache(
     }
 
     osrfCachePutObject(cache_key, seed_object, _oilsAuthSeedTimeout);
-    osrfCachePutObject(count_key, count_object, _oilsAuthBlockTimeout);
+
+    if (user_id != -1) {
+        // Only track login counts for existing users, since a 
+        // login for a nonexistent user will never succeed anyway.
+        osrfCachePutObject(count_key, count_object, _oilsAuthBlockTimeout);
+    }
 
     osrfLogDebug(OSRF_LOG_MARK, 
         "oilsAuthInit(): has seed %s and key %s", auth_seed, cache_key);
@@ -226,26 +237,18 @@ static int oilsAuthInitUsernameHandler(
     osrfLogInfo(OSRF_LOG_MARK, 
         "User logging in with username %s", username);
 
+    int user_id = -1;
     jsonObject* resp = NULL; // free
     jsonObject* user_obj = oilsUtilsFetchUserByUsername(username); // free
 
-    if (user_obj) {
-
-        if (JSON_NULL == user_obj->type) { // user not found
-            resp = jsonNewObject("x");
+    if (user_obj && user_obj->type != JSON_NULL) 
+        user_id = oilsFMGetObjectId(user_obj);
 
-        } else {
-            char* seed = oilsAuthBuildInitCache(
-                oilsFMGetObjectId(user_obj), username, "username", nonce);
-            resp = jsonNewObject(seed);
-            free(seed);
-        }
-
-        jsonObjectFree(user_obj);
+    jsonObjectFree(user_obj); // NULL OK
 
-    } else {
-        resp = jsonNewObject("x");
-    }
+    char* seed = oilsAuthBuildInitCache(user_id, username, "username", nonce);
+    resp = jsonNewObject(seed);
+    free(seed);
 
     osrfAppRespondComplete(ctx, resp);
     jsonObjectFree(resp);
@@ -276,23 +279,18 @@ static int oilsAuthInitBarcodeHandler(
     osrfLogInfo(OSRF_LOG_MARK, 
         "User logging in with barcode %s", barcode);
 
+    int user_id = -1;
     jsonObject* resp = NULL; // free
     jsonObject* user_obj = oilsUtilsFetchUserByBarcode(barcode); // free
 
-    if (user_obj) {
-        if (JSON_NULL == user_obj->type) { // not found
-            resp = jsonNewObject("x");
-        } else {
-            char* seed = oilsAuthBuildInitCache(
-                oilsFMGetObjectId(user_obj), barcode, "barcode", nonce);
-            resp = jsonNewObject(seed);
-            free(seed);
-        }
+    if (user_obj && user_obj->type != JSON_NULL) 
+        user_id = oilsFMGetObjectId(user_obj);
 
-        jsonObjectFree(user_obj);
-    } else {
-        resp = jsonNewObject("x");
-    }
+    jsonObjectFree(user_obj); // NULL OK
+
+    char* seed = oilsAuthBuildInitCache(user_id, barcode, "barcode", nonce);
+    resp = jsonNewObject(seed);
+    free(seed);
 
     osrfAppRespondComplete(ctx, resp);
     jsonObjectFree(resp);
@@ -630,6 +628,17 @@ int oilsAuthComplete( osrfMethodContext* ctx ) {
     int user_id = jsonObjectGetNumber(
         jsonObjectGetKeyConst(cacheObj, "user_id"));
 
+    if (user_id == -1) {
+        // User was not found during init.  Clean up and exit early.
+        response = oilsNewEvent(
+            __FILE__, harmless_line_number, OILS_EVENT_AUTH_FAILED);
+        osrfAppRespondComplete(ctx, oilsEventToJSON(response));
+        oilsEventFree(response); // frees event JSON
+        osrfCacheRemove(cache_key);
+        jsonObjectFree(cacheObj);
+        return 0;
+    }
+
     jsonObject* param = jsonNewNumberObject(user_id); // free
     userObj = oilsUtilsCStoreReq(
         "open-ils.cstore.direct.actor.user.retrieve", param);
@@ -747,6 +756,7 @@ int oilsAuthComplete( osrfMethodContext* ctx ) {
     oilsEventFree(response);
     jsonObjectFree(userObj);
     jsonObjectFree(authEvt);
+    jsonObjectFree(cacheObj);
     if(freeable_uname)
         free(freeable_uname);
 
-- 
2.43.2