Merge pull request #1694 from yahoo/gjoranv/access-control-exclude

Gjoranv/access control exclude

3 files changed, 69 insertions, 23 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java
index f5b6443e6d3..7864d939b68 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java
@@ -10,6 +10,7 @@ import com.yahoo.vespa.model.container.component.Handler;
 import com.yahoo.vespa.model.container.http.Http.Binding;
 
 import java.util.List;
+import java.util.Set;
 
 /**
  * Helper class for http access control.
@@ -28,8 +29,15 @@ public final class AccessControl {
             ContainerCluster.STATISTICS_HANDLER_CLASS
     );
 
-    public static boolean shouldHandlerBeProtected(Handler<?> handler) {
-        return ! UNPROTECTED_HANDLERS.contains(handler.getClassId().getName());
+    private final Set<String> excludedBindings;
+
+    public AccessControl(Set<String> excludedBindings) {
+        this.excludedBindings = excludedBindings;
+    }
+
+    public boolean shouldHandlerBeProtected(Handler<?> handler) {
+        return ! UNPROTECTED_HANDLERS.contains(handler.getClassId().getName())
+                && handler.getServerBindings().stream().noneMatch(excludedBindings::contains);
     }
 
     public static Binding accessControlBinding(String binding) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java
index 8cb4ce2a096..1277250c9a4 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java
@@ -18,12 +18,12 @@ import com.yahoo.vespa.model.container.http.Http.Binding;
 import org.w3c.dom.Element;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
 
 import static com.yahoo.vespa.model.container.http.AccessControl.ACCESS_CONTROL_CHAIN_ID;
-import static com.yahoo.vespa.model.container.http.AccessControl.accessControlBinding;
 
 /**
  * @author tonytv
@@ -43,7 +43,7 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http>
 
             Element accessControlElem = XML.getChild(filteringElem, "access-control");
             if (accessControlElem != null) {
-                bindings.addAll(getAccessControlBindings(ancestor));
+                bindings.addAll(getAccessControlBindings(ancestor, buildAccessControl(accessControlElem)));
                 filterChains.add(new Chain<>(FilterChains.emptyChainSpec(ACCESS_CONTROL_CHAIN_ID)));
             }
         } else {
@@ -58,10 +58,20 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http>
         return http;
     }
 
-    private static List<Binding> getAccessControlBindings(AbstractConfigProducer ancestor) {
+    private AccessControl buildAccessControl(Element accessControlElem) {
+        Element excludeElem = XML.getChild(accessControlElem, "exclude");
+        if (excludeElem != null) {
+            return new AccessControl(XML.getChildren(excludeElem, "binding").stream()
+                    .map(XML::getValue)
+                    .collect(Collectors.toSet()));
+        }
+        return new AccessControl(Collections.emptySet());
+    }
+
+    private static List<Binding> getAccessControlBindings(AbstractConfigProducer ancestor, AccessControl accessControl) {
         return getContainerCluster(ancestor)
                 .map(cluster -> cluster.getHandlers().stream()
-                        .filter(AccessControl::shouldHandlerBeProtected)
+                        .filter(accessControl::shouldHandlerBeProtected)
                         .flatMap(handler -> handler.getServerBindings().stream())
                         .map(AccessControl::accessControlBinding)
                         .collect(Collectors.toList()))
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java
index fcc9fc1d29e..fa49feabb9c 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java
@@ -9,9 +9,12 @@ import com.yahoo.vespa.model.container.ContainerCluster;
 import com.yahoo.vespa.model.container.http.AccessControl;
 import com.yahoo.vespa.model.container.http.Http;
 import com.yahoo.vespa.model.container.http.Http.Binding;
+import com.yahoo.vespa.model.container.http.xml.HttpBuilder;
 import org.junit.Test;
 import org.w3c.dom.Element;
+import org.xml.sax.SAXException;
 
+import java.io.IOException;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Set;
@@ -48,23 +51,14 @@ public class AccessControlTest extends ContainerModelBuilderTestBase {
     @Test
     public void access_control_filter_chain_is_set_up() throws Exception {
         Element clusterElem = DomBuilderTest.parse(
-                "<jdisc id='default' version='1.0'>",
-                "  <search/>",
-                "  <document-api/>",
-                "  <handler id='custom.Handler'>",
-                "    <binding>http://*/custom-handler/*</binding>",
-                "  </handler>",
                 "  <http>",
                 "    <filtering>",
                 "      <access-control domain='foo' />",
                 "    </filtering>",
-                "  </http>",
-                "</jdisc>");
+                "  </http>");
 
-        createModel(root, clusterElem);
-        ContainerCluster cluster = (ContainerCluster) root.getChildren().get("default");
-        Http http = cluster.getHttp();
-        assertNotNull(http);
+        Http http = new HttpBuilder().build(root, clusterElem);
+        root.freezeModelTopology();
 
         assertTrue(http.getFilterChains().hasChain(AccessControl.ACCESS_CONTROL_CHAIN_ID));
     }
@@ -72,7 +66,7 @@ public class AccessControlTest extends ContainerModelBuilderTestBase {
     @Test
     public void access_control_filter_chain_has_correct_handler_bindings() throws Exception {
         Element clusterElem = DomBuilderTest.parse(
-                "<jdisc id='default' version='1.0'>",
+                "<jdisc version='1.0'>",
                 "  <search/>",
                 "  <document-api/>",
                 "  <handler id='custom.Handler'>",
@@ -85,10 +79,7 @@ public class AccessControlTest extends ContainerModelBuilderTestBase {
                 "  </http>",
                 "</jdisc>");
 
-        createModel(root, clusterElem);
-        ContainerCluster cluster = (ContainerCluster) root.getChildren().get("default");
-        Http http = cluster.getHttp();
-        assertNotNull(http);
+        Http http = getHttp(clusterElem);
 
         Set<String> foundRequiredBindings = REQUIRED_BINDINGS.stream()
                 .filter(requiredBinding -> containsBinding(http.getBindings(), requiredBinding))
@@ -103,6 +94,43 @@ public class AccessControlTest extends ContainerModelBuilderTestBase {
                                        binding.binding.contains(forbiddenBinding))));
     }
 
+    @Test
+    public void handler_can_be_excluded_by_excluding_one_of_its_bindings() throws Exception {
+        final String notExcludedBinding = "http://*/custom-handler/*";
+        final String excludedBinding = "http://*/excluded/*";
+        Element clusterElem = DomBuilderTest.parse(
+                "<jdisc version='1.0'>",
+                "  <handler id='custom.Handler'>",
+                "    <binding>" + notExcludedBinding + "</binding>",
+                "    <binding>" + excludedBinding + "</binding>",
+                "  </handler>",
+                "  <http>",
+                "    <filtering>",
+                "      <access-control domain='foo'>",
+                "        <exclude>",
+                "          <binding>" + excludedBinding + "</binding>",
+                "        </exclude>",
+                "      </access-control>",
+                "    </filtering>",
+                "  </http>",
+                "</jdisc>");
+
+        Http http = getHttp(clusterElem);
+        assertFalse("Excluded binding was not removed.",
+                    containsBinding(http.getBindings(), excludedBinding));
+        assertFalse("Not all bindings of an excluded handler was removed.",
+                    containsBinding(http.getBindings(), notExcludedBinding));
+
+    }
+
+    private Http getHttp(Element clusterElem) throws SAXException, IOException {
+        createModel(root, clusterElem);
+        ContainerCluster cluster = (ContainerCluster) root.getChildren().get("jdisc");
+        Http http = cluster.getHttp();
+        assertNotNull(http);
+        return http;
+    }
+
     private boolean containsBinding(Collection<Binding> bindings, String binding) {
         for (Binding b : bindings) {
             if (b.binding.contains(binding))