Steps: 0) add a cn=Test User1 to AD - see that it is synced over to DS
1) add an entry to AD with a member: DN that only exists in AD - for example member: cn=Administrator,cn=Users,dc=example,dc=com
2) see that the ADD is synced over to DS - the DS will see that member is DN valued and try to map appropriately member: uid=administrator,ou=people,dc=example,dc=com
3) Change the member attribute in AD to a real user that is synced over member: cn=Test User1,cn=Users,dc=example,dc=com
DS will not update member because the code in windows_generate_update_mods() attempts to verify the existence of the DN member attribute value in DS in order to generate a mod/delete before doing the mod/add for the new value.
set default ticket origin to Community
Added initial screened field value.
Bug description: 2 case were fixed. {{{ 1) A group on AD has a member which is not a target of windows sync and exists only on AD. The member value in the group is synchronized to DS. If an operation is executed on AD so that the member is replaced with other members which are the target of the windows sync, the new member values are not synchronized. 2) If a group on AD and DS have members which are local and are not synchronized and the members are removed in the group on the other side, the delete operation is synchronized and deletes all the members including the local members. }}}
Fix description: {{{ 1) In windows_generate_update_mods, even if a sync'ed member value in a DS entry is not the target of windows sync and it is does not exist on DS, a following modify operation including the member value is proceeded by confirming the existence on AD. 2) AD->DS: in windows_map_mods_for_replay DS->AD: in windwos_generate_update_mods added the code to check if an attribute is completely deleted on one side, then the each value on the other side is in the sync scope or not. Put the value to the mod for the delete only if the value is in the sync scope. }}}
git patch file (master) -- fixed 2 typos in valueset.c 0001-Ticket-415-winsync-doesn-t-sync-DN-valued-attributes.patch
{{{ - / this might be a del: mod, in which case there are no values / - if (mod->mod_op & LDAP_MOD_DELETE) + if (slapi_valueset_isempty(vs) && (mod->mod_op & LDAP_MOD_DELETE)) }}} In this case - what happens is there are values in vs, if vs is not empty?
What happens if there is a member of a group, and the group member is within the winsync scope, but the entry doesn't exist in AD or DS because it is not being sync'ed? For example, say I have in AD
dn:cn=mygroup,cn=users,dc=example,dc=com member: cn=krbtgtservice,cn=users,dc=example,dc=com
But because cn=krbtgtservice is a pseudo-user (does not have an sn attribute) it is not sync'ed to DS. What happens when you attempt to sync the group entry? What happens when the group contains only pseudo-users, and the group entry in DS will have no members? What happens if the group contains a mix of pseudo-users and real users? Or, if not pseudo-users, users that you just don't want to sync? In DS you could have users that are members of sync'ed groups, but those users are not sync'ed - they don't have the ntUser objectclass and sync attributes.
Right. What I wanted to solve here is the bug description (2). But probably, we could do a little more to support the case you mentioned... Let me think about it again.
Replying to [comment:12 nhosoi]:
Ok. So are you saying that your changes do not change the way it currently works? If so, then that's fine. We can solve that problem another time.
But what about the change {{{ - / this might be a del: mod, in which case there are no values / - if (mod->mod_op & LDAP_MOD_DELETE) + if (slapi_valueset_isempty(vs) && (mod->mod_op & LDAP_MOD_DELETE)) }}}
Does this make the case where (mod->mod_op & LDAP_MOD_DELETE) && (!slapi_valueset_isempty(vs)) fail?
windows_map_mods_for_replay if (mapped_values) { The modified values are the target of winsync. remain intact. } else if (slapi_valueset_isempty(vs) && (mod->mod_op & LDAP_MOD_DELETE)) { modify|delete all the values of the attribute. modified in this patch for bug description (2). It used to delete all the AD values. The patch added a check if the AD values are in the sync scope or not. If in, the value is set to delete.
* As Rich pointed out, there are more cases this patch does not fix: e.g, a pseudo user in the sync scope but not being sync'ed would be deleted, which should not be. * To learn if an entry is in the scope is sync'ed or not, map_entry_dn_inbound would provide us the answer, which requires an entire entry. I.e., we need to search all the members to determine we could delete it or not. Would it be acceptable?
} else { The modified values are not the target of winsync. I think we should not do any op on AD... }
ok, I see - ack
We can do these later:
Replying to [comment:15 rmeggins]:
ok, I see - ack We can do these later: As Rich pointed out, there are more cases this patch does not fix: e.g, a pseudo user in the sync scope but not being sync'ed would be deleted, which should not be. To learn if an entry is in the scope is sync'ed or not, map_entry_dn_inbound would provide us the answer, which requires an entire entry. I.e., we need to search all the members to determine we could delete it or not. Would it be acceptable?
Thanks, Rich! Opened a ticket: Ticket #47464 - winsync - delete all members on AD/DS, if the other side has members which are in scope but not being sync'ed, the members are also deleted.
Reviewed by Rich (Thank you!!)
Pushed to master: ad32c62..03814dd master -> master commit 03814dd
Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1044142
Ticket 47642 fix for 389-ds-base-1.3.1 & 1.2.11 requires this Ticket #415 patch on 1.3.1 & 1.2.11.
Changing the target milestone to 1.2.11.26.
Pushed to 389-ds-base-1.2.11: 86fb137..737169e 389-ds-base-1.2.11 -> 389-ds-base-1.2.11 commit 737169e
Pushed to 389-ds-base-1.3.1: 377266e..8772ea1 389-ds-base-1.3.1 -> 389-ds-base-1.3.1 commit 8772ea1
git patch file (1.2.11 only) -- fixing a bug introduced by a cherry-pick 0001-Ticket-415-winsync-doesn-t-sync-DN-valued-attributes.2.patch
Thanks to Milan for figuring it out.
Pushed to 389-ds-base-1.2.11: caddfa2..19e4ea6 389-ds-base-1.2.11 -> 389-ds-base-1.2.11 commit 19e4ea6
Metadata Update from @nhosoi: - Issue assigned to nhosoi - Issue set to the milestone: 1.2.11.26
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/415
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Login to comment on this ticket.