#415 winsync doesn't sync DN valued attributes if DS DN value doesn't exist
Closed: wontfix None Opened 11 years ago by rmeggins.

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.
}}}

{{{
- / 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]:

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.

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:

  • 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?

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 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

7 years ago

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.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Login to comment on this ticket.

Metadata