Repository: ranger
Updated Branches:
  refs/heads/ranger-0.7 7d40b35a2 -> ac456e84c


RANGER-2218: Added validations for names duing service def updates


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

Branch: refs/heads/ranger-0.7
Commit: ac456e84c848cf3378ee97c95b4a381bbdd6f703
Parents: 7d40b35
Author: Abhay Kulkarni <[email protected]>
Authored: Thu Sep 27 17:03:50 2018 -0700
Committer: Abhay Kulkarni <[email protected]>
Committed: Thu Sep 27 17:03:50 2018 -0700

----------------------------------------------------------------------
 .../validation/RangerServiceDefValidator.java   | 123 +++++++++++++++++--
 .../TestRangerServiceDefValidator.java          |  36 +++---
 .../java/org/apache/ranger/biz/XUserMgr.java    |  11 +-
 .../org/apache/ranger/biz/TestXUserMgr.java     |   1 +
 4 files changed, 145 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ranger/blob/ac456e84/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
----------------------------------------------------------------------
diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
 
b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
index 79ac674..709c396 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
@@ -26,6 +26,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.HashMap;
 
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang.StringUtils;
@@ -40,6 +41,7 @@ import 
org.apache.ranger.plugin.model.RangerServiceDef.RangerEnumElementDef;
 import 
org.apache.ranger.plugin.model.RangerServiceDef.RangerPolicyConditionDef;
 import org.apache.ranger.plugin.model.RangerServiceDef.RangerResourceDef;
 import org.apache.ranger.plugin.model.RangerServiceDef.RangerServiceConfigDef;
+import org.apache.ranger.plugin.model.RangerServiceDef.RangerDataMaskTypeDef;
 import org.apache.ranger.plugin.store.ServiceStore;
 
 import com.google.common.collect.ImmutableSet;
@@ -130,8 +132,8 @@ public class RangerServiceDefValidator extends 
RangerValidator {
                        Long id = serviceDef.getId();
                        valid = isValidServiceDefId(id, action, failures) && 
valid;
                        valid = isValidServiceDefName(serviceDef.getName(), id, 
action, failures) && valid;
-                       valid = isValidAccessTypes(serviceDef.getAccessTypes(), 
failures) && valid;
-                       if (isValidResources(serviceDef, failures)) {
+                       valid = isValidAccessTypes(serviceDef.getId(), 
serviceDef.getAccessTypes(), failures, action) && valid;
+                       if (isValidResources(serviceDef, failures, action)) {
                                // Semantic check of resource graph can only be 
done if resources are "syntactically" valid
                                valid = isValidResourceGraph(serviceDef, 
failures) && valid;
                        } else {
@@ -144,7 +146,8 @@ public class RangerServiceDefValidator extends 
RangerValidator {
                        } else {
                                valid = false;
                        }
-                       valid = 
isValidPolicyConditions(serviceDef.getPolicyConditions(), failures) && valid;
+                       valid = isValidPolicyConditions(serviceDef.getId(), 
serviceDef.getPolicyConditions(), failures, action) && valid;
+                       valid = isValidDataMaskTypes(serviceDef.getId(), 
serviceDef.getDataMaskDef().getMaskTypes(), failures, action) && valid;
                }
                
                if(LOG.isDebugEnabled()) {
@@ -231,11 +234,10 @@ public class RangerServiceDefValidator extends 
RangerValidator {
                return valid;
        }
        
-       boolean isValidAccessTypes(final List<RangerAccessTypeDef> 
accessTypeDefs, final List<ValidationFailureDetails> failures) {
+       boolean isValidAccessTypes(final Long serviceDefId, final 
List<RangerAccessTypeDef> accessTypeDefs, final List<ValidationFailureDetails> 
failures, final Action action) {
                if(LOG.isDebugEnabled()) {
                        LOG.debug(String.format("==> 
RangerServiceDefValidator.isValidAccessTypes(%s, %s)", accessTypeDefs, 
failures));
                }
-               
                boolean valid = true;
                if (CollectionUtils.isEmpty(accessTypeDefs)) {
                        ValidationErrorCode error = 
ValidationErrorCode.SERVICE_DEF_VALIDATION_ERR_MISSING_FIELD;
@@ -247,13 +249,32 @@ public class RangerServiceDefValidator extends 
RangerValidator {
                                .build());
                        valid = false;
                } else {
+                       Map<Long, String> existingAccessTypeIDNameMap = new 
HashMap<>();
+                       if (action == Action.UPDATE) {
+                               List<RangerAccessTypeDef> existingAccessTypes = 
this.getServiceDef(serviceDefId).getAccessTypes();
+                               for (RangerAccessTypeDef existingAccessType : 
existingAccessTypes) {
+                                       
existingAccessTypeIDNameMap.put(existingAccessType.getItemId(), 
existingAccessType.getName());
+                               }
+                       }
+                       LOG.debug("accessType names from db = " + 
existingAccessTypeIDNameMap.values());
+
                        List<RangerAccessTypeDef> defsWithImpliedGrants = new 
ArrayList<RangerAccessTypeDef>();
                        Set<String> accessNames = new HashSet<String>();
                        Set<Long> ids = new HashSet<Long>();
                        for (RangerAccessTypeDef def : accessTypeDefs) {
                                String name = def.getName();
+                               Long itemId = def.getItemId();
+                               LOG.debug("accessType name from input = " + 
name);
                                valid = isUnique(name, accessNames, "access 
type name", "access types", failures) && valid;
                                valid = isUnique(def.getItemId(), ids, "access 
type itemId", "access types", failures) && valid;
+                               if (action == Action.UPDATE) {
+                                       if 
(existingAccessTypeIDNameMap.get(itemId) != null && 
!existingAccessTypeIDNameMap.get(itemId).equals(name)) {
+                                               ValidationErrorCode error;
+                                               error = 
ValidationErrorCode.SERVICE_DEF_VALIDATION_ERR_SERVICE_DEF_NAME_CONFICT;
+                                               failures.add((new 
ValidationFailureDetailsBuilder()).field("access type 
name").isSemanticallyIncorrect().errorCode(error.getErrorCode()).becauseOf(String.format("changing
 %s[%s] in %s is not supported", "access type name", name, "access 
types")).build());
+                                               valid = false;
+                                       }
+                               }
                                if 
(CollectionUtils.isNotEmpty(def.getImpliedGrants())) {
                                        defsWithImpliedGrants.add(def);
                                }
@@ -295,7 +316,7 @@ public class RangerServiceDefValidator extends 
RangerValidator {
                return valid;
        }
 
-       boolean isValidPolicyConditions(List<RangerPolicyConditionDef> 
policyConditions, List<ValidationFailureDetails> failures) {
+       boolean isValidPolicyConditions(Long serviceDefId, 
List<RangerPolicyConditionDef> policyConditions, List<ValidationFailureDetails> 
failures, final Action action) {
                if(LOG.isDebugEnabled()) {
                        LOG.debug(String.format("==> 
RangerServiceDefValidator.isValidPolicyConditions(%s, %s)", policyConditions, 
failures));
                }
@@ -304,12 +325,31 @@ public class RangerServiceDefValidator extends 
RangerValidator {
                if (CollectionUtils.isEmpty(policyConditions)) {
                        LOG.debug("Configs collection was null/empty! ok");
                } else {
+                       Map<Long, String> existingPolicyCondIDNameMap = new 
HashMap<>();
+                       if (action == Action.UPDATE) {
+                               List<RangerPolicyConditionDef> 
existingPolicyConditions = 
this.getServiceDef(serviceDefId).getPolicyConditions();
+                               for (RangerPolicyConditionDef 
existingPolicyCondition : existingPolicyConditions) {
+                                       
existingPolicyCondIDNameMap.put(existingPolicyCondition.getItemId(), 
existingPolicyCondition.getName());
+                               }
+                       }
+                       LOG.debug("policy condition names from db = " + 
existingPolicyCondIDNameMap.values());
+
                        Set<Long> ids = new HashSet<Long>();
                        Set<String> names = new HashSet<String>();
                        for (RangerPolicyConditionDef conditionDef : 
policyConditions) {
-                               valid = isUnique(conditionDef.getItemId(), ids, 
"policy condition def itemId", "policy condition defs", failures) && valid;
                                String name = conditionDef.getName();
+                               Long itemId = conditionDef.getItemId();
+                               LOG.debug("policy condition name from input = " 
+ name);
+                               valid = isUnique(itemId, ids, "policy condition 
def itemId", "policy condition defs", failures) && valid;
                                valid = isUnique(name, names, "policy condition 
def name", "policy condition defs", failures) && valid;
+                               if (action == Action.UPDATE) {
+                                       if 
(existingPolicyCondIDNameMap.get(itemId) != null && 
!existingPolicyCondIDNameMap.get(itemId).equals(name)) {
+                                               ValidationErrorCode error;
+                                               error = 
ValidationErrorCode.SERVICE_DEF_VALIDATION_ERR_SERVICE_DEF_NAME_CONFICT;
+                                               failures.add((new 
ValidationFailureDetailsBuilder()).field("policy condition def 
name").isSemanticallyIncorrect().errorCode(error.getErrorCode()).becauseOf(String.format("changing
 %s[%s] in %s is not supported", "policy condition def name", name, "policy 
condition defs")).build());
+                                               valid = false;
+                                       }
+                               }
                                if 
(StringUtils.isBlank(conditionDef.getEvaluator())) {
                                        ValidationErrorCode error = 
ValidationErrorCode.SERVICE_DEF_VALIDATION_ERR_POLICY_CONDITION_NULL_EVALUATOR;
                                        failures.add(new 
ValidationFailureDetailsBuilder()
@@ -445,7 +485,7 @@ public class RangerServiceDefValidator extends 
RangerValidator {
                return valid;
        }
 
-       boolean isValidResources(RangerServiceDef serviceDef, 
List<ValidationFailureDetails> failures) {
+       boolean isValidResources(RangerServiceDef serviceDef, 
List<ValidationFailureDetails> failures, final Action action) {
                if(LOG.isDebugEnabled()) {
                        LOG.debug(String.format("==> 
RangerServiceDefValidator.isValidResources(%s, %s)", serviceDef, failures));
                }
@@ -462,14 +502,34 @@ public class RangerServiceDefValidator extends 
RangerValidator {
                                        .build());
                        valid = false;
                } else {
+                       Map<Long, String> existingResourceIDNameMap = new 
HashMap<>();
+                       if (action == Action.UPDATE) {
+                               List<RangerResourceDef> existingResources = 
this.getServiceDef(serviceDef.getId()).getResources();
+                               for (RangerResourceDef existingResource : 
existingResources) {
+                                       
existingResourceIDNameMap.put(existingResource.getItemId(), 
existingResource.getName());
+                               }
+                       }
+                       LOG.debug("resource names from db = " + 
existingResourceIDNameMap.values());
+
                        Set<String> names = new 
HashSet<String>(resources.size());
                        Set<Long> ids = new HashSet<Long>(resources.size());
                        for (RangerResourceDef resource : resources) {
                                /*
                                 * While id is the natural key, name is a 
surrogate key.  At several places code expects resource name to be unique 
within a service.
                                 */
-                               valid = isUnique(resource.getName(), names, 
"resource name", "resources", failures) && valid;
-                               valid = isUnique(resource.getItemId(), ids, 
"resource itemId", "resources", failures) && valid;
+                               String name = resource.getName();
+                               Long itemId = resource.getItemId();
+                               LOG.debug("resource name from input = " + name);
+                               valid = isUnique(name, names, "resource name", 
"resources", failures) && valid;
+                               valid = isUnique(itemId, ids, "resource 
itemId", "resources", failures) && valid;
+                               if (action == Action.UPDATE) {
+                                       if 
(existingResourceIDNameMap.get(itemId) != null && 
!existingResourceIDNameMap.get(itemId).equals(name)) {
+                                               ValidationErrorCode error;
+                                               error = 
ValidationErrorCode.SERVICE_DEF_VALIDATION_ERR_SERVICE_DEF_NAME_CONFICT;
+                                               failures.add((new 
ValidationFailureDetailsBuilder()).field("resource 
name").isSemanticallyIncorrect().errorCode(error.getErrorCode()).becauseOf(String.format("changing
 %s[%s] in %s is not supported", "resource name", name, "resources")).build());
+                                               valid = false;
+                                       }
+                               }
                        }
                }
 
@@ -611,4 +671,47 @@ public class RangerServiceDefValidator extends 
RangerValidator {
                }
                return valid;
        }
+
+       boolean isValidDataMaskTypes(Long serviceDefId, 
List<RangerDataMaskTypeDef> dataMaskTypes, List<ValidationFailureDetails> 
failures, final Action action) {
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("==> 
RangerServiceDefValidator.isValidDataMaskTypes(%s, %s)", dataMaskTypes, 
failures));
+               }
+               boolean valid = true;
+
+               if (CollectionUtils.isEmpty(dataMaskTypes)) {
+                       LOG.debug("Configs collection was null/empty! ok");
+               } else {
+                       Map<Long, String> existingDataMaskTypeIDNameMap = new 
HashMap<>();
+                       if (action == Action.UPDATE) {
+                               List<RangerDataMaskTypeDef> 
existingDataMaskTypes = 
this.getServiceDef(serviceDefId).getDataMaskDef().getMaskTypes();
+                               for (RangerDataMaskTypeDef existingDataMaskType 
: existingDataMaskTypes) {
+                                       
existingDataMaskTypeIDNameMap.put(existingDataMaskType.getItemId(), 
existingDataMaskType.getName());
+                               }
+                       }
+                       LOG.debug("data mask type names from db = " + 
existingDataMaskTypeIDNameMap.values());
+
+                       Set<Long> ids = new HashSet<Long>();
+                       Set<String> names = new HashSet<String>();
+                       for (RangerDataMaskTypeDef dataMaskType : 
dataMaskTypes) {
+                               String name = dataMaskType.getName();
+                               Long itemId = dataMaskType.getItemId();
+                               LOG.debug("data mask type name from input = " + 
name);
+                               valid = isUnique(itemId, ids, "data mask type 
def itemId", "data mask type defs", failures) && valid;
+                               valid = isUnique(name, names, "data mask type 
def name", "data mask type defs", failures) && valid;
+                               if (action == Action.UPDATE) {
+                                       if 
(existingDataMaskTypeIDNameMap.get(itemId) != null && 
!existingDataMaskTypeIDNameMap.get(itemId).equals(name)) {
+                                               ValidationErrorCode error;
+                                               error = 
ValidationErrorCode.SERVICE_DEF_VALIDATION_ERR_SERVICE_DEF_NAME_CONFICT;
+                                               failures.add((new 
ValidationFailureDetailsBuilder()).field("data mask type def 
name").isSemanticallyIncorrect().errorCode(error.getErrorCode()).becauseOf(String.format("changing
 %s[%s] in %s is not supported", "data mask type def name", name, "data mask 
type defs")).build());
+                                               valid = false;
+                                       }
+                               }
+                       }
+               }
+
+               if(LOG.isDebugEnabled()) {
+                       LOG.debug(String.format("<== 
RangerServiceDefValidator.isValidDataMaskTypes(%s, %s): %s", dataMaskTypes, 
failures, valid));
+               }
+               return valid;
+       }
 }

http://git-wip-us.apache.org/repos/asf/ranger/blob/ac456e84/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java
----------------------------------------------------------------------
diff --git 
a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java
 
b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java
index 33e6f4a..370f549 100644
--- 
a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java
+++ 
b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java
@@ -211,46 +211,50 @@ public class TestRangerServiceDefValidator {
 
        @Test
        public final void test_isValidAccessTypes_happyPath() {
+               long id = 7;
+               when(_serviceDef.getId()).thenReturn(id);
                List<RangerAccessTypeDef> input = 
_utils.createAccessTypeDefs(accessTypes_good);
-               assertTrue(_validator.isValidAccessTypes(input, _failures));
+               assertTrue(_validator.isValidAccessTypes(id, input, _failures, 
Action.CREATE));
                assertTrue(_failures.isEmpty());
        }
        
        @Test
        public final void test_isValidAccessTypes_failures() {
+               long id = 7;
+               when(_serviceDef.getId()).thenReturn(id);
                // null or empty access type defs
                List<RangerAccessTypeDef> accessTypeDefs = null;
-               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(accessTypeDefs, _failures));
+               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(id, accessTypeDefs, _failures, 
Action.CREATE));
                _utils.checkFailureForMissingValue(_failures, "access types");
                
                accessTypeDefs = new ArrayList<RangerAccessTypeDef>();
-               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(accessTypeDefs, _failures));
+               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(id, accessTypeDefs, _failures, 
Action.CREATE));
                _utils.checkFailureForMissingValue(_failures, "access types");
 
                // null/empty access types
                accessTypeDefs = _utils.createAccessTypeDefs(new String[] { 
null, "", "         " });
-               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(accessTypeDefs, _failures));
+               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(id, accessTypeDefs, _failures, 
Action.CREATE));
                _utils.checkFailureForMissingValue(_failures, "access type 
name");
                
                // duplicate access types
                accessTypeDefs = _utils.createAccessTypeDefs(new String[] { 
"read", "write", "execute", "read" } );
-               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(accessTypeDefs, _failures));
+               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(id, accessTypeDefs, _failures, 
Action.CREATE));
                _utils.checkFailureForSemanticError(_failures, "access type 
name", "read");
                
                // duplicate access types - case-insensitive
                accessTypeDefs = _utils.createAccessTypeDefs(new String[] { 
"read", "write", "execute", "READ" } );
-               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(accessTypeDefs, _failures));
+               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(id, accessTypeDefs, _failures, 
Action.CREATE));
                _utils.checkFailureForSemanticError(_failures, "access type 
name", "READ");
                
                // unknown access type in implied grants list
                accessTypeDefs = 
_utils.createAccessTypeDefs(accessTypes_bad_unknownType);
-               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(accessTypeDefs, _failures));
+               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(id, accessTypeDefs, _failures, 
Action.CREATE));
                _utils.checkFailureForSemanticError(_failures, "implied 
grants", "execute");
                _utils.checkFailureForSemanticError(_failures, "access type 
itemId", "1"); // id 1 is duplicated
                
                // access type with implied grant referring to itself
                accessTypeDefs = 
_utils.createAccessTypeDefs(accessTypes_bad_selfReference);
-               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(accessTypeDefs, _failures));
+               _failures.clear(); 
assertFalse(_validator.isValidAccessTypes(id, accessTypeDefs, _failures, 
Action.CREATE));
                _utils.checkFailureForSemanticError(_failures, "implied 
grants", "admin");
        }
        
@@ -396,16 +400,16 @@ public class TestRangerServiceDefValidator {
        public final void test_isValidResources() {
                // null/empty resources are an error
                when(_serviceDef.getResources()).thenReturn(null);
-               _failures.clear(); 
assertFalse(_validator.isValidResources(_serviceDef, _failures));
+               _failures.clear(); 
assertFalse(_validator.isValidResources(_serviceDef, _failures, Action.CREATE));
                _utils.checkFailureForMissingValue(_failures, "resources");
                
                List<RangerResourceDef> resources = new 
ArrayList<RangerResourceDef>();
                when(_serviceDef.getResources()).thenReturn(resources);
-               _failures.clear(); 
assertFalse(_validator.isValidResources(_serviceDef, _failures));
+               _failures.clear(); 
assertFalse(_validator.isValidResources(_serviceDef, _failures, Action.CREATE));
                _utils.checkFailureForMissingValue(_failures, "resources");
                
                
resources.addAll(_utils.createResourceDefsWithIds(invalidResources));
-               _failures.clear(); 
assertFalse(_validator.isValidResources(_serviceDef, _failures));
+               _failures.clear(); 
assertFalse(_validator.isValidResources(_serviceDef, _failures, Action.CREATE));
                _utils.checkFailureForMissingValue(_failures, "resource name");
                _utils.checkFailureForMissingValue(_failures, "resource 
itemId");
                _utils.checkFailureForSemanticError(_failures, "resource 
itemId", "1"); // id 1 is duplicate
@@ -456,7 +460,7 @@ public class TestRangerServiceDefValidator {
                };
                List<RangerResourceDef> resources = 
_utils.createResourceDefsWithIds(data);
                when(_serviceDef.getResources()).thenReturn(resources);
-               assertTrue(_validator.isValidResources(_serviceDef, _failures));
+               assertTrue(_validator.isValidResources(_serviceDef, _failures, 
Action.CREATE));
                assertTrue(_failures.isEmpty());
        }
 
@@ -490,11 +494,13 @@ public class TestRangerServiceDefValidator {
 
        @Test
        public final void test_isValidPolicyConditions() {
+               long id = 7;
+               when(_serviceDef.getId()).thenReturn(id);
                // null/empty policy conditions are ok
-               assertTrue(_validator.isValidPolicyConditions(null, _failures));
+               assertTrue(_validator.isValidPolicyConditions(id,null, 
_failures, Action.CREATE));
                assertTrue(_failures.isEmpty());
                List<RangerPolicyConditionDef> conditionDefs = new 
ArrayList<RangerServiceDef.RangerPolicyConditionDef>();
-               assertTrue(_validator.isValidPolicyConditions(conditionDefs, 
_failures));
+               assertTrue(_validator.isValidPolicyConditions(id, 
conditionDefs, _failures, Action.CREATE));
                assertTrue(_failures.isEmpty());
                
                Object[][] policyCondition_data = {
@@ -506,7 +512,7 @@ public class TestRangerServiceDefValidator {
                };
                
                
conditionDefs.addAll(_utils.createPolicyConditionDefs(policyCondition_data));
-               _failures.clear(); 
assertFalse(_validator.isValidPolicyConditions(conditionDefs, _failures));
+               _failures.clear(); 
assertFalse(_validator.isValidPolicyConditions(id, conditionDefs, _failures, 
Action.CREATE));
                _utils.checkFailureForMissingValue(_failures, "policy condition 
def itemId");
                _utils.checkFailureForMissingValue(_failures, "policy condition 
def name");
                _utils.checkFailureForMissingValue(_failures, "policy condition 
def evaluator");

http://git-wip-us.apache.org/repos/asf/ranger/blob/ac456e84/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java
----------------------------------------------------------------------
diff --git a/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 
b/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java
index 4c01d57..410e3f8 100644
--- a/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java
+++ b/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java
@@ -353,11 +353,15 @@ public class XUserMgr extends XUserMgrBase {
                checkAccess(vXUser.getName());
                VXPortalUser oldUserProfile = 
userMgr.getUserProfileByLoginId(vXUser
                                .getName());
+               if (oldUserProfile == null) {
+                       throw restErrorUtil.createRESTException(
+                                       "user " + vXUser.getName() + " does not 
exist.",
+                                       MessageEnums.INVALID_INPUT_DATA);
+               }
                VXPortalUser vXPortalUser = new VXPortalUser();
                if (oldUserProfile != null && oldUserProfile.getId() != null) {
                        vXPortalUser.setId(oldUserProfile.getId());
                }
-               // TODO : There is a possibility that old user may not exist.
 
                vXPortalUser.setFirstName(vXUser.getFirstName());
                if("null".equalsIgnoreCase(vXPortalUser.getFirstName())){
@@ -841,6 +845,11 @@ public class XUserMgr extends XUserMgrBase {
        public VXGroup updateXGroup(VXGroup vXGroup) {
                checkAdminAccess();
                XXGroup xGroup = 
daoManager.getXXGroup().getById(vXGroup.getId());
+               if (vXGroup != null && xGroup != null && 
!vXGroup.getName().equals(xGroup.getName())) {
+                       throw restErrorUtil.createRESTException(
+                                       "group name updates are not allowed.",
+                                       MessageEnums.INVALID_INPUT_DATA);
+               }
                List<XXTrxLog> trxLogList = 
xGroupService.getTransactionLog(vXGroup,
                                xGroup, "update");
                xaBizUtil.createTrxLog(trxLogList);

http://git-wip-us.apache.org/repos/asf/ranger/blob/ac456e84/security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java 
b/security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java
index 0279883..826307e 100644
--- a/security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java
+++ b/security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java
@@ -477,6 +477,7 @@ public class TestXUserMgr {
                vXGroup.setName("grouptest");
 
                XXGroup xxGroup = new XXGroup();
+               xxGroup.setName("grouptest");
                Mockito.when(daoManager.getXXGroup()).thenReturn(xxGroupDao);
                
Mockito.when(xxGroupDao.getById(vXGroup.getId())).thenReturn(xxGroup);
                
Mockito.when(xGroupService.updateResource(vXGroup)).thenReturn(vXGroup);

Reply via email to