Issue 5367 - RFE - store full DN in database record

Description: For the Full unnormalized DN in the entry via "dsEntryDN" operational attribute. This allows for maintaining the case of a DN from when it was first added. There is also a significant performance improvement when loading an entry from disk to the entry cache (using entryrdn index was very exspensive). Added config setting to turn this behavior on and off (default on) relates: 389ds#5267 Reviewed by: firstyear, progier, tbordaz, and spichugi(Thanks!!!!)
mreynolds389 · Oct 27, 2022 · f08a415 · f08a415
1 parent 8fedec0
commit f08a415
Show file tree

Hide file tree

Showing 19 changed files with 410 additions and 75 deletions.
diff --git a/dirsrvtests/create_test.py b/dirsrvtests/create_test.py
@@ -198,14 +198,14 @@ def display_uuid():
               'can not be greater than 99')
         display_usage()
 
-    if args.inst:
-        if not args.inst.isdigit() or \
-            int(args.inst) > 99 or \
-            int(args.inst) < 0:
+    if args.instances:
+        if not args.instances.isdigit() or \
+            int(args.instances) > 99 or \
+            int(args.instances) < 0:
             print('Invalid value for "--instances", it must be a number ' +
                   'greater than 0 and not greater than 99')
             display_usage()
-        if int(args.inst) > 0:
+        if int(args.instances) > 0:
             if int(args.suppliers) > 0 or \
                             int(args.hubs) > 0 or \
                             int(args.consumers) > 0:
@@ -216,12 +216,12 @@ def display_uuid():
     ticket = args.ticket
     suite = args.suite
 
-    if args.inst == '0' and args.suppliers == '0' and args.hubs == '0' \
+    if args.instances == '0' and args.suppliers == '0' and args.hubs == '0' \
        and args.consumers == '0':
         instances = 1
         my_topology = [True, 'topology_st', "Standalone Instance"]
     else:
-        instances = int(args.inst)
+        instances = int(args.instances)
         suppliers = int(args.suppliers)
         hubs = int(args.hubs)
         consumers = int(args.consumers)

diff --git a/dirsrvtests/tests/suites/basic/ds_entrydn_test.py b/dirsrvtests/tests/suites/basic/ds_entrydn_test.py
@@ -0,0 +1,96 @@
+# --- BEGIN COPYRIGHT BLOCK ---
+# Copyright (C) 2022 Red Hat, Inc.
+# All rights reserved.
+#
+# License: GPL (version 3 or any later version).
+# See LICENSE for details.
+# --- END COPYRIGHT BLOCK ---
+
+import logging
+import pytest
+import os
+import time
+from lib389.topologies import topology_st as topo
+from lib389.idm.user import UserAccount, UserAccounts
+from lib389.idm.organizationalunit import OrganizationalUnit
+
+log = logging.getLogger(__name__)
+
+SUFFIX = "dc=Example,DC=COM"
+SUBTREE = "ou=People,dc=Example,DC=COM"
+NEW_SUBTREE = "ou=humans,dc=Example,DC=COM"
+USER_DN = "uid=tUser,ou=People,dc=Example,DC=COM"
+NEW_USER_DN = "uid=tUser,ou=humans,dc=Example,DC=COM"
+NEW_USER_NORM_DN = "uid=tUser,ou=humans,dc=example,dc=com"
+
+
+def test_dsentrydn(topo):
+    """Test that the dsentrydn attribute is properly maintained and preserves the case of the DN
+
+    :id: f0f2fe6b-c70d-4de1-a9a9-06dda74e7c30
+    :setup: Standalone Instance
+    :steps:
+        1. Create user and make sure dsentrydn is set to the correct value/case
+        2. Moddn of "ou=people" and check dsentrydn is correct for parent and the children
+        3. Check the DN matches dsEntryDN
+        4. Disable nsslapd-return-original-entrydn
+        5. Check the DN matches normalized DN
+    :expectedresults:
+        1. Success
+        2. Success
+        3. Success
+        4. Success
+        5. Success
+    """
+
+    inst = topo.standalone
+    inst.config.replace('nsslapd-return-original-entrydn', 'on')
+
+    # Create user and makes sure "dsEntryDN" is set correctly
+    users = UserAccounts(inst, SUFFIX)
+    user_properties = {
+        'uid': 'tUser',
+        'givenname': 'test',
+        'cn': 'Test User',
+        'sn': 'user',
+        'userpassword': 'password',
+        'uidNumber': '1000',
+        'gidNumber': '2000',
+        'homeDirectory': '/home/tUser'
+    }
+    user = users.create(properties=user_properties)
+    assert user.get_attr_val_utf8('dsentrydn') == USER_DN
+
+    # Move subtree ou=people to ou=humans
+    ou = OrganizationalUnit(inst, SUBTREE)
+    ou.rename("ou=humans", SUFFIX)  # NEW_SUBTREE
+
+    # check dsEntryDN is properly updated to new subtree
+    ou = OrganizationalUnit(inst, NEW_SUBTREE)
+    assert ou.get_attr_val_utf8('dsentrydn') == NEW_SUBTREE
+
+    user = UserAccount(inst, NEW_USER_DN)
+    assert user.get_attr_val_utf8('dsentrydn') == NEW_USER_DN
+
+    # Check DN retruend to client matches "dsEntryDN"
+    users = UserAccounts(inst, SUFFIX).list()
+    for user in users:
+        if user.dn.startswith("tUser"):
+            assert user.dn == NEW_USER_DN
+            break
+
+    # Disable 'nsslapd-return-original-entrydn' andcheck DN is normalized
+    inst.config.replace('nsslapd-return-original-entrydn', 'on')
+    users = UserAccounts(inst, SUFFIX).list()
+    for user in users:
+        if user.dn.startswith("tUser"):
+            assert user.dn == NEW_USER_NORM_DN
+            break
+
+
+if __name__ == '__main__':
+    # Run isolated
+    # -s for DEBUG mode
+    CURRENT_FILE = os.path.realpath(__file__)
+    pytest.main(["-s", CURRENT_FILE])
+
diff --git a/dirsrvtests/tests/suites/ds_logs/ds_logs_test.py b/dirsrvtests/tests/suites/ds_logs/ds_logs_test.py
@@ -656,7 +656,7 @@ def test_internal_log_level_516(topology_st, add_user_log_level_516, disable_acc
                                     r'SRCH base="cn=group,ou=Groups,dc=example,dc=com".*')
     # (Internal) op=10(1)(2) ENTRY dn="cn=group,ou=Groups,dc=example,dc=com"
     assert topo.ds_access_log.match(r'.*\(Internal\) op=[0-9]+\([0-9]+\)\([0-9]+\) '
-                                    r'ENTRY dn="cn=group,ou=groups,dc=example,dc=com".*')
+                                    r'ENTRY dn="cn=group,ou=Groups,dc=example,dc=com".*')
     # (Internal) op=10(1)(2) RESULT err=0 tag=48 nentries=1*')
     assert topo.ds_access_log.match(r'.*\(Internal\) op=[0-9]+\([0-9]+\)\([0-9]+\) RESULT err=0 tag=48 nentries=1*')
     # (Internal) op=10(1)(1) RESULT err=0 tag=48

diff --git a/dirsrvtests/tests/suites/filter/filter_cert_test.py b/dirsrvtests/tests/suites/filter/filter_cert_test.py
@@ -57,11 +57,11 @@ def test_positive(topo):
     assert Accounts(topo.standalone, DEFAULT_SUFFIX).filter("(userCertificate;binary=*)")
     user1_cert = users_people.list()[0].get_attr_val("userCertificate;binary")
     assert Accounts(topo.standalone, DEFAULT_SUFFIX).filter(
-        f'(userCertificate;binary={search_filter_escape_bytes(user1_cert)})')[0].dn == \
+        f'(userCertificate;binary={search_filter_escape_bytes(user1_cert)})')[0].dn.lower() == \
            'uid=test_user_1,ou=people,dc=example,dc=com'
     user2_cert = users_people.list()[1].get_attr_val("userCertificate;binary")
     assert Accounts(topo.standalone, DEFAULT_SUFFIX).filter(
-        f'(userCertificate;binary={search_filter_escape_bytes(user2_cert)})')[0].dn == \
+        f'(userCertificate;binary={search_filter_escape_bytes(user2_cert)})')[0].dn.lower() == \
            'uid=test_user_2,ou=people,dc=example,dc=com'
 
 

diff --git a/dirsrvtests/tests/suites/filter/filter_logic_test.py b/dirsrvtests/tests/suites/filter/filter_logic_test.py
@@ -74,7 +74,7 @@ def _check_filter(topology_st_f, filt, expect_len, expect_dns):
     # print("checking %s" % filt)
     results = topology_st_f.search_s("ou=people,%s" % DEFAULT_SUFFIX, ldap.SCOPE_ONELEVEL, filt, ['uid',])
     assert len(results) == expect_len
-    result_dns = [result.dn for result in results]
+    result_dns = [result.dn.lower() for result in results]
     assert set(expect_dns) == set(result_dns)
 
 

diff --git a/dirsrvtests/tests/suites/filter/rfc3673_all_oper_attrs_test.py b/dirsrvtests/tests/suites/filter/rfc3673_all_oper_attrs_test.py
@@ -45,21 +45,22 @@
                (DN_PEOPLE, False, [
                 'aci', 'createTimestamp', 'creatorsName', 'entrydn',
                 'entryid', 'modifiersName', 'modifyTimestamp',
-                'nsUniqueId', 'numSubordinates', 'parentid'
+                'nsUniqueId', 'numSubordinates', 'parentid', 'dsEntryDN'
                ]),
                (DN_PEOPLE, True, [
                 'createTimestamp', 'creatorsName', 'entrydn',
                 'entryid', 'modifyTimestamp', 'nsUniqueId',
-                'numSubordinates', 'parentid'
+                'numSubordinates', 'parentid', 'dsEntryDN'
                ]),
                (TEST_USER_DN, False, [
                 'createTimestamp', 'creatorsName', 'entrydn',
                 'entryid', 'modifiersName', 'modifyTimestamp',
-                'nsUniqueId', 'parentid', 'entryUUID'
+                'nsUniqueId', 'parentid', 'entryUUID', 'dsEntryDN'
                ]),
                (TEST_USER_DN, True, [
                 'createTimestamp', 'creatorsName', 'entrydn',
-                'entryid', 'modifyTimestamp', 'nsUniqueId', 'parentid', 'entryUUID'
+                'entryid', 'modifyTimestamp', 'nsUniqueId', 'parentid',
+                'entryUUID', 'dsEntryDN'
                ]),
                (DN_CONFIG, False, [
                 'numSubordinates', 'passwordHistory',  'modifyTimestamp',

diff --git a/dirsrvtests/tests/suites/memberof_plugin/regression_test.py b/dirsrvtests/tests/suites/memberof_plugin/regression_test.py
@@ -319,7 +319,7 @@ def test_scheme_violation_errors_logged(topo_m2):
     assert user_memberof_attr
     log.info('memberOf attr value - {}'.format(user_memberof_attr))
 
-    pattern = ".*oc_check_allowed_sv.*{}.*memberOf.*not allowed.*".format(testuser.dn.lower())
+    pattern = ".*oc_check_allowed_sv.*{}.*memberOf.*not allowed.*".format(testuser.dn)
     log.info("pattern = %s" % pattern)
     assert inst.ds_error_log.match(pattern)
 

diff --git a/dirsrvtests/tests/suites/psearch/psearch_test.py b/dirsrvtests/tests/suites/psearch/psearch_test.py
@@ -65,7 +65,7 @@ def test_psearch(topology_st):
     # Now run the result again and see what's there.
     results = _run_psearch(topology_st.standalone, msg_id)
     # assert our group is in the changeset.
-    assert(group.dn.lower() == results[0])
+    assert(group.dn == results[0])
 
 
 if __name__ == '__main__':

diff --git a/dirsrvtests/tests/suites/replication/regression_m2_test.py b/dirsrvtests/tests/suites/replication/regression_m2_test.py
@@ -15,13 +15,12 @@
 import pytest
 import subprocess
 import time
-from lib389.idm.user import TEST_USER_PROPERTIES, UserAccounts
+from lib389.idm.user import TEST_USER_PROPERTIES, UserAccount, UserAccounts
 from lib389.pwpolicy import PwPolicyManager
 from lib389.utils import *
 from lib389._constants import *
 from lib389.idm.domain import Domain
 from lib389.idm.organizationalunit import OrganizationalUnits
-from lib389.idm.user import UserAccount
 from lib389.idm.group import Groups, Group
 from lib389.idm.domain import Domain
 from lib389.idm.directorymanager import DirectoryManager

diff --git a/ldap/schema/01core389.ldif b/ldap/schema/01core389.ldif
@@ -329,6 +329,8 @@ attributeTypes: ( 2.16.840.1.113730.3.1.2374 NAME 'nsDS5ReplicaBootstrapTranspor
 attributeTypes: ( 2.16.840.1.113730.3.1.2387 NAME 'nsslapd-tcp-fin-timeout' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' )
 attributeTypes: ( 2.16.840.1.113730.3.1.2388 NAME 'nsslapd-tcp-keepalive-time' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' )
 attributeTypes: ( 2.16.840.1.113730.3.1.2390 NAME 'nsds5ReplicaKeepAliveUpdateInterval' DESC '389 defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
+attributeTypes: ( 2.16.840.1.113730.3.1.9998 NAME 'dsEntryDN' DESC '389 Directory Server defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 NO-USER-MODIFICATION SINGLE-VALUE USAGE directoryOperation X-ORIGIN '389 Directory Server' )
+attributeTypes: ( 2.16.840.1.113730.3.1.9999 NAME 'nsslapd-return-original-entrydn' DESC '389 Directory Server defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
 #
 # objectclasses
 #

diff --git a/ldap/servers/slapd/back-ldbm/dn2entry.c b/ldap/servers/slapd/back-ldbm/dn2entry.c
@@ -54,16 +54,17 @@ dn2entry_ext(
     e = cache_find_dn(&inst->inst_cache, ndnv.bv_val, ndnv.bv_len);
     if (e == NULL) {
         ID id = ALLID;
+
         /* convert dn to entry id */
         if (entryrdn_get_switch()) { /* subtree-rename: on */
             *err = entryrdn_index_read_ext(be, sdn, &id,
                                            flags & TOMBSTONE_INCLUDED, txn);
+            indexname = LDBM_ENTRYRDN_STR;
             if (*err) {
                 if (DBI_RC_NOTFOUND != *err) {
-                    slapi_log_err(SLAPI_LOG_ERR,
-                                  "dn2entry_ext", "Failed to get id for %s "
-                                                  "from entryrdn index (%d)\n",
-                                  slapi_sdn_get_dn(sdn), *err);
+                    slapi_log_err(SLAPI_LOG_ERR, "dn2entry_ext",
+                                  "Failed to get id for %s from %s index: (%d)\n",
+                                  slapi_sdn_get_dn(sdn), indexname, *err);
                 }
                 /* There's no entry with this DN. */
                 goto bail;
@@ -78,7 +79,6 @@ dn2entry_ext(
                 /* There's no entry with this suffix. */
                 goto bail;
             }
-            indexname = LDBM_ENTRYRDN_STR;
         } else {
             IDList *idl = NULL;
             if ((idl = index_read(be, LDBM_ENTRYDN_STR, indextype_EQUALITY,
@@ -88,7 +88,6 @@ dn2entry_ext(
             }
             id = idl_firstid(idl);
             slapi_ch_free((void **)&idl);
-            indexname = LDBM_ENTRYDN_STR;
         }
         /* convert entry id to entry */
         if ((e = id2entry(be, id, txn, err)) != NULL) {

diff --git a/ldap/servers/slapd/back-ldbm/id2entry.c b/ldap/servers/slapd/back-ldbm/id2entry.c
@@ -67,6 +67,10 @@ id2entry_add_ext(backend *be, struct backentry *e, back_txn *txn, int encrypt, i
         int options = SLAPI_DUMP_STATEINFO | SLAPI_DUMP_UNIQUEID;
         Slapi_Entry *entry_to_use = encrypted_entry ? encrypted_entry->ep_entry : e->ep_entry;
         memset(&data, 0, sizeof(data));
+        entrydn = slapi_entry_get_dn(entry_to_use);
+        slapi_entry_attr_set_charptr(entry_to_use, SLAPI_ATTR_DS_ENTRYDN,
+                entrydn);
+
         if (entryrdn_get_switch()) {
             struct backdn *oldbdn = NULL;
             Slapi_DN *sdn =
@@ -141,7 +145,7 @@ id2entry_add_ext(backend *be, struct backentry *e, back_txn *txn, int encrypt, i
                         myparentdn = slapi_dn_parent_ext(
                             slapi_entry_get_dn_const(e->ep_entry),
                             is_tombstone);
-                        if (myparentdn && PL_strcmp(parentdn, myparentdn)) {
+                        if (myparentdn && PL_strcasecmp(parentdn, myparentdn)) {
                             Slapi_DN *sdn = slapi_entry_get_sdn(e->ep_entry);
                             char *newdn = NULL;
                             CACHE_LOCK(&inst->inst_cache);
@@ -350,21 +354,28 @@ id2entry(backend *be, ID id, back_txn *txn, int *err)
                 CACHE_RETURN(&inst->inst_dncache, &bdn);
             } else {
                 Slapi_DN *sdn = NULL;
-                rc = entryrdn_lookup_dn(be, rdn, id, &normdn, &srdn, txn);
-                if (rc) {
-                    slapi_log_err(SLAPI_LOG_TRACE, ID2ENTRY,
-                                  "id2entry: entryrdn look up failed "
-                                  "(rdn=%s, ID=%d)\n",
-                                  rdn, id);
-                    /* Try rdn as dn. Could be RUV. */
-                    normdn = slapi_ch_strdup(rdn);
-                } else if (NULL == normdn) {
-                    slapi_log_err(SLAPI_LOG_ERR, ID2ENTRY,
-                                  "id2entry( %lu ) entryrdn_lookup_dn returned NULL. "
-                                  "Index file may be deleted or corrupted.\n",
-                                  (u_long)id);
-                    goto bail;
+                if (config_get_return_orig_dn() &&
+                    !get_value_from_string((const char *)data.dptr, SLAPI_ATTR_DS_ENTRYDN, &normdn))
+                {
+                    srdn = slapi_rdn_new_all_dn(normdn);
+                } else {
+                    rc = entryrdn_lookup_dn(be, rdn, id, &normdn, &srdn, txn);
+                    if (rc) {
+                        slapi_log_err(SLAPI_LOG_TRACE, ID2ENTRY,
+                                      "id2entry: entryrdn look up failed "
+                                      "(rdn=%s, ID=%d)\n",
+                                      rdn, id);
+                        /* Try rdn as dn. Could be RUV. */
+                        normdn = slapi_ch_strdup(rdn);
+                    } else if (NULL == normdn) {
+                        slapi_log_err(SLAPI_LOG_ERR, ID2ENTRY,
+                                      "id2entry( %lu ) entryrdn_lookup_dn returned NULL. "
+                                      "Index file may be deleted or corrupted.\n",
+                                      (u_long)id);
+                        goto bail;
+                    }
                 }
+
                 sdn = slapi_sdn_new_normdn_byval((const char *)normdn);
                 bdn = backdn_init(sdn, id, 0);
                 if (CACHE_ADD(&inst->inst_dncache, bdn, NULL)) {
@@ -395,6 +406,7 @@ id2entry(backend *be, ID id, back_txn *txn, int *err)
 
         /* All entries should have uniqueids */
         PR_ASSERT(slapi_entry_get_uniqueid(ee) != NULL);
+
         /* ownership of the entry is passed into the backentry */
         e = backentry_init(ee);
         e->ep_id = id;