Make some LightResponse fields optional

Ref #16
CZ-NIC · Sep 10, 2019 · cbce40a · cbce40a
1 parent 223ac33
commit cbce40a
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 20 deletions.
diff --git a/eidas_node/models.py b/eidas_node/models.py
@@ -220,11 +220,17 @@ def validate(self) -> None:
         """Validate this data model."""
         self.validate_fields(Status, 'status', required=True)
         self.status.validate()
-        self.validate_fields(str, 'id', 'in_response_to_id', 'subject', required=True)
-        self.validate_fields(str, 'issuer', 'ip_address', 'relay_state', required=False)
-        self.validate_fields(NameIdFormat, 'subject_name_id_format', required=True)
-        self.validate_fields(LevelOfAssurance, 'level_of_assurance', required=True)
         validate_attributes(self, 'attributes')
+        if self.status.failure:
+            self.validate_fields(str, 'id', 'in_response_to_id', required=True)
+            self.validate_fields(str, 'subject', 'issuer', 'ip_address', 'relay_state', required=False)
+            self.validate_fields(NameIdFormat, 'subject_name_id_format', required=False)
+            self.validate_fields(LevelOfAssurance, 'level_of_assurance', required=False)
+        else:
+            self.validate_fields(str, 'id', 'in_response_to_id', 'subject', required=True)
+            self.validate_fields(str, 'issuer', 'ip_address', 'relay_state', required=False)
+            self.validate_fields(NameIdFormat, 'subject_name_id_format', required=True)
+            self.validate_fields(LevelOfAssurance, 'level_of_assurance', required=True)
 
     def deserialize_subject_name_id_format(self, elm: Element) -> Optional[NameIdFormat]:
         """Deserialize field 'subject_name_name_id_format'."""

diff --git a/eidas_node/saml.py b/eidas_node/saml.py
@@ -135,7 +135,7 @@ def __init__(self, document: ElementTree, relay_state: Optional[str] = None):
 
     def create_light_response(self) -> LightResponse:
         """Convert SAML response to light response."""
-        response = LightResponse()
+        response = LightResponse(attributes=OrderedDict())
         root = self.document.getroot()
         if root.tag != Q_NAMES['saml2p:Response']:
             raise ValidationError({
@@ -168,12 +168,6 @@ def create_light_response(self) -> LightResponse:
             elif elm.tag == Q_NAMES['saml2:Assertion']:
                 self._parse_assertion(response, elm)
         response.relay_state = self.relay_state
-        if response.status and response.status.failure:
-            # Fill in dummy data
-            response.attributes = OrderedDict()
-            response.level_of_assurance = LevelOfAssurance.LOW
-            response.subject_name_id_format = NameIdFormat.UNSPECIFIED
-            response.subject = 'unknown'
         return response
 
     def _parse_assertion(self, response: LightResponse, assertion: Element) -> None:

diff --git a/eidas_node/tests/data/light_response_failure.xml b/eidas_node/tests/data/light_response_failure.xml
@@ -4,9 +4,6 @@
   <inResponseToId>test-light-request-id</inResponseToId>
   <issuer>test-light-response-issuer</issuer>
   <relayState>relay123</relayState>
-  <subject>unknown</subject>
-  <subjectNameIdFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</subjectNameIdFormat>
-  <levelOfAssurance>http://eidas.europa.eu/LoA/low</levelOfAssurance>
   <status>
     <failure>true</failure>
     <statusCode>urn:oasis:names:tc:SAML:2.0:status:Requester</statusCode>

diff --git a/eidas_node/tests/test_models.py b/eidas_node/tests/test_models.py
@@ -92,9 +92,9 @@
          ('issuer', 'test-light-response-issuer'),
          ('ip_address', None),
          ('relay_state', 'relay123'),
-         ('subject', 'unknown'),
-         ('subject_name_id_format', NameIdFormat.UNSPECIFIED),
-         ('level_of_assurance', LevelOfAssurance.LOW),
+         ('subject', None),
+         ('subject_name_id_format', None),
+         ('level_of_assurance', None),
          ('status', OrderedDict(
              [('failure', True),
               ('status_code', StatusCode.REQUESTER),
@@ -355,6 +355,8 @@ class TestStatus(ValidationMixin, SimpleTestCase):
 class TestLightResponse(ValidationMixin, SimpleTestCase):
     MODEL = LightResponse
     OPTIONAL = {'issuer', 'ip_address', 'relay_state'}
+    OPTIONAL_FAILURE = {'issuer', 'ip_address', 'relay_state',
+                        'subject', 'subject_name_id_format', 'level_of_assurance'}
     VALID_DATA = {
         'id': 'uuid',
         'in_response_to_id': 'uuid2',
@@ -385,6 +387,8 @@ class TestLightResponse(ValidationMixin, SimpleTestCase):
     def tearDown(self) -> None:
         if self.VALID_DATA is not self.__class__.VALID_DATA:
             del self.VALID_DATA
+        if self.OPTIONAL is not self.__class__.OPTIONAL:
+            del self.OPTIONAL
 
     def create_response(self, success: bool) -> LightResponse:
         data = (LIGHT_RESPONSE_DICT if success else FAILED_LIGHT_RESPONSE_DICT).copy()
@@ -394,16 +398,19 @@ def create_response(self, success: bool) -> LightResponse:
     def set_failure(self, failure: bool) -> None:
         data = self.__class__.VALID_DATA.copy()
         if failure:
+            self.OPTIONAL = self.__class__.OPTIONAL_FAILURE
             data.update({
                 'status': Status(failure=failure,
                                  status_code=StatusCode.REQUESTER,
                                  sub_status_code=SubStatusCode.REQUEST_DENIED,
                                  status_message='Oops.'),
                 'attributes': OrderedDict(),
-                'subject': 'unknown',
-                'subject_name_id_format': NameIdFormat.UNSPECIFIED,
+                'subject': None,
+                'subject_name_id_format': None,
+                'level_of_assurance': None,
             })
         else:
+            self.OPTIONAL = self.__class__.OPTIONAL
             data['status'] = Status(failure=False)
         self.VALID_DATA = data
 

diff --git a/eidas_node/tests/test_saml.py b/eidas_node/tests/test_saml.py
@@ -21,9 +21,9 @@
     'id': 'test-saml-response-id',
     'in_response_to_id': 'Ttest-saml-request-id',
     'issuer': 'test-saml-response-issuer',
-    'level_of_assurance': LevelOfAssurance.LOW,
 }
 LIGHT_RESPONSE_DICT.update(OVERRIDES)
+LIGHT_RESPONSE_DICT['level_of_assurance'] = LevelOfAssurance.LOW
 FAILED_LIGHT_RESPONSE_DICT.update(OVERRIDES)