-
Notifications
You must be signed in to change notification settings - Fork 474
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
not parsing FriendlyName value for Attribute within AttributeStatment #282
Comments
This is interesting. When compared to a response that this module does part correctly, what exactly is different about this one? Can you confirm that the IdP is returning a response that is valid with the SAML spec, and we are failing to be spec-compliant by not parsing this? |
Closing due to lack of follow-up from the OP or anyone else about this. |
@markstos , so sorry. I didn't get your reply originally. I don't know if the response formatting is spec compliant, but I can include it here. It's from a shiboleth idp. Here's the relevant attribute section from the response, from the idp, in xml:
|
Here's the json json passport provides from a response from this IDP, which does not use the FriendlyName, and also fails to parse several other attributes:
|
From reading our source code, docs and tests, the FriendlyName is not something we attempt to parse. From reading the SAML spec here, the FriendlyName is an optional part of the SAML spec. What I do personally is keep a mapping in my configuration system of the name I want to use to the name in the SAML being sent, which is sometimes a long OID. Then when I get back the parsed SAML data, I map the SAML profile field names to the names I want. I agree it could be an interesting feature to support returning profile fields by their FriendlyName if it's present. It could also be interesting to allow defining a mapping like that I one I use, to make it easier to get back a data structure with more usable key names. So I'm leaving this issue open in case someone wants to work on improvements here. |
Yes, we do the same with our configuration, and after inspecting their xml a bit more closely, I believe the issue is that they're repeating the "Name" key on multiple attributes, which then results in the plugin to overwrite the key in the profile such that the last one wins. So any previous key is not available to us in the profile. I'd say this is more a bug with this specific IDP, and not a passport-saml issue. Sadly, this does not make life easier at the moment ;-) |
As a follow up, the IDP changed the Name value for each of the attributes to be distinct, and the problem is resolved. I think it's still a good idea to consider optionally parsing on the FriendlyName value, as that seems to be very common in general use. I'd like to contribute to this project in general, so I'm going to fork. Is there a formal prioritized ticket list, or just doing this adhoc? I see a lot of open PR's, so just wondering. Thanks, @markstos |
@whatch Right now we are focused on issues and PRS labeled "1.0". I'd like to put a new release within about a week: https://github.com/bergie/passport-saml/issues?q=is%3Aissue+is%3Aopen+label%3A1.0 Since there will be a major version bump, this release is eligible to contain breaking changes. |
I try to triage PRs but leave most refinements up to contributor. If they don't follow up, the PRs often linger in case someone else wants to pick them up. There's also a new v2 branch and an issue about plans for v2. There is no primary developer now, so patches are very much community-driven at this point. |
The IDP is returning attribute values in their saml response to our post sso callback, using an attribute key for 'FriendlyName' on the Attribute tag.
The plugin doesn't seem to parse this correctly, so profile.uid, in this example, isn't valid, but profile.['urn:oid:0.9.2342.19200300.100.1.1'] works fine. I'm asking if there's a configuration option I'm not seeing that will enable me to use the friendly name value, instead of the urn value?
The text was updated successfully, but these errors were encountered: