Remove RoleId and move serialisation etc to UserRoles

3 files changed, 30 insertions, 58 deletions

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 8909975fdfd..6f9da46a92b 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
@@ -14,9 +14,9 @@ import com.yahoo.slime.Inspector;
 import com.yahoo.slime.Slime;
 import com.yahoo.vespa.config.SlimeUtils;
 import com.yahoo.vespa.hosted.controller.Controller;
-import com.yahoo.vespa.hosted.controller.api.integration.user.RoleId;
 import com.yahoo.vespa.hosted.controller.api.integration.user.UserId;
 import com.yahoo.vespa.hosted.controller.api.integration.user.UserManagement;
+import com.yahoo.vespa.hosted.controller.api.integration.user.UserRoles;
 import com.yahoo.vespa.hosted.controller.api.role.ApplicationRole;
 import com.yahoo.vespa.hosted.controller.api.role.Role;
 import com.yahoo.vespa.hosted.controller.api.role.Roles;
@@ -43,6 +43,7 @@ public class UserApiHandler extends LoggingRequestHandler {
     private final static Logger log = Logger.getLogger(UserApiHandler.class.getName());
 
     private final Roles roles;
+    private final UserRoles userRoles;
     private final UserManagement users;
     private final Controller controller;
 
@@ -50,6 +51,7 @@ public class UserApiHandler extends LoggingRequestHandler {
     public UserApiHandler(Context parentCtx, Roles roles, UserManagement users, Controller controller) {
         super(parentCtx);
         this.roles = roles;
+        this.userRoles = new UserRoles(roles);
         this.users = users;
         this.controller = controller;
     }
@@ -117,11 +119,12 @@ public class UserApiHandler extends LoggingRequestHandler {
         Cursor root = slime.setObject();
         root.setString("tenant", tenantName);
         Cursor rolesArray = root.setArray("roles");
-        for (TenantRole role : tenantRoles(TenantName.from(tenantName))) {
+        // TODO jvenstad: Move these two to CloudRoles utility class.
+        for (TenantRole role : userRoles.tenantRoles(TenantName.from(tenantName))) {
             Cursor roleObject = rolesArray.addObject();
             roleObject.setString("name", role.definition().name());
             Cursor membersArray = roleObject.setArray("members");
-            for (UserId user : users.listUsers(RoleId.fromRole(role)))
+            for (UserId user : users.listUsers(role))
                 membersArray.addString(user.value());
         }
         return new SlimeJsonResponse(slime);
@@ -133,11 +136,11 @@ public class UserApiHandler extends LoggingRequestHandler {
         root.setString("tenant", tenantName);
         root.setString("application", applicationName);
         Cursor rolesArray = root.setArray("roles");
-        for (ApplicationRole role : applicationRoles(TenantName.from(tenantName), ApplicationName.from(applicationName))) {
+        for (ApplicationRole role : userRoles.applicationRoles(TenantName.from(tenantName), ApplicationName.from(applicationName))) {
             Cursor roleObject = rolesArray.addObject();
             roleObject.setString("name", role.definition().name());
             Cursor membersArray = roleObject.setArray("members");
-            for (UserId user : users.listUsers(RoleId.fromRole(role)))
+            for (UserId user : users.listUsers(role))
                 membersArray.addString(user.value());
         }
         return new SlimeJsonResponse(slime);
@@ -147,49 +150,36 @@ public class UserApiHandler extends LoggingRequestHandler {
         Inspector requestObject = bodyInspector(request);
         String roleName = require("roleName", Inspector::asString, requestObject);
         String user = require("user", Inspector::asString, requestObject);
-        RoleId roleId = RoleId.fromValue(tenantName + "." + roleName); // TODO jvenstad: Move this logic to utility class CloudRoles, with validation.
-        users.addUsers(roleId, List.of(new UserId(user)));
-        return new MessageResponse("User '" + user + "' is now a member of role '" + roleId + "'.");
+        Role role = userRoles.toRole(TenantName.from(tenantName), roleName);
+        users.addUsers(role, List.of(new UserId(user)));
+        return new MessageResponse(user + " is now a member of " + role);
     }
 
     private HttpResponse addApplicationRoleMember(String tenantName, String applicationName, HttpRequest request) {
         Inspector requestObject = bodyInspector(request);
         String roleName = require("roleName", Inspector::asString, requestObject);
         String user = require("user", Inspector::asString, requestObject);
-        RoleId roleId = RoleId.fromValue(tenantName + "." + applicationName + "." + roleName); // TODO jvenstad: Move this logic to utility class CloudRoles, with validation.
-        users.addUsers(roleId, List.of(new UserId(user)));
-        return new MessageResponse("User '" + user + "' is now a member of role '" + roleId + "'.");
+        Role role = userRoles.toRole(TenantName.from(tenantName), ApplicationName.from(applicationName), roleName);
+        users.addUsers(role, List.of(new UserId(user)));
+        return new MessageResponse(user + " is now a member of " + role);
     }
 
     private HttpResponse removeTenantRoleMember(String tenantName, HttpRequest request) {
         Inspector requestObject = bodyInspector(request);
         String roleName = require("roleName", Inspector::asString, requestObject);
         String user = require("user", Inspector::asString, requestObject);
-        RoleId roleId = RoleId.fromValue(tenantName + "." + roleName); // TODO jvenstad: Move this logic to utility class CloudRoles, with validation.
-        users.removeUsers(roleId, List.of(new UserId(user)));
-        return new MessageResponse("User '" + user + "' is no longer a member of role '" + roleId + "'.");
+        Role role = userRoles.toRole(TenantName.from(tenantName), roleName);
+        users.removeUsers(role, List.of(new UserId(user)));
+        return new MessageResponse(user + " is no longer a member of " + role);
     }
 
     private HttpResponse removeApplicationRoleMember(String tenantName, String applicationName, HttpRequest request) {
         Inspector requestObject = bodyInspector(request);
         String roleName = require("roleName", Inspector::asString, requestObject);
         String user = require("user", Inspector::asString, requestObject);
-        RoleId roleId = RoleId.fromValue(tenantName + "." + applicationName + "." + roleName); // TODO jvenstad: Move this logic to utility class CloudRoles, with validation.
-        users.removeUsers(roleId, List.of(new UserId(user)));
-        return new MessageResponse("User '" + user + "' is no longer a member of role '" + roleId + "'.");
-    }
-
-    private List<TenantRole> tenantRoles(TenantName tenant) { // TODO jvenstad: Move these two to CloudRoles utility class.
-        return List.of(roles.tenantOperator(tenant),
-                       roles.tenantAdmin(tenant),
-                       roles.tenantOwner(tenant));
-    }
-
-    private List<ApplicationRole> applicationRoles(TenantName tenant, ApplicationName application) {
-        return List.of(roles.applicationReader(tenant, application),
-                       roles.applicationDeveloper(tenant, application),
-                       roles.applicationOperator(tenant, application),
-                       roles.applicationAdmin(tenant, application));
+        Role role = userRoles.toRole(TenantName.from(tenantName), ApplicationName.from(applicationName), roleName);
+        users.removeUsers(role, List.of(new UserId(user)));
+        return new MessageResponse(user + " is no longer a member of " + role);
     }
 
     private static Inspector bodyInspector(HttpRequest request) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java
index c00d38a4d45..fba2b7597b0 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudAccessControl.java
@@ -2,15 +2,15 @@ package com.yahoo.vespa.hosted.controller.security;
 
 import com.google.inject.Inject;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.BillingInfo;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.Marketplace;
-import com.yahoo.vespa.hosted.controller.api.integration.user.RoleId;
 import com.yahoo.vespa.hosted.controller.api.integration.user.UserId;
 import com.yahoo.vespa.hosted.controller.api.integration.user.UserManagement;
+import com.yahoo.vespa.hosted.controller.api.integration.user.UserRoles;
 import com.yahoo.vespa.hosted.controller.api.role.ApplicationRole;
+import com.yahoo.vespa.hosted.controller.api.role.Role;
 import com.yahoo.vespa.hosted.controller.api.role.Roles;
 import com.yahoo.vespa.hosted.controller.api.role.TenantRole;
 import com.yahoo.vespa.hosted.controller.tenant.CloudTenant;
@@ -28,12 +28,14 @@ public class CloudAccessControl implements AccessControl {
     private final Marketplace marketplace;
     private final UserManagement userManagement;
     private final Roles roles;
+    private final UserRoles userRoles;
 
     @Inject
     public CloudAccessControl(Marketplace marketplace, UserManagement userManagement, Roles roles) {
         this.marketplace = marketplace;
         this.userManagement = userManagement;
         this.roles = roles;
+        this.userRoles = new UserRoles(roles);
     }
 
     @Override
@@ -43,7 +45,7 @@ public class CloudAccessControl implements AccessControl {
         // CloudTenant tenant new CloudTenant(spec.tenant(), marketplace.resolveCustomer(spec.getRegistrationToken()));
         // TODO Enable the above when things work.
 
-        RoleId ownerRole = RoleId.fromRole(roles.tenantOwner(spec.tenant()));
+        Role ownerRole = roles.tenantOwner(spec.tenant());
         userManagement.createRole(ownerRole);
         userManagement.addUsers(ownerRole, List.of(new UserId(credentials.user().getName())));
 
@@ -59,25 +61,21 @@ public class CloudAccessControl implements AccessControl {
     public void deleteTenant(TenantName tenant, Credentials credentials) {
         // Probably terminate customer subscription?
 
-        tenantRoles(tenant).stream()
-                           .map(RoleId::fromRole)
-                           .filter(userManagement.listRoles()::contains)
-                           .forEach(userManagement::deleteRole);
+        for (TenantRole role : userRoles.tenantRoles(tenant))
+            userManagement.deleteRole(role);
     }
 
     @Override
     public void createApplication(ApplicationId application, Credentials credentials) {
-        RoleId ownerRole = RoleId.fromRole(roles.applicationAdmin(application.tenant(), application.application()));
+        Role ownerRole = roles.applicationAdmin(application.tenant(), application.application());
         userManagement.createRole(ownerRole);
         userManagement.addUsers(ownerRole, List.of(new UserId(credentials.user().getName())));
     }
 
     @Override
     public void deleteApplication(ApplicationId id, Credentials credentials) {
-        applicationRoles(id.tenant(), id.application()).stream()
-                                                       .map(RoleId::fromRole)
-                                                       .filter(userManagement.listRoles()::contains)
-                                                       .forEach(userManagement::deleteRole);
+        for (ApplicationRole role : userRoles.applicationRoles(id.tenant(), id.application()))
+            userManagement.deleteRole(role);
     }
 
     @Override
@@ -87,17 +85,4 @@ public class CloudAccessControl implements AccessControl {
         return Collections.emptyList();
     }
 
-    private List<TenantRole> tenantRoles(TenantName tenant) { // TODO jvenstad: Move these two to CloudRoles utility class.
-        return List.of(roles.tenantOperator(tenant),
-                       roles.tenantAdmin(tenant),
-                       roles.tenantOwner(tenant));
-    }
-
-    private List<ApplicationRole> applicationRoles(TenantName tenant, ApplicationName application) {
-        return List.of(roles.applicationReader(tenant, application),
-                       roles.applicationDeveloper(tenant, application),
-                       roles.applicationOperator(tenant, application),
-                       roles.applicationAdmin(tenant, application));
-    }
-
 }
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 c21d4b4b0bf..4df6ae74b9a 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
@@ -104,9 +104,6 @@ public class ControllerContainerTest {
             "  <handler id='com.yahoo.vespa.hosted.controller.restapi.athenz.AthenzApiHandler'>\n" +
             "    <binding>http://*/athenz/v1/*</binding>\n" +
             "  </handler>\n" +
-            "  <handler id='com.yahoo.vespa.hosted.controller.restapi.user.UserApiHandler'>\n" +
-            "    <binding>http://*/user/v1/*</binding>\n" +
-            "  </handler>\n" +
             "  <handler id='com.yahoo.vespa.hosted.controller.restapi.deployment.DeploymentApiHandler'>\n" +
             "    <binding>http://*/deployment/v1/*</binding>\n" +
             "  </handler>\n" +