search

italia / spid-sp-test

SAML2 SPID/CIE Service Provider validation tool
European Union Public License 1.2
39 stars 17 forks source link

ACR reponse check should consider "Comparison" attribute #111

Closed ewedlund closed 3 years ago

ewedlund commented 3 years ago

According to the documentation, the accepted AuthnContextClassRef depends on the attribute "Comparison". Currently, the method https://github.com/italia/spid-sp-test/blob/6a9302a210c6f6b80a172ad876716433787857f3/src/spid_sp_test/responses/response_mods.py#L5 takes for granted that Comparison=="minimum", which makes e.g. test n. 96 fail for the case when Comparison="exact".

Valid Comparison values are

exact minimum better maximum

Not sure how "better" and "maximum" should be handled. I propose to at least add handling "exact".

peppelinux commented 3 years ago

it was "exact" before

these matches can be configured in the response/settings.py file. I have to understand how to fit all the users needs, I won't introduce a breaking change.

Consider me available to get your help and hints for improve this task

ewedlund commented 3 years ago

The problem with the settings approach is that then you are expecting people to a) actually read the documentation, and b) have understood all the details of the SPID guidelines, and interpreted them correctly. Actually, since we are using "exact", at first I thought that we were not compliant, since of course I did not remember all the rules.

I implemented a check for Comparison in dynamic_acr, let me know if you want me to make a PR.

peppelinux commented 3 years ago

sure, go ahead thank you!

peppelinux commented 3 years ago

closed by https://github.com/italia/spid-sp-test/pull/112

mauromol commented 2 years ago

Unfortunately, this change, although correct from a SAML 2.0 point of view, is against what SPID specification says. See #124.