From 241ebbc8c73fda2fd636301588189de1d1edeabf Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Tue, 4 Apr 2017 15:19:59 -0700 Subject: [PATCH 1/3] fix #141 and other issues. * cleaned up connector names in logging * added filename on CSV unrecognized column error * catch UMAPI connection failures and fail cleanly --- user_sync/connector/directory_csv.py | 2 +- user_sync/connector/directory_ldap.py | 2 +- user_sync/connector/umapi.py | 28 +++++++++++++++------------ user_sync/helper.py | 2 +- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/user_sync/connector/directory_csv.py b/user_sync/connector/directory_csv.py index 2a3e8b27d..811f13cf5 100644 --- a/user_sync/connector/directory_csv.py +++ b/user_sync/connector/directory_csv.py @@ -65,7 +65,7 @@ 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', 'connector.' + CSVDirectoryConnector.name) + builder.set_string_value('logger_name', CSVDirectoryConnector.name) builder.require_string_value('file_path') options = builder.get_options() diff --git a/user_sync/connector/directory_ldap.py b/user_sync/connector/directory_ldap.py index 05180b89e..8f4ec308f 100644 --- a/user_sync/connector/directory_ldap.py +++ b/user_sync/connector/directory_ldap.py @@ -66,7 +66,7 @@ def __init__(self, caller_options): builder.set_string_value('user_domain_format', None) builder.set_string_value('user_identity_type', None) builder.set_int_value('search_page_size', 200) - builder.set_string_value('logger_name', 'connector.' + LDAPDirectoryConnector.name) + builder.set_string_value('logger_name', LDAPDirectoryConnector.name) host = builder.require_string_value('host') username = builder.require_string_value('username') builder.require_string_value('base_dn') diff --git a/user_sync/connector/umapi.py b/user_sync/connector/umapi.py index 8a81aa1de..057c09c90 100644 --- a/user_sync/connector/umapi.py +++ b/user_sync/connector/umapi.py @@ -26,9 +26,9 @@ import helper import user_sync.config -import user_sync.error import user_sync.helper import user_sync.identity_type +from user_sync.error import AssertionException from user_sync.version import __version__ as APP_VERSION try: @@ -84,16 +84,20 @@ def __init__(self, name, caller_options): "client_secret": enterprise_options['client_secret'], "private_key_file": private_key_file_path } - self.connection = connection = umapi_client.Connection( - org_id=org_id, - auth_dict=auth_dict, - ims_host=ims_host, - ims_endpoint_jwt=server_options['ims_endpoint_jwt'], - user_management_endpoint=um_endpoint, - test_mode=options['test_mode'], - user_agent="user-sync/" + APP_VERSION, - logger=self.logger, - ) + try: + self.connection = connection = umapi_client.Connection( + org_id=org_id, + auth_dict=auth_dict, + ims_host=ims_host, + ims_endpoint_jwt=server_options['ims_endpoint_jwt'], + user_management_endpoint=um_endpoint, + test_mode=options['test_mode'], + user_agent="user-sync/" + APP_VERSION, + logger=self.logger, + ) + except Exception as e: + raise AssertionException("UMAPI connection to org id '%s' failed: %s" % (org_id, e)) + logger.debug('API initialized on: %s', um_endpoint) self.action_manager = ActionManager(connection, org_id, logger) @@ -184,7 +188,7 @@ def add_user(self, attributes): email = self.email if self.email else self.username if not email: errorMessage = "ERROR: you must specify an email with an Adobe ID" - raise user_sync.error.AssertionException(errorMessage) + raise AssertionException(errorMessage) params = self.convert_user_attributes_to_params({'email': email}) else: params = self.convert_user_attributes_to_params(attributes) diff --git a/user_sync/helper.py b/user_sync/helper.py index d41d98581..771526eba 100644 --- a/user_sync/helper.py +++ b/user_sync/helper.py @@ -69,7 +69,7 @@ def iter_csv_rows(file_path, delimiter = None, recognized_column_names = None, l if (recognized_column_names != None): unrecognized_column_names = [column_name for column_name in reader.fieldnames if column_name not in recognized_column_names] if (len(unrecognized_column_names) > 0 and logger != None): - logger.warn("Unrecognized column names: %s", unrecognized_column_names) + logger.warn("In file '%s': unrecognized column names: %s", file_path, unrecognized_column_names) for row in reader: yield row From 8d29e10f9cc42522b65361b062db47d186f6de42 Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Tue, 4 Apr 2017 16:17:14 -0700 Subject: [PATCH 2/3] fix #142. We now log excluded users if there is any chance that an Adobe user might be affected by the run. And if the user action for Adobe only users is exclude, we log them as Adobe-only and excluded. --- user_sync/rules.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/user_sync/rules.py b/user_sync/rules.py index 07e221180..c728ce499 100644 --- a/user_sync/rules.py +++ b/user_sync/rules.py @@ -679,6 +679,7 @@ def update_umapi_users_for_connector(self, umapi_info, umapi_connector): options = self.options update_user_info = options['update_user_info'] manage_groups = self.will_manage_groups() + exclude_strays = self.options['exclude_strays'] will_process_strays = self.will_process_strays # prepare the strays map if we are going to be processing them @@ -689,8 +690,8 @@ def update_umapi_users_for_connector(self, umapi_info, umapi_connector): in_primary_org = umapi_info.get_name() == PRIMARY_UMAPI_NAME # we only log certain users if they are relevant to our processing. - log_excluded_users = update_user_info or manage_groups - log_stray_users = will_process_strays + log_excluded_users = update_user_info or manage_groups or will_process_strays + log_stray_users = will_process_strays or (exclude_strays and log_excluded_users) log_matching_users = update_user_info or manage_groups @@ -721,7 +722,10 @@ def update_umapi_users_for_connector(self, umapi_info, umapi_connector): # so we mark this adobe user as a stray, and we mark him # for removal from any mapped groups. if log_stray_users: - self.logger.debug("Found Adobe-only user: %s", user_key) + if exclude_strays and log_excluded_users: + self.logger.debug("Excluding Adobe-only user: %s", user_key) + else: + self.logger.debug("Found Adobe-only user: %s", user_key) if will_process_strays: self.add_stray(umapi_info.get_name(), user_key, None if not manage_groups else current_groups & umapi_info.get_mapped_groups()) From cf7bf56bcaf64bdbc0b6dd572722793a4454b6e0 Mon Sep 17 00:00:00 2001 From: Daniel Brotsky Date: Tue, 4 Apr 2017 17:02:01 -0700 Subject: [PATCH 3/3] fix #142 - logging of excluded users. This was trickier than it appeared! Now that the logging of matched, excluded, and updated users is all at the debug level, we don't need to be so careful about logging them. Basically, we should always log them unless there is not chance they might be affected by their status. --- user_sync/rules.py | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/user_sync/rules.py b/user_sync/rules.py index c728ce499..18a566e50 100644 --- a/user_sync/rules.py +++ b/user_sync/rules.py @@ -689,12 +689,6 @@ def update_umapi_users_for_connector(self, umapi_info, umapi_connector): # there are certain operations we only do in the primary umapi in_primary_org = umapi_info.get_name() == PRIMARY_UMAPI_NAME - # we only log certain users if they are relevant to our processing. - log_excluded_users = update_user_info or manage_groups or will_process_strays - log_stray_users = will_process_strays or (exclude_strays and log_excluded_users) - log_matching_users = update_user_info or manage_groups - - # Walk all the adobe users, getting their group data, matching them with directory users, # and adjusting their attribute and group data accordingly. for umapi_user in umapi_connector.iter_users(): @@ -713,7 +707,7 @@ def update_umapi_users_for_connector(self, umapi_info, umapi_connector): desired_groups = user_to_group_map.pop(user_key, None) or set() # check for excluded users - if self.is_umapi_user_excluded(in_primary_org, user_key, current_groups, log_excluded_users): + if self.is_umapi_user_excluded(in_primary_org, user_key, current_groups): continue directory_user = filtered_directory_user_by_user_key.get(user_key) @@ -721,19 +715,17 @@ def update_umapi_users_for_connector(self, umapi_info, umapi_connector): # There's no selected directory user matching this adobe user # so we mark this adobe user as a stray, and we mark him # for removal from any mapped groups. - if log_stray_users: - if exclude_strays and log_excluded_users: - self.logger.debug("Excluding Adobe-only user: %s", user_key) - else: - self.logger.debug("Found Adobe-only user: %s", user_key) - if will_process_strays: + if exclude_strays: + self.logger.debug("Excluding Adobe-only user: %s", user_key) + elif will_process_strays: + self.logger.debug("Found Adobe-only user: %s", user_key) self.add_stray(umapi_info.get_name(), user_key, None if not manage_groups else current_groups & umapi_info.get_mapped_groups()) else: # There is a selected directory user who matches this adobe user, # so mark any changed umapi attributes, # and mark him for addition and removal of the appropriate mapped groups - if log_matching_users: + if update_user_info or manage_groups: self.logger.debug("Adobe user matched on customer side: %s", user_key) if update_user_info and in_primary_org: attribute_differences = self.get_user_attribute_difference(directory_user, umapi_user) @@ -749,24 +741,21 @@ def update_umapi_users_for_connector(self, umapi_info, umapi_connector): umapi_info.set_umapi_users_loaded() return user_to_group_map - def is_umapi_user_excluded(self, in_primary_org, user_key, current_groups, do_logging): + def is_umapi_user_excluded(self, in_primary_org, user_key, current_groups): if in_primary_org: # in the primary umapi, we actually check the exclusion conditions identity_type, username, domain = self.parse_user_key(user_key) if identity_type in self.exclude_identity_types: - if do_logging: - self.logger.debug("Excluding adobe user (due to type): %s", user_key) + self.logger.debug("Excluding adobe user (due to type): %s", user_key) self.excluded_user_count += 1 return True if len(current_groups & self.exclude_groups) > 0: - if do_logging: - self.logger.debug("Excluding adobe user (due to group): %s", user_key) + self.logger.debug("Excluding adobe user (due to group): %s", user_key) self.excluded_user_count += 1 return True for re in self.exclude_users: if re.match(username): - if do_logging: - self.logger.debug("Excluding adobe user (due to name): %s", user_key) + self.logger.debug("Excluding adobe user (due to name): %s", user_key) self.excluded_user_count += 1 return True self.included_user_keys.add(user_key)