Skip to content

Commit afa8f4b

Browse files
bernardodemarcobernardodemarco
committed
start implementation of implicit rules
1 parent 136ea7b commit afa8f4b

File tree

15 files changed

+154
-97
lines changed

15 files changed

+154
-97
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/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -338,15 +338,16 @@ public List<Class<?>> getCommands() {
338338

339339
protected List<ApiDiscoveryResponse> listApisForKeyPair(String apiKey, String apiName, Account account, User user, Role role, List<String> apisAllowed) {
340340
ApiKeyPair keyPair = accountService.getKeyPairByApiKey(apiKey);
341-
List<RolePermissionEntity> rolePermissionEntities = apiKeyPairService.findAllPermissionsByKeyPairId(keyPair.getId(), account.getRoleId()).stream()
342-
.map(apiKeyPairPermission -> (RolePermissionEntity) apiKeyPairPermission).collect(Collectors.toList());
341+
List<RolePermissionEntity> keyPairPermissions = apiKeyPairService.findAllPermissionsByKeyPairId(keyPair.getId(), account.getRoleId()).stream()
342+
.map(apiKeyPairPermission -> (RolePermissionEntity) apiKeyPairPermission)
343+
.collect(Collectors.toList());
343344

344345
List<String> filteredApis = new ArrayList<>();
345-
if (apiName != null && isApiAllowedForKey(rolePermissionEntities, apiName)) {
346+
if (apiName != null && isApiAllowedForKey(keyPairPermissions, apiName)) {
346347
filteredApis = List.of(apiName);
347348
} else {
348349
for (String api : apisAllowed) {
349-
if (isApiAllowedForKey(rolePermissionEntities, api)) {
350+
if (isApiAllowedForKey(keyPairPermissions, api)) {
350351
filteredApis.add(api);
351352
}
352353
}

‎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: 2 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) {
@@ -1141,9 +1141,8 @@ public boolean verifyRequest(final Map<String, Object[]> requestParameters, fina
11411141
CallContext.register(user, account);
11421142

11431143
List<ApiKeyPairPermission> keyPairPermissions = keyPairManager.findAllPermissionsByKeyPairId(keyPair.getId(), account.getRoleId());
1144-
11451144
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()));
1145+
logger.info("API accessed through API KeyPair. API Key: [{}]", keyPair.getApiKey());
11471146
return true;
11481147
}
11491148
} catch (final ServerApiException ex) {

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

Lines changed: 26 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -3168,17 +3168,17 @@ public ListResponse<ApiKeyPairResponse> listKeys(ListUserKeysCmd cmd) {
31683168
List<ApiKeyPairResponse> responses = new ArrayList<>();
31693169

31703170
if (cmd.getKeyId() != null || cmd.getApiKeyFilter() != null) {
3171-
fetchOnlyOneKeypair(responses, cmd);
3171+
fetchOnlyOneKeyPair(responses, cmd);
31723172
finalResponse.setResponses(responses);
31733173
return finalResponse;
31743174
}
31753175

3176-
Integer total = fetchMultipleKeypairs(responses, cmd);
3176+
Integer total = fetchMultipleKeyPairs(responses, cmd);
31773177
finalResponse.setResponses(responses, total);
31783178
return finalResponse;
31793179
}
31803180

3181-
private void fetchOnlyOneKeypair(List<ApiKeyPairResponse> responses, ListUserKeysCmd cmd) {
3181+
private void fetchOnlyOneKeyPair(List<ApiKeyPairResponse> responses, ListUserKeysCmd cmd) {
31823182
ApiKeyPair keyPair;
31833183
if (cmd.getKeyId() != null) {
31843184
keyPair = _accountService.getKeyPairById(cmd.getKeyId());
@@ -3188,14 +3188,13 @@ private void fetchOnlyOneKeypair(List<ApiKeyPairResponse> responses, ListUserKey
31883188

31893189
validateKeyPairIsNotNull(keyPair);
31903190
validateAccessingKeyPairPermissionsIsSupersetOfAccessedKeyPair(keyPair, cmd);
3191-
31923191
_accountService.validateCallingUserHasAccessToDesiredUser(keyPair.getUserId());
3193-
markExpiredKeysWithStateExpired(keyPair);
3192+
removeApiKeyPairIfExpired(keyPair);
31943193

31953194
addKeypairResponse(keyPair, responses, cmd);
31963195
}
31973196

3198-
private Integer fetchMultipleKeypairs(List<ApiKeyPairResponse> responses, ListUserKeysCmd cmd) {
3197+
private Integer fetchMultipleKeyPairs(List<ApiKeyPairResponse> responses, ListUserKeysCmd cmd) {
31993198
List<Long> users;
32003199
if (cmd.getUserId() != null) {
32013200
_accountService.validateCallingUserHasAccessToDesiredUser(cmd.getUserId());
@@ -3210,7 +3209,7 @@ private Integer fetchMultipleKeypairs(List<ApiKeyPairResponse> responses, ListUs
32103209
.filter(keyPair -> isAccessingKeypairSuperset(keyPair, cmd))
32113210
.forEach(keyPair -> {
32123211
addKeypairResponse(keyPair, responses, cmd);
3213-
markExpiredKeysWithStateExpired(keyPair);
3212+
removeApiKeyPairIfExpired(keyPair);
32143213
});
32153214

32163215
return keyPairs.second();
@@ -3281,7 +3280,7 @@ private Boolean isApiKeySupersetOfPermission(List<RolePermissionEntity> baseKeyP
32813280
return roleService.roleHasPermission(apiNameToBaseKeyPermissions, comparedPermissions);
32823281
}
32833282

3284-
private void markExpiredKeysWithStateExpired(ApiKeyPair apiKeyPair) {
3283+
private void removeApiKeyPairIfExpired(ApiKeyPair apiKeyPair) {
32853284
if (apiKeyPair.hasEndDatePassed()) {
32863285
internalDeleteApiKey(apiKeyPair);
32873286
}
@@ -3406,7 +3405,7 @@ public ApiKeyPair createApiKeyAndSecretKey(RegisterUserKeysCmd cmd) {
34063405
Account caller = getCurrentCallingAccount();
34073406
User user = _userDao.findById(cmd.getUserId());
34083407
if (user == null) {
3409-
throw new InvalidParameterValueException(String.format("Unable to find user by id: %d", cmd.getUserId()));
3408+
throw new InvalidParameterValueException(String.format("Unable to find user by ID: %d", cmd.getUserId()));
34103409
}
34113410

34123411
final String name = cmd.getName();
@@ -3415,32 +3414,29 @@ public ApiKeyPair createApiKeyAndSecretKey(RegisterUserKeysCmd cmd) {
34153414
final Date startDate = cmd.getStartDate();
34163415
final Date endDate = cmd.getEndDate();
34173416
final List<Map<String, Object>> rules = cmd.getRules();
3418-
final RegisterUserKeysCmd registerCmd = cmd;
34193417

34203418
Account account = _accountDao.findById(user.getAccountId());
34213419
checkAccess(caller, null, true, account);
34223420
verifyCallerPrivilegeForUserOrAccountOperations(user);
34233421

3424-
// don't allow baremetal or system user
34253422
if (BaremetalUtils.BAREMETAL_SYSTEM_ACCOUNT_NAME.equals(user.getUsername()) || user.getId() == User.UID_SYSTEM) {
3426-
throw new PermissionDeniedException(String.format("User id: [%s] is system account, update is not allowed.", user.getId()));
3423+
throw new PermissionDeniedException(String.format("User ID: [%s] is a system account and, thus, the operation is not allowed.", user.getId()));
34273424
}
34283425

34293426
Date now = DateTime.now().toDate();
34303427

34313428
if (endDate != null && endDate.compareTo(now) <= 0) {
3432-
throw new InvalidParameterValueException("Keypair cannot be created with expired date, please input a date on the future.");
3429+
throw new InvalidParameterValueException("Keypair cannot be created with expired date, please input a date in the future.");
34333430
}
34343431
if (ObjectUtils.allNotNull(startDate, endDate) && startDate.compareTo(endDate) > -1) {
34353432
throw new InvalidParameterValueException("Please specify an end date that is after the start date.");
34363433
}
34373434

3438-
// generate both an api key and a secret key, return the keypair to the user
34393435
final ApiKeyPairVO newApiKeyPair = new ApiKeyPairVO(name, userId, description, startDate, endDate, account);
34403436
return Transaction.execute((TransactionCallback<ApiKeyPairVO>) status -> {
34413437
createUserApiKey(userId, newApiKeyPair);
34423438
createUserSecretKey(userId, newApiKeyPair);
3443-
return validateAndPersistKeyPairAndPermissions(account, newApiKeyPair, rules, registerCmd);
3439+
return validateAndPersistKeyPairAndPermissions(account, newApiKeyPair, rules, cmd);
34443440
});
34453441
}
34463442

@@ -3486,28 +3482,22 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
34863482
@DB
34873483
private ApiKeyPairVO validateAndPersistKeyPairAndPermissions(Account account, ApiKeyPairVO newApiKeyPair,
34883484
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
34903485
String accessingApiKey = getAccessingApiKey(cmd);
34913486
final Role accountRole = roleService.findRole(account.getRoleId());
3487+
List<RolePermissionEntity> allPermissions = accessingApiKey == null ?
3488+
roleService.findAllRolePermissionsEntityBy(accountRole.getId(), true) : getAllKeypairPermissions(accessingApiKey);
34923489

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-
}
3490+
List<RolePermissionEntity> permissions = new ArrayList<>();
3491+
for (Map<String, Object> ruleDetail : rules) {
3492+
String rule = ruleDetail.get(ApiConstants.RULE).toString();
3493+
RolePermission.Permission rulePermission = (RolePermission.Permission) ruleDetail.get(ApiConstants.PERMISSION);
3494+
String ruleDescription = (String) ruleDetail.get(ApiConstants.DESCRIPTION);
3495+
permissions.add(new ApiKeyPairPermissionVO(0, rule, rulePermission, ruleDescription));
3496+
}
3497+
3498+
if (!isApiKeySupersetOfPermission(allPermissions, permissions)) {
3499+
throw new InvalidParameterValueException(String.format("The keypair being created has a bigger set of permissions than the account [%s] that owns it. This is " +
3500+
"not allowed.", account.getUuid()));
35113501
}
35123502

35133503
ApiKeyPairVO savedApiKeyPair = apiKeyPairDao.persist(newApiKeyPair);
@@ -3519,16 +3509,6 @@ private ApiKeyPairVO validateAndPersistKeyPairAndPermissions(Account account, Ap
35193509
return savedApiKeyPair;
35203510
}
35213511

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-
35323512
/**
35333513
* Gets all API keypair permissions for the given apiKey
35343514
*/
@@ -3539,7 +3519,8 @@ private List<RolePermissionEntity> getAllKeypairPermissions(String apiKey) {
35393519
ApiKeyPair apiKeyPair = keyPairManager.findByApiKey(apiKey);
35403520
Account account = _accountDao.findById(apiKeyPair.getAccountId());
35413521
List<ApiKeyPairPermission> allApiKeyRolePermissions = keyPairManager.findAllPermissionsByKeyPairId(apiKeyPair.getId(), account.getRoleId());
3542-
return allApiKeyRolePermissions.stream().map(permission -> (RolePermissionEntity) permission)
3522+
return allApiKeyRolePermissions.stream()
3523+
.map(permission -> (RolePermissionEntity) permission)
35433524
.collect(Collectors.toList());
35443525
}
35453526

0 commit comments

Comments
 (0)