If I set in smb.conf
[global]
ldap dn delete = yes

then user deleted via pdbedit without errors. But in this case pdbedit delete all attributes... 

Created attachment 281 [details]
Fix for attribute deletion when using lp_ldap_delete_dn

Hi!

Experienced the same problem when deleting user attributes for
"sambaSamAccount".
Took a look at the source and found out that "sambaGroupMapping" already had
the solution implemented by using a separate list of attributes for deletion.

Attached you find a patch, resolving this incident for me. But beware, I'm not
a C programmer. ;-)

Bye,
Daniel

Problem is not just pdbedit but with the ldap delete code in general.

ldapsam_delete_sam_account: Deleting user foo-you from LDAP.
smbldap_search_suffix: searching for:[(&(uid=foo-you)(objectclass=sambaSamAccount))]
....
ldapsam_delete_entry: Could not delete attributes for
uid=foo-you,ou=people,dc=plainjoe,dc=org, error: Naming violation (naming
attribute 'uid' is not present in entry)
_samr_delete_dom_user:Failed to delete entry for user foo-you.
000000 samr_io_r_delete_dom_user
    000000 smb_io_pol_hnd pol
        0000 data1: 00000000
        0004 data2: 00000000
        0008 data3: 0000
        000a data4: 0000
        000c data5: 00 00 00 00 00 00 00 00
    0014 status: NT_STATUS_CANNOT_DELETE

Still reviewing the submitted patch

There is the same problem with renaming machines accounts.

When a dn is uid=bla$,ou=people,dc=toto,dc=com
and you try to replace the uid attribut by uid=foo$, the ldap server rejects the
request (with openldap 2.1 not in 2.0).

To change the uid attribut, you must rename the entry and not replace the
attribut. If you replace the attribut, you have a naming violation.

(it's the same thing with a good LDAPv3 server)

Created attachment 410 [details]
LDAP Modify  Naming Violation Fix

Fix the problem of removing/changing the naming attribut with openldap 2.1/2.2
and other X500 server compliant.

Comment on attachment 410 [details]
LDAP Modify  Naming Violation Fix

It's for Samba 3.0.2

Comment on attachment 410 [details]
LDAP Modify  Naming Violation Fix

--- a/smbldap.c Thu Feb 19 15:52:00 2004
+++ b/smbldap.c Wed Feb 25 17:28:43 2004
@@ -971,21 +971,284 @@
	int		attempts = 0;
	char	       *utf8_dn;

-	SMB_ASSERT(ldap_state);
+	BOOL		do_rename = False;
+	BOOL		naming_deleted = False;
+	BOOL		naming_more_value = False;
+	int		i,j,k;
+	char	       *rdn_attribut;
+	char	       *first_rdn;
+	char	       *new_rdn_value;
+	char	       *new_rdn;
+	char	       *rdn_value;
+	TALLOC_CTX     *t_ctx;
+	LDAPDN	       *parts;
+	LDAPDN	       *part;
+
+	/* 
+	 *  The naming attribute is the attribute used in the first RDN
+	 *  of the DN (first from the left). 
+	 *  Ex : uid=foo,ou=people,dc=boo,dc=com => Naming attribut is "uid"
+	 *
+	 *  In fact, it's a little bit more complex with multiple naming 
+	 *  attributes :
+	 *  Ex : uid=foo+cn=bar,ou=people,dc=boo,dc=com 
+	 *  (Not supported by that patch)
+	 * 
+	 *  The attrs array contains the list of the modification. 
+	 *  If the naming attribute is modified or deleted, the DN won't be
+	 *  correct.
+	 *
+	 *  To apply a modification on the naming attribute, you must modify 
+	 *  the DN with the ldap_modrdn2_s function. That function change the 
+	 *  naming attribute both in the DN and in the entry.
+	 * 
+	 *  The problem is that we must extract and remove the modification 
+	 *  of the naming attribute from the attrs array.
+	 *
+	 * */
+
+	t_ctx=talloc_init("smbldap_modify");

-	DEBUG(5,("smbldap_modify: dn => [%s]\n", dn ));

	if (push_utf8_allocate(&utf8_dn, dn) == (size_t)-1) {
+		talloc_destroy(t_ctx);
+		return LDAP_NO_MEMORY;
+	}
+
+	/*  
+	 *  All strings are now in UTF8
+	 * */
+
+	/* 
+	 *  Extraction of the naming attribute from the DN
+	 *  rdn : array of rdn extrated from the DN
+	 *  rdn_attribut : naming attribute (string)
+	 *  utf8_rdn_value : value of the naming attribute 
+	 *			in the DN (UTF8 string)
+	 *  
+	 * */
+	
+	/*
+	 *  ldap_explode_dn is deprecated in favor of 
+	 *  ldap_dn2str() and ldap_str_2dn.
+	 *
+	 *  The flag LDAP_DN_PRETTY causes UTF-8 to be represented.
+	 *  
+	 * */
+	
+	/* We build the first RDN in firstrdn */
+	ldap_str2dn(dn, &parts, LDAP_DN_FORMAT_LDAP);
+	
+	/*  We can use
+	 *  ldap_rdn2str(parts[0][0], &first_rdn,LDAP_DN_FORMAT_LDAPV3 |
LDAP_DN_PRETTY ); 
+	 *  or */
+	part=talloc(t_ctx,sizeof(char *) * 2);
+	if (!part) {
+		talloc_destroy(t_ctx);
+		SAFE_FREE(utf8_dn);
		return LDAP_NO_MEMORY;
	}
+	part[0]=talloc(t_ctx,sizeof(char *) * 2);
+	if (!part[0]) {
+		talloc_destroy(t_ctx);
+		SAFE_FREE(utf8_dn);
+		return LDAP_NO_MEMORY;
+	}
+	part[0][0]=parts[0][0];
+	part[0][1]=NULL;
+	part[1]=NULL;
+	ldap_dn2str(part, &first_rdn,LDAP_DN_FORMAT_LDAPV3 | LDAP_DN_PRETTY );
+	/* but ldap_rdn2str is not documented */
+
+	/* we take the rdn value */
+	rdn_value=talloc_strdup(t_ctx, strchr(first_rdn,'=')+1);
+	
+	if (!rdn_value) {
+		talloc_destroy(t_ctx);
+		SAFE_FREE(utf8_dn);
+		return LDAP_NO_MEMORY;
+	}
+	
+	
+	/*  we take the rdn attribute */
+	first_rdn[strlen(first_rdn)-strlen(rdn_value)-1]='\0';
+	rdn_attribut=talloc_strdup(t_ctx, first_rdn);
+	if (!rdn_attribut) {
+		SAFE_FREE(utf8_dn);
+		talloc_destroy(t_ctx);
+		return LDAP_NO_MEMORY;
+	}	
+	
+	/* we release the memory */
+	SAFE_FREE(first_rdn);
+	ldap_dnfree(parts);
+
+	DEBUG(5,("smbldap_modify: dn => [%s]\n", dn ));
+	
+	/* 
+	 *  We have to walk into the attrs array to find if the naming
+	 *  attributes is changed or deleted
+	 * 
+	 * */
+	 for (i = 0; attrs[i] != NULL; i++) {
+		/*  
+		 *  If mod_type is the naming attribute
+		 *  Three cases : the operation is DELETE or ADD or REPLACE
+		 *  
+		 *  Note: Samba doesn't use REPLACE operation 
+		 *  for LDAP manipulation. Instead it uses two 
+		 *  operations : DELETE and ADD
+		 *  
+		 * */
+		if ( ( attrs[i]->mod_op == LDAP_MOD_DELETE )
+			&&  strequal(attrs[i]->mod_type,rdn_attribut) ) {
+			/*
+			 *  In the DELETE operation.
+			 *  If we have more than one value in the entry for 
+			 *  the naming attribute, the DELETE operation can
+			 *  delete other values than the one uses in the DN.
+			 *  
+			 *  So we have to walk into the mod_values arrray.
+			 *
+			 *  The values are in UTF8 (see smbldap_set_mod())
+			 *  So a simple strcmp is good.
+			 *  
+			 * */
+			for (j=0;attrs[i]->mod_values[j] != NULL; j++) {
+				if (!strcmp(attrs[i]->mod_values[j],
+							rdn_value)) {
+					/* The modification deletes 
+					 * the naming attribute. 
+					 * We have to remove that value 
+					 * from the mod_values array.
+					 * */
+					SAFE_FREE(attrs[i]->mod_values[j]);
+				       
attrs[i]->mod_values[j]=attrs[i]->mod_values[j+1];
+					for (k=j+1;attrs[i]->mod_values[k] !=
NULL; k++)
+					       
attrs[i]->mod_values[k]=attrs[i]->mod_values[k+1];
+					naming_deleted = True;
+				} else {
+					/* The modification deletes more than 
+					 * the naming attribute value */
+					naming_more_value = True;
+				}
+			}
+
+			/*  
+			 *  If we the DELETE operation delete only 
+			 *  the good value of the naming attribute, 
+			 *  we can remove that modification
+			 *  from the attrs array.
+			 *  
+			 * */
+			if (!naming_more_value) {
+				SAFE_FREE(attrs[i]->mod_type);
+				for (j=0;attrs[i]->mod_values[j] != NULL; j++)
+					SAFE_FREE(attrs[i]->mod_values[j]);
+				SAFE_FREE(attrs[i]->mod_values);
+				SAFE_FREE(attrs[i]); 
+				attrs[i]=attrs[i+1];
+				for (j=i+1; attrs[j] != NULL; j++) {
+					attrs[j]=attrs[j+1];
+				}
+			}
+		}
+
+		/*
+		 *  If we have removed the DELETE modification, we can be at
the last item from attrs
+		 *  array.
+		 *  
+		 * */
+		if ( ( attrs[i] != NULL ) 
+		      && ( ( attrs[i]->mod_op == LDAP_MOD_ADD 
+				      && naming_deleted ) 
+			      || attrs[i]->mod_op == LDAP_MOD_REPLACE )
+		      && ( attrs[i]->mod_values[0] != NULL)
+		      && ( strequal(attrs[i]->mod_type,rdn_attribut) ) ) {
+			/*
+			 *  In the ADD or REPLACE operation.
+			 *  If we add a naming attribute after 
+			 *  to have deleted it or if we replace 
+			 *  the naming attribute, we have to build 
+			 *  the new RDN of the entry.
+			 *
+			 *  We use the first value added to build the RDN.
+			 * */
+			do_rename = True;
+			new_rdn_value = talloc_strdup(t_ctx, 
+					attrs[i]->mod_values[0]);
+			if (!new_rdn_value) {
+				talloc_destroy(t_ctx);
+				SAFE_FREE(utf8_dn);
+				return LDAP_NO_MEMORY;
+			}
+			
+			/*  
+			 *  We build the new RDN in UTF8
+			 * */
+			new_rdn = talloc_asprintf(t_ctx,
+					"%s=%s",rdn_attribut,new_rdn_value);
+			if (!new_rdn) {
+				talloc_destroy(t_ctx);
+				SAFE_FREE(utf8_dn);
+				return LDAP_NO_MEMORY;
+			}
+			
+			DEBUG(5,("smbldap_modify: newdn => [%s]\n", new_rdn ));
+			/*  
+			 *  If it's an ADD operation, we have to remove 
+			 *  the first value from the operation 
+			 *  (or the complete operation if there is 
+			 *  only one value added).
+			 * */
+			if (attrs[i]->mod_op != LDAP_MOD_REPLACE) {
+				if (attrs[i]->mod_values[1] == NULL) {
+					SAFE_FREE(attrs[i]->mod_type);
+					for (j=0;attrs[i]->mod_values[j] !=
NULL; j++)
+					       
SAFE_FREE(attrs[i]->mod_values[j]);
+					SAFE_FREE(attrs[i]->mod_values);
+					SAFE_FREE(attrs[i]); 
+					attrs[i]=attrs[i+1];
+					for (j=i+1; attrs[j] != NULL; j++) {
+						attrs[j]=attrs[j+1];
+					}
+				} else {
+					SAFE_FREE(attrs[i]->mod_values[0]);
+				       
attrs[i]->mod_values[0]=attrs[i]->mod_values[1];
+					for (j=1;attrs[i]->mod_values[j] !=
NULL; j++)
+					       
attrs[i]->mod_values[j]=attrs[i]->mod_values[j+1];
+				}
+			}
+			/* We have our new RDN,
+			 * we can go out of the attrs array */
+			continue;
+		}
+	}
+
+	
+	SMB_ASSERT(ldap_state);

	while ((rc == LDAP_SERVER_DOWN) && (attempts < SMBLDAP_NUM_RETRIES)) {

		if ((rc = smbldap_retry_open(ldap_state,&attempts)) !=
LDAP_SUCCESS)
			continue;
+
+		/*  
+		 *  We apply modifications on the entry.
+		 *
+		 * */
+		if ((rc = ldap_modify_s(ldap_state->ldap_struct, utf8_dn,
attrs)) != LDAP_SUCCESS)
+			continue;
+		
+		/* 
+		 * We have detected a modification of the naming attribute 
+		 * and we have a new RDN.
+		 * */
+		if (do_rename)
+			rc = ldap_modrdn2_s(ldap_state->ldap_struct, utf8_dn,
new_rdn, 1);

-		rc = ldap_modify_s(ldap_state->ldap_struct, utf8_dn, attrs);
	}
+

	if (rc == LDAP_SERVER_DOWN) {
		DEBUG(0,("%s: LDAP server is down!\n",FUNCTION_MACRO));
@@ -995,6 +1258,7 @@
	ldap_state->last_use = time(NULL);

	SAFE_FREE(utf8_dn);
+	talloc_destroy(t_ctx);
	return rc;
 }

Created attachment 427 [details]
LDAP Modify Naming Violation Fix

The same patch but in attachement...
I've cleaned the precedent patch.

I believe that solution for this problem should be a little bit different.

The problem is not only in naming attribute being deleted but in the whole idea
of deleting all attributes instead of only those belonging to Samba.
ldapsam_delete_sam_account() should delete only attributes belonging to
sambaSamAccount objectclass and only those which do not belong to others
objectclasses.

This way 'ldap delete user' will work with none samba components of the user
records and smbpasswd -x will do what it suppose to do - remove Samba properties
from the record.

I'll try to come up with the patch. It will need to load schemas from server and
during deletion check what objectclasses need to stay and leave their attributes
along. It maybe not that simple, but in my opinion it's the right way to do it.

Igor

Created attachment 640 [details]
Patch to fix deletion of Samba attributes

This is a patch implementing changes I've suggested previousely.

It does look like an overkill for a simple fix to Samba account deletion
problem but I hope it can be used to update other functions as well to use
samba.schema loaded into LDAP. Reading schema from LDAP helps to use newer
samba version with older samba.schema.

Change makes Samba read schemas from "cn=subschema" entry and hash it.
In smbldap_delete_entry it choose attributes from the entry which present
in objectClass (passed as a function argument) but not in any other
objectClasses listed in the entry.

As a safety measure if anything goes wrong with the change the previous way
of finding attributes is used (even though it doesn't work).

Let me know what you think.
Igor

Created attachment 641 [details]
Slightly better patch.

Removed unnecessary loop and make just one ldap call for cn=subschema
instead of two.

Created attachment 642 [details]
Delete UNIX account ("delete user script") after deletion of SAM account.

Additional patch related to deletion of user account.

When user is deleted "delete user script" is incorrectly called before
information about SAM account is deleted. The correct way is to delete
SAM related information from the account and then call "delete user script"
script to remove (if necessary) UNIX account as well. Patch just switches
the order of those two calls.

With those two patches I can use usrmgr.exe from Windows Resource Kit the
way I like it - only adding and removing Samba information leaving UNIX
accounts in LDAP as they are.

Cheers,
Igor

Created attachment 643 [details]
Another revision of the same changes.

After looking closely I understood that hash tables for schema should be part
of the 'struct smbldap_state' and not global variables. This way connections
to more than one different LDAP server can coexist and each of them will have
its own hash of its schema.

The new delta includes change from both previouse patches.

Cheers,
Igor

P.S. My long weekend is over so it's safe to assume that I won't change
this last patch for a while. :o)

I beleieve this bug have been loong fixed now.
Reopen if not.