Added the ability to pull alternative ldap attributes for givenName, sn and c #321
Conversation
|
@bhunut-adobe Please file an issue for this. Do you think the same enhancement is needed for the Okta connector? |
|
Ok, I will work on directory_okta also |
|
Dan, I added the feature to okta connector and created issue #322 |
There was a problem hiding this comment.
So the code looks very good (see the few comments in line). The overall question I have is a philosophical one: does it actually make sense, given that Okta has mandated directory values already for first name, last name, and email, to allow mapping those differently to Adobe than they are in Okta? It seems that any customization the customer would want to do of these fields should be done in the way they map their directory system to Okta rather than the way Okta's directory gets mapped to Adobe. So why don't you talk to @adorton-adobe and see what you guys think about whether we should allow customization for Okta.
| # combining constant strings with the values of specific directory attributes. | ||
| # Any names in curly braces are taken as attribute names, and everything including | ||
| # the braces will be replaced on a per-user basis with the values of the attributes. | ||
| # The default value used here is simple, and suitable for OpenLDAP systems. If you |
There was a problem hiding this comment.
Seems silly to talk about OpenLDAP systems versus AD systems in an Okta-specific connector configuration. Please consider whether Okta actually allows flexibility about each of these fields (because I suspect not for email, first name, and last name, which they have specific fields for in their directory system) and update the docs/impl accordingly.
There was a problem hiding this comment.
haha ok I will update it
| # The default value used here is simple, and suitable for OpenLDAP systems. | ||
| # NOTE: for this and every format setting, the constant strings must be in | ||
| # the encoding specified by the string_encoding setting, above. | ||
| #user_country_code_format: "{countryCode}" |
There was a problem hiding this comment.
Need a EOL at the end of this line.
| source_attributes['id'] = user['uid'] = record.id | ||
| source_attributes['email'] = user['email'] = profile.email | ||
| source_attributes['email'] = email |
There was a problem hiding this comment.
Why did you not just change profile.email to email on this line? (OK to leave it split; I was just wondering.)
There was a problem hiding this comment.
oh because I just copy the stuff from directory_ldap.
| source_attributes[extended_attribute] = extended_attribute_value | ||
| else: | ||
| source_attributes[extended_attribute] = None | ||
| extended_attribute_value = OKTAValueFormatter.get_profile_value(record, extended_attribute) |
|
@adobeDan Let me know if the changes are good. |
|
@bhunut-adobe @adorton-adobe Just wanted to confirm that you two had talked and agree we should allow customization of all of these fields so that they differ on the Adobe side than how they appear on the Okta side? Let me know and then I'll merge the fix. |
|
I don't know if it makes sense to allow customization for Okta. Do we even have a way to test it in an Okta instance? I thought Okta didn't allow certain fields to be customized. |
|
@adorton-adobe We are not really customizing on the Okta side, we are allowing the Adobe side values to differ from the Okta side. I can't think of a case where that matters (because Okta never sees the values on the Adobe side and we ignore the values on the Okta side), but since the Okta fields have exactly the same semantics as the Adobe fields I just don't really see the customer need to make them different. So I can't tell if we're just doing this to be symmetric with LDAP and it would be simpler not to. |
|
@adobeDan I assume that there would need to be configuration changes to populate first name, last name and country to non-standard fields. That is what I meant by customization - sorry for not being more clear. I confirmed with Kevin that it is possible to do that. I don't know if it is something that customers are likely to do. I'd err on the side of adding the functionality as long as we test it sufficiently. |
Customer requested that they want to use an alternative attribute for givenName and surname.