From 963b19783ae9518c0eeed9d4063065d0ced65a6d Mon Sep 17 00:00:00 2001
From: Bjørn Christian Seime <bjorncs@verizonmedia.com>
Date: Thu, 27 Aug 2020 16:58:43 +0200
Subject: Ensure access control chains does not duplicate bindings from user
 filter chains

---
 .../vespa/model/container/http/AccessControl.java  | 15 +++++++--
 .../model/container/xml/AccessControlTest.java     | 36 ++++++++++++++++++++++
 2 files changed, 49 insertions(+), 2 deletions(-)

(limited to 'config-model')

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 4349a8781e7..efde2d43350 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
@@ -135,12 +135,17 @@ public class AccessControl {
 
     // Remove bindings from access control chain that have binding pattern as a different filter chain
     private void removeDuplicateBindingsFromAccessControlChain(Http http) {
+        removeDuplicateBindingsFromChain(http, ACCESS_CONTROL_CHAIN_ID);
+        removeDuplicateBindingsFromChain(http, ACCESS_CONTROL_EXCLUDED_CHAIN_ID);
+    }
+
+    private void removeDuplicateBindingsFromChain(Http http, ComponentId chainId) {
         Set<FilterBinding> duplicateBindings = new HashSet<>();
         for (FilterBinding binding : http.getBindings()) {
-            if (binding.chainId().toId().equals(ACCESS_CONTROL_CHAIN_ID)) {
+            if (binding.chainId().toId().equals(chainId)) {
                 for (FilterBinding otherBinding : http.getBindings()) {
                     if (!binding.chainId().equals(otherBinding.chainId())
-                            && binding.binding().equals(otherBinding.binding())) {
+                            && effectivelyDuplicateOf(binding.binding(), otherBinding.binding())) {
                         duplicateBindings.add(binding);
                     }
                 }
@@ -149,6 +154,12 @@ public class AccessControl {
         duplicateBindings.forEach(http.getBindings()::remove);
     }
 
+    private static boolean effectivelyDuplicateOf(BindingPattern accessControlBinding, BindingPattern other) {
+        return accessControlBinding.equals(other)
+                || (accessControlBinding.path().equals(other.path()) && other.matchesAnyPort());
+    }
+
+
     private static FilterBinding createAccessControlBinding(String path) {
         return FilterBinding.create(
                 new ComponentSpecification(ACCESS_CONTROL_CHAIN_ID.stringValue()),
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..f5d0c2d1825 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
@@ -191,6 +191,42 @@ public class AccessControlTest extends ContainerModelBuilderTestBase {
         assertThat(http.getFilterChains().hasChain(ComponentId.fromString("myChain")), is(true));
     }
 
+    @Test
+    public void access_control_chains_does_not_contain_duplicate_bindings_to_user_filter_chain() {
+        Http http = createModelAndGetHttp(
+                "  <http>",
+                "    <handler id='custom.Handler'>",
+                "      <binding>http://*/custom-handler/*</binding>",
+                "      <binding>http://*/</binding>",
+                "    </handler>",
+                "    <filtering>",
+                "      <access-control/>",
+                "      <request-chain id='my-custom-request-chain'>",
+                "        <filter id='my-custom-request-filter' />",
+                "        <binding>http://*/custom-handler/*</binding>",
+                "        <binding>http://*/</binding>",
+                "      </request-chain>",
+                "    </filtering>",
+                "  </http>");
+
+        Set<String> actualExcludeBindings = getFilterBindings(http, AccessControl.ACCESS_CONTROL_EXCLUDED_CHAIN_ID);
+        assertThat(actualExcludeBindings, 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/*"));
+
+        Set<String> actualAccessControlBindings = getFilterBindings(http, AccessControl.ACCESS_CONTROL_CHAIN_ID);
+        assertThat(actualAccessControlBindings, containsInAnyOrder("http://*:4443/*"));
+
+        Set<String> actualCustomChainBindings = getFilterBindings(http, ComponentId.fromString("my-custom-request-chain"));
+        assertThat(actualCustomChainBindings, containsInAnyOrder("http://*/custom-handler/*", "http://*/"));
+    }
+
     private Http createModelAndGetHttp(String... httpElement) {
         List<String> servicesXml = new ArrayList<>();
         servicesXml.add("<container version='1.0'>");
-- 
cgit v1.2.3