218 lines
8.0 KiB
Diff
218 lines
8.0 KiB
Diff
From 58efd22cdefed368c97ef235f2abb66ac4aec234 Mon Sep 17 00:00:00 2001
|
|
From: tbordaz <tbordaz@redhat.com>
|
|
Date: Thu, 5 Sep 2024 10:19:59 +0200
|
|
Subject: [PATCH] Issue 6307 - Wrong set of entries returned for some search
|
|
filters (#6308)
|
|
|
|
Bug description:
|
|
When the server returns an entry to a search it
|
|
checks both access and matching of the filter.
|
|
When evaluating a '!' (NOT) logical expression the server,
|
|
in a first phase evaluates ONLY the right to access the
|
|
related component (and its subcomponents).
|
|
Then in a second phase verifies the matching.
|
|
If the related component is a OR, in the first phase it
|
|
evaluates access AND matching, this even if the call was
|
|
to evaluate only access.
|
|
This result in incoherent results.
|
|
|
|
Fix description:
|
|
Make sure that when the function vattr_test_filter_list_or
|
|
is called to only check access, it does not evaluate the matching.
|
|
|
|
fixes: #6307
|
|
|
|
Reviewed by: Pierre Rogier (Thanks !!)
|
|
---
|
|
.../tests/suites/filter/filter_test.py | 116 ++++++++++++++++++
|
|
ldap/servers/slapd/filterentry.c | 11 ++
|
|
2 files changed, 127 insertions(+)
|
|
|
|
diff --git a/dirsrvtests/tests/suites/filter/filter_test.py b/dirsrvtests/tests/suites/filter/filter_test.py
|
|
index b17846c01..e1d02f133 100644
|
|
--- a/dirsrvtests/tests/suites/filter/filter_test.py
|
|
+++ b/dirsrvtests/tests/suites/filter/filter_test.py
|
|
@@ -10,12 +10,14 @@ import logging
|
|
|
|
import pytest
|
|
import time
|
|
+from ldap import SCOPE_SUBTREE
|
|
from lib389.dirsrv_log import DirsrvAccessLog
|
|
from lib389.tasks import *
|
|
from lib389.backend import Backends, Backend
|
|
from lib389.dbgen import dbgen_users, dbgen_groups
|
|
from lib389.topologies import topology_st
|
|
from lib389._constants import PASSWORD, DEFAULT_SUFFIX, DN_DM, SUFFIX, DN_CONFIG_LDBM
|
|
+from lib389.idm.user import UserAccount, UserAccounts
|
|
from lib389.utils import *
|
|
|
|
pytestmark = pytest.mark.tier1
|
|
@@ -432,6 +434,120 @@ def test_match_large_valueset(topology_st):
|
|
assert len(entries) == groups_number
|
|
assert (etime < 5)
|
|
|
|
+def test_filter_not_operator(topology_st, request):
|
|
+ """Test ldapsearch with scope one gives only single entry
|
|
+
|
|
+ :id: b3711e02-7e76-444d-82f3-495c6dadd97f
|
|
+ :setup: Standalone instance
|
|
+ :steps:
|
|
+ 1. Creating user1..user9
|
|
+ 2. Adding specific 'telephonenumber' to the users
|
|
+ 3. Check returned set (5 users) with the first filter
|
|
+ 4. Check returned set (4 users) with the second filter
|
|
+ :expectedresults:
|
|
+ 1. This should pass
|
|
+ 2. This should pass
|
|
+ 3. This should pass
|
|
+ 4. This should pass
|
|
+ """
|
|
+
|
|
+ topology_st.standalone.start()
|
|
+ # Creating Users
|
|
+ log.info('Create users from user1 to user9')
|
|
+ users = UserAccounts(topology_st.standalone, "ou=people,%s" % DEFAULT_SUFFIX, rdn=None)
|
|
+
|
|
+ for user in ['user1',
|
|
+ 'user2',
|
|
+ 'user3',
|
|
+ 'user4',
|
|
+ 'user5',
|
|
+ 'user6',
|
|
+ 'user7',
|
|
+ 'user8',
|
|
+ 'user9']:
|
|
+ users.create(properties={
|
|
+ 'mail': f'{user}@redhat.com',
|
|
+ 'uid': user,
|
|
+ 'givenName': user.title(),
|
|
+ 'cn': f'bit {user}',
|
|
+ 'sn': user.title(),
|
|
+ 'manager': f'uid={user},{SUFFIX}',
|
|
+ 'userpassword': PW_DM,
|
|
+ 'homeDirectory': '/home/' + user,
|
|
+ 'uidNumber': '1000',
|
|
+ 'gidNumber': '2000',
|
|
+ })
|
|
+ # Adding specific values to the users
|
|
+ log.info('Adding telephonenumber values')
|
|
+ user = UserAccount(topology_st.standalone, 'uid=user1, ou=people, %s' % DEFAULT_SUFFIX)
|
|
+ user.add('telephonenumber', ['1234', '2345'])
|
|
+
|
|
+ user = UserAccount(topology_st.standalone, 'uid=user2, ou=people, %s' % DEFAULT_SUFFIX)
|
|
+ user.add('telephonenumber', ['1234', '4567'])
|
|
+
|
|
+ user = UserAccount(topology_st.standalone, 'uid=user3, ou=people, %s' % DEFAULT_SUFFIX)
|
|
+ user.add('telephonenumber', ['1234', '4567'])
|
|
+
|
|
+ user = UserAccount(topology_st.standalone, 'uid=user4, ou=people, %s' % DEFAULT_SUFFIX)
|
|
+ user.add('telephonenumber', ['1234'])
|
|
+
|
|
+ user = UserAccount(topology_st.standalone, 'uid=user5, ou=people, %s' % DEFAULT_SUFFIX)
|
|
+ user.add('telephonenumber', ['2345'])
|
|
+
|
|
+ user = UserAccount(topology_st.standalone, 'uid=user6, ou=people, %s' % DEFAULT_SUFFIX)
|
|
+ user.add('telephonenumber', ['3456'])
|
|
+
|
|
+ user = UserAccount(topology_st.standalone, 'uid=user7, ou=people, %s' % DEFAULT_SUFFIX)
|
|
+ user.add('telephonenumber', ['4567'])
|
|
+
|
|
+ user = UserAccount(topology_st.standalone, 'uid=user8, ou=people, %s' % DEFAULT_SUFFIX)
|
|
+ user.add('telephonenumber', ['1234'])
|
|
+
|
|
+ user = UserAccount(topology_st.standalone, 'uid=user9, ou=people, %s' % DEFAULT_SUFFIX)
|
|
+ user.add('telephonenumber', ['1234', '4567'])
|
|
+
|
|
+ # Do a first test of filter containing a NOT
|
|
+ # and check the expected values are retrieved
|
|
+ log.info('Search with filter containing NOT')
|
|
+ log.info('expect user2, user3, user6, user8 and user9')
|
|
+ filter1 = "(|(telephoneNumber=3456)(&(telephoneNumber=1234)(!(|(uid=user1)(uid=user4)))))"
|
|
+ entries = topology_st.standalone.search_s(DEFAULT_SUFFIX, SCOPE_SUBTREE, filter1)
|
|
+ uids = []
|
|
+ for entry in entries:
|
|
+ assert entry.hasAttr('uid')
|
|
+ uids.append(entry.getValue('uid'))
|
|
+
|
|
+ assert len(uids) == 5
|
|
+ for uid in [b'user2', b'user3', b'user6', b'user8', b'user9']:
|
|
+ assert uid in uids
|
|
+
|
|
+ # Do a second test of filter containing a NOT
|
|
+ # and check the expected values are retrieved
|
|
+ log.info('Search with a second filter containing NOT')
|
|
+ log.info('expect user2, user3, user8 and user9')
|
|
+ filter1 = "(|(&(telephoneNumber=1234)(!(|(uid=user1)(uid=user4)))))"
|
|
+ entries = topology_st.standalone.search_s(DEFAULT_SUFFIX, SCOPE_SUBTREE, filter1)
|
|
+ uids = []
|
|
+ for entry in entries:
|
|
+ assert entry.hasAttr('uid')
|
|
+ uids.append(entry.getValue('uid'))
|
|
+
|
|
+ assert len(uids) == 4
|
|
+ for uid in [b'user2', b'user3', b'user8', b'user9']:
|
|
+ assert uid in uids
|
|
+
|
|
+ def fin():
|
|
+ """
|
|
+ Deletes entries after the test.
|
|
+ """
|
|
+ for user in users.list():
|
|
+ pass
|
|
+ user.delete()
|
|
+
|
|
+
|
|
+ request.addfinalizer(fin)
|
|
+
|
|
+
|
|
if __name__ == '__main__':
|
|
# Run isolated
|
|
# -s for DEBUG mode
|
|
diff --git a/ldap/servers/slapd/filterentry.c b/ldap/servers/slapd/filterentry.c
|
|
index d2c7e3082..f5604161d 100644
|
|
--- a/ldap/servers/slapd/filterentry.c
|
|
+++ b/ldap/servers/slapd/filterentry.c
|
|
@@ -948,14 +948,17 @@ slapi_vattr_filter_test_ext_internal(
|
|
break;
|
|
|
|
case LDAP_FILTER_NOT:
|
|
+ slapi_log_err(SLAPI_LOG_FILTER, "vattr_test_filter_list_NOT", "=>\n");
|
|
rc = slapi_vattr_filter_test_ext_internal(pb, e, f->f_not, verify_access, only_check_access, access_check_done);
|
|
if (verify_access && only_check_access) {
|
|
/* dont play with access control return codes
|
|
* do not negate return code */
|
|
+ slapi_log_err(SLAPI_LOG_FILTER, "vattr_test_filter_list_NOT only check access", "<= %d\n", rc);
|
|
break;
|
|
}
|
|
if (rc > 0) {
|
|
/* an error occurred or access denied, don't negate */
|
|
+ slapi_log_err(SLAPI_LOG_FILTER, "vattr_test_filter_list_NOT slapi_vattr_filter_test_ext_internal fails", "<= %d\n", rc);
|
|
break;
|
|
}
|
|
if (verify_access) {
|
|
@@ -980,6 +983,7 @@ slapi_vattr_filter_test_ext_internal(
|
|
/* filter verification only, no error */
|
|
rc = (rc == 0) ? -1 : 0;
|
|
}
|
|
+ slapi_log_err(SLAPI_LOG_FILTER, "vattr_test_filter_list_NOT", "<= %d\n", rc);
|
|
break;
|
|
|
|
default:
|
|
@@ -1084,6 +1088,13 @@ vattr_test_filter_list_or(
|
|
continue;
|
|
}
|
|
}
|
|
+ /* we are not evaluating if the entry matches
|
|
+ * but only that we have access to ALL components
|
|
+ * so check the next one
|
|
+ */
|
|
+ if (only_check_access) {
|
|
+ continue;
|
|
+ }
|
|
/* now check if filter matches */
|
|
/*
|
|
* We can NOT skip this because we need to know if the item we matched on
|
|
--
|
|
2.47.1
|
|
|