You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ranger.apache.org by sp...@apache.org on 2021/03/23 20:41:45 UTC
[ranger] branch ranger-2.2 updated: RANGER-3207: Incorporate some
of the review comments
This is an automated email from the ASF dual-hosted git repository.
spolavarapu pushed a commit to branch ranger-2.2
in repository https://gitbox.apache.org/repos/asf/ranger.git
The following commit(s) were added to refs/heads/ranger-2.2 by this push:
new 50992b3 RANGER-3207: Incorporate some of the review comments
50992b3 is described below
commit 50992b38e6c5025b58d5cb75c79117660d65d209
Author: Sailaja Polavarapu <sp...@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)) {