cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mc...@apache.org
Subject [2/2] git commit: updated refs/heads/master to 51cb0f9
Date Fri, 09 May 2014 00:56:32 GMT
CLOUDSTACK-6600:IAM Security checker needs to have cache to improve
checkAccess performance.


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/218158b9
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/218158b9
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/218158b9

Branch: refs/heads/master
Commit: 218158b9ab5f2d113287874d0ba3dd0f87fc2f2c
Parents: f21e527
Author: Min Chen <min.chen@citrix.com>
Authored: Thu May 8 15:37:05 2014 -0700
Committer: Min Chen <min.chen@citrix.com>
Committed: Thu May 8 17:56:20 2014 -0700

----------------------------------------------------------------------
 .../iam/RoleBasedEntityAccessChecker.java       | 98 ++++++++++++++++++--
 services/iam/server/pom.xml                     |  4 +
 .../core/spring-iam-server-context.xml          |  9 +-
 .../apache/cloudstack/iam/api/IAMService.java   |  7 ++
 .../cloudstack/iam/server/IAMServiceImpl.java   | 88 +++++++++++++++++-
 5 files changed, 193 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/218158b9/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java
----------------------------------------------------------------------
diff --git a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java
b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java
index cc29ab5..b384d7c 100644
--- a/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java
+++ b/services/iam/plugin/src/org/apache/cloudstack/iam/RoleBasedEntityAccessChecker.java
@@ -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;
+        }
+
+    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/218158b9/services/iam/server/pom.xml
----------------------------------------------------------------------
diff --git a/services/iam/server/pom.xml b/services/iam/server/pom.xml
index 8b05111..42daa33 100644
--- a/services/iam/server/pom.xml
+++ b/services/iam/server/pom.xml
@@ -32,6 +32,10 @@
       <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>
       <version>${project.version}</version>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/218158b9/services/iam/server/resources/META-INF/cloudstack/core/spring-iam-server-context.xml
----------------------------------------------------------------------
diff --git a/services/iam/server/resources/META-INF/cloudstack/core/spring-iam-server-context.xml
b/services/iam/server/resources/META-INF/cloudstack/core/spring-iam-server-context.xml
index c9f383f..4994a34 100644
--- a/services/iam/server/resources/META-INF/cloudstack/core/spring-iam-server-context.xml
+++ b/services/iam/server/resources/META-INF/cloudstack/core/spring-iam-server-context.xml
@@ -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>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/218158b9/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java
----------------------------------------------------------------------
diff --git a/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java b/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java
index c396fa9..0c0e9c6 100644
--- a/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java
+++ b/services/iam/server/src/org/apache/cloudstack/iam/api/IAMService.java
@@ -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();
+
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/218158b9/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java
----------------------------------------------------------------------
diff --git a/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java
b/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java
index d6a61a1..33869c8 100644
--- a/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java
+++ b/services/iam/server/src/org/apache/cloudstack/iam/server/IAMServiceImpl.java
@@ -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;
     }
 


Mime
View raw message