Skip to content

Commit 9f38fd3

Browse files
start implementation of implicit rules
1 parent 136ea7b commit 9f38fd3

File tree

13 files changed

+123
-72
lines changed

13 files changed

+123
-72
lines changed

api/src/main/java/org/apache/cloudstack/acl/APIChecker.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,5 @@ public interface APIChecker extends Adapter {
4444
*/
4545
List<String> getApisAllowedToUser(Role role, User user, List<String> apiNames) throws PermissionDeniedException;
4646
boolean isEnabled();
47+
List<RolePermissionEntity> getImplicitRolePermissions(RoleType roleType);
4748
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public interface RoleService {
104104

105105
List<RolePermission> findAllPermissionsBy(Long roleId);
106106

107-
List<RolePermissionEntity> findAllRolePermissionsEntityBy(Long roleId);
107+
List<RolePermissionEntity> findAllRolePermissionsEntityBy(Long roleId, boolean considerImplicitRules);
108108

109109
Permission getRolePermission(String permission);
110110

engine/schema/src/main/java/org/apache/cloudstack/acl/RolePermissionBaseVO.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ public class RolePermissionBaseVO implements RolePermissionEntity {
5050

5151
public RolePermissionBaseVO() { this.uuid = UUID.randomUUID().toString(); }
5252

53+
public RolePermissionBaseVO(final String rule, final Permission permission) {
54+
this();
55+
this.rule = rule;
56+
this.permission = permission;
57+
}
58+
5359
public RolePermissionBaseVO(final String rule, final Permission permission, final String description) {
5460
this();
5561
this.rule = rule;

plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements API
5050
private RoleService roleService;
5151

5252
private List<PluggableService> services;
53-
private Map<RoleType, Set<String>> annotationRoleBasedApisMap = new HashMap<RoleType, Set<String>>();
53+
private Map<RoleType, Set<String>> annotationRoleBasedApisMap = new HashMap<>();
5454

5555
private LazyCache<Long, Account> accountCache;
5656
private LazyCache<Long, Pair<Role, List<RolePermission>>> rolePermissionsCache;
@@ -59,7 +59,7 @@ public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements API
5959
protected DynamicRoleBasedAPIAccessChecker() {
6060
super();
6161
for (RoleType roleType : RoleType.values()) {
62-
annotationRoleBasedApisMap.put(roleType, new HashSet<String>());
62+
annotationRoleBasedApisMap.put(roleType, new HashSet<>());
6363
}
6464
}
6565

@@ -206,6 +206,14 @@ public List<RolePermissionEntity> defineNewKeypairRules(Role accountRole, ApiKey
206206
return allPermissions;
207207
}
208208

209+
@Override
210+
public List<RolePermissionEntity> getImplicitRolePermissions(RoleType roleType) {
211+
return annotationRoleBasedApisMap.get(roleType)
212+
.stream()
213+
.map(implicitApi -> new RolePermissionBaseVO(implicitApi, Permission.ALLOW))
214+
.collect(Collectors.toList());
215+
}
216+
209217
/**
210218
* Only one strategy should be used between StaticRoleBasedAPIAccessChecker and DynamicRoleBasedAPIAccessChecker
211219
* Default behavior is to use the Dynamic version. The StaticRoleBasedAPIAccessChecker is the legacy version.

plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,11 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
183183
return true;
184184
}
185185

186+
@Override
187+
public List<RolePermissionEntity> getImplicitRolePermissions(RoleType roleType) {
188+
return List.of();
189+
}
190+
186191
@Override
187192
public boolean start() {
188193
return super.start();

plugins/acl/static-role-based/src/main/java/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,11 @@ public boolean start() {
164164
return super.start();
165165
}
166166

167+
@Override
168+
public List<RolePermissionEntity> getImplicitRolePermissions(RoleType roleType) {
169+
return List.of();
170+
}
171+
167172
private void processMapping(Map<String, String> configMap) {
168173
for (Map.Entry<String, String> entry : configMap.entrySet()) {
169174
String apiName = entry.getKey();

plugins/api/rate-limit/src/main/java/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import net.sf.ehcache.CacheManager;
2828

2929
import org.apache.cloudstack.acl.Role;
30+
import org.apache.cloudstack.acl.RolePermissionEntity;
31+
import org.apache.cloudstack.acl.RoleType;
3032
import org.apache.cloudstack.acl.apikeypair.ApiKeyPairPermission;
3133
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
3234
import org.springframework.stereotype.Component;
@@ -208,6 +210,11 @@ public boolean hasApiRateLimitBeenExceeded(Long accountId) {
208210
return true;
209211
}
210212

213+
@Override
214+
public List<RolePermissionEntity> getImplicitRolePermissions(RoleType roleType) {
215+
return List.of();
216+
}
217+
211218
@Override
212219
public boolean isEnabled() {
213220
if (!enabled) {

server/src/main/java/com/cloud/api/ApiServer.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,7 +1106,7 @@ public boolean verifyRequest(final Map<String, Object[]> requestParameters, fina
11061106
}
11071107

11081108
if (!commandAvailable(remoteAddress, commandName, user)) {
1109-
return false;
1109+
return false;// remove this one, not required, does not need to check against rules role here
11101110
}
11111111

11121112
if (keyPair.getRemoved() != null) {
@@ -1140,10 +1140,10 @@ public boolean verifyRequest(final Map<String, Object[]> requestParameters, fina
11401140
}
11411141
CallContext.register(user, account);
11421142

1143-
List<ApiKeyPairPermission> keyPairPermissions = keyPairManager.findAllPermissionsByKeyPairId(keyPair.getId(), account.getRoleId());
1143+
List<ApiKeyPairPermission> keyPairPermissions = keyPairManager.findAllPermissionsByKeyPairId(keyPair.getId(), account.getRoleId());// so deveria pegar as permissions do keypair explicitas ouuuu busca pelas implicitas aqui
11441144

11451145
if (commandAvailable(remoteAddress, commandName, user, keyPairPermissions.toArray(new ApiKeyPairPermission[0]))) {
1146-
logger.info(String.format("API accessed through API KeyPair. API Key: [%s]", keyPair.getApiKey()));
1146+
logger.info("API accessed through API KeyPair. API Key: [{}]", keyPair.getApiKey());
11471147
return true;
11481148
}
11491149
} catch (final ServerApiException ex) {

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

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3486,28 +3486,22 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
34863486
@DB
34873487
private ApiKeyPairVO validateAndPersistKeyPairAndPermissions(Account account, ApiKeyPairVO newApiKeyPair,
34883488
List<Map<String, Object>> rules, RegisterUserKeysCmd cmd) {
3489-
// this is only used to determine if we should use api key permissions or account permissions to base our new key on
34903489
String accessingApiKey = getAccessingApiKey(cmd);
34913490
final Role accountRole = roleService.findRole(account.getRoleId());
3491+
List<RolePermissionEntity> allPermissions = accessingApiKey == null ?
3492+
roleService.findAllRolePermissionsEntityBy(accountRole.getId(), true) : getAllKeypairPermissions(accessingApiKey);
3493+
List<RolePermissionEntity> permissions = new ArrayList<>();
34923494

3493-
List<RolePermissionEntity> allPermissions = accessingApiKey == null ? getAllAccountRolePermissions(accountRole) : getAllKeypairPermissions(accessingApiKey);
3494-
List<RolePermissionEntity> permissions;
3495-
if (CollectionUtils.isEmpty(rules)) {
3496-
permissions = allPermissions.stream()
3497-
.map(permission -> new ApiKeyPairPermissionVO(0, permission.getRule().toString(), permission.getPermission(), permission.getDescription()))
3498-
.collect(Collectors.toList());
3499-
} else {
3500-
permissions = new ArrayList<>();
3501-
for (Map<String, Object> ruleDetail : rules) {
3502-
String rule = ruleDetail.get(ApiConstants.RULE).toString();
3503-
RolePermission.Permission rulePermission = (RolePermission.Permission) ruleDetail.get(ApiConstants.PERMISSION);
3504-
String ruleDescription = (String) ruleDetail.get(ApiConstants.DESCRIPTION);
3505-
permissions.add(new ApiKeyPairPermissionVO(0, rule, rulePermission, ruleDescription));
3506-
}
3507-
if (!isApiKeySupersetOfPermission(allPermissions, permissions)) {
3508-
throw new InvalidParameterValueException(String.format("The keypair being created has a bigger set of permissions than the account [%s] that owns it. This is " +
3509-
"not allowed.", account.getUuid()));
3510-
}
3495+
for (Map<String, Object> ruleDetail : rules) {
3496+
String rule = ruleDetail.get(ApiConstants.RULE).toString();
3497+
RolePermission.Permission rulePermission = (RolePermission.Permission) ruleDetail.get(ApiConstants.PERMISSION);
3498+
String ruleDescription = (String) ruleDetail.get(ApiConstants.DESCRIPTION);
3499+
permissions.add(new ApiKeyPairPermissionVO(0, rule, rulePermission, ruleDescription));
3500+
}
3501+
3502+
if (!isApiKeySupersetOfPermission(allPermissions, permissions)) {
3503+
throw new InvalidParameterValueException(String.format("The keypair being created has a bigger set of permissions than the account [%s] that owns it. This is " +
3504+
"not allowed.", account.getUuid()));
35113505
}
35123506

35133507
ApiKeyPairVO savedApiKeyPair = apiKeyPairDao.persist(newApiKeyPair);
@@ -3519,16 +3513,6 @@ private ApiKeyPairVO validateAndPersistKeyPairAndPermissions(Account account, Ap
35193513
return savedApiKeyPair;
35203514
}
35213515

3522-
/**
3523-
* Gets all account role permissions
3524-
* @param accountRole base account role of the user.
3525-
*/
3526-
private List<RolePermissionEntity> getAllAccountRolePermissions(Role accountRole) {
3527-
List<RolePermission> allAccountRolePermissions = roleService.findAllPermissionsBy(accountRole.getId());
3528-
return allAccountRolePermissions.stream().map(permission -> (RolePermissionEntity) permission)
3529-
.collect(Collectors.toList());
3530-
}
3531-
35323516
/**
35333517
* Gets all API keypair permissions for the given apiKey
35343518
*/

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

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818

1919
import com.cloud.utils.component.ManagerBase;
2020

21+
import java.util.HashSet;
2122
import java.util.Map;
23+
import java.util.Set;
2224
import java.util.stream.Collectors;
2325
import org.apache.cloudstack.acl.apikeypair.ApiKeyPairService;
2426
import org.apache.cloudstack.acl.apikeypair.ApiKeyPair;
@@ -40,36 +42,39 @@ public class ApiKeyPairManagerImpl extends ManagerBase implements ApiKeyPairServ
4042

4143
@Override
4244
public List<ApiKeyPairPermission> findAllPermissionsByKeyPairId(Long apiKeyPairId, Long roleId) {
43-
List<ApiKeyPairPermissionVO> allPermissions = apiKeyPairPermissionsDao.findAllByKeyPairIdSorted(apiKeyPairId);
44-
List<RolePermissionEntity> rolePermissionsEntity = roleService.findAllRolePermissionsEntityBy(roleId);
45+
List<ApiKeyPairPermissionVO> keyPairPermissions = apiKeyPairPermissionsDao.findAllByKeyPairIdSorted(apiKeyPairId);
46+
List<RolePermissionEntity> rolePermissionsEntity = roleService.findAllRolePermissionsEntityBy(roleId, true);
4547

46-
if (!CollectionUtils.isEmpty(allPermissions)) {
47-
List<RolePermissionEntity> keyPairPermissionsEntity = allPermissions.stream()
48-
.map(p -> (RolePermissionEntity) p).collect(Collectors.toList());
49-
50-
Map<String, RolePermissionEntity.Permission> rolePermissionInfo = roleService.getRoleRulesAndPermissions(rolePermissionsEntity);
48+
if (CollectionUtils.isEmpty(keyPairPermissions)) {
49+
return rolePermissionsEntity.stream().map(p -> {
50+
ApiKeyPairPermissionVO permission = new ApiKeyPairPermissionVO();
51+
permission.setRule(p.getRule().getRuleString());
52+
permission.setDescription(p.getDescription());
53+
permission.setPermission(p.getPermission());
54+
return permission;
55+
}).collect(Collectors.toList());
56+
}
5157

52-
if (roleService.roleHasPermission(rolePermissionInfo, keyPairPermissionsEntity)) {
53-
return allPermissions.stream().map(p -> (ApiKeyPairPermission) p).collect(Collectors.toList());
54-
}
58+
List<RolePermissionEntity> keyPairPermissionsEntity = keyPairPermissions.stream()
59+
.map(p -> (RolePermissionEntity) p).collect(Collectors.toList());
60+
Map<String, RolePermissionEntity.Permission> rolePermissionInfo = roleService.getRoleRulesAndPermissions(rolePermissionsEntity);
61+
if (roleService.roleHasPermission(rolePermissionInfo, keyPairPermissionsEntity)) {
62+
return keyPairPermissions.stream().map(p -> (ApiKeyPairPermission) p).collect(Collectors.toList());
63+
}
5564

56-
Map<String, RolePermissionEntity.Permission> keyPairPermissionInfo = roleService.getRoleRulesAndPermissions(keyPairPermissionsEntity);
57-
if (!roleService.roleHasPermission(keyPairPermissionInfo, rolePermissionsEntity)) {
58-
for (RolePermissionEntity rolePermission : keyPairPermissionsEntity) {
59-
if (rolePermission.getPermission() == RolePermissionEntity.Permission.DENY && !rolePermissionsEntity.contains(rolePermission)) {
60-
rolePermissionsEntity.add(0, rolePermission);
61-
}
62-
}
65+
Set<String> rulesToBeDeniedForTheKeyPair = new HashSet<>();
66+
Map<String, RolePermissionEntity.Permission> keyPairPermissionInfo = roleService.getRoleRulesAndPermissions(keyPairPermissionsEntity);
67+
for (RolePermissionEntity rolePermission : rolePermissionsEntity) {
68+
boolean isPermissionAllowedByTheKeyPair = keyPairPermissionInfo.containsKey(rolePermission.getRule().getRuleString())
69+
&& RolePermissionEntity.Permission.ALLOW == keyPairPermissionInfo.get(rolePermission.getRule().getRuleString());
70+
if (rolePermission.getPermission() == RolePermissionEntity.Permission.DENY && isPermissionAllowedByTheKeyPair) {
71+
rulesToBeDeniedForTheKeyPair.add(rolePermission.getRule().getRuleString());
6372
}
6473
}
65-
66-
return rolePermissionsEntity.stream().map(p -> {
67-
ApiKeyPairPermissionVO permission = new ApiKeyPairPermissionVO();
68-
permission.setRule(p.getRule().getRuleString());
69-
permission.setDescription(p.getDescription());
70-
permission.setPermission(p.getPermission());
71-
return permission;
72-
}).collect(Collectors.toList());
74+
return keyPairPermissions.stream()
75+
.map(permission -> (ApiKeyPairPermission) permission)
76+
.filter(permission -> !rulesToBeDeniedForTheKeyPair.contains(permission.getRule().getRuleString()))
77+
.collect(Collectors.toList());
7378
}
7479

7580
@Override

0 commit comments

Comments
 (0)