AD uses hierarchies to group user entries e.g. cn=users,dc=example,dc=com cn=adminuser,dc=example,dc=com etc.
There is currently no way to set up a winsync agreement for both subtrees - you cannot just use the suffix, and you cannot specify each AD subtree.
There should be a way to specify the sync base and filter in the sync agreement so that you could specify dc=example,dc=com and sync everything under it, using the filter to include or exclude users/groups to sync.
If that's not possible, then perhaps be able to specify multiple sync subtree pairs e.g. winSyncSubtreePair: ou=adminpeople,dc=example,dc=com cn=adminusers,dc=example,dc=com ...
Should also have the ability to filter out windows entries, both explicitly in the search filter used to do the dirsync search, and implicitly inside the code, to test against entries returned by dirsync.
Some random thoughts...
To avoid potential collisions we need to get SID if the windows user, store it and match in case the user is being overwritten. We should report collision if the user names match but SIDs are different. There should be a policy to overwrite or not which by default would be set to not overwrite.
Would it be acceptable to create an agreement for each subtree? cn=users,dc=example,dc=com cn=adminuser,dc=example,dc=com
I have 2 agreements for testing different bugs: dn: cn=WinSync,cn=replica,cn=dc\3Dexample\2Cdc\3Dcom,cn=mapping tree,cn=config [...] nsds7WindowsReplicaSubtree: cn=Users,dc=TEST,dc=EXAMPLE,dc=COM nsds7DirectoryReplicaSubtree: ou=People,dc=example,dc=com
dn: cn=tests,cn=replica,cn=dc\3Dexample\2Cdc\3Dcom,cn=mapping tree,cn=config [...] nsds7WindowsReplicaSubtree: ou=OU0,dc=TEST,dc=EXAMPLE,dc=COM nsds7DirectoryReplicaSubtree: ou=OU0,dc=example,dc=com
Replying to [comment:7 nhosoi]:
In order to satisfy the requirements of this ticket? Not entirely. There are many customers who want to be able to filter AD users e.g. only sync users that have a certain attribute and/or value. That can only be done with a search filter.
I have 2 agreements for testing different bugs: dn: cn=WinSync,cn=replica,cn=dc\3Dexample\2Cdc\3Dcom,cn=mapping tree,cn=config [...] nsds7WindowsReplicaSubtree: cn=Users,dc=TEST,dc=EXAMPLE,dc=COM nsds7DirectoryReplicaSubtree: ou=People,dc=example,dc=com dn: cn=tests,cn=replica,cn=dc\3Dexample\2Cdc\3Dcom,cn=mapping tree,cn=config [...] nsds7WindowsReplicaSubtree: ou=OU0,dc=TEST,dc=EXAMPLE,dc=COM nsds7DirectoryReplicaSubtree: ou=OU0,dc=example,dc=com
Replying to [comment:8 rmeggins]:
Replying to [comment:7 nhosoi]: Would it be acceptable to create an agreement for each subtree? cn=users,dc=example,dc=com cn=adminuser,dc=example,dc=com In order to satisfy the requirements of this ticket? Not entirely. There are many customers who want to be able to filter AD users e.g. only sync users that have a certain attribute and/or value. That can only be done with a search filter.
I see. So, can we supporting "filter" first separated from the "multiple" requirement?
Replying to [comment:9 nhosoi]:
Replying to [comment:8 rmeggins]: Replying to [comment:7 nhosoi]: Would it be acceptable to create an agreement for each subtree? cn=users,dc=example,dc=com cn=adminuser,dc=example,dc=com In order to satisfy the requirements of this ticket? Not entirely. There are many customers who want to be able to filter AD users e.g. only sync users that have a certain attribute and/or value. That can only be done with a search filter. I see. So, can we supporting "filter" first separated from the "multiple" requirement?
I don't think it matters which is done first. Whatever you want to do is fine. I don't know any customers who are urgently asking for one over the other.
Replying to [comment:10 rmeggins]:
All right. Thanks!
Description: {{{ 1. support multiple subtrees new config parameter in windwows sync agreement: winSyncSubtreePair: <ds subtree="">:<ad subtree="">
Example: winSyncSubtreePair: ou=OU1,dc=DSexample,dc=com:ou=OU1,DC=ADexample,DC=com winSyncSubtreePair: ou=OU2,dc=DSexample,dc=com:ou=OU2,DC=ADexample,DC=com winSyncSubtreePair: ou=OU3,dc=DSexample,dc=com:ou=OU3,DC=ADexample,DC=com
. Attribute type "winSyncSubtreePair" is added to the objectclass "nsDSWindowsReplicationAgreement". . If "winSyncSubtreePair" is not set, there is not behavioral difference: the AD subtree "nsds7WindowsReplicaSubtree" and the DS subtree "nsds7DirectoryReplicaSubtree" are used for the sync target checks. . When "winSyncSubtreePair" is set, the above 2 config parameters are ignored. To determine if an entry is the target of the synchronization, the DN is examined whether the DN is a descendent of any of the subtrees or not. If it is, the subtree of the counter part is retrieved. Moving an entry from one subtree to another is synchronized. Members of a group is synchronized as long as the member entry is in any of the defined subtrees.
Example: nsds7WindowsFilter: (|(cn=user)(cn=group)) nsds7DirectoryFilter: (|(uid=user)(cn=group))
. The filters are set to the windows_userfilter and directory_ userfilter in the private area in the windows agreement. And when each server is searched the filters are added to the internal filter. For instance, filters shown in the above Example allow synchronizing the entries which CN contains "user" or "group".
git patch file (master) 0001-Ticket-460-support-multiple-subtrees-and-filters.patch
The new winsync attributes should begin with "winSync" not "nsds7"
{{{ 718 LDAPControl *server_controls[2]; 719 server_controls[0] = &manageDSAITControl; 720 server_controls[1] = NULL; 721 return windows_search_entry_ext(conn, searchbase, filter, entry, server_controls, LDAP_SCOPE_SUBTREE); }}} Do we have any tickets associated with the fact that windows returns continuation references? If so, does this fix those tickets?
In create_subtree_pairs - should add some error handling/reporting in case the dn string is not actually a valid DN. You might also want to use ldap_utf8strtok_r (works just like strtok_r) - that function will write to chars[i] (*ptr) so make sure it is writable.
In process_replay_rename() - I see that the current code uses strstr() - shouldn't it convert newparent to a Slapi_DN and use parent/suffix/scope tests instead of strstr()? It just seems problematic to do DN comparisons using strings. Maybe there is a good reason to use strstr() here. Same with using PR_smprintf instead of slapi_create_dn_string. Same for windows_get_superior_change().
{{{
3396 suffix = (const char ) PL_strcasestr(suffix,"dc="); 3531 suffix = (const char )PL_strstr(suffix, "dc="); }}}
Why change from case insensitive to case sensitive? Windows sometimes uses "DC=foo".
Replying to [comment:16 rmeggins]:
The new winsync attributes should begin with "winSync" not "nsds7" A good idea! Renamed them... {{{ 718 LDAPControl *server_controls[2]; 719 server_controls[0] = &manageDSAITControl; 720 server_controls[1] = NULL; 721 return windows_search_entry_ext(conn, searchbase, filter, entry, server_controls, LDAP_SCOPE_SUBTREE); }}} Do we have any tickets associated with the fact that windows returns continuation references? If so, does this fix those tickets? There is no corresponding ticket. It was a part of my debugging... I did undo this diff. In create_subtree_pairs - should add some error handling/reporting in case the dn string is not actually a valid DN. Done! You might also want to use ldap_utf8strtok_r (works just like strtok_r) - that function will write to chars[i] (*ptr) so make sure it is writable. Done! In process_replay_rename() - I see that the current code uses strstr() - shouldn't it convert newparent to a Slapi_DN and use parent/suffix/scope tests instead of strstr()? It just seems problematic to do DN comparisons using strings. Maybe there is a good reason to use strstr() here. We are trying to retrieve intermediate nodes here. E.g., parent = <nodea>,<nodeb>,<dssubtree> is given, we get <nodea>,<nodeb>, then use it to generate new superior on AD as <nodea>,<nodeb>,<adsubtree>. So, we need the string anyway... For the string operations, case-lowered, normalized strings (returned from slapi_sdn_get_ndn) are used. So, it should be safe. Same with using PR_smprintf instead of slapi_create_dn_string. Same for windows_get_superior_change(). They are all from slapi_sdn_get_ndn, which are already normalized, so we don't have to normalize them again... (I'm trying to avoid dn_normalization as much as possible...) {{{ 3396 suffix = (const char ) PL_strcasestr(suffix,"dc="); 3531 suffix = (const char )PL_strstr(suffix, "dc="); }}} Why change from case insensitive to case sensitive? Windows sometimes uses "DC=foo". suffix is now returned from slapi_sdn_get_ndn, which case is lowered: suffix = slapi_sdn_get_ndn(windows_private_get_windows_treetop(prp->agmt));
The new winsync attributes should begin with "winSync" not "nsds7" A good idea! Renamed them...
{{{ 718 LDAPControl *server_controls[2]; 719 server_controls[0] = &manageDSAITControl; 720 server_controls[1] = NULL; 721 return windows_search_entry_ext(conn, searchbase, filter, entry, server_controls, LDAP_SCOPE_SUBTREE); }}} Do we have any tickets associated with the fact that windows returns continuation references? If so, does this fix those tickets? There is no corresponding ticket. It was a part of my debugging... I did undo this diff.
In create_subtree_pairs - should add some error handling/reporting in case the dn string is not actually a valid DN. Done!
You might also want to use ldap_utf8strtok_r (works just like strtok_r) - that function will write to chars[i] (*ptr) so make sure it is writable. Done!
In process_replay_rename() - I see that the current code uses strstr() - shouldn't it convert newparent to a Slapi_DN and use parent/suffix/scope tests instead of strstr()? It just seems problematic to do DN comparisons using strings. Maybe there is a good reason to use strstr() here. We are trying to retrieve intermediate nodes here. E.g., parent = <nodea>,<nodeb>,<dssubtree> is given, we get <nodea>,<nodeb>, then use it to generate new superior on AD as <nodea>,<nodeb>,<adsubtree>. So, we need the string anyway... For the string operations, case-lowered, normalized strings (returned from slapi_sdn_get_ndn) are used. So, it should be safe.
Same with using PR_smprintf instead of slapi_create_dn_string. Same for windows_get_superior_change(). They are all from slapi_sdn_get_ndn, which are already normalized, so we don't have to normalize them again... (I'm trying to avoid dn_normalization as much as possible...)
Why change from case insensitive to case sensitive? Windows sometimes uses "DC=foo". suffix is now returned from slapi_sdn_get_ndn, which case is lowered: suffix = slapi_sdn_get_ndn(windows_private_get_windows_treetop(prp->agmt));
git patch file (master) -- take 2 0001-Ticket-460-support-multiple-subtrees-and-filters.2.patch
strtok - the first time you use it, you specify the string to parse in the first argument. The next time you use it, you specify a NULL for the string - the 3rd parameter saveptr keeps track of the place in the original string. {{{ 960 p0 = ldap_utf8strtok_r(*ptr, ":", &saveptr); 961 p1 = ldap_utf8strtok_r(NULL, ":", &saveptr); }}}
otherwise, ack
git patch file (master) -- take 3 0001-Ticket-460-support-multiple-subtrees-and-filters.3.patch
Thanks for the reviews and the suggestions, Rich. I've fixed the line 961.
Pushed to master: 55ff5cf..c410b87 master -> master commit c410b87
Design page: http://www.port389.org/docs/389ds/design/winsync-rfe.html
Metadata Update from @rmeggins: - Issue assigned to nhosoi - Issue set to the milestone: 1.3.2 - 09/13 (September)
Login to comment on this ticket.