Merge pull request #13942 from vespa-engine/bjorncs/improve-athenz-access-control

Bjorncs/improve athenz access control

8 files changed, 193 insertions, 283 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 559651b3b4f..3ae531539ef 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,6 +13,7 @@ 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 87c6d41c80d..e96951dc83a 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,17 +8,15 @@ 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.
@@ -26,11 +24,15 @@ import java.util.stream.Stream;
  * @author gjoranv
  * @author bjorncs
  */
-public final class AccessControl {
+public 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");
 
-    public static final List<String> UNPROTECTED_HANDLERS = List.of(
+    private static final int HOSTED_CONTAINER_PORT = 4443;
+
+    // Handlers that are excluded from access control
+    private static final List<String> EXCLUDED_HANDLERS = List.of(
             FileStatusHandlerComponent.CLASS,
             ContainerCluster.APPLICATION_STATUS_HANDLER_CLASS,
             ContainerCluster.BINDINGS_OVERVIEW_HANDLER_CLASS,
@@ -40,13 +42,12 @@ public final class AccessControl {
             ApplicationContainerCluster.PROMETHEUS_V1_HANDLER_CLASS
     );
 
-    public static final class Builder {
-        private String domain;
+    public static class Builder {
+        private final 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;
@@ -57,8 +58,8 @@ public final class AccessControl {
             return this;
         }
 
-        public Builder writeEnabled(boolean writeEnalbed) {
-            this.writeEnabled = writeEnalbed;
+        public Builder writeEnabled(boolean writeEnabled) {
+            this.writeEnabled = writeEnabled;
             return this;
         }
 
@@ -69,13 +70,11 @@ public final 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, servlets, handlers);
+            return new AccessControl(domain, writeEnabled, readEnabled, excludeBindings, handlers);
         }
     }
 
@@ -84,69 +83,83 @@ public final 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 List<FilterBinding> getBindings() {
-        return Stream.concat(getHandlerBindings(), getServletBindings())
-                .collect(Collectors.toCollection(ArrayList::new));
+    public void configureHttpFilterChains(Http http) {
+        http.setAccessControl(this);
+        addAccessControlFilterChain(http);
+        addAccessControlExcludedChain(http);
+        removeDuplicateBindingsFromAccessControlChain(http);
     }
 
     public static boolean hasHandlerThatNeedsProtection(ApplicationContainerCluster cluster) {
-        return cluster.getHandlers().stream().anyMatch(AccessControl::handlerNeedsProtection);
-    }
-
-    private Stream<FilterBinding> getHandlerBindings() {
-        return handlers.stream()
-                        .filter(this::shouldHandlerBeProtected)
-                        .flatMap(handler -> handler.getServerBindings().stream())
-                        .map(binding -> accessControlBinding(binding));
+        return cluster.getHandlers().stream()
+                .anyMatch(handler -> ! isExcludedHandler(handler) && hasNonMbusBinding(handler));
     }
 
-    private Stream<FilterBinding> getServletBindings() {
-        return servlets.stream()
-                .filter(this::shouldServletBeProtected)
-                .flatMap(AccessControl::servletBindings)
-                .map(binding -> accessControlBinding(binding));
+    private void addAccessControlFilterChain(Http http) {
+        http.getFilterChains().add(createChain(ACCESS_CONTROL_CHAIN_ID));
+        http.getBindings().addAll(List.of(createAccessControlBinding("/"), createAccessControlBinding("/*")));
     }
 
-    private boolean shouldHandlerBeProtected(Handler<?> handler) {
-        return ! isBuiltinGetOnly(handler)
-                && handler.getServerBindings().stream().noneMatch(excludedBindings::contains);
+    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 static boolean isBuiltinGetOnly(Handler<?> handler) {
-        return UNPROTECTED_HANDLERS.contains(handler.getClassId().getName());
+    // 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 shouldServletBeProtected(Servlet servlet) {
-        return servletBindings(servlet).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 FilterBinding accessControlBinding(BindingPattern binding) {
-        return FilterBinding.create(new ComponentSpecification(ACCESS_CONTROL_CHAIN_ID.stringValue()), binding);
+    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 static Stream<BindingPattern> servletBindings(Servlet servlet) {
-        return Stream.of(SystemBindingPattern.fromHttpPath("/" + servlet.bindingPath));
-    }
+    private static Chain<Filter> createChain(ComponentId id) { return new Chain<>(FilterChains.emptyChainSpec(id)); }
 
-    private static boolean handlerNeedsProtection(Handler<?> handler) {
-        return ! isBuiltinGetOnly(handler) && hasNonMbusBinding(handler);
-    }
+    private static boolean isExcludedHandler(Handler<?> handler) { return EXCLUDED_HANDLERS.contains(handler.getClassId().getName()); }
 
     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 8ae06b7cebd..1ca54769683 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,29 +4,44 @@ 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 filterId;
+    private final ComponentSpecification chainId;
     private final BindingPattern binding;
 
-    private FilterBinding(ComponentSpecification filterId, BindingPattern binding) {
-        this.filterId = filterId;
+    private FilterBinding(ComponentSpecification chainId, BindingPattern binding) {
+        this.chainId = chainId;
         this.binding = binding;
     }
 
-    public static FilterBinding create(ComponentSpecification filterId, BindingPattern binding) {
-        return new FilterBinding(filterId, binding);
+    public static FilterBinding create(ComponentSpecification chainId, BindingPattern binding) {
+        return new FilterBinding(chainId, binding);
     }
 
-    public ComponentSpecification filterId() {
-        return filterId;
+    public ComponentSpecification chainId() {
+        return chainId;
     }
 
     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 3155669527a..f58f5faa382 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.filterId().stringValue())
+                    .id(binding.chainId().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.filterId()) == null && chains.getComponent(binding.filterId()) == null)
-                throw new RuntimeException("Can't find filter " + binding.filterId() + " for binding " + binding.binding());
+            if (filters.getComponent(binding.chainId()) == null && chains.getComponent(binding.chainId()) == null)
+                throw new RuntimeException("Can't find filter " + binding.chainId() + " 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 9d5fead7dfb..c86d8b206d5 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,7 +14,6 @@ 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;
@@ -26,8 +25,6 @@ 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
@@ -48,8 +45,6 @@ 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);
@@ -57,8 +52,10 @@ 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 d11e0f9a891..51583588201 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,7 +61,6 @@ 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;
@@ -94,7 +93,6 @@ 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;
 
 /**
@@ -371,15 +369,12 @@ 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.
-        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());
+        new AccessControl.Builder(tenantDomain.value())
+                .setHandlers(cluster)
+                .readEnabled(false)
+                .writeEnabled(false)
+                .build()
+                .configureHttpFilterChains(http);
     }
 
     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 4ca7590673d..5b0c13a4038 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.filterId().getName());
+        assertEquals("my-request-chain", binding.chainId().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.filterId().getName());
+        assertEquals("my-response-chain", binding.chainId().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 e054698d40b..4c3a1084005 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,268 +7,176 @@ 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.FilterBinding;
+import com.yahoo.vespa.model.container.http.FilterChains;
 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.Collection;
-import java.util.HashSet;
+import java.util.ArrayList;
+import java.util.List;
 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_chain_is_set_up() {
-        Element clusterElem = DomBuilderTest.parse(
+    public void access_control_filter_chains_are_set_up() {
+        Http http = createModelAndGetHttp(
                 "  <http>",
                 "    <filtering>",
-                "      <access-control domain='foo' />",
+                "      <access-control domain='my-tenant-domain' />",
                 "    </filtering>",
                 "  </http>");
 
-        Http http = new HttpBuilder().build(root.getDeployState(), root, clusterElem);
-        root.freezeModelTopology();
-
-        assertTrue(http.getFilterChains().hasChain(AccessControl.ACCESS_CONTROL_CHAIN_ID));
+        FilterChains filterChains = http.getFilterChains();
+        assertTrue(filterChains.hasChain(AccessControl.ACCESS_CONTROL_CHAIN_ID));
+        assertTrue(filterChains.hasChain(AccessControl.ACCESS_CONTROL_EXCLUDED_CHAIN_ID));
     }
 
     @Test
     public void properties_are_set_from_xml() {
-        Element clusterElem = DomBuilderTest.parse(
+        Http http = createModelAndGetHttp(
                 "  <http>",
                 "    <filtering>",
-                "      <access-control domain='my-domain'/>",
+                "      <access-control domain='my-tenant-domain'/>",
                 "    </filtering>",
                 "  </http>");
 
-        Http http = new HttpBuilder().build(root.getDeployState(), root, clusterElem);
-        root.freezeModelTopology();
         AccessControl accessControl = http.getAccessControl().get();
 
-        assertEquals("Wrong domain.", "my-domain", accessControl.domain);
+        assertEquals("Wrong domain.", "my-tenant-domain", accessControl.domain);
     }
 
     @Test
     public void read_is_disabled_and_write_is_enabled_by_default() {
-        Element clusterElem = DomBuilderTest.parse(
+        Http http = createModelAndGetHttp(
                 "  <http>",
                 "    <filtering>",
-                "      <access-control domain='foo' />",
+                "      <access-control domain='my-tenant-domain'/>",
                 "    </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() {
-        Element clusterElem = DomBuilderTest.parse(
+        Http http = createModelAndGetHttp(
                 "  <http>",
                 "    <filtering>",
-                "      <access-control domain='foo' read='true' write='false'/>",
+                "      <access-control domain='my-tenant-domain' 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_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>",
+    public void access_control_excluded_filter_chain_has_all_bindings_from_excluded_handlers() {
+        Http http = createModelAndGetHttp(
                 "  <http>",
                 "    <filtering>",
-                "      <access-control domain='foo' />",
+                "      <access-control/>",
                 "    </filtering>",
-                "  </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());
-
-        FORBIDDEN_HANDLER_BINDINGS.forEach(forbiddenBinding -> {
-            http.getBindings().forEach(
-                    binding -> assertNotEquals("Access control chain was bound to: " + binding.binding(), binding.binding(), forbiddenBinding));
-        });
-    }
-
-    @Test
-    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>");
-
-        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));
+                "  </http>");
 
+        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/"));
     }
 
     @Test
-    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 + "' />",
+    public void access_control_excluded_chain_does_not_contain_any_bindings_from_access_control_chain() {
+        Http http = createModelAndGetHttp(
                 "  <http>",
                 "    <filtering>",
-                "      <access-control domain='foo' />",
+                "      <access-control/>",
                 "    </filtering>",
-                "  </http>",
-                "</container>");
-
-        Http http = getHttp(clusterElem);
+                "  </http>");
 
-        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());
+        Set<String> bindings = getFilterBindings(http, AccessControl.ACCESS_CONTROL_CHAIN_ID);
+        Set<String> excludedBindings = getFilterBindings(http, AccessControl.ACCESS_CONTROL_EXCLUDED_CHAIN_ID);
 
-        assertTrue("Access control chain was not bound to: " + prettyString(missingRequiredBindings),
-                   missingRequiredBindings.isEmpty());
+        for (String binding : bindings) {
+            assertThat(excludedBindings, not(hasItem(binding)));
+        }
     }
 
-    @Test
-    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 access_control_excluded_filter_chain_has_user_provided_excluded_bindings() {
+        Http http = createModelAndGetHttp(
+                "  <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>",
+                "    </filtering>",
+                "  </http>");
 
+        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"));
     }
 
     @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));
-
+    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/*"));
     }
 
-
     @Test
     public void access_control_is_implicitly_added_for_hosted_apps() {
-        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();
+        Http http = createModelAndGetHttp("<container version='1.0'/>");
+        Optional<AccessControl> maybeAccessControl = http.getAccessControl();
         assertThat(maybeAccessControl.isPresent(), is(true));
         AccessControl accessControl = maybeAccessControl.get();
         assertThat(accessControl.writeEnabled, is(false));
         assertThat(accessControl.readEnabled, is(false));
-        assertThat(accessControl.domain, equalTo(tenantDomain.value()));
+        assertThat(accessControl.domain, equalTo("my-tenant-domain"));
     }
 
     @Test
     public void access_control_is_implicitly_added_for_hosted_apps_with_existing_http_element() {
-        Element clusterElem = DomBuilderTest.parse(
-                "<container version='1.0'>",
+        Http http = createModelAndGetHttp(
                 "  <http>",
                 "    <server port='" + getDefaults().vespaWebServicePort() + "' id='main' />",
                 "    <filtering>",
@@ -277,52 +185,33 @@ public class AccessControlTest extends ContainerModelBuilderTestBase {
                 "        <filter id='inner' />",
                 "      </request-chain>",
                 "    </filtering>",
-                "  </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, clusterElem);
-        Http http = ((ApplicationContainer) root.getProducer("container/container.0")).getHttp();
+                "  </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 static String prettyString(Set<BindingPattern> missingRequiredBindings) {
-        return missingRequiredBindings.stream().map(BindingPattern::patternString).collect(Collectors.joining(", "));
-    }
+    private Http createModelAndGetHttp(String... httpElement) {
+        List<String> servicesXml = new ArrayList<>();
+        servicesXml.add("<container version='1.0'>");
+        servicesXml.addAll(List.of(httpElement));
+        servicesXml.add("</container>");
 
-    private String httpWithExcludedBinding(BindingPattern excludedBinding) {
-        return joinLines(
-                "  <http>",
-                "    <filtering>",
-                "      <access-control domain='foo'>",
-                "        <exclude>",
-                "          <binding>" + excludedBinding.patternString() + "</binding>",
-                "        </exclude>",
-                "      </access-control>",
-                "    </filtering>",
-                "  </http>");
+        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();
     }
 
-    private Http getHttp(Element clusterElem) {
-        createModel(root, clusterElem);
-        ContainerCluster cluster = (ContainerCluster) root.getChildren().get("container");
-        Http http = cluster.getHttp();
-        assertNotNull(http);
-        return http;
+    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 boolean containsBinding(Collection<FilterBinding> bindings, BindingPattern binding) {
-        for (FilterBinding b : bindings) {
-            if (b.binding().equals(binding))
-                return true;
-        }
-        return false;
-    }
 }