etan39
/
cloudstack
mirror of https://github.com/apache/cloudstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #15 from shapeblue/dynamicrbac-4.5 

CLOUDSTACK-8562: Make role permissions orderable
This commit is contained in:
Rohit Yadav 2016-05-09 11:02:32 +05:30
parent 50355907a3 6f1e834753
commit 7ba881b152
11 changed files with 166 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
api/src/org/apache/cloudstack/acl/RoleService.java
View File

@ -39,11 +39,10 @@ public interface RoleService {
RolePermission createRolePermission(final Role role, final Rule rule, final RolePermission.Permission permission, final String description);
/**
* updateRolePermission updates the order/position of an role permission
* The list of role permissions is treated and consumed as an ordered linked list
* @param rolePermission The role permission that needs to be re-ordered
* @param parentRolePermission The parent role permission after which the role permission should be placed
* @param role The role whose permissions needs to be re-ordered
* @param newOrder The new list of ordered role permissions
*/
boolean updateRolePermission(final RolePermission rolePermission, final RolePermission parentRolePermission);
boolean updateRolePermission(final Role role, final List<RolePermission> newOrder);
boolean deleteRolePermission(final RolePermission rolePermission);
List<Role> listRoles();

1
api/src/org/apache/cloudstack/api/ApiConstants.java
View File

@ -359,6 +359,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_ORDER = "ruleorder";
public static final String USER = "user";
public static final String ACTIVE_ONLY = "activeonly";
public static final String TOKEN = "token";

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

@ -18,6 +18,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.RoleType;
import org.apache.cloudstack.api.APICommand;
@ -28,9 +29,13 @@ import org.apache.cloudstack.api.BaseCmd;
import org.apache.cloudstack.api.Parameter;
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.response.RolePermissionResponse;
import org.apache.cloudstack.api.response.RoleResponse;
import org.apache.cloudstack.api.response.SuccessResponse;
import org.apache.cloudstack.context.CallContext;
import java.util.ArrayList;
import java.util.List;
@APICommand(name = UpdateRolePermissionCmd.APINAME, description = "Updates a role permission order", responseObject = SuccessResponse.class,
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
since = "4.9.0",
@ -42,32 +47,24 @@ public class UpdateRolePermissionCmd extends BaseCmd {
//////////////// API parameters /////////////////////
/////////////////////////////////////////////////////
@Parameter(name = ApiConstants.ID, type = CommandType.UUID, required = true, entityType = RolePermissionResponse.class,
description = "ID of the role permission", validations = {ApiArgValidator.PositiveNumber})
private Long ruleId;
@Parameter(name = ApiConstants.ROLE_ID, type = CommandType.UUID, required = true, entityType = RoleResponse.class,
description = "ID of the role", validations = {ApiArgValidator.PositiveNumber})
private Long roleId;
@Parameter(name = ApiConstants.PARENT, type = CommandType.STRING, required = true, entityType = RolePermissionResponse.class,
description = "The parent role permission uuid, use 0 to move this rule at the top of the list", validations = {ApiArgValidator.NotNullOrEmpty})
private String parentRuleUuid;
@Parameter(name = ApiConstants.RULE_ORDER, type = CommandType.LIST, collectionType = CommandType.UUID, required = true, 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;
/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
/////////////////////////////////////////////////////
public Long getRuleId() {
return ruleId;
public Long getRoleId() {
return roleId;
}
public RolePermission getParentRolePermission() {
// A null or 0 previous id means this rule is moved to the top of the list
if (parentRuleUuid.equals("0")) {
return null;
}
final RolePermission parentRolePermission = roleService.findRolePermissionByUuid(parentRuleUuid);
if (parentRolePermission == null) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid parent role permission id provided");
}
return parentRolePermission;
public List<Long> getRulePermissionOrder() {
return rulePermissionOrder;
}
/////////////////////////////////////////////////////
@ -86,12 +83,20 @@ public class UpdateRolePermissionCmd extends BaseCmd {
@Override
public void execute() {
final RolePermission rolePermission = roleService.findRolePermission(getRuleId());
if (rolePermission == null) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid role permission id provided");
final Role role = roleService.findRole(getRoleId());
if (role == null) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid role id provided");
}
CallContext.current().setEventDetails("Role permission id: " + rolePermission.getId() + ", parent role permission uuid:" + parentRuleUuid);
boolean result = roleService.updateRolePermission(rolePermission, getParentRolePermission());
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);
}
boolean result = roleService.updateRolePermission(role, rolePermissionsOrder);
SuccessResponse response = new SuccessResponse(getCommandName());
response.setSuccess(result);
setResponseObject(response);

1
client/WEB-INF/classes/resources/messages.properties
View File

@ -1075,6 +1075,7 @@ label.roletype=Role Type
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.
label.root.certificate=Root certificate
label.root.disk.controller=Root disk controller
label.root.disk.offering=Root Disk Offering

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

@ -18,6 +18,8 @@
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.RolePermissionVO;
import java.util.List;
@ -32,18 +34,11 @@ public interface RolePermissionsDao extends GenericDao<RolePermissionVO, Long> {
/**
* Moves an existing role permission under a given parent role permission
* @param item an existing role permission
* @param parentId the new parent ID after move for the role permission
* @param role the existing role
* @param newOrder the new role permissions order
* @return returns true on success
*/
boolean update(final RolePermissionVO item, final RolePermissionVO parent);
/**
* Removes an existing role permission and updates the ordered linked-list of role's permissions list
* @param id the ID of the role permission to be removed
* @return returns true on success
*/
boolean remove(Long id);
boolean update(final Role role, final List<RolePermission> newOrder);
/**
* Returns ordered linked-list of role permission for a given role

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

@ -17,6 +17,7 @@
package org.apache.cloudstack.acl.dao;
import com.cloud.utils.db.Attribute;
import com.cloud.utils.db.Filter;
import com.cloud.utils.db.GenericDaoBase;
import com.cloud.utils.db.SearchBuilder;
@ -24,6 +25,10 @@ import com.cloud.utils.db.SearchCriteria;
import com.cloud.utils.db.Transaction;
import com.cloud.utils.db.TransactionCallback;
import com.cloud.utils.db.TransactionStatus;
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.RolePermissionVO;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
@ -31,7 +36,9 @@ import org.springframework.stereotype.Component;
import javax.ejb.Local;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@Component
@Local(value = {RolePermissionsDao.class})
@ -39,14 +46,20 @@ public class RolePermissionsDaoImpl extends GenericDaoBase<RolePermissionVO, Lon
protected static final Logger LOGGER = Logger.getLogger(RolePermissionsDaoImpl.class);
private final SearchBuilder<RolePermissionVO> RolePermissionsSearch;
private Attribute sortOrderAttribute;
public RolePermissionsDaoImpl() {
super();
RolePermissionsSearch = createSearchBuilder();
RolePermissionsSearch.and("uuid", RolePermissionsSearch.entity().getUuid(), SearchCriteria.Op.EQ);
RolePermissionsSearch.and("roleId", RolePermissionsSearch.entity().getRoleId(), SearchCriteria.Op.EQ);
RolePermissionsSearch.and("sortOrder", RolePermissionsSearch.entity().getSortOrder(), SearchCriteria.Op.EQ);
RolePermissionsSearch.done();
sortOrderAttribute = _allAttributes.get("sortOrder");
assert (sortOrderAttribute != null) : "Couldn't find one of these attributes";
}
private boolean updateSortOrder(final RolePermissionVO permissionBeingMoved, final RolePermissionVO parentPermission) {
@ -83,59 +96,68 @@ public class RolePermissionsDaoImpl extends GenericDaoBase<RolePermissionVO, Lon
item.setSortOrder(0);
final List<RolePermissionVO> permissionsList = findAllByRoleIdSorted(item.getRoleId());
if (permissionsList != null && permissionsList.size() > 0) {
item.setSortOrder(permissionsList.size());
RolePermission lastRule = permissionsList.get(permissionsList.size() - 1);
item.setSortOrder(lastRule.getSortOrder() + 1);
}
return super.persist(item);
}
@Override
public boolean remove(Long id) {
final RolePermissionVO itemToBeRemoved = findById(id);
if (itemToBeRemoved == null) {
public boolean update(final Role role, final List<RolePermission> newOrder) {
if (role == null || newOrder == null || newOrder.isEmpty()) {
return false;
}
boolean updateStatus = true;
final boolean removal = super.remove(id);
if (removal) {
long sortOrder = 0L;
for (final RolePermissionVO permission : findAllByRoleIdSorted(itemToBeRemoved.getRoleId())) {
permission.setSortOrder(sortOrder++);
if (!update(permission.getId(), permission)) {
updateStatus = false;
break;
}
}
}
return removal && updateStatus;
}
@Override
public boolean update(final RolePermissionVO item, final RolePermissionVO parent) {
return Transaction.execute(new TransactionCallback<Boolean>() {
@Override
public Boolean doInTransaction(TransactionStatus status) {
return updateSortOrder(item, parent);
final String failMessage = "The role's rule permissions list has changed while you were making updates, aborted re-ordering of rules. Please try again.";
final List<RolePermissionVO> currentOrder = findAllByRoleIdSorted(role.getId());
if (role.getId() < 1L || newOrder.size() != currentOrder.size()) {
throw new CloudRuntimeException(failMessage);
}
final Set<Long> newOrderSet = new HashSet<>();
for (final RolePermission permission : newOrder) {
if (permission == null) {
continue;
}
newOrderSet.add(permission.getId());
}
final Set<Long> currentOrderSet = new HashSet<>();
for (final RolePermission permission : currentOrder) {
currentOrderSet.add(permission.getId());
}
if (!newOrderSet.equals(currentOrderSet)) {
throw new CloudRuntimeException(failMessage);
}
long sortOrder = 0L;
for (RolePermission rolePermission : newOrder) {
final SearchCriteria<RolePermissionVO> sc = RolePermissionsSearch.create();
sc.setParameters("uuid", rolePermission.getUuid());
sc.setParameters("roleId", role.getId());
sc.setParameters("sortOrder", rolePermission.getSortOrder());
final UpdateBuilder ub = getUpdateBuilder(rolePermission);
ub.set(rolePermission, sortOrderAttribute, sortOrder);
final int result = update(ub, sc, null);
if (result < 1) {
throw new CloudRuntimeException(failMessage);
}
sortOrder++;
}
return true;
}
});
}
public List<RolePermissionVO> findAllByRoleAndParentId(final Long parentId, final Long roleId) {
if (parentId == null || roleId == null) {
return Collections.emptyList();
}
SearchCriteria<RolePermissionVO> sc = RolePermissionsSearch.create();
sc.setParameters("roleId", roleId);
sc.setParameters("parentId", parentId);
return listBy(sc);
}
@Override
public List<RolePermissionVO> findAllByRoleIdSorted(final Long roleId) {
SearchCriteria<RolePermissionVO> sc = RolePermissionsSearch.create();
final SearchCriteria<RolePermissionVO> sc = RolePermissionsSearch.create();
if (roleId != null && roleId > 0L) {
sc.setParameters("roleId", roleId);
}
List<RolePermissionVO> rolePermissionList = listBy(sc, new Filter(RolePermissionVO.class, "sortOrder", true, null, null));
final Filter searchBySorted = new Filter(RolePermissionVO.class, "sortOrder", true, null, null);
searchBySorted.addOrderBy(RolePermissionVO.class, "id", true);
final List<RolePermissionVO> rolePermissionList = listBy(sc, searchBySorted);
if (rolePermissionList == null) {
return Collections.emptyList();
}

4
server/src/org/apache/cloudstack/acl/RoleManagerImpl.java
View File

@ -193,9 +193,9 @@ public class RoleManagerImpl extends ManagerBase implements RoleService, Configu
@Override
@ActionEvent(eventType = EventTypes.EVENT_ROLE_PERMISSION_UPDATE, eventDescription = "updating Role Permission order")
public boolean updateRolePermission(final RolePermission rolePermission, final RolePermission parentRolePermission) {
public boolean updateRolePermission(final Role role, final List<RolePermission> newOrder) {
checkCallerAccess();
return rolePermission != null && rolePermissionsDao.update((RolePermissionVO) rolePermission, (RolePermissionVO) parentRolePermission);
return role != null && newOrder != null && rolePermissionsDao.update(role, newOrder);
}
@Override

50
test/integration/smoke/test_dynamicroles.py
View File

@ -21,6 +21,7 @@ from marvin.cloudstackException import CloudstackAPIException
from marvin.lib.base import Account, Role, RolePermission
from marvin.lib.utils import cleanup_resources
from nose.plugins.attrib import attr
from random import shuffle
import copy
import random
@ -378,21 +379,54 @@ class TestDynamicRoles(cloudstackTestCase):
# Move last item to the top
rule = permissions.pop(len(permissions)-1)
permissions = [rule] + permissions
rule.update(self.apiclient, parent=0)
rule.update(self.apiclient, ruleorder=",".join(map(lambda x: x.id, permissions)))
validate_permissions_list(permissions)
# Move to the bottom
rule = permissions.pop(0)
permissions = permissions + [rule]
rule.update(self.apiclient, parent=permissions[-2].id)
rule.update(self.apiclient, ruleorder=",".join(map(lambda x: x.id, permissions)))
validate_permissions_list(permissions)
# Move item in mid
rule = permissions.pop(1)
new_parent = permissions[1]
permissions.insert(2, rule)
rule.update(self.apiclient, parent=new_parent.id)
validate_permissions_list(permissions)
# Random shuffles
for _ in range(3):
shuffle(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_concurrent_updates(self):
"""
Tests concurrent order updation of role permission
"""
permissions = [self.rolepermission]
rules = ['list*', '*Vol*', 'listCapabilities']
for rule in rules:
data = copy.deepcopy(self.testdata["rolepermission"])
data['rule'] = rule
permission = RolePermission.create(
self.apiclient,
data
)
self.cleanup.append(permission)
permissions.append(permission)
# The following rule is considered to be created by another mgmt server
data = copy.deepcopy(self.testdata["rolepermission"])
data['rule'] = "someRule*"
permission = RolePermission.create(
self.apiclient,
data
)
self.cleanup.append(permission)
shuffle(permissions)
try:
permission.update(self.apiclient, ruleorder=",".join(map(lambda x: x.id, permissions)))
self.fail("Reordering should fail in case of concurrent updates by other user")
except CloudstackAPIException: pass
@attr(tags=['advanced', 'simulator', 'basic', 'sg'], required_hardware=False)

2
tools/marvin/marvin/lib/base.py
View File

@ -156,7 +156,7 @@ class RolePermission:
"""Update the role permission"""
cmd = updateRolePermission.updateRolePermissionCmd()
cmd.id = self.id
cmd.roleid = self.roleid
[setattr(cmd, k, v) for k, v in kwargs.items()]
return apiclient.updateRolePermission(cmd)

1
ui/dictionary.jsp
View File

@ -1074,6 +1074,7 @@ dictionary = {
'label.add.role': '<fmt:message key="label.add.role" />',
'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" />',
'label.root.disk.controller': '<fmt:message key="label.root.disk.controller" />',
'label.root.disk.offering': '<fmt:message key="label.root.disk.offering" />',
};

30
ui/scripts/roles.js
View File

@ -16,6 +16,7 @@
// under the License.
(function($, cloudStack) {
var apiList = [];
var rolePermissions = [];
cloudStack.sections.roles = {
title: 'label.roles',
id: 'roles',
@ -174,19 +175,37 @@
moveDrag: {
action: function(args) {
var rule = args.context.multiRule[0];
var newParentId = args.prevItem ? args.prevItem.id : 0;
var prevItemId = args.prevItem ? args.prevItem.id : 0;
var ruleOrder = [];
$.each(rolePermissions, function(idx, item) {
var itemId = item.id;
if (idx == 0 && prevItemId == 0) {
ruleOrder.push(rule.id);
}
if (itemId == rule.id) {
return true;
}
ruleOrder.push(item.id);
if (prevItemId == itemId) {
ruleOrder.push(rule.id);
}
});
$.ajax({
url: createURL('updateRolePermission'),
data: {
id: rule.id,
parent: newParentId
roleid: rule.roleid,
ruleorder: ruleOrder.join()
},
success: function(json) {
args.response.success();
$(window).trigger('cloudStack.fullRefresh');
},
error: function(json) {
args.response.error(parseXMLHttpResponse(json));
cloudStack.dialog.notice({
message: 'message.role.ordering.fail'
});
}
});
}
@ -277,6 +296,9 @@
dataType: 'json',
success: function(json) {
var rules = json.listrolepermissionsresponse.rolepermission;
if (rules) {
rolePermissions = rules;
}
args.response.success({
data: rules
});