Skip to content

Commit ae5a919

Browse files
hsato03Henrique Sato
authored andcommitted
Limit listRoles API visibility (apache#8639)
Co-authored-by: Henrique Sato <henrique.sato@scclouds.com.br>
1 parent 989b2ff commit ae5a919

6 files changed

Lines changed: 227 additions & 54 deletions

File tree

plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,11 @@ public UserTwoFactorAuthenticationSetupResponse setupUserTwoFactorAuthentication
346346
return null;
347347
}
348348

349+
@Override
350+
public List<String> getApiNameList() {
351+
return null;
352+
}
353+
349354
@Override
350355
public boolean deleteUserAccount(long arg0) {
351356
// TODO Auto-generated method stub

server/src/main/java/com/cloud/user/AccountManager.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,8 @@ void buildACLViewSearchCriteria(SearchCriteria<? extends ControlledViewEntity> s
195195
UserTwoFactorAuthenticator getUserTwoFactorAuthenticator(final Long domainId, final Long userAccountId);
196196

197197
void verifyUsingTwoFactorAuthenticationCode(String code, Long domainId, Long userAccountId);
198+
198199
UserTwoFactorAuthenticationSetupResponse setupUserTwoFactorAuthentication(SetupUserTwoFactorAuthenticationCmd cmd);
199200

201+
List<String> getApiNameList();
200202
}

server/src/main/java/com/cloud/user/AccountManagerImpl.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,11 @@ public void setQuerySelectors(List<QuerySelector> querySelectors) {
436436
_querySelectors = querySelectors;
437437
}
438438

439+
@Override
440+
public List<String> getApiNameList() {
441+
return apiNameList;
442+
}
443+
439444
@Override
440445
public boolean configure(final String name, final Map<String, Object> params) throws ConfigurationException {
441446
_systemAccount = _accountDao.findById(Account.ACCOUNT_ID_SYSTEM);

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

Lines changed: 82 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,12 @@
1818

1919
import java.util.ArrayList;
2020
import java.util.Collections;
21+
import java.util.HashMap;
22+
import java.util.HashSet;
2123
import java.util.Iterator;
2224
import java.util.List;
2325
import java.util.Map;
26+
import java.util.Set;
2427

2528
import javax.inject.Inject;
2629

@@ -407,7 +410,7 @@ public List<Role> findRolesByName(String name) {
407410
public Pair<List<Role>, Integer> findRolesByName(String name, String keyword, Long startIndex, Long limit) {
408411
if (StringUtils.isNotBlank(name) || StringUtils.isNotBlank(keyword)) {
409412
Pair<List<RoleVO>, Integer> data = roleDao.findAllByName(name, keyword, startIndex, limit, isCallerRootAdmin());
410-
int removed = removeRootAdminRolesIfNeeded(data.first());
413+
int removed = removeRolesIfNeeded(data.first());
411414
return new Pair<List<Role>,Integer>(ListUtils.toListOfInterface(data.first()), Integer.valueOf(data.second() - removed));
412415
}
413416
return new Pair<List<Role>, Integer>(new ArrayList<Role>(), 0);
@@ -433,35 +436,93 @@ public Pair<List<Role>, Integer> findRolesByKeyword(String keyword, Long startIn
433436
return new Pair<List<Role>, Integer>(new ArrayList<Role>(), 0);
434437
}
435438
/**
436-
* Removes roles of the given list that have the type '{@link RoleType#Admin}' if the user calling the method is not a 'root admin'.
437-
* The actual removal is executed via {@link #removeRootAdminRoles(List)}. Therefore, if the method is called by a 'root admin', we do nothing here.
438-
*/
439-
protected int removeRootAdminRolesIfNeeded(List<? extends Role> roles) {
440-
if (!isCallerRootAdmin()) {
441-
return removeRootAdminRoles(roles);
442-
}
443-
return 0;
444-
}
445-
446-
/**
447-
* Remove all roles that have the {@link RoleType#Admin}.
439+
* Removes roles from the given list if the role has different or more permissions than the user's calling the method role
448440
*/
449-
protected int removeRootAdminRoles(List<? extends Role> roles) {
450-
if (CollectionUtils.isEmpty(roles)) {
441+
protected int removeRolesIfNeeded(List<? extends Role> roles) {
442+
if (roles.isEmpty()) {
451443
return 0;
452444
}
453-
Iterator<? extends Role> rolesIterator = roles.iterator();
445+
446+
Long callerRoleId = getCurrentAccount().getRoleId();
447+
Map<String, Permission> callerRolePermissions = getRoleRulesAndPermissions(callerRoleId);
448+
454449
int count = 0;
450+
Iterator<? extends Role> rolesIterator = roles.iterator();
455451
while (rolesIterator.hasNext()) {
456452
Role role = rolesIterator.next();
457-
if (RoleType.Admin == role.getRoleType()) {
458-
count++;
459-
rolesIterator.remove();
453+
454+
if (role.getId() == callerRoleId || roleHasPermission(callerRolePermissions, role)) {
455+
continue;
460456
}
457+
458+
count++;
459+
rolesIterator.remove();
461460
}
461+
462462
return count;
463463
}
464464

465+
/**
466+
* Checks if the role of the caller account has compatible permissions of the specified role.
467+
* For each permission of the role of the caller, the target role needs to contain the same permission.
468+
*
469+
* @param sourceRolePermissions the permissions of the caller role.
470+
* @param targetRole the role that the caller role wants to access.
471+
* @return True if the role can be accessed with the given permissions; false otherwise.
472+
*/
473+
protected boolean roleHasPermission(Map<String, Permission> sourceRolePermissions, Role targetRole) {
474+
Set<String> rulesAlreadyCompared = new HashSet<>();
475+
for (RolePermission rolePermission : findAllPermissionsBy(targetRole.getId())) {
476+
boolean permissionIsRegex = rolePermission.getRule().getRuleString().contains("*");
477+
478+
for (String apiName : accountManager.getApiNameList()) {
479+
if (!rolePermission.getRule().matches(apiName) || rulesAlreadyCompared.contains(apiName)) {
480+
continue;
481+
}
482+
483+
if (rolePermission.getPermission() == Permission.ALLOW && (!sourceRolePermissions.containsKey(apiName) || sourceRolePermissions.get(apiName) == Permission.DENY)) {
484+
return false;
485+
}
486+
487+
rulesAlreadyCompared.add(apiName);
488+
489+
if (!permissionIsRegex) {
490+
break;
491+
}
492+
}
493+
}
494+
495+
return true;
496+
}
497+
498+
/**
499+
* Given a role ID, returns a {@link Map} containing the API name as the key and the {@link Permission} for the API as the value.
500+
*
501+
* @param roleId ID from role.
502+
*/
503+
public Map<String, Permission> getRoleRulesAndPermissions(Long roleId) {
504+
Map<String, Permission> roleRulesAndPermissions = new HashMap<>();
505+
506+
for (RolePermission rolePermission : findAllPermissionsBy(roleId)) {
507+
boolean permissionIsRegex = rolePermission.getRule().getRuleString().contains("*");
508+
509+
for (String apiName : accountManager.getApiNameList()) {
510+
if (!rolePermission.getRule().matches(apiName)) {
511+
continue;
512+
}
513+
514+
if (!roleRulesAndPermissions.containsKey(apiName)) {
515+
roleRulesAndPermissions.put(apiName, rolePermission.getPermission());
516+
}
517+
518+
if (!permissionIsRegex) {
519+
break;
520+
}
521+
}
522+
}
523+
return roleRulesAndPermissions;
524+
}
525+
465526
@Override
466527
public List<Role> findRolesByType(RoleType roleType) {
467528
return findRolesByType(roleType, null, null).first();
@@ -479,14 +540,14 @@ public Pair<List<Role>, Integer> findRolesByType(RoleType roleType, Long startIn
479540
@Override
480541
public List<Role> listRoles() {
481542
List<? extends Role> roles = roleDao.listAll();
482-
removeRootAdminRolesIfNeeded(roles);
543+
removeRolesIfNeeded(roles);
483544
return ListUtils.toListOfInterface(roles);
484545
}
485546

486547
@Override
487548
public Pair<List<Role>, Integer> listRoles(Long startIndex, Long limit) {
488549
Pair<List<RoleVO>, Integer> data = roleDao.listAllRoles(startIndex, limit, isCallerRootAdmin());
489-
int removed = removeRootAdminRolesIfNeeded(data.first());
550+
int removed = removeRolesIfNeeded(data.first());
490551
return new Pair<List<Role>,Integer>(ListUtils.toListOfInterface(data.first()), Integer.valueOf(data.second() - removed));
491552
}
492553

server/src/test/java/com/cloud/user/MockAccountManagerImpl.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,4 +479,8 @@ public ConfigKey<?>[] getConfigKeys() {
479479
return null;
480480
}
481481

482+
@Override
483+
public List<String> getApiNameList() {
484+
return null;
485+
}
482486
}

0 commit comments

Comments
 (0)