hsato03 commented on code in PR #8639:
URL: https://github.com/apache/cloudstack/pull/8639#discussion_r1491679122
##########
server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java:
##########
@@ -382,42 +385,100 @@ public List<Role> findRolesByName(String name) {
public Pair<List<Role>, Integer> findRolesByName(String name, String
keyword, Long startIndex, Long limit) {
if (StringUtils.isNotBlank(name) || StringUtils.isNotBlank(keyword)) {
Pair<List<RoleVO>, Integer> data = roleDao.findAllByName(name,
keyword, startIndex, limit, isCallerRootAdmin());
- int removed = removeRootAdminRolesIfNeeded(data.first());
+ int removed = removeRolesIfNeeded(data.first());
return new
Pair<List<Role>,Integer>(ListUtils.toListOfInterface(data.first()),
Integer.valueOf(data.second() - removed));
}
return new Pair<List<Role>, Integer>(new ArrayList<Role>(), 0);
}
/**
- * Removes roles of the given list that have the type '{@link
RoleType#Admin}' if the user calling the method is not a 'root admin'.
- * The actual removal is executed via {@link
#removeRootAdminRoles(List)}. Therefore, if the method is called by a 'root
admin', we do nothing here.
+ * Removes roles from the given list if the role has different or more
permissions than the user's calling the method role
*/
- protected int removeRootAdminRolesIfNeeded(List<? extends Role> roles) {
- if (!isCallerRootAdmin()) {
- return removeRootAdminRoles(roles);
- }
- return 0;
- }
-
- /**
- * Remove all roles that have the {@link RoleType#Admin}.
- */
- protected int removeRootAdminRoles(List<? extends Role> roles) {
- if (CollectionUtils.isEmpty(roles)) {
+ protected int removeRolesIfNeeded(List<? extends Role> roles) {
+ if (roles.isEmpty()) {
return 0;
}
- Iterator<? extends Role> rolesIterator = roles.iterator();
+
+ Long callerRoleId = getCurrentAccount().getRoleId();
+ Map<String, Permission> callerRolePermissions =
getRoleRulesAndPermissions(callerRoleId);
+
int count = 0;
+ Iterator<? extends Role> rolesIterator = roles.iterator();
while (rolesIterator.hasNext()) {
Role role = rolesIterator.next();
- if (RoleType.Admin == role.getRoleType()) {
- count++;
- rolesIterator.remove();
+
+ if (role.getId() == callerRoleId ||
roleHasPermission(callerRolePermissions, role)) {
+ continue;
}
+
+ count++;
+ rolesIterator.remove();
}
+
return count;
}
+ /**
+ * Checks if the role of the caller account has compatible permissions of
the specified role.
+ * For each permission of the role of the caller, the roleToAccess needs
to contain the same permission.
+ *
+ * @param rolePermissions the permissions of the caller role.
+ * @param roleToAccess the role that the caller role wants to access.
+ * @return True if the role can be accessed with the given permissions;
false otherwise.
Review Comment:
Yes, I agree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
