From 290fedb1d35c6bb099e081e720ce1a600f270e46 Mon Sep 17 00:00:00 2001 From: William Brown Date: Fri, 5 May 2017 11:21:03 +1000 Subject: [PATCH] Ticket 48985 - Add schema for nested groups to work out of box. Bug Description: Previously, nestedGroups didn't work on pure ds because you needed to add inetUser to a group. As well, the auto objectClass didn't work as it would be NULL by default. Fix Description: Add schema and update defaults for memberof to work out of the box with no alterations. Make it the default add objectClass, and because of it's simple nature, it can apply to groups and users. https://pagure.io/389-ds-base/issue/48985 Author: wibrown Review by: ??? --- dirsrvtests/tests/suites/plugins/memberof_test.py | 51 +++++++++++++++++++---- ldap/admin/src/scripts/fixup-memberof.pl.in | 2 +- ldap/schema/30ns-common.ldif | 2 + ldap/servers/plugins/memberof/memberof.c | 2 +- ldap/servers/plugins/memberof/memberof.h | 1 + ldap/servers/plugins/memberof/memberof_config.c | 15 ++++++- 6 files changed, 60 insertions(+), 13 deletions(-) diff --git a/dirsrvtests/tests/suites/plugins/memberof_test.py b/dirsrvtests/tests/suites/plugins/memberof_test.py index 5880964..f4bf225 100644 --- a/dirsrvtests/tests/suites/plugins/memberof_test.py +++ b/dirsrvtests/tests/suites/plugins/memberof_test.py @@ -128,16 +128,12 @@ def text_memberof_683241_01(topology_st): [(ldap.MOD_REPLACE, PLUGIN_TYPE, 'betxnpostoperation')]) - topology_st.standalone.restart(timeout=10) + topology_st.standalone.restart() ent = topology_st.standalone.getEntry(MEMBEROF_PLUGIN_DN, ldap.SCOPE_BASE, "(objectclass=*)", [PLUGIN_TYPE]) assert ent.hasAttr(PLUGIN_TYPE) assert ent.getValue(PLUGIN_TYPE) == 'betxnpostoperation' -def test_memberof_setloging(topology_st): - topology_st.standalone.modify_s('cn=config', [(ldap.MOD_REPLACE, 'nsslapd-errorlog-level', str(65536))]) - - def test_memberof_MultiGrpAttr_001(topology_st): """ Checking multiple grouping attributes supported @@ -156,7 +152,7 @@ def test_memberof_MultiGrpAttr_003(topology_st): """ log.info("Enable MemberOf plugin") topology_st.standalone.plugins.enable(name=PLUGIN_MEMBER_OF) - topology_st.standalone.restart(timeout=10) + topology_st.standalone.restart() ent = topology_st.standalone.getEntry(MEMBEROF_PLUGIN_DN, ldap.SCOPE_BASE, "(objectclass=*)", [PLUGIN_ENABLED]) assert ent.hasAttr(PLUGIN_ENABLED) assert ent.getValue(PLUGIN_ENABLED).lower() == 'on' @@ -294,7 +290,7 @@ def test_memberof_MultiGrpAttr_008(topology_st): [(ldap.MOD_DELETE, PLUGIN_MEMBEROF_GRP_ATTR, ['uniqueMember'])]) - topology_st.standalone.restart(timeout=10) + topology_st.standalone.restart() log.info("Assert that this change of configuration did change the already set values") # assert enh1 is member of grp1 and is NOT member of grp2 @@ -306,7 +302,7 @@ def test_memberof_MultiGrpAttr_008(topology_st): assert _check_memberof(topology_st, member=memofenh2, group=memofegrp2) _set_memberofgroupattr_add(topology_st, 'uniqueMember') - topology_st.standalone.restart(timeout=10) + topology_st.standalone.restart() def test_memberof_MultiGrpAttr_009(topology_st): @@ -2420,7 +2416,43 @@ def test_memberof_auto_add_oc(topology_st): # Enable the plugin topology_st.standalone.plugins.enable(name=PLUGIN_MEMBER_OF) - # First test invalid value (config validation) + # Test that the default add OC works. + + try: + topology_st.standalone.add_s(Entry((USER1_DN, + {'objectclass': 'top', + 'objectclass': 'person', + 'objectclass': 'organizationalPerson', + 'objectclass': 'inetorgperson', + 'sn': 'last', + 'cn': 'full', + 'givenname': 'user1', + 'uid': 'user1' + }))) + except ldap.LDAPError as e: + log.fatal('Failed to add user1 entry, error: ' + e.message['desc']) + assert False + + # Add a group(that already includes one user + try: + topology_st.standalone.add_s(Entry((GROUP_DN, + {'objectclass': 'top', + 'objectclass': 'groupOfNames', + 'cn': 'group', + 'member': USER1_DN + }))) + except ldap.LDAPError as e: + log.fatal('Failed to add group entry, error: ' + e.message['desc']) + assert False + + # Assert memberOf on user1 + _check_memberof(topology_st, USER1_DN, GROUP_DN) + + # Reset for the next test .... + topology_st.standalone.delete_s(USER1_DN) + topology_st.standalone.delete_s(GROUP_DN) + + # Test invalid value (config validation) topology_st.standalone.plugins.enable(name=PLUGIN_MEMBER_OF) try: topology_st.standalone.modify_s(MEMBEROF_PLUGIN_DN, @@ -2435,6 +2467,7 @@ def test_memberof_auto_add_oc(topology_st): ldap.error('Unexpected error adding invalid objectclass - error: ' + e.message['desc']) assert False + # Add valid objectclass topology_st.standalone.plugins.enable(name=PLUGIN_MEMBER_OF) try: diff --git a/ldap/admin/src/scripts/fixup-memberof.pl.in b/ldap/admin/src/scripts/fixup-memberof.pl.in index fc00c2f..167ed7f 100644 --- a/ldap/admin/src/scripts/fixup-memberof.pl.in +++ b/ldap/admin/src/scripts/fixup-memberof.pl.in @@ -32,7 +32,7 @@ sub usage { print(STDERR " -j filename - Read Directory Manager's password from file\n"); print(STDERR " -b baseDN - Base DN that contains entries to fix up.\n"); print(STDERR " -f filter - Filter for entries to fix up\n"); - print(STDERR " If omitted, all entries with objectclass inetuser/inetadmin under the\n"); + print(STDERR " If omitted, all entries with objectclass inetuser/inetadmin/nsmemberof under the\n"); print(STDERR " specified base will have their memberOf attribute regenerated.\n"); print(STDERR " -P protocol - STARTTLS, LDAPS, LDAPI, LDAP (default: uses most secure protocol available)\n"); print(STDERR " -h - Display usage\n"); diff --git a/ldap/schema/30ns-common.ldif b/ldap/schema/30ns-common.ldif index ebe540a..b095909 100644 --- a/ldap/schema/30ns-common.ldif +++ b/ldap/schema/30ns-common.ldif @@ -63,3 +63,5 @@ objectClasses: ( nsTaskGroup-oid NAME 'nsTaskGroup' DESC 'Netscape defined objec objectClasses: ( nsAdminObject-oid NAME 'nsAdminObject' DESC 'Netscape defined objectclass' SUP top MUST ( cn ) MAY ( nsJarFilename $ nsClassName ) X-ORIGIN 'Netscape' ) objectClasses: ( nsConfig-oid NAME 'nsConfig' DESC 'Netscape defined objectclass' SUP top MUST ( cn ) MAY ( description $ nsServerPort $ nsServerAddress $ nsSuiteSpotUser $ nsErrorLog $ nsPidLog $ nsAccessLog $ nsDefaultAcceptLanguage $ nsServerSecurity ) X-ORIGIN 'Netscape' ) objectClasses: ( nsDirectoryInfo-oid NAME 'nsDirectoryInfo' DESC 'Netscape defined objectclass' SUP top MUST ( cn ) MAY ( nsBindDN $ nsBindPassword $ nsDirectoryURL $ nsDirectoryFailoverList $ nsDirectoryInfoRef ) X-ORIGIN 'Netscape' ) +objectClasses: ( 2.16.840.1.113730.3.2.329 NAME 'nsMemberOf' DESC 'Allow memberOf assignment on groups for nesting and users' SUP top AUXILIARY MAY ( memberOf ) X-ORIGIN '389 Directory Server Project' ) + diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c index 8d36b80..b37f1a1 100644 --- a/ldap/servers/plugins/memberof/memberof.c +++ b/ldap/servers/plugins/memberof/memberof.c @@ -3140,7 +3140,7 @@ int memberof_task_add(Slapi_PBlock *pb, goto out; } - if ((filter = fetch_attr(e, "filter", "(|(objectclass=inetuser)(objectclass=inetadmin))")) == NULL) + if ((filter = fetch_attr(e, "filter", "(|(objectclass=inetuser)(objectclass=inetadmin)(objectclass=nsmemberof))")) == NULL) { *returncode = LDAP_OBJECT_CLASS_VIOLATION; rv = SLAPI_DSE_CALLBACK_ERROR; diff --git a/ldap/servers/plugins/memberof/memberof.h b/ldap/servers/plugins/memberof/memberof.h index 9a3a6a2..723c32e 100644 --- a/ldap/servers/plugins/memberof/memberof.h +++ b/ldap/servers/plugins/memberof/memberof.h @@ -42,6 +42,7 @@ #define MEMBEROF_ENTRY_SCOPE_EXCLUDE_SUBTREE "memberOfEntryScopeExcludeSubtree" #define MEMBEROF_SKIP_NESTED_ATTR "memberOfSkipNested" #define MEMBEROF_AUTO_ADD_OC "memberOfAutoAddOC" +#define NSMEMBEROF "nsMemberOf" #define DN_SYNTAX_OID "1.3.6.1.4.1.1466.115.121.1.12" #define NAME_OPT_UID_SYNTAX_OID "1.3.6.1.4.1.1466.115.121.1.34" diff --git a/ldap/servers/plugins/memberof/memberof_config.c b/ldap/servers/plugins/memberof/memberof_config.c index 522bfb7..c1bba2f 100644 --- a/ldap/servers/plugins/memberof/memberof_config.c +++ b/ldap/servers/plugins/memberof/memberof_config.c @@ -285,13 +285,19 @@ memberof_validate_config (Slapi_PBlock *pb, } } - if ((auto_add_oc = slapi_entry_attr_get_charptr(e, MEMBEROF_AUTO_ADD_OC))){ + /* Setup a default auto add OC */ + auto_add_oc = slapi_entry_attr_get_charptr(e, MEMBEROF_AUTO_ADD_OC); + if (auto_add_oc == NULL) { + auto_add_oc = slapi_ch_strdup(NSMEMBEROF); + } + + if (auto_add_oc != NULL) { char *sup = NULL; /* Check if the objectclass exists by looking for its superior oc */ if((sup = slapi_schema_get_superior_name(auto_add_oc)) == NULL){ PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE, - "The %s configuration attribute must be set to " + "The %s configuration attribute must be set " "to an existing objectclass (unknown: %s)", MEMBEROF_AUTO_ADD_OC, auto_add_oc); *returncode = LDAP_UNWILLING_TO_PERFORM; @@ -515,6 +521,10 @@ memberof_apply_config (Slapi_PBlock *pb __attribute__((unused)), skip_nested = slapi_entry_attr_get_charptr(e, MEMBEROF_SKIP_NESTED_ATTR); auto_add_oc = slapi_entry_attr_get_charptr(e, MEMBEROF_AUTO_ADD_OC); + if (auto_add_oc == NULL) { + auto_add_oc = slapi_ch_strdup(NSMEMBEROF); + } + /* * We want to be sure we don't change the config in the middle of * a memberOf operation, so we obtain an exclusive lock here @@ -638,6 +648,7 @@ memberof_apply_config (Slapi_PBlock *pb __attribute__((unused)), theConfig.allBackends = 0; } + slapi_ch_free_string(&(theConfig.auto_add_oc)); theConfig.auto_add_oc = auto_add_oc; /* -- 1.8.3.1