Repository: incubator-ranger Updated Branches: refs/heads/ranger-0.5 04daba455 -> 0b09a8717
RANGER-789 : Fix incorrect policy list paging for non-admin users Project: http://git-wip-us.apache.org/repos/asf/incubator-ranger/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ranger/commit/0b09a871 Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/0b09a871 Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/0b09a871 Branch: refs/heads/ranger-0.5 Commit: 0b09a8717b4db832da720d30193781eaa327b6ca Parents: 04daba4 Author: Gautam Borad <[email protected]> Authored: Mon Jan 18 16:54:41 2016 +0530 Committer: Gautam Borad <[email protected]> Committed: Tue Feb 2 11:16:28 2016 +0530 ---------------------------------------------------------------------- .../plugin/store/AbstractPredicateUtil.java | 150 ++++++++++++++++++- .../apache/ranger/plugin/util/SearchFilter.java | 2 +- .../org/apache/ranger/biz/ServiceDBStore.java | 23 +-- .../apache/ranger/common/RangerSearchUtil.java | 2 +- .../org/apache/ranger/rest/ServiceREST.java | 143 ++++++++++++++---- .../org/apache/ranger/rest/TestServiceREST.java | 19 +-- 6 files changed, 285 insertions(+), 54 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/0b09a871/agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractPredicateUtil.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractPredicateUtil.java b/agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractPredicateUtil.java index 772c2d7..b63c1d7 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractPredicateUtil.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractPredicateUtil.java @@ -79,6 +79,7 @@ public class AbstractPredicateUtil { } public void addPredicates(SearchFilter filter, List<Predicate> predicates) { + addPredicateForServiceType(filter.getParam(SearchFilter.SERVICE_TYPE), predicates); addPredicateForServiceTypeId(filter.getParam(SearchFilter.SERVICE_TYPE_ID), predicates); addPredicateForServiceName(filter.getParam(SearchFilter.SERVICE_NAME), predicates); addPredicateForPolicyName(filter.getParam(SearchFilter.POLICY_NAME), predicates); @@ -89,6 +90,10 @@ public class AbstractPredicateUtil { addPredicateForGroupName(filter.getParam(SearchFilter.GROUP), predicates); addPredicateForResourceSignature(filter.getParam(SearchFilter.RESOURCE_SIGNATURE), predicates); addPredicateForResources(filter.getParamsWithPrefix(SearchFilter.RESOURCE_PREFIX, true), predicates); + addPredicateForPolicyResource(filter.getParam(SearchFilter.POL_RESOURCE), predicates); + addPredicateForPartialPolicyName(filter.getParam(SearchFilter.POLICY_NAME_PARTIAL), predicates); + addPredicateForResourceSignature(filter.getParam(SearchFilter.RESOURCE_SIGNATURE), predicates); + addPredicateForPolicyType(filter.getParam(SearchFilter.POLICY_TYPE), predicates); } public Comparator<RangerBaseModelObject> getSorter(SearchFilter filter) { @@ -216,6 +221,38 @@ public class AbstractPredicateUtil { sorterMap.put(SearchFilter.UPDATE_TIME, updateTimeComparator); } + private Predicate addPredicateForServiceType(final String serviceType, List<Predicate> predicates) { + if (StringUtils.isEmpty(serviceType)) { + return null; + } + + Predicate ret = new Predicate() { + @Override + public boolean evaluate(Object object) { + if (object == null) { + return false; + } + + boolean ret = false; + + if (object instanceof RangerServiceDef) { + RangerServiceDef serviceDef = (RangerServiceDef) object; + String svcType = serviceDef.getName(); + + ret = StringUtils.equals(svcType, serviceType); + } else { + ret = true; + } + + return ret; + } + }; + if(predicates != null) { + predicates.add(ret); + } + return ret; + } + private Predicate addPredicateForServiceTypeId(final String serviceTypeId, List<Predicate> predicates) { if(StringUtils.isEmpty(serviceTypeId)) { return null; @@ -248,7 +285,7 @@ public class AbstractPredicateUtil { if(predicates != null) { predicates.add(ret); } - + return ret; } @@ -322,6 +359,32 @@ public class AbstractPredicateUtil { return ret; } + private Predicate addPredicateForPartialPolicyName(final String policyName, List<Predicate> predicates) { + if(StringUtils.isEmpty(policyName)) { + return null; + } + Predicate ret = new Predicate() { + @Override + public boolean evaluate(Object object) { + if(object == null) { + return false; + } + boolean ret = false; + if(object instanceof RangerPolicy) { + RangerPolicy policy = (RangerPolicy)object; + ret = StringUtils.containsIgnoreCase(policy.getName(), policyName); + } else { + ret = true; + } + return ret; + } + }; + if(predicates != null) { + predicates.add(ret); + } + return ret; + } + private Predicate addPredicateForPolicyId(final String policyId, List<Predicate> predicates) { if(StringUtils.isEmpty(policyId)) { return null; @@ -535,6 +598,57 @@ public class AbstractPredicateUtil { return ret; } + private Predicate addPredicateForPolicyResource(final String resourceValue, List<Predicate> predicates) { + if (StringUtils.isEmpty(resourceValue)) { + return null; + } + Predicate ret = new Predicate() { + @Override + public boolean evaluate(Object object) { + if (object == null) { + return false; + } + + boolean ret = false; + + if (object instanceof RangerPolicy) { + RangerPolicy policy = (RangerPolicy) object; + Map<String, RangerPolicyResource> policyResources = policy.getResources(); + + if (MapUtils.isNotEmpty(policyResources)) { + for (String resourceName : policyResources.keySet()) { + RangerPolicyResource policyResource = policyResources.get(resourceName); + + if (policyResource != null && CollectionUtils.isNotEmpty(policyResource.getValues())) { + for (String policyResourceVal : policyResource.getValues()) { + if (StringUtils.containsIgnoreCase(policyResourceVal, resourceValue)) { + ret = true; + + break; + } + } + } + + if (ret) { + break; + } + } + } + } else { + ret = true; + } + + return ret; + } + }; + + if (predicates != null) { + predicates.add(ret); + } + + return ret; + } + private Predicate addPredicateForIsRecursive(final String isRecursiveStr, List<Predicate> predicates) { if(StringUtils.isEmpty(isRecursiveStr)) { return null; @@ -592,7 +706,39 @@ public class AbstractPredicateUtil { return ret; } - + + private Predicate addPredicateForPolicyType(final String policyType, List<Predicate> predicates) { + if (StringUtils.isEmpty(policyType)) { + return null; + } + + Predicate ret = new Predicate() { + @Override + public boolean evaluate(Object object) { + if (object == null) { + return false; + } + + boolean ret = true; + + if (object instanceof RangerPolicy) { + RangerPolicy policy = (RangerPolicy) object; + + if (policy.getPolicyType() != null) { + ret = StringUtils.equalsIgnoreCase(policyType, policy.getPolicyType().toString()); + } + } + + return ret; + } + }; + + if (predicates != null) { + predicates.add(ret); + } + + return ret; + } /** * @param policySignature * @return http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/0b09a871/agents-common/src/main/java/org/apache/ranger/plugin/util/SearchFilter.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/util/SearchFilter.java b/agents-common/src/main/java/org/apache/ranger/plugin/util/SearchFilter.java index 17738be..7171cb1 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/util/SearchFilter.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/util/SearchFilter.java @@ -47,7 +47,7 @@ public class SearchFilter { public static final String PAGE_SIZE = "pageSize"; public static final String SORT_BY = "sortBy"; public static final String RESOURCE_SIGNATURE = "resourceSignature:"; // search - + public static final String POLICY_TYPE = "policyType"; private Map<String, String> params = null; private int startIndex = 0; private int maxRows = Integer.MAX_VALUE; http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/0b09a871/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java index 278ebc0..199d041 100644 --- a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java @@ -1553,9 +1553,11 @@ public class ServiceDBStore implements ServiceStore { if (service == null) { throw new Exception("service does not exist - id='" + serviceId); } - - List<RangerPolicy> ret = getServicePolicies(service.getName(), filter); - + RangerPolicyRetriever policyRetriever = new RangerPolicyRetriever(daoMgr); + List<RangerPolicy> ret = policyRetriever.getServicePolicies(service); + if(filter != null) { + predicateUtil.applyFilter(ret, filter); + } return ret; } @@ -1600,14 +1602,15 @@ public class ServiceDBStore implements ServiceStore { if(LOG.isDebugEnabled()) { LOG.debug("==> ServiceDBStore.getServicePolicies(" + serviceName + ")"); } - - if(filter == null) { - filter = new SearchFilter(); + XXService service = daoMgr.getXXService().findByName(serviceName); + if (service == null) { + throw new Exception("service does not exist - name='" + serviceName); + } + RangerPolicyRetriever policyRetriever = new RangerPolicyRetriever(daoMgr); + List<RangerPolicy> ret = policyRetriever.getServicePolicies(service); + if(filter != null) { + predicateUtil.applyFilter(ret, filter); } - - filter.setParam(SearchFilter.SERVICE_NAME, serviceName); - - List<RangerPolicy> ret = getPolicies(filter); if(LOG.isDebugEnabled()) { LOG.debug("<== ServiceDBStore.getServicePolicies(" + serviceName + "): count=" + ((ret == null) ? 0 : ret.size())); http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/0b09a871/security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java b/security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java index 897ed5d..8469d65 100644 --- a/security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java +++ b/security-admin/src/main/java/org/apache/ranger/common/RangerSearchUtil.java @@ -158,7 +158,7 @@ public class RangerSearchUtil extends SearchUtil { int pageSize = restErrorUtil.parseInt(request.getParameter(SearchFilter.PAGE_SIZE), configUtil.getDefaultMaxRows(), "Invalid value for parameter pageSize", MessageEnums.INVALID_INPUT_DATA, null, SearchFilter.PAGE_SIZE); - ret.setMaxRows(pageSize); + ret.setMaxRows(validatePageSize(pageSize)); ret.setGetCount(restErrorUtil.parseBoolean(request.getParameter("getCount"), true)); String sortBy = restErrorUtil.validateString(request.getParameter(SearchFilter.SORT_BY), http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/0b09a871/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java b/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java index 7718078..8129124 100644 --- a/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java +++ b/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java @@ -39,6 +39,7 @@ import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Context; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.collections.MapUtils; import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -1245,19 +1246,39 @@ public class ServiceREST { LOG.debug("==> ServiceREST.getPolicies()"); } - RangerPolicyList ret = null; + RangerPolicyList ret = new RangerPolicyList(); RangerPerfTracer perf = null; + SearchFilter filter = searchUtil.getSearchFilter(request, policyService.sortFields); + try { if(RangerPerfTracer.isPerfTraceEnabled(PERF_LOG)) { perf = RangerPerfTracer.getPerfTracer(PERF_LOG, "ServiceREST.getPolicies()"); } + if(isAdminUserWithNoFilterParams(filter)) { + ret = svcStore.getPaginatedPolicies(filter); + } + else { + // get all policies from the store; pick the page to return after applying filter + int savedStartIndex = filter == null ? 0 : filter.getStartIndex(); + int savedMaxRows = filter == null ? Integer.MAX_VALUE : filter.getMaxRows(); - SearchFilter filter = searchUtil.getSearchFilter(request, policyService.sortFields); + if(filter != null) { + filter.setStartIndex(0); + filter.setMaxRows(Integer.MAX_VALUE); + } - try { - ret = svcStore.getPaginatedPolicies(filter); + List<RangerPolicy> policies = svcStore.getPolicies(filter); + + if(filter != null) { + filter.setStartIndex(savedStartIndex); + filter.setMaxRows(savedMaxRows); + } + + applyAdminAccessFilter(policies); + + ret = toRangerPolicyList(policies, filter); + } - applyAdminAccessFilter(ret); } catch(WebApplicationException excp) { throw excp; } catch (Throwable excp) { @@ -1354,19 +1375,38 @@ public class ServiceREST { LOG.debug("==> ServiceREST.getServicePolicies(" + serviceId + ")"); } - RangerPolicyList ret = null; + RangerPolicyList ret = new RangerPolicyList(); RangerPerfTracer perf = null; - - if(RangerPerfTracer.isPerfTraceEnabled(PERF_LOG)) { - perf = RangerPerfTracer.getPerfTracer(PERF_LOG, "ServiceREST.getServicePolicies(serviceId=" + serviceId + ")"); - } - SearchFilter filter = searchUtil.getSearchFilter(request, policyService.sortFields); try { - ret = svcStore.getPaginatedServicePolicies(serviceId, filter); + if(RangerPerfTracer.isPerfTraceEnabled(PERF_LOG)) { + perf = RangerPerfTracer.getPerfTracer(PERF_LOG, "ServiceREST.getServicePolicies(serviceId=" + serviceId + ")"); + } + if(isAdminUserWithNoFilterParams(filter)) { + ret = svcStore.getPaginatedServicePolicies(serviceId, filter); + } else { + // get all policies from the store; pick the page to return after applying filter + int savedStartIndex = filter == null ? 0 : filter.getStartIndex(); + int savedMaxRows = filter == null ? Integer.MAX_VALUE : filter.getMaxRows(); + + if(filter != null) { + filter.setStartIndex(0); + filter.setMaxRows(Integer.MAX_VALUE); + } + + List<RangerPolicy> servicePolicies = svcStore.getServicePolicies(serviceId, filter); + + if(filter != null) { + filter.setStartIndex(savedStartIndex); + filter.setMaxRows(savedMaxRows); + } + + applyAdminAccessFilter(servicePolicies); + + ret = toRangerPolicyList(servicePolicies, filter); + } - applyAdminAccessFilter(ret); } catch(WebApplicationException excp) { throw excp; } catch (Throwable excp) { @@ -1377,10 +1417,6 @@ public class ServiceREST { RangerPerfTracer.log(perf); } - if (ret == null) { - LOG.info("No Policies found for given service id: " + serviceId); - } - if (LOG.isDebugEnabled()) { LOG.debug("<== ServiceREST.getServicePolicies(" + serviceId + "): count=" + ret.getListSize()); @@ -1397,19 +1433,41 @@ public class ServiceREST { LOG.debug("==> ServiceREST.getServicePolicies(" + serviceName + ")"); } - RangerPolicyList ret = null; + RangerPolicyList ret = new RangerPolicyList();; RangerPerfTracer perf = null; + SearchFilter filter = searchUtil.getSearchFilter(request, policyService.sortFields); + + try { + if(RangerPerfTracer.isPerfTraceEnabled(PERF_LOG)) { perf = RangerPerfTracer.getPerfTracer(PERF_LOG, "ServiceREST.getServicePolicies(serviceName=" + serviceName + ")"); } - SearchFilter filter = searchUtil.getSearchFilter(request, policyService.sortFields); - - try { + if(isAdminUserWithNoFilterParams(filter)) { ret = svcStore.getPaginatedServicePolicies(serviceName, filter); + } else { + // get all policies from the store; pick the page to return after applying filter + int savedStartIndex = filter == null ? 0 : filter.getStartIndex(); + int savedMaxRows = filter == null ? Integer.MAX_VALUE : filter.getMaxRows(); + + if(filter != null) { + filter.setStartIndex(0); + filter.setMaxRows(Integer.MAX_VALUE); + } + + List<RangerPolicy> servicePolicies = svcStore.getServicePolicies(serviceName, filter); + + if(filter != null) { + filter.setStartIndex(savedStartIndex); + filter.setMaxRows(savedMaxRows); + } + + applyAdminAccessFilter(servicePolicies); + + ret = toRangerPolicyList(servicePolicies, filter); + } - applyAdminAccessFilter(ret); } catch(WebApplicationException excp) { throw excp; } catch (Throwable excp) { @@ -1714,11 +1772,6 @@ public class ServiceREST { return svcStore.getPolicyForVersionNumber(policyId, versionNo); } - private void applyAdminAccessFilter(RangerPolicyList policies) { - if(policies != null && !CollectionUtils.isEmpty(policies.getList())) { - applyAdminAccessFilter(policies.getPolicies()); - } - } private void applyAdminAccessFilter(List<RangerPolicy> policies) { boolean isAdmin = bizUtil.isAdmin(); @@ -1845,4 +1898,38 @@ public class ServiceREST { return ret; } -} + boolean isAdminUserWithNoFilterParams(SearchFilter filter) { + return (filter == null || MapUtils.isEmpty(filter.getParams())) && + (bizUtil.isAdmin() || bizUtil.isKeyAdmin()); + } + + private RangerPolicyList toRangerPolicyList(List<RangerPolicy> policyList, SearchFilter filter) { + RangerPolicyList ret = new RangerPolicyList(); + + if(CollectionUtils.isNotEmpty(policyList)) { + int totalCount = policyList.size(); + int startIndex = filter == null ? 0 : filter.getStartIndex(); + int pageSize = filter == null ? totalCount : filter.getMaxRows(); + int toIndex = Math.min(startIndex + pageSize, totalCount); + String sortType = filter == null ? null : filter.getSortType(); + String sortBy = filter == null ? null : filter.getSortBy(); + + List<RangerPolicy> retList = new ArrayList<RangerPolicy>(); + for(int i = startIndex; i < toIndex; i++) { + retList.add(policyList.get(i)); + } + + ret.setPolicies(retList); + ret.setPageSize(pageSize); + ret.setResultSize(retList.size()); + ret.setStartIndex(startIndex); + ret.setTotalCount(totalCount); + ret.setSortBy(sortBy); + ret.setSortType(sortType); + } + + return ret; + } + + +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/0b09a871/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java ---------------------------------------------------------------------- diff --git a/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java b/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java index 8b3e348..6f4f702 100644 --- a/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java +++ b/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java @@ -860,7 +860,8 @@ public class TestServiceREST { searchUtil.getSearchFilter(request, policyService.sortFields)) .thenReturn(filter); RangerPolicyList dbRangerPolicy = serviceREST.getPolicies(request); - Assert.assertNull(dbRangerPolicy); + Assert.assertNotNull(dbRangerPolicy); + Assert.assertEquals(dbRangerPolicy.getListSize(), 0); Mockito.verify(searchUtil).getSearchFilter(request, policyService.sortFields); } @@ -874,15 +875,12 @@ public class TestServiceREST { filter.setParam(SearchFilter.SERVICE_NAME, "serviceName"); Mockito.when( searchUtil.getSearchFilter(request, policyService.sortFields)) - .thenReturn(filter); - - Mockito.when(svcStore.getPaginatedPolicies(filter)).thenReturn(ret); + .thenReturn(filter); Long data = serviceREST.countPolicies(request); Assert.assertNotNull(data); Mockito.verify(searchUtil).getSearchFilter(request, - policyService.sortFields); - Mockito.verify(svcStore).getPaginatedPolicies(filter); + policyService.sortFields); } @Test @@ -898,22 +896,19 @@ public class TestServiceREST { searchUtil.getSearchFilter(request, policyService.sortFields)) .thenReturn(filter); - Mockito.when(svcStore.getPaginatedServicePolicies(Id, filter)) - .thenReturn(ret); - RangerPolicyList dbRangerPolicy = serviceREST.getServicePolicies( rangerPolicy.getId(), request); Assert.assertNotNull(dbRangerPolicy); Mockito.verify(searchUtil).getSearchFilter(request, policyService.sortFields); - Mockito.verify(svcStore).getPaginatedServicePolicies(Id, filter); } @Test public void test23getServicePoliciesByName() throws Exception { HttpServletRequest request = Mockito.mock(HttpServletRequest.class); RangerPolicy rangerPolicy = rangerPolicy(); - RangerPolicyList ret = Mockito.mock(RangerPolicyList.class); + List<RangerPolicy> ret = new ArrayList<RangerPolicy>(); + ret.add(rangerPolicy); SearchFilter filter = new SearchFilter(); filter.setParam(SearchFilter.POLICY_NAME, "policyName"); filter.setParam(SearchFilter.SERVICE_NAME, "serviceName"); @@ -922,7 +917,7 @@ public class TestServiceREST { .thenReturn(filter); Mockito.when( - svcStore.getPaginatedServicePolicies(rangerPolicy.getName(), + svcStore.getServicePolicies(rangerPolicy.getName(), filter)).thenReturn(ret); RangerPolicyList dbRangerPolicy = serviceREST.getServicePoliciesByName(
