Merge pull request #308 from kjsanger/feature/non-expanded-collection…

…-acl

Add non-expanded ACL reporting for collections

src/json.c

@@ -1254,14 +1254,15 @@ char *json_to_local_path(json_t *object, baton_error_t *error) {
 }
 
 void print_json_stream(json_t *json, FILE *stream) {
-    char *json_str = json_dumps(json, JSON_INDENT(0));
+    char *json_str = json_dumps(json, JSON_INDENT(0) | JSON_SORT_KEYS);
     if (json_str) {
         fprintf(stream, "%s\n", json_str);
         free(json_str);
     }
 
     return;
 }
+
 void print_json(json_t *json) {
     print_json_stream(json, stdout);
 

src/json_query.c

@@ -625,8 +625,12 @@ json_t *make_json_objects(genQueryOut_t *query_out, const char *labels[]) {
                     json_t *jvalue = json_pack("s", value);
                     if (!jvalue) goto error;
 
-                    // TODO: check return value
-                    json_object_set_new(jrow, labels[i], jvalue);
+                    int set = json_object_set_new(jrow, labels[i], jvalue);
+                    if (set != 0) {
+                        logmsg(ERROR, "Failed to set column %d '%s' value '%s' ",
+                               i, labels[i], value);
+                        goto error;
+                    }
                 }
             }
         }
@@ -1260,7 +1264,7 @@ json_t *map_access_args(json_t *query, baton_error_t *error) {
     return NULL;
 }
 
-json_t *revmap_access_result(json_t *acl,  baton_error_t *error) {
+json_t *revmap_access_result(json_t *acl, baton_error_t *error) {
     size_t num_elts;
 
     init_baton_error(error);

src/list.c

@@ -387,25 +387,9 @@ json_t *list_permissions(rcComm_t *conn, rodsPath_t *rods_path,
     // while a collections readable by public would show as
     //
     //     irods#testZone:own irods#testZone:read object
-    //     john#testZone:read object alice#testZone:read object
+    //     bob#testZone:read object alice#testZone:read object
     //
-    // where irods, john and alice are the members of "public".
-
-    // This reports groups unexpanded (substuting COL_DATA_USER_NAME
-    // for COL_USER_NAME reports constituent users):
-    query_format_in_t obj_format =
-        { .num_columns = 3,
-          .columns     = { COL_USER_NAME, COL_USER_ZONE,
-                           COL_DATA_ACCESS_NAME },
-          .labels      = { JSON_OWNER_KEY, JSON_ZONE_KEY, JSON_LEVEL_KEY } };
-
-    // This reports constituent users (substituing COL_USER_NAME
-    // for COL_COLL_USER_NAME causes incorrect reporting)
-    query_format_in_t col_format =
-        { .num_columns = 3,
-          .columns     = { COL_COLL_USER_NAME, COL_COLL_USER_ZONE,
-                           COL_COLL_ACCESS_NAME },
-          .labels      = { JSON_OWNER_KEY, JSON_ZONE_KEY, JSON_LEVEL_KEY } };
+    // where irods, bob and alice are the members of "public".
 
     init_baton_error(error);
 
@@ -420,17 +404,75 @@ json_t *list_permissions(rcComm_t *conn, rodsPath_t *rods_path,
         case DATA_OBJ_T:
             logmsg(TRACE, "Identified '%s' as a data object",
                    rods_path->outPath);
+
+            // This reports groups unexpanded (substuting COL_DATA_USER_NAME
+            // for COL_USER_NAME reports constituent users):
+            query_format_in_t obj_format =
+                { .num_columns = 3,
+                  .columns     = { COL_USER_NAME, COL_USER_ZONE,
+                                   COL_DATA_ACCESS_NAME },
+                  .labels      = { JSON_OWNER_KEY, JSON_ZONE_KEY,
+                                   JSON_LEVEL_KEY }};
+
             query_in = make_query_input(SEARCH_MAX_ROWS, obj_format.num_columns,
                                         obj_format.columns);
             query_in = prepare_obj_acl_list(query_in, rods_path);
+
+            // We need to add a zone hint to return results from other zones.
+            // Without it, we will only see ACLs in the current zone. The
+            // iRODS path seems to work for this purpose
+            addKeyVal(&query_in->condInput, ZONE_KW, rods_path->outPath);
+            logmsg(DEBUG, "Using zone hint '%s'", rods_path->outPath);
+            results = do_query(conn, query_in, obj_format.labels, error);
+            if (error->code != 0) goto error;
+
+            logmsg(DEBUG, "Obtained ACL data on '%s'", rods_path->outPath);
+            free_query_input(query_in);
+            if (error->code != 0) goto error;
             break;
 
         case COLL_OBJ_T:
             logmsg(TRACE, "Identified '%s' as a collection",
                    rods_path->outPath);
-            query_in = make_query_input(SEARCH_MAX_ROWS, col_format.num_columns,
-                                        col_format.columns);
-            query_in = prepare_col_acl_list(query_in, rods_path);
+
+            // This specific query reports the groups without expansion. It is used by ils,
+            // if available and is installed by default on iRODS servers.
+            genQueryOut_t *query_out = NULL;
+
+            // We don't use the queryCollAclSpecific function here because it leaks memory:
+            //
+            // int status = queryCollAclSpecific(conn, rods_path->outPath, rods_path->outPath,
+            //                                   &query_out);
+            //
+            // Instead call lower level rcSpecificQuery directly, avoiding the problem malloc.
+            specificQueryInp_t specificQueryInp;
+            memset(&specificQueryInp, 0, sizeof(specificQueryInp_t));
+            specificQueryInp.maxRows = MAX_SQL_ROWS;
+            specificQueryInp.continueInx = 0;
+            specificQueryInp.sql = "ShowCollAcls";
+            specificQueryInp.args[0] = rods_path->outPath;
+
+            addKeyVal(&specificQueryInp.condInput, ZONE_KW, rods_path->outPath);
+            logmsg(DEBUG, "Using zone hint '%s'", rods_path->outPath);
+            int status = rcSpecificQuery(conn, &specificQueryInp, &query_out);
+            if (status < 0) {
+                set_baton_error(error, status,
+                                "Failed to query ACL on '%s': error %d",
+                                rods_path->outPath, status);
+                goto error;
+            }
+
+            // The query returns 4 columns, the last being the user type (e.g. rodsgroup)
+            // but we only want the first 3, so temporarily decrement the attribute count
+            // to avoid reporting it.
+            query_out->attriCnt--;
+            results = make_json_objects(query_out,
+                 (const char *[]) { JSON_OWNER_KEY, JSON_ZONE_KEY, JSON_LEVEL_KEY });
+            // Restore the attribute count before calling the free function.
+            query_out->attriCnt++;
+
+            logmsg(DEBUG, "Obtained ACL data on '%s'", rods_path->outPath);
+            free_query_output(query_out);
             break;
 
         default:
@@ -441,17 +483,6 @@ json_t *list_permissions(rcComm_t *conn, rodsPath_t *rods_path,
             goto error;
     }
 
-    // We need to add a zone hint to return results from other zones.
-    // Without it, we will only see ACLs in the current zone. The
-    // iRODS path seems to work for this purpose
-    addKeyVal(&query_in->condInput, ZONE_KW, rods_path->outPath);
-    logmsg(DEBUG, "Using zone hint '%s'", rods_path->outPath);
-    results = do_query(conn, query_in, obj_format.labels, error);
-    if (error->code != 0) goto error;
-
-    logmsg(DEBUG, "Obtained ACL data on '%s'", rods_path->outPath);
-    free_query_input(query_in);
-
     results = revmap_access_result(results, error);
     if (error->code != 0) goto error;
 

tests/.irods/irods_environment.json

@@ -1,9 +1,9 @@
 {
+    "irods_default_hash_scheme": "MD5",
+    "irods_default_resource": "replResc",
+    "irods_home": "/testZone/home/irods",
     "irods_host": "irods-server",
     "irods_port": 1247,
     "irods_user_name": "irods",
-    "irods_zone_name": "testZone",
-    "irods_home": "/testZone/home/irods",
-    "irods_default_resource": "replResc",
-    "irods_default_hash_scheme": "MD5"
+    "irods_zone_name": "testZone"
 }

tests/check_baton.c

@@ -1650,7 +1650,6 @@ START_TEST(test_modify_json_permissions_obj) {
     int found = 0;
     for (int i = 0; i < num_elts; i++) {
         json_t *elt = json_array_get(acl_with_zone, i);
-
         // Return value should equal the input
         ck_assert(json_is_object(elt));
         if (json_equal(perm_with_zone, elt)) found = 1;