RANGER-5110: Prevent entity soft deletion when LDAP DN is updated - #1162
RANGER-5110: Prevent entity soft deletion when LDAP DN is updated#1162krishnamuttevi wants to merge 2 commits into
Conversation
|
|
||
| if (StringUtils.equalsIgnoreCase(curUserAttrsStr, newUserAttrsStr)) { | ||
| userNameMap.put(userDN, userName); | ||
| boolean isLdapAdSync = StringUtils.equalsIgnoreCase(newSyncSource, "LDAP/AD") |
There was a problem hiding this comment.
Same comments as above apply here as well
There was a problem hiding this comment.
Have updated in the following commit.
There was a problem hiding this comment.
Please check the below the below doc for the testcaes which was verified by runtime
…h proper syncing of U&G
…essed reviewer comments
b117e75 to
47a7c81
Compare
| String newSyncSource = newGroupAttrs.get(UgsyncCommonConstants.SYNC_SOURCE); | ||
|
|
||
| // Universal DN Validation Check (Executes first for both startup & runtime) | ||
| if (MapUtils.isNotEmpty(curGroupAttrs) && !StringUtils.equalsIgnoreCase(groupDN, curGroupDN)) { |
There was a problem hiding this comment.
How about the case where line 755 condition is true? i.e., Customer has initially configured usersync with Unix as sync source and all the users/groups have the sync source as Unix and now they changed the sync source as AD/LDAP and they had isSyncSourceValidationEnabled is set to false and restarted ranger usersync. Here isDnValidationEnabled is set to false. Then the username/groupname DN is not updated isn't it? It will not come to line 755 at all right?
There was a problem hiding this comment.
Yes, As per the current version of the code if isDnValidationEnabled = false, means blocking the domain name change and in this though inter sync source change was accepted by setting the issyncsourcevalidation enabled = false, the dn would be changed from Unix to dn of LDAP/Ad which would be eventually blocked as per the property isDnValidationEnabled.
So to prevent code duplication for both incremental and at startup (restart) as the check is necessary placed the dn validation block upfront.
If this
Dn = Unix to Dn = {ou=xxx,dc=xx} of LDAP is also considered as dn change and currently it is blocked.
Should we allow this condition let it change at startup?
2 participants
Fixes an issue in LDAP/AD sync where users or groups are incorrectly skipped or marked for deletion when their DN changes (e.g., due to OU moves). This PR correctly detects, maps, and updates DN changes efficiently using the existing cache.
Key Changes:
Allow DN Updates: Added an isLdapAdSync check in delta computation (computeUserDelta/computeGroupDelta). If a DN changes, the update is now explicitly allowed rather than skipped, and old DN mappings are cleaned up.
Smart Deletion Check:
Before marking an entity as deleted due to a missing DN (computeDeletedUsers/computeDeletedGroups), we now verify if the base name still exists in the source. If it does, deletion is bypassed so the delta sync can naturally correct the DN.
Why this approach (vs. PR #516)
Better Performance: Avoids the heavy REST reloads and cache wiping introduced in #516. We utilize the existing cache.
Complete Fix: Actually updates the entity with the new DN across cycles (PR #516 only attempts a soft-delete and fails to map the new DN).
Cleaner: Highly efficient fix with a significantly smaller LOC footprint.
Testing:
Verified LDAP/AD users and groups update correctly during OU moves.
Confirmed no extra cache-clearing loads are triggered.
Verified non-LDAP/AD sync behavior remains unaffected.