This is an automated email from the ASF dual-hosted git repository.
spolavarapu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ranger.git
The following commit(s) were added to refs/heads/master by this push:
new bf25b9b RANGER-3207: Incorporate some of the review comments
bf25b9b is described below
commit bf25b9be7880fc70afae2ead42303671c341f9b8
Author: Sailaja Polavarapu <spolavar...@cloudera.com>
AuthorDate: Tue Mar 23 10:24:22 2021 -0700
RANGER-3207: Incorporate some of the review comments
---
.../main/java/org/apache/ranger/biz/XUserMgr.java | 28 ++++++++++++++++--
.../org/apache/ranger/service/XUserService.java | 2 +-
.../unixusersync/config/UserGroupSyncConfig.java | 13 +++++++++
.../process/PolicyMgrUserGroupBuilder.java | 34 +++++++++++++++++-----
4 files changed, 65 insertions(+), 12 deletions(-)
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 ceb8208..b903955 100755
--- a/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java
+++ b/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java
@@ -2643,8 +2643,8 @@ public class XUserMgr extends XUserMgrBase {
if (vXUser == null || vXUser.getName() == null
||
"null".equalsIgnoreCase(vXUser.getName())
|| vXUser.getName().trim().isEmpty()) {
- throw restErrorUtil.createRESTException("Please
provide a valid "
- + "username.",
MessageEnums.INVALID_INPUT_DATA);
+ logger.warn("Ignoring invalid username " +
vXUser==null? null : vXUser.getName());
+ continue;
}
checkAccess(vXUser.getName());
TransactionTemplate txTemplate = new
TransactionTemplate(txManager);
@@ -2795,6 +2795,12 @@ public class XUserMgr extends XUserMgrBase {
@Transactional(readOnly = false, propagation = Propagation.REQUIRED)
public int createOrUpdateXGroups(VXGroupList groups) {
for (VXGroup vXGroup : groups.getList()) {
+ if (vXGroup == null || vXGroup.getName() == null
+ || "null".equalsIgnoreCase(vXGroup.getName())
+ || vXGroup.getName().trim().isEmpty()) {
+ logger.warn("Ignoring invalid groupname " + vXGroup==null?
null : vXGroup.getName());
+ continue;
+ }
createXGroupWithoutLogin(vXGroup);
}
try {
@@ -2820,7 +2826,16 @@ public class XUserMgr extends XUserMgrBase {
if (CollectionUtils.isNotEmpty(groupUserInfo.getDelUsers())) {
for (String username : groupUserInfo.getDelUsers()) {
- deleteXGroupAndXUser(groupName, username);
+ try {
+ deleteXGroupAndXUser(groupName,
username);
+ } catch (Exception excp) {
+ logger.warn("Ignoring invalid
group/user found while updating group memberships group = "
+ + groupName + " and
user = " + username);
+ if (logger.isDebugEnabled()) {
+
logger.debug("createOrDeleteXGroupUsers(): Ignoring invalid group/user found
while updating group memberships "
+ + groupName + "
and " + username, excp);
+ }
+ }
}
}
}
@@ -3084,7 +3099,14 @@ public class XUserMgr extends XUserMgrBase {
}
logger.info("xUser.getName() = " + xUser.getName() + "
vXUser.getName() = " + vXUser.getName());
vXUser.setId(xUser.getId());
+ try {
vXUser = xUserService.updateResource(vXUser);
+ } catch (Exception ex) {
+ logger.warn("Failed to update username " +
vXUser.getName());
+ if (logger.isDebugEnabled()) {
+ logger.debug("Failed to update username " +
vXUser.getName(), ex);
+ }
+ }
vXUser.setUserRoleList(roleList);
if (oldUserProfile != null) {
if (oldUserProfile.getUserSource() ==
RangerCommonEnums.USER_APP) {
diff --git
a/security-admin/src/main/java/org/apache/ranger/service/XUserService.java
b/security-admin/src/main/java/org/apache/ranger/service/XUserService.java
index adb8e60..8566905 100644
--- a/security-admin/src/main/java/org/apache/ranger/service/XUserService.java
+++ b/security-admin/src/main/java/org/apache/ranger/service/XUserService.java
@@ -128,7 +128,7 @@ public class XUserService extends XUserServiceBase<XXUser,
VXUser> {
XXUser xUser =
daoManager.getXXUser().findByUserName(vObj.getName());
if (xUser != null) {
- throw restErrorUtil.createRESTException("XUser already
exists",
+ throw restErrorUtil.createRESTException(vObj.getName()
+ " already exists",
MessageEnums.ERROR_DUPLICATE_OBJECT);
}
diff --git
a/ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java
b/ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java
index dc47ec1..2271bd9 100644
---
a/ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java
+++
b/ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java
@@ -273,6 +273,8 @@ public class UserGroupSyncConfig {
private static final boolean DEFAULT_UGSYNC_DELETES_ENABLED = false;
private static final String UGSYNC_DELETES_FREQUENCY =
"ranger.usersync.deletes.frequency";
private static final long DEFAULT_UGSYNC_DELETES_FREQUENCY = 10L; //
After every 10 sync cycles
+ private static final String UGSYNC_NAME_VALIDATION_ENABLED =
"ranger.usersync.name.validation.enabled";
+ private static final boolean DEFAULT_UGSYNC_NAME_VALIDATION_ENABLED =
false;
private Properties prop = new Properties();
@@ -1270,4 +1272,15 @@ public class UserGroupSyncConfig {
}
return currentSyncSource;
}
+
+ public boolean isUserSyncNameValidationEnabled() {
+ boolean isUserSyncNameValidationEnabled;
+ String val = prop.getProperty(UGSYNC_NAME_VALIDATION_ENABLED);
+ if(StringUtils.isEmpty(val)) {
+ isUserSyncNameValidationEnabled =
DEFAULT_UGSYNC_NAME_VALIDATION_ENABLED;
+ } else {
+ isUserSyncNameValidationEnabled = Boolean.valueOf(val);
+ }
+ return isUserSyncNameValidationEnabled;
+ }
}
diff --git
a/ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
b/ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
index 15a7a38..4d8a32a 100644
---
a/ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
+++
b/ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
@@ -31,6 +31,7 @@ import java.util.HashSet;
import java.util.ArrayList;
import java.util.StringTokenizer;
import java.util.LinkedHashMap;
+import java.util.regex.Pattern;
import javax.security.auth.Subject;
import javax.servlet.http.HttpServletResponse;
@@ -85,6 +86,9 @@ public class PolicyMgrUserGroupBuilder extends
AbstractUserGroupSource implement
private static final String PM_UPDATE_DELETED_USERS_URI =
"/service/xusers/ugsync/users/visibility"; // POST
+ private static final Pattern USER_OR_GROUP_NAME_VALIDATION_REGEX =
+
Pattern.compile("^([A-Za-z0-9_]|[\u00C0-\u017F])([a-zA-Z0-9\\s,._\\-+/@=
]|[\u00C0-\u017F])+$", Pattern.CASE_INSENSITIVE);
+
private static final String SOURCE_EXTERNAL ="1";
private static final String STATUS_ENABLED = "1";
private static final String ISVISIBLE = "1";
@@ -128,6 +132,7 @@ public class PolicyMgrUserGroupBuilder extends
AbstractUserGroupSource implement
private boolean groupNameLowerCaseFlag = false;
private String currentSyncSource;
private String ldapUrl;
+ private boolean isUserSyncNameValidationEnabled = false;
private String authenticationType = null;
String principal;
@@ -177,6 +182,7 @@ public class PolicyMgrUserGroupBuilder extends
AbstractUserGroupSource implement
}
synchronized public void init() throws Throwable {
+ isUserSyncNameValidationEnabled =
config.isUserSyncNameValidationEnabled();
recordsToPullPerCall = config.getMaxRecordsPerAPICall();
policyMgrBaseUrl = config.getPolicyManagerBaseURL();
isMockRun = config.isMockRunEnabled();
@@ -595,11 +601,12 @@ public class PolicyMgrUserGroupBuilder extends
AbstractUserGroupSource implement
String newGroupAttrsStr = gson.toJson(newGroupAttrs);
String groupName = groupNameMap.get(groupDN);
if (StringUtils.isEmpty(groupName)) {
- groupName =
groupNameTransform(newGroupAttrs.get(UgsyncCommonConstants.ORIGINAL_NAME));
- if (StringUtils.isNotEmpty(groupName) &&
!groupNameMap.containsValue(groupName)) {
- // This is to avoid updating same
groupName with different DN that already exists
- groupNameMap.put(groupDN, groupName);
+ groupName =
groupNameTransform(newGroupAttrs.get(UgsyncCommonConstants.ORIGINAL_NAME).trim());
+ if (!isValidString(groupName)) {
+ LOG.warn("Ignoring invalid group " +
groupName + " Full name = " + groupDN);
+ continue;
}
+ groupNameMap.put(groupDN, groupName);
}
if (!groupCache.containsKey(groupName)) {
XGroupInfo newGroup = addXGroupInfo(groupName,
newGroupAttrs, newGroupAttrsStr);
@@ -646,11 +653,12 @@ public class PolicyMgrUserGroupBuilder extends
AbstractUserGroupSource implement
String newUserAttrsStr = gson.toJson(newUserAttrs);
String userName = userNameMap.get(userDN);
if (StringUtils.isEmpty(userName)) {
- userName =
userNameTransform(newUserAttrs.get(UgsyncCommonConstants.ORIGINAL_NAME));
- if (StringUtils.isNotEmpty(userName) &&
!userNameMap.containsValue(userName)) {
- // This is to avoid updating same
username with different DN that already exists
- userNameMap.put(userDN, userName);
+ userName =
userNameTransform(newUserAttrs.get(UgsyncCommonConstants.ORIGINAL_NAME).trim());
+ if (!isValidString(userName)) {
+ LOG.warn("Ignoring invalid user " +
userName + " Full name = " + userDN);
+ continue;
}
+ userNameMap.put(userDN, userName);
}
if (!userCache.containsKey(userName)) {
@@ -1581,6 +1589,16 @@ public class PolicyMgrUserGroupBuilder extends
AbstractUserGroupSource implement
return groupName;
}
+ private boolean isValidString(final String name) {
+ if (StringUtils.isBlank(name)) {
+ return false;
+ }
+ if (isUserSyncNameValidationEnabled) {
+ return
USER_OR_GROUP_NAME_VALIDATION_REGEX.matcher(name).matches();
+ }
+ return true;
+ }
+
private void updateDeletedGroups(Map<String, Map<String, String>>
sourceGroups) throws Throwable {
computeDeletedGroups(sourceGroups);
if (MapUtils.isNotEmpty(deletedGroups)) {
