spolavarpau1 commented on code in PR #1162:
URL: https://github.com/apache/ranger/pull/1162#discussion_r3799796435
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java:
##########
@@ -838,15 +850,25 @@ private void computeUserDelta(Map<String, Map<String,
String>> sourceUsers) {
noOfModifiedGroups++;
userNameMap.put(userDN, userName);
} else {
- if (MapUtils.isNotEmpty(curUserAttrs) &&
!StringUtils.equalsIgnoreCase(userDN, curUserDN)) { // skip update
- // Same username with different DN already exists
- LOG.debug("[{}]: SyncSource update skipped, current
user DN = {} new user DN = {}", userName, curUserDN, userDN);
-
- if (StringUtils.equalsIgnoreCase(curUserAttrsStr,
newUserAttrsStr)) {
- userNameMap.put(userDN, userName);
+ boolean isLdapAdSync =
StringUtils.equalsIgnoreCase(newSyncSource, "LDAP/AD")
Review Comment:
Same comments as above apply here as well
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java:
##########
@@ -747,14 +747,26 @@ private void computeGroupDelta(Map<String, Map<String,
String>> sourceGroups) {
noOfModifiedGroups++;
groupNameMap.put(groupDN, groupName);
} else {
- if (MapUtils.isNotEmpty(curGroupAttrs) &&
!StringUtils.equalsIgnoreCase(groupDN, curGroupDN)) { // skip update
- LOG.debug("[{}]: SyncSource update skipped, current
group DN = {} new user DN = {}", groupName, curGroupDN, groupDN);
-
- if (StringUtils.equalsIgnoreCase(curGroupAttrsStr,
newGroupAttrsStr)) {
- groupNameMap.put(groupDN, groupName);
+ boolean isLdapAdSync =
StringUtils.equalsIgnoreCase(newSyncSource, "LDAP/AD")
+ && MapUtils.isNotEmpty(curGroupAttrs);
+
+ if (isLdapAdSync) {
+ // ALLOW the update. A DN change (like an OU move) is
normal for LDAP/AD.
+ LOG.debug("ALLOW UPDATE: LDAP/AD group moved. DN
changed (Current: {}, New: {}) for {}.", curGroupDN, groupDN, groupName);
+ // Notice there is NO continue; here, allowing it to
fall through to the update logic below.
+ if (!StringUtils.equalsIgnoreCase(groupDN,
curGroupDN)) {
+ groupNameMap.remove(curGroupDN);
}
+ }
+ else {
+ if (MapUtils.isNotEmpty(curGroupAttrs) &&
!StringUtils.equalsIgnoreCase(groupDN, curGroupDN)) { // skip update
+ LOG.debug("[{}]: SyncSource update skipped,
current group DN = {} new user DN = {}", groupName, curGroupDN, groupDN);
- continue;
+ if (StringUtils.equalsIgnoreCase(curGroupAttrsStr,
newGroupAttrsStr)) {
+ groupNameMap.put(groupDN, groupName);
+ }
+ continue;
+ }
}
if (StringUtils.isEmpty(curSyncSource) ||
(!StringUtils.equalsIgnoreCase(curGroupAttrsStr, newGroupAttrsStr) &&
StringUtils.equalsIgnoreCase(curSyncSource, newSyncSource))) { // update
Review Comment:
When sync source is same and group OU is updated, then the groupAttrsStr
will be different isn't it? As one of the groupAttrs is groupFullName. For this
feature, can you please add a new config flag like
"ranger.usersync.dn.validation.enabled" and in this if block at the end we need
to add below change
if (isDNValidationEnabled) {
groupNameMap.remove(curGroupDN);
}
Please check if this works.
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java:
##########
@@ -747,14 +747,26 @@ private void computeGroupDelta(Map<String, Map<String,
String>> sourceGroups) {
noOfModifiedGroups++;
groupNameMap.put(groupDN, groupName);
} else {
- if (MapUtils.isNotEmpty(curGroupAttrs) &&
!StringUtils.equalsIgnoreCase(groupDN, curGroupDN)) { // skip update
- LOG.debug("[{}]: SyncSource update skipped, current
group DN = {} new user DN = {}", groupName, curGroupDN, groupDN);
-
- if (StringUtils.equalsIgnoreCase(curGroupAttrsStr,
newGroupAttrsStr)) {
- groupNameMap.put(groupDN, groupName);
+ boolean isLdapAdSync =
StringUtils.equalsIgnoreCase(newSyncSource, "LDAP/AD")
Review Comment:
This will cause regressions in case where a user left the company and
another person with same name joined in a different OU. In this case, we cannot
assume user is same and update the DN of the user right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
