From: Bill Erickson <berickxx@gmail.com>
Date: Wed, 27 Jan 2016 17:11:33 +0000 (-0500)
Subject: LP#1468422 Auth efficiency improvements
X-Git-Tag: sprint4-merge-nov22~520
X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=c427559c208c173700d622610fe440225550eb54;p=working%2FEvergreen.git

LP#1468422 Auth efficiency improvements

1. Adds an oils_utils function for retrieving the ID of the root org
unit.

2. Avoid multiple cstore/db lookups for the root org unit by caching the
ID at the process level.

3. Move permission checks from open-ils.storage to open-ils.cstore.

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

diff --git a/Open-ILS/include/openils/oils_utils.h b/Open-ILS/include/openils/oils_utils.h
index 3554d21d0b..6b391f2323 100644
--- a/Open-ILS/include/openils/oils_utils.h
+++ b/Open-ILS/include/openils/oils_utils.h
@@ -110,6 +110,11 @@ long oilsUtilsIntervalToSeconds( const char* interval );
  */
 int oilsUtilsTrackUserActivity( long usr, const char* ewho, const char* ewhat, const char* ehow );
 
+/**
+ * Returns the ID of the root org unit (parent_ou = NULL)
+ */
+int oilsUtilsGetRootOrgId();
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/Open-ILS/src/c-apps/oils_auth.c b/Open-ILS/src/c-apps/oils_auth.c
index 22cf96e94e..fd9b5e74fa 100644
--- a/Open-ILS/src/c-apps/oils_auth.c
+++ b/Open-ILS/src/c-apps/oils_auth.c
@@ -319,18 +319,10 @@ int oilsAuthInitBarcode(osrfMethodContext* ctx) {
 // returns true if the provided identifier matches the barcode regex.
 static int oilsAuthIdentIsBarcode(const char* identifier) {
 
-    // before we can fetch the barcode regex unit setting,
-    // first determine what the root org unit ID is.
+    // Assumes barcode regex is a global setting.
     // TODO: add an org_unit param to the .init API for future use?
-    
-    jsonObject *params = jsonParse("{\"parent_ou\":null}");
-    jsonObject *org_unit_id = oilsUtilsCStoreReq(
-        "open-ils.cstore.direct.actor.org_unit.id_list", params);
-    jsonObjectFree(params);
-
     char* bc_regex = oilsUtilsFetchOrgSetting(
-        (int) jsonObjectGetNumber(org_unit_id), "opac.barcode_regex");
-    jsonObjectFree(org_unit_id);
+        oilsUtilsGetRootOrgId(), "opac.barcode_regex");
 
     if (!bc_regex) {
         // if no regex is set, assume any identifier starting
diff --git a/Open-ILS/src/c-apps/oils_utils.c b/Open-ILS/src/c-apps/oils_utils.c
index cb334acb6d..155274f99c 100644
--- a/Open-ILS/src/c-apps/oils_utils.c
+++ b/Open-ILS/src/c-apps/oils_utils.c
@@ -164,49 +164,65 @@ int oilsUtilsTrackUserActivity(long usr, const char* ewho, const char* ewhat, co
 }
 
 
+static int rootOrgId = 0; // cache the ID of the root org unit.
+int oilsUtilsGetRootOrgId() {
+
+    // return the cached value if we have it.
+    if (rootOrgId > 0) return rootOrgId;
+
+    jsonObject* where_clause = jsonParse("{\"parent_ou\":null}");
+    jsonObject* org = oilsUtilsQuickReq(
+        "open-ils.cstore",
+        "open-ils.cstore.direct.actor.org_unit.search",
+        where_clause
+    );
+
+    rootOrgId = (int) 
+        jsonObjectGetNumber(oilsFMGetObject(org, "id"));
+
+    jsonObjectFree(where_clause);
+    jsonObjectFree(org);
+
+    return rootOrgId;
+}
 
 oilsEvent* oilsUtilsCheckPerms( int userid, int orgid, char* permissions[], int size ) {
-	if (!permissions) return NULL;
-	int i;
-	oilsEvent* evt = NULL;
-
-	// Find the root org unit, i.e. the one with no parent.
-	// Assumption: there is only one org unit with no parent.
-	if (orgid == -1) {
-		jsonObject* where_clause = jsonParse( "{\"parent_ou\":null}" );
-		jsonObject* org = oilsUtilsQuickReq(
-			"open-ils.cstore",
-			"open-ils.cstore.direct.actor.org_unit.search",
-			where_clause
-		);
-		jsonObjectFree( where_clause );
-
-		orgid = (int)jsonObjectGetNumber( oilsFMGetObject( org, "id" ) );
-
-		jsonObjectFree(org);
-	}
+    if (!permissions) return NULL;
+    int i;
 
-	for( i = 0; i < size && permissions[i]; i++ ) {
+    // Check perms against the root org unit if no org unit is provided.
+    if (orgid == -1)
+        orgid = oilsUtilsGetRootOrgId();
 
-		char* perm = permissions[i];
-		jsonObject* params = jsonParseFmt("[%d, \"%s\", %d]", userid, perm, orgid);
-		jsonObject* o = oilsUtilsQuickReq( "open-ils.storage",
-			"open-ils.storage.permission.user_has_perm", params );
+    for( i = 0; i < size && permissions[i]; i++ ) {
+        oilsEvent* evt = NULL;
+        char* perm = permissions[i];
 
-		char* r = jsonObjectToSimpleString(o);
+        jsonObject* params = jsonParseFmt(
+            "{\"from\":[\"permission.usr_has_perm\",\"%d\",\"%s\",\"%d\"]}",
+            userid, perm, orgid
+        );
 
-		if(r && !strcmp(r, "0"))
-			evt = oilsNewEvent3( OSRF_LOG_MARK, OILS_EVENT_PERM_FAILURE, perm, orgid );
+        // Execute the query
+        jsonObject* result = oilsUtilsCStoreReq(
+            "open-ils.cstore.json_query", params);
 
-		jsonObjectFree(params);
-		jsonObjectFree(o);
-		free(r);
+        const jsonObject* hasPermStr = 
+            jsonObjectGetKeyConst(result, "permission.usr_has_perm");
 
-		if(evt)
-			break;
-	}
+        if (!oilsUtilsIsDBTrue(jsonObjectGetString(hasPermStr))) {
+            evt = oilsNewEvent3(
+                OSRF_LOG_MARK, OILS_EVENT_PERM_FAILURE, perm, orgid);
+        }
+
+        jsonObjectFree(params);
+        jsonObjectFree(result);
+
+        // return first failed permission check.
+        if (evt) return evt;
+    }
 
-	return evt;
+    return NULL; // all perm checks succeeded
 }
 
 /**