Merge pull request #1601 from coreos-inc/multiple-rdns

Allow for multiple user RDNs in LDAP

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')

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)
+      for user_search_dn in self._user_dns:
-      try:
+        (pairs, err_msg) = self._ldap_user_search_with_rdn(conn, username_or_email, user_search_dn)
-        pairs = conn.search_s(user_search_dn, ldap.SCOPE_SUBTREE, query.encode('utf-8'))
+        if pairs is not None and len(pairs) > 0:
-      except ldap.REFERRAL as re:
+          break
-        referral_dn = self._get_ldap_referral_dn(re)
-        if not referral_dn:
-          return (None, 'Failed to follow referral when looking up username')
 
-        try:
+      if err_msg is not None:
-          subquery = u'(%s=%s)' % (self._uid_attr, username_or_email)
+        return (None, err_msg)
-          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')
 
       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

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;
 }

static/directives/config/config-setup-tool.html

@@ -595,23 +595,36 @@
           <tr>
             <td>Base DN:</td>
             <td>
-              <span class="config-list-field" item-title="DN" binding="config.LDAP_BASE_DN"></span>
+              <span class="config-string-list-field" item-title="DN piece" item-delimiter="," binding="config.LDAP_BASE_DN"></span>
               <div class="help-text">
-              A list of Distinguished Name pieces which forms the base path for
+              A Distinguished Name path which forms the base path for looking up all LDAP records.
-              looking up all LDAP records.
               </div>
               <div class="help-text">
-                Example: [dc=my,dc=domain,dc=com]
+                Example: dc=my,dc=domain,dc=com
               </div>
             </td>
           </tr>
           <tr>
             <td>User Relative DN:</td>
             <td>
-              <span class="config-list-field" item-title="RDN" binding="config.LDAP_USER_RDN"></span>
+              <span class="config-string-list-field" item-title="RDN piece" item-delimiter="," binding="config.LDAP_USER_RDN"></span>
               <div class="help-text">
-              A list of Distinguished Name pieces which forms the base path for
+              A Distinguished Name path which forms the base path for looking up all user LDAP records,
-              looking up all user LDAP records, relative to the Base DN defined above.
+              relative to the Base DN defined above.
+              </div>
+              <div class="help-text">
+                Example: ou=employees
+              </div>
+            </td>
+          </tr>
+          <tr>
+            <td>Secondary User Relative DNs:</td>
+            <td>
+              <span class="config-list-field" item-title="RDN" binding="config.LDAP_SECONDARY_USER_RDNS"></span>
+              <div class="help-text">
+              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.
               </div>
               <div class="help-text">
                 Example: [ou=employees]

static/directives/config/config-string-list-field.html

@@ -0,0 +1,6 @@
+<div class="config-string-list-field-element">
+  <form name="fieldform" novalidate>
+    <input type="text" class="form-control" placeholder="{{ placeholder || '' }}"
+           ng-model="internalBinding" ng-trim="true" ng-minlength="1" ng-required="!isOptional">
+  </form>
+</div>

static/js/core-config-setup.js

@@ -1162,16 +1162,16 @@ angular.module("core-config-setup", ['angularFileUpload'])
         $scope.patternMap = {};
 
         $scope.getRegexp = function(pattern) {
-        if (!pattern) {
+          if (!pattern) {
-          pattern = '.*';
+            pattern = '.*';
-        }
+          }
 
-        if ($scope.patternMap[pattern]) {
+          if ($scope.patternMap[pattern]) {
-          return $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;
   });
+

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')