Fix attribute filtering #423
Conversation
Added a test that fails when the friendlyName of the requested attribute is not the same with the name of the internal attribute (even though the OIDs and the internal representation names of the attribute are the same)
Instead on relying on the FriendlyName from metadatata, use the name of the internal representation of an attribute in order to perform filtering. Resolves IdentityPython#422
|
I thought only the tests in |
|
After looking at this, I believe that the tests fail because of the tests themselves and not because of the changes introduced in the PR. def setup_class(self):
self.server = Server("idp_conf")
conf = config.SPConfig()
conf.load_file("server_conf")
self.client = client.Saml2Client(conf)
self.name_id = self.server.ident.transient_nameid(
"urn:mace:example.com:saml:roland:sp", "id12")
self.ava = {"givenName": ["Derek"], "surName": ["Jeter"],
"mail": ["derek@nyy.mlb.com"], "title": "The man"}However, if we see how ava is populated, then there is no way to get a "surName" key in there. In read_attribute_statement , to_local is used to convert the attribute names from the Attribute Statement to the internal representation names using the attributemaps so the key in the ava should be |
As explained in IdentityPython#423 (comment) , ava cannot contain an 'surName' key, it should be named 'sn'
|
@rohe or anyone else interested, can I get a review or some feedback on the changes proposed above ? |
|
It really would be great if this issue could be fixed by accepting Ioanni's pull request. |
src/saml2/assertion.py
Outdated
| friendly_name = attr["friendly_name"] | ||
| except KeyError: | ||
| friendly_name = get_local_name(acs, attr["name"], | ||
| friendly_name = get_local_name(acs, attr["name"], |
There was a problem hiding this comment.
change friendly_name to local_name
src/saml2/assertion.py
Outdated
| @@ -79,10 +79,15 @@ def filter_on_attributes(ava, required=None, optional=None, acs=None, | |||
|
|
|||
| def _match_attr_name(attr, ava): | |||
| try: | |||
|
get_local_name raises an exception when acs is None, hence the failures above. I will revisit the change |
Since acs can't be None ( it get's a value in __init__() https://github.com/rohe/pysaml2/blob/master/src/saml2/assertion.py#L319) there is no reason to test for it. So we add a default value to acs using ac_factory() before passing it to filter_on_attributes
Use the internal representation names instead of metadata FriendlyNames for attributes in order to do name filtering. Solves #422