From: Andrew Tridgell <tridge@samba.org>
Date: Mon, 11 Jul 2011 05:32:12 +0000 (+1000)
Subject: s4-dsdb: moved checking of duplicate member entries to repl_meta_data.c
X-Git-Url: http://git.samba.org/?p=metze%2Fsamba%2Fwip.git;a=commitdiff_plain;h=0214b7f20cfcecd3d51aa12ae4e31cc4e095d73b

s4-dsdb: moved checking of duplicate member entries to repl_meta_data.c

the samldb checks failed to account for the possibility of a member
being removed and added in the same modify operation. This happens
(for example) when dbcheck is fixing a SID in a DN.

The repl_meta_data.c code already has this check, it just wasn't
giving the right specialised error code for the 'member' attribute

Pair-Programmed-With: Andrew Bartlett <abartlet@samba.org>
Pair-Programmed-With: Amitay Isaacs <amitay@gmail.com>
---

diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index a76b88ecbc81..1511b447eeff 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -1774,7 +1774,13 @@ static int replmd_modify_la_add(struct ldb_module *module,
 				ldb_asprintf_errstring(ldb, "Attribute %s already exists for target GUID %s",
 						       el->name, GUID_string(tmp_ctx, p->guid));
 				talloc_free(tmp_ctx);
-				return LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS;
+				/* error codes for 'member' need to be
+				   special cased */
+				if (ldb_attr_cmp(el->name, "member") == 0) {
+					return LDB_ERR_ENTRY_ALREADY_EXISTS;
+				} else {
+					return LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS;
+				}
 			}
 			ret = replmd_update_la_val(old_el->values, p->v, dns[i].dsdb_dn, p->dsdb_dn,
 						   invocation_id, seq_num, seq_num, now, 0, false);
@@ -1886,13 +1892,21 @@ static int replmd_modify_la_delete(struct ldb_module *module,
 		if (!p2) {
 			ldb_asprintf_errstring(ldb, "Attribute %s doesn't exist for target GUID %s",
 					       el->name, GUID_string(tmp_ctx, p->guid));
-			return LDB_ERR_NO_SUCH_ATTRIBUTE;
+			if (ldb_attr_cmp(el->name, "member") == 0) {
+				return LDB_ERR_UNWILLING_TO_PERFORM;
+			} else {
+				return LDB_ERR_NO_SUCH_ATTRIBUTE;
+			}
 		}
 		rmd_flags = dsdb_dn_rmd_flags(p2->dsdb_dn->dn);
 		if (rmd_flags & DSDB_RMD_FLAG_DELETED) {
 			ldb_asprintf_errstring(ldb, "Attribute %s already deleted for target GUID %s",
 					       el->name, GUID_string(tmp_ctx, p->guid));
-			return LDB_ERR_NO_SUCH_ATTRIBUTE;
+			if (ldb_attr_cmp(el->name, "member") == 0) {
+				return LDB_ERR_UNWILLING_TO_PERFORM;
+			} else {
+				return LDB_ERR_NO_SUCH_ATTRIBUTE;
+			}
 		}
 	}
 
diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index a61920f2af91..0fe13e53cf8c 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -1568,7 +1568,6 @@ static int samldb_member_check(struct samldb_ctx *ac)
 	struct ldb_result *res;
 	struct dom_sid *group_sid;
 	unsigned int i, j;
-	int cnt;
 	int ret;
 
 	/* Fetch information from the existing object */
@@ -1596,7 +1595,6 @@ static int samldb_member_check(struct samldb_ctx *ac)
 
 		el = &ac->msg->elements[i];
 		for (j = 0; j < el->num_values; j++) {
-			struct ldb_message_element *mo;
 			struct ldb_result *group_res;
 			const char *group_attrs[] = { "primaryGroupID" , NULL };
 			uint32_t prim_group_rid;
@@ -1607,36 +1605,6 @@ static int samldb_member_check(struct samldb_ctx *ac)
 				return ldb_operr(ldb);
 			}
 
-			/* The "member" attribute can be modified with the
-			 * following restrictions (beside a valid DN):
-			 *
-			 * - "add" operations can only be performed when the
-			 *   member still doesn't exist - if not then return
-			 *   ERR_ENTRY_ALREADY_EXISTS (not
-			 *   ERR_ATTRIBUTE_OR_VALUE_EXISTS!)
-			 * - "delete" operations can only be performed when the
-			 *   member does exist - if not then return
-			 *   ERR_UNWILLING_TO_PERFORM (not
-			 *   ERR_NO_SUCH_ATTRIBUTE!)
-			 * - primary group check
-			 */
-			mo = samdb_find_attribute(ldb, res->msgs[0], "member",
-						  ldb_dn_get_linearized(member_dn));
-			if (mo == NULL) {
-				cnt = 0;
-			} else {
-				cnt = 1;
-			}
-
-			if ((cnt > 0) && (LDB_FLAG_MOD_TYPE(el->flags)
-			    == LDB_FLAG_MOD_ADD)) {
-				return LDB_ERR_ENTRY_ALREADY_EXISTS;
-			}
-			if ((cnt == 0) && LDB_FLAG_MOD_TYPE(el->flags)
-			    == LDB_FLAG_MOD_DELETE) {
-				return LDB_ERR_UNWILLING_TO_PERFORM;
-			}
-
 			/* Denies to add "member"s to groups which are primary
 			 * ones for them - in this case return
 			 * ERR_ENTRY_ALREADY_EXISTS. */
@@ -1665,6 +1633,9 @@ static int samldb_member_check(struct samldb_ctx *ac)
 			}
 
 			if (dom_sid_equal(group_sid, sid)) {
+				ldb_asprintf_errstring(ldb,
+						       "samldb: member %s already set via primaryGroupID %u",
+						       ldb_dn_get_linearized(member_dn), prim_group_rid);
 				return LDB_ERR_ENTRY_ALREADY_EXISTS;
 			}
 		}