From 60198729ba59f673aae2ae1db1d9668b674ad429 Mon Sep 17 00:00:00 2001

From: Thierry Bordaz <tbordaz@redhat.com>

Date: Fri, 5 Jan 2018 15:31:44 +0100

Subject: [PATCH] Ticket 49523 - memberof: schema violation error message is

 confusing as memberof will likely repair target entry

Bug Description:

	When memberof is enabled it adds 'memberof' attribute to members entries.

	If a member entry has not the appropriate objectclass to support 'memberof' attribute an ERR is logged.

	ERR - oc_check_allowed_sv - Entry "cn=user_1,ou=People,dc=example,dc=com" -- attribute "memberOf" not allowed

	This is confusing because memberof will catch this violation and may try to repair it.

	So although this message is alarming, the target entry may finally have the 'memberof' attribute.

	This is especially confusing since https://pagure.io/389-ds-base/issue/48985 where the repair operation

	is done by default (if schema is violated)

	We can not (and should not) eliminate the schema violation message.

	But memberof should log a additional warning (beside the schema violation msg) stating it repaired the violation.

Fix Description:

	Add a warning message upon repair operation

		ERR - oc_check_allowed_sv - Entry "<entry_dn>" -- attribute "memberOf" not allowed

		WARN - memberof-plugin - Entry <entry_dn> - schema violation caught - repair operation succeeded

https://pagure.io/389-ds-base/issue/49523

Reviewed by: Mark Reynolds

Platforms tested: F26

Flag Day: no

Doc impact: no

---

 dirsrvtests/tests/tickets/ticket49523_test.py | 154 ++++++++++++++++++++++++++

 ldap/servers/plugins/memberof/memberof.c      |   8 +-

 2 files changed, 161 insertions(+), 1 deletion(-)

 create mode 100644 dirsrvtests/tests/tickets/ticket49523_test.py

diff --git a/dirsrvtests/tests/tickets/ticket49523_test.py b/dirsrvtests/tests/tickets/ticket49523_test.py

new file mode 100644

index 000000000..c3296ef07

--- /dev/null

+++ b/dirsrvtests/tests/tickets/ticket49523_test.py

@@ -0,0 +1,154 @@

+import logging

+import pytest

+import os

+import ldap

+import time

+import re

+from lib389.plugins import MemberOfPlugin

+from lib389._constants import *

+from lib389.topologies import topology_st as topo

+from lib389 import Entry

+

+DEBUGGING = os.getenv("DEBUGGING", default=False)

+if DEBUGGING:

+    logging.getLogger(__name__).setLevel(logging.DEBUG)

+else:

+    logging.getLogger(__name__).setLevel(logging.INFO)

+log = logging.getLogger(__name__)

+

+

+USER_CN='user_'

+GROUP_CN='group_'

+def _user_get_dn(no):

+    cn = '%s%d' % (USER_CN, no)

+    dn = 'cn=%s,ou=people,%s' % (cn, SUFFIX)

+    return (cn, dn)

+

+def add_user(server, no, desc='dummy', sleep=True):

+    (cn, dn) = _user_get_dn(no)

+    log.fatal('Adding user (%s): ' % dn)

+    server.add_s(Entry((dn, {'objectclass': ['top', 'person'],

+                             'cn': [cn],

+                             'description': [desc],

+                             'sn': [cn],

+                             'description': ['add on that host']})))

+    if sleep:

+        time.sleep(2)

+        

+def add_group(server, nr, sleep=True):

+    cn = '%s%d' % (GROUP_CN, nr)

+    dn = 'cn=%s,ou=groups,%s' % (cn, SUFFIX)

+    server.add_s(Entry((dn, {'objectclass': ['top', 'groupofnames'],

+                             'description': 'group %d' % nr})))

+    if sleep:

+        time.sleep(2)

+

+def update_member(server, member_dn, group_dn, op, sleep=True):

+    mod = [(op, 'member', member_dn)]

+    server.modify_s(group_dn, mod)

+    if sleep:

+        time.sleep(2)

+        

+def _find_memberof(server, member_dn, group_dn, find_result=True):

+    ent = server.getEntry(member_dn, ldap.SCOPE_BASE, "(objectclass=*)", ['memberof'])

+    found = False

+    if ent.hasAttr('memberof'):

+

+        for val in ent.getValues('memberof'):

+            server.log.info("!!!!!!! %s: memberof->%s" % (member_dn, val))

+            server.log.info("!!!!!!! %s" % (val))

+            server.log.info("!!!!!!! %s" % (group_dn))

+            if val.lower() == group_dn.lower():

+                found = True

+                break

+

+    if find_result:

+        assert (found)

+    else:

+        assert (not found)

+        

+def pattern_accesslog(server, log_pattern):

+    file_obj = open(server.accesslog, "r")

+

+    found = False

+    # Use a while true iteration because 'for line in file: hit a

+    while True:

+        line = file_obj.readline()

+        found = log_pattern.search(line)

+        if ((line == '') or (found)):

+            break

+

+    return found

+

+def pattern_errorlog(server, log_pattern):

+    file_obj = open(server.errlog, "r")

+

+    found = None

+    # Use a while true iteration because 'for line in file: hit a

+    while True:

+        line = file_obj.readline()

+        found = log_pattern.search(line)

+        server.log.fatal("%s --> %s" % (line, found))

+        if ((line == '') or (found)):

+            break

+

+    return found

+        

+def test_ticket49523(topo):

+    """Specify a test case purpose or name here

+

+    :id: e2af0aaa-447e-4e85-a5ce-57ae66260d0b

+    :setup: Fill in set up configuration here

+    :steps:

+        1. Fill in test case steps here

+        2. And indent them like this (RST format requirement)

+    :expectedresults:

+        1. Fill in the result that is expected

+        2. For each test step

+    """

+

+    # If you need any test suite initialization,

+    # please, write additional fixture for that (including finalizer).

+    # Topology for suites are predefined in lib389/topologies.py.

+

+    # If you need host, port or any other data about instance,

+    # Please, use the instance object attributes for that (for example, topo.ms["master1"].serverid)

+    inst = topo.standalone

+    memberof = MemberOfPlugin(inst)

+    memberof.enable()

+    memberof.set_autoaddoc('nsMemberOf')

+    inst.restart()

+    

+    # Step 2

+    for i in range(10):

+        add_user(inst, i, desc='add user')

+    

+    add_group(inst, 1)

+    

+    group_parent_dn = 'ou=groups,%s' % (SUFFIX)

+    group_rdn = 'cn=%s%d' % (GROUP_CN, 1)

+    group_dn  = '%s,%s' % (group_rdn, group_parent_dn)

+    (member_cn, member_dn) = _user_get_dn(1)

+    update_member(inst, member_dn, group_dn, ldap.MOD_ADD, sleep=False)

+    

+    _find_memberof(inst, member_dn, group_dn, find_result=True)

+    

+    pattern = ".*oc_check_allowed_sv - Entry.*cn=%s.* -- attribute.*not allowed.*" % member_cn

+    log.fatal("pattern = %s" % pattern)

+    regex = re.compile(pattern)

+    assert pattern_errorlog(inst, regex)

+    

+    regex = re.compile(".*schema violation caught - repair operation.*")

+    assert pattern_errorlog(inst, regex)

+

+    if DEBUGGING:

+        # Add debugging steps(if any)...

+        pass

+

+

+if __name__ == '__main__':

+    # Run isolated

+    # -s for DEBUG mode

+    CURRENT_FILE = os.path.realpath(__file__)

+    pytest.main("-s %s" % CURRENT_FILE)

+

diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c

index 44b52edbb..fcfa7817d 100644

--- a/ldap/servers/plugins/memberof/memberof.c

+++ b/ldap/servers/plugins/memberof/memberof.c

@@ -3236,8 +3236,14 @@ memberof_add_memberof_attr(LDAPMod **mods, const char *dn, char *add_oc)

                  */

                 break;

             }

-            if (memberof_add_objectclass(add_oc, dn)) {

+            rc = memberof_add_objectclass(add_oc, dn);

+            slapi_log_err(SLAPI_LOG_WARNING, MEMBEROF_PLUGIN_SUBSYSTEM,

+                    "Entry %s - schema violation caught - repair operation %s\n",

+                    dn ? dn : "unknown",

+                    rc ? "failed" : "succeeded");

+            if (rc) {

                 /* Failed to add objectclass */

+                rc = LDAP_OBJECT_CLASS_VIOLATION;

                 break;

             }

             added_oc = 1;

-- 

2.13.6