Merge pull request #9074 from vespa-engine/jvenstad/access-control-allow-api-prefix

Jvenstad/access control allow api prefix 

6 files changed, 77 insertions, 38 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 797ca10ed3d..ba6f19c19ba 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
@@ -15,6 +15,7 @@ import java.util.Set;
  * When creating a new API, its paths must be added here and a policy must be declared in {@link Policy}.
  *
  * @author mpolden
+ * @author jonmv
  */
 public enum PathGroup {
 
@@ -32,29 +33,35 @@ public enum PathGroup {
 
     /** Paths used for creating tenants with proper access control. */
     tenant(Matcher.tenant,
+           Optional.of("/api"),
            "/application/v4/tenant/{tenant}"),
 
     /** Paths used for user management on the tenant level. */
     tenantUsers(Matcher.tenant,
+                Optional.of("/api"),
                 "/user/v1/tenant/{tenant}"),
 
     /** Paths used by tenant administrators. */
     tenantInfo(Matcher.tenant,
+               Optional.of("/api"),
                "/application/v4/tenant/{tenant}/application/"),
 
     /** Path for the base application resource. */
     application(Matcher.tenant,
                 Matcher.application,
+                Optional.of("/api"),
                 "/application/v4/tenant/{tenant}/application/{application}"),
 
     /** Paths used for user management on the application level. */
     applicationUsers(Matcher.tenant,
                      Matcher.application,
+                     Optional.of("/api"),
                      "/user/v1/tenant/{tenant}/application/{application}"),
 
     /** Paths used by application administrators. */
     applicationInfo(Matcher.tenant,
                     Matcher.application,
+                    Optional.of("/api"),
                     "/application/v4/tenant/{tenant}/application/{application}/deploying/{*}",
                     "/application/v4/tenant/{tenant}/application/{application}/instance/{*}",
                     "/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/logs",
@@ -65,10 +72,12 @@ public enum PathGroup {
     /** Path used to restart application nodes. */ // TODO move to the above when everyone is on new pipeline.
     applicationRestart(Matcher.tenant,
                        Matcher.application,
+                       Optional.of("/api"),
                        "/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{ignored}/restart"),
     /** Paths used for development deployments. */
     developmentDeployment(Matcher.tenant,
                           Matcher.application,
+                          Optional.of("/api"),
                           "/application/v4/tenant/{tenant}/application/{application}/environment/dev/region/{region}/instance/{instance}",
                           "/application/v4/tenant/{tenant}/application/{application}/environment/dev/region/{region}/instance/{instance}/deploy",
                           "/application/v4/tenant/{tenant}/application/{application}/environment/perf/region/{region}/instance/{instance}",
@@ -77,6 +86,7 @@ public enum PathGroup {
     /** Paths used for production deployments. */
     productionDeployment(Matcher.tenant,
                          Matcher.application,
+                         Optional.of("/api"),
                          "/application/v4/tenant/{tenant}/application/{application}/environment/prod/region/{region}/instance/{instance}",
                          "/application/v4/tenant/{tenant}/application/{application}/environment/prod/region/{region}/instance/{instance}/deploy",
                          "/application/v4/tenant/{tenant}/application/{application}/environment/test/region/{region}/instance/{instance}",
@@ -87,21 +97,26 @@ public enum PathGroup {
     /** Paths used for continuous deployment to production. */
     submission(Matcher.tenant,
                Matcher.application,
+               Optional.of("/api"),
                "/application/v4/tenant/{tenant}/application/{application}/submit"),
 
     /** Paths used for other tasks by build services. */ // TODO: This will vanish.
     buildService(Matcher.tenant,
                  Matcher.application,
+                 Optional.of("/api"),
                  "/application/v4/tenant/{tenant}/application/{application}/jobreport",
                  "/application/v4/tenant/{tenant}/application/{application}/promote",
                  "/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/promote"),
 
+    /** Paths which contain (not very strictly) classified information about customers. */
+    classifiedTenantInfo(Optional.of("/api"),
+                         "/application/v4/",
+                         "/application/v4/tenant/"),
+
     /** Paths which contain (not very strictly) classified information about, e.g., customers. */
     classifiedInfo("/athenz/v1/{*}",
                    "/cost/v1/{*}",
                    "/deployment/v1/{*}",
-                   "/application/v4/",
-                   "/application/v4/tenant/",
                    "/",
                    "/d/{*}",
                    "/statuspage/v1/{*}"),
@@ -111,30 +126,43 @@ public enum PathGroup {
                "/zone/v1/{*}");
 
     final List<String> pathSpecs;
+    final String prefix;
     final List<Matcher> matchers;
 
     PathGroup(String... pathSpecs) {
-        this(List.of(), List.of(pathSpecs));
+        this(List.of(), Optional.empty(), List.of(pathSpecs));
+    }
+
+    PathGroup(Optional<String> prefix, String... pathSpecs) {
+        this(List.of(), prefix, List.of(pathSpecs));
     }
 
     PathGroup(Matcher first, String... pathSpecs) {
-        this(List.of(first), List.of(pathSpecs));
+        this(List.of(first), Optional.empty(), List.of(pathSpecs));
+    }
+
+    PathGroup(Matcher first, Optional<String> prefix, String... pathSpecs) {
+        this(List.of(first), prefix, List.of(pathSpecs));
     }
 
     PathGroup(Matcher first, Matcher second, String... pathSpecs) {
-        this(List.of(first, second), List.of(pathSpecs));
+        this(List.of(first, second), Optional.empty(), List.of(pathSpecs));
+    }
+
+    PathGroup(Matcher first, Matcher second, Optional<String> prefix, String... pathSpecs) {
+        this(List.of(first, second), prefix, List.of(pathSpecs));
     }
 
     /** Creates a new path group, if the given context matchers are each present exactly once in each of the given specs. */
-    PathGroup(List<Matcher> matchers, List<String> pathSpecs) {
+    PathGroup(List<Matcher> matchers, Optional<String> prefix, List<String> pathSpecs) {
         this.matchers = matchers;
+        this.prefix = prefix.orElse("");
         this.pathSpecs = pathSpecs;
     }
 
     /** Returns path if it matches any spec in this group, with match groups set by the match. */
-    @SuppressWarnings("deprecation")
     private Optional<Path> get(URI uri) {
-        Path matcher = new Path(uri); // TODO Get URI down here.
+        Path matcher = new Path(uri, prefix);
         for (String spec : pathSpecs) // Iterate to be sure the Path's state is that of the match.
             if (matcher.matches(spec)) return Optional.of(matcher);
         return Optional.empty();
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index 44794600551..547e53acb77 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -132,12 +132,13 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     @Override
     public HttpResponse handle(HttpRequest request) {
         try {
+            Path path = new Path(request.getUri(), OPTIONAL_PREFIX);
             switch (request.getMethod()) {
-                case GET: return handleGET(request);
-                case PUT: return handlePUT(request);
-                case POST: return handlePOST(request);
-                case PATCH: return handlePATCH(request);
-                case DELETE: return handleDELETE(request);
+                case GET: return handleGET(path, request);
+                case PUT: return handlePUT(path, request);
+                case POST: return handlePOST(path, request);
+                case PATCH: return handlePATCH(path, request);
+                case DELETE: return handleDELETE(path, request);
                 case OPTIONS: return handleOPTIONS();
                 default: return ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is not supported");
             }
@@ -163,8 +164,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         }
     }
 
-    private HttpResponse handleGET(HttpRequest request) {
-        Path path = new Path(request.getUri(), OPTIONAL_PREFIX);
+    private HttpResponse handleGET(Path path, HttpRequest request) {
         if (path.matches("/application/v4/")) return root(request);
         if (path.matches("/application/v4/user")) return authenticatedUser(request);
         if (path.matches("/application/v4/tenant")) return tenants(request);
@@ -186,8 +186,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         return ErrorResponse.notFoundError("Nothing at " + path);
     }
 
-    private HttpResponse handlePUT(HttpRequest request) {
-        Path path = new Path(request.getUri(), OPTIONAL_PREFIX);
+    private HttpResponse handlePUT(Path path, HttpRequest request) {
         if (path.matches("/application/v4/user")) return createUser(request);
         if (path.matches("/application/v4/tenant/{tenant}")) return updateTenant(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/global-rotation/override"))
@@ -195,8 +194,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         return ErrorResponse.notFoundError("Nothing at " + path);
     }
 
-    private HttpResponse handlePOST(HttpRequest request) {
-        Path path = new Path(request.getUri(), OPTIONAL_PREFIX);
+    private HttpResponse handlePOST(Path path, HttpRequest request) {
         if (path.matches("/application/v4/tenant/{tenant}")) return createTenant(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return createApplication(path.get("tenant"), path.get("application"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/promote")) return promoteApplication(path.get("tenant"), path.get("application"), request);
@@ -214,15 +212,13 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         return ErrorResponse.notFoundError("Nothing at " + path);
     }
 
-    private HttpResponse handlePATCH(HttpRequest request) {
-        Path path = new Path(request.getUri(), OPTIONAL_PREFIX);
+    private HttpResponse handlePATCH(Path path, HttpRequest request) {
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}"))
             return setMajorVersion(path.get("tenant"), path.get("application"), request);
         return ErrorResponse.notFoundError("Nothing at " + path);
     }
 
-    private HttpResponse handleDELETE(HttpRequest request) {
-        Path path = new Path(request.getUri(), OPTIONAL_PREFIX);
+    private HttpResponse handleDELETE(Path path, HttpRequest request) {
         if (path.matches("/application/v4/tenant/{tenant}")) return deleteTenant(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return deleteApplication(path.get("tenant"), path.get("application"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying")) return cancelDeploy(path.get("tenant"), path.get("application"), "all");
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 03ffdbb0208..f5532a964fd 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
@@ -42,6 +42,7 @@ import java.util.logging.Logger;
 public class UserApiHandler extends LoggingRequestHandler {
 
     private final static Logger log = Logger.getLogger(UserApiHandler.class.getName());
+    private static final String optionalPrefix = "/api";
 
     private final UserRoles roles;
     private final UserManagement users;
@@ -56,10 +57,11 @@ public class UserApiHandler extends LoggingRequestHandler {
     @Override
     public HttpResponse handle(HttpRequest request) {
         try {
+            Path path = new Path(request.getUri(), optionalPrefix);
             switch (request.getMethod()) {
-                case GET: return handleGET(request);
-                case POST: return handlePOST(request);
-                case DELETE: return handleDELETE(request);
+                case GET: return handleGET(path, request);
+                case POST: return handlePOST(path, request);
+                case DELETE: return handleDELETE(path, request);
                 case OPTIONS: return handleOPTIONS();
                 default: return ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is not supported");
             }
@@ -73,8 +75,7 @@ public class UserApiHandler extends LoggingRequestHandler {
         }
     }
 
-    private HttpResponse handleGET(HttpRequest request) {
-        Path path = new Path(request.getUri());
+    private HttpResponse handleGET(Path path, HttpRequest request) {
         if (path.matches("/user/v1/tenant/{tenant}")) return listTenantRoleMembers(path.get("tenant"));
         if (path.matches("/user/v1/tenant/{tenant}/application/{application}")) return listApplicationRoleMembers(path.get("tenant"), path.get("application"));
 
@@ -82,8 +83,7 @@ public class UserApiHandler extends LoggingRequestHandler {
                                                          request.getUri().getPath()));
     }
 
-    private HttpResponse handlePOST(HttpRequest request) {
-        Path path = new Path(request.getUri());
+    private HttpResponse handlePOST(Path path, HttpRequest request) {
         if (path.matches("/user/v1/tenant/{tenant}")) return addTenantRoleMember(path.get("tenant"), request);
         if (path.matches("/user/v1/tenant/{tenant}/application/{application}")) return addApplicationRoleMember(path.get("tenant"), path.get("application"), request);
 
@@ -91,8 +91,7 @@ public class UserApiHandler extends LoggingRequestHandler {
                                                          request.getUri().getPath()));
     }
 
-    private HttpResponse handleDELETE(HttpRequest request) {
-        Path path = new Path(request.getUri());
+    private HttpResponse handleDELETE(Path path, HttpRequest request) {
         if (path.matches("/user/v1/tenant/{tenant}")) return removeTenantRoleMember(path.get("tenant"), request);
         if (path.matches("/user/v1/tenant/{tenant}/application/{application}")) return removeApplicationRoleMember(path.get("tenant"), path.get("application"), request);
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java
index caa44b033af..95477758deb 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java
@@ -24,14 +24,20 @@ public class ControllerContainerCloudTest extends ControllerContainerTest {
     }
 
     @Override
-    protected String securityXml() {
+    protected String variablePartXml() {
         return "  <component id='com.yahoo.vespa.hosted.controller.security.CloudAccessControlRequests'/>\n" +
                "  <component id='com.yahoo.vespa.hosted.controller.security.CloudAccessControl'/>\n" +
                "  <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.MockUserManagement'/>\n" +
                "  <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMarketplace'/>\n" +
 
+               "  <handler id='com.yahoo.vespa.hosted.controller.restapi.application.ApplicationApiHandler'>\n" +
+               "    <binding>http://*/application/v4/*</binding>\n" +
+               "    <binding>http://*/api/application/v4/*</binding>\n" +
+               "  </handler>\n" +
+
                "  <handler id='com.yahoo.vespa.hosted.controller.restapi.user.UserApiHandler'>\n" +
                "    <binding>http://*/user/v1/*</binding>\n" +
+               "    <binding>http://*/api/user/v1/*</binding>\n" +
                "  </handler>\n" +
 
                "  <http>\n" +
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java
index 716cf622e76..6abfa7fa72d 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java
@@ -97,9 +97,6 @@ public class ControllerContainerTest {
                "  <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.MockTesterCloud'/>\n" +
                "  <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMailer'/>\n" +
                "  <component id='com.yahoo.vespa.hosted.controller.api.role.Roles'/>\n" +
-               "  <handler id='com.yahoo.vespa.hosted.controller.restapi.application.ApplicationApiHandler'>\n" +
-               "    <binding>http://*/application/v4/*</binding>\n" +
-               "  </handler>\n" +
                "  <handler id='com.yahoo.vespa.hosted.controller.restapi.deployment.DeploymentApiHandler'>\n" +
                "    <binding>http://*/deployment/v1/*</binding>\n" +
                "  </handler>\n" +
@@ -123,7 +120,7 @@ public class ControllerContainerTest {
                "    <binding>http://*/zone/v2</binding>\n" +
                "    <binding>http://*/zone/v2/*</binding>\n" +
                "  </handler>\n" +
-               securityXml() +
+               variablePartXml() +
                "</jdisc>";
     }
 
@@ -131,10 +128,13 @@ public class ControllerContainerTest {
         return SystemName.main;
     }
 
-    protected String securityXml() {
+    protected String variablePartXml() {
         return "  <component id='com.yahoo.vespa.hosted.controller.security.AthenzAccessControlRequests'/>\n" +
                "  <component id='com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade'/>\n" +
 
+               "  <handler id='com.yahoo.vespa.hosted.controller.restapi.application.ApplicationApiHandler'>\n" +
+               "    <binding>http://*/application/v4/*</binding>\n" +
+               "  </handler>\n" +
                "  <handler id='com.yahoo.vespa.hosted.controller.restapi.athenz.AthenzApiHandler'>\n" +
                "    <binding>http://*/athenz/v1/*</binding>\n" +
                "  </handler>\n" +
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java
index caf0ec82aae..3a78e9fc262 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java
@@ -42,6 +42,11 @@ public class UserApiTest extends ControllerContainerCloudTest {
                                       .roles(operator),
                               "[]");
 
+        // GET at application/v4/tenant is available also under the /api prefix.
+        tester.assertResponse(request("/api/application/v4/tenant")
+                                      .roles(operator),
+                              "[]");
+
         // POST a tenant is not available to everyone.
         tester.assertResponse(request("/application/v4/tenant/my-tenant", POST)
                                       .data("{\"token\":\"hello\"}"),
@@ -120,6 +125,11 @@ public class UserApiTest extends ControllerContainerCloudTest {
                                       .roles(Set.of(roles.tenantOperator(id.tenant()))),
                               new File("application-roles.json"));
 
+        // GET application role information is available also under the /api prefix.
+        tester.assertResponse(request("/api/user/v1/tenant/my-tenant/application/my-app")
+                                      .roles(Set.of(roles.tenantOperator(id.tenant()))),
+                              new File("application-roles.json"));
+
         // DELETE an application role is allowed for an application admin.
         tester.assertResponse(request("/user/v1/tenant/my-tenant/application/my-app", DELETE)
                                       .roles(Set.of(roles.applicationAdmin(id.tenant(), id.application())))