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

joelpittet created an issue. See original summary.

joelpittet’s picture

Status: Active » Needs review
StatusFileSize
new856 bytes

Here's the simplest patch I could think of to solve this issue, no extra config considering the keys are not likely to collide.

joelpittet’s picture

Tested 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.

joelpittet’s picture

Title: Friendly Name support » Enable support for FriendlyName Attributes
joelpittet’s picture

Status: Needs review » Needs work

I 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.

joelpittet’s picture

Status: Needs work » Needs review
StatusFileSize
new6.68 KB

Here'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.

roderik’s picture

Thank 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:

  • Currently: "the collection of all attributes, keyed by their attribute name".
  • After: "A hash to look up attributes by attribute name or FriendlyName". (Which therefore can have duplicate attiribute values.)

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.

joelpittet’s picture

@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?

  • roderik committed 0c65e42 on 8.x-3.x authored by joelpittet
    Issue #3170734 by joelpittet: Enable support for FriendlyName Attributes
    
roderik’s picture

Status: Needs review » Fixed

I 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.

joelpittet’s picture

Hurray🎉🚀 Thanks for weighing all the things @roderik!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

pshakya’s picture

Title: Enable support for FriendlyName Attributes » Configured unique ID is not present in SAML response OKta
Issue tags: +okta

Issue 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

roderik’s picture

Title: Configured unique ID is not present in SAML response OKta » Enable support for FriendlyName Attributes
Issue tags: -okta

@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.