Merge pull request #12601 from vespa-engine/bjorncs/implicit-access-control

Bjorncs/implicit access control

6 files changed, 110 insertions, 27 deletions

diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
index 2e4cdd706f7..e49ceffabc1 100644
--- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
+++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
@@ -8,6 +8,7 @@ import com.yahoo.config.model.api.EndpointCertificateSecrets;
 import com.yahoo.config.model.api.ModelContext;
 import com.yahoo.config.model.api.TlsSecrets;
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.AthenzDomain;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.Zone;
 
@@ -43,6 +44,7 @@ public class TestProperties implements ModelContext.Properties {
     private Optional<EndpointCertificateSecrets> endpointCertificateSecrets = Optional.empty();
     private boolean useNewAthenzFilter = false;
     private boolean usePhraseSegmenting = false;
+    private AthenzDomain athenzDomain;
 
     @Override public boolean multitenant() { return multitenant; }
     @Override public ApplicationId applicationId() { return applicationId; }
@@ -64,6 +66,7 @@ public class TestProperties implements ModelContext.Properties {
     @Override public boolean useBucketSpaceMetric() { return true; }
     @Override public boolean useNewAthenzFilter() { return useNewAthenzFilter; }
     @Override public boolean usePhraseSegmenting() { return usePhraseSegmenting; }
+    @Override public Optional<AthenzDomain> athenzDomain() { return Optional.ofNullable(athenzDomain); }
 
     public TestProperties setDefaultTermwiseLimit(double limit) {
         defaultTermwiseLimit = limit;
@@ -120,6 +123,11 @@ public class TestProperties implements ModelContext.Properties {
         return this;
     }
 
+    public TestProperties setAthenzDomain(AthenzDomain domain) {
+        this.athenzDomain = domain;
+        return this;
+    }
+
     public static class Spec implements ConfigServerSpec {
 
         private final String hostName;
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 37657bea0be..dd48e65c340 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
@@ -23,6 +23,7 @@ import java.util.stream.Stream;
  * Helper class for http access control.
  *
  * @author gjoranv
+ * @author bjorncs
  */
 public final class AccessControl {
 
@@ -66,13 +67,9 @@ public final class AccessControl {
             return this;
         }
 
-        public Builder setHandlers(Collection<Handler<?>> handlers) {
-            this.handlers = handlers;
-            return this;
-        }
-
-        public Builder setServlets(Collection<Servlet> servlets) {
-            this.servlets = servlets;
+        public Builder setHandlers(ApplicationContainerCluster cluster) {
+            this.handlers = cluster.getHandlers();
+            this.servlets = cluster.getAllServlets();
             return this;
         }
 
@@ -111,6 +108,10 @@ public final class AccessControl {
                 .collect(Collectors.toCollection(ArrayList::new));
     }
 
+    public static boolean hasHandlerThatNeedsProtection(ApplicationContainerCluster cluster) {
+        return cluster.getHandlers().stream().anyMatch(AccessControl::handlerNeedsProtection);
+    }
+
     private Stream<Binding> getHandlerBindings() {
         return handlers.stream()
                         .filter(this::shouldHandlerBeProtected)
@@ -130,7 +131,7 @@ public final class AccessControl {
                 && handler.getServerBindings().stream().noneMatch(excludedBindings::contains);
     }
 
-    public static boolean isBuiltinGetOnly(Handler<?> handler) {
+    private static boolean isBuiltinGetOnly(Handler<?> handler) {
         return UNPROTECTED_HANDLERS.contains(handler.getClassId().getName());
     }
 
@@ -146,10 +147,6 @@ public final class AccessControl {
         return Stream.of("http://*/").map(protocol -> protocol + servlet.bindingPath);
     }
 
-    public static boolean hasHandlerThatNeedsProtection(ApplicationContainerCluster cluster) {
-        return cluster.getHandlers().stream().anyMatch(AccessControl::handlerNeedsProtection);
-    }
-
     private static boolean handlerNeedsProtection(Handler<?> handler) {
         return ! isBuiltinGetOnly(handler) && hasNonMbusBinding(handler);
     }
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 9e0b8ad7424..dc74e2afbb1 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
@@ -6,6 +6,7 @@ import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.model.builder.xml.XmlHelper;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
+import com.yahoo.config.provision.AthenzDomain;
 import com.yahoo.text.XML;
 import com.yahoo.vespa.defaults.Defaults;
 import com.yahoo.vespa.model.builder.xml.dom.ModelElement;
@@ -22,6 +23,7 @@ import org.w3c.dom.Element;
 import java.util.ArrayList;
 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;
 
@@ -61,12 +63,10 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http>
     }
 
     private AccessControl buildAccessControl(DeployState deployState, AbstractConfigProducer ancestor, Element accessControlElem) {
-        AccessControl.Builder builder = new AccessControl.Builder(accessControlElem.getAttribute("domain"), deployState.getDeployLogger());
+        AthenzDomain domain = getAccessControlDomain(deployState, accessControlElem);
+        AccessControl.Builder builder = new AccessControl.Builder(domain.value(), deployState.getDeployLogger());
 
-        getContainerCluster(ancestor).ifPresent(cluster -> {
-            builder.setHandlers(cluster.getHandlers());
-            builder.setServlets(cluster.getAllServlets());
-        });
+        getContainerCluster(ancestor).ifPresent(builder::setHandlers);
 
         XmlHelper.getOptionalAttribute(accessControlElem, "read").ifPresent(
                 readAttr -> builder.readEnabled(Boolean.valueOf(readAttr)));
@@ -82,6 +82,28 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http>
         return builder.build();
     }
 
+    // TODO Fail if domain is not provided through deploy properties
+    private static AthenzDomain getAccessControlDomain(DeployState deployState, Element accessControlElem) {
+        AthenzDomain tenantDomain = deployState.getProperties().athenzDomain().orElse(null);
+        AthenzDomain explicitDomain = XmlHelper.getOptionalAttribute(accessControlElem, "domain")
+                .map(AthenzDomain::from)
+                .orElse(null);
+        if (tenantDomain == null) {
+            if (explicitDomain == null) {
+                throw new IllegalStateException("No Athenz domain provided for 'access-control'");
+            }
+            deployState.getDeployLogger().log(Level.WARNING, "Athenz tenant is not provided by deploy call. This will soon be handled as failure.");
+        }
+        if (explicitDomain != null) {
+            if (tenantDomain != null && !explicitDomain.equals(tenantDomain)) {
+                throw new IllegalArgumentException(
+                        String.format("Domain in access-control ('%s') does not match tenant domain ('%s')", tenantDomain, explicitDomain));
+            }
+            deployState.getDeployLogger().log(Level.WARNING, "Domain in 'access-control' is deprecated and will be removed soon");
+        }
+        return tenantDomain != null ? tenantDomain : explicitDomain;
+    }
+
     private static Optional<ApplicationContainerCluster> getContainerCluster(AbstractConfigProducer configProducer) {
         AbstractConfigProducer currentProducer = configProducer;
         while (! ApplicationContainerCluster.class.isAssignableFrom(currentProducer.getClass())) {
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 7a620fad10c..165404ce9d4 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
@@ -19,6 +19,7 @@ import com.yahoo.config.model.builder.xml.ConfigModelBuilder;
 import com.yahoo.config.model.builder.xml.ConfigModelId;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
+import com.yahoo.config.provision.AthenzDomain;
 import com.yahoo.config.provision.AthenzService;
 import com.yahoo.config.provision.Capacity;
 import com.yahoo.config.provision.ClusterMembership;
@@ -57,9 +58,11 @@ import com.yahoo.vespa.model.container.SecretStore;
 import com.yahoo.vespa.model.container.component.Component;
 import com.yahoo.vespa.model.container.component.FileStatusHandlerComponent;
 import com.yahoo.vespa.model.container.component.Handler;
+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;
+import com.yahoo.vespa.model.container.http.AccessControl;
 import com.yahoo.vespa.model.container.http.ConnectorFactory;
 import com.yahoo.vespa.model.container.http.FilterChains;
 import com.yahoo.vespa.model.container.http.Http;
@@ -88,6 +91,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;
 
 /**
@@ -192,7 +196,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         addStatusHandlers(cluster, context.getDeployState().isHosted());
         addUserHandlers(deployState, cluster, spec);
 
-        addHttp(deployState, spec, cluster, context.getApplicationType(), deployState.getProperties().applicationId().instance().isTester());
+        addHttp(deployState, spec, cluster, context);
 
         addAccessLogs(deployState, cluster, spec);
         addRoutingAliases(cluster, spec, deployState.zone().environment());
@@ -314,18 +318,18 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
     }
 
 
-    private void addHttp(DeployState deployState, Element spec, ApplicationContainerCluster cluster, ApplicationType applicationType, boolean isTesterApplication) {
+    private void addHttp(DeployState deployState, Element spec, ApplicationContainerCluster cluster, ConfigModelContext context) {
         Element httpElement = XML.getChild(spec, "http");
         if (httpElement != null) {
             cluster.setHttp(buildHttp(deployState, cluster, httpElement));
         }
-        if (deployState.isHosted() && applicationType == ApplicationType.DEFAULT && !isTesterApplication) {
+        if (isHostedTenantApplication(context)) {
+            addHostedImplicitHttpIfNotPresent(deployState, cluster);
             addAdditionalHostedConnector(deployState, cluster);
         }
     }
 
     private void addAdditionalHostedConnector(DeployState deployState, ApplicationContainerCluster cluster) {
-        addImplicitHttpIfNotPresent(cluster);
         JettyHttpServer server = cluster.getHttp().getHttpServer();
         String serverName = server.getComponentId().getName();
 
@@ -346,10 +350,17 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         }
     }
 
-    private static void addImplicitHttpIfNotPresent(ApplicationContainerCluster cluster) {
+    private static boolean isHostedTenantApplication(ConfigModelContext context) {
+        var deployState = context.getDeployState();
+        boolean isTesterApplication = deployState.getProperties().applicationId().instance().isTester();
+        return deployState.isHosted() && context.getApplicationType() == ApplicationType.DEFAULT && !isTesterApplication;
+    }
+
+    private static void addHostedImplicitHttpIfNotPresent(DeployState deployState, ApplicationContainerCluster cluster) {
         if(cluster.getHttp() == null) {
-            Http http = new Http(Collections.emptyList());
-            http.setFilterChains(new FilterChains(cluster));
+            Http http = deployState.getProperties().athenzDomain()
+                    .map(tenantDomain -> createHostedImplicitHttpWithAccessControl(deployState, tenantDomain, cluster))
+                    .orElseGet(() -> createHostedImplicitHttpWithoutAccessControl(cluster));
             cluster.setHttp(http);
         }
         if(cluster.getHttp().getHttpServer() == null) {
@@ -359,6 +370,27 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         }
     }
 
+    private static Http createHostedImplicitHttpWithAccessControl(
+            DeployState deployState, AthenzDomain tenantDomain, ApplicationContainerCluster cluster) {
+        AccessControl accessControl =
+                new AccessControl.Builder(tenantDomain.value(), deployState.getDeployLogger())
+                        .setHandlers(cluster)
+                        .readEnabled(false)
+                        .writeEnabled(false)
+                        .build();
+        Http http = new Http(accessControl.getBindings(), accessControl);
+        FilterChains filterChains = new FilterChains(cluster);
+        filterChains.add(new Chain<>(FilterChains.emptyChainSpec(ACCESS_CONTROL_CHAIN_ID)));
+        http.setFilterChains(filterChains);
+        return http;
+    }
+
+    private static Http createHostedImplicitHttpWithoutAccessControl(ApplicationContainerCluster cluster) {
+        Http http = new Http(Collections.emptyList());
+        http.setFilterChains(new FilterChains(cluster));
+        return http;
+    }
+
     private Http buildHttp(DeployState deployState, ApplicationContainerCluster cluster, Element httpElement) {
         Http http = new HttpBuilder().build(deployState, cluster, httpElement);
 
diff --git a/config-model/src/main/resources/schema/container.rnc b/config-model/src/main/resources/schema/container.rnc
index b9cb5e1fae6..034de1113c2 100644
--- a/config-model/src/main/resources/schema/container.rnc
+++ b/config-model/src/main/resources/schema/container.rnc
@@ -23,9 +23,9 @@ Server = element server {
 }
 
 AccessControl = element access-control {
-    attribute domain { xsd:NCName } &
-    attribute read { string "true" | string "false" }? &
-    attribute write { string "true" | string "false" }? &
+    attribute domain { xsd:NCName }? &  # TODO Vespa 8 Remove
+    attribute read { string "true" | string "false" }? & # TODO Vespa 8 Remove
+    attribute write { string "true" | string "false" }? & # TODO Vespa 8 Remove
     element vespa-domain { xsd:NCName }? & # TODO Remove after end of March 2020
     element exclude {
         Binding+
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java
index 17dff46bed4..73db6e35428 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java
@@ -14,6 +14,7 @@ import com.yahoo.config.model.provision.InMemoryProvisioner;
 import com.yahoo.config.model.provision.SingleNodeProvisioner;
 import com.yahoo.config.model.test.MockApplicationPackage;
 import com.yahoo.config.model.test.MockRoot;
+import com.yahoo.config.provision.AthenzDomain;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.RegionName;
@@ -43,6 +44,7 @@ import com.yahoo.vespa.model.container.ApplicationContainer;
 import com.yahoo.vespa.model.container.ContainerCluster;
 import com.yahoo.vespa.model.container.SecretStore;
 import com.yahoo.vespa.model.container.component.Component;
+import com.yahoo.vespa.model.container.http.AccessControl;
 import com.yahoo.vespa.model.container.http.ConnectorFactory;
 import com.yahoo.vespa.model.content.utils.ContentClusterUtils;
 import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithFilePkg;
@@ -833,6 +835,28 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase {
         assertThat(connectorConfig.ssl().caCertificate(), isEmptyString());
     }
 
+    @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();
+        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()));
+    }
+
 
     private Element generateContainerElementWithRenderer(String rendererId) {
         return DomBuilderTest.parse(