SAML asks for transient nameId, but uses it for Member->GUID

[chillu]

SAMLController reads $auth->getNameId(), and saves it to Member->GUID.

The default service provider metadata asks for a transient value.

“transient” (vs. persistent) is defined as “Indicates that the content of the element is an identifierwith transient semantics and SHOULD be treated as an opaque and temporary value by the relying party.” . See https://stackoverflow.com/questions/11693297/what-are-the-different-nameid-format-used-for#21682789. Depending on which value ADFS uses for this nameId by default, this might cause new members being created for the same email address on every login.

The module describes this in a misleading way: https://github.com/silverstripe/silverstripe-activedirectory/blob/master/docs/en/adfs.md#rule-2-send-objectid-as-nameidentifier.

This use case is also outlined by the wonderful OASIS standards descriptions in 5.3.4 - did I mention how much I loathe OASIS standards?

We shouldn't rely on transient identifiers when mapping to members, but rather use "persistent", right?

[sminnee]

My guess is that by asking to send objectid as the transient nameId we've worked around this flaw. So it's not a critical issue for current implementations.

However, we may be relying on an odd config which isn't a good long-term approach

[chillu]

Yeah, it's sending the wrong intentions to the identity provider. A SAML implementer reading our auth requests could decide to randomly generate GUIDs for users on every request (e.g. to avoid a user being tracked across logins), and we'd just pile up Member records. If you then associate functionality to these records in a SilverStripe database (e.g. personal preferences), they'll all be lost on subsequent logins by the same user. Identity providers can send you something persistent (looks like all the current ones do), but there's no technical obligation to do that.

[maxime-rainville]

We EOLfing the module.