From ef53ee3a7098a6ef2b1e264f1757e3ff6399091d Mon Sep 17 00:00:00 2001 From: tobias haubold Date: Mon, 9 Nov 2020 21:50:34 +0000 Subject: [PATCH 1/5] [task] add guide and config to develop in a docker container with visual studio code --- .devcontainer.json | 13 +++++++++++++ CONTRIBUTING.md | 22 ++++++++++++++++++++++ dev.docker-compose.yml | 32 ++++++++++++++++++++++++++++++++ dev.dockerfile | 6 ++++++ 4 files changed, 73 insertions(+) create mode 100644 .devcontainer.json create mode 100644 dev.docker-compose.yml create mode 100644 dev.dockerfile diff --git a/.devcontainer.json b/.devcontainer.json new file mode 100644 index 0000000..fdffd1d --- /dev/null +++ b/.devcontainer.json @@ -0,0 +1,13 @@ +{ + "dockerComposeFile": "dev.docker-compose.yml", + "workspaceFolder": "/workspace", + "service": "jh-ldap-dev", + // install runtime dependencies + install pre-commit hooks + // note: although it is somehow weird to install the software you want to develop + // it installs all runtime dependencies which are maintained in `setup.py` + "postCreateCommand": "pip install --editable . && pre-commit install", + "extensions": [ + "ms-azuretools.vscode-docker", + "ms-python.python" + ] +} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 01086fc..9eee301 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -50,6 +50,28 @@ To clean up your development LDAP deployment, run: docker rm -f ldap ``` +## Alternative Development Environment Setup + +You can use [*Visual Studio Code*](https://code.visualstudio.com/) with the [*remote containers*](https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.remote-containers) extension to develop in docker container: + +1. install visual studio code and the remote containers extension +1. clone the git repository and open the project in visual studio code +1. open the development container, either do: + - click on the green button in the bottom left and choose `reopen in container` + - press `Shift + Ctrl + P` and start typing `reopen in container` and choose it + - open menu `View` and click on `Command Palette..` and start typing `reopen in container` and choose it +1. open a new terminal (menu `Terminal` » `New Terminal`) and run all tests with: `pytest -v` + - note: all should run successfully + +Troubleshooting (note the early development stage of the remote containers extension): + +- after you run `reopen in container` use the command palette and choose `remote containers: show log` + - sometimes the `postCreateCommand` does not run successfully +- further information + - guide for dev containers: https://code.visualstudio.com/docs/remote/create-dev-container#_use-docker-compose + - `.devcontainer.json` reference: https://code.visualstudio.com/docs/remote/devcontainerjson-reference + - docker compose file reference: https://docs.docker.com/compose/compose-file/ + ## Contributing JupyterHub has adopted automatic code formatting so you shouldn't diff --git a/dev.docker-compose.yml b/dev.docker-compose.yml new file mode 100644 index 0000000..f2c21e8 --- /dev/null +++ b/dev.docker-compose.yml @@ -0,0 +1,32 @@ +version: '3' +services: + # open ldap server to test authentication gainst + # see: https://github.com/rroemhild/docker-test-openldap + ldap: + image: rroemhild/test-openldap + networks: + ldapnet: + aliases: + - hub-test-ldap + + jh-ldap-dev: + build: + dockerfile: dev.dockerfile + context: . + args: + PYTHON_VERSION: 3.6 + depends_on: + - ldap + # prevent container from exiting before vs code executes commands in it + command: /bin/sh -c "while sleep 1000; do :; done" + # the ldap host for testing + environment: + LDAP_HOST: hub-test-ldap + volumes: + # bind mount the project directory for development inside the container + - .:/workspace:cached + networks: + - ldapnet + +networks: + ldapnet: diff --git a/dev.dockerfile b/dev.dockerfile new file mode 100644 index 0000000..057c27d --- /dev/null +++ b/dev.dockerfile @@ -0,0 +1,6 @@ +ARG PYTHON_VERSION +FROM python:$PYTHON_VERSION + +# install all development dependencies +COPY dev-requirements.txt /home/ +RUN pip install -r /home/dev-requirements.txt From 830836c43c979ceaf5a37e616e90e2f213df5f89 Mon Sep 17 00:00:00 2001 From: tobias haubold Date: Mon, 30 Nov 2020 17:49:19 +0100 Subject: [PATCH 2/5] [task] add link to info about ldap structure used in tests --- ldapauthenticator/tests/conftest.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ldapauthenticator/tests/conftest.py b/ldapauthenticator/tests/conftest.py index 545744f..575a2f4 100644 --- a/ldapauthenticator/tests/conftest.py +++ b/ldapauthenticator/tests/conftest.py @@ -34,4 +34,7 @@ def authenticator(): "cn=ship_crew,ou=people,dc=planetexpress,dc=com", ] + # see link below for ldap structure including users: + # https://github.com/rroemhild/docker-test-openldap + return authenticator From fed3cbe6ce55ab8f00fb24d7ac337add171cc8a0 Mon Sep 17 00:00:00 2001 From: tobias haubold Date: Mon, 30 Nov 2020 18:44:41 +0100 Subject: [PATCH 3/5] [task] add option to check allowed groups using the ldap connection of the search user instead of the ldap connection of the user being authenticated, see #183 --- ldapauthenticator/ldapauthenticator.py | 87 ++++++++---- .../tests/test_ldapconnection_usage.py | 126 ++++++++++++++++++ 2 files changed, 187 insertions(+), 26 deletions(-) create mode 100755 ldapauthenticator/tests/test_ldapconnection_usage.py diff --git a/ldapauthenticator/ldapauthenticator.py b/ldapauthenticator/ldapauthenticator.py index 36f2e0d..4b90b0c 100644 --- a/ldapauthenticator/ldapauthenticator.py +++ b/ldapauthenticator/ldapauthenticator.py @@ -229,19 +229,29 @@ def _server_port_default(self): """, ) - def resolve_username(self, username_supplied_by_user): - search_dn = self.lookup_dn_search_user - if self.escape_userdn: - search_dn = escape_filter_chars(search_dn) - conn = self.get_connection( - userdn=search_dn, password=self.lookup_dn_search_password - ) - is_bound = conn.bind() - if not is_bound: - msg = "Failed to connect to LDAP server with search user '{search_dn}'" - self.log.warning(msg.format(search_dn=search_dn)) - return (None, None) + use_search_user_to_check_groups = Bool( + False, + config=True, + help="""If set to true uses the ldap connection of the configured search user to check the `allowed_groups` for the + user to be authenticated. + + By default the ldap connection of the user to be authenticated is used. This can be useful in ldap environments where + the users itself don't have the permission to access the ldap groups. The configured search user needs the + permission to access ldap groups though. + """ + ) + + def resolve_username(self, connection, username_supplied_by_user): + """Resolves the user name and user dn of the user being authenticated. + + Args: + connection: the ldap connection to use + username_supplied_by_user: the username supplied by the user + + Returns: + tuple: value of ldap attribute `lookup_dn_user_dn_attribute` on user entry and the user dn + """ search_filter = self.lookup_dn_search_filter.format( login_attr=self.user_attribute, login=username_supplied_by_user ) @@ -260,13 +270,13 @@ def resolve_username(self, username_supplied_by_user): attributes=self.user_attribute, ) ) - conn.search( + connection.search( search_base=self.user_search_base, search_scope=ldap3.SUBTREE, search_filter=search_filter, attributes=[self.lookup_dn_user_dn_attribute], ) - response = conn.response + response = connection.response if len(response) == 0 or "attributes" not in response[0].keys(): msg = ( "No entry found for user '{username}' " @@ -357,8 +367,21 @@ def authenticate(self, handler, data): ) return None + # connect to ldap with search user + search_dn = self.lookup_dn_search_user + if self.escape_userdn: + search_dn = escape_filter_chars(search_dn) + connection_search = self.get_connection( + userdn=search_dn, password=self.lookup_dn_search_password + ) + if not connection_search.bind(): + msg = "Failed to connect to LDAP server with search user '{search_dn}'" + self.log.warning(msg.format(search_dn=search_dn)) + return None + + # lookup dn of user to be authenticated if self.lookup_dn: - username, resolved_dn = self.resolve_username(username) + username, resolved_dn = self.resolve_username(connection_search, username) if not username: return None if str(self.lookup_dn_user_dn_attribute).upper() == "CN": @@ -367,6 +390,7 @@ def authenticate(self, handler, data): if not bind_dn_template: bind_dn_template = [resolved_dn] + # try to authenticate user is_bound = False for dn in bind_dn_template: if not dn: @@ -379,7 +403,7 @@ def authenticate(self, handler, data): self.log.debug(msg.format(username=username, userdn=userdn)) msg = "Status of user bind {username} with {userdn} : {is_bound}" try: - conn = self.get_connection(userdn, password) + connection_user = self.get_connection(userdn, password) except ldap3.core.exceptions.LDAPBindError as exc: is_bound = False msg += "\n{exc_type}: {exc_msg}".format( @@ -387,7 +411,7 @@ def authenticate(self, handler, data): exc_msg=exc.args[0] if exc.args else "", ) else: - is_bound = True if conn.bound else conn.bind() + is_bound = True if connection_user.bound else connection_user.bind() msg = msg.format(username=username, userdn=userdn, is_bound=is_bound) self.log.debug(msg) if is_bound: @@ -398,17 +422,18 @@ def authenticate(self, handler, data): self.log.warning(msg.format(username=username)) return None + # validate user access by applying the search filter if self.search_filter: search_filter = self.search_filter.format( userattr=self.user_attribute, username=username ) - conn.search( + connection_user.search( search_base=self.user_search_base, search_scope=ldap3.SUBTREE, search_filter=search_filter, attributes=self.attributes, ) - n_users = len(conn.response) + n_users = len(connection_user.response) if n_users == 0: msg = "User with '{userattr}={username}' not found in directory" self.log.warning( @@ -427,6 +452,7 @@ def authenticate(self, handler, data): ) return None + # check if user is member in any of the allowed ldap groups if self.allowed_groups: self.log.debug("username:%s Using dn %s", username, userdn) found = False @@ -440,12 +466,21 @@ def authenticate(self, handler, data): ) group_filter = group_filter.format(userdn=userdn, uid=username) group_attributes = ["member", "uniqueMember", "memberUid"] - found = conn.search( - group, - search_scope=ldap3.BASE, - search_filter=group_filter, - attributes=group_attributes, - ) + # check which ldap connection to use: user (default) or search user + if self.use_search_user_to_check_groups is True: + found = connection_search.search( + group, + search_scope=ldap3.BASE, + search_filter=group_filter, + attributes=group_attributes, + ) + else: + found = connection_user.search( + group, + search_scope=ldap3.BASE, + search_filter=group_filter, + attributes=group_attributes, + ) if found: break if not found: @@ -457,7 +492,7 @@ def authenticate(self, handler, data): if not self.use_lookup_dn_username: username = data["username"] - user_info = self.get_user_attributes(conn, userdn) + user_info = self.get_user_attributes(connection_user, userdn) if user_info: self.log.debug("username:%s attributes:%s", username, user_info) return {"name": username, "auth_state": user_info} diff --git a/ldapauthenticator/tests/test_ldapconnection_usage.py b/ldapauthenticator/tests/test_ldapconnection_usage.py new file mode 100755 index 0000000..ca68cdf --- /dev/null +++ b/ldapauthenticator/tests/test_ldapconnection_usage.py @@ -0,0 +1,126 @@ +"""Tests the usage of both ldap connections of the ldap authenticator. +""" +import os +from unittest.mock import MagicMock, call, ANY +import pytest + +from ..ldapauthenticator import LDAPAuthenticator + + +@pytest.fixture +def authenticator_setup(): + """Provides a configured and mocked authenticator as well as a mocked search and user connection. + + Note: don't be confused with the connection settings here, they just serve as valid configuration and are not used. + """ + # configure authenticator + authenticator = LDAPAuthenticator() + authenticator.server_address = "localhost" + authenticator.lookup_dn = False + authenticator.bind_dn_template = "cn={username},ou=people,dc=planetexpress,dc=com" + authenticator.user_search_base = "ou=people,dc=planetexpress,dc=com" + authenticator.user_attribute = "uid" + authenticator.lookup_dn_user_dn_attribute = "cn" + authenticator.escape_userdn = True + authenticator.attributes = ["uid", "cn", "mail", "ou"] + authenticator.use_lookup_dn_username = False + # leela is being authenticated, she's member of that group + authenticator.allowed_groups = [ + "cn=ship_crew,ou=people,dc=planetexpress,dc=com", + ] + # search user is 'hermes' + authenticator.lookup_dn_search_user = 'hermes' + authenticator.lookup_dn_search_password = 'hermes' + + # mock ldap connections: return either the one for the search user or for the user to be authenticated + connection_search_mock = MagicMock() + connection_user_mock = MagicMock() + def connection_mock(*args, **kwargs): + if 'userdn' in kwargs and kwargs['userdn'] == 'hermes': + return connection_search_mock + else: + return connection_user_mock + authenticator.get_connection = MagicMock( side_effect = connection_mock ) + + # 1) search: bind method should return True + connection_search_mock.bind = MagicMock( return_value = True ) + + # 2) search: lookup dn of user to be authenticated » deactivated + + # 3) user: bound should be False so that bind method is called returning True + connection_user_mock.bound = False + connection_user_mock.bind = MagicMock( return_value = True) + + # 4) user: search filter are empty » deactivated + + # 5) search or user: allowed groups » configured in test methods + + # 6) user: get_user_attributes(connection, userdn) » returns dict with entry attributes + authenticator.get_user_attributes = MagicMock( return_value = { 'uid': 'leela', 'cn': 'Turanga Leela' } ) + + return authenticator, connection_search_mock, connection_user_mock + + +async def test_allowed_groups_check_with_user_connection(authenticator_setup): + """Checks the method calls on both ldap connections when the `allowed_groups` are check with the + connection of the user being authenticated (default). + """ + # unpack + assert object setup + authenticator, connection_search_mock, connection_user_mock = authenticator_setup + assert authenticator is not None and connection_search_mock is not None and connection_user_mock is not None + assert authenticator.lookup_dn is False + assert not authenticator.search_filter + + # assert default value + assert authenticator.use_search_user_to_check_groups is False + + # 5) user: allowed groups » simply return True for the one group + connection_user_mock.search = MagicMock( return_value = True ) + + # authenticate leela + result = await authenticator.authenticate( None, { 'username': 'leela', 'password': 'leela' } ) + assert 'name' in result + assert result['name'] == 'leela' + + # assert method calls on mocks + expected_search_mock_calls = [ + call.bind(), + ] + assert connection_search_mock.mock_calls == expected_search_mock_calls + expected_user_mock_calls = [ + call.bind(), + call.search('cn=ship_crew,ou=people,dc=planetexpress,dc=com', search_scope = ANY, search_filter = ANY, attributes = ANY) + ] + assert connection_user_mock.mock_calls == expected_user_mock_calls + +async def test_allowed_groups_check_with_search_connection(authenticator_setup): + """Checks the method calls on both ldap connections when the `allowed_groups` are check with the + connection of the configured search user. + """ + # unpack + assert object setup + authenticator, connection_search_mock, connection_user_mock = authenticator_setup + assert authenticator is not None and connection_search_mock is not None and connection_user_mock is not None + assert authenticator.lookup_dn is False + assert not authenticator.search_filter + + # enable allowed groups check using the search user connection + authenticator.use_search_user_to_check_groups = True + + # 5) search: allowed groups » simply return True for the one group + connection_search_mock.search = MagicMock( return_value = True ) + + # authenticate leela + result = await authenticator.authenticate( None, { 'username': 'leela', 'password': 'leela' } ) + assert 'name' in result + assert result['name'] == 'leela' + + # assert method calls on mocks + expected_search_mock_calls = [ + call.bind(), + call.search('cn=ship_crew,ou=people,dc=planetexpress,dc=com', search_scope = ANY, search_filter = ANY, attributes = ANY) + ] + assert connection_search_mock.mock_calls == expected_search_mock_calls + expected_user_mock_calls = [ + call.bind(), + ] + assert connection_user_mock.mock_calls == expected_user_mock_calls From 4b89d9090d8679f023d03ae0eb86a0c05ba9ea31 Mon Sep 17 00:00:00 2001 From: tobias haubold Date: Tue, 26 Jan 2021 19:10:52 +0100 Subject: [PATCH 4/5] Revert "[task] add link to info about ldap structure used in tests" This reverts commit 830836c43c979ceaf5a37e616e90e2f213df5f89. --- ldapauthenticator/tests/conftest.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/ldapauthenticator/tests/conftest.py b/ldapauthenticator/tests/conftest.py index 575a2f4..545744f 100644 --- a/ldapauthenticator/tests/conftest.py +++ b/ldapauthenticator/tests/conftest.py @@ -34,7 +34,4 @@ def authenticator(): "cn=ship_crew,ou=people,dc=planetexpress,dc=com", ] - # see link below for ldap structure including users: - # https://github.com/rroemhild/docker-test-openldap - return authenticator From 90b7d5d695f5c112eba70c7087aa9e5867c3a878 Mon Sep 17 00:00:00 2001 From: tobias haubold Date: Tue, 26 Jan 2021 19:11:02 +0100 Subject: [PATCH 5/5] Revert "[task] add guide and config to develop in a docker container with visual studio code" This reverts commit ef53ee3a7098a6ef2b1e264f1757e3ff6399091d. --- .devcontainer.json | 13 ------------- CONTRIBUTING.md | 22 ---------------------- dev.docker-compose.yml | 32 -------------------------------- dev.dockerfile | 6 ------ 4 files changed, 73 deletions(-) delete mode 100644 .devcontainer.json delete mode 100644 dev.docker-compose.yml delete mode 100644 dev.dockerfile diff --git a/.devcontainer.json b/.devcontainer.json deleted file mode 100644 index fdffd1d..0000000 --- a/.devcontainer.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "dockerComposeFile": "dev.docker-compose.yml", - "workspaceFolder": "/workspace", - "service": "jh-ldap-dev", - // install runtime dependencies + install pre-commit hooks - // note: although it is somehow weird to install the software you want to develop - // it installs all runtime dependencies which are maintained in `setup.py` - "postCreateCommand": "pip install --editable . && pre-commit install", - "extensions": [ - "ms-azuretools.vscode-docker", - "ms-python.python" - ] -} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9eee301..01086fc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -50,28 +50,6 @@ To clean up your development LDAP deployment, run: docker rm -f ldap ``` -## Alternative Development Environment Setup - -You can use [*Visual Studio Code*](https://code.visualstudio.com/) with the [*remote containers*](https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.remote-containers) extension to develop in docker container: - -1. install visual studio code and the remote containers extension -1. clone the git repository and open the project in visual studio code -1. open the development container, either do: - - click on the green button in the bottom left and choose `reopen in container` - - press `Shift + Ctrl + P` and start typing `reopen in container` and choose it - - open menu `View` and click on `Command Palette..` and start typing `reopen in container` and choose it -1. open a new terminal (menu `Terminal` » `New Terminal`) and run all tests with: `pytest -v` - - note: all should run successfully - -Troubleshooting (note the early development stage of the remote containers extension): - -- after you run `reopen in container` use the command palette and choose `remote containers: show log` - - sometimes the `postCreateCommand` does not run successfully -- further information - - guide for dev containers: https://code.visualstudio.com/docs/remote/create-dev-container#_use-docker-compose - - `.devcontainer.json` reference: https://code.visualstudio.com/docs/remote/devcontainerjson-reference - - docker compose file reference: https://docs.docker.com/compose/compose-file/ - ## Contributing JupyterHub has adopted automatic code formatting so you shouldn't diff --git a/dev.docker-compose.yml b/dev.docker-compose.yml deleted file mode 100644 index f2c21e8..0000000 --- a/dev.docker-compose.yml +++ /dev/null @@ -1,32 +0,0 @@ -version: '3' -services: - # open ldap server to test authentication gainst - # see: https://github.com/rroemhild/docker-test-openldap - ldap: - image: rroemhild/test-openldap - networks: - ldapnet: - aliases: - - hub-test-ldap - - jh-ldap-dev: - build: - dockerfile: dev.dockerfile - context: . - args: - PYTHON_VERSION: 3.6 - depends_on: - - ldap - # prevent container from exiting before vs code executes commands in it - command: /bin/sh -c "while sleep 1000; do :; done" - # the ldap host for testing - environment: - LDAP_HOST: hub-test-ldap - volumes: - # bind mount the project directory for development inside the container - - .:/workspace:cached - networks: - - ldapnet - -networks: - ldapnet: diff --git a/dev.dockerfile b/dev.dockerfile deleted file mode 100644 index 057c27d..0000000 --- a/dev.dockerfile +++ /dev/null @@ -1,6 +0,0 @@ -ARG PYTHON_VERSION -FROM python:$PYTHON_VERSION - -# install all development dependencies -COPY dev-requirements.txt /home/ -RUN pip install -r /home/dev-requirements.txt