Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add useFriendlyNames config option #389

Closed
wants to merge 1 commit into from

Conversation

warncke
Copy link

@warncke warncke commented Sep 5, 2019

Adds a new useFriendlyNames config option that when set to true will use the FriendlyName instead of Name for profile properties from the SAML assertion.

Fixes #282

@markstos
Copy link
Contributor

I'm not opposed, but FriendlyName is optional in the spec and the name is required.

One benefit would be making configurations a bit more readable by referencing the FriendlyName. On the other hand, since FriendlyName is optional, depending on it being there could increase the chance of problems.

I'm leaving this open in case someone else wants to comment for or against.

@cjbarth
Copy link
Collaborator

cjbarth commented Nov 18, 2019

@warncke I'm not against this either, but even if the option is set, there should be logic to fallback to Name in the case that FriendlyName doesn't exist.

@markstos
Copy link
Contributor

@cjbarth There is logic to fallback to "name" if FriendlyName does not exist: 50024ff#diff-41fc97a3e28a2b6f8b5ec3bdae6130d5R1034

But that seems that could lead to confusion there the resulting profile names are a mix of the "Friendly" and "Non-friendly" names.

What if we add a "friendly" key to the profile returned and make the "friendly" names sub fields of that? It would be up to the end user to default to the non-friendly names if they want to do that.

@cjbarth
Copy link
Collaborator

cjbarth commented Nov 19, 2019

Ah yes @markstos , I must have missed that due to the wrap. Sorry. But I do agree that we could have implementer confusion if we mix them in the library. It does make more sense to have multiple keys and let the implementer sort it out.

@markstos
Copy link
Contributor

I'm rejecting this because as proposed the result can be a mix of "friendly" and non-friendly names based on the incoming data which can sew confusion in an already complex library.

Friendly names are appealing but because they are optional they can lead to complications.

@markstos markstos closed this Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

not parsing FriendlyName value for Attribute within AttributeStatment
3 participants