Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions openldap/changelog.d/21385.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix custom queries to properly use search_scope parameter
14 changes: 13 additions & 1 deletion openldap/datadog_checks/openldap/openldap.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
class OpenLDAP(AgentCheck):
METRIC_PREFIX = 'openldap'
SERVICE_CHECK_CONNECT = '{}.can_connect'.format(METRIC_PREFIX)
DEFAULT_SEARCH_SCOPE = ldap3.SUBTREE
SEARCH_SCOPE_MAPPING = {
"base": ldap3.BASE,
"level": ldap3.LEVEL,
"subtree": ldap3.SUBTREE,
}

SEARCH_BASE = 'cn=Monitor'
SEARCH_FILTER = '(objectClass=*)'
Expand Down Expand Up @@ -155,6 +161,12 @@ def _perform_custom_queries(self, conn, custom_queries, tags, instance):
self.log.error("`search_filter` field is required for custom query #%s", name)
continue
attrs = query.get("attributes")
search_scope_str = query.get("search_scope", "subtree").lower()
search_scope = self.SEARCH_SCOPE_MAPPING.get(search_scope_str)

if search_scope is None:
self.log.error("`search_scope` field needs to be one of 'base', 'level' or 'subtree'. Value provided: '%s'", search_scope_str)
continue
if "username" in query:
username = query.get("username")
password = query.get("password")
Expand All @@ -181,7 +193,7 @@ def _perform_custom_queries(self, conn, custom_queries, tags, instance):

try:
# Perform the search query
conn.search(search_base, search_filter, attributes=attrs)
conn.search(search_base, search_filter, search_scope=search_scope, attributes=attrs)
except ldap3.core.exceptions.LDAPException:
self.log.exception("Unable to perform search query for %s", name)
continue
Expand Down
101 changes: 99 additions & 2 deletions openldap/tests/test_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ def test__perform_custom_queries(check, mocker):
_, _, _, _, queries, tags = check._get_instance_params(instance)
check._perform_custom_queries(conn_mock, queries, tags, instance)
conn_mock.rebind.assert_called_once_with(user="user", password="pass", authentication=ldap3.SIMPLE)
conn_mock.search.assert_called_once_with("base", "filter", attributes=None)
conn_mock.search.assert_called_once_with("base", "filter", search_scope=ldap3.SUBTREE, attributes=None)
log_mock.error.assert_not_called() # No error logged

# Check query rebind different user
Expand All @@ -248,10 +248,107 @@ def test__perform_custom_queries(check, mocker):
_, _, _, _, queries, tags = check._get_instance_params(instance)
check._perform_custom_queries(conn_mock, queries, tags, instance)
conn_mock.rebind.assert_called_once_with(user="user2", password="pass2", authentication=ldap3.SIMPLE)
conn_mock.search.assert_called_once_with("base", "filter", attributes=["*"])
conn_mock.search.assert_called_once_with("base", "filter", search_scope=ldap3.SUBTREE, attributes=["*"])
log_mock.error.assert_not_called() # No error logged


@pytest.mark.parametrize(
"search_scope_str,expected_scope",
[
("base", ldap3.BASE),
("level", ldap3.LEVEL),
("subtree", ldap3.SUBTREE),
("BASE", ldap3.BASE),
("LEVEL", ldap3.LEVEL),
("SUBTREE", ldap3.SUBTREE),
]
)
def test_custom_query_search_scope_valid(check, mocker, search_scope_str, expected_scope):
"""Test that valid search_scope parameters are properly converted"""
instance = {
"url": "url",
"custom_queries": [
{
"name": "test_query",
"search_base": "ou=users,dc=example,dc=com",
"search_filter": "(objectClass=person)",
"search_scope": search_scope_str,
}
],
}

log_mock = mocker.MagicMock()
check.log = log_mock
conn_mock = mocker.MagicMock()
conn_mock.rebind.return_value = True
conn_mock.entries = []

_, _, _, _, queries, tags = check._get_instance_params(instance)
check._perform_custom_queries(conn_mock, queries, tags, instance)

conn_mock.search.assert_called_once_with(
"ou=users,dc=example,dc=com", "(objectClass=person)", search_scope=expected_scope, attributes=None
)


def test_custom_query_search_scope_default(check, mocker):
"""Test that missing search_scope defaults to SUBTREE"""
instance = {
"url": "url",
"custom_queries": [
{
"name": "test_query",
"search_base": "ou=users,dc=example,dc=com",
"search_filter": "(objectClass=person)",
# No search_scope specified
}
],
}

log_mock = mocker.MagicMock()
check.log = log_mock
conn_mock = mocker.MagicMock()
conn_mock.rebind.return_value = True
conn_mock.entries = []

_, _, _, _, queries, tags = check._get_instance_params(instance)
check._perform_custom_queries(conn_mock, queries, tags, instance)

conn_mock.search.assert_called_once_with(
"ou=users,dc=example,dc=com", "(objectClass=person)", search_scope=ldap3.SUBTREE, attributes=None
)


def test_custom_query_search_scope_invalid(check, mocker):
"""Test that invalid search_scope logs error and skips the query"""
instance = {
"url": "url",
"custom_queries": [
{
"name": "test_query",
"search_base": "ou=users,dc=example,dc=com",
"search_filter": "(objectClass=person)",
"search_scope": "invalid_scope",
}
],
}

log_mock = mocker.MagicMock()
check.log = log_mock
conn_mock = mocker.MagicMock()
conn_mock.rebind.return_value = True
conn_mock.entries = []

_, _, _, _, queries, tags = check._get_instance_params(instance)
check._perform_custom_queries(conn_mock, queries, tags, instance)

log_mock.error.assert_called_once_with(
"`search_scope` field needs to be one of 'base', 'level' or 'subtree'. Value provided: '%s'", "invalid_scope"
)
# Verify that search was NOT called due to invalid scope
conn_mock.search.assert_not_called()


def test__extract_common_name(check):
# Check lower case and spaces converted
dn = "cn=Max File Descriptors,cn=Connections,cn=Monitor"
Expand Down
Loading