Revert "Bjorncs/implicit access control"

6 files changed, 27 insertions, 110 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 e49ceffabc1..2e4cdd706f7 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,7 +8,6 @@ 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;
 
@@ -44,7 +43,6 @@ 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; }
@@ -66,7 +64,6 @@ 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;
@@ -123,11 +120,6 @@ 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 dd48e65c340..37657bea0be 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,7 +23,6 @@ import java.util.stream.Stream;
  * Helper class for http access control.
  *
  * @author gjoranv
- * @author bjorncs
  */
 public final class AccessControl {
 
@@ -67,9 +66,13 @@ public final class AccessControl {
             return this;
         }
 
-        public Builder setHandlers(ApplicationContainerCluster cluster) {
-            this.handlers = cluster.getHandlers();
-            this.servlets = cluster.getAllServlets();
+        public Builder setHandlers(Collection<Handler<?>> handlers) {
+            this.handlers = handlers;
+            return this;
+        }
+
+        public Builder setServlets(Collection<Servlet> servlets) {
+            this.servlets = servlets;
             return this;
         }
 
@@ -108,10 +111,6 @@ 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)
@@ -131,7 +130,7 @@ public final class AccessControl {
                 && handler.getServerBindings().stream().noneMatch(excludedBindings::contains);
     }
 
-    private static boolean isBuiltinGetOnly(Handler<?> handler) {
+    public static boolean isBuiltinGetOnly(Handler<?> handler) {
         return UNPROTECTED_HANDLERS.contains(handler.getClassId().getName());
     }
 
@@ -147,6 +146,10 @@ 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 dc74e2afbb1..9e0b8ad7424 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,7 +6,6 @@ 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;
@@ -23,7 +22,6 @@ 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;
 
@@ -63,10 +61,12 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http>
     }
 
     private AccessControl buildAccessControl(DeployState deployState, AbstractConfigProducer ancestor, Element accessControlElem) {
-        AthenzDomain domain = getAccessControlDomain(deployState, accessControlElem);
-        AccessControl.Builder builder = new AccessControl.Builder(domain.value(), deployState.getDeployLogger());
+        AccessControl.Builder builder = new AccessControl.Builder(accessControlElem.getAttribute("domain"), deployState.getDeployLogger());
 
-        getContainerCluster(ancestor).ifPresent(builder::setHandlers);
+        getContainerCluster(ancestor).ifPresent(cluster -> {
+            builder.setHandlers(cluster.getHandlers());
+            builder.setServlets(cluster.getAllServlets());
+        });
 
         XmlHelper.getOptionalAttribute(accessControlElem, "read").ifPresent(
                 readAttr -> builder.readEnabled(Boolean.valueOf(readAttr)));
@@ -82,28 +82,6 @@ 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 165404ce9d4..7a620fad10c 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,7 +19,6 @@ 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;
@@ -58,11 +57,9 @@ 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;
@@ -91,7 +88,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;
 
 /**
@@ -196,7 +192,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         addStatusHandlers(cluster, context.getDeployState().isHosted());
         addUserHandlers(deployState, cluster, spec);
 
-        addHttp(deployState, spec, cluster, context);
+        addHttp(deployState, spec, cluster, context.getApplicationType(), deployState.getProperties().applicationId().instance().isTester());
 
         addAccessLogs(deployState, cluster, spec);
         addRoutingAliases(cluster, spec, deployState.zone().environment());
@@ -318,18 +314,18 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
     }
 
 
-    private void addHttp(DeployState deployState, Element spec, ApplicationContainerCluster cluster, ConfigModelContext context) {
+    private void addHttp(DeployState deployState, Element spec, ApplicationContainerCluster cluster, ApplicationType applicationType, boolean isTesterApplication) {
         Element httpElement = XML.getChild(spec, "http");
         if (httpElement != null) {
             cluster.setHttp(buildHttp(deployState, cluster, httpElement));
         }
-        if (isHostedTenantApplication(context)) {
-            addHostedImplicitHttpIfNotPresent(deployState, cluster);
+        if (deployState.isHosted() && applicationType == ApplicationType.DEFAULT && !isTesterApplication) {
             addAdditionalHostedConnector(deployState, cluster);
         }
     }
 
     private void addAdditionalHostedConnector(DeployState deployState, ApplicationContainerCluster cluster) {
+        addImplicitHttpIfNotPresent(cluster);
         JettyHttpServer server = cluster.getHttp().getHttpServer();
         String serverName = server.getComponentId().getName();
 
@@ -350,17 +346,10 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         }
     }
 
-    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) {
+    private static void addImplicitHttpIfNotPresent(ApplicationContainerCluster cluster) {
         if(cluster.getHttp() == null) {
-            Http http = deployState.getProperties().athenzDomain()
-                    .map(tenantDomain -> createHostedImplicitHttpWithAccessControl(deployState, tenantDomain, cluster))
-                    .orElseGet(() -> createHostedImplicitHttpWithoutAccessControl(cluster));
+            Http http = new Http(Collections.emptyList());
+            http.setFilterChains(new FilterChains(cluster));
             cluster.setHttp(http);
         }
         if(cluster.getHttp().getHttpServer() == null) {
@@ -370,27 +359,6 @@ 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 034de1113c2..b9cb5e1fae6 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 }? &  # TODO Vespa 8 Remove
-    attribute read { string "true" | string "false" }? & # TODO Vespa 8 Remove
-    attribute write { string "true" | string "false" }? & # TODO Vespa 8 Remove
+    attribute domain { xsd:NCName } &
+    attribute read { string "true" | string "false" }? &
+    attribute write { string "true" | string "false" }? &
     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 73db6e35428..17dff46bed4 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,7 +14,6 @@ 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;
@@ -44,7 +43,6 @@ 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;
@@ -835,28 +833,6 @@ 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(