mirror of https://github.com/apache/cloudstack.git
CLOUDSTACK-6600:IAM Security checker needs to have cache to improve
checkAccess performance.
This commit is contained in:
parent
f21e527923
commit
218158b9ab
|
|
@ -59,6 +59,22 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur
|
|||
return checkAccess(caller, entity, accessType, null);
|
||||
}
|
||||
|
||||
private String buildAccessCacheKey(Account caller, ControlledEntity entity, AccessType accessType, String action) {
|
||||
StringBuffer key = new StringBuffer();
|
||||
key.append(caller.getAccountId());
|
||||
key.append("-");
|
||||
String entityType = null;
|
||||
if (entity != null && entity.getEntityType() != null) {
|
||||
entityType = entity.getEntityType().getSimpleName();
|
||||
}
|
||||
key.append(entityType != null ? entityType : "null");
|
||||
key.append("-");
|
||||
key.append(accessType != null ? accessType.toString() : "null");
|
||||
key.append("-");
|
||||
key.append(action != null ? action : "null");
|
||||
return key.toString();
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean checkAccess(Account caller, ControlledEntity entity, AccessType accessType, String action)
|
||||
throws PermissionDeniedException {
|
||||
|
|
@ -66,24 +82,46 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur
|
|||
if (caller == null) {
|
||||
throw new InvalidParameterValueException("Caller cannot be passed as NULL to IAM!");
|
||||
}
|
||||
|
||||
if (entity == null && action == null) {
|
||||
throw new InvalidParameterValueException("Entity and action cannot be both NULL in checkAccess!");
|
||||
}
|
||||
|
||||
// check IAM cache first
|
||||
String accessKey = buildAccessCacheKey(caller, entity, accessType, action);
|
||||
CheckAccessResult allowDeny = (CheckAccessResult)_iamSrv.getFromIAMCache(accessKey);
|
||||
if (allowDeny != null) {
|
||||
s_logger.debug("IAM access check for " + accessKey + " from cache");
|
||||
if (allowDeny.isAllow()) {
|
||||
return true;
|
||||
} else {
|
||||
if (allowDeny.getDenyMsg() != null) {
|
||||
throw new PermissionDeniedException(allowDeny.getDenyMsg());
|
||||
} else {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (entity == null && action != null) {
|
||||
// check if caller can do this action
|
||||
List<IAMPolicy> policies = _iamSrv.listIAMPolicies(caller.getAccountId());
|
||||
|
||||
boolean isAllowed = _iamSrv.isActionAllowedForPolicies(action, policies);
|
||||
if (!isAllowed) {
|
||||
throw new PermissionDeniedException("The action '" + action + "' not allowed for account " + caller);
|
||||
String msg = "The action '" + action + "' not allowed for account " + caller;
|
||||
_iamSrv.addToIAMCache(accessKey, new CheckAccessResult(msg));
|
||||
throw new PermissionDeniedException(msg);
|
||||
}
|
||||
_iamSrv.addToIAMCache(accessKey, new CheckAccessResult(true));
|
||||
return true;
|
||||
}
|
||||
|
||||
if (entity == null) {
|
||||
throw new InvalidParameterValueException("Entity and action cannot be both NULL in checkAccess!");
|
||||
}
|
||||
|
||||
// if a Project entity, skip
|
||||
Account entityAccount = _accountService.getAccount(entity.getAccountId());
|
||||
if (entityAccount != null && entityAccount.getType() == Account.ACCOUNT_TYPE_PROJECT) {
|
||||
_iamSrv.addToIAMCache(accessKey, new CheckAccessResult(false));
|
||||
return false;
|
||||
}
|
||||
|
||||
|
|
@ -96,8 +134,8 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur
|
|||
accessType = AccessType.UseEntry;
|
||||
}
|
||||
|
||||
// get all Policies of this caller w.r.t the entity
|
||||
List<IAMPolicy> policies = getEffectivePolicies(caller, entity);
|
||||
// get all Policies of this caller by considering recursive domain group policy
|
||||
List<IAMPolicy> policies = getEffectivePolicies(caller);
|
||||
HashMap<IAMPolicy, Boolean> policyPermissionMap = new HashMap<IAMPolicy, Boolean>();
|
||||
|
||||
for (IAMPolicy policy : policies) {
|
||||
|
|
@ -136,6 +174,7 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur
|
|||
}
|
||||
}
|
||||
if (policyPermissionMap.containsKey(policy) && policyPermissionMap.get(policy)) {
|
||||
_iamSrv.addToIAMCache(accessKey, new CheckAccessResult(true));
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
|
@ -143,13 +182,16 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur
|
|||
if (!policies.isEmpty()) { // Since we reach this point, none of the
|
||||
// roles granted access
|
||||
|
||||
String msg = "Account " + caller + " does not have permission to access resource " + entity
|
||||
+ " for access type: " + accessType;
|
||||
if (s_logger.isDebugEnabled()) {
|
||||
s_logger.debug("Account " + caller + " does not have permission to access resource " + entity
|
||||
+ " for access type: " + accessType);
|
||||
s_logger.debug(msg);
|
||||
}
|
||||
throw new PermissionDeniedException(caller + " does not have permission to access resource " + entity);
|
||||
_iamSrv.addToIAMCache(accessKey, new CheckAccessResult(msg));
|
||||
throw new PermissionDeniedException(msg);
|
||||
}
|
||||
|
||||
_iamSrv.addToIAMCache(accessKey, new CheckAccessResult(false));
|
||||
return false;
|
||||
}
|
||||
|
||||
|
|
@ -233,7 +275,7 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur
|
|||
return false;
|
||||
}
|
||||
|
||||
private List<IAMPolicy> getEffectivePolicies(Account caller, ControlledEntity entity) {
|
||||
private List<IAMPolicy> getEffectivePolicies(Account caller) {
|
||||
|
||||
List<IAMPolicy> policies = _iamSrv.listIAMPolicies(caller.getId());
|
||||
|
||||
|
|
@ -248,4 +290,40 @@ public class RoleBasedEntityAccessChecker extends DomainChecker implements Secur
|
|||
|
||||
return policies;
|
||||
}
|
||||
|
||||
private class CheckAccessResult {
|
||||
boolean allow;
|
||||
String denyMsg;
|
||||
|
||||
public CheckAccessResult(boolean allow) {
|
||||
this(allow, null);
|
||||
}
|
||||
|
||||
public CheckAccessResult(String msg) {
|
||||
this(false, msg);
|
||||
}
|
||||
|
||||
public CheckAccessResult(boolean allow, String msg) {
|
||||
allow = allow;
|
||||
denyMsg = msg;
|
||||
}
|
||||
|
||||
public boolean isAllow() {
|
||||
return allow;
|
||||
}
|
||||
|
||||
public void setAllow(boolean allow) {
|
||||
this.allow = allow;
|
||||
}
|
||||
|
||||
|
||||
public String getDenyMsg() {
|
||||
return denyMsg;
|
||||
}
|
||||
|
||||
public void setDenyMsg(String denyMsg) {
|
||||
this.denyMsg = denyMsg;
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,6 +31,10 @@
|
|||
<groupId>commons-io</groupId>
|
||||
<artifactId>commons-io</artifactId>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>net.sf.ehcache</groupId>
|
||||
<artifactId>ehcache-core</artifactId>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.apache.cloudstack</groupId>
|
||||
<artifactId>cloud-utils</artifactId>
|
||||
|
|
|
|||
|
|
@ -35,6 +35,13 @@
|
|||
<bean id="IAMAccountPolicyMapDaoImpl" class="org.apache.cloudstack.iam.server.dao.IAMAccountPolicyMapDaoImpl" />
|
||||
|
||||
|
||||
<bean id="IAMServiceImpl" class="org.apache.cloudstack.iam.server.IAMServiceImpl" />
|
||||
<bean id="IAMServiceImpl" class="org.apache.cloudstack.iam.server.IAMServiceImpl" >
|
||||
<property name="configParams">
|
||||
<map>
|
||||
<entry key="cache.size" value="5000" />
|
||||
<entry key="cache.time.to.live" value="300" />
|
||||
</map>
|
||||
</property>
|
||||
</bean>
|
||||
|
||||
</beans>
|
||||
|
|
|
|||
|
|
@ -90,4 +90,11 @@ public interface IAMService {
|
|||
|
||||
List<IAMPolicy> listRecursiveIAMPoliciesByGroup(long groupId);
|
||||
|
||||
/* Interface used for cache IAM checkAccess result */
|
||||
void addToIAMCache(Object accessKey, Object allowDeny);
|
||||
|
||||
Object getFromIAMCache(Object accessKey);
|
||||
|
||||
void invalidateIAMCache();
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -18,9 +18,15 @@ package org.apache.cloudstack.iam.server;
|
|||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import javax.ejb.Local;
|
||||
import javax.inject.Inject;
|
||||
import javax.naming.ConfigurationException;
|
||||
|
||||
import net.sf.ehcache.Cache;
|
||||
import net.sf.ehcache.CacheManager;
|
||||
import net.sf.ehcache.Element;
|
||||
|
||||
import org.apache.log4j.Logger;
|
||||
|
||||
|
|
@ -39,6 +45,7 @@ import org.apache.cloudstack.iam.server.dao.IAMPolicyDao;
|
|||
import org.apache.cloudstack.iam.server.dao.IAMPolicyPermissionDao;
|
||||
|
||||
import com.cloud.exception.InvalidParameterValueException;
|
||||
import com.cloud.utils.NumbersUtil;
|
||||
import com.cloud.utils.Pair;
|
||||
import com.cloud.utils.component.Manager;
|
||||
import com.cloud.utils.component.ManagerBase;
|
||||
|
|
@ -83,6 +90,62 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
@Inject
|
||||
IAMPolicyPermissionDao _policyPermissionDao;
|
||||
|
||||
private Cache _iamCache;
|
||||
|
||||
private void createIAMCache(final Map<String, ? extends Object> params) {
|
||||
final String value = (String)params.get("cache.size");
|
||||
|
||||
if (value != null) {
|
||||
final CacheManager cm = CacheManager.create();
|
||||
final int maxElements = NumbersUtil.parseInt(value, 0);
|
||||
final int live = NumbersUtil.parseInt((String)params.get("cache.time.to.live"), 300);
|
||||
final int idle = NumbersUtil.parseInt((String)params.get("cache.time.to.idle"), 300);
|
||||
_iamCache = new Cache(getName(), maxElements, false, live == -1, live == -1 ? Integer.MAX_VALUE : live, idle);
|
||||
cm.addCache(_iamCache);
|
||||
s_logger.info("IAM Cache created: " + _iamCache.toString());
|
||||
} else {
|
||||
_iamCache = null;
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void addToIAMCache(Object accessKey, Object allowDeny) {
|
||||
if (_iamCache != null) {
|
||||
try {
|
||||
s_logger.debug("Put IAM access check for " + accessKey + " in cache");
|
||||
_iamCache.put(new Element(accessKey, allowDeny));
|
||||
} catch (final Exception e) {
|
||||
s_logger.debug("Can't put " + accessKey + " to IAM cache", e);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void invalidateIAMCache() {
|
||||
//This may need to use event bus to publish to other MS, but event bus now is missing this functionality to handle PublishScope.GLOBAL
|
||||
if (_iamCache != null) {
|
||||
s_logger.debug("Invalidate IAM cache");
|
||||
_iamCache.removeAll();
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object getFromIAMCache(Object accessKey) {
|
||||
if (_iamCache != null) {
|
||||
final Element element = _iamCache.get(accessKey);
|
||||
return element == null ? null : element.getObjectValue();
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean configure(final String name, final Map<String, Object> params) throws ConfigurationException {
|
||||
boolean result = super.configure(name, params);
|
||||
// create IAM cache
|
||||
createIAMCache(params);
|
||||
return result;
|
||||
}
|
||||
|
||||
@DB
|
||||
@Override
|
||||
public IAMGroup createIAMGroup(String iamGroupName, String description, String path) {
|
||||
|
|
@ -112,7 +175,7 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
Transaction.execute(new TransactionCallbackNoReturn() {
|
||||
@Override
|
||||
public void doInTransactionWithoutResult(TransactionStatus status) {
|
||||
// remove this group related entry in acl_group_role_map
|
||||
// remove this group related entry in acl_group_policy_map
|
||||
List<IAMGroupPolicyMapVO> groupPolicyMap = _aclGroupPolicyMapDao.listByGroupId(grp.getId());
|
||||
if (groupPolicyMap != null) {
|
||||
for (IAMGroupPolicyMapVO gr : groupPolicyMap) {
|
||||
|
|
@ -133,6 +196,7 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
}
|
||||
});
|
||||
|
||||
invalidateIAMCache();
|
||||
return true;
|
||||
}
|
||||
|
||||
|
|
@ -185,6 +249,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
}
|
||||
}
|
||||
});
|
||||
|
||||
invalidateIAMCache();
|
||||
return group;
|
||||
}
|
||||
|
||||
|
|
@ -211,6 +277,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
}
|
||||
}
|
||||
});
|
||||
|
||||
invalidateIAMCache();
|
||||
return group;
|
||||
}
|
||||
|
||||
|
|
@ -346,7 +414,7 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
Transaction.execute(new TransactionCallbackNoReturn() {
|
||||
@Override
|
||||
public void doInTransactionWithoutResult(TransactionStatus status) {
|
||||
// remove this role related entry in acl_group_role_map
|
||||
// remove this policy related entry in acl_group_policy_map
|
||||
List<IAMGroupPolicyMapVO> groupPolicyMap = _aclGroupPolicyMapDao.listByPolicyId(policy.getId());
|
||||
if (groupPolicyMap != null) {
|
||||
for (IAMGroupPolicyMapVO gr : groupPolicyMap) {
|
||||
|
|
@ -375,6 +443,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
}
|
||||
});
|
||||
|
||||
invalidateIAMCache();
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
|
|
@ -537,6 +607,7 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
}
|
||||
});
|
||||
|
||||
invalidateIAMCache();
|
||||
return group;
|
||||
}
|
||||
|
||||
|
|
@ -569,6 +640,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
}
|
||||
}
|
||||
});
|
||||
|
||||
invalidateIAMCache();
|
||||
return group;
|
||||
}
|
||||
|
||||
|
|
@ -595,6 +668,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
}
|
||||
}
|
||||
});
|
||||
|
||||
invalidateIAMCache();
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
@ -618,6 +693,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
}
|
||||
}
|
||||
});
|
||||
|
||||
invalidateIAMCache();
|
||||
}
|
||||
|
||||
@DB
|
||||
|
|
@ -639,6 +716,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
recursive);
|
||||
_policyPermissionDao.persist(permit);
|
||||
}
|
||||
|
||||
invalidateIAMCache();
|
||||
return policy;
|
||||
|
||||
}
|
||||
|
|
@ -659,6 +738,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
// not removed yet
|
||||
_policyPermissionDao.remove(permit.getId());
|
||||
}
|
||||
|
||||
invalidateIAMCache();
|
||||
return policy;
|
||||
}
|
||||
|
||||
|
|
@ -681,6 +762,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
}
|
||||
}
|
||||
});
|
||||
|
||||
invalidateIAMCache();
|
||||
}
|
||||
|
||||
@DB
|
||||
|
|
@ -701,6 +784,7 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
|
|||
permissionSC.setParameters("policyId", iamPolicyId);
|
||||
_policyPermissionDao.expunge(permissionSC);
|
||||
|
||||
invalidateIAMCache();
|
||||
return policy;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue