From 276ab02cf5630d6bc81dffe1e93b753a8416d45a Mon Sep 17 00:00:00 2001 From: Josh Humphries Date: Tue, 5 Jan 2021 01:14:39 +0000 Subject: [PATCH 1/9] Remove paster command and implement with click Also update readme to reflect this change. The paster command is completely removed and therefore from this commit the plugin only supports 2.9+ (i.e. where the IClick interface is available). --- README.md | 12 +++--- ckanext/ldap/cli.py | 47 ++++++++++++++++++++++ ckanext/ldap/commands/__init__.py | 14 ------- ckanext/ldap/commands/ldap.py | 66 ------------------------------- ckanext/ldap/plugin.py | 12 ++++-- setup.py | 3 -- 6 files changed, 61 insertions(+), 93 deletions(-) create mode 100644 ckanext/ldap/cli.py delete mode 100644 ckanext/ldap/commands/__init__.py delete mode 100644 ckanext/ldap/commands/ldap.py diff --git a/README.md b/README.md index 4385fc1..0da248e 100644 --- a/README.md +++ b/README.md @@ -75,10 +75,10 @@ These are the options that can be specified in your .ini config file. Name|Description|Options --|--|-- `ckanext.ldap.uri`|The URI of the LDAP server, of the form _ldap://example.com_. You can use the URI to specify TLS (use 'ldaps' protocol), and the port number (suffix ':port').|True/False -`ckanext.ldap.base_dn`|The base dn in which to perform the search. Example: 'ou=USERS,dc=example,dc=com'.| -`ckanext.ldap.search.filter`|This is the search string that is sent to the LDAP server, in which '{login}' is replaced by the user name provided by the user. Example: 'sAMAccountName={login}'. The search performed here **must** return exactly 0 or 1 entry.| -`ckanext.ldap.username`|The LDAP attribute that will be used as the CKAN username. This **must** be unique.| -`ckanext.ldap.email`|The LDAP attribute to map to the user's email address. This **must** be unique.| +`ckanext.ldap.base_dn`|The base dn in which to perform the search. Example: 'ou=USERS,dc=example,dc=com'.| +`ckanext.ldap.search.filter`|This is the search string that is sent to the LDAP server, in which '{login}' is replaced by the user name provided by the user. Example: 'sAMAccountName={login}'. The search performed here **must** return exactly 0 or 1 entry.| +`ckanext.ldap.username`|The LDAP attribute that will be used as the CKAN username. This **must** be unique.| +`ckanext.ldap.email`|The LDAP attribute to map to the user's email address. This **must** be unique.| ## Other options @@ -107,9 +107,9 @@ Name|Description|Options|Default ### `ldap` -1. `setup-org`: create the organisation specified in `ckanext.ldap.organization.id`. +1. `setup_org`: create the organisation specified in `ckanext.ldap.organization.id`. ```bash - paster --plugin=ckanext-ldap ldap setup-org -c $CONFIG_FILE + ckan -c $CONFIG_FILE ldap setup_org ``` ## Templates diff --git a/ckanext/ldap/cli.py b/ckanext/ldap/cli.py new file mode 100644 index 0000000..97ec5fc --- /dev/null +++ b/ckanext/ldap/cli.py @@ -0,0 +1,47 @@ +import click + +from ckan.plugins import toolkit + + +def get_commands(): + return [ldap] + + +@click.group() +def ldap(): + ''' + The LDAP CLI. + ''' + pass + + +@ldap.command() +def setup_org(): + ''' + Sets up the default organisation which all ldap users will be automatically made members of. + ''' + # get the organisation all users will be added to + organization_id = toolkit.config[u'ckanext.ldap.organization.id'] + + # set up context + user = toolkit.get_action(u'get_site_user')({ + u'ignore_auth': True + }, {}) + context = { + u'user': user[u'name'] + } + + try: + toolkit.get_action(u'organization_show')(context, { + u'id': organization_id + }) + click.secho(u"Organisation already exists, doing nothing", fg=u"green") + except toolkit.ObjectNotFound: + # see the following commit to understand why this line is here + # http://github.com/ckan/ckanext-harvest/commit/f315f41c86cbde4a49ef869b6993598f8cb11e2d + context.pop(u'__auth_audit', None) + toolkit.get_action(u'organization_create')(context, { + u'id': organization_id, + u'name': organization_id + }) + click.secho(u"New organisation created", fg=u"green") diff --git a/ckanext/ldap/commands/__init__.py b/ckanext/ldap/commands/__init__.py deleted file mode 100644 index 2779051..0000000 --- a/ckanext/ldap/commands/__init__.py +++ /dev/null @@ -1,14 +0,0 @@ -# !/usr/bin/env python -# encoding: utf-8 -# -# This file is part of ckanext-ldap -# Created by the Natural History Museum in London, UK - - -def main(): - ''' ''' - pass - - -if __name__ == u'__main__': - main() diff --git a/ckanext/ldap/commands/ldap.py b/ckanext/ldap/commands/ldap.py deleted file mode 100644 index 8e114d9..0000000 --- a/ckanext/ldap/commands/ldap.py +++ /dev/null @@ -1,66 +0,0 @@ -#!/usr/bin/env python -# encoding: utf-8 -# -# This file is part of ckanext-ldap -# Created by the Natural History Museum in London, UK - - -import logging - -from ckan.plugins import toolkit - -log = logging.getLogger() - - -class LDAPCommand(toolkit.CkanCommand): - ''' - Paster function to set up the default organisation - - Paster function can be included to provision scripts - otherwise, get an error after - provisioning new CKAN instance - - Commands: - - paster ldap setup-org -c /etc/ckan/default/development.ini - - ''' - summary = __doc__.split(u'\n')[0] - usage = __doc__ - - def command(self): - if not self.args or self.args[0] in [u'--help', u'-h', u'help']: - print self.__doc__ - return - - self._load_config() - - cmd = self.args[0] - if cmd == u'setup-org': - self.setup_org() - else: - print u'Command %s not recognized' % cmd - - def setup_org(self): - # get the organisation all users will be added to - organization_id = toolkit.config[u'ckanext.ldap.organization.id'] - - # set up context - user = toolkit.get_action(u'get_site_user')({ - u'ignore_auth': True - }, {}) - context = { - u'user': user[u'name'] - } - - try: - toolkit.get_action(u'organization_show')(context, { - u'id': organization_id - }) - except toolkit.ObjectNotFound: - # see the following commit to understand why this line is here - # http://github.com/ckan/ckanext-harvest/commit/f315f41c86cbde4a49ef869b6993598f8cb11e2d - context.pop(u'__auth_audit', None) - toolkit.get_action(u'organization_create')(context, { - u'id': organization_id, - u'name': organization_id - }) diff --git a/ckanext/ldap/plugin.py b/ckanext/ldap/plugin.py index aa9b870..20762c5 100644 --- a/ckanext/ldap/plugin.py +++ b/ckanext/ldap/plugin.py @@ -6,7 +6,7 @@ import logging -from ckanext.ldap import routes +from ckanext.ldap import routes, cli from ckanext.ldap.lib.helpers import get_login_action, is_ldap_user from ckanext.ldap.logic.auth.create import user_create from ckanext.ldap.logic.auth.update import user_update @@ -24,12 +24,11 @@ class ConfigError(Exception): class LdapPlugin(SingletonPlugin): - '''"LdapPlugin + ''' + LdapPlugin This plugin provides Ldap authentication by implementing the IAuthenticator interface. - - ''' implements(interfaces.IAuthenticator) implements(interfaces.IConfigurable) @@ -37,6 +36,11 @@ class LdapPlugin(SingletonPlugin): implements(interfaces.IBlueprint, inherit=True) implements(interfaces.IAuthFunctions) implements(interfaces.ITemplateHelpers, inherit=True) + implements(interfaces.IClick) + + ## IClick + def get_commands(self): + return cli.get_commands() def update_config(self, config): '''Implement IConfiguer.update_config diff --git a/setup.py b/setup.py index 12eb143..73567b6 100755 --- a/setup.py +++ b/setup.py @@ -37,8 +37,5 @@ u''' [ckan.plugins] ldap=ckanext.ldap.plugin:LdapPlugin - - [paste.paster_command] - ldap=ckanext.ldap.commands.ldap:LDAPCommand ''', ) From f4143d0a6466b2ecacba944d815fe056c5913405 Mon Sep 17 00:00:00 2001 From: Josh Humphries Date: Tue, 5 Jan 2021 11:51:45 +0000 Subject: [PATCH 2/9] Change name of command to setup-org --- README.md | 4 ++-- ckanext/ldap/cli.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0da248e..2713d9e 100644 --- a/README.md +++ b/README.md @@ -107,9 +107,9 @@ Name|Description|Options|Default ### `ldap` -1. `setup_org`: create the organisation specified in `ckanext.ldap.organization.id`. +1. `setup-org`: create the organisation specified in `ckanext.ldap.organization.id`. ```bash - ckan -c $CONFIG_FILE ldap setup_org + ckan -c $CONFIG_FILE ldap setup-org ``` ## Templates diff --git a/ckanext/ldap/cli.py b/ckanext/ldap/cli.py index 97ec5fc..221bcdb 100644 --- a/ckanext/ldap/cli.py +++ b/ckanext/ldap/cli.py @@ -15,7 +15,7 @@ def ldap(): pass -@ldap.command() +@ldap.command(name=u'setup-org') def setup_org(): ''' Sets up the default organisation which all ldap users will be automatically made members of. From 900985b2d357fe090c63bf8609f310c8dec05342 Mon Sep 17 00:00:00 2001 From: Josh Humphries Date: Mon, 25 Jan 2021 20:46:08 +0000 Subject: [PATCH 3/9] Switch tests over to docker-compose and bump version --- .coveragerc | 2 + .gitignore | 4 +- .travis.yml | 20 +++---- CHANGELOG.md | 4 ++ README.md | 22 ++++++-- ckanext/ldap/lib/helpers.py | 28 +++++---- ckanext/ldap/lib/search.py | 13 ++--- ckanext/ldap/logic/auth/create.py | 7 +-- ckanext/ldap/logic/auth/update.py | 19 +++---- ckanext/ldap/model/ldap_user.py | 5 +- ckanext/ldap/plugin.py | 94 +++++++++++-------------------- ckanext/ldap/routes/_helpers.py | 73 ++++++++++++------------ ckanext/ldap/routes/login.py | 10 ++-- ckanext/ldap/tests/bin/build.sh | 36 ------------ ckanext/ldap/tests/bin/test.ini | 41 -------------- dev_requirements.txt | 7 +-- docker-compose.yml | 39 +++++++++++++ docker/Dockerfile | 27 +++++++++ docker/entrypoint.sh | 24 ++++++++ setup.py | 17 +++--- test.ini | 52 +++++++++++++++++ tests/__init__.py | 0 tests/test_helpers.py | 11 ++++ 23 files changed, 305 insertions(+), 250 deletions(-) create mode 100644 .coveragerc delete mode 100644 ckanext/ldap/tests/bin/build.sh delete mode 100644 ckanext/ldap/tests/bin/test.ini create mode 100644 docker-compose.yml create mode 100644 docker/Dockerfile create mode 100644 docker/entrypoint.sh create mode 100644 test.ini create mode 100644 tests/__init__.py create mode 100644 tests/test_helpers.py diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 0000000..4edd7b1 --- /dev/null +++ b/.coveragerc @@ -0,0 +1,2 @@ +[run] +relative_files = True diff --git a/.gitignore b/.gitignore index 75ff375..1efc416 100644 --- a/.gitignore +++ b/.gitignore @@ -2,4 +2,6 @@ *.pyc .noseids -\.coverage + +.coverage +.idea diff --git a/.travis.yml b/.travis.yml index 82d57f5..325f714 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,19 +1,13 @@ -dist: trusty -language: python - -python: - - "2.7" +sudo: required -install: - - sh ckanext/ldap/tests/bin/build.sh +language: python services: - - redis-server - - postgresql - -addons: - postgresql: "9.4" + - docker -script: coverage run --source=ckanext.ldap setup.py nosetests --ckan --with-pylons=ckanext/ldap/tests/bin/test.ini --nologcapture --debug=ckantest,ckanext.ldap --rednose +script: + - pip install coveralls + - docker-compose build + - docker-compose run ckan after_success: coveralls diff --git a/CHANGELOG.md b/CHANGELOG.md index 494ace4..39fdfa0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ (This file may not be historically complete, as it is a recent addition to the project). +## [2.1.0] - 2021-01-25 + +- Updated to work with CKAN 2.9.1. +- Switched to docker based testing. ## [2.0.0-alpha] - 2019-07-23 diff --git a/README.md b/README.md index 2713d9e..676b5d9 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ [![Travis](https://img.shields.io/travis/NaturalHistoryMuseum/ckanext-ldap/master.svg?style=flat-square)](https://travis-ci.org/NaturalHistoryMuseum/ckanext-ldap) [![Coveralls](https://img.shields.io/coveralls/github/NaturalHistoryMuseum/ckanext-ldap/master.svg?style=flat-square)](https://coveralls.io/github/NaturalHistoryMuseum/ckanext-ldap) -[![CKAN](https://img.shields.io/badge/ckan-2.9.0a-orange.svg?style=flat-square)](https://github.com/ckan/ckan) +[![CKAN](https://img.shields.io/badge/ckan-2.9.1-orange.svg?style=flat-square)](https://github.com/ckan/ckan) _A CKAN extension that provides LDAP authentication._ @@ -127,10 +127,24 @@ The helper function `h.is_ldap_user()` is also provided for templates. # Testing - _Test coverage is currently extremely limited._ -To run the tests, use nosetests inside your virtualenv. The `--nocapture` flag will allow you to see the debug statements. +To run the tests in this extension, there is a Docker compose configuration available in this +repository to make it easy. + +To run the tests against ckan 2.9.x on Python2: + +1. Build the required images ```bash -nosetests --ckan --with-pylons=$TEST_CONFIG_FILE --where=$INSTALL_FOLDER/src/ckanext-ldap --nologcapture --nocapture +docker-compose build ``` + +2. Then run the tests. + The root of the repository is mounted into the ckan container as a volume by the Docker compose + configuration, so you should only need to rebuild the ckan image if you change the extension's + dependencies. +```bash +docker-compose run ckan +``` + +The ckan image uses the Dockerfile in the `docker/` folder which is based on `openknowledge/ckan-dev:2.9-py2`. diff --git a/ckanext/ldap/lib/helpers.py b/ckanext/ldap/lib/helpers.py index 5edfb67..024f69b 100644 --- a/ckanext/ldap/lib/helpers.py +++ b/ckanext/ldap/lib/helpers.py @@ -3,38 +3,36 @@ # # This file is part of ckanext-ldap # Created by the Natural History Museum in London, UK - +import six from ckan.common import session from ckan.plugins import toolkit -try: - # In case we are running Python3 +if six.PY3: from urllib.parse import urlparse, parse_qs -except ImportError: +else: from urlparse import urlparse, parse_qs def is_ldap_user(): - '''Helper function for determining if current user is LDAP user + ''' + Helper function for determining if current user is LDAP user. :returns: boolean - ''' - return u'ckanext-ldap-user' in session def get_login_action(): - ''' Returns ldap login handler. Preserves parameter `came_from` - as stored in context object's login_handler. - ''' - if hasattr(toolkit.c, 'login_handler'): - camefrom = parse_qs(urlparse(toolkit.c.login_handler).query).get(u'came_from') + Returns ldap login handler. Preserves parameter `came_from` as stored in context object's + login_handler. + ''' + if hasattr(toolkit.c, u'login_handler'): + came_from = parse_qs(urlparse(toolkit.c.login_handler).query).get(u'came_from') else: - camefrom = None - if camefrom: - action = toolkit.url_for(u'ldap.login_handler', came_from=str(camefrom[0])) + came_from = None + if came_from: + action = toolkit.url_for(u'ldap.login_handler', came_from=str(came_from[0])) else: action = toolkit.url_for(u'ldap.login_handler') return action diff --git a/ckanext/ldap/lib/search.py b/ckanext/ldap/lib/search.py index fa65366..9a1fa9c 100644 --- a/ckanext/ldap/lib/search.py +++ b/ckanext/ldap/lib/search.py @@ -7,21 +7,20 @@ import logging import ldap +from ckan.plugins import toolkit from ckanext.ldap.lib import helpers from ckanext.ldap.lib.exceptions import MultipleMatchError -from ckan.plugins import toolkit - log = logging.getLogger(u'ckanext.ldap') def find_ldap_user(login): - '''Find the LDAP user identified by 'login' in the configured ldap database + ''' + Find the LDAP user identified by 'login' in the configured ldap database. :param login: The login to find in the LDAP database - :returns: None if no user is found, a dictionary defining 'cn', 'username', - 'fullname' and 'email otherwise. - + :returns: None if no user is found, a dictionary defining 'cn', 'username', 'fullname' and + 'email otherwise. ''' cnx = ldap.initialize(toolkit.config[u'ckanext.ldap.uri'], bytes_mode=False, trace_level=toolkit.config[u'ckanext.ldap.trace_level']) @@ -123,7 +122,7 @@ def ldap_search(cnx, filter_str, attributes, non_unique=u'raise'): attr = res[0][1] ret = { u'cn': cn, - } + } # Check required fields for i in [u'username', u'email']: diff --git a/ckanext/ldap/logic/auth/create.py b/ckanext/ldap/logic/auth/create.py index 2a88403..6178f5f 100644 --- a/ckanext/ldap/logic/auth/create.py +++ b/ckanext/ldap/logic/auth/create.py @@ -4,20 +4,17 @@ # This file is part of ckanext-ldap # Created by the Natural History Museum in London, UK -from ckanext.ldap.lib.search import find_ldap_user - from ckan.plugins import toolkit +from ckanext.ldap.lib.search import find_ldap_user @toolkit.chained_auth_function @toolkit.auth_allow_anonymous_access def user_create(next_auth, context, data_dict=None): ''' - :param next_auth: the next auth function in the chain :param context: :param data_dict: (Default value = None) - ''' if data_dict and u'name' in data_dict: ldap_user_dict = find_ldap_user(data_dict[u'name']) @@ -25,6 +22,6 @@ def user_create(next_auth, context, data_dict=None): return { u'success': False, u'msg': toolkit._(u'An LDAP user by that name already exists') - } + } return next_auth(context, data_dict) diff --git a/ckanext/ldap/logic/auth/update.py b/ckanext/ldap/logic/auth/update.py index 5fbc013..e7da122 100644 --- a/ckanext/ldap/logic/auth/update.py +++ b/ckanext/ldap/logic/auth/update.py @@ -4,22 +4,21 @@ # This file is part of ckanext-ldap # Created by the Natural History Museum in London, UK -from ckanext.ldap.model.ldap_user import LdapUser -from ckanext.ldap.lib.search import find_ldap_user - from ckan.logic import auth from ckan.plugins import toolkit +from ckanext.ldap.lib.search import find_ldap_user +from ckanext.ldap.model.ldap_user import LdapUser @toolkit.chained_auth_function @toolkit.auth_allow_anonymous_access def user_update(next_auth, context, data_dict): - '''Ensure LDAP users cannot be edited, and name clash with ldap users + ''' + Ensure LDAP users cannot be edited, and name clash with ldap users. :param next_auth: the next auth function in the chain :param context: :param data_dict: - ''' user_obj = None try: @@ -28,20 +27,20 @@ def user_update(next_auth, context, data_dict): pass # Prevent edition of LDAP users (if so configured) if toolkit.config[u'ckanext.ldap.prevent_edits'] and user_obj and LdapUser.by_user_id( - user_obj.id): + user_obj.id): return { u'success': False, u'msg': toolkit._(u'Cannot edit LDAP users') - } + } # Prevent name clashes! if u'name' in data_dict and user_obj and user_obj.name != data_dict[u'name']: ldap_user_dict = find_ldap_user(data_dict[u'name']) if ldap_user_dict: - if len(user_obj.ldap_user) == 0 or user_obj.ldap_user[0].ldap_id != \ - ldap_user_dict[u'ldap_id']: + if (len(user_obj.ldap_user) == 0 or + user_obj.ldap_user[0].ldap_id != ldap_user_dict[u'ldap_id']): return { u'success': False, u'msg': toolkit._(u'An LDAP user by that name already exists') - } + } return next_auth(context, data_dict) diff --git a/ckanext/ldap/model/ldap_user.py b/ckanext/ldap/model/ldap_user.py index e3e2c93..e2c1f77 100644 --- a/ckanext/ldap/model/ldap_user.py +++ b/ckanext/ldap/model/ldap_user.py @@ -6,9 +6,8 @@ import datetime -from sqlalchemy import Column, ForeignKey, Table, orm, types - from ckan import model +from sqlalchemy import Column, ForeignKey, Table, orm, types __all__ = [u'LdapUser'] @@ -63,4 +62,4 @@ def by_user_id(cls, user_id, autoflush=True): u'user': orm.relation(model.user.User, backref=orm.backref(u'ldap_user', cascade=u'all, delete, delete-orphan')) - }, ) +}, ) diff --git a/ckanext/ldap/plugin.py b/ckanext/ldap/plugin.py index 20762c5..f13536e 100644 --- a/ckanext/ldap/plugin.py +++ b/ckanext/ldap/plugin.py @@ -6,15 +6,14 @@ import logging +from ckan.common import session +from ckan.plugins import SingletonPlugin, implements, interfaces, toolkit from ckanext.ldap import routes, cli from ckanext.ldap.lib.helpers import get_login_action, is_ldap_user from ckanext.ldap.logic.auth.create import user_create from ckanext.ldap.logic.auth.update import user_update from ckanext.ldap.model.ldap_user import setup as model_setup -from ckan.common import session -from ckan.plugins import SingletonPlugin, implements, interfaces, toolkit - log = logging.getLogger(__name__) @@ -61,13 +60,13 @@ def get_auth_functions(self): return { u'user_update': user_update, u'user_create': user_create - } + } def configure(self, config): - '''Implementation of IConfigurable.configure + ''' + Implementation of IConfigurable.configure :param config: - ''' # Setup our models model_setup() @@ -76,62 +75,62 @@ def configure(self, config): schema = { u'ckanext.ldap.uri': { u'required': True - }, + }, u'ckanext.ldap.base_dn': { u'required': True - }, + }, u'ckanext.ldap.search.filter': { u'required': True - }, + }, u'ckanext.ldap.username': { u'required': True - }, + }, u'ckanext.ldap.email': { u'required': True - }, + }, u'ckanext.ldap.auth.dn': {}, u'ckanext.ldap.auth.password': { u'required_if': u'ckanext.ldap.auth.dn' - }, + }, u'ckanext.ldap.auth.method': { u'default': u'SIMPLE', u'validate': _allowed_auth_methods - }, + }, u'ckanext.ldap.auth.mechanism': { u'default': u'DIGEST-MD5', u'validate': _allowed_auth_mechanisms - }, + }, u'ckanext.ldap.search.alt': {}, u'ckanext.ldap.search.alt_msg': { u'required_if': u'ckanext.ldap.search.alt' - }, + }, u'ckanext.ldap.fullname': {}, u'ckanext.ldap.organization.id': {}, u'ckanext.ldap.organization.role': { u'default': u'member', u'validate': _allowed_roles - }, + }, u'ckanext.ldap.ckan_fallback': { u'default': False, u'parse': toolkit.asbool - }, + }, u'ckanext.ldap.prevent_edits': { u'default': False, u'parse': toolkit.asbool - }, + }, u'ckanext.ldap.migrate': { u'default': False, u'parse': toolkit.asbool - }, + }, u'ckanext.ldap.debug_level': { u'default': 0, u'parse': toolkit.asint - }, + }, u'ckanext.ldap.trace_level': { u'default': 0, u'parse': toolkit.asint - }, - } + }, + } errors = [] for key, options in schema.items(): config_value = config.get(key, None) @@ -162,22 +161,18 @@ def configure(self, config): if len(errors): raise ConfigError(u'\n'.join(errors)) + # IAuthenticator def login(self): - '''Implementation of IAuthenticator.login - + ''' We don't need to do anything here as we override the form & implement our own controller - action - - + action. ''' pass + # IAuthenticator def identify(self): - '''Implementiation of IAuthenticator.identify - + ''' Identify which user (if any) is logged in via this plugin - - ''' # FIXME: This breaks if the current user changes their own user name. user = session.get(u'ckanext-ldap-user') @@ -187,61 +182,40 @@ def identify(self): # add the 'user' attribute to the context to avoid issue #4247 toolkit.c.user = None + # IAuthenticator def logout(self): - '''Implementation of IAuthenticator.logout''' self._delete_session_items() + # IAuthenticator def abort(self, status_code, detail, headers, comment): - '''Implementation of IAuthenticator.abort - - :param status_code: - :param detail: - :param headers: - :param comment: - - ''' return status_code, detail, headers, comment def _delete_session_items(self): - '''Delete user details stored in the session by this plugin''' + ''' + Delete user details stored in the session by this plugin. + ''' if u'ckanext-ldap-user' in session: del session[u'ckanext-ldap-user'] session.save() def get_helpers(self): - ''' ''' return { u'is_ldap_user': is_ldap_user, u'get_login_action': get_login_action - } + } def _allowed_roles(v): - ''' - - :param v: - - ''' if v not in [u'member', u'editor', u'admin']: raise ConfigError(u'role must be one of "member", "editor" or "admin"') def _allowed_auth_methods(v): - ''' - - :param v: - - ''' if v.upper() not in [u'SIMPLE', u'SASL']: raise ConfigError(u'Only SIMPLE and SASL authentication methods are supported') def _allowed_auth_mechanisms(v): - ''' - - :param v: - - ''' - if v.upper() not in [ - u'DIGEST-MD5', ]: # Only DIGEST-MD5 is supported when the auth method is SASL + # Only DIGEST-MD5 is supported when the auth method is SASL + if v.upper() != u'DIGEST-MD5': raise ConfigError(u'Only DIGEST-MD5 is supported as an authentication mechanism') diff --git a/ckanext/ldap/routes/_helpers.py b/ckanext/ldap/routes/_helpers.py index dda4a83..2c00092 100644 --- a/ckanext/ldap/routes/_helpers.py +++ b/ckanext/ldap/routes/_helpers.py @@ -5,29 +5,26 @@ # Created by the Natural History Museum in London, UK import logging +import re import uuid import ldap import ldap.filter -import re -from ckanext.ldap.lib.exceptions import UserConflictError -from ckanext.ldap.model.ldap_user import LdapUser - from ckan.common import session from ckan.model import Session from ckan.plugins import toolkit +from ckanext.ldap.lib.exceptions import UserConflictError +from ckanext.ldap.model.ldap_user import LdapUser log = logging.getLogger(__name__) def login_failed(notice=None, error=None): - '''Handle login failures - - Redirect to /user/login and flash an optional message + ''' + Handle login failures. Redirect to /user/login and flash an optional message. :param notice: Optional notice for the user (Default value = None) :param error: Optional error message for the user (Default value = None) - ''' if notice: toolkit.h.flash_notice(notice) @@ -37,9 +34,8 @@ def login_failed(notice=None, error=None): def login_success(user_name, came_from): - '''Handle login success - - Saves the user in the session and redirects to user/logged_in + ''' + Handle login success. Saves the user in the session and redirects to user/logged_in. :param user_name: The user name ''' @@ -49,25 +45,26 @@ def login_success(user_name, came_from): def get_user_dict(user_id): - """Calls the action API to get the detail for a user given their id + ''' + Calls the action API to get the detail for a user given their id. @param user_id: The user id - """ + ''' context = { u'ignore_auth': True - } + } data_dict = { u'id': user_id - } + } return toolkit.get_action(u'user_show')(context, data_dict) def ckan_user_exists(user_name): - '''Check if a CKAN user name exists, and if that user is an LDAP user. + ''' + Check if a CKAN user name exists, and if that user is an LDAP user. :param user_name: User name to check - :returns: Dictionary defining 'exists' and 'ldap'. - + :return: Dictionary defining 'exists' and 'ldap'. ''' try: user = get_user_dict(user_name) @@ -75,27 +72,27 @@ def ckan_user_exists(user_name): return { u'exists': False, u'is_ldap': False - } + } ldap_user = LdapUser.by_user_id(user[u'id']) if ldap_user: return { u'exists': True, u'is_ldap': True - } + } else: return { u'exists': True, u'is_ldap': False - } + } def get_unique_user_name(base_name): - '''Create a unique, valid, non existent user name from the given base name + ''' + Create a unique, valid, non existent user name from the given base name. :param base_name: Base name - :returns: A valid user name not currently in use based on base_name - + :return: A valid user name not currently in use based on base_name ''' base_name = re.sub(u'[^-a-z0-9_]', u'_', base_name.lower()) base_name = base_name[0:100] @@ -111,11 +108,11 @@ def get_unique_user_name(base_name): def get_or_create_ldap_user(ldap_user_dict): - '''Get or create a CKAN user from the data returned by the LDAP server + ''' + Get or create a CKAN user from the data returned by the LDAP server. :param ldap_user_dict: Dictionary as returned by _find_ldap_user - :returns: The CKAN username of an existing user - + :return: The CKAN username of an existing user ''' # Look for existing user, and if found return it. ldap_user = LdapUser.by_ldap_id(ldap_user_dict[u'username']) @@ -150,7 +147,7 @@ def get_or_create_ldap_user(ldap_user_dict): u'name': user_name, u'email': ldap_user_dict[u'email'], u'password': str(uuid.uuid4()) - }) + }) if u'fullname' in ldap_user_dict: user_dict[u'fullname'] = ldap_user_dict[u'fullname'] if u'about' in ldap_user_dict: @@ -159,16 +156,16 @@ def get_or_create_ldap_user(ldap_user_dict): ckan_user = toolkit.get_action(u'user_update')( context={ u'ignore_auth': True - }, + }, data_dict=user_dict - ) + ) else: ckan_user = toolkit.get_action(u'user_create')( context={ u'ignore_auth': True - }, + }, data_dict=user_dict - ) + ) ldap_user = LdapUser(user_id=ckan_user[u'id'], ldap_id=ldap_user_dict[u'username']) Session.add(ldap_user) Session.commit() @@ -177,24 +174,24 @@ def get_or_create_ldap_user(ldap_user_dict): toolkit.get_action(u'member_create')( context={ u'ignore_auth': True - }, + }, data_dict={ u'id': toolkit.config[u'ckanext.ldap.organization.id'], u'object': user_name, u'object_type': u'user', u'capacity': toolkit.config[u'ckanext.ldap.organization.role'] - } - ) + } + ) return user_name def check_ldap_password(cn, password): - '''Checks that the given cn/password credentials work on the given CN. + ''' + Checks that the given cn/password credentials work on the given CN. :param cn: Common name to log on :param password: Password for cn - :returns: True on success, False on failure - + :return: True on success, False on failure ''' cnx = ldap.initialize(toolkit.config[u'ckanext.ldap.uri'], bytes_mode=False, trace_level=toolkit.config[u'ckanext.ldap.trace_level']) diff --git a/ckanext/ldap/routes/login.py b/ckanext/ldap/routes/login.py index 5b798d1..3dc6ecf 100644 --- a/ckanext/ldap/routes/login.py +++ b/ckanext/ldap/routes/login.py @@ -5,13 +5,13 @@ # Created by the Natural History Museum in London, UK import ldap -from flask import Blueprint - from ckan.model import User from ckan.plugins import toolkit -from . import _helpers from ckanext.ldap.lib.exceptions import MultipleMatchError, UserConflictError from ckanext.ldap.lib.search import find_ldap_user +from flask import Blueprint + +from . import _helpers blueprint = Blueprint(name=u'ldap', import_name=__name__) @@ -23,7 +23,9 @@ def initialise(): @blueprint.route('/ldap_login_handler', methods=['POST']) def login_handler(): - '''Action called when login in via the LDAP login form''' + ''' + Action called when login in via the LDAP login form. + ''' params = toolkit.request.values came_from = params.get(u'came_from', None) if u'login' in params and u'password' in params: diff --git a/ckanext/ldap/tests/bin/build.sh b/ckanext/ldap/tests/bin/build.sh deleted file mode 100644 index 3ff4be1..0000000 --- a/ckanext/ldap/tests/bin/build.sh +++ /dev/null @@ -1,36 +0,0 @@ -#!/bin/bash - -HERE=$(pwd) - -sudo git clone --branch=2.8 https://github.com/ckan/ckan /ckan -sudo chmod -R a+rw /ckan - -wget http://archive.apache.org/dist/lucene/solr/5.5.4/solr-5.5.4.tgz -O /tmp/solr.tgz -sudo mkdir /solr -sudo tar xzf /tmp/solr.tgz -C /solr -sudo /solr/solr-5.5.4/bin/solr start -sudo /solr/solr-5.5.4/bin/solr create_core -c ckan -sudo cp /ckan/ckan/config/solr/schema.xml /solr/solr-5.5.4/server/solr/ckan/conf/ -sudo wget -O /solr/solr-5.5.4/server/solr/ckan/conf/solrconfig.xml $SOLR_CONFIG_URL -sudo /solr/solr-5.5.4/bin/solr restart - -cd /ckan -pip install --upgrade pip -pip install -r requirement-setuptools.txt -pip install -r requirements.txt -pip install -r dev-requirements.txt -python setup.py develop - -sudo -u postgres psql -c "CREATE USER ckan_default WITH PASSWORD 'pass';" -sudo -u postgres psql -c 'CREATE DATABASE ckan_test WITH OWNER ckan_default;' - -sudo -u postgres psql -c "CREATE USER datastore_default WITH PASSWORD 'pass';" -sudo -u postgres psql -c 'CREATE DATABASE datastore_test WITH OWNER ckan_default;' - -paster db init -c /ckan/test-core.ini -paster datastore set-permissions -c test-core.ini | sudo -u postgres psql - -cd $HERE -pip install -r requirements.txt -pip install -r dev_requirements.txt -pip install -e . diff --git a/ckanext/ldap/tests/bin/test.ini b/ckanext/ldap/tests/bin/test.ini deleted file mode 100644 index 9441f38..0000000 --- a/ckanext/ldap/tests/bin/test.ini +++ /dev/null @@ -1,41 +0,0 @@ -[server:main] -use = egg:Paste#http -host = 0.0.0.0 -port = 5000 - -[app:main] -use = config:/ckan/test-core.ini - -[loggers] -keys = root, ckan, ckanext - -[handlers] -keys = console - -[formatters] -keys = generic - -[logger_root] -level = WARNING -handlers = console - -[logger_ckan] -level = INFO -handlers = console -qualname = ckan -propagate = 0 - -[logger_ckanext] -level = DEBUG -handlers = console -qualname = ckanext -propagate = 0 - -[handler_console] -class = StreamHandler -args = (sys.stderr,) -level = NOTSET -formatter = generic - -[formatter_generic] -format = %(asctime)s %(levelname)-5.5s [%(name)s] %(message)s \ No newline at end of file diff --git a/dev_requirements.txt b/dev_requirements.txt index 94cc5f0..0a53fed 100644 --- a/dev_requirements.txt +++ b/dev_requirements.txt @@ -1,6 +1,3 @@ -nose -rednose mock -httpretty -coveralls --e git+https://github.com/NaturalHistoryMuseum/ckantest.git#egg=ckantest \ No newline at end of file +pytest>=4.6.5 +pytest-cov>=2.7.1 diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 0000000..b1271f0 --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,39 @@ +version: "3" + +services: + ckan: + build: + context: . + dockerfile: docker/Dockerfile + environment: + PYTHONUNBUFFERED: 1 + PYTHONDONTWRITEBYTECODE: 1 + depends_on: + - db + - solr + - redis + volumes: + - .:/srv/app/src/ckanext-ldap + + solr: + build: + context: https://github.com/okfn/docker-ckan.git#:solr + logging: + driver: none + + db: + build: + context: https://github.com/okfn/docker-ckan.git#:postgresql + args: + - DATASTORE_READONLY_PASSWORD=password + - POSTGRES_PASSWORD=password + environment: + - DATASTORE_READONLY_PASSWORD=password + - POSTGRES_PASSWORD=password + logging: + driver: none + + redis: + image: redis:latest + logging: + driver: none diff --git a/docker/Dockerfile b/docker/Dockerfile new file mode 100644 index 0000000..bef5cf3 --- /dev/null +++ b/docker/Dockerfile @@ -0,0 +1,27 @@ +FROM openknowledge/ckan-dev:2.9-py2 + +RUN apk add openldap-dev + +# ckan is installed in /srv/app/src/ckan in the ckan-dev image we're basing this image on +WORKDIR /srv/app/src/ckanext-ldap + +# copy over the ckanext-ldap source +COPY . . + +# might as well update pip while we're here! +RUN pip2 install --upgrade pip + +# fixes this https://github.com/ckan/ckan/issues/5570 +RUN pip2 install pytest-ckan + +# install the dependencies +RUN python2 setup.py develop && \ + pip2 install -r requirements.txt && \ + pip2 install -r dev_requirements.txt + +# this entrypoint ensures our service dependencies (postgresql, solr and redis) are running before +# running the cmd +ENTRYPOINT ["/bin/bash", "docker/entrypoint.sh"] + +# run the tests with coverage output +CMD ["pytest", "--cov=ckanext.ldap", "--ckan-ini=test.ini", "tests"] diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh new file mode 100644 index 0000000..5f18abf --- /dev/null +++ b/docker/entrypoint.sh @@ -0,0 +1,24 @@ +#!/bin/bash +set -e + +echo "Wait for PostgreSQL to start..." +while ! pg_isready -h db -U ckan; do + sleep 1; +done +echo "PostgreSQL started" + +echo "Wait for Solr to start..." +while ! curl -s "http://solr:8983/solr/ckan/admin/ping" | grep -q OK; do + sleep 1; +done +echo "Solr started" + +echo "Wait for Redis to start..." +while ! echo -e "PING" | nc -w 1 redis 6379 | grep -q "+PONG"; do + sleep 1; +done +echo "Redis started" + +echo "All services up, running command" + +exec "$@" diff --git a/setup.py b/setup.py index 73567b6..b9ce754 100755 --- a/setup.py +++ b/setup.py @@ -6,7 +6,7 @@ from setuptools import find_packages, setup -__version__ = u'2.0.0-alpha' +__version__ = u'2.1.0' with open(u'README.md', u'r') as f: __long_description__ = f.read() @@ -17,8 +17,9 @@ description=u'A CKAN extension that provides LDAP authentication.', long_description=__long_description__, classifiers=[ - u'Development Status :: 3 - Alpha', - u'Framework :: Flask', + u'Development Status :: 5 - Production/Stable', + u'License :: OSI Approved :: GNU General Public License v3 (GPLv3)', + u'Programming Language :: Python', u'Programming Language :: Python :: 2.7' ], keywords=u'CKAN data ldap', @@ -31,11 +32,11 @@ include_package_data=True, zip_safe=False, install_requires=[ - 'python-ldap==3.0.0', - ], - entry_points= \ - u''' + u'python-ldap==3.0.0', + u'six', + ], + entry_points=u''' [ckan.plugins] ldap=ckanext.ldap.plugin:LdapPlugin ''', - ) +) diff --git a/test.ini b/test.ini new file mode 100644 index 0000000..ab70ba6 --- /dev/null +++ b/test.ini @@ -0,0 +1,52 @@ +[server:main] +use = egg:Paste#http +host = 0.0.0.0 +port = 5000 + +[app:main] +# use a full path to ensure the config is picked up wherever the tests are run from (this covers off +# both the default setup, where the tests are run from /srv/app/src/ckanext-ldap/, and running them +# from an IDE like PyCharm which will copy the code into /opt/project/ and run it from there +use = config:/srv/app/src/ckan/test-core.ini + +# the hosts referenced here resolve to the other docker containers configured in docker-compose.yml +sqlalchemy.url = postgresql://ckan:password@db/ckan +ckan.datastore.write_url = postgresql://ckan:password@db/datastore +ckan.datastore.read_url = postgresql://datastore_ro:password@db/datastore +ckan.redis.url = redis://redis:6379/1 +solr_url = http://solr:8983/solr/ckan + +[loggers] +keys = root, ckan, ckanext + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = WARNING +handlers = console + +[logger_ckan] +level = INFO +handlers = console +qualname = ckan +propagate = 0 + +[logger_ckanext] +level = DEBUG +handlers = console +qualname = ckanext +propagate = 0 + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(asctime)s %(levelname)-5.5s [%(name)s] %(message)s + diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_helpers.py b/tests/test_helpers.py new file mode 100644 index 0000000..53cfdaf --- /dev/null +++ b/tests/test_helpers.py @@ -0,0 +1,11 @@ +from ckanext.ldap.lib.helpers import is_ldap_user +from mock import MagicMock, patch + + +def test_is_ldap_user(): + mock_session = {u'ckanext-ldap-user': MagicMock()} + with patch(u'ckanext.ldap.lib.helpers.session', mock_session): + assert is_ldap_user() + + del mock_session[u'ckanext-ldap-user'] + assert not is_ldap_user() From 0f2d18ad1167bb5429b3540cf8bcffe63a13f14d Mon Sep 17 00:00:00 2001 From: Josh Humphries Date: Mon, 25 Jan 2021 20:57:28 +0000 Subject: [PATCH 4/9] Python3 support --- CHANGELOG.md | 11 ++- ckanext/ldap/cli.py | 22 ++--- ckanext/ldap/lib/helpers.py | 18 ++-- ckanext/ldap/lib/search.py | 102 +++++++++++----------- ckanext/ldap/logic/auth/create.py | 8 +- ckanext/ldap/logic/auth/update.py | 16 ++-- ckanext/ldap/model/ldap_user.py | 17 ++-- ckanext/ldap/plugin.py | 136 +++++++++++++++--------------- ckanext/ldap/routes/_helpers.py | 103 +++++++++++----------- ckanext/ldap/routes/login.py | 32 +++---- dev_requirements.txt | 1 - docker/Dockerfile | 12 +-- setup.py | 38 +++++---- tests/test_helpers.py | 6 +- 14 files changed, 263 insertions(+), 259 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39fdfa0..29198a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,14 +2,23 @@ (This file may not be historically complete, as it is a recent addition to the project). +## [3.0.0] - 2021-01-25 + +- Drop python2 support, add complete python3.6-3.8 support +- Switch docker based tests to run on python3 +- Remove python2 specific code (_u_ string prefixes primarily), embrace some python3 code (use of + fstrings) +- Remove dependency on six + ## [2.1.0] - 2021-01-25 - Updated to work with CKAN 2.9.1. - Switched to docker based testing. +- Add dependency on six ## [2.0.0-alpha] - 2019-07-23 - Updated to work with CKAN 2.9.0a, e.g.: - uses toolkit wherever possible - references to Pylons removed -- Standardised README, CHANGELOG, setup.py and .github files to match other Museum extensions \ No newline at end of file +- Standardised README, CHANGELOG, setup.py and .github files to match other Museum extensions diff --git a/ckanext/ldap/cli.py b/ckanext/ldap/cli.py index 221bcdb..332488a 100644 --- a/ckanext/ldap/cli.py +++ b/ckanext/ldap/cli.py @@ -15,33 +15,33 @@ def ldap(): pass -@ldap.command(name=u'setup-org') +@ldap.command(name='setup-org') def setup_org(): ''' Sets up the default organisation which all ldap users will be automatically made members of. ''' # get the organisation all users will be added to - organization_id = toolkit.config[u'ckanext.ldap.organization.id'] + organization_id = toolkit.config['ckanext.ldap.organization.id'] # set up context - user = toolkit.get_action(u'get_site_user')({ - u'ignore_auth': True + user = toolkit.get_action('get_site_user')({ + 'ignore_auth': True }, {}) context = { - u'user': user[u'name'] + 'user': user['name'] } try: - toolkit.get_action(u'organization_show')(context, { - u'id': organization_id + toolkit.get_action('organization_show')(context, { + 'id': organization_id }) click.secho(u"Organisation already exists, doing nothing", fg=u"green") except toolkit.ObjectNotFound: # see the following commit to understand why this line is here # http://github.com/ckan/ckanext-harvest/commit/f315f41c86cbde4a49ef869b6993598f8cb11e2d - context.pop(u'__auth_audit', None) - toolkit.get_action(u'organization_create')(context, { - u'id': organization_id, - u'name': organization_id + context.pop('__auth_audit', None) + toolkit.get_action('organization_create')(context, { + 'id': organization_id, + 'name': organization_id }) click.secho(u"New organisation created", fg=u"green") diff --git a/ckanext/ldap/lib/helpers.py b/ckanext/ldap/lib/helpers.py index 024f69b..84c1d3c 100644 --- a/ckanext/ldap/lib/helpers.py +++ b/ckanext/ldap/lib/helpers.py @@ -3,14 +3,10 @@ # # This file is part of ckanext-ldap # Created by the Natural History Museum in London, UK -import six from ckan.common import session from ckan.plugins import toolkit -if six.PY3: - from urllib.parse import urlparse, parse_qs -else: - from urlparse import urlparse, parse_qs +from urllib.parse import urlparse, parse_qs def is_ldap_user(): @@ -19,7 +15,7 @@ def is_ldap_user(): :returns: boolean ''' - return u'ckanext-ldap-user' in session + return 'ckanext-ldap-user' in session def get_login_action(): @@ -27,18 +23,18 @@ def get_login_action(): Returns ldap login handler. Preserves parameter `came_from` as stored in context object's login_handler. ''' - if hasattr(toolkit.c, u'login_handler'): - came_from = parse_qs(urlparse(toolkit.c.login_handler).query).get(u'came_from') + if hasattr(toolkit.c, 'login_handler'): + came_from = parse_qs(urlparse(toolkit.c.login_handler).query).get('came_from') else: came_from = None if came_from: - action = toolkit.url_for(u'ldap.login_handler', came_from=str(came_from[0])) + action = toolkit.url_for('ldap.login_handler', came_from=str(came_from[0])) else: - action = toolkit.url_for(u'ldap.login_handler') + action = toolkit.url_for('ldap.login_handler') return action -def decode_str(s, encoding=u'utf-8'): +def decode_str(s, encoding='utf-8'): try: # this try throws NameError if this is python3 if isinstance(s, basestring) and isinstance(s, str): diff --git a/ckanext/ldap/lib/search.py b/ckanext/ldap/lib/search.py index 9a1fa9c..4aee72a 100644 --- a/ckanext/ldap/lib/search.py +++ b/ckanext/ldap/lib/search.py @@ -11,7 +11,7 @@ from ckanext.ldap.lib import helpers from ckanext.ldap.lib.exceptions import MultipleMatchError -log = logging.getLogger(u'ckanext.ldap') +log = logging.getLogger('ckanext.ldap') def find_ldap_user(login): @@ -20,61 +20,59 @@ def find_ldap_user(login): :param login: The login to find in the LDAP database :returns: None if no user is found, a dictionary defining 'cn', 'username', 'fullname' and - 'email otherwise. + 'email' otherwise. ''' - cnx = ldap.initialize(toolkit.config[u'ckanext.ldap.uri'], bytes_mode=False, - trace_level=toolkit.config[u'ckanext.ldap.trace_level']) + cnx = ldap.initialize(toolkit.config['ckanext.ldap.uri'], bytes_mode=False, + trace_level=toolkit.config['ckanext.ldap.trace_level']) cnx.set_option(ldap.OPT_NETWORK_TIMEOUT, 10) - if toolkit.config.get(u'ckanext.ldap.auth.dn'): + if toolkit.config.get('ckanext.ldap.auth.dn'): try: - if toolkit.config[u'ckanext.ldap.auth.method'] == u'SIMPLE': - cnx.bind_s(toolkit.config[u'ckanext.ldap.auth.dn'], - toolkit.config[u'ckanext.ldap.auth.password']) - elif toolkit.config[u'ckanext.ldap.auth.method'] == u'SASL': - if toolkit.config[u'ckanext.ldap.auth.mechanism'] == u'DIGEST-MD5': - auth_tokens = ldap.sasl.digest_md5(toolkit.config[u'ckanext.ldap.auth.dn'], - toolkit.config[ - u'ckanext.ldap.auth.password']) - cnx.sasl_interactive_bind_s(u'', auth_tokens) + if toolkit.config['ckanext.ldap.auth.method'] == 'SIMPLE': + cnx.bind_s(toolkit.config['ckanext.ldap.auth.dn'], + toolkit.config['ckanext.ldap.auth.password']) + elif toolkit.config['ckanext.ldap.auth.method'] == 'SASL': + if toolkit.config['ckanext.ldap.auth.mechanism'] == 'DIGEST-MD5': + auth_tokens = ldap.sasl.digest_md5(toolkit.config['ckanext.ldap.auth.dn'], + toolkit.config['ckanext.ldap.auth.password']) + cnx.sasl_interactive_bind_s('', auth_tokens) else: - log.error(u'SASL mechanism not supported: {0}'.format( - toolkit.config[u'ckanext.ldap.auth.mechanism'])) + log.error(f'SASL mechanism not supported: ' + f'{toolkit.config["ckanext.ldap.auth.mechanism"]}') return None else: - log.error(u'LDAP authentication method is not supported: {0}'.format( - toolkit.config[u'ckanext.ldap.auth.method'])) + log.error(f'LDAP authentication method is not supported: ' + f'{toolkit.config["ckanext.ldap.auth.method"]}') return None except ldap.SERVER_DOWN: - log.error(u'LDAP server is not reachable') + log.error('LDAP server is not reachable') return None except ldap.INVALID_CREDENTIALS: - log.error( - u'LDAP server credentials (ckanext.ldap.auth.dn and ckanext.ldap.auth.password) ' - u'invalid') + log.error('LDAP server credentials (ckanext.ldap.auth.dn and ' + 'ckanext.ldap.auth.password) invalid') return None except ldap.LDAPError as e: - log.error(u'Fatal LDAP Error: {0}'.format(e)) + log.error(f'Fatal LDAP Error: {e}') return None - filter_str = toolkit.config[u'ckanext.ldap.search.filter'].format( + filter_str = toolkit.config['ckanext.ldap.search.filter'].format( login=ldap.filter.escape_filter_chars(login)) - attributes = [toolkit.config[u'ckanext.ldap.username']] - if u'ckanext.ldap.fullname' in toolkit.config: - attributes.append(toolkit.config[u'ckanext.ldap.fullname']) - if u'ckanext.ldap.email' in toolkit.config: - attributes.append(toolkit.config[u'ckanext.ldap.email']) + attributes = [toolkit.config['ckanext.ldap.username']] + if 'ckanext.ldap.fullname' in toolkit.config: + attributes.append(toolkit.config['ckanext.ldap.fullname']) + if 'ckanext.ldap.email' in toolkit.config: + attributes.append(toolkit.config['ckanext.ldap.email']) try: - ret = ldap_search(cnx, filter_str, attributes, non_unique=u'log') - if ret is None and u'ckanext.ldap.search.alt' in toolkit.config: - filter_str = toolkit.config[u'ckanext.ldap.search.alt'].format( + ret = ldap_search(cnx, filter_str, attributes, non_unique='log') + if ret is None and 'ckanext.ldap.search.alt' in toolkit.config: + filter_str = toolkit.config['ckanext.ldap.search.alt'].format( login=ldap.filter.escape_filter_chars(login)) - ret = ldap_search(cnx, filter_str, attributes, non_unique=u'raise') + ret = ldap_search(cnx, filter_str, attributes, non_unique='raise') finally: cnx.unbind() return ret -def ldap_search(cnx, filter_str, attributes, non_unique=u'raise'): +def ldap_search(cnx, filter_str, attributes, non_unique='raise'): '''Helper function to perform the actual LDAP search :param cnx: The LDAP connection object @@ -86,53 +84,53 @@ def ldap_search(cnx, filter_str, attributes, non_unique=u'raise'): admin, not by the current user) or 'raise' (raise an exception with a message that will be displayed to the current user - such as 'please use your unique id instead'). Other values will - silently ignore the error. (Default value = u'raise') + silently ignore the error. (Default value = 'raise') :returns: A dictionary defining 'cn', self.ldap_username and any other attributes that were defined in attributes; or None if no user was found. ''' try: - res = cnx.search_s(toolkit.config[u'ckanext.ldap.base_dn'], ldap.SCOPE_SUBTREE, + res = cnx.search_s(toolkit.config['ckanext.ldap.base_dn'], ldap.SCOPE_SUBTREE, filterstr=filter_str, attrlist=attributes) except ldap.SERVER_DOWN: - log.error(u'LDAP server is not reachable') + log.error('LDAP server is not reachable') return None except ldap.OPERATIONS_ERROR as e: log.error( - u'LDAP query failed. Maybe you need auth credentials for performing searches? Error ' - u'returned by the server: ' + e.info) + 'LDAP query failed. Maybe you need auth credentials for performing searches? Error ' + 'returned by the server: ' + e.info) return None except (ldap.NO_SUCH_OBJECT, ldap.REFERRAL) as e: log.error( - u'LDAP distinguished name (ckanext.ldap.base_dn) is malformed or does not exist.') + 'LDAP distinguished name (ckanext.ldap.base_dn) is malformed or does not exist.') return None except ldap.FILTER_ERROR: - log.error(u'LDAP filter (ckanext.ldap.search) is malformed') + log.error('LDAP filter (ckanext.ldap.search) is malformed') return None if len(res) > 1: - if non_unique == u'log': + if non_unique == 'log': log.error( - u'LDAP search.filter search returned more than one entry, ignoring. Fix the ' - u'search to return only 1 or 0 results.') - elif non_unique == u'raise': - raise MultipleMatchError(toolkit.config[u'ckanext.ldap.search.alt_msg']) + 'LDAP search.filter search returned more than one entry, ignoring. Fix the ' + 'search to return only 1 or 0 results.') + elif non_unique == 'raise': + raise MultipleMatchError(toolkit.config['ckanext.ldap.search.alt_msg']) return None elif len(res) == 1: cn = res[0][0] attr = res[0][1] ret = { - u'cn': cn, + 'cn': cn, } # Check required fields - for i in [u'username', u'email']: - cname = u'ckanext.ldap.' + i + for i in ['username', 'email']: + cname = 'ckanext.ldap.' + i if toolkit.config[cname] not in attr or not attr[toolkit.config[cname]]: - log.error(u'LDAP search did not return a {}.'.format(i)) + log.error('LDAP search did not return a {}.'.format(i)) return None # Set return dict - for i in [u'username', u'fullname', u'email', u'about']: - cname = u'ckanext.ldap.' + i + for i in ['username', 'fullname', 'email', 'about']: + cname = 'ckanext.ldap.' + i if cname in toolkit.config and toolkit.config[cname] in attr: v = attr[toolkit.config[cname]] if v: diff --git a/ckanext/ldap/logic/auth/create.py b/ckanext/ldap/logic/auth/create.py index 6178f5f..f85da59 100644 --- a/ckanext/ldap/logic/auth/create.py +++ b/ckanext/ldap/logic/auth/create.py @@ -16,12 +16,12 @@ def user_create(next_auth, context, data_dict=None): :param context: :param data_dict: (Default value = None) ''' - if data_dict and u'name' in data_dict: - ldap_user_dict = find_ldap_user(data_dict[u'name']) + if data_dict and 'name' in data_dict: + ldap_user_dict = find_ldap_user(data_dict['name']) if ldap_user_dict: return { - u'success': False, - u'msg': toolkit._(u'An LDAP user by that name already exists') + 'success': False, + 'msg': toolkit._('An LDAP user by that name already exists') } return next_auth(context, data_dict) diff --git a/ckanext/ldap/logic/auth/update.py b/ckanext/ldap/logic/auth/update.py index e7da122..a5d37c9 100644 --- a/ckanext/ldap/logic/auth/update.py +++ b/ckanext/ldap/logic/auth/update.py @@ -26,21 +26,21 @@ def user_update(next_auth, context, data_dict): except toolkit.ObjectNotFound: pass # Prevent edition of LDAP users (if so configured) - if toolkit.config[u'ckanext.ldap.prevent_edits'] and user_obj and LdapUser.by_user_id( + if toolkit.config['ckanext.ldap.prevent_edits'] and user_obj and LdapUser.by_user_id( user_obj.id): return { - u'success': False, - u'msg': toolkit._(u'Cannot edit LDAP users') + 'success': False, + 'msg': toolkit._('Cannot edit LDAP users') } # Prevent name clashes! - if u'name' in data_dict and user_obj and user_obj.name != data_dict[u'name']: - ldap_user_dict = find_ldap_user(data_dict[u'name']) + if 'name' in data_dict and user_obj and user_obj.name != data_dict['name']: + ldap_user_dict = find_ldap_user(data_dict['name']) if ldap_user_dict: if (len(user_obj.ldap_user) == 0 or - user_obj.ldap_user[0].ldap_id != ldap_user_dict[u'ldap_id']): + user_obj.ldap_user[0].ldap_id != ldap_user_dict['ldap_id']): return { - u'success': False, - u'msg': toolkit._(u'An LDAP user by that name already exists') + 'success': False, + 'msg': toolkit._('An LDAP user by that name already exists') } return next_auth(context, data_dict) diff --git a/ckanext/ldap/model/ldap_user.py b/ckanext/ldap/model/ldap_user.py index e2c1f77..5d24d2d 100644 --- a/ckanext/ldap/model/ldap_user.py +++ b/ckanext/ldap/model/ldap_user.py @@ -9,16 +9,16 @@ from ckan import model from sqlalchemy import Column, ForeignKey, Table, orm, types -__all__ = [u'LdapUser'] +__all__ = ['LdapUser'] -ldap_user_table = Table(u'ldap_user', model.meta.metadata, - Column(u'id', types.UnicodeText, primary_key=True, +ldap_user_table = Table('ldap_user', model.meta.metadata, + Column('id', types.UnicodeText, primary_key=True, default=model.types.make_uuid), - Column(u'user_id', types.UnicodeText, ForeignKey(u'user.id'), + Column('user_id', types.UnicodeText, ForeignKey('user.id'), unique=True, nullable=False), - Column(u'ldap_id', types.UnicodeText, index=True, unique=True, + Column('ldap_id', types.UnicodeText, index=True, unique=True, nullable=False), - Column(u'created', types.DateTime, default=datetime.datetime.now) + Column('created', types.DateTime, default=datetime.datetime.now) ) @@ -59,7 +59,6 @@ def by_user_id(cls, user_id, autoflush=True): model.meta.mapper(LdapUser, ldap_user_table, properties={ - u'user': orm.relation(model.user.User, - backref=orm.backref(u'ldap_user', - cascade=u'all, delete, delete-orphan')) + 'user': orm.relation(model.user.User, + backref=orm.backref('ldap_user', cascade='all, delete, delete-orphan')) }, ) diff --git a/ckanext/ldap/plugin.py b/ckanext/ldap/plugin.py index f13536e..47e35de 100644 --- a/ckanext/ldap/plugin.py +++ b/ckanext/ldap/plugin.py @@ -49,7 +49,7 @@ def update_config(self, config): :param config: ''' - toolkit.add_template_directory(config, u'templates') + toolkit.add_template_directory(config, 'templates') ## IBlueprint def get_blueprint(self): @@ -58,8 +58,8 @@ def get_blueprint(self): def get_auth_functions(self): '''Implements IAuthFunctions.get_auth_functions''' return { - u'user_update': user_update, - u'user_create': user_create + 'user_update': user_update, + 'user_create': user_create } def configure(self, config): @@ -73,62 +73,62 @@ def configure(self, config): # Our own config schema, defines required items, default values and # transform functions schema = { - u'ckanext.ldap.uri': { - u'required': True + 'ckanext.ldap.uri': { + 'required': True }, - u'ckanext.ldap.base_dn': { - u'required': True + 'ckanext.ldap.base_dn': { + 'required': True }, - u'ckanext.ldap.search.filter': { - u'required': True + 'ckanext.ldap.search.filter': { + 'required': True }, - u'ckanext.ldap.username': { - u'required': True + 'ckanext.ldap.username': { + 'required': True }, - u'ckanext.ldap.email': { - u'required': True + 'ckanext.ldap.email': { + 'required': True }, - u'ckanext.ldap.auth.dn': {}, - u'ckanext.ldap.auth.password': { - u'required_if': u'ckanext.ldap.auth.dn' + 'ckanext.ldap.auth.dn': {}, + 'ckanext.ldap.auth.password': { + 'required_if': 'ckanext.ldap.auth.dn' }, - u'ckanext.ldap.auth.method': { - u'default': u'SIMPLE', - u'validate': _allowed_auth_methods + 'ckanext.ldap.auth.method': { + 'default': 'SIMPLE', + 'validate': _allowed_auth_methods }, - u'ckanext.ldap.auth.mechanism': { - u'default': u'DIGEST-MD5', - u'validate': _allowed_auth_mechanisms + 'ckanext.ldap.auth.mechanism': { + 'default': 'DIGEST-MD5', + 'validate': _allowed_auth_mechanisms }, - u'ckanext.ldap.search.alt': {}, - u'ckanext.ldap.search.alt_msg': { - u'required_if': u'ckanext.ldap.search.alt' + 'ckanext.ldap.search.alt': {}, + 'ckanext.ldap.search.alt_msg': { + 'required_if': 'ckanext.ldap.search.alt' }, - u'ckanext.ldap.fullname': {}, - u'ckanext.ldap.organization.id': {}, - u'ckanext.ldap.organization.role': { - u'default': u'member', - u'validate': _allowed_roles + 'ckanext.ldap.fullname': {}, + 'ckanext.ldap.organization.id': {}, + 'ckanext.ldap.organization.role': { + 'default': 'member', + 'validate': _allowed_roles }, - u'ckanext.ldap.ckan_fallback': { - u'default': False, - u'parse': toolkit.asbool + 'ckanext.ldap.ckan_fallback': { + 'default': False, + 'parse': toolkit.asbool }, - u'ckanext.ldap.prevent_edits': { - u'default': False, - u'parse': toolkit.asbool + 'ckanext.ldap.prevent_edits': { + 'default': False, + 'parse': toolkit.asbool }, - u'ckanext.ldap.migrate': { - u'default': False, - u'parse': toolkit.asbool + 'ckanext.ldap.migrate': { + 'default': False, + 'parse': toolkit.asbool }, - u'ckanext.ldap.debug_level': { - u'default': 0, - u'parse': toolkit.asint + 'ckanext.ldap.debug_level': { + 'default': 0, + 'parse': toolkit.asint }, - u'ckanext.ldap.trace_level': { - u'default': 0, - u'parse': toolkit.asint + 'ckanext.ldap.trace_level': { + 'default': 0, + 'parse': toolkit.asint }, } errors = [] @@ -136,21 +136,21 @@ def configure(self, config): config_value = config.get(key, None) if config_value: - if u'parse' in options: - config_value = (options[u'parse'])(config_value) + if 'parse' in options: + config_value = (options['parse'])(config_value) try: - if u'validate' in options: - (options[u'validate'])(config_value) + if 'validate' in options: + (options['validate'])(config_value) toolkit.config[key] = config_value except ConfigError as e: errors.append(str(e)) - elif options.get(u'required', False): - errors.append(u'Configuration parameter {0} is required'.format(key)) - elif u'required_if' in options and options[u'required_if'] in toolkit.config: - errors.append(u'Configuration parameter {0} is required ' - u'when {1} is present'.format(key, options[u'required_if'])) - elif u'default' in options: - toolkit.config[key] = options[u'default'] + elif options.get('required', False): + errors.append('Configuration parameter {0} is required'.format(key)) + elif 'required_if' in options and options['required_if'] in toolkit.config: + errors.append('Configuration parameter {0} is required ' + 'when {1} is present'.format(key, options['required_if'])) + elif 'default' in options: + toolkit.config[key] = options['default'] # make sure the config options are all unicode for LDAP if isinstance(toolkit.config.get(key, None), str): @@ -159,7 +159,7 @@ def configure(self, config): except NameError: toolkit.config[key] = str(toolkit.config.get(key)) if len(errors): - raise ConfigError(u'\n'.join(errors)) + raise ConfigError('\n'.join(errors)) # IAuthenticator def login(self): @@ -175,7 +175,7 @@ def identify(self): Identify which user (if any) is logged in via this plugin ''' # FIXME: This breaks if the current user changes their own user name. - user = session.get(u'ckanext-ldap-user') + user = session.get('ckanext-ldap-user') if user: toolkit.c.user = user else: @@ -194,28 +194,28 @@ def _delete_session_items(self): ''' Delete user details stored in the session by this plugin. ''' - if u'ckanext-ldap-user' in session: - del session[u'ckanext-ldap-user'] + if 'ckanext-ldap-user' in session: + del session['ckanext-ldap-user'] session.save() def get_helpers(self): return { - u'is_ldap_user': is_ldap_user, - u'get_login_action': get_login_action + 'is_ldap_user': is_ldap_user, + 'get_login_action': get_login_action } def _allowed_roles(v): - if v not in [u'member', u'editor', u'admin']: - raise ConfigError(u'role must be one of "member", "editor" or "admin"') + if v not in ['member', 'editor', 'admin']: + raise ConfigError('role must be one of "member", "editor" or "admin"') def _allowed_auth_methods(v): - if v.upper() not in [u'SIMPLE', u'SASL']: - raise ConfigError(u'Only SIMPLE and SASL authentication methods are supported') + if v.upper() not in ['SIMPLE', 'SASL']: + raise ConfigError('Only SIMPLE and SASL authentication methods are supported') def _allowed_auth_mechanisms(v): # Only DIGEST-MD5 is supported when the auth method is SASL - if v.upper() != u'DIGEST-MD5': - raise ConfigError(u'Only DIGEST-MD5 is supported as an authentication mechanism') + if v.upper() != 'DIGEST-MD5': + raise ConfigError('Only DIGEST-MD5 is supported as an authentication mechanism') diff --git a/ckanext/ldap/routes/_helpers.py b/ckanext/ldap/routes/_helpers.py index 2c00092..d978e09 100644 --- a/ckanext/ldap/routes/_helpers.py +++ b/ckanext/ldap/routes/_helpers.py @@ -13,6 +13,7 @@ from ckan.common import session from ckan.model import Session from ckan.plugins import toolkit + from ckanext.ldap.lib.exceptions import UserConflictError from ckanext.ldap.model.ldap_user import LdapUser @@ -30,7 +31,7 @@ def login_failed(notice=None, error=None): toolkit.h.flash_notice(notice) if error: toolkit.h.flash_error(error) - return toolkit.redirect_to(u'user.login') + return toolkit.redirect_to('user.login') def login_success(user_name, came_from): @@ -39,9 +40,9 @@ def login_success(user_name, came_from): :param user_name: The user name ''' - session[u'ckanext-ldap-user'] = user_name + session['ckanext-ldap-user'] = user_name session.save() - return toolkit.redirect_to(u'user.logged_in', came_from=came_from) + return toolkit.redirect_to('user.logged_in', came_from=came_from) def get_user_dict(user_id): @@ -51,12 +52,12 @@ def get_user_dict(user_id): @param user_id: The user id ''' context = { - u'ignore_auth': True + 'ignore_auth': True } data_dict = { - u'id': user_id + 'id': user_id } - return toolkit.get_action(u'user_show')(context, data_dict) + return toolkit.get_action('user_show')(context, data_dict) def ckan_user_exists(user_name): @@ -70,20 +71,20 @@ def ckan_user_exists(user_name): user = get_user_dict(user_name) except toolkit.ObjectNotFound: return { - u'exists': False, - u'is_ldap': False + 'exists': False, + 'is_ldap': False } - ldap_user = LdapUser.by_user_id(user[u'id']) + ldap_user = LdapUser.by_user_id(user['id']) if ldap_user: return { - u'exists': True, - u'is_ldap': True + 'exists': True, + 'is_ldap': True } else: return { - u'exists': True, - u'is_ldap': False + 'exists': True, + 'is_ldap': False } @@ -94,16 +95,16 @@ def get_unique_user_name(base_name): :param base_name: Base name :return: A valid user name not currently in use based on base_name ''' - base_name = re.sub(u'[^-a-z0-9_]', u'_', base_name.lower()) + base_name = re.sub('[^-a-z0-9_]', '_', base_name.lower()) base_name = base_name[0:100] if len(base_name) < 2: - base_name = (base_name + u'__')[0:2] + base_name = (base_name + '__')[0:2] count = 0 user_name = base_name - while (ckan_user_exists(user_name))[u'exists']: + while (ckan_user_exists(user_name))['exists']: count += 1 - user_name = u'{base}{count}'.format(base=base_name[0:100 - len(str(count))], - count=str(count)) + user_name = '{base}{count}'.format(base=base_name[0:100 - len(str(count))], + count=str(count)) return user_name @@ -115,7 +116,7 @@ def get_or_create_ldap_user(ldap_user_dict): :return: The CKAN username of an existing user ''' # Look for existing user, and if found return it. - ldap_user = LdapUser.by_ldap_id(ldap_user_dict[u'username']) + ldap_user = LdapUser.by_ldap_id(ldap_user_dict['username']) if ldap_user: # TODO: Update the user detail. return ldap_user.user.name @@ -123,14 +124,14 @@ def get_or_create_ldap_user(ldap_user_dict): update = False # Check whether we have a name conflict (based on the ldap name, without mapping # it to allowed chars) - exists = ckan_user_exists(ldap_user_dict[u'username']) - if exists[u'exists'] and not exists[u'is_ldap']: + exists = ckan_user_exists(ldap_user_dict['username']) + if exists['exists'] and not exists['is_ldap']: # If ckanext.ldap.migrate is set, update exsting user_dict. - if not toolkit.config[u'ckanext.ldap.migrate']: + if not toolkit.config['ckanext.ldap.migrate']: raise UserConflictError(toolkit._( - u'There is a username conflict. Please inform the site administrator.')) + 'There is a username conflict. Please inform the site administrator.')) else: - user_dict = get_user_dict(ldap_user_dict[u'username']) + user_dict = get_user_dict(ldap_user_dict['username']) update = True # If a user with the same ckan name already exists but is an LDAP user, this means @@ -141,45 +142,45 @@ def get_or_create_ldap_user(ldap_user_dict): # Now get a unique user name (if not "migrating"), and create the CKAN user and # the LdapUser entry. - user_name = user_dict[u'name'] if update else get_unique_user_name( - ldap_user_dict[u'username']) + user_name = user_dict['name'] if update else get_unique_user_name( + ldap_user_dict['username']) user_dict.update({ - u'name': user_name, - u'email': ldap_user_dict[u'email'], - u'password': str(uuid.uuid4()) + 'name': user_name, + 'email': ldap_user_dict['email'], + 'password': str(uuid.uuid4()) }) - if u'fullname' in ldap_user_dict: - user_dict[u'fullname'] = ldap_user_dict[u'fullname'] - if u'about' in ldap_user_dict: - user_dict[u'about'] = ldap_user_dict[u'about'] + if 'fullname' in ldap_user_dict: + user_dict['fullname'] = ldap_user_dict['fullname'] + if 'about' in ldap_user_dict: + user_dict['about'] = ldap_user_dict['about'] if update: - ckan_user = toolkit.get_action(u'user_update')( + ckan_user = toolkit.get_action('user_update')( context={ - u'ignore_auth': True + 'ignore_auth': True }, data_dict=user_dict ) else: - ckan_user = toolkit.get_action(u'user_create')( + ckan_user = toolkit.get_action('user_create')( context={ - u'ignore_auth': True + 'ignore_auth': True }, data_dict=user_dict ) - ldap_user = LdapUser(user_id=ckan_user[u'id'], ldap_id=ldap_user_dict[u'username']) + ldap_user = LdapUser(user_id=ckan_user['id'], ldap_id=ldap_user_dict['username']) Session.add(ldap_user) Session.commit() # Add the user to it's group if needed - if u'ckanext.ldap.organization.id' in toolkit.config: - toolkit.get_action(u'member_create')( + if 'ckanext.ldap.organization.id' in toolkit.config: + toolkit.get_action('member_create')( context={ - u'ignore_auth': True + 'ignore_auth': True }, data_dict={ - u'id': toolkit.config[u'ckanext.ldap.organization.id'], - u'object': user_name, - u'object_type': u'user', - u'capacity': toolkit.config[u'ckanext.ldap.organization.role'] + 'id': toolkit.config['ckanext.ldap.organization.id'], + 'object': user_name, + 'object_type': 'user', + 'capacity': toolkit.config['ckanext.ldap.organization.role'] } ) return user_name @@ -193,19 +194,19 @@ def check_ldap_password(cn, password): :param password: Password for cn :return: True on success, False on failure ''' - cnx = ldap.initialize(toolkit.config[u'ckanext.ldap.uri'], bytes_mode=False, - trace_level=toolkit.config[u'ckanext.ldap.trace_level']) + cnx = ldap.initialize(toolkit.config['ckanext.ldap.uri'], bytes_mode=False, + trace_level=toolkit.config['ckanext.ldap.trace_level']) try: cnx.bind_s(cn, password) except ldap.SERVER_DOWN: - log.error(u'LDAP server is not reachable') + log.error('LDAP server is not reachable') return False except ldap.INVALID_CREDENTIALS: - log.debug(u'Invalid LDAP credentials') + log.debug('Invalid LDAP credentials') return False # Fail on empty password - if password == u'': - log.debug(u'Invalid LDAP credentials') + if password == '': + log.debug('Invalid LDAP credentials') return False cnx.unbind_s() return True diff --git a/ckanext/ldap/routes/login.py b/ckanext/ldap/routes/login.py index 3dc6ecf..4b01ddd 100644 --- a/ckanext/ldap/routes/login.py +++ b/ckanext/ldap/routes/login.py @@ -13,12 +13,12 @@ from . import _helpers -blueprint = Blueprint(name=u'ldap', import_name=__name__) +blueprint = Blueprint(name='ldap', import_name=__name__) @blueprint.before_app_first_request def initialise(): - ldap.set_option(ldap.OPT_DEBUG_LEVEL, toolkit.config[u'ckanext.ldap.debug_level']) + ldap.set_option(ldap.OPT_DEBUG_LEVEL, toolkit.config['ckanext.ldap.debug_level']) @blueprint.route('/ldap_login_handler', methods=['POST']) @@ -27,16 +27,16 @@ def login_handler(): Action called when login in via the LDAP login form. ''' params = toolkit.request.values - came_from = params.get(u'came_from', None) - if u'login' in params and u'password' in params: - login = params[u'login'] - password = params[u'password'] + came_from = params.get('came_from', None) + if 'login' in params and 'password' in params: + login = params['login'] + password = params['password'] try: ldap_user_dict = find_ldap_user(login) except MultipleMatchError as e: # Multiple users match. Inform the user and try again. return _helpers.login_failed(notice=str(e)) - if ldap_user_dict and _helpers.check_ldap_password(ldap_user_dict[u'cn'], password): + if ldap_user_dict and _helpers.check_ldap_password(ldap_user_dict['cn'], password): try: user_name = _helpers.get_or_create_ldap_user(ldap_user_dict) except UserConflictError as e: @@ -46,26 +46,26 @@ def login_handler(): # There is an LDAP user, but the auth is wrong. There could be a # CKAN user of the same name if the LDAP user had been created # later - in which case we have a conflict we can't solve. - if toolkit.config[u'ckanext.ldap.ckan_fallback']: + if toolkit.config['ckanext.ldap.ckan_fallback']: exists = _helpers.ckan_user_exists(login) - if exists[u'exists'] and not exists[u'is_ldap']: + if exists['exists'] and not exists['is_ldap']: return _helpers.login_failed(error=toolkit._( - u'Username conflict. Please contact the site administrator.')) - return _helpers.login_failed(error=toolkit._(u'Bad username or password.')) - elif toolkit.config[u'ckanext.ldap.ckan_fallback']: + 'Username conflict. Please contact the site administrator.')) + return _helpers.login_failed(error=toolkit._('Bad username or password.')) + elif toolkit.config['ckanext.ldap.ckan_fallback']: # No LDAP user match, see if we have a CKAN user match try: user_dict = _helpers.get_user_dict(login) # We need the model to validate the password - user = User.by_name(user_dict[u'name']) + user = User.by_name(user_dict['name']) except toolkit.ObjectNotFound: user = None if user and user.validate_password(password): return _helpers.login_success(user.name, came_from=came_from) else: return _helpers.login_failed( - error=toolkit._(u'Bad username or password.')) + error=toolkit._('Bad username or password.')) else: - return _helpers.login_failed(error=toolkit._(u'Bad username or password.')) + return _helpers.login_failed(error=toolkit._('Bad username or password.')) return _helpers.login_failed( - error=toolkit._(u'Please enter a username and password')) + error=toolkit._('Please enter a username and password')) diff --git a/dev_requirements.txt b/dev_requirements.txt index 0a53fed..fc8c3a2 100644 --- a/dev_requirements.txt +++ b/dev_requirements.txt @@ -1,3 +1,2 @@ -mock pytest>=4.6.5 pytest-cov>=2.7.1 diff --git a/docker/Dockerfile b/docker/Dockerfile index bef5cf3..221796a 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -1,4 +1,4 @@ -FROM openknowledge/ckan-dev:2.9-py2 +FROM openknowledge/ckan-dev:2.9 RUN apk add openldap-dev @@ -9,15 +9,15 @@ WORKDIR /srv/app/src/ckanext-ldap COPY . . # might as well update pip while we're here! -RUN pip2 install --upgrade pip +RUN pip3 install --upgrade pip # fixes this https://github.com/ckan/ckan/issues/5570 -RUN pip2 install pytest-ckan +RUN pip3 install pytest-ckan # install the dependencies -RUN python2 setup.py develop && \ - pip2 install -r requirements.txt && \ - pip2 install -r dev_requirements.txt +RUN python3 setup.py develop && \ + pip3 install -r requirements.txt && \ + pip3 install -r dev_requirements.txt # this entrypoint ensures our service dependencies (postgresql, solr and redis) are running before # running the cmd diff --git a/setup.py b/setup.py index b9ce754..e0c2666 100755 --- a/setup.py +++ b/setup.py @@ -6,36 +6,38 @@ from setuptools import find_packages, setup -__version__ = u'2.1.0' +__version__ = '3.0.0' -with open(u'README.md', u'r') as f: +with open('README.md', 'r') as f: __long_description__ = f.read() setup( - name=u'ckanext-ldap', + name='ckanext-ldap', version=__version__, - description=u'A CKAN extension that provides LDAP authentication.', + description='A CKAN extension that provides LDAP authentication.', long_description=__long_description__, classifiers=[ - u'Development Status :: 5 - Production/Stable', - u'License :: OSI Approved :: GNU General Public License v3 (GPLv3)', - u'Programming Language :: Python', - u'Programming Language :: Python :: 2.7' + 'Development Status :: 5 - Production/Stable', + 'License :: OSI Approved :: GNU General Public License v3 (GPLv3)', + 'Programming Language :: Python', + 'Programming Language :: Python :: 3 :: Only', + 'Programming Language :: Python :: 3.6', + 'Programming Language :: Python :: 3.7', + 'Programming Language :: Python :: 3.8', ], - keywords=u'CKAN data ldap', - author=u'Natural History Museum', - author_email=u'data@nhm.ac.uk', - url=u'https://github.com/NaturalHistoryMuseum/ckanext-ldap', - license=u'GNU GPLv3', - packages=find_packages(exclude=[u'tests']), - namespace_packages=[u'ckanext', u'ckanext.ldap'], + keywords='CKAN data ldap', + author='Natural History Museum', + author_email='data@nhm.ac.uk', + url='https://github.com/NaturalHistoryMuseum/ckanext-ldap', + license='GNU GPLv3', + packages=find_packages(exclude=['tests']), + namespace_packages=['ckanext', 'ckanext.ldap'], include_package_data=True, zip_safe=False, install_requires=[ - u'python-ldap==3.0.0', - u'six', + 'python-ldap==3.0.0', ], - entry_points=u''' + entry_points=''' [ckan.plugins] ldap=ckanext.ldap.plugin:LdapPlugin ''', diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 53cfdaf..943c678 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -3,9 +3,9 @@ def test_is_ldap_user(): - mock_session = {u'ckanext-ldap-user': MagicMock()} - with patch(u'ckanext.ldap.lib.helpers.session', mock_session): + mock_session = {'ckanext-ldap-user': MagicMock()} + with patch('ckanext.ldap.lib.helpers.session', mock_session): assert is_ldap_user() - del mock_session[u'ckanext-ldap-user'] + del mock_session['ckanext-ldap-user'] assert not is_ldap_user() From c79e933c9f2e8ae21ec1091764427c360b889446 Mon Sep 17 00:00:00 2001 From: Josh Humphries Date: Mon, 25 Jan 2021 20:59:13 +0000 Subject: [PATCH 5/9] Update readme --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 676b5d9..c778c8c 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,7 @@ [![Travis](https://img.shields.io/travis/NaturalHistoryMuseum/ckanext-ldap/master.svg?style=flat-square)](https://travis-ci.org/NaturalHistoryMuseum/ckanext-ldap) [![Coveralls](https://img.shields.io/coveralls/github/NaturalHistoryMuseum/ckanext-ldap/master.svg?style=flat-square)](https://coveralls.io/github/NaturalHistoryMuseum/ckanext-ldap) [![CKAN](https://img.shields.io/badge/ckan-2.9.1-orange.svg?style=flat-square)](https://github.com/ckan/ckan) +[![Python](https://img.shields.io/badge/python-3.6%20%7C%203.7%20%7C%203.8-blue.svg?style=flat-square)](https://www.python.org/) _A CKAN extension that provides LDAP authentication._ @@ -132,7 +133,7 @@ _Test coverage is currently extremely limited._ To run the tests in this extension, there is a Docker compose configuration available in this repository to make it easy. -To run the tests against ckan 2.9.x on Python2: +To run the tests against ckan 2.9.x on Python3: 1. Build the required images ```bash @@ -147,4 +148,4 @@ docker-compose build docker-compose run ckan ``` -The ckan image uses the Dockerfile in the `docker/` folder which is based on `openknowledge/ckan-dev:2.9-py2`. +The ckan image uses the Dockerfile in the `docker/` folder which is based on `openknowledge/ckan-dev:2.9`. From 29713606aa06fb879dff5a243173df00c6238574 Mon Sep 17 00:00:00 2001 From: Josh Humphries Date: Mon, 25 Jan 2021 21:26:24 +0000 Subject: [PATCH 6/9] Fix unicode issue and add a couple more tests Good golly we need more tests. --- ckanext/ldap/lib/helpers.py | 9 ++------- ckanext/ldap/lib/search.py | 2 +- ckanext/ldap/plugin.py | 6 ------ tests/test_helpers.py | 32 +++++++++++++++++++++++++++++++- 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/ckanext/ldap/lib/helpers.py b/ckanext/ldap/lib/helpers.py index 84c1d3c..524f280 100644 --- a/ckanext/ldap/lib/helpers.py +++ b/ckanext/ldap/lib/helpers.py @@ -35,11 +35,6 @@ def get_login_action(): def decode_str(s, encoding='utf-8'): - try: - # this try throws NameError if this is python3 - if isinstance(s, basestring) and isinstance(s, str): - return unicode(s, encoding) - except NameError: - if isinstance(s, bytes): - return s.decode(encoding) + if isinstance(s, bytes): + return s.decode(encoding) return s diff --git a/ckanext/ldap/lib/search.py b/ckanext/ldap/lib/search.py index 4aee72a..680811e 100644 --- a/ckanext/ldap/lib/search.py +++ b/ckanext/ldap/lib/search.py @@ -130,7 +130,7 @@ def ldap_search(cnx, filter_str, attributes, non_unique='raise'): return None # Set return dict for i in ['username', 'fullname', 'email', 'about']: - cname = 'ckanext.ldap.' + i + cname = f'ckanext.ldap.{i}' if cname in toolkit.config and toolkit.config[cname] in attr: v = attr[toolkit.config[cname]] if v: diff --git a/ckanext/ldap/plugin.py b/ckanext/ldap/plugin.py index 47e35de..f44dc6f 100644 --- a/ckanext/ldap/plugin.py +++ b/ckanext/ldap/plugin.py @@ -152,12 +152,6 @@ def configure(self, config): elif 'default' in options: toolkit.config[key] = options['default'] - # make sure the config options are all unicode for LDAP - if isinstance(toolkit.config.get(key, None), str): - try: - toolkit.config[key] = unicode(toolkit.config.get(key)) - except NameError: - toolkit.config[key] = str(toolkit.config.get(key)) if len(errors): raise ConfigError('\n'.join(errors)) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 943c678..b40b4c2 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -1,4 +1,7 @@ -from ckanext.ldap.lib.helpers import is_ldap_user +import pytest +from ckan.plugins import toolkit + +from ckanext.ldap.lib.helpers import is_ldap_user, get_login_action, decode_str from mock import MagicMock, patch @@ -9,3 +12,30 @@ def test_is_ldap_user(): del mock_session['ckanext-ldap-user'] assert not is_ldap_user() + + +@pytest.mark.ckan_config('ckan.plugins', 'ldap') +@pytest.mark.ckan_config('ckanext.ldap.uri', 'n/a') +@pytest.mark.ckan_config('ckanext.ldap.base_dn', 'n/a') +@pytest.mark.ckan_config('ckanext.ldap.search.filter', 'n/a') +@pytest.mark.ckan_config('ckanext.ldap.username', 'n/a') +@pytest.mark.ckan_config('ckanext.ldap.email', 'n/a') +@pytest.mark.usefixtures('with_plugins', 'with_request_context') +@pytest.mark.filterwarnings('ignore::sqlalchemy.exc.SADeprecationWarning') +class TestGetLoginAction: + + def test_with_came_from(self): + toolkit.c.login_handler = '/somewhere?came_from=beans' + action = get_login_action() + assert action == '/ldap_login_handler?came_from=beans' + + def test_without_came_from(self): + toolkit.c.login_handler = '/somewhere?lemons=xyz' + action = get_login_action() + assert action == '/ldap_login_handler' + + +def test_decode_str(): + assert decode_str(b'beans') == 'beans' + assert decode_str(b'\xf0\x9f\x92\xa9') == '💩' + assert decode_str('beans') == 'beans' From cc9df5c97a066f55e1157c8f156a65edeb2a0a8d Mon Sep 17 00:00:00 2001 From: Josh Humphries Date: Wed, 27 Jan 2021 00:39:27 +0000 Subject: [PATCH 7/9] Install coveralls in the install section --- .travis.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 325f714..84a2a24 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,9 +5,12 @@ language: python services: - docker +# we need coveralls and this also prevents travis from running pip install -r requirements.txt +install: pip install coveralls + script: - - pip install coveralls - docker-compose build - docker-compose run ckan after_success: coveralls + From 42165de88b26c11eee8ab4b849c39e587c30b40b Mon Sep 17 00:00:00 2001 From: Josh Humphries Date: Tue, 9 Feb 2021 14:45:28 +0000 Subject: [PATCH 8/9] Add an initdb command for the ldap tables Currently the tables are initialised in the plugin's configure hook which is like well not right. I've left that there though for the moment as it would cause downstream problems for new installers if I just removed it without providing new documentation about how to setup the plugin. --- README.md | 5 +++++ ckanext/ldap/cli.py | 13 +++++++++++++ ckanext/ldap/model/ldap_user.py | 2 -- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index c778c8c..10e9c37 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,11 @@ Name|Description|Options|Default ckan -c $CONFIG_FILE ldap setup-org ``` +2. `initdb`: ensure the tables needed by this extension exist. + ```bash + ckan -c $CONFIG_FILE ldap initdb + ``` + ## Templates This extension overrides `templates/user/login.html` and sets the form action to the LDAP login handler. diff --git a/ckanext/ldap/cli.py b/ckanext/ldap/cli.py index 332488a..0be24f5 100644 --- a/ckanext/ldap/cli.py +++ b/ckanext/ldap/cli.py @@ -1,6 +1,7 @@ import click from ckan.plugins import toolkit +from ckanext.ldap.model.ldap_user import ldap_user_table def get_commands(): @@ -15,6 +16,18 @@ def ldap(): pass +@ldap.command(name='initdb') +def init_db(): + ''' + Ensures the database tables we need exist in the database and creates them if they don't. + ''' + if not ldap_user_table.exists(): + ldap_user_table.create() + click.secho(f'Created {ldap_user_table.name} table', fg='green') + else: + click.secho(f'Table {ldap_user_table.name} already exists', fg='green') + + @ldap.command(name='setup-org') def setup_org(): ''' diff --git a/ckanext/ldap/model/ldap_user.py b/ckanext/ldap/model/ldap_user.py index 5d24d2d..0bd9d1f 100644 --- a/ckanext/ldap/model/ldap_user.py +++ b/ckanext/ldap/model/ldap_user.py @@ -9,8 +9,6 @@ from ckan import model from sqlalchemy import Column, ForeignKey, Table, orm, types -__all__ = ['LdapUser'] - ldap_user_table = Table('ldap_user', model.meta.metadata, Column('id', types.UnicodeText, primary_key=True, default=model.types.make_uuid), From 2de8cf8d80b70542523aaa67e731de4414c2da24 Mon Sep 17 00:00:00 2001 From: Josh Humphries Date: Thu, 18 Feb 2021 15:48:11 +0000 Subject: [PATCH 9/9] Convert exception to string instead of accessing non-existant attribute --- ckanext/ldap/lib/search.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ckanext/ldap/lib/search.py b/ckanext/ldap/lib/search.py index 680811e..9968048 100644 --- a/ckanext/ldap/lib/search.py +++ b/ckanext/ldap/lib/search.py @@ -96,9 +96,8 @@ def ldap_search(cnx, filter_str, attributes, non_unique='raise'): log.error('LDAP server is not reachable') return None except ldap.OPERATIONS_ERROR as e: - log.error( - 'LDAP query failed. Maybe you need auth credentials for performing searches? Error ' - 'returned by the server: ' + e.info) + log.error(f'LDAP query failed. Maybe you need auth credentials for performing searches? ' + f'Error returned by the server: {e}') return None except (ldap.NO_SUCH_OBJECT, ldap.REFERRAL) as e: log.error(