[PLAT-10689] Support allowAll=false and empty resource UUID set

Summary:
This diff contains the following backend changes:
* Support allowAll=false and empty resource UUID set in the new RBAC when creating user / updating role bindings.
* Restrict `OTHER.SUPER_ADMIN_ACTIONS` permission in custom role.
* Restrict user assigning built-in `SuperAdmin` role to any role bindings.
* Restrict scoping down system defined roles when setting role bindings. We should not pass `resourceGroup` in the payload for `roleResourceDefinitions` anymore for any system defined roles.

Test Plan:
Manually tested all 4 above cases.
Adjusted UTs to work with fixes above.
Added more UTs to test above case 4.
Run UTs.
Run itests.

Reviewers: #yba-api-review, vpatibandla, sneelakantan

Reviewed By: #yba-api-review, sneelakantan

Subscribers: satyam.garg, mzafar, kkannan, yugaware

Differential Revision: https://phorge.dev.yugabyte.com/D29107

managed/src/main/java/com/yugabyte/yw/common/rbac/PermissionUtil.java

@@ -65,6 +65,14 @@ public PermissionInfo getPermissionInfo(Permission permission) {
 
   public void validatePermissionList(Set<Permission> permissionList)
       throws PlatformServiceException {
+    // Ensure that the given permission list does not contain super admin actions.
+    if (permissionList.contains(new Permission(ResourceType.OTHER, Action.SUPER_ADMIN_ACTIONS))) {
+      String errorMsg = "Super admin actions not allowed in custom role.";
+      log.error(errorMsg);
+      throw new PlatformServiceException(BAD_REQUEST, errorMsg);
+    }
+
+    // Validate if all the prerequisite permissions are given.
     Set<Permission> missingPermissions = new HashSet<>();
     for (Permission permission : permissionList) {
       Set<Permission> prerequisitePermissions =

managed/src/main/java/com/yugabyte/yw/common/rbac/RoleBindingUtil.java

@@ -14,6 +14,7 @@
 import com.yugabyte.yw.models.rbac.ResourceGroup;
 import com.yugabyte.yw.models.rbac.ResourceGroup.ResourceDefinition;
 import com.yugabyte.yw.models.rbac.Role;
+import com.yugabyte.yw.models.rbac.Role.RoleType;
 import com.yugabyte.yw.models.rbac.RoleBinding;
 import com.yugabyte.yw.models.rbac.RoleBinding.RoleBindingType;
 import io.ebean.annotation.Transactional;
@@ -86,10 +87,18 @@ public List<RoleBinding> setUserRoleBindings(
     return RoleBinding.getAll(userUUID);
   }
 
-  public void validateRoles(UUID userUUID, List<RoleResourceDefinition> roleResourceDefinitions) {
-    UUID customerUUID = Users.getOrBadRequest(userUUID).getCustomerUUID();
+  public void validateRoles(
+      UUID customerUUID, List<RoleResourceDefinition> roleResourceDefinitions) {
     for (RoleResourceDefinition roleResourceDefinition : roleResourceDefinitions) {
-      Role.getOrBadRequest(customerUUID, roleResourceDefinition.getRoleUUID());
+      Role role = Role.getOrBadRequest(customerUUID, roleResourceDefinition.getRoleUUID());
+      if (Users.Role.SuperAdmin.name().equals(role.getName())) {
+        String errMsg =
+            String.format(
+                "Cannot assign SuperAdmin role to a user in the roleResourceDefinition: %s.",
+                roleResourceDefinition.toString());
+        log.error(errMsg);
+        throw new PlatformServiceException(BAD_REQUEST, errMsg);
+      }
     }
   }
 
@@ -102,50 +111,73 @@ public void validateResourceGroups(
 
   public void validateRoleResourceDefinition(
       UUID customerUUID, RoleResourceDefinition roleResourceDefinition) {
-    // Check that the resource definition set in the resource group is not empty.
-    if (roleResourceDefinition.getResourceGroup().getResourceDefinitionSet() == null
-        || roleResourceDefinition.getResourceGroup().getResourceDefinitionSet().isEmpty()) {
-      String errMsg =
-          String.format(
-              "resourceDefinitionSet cannot be empty in the roleResourceDefinition: %s.",
-              roleResourceDefinition.toString());
-      log.error(errMsg);
-      throw new PlatformServiceException(BAD_REQUEST, errMsg);
-    }
 
-    // Basic validatation on each resource definition individually.
-    for (ResourceDefinition resourceDefinition :
-        roleResourceDefinition.getResourceGroup().getResourceDefinitionSet()) {
-      validateResourceDefinition(customerUUID, resourceDefinition);
-    }
-
-    // Validate that for the given role, there is some resource definition that has allowAll = true
-    // from the permissions which have "permissionValidOnResource" = false. Which indicates that it
-    // is a generic permission, not valid on a specific resource.
+    // System role validation.
     Role role = Role.get(customerUUID, roleResourceDefinition.getRoleUUID());
-    for (Permission permission : role.getPermissionDetails().getPermissionList()) {
-      PermissionInfo permissionInfo = permissionUtil.getPermissionInfo(permission);
-      if (!permissionInfo.isPermissionValidOnResource()) {
-        if (!hasGenericResourceDefinition(
-            customerUUID,
-            roleResourceDefinition.getResourceGroup().getResourceDefinitionSet(),
-            permission.getResourceType())) {
-          if (ResourceType.OTHER.equals(permission.getResourceType())) {
-            String errMsg =
-                String.format(
-                    "For permission '%s' from role '%s' to be valid, it needs a "
-                        + "resource definition with '%s' and customerUUID in the resourceUUIDSet.",
-                    permission.toString(), role.getName(), permission.getResourceType());
-            log.error(errMsg);
-            throw new PlatformServiceException(BAD_REQUEST, errMsg);
-          } else {
-            String errMsg =
-                String.format(
-                    "For permission '%s' from role '%s' to be valid, "
-                        + "it needs a resource definition with '%s' and allowAll = true.",
-                    permission.toString(), role.getName(), permission.getResourceType());
-            log.error(errMsg);
-            throw new PlatformServiceException(BAD_REQUEST, errMsg);
+    if (RoleType.System.equals(role.getRoleType())) {
+      // Validate system roles cannot be scoped down.
+      if (roleResourceDefinition.getResourceGroup() != null) {
+        String errMsg =
+            String.format(
+                "Cannot specify a resource group for system role ('%s':'%s').",
+                role.getName(), role.getRoleUUID());
+        log.error(errMsg);
+        throw new PlatformServiceException(BAD_REQUEST, errMsg);
+      }
+    } else {
+      // Custom role validation.
+      if (roleResourceDefinition.getResourceGroup() == null) {
+        String errMsg =
+            String.format(
+                "Must specify resource group for custom role ('%s':'%s').",
+                role.getName(), role.getRoleUUID());
+        log.error(errMsg);
+        throw new PlatformServiceException(BAD_REQUEST, errMsg);
+      }
+      // Check that the resource definition set in the resource group is not empty for custom roles.
+      if (roleResourceDefinition.getResourceGroup().getResourceDefinitionSet() == null
+          || roleResourceDefinition.getResourceGroup().getResourceDefinitionSet().isEmpty()) {
+        String errMsg =
+            String.format(
+                "resourceDefinitionSet cannot be empty in the roleResourceDefinition: %s.",
+                roleResourceDefinition.toString());
+        log.error(errMsg);
+        throw new PlatformServiceException(BAD_REQUEST, errMsg);
+      }
+
+      // Basic validatation on each resource definition individually on custom roles.
+      for (ResourceDefinition resourceDefinition :
+          roleResourceDefinition.getResourceGroup().getResourceDefinitionSet()) {
+        validateResourceDefinition(customerUUID, resourceDefinition);
+      }
+
+      // Validate that for the given custom role, there is some resource definition that has
+      // allowAll = true from the permissions which have "permissionValidOnResource" = false. Which
+      // indicates that it is a generic permission, not valid on a specific resource.
+      for (Permission permission : role.getPermissionDetails().getPermissionList()) {
+        PermissionInfo permissionInfo = permissionUtil.getPermissionInfo(permission);
+        if (!permissionInfo.isPermissionValidOnResource()) {
+          if (!hasGenericResourceDefinition(
+              customerUUID,
+              roleResourceDefinition.getResourceGroup().getResourceDefinitionSet(),
+              permission.getResourceType())) {
+            if (ResourceType.OTHER.equals(permission.getResourceType())) {
+              String errMsg =
+                  String.format(
+                      "For permission '%s' from role '%s' to be valid, it needs a resource"
+                          + " definition with '%s' and customerUUID in the resourceUUIDSet.",
+                      permission.toString(), role.getName(), permission.getResourceType());
+              log.error(errMsg);
+              throw new PlatformServiceException(BAD_REQUEST, errMsg);
+            } else {
+              String errMsg =
+                  String.format(
+                      "For permission '%s' from role '%s' to be valid, "
+                          + "it needs a resource definition with '%s' and allowAll = true.",
+                      permission.toString(), role.getName(), permission.getResourceType());
+              log.error(errMsg);
+              throw new PlatformServiceException(BAD_REQUEST, errMsg);
+            }
           }
         }
       }
@@ -173,11 +205,11 @@ public void validateResourceDefinition(UUID customerUUID, ResourceDefinition res
         throw new PlatformServiceException(BAD_REQUEST, errMsg);
       }
     }
-    // Check that only one of the fields `allowAll` or `resourceUUIDSet` exists.
-    if (resourceDefinition.isAllowAll() ^ resourceDefinition.getResourceUUIDSet().isEmpty()) {
+    // Check that both of the fields `allowAll` or `resourceUUIDSet` are not filled.
+    if (resourceDefinition.isAllowAll() && !resourceDefinition.getResourceUUIDSet().isEmpty()) {
       String errMsg =
           String.format(
-              "One of 'allowAll' or 'resourceUUIDSet' should be filled in resourceDefinition: %s.",
+              "Both 'allowAll' and 'resourceUUIDSet' cannot be filled in resourceDefinition: %s.",
               resourceDefinition.toString());
       log.error(errMsg);
       throw new PlatformServiceException(BAD_REQUEST, errMsg);
@@ -233,6 +265,27 @@ public boolean hasGenericResourceDefinition(
     return false;
   }
 
+  /**
+   * Populates the list of role resource definitions with the system default resource groups for all
+   * system defined roles.
+   *
+   * @param customerUUID
+   * @param userUUID
+   * @param roleResourceDefinitions
+   */
+  public void populateSystemRoleResourceGroups(
+      UUID customerUUID, UUID userUUID, List<RoleResourceDefinition> roleResourceDefinitions) {
+    for (RoleResourceDefinition roleResourceDefinition : roleResourceDefinitions) {
+      Role role = Role.getOrBadRequest(customerUUID, roleResourceDefinition.getRoleUUID());
+      if (RoleType.System.equals(role.getRoleType())) {
+        ResourceGroup systemDefaultResourceGroup =
+            ResourceGroup.getSystemDefaultResourceGroup(
+                customerUUID, userUUID, Users.Role.valueOf(role.getName()));
+        roleResourceDefinition.setResourceGroup(systemDefaultResourceGroup);
+      }
+    }
+  }
+
   /**
    * This function returns a set of ResourcePermissionData. This is required for UI to process the
    * resources and permissions available for a user. Whenever this function is called, we compute

managed/src/main/java/com/yugabyte/yw/common/rbac/RoleResourceDefinition.java

@@ -7,6 +7,7 @@
 import io.swagger.annotations.ApiModel;
 import io.swagger.annotations.ApiModelProperty;
 import java.util.UUID;
+import javax.annotation.Nullable;
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
@@ -25,7 +26,10 @@ public class RoleResourceDefinition {
   @JsonProperty("roleUUID")
   private UUID roleUUID;
 
-  @ApiModelProperty(value = "Resource group definition for the role.", required = true)
+  @ApiModelProperty(
+      value = "Resource group definition for the role. Only applicable for custom roles.",
+      required = false)
   @JsonProperty("resourceGroup")
+  @Nullable
   private ResourceGroup resourceGroup;
 }

managed/src/main/java/com/yugabyte/yw/controllers/RBACController.java

@@ -463,10 +463,14 @@ public Result setRoleBindings(UUID customerUUID, UUID userUUID, Http.Request req
         formFactory.getFormDataOrBadRequest(requestBody, RoleBindingFormData.class);
 
     // Validate the roles and resource group definitions.
-    roleBindingUtil.validateRoles(userUUID, roleBindingFormData.getRoleResourceDefinitions());
+    roleBindingUtil.validateRoles(customerUUID, roleBindingFormData.getRoleResourceDefinitions());
     roleBindingUtil.validateResourceGroups(
         customerUUID, roleBindingFormData.getRoleResourceDefinitions());
 
+    // Populate all the system default resource groups for all system defined roles.
+    roleBindingUtil.populateSystemRoleResourceGroups(
+        customerUUID, userUUID, roleBindingFormData.getRoleResourceDefinitions());
+
     // Delete all existing user role bindings and create new given role bindings.
     List<RoleBinding> createdRoleBindings =
         roleBindingUtil.setUserRoleBindings(

managed/src/main/java/com/yugabyte/yw/controllers/UsersController.java

@@ -228,7 +228,8 @@ public Result create(UUID customerUUID, Http.Request request) {
       // Check the role and resource definitions list field. New RBAC APIs use case. To be
       // standardized.
       else if (formData.getRole() == null && formData.getRoleResourceDefinitions() != null) {
-        // Validate the resource group definitions given.
+        // Validate the roles and resource group definitions given.
+        roleBindingUtil.validateRoles(customerUUID, formData.getRoleResourceDefinitions());
         roleBindingUtil.validateResourceGroups(customerUUID, formData.getRoleResourceDefinitions());
 
         // Create the user.
@@ -239,6 +240,10 @@ else if (formData.getRole() == null && formData.getRoleResourceDefinitions() !=
                 Users.Role.ConnectOnly,
                 customerUUID,
                 false);
+
+        // Populate all the system default resource groups for all system defined roles.
+        roleBindingUtil.populateSystemRoleResourceGroups(
+            customerUUID, user.getUuid(), formData.getRoleResourceDefinitions());
         // Add all the role bindings for the user.
         List<RoleBinding> createdRoleBindings =
             roleBindingUtil.setUserRoleBindings(

managed/src/main/java/com/yugabyte/yw/models/rbac/ResourceGroup.java

@@ -47,16 +47,21 @@ public static class ResourceDefinition {
   private Set<ResourceDefinition> resourceDefinitionSet = new HashSet<>();
 
   public static ResourceGroup getSystemDefaultResourceGroup(UUID customerUUID, Users user) {
+    return getSystemDefaultResourceGroup(customerUUID, user.getUuid(), user.getRole());
+  }
+
+  public static ResourceGroup getSystemDefaultResourceGroup(
+      UUID customerUUID, UUID userUUID, Users.Role usersRole) {
     ResourceGroup defaultResourceGroup = new ResourceGroup();
     ResourceDefinition resourceDefinition;
-    switch (user.getRole()) {
+    switch (usersRole) {
       case ConnectOnly:
         // For connect only role, user should have access to only his user profile, nothing else.
         resourceDefinition =
             ResourceDefinition.builder()
                 .resourceType(ResourceType.USER)
                 .allowAll(false)
-                .resourceUUIDSet(new HashSet<>(Arrays.asList(user.getUuid())))
+                .resourceUUIDSet(new HashSet<>(Arrays.asList(userUUID)))
                 .build();
         defaultResourceGroup.resourceDefinitionSet.add(resourceDefinition);
         break;

managed/src/main/resources/swagger-strict.json

@@ -9575,15 +9575,15 @@
       "properties" : {
         "resourceGroup" : {
           "$ref" : "#/definitions/ResourceGroup",
-          "description" : "Resource group definition for the role."
+          "description" : "Resource group definition for the role. Only applicable for custom roles."
         },
         "roleUUID" : {
           "description" : "UUID of the role to attach resource group to.",
           "format" : "uuid",
           "type" : "string"
         }
       },
-      "required" : [ "resourceGroup", "roleUUID" ],
+      "required" : [ "roleUUID" ],
       "type" : "object"
     },
     "RunQueryFormData" : {

managed/src/main/resources/swagger.json

@@ -9664,15 +9664,15 @@
       "properties" : {
         "resourceGroup" : {
           "$ref" : "#/definitions/ResourceGroup",
-          "description" : "Resource group definition for the role."
+          "description" : "Resource group definition for the role. Only applicable for custom roles."
         },
         "roleUUID" : {
           "description" : "UUID of the role to attach resource group to.",
           "format" : "uuid",
           "type" : "string"
         }
       },
-      "required" : [ "resourceGroup", "roleUUID" ],
+      "required" : [ "roleUUID" ],
       "type" : "object"
     },
     "RunQueryFormData" : {

managed/src/test/java/com/yugabyte/yw/common/rbac/RoleBindingUtilTest.java

@@ -218,16 +218,15 @@ public void testValidateResourceDefinitionNonDefaultResourceType() {
             .build();
     roleBindingUtil.validateResourceDefinition(customer.getUuid(), resourceDefinition3);
 
-    // Assert that exception is thrown if none of the fields 'allowAll' or 'resourceUUIDSet' are
+    // Assert that no exception is thrown if none of the fields 'allowAll' or 'resourceUUIDSet' are
     // filled.
     ResourceDefinition resourceDefinition4 =
         ResourceDefinition.builder()
             .resourceType(ResourceType.UNIVERSE)
             .allowAll(false)
             .resourceUUIDSet(new HashSet<>())
             .build();
-    assertPlatformException(
-        () -> roleBindingUtil.validateResourceDefinition(customer.getUuid(), resourceDefinition4));
+    roleBindingUtil.validateResourceDefinition(customer.getUuid(), resourceDefinition4);
   }
 
   @Test
@@ -379,4 +378,55 @@ public void testValidateRoleResourceDefinitionInvalid() {
             roleBindingUtil.validateRoleResourceDefinition(
                 customer.getUuid(), roleResourceDefinition1));
   }
+
+  @Test
+  public void testValidateRoleResourceDefinitionSystemRoleInvalid() {
+    // Create custom test role.
+    Role role =
+        Role.create(
+            customer.getUuid(),
+            "FakeRole2",
+            "FakeRoleDescription1",
+            RoleType.System,
+            new HashSet<>(
+                Arrays.asList(
+                    new Permission(ResourceType.UNIVERSE, Action.READ),
+                    new Permission(ResourceType.UNIVERSE, Action.CREATE))));
+
+    // Assert that exception is thrown when resource group is given for system defined roles.
+    ResourceDefinition resourceDefinition1 =
+        ResourceDefinition.builder().resourceType(ResourceType.UNIVERSE).allowAll(true).build();
+    ResourceGroup resourceGroup1 = new ResourceGroup();
+    resourceGroup1.setResourceDefinitionSet(new HashSet<>(Arrays.asList(resourceDefinition1)));
+    RoleResourceDefinition roleResourceDefinition1 = new RoleResourceDefinition();
+    roleResourceDefinition1.setRoleUUID(role.getRoleUUID());
+    roleResourceDefinition1.setResourceGroup(resourceGroup1);
+    // Exception should be thrown because system roles should not have a resource group attached.
+    assertPlatformException(
+        () ->
+            roleBindingUtil.validateRoleResourceDefinition(
+                customer.getUuid(), roleResourceDefinition1));
+  }
+
+  @Test
+  public void testValidateRoleResourceDefinitionSystemRoleValid() {
+    // Create custom test role.
+    Role role =
+        Role.create(
+            customer.getUuid(),
+            "FakeRole2",
+            "FakeRoleDescription1",
+            RoleType.System,
+            new HashSet<>(
+                Arrays.asList(
+                    new Permission(ResourceType.UNIVERSE, Action.READ),
+                    new Permission(ResourceType.UNIVERSE, Action.CREATE))));
+
+    // Assert that exception is not thrown when resource group is not given for system defined
+    // roles.
+    RoleResourceDefinition roleResourceDefinition1 = new RoleResourceDefinition();
+    roleResourceDefinition1.setRoleUUID(role.getRoleUUID());
+    // Exception should not be thrown since system roles do not have a resource group attached.
+    roleBindingUtil.validateRoleResourceDefinition(customer.getUuid(), roleResourceDefinition1);
+  }
 }