From adaeeba5d0cd98fe4e024ead99b1ed0af8baa43b Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 7 Jul 2016 14:26:14 -0400 Subject: [PATCH] Allow for multiple user RDNs in LDAP Fixes #1600 --- data/users/__init__.py | 3 +- data/users/externalldap.py | 59 +++++++++++-------- static/css/core-ui.css | 4 ++ .../directives/config/config-setup-tool.html | 27 ++++++--- .../config/config-string-list-field.html | 6 ++ static/js/core-config-setup.js | 48 ++++++++++++--- test/test_ldap.py | 25 +++++++- 7 files changed, 129 insertions(+), 43 deletions(-) create mode 100644 static/directives/config/config-string-list-field.html diff --git a/data/users/__init__.py b/data/users/__init__.py index e093f6993..70f736c1f 100644 --- a/data/users/__init__.py +++ b/data/users/__init__.py @@ -44,10 +44,11 @@ def get_users_handler(config, config_provider, override_config_dir): user_rdn = config.get('LDAP_USER_RDN', []) uid_attr = config.get('LDAP_UID_ATTR', 'uid') email_attr = config.get('LDAP_EMAIL_ATTR', 'mail') + secondary_user_rds = config.get('LDAP_SECONDARY_USER_RDNS', []) allow_tls_fallback = config.get('LDAP_ALLOW_INSECURE_FALLBACK', False) return LDAPUsers(ldap_uri, base_dn, admin_dn, admin_passwd, user_rdn, uid_attr, email_attr, - allow_tls_fallback) + allow_tls_fallback, secondary_user_rds=secondary_user_rds) if authentication_type == 'JWT': verify_url = config.get('JWT_VERIFY_ENDPOINT') diff --git a/data/users/externalldap.py b/data/users/externalldap.py index 561891438..ad885b8eb 100644 --- a/data/users/externalldap.py +++ b/data/users/externalldap.py @@ -48,17 +48,21 @@ class LDAPUsers(FederatedUsers): _LDAPResult = namedtuple('LDAPResult', ['dn', 'attrs']) def __init__(self, ldap_uri, base_dn, admin_dn, admin_passwd, user_rdn, uid_attr, email_attr, - allow_tls_fallback=False): + allow_tls_fallback=False, secondary_user_rdns=None): super(LDAPUsers, self).__init__('ldap') self._ldap = LDAPConnectionBuilder(ldap_uri, admin_dn, admin_passwd, allow_tls_fallback) self._ldap_uri = ldap_uri - self._base_dn = base_dn - self._user_rdn = user_rdn self._uid_attr = uid_attr self._email_attr = email_attr self._allow_tls_fallback = allow_tls_fallback + # Note: user_rdn is a list of RDN pieces (for historical reasons), and secondary_user_rds + # is a list of RDN strings. + relative_user_dns = [','.join(user_rdn)] + (secondary_user_rdns or []) + self._user_dns = [','.join(relative_dn.split(',') + base_dn) + for relative_dn in relative_user_dns] + def _get_ldap_referral_dn(self, referral_exception): logger.debug('Got referral: %s', referral_exception.args[0]) if not referral_exception.args[0] or not referral_exception.args[0].get('info'): @@ -78,6 +82,28 @@ class LDAPUsers(FederatedUsers): referral_dn = referral_uri[len('ldap:///'):] return referral_dn + def _ldap_user_search_with_rdn(self, conn, username_or_email, user_search_dn): + query = u'(|({0}={2})({1}={2}))'.format(self._uid_attr, self._email_attr, + username_or_email) + logger.debug('Conducting user search: %s under %s', query, user_search_dn) + try: + return (conn.search_s(user_search_dn, ldap.SCOPE_SUBTREE, query.encode('utf-8')), None) + except ldap.REFERRAL as re: + referral_dn = self._get_ldap_referral_dn(re) + if not referral_dn: + return (None, 'Failed to follow referral when looking up username') + + try: + subquery = u'(%s=%s)' % (self._uid_attr, username_or_email) + return (conn.search_s(referral_dn, ldap.SCOPE_BASE, subquery), None) + except ldap.LDAPError: + logger.exception('LDAP referral search exception') + return (None, 'Username not found') + + except ldap.LDAPError: + logger.exception('LDAP search exception') + return (None, 'Username not found') + def _ldap_user_search(self, username_or_email): # Verify the admin connection works first. We do this here to avoid wrapping # the entire block in the INVALID CREDENTIALS check. @@ -89,31 +115,16 @@ class LDAPUsers(FederatedUsers): with self._ldap.get_connection() as conn: logger.debug('Incoming username or email param: %s', username_or_email.__repr__()) - user_search_dn = ','.join(self._user_rdn + self._base_dn) - query = u'(|({0}={2})({1}={2}))'.format(self._uid_attr, self._email_attr, - username_or_email) - logger.debug('Conducting user search: %s under %s', query, user_search_dn) - try: - pairs = conn.search_s(user_search_dn, ldap.SCOPE_SUBTREE, query.encode('utf-8')) - except ldap.REFERRAL as re: - referral_dn = self._get_ldap_referral_dn(re) - if not referral_dn: - return (None, 'Failed to follow referral when looking up username') + for user_search_dn in self._user_dns: + (pairs, err_msg) = self._ldap_user_search_with_rdn(conn, username_or_email, user_search_dn) + if pairs is not None and len(pairs) > 0: + break - try: - subquery = u'(%s=%s)' % (self._uid_attr, username_or_email) - pairs = conn.search_s(referral_dn, ldap.SCOPE_BASE, subquery) - except ldap.LDAPError: - logger.exception('LDAP referral search exception') - return (None, 'Username not found') - - except ldap.LDAPError: - logger.exception('LDAP search exception') - return (None, 'Username not found') + if err_msg is not None: + return (None, err_msg) logger.debug('Found matching pairs: %s', pairs) - results = [LDAPUsers._LDAPResult(*pair) for pair in pairs] # Filter out pairs without DNs. Some LDAP impls will return such diff --git a/static/css/core-ui.css b/static/css/core-ui.css index 667fcc781..8a6f1ae08 100644 --- a/static/css/core-ui.css +++ b/static/css/core-ui.css @@ -467,6 +467,10 @@ a:focus { width: 400px; } +.config-setup-tool-element .config-table > tbody > tr > td .config-string-list-field-element { + width: 400px; +} + .config-map-field-element table { margin-bottom: 10px; } diff --git a/static/directives/config/config-setup-tool.html b/static/directives/config/config-setup-tool.html index 8e07f1521..9afaf4a23 100644 --- a/static/directives/config/config-setup-tool.html +++ b/static/directives/config/config-setup-tool.html @@ -595,23 +595,36 @@ Base DN: - +
- A list of Distinguished Name pieces which forms the base path for - looking up all LDAP records. + A Distinguished Name path which forms the base path for looking up all LDAP records.
- Example: [dc=my,dc=domain,dc=com] + Example: dc=my,dc=domain,dc=com
User Relative DN: - +
- A list of Distinguished Name pieces which forms the base path for - looking up all user LDAP records, relative to the Base DN defined above. + A Distinguished Name path which forms the base path for looking up all user LDAP records, + relative to the Base DN defined above. +
+
+ Example: ou=employees +
+ + + + Secondary User Relative DNs: + + +
+ A list of Distinguished Name path(s) which forms the secondary base path(s) for + looking up all user LDAP records, relative to the Base DN defined above. These path(s) + will be tried if the user is not found via the primary relative DN.
Example: [ou=employees] diff --git a/static/directives/config/config-string-list-field.html b/static/directives/config/config-string-list-field.html new file mode 100644 index 000000000..de29dfb91 --- /dev/null +++ b/static/directives/config/config-string-list-field.html @@ -0,0 +1,6 @@ +
+
+ +
+
diff --git a/static/js/core-config-setup.js b/static/js/core-config-setup.js index b2065b8c4..1d41ac031 100644 --- a/static/js/core-config-setup.js +++ b/static/js/core-config-setup.js @@ -1162,16 +1162,16 @@ angular.module("core-config-setup", ['angularFileUpload']) $scope.patternMap = {}; $scope.getRegexp = function(pattern) { - if (!pattern) { - pattern = '.*'; - } + if (!pattern) { + pattern = '.*'; + } - if ($scope.patternMap[pattern]) { - return $scope.patternMap[pattern]; - } + if ($scope.patternMap[pattern]) { + return $scope.patternMap[pattern]; + } - return $scope.patternMap[pattern] = new RegExp(pattern); - }; + return $scope.patternMap[pattern] = new RegExp(pattern); + }; $scope.$watch('binding', function(binding) { if (firstSet && !binding && $scope.defaultValue) { @@ -1184,4 +1184,36 @@ angular.module("core-config-setup", ['angularFileUpload']) } }; return directiveDefinitionObject; + }) + + .directive('configStringListField', function () { + var directiveDefinitionObject = { + priority: 0, + templateUrl: '/static/directives/config/config-string-list-field.html', + replace: false, + transclude: false, + restrict: 'C', + scope: { + 'binding': '=binding', + 'itemTitle': '@itemTitle', + 'itemDelimiter': '@itemDelimiter', + 'placeholder': '@placeholder', + 'isOptional': '=isOptional' + }, + controller: function($scope, $element) { + $scope.$watch('internalBinding', function(value) { + if (value) { + $scope.binding = value.split($scope.itemDelimiter); + } + }); + + $scope.$watch('binding', function(value) { + if (value) { + $scope.internalBinding = value.join($scope.itemDelimiter); + } + }); + } + }; + return directiveDefinitionObject; }); + diff --git a/test/test_ldap.py b/test/test_ldap.py index 3c1252f05..f00e62a4f 100644 --- a/test/test_ldap.py +++ b/test/test_ldap.py @@ -2,9 +2,7 @@ import unittest from app import app from initdb import setup_database_for_testing, finished_database_for_testing -from data import model from data.users import LDAPUsers - from mockldap import MockLdap class TestLDAP(unittest.TestCase): @@ -20,6 +18,10 @@ class TestLDAP(unittest.TestCase): 'dc': ['quay', 'io'], 'ou': 'employees' }, + 'ou=otheremployees,dc=quay,dc=io': { + 'dc': ['quay', 'io'], + 'ou': 'otheremployees' + }, 'uid=testy,ou=employees,dc=quay,dc=io': { 'dc': ['quay', 'io'], 'ou': 'employees', @@ -63,6 +65,13 @@ class TestLDAP(unittest.TestCase): 'uid': ['multientry'], 'another': ['key'] }, + 'uid=secondaryuser,ou=otheremployees,dc=quay,dc=io': { + 'dc': ['quay', 'io'], + 'ou': 'otheremployees', + 'uid': ['secondaryuser'], + 'userPassword': ['somepass'], + 'mail': ['foosecondary@bar.com'] + }, }) self.mockldap.start() @@ -73,9 +82,10 @@ class TestLDAP(unittest.TestCase): user_rdn = ['ou=employees'] uid_attr = 'uid' email_attr = 'mail' + secondary_user_rdns = ['ou=otheremployees'] ldap = LDAPUsers('ldap://localhost', base_dn, admin_dn, admin_passwd, user_rdn, - uid_attr, email_attr) + uid_attr, email_attr, secondary_user_rdns=secondary_user_rdns) self.ldap = ldap @@ -112,6 +122,15 @@ class TestLDAP(unittest.TestCase): (response, _) = self.ldap.confirm_existing_user('someuser', 'somepass') self.assertEquals(response.username, 'someuser') + def test_login_secondary(self): + # Verify we can login. + (response, _) = self.ldap.verify_and_link_user('secondaryuser', 'somepass') + self.assertEquals(response.username, 'secondaryuser') + + # Verify we can confirm the user. + (response, _) = self.ldap.confirm_existing_user('secondaryuser', 'somepass') + self.assertEquals(response.username, 'secondaryuser') + def test_invalid_password(self): # Verify we cannot login with an invalid password. (response, err_msg) = self.ldap.verify_and_link_user('someuser', 'invalidpass')