Merge pull request #5217 from ryanpetrello/fix-5208

enforce a sane default OPT_NETWORK_TIMEOUT for LDAP connections

Makefile

@@ -473,7 +473,7 @@ pylint: reports
 
 check: flake8 pep8 # pyflakes pylint
 
-TEST_DIRS ?= awx/main/tests awx/conf/tests
+TEST_DIRS ?= awx/main/tests awx/conf/tests awx/sso/tests
 # Run all API unit tests.
 test:
 	@if [ "$(VENV_BASE)" ]; then \
@@ -485,7 +485,7 @@ test_unit:
 	@if [ "$(VENV_BASE)" ]; then \
 		. $(VENV_BASE)/tower/bin/activate; \
 	fi; \
-	py.test awx/main/tests/unit awx/conf/tests/unit
+	py.test awx/main/tests/unit awx/conf/tests/unit awx/sso/tests/unit
 
 # Run all API unit tests with coverage enabled.
 test_coverage:

awx/settings/defaults.py

@@ -304,6 +304,7 @@ AUTH_LDAP_SERVER_URI = None
 # Note: This setting may be overridden by database settings.
 AUTH_LDAP_CONNECTION_OPTIONS = {
     ldap.OPT_REFERRALS: 0,
+    ldap.OPT_NETWORK_TIMEOUT: 30
 }
 
 # Radius server settings (default to empty string to skip using Radius auth).

awx/sso/backends.py

@@ -5,6 +5,8 @@
 import logging
 import uuid
 
+import ldap
+
 # Django
 from django.dispatch import receiver
 from django.contrib.auth.models import User
@@ -38,6 +40,16 @@ class LDAPSettings(BaseLDAPSettings):
         'TEAM_MAP': {},
     }.items())
 
+    def __init__(self, prefix='AUTH_LDAP_', defaults={}):
+        super(LDAPSettings, self).__init__(prefix, defaults)
+
+        # If a DB-backed setting is specified that wipes out the
+        # OPT_NETWORK_TIMEOUT, fall back to a sane default
+        if ldap.OPT_NETWORK_TIMEOUT not in getattr(self, 'CONNECTION_OPTIONS', {}):
+            options = getattr(self, 'CONNECTION_OPTIONS', {})
+            options[ldap.OPT_NETWORK_TIMEOUT] = 30
+            self.CONNECTION_OPTIONS = options
+
 
 class LDAPBackend(BaseLDAPBackend):
     '''

awx/sso/conf.py

@@ -228,7 +228,7 @@ register(
 register(
     'AUTH_LDAP_CONNECTION_OPTIONS',
     field_class=fields.LDAPConnectionOptionsField,
-    default={'OPT_REFERRALS': 0},
+    default={'OPT_REFERRALS': 0, 'OPT_NETWORK_TIMEOUT': 30},
     label=_('LDAP Connection Options'),
     help_text=_('Additional options to set for the LDAP connection.  LDAP '
                 'referrals are disabled by default (to prevent certain LDAP '
@@ -240,6 +240,7 @@ register(
     category_slug='ldap',
     placeholder=collections.OrderedDict([
         ('OPT_REFERRALS', 0),
+        ('OPT_NETWORK_TIMEOUT', 30)
     ]),
     feature_required='ldap',
 )

awx/sso/tests/functional/test_ldap.py

@@ -0,0 +1,24 @@
+from django.test.utils import override_settings
+import ldap
+import pytest
+
+from awx.sso.backends import LDAPSettings
+
+
+@override_settings(AUTH_LDAP_CONNECTION_OPTIONS = {ldap.OPT_NETWORK_TIMEOUT: 60})
+@pytest.mark.django_db
+def test_ldap_with_custom_timeout():
+    settings = LDAPSettings()
+    assert settings.CONNECTION_OPTIONS == {
+        ldap.OPT_NETWORK_TIMEOUT: 60
+    }
+
+
+@override_settings(AUTH_LDAP_CONNECTION_OPTIONS = {ldap.OPT_REFERRALS: 0})
+@pytest.mark.django_db
+def test_ldap_with_missing_timeout():
+    settings = LDAPSettings()
+    assert settings.CONNECTION_OPTIONS == {
+        ldap.OPT_REFERRALS: 0,
+        ldap.OPT_NETWORK_TIMEOUT: 30
+    }

awx/sso/tests/unit/test_ldap.py

@@ -0,0 +1,21 @@
+import ldap
+
+from awx.sso.backends import LDAPSettings
+
+
+def test_ldap_default_settings(mocker):
+    from_db = mocker.Mock(**{'order_by.return_value': []})
+    with mocker.patch('awx.conf.models.Setting.objects.filter', return_value=from_db):
+        settings = LDAPSettings()
+        assert settings.ORGANIZATION_MAP == {}
+        assert settings.TEAM_MAP == {}
+
+
+def test_ldap_default_network_timeout(mocker):
+    from_db = mocker.Mock(**{'order_by.return_value': []})
+    with mocker.patch('awx.conf.models.Setting.objects.filter', return_value=from_db):
+        settings = LDAPSettings()
+        assert settings.CONNECTION_OPTIONS == {
+            ldap.OPT_REFERRALS: 0,
+            ldap.OPT_NETWORK_TIMEOUT: 30
+        }

tools/docker-compose/unit-tests/docker-compose.yml

@@ -8,7 +8,7 @@ services:
     image: gcr.io/ansible-tower-engineering/unit-test-runner:latest
     environment:
       SWIG_FEATURES: "-cpperraswarn -includeall -I/usr/include/openssl"
-      TEST_DIRS: awx/main/tests/functional awx/main/tests/unit awx/conf/tests
+      TEST_DIRS: awx/main/tests/functional awx/main/tests/unit awx/conf/tests awx/sso/tests
     command: ["make test"]
     volumes:
       - ../../../:/tower_devel

tox.ini

@@ -48,7 +48,7 @@ commands =
     python setup.py develop
     # coverage run --help
     # coverage run -p --source awx/main/tests -m pytest {posargs}
-    py.test awx/main/tests awx/conf/tests {posargs:-k 'not old'}
+    py.test awx/main/tests awx/conf/tests awx/sso/tests {posargs:-k 'not old'}
 
 [testenv:ui]
 deps =