[PATCH] s4-drs: replmd_delete implementation

[tridge at samba.org]

Hi Eduardo,

 > This is the replmd_delete implementation patch.

thanks for the patch!

 > The DN of the deleted user is modified to the windows DN's format, the user
 > is moved to Deleted Objects, isDeleted=TRUE and LastKnownParent fields were
 > created.

Currently your patch doesn't always move the object into the "Deleted
Objects" container. It takes the current DN, finds its parent, then
finds the parent of that, then adds "CN=Deleted Objects". That works
for users in the CN=Users container, but won't work for any objects
that are at a different depth.

What you'll need to do is first find the Deleted Objects base DN. You
should probably add a utility function in dsdb/common/util.c to do
this, perhaps called dsdb_get_deleted_obects_dn(). Have a look at
samdb_ntds_settings_dn() for an example. This function would get the
Deleted Objects DN either by searching for it, or by adding
"CN=Deleted Objects" to the defaultNamingContext DN from the rootDSE.

Your code should also have a few more error messages when something
goes wrong. For example, when you call ldb_rename() you should print a
error messaging saying that something went wrong if it did. For
example:

if (ret != LDB_SUCCESS) {
   DEBUG(0,__location__ ": Failed to rename object from '%s' to '%s'\n", 
   			 ldb_dn_get_linearized(old_dn), 
   			 ldb_dn_get_linearized(new_dn));
   goto done;
}

Please also try to follow the existing coding style. For example, you
put a { at the end of the replmd_delete() declaration, whereas other
samba code puts it on the next line.

 > Doing a diff between Windows and Samba deleted objects it shows that Samba
 > still has some fields that does not appear in Windows. I tried to remove
 > them using ldb_msg_remove_attr but they weren't removed. I need to fix this
 > later.

your code is doing a ldb modify, so in the modify you would need to
list them as LDB_FLAG_MOD_DELETE. So you actually should use a
ldb_msg_add_*() call, but sets the element flags to mark it as an
element to delete.

 > Tridge said that each deleted object needs to be marked with the time that
 > deletion happened but I could not find this field in the Windows' object.
 > Does anyone know how I can see this field?

It probably is just the whenChanged field, but also have a look at the
replPropertyMeta record and compare with what Windows does (the
--show-binary flag to ldbsearch will be useful for that).

Another aspect of your code that could be fixed is that you currently
do the ldb_rename() using the top level ldb context. This means that
all modules above this one will be getting the rename. That may not be
the right thing to do (depending on whether the modules above need to
do any rename processing).

We should probably have a ldb_next_rename() function that uses
ldb_build_rename_req() to make a rename call only to the lower modules
in the module stack. It would also be nice to have a ldb_next_modify()
call that does all the messy work for a modify in a similar
fashion. That would make the end of your replmd_delete() function
where you do the modify much simpler.

If you want to work on that, then a separate patch that adds these
helper functions would be best.

Cheers, Tridge