From b0d0f9949280c82c25a6eddd4350a161c74b4318 Mon Sep 17 00:00:00 2001 From: Alexander PapaTutuWawa Date: Fri, 6 Nov 2020 20:55:04 +0100 Subject: [PATCH] Refactor LDAP support and add LDAP caching --- django_etebase/app_settings.py | 28 ------- django_etebase/ldap.py | 45 ---------- django_etebase/token_auth/authentication.py | 8 -- django_etebase/utils.py | 6 -- etebase_server/settings.py | 13 ++- myauth/ldap.py | 93 +++++++++++++++++++++ 6 files changed, 98 insertions(+), 95 deletions(-) delete mode 100644 django_etebase/ldap.py create mode 100644 myauth/ldap.py diff --git a/django_etebase/app_settings.py b/django_etebase/app_settings.py index 3ff9c5e..3c580b2 100644 --- a/django_etebase/app_settings.py +++ b/django_etebase/app_settings.py @@ -78,34 +78,6 @@ class AppSettings: @cached_property def CHALLENGE_VALID_SECONDS(self): # pylint: disable=invalid-name return self._setting("CHALLENGE_VALID_SECONDS", 60) - - @cached_property - def USE_LDAP(self): # pylint: disable=invalid-name - return self._setting("USE_LDAP", False) - - @cached_property - def LDAP_FILTER(self): # pylint: disable=invalid-name - return self._setting("LDAP_FILTER", "") - - @cached_property - def LDAP_SEARCH_BASE(self): # pylint: disable=invalid-name - return self._setting("LDAP_SEARCH_BASE", "") - - @cached_property - def LDAP_SERVER(self): # pylint: disable=invalid-name - return self._setting("LDAP_SERVER", "") - - @cached_property - def LDAP_BIND_DN(self): # pylint: disable=invalid-name - return self._setting("LDAP_BIND_DN", "") - - @cached_property - def LDAP_BIND_PASSWORD(self): # pylint: disable=invalid-name - return self._setting("LDAP_BIND_PW", "") - - @cached_property - def LDAP_SEARCH_BASE(self): # pylint: disable=invalid-name - return self._setting("LDAP_SEARCH_BASE", "") app_settings = AppSettings('ETEBASE_') diff --git a/django_etebase/ldap.py b/django_etebase/ldap.py deleted file mode 100644 index 4a2a1ea..0000000 --- a/django_etebase/ldap.py +++ /dev/null @@ -1,45 +0,0 @@ -import logging - -from . import app_settings - -import ldap - -class LDAPConnection: - __instance__ = None - - @staticmethod - def get_instance(): - '''To get a Singleton''' - if not LDAPConnection.__instance__: - return LDAPConnection() - else: - return LDAPConnection.__instance__ - - def __init__(self): - # Pull settings from django.conf.settings - # NOTE: We assume that settings.USE_LDAP is True - self.__ldap_connection = ldap.initialize(app_settings.LDAP_SERVER) - try: - self.__ldap_connection.simple_bind_s(app_settings.LDAP_BIND_DN, - app_settings.LDAP_BIND_PASSWORD) - except ldap.LDAPError as err: - logging.error(f'LDAP Error occuring during initialization: {err.desc}') - - def has_user(self, username): - ''' - Since we don't care about the password and so authentication - another way, all we care about is whether the user exists. - ''' - filterstr = app_settings.LDAP_FILTER.replace('%s', username) - try: - result = self.__ldap_connection.search_s(app_settings.LDAP_SEARCH_BASE, - ldap.SCOPE_SUBTREE, - filterstr=filterstr) - except ldap.NO_RESULTS_RETURNED: - # We handle the specific error first and the the generic error, as - # we may expect ldap.NO_RESULTS_RETURNED, but not any other error - return False - except ldap.LDAPError as err: - logging.error(f'Error occured while performing an LDAP query: {err.desc}') - return False - return len(result) == 1 diff --git a/django_etebase/token_auth/authentication.py b/django_etebase/token_auth/authentication.py index 3e05951..8b1bca5 100644 --- a/django_etebase/token_auth/authentication.py +++ b/django_etebase/token_auth/authentication.py @@ -1,15 +1,11 @@ from django.utils import timezone from django.utils.translation import gettext_lazy as _ -from .. import app_settings from rest_framework import exceptions from rest_framework.authentication import TokenAuthentication as DRFTokenAuthentication from .models import AuthToken, get_default_expiry -if app_settings.USE_LDAP: - from ..ldap import LDAPConnection - AUTO_REFRESH = True MIN_REFRESH_INTERVAL = 60 @@ -26,10 +22,6 @@ class TokenAuthentication(DRFTokenAuthentication): except model.DoesNotExist: raise exceptions.AuthenticationFailed(msg) - if app_settings.USE_LDAP: - if not LDAPConnection.get_instance().has_user(token.user.username): - raise exceptions.AuthenticationFailed('User is not listed in the LDAP registry.') - if not token.user.is_active: raise exceptions.AuthenticationFailed(_('User inactive or deleted.')) diff --git a/django_etebase/utils.py b/django_etebase/utils.py index 7a352d1..1351f9b 100644 --- a/django_etebase/utils.py +++ b/django_etebase/utils.py @@ -2,7 +2,6 @@ from django.contrib.auth import get_user_model from django.core.exceptions import PermissionDenied from . import app_settings -from .ldap import LDAPConnection User = get_user_model() @@ -16,11 +15,6 @@ def get_user_queryset(queryset, view): def create_user(*args, **kwargs): - # Check if the LDAP query returns exactly one user - if app_settings.USE_LDAP: - if not LDAPConnection.get_instance().has_user(kwargs['username']): - raise PermissionDenied('User is not listed in the LDAP registry.') - custom_func = app_settings.CREATE_USER_FUNC if custom_func is not None: return custom_func(*args, **kwargs) diff --git a/etebase_server/settings.py b/etebase_server/settings.py index c70f910..cde7bcd 100644 --- a/etebase_server/settings.py +++ b/etebase_server/settings.py @@ -166,14 +166,11 @@ if any(os.path.isfile(x) for x in config_locations): if 'ldap' in config: ldap = config['ldap'] - ETEBASE_USE_LDAP = True - ETEBASE_LDAP_SERVER = ldap.get('server', '') - ETEBASE_LDAP_SEARCH_BASE = ldap.get('search_base', '') - ETEBASE_LDAP_FILTER = ldap.get('filter', '') - ETEBASE_LDAP_BIND_DN = ldap.get('bind_dn', '') - ETEBASE_LDAP_BIND_PW = ldap.get('bind_pw', '') - else: - ETEBASE_USE_LDAP = False + LDAP_SERVER = ldap.get('server', '') + LDAP_SEARCH_BASE = ldap.get('search_base', '') + LDAP_FILTER = ldap.get('filter', '') + LDAP_BIND_DN = ldap.get('bind_dn', '') + LDAP_BIND_PW = ldap.get('bind_pw', '') ETEBASE_API_PERMISSIONS = ('rest_framework.permissions.IsAuthenticated', ) ETEBASE_API_AUTHENTICATORS = ('django_etebase.token_auth.authentication.TokenAuthentication', diff --git a/myauth/ldap.py b/myauth/ldap.py new file mode 100644 index 0000000..9cd8e55 --- /dev/null +++ b/myauth/ldap.py @@ -0,0 +1,93 @@ +import logging + +from django.utils import timezone +from django.conf import settings +from django.core.exceptions import PermissionDenied +from rest_framework.permissions import BasePermission + +import ldap + + +def ldap_setting(name, default): + '''Wrapper around django.conf.settings''' + return getattr(settings, f'LDAP_{name}', default) + +class LDAPConnection: + __instance__ = None + __user_cache = {} # Username -> Valid until + + @staticmethod + def get_instance(): + '''To get a Singleton''' + if not LDAPConnection.__instance__: + return LDAPConnection() + else: + return LDAPConnection.__instance__ + + def __init__(self): + # Cache some settings + self.__LDAP_FILTER = ldap_setting('FILTER', '') + self.__LDAP_SEARCH_BASE = ldap_setting('SEARCH_BASE', '') + + self.__ldap_connection = ldap.initialize(ldap_setting('SERVER', '')) + try: + self.__ldap_connection.simple_bind_s(ldap_setting('BIND_DN', ''), + ldap_setting('BIND_PW', '')) + except ldap.LDAPError as err: + logging.error(f'LDAP Error occuring during bind: {err.desc}') + + def __is_cache_valid(self, username): + '''Returns True if the cache entry is still valid. Returns False otherwise.''' + if username in self.__user_cache: + if timezone.now() <= self.__user_cache[username]: + # Cache entry is still valid + return True + return False + + def __remove_cache(self, username): + del self.__user_cache[username] + + def has_user(self, username): + ''' + Since we don't care about the password and so authentication + another way, all we care about is whether the user exists. + ''' + if self.__is_cache_valid(username): + return True + if username in self.__user_cache: + self.__remove_cache(username) + + filterstr = self.__LDAP_FILTER.replace('%s', username) + try: + result = self.__ldap_connection.search_s(self.__LDAP_SEARCH_BASE, + ldap.SCOPE_SUBTREE, + filterstr=filterstr) + except ldap.NO_RESULTS_RETURNED: + # We handle the specific error first and the the generic error, as + # we may expect ldap.NO_RESULTS_RETURNED, but not any other error + return False + except ldap.LDAPError as err: + logging.error(f'Error occured while performing an LDAP query: {err.desc}') + return False + + if len(result) == 1: + self.__user_cache[username] = timezone.now() + timezone.timedelta(hours=1) + return True + return False + +class LDAPUserExists(BasePermission): + ''' + A permission check which first checks with the LDAP directory if the user + exists. + ''' + def has_permission(self, request, view): + return LDAPConnection.get_instance().has_user(request.user.username): + +def create_user(*args, **kwargs): + ''' + A create_user function which first checks if the user already exists in the + configured LDAP directory. + ''' + if not LDAPConnection.get_instance().has_user(kwargs['username']): + raise PermissionDenied('User not in the LDAP directory.') + return User.objects.create_user(*args, **kwargs)