Merge pull request #172 from adobe-apiplatform/issue-159

intermediate work on #159.  Fix #173.

Author: adobeDan

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

@@ -17,11 +17,19 @@
 # You must specify all four of these settings.  Consult with your
 # enterprise directory administrators to get suitable values.
 # These access credentials are sensitive and must be protected.
-username: "LDAP username goes here"
+username: "LDAP or Credential Manager username goes here"
 password: "LDAP password goes here"
 host: "LDAP host URL goes here.  e.g. ldap://ldap.example.com"
 base_dn: "defines the base DN. e.g. DC=example,DC=com"
 
+#(optional)
+credential_manager:
+    #This will pull credential from Windows Credential Manager in Control Panel.
+    #value: windows_credential_manager
+    type: windows_credential_manager
+    #service_name is Required for Windows Credential Manger
+    service_name: "Internet or Network Address field in Credential Manager"
+
 # (optional) user_identity_type (default is inherited from main configuration)
 # user_identity_type specifies a default identity type for when directory users
 # are created on the Adobe side (one of adobeID, enterpriseID, federatedID).

misc/build/requirements.txt

@@ -2,3 +2,4 @@ pycrypto
 PyYAML
 psutil
 umapi-client>=2.0.2
+keyring

setup.py

@@ -43,8 +43,9 @@
           'pycrypto',
           'python-ldap==2.4.25',
           'PyYAML',
-          'umapi-client>=2.2',
+          'umapi-client>=2.3',
           'psutil',
+          'keyring'
       ],
       setup_requires=['nose>=1.0'],
       tests_require=[

user_sync/config.py

@@ -21,13 +21,13 @@
 import logging
 import os
 import re
-
 import types
+
+import keyring
 import yaml
 
 import user_sync.identity_type
 import user_sync.rules
-from user_sync import credential_manager
 from user_sync.error import AssertionException
 
 DEFAULT_MAIN_CONFIG_FILENAME = 'user-sync-config.yml'
@@ -139,20 +139,10 @@ def get_directory_connector_options(self, connector_name):
         '''                
         options = {}
         connectors_config = self.get_directory_connector_configs()
-        if (connectors_config != None):
+        if connectors_config is not None:
             connector_item = connectors_config.get_list(connector_name, True)
             options = self.get_dict_from_sources(connector_item)
-        
         options = self.combine_dicts([options, self.options['directory_connector_overridden_options']])
-        # credentials are None, a dict, or a config filename to read to get a dict
-        credentials = credential_manager.get_credentials(credential_manager.DIRECTORY_CREDENTIAL_TYPE,
-                                                         connector_name,
-                                                         config=options,
-                                                         config_loader = self)
-        if isinstance(credentials, types.StringTypes):
-            credentials = ConfigFileLoader.load_other_config(credentials)
-        if isinstance(credentials, dict):
-            options = self.combine_dicts([options, credentials])
         return options
 
     def get_directory_groups(self):
@@ -164,9 +154,9 @@ def get_directory_groups(self):
             raise AssertionException("Your main configuration file is still in v1 format.  Please convert it to v2.")
         groups_config = None
         directory_config = self.main_config.get_dict_config('directory_users', True)
-        if (directory_config != None):
+        if directory_config is not None:
             groups_config = directory_config.get_list_config('groups', True)         
-        if (groups_config == None):
+        if groups_config is None:
             return adobe_groups_by_directory_group
 
         for item in groups_config.iter_dict_configs():
@@ -215,7 +205,6 @@ def get_dict_from_sources(self, sources):
         Given a list of config file paths, return the dictionary composed of all the contents
         of those config files, or None if the list is empty
         :param sources: a list of strings
-        :param owner: a string to use in error messages if we can't find a config file.
         :rtype dict
         '''
         if not sources:
@@ -249,10 +238,10 @@ def combine_dicts(dicts):
         '''
         result = {}
         for dict_item in dicts:
-            if (isinstance(dict_item, dict)):
+            if isinstance(dict_item, dict):
                 for dict_key, dict_item in dict_item.iteritems():
                     result_item = result.get(dict_key)
-                    if (isinstance(result_item, dict) and isinstance(dict_item, dict)):
+                    if isinstance(result_item, dict) and isinstance(dict_item, dict):
                         result_item.update(dict_item)
                     else:
                         result[dict_key] = dict_item
@@ -357,26 +346,12 @@ def get_rule_options(self):
     def create_umapi_options(self, connector_config_sources):
         options = self.get_dict_from_sources(connector_config_sources)
         options['test_mode'] = self.options['test_mode']
-        enterprise_section = options.get('enterprise')
-        if isinstance(enterprise_section, dict):
-            org_id = enterprise_section.get('org_id')
-            if (org_id != None):
-                # credentials are None, a dict, or a config filename to read to get a dict
-                credentials = credential_manager.get_credentials(credential_manager.UMAPI_CREDENTIAL_TYPE,
-                                                                 org_id,
-                                                                 config = enterprise_section,
-                                                                 config_loader = self)
-                if isinstance(credentials, types.StringTypes):
-                    credentials = ConfigFileLoader.load_other_config(credentials)
-                if isinstance(credentials, dict):
-                    options['enterprise'] = self.combine_dicts([enterprise_section, credentials])
         return options
 
     def check_unused_config_keys(self):
         directory_connectors_config = self.get_directory_connector_configs()
         self.main_config.report_unused_values(self.logger, [directory_connectors_config])
 
-    
 class ObjectConfig(object):
     def __init__(self, scope):
         '''
@@ -420,30 +395,31 @@ def create_assertion_error(self, message):
         return AssertionException("%s in: %s" % (message, self.get_full_scope()))
 
     def describe_types(self, types_to_describe):
-        if (types_to_describe == types.StringTypes):
+        if types_to_describe == types.StringTypes:
             result = self.describe_types(types.StringType)
-        elif (isinstance(types_to_describe, tuple)):
+        elif isinstance(types_to_describe, tuple):
             result = []
             for type_to_describe in types_to_describe:
                 result.extend(self.describe_types(type_to_describe))
         else:
             result = [types_to_describe.__name__]
         return result
 
-    def report_unused_values(self, logger, optional_configs = []):
+    def report_unused_values(self, logger, optional_configs=None):
+        optional_configs = [] if optional_configs is None else optional_configs
         has_error = False        
         for config in self.iter_configs():
             messages = config.describe_unused_values()
-            if (len(messages) > 0):
-                if (config in optional_configs):
+            if len(messages) > 0:
+                if config in optional_configs:
                     log_level = logging.WARNING
                 else:
                     log_level = logging.ERROR
                     has_error = True
                 for message in messages:
                     logger.log(log_level, message)
 
-        if (has_error):
+        if has_error:
             raise AssertionException('Detected unused keys that are not ignorable.')
 
     def describe_unused_values(self):
@@ -465,7 +441,7 @@ def iter_values(self, allowed_types):
         '''
         index = 0
         for item in self.value:
-            if (not isinstance(item, allowed_types)):
+            if not isinstance(item, allowed_types):
                 reported_types = self.describe_types(allowed_types)
                 raise self.create_assertion_error("Value should be one of these types: %s for index: %s" % (reported_types, index))
             index += 1
@@ -475,7 +451,7 @@ def iter_dict_configs(self):
         index = 0
         for value in self.iter_values(dict):
             config = self.find_child_config(index)
-            if (config == None):
+            if config is None:
                 config = DictConfig("[%s]" % index, value)
                 self.add_child(config)
             yield config
@@ -500,17 +476,17 @@ def iter_keys(self):
 
     def iter_unused_keys(self):
         for key in self.iter_keys():
-            if (key not in self.accessed_keys):
+            if key not in self.accessed_keys:
                 yield key
 
     def get_dict_config(self, key, none_allowed = False):
         '''
         :rtype DictConfig
         '''
         result = self.find_child_config(key)
-        if (result == None):
+        if result is None:
             value = self.get_dict(key, none_allowed)
-            if (value != None):
+            if value is not None:
                 result = DictConfig(key, value)
                 self.add_child(result)
         return result        
@@ -530,7 +506,7 @@ def get_bool(self, key, none_allowed = False):
 
     def get_list(self, key, none_allowed = False):        
         value = self.get_value(key, None, none_allowed)
-        if (value != None and not isinstance(value, list)):
+        if value is not None and not isinstance(value, list):
             value = [value]
         return value
 
@@ -539,9 +515,9 @@ def get_list_config(self, key, none_allowed = False):
         :rtype ListConfig
         '''
         result = self.find_child_config(key)
-        if (result == None):
+        if result is None:
             value = self.get_list(key, none_allowed)
-            if (value != None):
+            if value is not None:
                 result = ListConfig(key, value)
                 self.add_child(result)
         return result
@@ -554,21 +530,77 @@ def get_value(self, key, allowed_types, none_allowed = False):
         '''
         self.accessed_keys.add(key)
         result = self.value.get(key)
-        if (result == None):
-            if (not none_allowed):
+        if result is None:
+            if not none_allowed:
                 raise self.create_assertion_error("Value not found for key: %s" % key)
-        elif (allowed_types != None and not isinstance(result, allowed_types)):
+        elif allowed_types is not None and not isinstance(result, allowed_types):
             reported_types = self.describe_types(allowed_types)
             raise self.create_assertion_error("Value should be one of these types: %s for key: %s" % (reported_types, key))
         return result
 
     def describe_unused_values(self):
         messages = []
         unused_keys = list(self.iter_unused_keys())
-        if (len(unused_keys) > 0):
+        if len(unused_keys) > 0:
             messages.append("Found unused keys: %s in: %s" % (unused_keys, self.get_full_scope()))
         return messages
-    
+
+    keyring_prefix = 'secure_'
+    keyring_suffix = '_key'
+
+    def has_credential(self, name):
+        '''
+        Check if there is a credential setting with the given name
+        :param name: plaintext setting name for the credential
+        :return: setting that was specified, or None if none was
+        '''
+        scope = self.get_full_scope()
+        keyring_name = self.keyring_prefix + name + self.keyring_suffix
+        plaintext = self.get_string(name, True)
+        secure = self.get_string(keyring_name, True)
+        if plaintext and secure:
+            raise AssertionException('%s: cannot contain setting for both "%s" and "%s"' % (scope, name, keyring_name))
+        if plaintext is not None:
+            return name
+        elif secure is not None:
+            return keyring_name
+        else:
+            return None
+
+    def get_credential(self, name, user_name, none_allowed=False):
+        '''
+        Get the credential with the given name.  Raises an AssertionException if there
+        is no credential, or if the credential is specified both in plaintext and the keyring.
+        If the credential is kept in the keyring, the value of the keyring_name setting
+        gives the secure storage key, and we fetch that key for the given user.
+        :param name: setting name for the plaintext credential
+        :param user_name: the user for whom we should fetch the service name password in secure storage
+        :param none_allowed: whether the credential can be missing or empty
+        :return: credential string
+        '''
+        keyring_name = self.keyring_prefix + name + self.keyring_suffix
+        scope = self.get_full_scope()
+        # sometimes the credential is in plain text
+        cleartext_value = self.get_string(name, True)
+        # sometimes the value is in the keyring
+        secure_value_key = self.get_string(keyring_name, True)
+        # but it has to be in exactly one of those two places!
+        if not cleartext_value and not secure_value_key and not none_allowed:
+            raise AssertionException('%s: must contain setting for "%s" or "%s"' % (scope, name, keyring_name))
+        if cleartext_value and secure_value_key:
+            raise AssertionException('%s: cannot contain setting for both "%s" and "%s"' % (scope, name, keyring_name))
+        if secure_value_key:
+            try:
+                value = keyring.get_password(service_name=secure_value_key, username=user_name)
+            except Exception as e:
+                raise AssertionException('%s: Error accessing secure storage: %s' % (scope, e))
+        else:
+            value = cleartext_value
+        if not value and not none_allowed:
+            raise AssertionException(
+                '%s: No value in secure storage for user "%s", key "%s"' % (scope, user_name, secure_value_key))
+        return value
+
 class ConfigFileLoader:
     '''
     Loads config files and does pathname expansion on settings that refer to files or directories
@@ -717,8 +749,9 @@ def process_path_value(cls, val, must_exist, can_have_subdict):
         does the relative path processing for a value from the dictionary,
         which can be a string, a list of strings, or a list of strings
         and "tagged" strings (sub-dictionaries whose values are strings)
-        :param key: the key whose value we are processing, for error messages
         :param val: the value we are processing, for error messages
+        :param must_exist: whether there must be a value
+        :param can_have_subdict: whether the value can be a tagged string
         '''
         if isinstance(val, types.StringTypes):
             return cls.relative_path(val, must_exist)
@@ -793,7 +826,7 @@ def set_value(self, key, allowed_types, default_value):
         '''
         value = default_value
         config = self.default_config
-        if (config != None and config.has_key(key)):            
+        if config is not None and config.has_key(key):
             value = config.get_value(key, allowed_types, False)
         self.options[key] = value
 
@@ -809,7 +842,7 @@ def require_value(self, key, allowed_types):
         :type key: str
         '''
         config = self.default_config
-        if (config == None):
+        if config is None:
             raise AssertionException("No config found.")
         self.options[key] = value = config.get_value(key, allowed_types)
         return value

user_sync/connector/directory_csv.py

@@ -53,7 +53,7 @@ class CSVDirectoryConnector(object):
     name = 'csv'
 
     def __init__(self, caller_options):
-        caller_config = user_sync.config.DictConfig('"%s options"' % CSVDirectoryConnector.name, caller_options)
+        caller_config = user_sync.config.DictConfig('%s configuration' % self.name, caller_options)
         builder = user_sync.config.OptionsBuilder(caller_config)
         builder.set_string_value('delimiter', None)
         builder.set_string_value('first_name_column_name', 'firstname')
@@ -65,18 +65,16 @@ def __init__(self, caller_options):
         builder.set_string_value('domain_column_name', 'domain')
         builder.set_string_value('identity_type_column_name', 'type')
         builder.set_string_value('user_identity_type', None)
-        builder.set_string_value('logger_name', CSVDirectoryConnector.name)
+        builder.set_string_value('logger_name', self.name)
         builder.require_string_value('file_path')
         options = builder.get_options()
-
-        # identity type for new users if not specified in column
-        self.user_identity_type = user_sync.identity_type.parse_identity_type(options['user_identity_type'])
-
         self.options = options
-        self.logger = logger = user_sync.connector.helper.create_logger(options)        
+        self.logger = logger = user_sync.connector.helper.create_logger(options)
+        logger.debug('%s initialized with options: %s', self.name, options)
         caller_config.report_unused_values(logger)
 
-        logger.debug('Initialized with options: %s', options)            
+        # identity type for new users if not specified in column
+        self.user_identity_type = user_sync.identity_type.parse_identity_type(options['user_identity_type'])
 
     def load_users_and_groups(self, groups, extended_attributes):
         '''
@@ -129,7 +127,7 @@ def get_column_name(key):
             email = self.get_column_value(row, email_column_name)
             if email is None or email.find('@') < 0:
                 logger.warning('Missing or invalid email at row: %d; skipping', line_read)
-                continue;
+                continue
 
             user = users.get(email)
             if user is None: