Repository: italia/spid-sp-test (language: python)

Issue:
spid-sp-test emits all attributes in responses
When producing SAML responses, spid-sp-test seems to always emit all the authenticated subject attributes, disregarding what was requested within the `AuthnRequest` by the SP.
I would expect it to honour the SP request (unless a specific check should test the SP behaviour otherwise) and perhaps provide some command-line argument to force the addition or removal of other attributes.


Discussion / hints:
that's something I decided to handle by hand, using `-aj` parameter with a custom collection of attributes.

I think that this issue could be covered with an automatic selection of attributes to be release, in base of which have been defined in the authn request

thank you @mauromol 
Hi @peppelinux , with 0.9.11 I see that you're returning all the attributes of all the attribute consuming services listed in metadata. However, the expected behaviour is that only the attributes of the attribute consuming service specified in the `AuthnRequest` (through the `AttributeConsumingServiceIndex` attribute) are returned.
however
https://github.com/italia/spid-sp-test/commit/d6141a33f6fb83c52712e8144c582669b8100f7a

gets all the attributes from all the ACS, this MUST be fixed, filtering on top of the ACS defined in the authn request

Fix patch:
diff --git a/setup.py b/setup.py
index 3f476aa..04234de 100644
--- a/setup.py
+++ b/setup.py
@@ -10,7 +10,7 @@ def readme():
 
 setup(
     name=_pkg_name,
-    version='0.9.11',
+    version='0.9.12',
     description="SAML2 SPID Service Provider validation tool that can be run from the command line",
     long_description=readme(),
     long_description_content_type='text/markdown',


Test patch:
diff --git a/src/spid_sp_test/response.py b/src/spid_sp_test/response.py
index ac2ba20..025a1e3 100644
--- a/src/spid_sp_test/response.py
+++ b/src/spid_sp_test/response.py
@@ -110,19 +110,7 @@ def __init__(self, *args, **kwargs):
         self.category = "response"
 
         self.template_path = kwargs.get("template_path", self.template_path)
-
         self.metadata_etree = kwargs.get("metadata_etree")
-        self.requested_attrs_raw = self.metadata_etree.xpath(
-            "//EntityDescriptor/SPSSODescriptor"
-            "/AttributeConsumingService/RequestedAttribute"
-        )
-        self.requested_attrs = [
-            i.attrib["Name"] for i in self.requested_attrs_raw
-        ]
-
-        self.acs_url = self.metadata_etree.xpath(
-            "//SPSSODescriptor/AssertionConsumerService[@index=0]"
-        )[0].attrib["Location"]
 
         self.authn_request_url = kwargs.get("authn_request_url")
         self.authn_request_data = {}
@@ -142,16 +130,7 @@ def __init__(self, *args, **kwargs):
             self.tests.update(settings.RESPONSE_TESTS)
         self.test_names = kwargs.get("test_names") or self.tests.keys()
 
-        # attributes
-        if kwargs.get("attr_json"):
-            with open(kwargs["attr_json"], "r") as json_data:
-                self.user_attrs = json.loads(json_data.read())
-        else:
-            # returns ONLY the requested attributes shown in the metadata
-            # otherwise it returns all the attributes (for test purpose)
-            self.user_attrs = {
-                i:settings.ATTRIBUTES[i] for i in self.requested_attrs
-            } or settings.ATTRIBUTES
+        self.attr_json = kwargs.get("attr_json")
 
         self.html_path = kwargs.get("html_path")
         self.no_send_response = kwargs.get("no_send_response")
@@ -170,6 +149,26 @@ def get_acr(self):
         if _acr:
             return _acr[0].text
 
+    def load_user_attributes(self):
+        self.requested_attrs_raw = self.metadata_etree.xpath(
+            f"//SPSSODescriptor/AttributeConsumingService[@index={self.acs_index}]"
+            "/RequestedAttribute"
+        )
+        self.requested_attrs = [
+            i.attrib["Name"] for i in self.requested_attrs_raw
+        ]
+
+        # attributes
+        if self.attr_json:
+            with open(self.attr_json, "r") as json_data:
+                self.user_attrs = json.loads(json_data.read())
+        else:
+            # returns ONLY the requested attributes shown in the metadata
+            # otherwise it returns all the attributes (for test purpose)
+            self.user_attrs = {
+                i:settings.ATTRIBUTES[i] for i in self.requested_attrs
+            } or settings.ATTRIBUTES
+
     def do_authnrequest(self):
         self.authn_request_data = get_authn_request(
             self.authn_request_url,
@@ -186,19 +185,22 @@ def do_authnrequest(self):
 
         self.issuer = self.kwargs.get("issuer", SAML2_IDP_CONFIG["entityid"])
         self.authnreq_attrs = self.authnreq_etree.xpath("/AuthnRequest")[0].attrib
-
         self.authnreq_issuer = self.authnreq_etree.xpath("/AuthnRequest/Issuer")[
             0
         ].attrib["NameQualifier"]
+
         now = datetime.datetime.utcnow()
 
+        self.acs_index = self.authnreq_attrs.get('AttributeConsumingServiceIndex')
+        self.acs_url = self.metadata_etree.xpath(
+            f"//SPSSODescriptor/AssertionConsumerService[@index={self.acs_index}]"
+        )[0].attrib["Location"]
         self.acr = self.get_acr()
 
         if self.acr in NOSESINDEX_ACRS:
             _session_index = None
         else:
             _session_index = saml_rnd_id()
-
         self.response_attrs = {
             "ResponseID": saml_rnd_id(),
             "AuthnRequestID": self.authnreq_attrs["ID"],
@@ -355,6 +357,7 @@ def send_response(self, xmlstr):
     def test_profile_spid_sp(self):
         for i in self.test_names:
             self.do_authnrequest()
+            self.load_user_attributes()
             response_obj = self.load_test(test_name=i)
             test_display_desc = response_obj.conf["description"]
             msg = f'Response [{i}] "{test_display_desc}"'