Repository: ranger Updated Branches: refs/heads/master 12a27e7c7 -> f2e148abb
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/f2e148ab Tree: http://git-wip-us.apache.org/repos/asf/ranger/tree/f2e148ab Diff: http://git-wip-us.apache.org/repos/asf/ranger/diff/f2e148ab Branch: refs/heads/master Commit: f2e148abbe473a5fa23419373897082a3bf63974 Parents: 12a27e7 Author: Sailaja Polavarapu <[email protected]> Authored: Wed Sep 26 15:06:26 2018 -0700 Committer: Sailaja Polavarapu <[email protected]> Committed: Wed Sep 26 15:06:26 2018 -0700 ---------------------------------------------------------------------- .../validation/RangerServiceDefValidator.java | 126 +++++++++++++++++-- .../TestRangerServiceDefValidator.java | 38 +++--- .../java/org/apache/ranger/biz/XUserMgr.java | 11 +- 3 files changed, 149 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ranger/blob/f2e148ab/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 d73210e..45821e8 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; @@ -137,8 +139,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 { @@ -151,7 +153,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()) { @@ -238,7 +241,8 @@ 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)); } @@ -254,13 +258,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()); + } + } + if(LOG.isDebugEnabled()) { + LOG.debug("accessType names from db = " + existingAccessTypeIDNameMap.values()); + } List<RangerAccessTypeDef> defsWithImpliedGrants = new ArrayList<>(); Set<String> accessNames = new HashSet<>(); Set<Long> ids = new HashSet<>(); for (RangerAccessTypeDef def : accessTypeDefs) { String name = def.getName(); + Long itemId = def.getItemId(); 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); } @@ -302,7 +325,8 @@ 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)); } @@ -311,12 +335,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()); + } + } + if(LOG.isDebugEnabled()) { + LOG.debug("policy condition names from db = " + existingPolicyCondIDNameMap.values()); + } Set<Long> ids = new HashSet<>(); Set<String> names = new HashSet<>(); for (RangerPolicyConditionDef conditionDef : policyConditions) { - valid = isUnique(conditionDef.getItemId(), ids, "policy condition def itemId", "policy condition defs", failures) && valid; + Long itemId = conditionDef.getItemId(); + valid = isUnique(itemId, ids, "policy condition def itemId", "policy condition defs", failures) && valid; String name = conditionDef.getName(); 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() @@ -452,7 +495,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)); } @@ -469,6 +512,17 @@ 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()); + } + } + if(LOG.isDebugEnabled()) { + 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) { @@ -477,8 +531,18 @@ public class RangerServiceDefValidator extends RangerValidator { /* * 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(); + 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; + } + } } } @@ -640,4 +704,48 @@ 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()); + } + } + if(LOG.isDebugEnabled()) { + 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(); + 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/f2e148ab/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 decf07c..f4e29c7 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<>(); - _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"); } @@ -403,23 +407,23 @@ 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<>(); 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 _utils.checkFailureForSemanticError(_failures, "resource name", "DataBase"); resources.clear(); resources.addAll(_utils.createResourceDefsWithIds(mixedCaseResources)); - _failures.clear(); assertFalse(_validator.isValidResources(_serviceDef, _failures)); + _failures.clear(); assertFalse(_validator.isValidResources(_serviceDef, _failures, Action.CREATE)); _utils.checkFailure(_failures, null, null, null, "DBase",null); _utils.checkFailure(_failures, null, null, null, "TABLE",null); _utils.checkFailure(_failures, null, null, null, "Column",null); @@ -499,7 +503,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()); } @@ -533,11 +537,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<>(); - assertTrue(_validator.isValidPolicyConditions(conditionDefs, _failures)); + assertTrue(_validator.isValidPolicyConditions(id, conditionDefs, _failures, Action.CREATE)); assertTrue(_failures.isEmpty()); Object[][] policyCondition_data = { @@ -549,7 +555,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/f2e148ab/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 b1ea280..23b3fe4 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 @@ -347,11 +347,15 @@ public class XUserMgr extends XUserMgrBase { xaBizUtil.blockAuditorRoleUser(); 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())){ @@ -855,6 +859,11 @@ public class XUserMgr extends XUserMgrBase { checkAdminAccess(); xaBizUtil.blockAuditorRoleUser(); 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);
