Bug 171338 - Enhancement: winsync modrdn not synced
Summary: Enhancement: winsync modrdn not synced
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: 389
Classification: Retired
Component: Sync Service
Version: 1.3.0
Hardware: All
OS: Linux
high
medium
Target Milestone: ---
Assignee: Noriko Hosoi
QA Contact: Viktor Ashirov
URL:
Whiteboard:
: 213919 537357 (view as bug list)
Depends On:
Blocks: 389_1.2.6 639035
TreeView+ depends on / blocked
 
Reported: 2005-10-20 21:23 UTC by Orla Hegarty
Modified: 2018-12-01 19:05 UTC (History)
10 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-12-07 16:37:48 UTC
Embargoed:


Attachments (Terms of Use)
git diff ldap/servers/plugins/replication/{windows_connection.c, windows_protocol_util.c} (14.53 KB, patch)
2010-01-20 20:12 UTC, Noriko Hosoi
no flags Details | Diff
git diff windowsrepl.h windows_connection.c windows_protocol_util.c (26.73 KB, patch)
2010-01-22 00:54 UTC, Noriko Hosoi
nhosoi: review?
rmeggins: review+
Details | Diff

Comment 5 Rich Megginson 2009-04-09 18:14:08 UTC
This falls under the category of "support for subtree rename/entry move"

Comment 6 Rich Megginson 2010-01-07 16:14:15 UTC
Noriko, is the subtree rename feature stable enough to test using it with winsync?

Comment 8 Noriko Hosoi 2010-01-15 00:51:47 UTC
The current WinSync code does not support moving nodes.

Test case:
AD has an entry: CN=ad user1,OU=ou1,OU=ou0,DC=example,DC=com
DS has the corresponding entry: uid=aduser1,OU=ou1,ou=ou0,dc=example,dc=com

On AD, move
 CN=ad user1,OU=ou1,OU=ou0,DC=example,DC=com
to
 CN=ad user1,OU=ou2,OU=ou1,OU=ou0,DC=example,DC=com

The move (modrdn with newsuperior) does not occur on the DS.

Where to fix:
In windows_update_local_entry, windows_generate_update_mods is called to create smods to update the local entry.

(gdb) bt
#0  windows_update_local_entry (prp=0x7f17a8008250, 
    remote_entry=0x7f1748022080, local_entry=0x7f1748018e00)
    at ldap/servers/plugins/replication/windows_protocol_util.c:3949
#1  0x00007f17cbc2be47 in windows_process_dirsync_entry (prp=0x7f17a8008250, 
    e=0x7f1748005590, is_total=0)
    at ldap/servers/plugins/replication/windows_protocol_util.c:4248
#2  0x00007f17cbc2c0e1 in windows_dirsync_inc_run (prp=0x7f17a8008250)
    at ldap/servers/plugins/replication/windows_protocol_util.c:4312
#3  0x00007f17cbc1df2d in windows_inc_run (prp=0x7f17a8008250)
    at ldap/servers/plugins/replication/windows_inc_protocol.c:870
#4  0x00007f17cbc02f08 in prot_thread_main (arg=0x7f17a800adc0)
    at ldap/servers/plugins/replication/repl5_protocol.c:314
#5  0x0000003816029263 in _pt_root (arg=<value optimized out>)
    at ../../../mozilla/nsprpub/pr/src/pthreads/ptthread.c:228
#6  0x000000380740685a in start_thread (arg=<value optimized out>)
    at pthread_create.c:297
#7  0x00000038068de22d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#8  0x0000000000000000 in ?? ()

"windows_protocol_util.c"
3937 static int
3938 windows_update_local_entry(Private_Repl_Protocol *prp,Slapi_Entry *remote_entry,Slapi_Entry *local_entry)
3939 {
3940     Slapi_Mods smods = {0};
3941     int retval = 0;
3942     Slapi_PBlock *pb = NULL;
3943     int do_modify = 0;
3944 
3945     slapi_mods_init (&smods, 0);
3946 
3947     retval = windows_generate_update_mods(prp,remote_entry,local_entry,0,&s     mods,&do_modify);
3948     /* Now perform the modify if we need to */
3949     if (0 == retval && do_modify)

The function windows_generate_update_mods needs to examine the path difference in the DNs (using the above test case):
 remote: CN=ad user1,OU=ou2,OU=ou1,OU=ou0,DC=example,DC=com
 local: uid=aduser1,OU=ou1,ou=ou0,dc=example,dc=com
If they are different, add modrdn with newsuperior to smods.

Comment 9 Noriko Hosoi 2010-01-20 20:12:47 UTC
Created attachment 385774 [details]
git diff ldap/servers/plugins/replication/{windows_connection.c, windows_protocol_util.c}

Fix Descriptions:
windows_connection.c: fixed a typo
windows_protocol_util.c:
1. Implemented process_replay_rename for DS --> AD rename (modrdn) synchronization. It checks the new local superior and the current remote superior.  If they don't match, it calls windows_conn_send_rename.  If the entry is a user, the leaf RDN on AD is ntUserDomainId, which is not the leaf RDN on DS.  Thus, newrdn passed to windows_conn_send_rename is the leaf RDN of AD, which won't be changed in this process_replay_rename.  If the entry is a group, the leaf RDN on AD is the same as the one on DS.  Thus, if newrdn is passed to windows_conn_send_rename.  If the rename on AD failed by LDAP_NO_SUCH_OBJECT, the target entry might have been deleted on AD.  To recover the problem, add the renamed entry to AD.
2. Modified windows_update_local_entry to update the local entry if the rename operation occurred on AD (AD --> DS).  In the function, added a code to check the local and remote superiors.  If they don't match, the local entry is renamed.  The same as for DS --> AD, the rename only takes care of newsuperior change for a user; it could update both newrdn and newsuperior for a group.

Comment 10 Noriko Hosoi 2010-01-20 20:17:34 UTC
[Basic Test Cases]
Winsync agreement:
    dn: cn=test_winsync, cn=replica, cn="dc=test,dc=com", cn=mapping tree, cn=config
    objectClass: top
    objectClass: nsDSWindowsReplicationAgreement
    description: test winsync
    cn: test_winsync
    nsds7WindowsReplicaSubtree: ou=ou0,dc=example,dc=com
    nsds7DirectoryReplicaSubtree: ou=ou0,ou=top,dc=test,dc=com
    nsds7NewWinUserSyncEnabled: on
    nsds7NewWinGroupSyncEnabled: on
    nsds7WindowsDomain: example.com
    nsDS5ReplicaRoot: dc=test,dc=com
    nsDS5ReplicaHost: <hostname>
    nsDS5ReplicaPort: 389
    nsDS5ReplicaBindDN: cn=administrator,cn=users,DC=example,DC=com
    nsDS5ReplicaBindMethod: SIMPLE
    nsDS5ReplicaCredentials: {DES}18QB3vY9iJ54ToXu5S4DLA==
    creatorsName: cn=directory manager

Note: The AD subtree and DS subtree to be synchronized are different:
AD subtree: ou=ou0,dc=example,dc=com
DS subtree: ou=ou0,ou=top,dc=test,dc=com

AD has entries:
dn: CN=ad user0,OU=ou0,DC=example,DC=com
dn: CN=ad user1,OU=ou1,OU=ou0,DC=example,DC=com
    ...
dn: CN=adgroup0,OU=ou0,DC=example,DC=com
dn: cn=adgroup1,OU=ou1,ou=ou0,DC=example,DC=com
    ...

DS has entries:
dn: uid=duser0,ou=ou0,ou=top,dc=test,dc=com
dn: uid=duser1,ou=ou1,ou=ou0,ou=top,dc=test,dc=com
    ...
dn: cn=dsgroup0,ou=ou0,ou=top,dc=test,dc=com
dn: cn=dsgroup1,ou=ou1,ou=ou0,ou=top,dc=test,dc=com
    ...

1. Initiate Full Re-synchronization
    --> all the users and groups are synchronized and shared.
2. Add a user and a group on AD and DS
    --> the user and the group added to DS are replicated to AD immediately
    --> the user and the group added to AD are replicated in 5 min. as well as by "Send and Receive Updates Now"
3. Run ldapmodify with modrdn to move a user and a group to the new parent on DS.
    --> the user and the group are moved on AD
3.1 Delete the corresponding user and the group on AD, and move the entry on DS.
    --> the user and the group appear at the moved location on AD
4. Move a user and a group on AD
    --> the user and the group are moved on DS
(Deleting the entry on DS is replicated immediately, so I could not run the test similar to 3.1.)
5.  Run ldapmodify with modrdn to rename a group and move a group to the new parent at the same time on DS.
    --> the group is renamed and moved on AD
6. Rename a group on AD
    --> the group is renamed on DS

Comment 11 Noriko Hosoi 2010-01-20 22:15:24 UTC
Comment by Nathan:
<nkinder> I noticed some error checking in process_replay_rename() that seems to notice if an entry is moved out of scope of the agreement.
<nkinder> What happens in that case?  Do we do a delete?
<nhosoi> For now, I'm stopping the replay.  Do you think the entry should be removed on the other server?
<nkinder> it seems like it.  If you move an entry out of tree and do nothing on the other side, then things will be out of sync.
<nhosoi> Yeah...  I'm a bit worried to remove it.   But I guess they are just converted to tombstones, right?
<nkinder> I guess the case to worry about is if you move an entry out of scope on the DS side, which causes us to delete the entry on AD.  When that tombstone comes back, are we going to delete the moved entry since the link is via GUID?
<nhosoi> Hmmm, I'm to sure about it.  Is it being done when add and delete are repeated on the same entry?
<nkinder> I'm thinking of the existing code that handles deletions
<nhosoi> All right.  I'm adding the code and test the case...
<nkinder> it's at least something to play with to see what would be best to do.

Comment 12 Rich Megginson 2010-01-20 23:40:00 UTC
AD admins use ou's as the default grouping mechanism - used to set access control and other group policy.  We need to make sure we support the following cases:

1) Move a user or group (cn=name static group and an ou=name container) from one parent to another e.g.

AD: cn=Joe User,ou=admins,cn=Users,dc=example,dc=com
DS: uid=juser,ou=admins,ou=people,dc=example,dc=com

AD admin moves cn=Joe User from ou=admins to ou=super admins:

AD: cn=Joe User,ou=super admins,cn=Users,dc=example,dc=com
DS: uid=juser,ou=super admins,ou=people,dc=example,dc=com

2) Rename an ou e.g. rename the admins group to the super admins group:

AD: ou=admins,cn=Users,dc=example,dc=com
DS: ou=admins,ou=people,dc=example,dc=com

Rename ou=admins to ou=super admins:

AD: ou=super admins,cn=Users,dc=example,dc=com
DS: ou=super admins,ou=people,dc=example,dc=com

Comment 13 Noriko Hosoi 2010-01-21 01:24:43 UTC
(In reply to comment #11)
> Comment by Nathan:
> <nkinder> I noticed some error checking in process_replay_rename() that seems
> to notice if an entry is moved out of scope of the agreement.
> <nkinder> What happens in that case?  Do we do a delete?
> <nhosoi> For now, I'm stopping the replay.  Do you think the entry should be
> removed on the other server?
> <nkinder> it seems like it.  If you move an entry out of tree and do nothing on
> the other side, then things will be out of sync.
> <nhosoi> Yeah...  I'm a bit worried to remove it.   But I guess they are just
> converted to tombstones, right?
> <nkinder> I guess the case to worry about is if you move an entry out of scope
> on the DS side, which causes us to delete the entry on AD.  When that tombstone
> comes back, are we going to delete the moved entry since the link is via GUID?
> <nhosoi> Hmmm, I'm to sure about it.  Is it being done when add and delete are
> repeated on the same entry?
> <nkinder> I'm thinking of the existing code that handles deletions
> <nhosoi> All right.  I'm adding the code and test the case...
> <nkinder> it's at least something to play with to see what would be best to do.    

I implemented the above scenario.
0. A sync'ed entry exists on DS and AD.
1. On DS, moved it out of scope of the agreement.
   The corresponding entry on AD is removed.
2. On DS, moved the entry back to the position in scope of the agreement.
   The corresponding entry on AD is added.

Do we need the other direction, as well?  If an entry is moved out of scope of the agreement, should we removed it on DS, too?

Comment 14 Noriko Hosoi 2010-01-21 01:38:35 UTC
(In reply to comment #12)
> AD admins use ou's as the default grouping mechanism - used to set access
> control and other group policy.  We need to make sure we support the following
> cases:
> 
> 1) Move a user or group (cn=name static group and an ou=name container) from
> one parent to another e.g.
> 
> AD: cn=Joe User,ou=admins,cn=Users,dc=example,dc=com
> DS: uid=juser,ou=admins,ou=people,dc=example,dc=com
> 
> AD admin moves cn=Joe User from ou=admins to ou=super admins:
> 
> AD: cn=Joe User,ou=super admins,cn=Users,dc=example,dc=com
> DS: uid=juser,ou=super admins,ou=people,dc=example,dc=com

This case works fine.

> 2) Rename an ou e.g. rename the admins group to the super admins group:
> 
> AD: ou=admins,cn=Users,dc=example,dc=com
> DS: ou=admins,ou=people,dc=example,dc=com
> 
> Rename ou=admins to ou=super admins:
> 
> AD: ou=super admins,cn=Users,dc=example,dc=com
> DS: ou=super admins,ou=people,dc=example,dc=com    

WinSync only sync's either users (ntuser) or groups (ntgroup), doesn't it?  For instance, if you create "ou=newadmins,ou=people,dc=example,dc=com" on DS, it won't be replicated to AD.  And vice versa.

Comment 15 Rich Megginson 2010-01-21 02:10:43 UTC
(In reply to comment #14)
> (In reply to comment #12)
> > AD admins use ou's as the default grouping mechanism - used to set access
> > control and other group policy.  We need to make sure we support the following
> > cases:
> > 
> > 1) Move a user or group (cn=name static group and an ou=name container) from
> > one parent to another e.g.
> > 
> > AD: cn=Joe User,ou=admins,cn=Users,dc=example,dc=com
> > DS: uid=juser,ou=admins,ou=people,dc=example,dc=com
> > 
> > AD admin moves cn=Joe User from ou=admins to ou=super admins:
> > 
> > AD: cn=Joe User,ou=super admins,cn=Users,dc=example,dc=com
> > DS: uid=juser,ou=super admins,ou=people,dc=example,dc=com
> 
> This case works fine.
> 
> > 2) Rename an ou e.g. rename the admins group to the super admins group:
> > 
> > AD: ou=admins,cn=Users,dc=example,dc=com
> > DS: ou=admins,ou=people,dc=example,dc=com
> > 
> > Rename ou=admins to ou=super admins:
> > 
> > AD: ou=super admins,cn=Users,dc=example,dc=com
> > DS: ou=super admins,ou=people,dc=example,dc=com    
> 
> WinSync only sync's either users (ntuser) or groups (ntgroup), doesn't it?  For
> instance, if you create "ou=newadmins,ou=people,dc=example,dc=com" on DS, it
> won't be replicated to AD.  And vice versa.    

hmm - yes, you are correct - we should note that.  There is already a bug/enhancement request about syncing new ou's.  If we ever support new ou's, we should also support ou renames.

everything else looks good

Comment 16 Noriko Hosoi 2010-01-22 00:54:14 UTC
Created attachment 386059 [details]
git diff windowsrepl.h windows_connection.c windows_protocol_util.c

Files:
ldap/servers/plugins/replication/windowsrepl.h
                                /windows_connection.c
                                /windows_protocol_util.c

Fix Descriptions:
windows_protocol_util.c:
1. Implemented process_replay_rename for DS --> AD rename (modrdn)
synchronization. It checks the new local superior and the current remote
superior. If they don't match, it calls windows_conn_send_rename. If the
entry is a user, the leaf RDN on AD is ntUserDomainId, which is not the leaf
RDN on DS. Thus, newrdn passed to windows_conn_send_rename is the leaf RDN of
AD, which won't be changed in this process_replay_rename. If the entry is a
group, the leaf RDN on AD is the same as the one on DS. Thus, newrdn is
passed to windows_conn_send_rename. If the rename on AD failed by
LDAP_NO_SUCH_OBJECT, the target entry might have been deleted on AD. To
recover the problem, add the renamed entry to AD.
2. Modified windows_update_local_entry to update the local entry if the rename
operation occurred on AD (AD --> DS). In the function, a code to check
the local and remote superiors is added. If they don't match, the local entry is
renamed. Similar to DS --> AD, the rename only takes care of newsuperior
change for a user; it could update both newrdn and newsuperior for a group.
3. If either a local entry or a remote entry is moved out of scope of the agreement, the counter part entry is removed.  If a local entry or a remote entry is moved into the scope, the corresponding entry is added.

windows_connection.c: fixed a typo.  To support

Comment 17 Noriko Hosoi 2010-01-22 01:07:10 UTC
> windows_connection.c: fixed a typo.  To support

windows_connection.c: fixed a typo.  To support the item 3 of windows_protocol_util.c, a helper function windows_conn_set_error is added.

Comment 18 Noriko Hosoi 2010-01-22 01:13:34 UTC
In addition to the Baisc Test Cases in the above comment,

1. move a local entry (user / group) out of scope of the agreement.
  --> the corresponding remote entry is removed.
2. moved the local entry back to the inside of the agreement (it could be the original position or somewhere else under the agreement subtree)
  --> the corresponding remote entry is added to the position.
3. move a remote entry (user / group) out of scope of the agreement.
  --> the corresponding local entry is removed.
4. moved the remote entry back to the inside of the agreement (it could be the original position or somewhere else under the agreement subtree)
  --> the corresponding local entry is added to the position.

Comment 19 Noriko Hosoi 2010-01-22 17:35:10 UTC
*** Bug 213919 has been marked as a duplicate of this bug. ***

Comment 20 Noriko Hosoi 2010-01-22 19:09:12 UTC
Reviewed by Rich and Nathan (Thank you!!!)

[git log]
commit b5e653a844af60596f9bc6b16349ee902ddb51f5
Author: Noriko Hosoi <nhosoi>
Date:   Fri Jan 22 09:47:52 2010 -0800

    Allow modrdn to move subtree and rename non-leaf node
    
    This patch includes
    - replacing the entrydn index with the entryrdn index
    - replacing a full DN in each entry in the DB with an RDN
    - extending Slapi_Entry, entry2str, and str2entry to absorb the changes
      made on the entry
    - adding DN/RDN helper functions
    - adding DN cache
    - adding a utility and a migration script to convert the DN format database
      to the RDN format
    - extending a database dump utility dbscan to support the entryrdn
    - slapi_dn_syntax_check by nkinder is added to check the dn befor
      modify operations
    - big fix for 171338 - Enhancement: winsync modrdn not synced
    
    In addition to the above, compile warnings and memory leaks found in testing
    the new feature are fixed.
    
    For more details, see the feature design document at:
        http://directory.fedoraproject.org/wiki/Subtree_Rename
    
    and bugzilla at:
        https://bugzilla.redhat.com/show_bug.cgi?id=171338

Comment 21 Rich Megginson 2010-01-22 19:19:26 UTC
*** Bug 537357 has been marked as a duplicate of this bug. ***

Comment 24 Sankar Ramalingam 2011-08-03 13:28:09 UTC
I was trying to verify this bug using modrdn operations. Based on modrdn operations I am not seeing the entries are being renamed at both sides.

ModRDN at AD side.

USERDN="cn=Administrator,cn=Users,DC=win2k8sync64,DC=com"; /usr/lib64/mozldap/ldapmodify -Z -P /etc/dirsrv/slapd-weelie/cert8.db -p 636 -h 10.65.201.30 -D "$USERDN" -w Secret123 << EOF                                                         
dn: CN=DS2ADmodRDNDS1,OU=pass_sync,DC=win2k8sync64,DC=com
changetype: modrdn
newrdn: CN=testbug171338
deleteoldrdn: 1
> EOF
modifying RDN of entry CN=DS2ADmodRDNDS1,OU=pass_sync,DC=win2k8sync64,DC=com

When I was trying to search in DS for the same user, it reflects that the CN of the entry has changed, not the uid.

This results in two different uids for the same user.

uid(CN) at AD is - testbug171338
uid at DS is - DS2ADmodRDNDS1

ModRDN at DS side.

Host=`hostname`;Port=1389;SBase="uid=bug171338test2,dc=pass_sync,dc=com"; /usr/lib64/mozldap/ldapmodify -h $Host -p $Port -D "cn=Directory Manager" -w "Secret123" << EOF
dn: $SBase
changetype: modrdn
newrdn: uid=newrdn_171338
deleteoldrdn: 1
EOF

modifying RDN of entry uid=bug171338test2,dc=pass_sync,dc=com

The above modrdn doesn't reflect anything at AD. where as the modify operation for the "CN" attribute is reflecting.

Host=`hostname`;Port=1389;SBase="uid=bug171338test1,dc=pass_sync,dc=com"; /usr/lib64/mozldap/ldapmodify -h $Host -p $Port -D "cn=Directory Manager" -w "Secret123" << EOF
dn: $SBase
changetype: modify
replace: cn               
cn: newCNTEST
EOF

modifying entry uid=bug171338test1,dc=pass_sync,dc=com

Comment 25 Nathan Kinder 2011-08-03 15:12:20 UTC
(In reply to comment #24)
> I was trying to verify this bug using modrdn operations. Based on modrdn
> operations I am not seeing the entries are being renamed at both sides.
> 
> ModRDN at AD side.
> 
> USERDN="cn=Administrator,cn=Users,DC=win2k8sync64,DC=com";
> /usr/lib64/mozldap/ldapmodify -Z -P /etc/dirsrv/slapd-weelie/cert8.db -p 636 -h
> 10.65.201.30 -D "$USERDN" -w Secret123 << EOF                                   
> dn: CN=DS2ADmodRDNDS1,OU=pass_sync,DC=win2k8sync64,DC=com
> changetype: modrdn
> newrdn: CN=testbug171338
> deleteoldrdn: 1
> > EOF
> modifying RDN of entry CN=DS2ADmodRDNDS1,OU=pass_sync,DC=win2k8sync64,DC=com
> 
> When I was trying to search in DS for the same user, it reflects that the CN of
> the entry has changed, not the uid.
> 
> This results in two different uids for the same user.
> 
> uid(CN) at AD is - testbug171338
> uid at DS is - DS2ADmodRDNDS1

The uid and the cn attribute are not the same thing.  They are not mapped together.  If you do a MODRDN to change the cn of the user on the AD side, only the cn value should change on the DS side.  This does not require a MODRDN on the DS side unless the cn attribute is used as the RDN.  This is the expected behavior.

> 
> ModRDN at DS side.
> 
> Host=`hostname`;Port=1389;SBase="uid=bug171338test2,dc=pass_sync,dc=com";
> /usr/lib64/mozldap/ldapmodify -h $Host -p $Port -D "cn=Directory Manager" -w
> "Secret123" << EOF
> dn: $SBase
> changetype: modrdn
> newrdn: uid=newrdn_171338
> deleteoldrdn: 1
> EOF
> 
> modifying RDN of entry uid=bug171338test2,dc=pass_sync,dc=com
> 
> The above modrdn doesn't reflect anything at AD. where as the modify operation
> for the "CN" attribute is reflecting.

The uid attribute in DS does not map to anything in AD.  We map sAMAccountName on the AD side to ntUserDomainId on the DS side.  This is the expected behavior.

> 
> Host=`hostname`;Port=1389;SBase="uid=bug171338test1,dc=pass_sync,dc=com";
> /usr/lib64/mozldap/ldapmodify -h $Host -p $Port -D "cn=Directory Manager" -w
> "Secret123" << EOF
> dn: $SBase
> changetype: modify
> replace: cn               
> cn: newCNTEST
> EOF
> 
> modifying entry uid=bug171338test1,dc=pass_sync,dc=com

To verify this bug, you should just move an entry without changing the RDN.  That is a somewhat common action that this bug was meant to fix.  AD administrators can drag/drop an entry from one OU to another to move them, which uses a MODRDN operation.

Comment 26 Noriko Hosoi 2011-08-03 16:07:26 UTC
Thanks to Nathan for providing the information to verify the bug.
Cleaning up the NEEDINFO flag...

Comment 27 Sankar Ramalingam 2011-08-04 15:01:35 UTC
Fix verified by moving the entries(using drag/drop feature) from one OU to another.


Note You need to log in before you can comment on or make changes to this bug.