Revert "Bjorncs/improve athenz access control"

8 files changed, 283 insertions, 193 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/component/SystemBindingPattern.java b/config-model/src/main/java/com/yahoo/vespa/model/container/component/SystemBindingPattern.java
index 3ae531539ef..559651b3b4f 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/component/SystemBindingPattern.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/component/SystemBindingPattern.java
@@ -13,7 +13,6 @@ public class SystemBindingPattern extends BindingPattern {
 
     public static SystemBindingPattern fromHttpPath(String path) { return new SystemBindingPattern("http", "*", null, path);}
     public static SystemBindingPattern fromPattern(String binding) { return new SystemBindingPattern(binding);}
-    public static SystemBindingPattern fromHttpPortAndPath(String port, String path) { return new SystemBindingPattern("http", "*", port, path); }
 
     @Override
     public String toString() {
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 e96951dc83a..87c6d41c80d 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
@@ -8,15 +8,17 @@ import com.yahoo.vespa.model.container.ContainerCluster;
 import com.yahoo.vespa.model.container.component.BindingPattern;
 import com.yahoo.vespa.model.container.component.FileStatusHandlerComponent;
 import com.yahoo.vespa.model.container.component.Handler;
+import com.yahoo.vespa.model.container.component.Servlet;
 import com.yahoo.vespa.model.container.component.SystemBindingPattern;
-import com.yahoo.vespa.model.container.component.chain.Chain;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  * Helper class for http access control.
@@ -24,15 +26,11 @@ import java.util.Set;
  * @author gjoranv
  * @author bjorncs
  */
-public class AccessControl {
+public final class AccessControl {
 
     public static final ComponentId ACCESS_CONTROL_CHAIN_ID = ComponentId.fromString("access-control-chain");
-    public static final ComponentId ACCESS_CONTROL_EXCLUDED_CHAIN_ID = ComponentId.fromString("access-control-excluded-chain");
 
-    private static final int HOSTED_CONTAINER_PORT = 4443;
-
-    // Handlers that are excluded from access control
-    private static final List<String> EXCLUDED_HANDLERS = List.of(
+    public static final List<String> UNPROTECTED_HANDLERS = List.of(
             FileStatusHandlerComponent.CLASS,
             ContainerCluster.APPLICATION_STATUS_HANDLER_CLASS,
             ContainerCluster.BINDINGS_OVERVIEW_HANDLER_CLASS,
@@ -42,12 +40,13 @@ public class AccessControl {
             ApplicationContainerCluster.PROMETHEUS_V1_HANDLER_CLASS
     );
 
-    public static class Builder {
-        private final String domain;
+    public static final class Builder {
+        private String domain;
         private boolean readEnabled = false;
         private boolean writeEnabled = true;
         private final Set<BindingPattern> excludeBindings = new LinkedHashSet<>();
         private Collection<Handler<?>> handlers = Collections.emptyList();
+        private Collection<Servlet> servlets = Collections.emptyList();
 
         public Builder(String domain) {
             this.domain = domain;
@@ -58,8 +57,8 @@ public class AccessControl {
             return this;
         }
 
-        public Builder writeEnabled(boolean writeEnabled) {
-            this.writeEnabled = writeEnabled;
+        public Builder writeEnabled(boolean writeEnalbed) {
+            this.writeEnabled = writeEnalbed;
             return this;
         }
 
@@ -70,11 +69,13 @@ public class AccessControl {
 
         public Builder setHandlers(ApplicationContainerCluster cluster) {
             this.handlers = cluster.getHandlers();
+            this.servlets = cluster.getAllServlets();
             return this;
         }
 
         public AccessControl build() {
-            return new AccessControl(domain, writeEnabled, readEnabled, excludeBindings, handlers);
+            return new AccessControl(domain, writeEnabled, readEnabled,
+                                     excludeBindings, servlets, handlers);
         }
     }
 
@@ -83,83 +84,69 @@ public class AccessControl {
     public final boolean writeEnabled;
     private final Set<BindingPattern> excludedBindings;
     private final Collection<Handler<?>> handlers;
+    private final Collection<Servlet> servlets;
 
     private AccessControl(String domain,
                           boolean writeEnabled,
                           boolean readEnabled,
                           Set<BindingPattern> excludedBindings,
+                          Collection<Servlet> servlets,
                           Collection<Handler<?>> handlers) {
         this.domain = domain;
         this.readEnabled = readEnabled;
         this.writeEnabled = writeEnabled;
         this.excludedBindings = Collections.unmodifiableSet(excludedBindings);
         this.handlers = handlers;
+        this.servlets = servlets;
     }
 
-    public void configureHttpFilterChains(Http http) {
-        http.setAccessControl(this);
-        addAccessControlFilterChain(http);
-        addAccessControlExcludedChain(http);
-        removeDuplicateBindingsFromAccessControlChain(http);
+    public List<FilterBinding> getBindings() {
+        return Stream.concat(getHandlerBindings(), getServletBindings())
+                .collect(Collectors.toCollection(ArrayList::new));
     }
 
     public static boolean hasHandlerThatNeedsProtection(ApplicationContainerCluster cluster) {
-        return cluster.getHandlers().stream()
-                .anyMatch(handler -> ! isExcludedHandler(handler) && hasNonMbusBinding(handler));
+        return cluster.getHandlers().stream().anyMatch(AccessControl::handlerNeedsProtection);
     }
 
-    private void addAccessControlFilterChain(Http http) {
-        http.getFilterChains().add(createChain(ACCESS_CONTROL_CHAIN_ID));
-        http.getBindings().addAll(List.of(createAccessControlBinding("/"), createAccessControlBinding("/*")));
+    private Stream<FilterBinding> getHandlerBindings() {
+        return handlers.stream()
+                        .filter(this::shouldHandlerBeProtected)
+                        .flatMap(handler -> handler.getServerBindings().stream())
+                        .map(binding -> accessControlBinding(binding));
     }
 
-    private void addAccessControlExcludedChain(Http http) {
-        http.getFilterChains().add(createChain(ACCESS_CONTROL_EXCLUDED_CHAIN_ID));
-        for (BindingPattern excludedBinding : excludedBindings) {
-            http.getBindings().add(createAccessControlExcludedBinding(excludedBinding));
-        }
-        for (Handler<?> handler : handlers) {
-            if (isExcludedHandler(handler)) {
-                for (BindingPattern binding : handler.getServerBindings()) {
-                    http.getBindings().add(createAccessControlExcludedBinding(binding));
-                }
-            }
-        }
+    private Stream<FilterBinding> getServletBindings() {
+        return servlets.stream()
+                .filter(this::shouldServletBeProtected)
+                .flatMap(AccessControl::servletBindings)
+                .map(binding -> accessControlBinding(binding));
     }
 
-    // Remove bindings from access control chain that have binding pattern as a different filter chain
-    private void removeDuplicateBindingsFromAccessControlChain(Http http) {
-        Set<FilterBinding> duplicateBindings = new HashSet<>();
-        for (FilterBinding binding : http.getBindings()) {
-            if (binding.chainId().toId().equals(ACCESS_CONTROL_CHAIN_ID)) {
-                for (FilterBinding otherBinding : http.getBindings()) {
-                    if (!binding.chainId().equals(otherBinding.chainId())
-                            && binding.binding().equals(otherBinding.binding())) {
-                        duplicateBindings.add(binding);
-                    }
-                }
-            }
-        }
-        duplicateBindings.forEach(http.getBindings()::remove);
+    private boolean shouldHandlerBeProtected(Handler<?> handler) {
+        return ! isBuiltinGetOnly(handler)
+                && handler.getServerBindings().stream().noneMatch(excludedBindings::contains);
     }
 
-    private static FilterBinding createAccessControlBinding(String path) {
-        return FilterBinding.create(
-                new ComponentSpecification(ACCESS_CONTROL_CHAIN_ID.stringValue()),
-                SystemBindingPattern.fromHttpPortAndPath(Integer.toString(HOSTED_CONTAINER_PORT), path));
+    private static boolean isBuiltinGetOnly(Handler<?> handler) {
+        return UNPROTECTED_HANDLERS.contains(handler.getClassId().getName());
     }
 
-    private static FilterBinding createAccessControlExcludedBinding(BindingPattern excludedBinding) {
-        BindingPattern rewrittenBinding = SystemBindingPattern.fromHttpPortAndPath(
-                Integer.toString(HOSTED_CONTAINER_PORT), excludedBinding.path()); // only keep path from excluded binding
-        return FilterBinding.create(
-                new ComponentSpecification(ACCESS_CONTROL_EXCLUDED_CHAIN_ID.stringValue()),
-                rewrittenBinding);
+    private boolean shouldServletBeProtected(Servlet servlet) {
+        return servletBindings(servlet).noneMatch(excludedBindings::contains);
     }
 
-    private static Chain<Filter> createChain(ComponentId id) { return new Chain<>(FilterChains.emptyChainSpec(id)); }
+    private static FilterBinding accessControlBinding(BindingPattern binding) {
+        return FilterBinding.create(new ComponentSpecification(ACCESS_CONTROL_CHAIN_ID.stringValue()), binding);
+    }
+
+    private static Stream<BindingPattern> servletBindings(Servlet servlet) {
+        return Stream.of(SystemBindingPattern.fromHttpPath("/" + servlet.bindingPath));
+    }
 
-    private static boolean isExcludedHandler(Handler<?> handler) { return EXCLUDED_HANDLERS.contains(handler.getClassId().getName()); }
+    private static boolean handlerNeedsProtection(Handler<?> handler) {
+        return ! isBuiltinGetOnly(handler) && hasNonMbusBinding(handler);
+    }
 
     private static boolean hasNonMbusBinding(Handler<?> handler) {
         return handler.getServerBindings().stream().anyMatch(binding -> ! binding.scheme().equals("mbus"));
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/FilterBinding.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/FilterBinding.java
index 1ca54769683..8ae06b7cebd 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/FilterBinding.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/FilterBinding.java
@@ -4,44 +4,29 @@ package com.yahoo.vespa.model.container.http;
 import com.yahoo.component.ComponentSpecification;
 import com.yahoo.vespa.model.container.component.BindingPattern;
 
-import java.util.Objects;
-
 /**
  * @author bjorncs
  */
 public class FilterBinding {
 
-    private final ComponentSpecification chainId;
+    private final ComponentSpecification filterId;
     private final BindingPattern binding;
 
-    private FilterBinding(ComponentSpecification chainId, BindingPattern binding) {
-        this.chainId = chainId;
+    private FilterBinding(ComponentSpecification filterId, BindingPattern binding) {
+        this.filterId = filterId;
         this.binding = binding;
     }
 
-    public static FilterBinding create(ComponentSpecification chainId, BindingPattern binding) {
-        return new FilterBinding(chainId, binding);
+    public static FilterBinding create(ComponentSpecification filterId, BindingPattern binding) {
+        return new FilterBinding(filterId, binding);
     }
 
-    public ComponentSpecification chainId() {
-        return chainId;
+    public ComponentSpecification filterId() {
+        return filterId;
     }
 
     public BindingPattern binding() {
         return binding;
     }
 
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
-        FilterBinding that = (FilterBinding) o;
-        return Objects.equals(chainId, that.chainId) &&
-                Objects.equals(binding, that.binding);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(chainId, binding);
-    }
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/Http.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/Http.java
index f58f5faa382..3155669527a 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/Http.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/Http.java
@@ -76,7 +76,7 @@ public class Http extends AbstractConfigProducer<AbstractConfigProducer<?>> impl
     public void getConfig(ServerConfig.Builder builder) {
         for (FilterBinding binding : bindings) {
             builder.filter(new ServerConfig.Filter.Builder()
-                    .id(binding.chainId().stringValue())
+                    .id(binding.filterId().stringValue())
                     .binding(binding.binding().patternString()));
         }
     }
@@ -92,8 +92,8 @@ public class Http extends AbstractConfigProducer<AbstractConfigProducer<?>> impl
         ComponentRegistry<Chain<Filter>> chains = filterChains.allChains();
 
         for (FilterBinding binding: bindings) {
-            if (filters.getComponent(binding.chainId()) == null && chains.getComponent(binding.chainId()) == null)
-                throw new RuntimeException("Can't find filter " + binding.chainId() + " for binding " + binding.binding());
+            if (filters.getComponent(binding.filterId()) == null && chains.getComponent(binding.filterId()) == null)
+                throw new RuntimeException("Can't find filter " + binding.filterId() + " for binding " + binding.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 c86d8b206d5..9d5fead7dfb 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
@@ -14,6 +14,7 @@ import com.yahoo.vespa.model.builder.xml.dom.VespaDomBuilder;
 import com.yahoo.vespa.model.container.ApplicationContainerCluster;
 import com.yahoo.vespa.model.container.Container;
 import com.yahoo.vespa.model.container.component.UserBindingPattern;
+import com.yahoo.vespa.model.container.component.chain.Chain;
 import com.yahoo.vespa.model.container.http.AccessControl;
 import com.yahoo.vespa.model.container.http.FilterBinding;
 import com.yahoo.vespa.model.container.http.FilterChains;
@@ -25,6 +26,8 @@ import java.util.List;
 import java.util.Optional;
 import java.util.logging.Level;
 
+import static com.yahoo.vespa.model.container.http.AccessControl.ACCESS_CONTROL_CHAIN_ID;
+
 /**
  * @author Tony Vaagenes
  * @author gjoranv
@@ -45,6 +48,8 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http>
             Element accessControlElem = XML.getChild(filteringElem, "access-control");
             if (accessControlElem != null) {
                 accessControl = buildAccessControl(deployState, ancestor, accessControlElem);
+                bindings.addAll(accessControl.getBindings());
+                filterChains.add(new Chain<>(FilterChains.emptyChainSpec(ACCESS_CONTROL_CHAIN_ID)));
             }
         } else {
             filterChains = new FilterChainsBuilder().newChainsInstance(ancestor);
@@ -52,10 +57,8 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http>
 
         Http http = new Http(filterChains);
         http.getBindings().addAll(bindings);
+        http.setAccessControl(accessControl);
         http.setHttpServer(new JettyHttpServerBuilder().build(deployState, ancestor, spec));
-        if (accessControl != null) {
-            accessControl.configureHttpFilterChains(http);
-        }
         return http;
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
index 51583588201..d11e0f9a891 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
@@ -61,6 +61,7 @@ import com.yahoo.vespa.model.container.component.FileStatusHandlerComponent;
 import com.yahoo.vespa.model.container.component.Handler;
 import com.yahoo.vespa.model.container.component.SystemBindingPattern;
 import com.yahoo.vespa.model.container.component.UserBindingPattern;
+import com.yahoo.vespa.model.container.component.chain.Chain;
 import com.yahoo.vespa.model.container.component.chain.ProcessingHandler;
 import com.yahoo.vespa.model.container.docproc.ContainerDocproc;
 import com.yahoo.vespa.model.container.docproc.DocprocChains;
@@ -93,6 +94,7 @@ import java.util.function.Consumer;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
+import static com.yahoo.vespa.model.container.http.AccessControl.ACCESS_CONTROL_CHAIN_ID;
 import static java.util.logging.Level.WARNING;
 
 /**
@@ -369,12 +371,15 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         if (http.getAccessControl().isPresent()) return; // access control added explicitly
         AthenzDomain tenantDomain = deployState.getProperties().athenzDomain().orElse(null);
         if (tenantDomain == null) return; // tenant domain not present, cannot add access control. this should eventually be a failure.
-        new AccessControl.Builder(tenantDomain.value())
-                .setHandlers(cluster)
-                .readEnabled(false)
-                .writeEnabled(false)
-                .build()
-                .configureHttpFilterChains(http);
+        AccessControl accessControl =
+                new AccessControl.Builder(tenantDomain.value())
+                        .setHandlers(cluster)
+                        .readEnabled(false)
+                        .writeEnabled(false)
+                        .build();
+        http.getFilterChains().add(new Chain<>(FilterChains.emptyChainSpec(ACCESS_CONTROL_CHAIN_ID)));
+        http.setAccessControl(accessControl);
+        http.getBindings().addAll(accessControl.getBindings());
     }
 
     private Http buildHttp(DeployState deployState, ApplicationContainerCluster cluster, Element httpElement) {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterBindingsTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterBindingsTest.java
index 5b0c13a4038..4ca7590673d 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterBindingsTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/http/FilterBindingsTest.java
@@ -51,7 +51,7 @@ public class FilterBindingsTest extends DomBuilderTest {
         Http http = buildHttp(xml);
 
         FilterBinding binding = first(http.getBindings());
-        assertEquals("my-request-chain", binding.chainId().getName());
+        assertEquals("my-request-chain", binding.filterId().getName());
         assertEquals(MY_CHAIN_BINDING, binding.binding());
 
         Chain<Filter> myChain = http.getFilterChains().allChains().getComponent("my-request-chain");
@@ -71,7 +71,7 @@ public class FilterBindingsTest extends DomBuilderTest {
         Http http = buildHttp(xml);
 
         FilterBinding binding = first(http.getBindings());
-        assertEquals("my-response-chain", binding.chainId().getName());
+        assertEquals("my-response-chain", binding.filterId().getName());
         assertEquals(MY_CHAIN_BINDING, binding.binding());
 
         Chain<Filter> myChain = http.getFilterChains().allChains().getComponent("my-response-chain");
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 4c3a1084005..e054698d40b 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
@@ -7,176 +7,268 @@ import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.deploy.TestProperties;
 import com.yahoo.config.provision.AthenzDomain;
 import com.yahoo.vespa.model.container.ApplicationContainer;
+import com.yahoo.vespa.model.container.ContainerCluster;
+import com.yahoo.vespa.model.container.component.BindingPattern;
+import com.yahoo.vespa.model.container.component.SystemBindingPattern;
+import com.yahoo.vespa.model.container.component.UserBindingPattern;
 import com.yahoo.vespa.model.container.http.AccessControl;
-import com.yahoo.vespa.model.container.http.FilterChains;
+import com.yahoo.vespa.model.container.http.FilterBinding;
 import com.yahoo.vespa.model.container.http.Http;
+import com.yahoo.vespa.model.container.http.xml.HttpBuilder;
+import com.yahoo.vespa.model.container.jersey.Jersey2Servlet;
 import org.junit.Test;
+import org.w3c.dom.Element;
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.Collection;
+import java.util.HashSet;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import static com.yahoo.config.model.test.TestUtil.joinLines;
 import static com.yahoo.vespa.defaults.Defaults.getDefaults;
-import static org.hamcrest.CoreMatchers.hasItem;
 import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.hasItems;
-import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 /**
  * @author gjoranv
- * @author bjorncs
  */
 public class AccessControlTest extends ContainerModelBuilderTestBase {
 
+    private static final Set<BindingPattern> REQUIRED_HANDLER_BINDINGS =
+            Set.of(
+                    UserBindingPattern.fromHttpPath("/custom-handler/*"),
+                    SystemBindingPattern.fromHttpPath("/search/*"),
+                    SystemBindingPattern.fromHttpPath("/document/v1/*"),
+                    SystemBindingPattern.fromHttpPath("/reserved-for-internal-use/feedapi"));
+
+    private static final Set<BindingPattern> FORBIDDEN_HANDLER_BINDINGS =
+            Set.of(
+                    SystemBindingPattern.fromHttpPath("/ApplicationStatus"),
+                    SystemBindingPattern.fromHttpPath("/status.html"),
+                    SystemBindingPattern.fromHttpPath("/statistics/"),
+                    SystemBindingPattern.fromHttpPath("/state/v1"),
+                    SystemBindingPattern.fromHttpPath("/"));
+
     @Test
-    public void access_control_filter_chains_are_set_up() {
-        Http http = createModelAndGetHttp(
+    public void access_control_filter_chain_is_set_up() {
+        Element clusterElem = DomBuilderTest.parse(
                 "  <http>",
                 "    <filtering>",
-                "      <access-control domain='my-tenant-domain' />",
+                "      <access-control domain='foo' />",
                 "    </filtering>",
                 "  </http>");
 
-        FilterChains filterChains = http.getFilterChains();
-        assertTrue(filterChains.hasChain(AccessControl.ACCESS_CONTROL_CHAIN_ID));
-        assertTrue(filterChains.hasChain(AccessControl.ACCESS_CONTROL_EXCLUDED_CHAIN_ID));
+        Http http = new HttpBuilder().build(root.getDeployState(), root, clusterElem);
+        root.freezeModelTopology();
+
+        assertTrue(http.getFilterChains().hasChain(AccessControl.ACCESS_CONTROL_CHAIN_ID));
     }
 
     @Test
     public void properties_are_set_from_xml() {
-        Http http = createModelAndGetHttp(
+        Element clusterElem = DomBuilderTest.parse(
                 "  <http>",
                 "    <filtering>",
-                "      <access-control domain='my-tenant-domain'/>",
+                "      <access-control domain='my-domain'/>",
                 "    </filtering>",
                 "  </http>");
 
+        Http http = new HttpBuilder().build(root.getDeployState(), root, clusterElem);
+        root.freezeModelTopology();
         AccessControl accessControl = http.getAccessControl().get();
 
-        assertEquals("Wrong domain.", "my-tenant-domain", accessControl.domain);
+        assertEquals("Wrong domain.", "my-domain", accessControl.domain);
     }
 
     @Test
     public void read_is_disabled_and_write_is_enabled_by_default() {
-        Http http = createModelAndGetHttp(
+        Element clusterElem = DomBuilderTest.parse(
                 "  <http>",
                 "    <filtering>",
-                "      <access-control domain='my-tenant-domain'/>",
+                "      <access-control domain='foo' />",
                 "    </filtering>",
                 "  </http>");
 
+        Http http = new HttpBuilder().build(root.getDeployState(), root, clusterElem);
+        root.freezeModelTopology();
+
         assertFalse("Wrong default value for read.", http.getAccessControl().get().readEnabled);
         assertTrue("Wrong default value for write.", http.getAccessControl().get().writeEnabled);
     }
 
     @Test
     public void read_and_write_can_be_overridden() {
-        Http http = createModelAndGetHttp(
+        Element clusterElem = DomBuilderTest.parse(
                 "  <http>",
                 "    <filtering>",
-                "      <access-control domain='my-tenant-domain' read='true' write='false'/>",
+                "      <access-control domain='foo' read='true' write='false'/>",
                 "    </filtering>",
                 "  </http>");
 
+        Http http = new HttpBuilder().build(root.getDeployState(), root, clusterElem);
+        root.freezeModelTopology();
+
         assertTrue("Given read value not honoured.", http.getAccessControl().get().readEnabled);
         assertFalse("Given write value not honoured.", http.getAccessControl().get().writeEnabled);
     }
 
     @Test
-    public void access_control_excluded_filter_chain_has_all_bindings_from_excluded_handlers() {
-        Http http = createModelAndGetHttp(
+    public void access_control_filter_chain_has_correct_handler_bindings() {
+        Element clusterElem = DomBuilderTest.parse(
+                "<container version='1.0'>",
+                "  <search/>",
+                "  <document-api/>",
+                "  <handler id='custom.Handler'>",
+                "    <binding>http://*/custom-handler/*</binding>",
+                "  </handler>",
                 "  <http>",
                 "    <filtering>",
-                "      <access-control/>",
+                "      <access-control domain='foo' />",
                 "    </filtering>",
-                "  </http>");
+                "  </http>",
+                "</container>");
+
+        Http http = getHttp(clusterElem);
+
+        Set<BindingPattern> foundRequiredBindings = REQUIRED_HANDLER_BINDINGS.stream()
+                .filter(requiredBinding -> containsBinding(http.getBindings(), requiredBinding))
+                .collect(Collectors.toSet());
+        Set<BindingPattern> missingRequiredBindings = new HashSet<>(REQUIRED_HANDLER_BINDINGS);
+        missingRequiredBindings.removeAll(foundRequiredBindings);
+        assertTrue("Access control chain was not bound to: " + prettyString(missingRequiredBindings),
+                   missingRequiredBindings.isEmpty());
 
-        Set<String> actualBindings = getFilterBindings(http, AccessControl.ACCESS_CONTROL_EXCLUDED_CHAIN_ID);
-        assertThat(actualBindings, containsInAnyOrder(
-                "http://*:4443/ApplicationStatus",
-                "http://*:4443/status.html",
-                "http://*:4443/state/v1",
-                "http://*:4443/state/v1/*",
-                "http://*:4443/prometheus/v1",
-                "http://*:4443/prometheus/v1/*",
-                "http://*:4443/metrics/v2",
-                "http://*:4443/metrics/v2/*",
-                "http://*:4443/"));
+        FORBIDDEN_HANDLER_BINDINGS.forEach(forbiddenBinding -> {
+            http.getBindings().forEach(
+                    binding -> assertNotEquals("Access control chain was bound to: " + binding.binding(), binding.binding(), forbiddenBinding));
+        });
     }
 
     @Test
-    public void access_control_excluded_chain_does_not_contain_any_bindings_from_access_control_chain() {
-        Http http = createModelAndGetHttp(
-                "  <http>",
-                "    <filtering>",
-                "      <access-control/>",
-                "    </filtering>",
-                "  </http>");
+    public void handler_can_be_excluded_by_excluding_one_of_its_bindings() {
+        BindingPattern notExcludedBinding = UserBindingPattern.fromHttpPath("/custom-handler/*");
+        BindingPattern excludedBinding = SystemBindingPattern.fromHttpPath("/excluded/*");
+        Element clusterElem = DomBuilderTest.parse(
+                "<container version='1.0'>",
+                httpWithExcludedBinding(excludedBinding),
+                "  <handler id='custom.Handler'>",
+                "    <binding>" + notExcludedBinding.patternString() + "</binding>",
+                "    <binding>" + excludedBinding.patternString() + "</binding>",
+                "  </handler>",
+                "</container>");
 
-        Set<String> bindings = getFilterBindings(http, AccessControl.ACCESS_CONTROL_CHAIN_ID);
-        Set<String> excludedBindings = getFilterBindings(http, AccessControl.ACCESS_CONTROL_EXCLUDED_CHAIN_ID);
+        Http http = getHttp(clusterElem);
+        assertFalse("Excluded binding was not removed.",
+                    containsBinding(http.getBindings(), excludedBinding));
+        assertFalse("Not all bindings of an excluded handler were removed.",
+                    containsBinding(http.getBindings(), notExcludedBinding));
 
-        for (String binding : bindings) {
-            assertThat(excludedBindings, not(hasItem(binding)));
-        }
     }
 
-
     @Test
-    public void access_control_excluded_filter_chain_has_user_provided_excluded_bindings() {
-        Http http = createModelAndGetHttp(
+    public void access_control_filter_chain_has_all_servlet_bindings() {
+        final String servletPath = "servlet/path";
+        final String restApiPath = "api/v0";
+        Element clusterElem = DomBuilderTest.parse(
+                "<container version='1.0'>",
+                "  <servlet id='foo' class='bar' bundle='baz'>",
+                "    <path>" + servletPath + "</path>",
+                "  </servlet>",
+                "  <rest-api jersey2='true' path='" + restApiPath + "' />",
                 "  <http>",
-                "    <handler id='custom.Handler'>",
-                "      <binding>http://*/custom-handler/*</binding>",
-                "    </handler>",
                 "    <filtering>",
-                "      <access-control>",
-                "        <exclude>",
-                "          <binding>http://*/custom-handler/*</binding>",
-                "          <binding>http://*/search/*</binding>",
-                "        </exclude>",
-                "      </access-control>",
+                "      <access-control domain='foo' />",
                 "    </filtering>",
-                "  </http>");
+                "  </http>",
+                "</container>");
+
+        Http http = getHttp(clusterElem);
 
-        Set<String> actualBindings = getFilterBindings(http, AccessControl.ACCESS_CONTROL_EXCLUDED_CHAIN_ID);
-        assertThat(actualBindings, hasItems("http://*:4443/custom-handler/*", "http://*:4443/search/*", "http://*:4443/status.html"));
+        Set<BindingPattern> requiredBindings = Set.of(
+                SystemBindingPattern.fromHttpPath("/" + servletPath),
+                SystemBindingPattern.fromHttpPath("/" + restApiPath + "/*"));
+        Set<BindingPattern> missingRequiredBindings = requiredBindings.stream()
+                .filter(requiredBinding -> ! containsBinding(http.getBindings(), requiredBinding))
+                .collect(Collectors.toSet());
+
+        assertTrue("Access control chain was not bound to: " + prettyString(missingRequiredBindings),
+                   missingRequiredBindings.isEmpty());
     }
 
     @Test
-    public void access_control_filter_chain_contains_catchall_bindings() {
-        Http http = createModelAndGetHttp(
-                "  <http>",
-                "    <filtering>",
-                "      <access-control/>",
-                "    </filtering>",
-                "  </http>");
-        Set<String> actualBindings = getFilterBindings(http, AccessControl.ACCESS_CONTROL_CHAIN_ID);
-        assertThat(actualBindings, containsInAnyOrder("http://*:4443/*"));
+    public void servlet_can_be_excluded_by_excluding_one_of_its_bindings() {
+        String servletPath = "servlet/path";
+        BindingPattern notExcludedBinding = SystemBindingPattern.fromPattern("http://*:8081/" + servletPath);
+        BindingPattern excludedBinding = SystemBindingPattern.fromPattern("http://*:8080/" + servletPath);
+        Element clusterElem = DomBuilderTest.parse(
+                "<container version='1.0'>",
+                httpWithExcludedBinding(excludedBinding),
+                "  <servlet id='foo' class='bar' bundle='baz'>",
+                "    <path>" + servletPath + "</path>",
+                "  </servlet>",
+                "</container>");
+
+        Http http = getHttp(clusterElem);
+        assertFalse("Excluded binding was not removed.",
+                    containsBinding(http.getBindings(), excludedBinding));
+        assertFalse("Not all bindings of an excluded servlet were removed.",
+                    containsBinding(http.getBindings(), notExcludedBinding));
+
+    }
+
+    @Test
+    public void rest_api_can_be_excluded_by_excluding_one_of_its_bindings() {
+        String restApiPath = "api/v0";
+        BindingPattern notExcludedBinding = SystemBindingPattern.fromPattern("http://*:8081/" + restApiPath + Jersey2Servlet.BINDING_SUFFIX);
+        BindingPattern excludedBinding = SystemBindingPattern.fromPattern("http://*:8080/" + restApiPath + Jersey2Servlet.BINDING_SUFFIX);;
+        Element clusterElem = DomBuilderTest.parse(
+                "<container version='1.0'>",
+                httpWithExcludedBinding(excludedBinding),
+                "  <rest-api jersey2='true' path='" + restApiPath + "' />",
+                "</container>");
+
+        Http http = getHttp(clusterElem);
+        assertFalse("Excluded binding was not removed.",
+                    containsBinding(http.getBindings(), excludedBinding));
+        assertFalse("Not all bindings of an excluded rest-api were removed.",
+                    containsBinding(http.getBindings(), notExcludedBinding));
+
     }
 
+
     @Test
     public void access_control_is_implicitly_added_for_hosted_apps() {
-        Http http = createModelAndGetHttp("<container version='1.0'/>");
-        Optional<AccessControl> maybeAccessControl = http.getAccessControl();
+        Element clusterElem = DomBuilderTest.parse(
+                "<container version='1.0'>",
+                nodesXml,
+                "</container>" );
+        AthenzDomain tenantDomain = AthenzDomain.from("my-tenant-domain");
+        DeployState state = new DeployState.Builder().properties(
+                new TestProperties()
+                        .setAthenzDomain(tenantDomain)
+                        .setHostedVespa(true))
+                .build();
+        createModel(root, state, null, clusterElem);
+        Optional<AccessControl> maybeAccessControl =
+                ((ApplicationContainer) root.getProducer("container/container.0")).getHttp().getAccessControl();
         assertThat(maybeAccessControl.isPresent(), is(true));
         AccessControl accessControl = maybeAccessControl.get();
         assertThat(accessControl.writeEnabled, is(false));
         assertThat(accessControl.readEnabled, is(false));
-        assertThat(accessControl.domain, equalTo("my-tenant-domain"));
+        assertThat(accessControl.domain, equalTo(tenantDomain.value()));
     }
 
     @Test
     public void access_control_is_implicitly_added_for_hosted_apps_with_existing_http_element() {
-        Http http = createModelAndGetHttp(
+        Element clusterElem = DomBuilderTest.parse(
+                "<container version='1.0'>",
                 "  <http>",
                 "    <server port='" + getDefaults().vespaWebServicePort() + "' id='main' />",
                 "    <filtering>",
@@ -185,33 +277,52 @@ public class AccessControlTest extends ContainerModelBuilderTestBase {
                 "        <filter id='inner' />",
                 "      </request-chain>",
                 "    </filtering>",
-                "  </http>");
-        assertThat(http.getAccessControl().isPresent(), is(true));
-        assertThat(http.getFilterChains().hasChain(AccessControl.ACCESS_CONTROL_CHAIN_ID), is(true));
-        assertThat(http.getFilterChains().hasChain(ComponentId.fromString("myChain")), is(true));
-    }
-
-    private Http createModelAndGetHttp(String... httpElement) {
-        List<String> servicesXml = new ArrayList<>();
-        servicesXml.add("<container version='1.0'>");
-        servicesXml.addAll(List.of(httpElement));
-        servicesXml.add("</container>");
-
+                "  </http>",
+                nodesXml,
+                "</container>" );
         AthenzDomain tenantDomain = AthenzDomain.from("my-tenant-domain");
         DeployState state = new DeployState.Builder().properties(
                 new TestProperties()
                         .setAthenzDomain(tenantDomain)
                         .setHostedVespa(true))
                 .build();
-        createModel(root, state, null, DomBuilderTest.parse(servicesXml.toArray(String[]::new)));
-        return  ((ApplicationContainer) root.getProducer("container/container.0")).getHttp();
+        createModel(root, state, null, clusterElem);
+        Http http = ((ApplicationContainer) root.getProducer("container/container.0")).getHttp();
+        assertThat(http.getAccessControl().isPresent(), is(true));
+        assertThat(http.getFilterChains().hasChain(AccessControl.ACCESS_CONTROL_CHAIN_ID), is(true));
+        assertThat(http.getFilterChains().hasChain(ComponentId.fromString("myChain")), is(true));
     }
 
-    private static Set<String> getFilterBindings(Http http, ComponentId filerChain) {
-        return http.getBindings().stream()
-                .filter(binding -> binding.chainId().toId().equals(filerChain))
-                .map(binding -> binding.binding().patternString())
-                .collect(Collectors.toSet());
+    private static String prettyString(Set<BindingPattern> missingRequiredBindings) {
+        return missingRequiredBindings.stream().map(BindingPattern::patternString).collect(Collectors.joining(", "));
+    }
+
+    private String httpWithExcludedBinding(BindingPattern excludedBinding) {
+        return joinLines(
+                "  <http>",
+                "    <filtering>",
+                "      <access-control domain='foo'>",
+                "        <exclude>",
+                "          <binding>" + excludedBinding.patternString() + "</binding>",
+                "        </exclude>",
+                "      </access-control>",
+                "    </filtering>",
+                "  </http>");
     }
 
+    private Http getHttp(Element clusterElem) {
+        createModel(root, clusterElem);
+        ContainerCluster cluster = (ContainerCluster) root.getChildren().get("container");
+        Http http = cluster.getHttp();
+        assertNotNull(http);
+        return http;
+    }
+
+    private boolean containsBinding(Collection<FilterBinding> bindings, BindingPattern binding) {
+        for (FilterBinding b : bindings) {
+            if (b.binding().equals(binding))
+                return true;
+        }
+        return false;
+    }
 }