From 6bd79c09db5ce9207ccbb63c9c3f660ef5ed4c1d Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 1 Jun 2021 03:54:04 +0300 Subject: [PATCH 1/6] Implemented checks --- .idea/runConfigurations/check.xml | 23 +++++++++ windows_auth/checks.py | 86 +++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 .idea/runConfigurations/check.xml create mode 100644 windows_auth/checks.py diff --git a/.idea/runConfigurations/check.xml b/.idea/runConfigurations/check.xml new file mode 100644 index 0000000..27dc2e2 --- /dev/null +++ b/.idea/runConfigurations/check.xml @@ -0,0 +1,23 @@ + + + + + \ No newline at end of file diff --git a/windows_auth/checks.py b/windows_auth/checks.py new file mode 100644 index 0000000..ef78475 --- /dev/null +++ b/windows_auth/checks.py @@ -0,0 +1,86 @@ +from typing import List + +from django.conf import settings +from django.core.checks import register, CheckMessage, Error, Warning +from django.db import DatabaseError +from django.db.models import Count +from ldap3.core.exceptions import LDAPException + +from windows_auth import logger +from windows_auth.conf import WAUTH_IGNORE_SETTING_WARNINGS, WAUTH_DOMAINS +from windows_auth.ldap import get_ldap_manager +from windows_auth.settings import DEFAULT_DOMAIN_SETTING + + +@register() +def check_widows_auth_settings(app_configs, **kwargs): + messages: List[CheckMessage] = [] + + if not hasattr(settings, "WAUTH_DOMAINS"): + messages.append( + Error( + "Required setting \"WAUTH_DOMAINS\" is not configured.", + hint="Add \"WAUTH_DOMAINS\" to your settings file.", + id="wauth.E001" + ) + ) + + # TODO deprecate WAUTH_IGNORE_SETTING_WARNINGS + if not WAUTH_IGNORE_SETTING_WARNINGS and DEFAULT_DOMAIN_SETTING not in WAUTH_DOMAINS: + try: + from windows_auth.models import LDAPUser + missing_domains = LDAPUser.objects.exclude(domain__in=WAUTH_DOMAINS.keys()) + for result in missing_domains.values("domain").annotate(count=Count("pk")): + messages.append( + Warning( + f"Found missing domain settings for \"{result.get('domain')}\" " + f"({result.get('count')} LDAP users from this domain found in the database).", + hint=f"Add settings for \"{result.get('domain')}\" in WAUTH_DOMAINS.", + obj=result.get("domain"), + id="wauth.W002" + ) + ) + except DatabaseError as e: + logger.exception(f"Unable to load LDAPUser model: {e}") + # Table probably does not exist yet, migration is pending + messages.append( + Error( + "Unable to load LDAPUser model", + hint="Try running \"py manage.py migrate windows_auth\".", + id="wauth.E003", + ) + ) + + return messages + + +@register() +def check_ldap_domains(app_configs, **kwargs): + messages: List[CheckMessage] = [] + + for domain in WAUTH_DOMAINS.keys(): + # skip loading default domain + if domain == DEFAULT_DOMAIN_SETTING: + continue + + try: + # attempt to load LDAP connection + manager = get_ldap_manager(domain) + if not manager.connection.bound: + messages.append( + Warning( + f"Unable to created LDAP connection with domain {domain}", + obj=domain, + id="wauth.W004", + ), + ) + except LDAPException as e: + messages.append( + Error( + f"Error while creating LDAP connection with domain {domain}: {e}", + obj=domain, + id="wauth.E005", + ), + ) + + return messages From 7a82f48f013a46ada7804b302bdfec7b845b7ef9 Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 1 Jun 2021 04:02:52 +0300 Subject: [PATCH 2/6] Removed unnecessary checks in App.ready --- windows_auth/apps.py | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/windows_auth/apps.py b/windows_auth/apps.py index b4b5359..322e9ad 100644 --- a/windows_auth/apps.py +++ b/windows_auth/apps.py @@ -1,9 +1,7 @@ import atexit -from django.db import DatabaseError from ldap3.core.exceptions import LDAPException from django.apps import AppConfig -from django.db.models import Count from windows_auth import logger @@ -13,7 +11,8 @@ class WindowsAuthConfig(AppConfig): default_auto_field = 'django.db.models.AutoField' def ready(self): - from windows_auth.conf import WAUTH_IGNORE_SETTING_WARNINGS, WAUTH_PRELOAD_DOMAINS, WAUTH_DOMAINS + from windows_auth import checks + from windows_auth.conf import WAUTH_PRELOAD_DOMAINS, WAUTH_DOMAINS from windows_auth.settings import DEFAULT_DOMAIN_SETTING from windows_auth.ldap import get_ldap_manager, close_connections @@ -23,19 +22,6 @@ def ready(self): # You can avoid this behavior by using "runserver --noreload" parameter, # or modifying the WAUTH_PRELOAD_DOMAINS setting to False. - # check about users with domain missing from settings - if not WAUTH_IGNORE_SETTING_WARNINGS and DEFAULT_DOMAIN_SETTING not in WAUTH_DOMAINS: - try: - from windows_auth.models import LDAPUser - missing_domains = LDAPUser.objects.exclude(domain__in=WAUTH_DOMAINS.keys()) - if missing_domains.exists(): - for result in missing_domains.values("domain").annotate(count=Count("pk")): - logger.warning(f"Settings for domain \"{result.get('domain')}\" are missing from WAUTH_DOMAINS " - f"({result.get('count')} users found)") - except DatabaseError as e: - # Table probably does not exist yet, migration is pending - logger.warn(e) - # configure default preload domains preload_domains = WAUTH_PRELOAD_DOMAINS if preload_domains in (None, True): From c71c693f37f4bea13836dc01ca49bae51d9f6436 Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 1 Jun 2021 04:03:38 +0300 Subject: [PATCH 3/6] Implemented checks for simulate middleware settings --- windows_auth/checks.py | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/windows_auth/checks.py b/windows_auth/checks.py index ef78475..0f6fe60 100644 --- a/windows_auth/checks.py +++ b/windows_auth/checks.py @@ -16,6 +16,7 @@ def check_widows_auth_settings(app_configs, **kwargs): messages: List[CheckMessage] = [] + # Require WAUTH_DOMAINS if not hasattr(settings, "WAUTH_DOMAINS"): messages.append( Error( @@ -25,7 +26,19 @@ def check_widows_auth_settings(app_configs, **kwargs): ) ) + # Require WAUTH_SIMULATE_USER + if 'windows_auth.middleware.SimulateWindowsAuthMiddleware' in settings.MIDDLEWARE \ + and not hasattr(settings, "WAUTH_SIMULATE_USER"): + messages.append( + Error( + "You have \'windows_auth.middleware.SimulateWindowsAuthMiddleware\' in your MIDDLEWARE, " + "but you have not configured WAUTH_SIMULATE_USER.", + id="wauth.E006", + ) + ) + # TODO deprecate WAUTH_IGNORE_SETTING_WARNINGS + # Search missing domains if not WAUTH_IGNORE_SETTING_WARNINGS and DEFAULT_DOMAIN_SETTING not in WAUTH_DOMAINS: try: from windows_auth.models import LDAPUser @@ -69,7 +82,7 @@ def check_ldap_domains(app_configs, **kwargs): if not manager.connection.bound: messages.append( Warning( - f"Unable to created LDAP connection with domain {domain}", + f"Unable to create LDAP connection with domain {domain}.", obj=domain, id="wauth.W004", ), @@ -84,3 +97,19 @@ def check_ldap_domains(app_configs, **kwargs): ) return messages + + +@register(deploy=True) +def check_simulate_wauth(app_configs, **kwargs): + messages: List[CheckMessage] = [] + + if 'windows_auth.middleware.SimulateWindowsAuthMiddleware' in settings.MIDDLEWARE: + messages.append( + Warning( + "You should not have \'windows_auth.middleware.SimulateWindowsAuthMiddleware\' " + "in your middleware in production.", + id="wauth.W010" + ) + ) + + return messages From c33953349d40f9b5e80a7148d8e44017ebb32c52 Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 1 Jun 2021 04:06:50 +0300 Subject: [PATCH 4/6] Separated check for missing domains --- windows_auth/checks.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/windows_auth/checks.py b/windows_auth/checks.py index 0f6fe60..c5fd6ee 100644 --- a/windows_auth/checks.py +++ b/windows_auth/checks.py @@ -37,8 +37,14 @@ def check_widows_auth_settings(app_configs, **kwargs): ) ) + return messages + + +@register() +def check_missing_domains(app_configs, **kwargs): + messages: List[CheckMessage] = [] + # TODO deprecate WAUTH_IGNORE_SETTING_WARNINGS - # Search missing domains if not WAUTH_IGNORE_SETTING_WARNINGS and DEFAULT_DOMAIN_SETTING not in WAUTH_DOMAINS: try: from windows_auth.models import LDAPUser From e31ebb9d7479e1ad3a371d265b0e543a193f538b Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 1 Jun 2021 04:50:41 +0300 Subject: [PATCH 5/6] Added deployment checks --- windows_auth/checks.py | 99 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 97 insertions(+), 2 deletions(-) diff --git a/windows_auth/checks.py b/windows_auth/checks.py index c5fd6ee..649ee38 100644 --- a/windows_auth/checks.py +++ b/windows_auth/checks.py @@ -1,14 +1,17 @@ +import os from typing import List +import ldap3 from django.conf import settings -from django.core.checks import register, CheckMessage, Error, Warning +from django.core.checks import register, CheckMessage, Error, Warning, Info from django.db import DatabaseError from django.db.models import Count from ldap3.core.exceptions import LDAPException from windows_auth import logger -from windows_auth.conf import WAUTH_IGNORE_SETTING_WARNINGS, WAUTH_DOMAINS +from windows_auth.conf import WAUTH_IGNORE_SETTING_WARNINGS, WAUTH_DOMAINS, WAUTH_USE_CACHE, WAUTH_USE_SPN from windows_auth.ldap import get_ldap_manager +from windows_auth.models import LDAPUser from windows_auth.settings import DEFAULT_DOMAIN_SETTING @@ -119,3 +122,95 @@ def check_simulate_wauth(app_configs, **kwargs): ) return messages + + +@register(deploy=True) +def check_deployment_settings(app_configs, **kwargs): + messages: List[CheckMessage] = [] + + # check cache system enabled + if not WAUTH_USE_CACHE: + messages.append( + Info( + "Using the database to check LDAP user last sync time is slow. " + "If you can, you should use cache system instead.", + hint="Set WAUTH_USE_CACHE to True", + id="wauth.I011", + ) + ) + + # check LDAP settings + for domain in WAUTH_DOMAINS.keys(): + # skip loading default domain + if domain == DEFAULT_DOMAIN_SETTING: + continue + + manager = get_ldap_manager(domain) + + # check SSL + if not manager.settings.USE_SSL: + messages.append( + Warning( + "USE_SSL is not set to True. It is recommended to use only secure LDAP connection.", + obj=domain, + id="wauth.W012", + ) + ) + + # check authentication method + if not manager.settings.CONNECTION_OPTIONS.get("authentication") in (ldap3.SASL, ldap3.NTLM): + messages.append( + Warning( + "You should use a stronger authentication method for you LDAP connection.", + hint="Configure \"authentication\" to SASL or NTLM in you CONNECTION_OPTIONS.", + obj=domain, + id="wauth.W013" + ) + ) + + # check bind user + if WAUTH_USE_SPN: + username, user_domain = manager.settings.USERNAME.rsplit("@", 2) + else: + user_domain, username = manager.settings.USERNAME.split("\\", 2) + + if LDAPUser.objects.filter(user__username=username, domain=user_domain).exists(): + messages.append( + Warning( + "You should use a dedicated bind account with the minimum permissions needed", + hint="Your bind account has logged in to website", + obj=domain, + id="wauth.W014", + ) + ) + + # check dedicated writeable connections + if not manager.settings.READ_ONLY and LDAPUser.objects.filter(domain=domain).exists(): + messages.append( + Warning( + "You should use a dedicated connection for you write operations. " + "Using a different connection, and even another bind account, is considered best-practice", + obj=domain, + id="wauth.W015", + ) + ) + + return messages + + +@register(deploy=True) +def check_deployment_setup(app_configs, **kwargs): + messages: List[CheckMessage] = [] + + # check project drive is not OS drive + system_drive = os.environ.get('SYSTEMDRIVE') + current_drive, path = os.path.splitdrive(__file__) + if system_drive == current_drive: + messages.append( + Info( + "You should keep your site and project files on a separate disk from the OS.", + id="wauth.I020", + ) + ) + + return messages From 5795e8028748d9e645de63e68a3cc421200b70a6 Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 1 Jun 2021 05:29:54 +0300 Subject: [PATCH 6/6] Added checks docs --- docs/source/conf.py | 1 + docs/source/index.rst | 1 + docs/source/installation/publish.rst | 2 + docs/source/reference/checks.rst | 108 +++++++++++++++++++++++++++ windows_auth/checks.py | 6 +- 5 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 docs/source/reference/checks.rst diff --git a/docs/source/conf.py b/docs/source/conf.py index df5432a..20f3a33 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -29,6 +29,7 @@ # ones. extensions = [ "sphinx_rtd_theme", + "sphinx.ext.autosectionlabel", ] # Add any paths that contain templates here, relative to this directory. diff --git a/docs/source/index.rst b/docs/source/index.rst index 38a6b93..8618bd1 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -51,4 +51,5 @@ Features reference/signals reference/models reference/management_commands + reference/checks reference/change_log diff --git a/docs/source/installation/publish.rst b/docs/source/installation/publish.rst index 43b8bb4..374d9a8 100644 --- a/docs/source/installation/publish.rst +++ b/docs/source/installation/publish.rst @@ -5,6 +5,8 @@ Deployment Checklist Before deploying your site to production it is important to go over some best practices and make sure your site is the **most stable and secure**. Provided here are some best practices related to ``django-windowsauth``, IIS and LDAP. +Many checks can be performed automatically using ``py manage.py check --deploy``. + .. seealso:: Check out `Django's deployment checklist `_ too. diff --git a/docs/source/reference/checks.rst b/docs/source/reference/checks.rst new file mode 100644 index 0000000..c9d3b05 --- /dev/null +++ b/docs/source/reference/checks.rst @@ -0,0 +1,108 @@ + +Checks +====== + +.. glossary:: + + wauth.E001 + Type: ``Error`` + + Required setting ``WAUTH_DOMAINS`` is not configured. :ref:`WAUTH_DOMAINS (Required)` + +.. glossary:: + + wauth.W002 + Type: ``Warning`` + + Found missing domain settings for LDAP users. + +.. glossary:: + + wauth.E003 + Type: ``Error`` + + | Unable to load LDAPUser model. + | Try running ``py manage.py migrate windows_auth``. + +.. glossary:: + + wauth.W004 + Type: ``Warning`` + + | Unable to create LDAP connection with a configured domain. + | Check your settings and the server. + +.. glossary:: + + wauth.E005 + Type: ``Error`` + + | Error while creating LDAP connection with a configured domain + | Check your settings and the server. + +.. glossary:: + + wauth.E006 + Type: ``Error`` + + You have ``windows_auth.middleware.SimulateWindowsAuthMiddleware`` in your ``MIDDLEWARE``, but you have not configured ``WAUTH_SIMULATE_USER``. :ref:`WAUTH_SIMULATE_USER` + +.. glossary:: + + wauth.W010 + Type: ``Warning``, Deploy only + + You should not have ``windows_auth.middleware.SimulateWindowsAuthMiddleware`` in your middleware in production. + :ref:`SimulateWindowsAuthMiddleware` + +.. glossary:: + + wauth.I011 + Type: ``Info``, Deploy only + + Using the database to check LDAP user last sync time is slow. + If you can, you should use cache system instead. + :ref:`WAUTH_USE_CACHE` + +.. glossary:: + + wauth.W012 + Type: ``Warning``, Deploy only + + ``USE_SSL`` is not set to True. It is recommended to use only secure LDAP connection. + :ref:`USE_SSL` + +.. glossary:: + + wauth.W013 + Type: ``Warning``, Deploy only + + You should use a stronger authentication method for you LDAP connection. + Configure ``authentication`` to SASL or NTLM in you ``CONNECTION_OPTIONS``. + :doc:`../howto/securing_ldap` + +.. glossary:: + + wauth.W014 + Type: ``Warning``, Deploy only + + You should use a dedicated bind account with the minimum permissions needed. + Your bind account has logged in to website. + :ref:`USERNAME` + +.. glossary:: + + wauth.W015 + Type: ``Warning``, Deploy only + + You should use a dedicated connection for you write operations. + Using a different connection, and even another bind account, is considered best-practice. + :ref:`READ_ONLY` + +.. glossary:: + + wauth.I020 + Type: ``Info``, Deploy only + + You should keep your site and project files on a separate disk from the OS. + :ref:`READ_ONLY` diff --git a/windows_auth/checks.py b/windows_auth/checks.py index 649ee38..c59193d 100644 --- a/windows_auth/checks.py +++ b/windows_auth/checks.py @@ -177,8 +177,8 @@ def check_deployment_settings(app_configs, **kwargs): if LDAPUser.objects.filter(user__username=username, domain=user_domain).exists(): messages.append( Warning( - "You should use a dedicated bind account with the minimum permissions needed", - hint="Your bind account has logged in to website", + "You should use a dedicated bind account with the minimum permissions needed.", + hint="Your bind account has logged in to website.", obj=domain, id="wauth.W014", ) @@ -189,7 +189,7 @@ def check_deployment_settings(app_configs, **kwargs): messages.append( Warning( "You should use a dedicated connection for you write operations. " - "Using a different connection, and even another bind account, is considered best-practice", + "Using a different connection, and even another bind account, is considered best-practice.", obj=domain, id="wauth.W015", )