diff --git a/guacamole_user_sync/ldap/ldap_client.py b/guacamole_user_sync/ldap/ldap_client.py index db759f5..666c96c 100644 --- a/guacamole_user_sync/ldap/ldap_client.py +++ b/guacamole_user_sync/ldap/ldap_client.py @@ -5,6 +5,7 @@ from ldap3.abstract.entry import Entry from ldap3.core.exceptions import ( LDAPBindError, + LDAPException, LDAPSessionTerminatedByServerError, LDAPSocketOpenError, ) @@ -26,12 +27,14 @@ def __init__( self, hostname: str, *, + auto_bind: bool = True, bind_dn: str | None = None, bind_password: str | None = None, ) -> None: + self.auto_bind = auto_bind self.bind_dn = bind_dn self.bind_password = bind_password - self.hostname = hostname + self.server = Server(hostname, get_info=ALL) @staticmethod def as_list(ldap_entry: str | list[str] | None) -> list[str]: @@ -45,20 +48,26 @@ def as_list(ldap_entry: str | list[str] | None) -> list[str]: raise ValueError(msg) def connect(self) -> Connection: - logger.info("Initialising connection to LDAP host at %s", self.hostname) + logger.info("Initialising connection to LDAP host at %s", self.server.host) try: - server = Server(self.hostname, get_info=ALL) - if self.bind_dn: - return Connection( - server, - self.bind_dn, - self.bind_password, - auto_bind=True, - ) - return Connection(server, auto_bind=True) + return Connection( + self.server, + user=self.bind_dn, + password=self.bind_password, + auto_bind=self.auto_bind, + ) + except LDAPSocketOpenError as exc: + msg = "Server could not be reached." + logger.exception(msg, exc_info=exc) + raise LDAPError(msg) from exc except LDAPBindError as exc: - logger.warning("Connection credentials were incorrect.") - raise LDAPError from exc + msg = "Connection credentials were incorrect." + logger.exception(msg, exc_info=exc) + raise LDAPError(msg) from exc + except LDAPException as exc: + msg = f"Unexpected LDAP exception of type {type(exc)}." + logger.exception(msg, exc_info=exc) + raise LDAPError(msg) from exc def search_groups(self, query: LDAPQuery) -> list[LDAPGroup]: output = [] @@ -96,13 +105,14 @@ def search(self, query: LDAPQuery) -> list[Entry]: try: connection = self.connect() connection.search(query.base_dn, query.filter, attributes=ALL_ATTRIBUTES) - except LDAPSocketOpenError as exc: - logger.warning("Server could not be reached.") - raise LDAPError from exc - except LDAPSessionTerminatedByServerError as exc: - logger.warning("Server terminated LDAP request.") - raise LDAPError from exc + msg = "Server terminated LDAP request." + logger.exception(msg, exc_info=exc) + raise LDAPError(msg) from exc + except LDAPException as exc: + msg = f"Unexpected LDAP exception of type {type(exc)}." + logger.exception(msg, exc_info=exc) + raise LDAPError(msg) from exc else: results = cast(list[Entry], connection.entries) logger.debug("Server returned %s results.", len(results)) diff --git a/guacamole_user_sync/postgresql/postgresql_client.py b/guacamole_user_sync/postgresql/postgresql_client.py index 36ee4d9..f23e8b9 100644 --- a/guacamole_user_sync/postgresql/postgresql_client.py +++ b/guacamole_user_sync/postgresql/postgresql_client.py @@ -89,7 +89,11 @@ def assign_users_to_groups( ) continue # Get the user_entity_id for each user belonging to this group - logger.debug("Group '%s' has %s members", group.name, len(group.member_uid)) + logger.debug( + "Group '%s' has %s member(s).", + group.name, + len(group.member_uid), + ) for user_uid in group.member_uid: try: user = next(filter(lambda u: u.uid == user_uid, users)) diff --git a/tests/conftest.py b/tests/conftest.py index 12afadf..a0e4515 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,7 +2,7 @@ import pytest -from guacamole_user_sync.models import LDAPGroup, LDAPQuery, LDAPSearchResult, LDAPUser +from guacamole_user_sync.models import LDAPGroup, LDAPQuery, LDAPUser from guacamole_user_sync.postgresql.orm import ( GuacamoleEntity, GuacamoleEntityType, @@ -10,6 +10,8 @@ GuacamoleUserGroup, ) +from .mocks import MockLDAPGroupEntry, MockLDAPUserEntry + @pytest.fixture def ldap_model_groups_fixture() -> list[LDAPGroup]: @@ -69,70 +71,45 @@ def ldap_query_users_fixture() -> LDAPQuery: @pytest.fixture -def ldap_response_groups_fixture() -> LDAPSearchResult: +def ldap_response_groups_fixture() -> list[MockLDAPGroupEntry]: return [ - ( - 0, - ( - "CN=plaintiffs,OU=groups,DC=rome,DC=la", - { - "cn": [b"plaintiffs"], - "memberOf": [], - "memberUid": [b"aulus.agerius"], - }, - ), - ), - ( - 1, - ( - "CN=defendants,OU=groups,DC=rome,DC=la", - { - "cn": [b"defendants"], - "memberOf": [], - "memberUid": [b"numerius.negidius"], - }, - ), - ), - ( - 2, - ( - "CN=everyone,OU=groups,DC=rome,DC=la", - { - "cn": [b"everyone"], - "memberOf": [], - "memberUid": [b"aulus.agerius", b"numerius.negidius"], - }, - ), + MockLDAPGroupEntry( + dn="CN=plaintiffs,OU=groups,DC=rome,DC=la", + cn="plaintiffs", + memberOf=[], + memberUid=["aulus.agerius"], + ), + MockLDAPGroupEntry( + dn="CN=defendants,OU=groups,DC=rome,DC=la", + cn="defendants", + memberOf=[], + memberUid=["numerius.negidius"], + ), + MockLDAPGroupEntry( + dn="CN=everyone,OU=groups,DC=rome,DC=la", + cn="everyone", + memberOf=[], + memberUid=["aulus.agerius", "numerius.negidius"], ), ] @pytest.fixture -def ldap_response_users_fixture() -> LDAPSearchResult: +def ldap_response_users_fixture() -> list[MockLDAPUserEntry]: return [ - ( - 0, - ( - "CN=aulus.agerius,OU=users,DC=rome,DC=la", - { - "displayName": [b"Aulus Agerius"], - "memberOf": [b"CN=plaintiffs,OU=groups,DC=rome,DC=la"], - "uid": [b"aulus.agerius"], - "userName": [b"aulus.agerius@rome.la"], - }, - ), - ), - ( - 1, - ( - "CN=numerius.negidius,OU=users,DC=rome,DC=la", - { - "displayName": [b"Numerius Negidius"], - "memberOf": [b"CN=defendants,OU=groups,DC=rome,DC=la"], - "uid": [b"numerius.negidius"], - "userName": [b"numerius.negidius@rome.la"], - }, - ), + MockLDAPUserEntry( + dn="CN=aulus.agerius,OU=users,DC=rome,DC=la", + displayName="Aulus Agerius", + memberOf=["CN=plaintiffs,OU=groups,DC=rome,DC=la"], + uid="aulus.agerius", + userName="aulus.agerius@rome.la", + ), + MockLDAPUserEntry( + dn="CN=numerius.negidius,OU=users,DC=rome,DC=la", + displayName="Numerius Negidius", + memberOf=["CN=defendants,OU=groups,DC=rome,DC=la"], + uid="numerius.negidius", + userName="numerius.negidius@rome.la", ), ] diff --git a/tests/mocks.py b/tests/mocks.py index 02a7b24..2895208 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -1,67 +1,88 @@ from typing import Any -import ldap +from ldap3.core.exceptions import LDAPBindError from sqlalchemy import TextClause -from guacamole_user_sync.models import LDAPSearchResult from guacamole_user_sync.postgresql.orm import GuacamoleBase -class MockLDAPObject: - """Mock LDAPObject.""" +class MockLDAPAttribute: + """Mock LDAP value.""" - def __init__(self, uri: str) -> None: - self.uri = uri - self.bind_dn = "" - self.bind_password = "" + def __init__(self, value: str | float | list[str]) -> None: + self.value = value - def simple_bind_s(self, bind_dn: str, bind_password: str) -> None: - if bind_password == "incorrect-password": # noqa: S105 - raise ldap.INVALID_CREDENTIALS - self.bind_dn = bind_dn - self.bind_password = bind_password - -class MockAsyncSearchList: - """Mock AsyncSearchList.""" +class MockLDAPGroupEntry: + """Mock LDAP group entry.""" def __init__( self, - partial: bool, # noqa: FBT001 - results: LDAPSearchResult, - *args: Any, # noqa: ANN401, ARG002 - **kwargs: Any, # noqa: ANN401, ARG002 + dn: str, + cn: str, + memberOf: list[str], # noqa: N803 + memberUid: list[str], # noqa: N803 ) -> None: - self.allResults = results - self.partial = partial + self.dn = MockLDAPAttribute(dn) + self.cn = MockLDAPAttribute(cn) + self.memberOf = MockLDAPAttribute(memberOf) + self.memberUid = MockLDAPAttribute(memberUid) - def startSearch( # noqa: N802 - self, - *args: Any, # noqa: ANN401 - **kwargs: Any, # noqa: ANN401 - ) -> None: - pass - def processResults( # noqa: N802 +class MockLDAPUserEntry: + """Mock LDAP user entry.""" + + def __init__( self, - *args: Any, # noqa: ANN401, ARG002 - **kwargs: Any, # noqa: ANN401, ARG002 - ) -> bool: - return self.partial + dn: str, + displayName: str, # noqa: N803 + memberOf: list[str], # noqa: N803 + uid: str, + userName: str, # noqa: N803 + ) -> None: + self.dn = MockLDAPAttribute(dn) + self.displayName = MockLDAPAttribute(displayName) + self.memberOf = MockLDAPAttribute(memberOf) + self.uid = MockLDAPAttribute(uid) + self.userName = MockLDAPAttribute(userName) -class MockAsyncSearchListFullResults(MockAsyncSearchList): - """Mock AsyncSearchList with full results.""" +class MockLDAPServer: + """Mock LDAP server.""" - def __init__(self, results: LDAPSearchResult) -> None: - super().__init__(results=results, partial=False) + def __init__( + self, + entries: list[MockLDAPGroupEntry] | list[MockLDAPUserEntry], + ) -> None: + self.entries = entries -class MockAsyncSearchListPartialResults(MockAsyncSearchList): - """Mock AsyncSearchList with partial results.""" +class MockLDAPConnection: + """Mock LDAP connection.""" - def __init__(self, results: LDAPSearchResult) -> None: - super().__init__(results=results, partial=True) + def __init__( + self, + server: MockLDAPServer | None = None, + user: str | None = None, + password: str | None = None, + *, + auto_bind: bool = False, + ) -> None: + self.auto_bind = auto_bind + self.password = password + if password == "incorrect-password": # noqa: S105 + raise LDAPBindError + self.server = server + self.user = user + + def search( + self, + base_dn: str, # noqa: ARG002 + ldap_filter: str, # noqa: ARG002 + attributes: str, # noqa: ARG002 + ) -> None: + if self.server: + self.entries = self.server.entries class MockPostgreSQLBackend: diff --git a/tests/test_ldap.py b/tests/test_ldap.py index af61854..dd887d0 100644 --- a/tests/test_ldap.py +++ b/tests/test_ldap.py @@ -1,23 +1,27 @@ import logging -from typing import Any from unittest import mock -import ldap import pytest +from ldap3 import Connection, Server +from ldap3.core.exceptions import ( + LDAPBindError, + LDAPException, + LDAPSessionTerminatedByServerError, +) from guacamole_user_sync.ldap import LDAPClient from guacamole_user_sync.models import ( LDAPError, LDAPGroup, LDAPQuery, - LDAPSearchResult, LDAPUser, ) from .mocks import ( - MockAsyncSearchListFullResults, - MockAsyncSearchListPartialResults, - MockLDAPObject, + MockLDAPConnection, + MockLDAPGroupEntry, + MockLDAPServer, + MockLDAPUserEntry, ) @@ -25,140 +29,143 @@ class TestLDAPClient: """Test LDAPClient.""" def test_constructor(self) -> None: - client = LDAPClient(hostname="test-host") - assert client.hostname == "test-host" + client = LDAPClient("ldap://test-host") + assert isinstance(client.server, Server) + assert client.server.host == "test-host" - def test_connect(self, monkeypatch: pytest.MonkeyPatch) -> None: - def mock_initialize(uri: str) -> MockLDAPObject: - return MockLDAPObject(uri) - - monkeypatch.setattr(ldap, "initialize", mock_initialize) + def test_connect_invalid_server(self) -> None: + client = LDAPClient("test-host") + with pytest.raises(LDAPError, match="Server could not be reached."): + client.connect() - client = LDAPClient(hostname="test-host") + def test_connect(self) -> None: + client = LDAPClient("test-host", auto_bind=False) cnxn = client.connect() - assert isinstance(cnxn, MockLDAPObject) - assert cnxn.uri == "ldap://test-host" - - def test_connect_with_bind(self, monkeypatch: pytest.MonkeyPatch) -> None: - def mock_initialize(uri: str) -> MockLDAPObject: - return MockLDAPObject(uri) - - monkeypatch.setattr(ldap, "initialize", mock_initialize) + assert isinstance(cnxn, Connection) + def test_connect_with_bind(self) -> None: client = LDAPClient( - hostname="test-host", + "test-host", + auto_bind=False, bind_dn="bind-dn", bind_password="bind_password", # noqa: S106 ) cnxn = client.connect() - assert isinstance(cnxn, MockLDAPObject) - assert cnxn.bind_dn == "bind-dn" - assert cnxn.bind_password == "bind_password" # noqa: S105 + assert isinstance(cnxn, Connection) + assert cnxn.user == "bind-dn" + assert cnxn.password == "bind_password" # noqa: S105 - def test_connect_with_failed_bind( - self, - monkeypatch: pytest.MonkeyPatch, - caplog: pytest.LogCaptureFixture, - ) -> None: - def mock_initialize(uri: str) -> MockLDAPObject: - return MockLDAPObject(uri) - - monkeypatch.setattr(ldap, "initialize", mock_initialize) - - client = LDAPClient( - hostname="test-host", - bind_dn="bind-dn", - bind_password="incorrect-password", # noqa: S106 - ) - with pytest.raises(LDAPError): - client.connect() - assert "Connection credentials were incorrect." in caplog.text - - def test_search_exception_server_down( - self, - monkeypatch: pytest.MonkeyPatch, - caplog: pytest.LogCaptureFixture, - ) -> None: - def mock_raise_server_down(*args: Any) -> None: # noqa: ANN401, ARG001 - raise ldap.SERVER_DOWN - - monkeypatch.setattr( - ldap.asyncsearch.List, - "startSearch", - mock_raise_server_down, - ) - client = LDAPClient(hostname="test-host") - with pytest.raises(LDAPError): - client.search(query=LDAPQuery(base_dn="", filter="", id_attr="")) - assert "Server could not be reached." in caplog.text + def test_connect_with_failed_bind(self, caplog: pytest.LogCaptureFixture) -> None: + with mock.patch( + "guacamole_user_sync.ldap.ldap_client.Connection", + return_value=MockLDAPConnection(), + side_effect=LDAPBindError(), + ): + client = LDAPClient( + hostname="test-host", + bind_dn="bind-dn", + bind_password="incorrect-password", # noqa: S106 + ) + with pytest.raises(LDAPError): + client.connect() + assert "Connection credentials were incorrect." in caplog.text - def test_search_exception_sizelimit_exceeded( + def test_connect_unknown_exception(self) -> None: + with mock.patch( + "guacamole_user_sync.ldap.ldap_client.Connection", + return_value=MockLDAPConnection(), + side_effect=LDAPException(), + ): + client = LDAPClient(hostname="test-host", auto_bind=False) + class_name = "" + with pytest.raises( + LDAPError, + match=f"Unexpected LDAP exception of type {class_name}", + ): + client.connect() + + @pytest.mark.parametrize( + ("test_input", "expected"), + [("test", ["test"]), (None, []), (["a", "b"], ["a", "b"])], + ) + def test_as_list_valid( self, - monkeypatch: pytest.MonkeyPatch, - caplog: pytest.LogCaptureFixture, + test_input: str | list[str] | None, + expected: list[str], ) -> None: - def mock_raise_sizelimit_exceeded(*args: Any) -> None: # noqa: ANN401, ARG001 - raise ldap.SIZELIMIT_EXCEEDED + assert LDAPClient.as_list(test_input) == expected - monkeypatch.setattr( - ldap.asyncsearch.List, - "startSearch", - mock_raise_sizelimit_exceeded, - ) - client = LDAPClient(hostname="test-host") - with pytest.raises(LDAPError): - client.search(query=LDAPQuery(base_dn="", filter="", id_attr="")) - assert "Server-side size limit exceeded." in caplog.text + @pytest.mark.parametrize(("test_input", "expected"), [(42, "int"), (1.5, "float")]) + def test_as_list_invalid(self, test_input: float, expected: str) -> None: + with pytest.raises( + ValueError, + match=f"Unexpected input {test_input} of type ", + ): + LDAPClient.as_list(test_input) # type: ignore[arg-type] - def test_search_failure_partial( - self, - caplog: pytest.LogCaptureFixture, - ldap_response_groups_fixture: LDAPSearchResult, - ) -> None: - caplog.set_level(logging.DEBUG) + def test_search_session_terminated(self) -> None: with mock.patch( - "guacamole_user_sync.ldap.ldap_client.AsyncSearchList", - ) as mock_async_search_list: - mock_async_search_list.return_value = MockAsyncSearchListPartialResults( - results=ldap_response_groups_fixture[0:1], - ) - client = LDAPClient(hostname="test-host") - client.search(query=LDAPQuery(base_dn="", filter="", id_attr="")) - assert "Only partial results received." in caplog.text - assert "Server returned 1 results." in caplog.text + "guacamole_user_sync.ldap.ldap_client.Connection.search", + return_value=[], + side_effect=LDAPSessionTerminatedByServerError(), + ): + client = LDAPClient(hostname="test-host", auto_bind=False) + with pytest.raises( + LDAPError, + match="Server terminated LDAP request.", + ): + client.search(query=LDAPQuery(base_dn="", filter="", id_attr="")) + + def test_search_unknown_exception(self) -> None: + with mock.patch( + "guacamole_user_sync.ldap.ldap_client.Connection.search", + return_value=[], + side_effect=LDAPException(), + ): + client = LDAPClient(hostname="test-host", auto_bind=False) + class_name = "" + with pytest.raises( + LDAPError, + match=f"Unexpected LDAP exception of type {class_name}", + ): + client.search(query=LDAPQuery(base_dn="", filter="", id_attr="")) def test_search_no_results( self, - monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, ) -> None: - def mock_raise_no_results(*args: Any) -> None: # noqa: ANN401, ARG001 - raise ldap.NO_SUCH_OBJECT - - monkeypatch.setattr(ldap.asyncsearch.List, "startSearch", mock_raise_no_results) - client = LDAPClient(hostname="test-host") - with pytest.raises(LDAPError): - client.search(query=LDAPQuery(base_dn="", filter="", id_attr="")) - assert "Server returned no results." in caplog.text + caplog.set_level(logging.DEBUG) + monkeypatch.setattr( + LDAPClient, + "connect", + lambda _: MockLDAPConnection(server=MockLDAPServer([])), + ) + client = LDAPClient(hostname="test-host", auto_bind=False) + client.search(query=LDAPQuery(base_dn="", filter="", id_attr="")) + assert "Server returned 0 results." in caplog.text def test_search_groups( self, caplog: pytest.LogCaptureFixture, ldap_query_groups_fixture: LDAPQuery, - ldap_response_groups_fixture: LDAPSearchResult, + ldap_response_groups_fixture: list[MockLDAPGroupEntry], ldap_model_groups_fixture: list[LDAPGroup], + monkeypatch: pytest.MonkeyPatch, ) -> None: caplog.set_level(logging.DEBUG) - with mock.patch( - "guacamole_user_sync.ldap.ldap_client.AsyncSearchList", - ) as mock_async_search_list: - mock_async_search_list.return_value = MockAsyncSearchListFullResults( - results=ldap_response_groups_fixture, - ) - client = LDAPClient(hostname="test-host") - users = client.search_groups(query=ldap_query_groups_fixture) - for user in ldap_model_groups_fixture: - assert user in users + monkeypatch.setattr( + LDAPClient, + "connect", + lambda _: MockLDAPConnection( + server=MockLDAPServer(ldap_response_groups_fixture), + ), + ) + client = LDAPClient(hostname="test-host", auto_bind=False) + groups = client.search_groups(query=ldap_query_groups_fixture) + for group in ldap_model_groups_fixture: + assert group in groups + assert "base DN: OU=groups,DC=rome,DC=la" in caplog.text assert "Server returned 3 results." in caplog.text assert "Loaded 3 LDAP groups" in caplog.text @@ -166,19 +173,22 @@ def test_search_users( self, caplog: pytest.LogCaptureFixture, ldap_query_users_fixture: LDAPQuery, - ldap_response_users_fixture: LDAPSearchResult, + ldap_response_users_fixture: list[MockLDAPUserEntry], ldap_model_users_fixture: list[LDAPUser], + monkeypatch: pytest.MonkeyPatch, ) -> None: caplog.set_level(logging.DEBUG) - with mock.patch( - "guacamole_user_sync.ldap.ldap_client.AsyncSearchList", - ) as mock_async_search_list: - mock_async_search_list.return_value = MockAsyncSearchListFullResults( - results=ldap_response_users_fixture, - ) - client = LDAPClient(hostname="test-host") - users = client.search_users(query=ldap_query_users_fixture) - for user in ldap_model_users_fixture: - assert user in users + monkeypatch.setattr( + LDAPClient, + "connect", + lambda _: MockLDAPConnection( + server=MockLDAPServer(ldap_response_users_fixture), + ), + ) + client = LDAPClient(hostname="test-host") + users = client.search_users(query=ldap_query_users_fixture) + for user in ldap_model_users_fixture: + assert user in users + assert "base DN: OU=users,DC=rome,DC=la" in caplog.text assert "Server returned 2 results." in caplog.text assert "Loaded 2 LDAP users" in caplog.text diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 3c8f6e4..e1ba8f2 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -340,9 +340,11 @@ def test_assign_users_to_groups_missing_usergroup( ) for output_line in ( + "Ensuring that 2 user(s) are correctly assigned among 3 group(s)", + "Working on group 'defendants'", "Could not determine user_group_id for group 'defendants'.", - "-> entity_id: 2; user_group_id: 12", - "-> entity_id: 3; user_group_id: 13", + "Group 'everyone' has entity_id: 2 and user_group_id: 12", + "Group 'plaintiffs' has entity_id: 3 and user_group_id: 13", ): assert output_line in caplog.text @@ -498,15 +500,18 @@ def test_update( "There are 2 valid user entit(y|ies)", "Ensuring that 2 user(s) are correctly assigned among 3 group(s)", "Working on group 'defendants'", - "-> entity_id: None; user_group_id: None", - "... group member 'LDAPUser(display_name='Numerius Negidius'", + "Group 'defendants' has entity_id: None and user_group_id: None", + "Group 'defendants' has 1 member(s).", + " ... group member 'numerius.negidius@rome.la' has entity_id 'None'", "Working on group 'everyone'", - "-> entity_id: None; user_group_id: None", - "... group member 'LDAPUser(display_name='Aulus Agerius'", - "... group member 'LDAPUser(display_name='Numerius Negidius'", + "Group 'everyone' has entity_id: None and user_group_id: None", + "Group 'everyone' has 2 member(s).", + " ... group member 'aulus.agerius@rome.la' has entity_id 'None'", + " ... group member 'numerius.negidius@rome.la' has entity_id 'None'", "Working on group 'plaintiffs'", - "-> entity_id: None; user_group_id: None", - "... group member 'LDAPUser(display_name='Aulus Agerius'", + "Group 'plaintiffs' has entity_id: None and user_group_id: None", + "Group 'plaintiffs' has 1 member(s).", + " ... group member 'aulus.agerius@rome.la' has entity_id 'None'", "... creating 4 user/group assignments.", ): assert output_line in caplog.text