diff options
author | Bjørn Christian Seime <bjorn.christian@seime.no> | 2020-08-19 11:57:46 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-08-19 11:57:46 +0200 |
commit | 320e626d482968bd0853e2841e5435530a3da174 (patch) | |
tree | 2ff9567eaa5ee1c15705245008b5342653bd3f7f /config-model | |
parent | 4fa221b7c20f492de0433b9b812f1cc509a18d97 (diff) |
Revert "Bjorncs/improve athenz access control"
Diffstat (limited to 'config-model')
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; + } } |