Problem/Motivation
From the documentation and the examples in the help text it appears that takes friendly names but from testing it doesn't.
Example: I must use urn:oid:0.9.2342.19200300.100.1.3 instead of mail
Error for using uid instead of urn:oid:0.9.2342.19200300.100.1.1 for unique_id_attribute
Configured unique ID is not present in SAML response
Steps to reproduce
Try to ma attributes using the friendly name from the IDP for mapping username/mail or unique id, login through SAML and see if they work or update correctly.
Proposed resolution
Use \OneLogin\Saml2\Auth::getAttributeWithFriendlyName() to map against friendly names too, it was added in v.2.13.0
Not sure if it should be configurable, separate array or tacked on to the same list...?
Remaining tasks
User interface changes
N/A
API changes
Data model changes
Comments
Comment #2
joelpittetHere's the simplest patch I could think of to solve this issue, no extra config considering the keys are not likely to collide.
Comment #3
joelpittetTested and works like a charm! Hope it's the right approach?
Also not sure if this is a bug report or task or feature request considering it should work but maybe wasn't finished... so I opted for task.
Comment #4
joelpittetComment #5
joelpittetI discussed in Slack a couple weeks ago with @roderik, that I should split these functions out, so I'll do that, been a bit busy but it's on my mind.
Comment #6
joelpittetHere's the alternative approach, though I think the complexity it introduces is not worth it and I personally prefer #2 but feel free to compare and contrast yourselves.
Comment #7
roderikThank you for this.
I neglected to respond in the issue earlier, so for the record: my earlier hesitation about including the patch is over the definition of the $attributes argument changing in concept:
This is still just as good for, I'm sure, most applications. But if there are any event subscribers out there who are doing something with the full set of attribute values, they may not like the duplication of values in that set.
Starting today I have some non-customer-work time and I hope it will extend to thinking about not only this issue but a layer below it... so I'm going to leave it for a few days. Rough thoughts I want to dive into:
1) when creating this event, I figured that just passing "the attributes" would be enough for subscribers to access all necessary info. That clearly isn't enough, because we at the very least have friendly names, which a) are a bit icky to combine into one array with the non-friendly names; b) having two separate arrays for the same info also feels strange. But then, there may also be other info I'm missing. This feels like it needs some 'value object' passed into the event instead of just a simple array. Maybe the Auth object itself or a simple wrapper around it.
2) I'm prepared to break compatibility over this in a new alpha release, if I can get investigation on #2925171: Use externalauth::register event vs hook_user_presave finished and discover there's something to improve there. (Basically, it the samlauth module's separate event has always felt clunky and I may be able to get an actual argument together for modifying the externalauth module / the ExternalAuthEvents::LOGIN event, to make it more logical.)
More news later this week.
Comment #8
joelpittet@roderik, if you're really worried about the duplicated values maybe we just make this configurable and off by default? Would that make it easier to commit?
Comment #10
roderikI have a plan now, for what to tell people if they suddenly come back saying they can't work with the duplicated values. So I'm less concerned now.
(Which is: fork a v4.x for them, where the event handler will be passed a value object resembling \OneLogin\Saml2\Auth instead of the array - i.e. something where the event handler can call $value->getAttribute() or $value->getAttributeWithFriendlyName(). They'll need to live with running a -dev release for likely a long time, but at least it's possible and in line with my plans for a 4.x release, per #2882568: Plan for SAML Authentication 4.x. And it likely won't happen anyway.)
With that, I'm happy to commit your first version. Thanks for your patience.
Comment #11
joelpittetHurray🎉🚀 Thanks for weighing all the things @roderik!
Comment #13
pshakya commentedIssue configuring Okta with SAMLauth.
Configured unique ID is not present in SAML response, though its set to NAMEID = emailaddress per schema. unable to do anything here
Comment #14
roderik@pshakya
You have re-titled an issue that 1) was already closed and 2) was not about the same issue.
As to your issue:
#3170734: Enable support for FriendlyName Attributes contains a patch you can try, It may well end up in the module, but I plan on not integrating it until 1) there is time to look at a more complete picture, in that issue; or 2) someone else invests the time / funds.