Control Horizon proxy usage through feature flag

6 files changed, 129 insertions, 45 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/HorizonApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/HorizonApiHandler.java
index 6f5b1f30592..19a5024833b 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/HorizonApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/HorizonApiHandler.java
@@ -3,22 +3,33 @@ package com.yahoo.vespa.hosted.controller.restapi.horizon;
 
 import com.google.inject.Inject;
 import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.TenantName;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.container.jdisc.LoggingRequestHandler;
 import com.yahoo.restapi.ErrorResponse;
 import com.yahoo.restapi.Path;
+import com.yahoo.vespa.flags.BooleanFlag;
+import com.yahoo.vespa.flags.FetchVector;
+import com.yahoo.vespa.flags.FlagSource;
+import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.integration.horizon.HorizonClient;
 import com.yahoo.vespa.hosted.controller.api.integration.horizon.HorizonResponse;
+import com.yahoo.vespa.hosted.controller.api.role.Role;
+import com.yahoo.vespa.hosted.controller.api.role.RoleDefinition;
 import com.yahoo.vespa.hosted.controller.api.role.SecurityContext;
+import com.yahoo.vespa.hosted.controller.api.role.TenantRole;
 import com.yahoo.yolean.Exceptions;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.util.EnumSet;
 import java.util.Optional;
+import java.util.Set;
 import java.util.logging.Level;
+import java.util.stream.Collectors;
 
 /**
  * Proxies metrics requests from Horizon UI
@@ -29,20 +40,32 @@ public class HorizonApiHandler extends LoggingRequestHandler {
 
     private final SystemName systemName;
     private final HorizonClient client;
+    private final BooleanFlag enabledHorizonDashboard;
+
+    private static final EnumSet<RoleDefinition> operatorRoleDefinitions =
+            EnumSet.of(RoleDefinition.hostedOperator, RoleDefinition.hostedSupporter);
 
     @Inject
-    public HorizonApiHandler(LoggingRequestHandler.Context parentCtx, Controller controller) {
+    public HorizonApiHandler(LoggingRequestHandler.Context parentCtx, Controller controller, FlagSource flagSource) {
         super(parentCtx);
         this.systemName = controller.system();
         this.client = controller.serviceRegistry().horizonClient();
+        this.enabledHorizonDashboard = Flags.ENABLED_HORIZON_DASHBOARD.bindTo(flagSource);
     }
 
     @Override
     public HttpResponse handle(HttpRequest request) {
+        var roles = getRoles(request);
+        var operator = roles.stream().map(Role::definition).anyMatch(operatorRoleDefinitions::contains);
+        var authorizedTenants = getAuthorizedTenants(roles);
+
+        if (!operator && authorizedTenants.isEmpty())
+            return ErrorResponse.forbidden("No tenant with enabled metrics view");
+
         try {
             switch (request.getMethod()) {
                 case GET: return get(request);
-                case POST: return post(request);
+                case POST: return post(request, authorizedTenants, operator);
                 case PUT: return put(request);
                 default: return ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is not supported");
             }
@@ -65,10 +88,10 @@ public class HorizonApiHandler extends LoggingRequestHandler {
         return ErrorResponse.notFoundError("Nothing at " + path);
     }
 
-    private HttpResponse post(HttpRequest request) {
+    private HttpResponse post(HttpRequest request, Set<TenantName> authorizedTenants, boolean operator) {
         Path path = new Path(request.getUri());
-        if (path.matches("/horizon/v1/tsdb/api/query/graph")) return tsdbQuery(request, true);
-        if (path.matches("/horizon/v1/meta/search/timeseries")) return tsdbQuery(request, false);
+        if (path.matches("/horizon/v1/tsdb/api/query/graph")) return tsdbQuery(request, authorizedTenants, operator, true);
+        if (path.matches("/horizon/v1/meta/search/timeseries")) return tsdbQuery(request, authorizedTenants, operator, false);
         return ErrorResponse.notFoundError("Nothing at " + path);
     }
 
@@ -78,10 +101,9 @@ public class HorizonApiHandler extends LoggingRequestHandler {
         return ErrorResponse.notFoundError("Nothing at " + path);
     }
 
-    private HttpResponse tsdbQuery(HttpRequest request, boolean isMetricQuery) {
-        SecurityContext securityContext = getAttribute(request, SecurityContext.ATTRIBUTE_NAME, SecurityContext.class);
+    private HttpResponse tsdbQuery(HttpRequest request, Set<TenantName> authorizedTenants, boolean operator, boolean isMetricQuery) {
         try {
-            byte[] data = TsdbQueryRewriter.rewrite(request.getData().readAllBytes(), securityContext.roles(), systemName);
+            byte[] data = TsdbQueryRewriter.rewrite(request.getData().readAllBytes(), authorizedTenants, operator, systemName);
             return new JsonInputStreamResponse(isMetricQuery ? client.getMetrics(data) : client.getMetaData(data));
         } catch (TsdbQueryRewriter.UnauthorizedException e) {
             return ErrorResponse.forbidden("Access denied");
@@ -90,11 +112,20 @@ public class HorizonApiHandler extends LoggingRequestHandler {
         }
     }
 
-    private static <T> T getAttribute(HttpRequest request, String attributeName, Class<T> clazz) {
-        return Optional.ofNullable(request.getJDiscRequest().context().get(attributeName))
-                .filter(clazz::isInstance)
-                .map(clazz::cast)
-                .orElseThrow(() -> new IllegalArgumentException("Attribute '" + attributeName + "' was not set on request"));
+    private static Set<Role> getRoles(HttpRequest request) {
+        return Optional.ofNullable(request.getJDiscRequest().context().get(SecurityContext.ATTRIBUTE_NAME))
+                .filter(SecurityContext.class::isInstance)
+                .map(SecurityContext.class::cast)
+                .map(SecurityContext::roles)
+                .orElseThrow(() -> new IllegalArgumentException("Attribute '" + SecurityContext.ATTRIBUTE_NAME + "' was not set on request"));
+    }
+
+    private Set<TenantName> getAuthorizedTenants(Set<Role> roles) {
+        return roles.stream()
+                .filter(TenantRole.class::isInstance)
+                .map(role -> ((TenantRole) role).tenant())
+                .filter(tenant -> enabledHorizonDashboard.with(FetchVector.Dimension.TENANT_ID, tenant.value()).value())
+                .collect(Collectors.toUnmodifiableSet());
     }
 
     private static class JsonInputStreamResponse extends HttpResponse {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriter.java
index e034be46063..3e20584dbac 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriter.java
@@ -7,12 +7,8 @@ import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.TenantName;
-import com.yahoo.vespa.hosted.controller.api.role.Role;
-import com.yahoo.vespa.hosted.controller.api.role.RoleDefinition;
-import com.yahoo.vespa.hosted.controller.api.role.TenantRole;
 
 import java.io.IOException;
-import java.util.EnumSet;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -23,20 +19,8 @@ import java.util.stream.Collectors;
 public class TsdbQueryRewriter {
 
     private static final ObjectMapper mapper = new ObjectMapper();
-    private static final EnumSet<RoleDefinition> operatorRoleDefinitions =
-            EnumSet.of(RoleDefinition.hostedOperator, RoleDefinition.hostedSupporter);
-
-    public static byte[] rewrite(byte[] data, Set<Role> roles, SystemName systemName) throws IOException {
-        boolean operator = roles.stream().map(Role::definition).anyMatch(operatorRoleDefinitions::contains);
-
-        // Anyone with any tenant relation can view metrics for apps within those tenants
-        Set<TenantName> authorizedTenants = roles.stream()
-                .filter(TenantRole.class::isInstance)
-                .map(role -> ((TenantRole) role).tenant())
-                .collect(Collectors.toUnmodifiableSet());
-        if (!operator && authorizedTenants.isEmpty())
-            throw new UnauthorizedException();
 
+    public static byte[] rewrite(byte[] data, Set<TenantName> authorizedTenants, boolean operator, SystemName systemName) throws IOException {
         JsonNode root = mapper.readTree(data);
         requireLegalType(root);
         getField(root, "executionGraph", ArrayNode.class)
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java
index 7b0a2c9d6d6..0ecc8ac81df 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java
@@ -22,6 +22,7 @@ import com.yahoo.text.Text;
 import com.yahoo.vespa.flags.BooleanFlag;
 import com.yahoo.vespa.flags.FetchVector;
 import com.yahoo.vespa.flags.FlagSource;
+import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.flags.IntFlag;
 import com.yahoo.vespa.flags.PermanentFlags;
 import com.yahoo.vespa.hosted.controller.Controller;
@@ -71,6 +72,7 @@ public class UserApiHandler extends LoggingRequestHandler {
     private final Controller controller;
     private final BooleanFlag enable_public_signup_flow;
     private final IntFlag maxTrialTenants;
+    private final BooleanFlag enabledHorizonDashboard;
 
     @Inject
     public UserApiHandler(Context parentCtx, UserManagement users, Controller controller, FlagSource flagSource) {
@@ -79,6 +81,7 @@ public class UserApiHandler extends LoggingRequestHandler {
         this.controller = controller;
         this.enable_public_signup_flow = PermanentFlags.ENABLE_PUBLIC_SIGNUP_FLOW.bindTo(flagSource);
         this.maxTrialTenants = PermanentFlags.MAX_TRIAL_TENANTS.bindTo(flagSource);
+        this.enabledHorizonDashboard = Flags.ENABLED_HORIZON_DASHBOARD.bindTo(flagSource);
     }
 
     @Override
@@ -184,6 +187,10 @@ public class UserApiHandler extends LoggingRequestHandler {
                     Cursor tenantRolesObject = tenantObject.setArray("roles");
                     tenantRolesByTenantName.getOrDefault(tenant, List.of())
                             .forEach(role -> tenantRolesObject.addString(role.definition().name()));
+                    if (controller.system().isPublic()) {
+                        tenantObject.setBool(enabledHorizonDashboard.id().toString(),
+                                enabledHorizonDashboard.with(FetchVector.Dimension.TENANT_ID, tenant.value()).value());
+                    }
                 });
 
         if (!operatorRoles.isEmpty()) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/horizon/HorizonApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/horizon/HorizonApiTest.java
new file mode 100644
index 00000000000..ae2e57d1529
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/horizon/HorizonApiTest.java
@@ -0,0 +1,64 @@
+package com.yahoo.vespa.hosted.controller.restapi.horizon;
+
+import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.TenantName;
+import com.yahoo.vespa.flags.Flags;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
+import com.yahoo.vespa.hosted.controller.api.role.Role;
+import com.yahoo.vespa.hosted.controller.restapi.ContainerTester;
+import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerCloudTest;
+import org.junit.Test;
+
+import java.util.Set;
+
+/**
+ * @author olaa
+ */
+public class HorizonApiTest extends ControllerContainerCloudTest {
+
+    @Test
+    public void only_operators_and_flag_enabled_tenants_allowed() {
+        ContainerTester tester = new ContainerTester(container, "");
+
+        tester.assertResponse(request("/horizon/v1/config/dashboard/topFolders")
+                        .roles(Set.of(Role.hostedOperator())),
+                "", 200);
+
+        tester.assertResponse(request("/horizon/v1/config/dashboard/topFolders")
+                        .roles(Set.of(Role.reader(TenantName.from("tenant")))),
+                "{\"error-code\":\"FORBIDDEN\",\"message\":\"No tenant with enabled metrics view\"}", 403);
+
+        ((InMemoryFlagSource) tester.controller().flagSource())
+                .withBooleanFlag(Flags.ENABLED_HORIZON_DASHBOARD.id(), true);
+
+        tester.assertResponse(request("/horizon/v1/config/dashboard/topFolders")
+                        .roles(Set.of(Role.reader(TenantName.from("tenant")))),
+                "", 200);
+    }
+
+    @Override
+    protected SystemName system() {
+        return SystemName.PublicCd;
+    }
+
+    @Override
+    protected String variablePartXml() {
+        return "  <component id='com.yahoo.vespa.hosted.controller.security.CloudAccessControlRequests'/>\n" +
+                "  <component id='com.yahoo.vespa.hosted.controller.security.CloudAccessControl'/>\n" +
+
+                "  <handler id=\"com.yahoo.vespa.hosted.controller.restapi.horizon.HorizonApiHandler\" bundle=\"controller-server\">\n" +
+                "       <binding>http://*/horizon/v1/*</binding>\n" +
+                "  </handler>\n" +
+
+                "  <http>\n" +
+                "    <server id='default' port='8080' />\n" +
+                "    <filtering>\n" +
+                "      <request-chain id='default'>\n" +
+                "        <filter id='com.yahoo.vespa.hosted.controller.restapi.filter.ControllerAuthorizationFilter'/>\n" +
+                "        <binding>http://*/*</binding>\n" +
+                "      </request-chain>\n" +
+                "    </filtering>\n" +
+                "  </http>\n";
+    }
+}
\ No newline at end of file
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriterTest.java
index ab9d50f8eae..d31d9c28c6c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriterTest.java
@@ -22,33 +22,28 @@ public class TsdbQueryRewriterTest {
 
     @Test
     public void rewrites_query() throws IOException {
-        assertRewrite("filters-complex.json", "filters-complex.expected.json", Role.reader(TenantName.from("tenant2")));
+        assertRewrite("filters-complex.json", "filters-complex.expected.json", Set.of(TenantName.from("tenant2")), false);
 
         assertRewrite("filter-in-execution-graph.json",
                 "filter-in-execution-graph.expected.json",
-                Role.reader(TenantName.from("tenant2")), Role.athenzTenantAdmin(TenantName.from("tenant3")));
+                Set.of(TenantName.from("tenant2"), TenantName.from("tenant3")), false);
 
         assertRewrite("filter-in-execution-graph.json",
                 "filter-in-execution-graph.expected.operator.json",
-                Role.reader(TenantName.from("tenant2")), Role.athenzTenantAdmin(TenantName.from("tenant3")), Role.hostedOperator());
+                Set.of(TenantName.from("tenant2"), TenantName.from("tenant3")), true);
 
         assertRewrite("no-filters.json",
                 "no-filters.expected.json",
-                Role.reader(TenantName.from("tenant2")), Role.athenzTenantAdmin(TenantName.from("tenant3")));
+                Set.of(TenantName.from("tenant2"), TenantName.from("tenant3")), false);
 
         assertRewrite("filters-meta-query.json",
                 "filters-meta-query.expected.json",
-                Role.reader(TenantName.from("tenant2")), Role.athenzTenantAdmin(TenantName.from("tenant3")));
+                Set.of(TenantName.from("tenant2"), TenantName.from("tenant3")), false);
     }
 
-    @Test(expected = TsdbQueryRewriter.UnauthorizedException.class)
-    public void throws_if_no_roles() throws IOException {
-        assertRewrite("filters-complex.json", "filters-complex.expected.json");
-    }
-
-    private static void assertRewrite(String initialFilename, String expectedFilename, Role... roles) throws IOException {
+    private static void assertRewrite(String initialFilename, String expectedFilename, Set<TenantName> tenants, boolean operator) throws IOException {
         byte[] data = Files.readAllBytes(Paths.get("src/test/resources/horizon", initialFilename));
-        data = TsdbQueryRewriter.rewrite(data, Set.of(roles), SystemName.Public);
+        data = TsdbQueryRewriter.rewrite(data, tenants, operator, SystemName.Public);
 
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         new JsonFormat(false).encode(baos, SlimeUtils.jsonToSlime(data));
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-with-applications-cloud.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-with-applications-cloud.json
index ae3dc68d9e3..e883993cb53 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-with-applications-cloud.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-with-applications-cloud.json
@@ -14,17 +14,20 @@
       "roles": [
         "developer",
         "reader"
-      ]
+      ],
+      "enabled-horizon-dashboard":false
     },
     "tenant1": {
       "roles": [
         "administrator"
-      ]
+      ],
+      "enabled-horizon-dashboard":false
     },
     "tenant2": {
       "roles": [
         "developer"
-      ]
+      ],
+      "enabled-horizon-dashboard":false
     }
   }
 }