Fix RoleBasedQuerySelector to handle new listAll semantics. If

listAll=true, show all resources that caller (or impersonater) has
ListEntry access type; otherwise, show all resources that caller (or
impersonater) has UseEntry access type.
This commit is contained in:
Min Chen 2014-04-04 18:44:32 -07:00
parent 7796128372
commit 5d59fc7f5a
8 changed files with 94 additions and 66 deletions

View File

@ -18,6 +18,8 @@ package org.apache.cloudstack.acl;
import java.util.List;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import com.cloud.user.Account;
import com.cloud.utils.component.Adapter;
@ -33,7 +35,7 @@ public interface QuerySelector extends Adapter {
* @param action action
* @return list of domain Ids granted to the caller account.
*/
List<Long> getAuthorizedDomains(Account caller, String action);
List<Long> getAuthorizedDomains(Account caller, String action, AccessType accessType);
/**
* List granted accounts for the caller, given a specific action.
@ -42,7 +44,7 @@ public interface QuerySelector extends Adapter {
* @param action action.
* @return list of domain Ids granted to the caller account.
*/
List<Long> getAuthorizedAccounts(Account caller, String action);
List<Long> getAuthorizedAccounts(Account caller, String action, AccessType accessType);
/**
@ -52,7 +54,7 @@ public interface QuerySelector extends Adapter {
* @param action action.
* @return list of domain Ids granted to the caller account.
*/
List<Long> getAuthorizedResources(Account caller, String action);
List<Long> getAuthorizedResources(Account caller, String action, AccessType accessType);
/**
* Check if this account is associated with a policy with scope of ALL
@ -60,7 +62,7 @@ public interface QuerySelector extends Adapter {
* @param action action.
* @return true if this account is attached with a policy for the given action of ALL scope.
*/
boolean isGrantedAll(Account caller, String action);
boolean isGrantedAll(Account caller, String action, AccessType accessType);
/**
* List of ACL group the given account belongs to

View File

@ -2165,6 +2165,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
List<Long> permittedResources, Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject, boolean listAll, boolean forProjectInvitation,
String action) {
Account owner = null; // for impersonation
Long domainId = domainIdRecursiveListProject.first();
if (id == null) {
// if id is specified, it will ignore all other parameters
@ -2178,7 +2179,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
checkAccess(caller, domain);
}
// specific account is specified
// specific account is specified, we need to impersonate that account instead of caller
if (accountName != null) {
if (projectId != null) {
throw new InvalidParameterValueException("Account and projectId can't be specified together");
@ -2197,14 +2198,10 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
if (userAccount != null) {
//check permissions
checkAccess(caller, null, userAccount);
permittedAccounts.add(userAccount.getId());
owner = userAccount;
} else {
throw new InvalidParameterValueException("could not find account " + accountName + " in domain " + domain.getUuid());
}
} else if (!listAll && domainId == null) {
// listAll=false with no domain specified will only show caller owned resources
// if domainId is specified, we will filter the list based caller visible resources (owned + granted)
permittedAccounts.add(caller.getId());
}
}
@ -2229,6 +2226,15 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
}
}
} else {
if (owner == null) {
// no impersonation, so we directly check permission for caller
owner = caller;
}
AccessType accessType = AccessType.UseEntry;
if (listAll) {
// listAll = true should show all resources that owner has ListEntry access type
accessType = AccessType.ListEntry;
}
domainIdRecursiveListProject.third(Project.ListProjectResourcesCriteria.SkipProjectResources);
// search for policy permissions associated with caller to get all his authorized domains, accounts, and resources
@ -2239,18 +2245,18 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
return; // no futher filtering
QuerySelector qs = _querySelectors.get(0);
boolean grantedAll = qs.isGrantedAll(caller, action);
boolean grantedAll = qs.isGrantedAll(owner, action, accessType);
if ( grantedAll ){
if (permittedAccounts.isEmpty() && domainId != null) {
if (domainId != null) {
permittedDomains.add(domainId);
}
}
else {
List<Long> grantedDomains = qs.getAuthorizedDomains(caller, action);
List<Long> grantedAccounts = qs.getAuthorizedAccounts(caller, action);
List<Long> grantedResources = qs.getAuthorizedResources(caller, action);
List<Long> grantedDomains = qs.getAuthorizedDomains(owner, action, accessType);
List<Long> grantedAccounts = qs.getAuthorizedAccounts(owner, action, accessType);
List<Long> grantedResources = qs.getAuthorizedResources(owner, action, accessType);
if (permittedAccounts.isEmpty() && domainId != null) {
if (domainId != null) {
// specific domain and no account is specified
if (grantedDomains.contains(domainId)) {
permittedDomains.add(domainId);
@ -2268,7 +2274,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
// like NetworkACLItem
permittedResources.addAll(grantedResources);
}
} else if (permittedAccounts.isEmpty()) {
} else {
// neither domain nor account is not specified
permittedDomains.addAll(grantedDomains);
permittedAccounts.addAll(grantedAccounts);

View File

@ -25,6 +25,7 @@ import org.apache.log4j.Logger;
import org.apache.cloudstack.acl.PermissionScope;
import org.apache.cloudstack.acl.QuerySelector;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.iam.api.IAMGroup;
import org.apache.cloudstack.iam.api.IAMPolicy;
import org.apache.cloudstack.iam.api.IAMPolicyPermission;
@ -41,14 +42,22 @@ public class RoleBasedEntityQuerySelector extends AdapterBase implements QuerySe
IAMService _iamService;
@Override
public List<Long> getAuthorizedDomains(Account caller, String action) {
public List<Long> getAuthorizedDomains(Account caller, String action, AccessType accessType) {
long accountId = caller.getAccountId();
if (accessType == null) {
accessType = AccessType.UseEntry; // default always show resources authorized to use
}
// Get the static Policies of the Caller
List<IAMPolicy> policies = _iamService.listIAMPolicies(accountId);
// for each policy, find granted permission with Domain scope
List<Long> domainIds = new ArrayList<Long>();
for (IAMPolicy policy : policies) {
List<IAMPolicyPermission> pp = _iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.DOMAIN.toString());
List<IAMPolicyPermission> pp = new ArrayList<IAMPolicyPermission>();
for (AccessType type : AccessType.values()) {
if (type.ordinal() >= accessType.ordinal()) {
pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.DOMAIN.toString(), type));
}
}
if (pp != null) {
for (IAMPolicyPermission p : pp) {
if (p.getScopeId() != null) {
@ -65,14 +74,22 @@ public class RoleBasedEntityQuerySelector extends AdapterBase implements QuerySe
}
@Override
public List<Long> getAuthorizedAccounts(Account caller, String action) {
public List<Long> getAuthorizedAccounts(Account caller, String action, AccessType accessType) {
long accountId = caller.getAccountId();
if (accessType == null) {
accessType = AccessType.UseEntry; // default always show resources authorized to use
}
// Get the static Policies of the Caller
List<IAMPolicy> policies = _iamService.listIAMPolicies(accountId);
// for each policy, find granted permission with Account scope
List<Long> accountIds = new ArrayList<Long>();
for (IAMPolicy policy : policies) {
List<IAMPolicyPermission> pp = _iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.ACCOUNT.toString());
List<IAMPolicyPermission> pp = new ArrayList<IAMPolicyPermission>();
for (AccessType type : AccessType.values()) {
if (type.ordinal() >= accessType.ordinal()) {
pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.ACCOUNT.toString(), type));
}
}
if (pp != null) {
for (IAMPolicyPermission p : pp) {
if (p.getScopeId() != null) {
@ -89,8 +106,11 @@ public class RoleBasedEntityQuerySelector extends AdapterBase implements QuerySe
}
@Override
public List<Long> getAuthorizedResources(Account caller, String action) {
public List<Long> getAuthorizedResources(Account caller, String action, AccessType accessType) {
long accountId = caller.getAccountId();
if (accessType == null) {
accessType = AccessType.UseEntry; // default always show resources authorized to use
}
// Get the static Policies of the Caller
List<IAMPolicy> policies = _iamService.listIAMPolicies(accountId);
@ -107,7 +127,12 @@ public class RoleBasedEntityQuerySelector extends AdapterBase implements QuerySe
// for each policy, find granted permission with Resource scope
List<Long> entityIds = new ArrayList<Long>();
for (IAMPolicy policy : policies) {
List<IAMPolicyPermission> pp = _iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.RESOURCE.toString());
List<IAMPolicyPermission> pp = new ArrayList<IAMPolicyPermission>();
for (AccessType type : AccessType.values()) {
if (type.ordinal() >= accessType.ordinal()) {
pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.RESOURCE.toString(), type));
}
}
if (pp != null) {
for (IAMPolicyPermission p : pp) {
if (p.getScopeId() != null) {
@ -120,15 +145,23 @@ public class RoleBasedEntityQuerySelector extends AdapterBase implements QuerySe
}
@Override
public boolean isGrantedAll(Account caller, String action) {
public boolean isGrantedAll(Account caller, String action, AccessType accessType) {
long accountId = caller.getAccountId();
if (accessType == null) {
accessType = AccessType.UseEntry; // default always show resources authorized to use
}
// Get the static Policies of the Caller
List<IAMPolicy> policies = _iamService.listIAMPolicies(accountId);
// for each policy, find granted permission with ALL scope
for (IAMPolicy policy : policies) {
List<IAMPolicyPermission> pp = _iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.ALL.toString());
if (pp != null && pp.size() > 0) {
return true;
List<IAMPolicyPermission> pp = new ArrayList<IAMPolicyPermission>();
for (AccessType type : AccessType.values()) {
if (type.ordinal() >= accessType.ordinal()) {
pp.addAll(_iamService.listPolicyPermissionsByScope(policy.getId(), action, PermissionScope.ALL.toString(), type));
}
if (pp != null && pp.size() > 0) {
return true;
}
}
}
return false;

View File

@ -18,6 +18,7 @@ package org.apache.cloudstack.iam.api;
import java.util.List;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.iam.api.IAMPolicyPermission.Permission;
import com.cloud.utils.Pair;
@ -72,7 +73,7 @@ public interface IAMService {
List<IAMPolicyPermission> listPolicyPermissions(long policyId);
List<IAMPolicyPermission> listPolicyPermissionsByScope(long policyId, String action, String scope);
List<IAMPolicyPermission> listPolicyPermissionsByScope(long policyId, String action, String scope, AccessType accessType);
List<IAMPolicyPermission> listPolicyPermissionByActionAndEntity(long policyId, String action, String entityType);

View File

@ -25,6 +25,7 @@ import javax.inject.Inject;
import org.apache.log4j.Logger;
import org.apache.cloudstack.acl.PermissionScope;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.iam.api.IAMGroup;
import org.apache.cloudstack.iam.api.IAMPolicy;
import org.apache.cloudstack.iam.api.IAMPolicyPermission;
@ -742,8 +743,8 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
// for each policy, find granted permission within the given scope
List<Long> entityIds = new ArrayList<Long>();
for (IAMPolicy policy : policies) {
List<IAMPolicyPermissionVO> pp = _policyPermissionDao.listGrantedByActionAndScope(policy.getId(), action,
scope);
List<IAMPolicyPermissionVO> pp = _policyPermissionDao.listByPolicyActionAndScope(policy.getId(), action,
scope, null);
if (pp != null) {
for (IAMPolicyPermissionVO p : pp) {
if (p.getScopeId() != null) {
@ -765,9 +766,9 @@ public class IAMServiceImpl extends ManagerBase implements IAMService, Manager {
@SuppressWarnings("unchecked")
@Override
public List<IAMPolicyPermission> listPolicyPermissionsByScope(long policyId, String action, String scope) {
public List<IAMPolicyPermission> listPolicyPermissionsByScope(long policyId, String action, String scope, AccessType accessType) {
@SuppressWarnings("rawtypes")
List pp = _policyPermissionDao.listGrantedByActionAndScope(policyId, action, scope);
List pp = _policyPermissionDao.listByPolicyActionAndScope(policyId, action, scope, accessType.toString());
return pp;
}

View File

@ -29,7 +29,7 @@ public interface IAMPolicyPermissionDao extends GenericDao<IAMPolicyPermissionVO
IAMPolicyPermissionVO findByPolicyAndEntity(long policyId, String entityType, String scope, Long scopeId,
String action, Permission perm);
List<IAMPolicyPermissionVO> listGrantedByActionAndScope(long policyId, String action, String scope);
List<IAMPolicyPermissionVO> listByPolicyActionAndScope(long policyId, String action, String scope, String accessType);
List<IAMPolicyPermissionVO> listByPolicyActionAndEntity(long policyId, String action, String entityType);

View File

@ -33,7 +33,6 @@ public class IAMPolicyPermissionDaoImpl extends GenericDaoBase<IAMPolicyPermissi
private SearchBuilder<IAMPolicyPermissionVO> policyIdSearch;
private SearchBuilder<IAMPolicyPermissionVO> fullSearch;
private SearchBuilder<IAMPolicyPermissionVO> actionScopeSearch;
private SearchBuilder<IAMPolicyPermissionVO> entitySearch;
@Override
@ -54,13 +53,6 @@ public class IAMPolicyPermissionDaoImpl extends GenericDaoBase<IAMPolicyPermissi
fullSearch.and("accessType", fullSearch.entity().getAccessType(), SearchCriteria.Op.EQ);
fullSearch.done();
actionScopeSearch = createSearchBuilder();
actionScopeSearch.and("policyId", actionScopeSearch.entity().getAclPolicyId(), SearchCriteria.Op.EQ);
actionScopeSearch.and("scope", actionScopeSearch.entity().getScope(), SearchCriteria.Op.EQ);
actionScopeSearch.and("action", actionScopeSearch.entity().getAction(), SearchCriteria.Op.EQ);
actionScopeSearch.and("permission", actionScopeSearch.entity().getPermission(), SearchCriteria.Op.EQ);
actionScopeSearch.done();
entitySearch = createSearchBuilder();
entitySearch.and("entityType", entitySearch.entity().getEntityType(), SearchCriteria.Op.EQ);
entitySearch.and("scopeId", entitySearch.entity().getScopeId(), SearchCriteria.Op.EQ);
@ -90,12 +82,16 @@ public class IAMPolicyPermissionDaoImpl extends GenericDaoBase<IAMPolicyPermissi
}
@Override
public List<IAMPolicyPermissionVO> listGrantedByActionAndScope(long policyId, String action, String scope) {
SearchCriteria<IAMPolicyPermissionVO> sc = actionScopeSearch.create();
public List<IAMPolicyPermissionVO> listByPolicyActionAndScope(long policyId, String action, String scope, String accessType) {
SearchCriteria<IAMPolicyPermissionVO> sc = fullSearch.create();
sc.setParameters("policyId", policyId);
sc.setParameters("action", action);
sc.setParameters("scope", scope);
sc.setParameters("permission", Permission.Allow);
if ( accessType != null ){
// accessType can be optional, used mainly in list apis with ListEntry and UseEntry distinction
sc.setParameters("accessType", accessType);
}
return listBy(sc);
}
@ -120,7 +116,7 @@ public class IAMPolicyPermissionDaoImpl extends GenericDaoBase<IAMPolicyPermissi
@Override
public List<IAMPolicyPermissionVO> listByEntity(String entityType, Long entityId) {
SearchCriteria<IAMPolicyPermissionVO> sc = fullSearch.create();
SearchCriteria<IAMPolicyPermissionVO> sc = entitySearch.create();
sc.setParameters("entityType", entityType);
sc.setParameters("scopeId", entityId);
return listBy(sc);

View File

@ -289,8 +289,7 @@ class TestVMIam(cloudstackTestCase):
self.apiclient.connection.apiKey = self.user_1A_apikey
self.apiclient.connection.securityKey = self.user_1A_secretkey
list_vm_response = list_virtual_machines(
self.apiclient,
listall=True
self.apiclient
)
self.assertEqual(
isinstance(list_vm_response, list),
@ -313,8 +312,7 @@ class TestVMIam(cloudstackTestCase):
self.apiclient.connection.apiKey = self.user_1B_apikey
self.apiclient.connection.securityKey = self.user_1B_secretkey
list_vm_response = list_virtual_machines(
self.apiclient,
listall=True
self.apiclient
)
self.assertEqual(
isinstance(list_vm_response, list),
@ -338,8 +336,7 @@ class TestVMIam(cloudstackTestCase):
self.apiclient.connection.apiKey = self.user_2A_apikey
self.apiclient.connection.securityKey = self.user_2A_secretkey
list_vm_response = list_virtual_machines(
self.apiclient,
listall=True
self.apiclient
)
self.assertEqual(
isinstance(list_vm_response, list),
@ -381,8 +378,7 @@ class TestVMIam(cloudstackTestCase):
self.apiclient.connection.apiKey = self.user_1B_apikey
self.apiclient.connection.securityKey = self.user_1B_secretkey
list_vm_response = list_virtual_machines(
self.apiclient,
listall=True
self.apiclient
)
self.assertEqual(
isinstance(list_vm_response, list),
@ -430,8 +426,7 @@ class TestVMIam(cloudstackTestCase):
self.apiclient.connection.apiKey = self.user_1B_apikey
self.apiclient.connection.securityKey = self.user_1B_secretkey
list_vm_response = list_virtual_machines(
self.apiclient,
listall=True
self.apiclient
)
self.assertEqual(
isinstance(list_vm_response, list),
@ -484,8 +479,7 @@ class TestVMIam(cloudstackTestCase):
self.apiclient.connection.apiKey = self.user_1B_apikey
self.apiclient.connection.securityKey = self.user_1B_secretkey
list_vm_response = list_virtual_machines(
self.apiclient,
listall=True
self.apiclient
)
self.assertEqual(
isinstance(list_vm_response, list),
@ -528,8 +522,7 @@ class TestVMIam(cloudstackTestCase):
self.apiclient.connection.apiKey = self.user_1B_apikey
self.apiclient.connection.securityKey = self.user_1B_secretkey
list_vm_response = list_virtual_machines(
self.apiclient,
listall=True
self.apiclient
)
self.assertEqual(
isinstance(list_vm_response, list),
@ -570,8 +563,7 @@ class TestVMIam(cloudstackTestCase):
self.apiclient.connection.apiKey = self.user_1B_apikey
self.apiclient.connection.securityKey = self.user_1B_secretkey
list_vm_response = list_virtual_machines(
self.apiclient,
listall=True
self.apiclient
)
self.assertEqual(
isinstance(list_vm_response, list),
@ -618,8 +610,7 @@ class TestVMIam(cloudstackTestCase):
self.apiclient.connection.apiKey = self.user_1B_apikey
self.apiclient.connection.securityKey = self.user_1B_secretkey
list_vm_response = list_virtual_machines(
self.apiclient,
listall=True
self.apiclient
)
self.assertEqual(
isinstance(list_vm_response, list),
@ -662,8 +653,7 @@ class TestVMIam(cloudstackTestCase):
self.apiclient.connection.apiKey = self.user_1B_apikey
self.apiclient.connection.securityKey = self.user_1B_secretkey
list_vm_response = list_virtual_machines(
self.apiclient,
listall=True
self.apiclient
)
self.assertEqual(
isinstance(list_vm_response, list),
@ -705,8 +695,7 @@ class TestVMIam(cloudstackTestCase):
self.apiclient.connection.apiKey = self.user_1B_apikey
self.apiclient.connection.securityKey = self.user_1B_secretkey
list_vm_response = list_virtual_machines(
self.apiclient,
listall=True
self.apiclient
)
self.assertEqual(
isinstance(list_vm_response, list),