Skip to content

Commit 5035590

Browse files
author
John Burwell
committed
Merge pull request #14 from shapeblue/dynamicrbac-4.5
Make role permissions orderable
2 parents 97089e1 + 9d8b1fd commit 5035590

File tree

17 files changed

+1159
-1094
lines changed

17 files changed

+1159
-1094
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,5 @@ enum Permission {ALLOW, DENY}
2727
Rule getRule();
2828
Permission getPermission();
2929
String getDescription();
30+
long getSortOrder();
3031
}

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,16 @@ public interface RoleService {
3434
boolean deleteRole(final Role role);
3535

3636
RolePermission findRolePermission(final Long id);
37+
RolePermission findRolePermissionByUuid(final String uuid);
38+
3739
RolePermission createRolePermission(final Role role, final Rule rule, final RolePermission.Permission permission, final String description);
38-
boolean updateRolePermission(final RolePermission rolePermission, final Rule rule, final RolePermission.Permission permission, final String description);
40+
/**
41+
* updateRolePermission updates the order/position of an role permission
42+
* The list of role permissions is treated and consumed as an ordered linked list
43+
* @param rolePermission The role permission that needs to be re-ordered
44+
* @param parentRolePermission The parent role permission after which the role permission should be placed
45+
*/
46+
boolean updateRolePermission(final RolePermission rolePermission, final RolePermission parentRolePermission);
3947
boolean deleteRolePermission(final RolePermission rolePermission);
4048

4149
List<Role> listRoles();

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ public class ApiConstants {
190190
public static final String PORTAL = "portal";
191191
public static final String PORTABLE_IP_ADDRESS = "portableipaddress";
192192
public static final String PORT_FORWARDING_SERVICE_ID = "portforwardingserviceid";
193+
public static final String PARENT = "parent";
193194
public static final String PRIVATE_INTERFACE = "privateinterface";
194195
public static final String PRIVATE_IP = "privateip";
195196
public static final String PRIVATE_PORT = "privateport";

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

Lines changed: 19 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,20 @@
1818
package org.apache.cloudstack.api.command.admin.acl;
1919

2020
import com.cloud.user.Account;
21-
import com.google.common.base.Strings;
2221
import org.apache.cloudstack.acl.RolePermission;
2322
import org.apache.cloudstack.acl.RoleType;
24-
import org.apache.cloudstack.acl.Rule;
2523
import org.apache.cloudstack.api.APICommand;
24+
import org.apache.cloudstack.api.ApiArgValidator;
2625
import org.apache.cloudstack.api.ApiConstants;
2726
import org.apache.cloudstack.api.ApiErrorCode;
2827
import org.apache.cloudstack.api.BaseCmd;
2928
import org.apache.cloudstack.api.Parameter;
3029
import org.apache.cloudstack.api.ServerApiException;
31-
import org.apache.cloudstack.api.ApiArgValidator;
3230
import org.apache.cloudstack.api.response.RolePermissionResponse;
3331
import org.apache.cloudstack.api.response.SuccessResponse;
3432
import org.apache.cloudstack.context.CallContext;
3533

36-
@APICommand(name = UpdateRolePermissionCmd.APINAME, description = "Updates a role permission", responseObject = SuccessResponse.class,
34+
@APICommand(name = UpdateRolePermissionCmd.APINAME, description = "Updates a role permission order", responseObject = SuccessResponse.class,
3735
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
3836
since = "4.9.0",
3937
authorized = {RoleType.Admin})
@@ -46,39 +44,30 @@ public class UpdateRolePermissionCmd extends BaseCmd {
4644

4745
@Parameter(name = ApiConstants.ID, type = CommandType.UUID, required = true, entityType = RolePermissionResponse.class,
4846
description = "ID of the role permission", validations = {ApiArgValidator.PositiveNumber})
49-
private Long rolePermissionId;
50-
51-
@Parameter(name = ApiConstants.RULE, type = CommandType.STRING, required = true,
52-
description = "The name of the API or wildcard rule such as list*", validations = {ApiArgValidator.NotNullOrEmpty})
53-
private String rule;
47+
private Long ruleId;
5448

55-
@Parameter(name = ApiConstants.PERMISSION, type = CommandType.STRING, required = true, description = "The rule permission, allow or deny. Default: deny.")
56-
private String permission;
57-
58-
@Parameter(name = ApiConstants.DESCRIPTION, type = CommandType.STRING, description = "The description of the role permission")
59-
private String description;
49+
@Parameter(name = ApiConstants.PARENT, type = CommandType.STRING, required = true, entityType = RolePermissionResponse.class,
50+
description = "The parent role permission uuid, use 0 to move this rule at the top of the list", validations = {ApiArgValidator.NotNullOrEmpty})
51+
private String parentRuleUuid;
6052

6153
/////////////////////////////////////////////////////
6254
/////////////////// Accessors ///////////////////////
6355
/////////////////////////////////////////////////////
6456

65-
public Long getRolePermissionId() {
66-
return rolePermissionId;
57+
public Long getRuleId() {
58+
return ruleId;
6759
}
6860

69-
public Rule getRule() {
70-
return new Rule(rule);
71-
}
72-
73-
public RolePermission.Permission getPermission() {
74-
if (Strings.isNullOrEmpty(permission)) {
61+
public RolePermission getParentRolePermission() {
62+
// A null or 0 previous id means this rule is moved to the top of the list
63+
if (parentRuleUuid.equals("0")) {
7564
return null;
7665
}
77-
return RolePermission.Permission.valueOf(permission.toUpperCase());
78-
}
79-
80-
public String getDescription() {
81-
return description;
66+
final RolePermission parentRolePermission = roleService.findRolePermissionByUuid(parentRuleUuid);
67+
if (parentRolePermission == null) {
68+
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid parent role permission id provided");
69+
}
70+
return parentRolePermission;
8271
}
8372

8473
/////////////////////////////////////////////////////
@@ -97,12 +86,12 @@ public long getEntityOwnerId() {
9786

9887
@Override
9988
public void execute() {
100-
RolePermission rolePermission = roleService.findRolePermission(getRolePermissionId());
89+
final RolePermission rolePermission = roleService.findRolePermission(getRuleId());
10190
if (rolePermission == null) {
10291
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid role permission id provided");
10392
}
104-
CallContext.current().setEventDetails("Role permission id: " + rolePermission.getId() + ", rule:" + getRule() + ", permission: " + getPermission() + ", description: " + getDescription());
105-
boolean result = roleService.updateRolePermission(rolePermission, getRule(), getPermission(), getDescription());
93+
CallContext.current().setEventDetails("Role permission id: " + rolePermission.getId() + ", parent role permission uuid:" + parentRuleUuid);
94+
boolean result = roleService.updateRolePermission(rolePermission, getParentRolePermission());
10695
SuccessResponse response = new SuccessResponse(getCommandName());
10796
response.setSuccess(result);
10897
setResponseObject(response);

engine/schema/src/com/cloud/upgrade/dao/Upgrade452to453.java

Lines changed: 25 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -62,68 +62,21 @@ public File[] getPrepareScripts() {
6262

6363
@Override
6464
public void performDataMigration(Connection conn) {
65-
setupRolesAndPermissionsForDynamicRBAC(conn);
65+
setupRolesAndPermissionsForDynamicChecker(conn);
6666
}
6767

68-
private void createDefaultRole(final Connection conn, final Long id, final String name, final RoleType roleType) {
69-
final String insertSql = String.format("INSERT INTO `cloud`.`roles` (`id`, `uuid`, `name`, `role_type`, `description`) values (%d, UUID(), '%s', '%s', 'Default %s role');",
70-
id, name, roleType.name(), roleType.name().toLowerCase());
71-
try ( PreparedStatement updatePstmt = conn.prepareStatement(insertSql) ) {
72-
updatePstmt.executeUpdate();
73-
} catch (SQLException e) {
74-
throw new CloudRuntimeException("Unable to create default role with id: " + id + " name: " + name, e);
75-
}
76-
}
77-
78-
private void createRoleMapping(final Connection conn, final Long roleId, final String apiName) {
79-
final String insertSql = String.format("INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`) values (UUID(), %d, '%s', 'ALLOW') ON DUPLICATE KEY UPDATE rule=rule;",
80-
roleId, apiName);
81-
try ( PreparedStatement updatePstmt = conn.prepareStatement(insertSql)) {
82-
updatePstmt.executeUpdate();
83-
} catch (SQLException ignored) {
84-
s_logger.debug("Unable to insert mapping for role id:" + roleId + " apiName: " + apiName);
85-
}
86-
}
87-
88-
private void addRoleColumnAndMigrateAccountTable(final Connection conn, final RoleType[] roleTypes) {
89-
// Add role_id column to account table
90-
final String alterTableSql = "ALTER TABLE `cloud`.`account` ADD COLUMN `role_id` bigint(20) unsigned COMMENT 'role id for this account' AFTER `type`, " +
91-
"ADD KEY `fk_account__role_id` (`role_id`), " +
92-
"ADD CONSTRAINT `fk_account__role_id` FOREIGN KEY (`role_id`) REFERENCES `roles` (`id`);";
93-
try (PreparedStatement pstmt = conn.prepareStatement(alterTableSql)) {
94-
pstmt.executeUpdate();
95-
s_logger.info("Altered cloud.account table and added column role_id");
96-
} catch (SQLException e) {
97-
if (e.getMessage().contains("role_id")) {
98-
s_logger.warn("cloud.account table already has the role_id column, skipping altering table and migration of accounts");
99-
return;
100-
} else {
101-
throw new CloudRuntimeException("Unable to create column quota_calculated in table cloud_usage.cloud_usage", e);
102-
}
103-
}
104-
// Migrate existing account to one of default roles based on account type
105-
migrateAccountsToDefaultRoles(conn, roleTypes);
106-
}
107-
108-
private void migrateAccountsToDefaultRoles(final Connection conn, final RoleType[] roleTypes) {
109-
// Migrate existing accounts to default roles based on account type
110-
try (PreparedStatement selectStatement = conn.prepareStatement("SELECT `id`, `type` FROM `cloud`.`account`;");
111-
ResultSet selectResultSet = selectStatement.executeQuery()) {
68+
private void migrateAccountsToDefaultRoles(final Connection conn) {
69+
try (final PreparedStatement selectStatement = conn.prepareStatement("SELECT `id`, `type` FROM `cloud`.`account`;");
70+
final ResultSet selectResultSet = selectStatement.executeQuery()) {
11271
while (selectResultSet.next()) {
113-
Long accountId = selectResultSet.getLong(1);
114-
Short accountType = selectResultSet.getShort(2);
115-
Long roleId = null;
116-
for (RoleType roleType : roleTypes) {
117-
if (roleType.getAccountType() == accountType) {
118-
roleId = roleType.getId();
119-
break;
120-
}
121-
}
122-
// Skip is account type does not match any of the default roles
123-
if (roleId == null) {
72+
final Long accountId = selectResultSet.getLong(1);
73+
final Short accountType = selectResultSet.getShort(2);
74+
final Long roleId = RoleType.getByAccountType(accountType).getId();
75+
if (roleId < 1L || roleId > 4L) {
76+
s_logger.warn("Skipping role ID migration due to invalid role_id resolved for account id=" + accountId);
12477
continue;
12578
}
126-
try (PreparedStatement updateStatement = conn.prepareStatement("UPDATE `cloud`.`account` SET role_id = ? WHERE id = ?;")) {
79+
try (final PreparedStatement updateStatement = conn.prepareStatement("UPDATE `cloud`.`account` SET account.role_id = ? WHERE account.id = ? ;")) {
12780
updateStatement.setLong(1, roleId);
12881
updateStatement.setLong(2, accountId);
12982
updateStatement.executeUpdate();
@@ -138,33 +91,25 @@ private void migrateAccountsToDefaultRoles(final Connection conn, final RoleType
13891
s_logger.debug("Done migrating existing accounts to use one of default roles based on account type");
13992
}
14093

141-
private void setupRolesAndPermissionsForDynamicRBAC(final Connection conn) {
142-
// If there are existing roles, avoid resetting data
143-
try (PreparedStatement selectStatement = conn.prepareStatement("SELECT * FROM `cloud`.`roles`")) {
144-
ResultSet resultSet = selectStatement.executeQuery();
145-
if (resultSet != null && resultSet.next()) {
146-
resultSet.close();
147-
if (s_logger.isDebugEnabled()) {
148-
s_logger.debug("Found existing roles. Skipping migration of commands.properties to dynamic roles table.");
149-
}
94+
private void setupRolesAndPermissionsForDynamicChecker(final Connection conn) {
95+
final String alterTableSql = "ALTER TABLE `cloud`.`account` " +
96+
"ADD COLUMN `role_id` bigint(20) unsigned COMMENT 'role id for this account' AFTER `type`, " +
97+
"ADD KEY `fk_account__role_id` (`role_id`), " +
98+
"ADD CONSTRAINT `fk_account__role_id` FOREIGN KEY (`role_id`) REFERENCES `roles` (`id`);";
99+
try (final PreparedStatement pstmt = conn.prepareStatement(alterTableSql)) {
100+
pstmt.executeUpdate();
101+
} catch (SQLException e) {
102+
if (e.getMessage().contains("role_id")) {
103+
s_logger.warn("cloud.account table already has the role_id column, skipping altering table and migration of accounts");
150104
return;
105+
} else {
106+
throw new CloudRuntimeException("Unable to create column quota_calculated in table cloud_usage.cloud_usage", e);
151107
}
152-
} catch (SQLException e) {
153-
s_logger.error("Unable to find existing roles, if you need to add default roles please add them manually. Giving up!");
154-
return;
155-
}
156-
157-
// Add default roles
158-
RoleType[] roleTypes = new RoleType[] {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User};
159-
for (RoleType roleType: roleTypes) {
160-
createDefaultRole(conn, roleType.getId(), roleType.name(), roleType);
161108
}
162109

163-
// Add role_id column to account and map existing accounts to default roles
164-
addRoleColumnAndMigrateAccountTable(conn, roleTypes);
110+
migrateAccountsToDefaultRoles(conn);
165111

166-
// Add default set of role-api mapping when commands.properties file is not found
167-
Map<String, String> apiMap = PropertiesUtil.processConfigFile(new String[] { PropertiesUtil.getDefaultApiCommandsFileName() });
112+
final Map<String, String> apiMap = PropertiesUtil.processConfigFile(new String[] { PropertiesUtil.getDefaultApiCommandsFileName() });
168113
if (apiMap == null || apiMap.isEmpty()) {
169114
if (s_logger.isDebugEnabled()) {
170115
s_logger.debug("The commands.properties file and default role permissions were not found. " +
@@ -175,33 +120,12 @@ private void setupRolesAndPermissionsForDynamicRBAC(final Connection conn) {
175120
s_logger.error("Unable to find default role-api mapping sql file, please configure api per role manually");
176121
return;
177122
}
178-
try(FileReader reader = new FileReader(new File(script));) {
123+
try(final FileReader reader = new FileReader(new File(script))) {
179124
ScriptRunner runner = new ScriptRunner(conn, false, true);
180125
runner.runScript(reader);
181126
} catch (SQLException | IOException e) {
182127
s_logger.error("Unable to insert default api-role mappings from file: " + script + ". Please configure api per role manually, giving up!", e);
183128
}
184-
} else {
185-
// If commands.properties file exists, use it to create the mappings
186-
for (RoleType roleType : roleTypes) {
187-
// Allow all for root admin
188-
if (roleType == RoleType.Admin) {
189-
createRoleMapping(conn, roleType.getId(), "*");
190-
continue;
191-
}
192-
for (Map.Entry<String, String> entry : apiMap.entrySet()) {
193-
String apiName = entry.getKey();
194-
String roleMask = entry.getValue();
195-
try {
196-
short cmdPermissions = Short.parseShort(roleMask);
197-
if ((cmdPermissions & roleType.getMask()) != 0) {
198-
createRoleMapping(conn, roleType.getId(), apiName);
199-
}
200-
} catch (NumberFormatException nfe) {
201-
s_logger.info("Malformed key=value pair for entry: " + entry.toString());
202-
}
203-
}
204-
}
205129
}
206130
}
207131

engine/schema/src/org/apache/cloudstack/acl/RolePermissionVO.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ public class RolePermissionVO implements RolePermission {
5151
@Column(name = "description")
5252
private String description;
5353

54+
@Column(name = "sort_order")
55+
private long sortOrder = 0;
56+
5457
public RolePermissionVO() {
5558
this.uuid = UUID.randomUUID().toString();
5659
}
@@ -106,4 +109,12 @@ public String getDescription() {
106109
public void setDescription(String description) {
107110
this.description = description;
108111
}
112+
113+
public long getSortOrder() {
114+
return sortOrder;
115+
}
116+
117+
public void setSortOrder(long sortOrder) {
118+
this.sortOrder = sortOrder;
119+
}
109120
}

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,32 @@
2323
import java.util.List;
2424

2525
public interface RolePermissionsDao extends GenericDao<RolePermissionVO, Long> {
26-
List<RolePermissionVO> findAllByRoleId(Long roleId);
26+
/**
27+
* Adds a new role permission at the end of the list of role permissions
28+
* @param item the new role permission
29+
* @return returns persisted role permission
30+
*/
31+
RolePermissionVO persist(final RolePermissionVO item);
32+
33+
/**
34+
* Moves an existing role permission under a given parent role permission
35+
* @param item an existing role permission
36+
* @param parentId the new parent ID after move for the role permission
37+
* @return returns true on success
38+
*/
39+
boolean update(final RolePermissionVO item, final RolePermissionVO parent);
40+
41+
/**
42+
* Removes an existing role permission and updates the ordered linked-list of role's permissions list
43+
* @param id the ID of the role permission to be removed
44+
* @return returns true on success
45+
*/
46+
boolean remove(Long id);
47+
48+
/**
49+
* Returns ordered linked-list of role permission for a given role
50+
* @param roleId the ID of the role
51+
* @return returns list of role permissions
52+
*/
53+
List<RolePermissionVO> findAllByRoleIdSorted(Long roleId);
2754
}

0 commit comments

Comments
 (0)