pysaml2 should not try to convert attribute Names to attribute FriendlyNames.

c00kiemon5ter commented 6 years ago

Expected Behavior

Attributes should be untouched and specified by their Name (which is a required attribute). As FriendlyName is an optional attribute, one cannot rely on that and must-not as specified by the specification.

Section 2.7.3.1, § FriendlyName, line 1271 This attribute's value MUST NOT be used as a basis for formally identifying SAML attributes

Convertion from Name to FriendlyName should be explicit and handled by the user.

Current Behavior

Internally pysaml2 converts attribute names to friendly names and uses the latter for most operations, like filtering, metadata generation (RequestedAttributes element), creation of class members, etc. This is achieved through the AttributeConverter class, the helper functions defined in attribute_converter module and the attributemaps files. These allows for some control over the conversion process.

Possible Solution

Only depend on attribute Name. It is the only way to guarantee robustness and is required to be in line with the specification. This questions the value of attribute_converter module; whether it is needed at all. The user of the library (the application) should be responsible for such a conversion, which most probably would be used to present an interface where friendly-names could be used in place of the non-descriptive name identifiers.

c00kiemon5ter commented 6 years ago

Related to #548

c00kiemon5ter commented 6 years ago

Semi-related to #524

c00kiemon5ter commented 6 years ago

Friendly names are used in the configuration:

Policy configuration

_nameform Which name-form that should be used when sending assertions. Using this information the attribute name in the data source will be mapped to the friendly name, and the saml attribute name will be taken from the uri/oid defined in the attribute map.

(breaking) This should be removed.

Requested attributes

It is mandatory that at least name or friendly_name is set.

(breaking) Name should be mandatory.

Optional attributes

Since the attribute names used here are the user friendly ones an attribute map must exist, so that the server can use the full name when communicating with other servers.

(breaking) Switch to using names.

Required attributes

the names given are expected to be the user friendly names.

(breaking) Switch to using names.

c00kiemon5ter commented 6 years ago

Call chains into attribute_converter

c00kiemon5ter commented 5 years ago

Related to #556

mrvanes commented 5 years ago

Mind that one added value of the attributemaps is the possibility to know attributename-format when generating attributes, it facilitates the notion of 'known' vs 'unknown' attributes and it may be used to generate friendyName attribute to the attribute xml element (how confusing!)

c00kiemon5ter commented 5 years ago

Related to #403

skoranda commented 4 years ago

Not all attribute names are globally unique, especially when integrating with cloud/vendor SPs. When the format is basic or unspecified SATOSA still needs to be able to take internal attributes and map them to non-globally unique attribute names that can differ from SP to SP.

For example, a deployment may have the internal name i_given_name and need for one SP to send the value with name="given" and format unspecified, but for another SP it needs to send the value with name="given_name" and format unspecified.

c00kiemon5ter commented 4 years ago

[E49]Attributes are identified/named by the combination of the NameFormat and Name XML attributes described above. Neither one in isolation can be assumed to be unique, but taken together, they ought to be unambiguous within a given deployment.

The SAML profiles specification [SAMLProf] includes a number of attribute profiles designed to improve the interoperability of attribute usage in some identified scenarios. Such profiles typically include constraints on attribute naming and value syntax. There is no explicit indicator when an attribute profile is in use, and it is assumed that deployments can establish this out of band, based on the combination of NameFormat and Name.

(emphasis mine)

The uniques comes by putting the NameFormat and Name together (not just the Name)

To have two entities talk to each other they have to agree on common language, through a shared dictionary - the dictionary for the attributes is implicit through the combination of NameFormat and Name. This hints on the attribute profile that is used by the entity.

The proxy has to coordinate the communication of multiple entities, each of them can speak their own language. All those entities talk to the proxy. The proxy needs to be able to talk all those languages and allow mappings between them. When we map NameFormat:basic and Name:familyName to NameFormat:foo and Name:family_name we map between the two dictionaries that the corresponding entities know, and the proxy becomes the translation point. The proxy translates between attribute profiles by mapping Attribute NameFormat&Name. One thing to note is that the mapping between the entities is not direct, but it takes place between an entity-language (attribute (and claims) profile) and the proxy-language (internal attributes/data). This is important, as this is what builds an abstraction between the different protocols, and allows us to treat saml2 and openid in a uniform way. This, of course, needs more work.

The question then becomes, should the proxy do the translation of attribute profiles per entity? (or, in more practical terms: can two entities use "NameFormat:foo" and "Name:family_name" but talk about different things?)

The quoted part of the spec does not help a lot - it says "together, they ought to be unambiguous within a given deployment". If we consider that the same NameFormat and Name are unique across entities, then they form a "unique identifier" (not necessarily global - unique at least within this deployment) and we don't need to scope them per entity. This will probably be much cleaner and easier to reason about. If we translate "deployment" to "connection" (or mapping) then we ought to scope the mappings under specific entities (which form this "deployment/connection/link".)

tl;dr

So, to sum up, what I am not certain about is whether we should scope the mapping under an entity or, if we can consider the proxy and all its connections a "deployment" that treats the same Attribute NameFormat&Name the same way (as the same attribute profile) regardless of the entity that sent that request.

peppelinux commented 4 years ago

Not all attribute names are globally unique, especially when integrating with cloud/vendor SPs. When the format is basic or unspecified SATOSA still needs to be able to take internal attributes and map them to non-globally unique attribute names that can differ from SP to SP.

For example, a deployment may have the internal name i_given_name and need for one SP to send the value with name="given" and format unspecified, but for another SP it needs to send the value with name="given_name" and format unspecified.

+1 this introduce why I was forced to specify in the attribute maps in uri format also the attributes in basic format coming from SPID, due to the fact that these and the R&S ones are released together in the same Response (if account linking is true, will return trasparently all those, as they are). Although the global format of the IdP is uri and not mixed .. .

Especially the fact that "email" in_to collides with the spid attribute of the same name.

It would be useful to specify the reference format for each type of attribute, in order to obtain a number of attributes with ad hoc format specifications and rely on a unique identifier defined in each attr format schema. Standing on [E49] the colliding unspecified still are dangerous but would have be handled, without real collisions

mrvanes commented 4 years ago

So, in that case just send both attributes with the same value to both. Either of them should discard the one it doesn't recognise as a valid attribute? As a SAML SP you are not obliged to consume (all the) attributes.

peppelinux commented 4 years ago

So, in that case just send both attributes with the same value to both. Either of them should discard the one it doesn't recognise as a valid attribute? As a SAML SP you are not obliged to consume (all the) attributes.

That's in SATOSA, a two years old configuration that got latest updates. I should try to improve this aspect but It Just works

skoranda commented 4 years ago

It is not always an option to just send additional attributes in the SAML assertion to an SP. This breaks some vendor/cloud SP SAML stacks. SATOSA needs to be capable of being customized to send any conformant SAML assertion to a particular SP that needs a specific combination of NameFormat and Name for an attribute.

skoranda commented 4 years ago

"whether we should scope the mapping under an entity or, if we can consider the proxy and all its connections a "deployment" that treats the same Attribute NameFormat&Name the same way (as the same attribute profile) regardless of the entity that sent that request."

We should make is possible to scope the mapping under an entity so that a single SATOSA deployment is capable of sending a conformant SAML assertion to any SP regardless of the Name and NameFormat combination the SP needs.

c00kiemon5ter commented 4 years ago

We should make is possible to scope the mapping under an entity so that a single SATOSA deployment is capable of sending a conformant SAML assertion to any SP regardless of the Name and NameFormat combination the SP needs.

This means that entities that use the same attribute profile, but talk about different things, or, in practical terms, two entities that use "NameFormat:foo" and "Name:family_name", talk about different things.

Is this really happening?

peppelinux commented 4 years ago

I just have my use case when I use SPID backend, if I want to return attributes as they come from backends I'd like to return them without any reprocessing. I would have some attributes that in different format could collide if I try to join them in a unique attr format.

I found my workaround to deal with that but increasing the granularity of this behaviour in a standard global configuration, in future satosa releases, could be interesting

skoranda commented 4 years ago

First, regarding signaling, it is important to note that many vendor/cloud SPs do not provide any SAML metadata for integration, nor do they indicate attribute requirements in the AuthnRequest. The attribute and other SAML requirements are often only documented in words, often poorly.

Second, here is the use case that prompted this dialogue from me:

I have internal attribute i_given_name. For a single SAML SATOSA front end, I need to assert that attribute to SP1 as

<ns1:Attribute FriendlyName="given_name" Name="given_name" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified">
  <ns1:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">Scott</ns1:AttributeValue>
</ns1:Attribute>

and I also need to assert that attribute to SP2 as

<ns1:Attribute FriendlyName="givenName" Name="givenName" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified">
  <ns1:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xsi:type="xs:string">Scott</ns1:AttributeValue>
</ns1:Attribute>

Doing so with today's SATOSA requires that copy i_given_name to a second internal attribute i_given_name_sp2 and then define in internal_attributes.yml a mapping from i_given_name_2 to givenName (friendly), and then in the pysaml2 attributemaps givenName to givenName. This is awkward and I would like the refactored code to more easily accommodate this (quite real) requirement.

peppelinux commented 4 years ago

What I do with SPID attributes that are in basic format: I have to map them in a uri format map using their "basic name" as it would be OID. collisions in "_to" would happen, we know. We filtered out all the collisions to avoid it

IdentityPython / pysaml2