[ENHANCEMENT] issue#8 : throw runtime error if configuration is incorrect.

Author: Simone Scarduzio

src/main/java/org/elasticsearch/rest/action/readonlyrest/ReadonlyRestAction.java

@@ -13,6 +13,7 @@
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.rest.action.readonlyrest.acl.ACL;
 import org.elasticsearch.rest.action.readonlyrest.acl.ACLRequest;
+import org.elasticsearch.rest.action.readonlyrest.acl.RuleConfigurationError;
 
 /**
  * Readonly REST plugin. Adding some access control to the fast Netty based REST interface of Elasticsearch.
@@ -48,8 +49,9 @@ public ReadonlyRestAction(final Settings settings, Client client, RestController
       acl = new ACL(logger, settings);
       logger.info("ACL configuration: OK");
     }
-    catch (Exception e) {
+    catch (RuleConfigurationError e) {
       logger.error("impossible to initialize ACL configuration", e);
+      throw e;
     }
     controller.registerFilter(new RestFilter() {
 
@@ -61,6 +63,7 @@ public void process(RestRequest request, RestChannel channel, RestFilterChain fi
           ok(request, filterChain, channel);
         }
         else {
+          logger.trace("forbidden request: " + aclReq + " Reason: " + reason);
           if(conf.forbiddenResponse != null){
             reason = conf.forbiddenResponse;
           }

src/main/java/org/elasticsearch/rest/action/readonlyrest/acl/ACL.java

@@ -14,13 +14,13 @@ public class ACL {
   private TreeMap<Integer, Rule>           rules  = new TreeMap<>();
   private final static String PREFIX = "readonlyrest.access_control_rules";
 
-  public ACL(ESLogger logger, Settings s) throws Exception {
+  public ACL(ESLogger logger, Settings s) {
     this.logger = logger;
     this.s = s;
     readRules();
   }
 
-  private void readRules() throws Exception {
+  private void readRules() {
     Map<String, Settings> g = s.getGroups(PREFIX);
     // Maintaining the order is not guaranteed, moving everything to tree map!
     TreeMap<String, Settings> tmp = new TreeMap<>();
@@ -39,7 +39,7 @@ private void readRules() throws Exception {
    * Check the request against configured ACL rules. This does not work with try/catch because stacktraces are expensive
    * for performance.
    * 
-   * @param r
+   * @param req the ACLRequest to be checked by the ACL rules.
    * @return null if request pass the rules or the name of the first violated rule
    */
   public String check(ACLRequest req) {

src/main/java/org/elasticsearch/rest/action/readonlyrest/acl/Rule.java

@@ -12,6 +12,15 @@ public class Rule {
 
   public enum Type {
     ALLOW, FORBID;
+
+    public static String valuesString(){
+      StringBuilder sb = new StringBuilder();
+      for(Type v: values()){
+        sb.append(v.toString()).append(",");
+      }
+      sb.deleteCharAt(sb.length()-1);
+      return sb.toString();
+    }
   }
 
   String               name;
@@ -32,25 +41,32 @@ public Rule(String name, Type type, Pattern uri_re, Integer bodyLenght, List<Str
     this.stringRepresentation = toString;
   }
 
-  public static Rule build(Settings s) throws Exception {
+  public static Rule build(Settings s) {
     List<String> hosts = null;
     String[] a = s.getAsArray("hosts");
     if (a != null && a.length > 0) {
-      hosts = Lists.newArrayList(a);
-      for (String address : a) {
-        address = address.trim();
+      hosts = Lists.newArrayList();
+      for (int i=0; i < a.length; i++) {
+        if(!ConfigurationHelper.isNullOrEmpty(a[i])) {
+          hosts.add(a[i].trim());
+        }
       }
     }
 
     a = s.getAsArray("methods");
     List<Method> methods = null;
     if (a != null && a.length > 0) {
-      for (String string : a) {
-        Method m = Method.valueOf(string.trim().toUpperCase());
-        if (methods == null) {
-          methods = Lists.newArrayList();
+      try {
+        for (String string : a) {
+          Method m = Method.valueOf(string.trim().toUpperCase());
+          if (methods == null) {
+            methods = Lists.newArrayList();
+          }
+          methods.add(m);
         }
-        methods.add(m);
+      }
+      catch(Throwable t){
+        throw new RuleConfigurationError("Invalid HTTP method found in configuration " + a, t);
       }
     }
 
@@ -60,13 +76,18 @@ public static Rule build(Settings s) throws Exception {
       uri_re = Pattern.compile(tmp.trim());
     }
     String name = s.get("name");
-    Rule.Type type = Type.valueOf(s.get("type").toUpperCase());
-    Integer maxBodyLenght = s.getAsInt("maxBodyLength", null);
+
+    String sType = s.get("type");
+    if(sType == null) {
+      throw new RuleConfigurationError("The field \"type\" is mandatory and should be either of " + Type.valuesString() + ". If this field is correct, check the YAML indentation is correct.", null);
+    }
+    Rule.Type type = Type.valueOf(sType.toUpperCase());
+    Integer maxBodyLength = s.getAsInt("maxBodyLength", null);
     if ((!ConfigurationHelper.isNullOrEmpty(name) && type != null) && 
-        (uri_re != null || maxBodyLenght != null || hosts != null || methods != null)) {
-      return new Rule(name.trim(), type, uri_re, maxBodyLenght, hosts, methods, s.toDelimitedString(' '));
+        (uri_re != null || maxBodyLength != null || hosts != null || methods != null)) {
+      return new Rule(name.trim(), type, uri_re, maxBodyLength, hosts, methods, s.toDelimitedString(' '));
     }
-    throw new Exception("insufficient or invalid configuration for rule: '" + name + "'");
+    throw new RuleConfigurationError("insufficient or invalid configuration for rule: '" + name + "'", null);
 
   }
 

src/main/java/org/elasticsearch/rest/action/readonlyrest/acl/RuleConfigurationError.java

@@ -0,0 +1,10 @@
+package org.elasticsearch.rest.action.readonlyrest.acl;
+
+/**
+ * Created by sscarduzio on 12/07/2015.
+ */
+public class RuleConfigurationError extends RuntimeException {
+    public RuleConfigurationError(String msg, Throwable cause) {
+        super(msg, cause);
+    }
+}

src/test/issue8.yml

@@ -0,0 +1,34 @@
+cluster:
+  name: elasticsearch
+
+index:
+    number_of_replicas: 0
+    number_of_shards: 1
+    analysis:
+        analyzer:
+            eulang:
+                type: custom
+                tokenizer: standard
+                filter: [standard, lowercase, asciifolding]
+            location:
+                type: custom
+                tokenizer: standard
+                filter: [standard, lowercase, asciifolding]
+
+transport.tcp.port : 9310
+
+readonlyrest:
+     # (De)activate plugin
+     enable: true
+
+     # HTTP response body in case of forbidden request.
+     # If this is null or omitted, the name of the first violated access control rule is returned (useful for debugging!)
+     response_if_req_forbidden: Sorry, your request is forbidden
+
+     # Default policy is to forbid everything, let's define a whitelist
+     access_control_rules:
+
+     - name: restricted access to all other hosts
+     type: allow
+     methods: [OPTIONS,GET]
+     maxBodyLength: 0

src/test/java/org/elasticsearch/rest/action/readonlyrest/acl/test/Issue8ACLTest.java

@@ -0,0 +1,29 @@
+package org.elasticsearch.rest.action.readonlyrest.acl.test;
+
+import org.elasticsearch.common.base.Charsets;
+import org.elasticsearch.common.logging.ESLoggerFactory;
+import org.elasticsearch.common.settings.ImmutableSettings;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.rest.RestRequest.Method;
+import org.elasticsearch.rest.action.readonlyrest.acl.ACL;
+import org.elasticsearch.rest.action.readonlyrest.acl.ACLRequest;
+import org.elasticsearch.rest.action.readonlyrest.acl.RuleConfigurationError;
+import org.junit.*;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+public class Issue8ACLTest {
+
+  @Test(expected=RuleConfigurationError.class)
+  public final void testRuleConfigurationError() throws Throwable{
+    byte[] encoded = Files.readAllBytes(Paths.get(System.getProperty("user.dir") + "/src/test/issue8.yml"));
+    String str = Charsets.UTF_8.decode(ByteBuffer.wrap(encoded)).toString();
+    Settings s = ImmutableSettings.builder().loadFromSource(str).build();
+    new ACL(ESLoggerFactory.getLogger(ACL.class.getName()), s);
+  }
+  
+
+}