RANGER-278 Add validation for policy create/update/delete operations. Moved 
action out of ctor to validate call.

Signed-off-by: Madhan Neethiraj <[email protected]>


Project: http://git-wip-us.apache.org/repos/asf/incubator-ranger/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-ranger/commit/f721abec
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/f721abec
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/f721abec

Branch: refs/heads/master
Commit: f721abec9cd95ac4c6b70fb964884346f2814ed1
Parents: 96fd15e
Author: Alok Lal <[email protected]>
Authored: Wed Mar 4 23:32:28 2015 -0800
Committer: Madhan Neethiraj <[email protected]>
Committed: Tue Mar 10 15:13:32 2015 -0700

----------------------------------------------------------------------
 .../apache/ranger/plugin/util/SearchFilter.java |  15 +
 pom.xml                                         |   2 +-
 .../ranger/rest/RangerPolicyValidator.java      | 430 +++++++++++++++++
 .../ranger/rest/RangerServiceValidator.java     | 135 +++---
 .../org/apache/ranger/rest/RangerValidator.java | 310 ++++++++++--
 .../ranger/rest/RangerValidatorFactory.java     |  10 +-
 .../org/apache/ranger/rest/ServiceREST.java     |  34 +-
 .../ranger/rest/TestRangerPolicyValidator.java  | 471 +++++++++++++++++++
 .../ranger/rest/TestRangerServiceValidator.java | 246 +++++-----
 .../apache/ranger/rest/TestRangerValidator.java | 333 +++++++++++++
 .../rest/TestServiceRESTForValidation.java      | 225 +++++++--
 .../ranger/rest/TestServiceValidator.java       | 202 --------
 .../apache/ranger/rest/ValidationTestUtils.java | 195 ++++++++
 13 files changed, 2126 insertions(+), 482 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f721abec/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 ab8384c..d67df8d 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
@@ -21,6 +21,7 @@ package org.apache.ranger.plugin.util;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 
 import org.apache.commons.collections.MapUtils;
 import org.apache.commons.lang.StringUtils;
@@ -113,4 +114,18 @@ public class SearchFilter {
        public boolean isEmpty() {
                return MapUtils.isEmpty(params);
        }
+       
+       @Override
+       public boolean equals(Object object) {
+               if (object == null || !(object instanceof SearchFilter)) {
+                       return false;
+               }
+               SearchFilter that = (SearchFilter)object;
+               return Objects.equals(params, that.params);
+       }
+       
+       @Override
+       public int hashCode() {
+               return Objects.hash(params);
+       }
 }

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f721abec/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 79a80b9..7df033d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -309,7 +309,7 @@
         <artifactId>maven-surefire-plugin</artifactId>
         <version>2.17</version>
         <configuration>
-           
<argLine>-Djava.library.path=${hadoop.library.path}${path.separator}${java.library.path}"</argLine>
+           
<argLine>-Djava.library.path=${hadoop.library.path}${path.separator}${java.library.path}</argLine>
            <skipTests>${skipTests}</skipTests>
            <encoding>UTF-8</encoding>
            <systemProperties>

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f721abec/security-admin/src/main/java/org/apache/ranger/rest/RangerPolicyValidator.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/main/java/org/apache/ranger/rest/RangerPolicyValidator.java
 
b/security-admin/src/main/java/org/apache/ranger/rest/RangerPolicyValidator.java
new file mode 100644
index 0000000..941bb21
--- /dev/null
+++ 
b/security-admin/src/main/java/org/apache/ranger/rest/RangerPolicyValidator.java
@@ -0,0 +1,430 @@
+package org.apache.ranger.rest;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.ranger.plugin.model.RangerPolicy;
+import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItem;
+import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItemAccess;
+import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyResource;
+import org.apache.ranger.plugin.model.RangerService;
+import org.apache.ranger.plugin.model.RangerServiceDef;
+import org.apache.ranger.plugin.store.ServiceStore;
+
+import com.google.common.collect.Sets;
+
+public class RangerPolicyValidator extends RangerValidator {
+
+       private static final Log LOG = 
LogFactory.getLog(RangerPolicyValidator.class);
+
+       public RangerPolicyValidator(ServiceStore store) {
+               super(store);
+       }
+
+       public void validate(RangerPolicy policy, Action action) throws 
Exception {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("==> 
RangerValidator.validate(%s, %s)", policy, action));
+               }
+
+               List<ValidationFailureDetails> failures = new 
ArrayList<ValidationFailureDetails>();
+               boolean valid = isValid(policy, action, failures);
+               String message = "";
+               try {
+                       if (!valid) {
+                               message = serializeFailures(failures);
+                               throw new Exception(message);
+                       }
+               } finally {
+                       if(LOG.isDebugEnabled()) {
+                               LOG.debug(String.format("<== 
RangerValidator.validate(%s, %s): %s, reason[%s]", policy, action, valid, 
message));
+                       }
+               }
+       }
+
+       @Override
+       boolean isValid(Long id, Action action, List<ValidationFailureDetails> 
failures) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("==> 
RangerPolicyValidator.isValid(%s, %s, %s)", id, action, failures));
+               }
+
+               boolean valid = true;
+               if (action != Action.DELETE) {
+                       failures.add(new ValidationFailureDetailsBuilder()
+                               .isAnInternalError()
+                               .becauseOf("isValid(Long) is only supported for 
DELETE")
+                               .build());
+                       valid = false;
+               } else if (id == null) {
+                       failures.add(new ValidationFailureDetailsBuilder()
+                               .field("id")
+                               .isMissing()
+                               .build());
+                       valid = false;
+               } else if (getPolicy(id) == null) {
+                       failures.add(new ValidationFailureDetailsBuilder()
+                               .field("id")
+                               .isSemanticallyIncorrect()
+                               .becauseOf("no policy found for id[" + id + "]")
+                               .build());
+                       valid = false;
+               }
+
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("<== 
RangerPolicyValidator.isValid(%s, %s, %s): %s", id, action, failures, valid));
+               }
+               return valid;
+       }
+
+       boolean isValid(RangerPolicy policy, Action action, 
List<ValidationFailureDetails> failures) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("==> 
RangerPolicyValidator.isValid(%s, %s, %s)", policy, action, failures));
+               }
+
+               if (!(action == Action.CREATE || action == Action.UPDATE)) {
+                       throw new 
IllegalArgumentException("isValid(RangerPolicy, ...) is only supported for 
create/update");
+               }
+               boolean valid = true;
+               if (policy == null) {
+                       String message = "policy object passed in was null";
+                       LOG.debug(message);
+                       failures.add(new ValidationFailureDetailsBuilder()
+                               .field("policy")
+                               .isMissing()
+                               .becauseOf(message)
+                               .build());
+                       valid = false;
+               } else {
+                       Long id = policy.getId();
+                       if (action == Action.UPDATE) { // id is ignored for 
CREATE
+                               if (id == null) {
+                                       String message = "policy id was 
null/empty/blank"; 
+                                       LOG.debug(message);
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
+                                               .field("id")
+                                               .isMissing()
+                                               .becauseOf(message)
+                                               .build());
+                                       valid = false;
+                               } else if (getPolicy(id) == null) {
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
+                                               .field("id")
+                                               .isSemanticallyIncorrect()
+                                               .becauseOf("no policy exists 
with id[" + id +"]")
+                                               .build());
+                                       valid = false;
+                               }
+                       }
+                       String policyName = policy.getName();
+                       String serviceName = policy.getService();
+                       if (StringUtils.isBlank(policyName)) {
+                               String message = "policy name was 
null/empty/blank[" + policyName + "]"; 
+                               LOG.debug(message);
+                               failures.add(new 
ValidationFailureDetailsBuilder()
+                                       .field("name")
+                                       .isMissing()
+                                       .becauseOf(message)
+                                       .build());
+                               valid = false;
+                       } else {
+                               List<RangerPolicy> policies = 
getPolicies(policyName, serviceName);
+                               if (CollectionUtils.isNotEmpty(policies)) {
+                                       if (policies.size() > 1) {
+                                               failures.add(new 
ValidationFailureDetailsBuilder()
+                                                       .isAnInternalError()
+                                                       .becauseOf("multiple 
policies found with the name[" + policyName + "]")
+                                                       .build());
+                                               valid = false;
+                                       } else if (action == Action.CREATE) { 
// size == 1
+                                               failures.add(new 
ValidationFailureDetailsBuilder()
+                                                       .field("name")
+                                                       
.isSemanticallyIncorrect()
+                                                       .becauseOf("policy 
already exists with name[" + policyName + "]; its id is[" + 
policies.iterator().next().getId() + "]")
+                                                       .build());
+                                               valid = false;
+                                       } else if 
(policies.iterator().next().getId() != id) { // size == 1 && action == UPDATE
+                                               failures.add(new 
ValidationFailureDetailsBuilder()
+                                                       .field("id/name")
+                                                       
.isSemanticallyIncorrect()
+                                                       .becauseOf("id/name 
conflict: policy already exists with name[" + policyName + "], its id is[" + 
policies.iterator().next().getId() + "]")
+                                                       .build());
+                                               valid = false;
+                                       }
+                               }
+                       }
+                       RangerService service = null;
+                       if (StringUtils.isBlank(serviceName)) {
+                               failures.add(new 
ValidationFailureDetailsBuilder()
+                               .field("service")
+                               .isMissing()
+                               .becauseOf("service name was null/empty/blank")
+                               .build());
+                               valid = false;
+                       } else {
+                               service = getService(serviceName);
+                               if (service == null) {
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
+                                               .field("service")
+                                               .isMissing()
+                                               .becauseOf("service name was 
null/empty/blank")
+                                               .build());
+                                       valid = false;
+                               }
+                       }
+                       List<RangerPolicyItem> policyItems = 
policy.getPolicyItems();
+                       boolean isAuditEnabled = getIsAuditEnabled(policy);
+                       RangerServiceDef serviceDef = null;
+                       String serviceDefName = null;
+                       if (CollectionUtils.isEmpty(policyItems) && 
!isAuditEnabled) {
+                               failures.add(new 
ValidationFailureDetailsBuilder()
+                                       .field("policy items")
+                                       .isMissing()
+                                       .becauseOf("at least one policy item 
must be specified if audit isn't enabled")
+                                       .build());
+                               valid = false;
+                       } else if (service != null) {
+                               serviceDefName = service.getType();
+                               serviceDef = getServiceDef(serviceDefName);
+                               if (serviceDef == null) {
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
+                                               .field("policy service def")
+                                               .isAnInternalError()
+                                               .becauseOf("Service def of 
policies service does not exist")
+                                               .build());
+                                       valid = false;
+                               } else {
+                                       valid = isValidPolicyItems(policyItems, 
failures, serviceDef) && valid;
+                               }
+                       }
+                       if (serviceDef != null) {
+                               Set<String> mandatoryResources = 
getMandatoryResourceNames(serviceDef);
+                               Set<String> policyResources = 
getPolicyResources(policy);
+                               Set<String> missingResources = 
Sets.difference(mandatoryResources, policyResources);
+                               if (!missingResources.isEmpty()) {
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
+                                               .field("resources")
+                                               
.subField(missingResources.iterator().next()) // we return any one parameter!
+                                               .isMissing()
+                                               .becauseOf("required 
resources[" + missingResources + "] are missing")
+                                               .build());
+                                       valid = false;
+                               }
+                               Set<String> allResource = 
getAllResourceNames(serviceDef);
+                               Set<String> unknownResources = 
Sets.difference(policyResources, allResource);
+                               if (!unknownResources.isEmpty()) {
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
+                                               .field("resources")
+                                               
.subField(unknownResources.iterator().next()) // we return any one parameter!
+                                               .isSemanticallyIncorrect()
+                                               .becauseOf("resource[" + 
unknownResources + "] is not valid for service-def[" + serviceDefName + "]")
+                                               .build());
+                                       valid = false;
+                               }
+                               Map<String, RangerPolicyResource> resourceMap = 
policy.getResources();
+                               // errors about if empty resource collection is 
ok or not has already happened above, this check is still needed 
+                               if (resourceMap != null && 
!resourceMap.isEmpty()) {
+                                       valid = 
isValidResourceValues(resourceMap, failures, serviceDef) && valid;
+                               }
+                       }
+               }
+               
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("<== 
RangerPolicyValidator.isValid(%s, %s, %s): %s", policy, action, failures, 
valid));
+               }
+               return valid;
+       }
+       
+       boolean isValidResourceValues(Map<String, RangerPolicyResource> 
resourceMap, List<ValidationFailureDetails> failures, RangerServiceDef 
serviceDef) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("==> 
RangerPolicyValidator.isValidResourceValues(%s, %s, %s)", resourceMap, 
failures, serviceDef));
+               }
+
+               boolean valid = true;
+               if (resourceMap == null) {
+                       LOG.debug("isValidResourceValues: resourceMap is null");
+               } else if (serviceDef == null) {
+                       LOG.debug("isValidResourceValues: service Def is null");
+               } else {
+                       Map<String, String> validationRegExMap = 
getValidationRegExes(serviceDef);
+                       for (Map.Entry<String, RangerPolicyResource> entry : 
resourceMap.entrySet()) {
+                               String name = entry.getKey();
+                               RangerPolicyResource policyResource = 
entry.getValue();
+                               if (validationRegExMap.containsKey(name) && 
policyResource != null && 
CollectionUtils.isNotEmpty(policyResource.getValues())) {
+                                       String regEx = 
validationRegExMap.get(name);
+                                       for (String aValue : 
policyResource.getValues()) {
+                                               if 
(StringUtils.isBlank(aValue)) {
+                                                       LOG.debug("resource 
value was blank");
+                                               } else if 
(!aValue.matches(regEx)) {
+                                                       failures.add(new 
ValidationFailureDetailsBuilder()
+                                                               
.field("resource-values")
+                                                               .subField(name)
+                                                               
.isSemanticallyIncorrect()
+                                                               
.becauseOf("resources value[" + aValue + "] does not match validation regex[" + 
regEx + "] defined on service-def[" + serviceDef.getName() + "]")
+                                                               .build());
+                                                       valid = false;
+                                               }
+                                       }
+                               }
+                       }
+               }
+
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("<== 
RangerPolicyValidator.isValidResourceValues(%s, %s, %s): %s", resourceMap, 
failures, serviceDef, valid));
+               }
+               return valid;
+       }
+
+       boolean isValidPolicyItems(List<RangerPolicyItem> policyItems, 
List<ValidationFailureDetails> failures, RangerServiceDef serviceDef) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("==> 
RangerPolicyValidator.isValid(%s, %s, %s)", policyItems, failures, serviceDef));
+               }
+
+               boolean valid = true;
+               if (CollectionUtils.isEmpty(policyItems)) {
+                       LOG.debug("policy items collection was null/empty");
+               } else {
+                       for (RangerPolicyItem policyItem : policyItems) {
+                               if (policyItem == null) {
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
+                                               .field("policy item")
+                                               .isMissing()
+                                               .becauseOf("policy items object 
was null")
+                                               .build());
+                                       valid = false;
+                               } else {
+                                       // we want to go through all elements 
even though one may be bad so all failures are captured
+                                       valid = isValidPolicyItem(policyItem, 
failures, serviceDef) && valid;
+                               }
+                       }
+               }
+
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("<== 
RangerPolicyValidator.isValid(%s, %s, %s): %s", policyItems, failures, 
serviceDef, valid));
+               }
+               return valid;
+       }
+
+       boolean isValidPolicyItem(RangerPolicyItem policyItem, 
List<ValidationFailureDetails> failures, RangerServiceDef serviceDef) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("==> 
RangerPolicyValidator.isValid(%s, %s, %s)", policyItem, failures, serviceDef));
+               }
+               
+               boolean valid = true;
+               if (policyItem == null) {
+                       LOG.debug("policy item was null!");
+               } else {
+                       // access items collection can't be empty and should be 
otherwise valid
+                       if (CollectionUtils.isEmpty(policyItem.getAccesses())) {
+                               failures.add(new 
ValidationFailureDetailsBuilder()
+                                       .field("policy item accesses")
+                                       .isMissing()
+                                       .becauseOf("policy items accesses 
collection was null")
+                                       .build());
+                               valid = false;
+                       } else {
+                               valid = 
isValidItemAccesses(policyItem.getAccesses(), failures, serviceDef) && valid;
+                       }
+                       // both users and user-groups collections can't be empty
+                       if (CollectionUtils.isEmpty(policyItem.getUsers()) && 
CollectionUtils.isEmpty(policyItem.getGroups())) {
+                               failures.add(new 
ValidationFailureDetailsBuilder()
+                                       .field("policy item users/user-groups")
+                                       .isMissing()
+                                       .becauseOf("both users and user-groups 
collections on the policy item were null/empty")
+                                       .build());
+                               valid = false;
+                       }
+               }
+
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("<== 
RangerPolicyValidator.isValid(%s, %s, %s): %s", policyItem, failures, 
serviceDef, valid));
+               }
+               return valid;
+       }
+
+       boolean isValidItemAccesses(List<RangerPolicyItemAccess> accesses, 
List<ValidationFailureDetails> failures, RangerServiceDef serviceDef) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("==> 
RangerPolicyValidator.isValid(%s, %s, %s)", accesses, failures, serviceDef));
+               }
+               
+               boolean valid = true;
+               if (CollectionUtils.isEmpty(accesses)) {
+                       LOG.debug("policy item accesses collection was 
null/empty!");
+               } else {
+                       Set<String> accessTypes = getAccessTypes(serviceDef);
+                       for (RangerPolicyItemAccess access : accesses) {
+                               if (access == null) {
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
+                                               .field("policy item access")
+                                               .isMissing()
+                                               .becauseOf("policy items access 
object was null")
+                                               .build());
+                                       valid = false;
+                               } else {
+                                       // we want to go through all elements 
even though one may be bad so all failures are captured
+                                       valid = isValidPolicyItemAccess(access, 
failures, accessTypes) && valid;
+                               }
+                       }
+               }
+
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("<== 
RangerPolicyValidator.isValid(%s, %s): %s", accesses, failures, serviceDef, 
valid));
+               }
+               return valid;
+       }
+
+       boolean isValidPolicyItemAccess(RangerPolicyItemAccess access, 
List<ValidationFailureDetails> failures, Set<String> accessTypes) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("==> 
RangerPolicyValidator.isValidPolicyItemAccess(%s, %s, %s)", access, failures, 
accessTypes));
+               }
+               
+               boolean valid = true;
+               if (CollectionUtils.isEmpty(accessTypes)) { // caller should 
firewall this argument!
+                       LOG.debug("isValidPolicyItemAccess: accessTypes was 
null!");
+               } else if (access == null) {
+                       LOG.debug("isValidPolicyItemAccess: policy item access 
was null!");
+               } else {
+                       String accessType = access.getType();
+                       if (StringUtils.isBlank(accessType)) {
+                               failures.add(new 
ValidationFailureDetailsBuilder()
+                                       .field("policy item access type")
+                                       .isMissing()
+                                       .becauseOf("policy items access type's 
name was null/empty/blank")
+                                       .build());
+                               valid = false;
+                       } else {
+                               if (!accessTypes.contains(accessType)) {
+                                       String message = String.format("access 
type[%s] not among valid types for service[%s]", accessType, accessTypes);
+                                       LOG.debug(message);
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
+                                               .field("policy item access 
type")
+                                               .isSemanticallyIncorrect()
+                                               .becauseOf(message)
+                                               .build());
+                                       valid = false;
+                               }
+                       }
+                       Boolean isAllowed = access.getIsAllowed();
+                       // it can be null (which is treated as allowed) but not 
false
+                       if (isAllowed != null && isAllowed == false) {
+                               String message = "access type is set to deny.  
Currently deny access types are not supported.";
+                               LOG.debug(message);
+                               failures.add(new 
ValidationFailureDetailsBuilder()
+                                       .field("policy item access type 
allowed")
+                                       .isSemanticallyIncorrect()
+                                       .becauseOf(message)
+                                       .build());
+                               valid = false;
+                       }
+               }
+               
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("<== 
RangerPolicyValidator.isValidPolicyItemAccess(%s, %s, %s): %s", access, 
failures, accessTypes, valid));
+               }
+               return valid;
+       }
+}

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f721abec/security-admin/src/main/java/org/apache/ranger/rest/RangerServiceValidator.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/main/java/org/apache/ranger/rest/RangerServiceValidator.java
 
b/security-admin/src/main/java/org/apache/ranger/rest/RangerServiceValidator.java
index 08184c7..11e2682 100644
--- 
a/security-admin/src/main/java/org/apache/ranger/rest/RangerServiceValidator.java
+++ 
b/security-admin/src/main/java/org/apache/ranger/rest/RangerServiceValidator.java
@@ -19,6 +19,8 @@
 
 package org.apache.ranger.rest;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Set;
 
 import org.apache.commons.lang.StringUtils;
@@ -34,135 +36,147 @@ public class RangerServiceValidator extends 
RangerValidator {
 
        private static final Log LOG = 
LogFactory.getLog(RangerServiceValidator.class);
 
-       public RangerServiceValidator(ServiceStore store, Action action) {
-               super(store, action);
+       public RangerServiceValidator(ServiceStore store) {
+               super(store);
        }
 
-       public void validate(RangerService service) throws Exception {
+       public void validate(RangerService service, Action action) throws 
Exception {
                if(LOG.isDebugEnabled()) {
                        LOG.debug("==> RangerValidator.validate(" + service + 
")");
                }
 
-               if (isValid(service)) {
+               List<ValidationFailureDetails> failures = new 
ArrayList<ValidationFailureDetails>();
+               if (isValid(service, action, failures)) {
                        if(LOG.isDebugEnabled()) {
                                LOG.debug("<== RangerValidator.validate(" + 
service + "): valid");
                        }
                } else {
-                       String message = getFailureMessage();
+                       String message = serializeFailures(failures);
                        LOG.debug("<== RangerValidator.validate(" + service + 
"): invalid, reason[" + message + "]");
                        throw new Exception(message);
                }
        }
        
-       public void validate(long id) throws Exception {
-               if (isValid(id)) {
-                       if(LOG.isDebugEnabled()) {
-                               LOG.debug("<== RangerValidator.validate(" + id 
+ "): valid");
-                       }
-               } else {
-                       String message = getFailureMessage();
-                       LOG.debug("<== RangerValidator.validate(" + id + "): 
invalid, reason[" + message + "]");
-                       throw new Exception(message);
-               }
-       }
-       
-       boolean isValid(Long id) {
+       boolean isValid(Long id, Action action, List<ValidationFailureDetails> 
failures) {
                if(LOG.isDebugEnabled()) {
                        LOG.debug("==> RangerServiceValidator.isValid(" + id + 
")");
                }
 
-               if (_action != Action.DELETE) {
-                       addFailure(new ValidationFailureDetailsBuilder()
+               boolean valid = true;
+               if (action != Action.DELETE) {
+                       failures.add(new ValidationFailureDetailsBuilder()
                                .isAnInternalError()
                                .becauseOf("isValid(Long) is only supported for 
DELETE")
                                .build());
+                       valid = false;
                } else if (id == null) {
-                       addFailure(new ValidationFailureDetailsBuilder()
+                       failures.add(new ValidationFailureDetailsBuilder()
                                .field("id")
                                .isMissing()
                                .build());
-               } else {
-                       boolean found = false;
-                       try {
-                               if (_store.getService(id) != null) {
-                                       found = true;
-                               }
-                       } catch (Exception e) {
-                               LOG.debug("Encountred exception while 
retrieving service from service store!", e);
-                       }
-                       if (!found) {
-                               addFailure(new ValidationFailureDetailsBuilder()
-                                       .field("id")
-                                       .isSemanticallyIncorrect()
-                                       .becauseOf("no service found for id[" + 
id + "]")
-                                       .build());
-                       }
+                       valid = false;
+               } else if (getService(id) == null) {
+                       failures.add(new ValidationFailureDetailsBuilder()
+                               .field("id")
+                               .isSemanticallyIncorrect()
+                               .becauseOf("no service found for id[" + id + 
"]")
+                               .build());
+                       valid = false;
                }
 
                if(LOG.isDebugEnabled()) {
-                       LOG.debug("<== RangerServiceValidator.isValid(" + id + 
"): " + _valid);
+                       LOG.debug("<== RangerServiceValidator.isValid(" + id + 
"): " + valid);
                }
-               return _valid;
+               return valid;
        }
        
-       boolean isValid(RangerService service) {
+       boolean isValid(RangerService service, Action action, 
List<ValidationFailureDetails> failures) {
                if(LOG.isDebugEnabled()) {
                        LOG.debug("==> RangerServiceValidator.isValid(" + 
service + ")");
                }
+               if (!(action == Action.CREATE || action == Action.UPDATE)) {
+                       throw new 
IllegalArgumentException("isValid(RangerService, ...) is only supported for 
CREATE/UPDATE");
+               }
                
+               boolean valid = true;
                if (service == null) {
                        String message = "service object passed in was null";
                        LOG.debug(message);
-                       addFailure(new ValidationFailureDetailsBuilder()
+                       failures.add(new ValidationFailureDetailsBuilder()
                                .field("service")
                                .isMissing()
                                .becauseOf(message)
                                .build());
+                       valid = false;
                } else {
+                       Long id = service.getId();
+                       if (action == Action.UPDATE) { // id is ignored for 
CREATE
+                               if (id == null) {
+                                       String message = "service id was 
null/empty/blank"; 
+                                       LOG.debug(message);
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
+                                               .field("id")
+                                               .isMissing()
+                                               .becauseOf(message)
+                                               .build());
+                                       valid = false;
+                               } else if (getService(id) == null) {
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
+                                               .field("id")
+                                               .isSemanticallyIncorrect()
+                                               .becauseOf("no service exists 
with id[" + id +"]")
+                                               .build());
+                                       valid = false;
+                               }
+                       }
                        String name = service.getName();
-                       String type = service.getType();
                        boolean nameSpecified = StringUtils.isNotBlank(name);
-                       boolean typeSpecified = StringUtils.isNotBlank(type);
-                       RangerService existingService = null;
                        RangerServiceDef serviceDef = null;
                        if (!nameSpecified) {
-                               String message = "service name was 
null/empty/blank"; 
+                               String message = "service name was 
null/empty/blank[" + name + "]"; 
                                LOG.debug(message);
-                               addFailure(new ValidationFailureDetailsBuilder()
+                               failures.add(new 
ValidationFailureDetailsBuilder()
                                        .field("name")
                                        .isMissing()
                                        .becauseOf(message)
                                        .build());
+                               valid = false;
                        } else {
-                               existingService = getService(name);
-                               if (existingService != null && _action == 
Action.CREATE) {
-                                       addFailure(new 
ValidationFailureDetailsBuilder()
+                               RangerService otherService = getService(name);
+                               if (otherService != null && action == 
Action.CREATE) {
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
                                                .field("name")
                                                .isSemanticallyIncorrect()
-                                               .becauseOf("service with the 
same name already exists")
+                                               .becauseOf("service already 
exists with name[" + name + "]")
                                                .build());
-                               } else if (existingService == null && _action 
== Action.UPDATE) {
-                                       addFailure(new 
ValidationFailureDetailsBuilder()
-                                               .field("name")
+                                       valid = false;
+                               } else if (otherService != null && 
otherService.getId() !=null && otherService.getId() != id) {
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
+                                               .field("id/name")
                                                .isSemanticallyIncorrect()
-                                               .becauseOf("service with the 
same name doesn't exist")
+                                               .becauseOf("id/name conflict: 
service already exists with name[" + name + "], its id is [" + 
otherService.getId() + "]")
                                                .build());
+                                       valid = false;
                                }
                        }
+                       String type = service.getType();
+                       boolean typeSpecified = StringUtils.isNotBlank(type);
                        if (!typeSpecified) {
-                               addFailure(new ValidationFailureDetailsBuilder()
+                               failures.add(new 
ValidationFailureDetailsBuilder()
                                        .field("type")
                                        .isMissing()
                                        .becauseOf("service def was 
null/empty/blank")
                                        .build());
+                               valid = false;
                        } else {
                                serviceDef = getServiceDef(type);
                                if (serviceDef == null) {
-                                       addFailure(new 
ValidationFailureDetailsBuilder()
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
                                                .field("type")
                                                .isSemanticallyIncorrect()
-                                               .becauseOf("service def not 
found")
+                                               .becauseOf("service def not 
found for type[" + type + "]")
                                                .build());
+                                       valid = false;
                                }
                        }
                        if (nameSpecified && serviceDef != null) {
@@ -170,19 +184,20 @@ public class RangerServiceValidator extends 
RangerValidator {
                                Set<String> inputParameters = 
getServiceConfigParameters(service);
                                Set<String> missingParameters = 
Sets.difference(reqiredParameters, inputParameters);
                                if (!missingParameters.isEmpty()) {
-                                       addFailure(new 
ValidationFailureDetailsBuilder()
+                                       failures.add(new 
ValidationFailureDetailsBuilder()
                                                .field("configuration")
                                                
.subField(missingParameters.iterator().next()) // we return any one parameter!
                                                .isMissing()
                                                .becauseOf("required 
configuration parameter is missing")
                                                .build());
+                                       valid = false;
                                }
                        }
                }
                
                if(LOG.isDebugEnabled()) {
-                       LOG.debug("<== RangerServiceValidator.isValid(" + 
service + "): " + _valid);
+                       LOG.debug("<== RangerServiceValidator.isValid(" + 
service + "): " + valid);
                }
-               return _valid;
+               return valid;
        }
 }

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f721abec/security-admin/src/main/java/org/apache/ranger/rest/RangerValidator.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/main/java/org/apache/ranger/rest/RangerValidator.java 
b/security-admin/src/main/java/org/apache/ranger/rest/RangerValidator.java
index 3f25266..b6948dc 100644
--- a/security-admin/src/main/java/org/apache/ranger/rest/RangerValidator.java
+++ b/security-admin/src/main/java/org/apache/ranger/rest/RangerValidator.java
@@ -19,75 +19,100 @@
 
 package org.apache.ranger.rest;
 
+
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang.StringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.ranger.plugin.model.RangerPolicy;
 import org.apache.ranger.plugin.model.RangerService;
 import org.apache.ranger.plugin.model.RangerServiceDef;
+import org.apache.ranger.plugin.model.RangerServiceDef.RangerAccessTypeDef;
+import org.apache.ranger.plugin.model.RangerServiceDef.RangerResourceDef;
 import org.apache.ranger.plugin.model.RangerServiceDef.RangerServiceConfigDef;
 import org.apache.ranger.plugin.store.ServiceStore;
+import org.apache.ranger.plugin.util.SearchFilter;
 
 public abstract class RangerValidator {
        
        private static final Log LOG = LogFactory.getLog(RangerValidator.class);
 
        ServiceStore _store;
-       boolean _valid = true;
-       List<ValidationFailureDetails> _failures;
-       Action _action;
 
        public enum Action {
                CREATE, UPDATE, DELETE;
        };
        
-       protected RangerValidator(ServiceStore store, Action action) {
+       protected RangerValidator(ServiceStore store) {
                if (store == null) {
                        throw new IllegalArgumentException("ServiceValidator(): 
store is null!");
                }
                _store = store;
-               if (action == null) {
-                       throw new IllegalArgumentException("ServiceValidator(): 
action is null!");
-               }
-               _action = action;
        }
 
-       protected List<ValidationFailureDetails> getFailures() {
-               if (_valid) {
-                       LOG.warn("getFailureDetails: called while _valid == 
true");
+       public void validate(Long id, Action action) throws Exception {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("==> RangerValidator.validate(" + id + ")");
+               }
+
+               List<ValidationFailureDetails> failures = new 
ArrayList<ValidationFailureDetails>();
+               if (isValid(id, action, failures)) {
+                       if(LOG.isDebugEnabled()) {
+                               LOG.debug("<== RangerValidator.validate(" + id 
+ "): valid");
+                       }
+               } else {
+                       String message = serializeFailures(failures);
+                       LOG.debug("<== RangerValidator.validate(" + id + "): 
invalid, reason[" + message + "]");
+                       throw new Exception(message);
                }
-               return _failures;
        }
        
-       String getFailureMessage() {
-               if (_valid) {
-                       LOG.warn("getFailureDetails: called while validator is 
true!");
-               }
-               if (_failures == null) {
-                       LOG.warn("getFailureDetails: called while list of 
failures is null!");
-                       return null;
+       /**
+        * This method is expected to be overridden by sub-classes.  Default 
implementation provided to not burden implementers from having to implement 
methods that they know would never be called. 
+        * @param id
+        * @param action
+        * @param failures
+        * @return
+        */
+       boolean isValid(Long id, Action action, List<ValidationFailureDetails> 
failures) {
+               failures.add(new ValidationFailureDetailsBuilder()
+                               .isAnInternalError()
+                               .becauseOf("unimplemented method called")
+                               .build());
+               return false;
+       }
+
+       String serializeFailures(List<ValidationFailureDetails> failures) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("==> RangerValidator.getFailureMessage()");
                }
-               StringBuilder builder = new StringBuilder();
-               for (ValidationFailureDetails aFailure : _failures) {
-                       builder.append(aFailure.toString());
-                       builder.append(";");
+
+               String message = null;
+               if (CollectionUtils.isEmpty(failures)) {
+                       LOG.warn("serializeFailures: called while list of 
failures is null/empty!");
+               } else {
+                       StringBuilder builder = new StringBuilder();
+                       for (ValidationFailureDetails aFailure : failures) {
+                               builder.append(aFailure.toString());
+                               builder.append(";");
+                       }
+                       message = builder.toString();
                }
-               return builder.toString();
-       }
 
-       void addFailure(ValidationFailureDetails aFailure) {
-               if (_failures == null) {
-                       _failures = new ArrayList<ValidationFailureDetails>();
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("<== RangerValidator.serializeFailures(): " + 
message);
                }
-               _failures.add(aFailure);
-               _valid = false;
+               return message;
        }
-       
+
        Set<String> getServiceConfigParameters(RangerService service) {
                if (service == null || service.getConfigs() == null) {
                        return new HashSet<String>();
@@ -98,7 +123,7 @@ public abstract class RangerValidator {
 
        Set<String> getRequiredParameters(RangerServiceDef serviceDef) {
                if(LOG.isDebugEnabled()) {
-                       LOG.debug("==> 
RangerServiceValidator.getRequiredParameters(" + serviceDef + ")");
+                       LOG.debug("==> RangerValidator.getRequiredParameters(" 
+ serviceDef + ")");
                }
 
                Set<String> result;
@@ -119,7 +144,7 @@ public abstract class RangerValidator {
                }
 
                if(LOG.isDebugEnabled()) {
-                       LOG.debug("<== 
RangerServiceValidator.getRequiredParameters(" + serviceDef + "): " + result);
+                       LOG.debug("<== RangerValidator.getRequiredParameters(" 
+ serviceDef + "): " + result);
                }
                return result;
        }
@@ -127,7 +152,7 @@ public abstract class RangerValidator {
        RangerServiceDef getServiceDef(String type) {
                
                if(LOG.isDebugEnabled()) {
-                       LOG.debug("==> RangerServiceValidator.getServiceDef(" + 
type + ")");
+                       LOG.debug("==> RangerValidator.getServiceDef(" + type + 
")");
                }
                RangerServiceDef result = null;
                try {
@@ -137,7 +162,25 @@ public abstract class RangerValidator {
                }
                
                if(LOG.isDebugEnabled()) {
-                       LOG.debug("<== RangerServiceValidator.getServiceDef(" + 
type + "): " + result);
+                       LOG.debug("<== RangerValidator.getServiceDef(" + type + 
"): " + result);
+               }
+               return result;
+       }
+
+       RangerService getService(Long id) {
+               
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("==> RangerValidator.getService(" + id + ")");
+               }
+               RangerService result = null;
+               try {
+                       result = _store.getService(id);
+               } catch (Exception e) {
+                       LOG.debug("Encountred exception while retrieving 
service from service store!", e);
+               }
+               
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("<== RangerValidator.getService(" + id + "): 
" + result);
                }
                return result;
        }
@@ -145,7 +188,7 @@ public abstract class RangerValidator {
        RangerService getService(String name) {
                
                if(LOG.isDebugEnabled()) {
-                       LOG.debug("==> RangerServiceValidator.getService(" + 
name + ")");
+                       LOG.debug("==> RangerValidator.getService(" + name + 
")");
                }
                RangerService result = null;
                try {
@@ -155,8 +198,201 @@ public abstract class RangerValidator {
                }
                
                if(LOG.isDebugEnabled()) {
-                       LOG.debug("<== RangerServiceValidator.getService(" + 
name + "): " + result);
+                       LOG.debug("<== RangerValidator.getService(" + name + 
"): " + result);
                }
                return result;
        }
+
+       RangerPolicy getPolicy(Long id) {
+               
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("==> RangerValidator.getPolicy(" + id + ")");
+               }
+               RangerPolicy result = null;
+               try {
+                       result = _store.getPolicy(id);
+               } catch (Exception e) {
+                       LOG.debug("Encountred exception while retrieving policy 
from service store!", e);
+               }
+               
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("<== RangerValidator.getPolicy(" + id + "): " 
+ result);
+               }
+               return result;
+       }
+
+       List<RangerPolicy> getPolicies(final String policyName, final String 
serviceName) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("==> RangerValidator.getPolicies(" + 
policyName + ", " + serviceName + ")");
+               }
+
+               List<RangerPolicy> policies = null;
+               try {
+                       SearchFilter filter = new SearchFilter();
+                       filter.setParam(SearchFilter.POLICY_NAME, policyName);
+                       filter.setParam(SearchFilter.SERVICE_NAME, serviceName);
+                       
+                       policies = _store.getPolicies(filter);
+               } catch (Exception e) {
+                       LOG.debug("Encountred exception while retrieving 
service from service store!", e);
+               }
+               
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("<== RangerValidator.getPolicies(" + 
policyName + ", " + serviceName + "): " + policies);
+               }
+               return policies;
+       }
+
+       Set<String> getAccessTypes(RangerServiceDef serviceDef) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("==> RangerValidator.getAccessTypes(" + 
serviceDef + ")");
+               }
+
+               Set<String> accessTypes = new HashSet<String>();
+               if (serviceDef == null) {
+                       LOG.warn("serviceDef passed in was null!");
+               } else if 
(CollectionUtils.isEmpty(serviceDef.getAccessTypes())) {
+                       LOG.warn("AccessTypeDef collection on serviceDef was 
null!");
+               } else {
+                       for (RangerAccessTypeDef accessTypeDef : 
serviceDef.getAccessTypes()) {
+                               if (accessTypeDef == null) {
+                                       LOG.warn("Access type def was null!");
+                               } else {
+                                       String accessType = 
accessTypeDef.getName();
+                                       if (StringUtils.isBlank(accessType)) {
+                                               LOG.warn("Access type def name 
was null/empty/blank!");
+                                       } else {
+                                               accessTypes.add(accessType);
+                                       }
+                               }
+                       }
+               }
+
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("<== RangerValidator.getAccessTypes(" + 
serviceDef + "): " + accessTypes);
+               }
+               return accessTypes;
+       }
+       
+       /**
+        * This function exists to encapsulates the current behavior of code 
which treats and unspecified audit preference to mean audit is enabled.
+        * @param policy
+        * @return
+        */
+       boolean getIsAuditEnabled(RangerPolicy policy) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("<== RangerValidator.getIsAuditEnabled(" + 
policy + ")");
+               }
+
+               boolean isEnabled = false;
+               if (policy == null) {
+                       LOG.warn("policy was null!");
+               } else if (policy.getIsAuditEnabled() == null) {
+                       isEnabled = true;
+               } else {
+                       isEnabled = policy.getIsAuditEnabled();
+               }
+
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("<== RangerValidator.getIsAuditEnabled(" + 
policy + "): " + isEnabled);
+               }
+               return isEnabled;
+       }
+       
+       Set<String> getMandatoryResourceNames(RangerServiceDef serviceDef) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("==> 
RangerValidator.getMandatoryResourceNames(" + serviceDef + ")");
+               }
+
+               Set<String> resourceNames = new HashSet<String>();
+               if (serviceDef == null) {
+                       LOG.warn("serviceDef passed in was null!");
+               } else if (CollectionUtils.isEmpty(serviceDef.getResources())) {
+                       LOG.warn("ResourceDef collection on serviceDef was 
null!");
+               } else {
+                       for (RangerResourceDef resourceTypeDef : 
serviceDef.getResources()) {
+                               if (resourceTypeDef == null) {
+                                       LOG.warn("resource type def was null!");
+                               } else {
+                                       Boolean mandatory = 
resourceTypeDef.getMandatory();
+                                       if (mandatory != null && mandatory == 
true) {
+                                               String resourceName = 
resourceTypeDef.getName();
+                                               if 
(StringUtils.isBlank(resourceName)) {
+                                                       LOG.warn("Resource def 
name was null/empty/blank!");
+                                               } else {
+                                                       
resourceNames.add(resourceName);
+                                               }
+                                       }
+                               }
+                       }
+               }
+
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("<== 
RangerValidator.getMandatoryResourceNames(" + serviceDef + "): " + 
resourceNames);
+               }
+               return resourceNames;
+       }
+
+       Set<String> getAllResourceNames(RangerServiceDef serviceDef) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("==> RangerValidator.getAllResourceNames(" + 
serviceDef + ")");
+               }
+
+               Set<String> resourceNames = new HashSet<String>();
+               if (serviceDef == null) {
+                       LOG.warn("serviceDef passed in was null!");
+               } else if (CollectionUtils.isEmpty(serviceDef.getResources())) {
+                       LOG.warn("ResourceDef collection on serviceDef was 
null!");
+               } else {
+                       for (RangerResourceDef resourceTypeDef : 
serviceDef.getResources()) {
+                               if (resourceTypeDef == null) {
+                                       LOG.warn("resource type def was null!");
+                               } else {
+                                       String resourceName = 
resourceTypeDef.getName();
+                                       if (StringUtils.isBlank(resourceName)) {
+                                               LOG.warn("Resource def name was 
null/empty/blank!");
+                                       } else {
+                                               resourceNames.add(resourceName);
+                                       }
+                               }
+                       }
+               }
+
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug("<== RangerValidator.getAllResourceNames(" + 
serviceDef + "): " + resourceNames);
+               }
+               return resourceNames;
+       }
+       
+       Set<String> getPolicyResources(RangerPolicy policy) {
+               if (policy == null || policy.getResources() == null || 
policy.getResources().isEmpty()) {
+                       return new HashSet<String>();
+               } else {
+                       return policy.getResources().keySet();
+               }
+       }
+
+       Map<String, String> getValidationRegExes(RangerServiceDef serviceDef) {
+               if (serviceDef == null || 
CollectionUtils.isEmpty(serviceDef.getResources())) {
+                       return new HashMap<String, String>();
+               } else {
+                       Map<String, String> result = new HashMap<String, 
String>();
+                       for (RangerResourceDef resourceDef : 
serviceDef.getResources()) {
+                               if (resourceDef == null) {
+                                       LOG.warn("A resource def in resource 
def collection is null");
+                               } else {
+                                       String name = resourceDef.getName();
+                                       String regEx = 
resourceDef.getValidationRegEx();
+                                       if (StringUtils.isBlank(name)) {
+                                               LOG.warn("resource name is 
null/empty/blank");
+                                       } else if (StringUtils.isBlank(regEx)) {
+                                               LOG.debug("validation regex is 
null/empty/blank");
+                                       } else {
+                                               result.put(name, regEx);
+                                       }
+                               }
+                       }
+                       return result;
+               }
+       }
 }

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f721abec/security-admin/src/main/java/org/apache/ranger/rest/RangerValidatorFactory.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/main/java/org/apache/ranger/rest/RangerValidatorFactory.java
 
b/security-admin/src/main/java/org/apache/ranger/rest/RangerValidatorFactory.java
index 6c75a2f..01b0a7e 100644
--- 
a/security-admin/src/main/java/org/apache/ranger/rest/RangerValidatorFactory.java
+++ 
b/security-admin/src/main/java/org/apache/ranger/rest/RangerValidatorFactory.java
@@ -19,11 +19,15 @@
 
 package org.apache.ranger.rest;
 
+import org.apache.ranger.biz.ServiceDBStore;
 import org.apache.ranger.plugin.store.ServiceStore;
-import org.apache.ranger.rest.RangerValidator.Action;
 
 public class RangerValidatorFactory {
-       RangerServiceValidator getServiceValidator(ServiceStore store, Action 
action) {
-               return new RangerServiceValidator(store, action);
+       public RangerServiceValidator getServiceValidator(ServiceStore store) {
+               return new RangerServiceValidator(store);
+       }
+
+       public RangerPolicyValidator getPolicyValidator(ServiceDBStore store) {
+               return new RangerPolicyValidator(store);
        }
 }

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f721abec/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 821562f..e127b35 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
@@ -45,6 +45,13 @@ import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.ranger.admin.client.datatype.RESTResponse;
+import org.apache.ranger.biz.AssetMgr;
+import org.apache.ranger.biz.ServiceDBStore;
+import org.apache.ranger.biz.ServiceMgr;
+import org.apache.ranger.common.RESTErrorUtil;
+import org.apache.ranger.common.ServiceUtil;
+import org.apache.ranger.entity.XXPolicyExportAudit;
 import org.apache.ranger.plugin.model.RangerPolicy;
 import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItem;
 import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItemAccess;
@@ -68,15 +75,6 @@ import 
org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.stereotype.Component;
 import org.springframework.transaction.annotation.Propagation;
 import org.springframework.transaction.annotation.Transactional;
-import org.apache.ranger.admin.client.datatype.RESTResponse;
-import org.apache.ranger.biz.AssetMgr;
-import org.apache.ranger.biz.ServiceMgr;
-import org.apache.ranger.biz.ServiceDBStore;
-import org.apache.ranger.common.MessageEnums;
-import org.apache.ranger.common.PropertiesUtil;
-import org.apache.ranger.common.RESTErrorUtil;
-import org.apache.ranger.common.ServiceUtil;
-import org.apache.ranger.entity.XXPolicyExportAudit;
 
 
 @Path("plugins")
@@ -281,8 +279,8 @@ public class ServiceREST {
                RangerService ret = null;
 
                try {
-                       RangerServiceValidator validator = 
validatorFactory.getServiceValidator(svcStore, Action.CREATE);
-                       validator.validate(service);
+                       RangerServiceValidator validator = 
validatorFactory.getServiceValidator(svcStore);
+                       validator.validate(service, Action.CREATE);
                        
                        ret = svcStore.createService(service);
                } catch(Exception excp) {
@@ -310,8 +308,8 @@ public class ServiceREST {
                RangerService ret = null;
 
                try {
-                       RangerServiceValidator validator = 
validatorFactory.getServiceValidator(svcStore, Action.UPDATE);
-                       validator.validate(service);
+                       RangerServiceValidator validator = 
validatorFactory.getServiceValidator(svcStore);
+                       validator.validate(service, Action.UPDATE);
                        ret = svcStore.updateService(service);
                } catch(Exception excp) {
                        LOG.error("updateService(" + service + ") failed", 
excp);
@@ -336,8 +334,8 @@ public class ServiceREST {
                }
 
                try {
-                       RangerServiceValidator validator = 
validatorFactory.getServiceValidator(svcStore, Action.DELETE);
-                       validator.validate(id);
+                       RangerServiceValidator validator = 
validatorFactory.getServiceValidator(svcStore);
+                       validator.validate(id, Action.DELETE);
                        svcStore.deleteService(id);
                } catch(Exception excp) {
                        LOG.error("deleteService(" + id + ") failed", excp);
@@ -795,6 +793,8 @@ public class ServiceREST {
                RangerPolicy ret = null;
 
                try {
+                       RangerPolicyValidator validator = 
validatorFactory.getPolicyValidator(svcStore);
+                       validator.validate(policy, Action.CREATE);
                        ret = svcStore.createPolicy(policy);
                } catch(Exception excp) {
                        LOG.error("createPolicy(" + policy + ") failed", excp);
@@ -820,6 +820,8 @@ public class ServiceREST {
                RangerPolicy ret = null;
 
                try {
+                       RangerPolicyValidator validator = 
validatorFactory.getPolicyValidator(svcStore);
+                       validator.validate(policy, Action.UPDATE);
                        ret = svcStore.updatePolicy(policy);
                } catch(Exception excp) {
                        LOG.error("updatePolicy(" + policy + ") failed", excp);
@@ -843,6 +845,8 @@ public class ServiceREST {
                }
 
                try {
+                       RangerPolicyValidator validator = 
validatorFactory.getPolicyValidator(svcStore);
+                       validator.validate(id, Action.DELETE);
                        svcStore.deletePolicy(id);
                } catch(Exception excp) {
                        LOG.error("deletePolicy(" + id + ") failed", excp);

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f721abec/security-admin/src/test/java/org/apache/ranger/rest/TestRangerPolicyValidator.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/test/java/org/apache/ranger/rest/TestRangerPolicyValidator.java
 
b/security-admin/src/test/java/org/apache/ranger/rest/TestRangerPolicyValidator.java
new file mode 100644
index 0000000..4e15753
--- /dev/null
+++ 
b/security-admin/src/test/java/org/apache/ranger/rest/TestRangerPolicyValidator.java
@@ -0,0 +1,471 @@
+package org.apache.ranger.rest;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.ranger.plugin.model.RangerPolicy;
+import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItem;
+import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyItemAccess;
+import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyResource;
+import org.apache.ranger.plugin.model.RangerService;
+import org.apache.ranger.plugin.model.RangerServiceDef;
+import org.apache.ranger.plugin.model.RangerServiceDef.RangerResourceDef;
+import org.apache.ranger.plugin.store.ServiceStore;
+import org.apache.ranger.plugin.util.SearchFilter;
+import org.apache.ranger.rest.RangerValidator.Action;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Sets;
+
+public class TestRangerPolicyValidator {
+
+       @Before
+       public void setUp() throws Exception {
+               _store = mock(ServiceStore.class);
+               _policy = mock(RangerPolicy.class);
+               _validator = new RangerPolicyValidator(_store);
+               _serviceDef = mock(RangerServiceDef.class);
+       }
+       
+       final Action[] cu = new Action[] { Action.CREATE, Action.UPDATE };
+       final Object[] policyItemsData = new Object[] {
+                       ImmutableMap.of(  // all good
+                               "users", new String[] {"user1" ," user2"},
+                               "groups", new String[] {"group1", "group2"},
+                               "accesses", new String[] { "r", "w" },
+                               "isAllowed", new Boolean[] { true, true }),
+                       ImmutableMap.of(   // no users
+                               "groups", new String[] {"group3", "group4"},
+                               "accesses", new String[]{"w", "x"}, 
+                               "isAllowed", new Boolean[] { true, true }),
+                       ImmutableMap.of(   // no groups
+                               "users", new String[] {"user3" ," user4"}, 
+                               "accesses", new String[] { "r", "x" },
+                               "isAllowed", new Boolean[] { true, true }),
+                       ImmutableMap.of( // isallowed on access types is null
+                               "users", new String[] {"user7" ," user6"},
+                               "accesses", new String[] { "a" },
+                               "isAllowed", new Boolean[] { null, null })
+       };
+       String[] accessTypes = new String[] { "r", "w", "x", "a" };
+       String[] accessTypes_bad = new String[] { "r", "w", "xx", }; // two 
missing (x, a), one new that isn't on bad (xx)
+       
+       final Object[][] resourceDefData = new Object[][] {
+                       { "db", true, "db\\d+" },        // valid: db1, db22, 
db983, etc.; invalid: db, db12x, ttx11, etc.
+                       { "tbl", true, null },           // anything goes
+                       { "col", false, "col\\d{1,2}" }  // valid: col1, col47, 
etc.; invalid: col, col238, col1, etc.
+       };
+       
+       final Map<String, String[]> policyResourceMap_good = ImmutableMap.of(
+                       "db", new String[] { "db1", "db2" },
+                       "tbl", new String[] { "tbl1", "tbl2" } );
+       final Map<String, String[]> policyResourceMap_bad = ImmutableMap.of(
+                       "db", new String[] { "db1", "db2" },            // 
mandatory "tbl" missing
+                       "col", new String[] { "col12", "col 1" },       // 
wrong format of value for "col"
+                       "extra", new String[] { "extra1", "extra2" } ); // 
spurious "extra" specified
+
+       @Test
+       public final void testIsValid_long() throws Exception {
+               // this validation should be removed if we start supporting 
other than delete action
+               assertFalse(_validator.isValid(3L, Action.CREATE, _failures));
+               _utils.checkFailureForInternalError(_failures);
+               
+               // should fail with appropriate error message if id is null
+               _failures.clear(); _failures.clear(); 
assertFalse(_validator.isValid((Long)null, Action.DELETE, _failures));
+               _utils.checkFailureForMissingValue(_failures, "id");
+               
+               // should fail with appropriate error message if policy can't 
be found for the specified id
+               when(_store.getPolicy(1L)).thenReturn(null);
+               when(_store.getPolicy(2L)).thenThrow(new Exception());
+               RangerPolicy existingPolicy = mock(RangerPolicy.class);
+               when(_store.getPolicy(3L)).thenReturn(existingPolicy);
+               _failures.clear(); assertFalse(_validator.isValid(1L, 
Action.DELETE, _failures));
+               _utils.checkFailureForSemanticError(_failures, "id");
+               _failures.clear(); assertFalse(_validator.isValid(2L, 
Action.DELETE, _failures));
+               _utils.checkFailureForSemanticError(_failures, "id");
+
+               // if policy exists then delete validation should pass 
+               assertTrue(_validator.isValid(3L, Action.DELETE, _failures));
+       }
+       
+       @Test
+       public final void testIsValid_happyPath() throws Exception {
+               // valid policy has valid non-empty name and service name 
+               when(_policy.getService()).thenReturn("service-name");
+               // service name exists
+               RangerService service = mock(RangerService.class);
+               when(service.getType()).thenReturn("service-type");
+               
when(_store.getServiceByName("service-name")).thenReturn(service);
+               // service points to a valid service-def
+               _serviceDef = 
_utils.createServiceDefWithAccessTypes(accessTypes);
+               
when(_store.getServiceDefByName("service-type")).thenReturn(_serviceDef);
+               // a matching policy should exist for create when checked by id 
and not exist when checked by name.
+               when(_store.getPolicy(7L)).thenReturn(null);
+               RangerPolicy existingPolicy = mock(RangerPolicy.class);
+               when(existingPolicy.getId()).thenReturn(8L);
+               when(_store.getPolicy(8L)).thenReturn(existingPolicy);
+               SearchFilter createFilter = new SearchFilter();
+               createFilter.setParam(SearchFilter.POLICY_NAME, "service-type");
+               createFilter.setParam(SearchFilter.POLICY_NAME, 
"policy-name-1"); // this name would be used for create
+               when(_store.getPolicies(createFilter)).thenReturn(new 
ArrayList<RangerPolicy>());
+               // a matching policy should not exist for update.
+               SearchFilter updateFilter = new SearchFilter();
+               updateFilter.setParam(SearchFilter.POLICY_NAME, "service-type");
+               updateFilter.setParam(SearchFilter.POLICY_NAME, 
"policy-name-2"); // this name would be used for update
+               List<RangerPolicy> existingPolicies = new 
ArrayList<RangerPolicy>();
+               existingPolicies.add(existingPolicy);
+               
when(_store.getPolicies(updateFilter)).thenReturn(existingPolicies);
+               // valid policy can have empty set of policy items if audit is 
turned on
+               // null value for audit is treated as audit on.
+               for (Action action : cu) {
+                       for (Boolean auditEnabled : new Boolean[] { null, true 
} ) {
+                               
when(_policy.getIsAuditEnabled()).thenReturn(auditEnabled);
+                               if (action == Action.CREATE) {
+                                       when(_policy.getId()).thenReturn(7L);
+                                       
when(_policy.getName()).thenReturn("policy-name-1");
+                                       assertTrue("" + action + ", " + 
auditEnabled, _validator.isValid(_policy, action, _failures));
+                                       assertTrue(_failures.isEmpty());
+                               } else {
+                                       // update should work both when by-name 
is found or not, since nothing found by-name means name is being updated.
+                                       when(_policy.getId()).thenReturn(8L);
+                                       
when(_policy.getName()).thenReturn("policy-name-1");
+                                       assertTrue("" + action + ", " + 
auditEnabled, _validator.isValid(_policy, action, _failures));
+                                       assertTrue(_failures.isEmpty());
+
+                                       
when(_policy.getName()).thenReturn("policy-name-2");
+                                       assertTrue("" + action + ", " + 
auditEnabled, _validator.isValid(_policy, action, _failures));
+                                       assertTrue(_failures.isEmpty());
+                               }
+                       }
+               }
+               // if audit is disabled then policy should have policy items 
and all of them should be valid
+               List<RangerPolicyItem> policyItems = 
_utils.createPolicyItems(policyItemsData);
+               when(_policy.getPolicyItems()).thenReturn(policyItems);
+               when(_policy.getIsAuditEnabled()).thenReturn(false);
+               for (Action action : cu) {
+                       if (action == Action.CREATE) {
+                               when(_policy.getId()).thenReturn(7L);
+                               
when(_policy.getName()).thenReturn("policy-name-1");
+                       } else {
+                               when(_policy.getId()).thenReturn(8L);
+                               
when(_policy.getName()).thenReturn("policy-name-2");
+                       }
+                       assertTrue("" + action , _validator.isValid(_policy, 
action, _failures));
+                       assertTrue(_failures.isEmpty());
+               }
+               
+               // above succeeded as service def did not have any resources on 
it, mandatory or otherwise.
+               // policy should have all mandatory resources specified, and 
they should conform to the validation pattern in resource definition
+               List<RangerResourceDef> resourceDefs = 
_utils.createResourceDefs(resourceDefData);
+               when(_serviceDef.getResources()).thenReturn(resourceDefs);
+               Map<String, RangerPolicyResource> resourceMap = 
_utils.createPolicyResourceMap(policyResourceMap_good);
+               when(_policy.getResources()).thenReturn(resourceMap);
+
+               for (Action action : cu) {
+                       if (action == Action.CREATE) {
+                               when(_policy.getId()).thenReturn(7L);
+                               
when(_policy.getName()).thenReturn("policy-name-1");
+                       } else {
+                               when(_policy.getId()).thenReturn(8L);
+                               
when(_policy.getName()).thenReturn("policy-name-2");
+                       }
+                       assertTrue("" + action , _validator.isValid(_policy, 
action, _failures));
+                       assertTrue(_failures.isEmpty());
+               }
+       }
+       
+       void checkFailure_isValid(Action action, String errorType, String 
field) {
+               checkFailure_isValid(action, errorType, field, null);
+       }
+       
+       void checkFailure_isValid(Action action, String errorType, String 
field, String subField) {
+               _failures.clear();
+               assertFalse(_validator.isValid(_policy, action, _failures));
+               switch (errorType) {
+               case "missing":
+                       _utils.checkFailureForMissingValue(_failures, field, 
subField);
+                       break;
+               case "semantic":
+                       _utils.checkFailureForSemanticError(_failures, field, 
subField);
+                       break;
+               case "internal error":
+                       _utils.checkFailureForInternalError(_failures);
+                       break;
+               default:
+                       fail("Unsupported errorType[" + errorType + "]");
+                       break;
+               }
+       }
+       
+       @Test
+       public final void testIsValid_failures() throws Exception {
+               for (Action action : cu) {
+                       // passing in a null policy should fail with 
appropriate failure reason
+                       _policy = null;
+                       checkFailure_isValid(action, "missing", "policy");
+                       
+                       // policy must have a name on it
+                       _policy = mock(RangerPolicy.class);
+                       for (String name : new String[] { null, "  " }) {
+                               when(_policy.getName()).thenReturn(name);
+                               checkFailure_isValid(action, "missing", "name");
+                       }
+                       
+                       // for update id is required!
+                       if (action == Action.UPDATE) {
+                               when(_policy.getId()).thenReturn(null);
+                               checkFailure_isValid(action, "missing", "id");
+                       }
+               }
+               /*
+                * Id is ignored for Create but name should not belong to an 
existing policy.  For update, policy should exist for its id and should match 
its name.
+                */
+               when(_policy.getName()).thenReturn("policy-name");
+               when(_policy.getService()).thenReturn("service-name");
+
+               RangerPolicy existingPolicy = mock(RangerPolicy.class);
+               when(existingPolicy.getId()).thenReturn(7L);
+               List<RangerPolicy> existingPolicies = new 
ArrayList<RangerPolicy>();
+               existingPolicies.add(existingPolicy);
+               SearchFilter filter = new SearchFilter();
+               filter.setParam(SearchFilter.SERVICE_NAME, "service-name");
+               filter.setParam(SearchFilter.POLICY_NAME, "policy-name");
+               when(_store.getPolicies(filter)).thenReturn(existingPolicies);
+               checkFailure_isValid(Action.CREATE, "semantic", "name");
+               
+               // update : does not exist for id
+               when(_policy.getId()).thenReturn(7L);
+               when(_store.getPolicy(7L)).thenReturn(null);
+               checkFailure_isValid(Action.UPDATE, "semantic", "id");
+
+               // Update: name should not point to an existing different 
policy, i.e. with a different id
+               when(_store.getPolicy(7L)).thenReturn(existingPolicy);
+               RangerPolicy anotherExistingPolicy = mock(RangerPolicy.class);
+               when(anotherExistingPolicy.getId()).thenReturn(8L);
+               existingPolicies.clear();
+               existingPolicies.add(anotherExistingPolicy);
+               when(_store.getPolicies(filter)).thenReturn(existingPolicies);
+               checkFailure_isValid(Action.UPDATE, "semantic", "id/name");
+
+               // more than one policies with same name is also an internal 
error
+               when(_policy.getName()).thenReturn("policy-name");
+               when(_store.getPolicies(filter)).thenReturn(existingPolicies);
+               existingPolicies.add(existingPolicy);
+               existingPolicy = mock(RangerPolicy.class);
+               existingPolicies.add(existingPolicy);
+               _failures.clear(); assertFalse(_validator.isValid(_policy, 
Action.UPDATE, _failures));
+               _utils.checkFailureForInternalError(_failures);
+               
+               // policy must have service name on it and it should be valid
+               when(_policy.getName()).thenReturn("policy-name");
+               when(_store.getServiceByName("service-name")).thenReturn(null);
+               
when(_store.getServiceByName("another-service-name")).thenThrow(new 
Exception());
+
+               for (Action action : cu) {
+                       when(_policy.getService()).thenReturn("service-name");
+                       _failures.clear(); 
assertFalse(_validator.isValid(_policy, action, _failures));
+                       _utils.checkFailureForMissingValue(_failures, 
"service");
+
+                       
when(_policy.getService()).thenReturn("another-service-name");
+                       _failures.clear(); 
assertFalse(_validator.isValid(_policy, action, _failures));
+                       _utils.checkFailureForMissingValue(_failures, 
"service");
+               }
+               
+               // policy must contain at least one policy item
+               List<RangerPolicyItem> policyItems = new 
ArrayList<RangerPolicy.RangerPolicyItem>();
+               when(_policy.getService()).thenReturn("service-name");
+               RangerService service = mock(RangerService.class);
+               
when(_store.getServiceByName("service-name")).thenReturn(service);
+               for (Action action : cu) {
+                       // when it is null
+                       when(_policy.getPolicyItems()).thenReturn(null);
+                       _failures.clear(); 
assertFalse(_validator.isValid(_policy, action, _failures));
+                       _utils.checkFailureForMissingValue(_failures, "policy 
items");
+                       // or when it is not null but empty.
+                       when(_policy.getPolicyItems()).thenReturn(policyItems);
+                       _failures.clear(); 
assertFalse(_validator.isValid(_policy, action, _failures));
+                       _utils.checkFailureForMissingValue(_failures, "policy 
items");
+               }
+               
+               // these are known good policy items -- same as used above in 
happypath
+               policyItems = _utils.createPolicyItems(policyItemsData);
+               when(_policy.getPolicyItems()).thenReturn(policyItems);
+               // policy item check requires that service def should exist
+               when(service.getType()).thenReturn("service-type");
+               
when(_store.getServiceDefByName("service-type")).thenReturn(null);
+               for (Action action : cu) {
+                       _failures.clear(); 
assertFalse(_validator.isValid(_policy, action, _failures));
+                       _utils.checkFailureForInternalError(_failures, "policy 
service def");
+               }
+               
+               // service-def should contain the right access types on it.
+               _serviceDef = 
_utils.createServiceDefWithAccessTypes(accessTypes_bad);
+               
when(_store.getServiceDefByName("service-type")).thenReturn(_serviceDef);
+               for (Action action : cu) {
+                       _failures.clear(); 
assertFalse(_validator.isValid(_policy, action, _failures));
+                       _utils.checkFailureForSemanticError(_failures, "policy 
item access type");
+               }
+
+               // create the right service def with right resource defs - this 
is the same as in the happypath test above.
+               _serviceDef = 
_utils.createServiceDefWithAccessTypes(accessTypes);
+               when(_store.getPolicies(filter)).thenReturn(null);
+               List<RangerResourceDef> resourceDefs = 
_utils.createResourceDefs(resourceDefData);
+               when(_serviceDef.getResources()).thenReturn(resourceDefs);
+               
when(_store.getServiceDefByName("service-type")).thenReturn(_serviceDef);
+               // one mandtory is missing (tbl) and one unknown resource is 
specified (extra), and values of option resource don't conform to validation 
pattern (col)
+               Map<String, RangerPolicyResource> policyResources = 
_utils.createPolicyResourceMap(policyResourceMap_bad);
+               when(_policy.getResources()).thenReturn(policyResources);
+               for (Action action : cu) {
+                       _failures.clear(); 
assertFalse(_validator.isValid(_policy, action, _failures));
+                       _utils.checkFailureForMissingValue(_failures, 
"resources", "tbl"); // for missing resource: tbl
+                       _utils.checkFailureForSemanticError(_failures, 
"resources", "extra"); // for spurious resource: "extra"
+                       _utils.checkFailureForSemanticError(_failures, 
"resource-values", "col"); // for spurious resource: "extra"
+               }
+       }
+       
+       @Test
+       public void test_isValidResourceValues() {
+               List<RangerResourceDef> resourceDefs = 
_utils.createResourceDefs(resourceDefData);
+               when(_serviceDef.getResources()).thenReturn(resourceDefs);
+               Map<String, RangerPolicyResource> policyResources = 
_utils.createPolicyResourceMap(policyResourceMap_bad);
+               assertFalse(_validator.isValidResourceValues(policyResources, 
_failures, _serviceDef));
+               _utils.checkFailureForSemanticError(_failures, 
"resource-values", "col");
+               
+               policyResources = 
_utils.createPolicyResourceMap(policyResourceMap_good);
+               assertTrue(_validator.isValidResourceValues(policyResources, 
_failures, _serviceDef));
+       }
+       
+       @Test
+       public void test_isValidPolicyItems_failures() {
+               // null/empty list is good because there is nothing
+               assertTrue(_validator.isValidPolicyItems(null, _failures, 
_serviceDef));
+               _failures.isEmpty();
+
+               List<RangerPolicyItem> policyItems = new 
ArrayList<RangerPolicy.RangerPolicyItem>();
+               assertTrue(_validator.isValidPolicyItems(policyItems, 
_failures, _serviceDef));
+               _failures.isEmpty();
+               
+               // null elements in the list are flagged
+               policyItems.add(null);
+               assertFalse(_validator.isValidPolicyItems(policyItems, 
_failures, _serviceDef));
+               _utils.checkFailureForMissingValue(_failures, "policy item");
+       }
+       
+       @Test
+       public void test_isValidPolicyItem_failures() {
+
+               // empty access collections are invalid
+               RangerPolicyItem policyItem = mock(RangerPolicyItem.class);
+               when(policyItem.getAccesses()).thenReturn(null);
+               _failures.clear(); 
assertFalse(_validator.isValidPolicyItem(policyItem, _failures, _serviceDef));
+               _utils.checkFailureForMissingValue(_failures, "policy item 
accesses");
+
+               List<RangerPolicyItemAccess> accesses = new 
ArrayList<RangerPolicy.RangerPolicyItemAccess>();
+               when(policyItem.getAccesses()).thenReturn(accesses);
+               _failures.clear(); 
assertFalse(_validator.isValidPolicyItem(policyItem, _failures, _serviceDef));
+               _utils.checkFailureForMissingValue(_failures, "policy item 
accesses");
+               
+               // both user and groups can't be null
+               RangerPolicyItemAccess access = 
mock(RangerPolicyItemAccess.class);
+               accesses.add(access);
+               when(policyItem.getUsers()).thenReturn(null);
+               when(policyItem.getGroups()).thenReturn(new 
ArrayList<String>());
+               _failures.clear(); 
assertFalse(_validator.isValidPolicyItem(policyItem, _failures, _serviceDef));
+               _utils.checkFailureForMissingValue(_failures, "policy item 
users/user-groups");
+       }
+       
+       @Test
+       public void test_isValidItemAccesses_happyPath() {
+               
+               // happy path
+               Object[][] data = new Object[][] {
+                               { "a", null }, // valid
+                               { "b", true }, // valid
+                               { "c", true }, // valid
+               };
+               List<RangerPolicyItemAccess> accesses = 
_utils.createItemAccess(data);
+               _serviceDef = _utils.createServiceDefWithAccessTypes(new 
String[] { "a", "b", "c", "d" });
+               assertTrue(_validator.isValidItemAccesses(accesses, _failures, 
_serviceDef));
+               assertTrue(_failures.isEmpty());
+       }
+       
+       @Test
+       public void test_isValidItemAccesses_failure() {
+
+               // null policy item access values are an error
+               List<RangerPolicyItemAccess> accesses = new 
ArrayList<RangerPolicyItemAccess>();
+               accesses.add(null);
+               _failures.clear(); 
assertFalse(_validator.isValidItemAccesses(accesses, _failures, _serviceDef));
+               _utils.checkFailureForMissingValue(_failures, "policy item 
access");
+
+               // all items must be valid for this call to be valid
+               Object[][] data = new Object[][] {
+                               { "a", null }, // valid
+                               { null, null }, // invalid - name can't be null
+                               { "c", true }, // valid
+               };
+               accesses = _utils.createItemAccess(data);
+               _serviceDef = _utils.createServiceDefWithAccessTypes(new 
String[] { "a", "b", "c", "d" });
+               _failures.clear(); 
assertFalse(_validator.isValidItemAccesses(accesses, _failures, _serviceDef));
+       }
+       
+       @Test
+       public void test_isValidPolicyItemAccess_happyPath() {
+               
+               RangerPolicyItemAccess access = 
mock(RangerPolicyItemAccess.class);
+               when(access.getType()).thenReturn("anAccess"); // valid
+
+               Set<String> validAccesses = Sets.newHashSet(new String[] { 
"anAccess", "anotherAccess" });
+               
+               // both null or true access types are the same and valid
+               for (Boolean allowed : new Boolean[] { null, true } ) {
+                       when(access.getIsAllowed()).thenReturn(allowed);
+                       assertTrue(_validator.isValidPolicyItemAccess(access, 
_failures, validAccesses));
+                       assertTrue(_failures.isEmpty());
+               }
+       }
+       
+       @Test
+       public void test_isValidPolicyItemAccess_failures() {
+               
+               Set<String> validAccesses = Sets.newHashSet(new String[] { 
"anAccess", "anotherAccess" });
+               // null/empty names are invalid
+               RangerPolicyItemAccess access = 
mock(RangerPolicyItemAccess.class);
+               when(access.getIsAllowed()).thenReturn(null); // valid since 
null == true
+               for (String type : new String[] { null, "       "}) {
+                       when(access.getType()).thenReturn(type); // invalid
+                       // null/empty validAccess set skips all checks
+                       assertTrue(_validator.isValidPolicyItemAccess(access, 
_failures, null));
+                       assertTrue(_validator.isValidPolicyItemAccess(access, 
_failures, new HashSet<String>()));
+                       _failures.clear(); 
assertFalse(_validator.isValidPolicyItemAccess(access, _failures, 
validAccesses));
+                       _utils.checkFailureForMissingValue(_failures, "policy 
item access type");
+               }
+               
+               when(access.getType()).thenReturn("anAccess"); // valid
+               when(access.getIsAllowed()).thenReturn(false); // invalid
+               
_failures.clear();assertFalse(_validator.isValidPolicyItemAccess(access, 
_failures, validAccesses));
+               _utils.checkFailureForSemanticError(_failures, "policy item 
access type allowed");
+               
+               when(access.getType()).thenReturn("newAccessType"); // invalid
+               _failures.clear(); 
assertFalse(_validator.isValidPolicyItemAccess(access, _failures, 
validAccesses));
+               _utils.checkFailureForSemanticError(_failures, "policy item 
access type");
+       }
+
+       private ValidationTestUtils _utils = new ValidationTestUtils();
+       private List<ValidationFailureDetails> _failures = new 
ArrayList<ValidationFailureDetails>();
+       private ServiceStore _store;
+       private RangerPolicy _policy;
+       private RangerPolicyValidator _validator;
+       private RangerServiceDef _serviceDef;
+}

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f721abec/security-admin/src/test/java/org/apache/ranger/rest/TestRangerServiceValidator.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/test/java/org/apache/ranger/rest/TestRangerServiceValidator.java
 
b/security-admin/src/test/java/org/apache/ranger/rest/TestRangerServiceValidator.java
index 3bbb123..3bad133 100644
--- 
a/security-admin/src/test/java/org/apache/ranger/rest/TestRangerServiceValidator.java
+++ 
b/security-admin/src/test/java/org/apache/ranger/rest/TestRangerServiceValidator.java
@@ -25,6 +25,7 @@ import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
@@ -46,122 +47,88 @@ public class TestRangerServiceValidator {
        public void before() {
                _store = mock(ServiceStore.class);
                _action = Action.CREATE; // by default we set action to create
-               _validator = new RangerServiceValidator(_store, _action);
+               _validator = new RangerServiceValidator(_store);
        }
 
+       void checkFailure_isValid(RangerServiceValidator validator, 
RangerService service, Action action, List<ValidationFailureDetails> failures, 
String errorType, String field) {
+               checkFailure_isValid(validator, service, action, failures, 
errorType, field, null);
+       }
+       
+       void checkFailure_isValid(RangerServiceValidator validator, 
RangerService service, Action action, List<ValidationFailureDetails> failures, 
String errorType, String field, String subField) {
+               failures.clear();
+               assertFalse(validator.isValid(service, action, failures));
+               switch (errorType) {
+               case "missing":
+                       _utils.checkFailureForMissingValue(failures, field, 
subField);
+                       break;
+               case "semantic":
+                       _utils.checkFailureForSemanticError(failures, field, 
subField);
+                       break;
+               case "internal error":
+                       _utils.checkFailureForInternalError(failures);
+                       break;
+               default:
+                       fail("Unsupported errorType[" + errorType + "]");
+                       break;
+               }
+       }
+       
        @Test
        public void testIsValid_failures() throws Exception {
                RangerService service = mock(RangerService.class);
-               List<ValidationFailureDetails> failures;
-               // create/update/delete: null, empty of blank name renders a 
service invalid
-               for (Action action : cud) {
-                       _validator = new RangerServiceValidator(_store, action);
-                       when(service.getName()).thenReturn(null);
-                       assertFalse(_validator.isValid(service));
-                       // let's verify the sort of error information that is 
returned (for one of these)
-                       failures = _validator.getFailures();
-                       // assert that among the failure reason is one about 
field name being missing.
-                       boolean found = false;
-                       for (ValidationFailureDetails f : failures) {
-                               if ("name".equals(f.getFieldName()) && 
-                                               f._internalError == false && 
-                                               f._missing == true &&
-                                               f._semanticError == false) {
-                                       found = true;
-                               }
-                       }
-                       assertTrue("Matching failure located", found);
-                       // let's assert behavior for other flavors of this 
condition, too.
-                       when(service.getName()).thenReturn("");
-                       assertFalse(_validator.isValid(service));
-                       when(service.getName()).thenReturn("  "); // spaces
-                       assertFalse(_validator.isValid(service));
-               }
-               
-               // Create/update: null, empty or blank type is also invalid
+               // passing in a null service to the check itself is an error
+               assertFalse(_validator.isValid((RangerService)null, _action, 
_failures));
+               _utils.checkFailureForMissingValue(_failures, "service");
+
+               // id is required for update
+               when(service.getId()).thenReturn(null);
+               // let's verify the failure and the sort of error information 
that is returned (for one of these)
+               // assert that among the failure reason is one about id being 
missing.
+               checkFailure_isValid(_validator, service, Action.UPDATE, 
_failures, "missing", "id");
+               when(service.getId()).thenReturn(7L);
+
                for (Action action : cu) {
-                       _validator = new RangerServiceValidator(_store, action);
-                       when(service.getName()).thenReturn("aName");
-                       when(service.getType()).thenReturn(null);
-                       assertFalse(_validator.isValid(service));
-                       when(service.getType()).thenReturn("");
-                       assertFalse(_validator.isValid(service));
-                       when(service.getType()).thenReturn("    "); // a tab
-                       assertFalse(_validator.isValid(service));
-                       // let's verify the error information returned (for the 
last scenario)
-                       failures = _validator.getFailures();
-                       boolean found = false;
-                       for (ValidationFailureDetails f : failures) {
-                               if ("type".equals(f._fieldName) && 
-                                               f._missing == true && 
-                                               f._semanticError == false) {
-                                       found = true;
-                               }
+                       // null, empty of blank name renders a service invalid
+                       for (String name : new String[] { null, "", "   " }) { 
// spaces and tabs
+                               when(service.getName()).thenReturn(name);
+                               checkFailure_isValid(_validator, service, 
action, _failures, "missing", "name");
+                       }
+                       // same is true for the type
+                       for (String type : new String[] { null, "", "    " }) {
+                               when(service.getType()).thenReturn(type);
+                               checkFailure_isValid(_validator, service, 
action, _failures, "missing", "type");
                        }
-                       assertTrue("Matching failure located", found);
                }
+               when(service.getName()).thenReturn("aName");
 
-               // Create/update: if non-empty, the type should also exist!
+               // if non-empty, then the type should exist!
+               when(_store.getServiceDefByName("null-type")).thenReturn(null);
+               when(_store.getServiceDefByName("throwing-type")).thenThrow(new 
Exception());
                for (Action action : cu) {
-                       _validator = new RangerServiceValidator(_store, action);
-                       when(service.getName()).thenReturn("aName");
-                       when(service.getType()).thenReturn("aType");
-                       
when(_store.getServiceDefByName("aType")).thenReturn(null);
-                       assertFalse(_validator.isValid(service));
-                       // let's verify the error information returned (for the 
last scenario)
-                       failures = _validator.getFailures();
-                       boolean found = false;
-                       for (ValidationFailureDetails f : failures) {
-                               if ("type".equals(f._fieldName) && 
-                                               f._missing == false && 
-                                               f._semanticError == true) {
-                                       found = true;
-                               }
+                       for (String type : new String[] { "null-type", 
"throwing-type" }) {
+                               when(service.getType()).thenReturn(type);
+                               checkFailure_isValid(_validator, service, 
action, _failures, "semantic", "type");
                        }
-                       assertTrue("Matching failure located", found);
                }
-               
-               // Create: if service already exists then that such a service 
should be considered invalid by create
-               when(service.getName()).thenReturn("aName");
                when(service.getType()).thenReturn("aType");
                RangerServiceDef serviceDef = mock(RangerServiceDef.class);
                
when(_store.getServiceDefByName("aType")).thenReturn(serviceDef);
-               // test both when service exists and when it doesn't -- the 
result is opposite for the two cases
-               RangerService existingService = mock(RangerService.class);
-               
when(_store.getServiceByName("aName")).thenReturn(existingService);
-
-               _validator = new RangerServiceValidator(_store, Action.CREATE);
-               assertFalse(_validator.isValid(service));
-               
-               // check the error returned: it is a semantic error about 
service's name
-               failures = _validator.getFailures();
-               boolean found = false;
-               for (ValidationFailureDetails f : failures) {
-                       if ("name".equals(f._fieldName) && 
-                                       f._missing == false && 
-                                       f._semanticError == true) {
-                               found = true;
-                       }
-               }
-               assertTrue("Matching failure located", found);
-               
-               // Update: Exact inverse is true, i.e. service must exist!
-               when(_store.getServiceByName("anotherName")).thenReturn(null);
-               when(service.getName()).thenReturn("anotherName");
                
-               _validator = new RangerServiceValidator(_store, Action.UPDATE);
-               assertFalse(_validator.isValid(service));
-               // check the error returned: it is a semantic error about 
service's name
-               failures = _validator.getFailures();
-               found = false;
-               for (ValidationFailureDetails f : failures) {
-                       if ("name".equals(f._fieldName) && 
-                                       f._missing == false && 
-                                       f._semanticError == true) {
-                               found = true;
-                       }
-               }
-               assertTrue("Matching failure located", found);
+               // Create: No service should exist matching its id and/or name
+               RangerService anExistingService = mock(RangerService.class);
+               
when(_store.getServiceByName("aName")).thenReturn(anExistingService);
+               checkFailure_isValid(_validator, service, Action.CREATE, 
_failures, "semantic", "name");
+
+               // Update: service should exist matching its id and name 
specified should not belong to a different service
+               when(_store.getService(7L)).thenReturn(null);
+               
when(_store.getServiceByName("aName")).thenReturn(anExistingService);
+               checkFailure_isValid(_validator, service, Action.UPDATE, 
_failures, "semantic", "id");
+
+               when(_store.getService(7L)).thenReturn(anExistingService);
+               RangerService anotherExistingService = 
mock(RangerService.class);
+               when(anotherExistingService.getId()).thenReturn(49L);
+               
when(_store.getServiceByName("aName")).thenReturn(anotherExistingService);
+               checkFailure_isValid(_validator, service, Action.UPDATE, 
_failures, "semantic", "id/name");
        }
        
        @Test
@@ -190,21 +157,7 @@ public class TestRangerServiceValidator {
                when(_store.getServiceByName("aService")).thenReturn(null);
                for (Action action : cu) {
                        // it should be invalid
-                       _validator = new RangerServiceValidator(_store, action);
-                       assertFalse(_validator.isValid(service));
-                       // check the error message
-                       List<ValidationFailureDetails> failures = 
_validator.getFailures();
-                       boolean found = false;
-                       for (ValidationFailureDetails f : failures) {
-                               if ("configuration".equals(f.getFieldName()) &&
-                                               
"param2".equals(f._subFieldName) &&
-                                               f._missing == true &&
-                                               f._internalError == false && 
-                                               f._semanticError == false) {
-                                       found = true;
-                               }
-                       }
-                       assertTrue(found);
+                       checkFailure_isValid(_validator, service, action, 
_failures, "missing", "configuration", "param2");
                }
        }
 
@@ -230,33 +183,56 @@ public class TestRangerServiceValidator {
                Map<String, String> configMap = _utils.createMap(configs);  
                when(service.getConfigs()).thenReturn(configMap);
                // wire then into the store
-               try {
-                       // service does not exists
-                       when(_store.getServiceByName("aName")).thenReturn(null);
-                       // service def exists
-                       
when(_store.getServiceDefByName("aType")).thenReturn(serviceDef);
-               } catch (Exception e) {
-                       e.printStackTrace();
-                       fail("Unexpected error encountered while mocking!");
-               }
-               _validator = new RangerServiceValidator(_store, Action.CREATE);
-               assertTrue(_validator.isValid(service));
-               // for update to work the only additional requirement is that 
service should exist
+               // service does not exists
+               when(_store.getServiceByName("aName")).thenReturn(null);
+               // service def exists
+               
when(_store.getServiceDefByName("aType")).thenReturn(serviceDef);
+
+               assertTrue(_validator.isValid(service, Action.CREATE, 
_failures));
+
+               // for update to work the only additional requirement is that 
id is required and service should exist
+               // if name is not null and it points to a service then it 
should match the id
+               when(service.getId()).thenReturn(7L);
                RangerService existingService = mock(RangerService.class);
+               when(existingService.getId()).thenReturn(7L);
+               when(_store.getService(7L)).thenReturn(existingService);
                
when(_store.getServiceByName("aName")).thenReturn(existingService);
-               _validator = new RangerServiceValidator(_store, Action.UPDATE);
-               assertTrue(_validator.isValid(service));
+               assertTrue(_validator.isValid(service, Action.UPDATE, 
_failures));
+               // name need not point to a service for update to work, of 
course.
+               when(_store.getServiceByName("aName")).thenReturn(null);
+               assertTrue(_validator.isValid(service, Action.UPDATE, 
_failures));
        }
 
-       ValidationFailureDetails getFailure(List<ValidationFailureDetails> 
failures) {
-               if (failures == null || failures.size() == 0) {
-                       return null;
-               } else {
-                       return failures.iterator().next();
-               }
+       @Test
+       public void test_isValid_withId_errorConditions() throws Exception {
+               // api that takes in long is only supported for delete currently
+               assertFalse(_validator.isValid(1L, Action.CREATE, _failures));
+               _utils.checkFailureForInternalError(_failures);
+               // passing in a null id is a failure!
+               _validator = new RangerServiceValidator(_store);
+               _failures.clear(); assertFalse(_validator.isValid((Long)null, 
Action.DELETE, _failures));
+               _utils.checkFailureForMissingValue(_failures, "id");
+               // if service with that id does not exist then that, too, is a 
failure
+               when(_store.getService(1L)).thenReturn(null);
+               when(_store.getService(2L)).thenThrow(new Exception());
+               _failures.clear(); assertFalse(_validator.isValid(1L, 
Action.DELETE, _failures));
+               _utils.checkFailureForSemanticError(_failures, "id");
+
+               _failures.clear(); assertFalse(_validator.isValid(2L, 
Action.DELETE, _failures));
+               _utils.checkFailureForSemanticError(_failures, "id");
        }
+       
+       @Test
+       public void test_isValid_withId_happyPath() throws Exception {
+               _validator = new RangerServiceValidator(_store);
+               RangerService service = mock(RangerService.class);
+               when(_store.getService(1L)).thenReturn(service);
+               assertTrue(_validator.isValid(1L, Action.DELETE, _failures));
+       }
+       
        private ServiceStore _store;
        private RangerServiceValidator _validator;
        private Action _action;
        private ValidationTestUtils _utils = new ValidationTestUtils();
+       private List<ValidationFailureDetails> _failures = new 
ArrayList<ValidationFailureDetails>();
 }

Reply via email to