FR26: Update rule permission of a role permission (#48)

This commit is contained in:
Nicolas Vazquez 2017-08-23 13:18:58 -03:00 committed by Rohit Yadav
parent 327360279d
commit c4f76a199b
11 changed files with 177 additions and 14 deletions

View File

@ -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();

View File

@ -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";

View File

@ -20,6 +20,7 @@ package org.apache.cloudstack.api.command.admin.acl;
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,16 @@ 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")
private Long ruleId;
@Parameter(name = ApiConstants.PERMISSION, type = CommandType.STRING, description = "Rule permission, can be: allow or deny")
private String rulePermission;
/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
/////////////////////////////////////////////////////
@ -67,6 +74,21 @@ public class UpdateRolePermissionCmd extends BaseCmd {
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 +106,35 @@ public class UpdateRolePermissionCmd extends BaseCmd {
@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 (rolePermission == null) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Provided role permission(s) do not exist");
if (getRulePermissionOrder() != null) {
if (getRuleId() != null || getRulePermission() != null) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Parameters permission and ruleid must be mutually exclusive with ruleorder");
}
rolePermissionsOrder.add(rolePermission);
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, "Invalid rule id provided");
}
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);

View File

@ -1106,6 +1106,7 @@ label.add.role=Add Role
label.edit.role=Edit Role
label.delete.role=Delete Role
message.role.ordering.fail=Reordering of rule permissions aborted as the list has changed while you were making changes. Please try again.
message.role.update.fail=Failed updating rule permission
label.root.certificate=Root certificate
label.root.disk.controller=Root disk controller
label.root.disk.offering=Root Disk Offering

View File

@ -20,6 +20,7 @@ package org.apache.cloudstack.acl.dao;
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

View File

@ -29,6 +29,7 @@ import com.cloud.utils.db.UpdateBuilder;
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 class RolePermissionsDaoImpl extends GenericDaoBase<RolePermissionVO, Lon
});
}
@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();

View File

@ -204,6 +204,12 @@ public class RoleManagerImpl extends ManagerBase implements RoleService, Configu
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) {

View File

@ -394,6 +394,46 @@ class TestDynamicRoles(cloudstackTestCase):
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):

View File

@ -1103,6 +1103,7 @@ dictionary = {
'label.edit.role': '<fmt:message key="label.edit.role" />',
'label.delete.role': '<fmt:message key="label.delete.role" />',
'message.role.ordering.fail': '<fmt:message key="message.role.ordering.fail" />',
'message.role.update.fail': '<fmt:message key="message.role.update.fail" />',
'label.root.disk.controller': '<fmt:message key="label.root.disk.controller" />',
'label.root.disk.offering': '<fmt:message key="label.root.disk.offering" />',
};

View File

@ -171,6 +171,28 @@
context: context,
noSelect: true,
noHeaderActionsColumn: true,
selectPermission: {
action: function(args){
$.ajax({
url: createURL("updateRolePermission"),
data: {
roleid: args.roleid,
ruleid: args.ruleid,
permission: args.permission
},
dataType: "json",
async: true,
success: function(json) {
$(window).trigger('cloudStack.fullRefresh');
},
error: function(json) {
cloudStack.dialog.notice({
message: 'message.role.update.fail'
});
}
});
}
},
reorder: {
moveDrag: {
action: function(args) {
@ -193,6 +215,7 @@
});
$.ajax({
type: 'POST',
url: createURL('updateRolePermission'),
data: {
roleid: rule.roleid,

View File

@ -31,6 +31,7 @@
var $item = $('<div>').addClass('data-item');
var multiRule = data;
var reorder = options.reorder;
var selectPermission = options.selectPermission;
$item.append($('<table>').append($('<tbody>')));
$tr = $('<tr>').appendTo($item.find('tbody'));
@ -189,10 +190,34 @@
return $(this).val() == data[fieldName];
});
var matchingValue = $matchingOption.size() ?
$matchingOption.html() : data[fieldName];
if (selectPermission) {
// Wrap div to get its html code
selectedOptionHtml = $matchingOption.clone().wrap('<div>').parent().html();
// Get html code from not matching option
$matchingSelect.find('option').each(
function() {
if ($(this).val() != data[fieldName]){
selectedOptionHtml += $(this).clone().wrap('<div>').parent().html();
}
}
);
$select = $('<select>');
$select.html(selectedOptionHtml);
$select.change(function(event) {
selectPermission.action({
roleid: data['roleid'],
ruleid: data['id'],
permission: $(this).val()
});
});
$td.append($select);
}
else {
var matchingValue = $matchingOption.size() ?
$matchingOption.html() : data[fieldName];
$td.append($('<span>').html(_s(matchingValue)));
$td.append($('<span>').html(_s(matchingValue)));
}
} else if (field.addButton && !options.noSelect) {
if (options.multipleAdd) {
$addButton.click(function() {
@ -862,6 +887,7 @@
var actionPreFilter = args.actionPreFilter;
var readOnlyCheck = args.readOnlyCheck;
var reorder = args.reorder;
var selectPermission = args.selectPermission;
var $thead = $('<tr>').appendTo(
$('<thead>').appendTo($inputTable)
@ -1193,7 +1219,8 @@
preFilter: actionPreFilter,
listView: listView,
tags: tags,
reorder: reorder
reorder: reorder,
selectPermission: selectPermission
}
).appendTo($dataBody);
});