create parameter to determine whether roles are public or private (#6960)

GaOrtiga · DaanHoogland · web-flow · commit 8b5bfb145e33 · 2023-05-01T15:26:10.000+02:00
Co-authored-by: Gabriel Ortiga Fernandes &lt;gabriel.fernandes@scclouds.com.br&gt;
Co-authored-by: dahn &lt;daan.hoogland@gmail.com&gt;
diff --git a/api/src/main/java/org/apache/cloudstack/acl/Role.java b/api/src/main/java/org/apache/cloudstack/acl/Role.java
@@ -23,4 +23,5 @@
 public interface Role extends RoleEntity, InternalIdentity, Identity {
     RoleType getRoleType();
     boolean isDefault();
+    boolean isPublicRole();
 }
diff --git a/api/src/main/java/org/apache/cloudstack/acl/RoleService.java b/api/src/main/java/org/apache/cloudstack/acl/RoleService.java
@@ -38,15 +38,17 @@ public interface RoleService {
      *  Moreover, we will check if the requested role is of 'Admin' type; roles with 'Admin' type should only be visible to 'root admins'.
      *  Therefore, if a non-'root admin' user tries to search for an 'Admin' role, this method will return null.
      */
+    Role findRole(Long id, boolean removePrivateRoles);
+
     Role findRole(Long id);
 
-    Role createRole(String name, RoleType roleType, String description);
+    Role createRole(String name, RoleType roleType, String description, boolean publicRole);
 
-    Role createRole(String name, Role role, String description);
+    Role createRole(String name, Role role, String description, boolean publicRole);
 
-    Role importRole(String name, RoleType roleType, String description, List<Map<String, Object>> rules, boolean forced);
+    Role importRole(String name, RoleType roleType, String description, List<Map<String, Object>> rules, boolean forced, boolean isPublicRole);
 
-    Role updateRole(Role role, String name, RoleType roleType, String description);
+    Role updateRole(Role role, String name, RoleType roleType, String description, Boolean publicRole);
 
     boolean deleteRole(Role role);
 
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/CreateRoleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/CreateRoleCmd.java
@@ -48,6 +48,10 @@ public class CreateRoleCmd extends RoleCmd {
             description = "ID of the role to be cloned from. Either roleid or type must be passed in")
     private Long roleId;
 
+    @Parameter(name = ApiConstants.IS_PUBLIC, type = CommandType.BOOLEAN, description = "Indicates whether the role will be visible to all users (public) or only to root admins (private)." +
+            " If this parameter is not specified during the creation of the role its value will be defaulted to true (public).")
+    private boolean publicRole = true;
+
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -60,6 +64,9 @@ public Long getRoleId() {
         return roleId;
     }
 
+    public boolean isPublicRole() {
+        return publicRole;
+    }
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////
@@ -81,10 +88,10 @@ public void execute() {
             }
 
             CallContext.current().setEventDetails("Role: " + getRoleName() + ", from role: " + getRoleId() + ", description: " + getRoleDescription());
-            role = roleService.createRole(getRoleName(), existingRole, getRoleDescription());
+            role = roleService.createRole(getRoleName(), existingRole, getRoleDescription(), isPublicRole());
         } else {
             CallContext.current().setEventDetails("Role: " + getRoleName() + ", type: " + getRoleType() + ", description: " + getRoleDescription());
-            role = roleService.createRole(getRoleName(), getRoleType(), getRoleDescription());
+            role = roleService.createRole(getRoleName(), getRoleType(), getRoleDescription(), isPublicRole());
         }
 
         if (role == null) {
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/ImportRoleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/ImportRoleCmd.java
@@ -64,6 +64,10 @@ public class ImportRoleCmd extends RoleCmd {
             description = "Force create a role with the same name. This overrides the role type, description and rule permissions for the existing role. Default is false.")
     private Boolean forced;
 
+    @Parameter(name = ApiConstants.IS_PUBLIC, type = CommandType.BOOLEAN, description = "Indicates whether the role will be visible to all users (public) or only to root admins (private)." +
+            " If this parameter is not specified during the creation of the role its value will be defaulted to true (public).")
+    private boolean publicRole = true;
+
     @Inject
     ApiServerService _apiServer;
 
@@ -114,6 +118,10 @@ public boolean isForced() {
         return (forced != null) ? forced : false;
     }
 
+    public boolean isPublicRole() {
+        return publicRole;
+    }
+
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////
@@ -130,7 +138,7 @@ public void execute() {
         }
 
         CallContext.current().setEventDetails("Role: " + getRoleName() + ", type: " + getRoleType() + ", description: " + getRoleDescription());
-        Role role = roleService.importRole(getRoleName(), getRoleType(), getRoleDescription(), getRules(), isForced());
+        Role role = roleService.importRole(getRoleName(), getRoleType(), getRoleDescription(), getRules(), isForced(), isPublicRole());
         if (role == null) {
             throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to import role");
         }
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/ListRolesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/ListRolesCmd.java
@@ -92,6 +92,7 @@ private void setupResponse(final Pair<List<Role>, Integer> roles) {
             roleResponse.setRoleType(role.getRoleType());
             roleResponse.setDescription(role.getDescription());
             roleResponse.setIsDefault(role.isDefault());
+            roleResponse.setPublicRole(role.isPublicRole());
             roleResponse.setObjectName("role");
             roleResponses.add(roleResponse);
         }
@@ -104,7 +105,7 @@ private void setupResponse(final Pair<List<Role>, Integer> roles) {
     public void execute() {
         Pair<List<Role>, Integer> roles;
         if (getId() != null && getId() > 0L) {
-            roles = new Pair<List<Role>, Integer>(Collections.singletonList(roleService.findRole(getId())), 1);
+            roles = new Pair<>(Collections.singletonList(roleService.findRole(getId(), true)), 1);
         } else if (StringUtils.isNotBlank(getName()) || StringUtils.isNotBlank(getKeyword())) {
             roles = roleService.findRolesByName(getName(), getKeyword(), getStartIndex(), getPageSizeVal());
         } else if (getRoleType() != null) {
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/RoleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/RoleCmd.java
@@ -58,6 +58,7 @@ protected void setupResponse(final Role role) {
         response.setRoleName(role.getName());
         response.setRoleType(role.getRoleType());
         response.setDescription(role.getDescription());
+        response.setPublicRole(role.isPublicRole());
         response.setResponseName(getCommandName());
         response.setObjectName("role");
         setResponseObject(response);
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/UpdateRoleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/acl/UpdateRoleCmd.java
@@ -52,6 +52,9 @@ public class UpdateRoleCmd extends RoleCmd {
     @Parameter(name = ApiConstants.DESCRIPTION, type = BaseCmd.CommandType.STRING, description = "The description of the role")
     private String roleDescription;
 
+    @Parameter(name = ApiConstants.IS_PUBLIC, type = CommandType.BOOLEAN, description = "Indicates whether the role will be visible to all users (public) or only to root admins (private).")
+    private Boolean publicRole;
+
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -64,6 +67,10 @@ public String getRoleName() {
         return roleName;
     }
 
+    public Boolean isPublicRole() {
+        return publicRole;
+    }
+
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////
@@ -80,7 +87,7 @@ public void execute() {
             throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid role id provided");
         }
         CallContext.current().setEventDetails("Role: " + getRoleName() + ", type:" + getRoleType() + ", description: " + getRoleDescription());
-        role = roleService.updateRole(role, getRoleName(), getRoleType(), getRoleDescription());
+        role = roleService.updateRole(role, getRoleName(), getRoleType(), getRoleDescription(), isPublicRole());
         setupResponse(role);
     }
 
diff --git a/api/src/main/java/org/apache/cloudstack/api/response/BaseRoleResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/BaseRoleResponse.java
@@ -36,6 +36,11 @@ public class BaseRoleResponse extends BaseResponse {
     @Param(description = "the description of the role")
     private String roleDescription;
 
+    @SerializedName(ApiConstants.IS_PUBLIC)
+    @Param(description = "Indicates whether the role will be visible to all users (public) or only to root admins (private)." +
+            " If this parameter is not specified during the creation of the role its value will be defaulted to true (public).")
+    private boolean publicRole = true;
+
     public void setId(String id) {
         this.id = id;
     }
@@ -47,4 +52,8 @@ public void setRoleName(String roleName) {
     public void setDescription(String description) {
         this.roleDescription = description;
     }
+
+    public void setPublicRole(boolean publicRole) {
+        this.publicRole = publicRole;
+    }
 }
diff --git a/api/src/test/java/org/apache/cloudstack/api/command/test/CreateRoleCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/test/CreateRoleCmdTest.java
@@ -54,7 +54,7 @@ public void testCreateRoleWithRoleType() {
         when(role.getDescription()).thenReturn("User test");
         when(role.getName()).thenReturn("testuser");
         when(role.getRoleType()).thenReturn(RoleType.User);
-        when(roleService.createRole(createRoleCmd.getRoleName(), createRoleCmd.getRoleType(), createRoleCmd.getRoleDescription())).thenReturn(role);
+        when(roleService.createRole(createRoleCmd.getRoleName(), createRoleCmd.getRoleType(), createRoleCmd.getRoleDescription(), true)).thenReturn(role);
         createRoleCmd.execute();
         RoleResponse response = (RoleResponse) createRoleCmd.getResponseObject();
         Assert.assertEquals((String) ReflectionTestUtils.getField(response, "roleName"), role.getName());
@@ -71,7 +71,7 @@ public void testCreateRoleWithExistingRole() {
         when(newRole.getDescription()).thenReturn("User test");
         when(newRole.getName()).thenReturn("testuser");
         when(newRole.getRoleType()).thenReturn(RoleType.User);
-        when(roleService.createRole(createRoleCmd.getRoleName(), role, createRoleCmd.getRoleDescription())).thenReturn(newRole);
+        when(roleService.createRole(createRoleCmd.getRoleName(), role, createRoleCmd.getRoleDescription(), true)).thenReturn(newRole);
         createRoleCmd.execute();
         RoleResponse response = (RoleResponse) createRoleCmd.getResponseObject();
         Assert.assertEquals((String) ReflectionTestUtils.getField(response, "roleName"), newRole.getName());
diff --git a/api/src/test/java/org/apache/cloudstack/api/command/test/ImportRoleCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/test/ImportRoleCmdTest.java
@@ -93,7 +93,7 @@ public void testImportRoleSuccess() {
         when(role.getDescription()).thenReturn("test user imported");
         when(role.getName()).thenReturn("Test User");
         when(role.getRoleType()).thenReturn(RoleType.User);
-        when(roleService.importRole(anyString(),any(), anyString(), any(), anyBoolean())).thenReturn(role);
+        when(roleService.importRole(anyString(), any(), anyString(), any(), anyBoolean(), anyBoolean())).thenReturn(role);
 
         importRoleCmd.execute();
         RoleResponse response = (RoleResponse) importRoleCmd.getResponseObject();
diff --git a/api/src/test/java/org/apache/cloudstack/api/command/test/UpdateRoleCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/test/UpdateRoleCmdTest.java
@@ -58,7 +58,7 @@ public void testUpdateSuccess() {
         when(role.getDescription()).thenReturn("Default user");
         when(role.getName()).thenReturn("User");
         when(role.getRoleType()).thenReturn(RoleType.User);
-        when(roleService.updateRole(role,updateRoleCmd.getRoleName(),updateRoleCmd.getRoleType(),updateRoleCmd.getRoleDescription())).thenReturn(role);
+        when(roleService.updateRole(role, updateRoleCmd.getRoleName(), updateRoleCmd.getRoleType(), updateRoleCmd.getRoleDescription(), updateRoleCmd.isPublicRole())).thenReturn(role);
         when(role.getId()).thenReturn(1L);
         when(role.getDescription()).thenReturn("Description Initial");
         when(role.getName()).thenReturn("User");
diff --git a/engine/schema/src/main/java/org/apache/cloudstack/acl/RoleVO.java b/engine/schema/src/main/java/org/apache/cloudstack/acl/RoleVO.java
@@ -55,6 +55,9 @@ public class RoleVO implements Role {
     @Column(name = "is_default")
     private boolean isDefault = false;
 
+    @Column(name = "public_role")
+    private boolean publicRole = true;
+
     @Column(name = GenericDao.REMOVED_COLUMN)
     private Date removed;
 
@@ -120,4 +123,12 @@ public boolean isDefault() {
     public String toString() {
         return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "name", "uuid", "roleType");
     }
+
+    public boolean isPublicRole() {
+        return publicRole;
+    }
+
+    public void setPublicRole(boolean publicRole) {
+        this.publicRole = publicRole;
+    }
 }
diff --git a/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDao.java b/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDao.java
@@ -26,13 +26,15 @@
 import java.util.List;
 
 public interface RoleDao extends GenericDao<RoleVO, Long> {
-    List<RoleVO> findAllByName(String roleName);
+    List<RoleVO> findAllByName(String roleName, boolean showPrivateRole);
 
-    Pair<List<RoleVO>, Integer> findAllByName(final String roleName, String keyword, Long offset, Long limit);
+    Pair<List<RoleVO>, Integer> findAllByName(final String roleName, String keyword, Long offset, Long limit, boolean showPrivateRole);
 
-    List<RoleVO> findAllByRoleType(RoleType type);
-    List<RoleVO> findByName(String roleName);
-    RoleVO findByNameAndType(String roleName, RoleType type);
+    List<RoleVO> findAllByRoleType(RoleType type, boolean showPrivateRole);
+    List<RoleVO> findByName(String roleName, boolean showPrivateRole);
+    RoleVO findByNameAndType(String roleName, RoleType type, boolean showPrivateRole);
 
-    Pair<List<RoleVO>, Integer> findAllByRoleType(RoleType type, Long offset, Long limit);
+    Pair<List<RoleVO>, Integer> findAllByRoleType(RoleType type, Long offset, Long limit, boolean showPrivateRole);
+
+    Pair<List<RoleVO>, Integer> listAllRoles(Long startIndex, Long limit, boolean showPrivateRole);
 }
diff --git a/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/acl/dao/RoleDaoImpl.java
@@ -35,32 +35,41 @@ public class RoleDaoImpl extends GenericDaoBase<RoleVO, Long> implements RoleDao
     private final SearchBuilder<RoleVO> RoleByNameSearch;
     private final SearchBuilder<RoleVO> RoleByTypeSearch;
     private final SearchBuilder<RoleVO> RoleByNameAndTypeSearch;
+    private final SearchBuilder<RoleVO> RoleByIsPublicSearch;
 
     public RoleDaoImpl() {
         super();
 
         RoleByNameSearch = createSearchBuilder();
         RoleByNameSearch.and("roleName", RoleByNameSearch.entity().getName(), SearchCriteria.Op.LIKE);
+        RoleByNameSearch.and("isPublicRole", RoleByNameSearch.entity().isPublicRole(), SearchCriteria.Op.EQ);
         RoleByNameSearch.done();
 
         RoleByTypeSearch = createSearchBuilder();
         RoleByTypeSearch.and("roleType", RoleByTypeSearch.entity().getRoleType(), SearchCriteria.Op.EQ);
+        RoleByTypeSearch.and("isPublicRole", RoleByTypeSearch.entity().isPublicRole(), SearchCriteria.Op.EQ);
         RoleByTypeSearch.done();
 
         RoleByNameAndTypeSearch = createSearchBuilder();
         RoleByNameAndTypeSearch.and("roleName", RoleByNameAndTypeSearch.entity().getName(), SearchCriteria.Op.EQ);
         RoleByNameAndTypeSearch.and("roleType", RoleByNameAndTypeSearch.entity().getRoleType(), SearchCriteria.Op.EQ);
+        RoleByNameAndTypeSearch.and("isPublicRole", RoleByNameAndTypeSearch.entity().isPublicRole(), SearchCriteria.Op.EQ);
         RoleByNameAndTypeSearch.done();
+
+        RoleByIsPublicSearch = createSearchBuilder();
+        RoleByIsPublicSearch.and("isPublicRole", RoleByIsPublicSearch.entity().isPublicRole(), SearchCriteria.Op.EQ);
+        RoleByIsPublicSearch.done();
     }
 
     @Override
-    public List<RoleVO> findAllByName(final String roleName) {
-        return findAllByName(roleName, null, null, null).first();
+    public List<RoleVO> findAllByName(final String roleName, boolean showPrivateRole) {
+        return findAllByName(roleName, null, null, null, showPrivateRole).first();
     }
 
     @Override
-    public Pair<List<RoleVO>, Integer> findAllByName(final String roleName, String keyword, Long offset, Long limit) {
+    public Pair<List<RoleVO>, Integer> findAllByName(final String roleName, String keyword, Long offset, Long limit, boolean showPrivateRole) {
         SearchCriteria<RoleVO> sc = RoleByNameSearch.create();
+        filterPrivateRolesIfNeeded(sc, showPrivateRole);
         if (StringUtils.isNotEmpty(roleName)) {
             sc.setParameters("roleName", roleName);
         }
@@ -72,28 +81,44 @@ public Pair<List<RoleVO>, Integer> findAllByName(final String roleName, String k
     }
 
     @Override
-    public List<RoleVO> findAllByRoleType(final RoleType type) {
-        return findAllByRoleType(type, null, null).first();
+    public List<RoleVO> findAllByRoleType(final RoleType type, boolean showPrivateRole) {
+        return findAllByRoleType(type, null, null, showPrivateRole).first();
     }
 
-    public Pair<List<RoleVO>, Integer> findAllByRoleType(final RoleType type, Long offset, Long limit) {
+    public Pair<List<RoleVO>, Integer> findAllByRoleType(final RoleType type, Long offset, Long limit, boolean showPrivateRole) {
         SearchCriteria<RoleVO> sc = RoleByTypeSearch.create();
+        filterPrivateRolesIfNeeded(sc, showPrivateRole);
         sc.setParameters("roleType", type);
         return searchAndCount(sc, new Filter(RoleVO.class, "id", true, offset, limit));
     }
 
     @Override
-    public List<RoleVO> findByName(String roleName) {
+    public List<RoleVO> findByName(String roleName, boolean showPrivateRole) {
         SearchCriteria<RoleVO> sc = RoleByNameSearch.create();
+        filterPrivateRolesIfNeeded(sc, showPrivateRole);
         sc.setParameters("roleName", roleName);
         return listBy(sc);
     }
 
     @Override
-    public RoleVO findByNameAndType(String roleName, RoleType type) {
+    public RoleVO findByNameAndType(String roleName, RoleType type, boolean showPrivateRole) {
         SearchCriteria<RoleVO> sc = RoleByNameAndTypeSearch.create();
+        filterPrivateRolesIfNeeded(sc, showPrivateRole);
         sc.setParameters("roleName", roleName);
         sc.setParameters("roleType", type);
         return findOneBy(sc);
     }
+
+    @Override
+    public Pair<List<RoleVO>, Integer> listAllRoles(Long startIndex, Long limit, boolean showPrivateRole) {
+        SearchCriteria<RoleVO> sc = RoleByIsPublicSearch.create();
+        filterPrivateRolesIfNeeded(sc, showPrivateRole);
+        return searchAndCount(sc, new Filter(RoleVO.class, "id", true, startIndex, limit));
+    }
+
+    public void filterPrivateRolesIfNeeded(SearchCriteria<RoleVO> sc, boolean showPrivateRole) {
+        if (!showPrivateRole) {
+            sc.setParameters("isPublicRole", true);
+        }
+    }
 }
diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41800to41810.sql b/engine/schema/src/main/resources/META-INF/db/schema-41800to41810.sql
@@ -19,3 +19,5 @@
 -- Schema upgrade from 4.18.0.0 to 4.18.1.0
 --;
 
+-- create_public_parameter_on_roles. #6960
+ALTER TABLE `cloud`.`roles` ADD COLUMN `public_role` tinyint(1) NOT NULL DEFAULT '1' COMMENT 'Indicates whether the role will be visible to all users (public) or only to root admins (private). If this parameter is not specified during the creation of the role its value will be defaulted to true (public).';
diff --git a/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java b/server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java
diff --git a/server/src/test/java/org/apache/cloudstack/acl/RoleManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/acl/RoleManagerImplTest.java
diff --git a/test/integration/smoke/test_private_roles.py b/test/integration/smoke/test_private_roles.py
diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py

Original file line number	Diff line number	Diff line change
`@@ -23,4 +23,5 @@`
`23`	`23`	`public interface Role extends RoleEntity, InternalIdentity, Identity {`
`24`	`24`	`RoleType getRoleType();`
`25`	`25`	`boolean isDefault();`
	`26`	`+ boolean isPublicRole();`
`26`	`27`	`}`
Original file line number	Diff line number	Diff line change
`@@ -36,6 +36,11 @@ public class BaseRoleResponse extends BaseResponse {`
`36`	`36`	`@Param(description = "the description of the role")`
`37`	`37`	`private String roleDescription;`
`38`	`38`
	`39`	`+ @SerializedName(ApiConstants.IS_PUBLIC)`
	`40`	`+ @Param(description = "Indicates whether the role will be visible to all users (public) or only to root admins (private)." +`
	`41`	`+ " If this parameter is not specified during the creation of the role its value will be defaulted to true (public).")`
	`42`	`+ private boolean publicRole = true;`
	`43`	`+`
`39`	`44`	`public void setId(String id) {`
`40`	`45`	`this.id = id;`
`41`	`46`	`}`
`@@ -47,4 +52,8 @@ public void setRoleName(String roleName) {`
`47`	`52`	`public void setDescription(String description) {`
`48`	`53`	`this.roleDescription = description;`
`49`	`54`	`}`
	`55`	`+`
	`56`	`+ public void setPublicRole(boolean publicRole) {`
	`57`	`+ this.publicRole = publicRole;`
	`58`	`+ }`
`50`	`59`	`}`