make response field configurable

Author: adwk67

src/main/java/tech/stackable/hadoop/OpaQueryResult.java

@@ -5,6 +5,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.security.GroupMappingServiceProvider;
+import org.apache.hadoop.util.Lists;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -13,11 +14,18 @@
 import java.net.http.HttpClient;
 import java.net.http.HttpRequest;
 import java.net.http.HttpResponse;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
+import java.util.function.UnaryOperator;
 
 public class StackableGroupMapper implements GroupMappingServiceProvider {
     public static final String OPA_MAPPING_URL_PROP = "hadoop.security.group.mapping.opa.url";
+    public static final String OPA_MAPPING_GROUP_NAME_PROP = "hadoop.security.group.mapping.opa.list.name";
+    // response base field: see https://www.openpolicyagent.org/docs/latest/rest-api/#response-message
+    public static final String OPA_RESULT_FIELD = "result";
+    public final String mappingGroupName;
     private final Logger LOG = LoggerFactory.getLogger(StackableGroupMapper.class);
     private final HttpClient httpClient = HttpClient.newHttpClient();
     private final ObjectMapper json;
@@ -37,6 +45,11 @@ public StackableGroupMapper() {
             throw new OpaException.UriInvalid(opaUri, e);
         }
 
+        this.mappingGroupName = configuration.get(OPA_MAPPING_GROUP_NAME_PROP);
+        if (mappingGroupName == null) {
+            throw new RuntimeException("Config \"" + OPA_MAPPING_GROUP_NAME_PROP + "\" missing");
+        }
+
         this.json = new ObjectMapper()
                 // https://github.com/stackabletech/trino-opa-authorizer/issues/24
                 // OPA server can send other fields, such as `decision_id`` when enabling decision logs
@@ -82,13 +95,31 @@ public List<String> getGroups(String user) throws IOException {
 
         String responseBody = response.body();
         LOG.debug("Response body [{}]", responseBody);
+        List<String> groups = Lists.newArrayList();
+
+        @SuppressWarnings("unchecked")
+        Map<String, Object> result = (Map<String, Object>) json.readValue(responseBody, HashMap.class).get(OPA_RESULT_FIELD);
+        List<String> rawGroups = (List<String>) result.get(this.mappingGroupName);
+
+        for (String rawGroup :  rawGroups) {
+            groups.add(removeSlashes.apply(rawGroup));
+        }
 
-        OpaQueryResult result = json.readValue(responseBody, OpaQueryResult.class);
-        LOG.info("Groups for [{}]: [{}]", user, result.groups);
+        LOG.info("Groups for [{}]: [{}]", user, groups);
 
-        return result.groups;
+        return groups;
     }
 
+    private final static UnaryOperator<String> removeSlashes = s -> {
+        if (s.startsWith("/")) {
+            s = s.substring(1);
+        }
+        if (s.endsWith("/")) {
+            s = s.substring(0, s.length() - 1);
+        }
+        return s;
+    };
+
     /**
      * Caches groups, no need to do that for this provider
      */

src/main/java/tech/stackable/hadoop/StackableGroupMapper.java

@@ -7,18 +7,22 @@
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 public class StackableGroupMapperTest {
 
+    @SuppressWarnings("unchecked")
     @Test
     public void testJsonMapper() throws JsonProcessingException {
         String input = "{\n" +
                 "    \"result\": {\n" +
-                "        \"groups\": {\n" +
-                "            \"groups\": [\n" +
-                "                \"/admin\",\n" +
-                "                \"/superuser\"\n" +
-                "            ]\n" +
-                "        },\n" +
+                "          \"groups\": [\n" +
+                "              \"/admin\",\n" +
+                "              \"/superuser\"\n" +
+                "          ]\n" +
+                "        ,\n" +
                 "        \"users_by_name\": {\n" +
                 "            \"alice\": {\n" +
                 "                \"customAttributes\": {},\n" +
@@ -52,8 +56,10 @@ public void testJsonMapper() throws JsonProcessingException {
         ObjectMapper json = new ObjectMapper()
                 .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
                 .setSerializationInclusion(JsonInclude.Include.NON_NULL);
-        OpaQueryResult result = json.readValue(input, OpaQueryResult.class);
-        Assert.assertEquals("admin", result.groups.get(0));
-        Assert.assertEquals("superuser", result.groups.get(1));
+
+        Map<String, List<String>> result = (Map<String, List<String>>) json.readValue(input, HashMap.class).get("result");
+        List<String> groups = result.get("groups");
+        Assert.assertEquals("/admin", groups.get(0));
+        Assert.assertEquals("/superuser", groups.get(1));
     }
 }

src/test/java/tech/stackable/hadoop/StackableGroupMapperTest.java

@@ -11,13 +11,12 @@ data:
 
     # this will return the group data in this form:
     #    "result": {
-    #        "groups":
-    #            "groups": [
-    #                "/admin",
-    #                "/superuser"
-    #            ]
-    #        },...
-    groups := json.filter(users_by_name[input.username], ["groups"])
+    #        "groups": [
+    #            "/admin",
+    #            "/superuser"
+    #        ]
+    #        ...
+    groups := users_by_name[input.username].groups
 
     # returning data in the form presented by the UIF
     users_by_name := {

test/stack/05-opa.yaml

@@ -44,6 +44,7 @@ spec:
             # the mapper is only handled on the namenode so no need to apply this config to all roles
             hadoop.security.group.mapping: "tech.stackable.hadoop.StackableGroupMapper"
             hadoop.security.group.mapping.opa.url: "http://test-opa.default.svc.cluster.local:8081/v1/data/hdfs"
+            hadoop.security.group.mapping.opa.list.name: "groups"
             # The operator adds a default static mapping when kerberos is activated, see:
             # https://github.com/stackabletech/hdfs-operator/blob/main/rust/operator-binary/src/kerberos.rs#L97-L101
             # This should be removed so that the mapping implementation can provide this information instead: