Merge pull request #231 from adobe-apiplatform/issue182

Fixes for #182, #129, and #233 - all the requested LDAP improvements

Author: adobeDan

examples/config files - basic/3 connector-ldap.yml

@@ -59,15 +59,26 @@ require_tls_cert: False
 all_users_filter: "(&(objectClass=user)(objectCategory=person)(!(userAccountControl:1.2.840.113556.1.4.803:=2)))"
 
 # (optional) group_filter_format (default value given below)
-# group_filter_format specifies the format string used to construct a group query,
-# as needed by the --users groups or --users mapped command-line arguments.
+# group_filter_format specifies the format string used to get the distinguished
+# name of a group given its common name (as specified in the directory to Adobe
+# group mapping, or in the --users group "name1,name2" command-line argument).
 # {group} is replaced with the name of the group to find.  The default value here is
 # complex, because it's meant to work for both AD-style and OpenLDAP-style directories.
 # You will likely want to replace it with a simpler query customized for your directory,
 # such as this one for Active Directory: "(&(objectCategory=group)(cn={group}))"
 # or this one for OpenLDAP: "(&(|(objectClass=groupOfNames)(objectClass=posixGroup))(cn={group}))"
 group_filter_format: "(&(|(objectCategory=group)(objectClass=groupOfNames)(objectClass=posixGroup))(cn={group}))"
 
+# (optional) group_member_filter_format (default value given below)
+# group_users_filter specifies the query used to find all members of a group,
+# where the string {group_dn} is replaced with the group distinguished name.
+# The default value just finds users who are immediate members of the group,
+# not those who are "indirectly" members by virtue of membership in a group
+# that is contained in the group.  If you want indirect containment, then
+# use this value instead of the default:
+# group_member_filter_format: "(memberOf:1.2.840.113556.1.4.1941:={group_dn})"
+group_member_filter_format: "(memberOf={group_dn})"
+
 # (optional) string_encoding (default value given below)
 # string_encoding specifies the Unicode string encoding used by the directory.
 # All values retrieved from the directory are converted to Unicode before being

tests/connector/directory_csv_test.py

@@ -32,7 +32,7 @@ def test_normal(self):
         }
         directory_connector.initialize(options)
 
-        actual_users = directory_connector.load_users_and_groups(all_groups)
+        actual_users = directory_connector.load_users_and_groups(groups=all_groups)
 
         tests.helper.assert_equal_users(self, all_users, actual_users)
 

tests/connector/directory_ldap_test.py

@@ -9,19 +9,11 @@
 class LDAPDirectoryTest(unittest.TestCase):
 
     def test_normal(self):
-        user1 = tests.helper.create_test_user(['Acrobat1', 'Acrobat2'])
-        user2 = tests.helper.create_test_user(['Acrobat3'])
+        user1 = tests.helper.create_test_user([])
+        user2 = tests.helper.create_test_user([])
         user3 = tests.helper.create_test_user([])
         all_users = [user1, user2, user3]
 
-        users_by_group = {}
-        for user in all_users:
-            for group in user['groups']:
-                users_with_same_group = users_by_group.get(group)
-                if (users_with_same_group == None):
-                    users_by_group[group] = users_with_same_group = []
-                users_with_same_group.append(user)
-
         ldap_options = {
             'host': 'test_host', 
             'username': 'test_user',
@@ -43,19 +35,8 @@ def mock_simple_bind_s(*args, **kwargs):
         def mock_search_s(*args, **kwargs):
             search_result = re.search('cn=(.*?)\)', kwargs['filterstr'])            
             group_name = search_result.group(1)
-            users = users_by_group.get(group_name, [])
-            return [(group_name, {
-                'member': [user['firstname'] for user in users if group_name in user['groups']]
-            })]
-
-        def mock_result(*args, **kwargs):
-            rtype = ldap.RES_SEARCH_RESULT
-            rdata = []
-            return rtype, rdata
+            return [(group_name, {})]
 
-        def mock_search(*args, **kwargs):
-            return kwargs['filterstr']
-    
         def mock_search_ext(*args, **kwargs):
             return kwargs['filterstr']
 
@@ -76,12 +57,10 @@ def mock_result3(*args, **kwargs):
         connection.search_s = mock_search_s
         connection.search_ext = mock_search_ext
         connection.result3 = mock_result3
-        connection.search = mock_search
-        connection.result = mock_result
 
         directory_connector = user_sync.connector.directory.DirectoryConnector(user_sync.connector.directory_ldap)
         directory_connector.initialize(ldap_options)
 
-        actual_users = directory_connector.load_users_and_groups(users_by_group.iterkeys())
+        actual_users = directory_connector.load_users_and_groups(None)
 
         tests.helper.assert_equal_users(self, all_users, actual_users)

tests/rules_test.py

@@ -40,21 +40,21 @@ def test_normal(self):
             directory_group_1: [user_sync.rules.AdobeGroup(primary_group_11, primary_umapi_name), user_sync.rules.AdobeGroup('acrobat12', secondary_1_umapi_name)],
             directory_group_2: [user_sync.rules.AdobeGroup(primary_group_21, primary_umapi_name)]
         }
-        all_users = [tests.helper.create_test_user([directory_group_1]), 
+        everybody = [tests.helper.create_test_user([directory_group_1]),
             tests.helper.create_test_user([directory_group_2]),
             tests.helper.create_test_user([])]
 
-        for user in all_users:
+        for user in everybody:
             user['username'] = user['email']
             user['domain'] = None
 
         primary_users = []
-        primary_user_1 = all_users[1].copy()
+        primary_user_1 = everybody[1].copy()
         primary_user_1['groups'] = [primary_group_11]
         primary_users.append(primary_user_1)
 
-        def mock_load_users_and_groups(groups, extended_attributes=None):
-            return list(all_users)
+        def mock_load_users_and_groups(groups=None, extended_attributes=None, all_users=True):
+            return list(everybody)
         mock_directory_connector = mock.mock.create_autospec(user_sync.connector.directory.DirectoryConnector)
         mock_directory_connector.load_users_and_groups = mock_load_users_and_groups
 
@@ -75,25 +75,25 @@ def mock_load_users_and_groups(groups, extended_attributes=None):
 
         expected_primary_commands_list = []
 
-        user = all_users[1]
+        user = everybody[1]
         commands = tests.helper.create_umapi_commands(user)
         commands.add_groups(set([primary_group_21]))
         commands.remove_groups(set([primary_group_11]))
         expected_primary_commands_list.append(commands)
 
-        user = all_users[0]
+        user = everybody[0]
         commands = tests.helper.create_umapi_commands(user)
         commands.add_user(self.create_user_attributes_for_commands(user, rule_options['update_user_info']))
         commands.add_groups(set([primary_group_11]))
         expected_primary_commands_list.append(commands)
 
-        user = all_users[2]
+        user = everybody[2]
         commands = tests.helper.create_umapi_commands(user)
         commands.add_user(self.create_user_attributes_for_commands(user, rule_options['update_user_info']))
         expected_primary_commands_list.append(commands)
 
         expected_secondary_commands_list = []
-        user = all_users[0]
+        user = everybody[0]
         commands = tests.helper.create_umapi_commands(user)
         commands.add_groups(set([primary_group_12]))
         expected_secondary_commands_list.append(commands)

user_sync/app.py

@@ -359,6 +359,11 @@ def main():
         if not e.is_reported():
             logger.critical(e.message)
             e.set_reported()
+    except KeyboardInterrupt:
+        try:
+            logger.critical('Keyboard interrupt, exiting immediately.')
+        except:
+            pass
     except:
         try:
             logger.error('Unhandled exception', exc_info=sys.exc_info())

user_sync/config.py

@@ -844,7 +844,7 @@ def set_int_value(self, key, default_value):
     def set_string_value(self, key, default_value):
         """
         :type key: str
-        :type default_value: str
+        :type default_value: Optional(str)
         """
         self.set_value(key, types.StringTypes, default_value)
 

user_sync/connector/directory.py

@@ -45,12 +45,16 @@ def initialize(self, options=None):
             options = {}
         self.state = self.implementation.connector_initialize(options)
 
-    def load_users_and_groups(self, groups, extended_attributes=None):
+    def load_users_and_groups(self, groups, extended_attributes=None, all_users=True):
         """
         :type groups: list(str)
-        :type extended_attributes: list(str)
+        :type extended_attributes: Optional(list(str))
+        :type all_users: bool
         :rtype (bool, iterable(dict))
         """
         if extended_attributes is None:
             extended_attributes = []
-        return self.implementation.connector_load_users_and_groups(self.state, groups, extended_attributes)
+        return self.implementation.connector_load_users_and_groups(self.state,
+                                                                   groups=groups,
+                                                                   extended_attributes=extended_attributes,
+                                                                   all_users=all_users)

user_sync/connector/directory_csv.py

@@ -39,17 +39,16 @@ def connector_initialize(options):
     return state
 
 
-def connector_load_users_and_groups(state, groups, extended_attributes):
+def connector_load_users_and_groups(state, groups=None, extended_attributes=None, all_users=True):
     """
     :type state: CSVDirectoryConnector
-    :type groups: list(str)
-    :type extended_attributes: list(str)
+    :type groups: Optional(list(str))
+    :type extended_attributes: Optional(list(str))
+    :type all_users: bool
     :rtype (bool, iterable(dict))
     """
-
-    # CSV supports arbitrary aka "extended" attrs by default,
-    # so the value of extended_attributes has no impact on this particular connector
-    return state.load_users_and_groups(groups, extended_attributes)
+    # CSV always reads all users, so we don't bother passing the all_users parameter into the implementation
+    return state.load_users_and_groups(groups or [], extended_attributes or [])
 
 
 class CSVDirectoryConnector(object):