Update path groups and policy to only allow access request approval by tenant admin

4 files changed, 20 insertions, 20 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 8bd7a9e726d..3a5bb96d985 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
@@ -233,7 +233,13 @@ enum PathGroup {
     secretStore(Matcher.tenant, "/application/v4/tenant/{tenant}/secret-store/{*}"),
 
     /** Paths used to proxy Horizon metric requests */
-    horizonProxy("/horizon/v1/{*}");
+    horizonProxy("/horizon/v1/{*}"),
+
+    /** Paths used to list and request access to tenant resources */
+    accessRequests(Matcher.tenant, "/application/v4/tenant/{tenant}/access/request/{*}"),
+
+    /** Paths used to approve requests to access tenant resources */
+    accessRequestApproval(Matcher.tenant, "/application/v4/tenant/{tenant}/access/approve/{*}");
 
     final List<String> pathSpecs;
     final List<Matcher> matchers;
@@ -281,9 +287,10 @@ enum PathGroup {
         return EnumSet.complementOf(EnumSet.copyOf(pathGroups));
     }
 
-    static Set<PathGroup> billingPaths() {
+    static Set<PathGroup> operatorRestrictedPaths() {
         var paths = billingPathsNoToken();
         paths.add(PathGroup.billingToken);
+        paths.add(accessRequestApproval);
         return paths;
     }
 
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 d40027d91a7..4d9a16c34f0 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
@@ -22,7 +22,7 @@ enum Policy {
 
     /** Full access to everything. */
     operator(Privilege.grant(Action.all())
-                      .on(PathGroup.allExcept(PathGroup.billingPaths()))
+                      .on(PathGroup.allExcept(PathGroup.operatorRestrictedPaths()))
                       .in(SystemName.all()),
              Privilege.grant(Action.read)
                       .on(PathGroup.billingPathsNoToken())
@@ -188,6 +188,10 @@ enum Policy {
                                     .on(PathGroup.billingList, PathGroup.billing)
                                     .in(SystemName.PublicCd, SystemName.Public)),
 
+    accessRequests(Privilege.grant(Action.all())
+            .on(PathGroup.accessRequests, PathGroup.accessRequestApproval)
+            .in(SystemName.PublicCd)),
+
     /** Invoice management */
     hostedAccountant(Privilege.grant(Action.all())
                                     .on(PathGroup.hostedAccountant, PathGroup.accountant)
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java
index aed5c08f0db..7f6e77faf03 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java
@@ -75,7 +75,9 @@ public enum RoleDefinition {
                   Policy.paymentInstrumentDelete,
                   Policy.paymentInstrumentCreate,
                   Policy.planUpdate,
-                  Policy.billingInformationRead),
+                  Policy.billingInformationRead,
+                  Policy.accessRequests
+            ),
 
     /** Headless — the application specific role identified by deployment keys for production */
     headless(Policy.submission),
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 71b5984c7cd..6b622d83e92 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
@@ -235,7 +235,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
         if (path.matches("/application/v4/")) return root(request);
         if (path.matches("/application/v4/tenant")) return tenants(request);
         if (path.matches("/application/v4/tenant/{tenant}")) return tenant(path.get("tenant"), request);
-        if (path.matches("/application/v4/tenant/{tenant}/access/ssh/request")) return accessRequests(path.get("tenant"), request);
+        if (path.matches("/application/v4/tenant/{tenant}/access/request/ssh")) return accessRequests(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/info")) return tenantInfo(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/notifications")) return notifications(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/secret-store/{name}/validate")) return validateSecretStore(path.get("tenant"), path.get("name"), request);
@@ -287,8 +287,8 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
 
     private HttpResponse handlePUT(Path path, HttpRequest request) {
         if (path.matches("/application/v4/tenant/{tenant}")) return updateTenant(path.get("tenant"), request);
-        if (path.matches("/application/v4/tenant/{tenant}/access/ssh/request")) return requestSshAccess(path.get("tenant"), request);
-        if (path.matches("/application/v4/tenant/{tenant}/access/ssh/approve")) return approveAccessRequest(path.get("tenant"), request);
+        if (path.matches("/application/v4/tenant/{tenant}/access/request/ssh")) return requestSshAccess(path.get("tenant"), request);
+        if (path.matches("/application/v4/tenant/{tenant}/access/approve/ssh")) return approveAccessRequest(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/info")) return updateTenantInfo(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/archive-access")) return allowArchiveAccess(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/secret-store/{name}")) return addSecretStore(path.get("tenant"), path.get("name"), request);
@@ -434,9 +434,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
         if (controller.tenants().require(tenant).type() != Tenant.Type.cloud)
             return ErrorResponse.badRequest("Can only see access requests for cloud tenants");
 
-        if (!isTenantAdmin(tenant, request)) {
-            return ErrorResponse.forbidden("Only tenant admins are allowed to approve access requests");
-        }
         var inspector = toSlime(request.getData()).get();
         var expiry = inspector.field("expiry").valid() ?
                 Instant.ofEpochMilli(inspector.field("expiry").asLong()) :
@@ -2782,16 +2779,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
                               .anyMatch(definition -> definition == RoleDefinition.hostedOperator);
     }
 
-    private static boolean isTenantAdmin(TenantName tenant, 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"))
-                .stream()
-                .anyMatch(role -> role.equals(Role.administrator(tenant)));
-    }
-
     private void ensureApplicationExists(TenantAndApplicationId id, HttpRequest request) {
         if (controller.applications().getApplication(id).isEmpty()) {
             log.fine("Application does not exist in public, creating: " + id);