dbcheck: Check for and remove duplicate values in attributes

This can happen with three DCs and custom schema, but we test
it by just forcing the values directly into the backing tdb.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
(cherry picked from commit c79c1e405d52c5dc54b8f03cd891e47f7ea04497)

python/samba/dbchecker.py

@@ -49,6 +49,7 @@ class dbcheck(object):
         self.remove_all_unknown_attributes = False
         self.remove_all_empty_attributes = False
         self.fix_all_normalisation = False
+        self.fix_all_duplicates = False
         self.fix_all_DN_GUIDs = False
         self.fix_all_binary_dn = False
         self.remove_all_deleted_DN_links = False
@@ -292,6 +293,23 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                           validate=False):
             self.report("Normalised attribute %s" % attrname)
 
+    def err_duplicate_values(self, dn, attrname, dup_values, values):
+        '''fix attribute normalisation errors'''
+        self.report("ERROR: Duplicate values for attribute '%s' in '%s'" % (attrname, dn))
+        self.report("Values contain a duplicate: [%s]/[%s]!" % (','.join(dup_values), ','.join(values)))
+        if not self.confirm_all("Fix duplicates for '%s' from '%s'?" % (attrname, dn), 'fix_all_duplicates'):
+            self.report("Not fixing attribute '%s'" % attrname)
+            return
+
+        m = ldb.Message()
+        m.dn = dn
+        m[attrname] = ldb.MessageElement(values, ldb.FLAG_MOD_REPLACE, attrname)
+
+        if self.do_modify(m, ["relax:0", "show_recycled:1"],
+                          "Failed to remove duplicate value on attribute %s" % attrname,
+                          validate=False):
+            self.report("Removed duplicate value on attribute %s" % attrname)
+
     def is_deleted_objects_dn(self, dsdb_dn):
         '''see if a dsdb_Dn is the special Deleted Objects DN'''
         return dsdb_dn.prefix == "B:32:%s:" % dsdb.DS_GUID_DELETED_OBJECTS_CONTAINER
@@ -1447,14 +1465,22 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                 # it's some form of DN, do specialised checking on those
                 error_count += self.check_dn(obj, attrname, syntax_oid)
 
+            values = set()
             # check for incorrectly normalised attributes
             for val in obj[attrname]:
+                values.add(str(val))
+
                 normalised = self.samdb.dsdb_normalise_attributes(self.samdb_schema, attrname, [val])
                 if len(normalised) != 1 or normalised[0] != val:
                     self.err_normalise_mismatch(dn, attrname, obj[attrname])
                     error_count += 1
                     break
 
+            if len(obj[attrname]) != len(values):
+                   self.err_duplicate_values(dn, attrname, obj[attrname], list(values))
+                   error_count += 1
+                   break
+
             if str(attrname).lower() == "instancetype":
                 calculated_instancetype = self.calculate_instancetype(dn)
                 if len(obj["instanceType"]) != 1 or obj["instanceType"][0] != str(calculated_instancetype):

source4/selftest/provisions/release-4-1-0rc3/expected-otherphone-after-dbcheck.ldif

@@ -0,0 +1,9 @@
+
+# 0 referrals
+# 1 entries
+dn: CN=Administrator,CN=Users,DC=release-4-1-0rc3,DC=samba,DC=corp
+otherHomePhone: 1
+otherHomePhone: 2
+otherHomePhone: 3
+# record 1
+# returned 1 records

source4/selftest/provisions/release-4-1-0rc3/forced-duplicate-value-for-dbcheck.ldif

@@ -0,0 +1,9 @@
+dn: cn=administrator,cn=users,dc=release-4-1-0rc3,dc=samba,dc=corp
+changetype: modify
+add: otherHomePhone
+otherHomePhone: 1
+otherHomePhone: 2
+otherHomePhone: 1
+otherHomePhone: 3
+otherHomePhone: 2
+-

testprogs/blackbox/dbcheck-oldrelease.sh

@@ -207,6 +207,39 @@ check_expected_after_values() {
     return 0
 }
 
+check_forced_duplicate_values() {
+    if [ x$RELEASE = x"release-4-1-0rc3" ]; then
+	ldif=$release_dir/forced-duplicate-value-for-dbcheck.ldif
+	TZ=UTC $ldbmodify -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb.d/DC%3DRELEASE-4-1-0RC3,DC%3DSAMBA,DC%3DCORP.ldb $ldif
+	if [ "$?" != "0" ]; then
+	    return 1
+	fi
+    else
+	return 0
+    fi
+}
+
+# This should 'fail', because it returns the number of modified records
+dbcheck_after_dup() {
+    if [ x$RELEASE = x"release-4-1-0rc3" ]; then
+	$PYTHON $BINDIR/samba-tool dbcheck --cross-ncs --fix --yes -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb $@
+    else
+	return 1
+    fi
+}
+
+check_expected_after_dup_values() {
+    if [ x$RELEASE = x"release-4-1-0rc3" ]; then
+	tmpldif=$PREFIX_ABS/$RELEASE/expected-otherphone-after-dbcheck.ldif.tmp
+	TZ=UTC $ldbsearch -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb cn=administrator -s base -b cn=administrator,cn=users,DC=release-4-1-0rc3,DC=samba,DC=corp otherHomePhone --sorted --show-binary | sort > $tmpldif
+	diff $tmpldif $release_dir/expected-otherphone-after-dbcheck.ldif
+	if [ "$?" != "0" ]; then
+	    return 1
+	fi
+    fi
+    return 0
+}
+
 # But having fixed it all up, this should pass
 dbcheck_clean() {
        $PYTHON $BINDIR/samba-tool dbcheck --cross-ncs -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb $@
@@ -269,6 +302,9 @@ if [ -d $release_dir ]; then
     testit "check_expected_before_values" check_expected_before_values
     testit_expect_failure "dbcheck" dbcheck
     testit "check_expected_after_values" check_expected_after_values
+    testit "check_forced_duplicate_values" check_forced_duplicate_values
+    testit_expect_failure "dbcheck_after_dup" dbcheck_after_dup
+    testit "check_expected_after_dup_values" check_expected_after_dup_values
     testit "dbcheck_clean" dbcheck_clean
     testit_expect_failure "dbcheck_acl_reset" dbcheck_acl_reset
     testit "dbcheck_acl_reset_clean" dbcheck_acl_reset_clean