#50531 Issue 50530 - Directory Server not RFC 4511 compliant with requested attr "1.1"
Merged 6 months ago by mreynolds. Opened 6 months ago by mreynolds.
mreynolds/389-ds-base issue50530  into  master

@@ -35,6 +35,7 @@ 

  USER1_DN = 'uid=user1,' + DEFAULT_SUFFIX

  USER2_DN = 'uid=user2,' + DEFAULT_SUFFIX

  USER3_DN = 'uid=user3,' + DEFAULT_SUFFIX

+ USER4_DN = 'uid=user4,' + DEFAULT_SUFFIX

  

  ROOTDSE_DEF_ATTR_LIST = ('namingContexts',

                           'supportedLDAPVersion',

@@ -440,8 +441,8 @@ 

                                               'uid': 'user1',

                                               'userpassword': PASSWORD})))

      except ldap.LDAPError as e:

-         log.fatal('test_basic_acl: Failed to add test user ' + USER1_DN

-                   + ': error ' + e.message['desc'])

+         log.fatal('test_basic_acl: Failed to add test user ' + USER1_DN +

+                   ': error ' + e.message['desc'])

          assert False

  

      try:

@@ -452,8 +453,8 @@ 

                                               'uid': 'user2',

                                               'userpassword': PASSWORD})))

      except ldap.LDAPError as e:

-         log.fatal('test_basic_acl: Failed to add test user ' + USER1_DN

-                   + ': error ' + e.message['desc'])

+         log.fatal('test_basic_acl: Failed to add test user ' + USER1_DN +

+                   ': error ' + e.message['desc'])

          assert False

  

      #

@@ -603,6 +604,50 @@ 

      log.info('test_basic_searches: PASSED')

  

  

+ @pytest.fixture(scope="module")

+ def add_test_entry(topology_st, request):

+     # Add test entry

+     topology_st.standalone.add_s(Entry((USER4_DN,

+                                         {'objectclass': "top extensibleObject".split(),

+                                          'cn': 'user1', 'uid': 'user1'})))

+ 

+ 

+ search_params = [(['1.1'], 'cn', False),

+                  (['1.1', 'cn'], 'cn', True),

+                  (['+'], 'nsUniqueId', True),

+                  (['*'], 'cn', True),

+                  (['cn'], 'cn', True)]

+ @pytest.mark.parametrize("attrs, attr, present", search_params)

+ def test_search_req_attrs(topology_st, add_test_entry, attrs, attr, present):

+     """Test requested attributes in search operations.

+     :id: 426a59ff-49b8-4a70-b377-0c0634a29b6e

+     :setup: Standalone instance

+     :steps:

+          1. Test "1.1" does not return any attributes.

+          2. Test "1.1" is ignored if there are other requested attributes

+          3. Test "+" returns all operational attributes

+          4. Test "*" returns all attributes

+          5. Test requested attributes

+ 

+     :expectedresults:

+          1. Success

+          2. Success

+          3. Success

+          4. Success

+          5. Success

+     """

+ 

+     log.info("Testing attrs: {} attr: {} present: {}".format(attrs, attr, present))

+     entry = topology_st.standalone.search_s(USER4_DN,

+                                             ldap.SCOPE_BASE,

+                                             'objectclass=top',

+                                             attrs)

+     if present:

+         assert entry[0].hasAttr(attr)

+     else:

+         assert not entry[0].hasAttr(attr)

+ 

+ 

  def test_basic_referrals(topology_st, import_example_ldif):

      """Test LDAP server in referral mode.

  

@@ -747,8 +792,8 @@ 

      log.info('Attempting to start the server with broken dse.ldif...')

      try:

          topology_st.standalone.start()

-     except:

-         log.info('Server failed to start as expected')

+     except Exception as e:

+         log.info('Server failed to start as expected: ' + str(e))

      log.info('Check the status...')

      assert (not topology_st.standalone.status())

      log.info('Server failed to start as expected')

file modified
+6 -1

@@ -1546,6 +1546,8 @@ 

       * "+" means all operational attributes (rfc3673)

       * operational attributes are only retrieved if they are named

       * specifically or when "+" is specified.

+      * In the case of "1.1", if there are other requested attributes

+      * then "1.1" should be ignored.

       */

  

      /* figure out if we want all user attributes or no attributes at all */

@@ -1560,7 +1562,10 @@ 

              if (strcmp(LDAP_ALL_USER_ATTRS, attrs[i]) == 0) {

                  alluserattrs = 1;

              } else if (strcmp(LDAP_NO_ATTRS, attrs[i]) == 0) {

-                 noattrs = 1;

+                 /* "1.1" is only valid if it's the only requested attribute */

+                 if (i == 0 && attrs[1] == NULL) {

+                     noattrs = 1;

+                 }

              } else if (strcmp(LDAP_ALL_OPERATIONAL_ATTRS, attrs[i]) == 0) {

                  alloperationalattrs = 1;

              } else {

Bug Description:

A regression was introduced some time back that changed the behavior of how the server handled the "1.1" requested attribute in a search request. If "1.1" was requested along with other attributes then no attributes were returned, but in this case "1.1" is expected to be ignored.

Fix Description: Only comply with "1.1" if it is the only requested attribute

relates: https://pagure.io/389-ds-base/issue/50530

Ack this seems good to me. You could use test case parameterisation to make this cleaner?

rebased onto 4159cf6

6 months ago

Ack this seems good to me. You could use test case parameterisation to make this cleaner?

Done, thanks!

Pull-Request has been merged by mreynolds

6 months ago