From e5f23fcf991a0510d43d199701be6b4c7c50ed23 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Thu, 31 Oct 2019 23:44:50 +0100 Subject: Limit reads under /configserver/v1 to operators --- .../hosted/controller/api/role/PathGroup.java | 10 ++++-- .../vespa/hosted/controller/api/role/Policy.java | 2 +- .../configserver/ConfigServerApiHandlerTest.java | 41 +++++++++++++++++++--- 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 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()); -- cgit v1.2.3