CLOUDSTACK-10044: Update role permission

api/src/org/apache/cloudstack/acl/RoleService.java

@@ -17,6 +17,7 @@
 
 package org.apache.cloudstack.acl;
 
+import org.apache.cloudstack.acl.RolePermission.Permission;
 import org.apache.cloudstack.framework.config.ConfigKey;
 
 import java.util.List;
@@ -36,13 +37,14 @@ public interface RoleService {
     RolePermission findRolePermission(final Long id);
     RolePermission findRolePermissionByUuid(final String uuid);
 
-    RolePermission createRolePermission(final Role role, final Rule rule, final RolePermission.Permission permission, final String description);
+    RolePermission createRolePermission(final Role role, final Rule rule, final Permission permission, final String description);
     /**
      * updateRolePermission updates the order/position of an role permission
      * @param role The role whose permissions needs to be re-ordered
      * @param newOrder The new list of ordered role permissions
      */
     boolean updateRolePermission(final Role role, final List<RolePermission> newOrder);
+    boolean updateRolePermission(final Role role, final RolePermission rolePermission, final Permission permission);
     boolean deleteRolePermission(final RolePermission rolePermission);
 
     List<Role> listRoles();

api/src/org/apache/cloudstack/api/ApiConstants.java

@@ -373,6 +373,7 @@ public class ApiConstants {
     public static final String ROLE_NAME = "rolename";
     public static final String PERMISSION = "permission";
     public static final String RULE = "rule";
+    public static final String RULE_ID = "ruleid";
     public static final String RULE_ORDER = "ruleorder";
     public static final String USER = "user";
     public static final String ACTIVE_ONLY = "activeonly";

api/src/org/apache/cloudstack/api/command/admin/acl/UpdateRolePermissionCmd.java

@@ -20,6 +20,7 @@
 import com.cloud.user.Account;
 import org.apache.cloudstack.acl.Role;
 import org.apache.cloudstack.acl.RolePermission;
+import org.apache.cloudstack.acl.RolePermission.Permission;
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiArgValidator;
@@ -51,10 +52,18 @@ public class UpdateRolePermissionCmd extends BaseCmd {
             description = "ID of the role", validations = {ApiArgValidator.PositiveNumber})
     private Long roleId;
 
-    @Parameter(name = ApiConstants.RULE_ORDER, type = CommandType.LIST, collectionType = CommandType.UUID, required = true, entityType = RolePermissionResponse.class,
+    @Parameter(name = ApiConstants.RULE_ORDER, type = CommandType.LIST, collectionType = CommandType.UUID, entityType = RolePermissionResponse.class,
             description = "The parent role permission uuid, use 0 to move this rule at the top of the list")
     private List<Long> rulePermissionOrder;
 
+    @Parameter(name = ApiConstants.RULE_ID, type = CommandType.UUID, entityType = RolePermissionResponse.class,
+            description = "Role permission rule id", since="4.11")
+    private Long ruleId;
+
+    @Parameter(name = ApiConstants.PERMISSION, type = CommandType.STRING,
+            description = "Rule permission, can be: allow or deny", since="4.11")
+    private String rulePermission;
+
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -67,6 +76,21 @@ public List<Long> getRulePermissionOrder() {
         return rulePermissionOrder;
     }
 
+    public Long getRuleId() {
+        return ruleId;
+    }
+
+    public Permission getRulePermission() {
+        if (this.rulePermission == null) {
+            return null;
+        }
+        if (!this.rulePermission.equalsIgnoreCase(Permission.ALLOW.toString()) &&
+                !this.rulePermission.equalsIgnoreCase(Permission.DENY.toString())) {
+            throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Values for permission parameter should be: allow or deny");
+        }
+        return rulePermission.equalsIgnoreCase(Permission.ALLOW.toString()) ? Permission.ALLOW : Permission.DENY;
+    }
+
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////
@@ -84,19 +108,35 @@ public long getEntityOwnerId() {
     @Override
     public void execute() {
         final Role role = roleService.findRole(getRoleId());
+        boolean result = false;
         if (role == null) {
             throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid role id provided");
         }
-        CallContext.current().setEventDetails("Reordering permissions for role id: " + role.getId());
-        final List<RolePermission> rolePermissionsOrder = new ArrayList<>();
-        for (Long rolePermissionId : getRulePermissionOrder()) {
-            final RolePermission rolePermission = roleService.findRolePermission(rolePermissionId);
+        if (getRulePermissionOrder() != null) {
+            if (getRuleId() != null || getRulePermission() != null) {
+                throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Parameters permission and ruleid must be mutually exclusive with ruleorder");
+            }
+            CallContext.current().setEventDetails("Reordering permissions for role id: " + role.getId());
+            final List<RolePermission> rolePermissionsOrder = new ArrayList<>();
+            for (Long rolePermissionId : getRulePermissionOrder()) {
+                final RolePermission rolePermission = roleService.findRolePermission(rolePermissionId);
+                if (rolePermission == null) {
+                    throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Provided role permission(s) do not exist");
+                }
+                rolePermissionsOrder.add(rolePermission);
+            }
+            result = roleService.updateRolePermission(role, rolePermissionsOrder);
+        } else if (getRuleId() != null && getRulePermission() != null) {
+            if (getRulePermissionOrder() != null) {
+                throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Parameters permission and ruleid must be mutually exclusive with ruleorder");
+            }
+            RolePermission rolePermission = roleService.findRolePermission(getRuleId());
             if (rolePermission == null) {
-                throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Provided role permission(s) do not exist");
+                throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid rule id provided");
             }
-            rolePermissionsOrder.add(rolePermission);
+            CallContext.current().setEventDetails("Updating permission for rule id: " + getRuleId() + " to: " + getRulePermission().toString());
+            result = roleService.updateRolePermission(role, rolePermission, getRulePermission());
         }
-        boolean result = roleService.updateRolePermission(role, rolePermissionsOrder);
         SuccessResponse response = new SuccessResponse(getCommandName());
         response.setSuccess(result);
         setResponseObject(response);

engine/schema/src/org/apache/cloudstack/acl/dao/RolePermissionsDao.java

@@ -20,6 +20,7 @@
 import com.cloud.utils.db.GenericDao;
 import org.apache.cloudstack.acl.Role;
 import org.apache.cloudstack.acl.RolePermission;
+import org.apache.cloudstack.acl.RolePermission.Permission;
 import org.apache.cloudstack.acl.RolePermissionVO;
 
 import java.util.List;
@@ -40,6 +41,15 @@ public interface RolePermissionsDao extends GenericDao<RolePermissionVO, Long> {
      */
     boolean update(final Role role, final List<RolePermission> newOrder);
 
+    /**
+     * Updates existing role permission
+     * @param role role of which rule belongs
+     * @param rolePermission role permission
+     * @param permission permission
+     * @return true on success, false if not
+     */
+    boolean update(final Role role, final RolePermission rolePermission, final Permission permission);
+
     /**
      * Returns ordered linked-list of role permission for a given role
      * @param roleId the ID of the role

engine/schema/src/org/apache/cloudstack/acl/dao/RolePermissionsDaoImpl.java

@@ -29,6 +29,7 @@
 import com.cloud.utils.exception.CloudRuntimeException;
 import org.apache.cloudstack.acl.Role;
 import org.apache.cloudstack.acl.RolePermission;
+import org.apache.cloudstack.acl.RolePermission.Permission;
 import org.apache.cloudstack.acl.RolePermissionVO;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
@@ -149,6 +150,19 @@ public Boolean doInTransaction(TransactionStatus status) {
         });
     }
 
+    @Override
+    public boolean update(Role role, RolePermission rolePermission, Permission permission) {
+        if (role == null || rolePermission == null || permission == null) {
+            return false;
+        }
+        RolePermissionVO rolePermissionVO = findById(rolePermission.getId());
+        if (rolePermissionVO == null || role.getId() != rolePermission.getRoleId() || rolePermissionVO.getId() != rolePermission.getId()) {
+            return false;
+        }
+        rolePermissionVO.setPermission(permission);
+        return update(rolePermission.getId(), rolePermissionVO);
+    }
+
     @Override
     public List<RolePermissionVO> findAllByRoleIdSorted(final Long roleId) {
         final SearchCriteria<RolePermissionVO> sc = RolePermissionsSearch.create();

server/src/org/apache/cloudstack/acl/RoleManagerImpl.java

@@ -204,6 +204,12 @@ public boolean updateRolePermission(final Role role, final List<RolePermission>
         return role != null && newOrder != null && rolePermissionsDao.update(role, newOrder);
     }
 
+    @Override
+    public boolean updateRolePermission(Role role, RolePermission rolePermission, RolePermission.Permission permission) {
+        checkCallerAccess();
+        return role != null && rolePermissionsDao.update(role, rolePermission, permission);
+    }
+
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_ROLE_PERMISSION_DELETE, eventDescription = "deleting Role Permission")
     public boolean deleteRolePermission(final RolePermission rolePermission) {

test/integration/smoke/test_dynamicroles.py

@@ -394,6 +394,46 @@ def validate_permissions_list(permissions):
             rule.update(self.apiclient, ruleorder=",".join(map(lambda x: x.id, permissions)))
             validate_permissions_list(permissions)
 
+    @attr(tags=['advanced', 'simulator', 'basic', 'sg'], required_hardware=False)
+    def test_rolepermission_lifecycle_update_permission(self):
+        """
+            Tests update of Allow to Deny permission of a rule
+        """
+        permissions = [self.rolepermission]
+
+        rule = permissions.pop(0)
+        rule.update(self.apiclient, ruleid=rule.id, permission='deny')
+
+        list_rolepermissions = RolePermission.list(self.apiclient, roleid=self.role.id)
+        self.assertEqual(
+            list_rolepermissions[0].permission,
+            'deny',
+            msg="List of role permissions do not match created list of permissions"
+        )
+
+        rule.update(self.apiclient, ruleid=rule.id, permission='allow')
+
+        list_rolepermissions = RolePermission.list(self.apiclient, roleid=self.role.id)
+        self.assertEqual(
+            list_rolepermissions[0].permission,
+            'allow',
+            msg="List of role permissions do not match created list of permissions"
+        )
+
+    @attr(tags=['advanced', 'simulator', 'basic', 'sg'], required_hardware=False)
+    def test_rolepermission_lifecycle_update_permission_negative(self):
+        """
+            Tests negative test for setting incorrect value as permission
+        """
+        permissions = [self.rolepermission]
+
+        rule = permissions.pop(0)
+        try:
+            rule.update(self.apiclient, ruleid=rule.id, permission='some_other_value')
+        except Exception:
+            pass
+        else:
+            self.fail("Negative test: Setting permission to 'some_other_value' should not be successful, failing")
 
     @attr(tags=['advanced', 'simulator', 'basic', 'sg'], required_hardware=False)
     def test_rolepermission_lifecycle_concurrent_updates(self):

ui/l10n/ar.js

@@ -2176,6 +2176,7 @@ var dictionary = {
     "message.restart.vpc": "يرجى تأكيد رغبتك في إعادة تشغيل الـVPN",
     "message.restart.vpc.remark": "Please confirm that you want to restart the VPC <p><small><i>Remark: making a non-redundant VPC redundant will force a clean up. The networks will not be available for a couple of minutes</i>.</small></p>",
     "message.restoreVM": "Do you want to restore the VM ?",
+    "message.role.update.fail": "Failed updating rule permission",
     "message.role.ordering.fail": "Reordering of rule permissions aborted as the list has changed while you were making changes. Please try again.",
     "message.security.group.usage": "(Use <strong>Ctrl-click</strong> to select all applicable security groups)",
     "message.select.a.zone": "A zone typically corresponds to a single datacenter. Multiple zones help make the cloud more reliable by providing physical isolation and redundancy.",

ui/l10n/ca.js

@@ -2176,6 +2176,7 @@ var dictionary = {
     "message.restart.vpc": "Please confirm that you want to restart the VPC",
     "message.restart.vpc.remark": "Please confirm that you want to restart the VPC <p><small><i>Remark: making a non-redundant VPC redundant will force a clean up. The networks will not be available for a couple of minutes</i>.</small></p>",
     "message.restoreVM": "Do you want to restore the VM ?",
+    "message.role.update.fail": "Failed updating rule permission",
     "message.role.ordering.fail": "Reordering of rule permissions aborted as the list has changed while you were making changes. Please try again.",
     "message.security.group.usage": "(Use <strong>Ctrl-click</strong> to select all applicable security groups)",
     "message.select.a.zone": "A zone typically corresponds to a single datacenter. Multiple zones help make the cloud more reliable by providing physical isolation and redundancy.",

ui/l10n/de_DE.js

@@ -2176,6 +2176,7 @@ var dictionary = {
     "message.restart.vpc": "Bitte bestätigen Sie, dass Sie den VPC neu starten möchten",
     "message.restart.vpc.remark": "Bitte bestätigen Sie, dass Sie die VPC neu starten möchten <p>small><i>Hinweis: Ein nicht-redundante VPC redundant zu machen wird eine Bereinigung erzwingen. Die Netzwerke werden dadurch einige Minuten nicht verfügbar sein</i>.</small></p>",
     "message.restoreVM": "Möchten Sie die VM wiederherstellen?",
+    "message.role.update.fail": "Failed updating rule permission",
     "message.role.ordering.fail": "Die Neuordnung der Regelberechtigungen wurde abgebrochen, es sind Änderungen eingetreten während Sie an der Liste Arbeiten durchgeführt haben. Bitte versuchen Sie es erneut.",
     "message.security.group.usage": "(Verwenden Sie <strong>Ctrl-click</strong> um alle passenden Sicherheits Gruppen auszuwählen)",
     "message.select.a.zone": "Eine Zone steht typischerweise für ein einzelnes Rechenzentrum. Mehrere Zonen helfen dabei, die Cloud zuverlässiger zu machen durch physikalische Isolation und Redundanz.",

ui/l10n/en.js

@@ -2207,6 +2207,7 @@ var dictionary = {"ICMP.code":"ICMP Code",
 "message.restart.vpc":"Please confirm that you want to restart the VPC",
 "message.restart.vpc.remark":"Please confirm that you want to restart the VPC <p><small><i>Remark: making a non-redundant VPC redundant will force a clean up. The networks will not be available for a couple of minutes</i>.</small></p>",
 "message.restoreVM":"Do you want to restore the VM ?",
+"message.role.update.fail": "Failed updating rule permission",
 "message.role.ordering.fail":"Reordering of rule permissions aborted as the list has changed while you were making changes. Please try again.",
 "message.security.group.usage":"(Use <strong>Ctrl-click</strong> to select all applicable security groups)",
 "message.select.a.zone":"A zone typically corresponds to a single datacenter. Multiple zones help make the cloud more reliable by providing physical isolation and redundancy.",

ui/l10n/es.js

@@ -2176,6 +2176,7 @@ var dictionary = {
     "message.restart.vpc": "Por favor confirme que usted quiere reiniciar el VPC",
     "message.restart.vpc.remark": "Por favor confirme que desea reiniciar el VPC <p><small><i>Atención: creando un VPC sin redundancia forzara la limpieza. Todas las redes dejaran de estar disponibles por unos minutos</i>.</small></p>",
     "message.restoreVM": "¿Desea recuperar la MV?",
+    "message.role.update.fail": "Failed updating rule permission",
     "message.role.ordering.fail": "Reordenación de permisos de reglas abortada ya que la lista ha cambiado mientras realizaba los cambios. Por favor, intente de nuevo. ",
     "message.security.group.usage": "(Use <strong> Ctrl-click </strong> para seleccionar todos los grupos de seguridad pertinentes)",
     "message.select.a.zone": "Una zona normalmente se corresponde con un solo datacenter. Múltiples zonas pueden ayudar a aumentar la disponibilidad del cloud al proveer aislamiento físico y redundancia.",

ui/l10n/fr_FR.js

@@ -2176,6 +2176,7 @@ var dictionary = {
     "message.restart.vpc": "Confirmer le redémarrage du VPC",
     "message.restart.vpc.remark": "Veuillez confirmer que vous voulez redémarrer le VPC <p><small><i>Note : transformer un VPC non-redondant en VPC redondant va forcer un nettoyage du routeur. Le réseau associé ne sera pas disponible durant quelques minutes</i>.</small></p>",
     "message.restoreVM": "Voulez-vous restaurer la VM ?",
+    "message.role.update.fail": "Failed updating rule permission",
     "message.role.ordering.fail": "La réorganisation des règles d'autorisations a été abandonnée car la liste a changé pendant que vous apportez des modifications. Veuillez réessayer.",
     "message.security.group.usage": "(Utilisez <strong>Ctrl-clic</strong> pour sélectionner les groupes de sécurité visés)",
     "message.select.a.zone": "Une zone correspond typiquement à un seul centre de données. Des zones multiples peuvent permettre de rendre votre cloud plus fiable en apportant une isolation physique et de la redondance.",

ui/l10n/hu.js

@@ -2176,6 +2176,7 @@ var dictionary = {
     "message.restart.vpc": "Erősítsd meg, hogy újra akarod indítani a VPC-t!",
     "message.restart.vpc.remark": "Erősítsd meg, hogy újra akarod indítani a VPC-t! <p><small><i>Megjegyzés: egy nem redundáns VPC redundánssá tétele takarítást tesz szükségessé. A hálózatok nem lesznek elérhetőek egy pár percig.</i>.</small></p>",
     "message.restoreVM": "Helyre akarod állítani a VM-et?",
+    "message.role.update.fail": "Failed updating rule permission",
     "message.role.ordering.fail": "Reordering of rule permissions aborted as the list has changed while you were making changes. Please try again.",
     "message.security.group.usage": "(A <strong>Ctrl-kattintás</strong> használatával tudod az összes alkalmazható biztonsági csoportot kiválasztani)",
     "message.select.a.zone": "Egy zóna tipikusan egy adatközpontnak felel meg. Több zóna segíthet a felhőt megbízhatóbbá tenni fizikai izolációval és redundanciával.",

ui/l10n/it_IT.js

@@ -2176,6 +2176,7 @@ var dictionary = {
     "message.restart.vpc": "Si prega di confermare di voler riavviare VPC",
     "message.restart.vpc.remark": "Please confirm that you want to restart the VPC <p><small><i>Remark: making a non-redundant VPC redundant will force a clean up. The networks will not be available for a couple of minutes</i>.</small></p>",
     "message.restoreVM": "Do you want to restore the VM ?",
+    "message.role.update.fail": "Failed updating rule permission",
     "message.role.ordering.fail": "Reordering of rule permissions aborted as the list has changed while you were making changes. Please try again.",
     "message.security.group.usage": "(Use <strong>Ctrl-click</strong> to select all applicable security groups)",
     "message.select.a.zone": "Una zona corrisponde tipicamente ad un singolo datacenter. Zone multiple consentono di aumentare l'affidabilità creando isolamento fisico e ridondanza.",