Merge pull request #9145 from vespa-engine/freva/fix-authorizer

Pass identities to filter through FilterConfig

5 files changed, 60 insertions, 53 deletions

diff --git a/container-disc/src/main/java/com/yahoo/container/FilterConfigProvider.java b/container-disc/src/main/java/com/yahoo/container/FilterConfigProvider.java
index c17b9d445a2..a127cb03a01 100644
--- a/container-disc/src/main/java/com/yahoo/container/FilterConfigProvider.java
+++ b/container-disc/src/main/java/com/yahoo/container/FilterConfigProvider.java
@@ -57,7 +57,11 @@ public final class FilterConfigProvider implements Provider<FilterConfig> {
     private final FilterConfig filterConfig;
 
     public FilterConfigProvider(HttpFilterConfig vespaConfig) {
-        filterConfig = new MapFilterConfig(toMap(vespaConfig), vespaConfig.filterName(), vespaConfig.filterClass());
+        this(new MapFilterConfig(toMap(vespaConfig), vespaConfig.filterName(), vespaConfig.filterClass()));
+    }
+
+    private FilterConfigProvider(FilterConfig filterConfig) {
+        this.filterConfig = filterConfig;
     }
 
     private static Map<String, String> toMap(HttpFilterConfig vespaConfig) {
@@ -67,6 +71,10 @@ public final class FilterConfigProvider implements Provider<FilterConfig> {
         return parameters;
     }
 
+    public static FilterConfigProvider from(String filterName, String filterClass, Map<String, String> initParameters) {
+        return new FilterConfigProvider(new MapFilterConfig(initParameters, filterName, filterClass));
+    }
+
     @Override
     public FilterConfig get() {
         return filterConfig;
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java
index 28ead318cc0..d15cd288f39 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java
@@ -2,10 +2,11 @@
 package com.yahoo.vespa.hosted.provision.restapi.v2.filter;
 
 import com.google.inject.Inject;
-import com.yahoo.config.provision.Zone;
 import com.yahoo.jdisc.handler.ResponseHandler;
 import com.yahoo.jdisc.http.filter.DiscFilterRequest;
+import com.yahoo.jdisc.http.filter.FilterConfig;
 import com.yahoo.jdisc.http.filter.SecurityRequestFilter;
+import com.yahoo.vespa.athenz.utils.AthenzIdentities;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.restapi.v2.ErrorResponse;
 import com.yahoo.yolean.chain.After;
@@ -31,8 +32,12 @@ public class AuthorizationFilter implements SecurityRequestFilter {
     private final BiConsumer<ErrorResponse, ResponseHandler> rejectAction;
 
     @Inject
-    public AuthorizationFilter(Zone zone, NodeRepository nodeRepository) {
-        this.authorizer = new Authorizer(zone.system(), nodeRepository);
+    public AuthorizationFilter(NodeRepository nodeRepository, FilterConfig filterConfig) {
+        this.authorizer = new Authorizer(nodeRepository,
+                AthenzIdentities.from(filterConfig.getInitParameter("controller.identity")),
+                AthenzIdentities.from(filterConfig.getInitParameter("configserver.identity")),
+                AthenzIdentities.from(filterConfig.getInitParameter("proxy.identity")),
+                AthenzIdentities.from(filterConfig.getInitParameter("tenant-host.identity")));
         this.rejectAction = AuthorizationFilter::logAndReject;
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/Authorizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/Authorizer.java
index afcde0949e3..44636727531 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/Authorizer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/Authorizer.java
@@ -2,9 +2,7 @@
 package com.yahoo.vespa.hosted.provision.restapi.v2.filter;
 
 import com.yahoo.config.provision.NodeType;
-import com.yahoo.config.provision.SystemName;
 import com.yahoo.vespa.athenz.api.AthenzIdentity;
-import com.yahoo.vespa.athenz.api.AthenzService;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import org.apache.http.NameValuePair;
@@ -15,7 +13,6 @@ import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -32,19 +29,15 @@ import java.util.stream.Collectors;
 public class Authorizer implements BiPredicate<NodePrincipal, URI> {
     private final NodeRepository nodeRepository;
     private final AthenzIdentity controllerIdentity;
-    private final AthenzIdentity configServerIdentity = new AthenzService("vespa.vespa", "configserver");
-    private final AthenzIdentity proxyIdentity = new AthenzService("vespa.vespa", "proxy");
-    private final AthenzIdentity tenantIdentity = new AthenzService("vespa.vespa", "tenant-host");
     private final Set<AthenzIdentity> trustedIdentities;
     private final Set<AthenzIdentity> hostAdminIdentities;
 
-    public Authorizer(SystemName system, NodeRepository nodeRepository) {
+    Authorizer(NodeRepository nodeRepository, AthenzIdentity controllerIdentity, AthenzIdentity configServerIdentity,
+               AthenzIdentity proxyIdentity, AthenzIdentity tenantIdentity) {
         this.nodeRepository = nodeRepository;
-        controllerIdentity = system == SystemName.main
-                ? new AthenzService("vespa.vespa", "hosting")
-                : new AthenzService("vespa.vespa.cd", "hosting");
-        this.trustedIdentities = new HashSet<>(Arrays.asList(controllerIdentity, configServerIdentity));
-        this.hostAdminIdentities = new HashSet<>(Arrays.asList(controllerIdentity, configServerIdentity, proxyIdentity, tenantIdentity));
+        this.controllerIdentity = controllerIdentity;
+        this.trustedIdentities = Set.of(controllerIdentity, configServerIdentity);
+        this.hostAdminIdentities = Set.of(controllerIdentity, configServerIdentity, proxyIdentity, tenantIdentity);
     }
 
     /** Returns whether principal is authorized to access given URI */
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java
index 2969b608d3a..2f1755dd106 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java
@@ -2,28 +2,23 @@
 package com.yahoo.vespa.hosted.provision.restapi.v2.filter;
 
 import com.yahoo.application.container.handler.Request.Method;
-import com.yahoo.config.provision.Environment;
-import com.yahoo.config.provision.RegionName;
-import com.yahoo.config.provision.SystemName;
-import com.yahoo.config.provision.Zone;
+import com.yahoo.container.FilterConfigProvider;
 import com.yahoo.vespa.curator.mock.MockCurator;
 import com.yahoo.vespa.hosted.provision.restapi.v2.filter.FilterTester.Request;
 import com.yahoo.vespa.hosted.provision.testutils.MockNodeFlavors;
 import com.yahoo.vespa.hosted.provision.testutils.MockNodeRepository;
-import org.junit.Before;
 import org.junit.Test;
 
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
 /**
  * @author mpolden
  */
 public class AuthorizationFilterTest {
 
-    private FilterTester tester;
-
-    @Before
-    public void before() {
-        tester = filterTester(SystemName.main);
-    }
+    private final FilterTester tester = filterTester();
 
     @Test
     public void filter() {
@@ -43,11 +38,12 @@ public class AuthorizationFilterTest {
         tester.assertSuccess(new Request(Method.GET, "/nodes/v2/node/foo").commonName("foo"));
     }
 
-    private static FilterTester filterTester(SystemName system) {
-        Zone zone = new Zone(system, Environment.prod, RegionName.defaultName());
+    private static FilterTester filterTester() {
+        Map<String, String> params = Stream.of("controller", "configserver", "proxy", "tenant-host")
+                .collect(Collectors.toMap(e -> e + ".identity", e -> "vespa." + e));
         return new FilterTester(new AuthorizationFilter(
-                zone,
-                new MockNodeRepository(new MockCurator(), new MockNodeFlavors())));
+                new MockNodeRepository(new MockCurator(), new MockNodeFlavors()),
+                FilterConfigProvider.from("my-filter", AuthorizationFilter.class.getName(), params).get()));
     }
 
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizerTest.java
index d696328cd7f..b539420a55a 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizerTest.java
@@ -4,7 +4,7 @@ package com.yahoo.vespa.hosted.provision.restapi.v2.filter;
 import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.config.provision.NodeType;
-import com.yahoo.config.provision.SystemName;
+import com.yahoo.vespa.athenz.utils.AthenzIdentities;
 import com.yahoo.vespa.curator.mock.MockCurator;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.testutils.MockNodeFlavors;
@@ -17,7 +17,6 @@ import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 
-import static java.util.Collections.emptyList;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -26,14 +25,22 @@ import static org.junit.Assert.assertTrue;
  */
 public class AuthorizerTest {
 
+    private static final String CONTROLLER_IDENTITY = "vespa.controller";
+    private static final String CONFIG_SERVER_IDENTITY = "vespa.configserver";
+    private static final String PROXY_IDENTITY = "vespa.proxy";
+    private static final String TENANT_HOST_IDENTITY = "vespa.tenant-host";
+
     private Authorizer authorizer;
-    private MockNodeRepository nodeRepository;
 
     @Before
     public void before() {
         NodeFlavors flavors = new MockNodeFlavors();
-        nodeRepository = new MockNodeRepository(new MockCurator(), flavors);
-        authorizer = new Authorizer(SystemName.main, nodeRepository);
+        MockNodeRepository nodeRepository = new MockNodeRepository(new MockCurator(), flavors);
+        authorizer = new Authorizer(nodeRepository,
+                AthenzIdentities.from(CONTROLLER_IDENTITY),
+                AthenzIdentities.from(CONFIG_SERVER_IDENTITY),
+                AthenzIdentities.from(PROXY_IDENTITY),
+                AthenzIdentities.from(TENANT_HOST_IDENTITY));
 
         Set<String> ipAddresses = Set.of("127.0.0.1", "::1");
         Flavor flavor = flavors.getFlavorOrThrow("default");
@@ -98,13 +105,11 @@ public class AuthorizerTest {
         assertTrue(authorizedTenantHostNode("host1", "/athenz/v1/provider/identity-document/node/child1-1"));
 
         // Trusted services can access everything in their own system
-        assertFalse(authorizedController("vespa.vespa.cd.hosting", "/")); // Wrong system
-        assertTrue(new Authorizer(SystemName.cd, nodeRepository).test(NodePrincipal.withAthenzIdentity("vespa.vespa.cd.hosting", emptyList()), uri("/")));
-        assertTrue(authorizedController("vespa.vespa.hosting", "/"));
-        assertTrue(authorizedController("vespa.vespa.configserver", "/"));
-        assertTrue(authorizedController("vespa.vespa.hosting", "/nodes/v2/node/"));
-        assertTrue(authorizedController("vespa.vespa.hosting", "/nodes/v2/node/node1"));
-        assertTrue(authorizedController("vespa.vespa.configserver", "/nodes/v2/node/node1"));
+        assertTrue(authorizedController(CONTROLLER_IDENTITY, "/"));
+        assertTrue(authorizedController(CONFIG_SERVER_IDENTITY, "/"));
+        assertTrue(authorizedController(CONTROLLER_IDENTITY, "/nodes/v2/node/"));
+        assertTrue(authorizedController(CONTROLLER_IDENTITY, "/nodes/v2/node/node1"));
+        assertTrue(authorizedController(CONFIG_SERVER_IDENTITY, "/nodes/v2/node/node1"));
     }
 
     @Test
@@ -145,14 +150,14 @@ public class AuthorizerTest {
         assertTrue(authorizedTenantHostNode("proxy1-host", "/flags/v1/data"));
         assertFalse(authorizedTenantHostNode("proxy1-host", "/flags/v1/data/flagid"));
         assertFalse(authorizedTenantHostNode("proxy1-host", "/flags/v1/foo"));
-        assertTrue(authorizedController("vespa.vespa.configserver", "/flags/v1/data"));
-        assertFalse(authorizedController("vespa.vespa.configserver", "/flags/v1/data/flagid"));
-        assertFalse(authorizedController("vespa.vespa.configserver", "/flags/v1/foo"));
+        assertTrue(authorizedController(CONFIG_SERVER_IDENTITY, "/flags/v1/data"));
+        assertFalse(authorizedController(CONFIG_SERVER_IDENTITY, "/flags/v1/data/flagid"));
+        assertFalse(authorizedController(CONFIG_SERVER_IDENTITY, "/flags/v1/foo"));
 
         // Controller can access everything
-        assertTrue(authorizedController("vespa.vespa.hosting", "/flags/v1/data"));
-        assertTrue(authorizedController("vespa.vespa.hosting", "/flags/v1/data/flagid"));
-        assertTrue(authorizedController("vespa.vespa.hosting", "/flags/v1/foo"));
+        assertTrue(authorizedController(CONTROLLER_IDENTITY, "/flags/v1/data"));
+        assertTrue(authorizedController(CONTROLLER_IDENTITY, "/flags/v1/data/flagid"));
+        assertTrue(authorizedController(CONTROLLER_IDENTITY, "/flags/v1/foo"));
     }
 
     @Test
@@ -170,19 +175,19 @@ public class AuthorizerTest {
     }
 
     private boolean authorizedTenantNode(String hostname, String path) {
-        return authorized(NodePrincipal.withAthenzIdentity("vespa.vespa.tenant", hostname, emptyList()), path);
+        return authorized(NodePrincipal.withAthenzIdentity("vespa.vespa.tenant", hostname, List.of()), path);
     }
 
     private boolean authorizedTenantHostNode(String hostname, String path) {
-        return authorized(NodePrincipal.withAthenzIdentity("vespa.vespa.tenant-host", hostname, emptyList()), path);
+        return authorized(NodePrincipal.withAthenzIdentity(TENANT_HOST_IDENTITY, hostname, List.of()), path);
     }
 
     private boolean authorizedLegacyNode(String hostname, String path) {
-        return authorized(NodePrincipal.withLegacyIdentity(hostname, emptyList()), path);
+        return authorized(NodePrincipal.withLegacyIdentity(hostname, List.of()), path);
     }
 
     private boolean authorizedController(String controllerIdentity, String path) {
-        return authorized(NodePrincipal.withAthenzIdentity(controllerIdentity, emptyList()), path);
+        return authorized(NodePrincipal.withAthenzIdentity(controllerIdentity, List.of()), path);
     }
 
     private boolean authorized(NodePrincipal principal, String path) {