Merge 4c8f4d4 into 23b1852

Kinto · Nov 23, 2016 · c15a5fd · c15a5fd
2 parents 23b1852 + 4c8f4d4
commit c15a5fd
Show file tree

Hide file tree

Showing 9 changed files with 111 additions and 40 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -7,7 +7,7 @@ This document describes changes between each past release.
 0.3.0 (unreleased)
 ------------------
 
-- Nothing changed yet.
+- Support login from multiple DN from the same LDAP server (#16)
 
 
 0.2.1 (2016-11-03)

diff --git a/README.rst b/README.rst
@@ -61,15 +61,18 @@ Configuration
 
     multiauth.policy.ldap.use = kinto_ldap.authentication.LDAPBasicAuthAuthenticationPolicy
 
-    # kinto.ldap.cache_ttl_seconds = 30
-    # kinto.ldap.endpoint = ldap://ldap.prod.mozaws.net
-    # kinto.ldap.fqn = "uid={uid},ou=users,dc=mozilla"
+    kinto.ldap.cache_ttl_seconds = 30
+    kinto.ldap.endpoint = ldap://ldap.prod.mozaws.net
+    # kinto.ldap.bind_dn = uid=read_user,ou=logins,dc=mozilla
+    # kinto.ldap.bind_password = user_password
 
 If necessary, override default values for authentication policy:
 
 ::
 
     # multiauth.policy.ldap.realm = Realm
+    # kinto.ldap.base_dn = dc=mozilla
+    # kinto.ldap.filters = (mail={mail})
     # kinto.ldap.pool_size = 10
     # kinto.ldap.pool_retry_max = 3
     # kinto.ldap.pool_retry_delay = .1

diff --git a/config/kinto.ini b/config/kinto.ini
@@ -27,6 +27,7 @@ kinto.permission_backend = kinto.core.permission.memory
 #
 kinto.userid_hmac_secret = 2a2c8d277cf3cf7a9c45e73170763d4cb44c9c04f112d3e5999432a08d1a8840
 multiauth.policies = ldap
+multiauth.policy.ldap.use = kinto_ldap.authentication.LDAPBasicAuthAuthenticationPolicy
 # multiauth.policies = fxa basicauth
 
 # kinto.readonly = false
@@ -43,8 +44,6 @@ multiauth.policies = ldap
 kinto.includes = kinto.plugins.default_bucket kinto_ldap
 
 
-multiauth.policy.ldap.use = kinto_ldap.authentication.LDAPBasicAuthAuthenticationPolicy
-
 #
 # Firefox Accounts configuration.
 #   These are working FxA credentials for localhost:8888

diff --git a/kinto_ldap/__init__.py b/kinto_ldap/__init__.py
@@ -14,11 +14,12 @@
                                   'LDAPBasicAuthAuthenticationPolicy'),
     'ldap.cache_ttl_seconds': 30,
     'ldap.endpoint': 'ldap://ldap.db.scl3.mozilla.com',
+    'ldap.base_dn': 'dc=mozilla',
+    'ldap.filters': '(mail={mail})',
     'ldap.pool_size': 10,
     'ldap.pool_retry_max': 3,
     'ldap.pool_retry_delay': .1,
     'ldap.pool_timeout': 30,
-    'ldap.fqn': 'mail={mail},o=com,dc=mozilla',
 }
 
 
@@ -38,6 +39,16 @@ def includeme(config):
         description="Basic Auth user are validated against an LDAP server.",
         url="https://github.com/mozilla-services/kinto-ldap")
 
+    try:
+        settings['ldap.filters'].format(mail='test')
+    except KeyError:
+        msg = "ldap.filters should take a 'mail' argument only, got: %r" % settings['ldap.filters']
+        raise ConfigurationError(msg)
+    else:
+        if settings['ldap.filters'].format(mail='test') == settings['ldap.filters']:
+            msg = "ldap.filters should take a 'mail' argument, got: %r" % settings['ldap.filters']
+            raise ConfigurationError(msg)
+
     # Register heartbeat to ping the LDAP server.
     config.registry.heartbeats['ldap'] = ldap_ping
 

diff --git a/kinto_ldap/authentication.py b/kinto_ldap/authentication.py
@@ -1,7 +1,7 @@
 import logging
 
 from kinto.core import utils
-from ldap import INVALID_CREDENTIALS
+from ldap import INVALID_CREDENTIALS, SCOPE_SUBTREE
 from ldappool import BackendError
 from pyramid.authentication import BasicAuthAuthenticationPolicy
 
@@ -20,18 +20,45 @@ def user_checker(username, password, request):
     settings = request.registry.settings
     cache_ttl = settings['ldap.cache_ttl_seconds']
     hmac_secret = settings['userid_hmac_secret']
-    cache_key = utils.hmac_digest(hmac_secret, '%s:%s' % (username, password))
+    cache_key = utils.hmac_digest(hmac_secret, '{}:{}'.format(username, password))
 
     cache = request.registry.cache
     cache_result = cache.get(cache_key)
 
-    ldap_fqn = settings['ldap.fqn']
+    bind_dn = settings.get('ldap.bind_dn')
+    bind_password = settings.get('ldap.bind_password')
 
     if cache_result is None:
         cm = request.registry.ldap_cm
 
+        # 0. Generate a search filter by combining the attribute and
+        # filter provided in the ldap.fqn_filters directive with the
+        # username passed by the HTTP client.
+        base_dn = settings['ldap.base_dn']
+        filters = settings['ldap.filters'].format(mail=username)
+
+        # 1. Search for the user
+        try:
+            with cm.connection(bind_dn, bind_password) as conn:
+                # import pdb; pdb.set_trace()
+                results = conn.search_s(base_dn, SCOPE_SUBTREE, filters)
+        except BackendError:
+            logger.exception("LDAP error")
+            return None
+
+        if len(results) != 1:
+            # If the search does not return exactly one entry, deny or decline access.
+            return None
+
+        dn, entry = results[0]
+        user_dn = str(dn)
+
+        # 2. Fetch the distinguished name of the entry retrieved from
+        # the search and attempt to bind to the LDAP server using that
+        # DN and the password passed by the HTTP client. If the bind
+        # is unsuccessful, deny or decline access.
         try:
-            with cm.connection(ldap_fqn.format(mail=username), password):
+            with cm.connection(user_dn, password):
                 cache.set(cache_key, "1", ttl=cache_ttl)
                 return []
         except BackendError:
@@ -61,12 +88,17 @@ def unauthenticated_userid(self, request):
 
 def ldap_ping(request):
     """Verify if the LDAP server is ready."""
+    settings = request.registry.settings
+    bind_dn = settings.get('ldap.bind_dn')
+    bind_password = settings.get('ldap.bind_password')
+    base_dn = settings['ldap.base_dn']
     cm = request.registry.ldap_cm
     try:
-        with cm.connection('mail=mail@test,o=com,dc=test', 'password'):
+        with cm.connection(bind_dn, bind_password) as conn:
+            # Perform a dumb query
+            filters = settings['ldap.filters'].format(mail="demo")
+            conn.search_s(base_dn, SCOPE_SUBTREE, filters)
             ldap = True
-    except INVALID_CREDENTIALS:
-        ldap = True
     except Exception:
         logger.exception("Heartbeat Failure")
         ldap = False

diff --git a/kinto_ldap/tests/test_authentication.py b/kinto_ldap/tests/test_authentication.py
@@ -20,6 +20,9 @@ def setUp(self):
         self.request = DummyRequest()
         self.request.registry.cache = self.backend
         self.request.registry.ldap_cm = mock.MagicMock()
+        self.conn = mock.MagicMock()
+        self.conn.search_s.return_value = [("dn", {})]
+        self.request.registry.ldap_cm.connection.return_value.__enter__.return_value = self.conn
         settings = DEFAULT_SETTINGS.copy()
         settings['userid_hmac_secret'] = 'abcdef'
         settings['ldap.cache_ttl_seconds'] = 0.01
@@ -37,6 +40,13 @@ def test_returns_none_if_server_is_unreachable(self):
         user_id = self.policy.authenticated_userid(self.request)
         self.assertIsNone(user_id)
 
+    def test_returns_none_if_server_is_unreachable_the_second_time(self):
+        error = BackendError("unreachable", backend=None)
+        self.request.registry.ldap_cm.connection \
+            .return_value.__enter__.side_effect = [self.conn, error]
+        user_id = self.policy.authenticated_userid(self.request)
+        self.assertIsNone(user_id)
+
     def test_returns_none_if_authorization_header_is_missing(self):
         self.request.headers.pop('Authorization')
         user_id = self.policy.authenticated_userid(self.request)
@@ -53,41 +63,43 @@ def test_returns_none_if_token_is_unknown(self):
         self.assertIsNone(user_id)
 
     def test_returns_ldap_userid(self):
-        mocked = mock.MagicMock()
-        self.request.registry.ldap_cm.connection \
-            .return_value.__enter__.return_value = mocked
-
         user_id = self.policy.authenticated_userid(self.request)
         self.assertEqual("username", user_id)
 
     def test_auth_verification_uses_cache(self):
-        mocked = mock.MagicMock()
-        self.request.registry.ldap_cm.connection \
-            .return_value.__enter__.return_value = mocked
-
         self.policy.authenticated_userid(self.request)
         self.policy.authenticated_userid(self.request)
         self.assertEqual(
-            1, self.request.registry.ldap_cm.connection.call_count)
+            2, self.request.registry.ldap_cm.connection.call_count)
 
     def test_auth_verification_cache_has_ttl(self):
-        mocked = mock.MagicMock()
-        self.request.registry.ldap_cm.connection \
-            .return_value.__enter__.return_value = mocked
-
         self.policy.authenticated_userid(self.request)
+        self.assertEqual(
+            2, self.request.registry.ldap_cm.connection.call_count)
         time.sleep(0.02)
         self.policy.authenticated_userid(self.request)
         self.assertEqual(
-            2, self.request.registry.ldap_cm.connection.call_count)
+            4, self.request.registry.ldap_cm.connection.call_count)
 
     def test_returns_none_if_user_password_mismatch(self):
         self.request.registry.ldap_cm.connection \
-            .return_value.__enter__.side_effect = ldap.INVALID_CREDENTIALS()
+            .return_value.__enter__.side_effect = [self.conn, ldap.INVALID_CREDENTIALS()]
         self.assertIsNone(self.policy.authenticated_userid(self.request))
 
     def test_forget_uses_realm(self):
         policy = authentication.LDAPBasicAuthAuthenticationPolicy(realm='Who')
         headers = policy.forget(self.request)
-        self.assertEqual(headers[0],
-                         ('WWW-Authenticate', 'Basic realm="Who"'))
+        self.assertEqual(headers[0], ('WWW-Authenticate', 'Basic realm="Who"'))
+
+    def test_returns_none_if_multiple_ldap_search_results_matches(self):
+        self.conn.search_s.return_value = [("dn", {}), ("dn2", {})]
+        user_id = self.policy.authenticated_userid(self.request)
+        self.assertIsNone(user_id)
+
+    def test_bind_dn_and_bind_password_settings_are_used_if_specified(self):
+        self.request.registry.settings['ldap.bind_dn'] = "bind_dn"
+        self.request.registry.settings['ldap.bind_password'] = "bind_password"
+        self.policy.authenticated_userid(self.request)
+        self.assertEqual(2, self.request.registry.ldap_cm.connection.call_count)
+        self.request.registry.ldap_cm.connection.assert_any_call("bind_dn", "bind_password")
+        self.request.registry.ldap_cm.connection.assert_any_call("dn", "password")
diff --git a/kinto_ldap/tests/test_includeme.py b/kinto_ldap/tests/test_includeme.py
@@ -39,3 +39,23 @@ def test_connection_manager_is_instantiated_with_settings(self):
                                       size=10,
                                       timeout=30,
                                       uri='ldap://ldap.db.scl3.mozilla.com')
+
+    def test_include_fails_if_ldap_filters_contains_multiple_keys(self):
+        config = testing.setUp()
+        settings = config.get_settings()
+        settings['ldap.filters'] = '{uid}{mail}'
+        kinto.core.initialize(config, '0.0.1')
+        with self.assertRaises(ConfigurationError) as e:
+            config.include(includeme)
+        message = "ldap.filters should take a 'mail' argument only, got: '{uid}{mail}'"
+        self.assertEqual(str(e.exception), message)
+
+    def test_include_fails_if_ldap_filters_is_missing_the_mail_key(self):
+        config = testing.setUp()
+        settings = config.get_settings()
+        settings['ldap.filters'] = 'toto'
+        kinto.core.initialize(config, '0.0.1')
+        with self.assertRaises(ConfigurationError) as e:
+            config.include(includeme)
+        message = "ldap.filters should take a 'mail' argument, got: 'toto'"
+        self.assertEqual(str(e.exception), message)
diff --git a/kinto_ldap/tests/test_views.py b/kinto_ldap/tests/test_views.py
@@ -6,7 +6,6 @@
 import webtest
 from kinto.core.utils import random_bytes_hex
 from pyramid.config import Configurator
-from ldap import INVALID_CREDENTIALS
 
 from kinto_ldap import __version__ as ldap_version
 
@@ -99,11 +98,3 @@ def test_heartbeat_returns_true_if_test_credentials_are_valid(self):
         resp = self.app.get('/__heartbeat__')
         heartbeat = resp.json['ldap']
         self.assertTrue(heartbeat)
-
-    def test_heartbeat_returns_true_if_credentials_are_invalid(self):
-        self.app.app.registry.ldap_cm = mock.MagicMock()
-        self.app.app.registry.ldap_cm.connection \
-            .return_value.__enter__.side_effect = INVALID_CREDENTIALS
-        resp = self.app.get('/__heartbeat__')
-        heartbeat = resp.json['ldap']
-        self.assertTrue(heartbeat)
diff --git a/tox.ini b/tox.ini
@@ -33,3 +33,6 @@ deps =
 commands = flake8 kinto_ldap
 deps =
     flake8
+
+[flake8]
+max-line-length = 99