From cbce40a6d85979b448527abc90617eb0d36ce0f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Janou=C5=A1ek?= Date: Thu, 5 Sep 2019 15:50:40 +0200 Subject: [PATCH] Make some LightResponse fields optional Ref #16 --- eidas_node/models.py | 14 ++++++++++---- eidas_node/saml.py | 8 +------- .../tests/data/light_response_failure.xml | 3 --- eidas_node/tests/test_models.py | 17 ++++++++++++----- eidas_node/tests/test_saml.py | 2 +- 5 files changed, 24 insertions(+), 20 deletions(-) diff --git a/eidas_node/models.py b/eidas_node/models.py index 5d9274b..29f8376 100644 --- 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 index 8548f2b..42a879e 100644 --- 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 index 5714585..bca074f 100644 --- a/eidas_node/tests/data/light_response_failure.xml +++ b/eidas_node/tests/data/light_response_failure.xml @@ -4,9 +4,6 @@ test-light-request-id test-light-response-issuer relay123 - unknown - urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified - http://eidas.europa.eu/LoA/low true urn:oasis:names:tc:SAML:2.0:status:Requester diff --git a/eidas_node/tests/test_models.py b/eidas_node/tests/test_models.py index 47c3d88..930ce5c 100644 --- 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 index bbeee63..d60448d 100644 --- 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)