Limit reads under /configserver/v1 to operators

3 files changed, 46 insertions, 7 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java
index b310bb69765..04ac3f421de 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java
@@ -19,9 +19,11 @@ import java.util.Set;
  */
 enum PathGroup {
 
+    /** Paths exclusive to operators (including read), used for system management. */
+    classifiedOperator("/configserver/v1/{*}"),
+
     /** Paths used for system management by operators. */
-    operator("/configserver/v1/{*}",
-             "/controller/v1/{*}",
+    operator("/controller/v1/{*}",
              "/flags/v1/{*}",
              "/nodes/v2/{*}",
              "/orchestrator/v1/{*}",
@@ -229,6 +231,10 @@ enum PathGroup {
         return EnumSet.allOf(PathGroup.class);
     }
 
+    static Set<PathGroup> allExcept(PathGroup... pathGroups) {
+        return EnumSet.complementOf(EnumSet.copyOf(List.of(pathGroups)));
+    }
+
     /** Returns whether this group matches path in given context */
     boolean matches(URI uri, Context context) {
         return get(uri).map(p -> {
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java
index db7dd5909b3..e0341d76950 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java
@@ -115,7 +115,7 @@ enum Policy {
 
     /** Read access to all information in select systems. */
     classifiedRead(Privilege.grant(Action.read)
-                            .on(PathGroup.all())
+                            .on(PathGroup.allExcept(PathGroup.classifiedOperator))
                             .in(SystemName.main, SystemName.cd, SystemName.dev)),
 
     /** Read access to public info. */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandlerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandlerTest.java
index 00e90114200..af10f8e9c49 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandlerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandlerTest.java
@@ -49,11 +49,11 @@ public class ConfigServerApiHandlerTest extends ControllerContainerTest {
     @Test
     public void test_requests() {
         // GET /configserver/v1
-        tester.containerTester().assertResponse(authenticatedRequest("http://localhost:8080/configserver/v1"),
+        tester.containerTester().assertResponse(operatorRequest("http://localhost:8080/configserver/v1"),
                 new File("root.json"));
 
         // GET /configserver/v1/nodes/v2/node/?recursive=true
-        tester.containerTester().assertResponse(authenticatedRequest("http://localhost:8080/configserver/v1/prod/us-north-1/nodes/v2/node/?recursive=true"),
+        tester.containerTester().assertResponse(operatorRequest("http://localhost:8080/configserver/v1/prod/us-north-1/nodes/v2/node/?recursive=true"),
                 "ok");
         assertLastRequest("https://cfg.prod.us-north-1.test.vip:4443/", "GET");
 
@@ -85,11 +85,11 @@ public class ConfigServerApiHandlerTest extends ControllerContainerTest {
     @Test
     public void test_allowed_apis() {
         // GET /configserver/v1/prod/us-north-1
-        tester.containerTester().assertResponse(() -> authenticatedRequest("http://localhost:8080/configserver/v1/prod/us-north-1"),
+        tester.containerTester().assertResponse(() -> operatorRequest("http://localhost:8080/configserver/v1/prod/us-north-1"),
                 "{\"error-code\":\"FORBIDDEN\",\"message\":\"Cannot access '/' through /configserver/v1, following APIs are permitted: /flags/v1/, /nodes/v2/, /orchestrator/v1/\"}",
                 403);
 
-        tester.containerTester().assertResponse(() -> authenticatedRequest("http://localhost:8080/configserver/v1/prod/us-north-1/application/v2/tenant/vespa"),
+        tester.containerTester().assertResponse(() -> operatorRequest("http://localhost:8080/configserver/v1/prod/us-north-1/application/v2/tenant/vespa"),
                 "{\"error-code\":\"FORBIDDEN\",\"message\":\"Cannot access '/application/v2/tenant/vespa' through /configserver/v1, following APIs are permitted: /flags/v1/, /nodes/v2/, /orchestrator/v1/\"}",
                 403);
     }
@@ -103,6 +103,39 @@ public class ConfigServerApiHandlerTest extends ControllerContainerTest {
         assertFalse(proxy.lastReceived().isPresent());
     }
 
+    @Test
+    public void non_operators_are_forbidden() {
+        // Read request
+        tester.containerTester().assertResponse(() -> authenticatedRequest("http://localhost:8080/configserver/v1/prod/us-north-1/nodes/v2/node"),
+                "{\n" +
+                "  \"code\" : 403,\n" +
+                "  \"message\" : \"Access denied\"\n" +
+                "}", 403);
+
+        // Write request
+        tester.containerTester().assertResponse(() -> authenticatedRequest("http://localhost:8080/configserver/v1/prod/us-north-1/nodes/v2/node", "", Request.Method.POST),
+                "{\n" +
+                "  \"code\" : 403,\n" +
+                "  \"message\" : \"Access denied\"\n" +
+                "}", 403);
+    }
+
+    @Test
+    public void unauthenticated_request_are_unauthorized() {
+        {
+            // Read request
+            Request request = new Request("http://localhost:8080/configserver/v1/prod/us-north-1/nodes/v2/node", "", Request.Method.GET);
+            tester.containerTester().assertResponse(() -> request, "{\n  \"message\" : \"Not authenticated\"\n}", 401);
+        }
+
+        {
+            // Write request
+            Request request = new Request("http://localhost:8080/configserver/v1/prod/us-north-1/nodes/v2/node", "", Request.Method.POST);
+            tester.containerTester().assertResponse(() -> request, "{\n  \"message\" : \"Not authenticated\"\n}", 401);
+        }
+    }
+
+
     private void assertLastRequest(String target, String method) {
         ProxyRequest last = proxy.lastReceived().orElseThrow();
         assertEquals(List.of(URI.create(target)), last.getTargets());