#460 support multiple subtrees and filters
Closed: Fixed None Opened 7 years ago by rmeggins.

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

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

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.

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.

  1. support filters
    new config parameters in windwows sync agreement:
    nsds7WindowsFilter: <additional filter="" on="" ad="">
    nsds7DirectoryFilter: <additional filter="" on="" ds="">

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

  1. Misc
    . Added slapi_sdn_set_ndn_byref, slapi_sdn_set_ndn_passin, and
    slapi_sdn_common_ancestor to dn.c (see also slapi-plugin.h).
    . Fixed memory leaks.
    . Fixed some of the mixed indentations.
    }}}

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

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

Thanks for the reviews and the suggestions, Rich. I've fixed the line 961.

Pushed to master:
55ff5cf..c410b87 master -> master
commit c410b87

Metadata Update from @rmeggins:
- Issue assigned to nhosoi
- Issue set to the milestone: 1.3.2 - 09/13 (September)

2 years ago

Login to comment on this ticket.

Metadata