Fix for 'invalid attribute type memberOf' at user-level LDAP request #578
Conversation
Read memberOf attribute from user_memberof_format. It accpets None and
has default value of '{memberOf}'
There was a problem hiding this comment.
It's a good start, but we need to think more about how this will integrate with the LDAP connector. We also already have a configuration option that is part of the two-step lookup feature which also adds a "member" attribute.
I think this new option should replace group_member_attribute_name. But we can't remove group_member_attribute_name for syncs that already use it.
This is what I propose:
- If two-step lookup is enabled and
group_member_attribute_nameis specified, then it should take precedence over this new config option - Otherwise, if two-step lookup is enabled and
group_member_attribute_nameis not specified, then this new config option is required to specify the group membership attribute - We also need to make sure we use the new config option value in
get_member_groups(). (see directory_ldap.py)
We also need to make all of the necessary documentation updates. And while we're at it, we should remove the documentation for group_member_attribute_name since we're deprecating it.
| @@ -77,6 +77,7 @@ def __init__(self, caller_options): | |||
| self.user_given_name_formatter = LDAPValueFormatter(options['user_given_name_format']) | |||
| self.user_surname_formatter = LDAPValueFormatter(options['user_surname_format']) | |||
| self.user_country_code_formatter = LDAPValueFormatter(options['user_country_code_format']) | |||
| self.user_memberof_format_formatter = LDAPValueFormatter(options['user_memberof_format']) | |||
There was a problem hiding this comment.
Let's change the name of the config option so we don't group it with the other user_ config options. Those options govern how user attributes are mapped. Those attributes translate directly to Admin Console user attributes. The group membership attribute is a special case. It also shouldn't be a "format" variable since it doesn't make sense to make it formattable. So we can remove this line to create a formatter.
I suggest naming it something like group_member_attribute.
| @@ -137,6 +138,7 @@ def get_options(caller_config): | |||
| builder.set_string_value('user_given_name_format', six.text_type('{givenName}')) | |||
| builder.set_string_value('user_surname_format', six.text_type('{sn}')) | |||
| builder.set_string_value('user_country_code_format', six.text_type('{c}')) | |||
| builder.set_string_value('user_memberof_format', six.text_type('{memberOf}')) | |||
There was a problem hiding this comment.
Since we aren't making this option formattable, we should remove the curly braces from the default value. Let's also change the default value to 'member' since that is the default for the two-step attribute (which we will need to reconcile with this new feature).
There was a problem hiding this comment.
On second thought, the default value should be None since this must be optional for basic use cases (e.g. when Dynamic group mapping and two-step lookup are not in use).
| @@ -310,7 +312,7 @@ def iter_users(self, base_dn, users_filter, extended_attributes): | |||
| user_attribute_names.extend(self.user_email_formatter.get_attribute_names()) | |||
| user_attribute_names.extend(self.user_username_formatter.get_attribute_names()) | |||
| user_attribute_names.extend(self.user_domain_formatter.get_attribute_names()) | |||
| user_attribute_names.append(six.text_type('memberOf')) | |||
| user_attribute_names.extend(self.user_memberof_format_formatter.get_attribute_names()) | |||
There was a problem hiding this comment.
We just need to append the value of our new config option here since we aren't doing formatting.
UST dynamtic group mappings now reads member information from new config dynamic_group_member_attribute. Implement usecases from https://jira.corp.adobe.com/browse/DMESVCS-99
Empty DN is valid DN for NetIQ eDirectory and AD Global Catalog.
…tform#578 From User Sync tool 2.5 onward, you are required to mention the `memberOf` LDAP attribute in connector-ldap.yml.
Configure dynamic_group_member_attribute when dynamic group mappings is defined
|
@bajwa-adobe is this ready for another review? |
|
Yes @adorton-adobe the PR is ready. |
UST dynamtic group mappings now reads member information from new config dynamic_group_member_attribute. Implement usecases from https://jira.corp.adobe.com/browse/DMESVCS-99
From User Sync tool 2.5 onward, you are required to mention the `memberOf` LDAP attribute in connector-ldap.yml.
Configure dynamic_group_member_attribute when dynamic group mappings is defined
Some OpenLDAP directories do not expect
memberOfattribute in user-level LDAP request. It now reads memberOf attribute fromuser_memberof_formatconfiguration. It is optional config and acceptsNone. The default value is{memberOf}.#503