From b7783766490dccc88dc75d0c1993715d13c0de06 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Fri, 20 Dec 2019 14:46:05 +0200 Subject: [PATCH 01/26] Add eIDAS NodeCountry and NodeCountryType basic support - Adds schemas and auto-generated .py files for eIDAS NodeCountry and NodeCountryType support - Adds node_country as a recognized attribute in the configuration of an SP/IdP - Adds node_country parsing for the construction of the entity descriptor as an extension element --- src/saml2/config.py | 2 ++ src/saml2/extension/node_country.py | 50 +++++++++++++++++++++++++++++ src/saml2/metadata.py | 8 +++++ tools/data/node_country.xsd | 15 +++++++++ 4 files changed, 75 insertions(+) create mode 100644 src/saml2/extension/node_country.py create mode 100644 tools/data/node_country.xsd diff --git a/src/saml2/config.py b/src/saml2/config.py index 274e3960f..1f423789a 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -97,6 +97,7 @@ "sp_type", "sp_type_in_metadata", "requested_attributes", + "node_country", ] AA_IDP_ARGS = [ @@ -118,6 +119,7 @@ "domain", "name_qualifier", "edu_person_targeted_id", + "node_country", ] PDP_ARGS = ["endpoints", "name_form", "name_id_format"] diff --git a/src/saml2/extension/node_country.py b/src/saml2/extension/node_country.py new file mode 100644 index 000000000..e9368fd2c --- /dev/null +++ b/src/saml2/extension/node_country.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python + +# +# Generated Thu Dec 12 18:16:51 2019 by parse_xsd.py version 0.5. +# + +import saml2 +from saml2 import SamlBase + + +NAMESPACE = 'http://eidas.europa.eu/saml-extensions' +class NodeCountryType_(SamlBase): + """The http://eidas.europa.eu/saml-extensions:NodeCountryType element """ + + c_tag = 'NodeCountryType' + c_namespace = NAMESPACE + c_children = SamlBase.c_children.copy() + c_attributes = SamlBase.c_attributes.copy() + c_child_order = SamlBase.c_child_order[:] + c_cardinality = SamlBase.c_cardinality.copy() + +def node_country_type__from_string(xml_string): + return saml2.create_class_from_xml_string(NodeCountryType_, xml_string) + + +class NodeCountry(NodeCountryType_): + """The http://eidas.europa.eu/saml-extensions:NodeCountry element """ + + c_tag = 'NodeCountry' + c_namespace = NAMESPACE + c_children = NodeCountryType_.c_children.copy() + c_attributes = NodeCountryType_.c_attributes.copy() + c_child_order = NodeCountryType_.c_child_order[:] + c_cardinality = NodeCountryType_.c_cardinality.copy() + +def node_country_from_string(xml_string): + return saml2.create_class_from_xml_string(NodeCountry, xml_string) + + +ELEMENT_FROM_STRING = { + NodeCountry.c_tag: node_country_from_string, + NodeCountryType_.c_tag: node_country_type__from_string, +} + +ELEMENT_BY_TAG = { + 'NodeCountry': NodeCountry, + 'NodeCountryType': NodeCountryType_, +} +def factory(tag, **kwargs): + return ELEMENT_BY_TAG[tag](**kwargs) diff --git a/src/saml2/metadata.py b/src/saml2/metadata.py index 5c465032b..10dee84a8 100644 --- a/src/saml2/metadata.py +++ b/src/saml2/metadata.py @@ -10,6 +10,7 @@ from saml2.extension import shibmd from saml2.extension import mdattr from saml2.extension import sp_type +from saml2.extension import node_country from saml2.saml import NAME_FORMAT_URI from saml2.saml import AttributeValue from saml2.saml import Attribute @@ -770,6 +771,13 @@ def entity_descriptor(confd): entd.authn_authority_descriptor = do_aq_descriptor(confd, mycert, enc_cert) + conf_node_country = confd.getattr('node_country', confd.context) + if conf_node_country: + if not entd.extensions: + entd.extensions = md.Extensions() + item = node_country.NodeCountry(text=conf_node_country) + entd.extensions.add_extension_element(item) + return entd diff --git a/tools/data/node_country.xsd b/tools/data/node_country.xsd new file mode 100644 index 000000000..c2a4e3722 --- /dev/null +++ b/tools/data/node_country.xsd @@ -0,0 +1,15 @@ + + + + + + + + + From 8f27876cdaa72baa21c339184444a0ce2127d36e Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Mon, 6 Jan 2020 18:54:07 +0200 Subject: [PATCH 02/26] Add tests for eIDAS support - Adds configs and test classes for eIDAS support --- tests/eidas/idp_conf.py | 68 +++++++++++++++++++++++++++++++++++ tests/eidas/sp_conf.py | 79 +++++++++++++++++++++++++++++++++++++++++ tests/eidas/test_idp.py | 13 +++++++ tests/eidas/test_sp.py | 62 ++++++++++++++++++++++++++++++++ 4 files changed, 222 insertions(+) create mode 100644 tests/eidas/idp_conf.py create mode 100644 tests/eidas/sp_conf.py create mode 100644 tests/eidas/test_idp.py create mode 100644 tests/eidas/test_sp.py diff --git a/tests/eidas/idp_conf.py b/tests/eidas/idp_conf.py new file mode 100644 index 000000000..1fd2251a8 --- /dev/null +++ b/tests/eidas/idp_conf.py @@ -0,0 +1,68 @@ +from saml2 import BINDING_SOAP +from saml2 import BINDING_HTTP_REDIRECT +from saml2 import BINDING_HTTP_POST +from saml2.saml import NAMEID_FORMAT_PERSISTENT, NAME_FORMAT_BASIC +from saml2.saml import NAME_FORMAT_URI + +from pathutils import full_path +from pathutils import xmlsec_path + +BASE = "http://localhost:8088" + +CONFIG = { + "entityid": "urn:mace:example.com:saml:roland:idp", + "name": "Rolands IdP", + "service": { + "idp": { + "endpoints": { + "single_sign_on_service": [ + ("%s/sso" % BASE, BINDING_HTTP_REDIRECT)], + "single_logout_service": [ + ("%s/slo" % BASE, BINDING_SOAP), + ("%s/slop" % BASE, BINDING_HTTP_POST)] + }, + "policy": { + "default": { + "lifetime": {"minutes": 15}, + "attribute_restrictions": None, # means all I have + "name_form": NAME_FORMAT_URI, + }, + "urn:mace:example.com:saml:roland:sp": { + "lifetime": {"minutes": 5}, + "nameid_format": NAMEID_FORMAT_PERSISTENT, + }, + "https://example.com/sp": { + "lifetime": {"minutes": 5}, + "nameid_format": NAMEID_FORMAT_PERSISTENT, + "name_form": NAME_FORMAT_BASIC + } + }, + "subject_data": full_path("subject_data.db"), + "node_country": "GR" + }, + }, + "debug": 1, + "key_file": full_path("test.key"), + "cert_file": full_path("test.pem"), + "xmlsec_binary": xmlsec_path, + "metadata": [{ + "class": "saml2.mdstore.MetaDataFile", + "metadata": [(full_path("metadata_sp_1.xml"), ), + (full_path("metadata_sp_2.xml"), ), + (full_path("vo_metadata.xml"), )], + }], + "attribute_map_dir": full_path("attributemaps"), + "organization": { + "name": "Exempel AB", + "display_name": [("Exempel AB", "se"), ("Example Co.", "en")], + "url": "http://www.example.com/roland", + }, + "contact_person": [ + { + "given_name": "John", + "sur_name": "Smith", + "email_address": ["john.smith@example.com"], + "contact_type": "technical", + }, + ], +} diff --git a/tests/eidas/sp_conf.py b/tests/eidas/sp_conf.py new file mode 100644 index 000000000..e33a5a2d8 --- /dev/null +++ b/tests/eidas/sp_conf.py @@ -0,0 +1,79 @@ +from pathutils import full_path +from pathutils import xmlsec_path + +CONFIG = { + "entityid": "urn:mace:example.com:saml:roland:sp", + "name": "urn:mace:example.com:saml:roland:sp", + "description": "My own SP", + "service": { + "sp": { + "endpoints": { + "assertion_consumer_service": [ + "http://lingon.catalogix.se:8087/"], + }, + "required_attributes": ["surName", "givenName", "mail"], + "optional_attributes": ["title"], + "idp": ["urn:mace:example.com:saml:roland:idp"], + "requested_attributes": [ + { + "name": "http://eidas.europa.eu/attributes/naturalperson/DateOfBirth", + "required": False, + }, + { + "friendly_name": "PersonIdentifier", + "required": True, + }, + { + "friendly_name": "PlaceOfBirth", + }, + ], + "sp_type": "public", + "sp_type_in_metadata": False, + "force_authn": True, + "node_country": "GR" + } + }, + "debug": 1, + "key_file": full_path("test.key"), + "cert_file": full_path("test.pem"), + "encryption_keypairs": [{"key_file": full_path("test_1.key"), "cert_file": full_path("test_1.crt")}, + {"key_file": full_path("test_2.key"), "cert_file": full_path("test_2.crt")}], + "ca_certs": full_path("cacerts.txt"), + "xmlsec_binary": xmlsec_path, + "metadata": [{ + "class": "saml2.mdstore.MetaDataFile", + "metadata": [(full_path("idp.xml"), ), (full_path("vo_metadata.xml"), )], + }], + "virtual_organization": { + "urn:mace:example.com:it:tek": { + "nameid_format": "urn:oid:1.3.6.1.4.1.1466.115.121.1.15-NameID", + "common_identifier": "umuselin", + } + }, + "subject_data": "subject_data.db", + "accepted_time_diff": 60, + "attribute_map_dir": full_path("attributemaps"), + "valid_for": 6, + "organization": { + "name": ("AB Exempel", "se"), + "display_name": ("AB Exempel", "se"), + "url": "http://www.example.org", + }, + "contact_person": [{ + "given_name": "Roland", + "sur_name": "Hedberg", + "telephone_number": "+46 70 100 0000", + "email_address": ["tech@eample.com", + "tech@example.org"], + "contact_type": "technical" + }, + ], + "logger": { + "rotating": { + "filename": full_path("sp.log"), + "maxBytes": 100000, + "backupCount": 5, + }, + "loglevel": "info", + } +} diff --git a/tests/eidas/test_idp.py b/tests/eidas/test_idp.py new file mode 100644 index 000000000..eeba4cfcb --- /dev/null +++ b/tests/eidas/test_idp.py @@ -0,0 +1,13 @@ +from saml2 import metadata +from saml2.config import IdPConfig + + +class TestIdP: + def setup_class(self): + self.conf = IdPConfig() + self.conf.load_file("idp_conf") + + def test_node_country_in_metadata(self): + entd = metadata.entity_descriptor(self.conf) + assert any(filter(lambda x: x.tag == "NodeCountry", + entd.extensions.extension_elements)) diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py new file mode 100644 index 000000000..38201508d --- /dev/null +++ b/tests/eidas/test_sp.py @@ -0,0 +1,62 @@ +from saml2 import metadata +from saml2 import samlp +from saml2.client import Saml2Client +from saml2.server import Server +from saml2.config import SPConfig +from eidas.sp_conf import CONFIG + + +class TestSP: + def setup_class(self): + self.server = Server("idp_conf") + + self.conf = SPConfig() + self.conf.load_file("sp_conf") + + self.client = Saml2Client(self.conf) + + def teardown_class(self): + self.server.close() + + def test_authn_request_force_authn(self): + req_str = "{0}".format(self.client.create_authn_request( + "http://www.example.com/sso", message_id="id1")[-1]) + req = samlp.authn_request_from_string(req_str) + assert req.force_authn == "true" + + def test_sp_type_only_in_request(self): + entd = metadata.entity_descriptor(self.conf) + req_str = "{0}".format(self.client.create_authn_request( + "http://www.example.com/sso", message_id="id1")[-1]) + req = samlp.authn_request_from_string(req_str) + sp_type_elements = filter(lambda x: x.tag == "SPType", + req.extensions.extension_elements) + assert any(filter(lambda x: x.text == "public", sp_type_elements)) + assert not any(filter(lambda x: x.tag == "SPType", + entd.extensions.extension_elements)) + + def test_sp_type_in_metadata(self): + CONFIG["service"]["sp"]["sp_type_in_metadata"] = True + sconf = SPConfig() + sconf.load(CONFIG) + custom_client = Saml2Client(sconf) + + req_str = "{0}".format(custom_client.create_authn_request( + "http://www.example.com/sso", message_id="id1")[-1]) + req = samlp.authn_request_from_string(req_str) + sp_type_elements = filter(lambda x: x.tag == "SPType", + req.extensions.extension_elements) + assert not any(filter(lambda x: x.text == "public", sp_type_elements)) + + entd = metadata.entity_descriptor(sconf) + assert any(filter(lambda x: x.tag == "SPType", + entd.extensions.extension_elements)) + + def test_node_country_in_metadata(self): + entd = metadata.entity_descriptor(self.conf) + assert any(filter(lambda x: x.tag == "NodeCountry", + entd.extensions.extension_elements)) + + +if __name__ == '__main__': + TestSP() From fded1f6dafd6fd33768650b3eb30e6acfaf8e8a8 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Mon, 13 Jan 2020 20:02:03 +0200 Subject: [PATCH 03/26] Add eIDAS SP config class and validation - Adds validate method in Config class to be used for configuration validation checks - Adds eIDASConfig as base eIDAS config class to host commonly (between IdP and SP) used validations and functions - Adds eIDASSPConfig class and override validate method with endpoint and keydescriptor validations based on eidas v1.2 specs - Adds utility->config module to host config helper classes and functions - Adds new ConfigValidationError for config error signaling - Adds RuleValidator class to be used for config elements validation rule crafting - Adds should_warning and must_error functions for signaling warnings and errors related to element rules using RFC2119 wording --- src/saml2/config.py | 50 ++++++++++++++++++++++++++ src/saml2/utility/__init__.py | 0 src/saml2/utility/config.py | 43 ++++++++++++++++++++++ tests/eidas/test_sp.py | 68 ++++++++++++++++++++++++++++++----- 4 files changed, 153 insertions(+), 8 deletions(-) create mode 100644 src/saml2/utility/__init__.py create mode 100644 src/saml2/utility/config.py diff --git a/src/saml2/config.py b/src/saml2/config.py index 1f423789a..8cb38617a 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -7,6 +7,7 @@ import os import re import sys +from functools import partial import six @@ -21,6 +22,7 @@ from saml2.mdstore import MetadataStore from saml2.saml import NAME_FORMAT_URI from saml2.virtual_org import VirtualOrg +from saml2.utility.config import RuleValidator, should_warning, must_error logger = logging.getLogger(__name__) @@ -542,6 +544,9 @@ def service_per_endpoint(self, context=None): res[endp] = (service, binding) return res + def validate(self): + pass + class SPConfig(Config): def_context = "sp" @@ -571,6 +576,47 @@ def ecp_endpoint(self, ipaddress): return None +class eIDASConfig(Config): + @classmethod + def assert_not_declared(cls, error_signal): + return (lambda x: x is None, + partial(error_signal, message="not be declared")) + + @classmethod + def assert_declared(cls, error_signal): + return (lambda x: x is not None, + partial(error_signal, message="be declared")) + + +class eIDASSPConfig(SPConfig, eIDASConfig): + def validate(self): + validators = [ + RuleValidator( + "single_logout_service", + self._sp_endpoints.get("single_logout_service"), + *self.assert_not_declared(should_warning) + ), + RuleValidator( + "artifact_resolution_service", + self._sp_endpoints.get("artifact_resolution_service"), + *self.assert_not_declared(should_warning) + ), + RuleValidator( + "manage_name_id_service", + self._sp_endpoints.get("manage_name_id_service"), + *self.assert_not_declared(should_warning) + ), + RuleValidator( + "KeyDescriptor", + self.cert_file or self.encryption_keypairs, + *self.assert_declared(must_error) + ) + ] + + for validator in validators: + validator.validate() + + class IdPConfig(Config): def_context = "idp" @@ -578,6 +624,10 @@ def __init__(self): Config.__init__(self) +class eIDASIdPConfig(IdPConfig): + pass + + def config_factory(_type, config): """ diff --git a/src/saml2/utility/__init__.py b/src/saml2/utility/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/saml2/utility/config.py b/src/saml2/utility/config.py new file mode 100644 index 000000000..5b9d4bb4e --- /dev/null +++ b/src/saml2/utility/config.py @@ -0,0 +1,43 @@ +import logging + + +logger = logging.getLogger(__name__) + + +class ConfigValidationError(Exception): + pass + + +class RuleValidator(object): + def __init__(self, element_name, element_value, validator, error_signal): + """ + :param element_name: the name of the element that will be + validated + :param element_value: function to be called + with config as parameter to fetch an element value + :param validator: function to be called + with a config element value as a parameter + :param error_signal: function to be called + with an element name and value to signal an error (can be a log + function, raise an error etc) + """ + self.element_name = element_name + self.element_value = element_value + self.validator = validator + self.error_signal = error_signal + + def validate(self): + if not self.validator(self.element_value): + self.error_signal(self.element_name) + + +def should_warning(element_name, message): + logger.warning("{element} SHOULD {message}".format( + element=element_name, message=message)) + + +def must_error(element_name, message): + error = "{element} MUST {message}".format( + element=element_name, message=message) + logger.error(error) + raise ConfigValidationError(error) diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py index 38201508d..005c486bf 100644 --- a/tests/eidas/test_sp.py +++ b/tests/eidas/test_sp.py @@ -1,16 +1,20 @@ +import pytest +import copy +from saml2 import BINDING_HTTP_POST from saml2 import metadata from saml2 import samlp from saml2.client import Saml2Client from saml2.server import Server -from saml2.config import SPConfig +from saml2.config import eIDASSPConfig from eidas.sp_conf import CONFIG +from saml2.utility.config import ConfigValidationError class TestSP: def setup_class(self): self.server = Server("idp_conf") - self.conf = SPConfig() + self.conf = eIDASSPConfig() self.conf.load_file("sp_conf") self.client = Saml2Client(self.conf) @@ -18,6 +22,10 @@ def setup_class(self): def teardown_class(self): self.server.close() + @pytest.fixture(scope="function") + def config(self): + return copy.deepcopy(CONFIG) + def test_authn_request_force_authn(self): req_str = "{0}".format(self.client.create_authn_request( "http://www.example.com/sso", message_id="id1")[-1]) @@ -35,10 +43,10 @@ def test_sp_type_only_in_request(self): assert not any(filter(lambda x: x.tag == "SPType", entd.extensions.extension_elements)) - def test_sp_type_in_metadata(self): - CONFIG["service"]["sp"]["sp_type_in_metadata"] = True - sconf = SPConfig() - sconf.load(CONFIG) + def test_sp_type_in_metadata(self, config): + config["service"]["sp"]["sp_type_in_metadata"] = True + sconf = eIDASSPConfig() + sconf.load(config) custom_client = Saml2Client(sconf) req_str = "{0}".format(custom_client.create_authn_request( @@ -58,5 +66,49 @@ def test_node_country_in_metadata(self): entd.extensions.extension_elements)) -if __name__ == '__main__': - TestSP() +class TestSPConfig: + @pytest.fixture(scope="function") + def raise_error_on_warning(self, monkeypatch): + def r(*args, **kwargs): + raise ConfigValidationError() + monkeypatch.setattr("saml2.utility.config.logger.warning", r) + + @pytest.fixture(scope="function") + def config(self): + return copy.deepcopy(CONFIG) + + def test_singlelogout_declared(self, config, raise_error_on_warning): + config["service"]["sp"]["endpoints"]["single_logout_service"] = \ + [("https://example.com", BINDING_HTTP_POST)] + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_artifact_resolution_declared(self, config, raise_error_on_warning): + config["service"]["sp"]["endpoints"]["artifact_resolution_service"] = \ + [("https://example.com", BINDING_HTTP_POST)] + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_manage_nameid_service_declared(self, config, raise_error_on_warning): + config["service"]["sp"]["endpoints"]["manage_name_id_service"] = \ + [("https://example.com", BINDING_HTTP_POST)] + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_no_keydescriptor(self, config): + del config["cert_file"] + del config["encryption_keypairs"] + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() From 51810681e3676905e8f488d34489bf37cc6fd897 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Fri, 17 Jan 2020 19:43:36 +0200 Subject: [PATCH 04/26] Add validator for NodeCountry element - Adds requirement iso3166 to lookup if a country code is valid (country exists and code is in ISO 3166-1 alpha-2 format) - Adds validator method in eIDASConfig class (since both IdP and SP MUST have the NodeCountry element) - Adds tests related to NodeCountry validation --- setup.cfg | 1 + src/saml2/config.py | 26 ++++++++++++++++++++++---- tests/eidas/test_sp.py | 16 ++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/setup.cfg b/setup.cfg index 76a0c9580..50a66d4d4 100644 --- a/setup.cfg +++ b/setup.cfg @@ -51,6 +51,7 @@ install_requires = pytz requests >= 1.0.0 six + iso3166 [options.packages.find] diff --git a/src/saml2/config.py b/src/saml2/config.py index 8cb38617a..927d6ef72 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -8,6 +8,7 @@ import re import sys from functools import partial +from iso3166 import countries import six @@ -587,30 +588,47 @@ def assert_declared(cls, error_signal): return (lambda x: x is not None, partial(error_signal, message="be declared")) + @staticmethod + def validate_node_country_format(node_country): + try: + return countries.get(node_country).alpha2 == node_country + except KeyError: + return False + class eIDASSPConfig(SPConfig, eIDASConfig): + def get_endpoint_element(self, element): + return getattr(self, "_sp_endpoints", {}).get(element, None) + def validate(self): validators = [ RuleValidator( "single_logout_service", - self._sp_endpoints.get("single_logout_service"), + self.get_endpoint_element("single_logout_service"), *self.assert_not_declared(should_warning) ), RuleValidator( "artifact_resolution_service", - self._sp_endpoints.get("artifact_resolution_service"), + self.get_endpoint_element("artifact_resolution_service"), *self.assert_not_declared(should_warning) ), RuleValidator( "manage_name_id_service", - self._sp_endpoints.get("manage_name_id_service"), + self.get_endpoint_element("manage_name_id_service"), *self.assert_not_declared(should_warning) ), RuleValidator( "KeyDescriptor", self.cert_file or self.encryption_keypairs, *self.assert_declared(must_error) - ) + ), + RuleValidator( + "node_country", + getattr(self, "_sp_node_country", None), + self.validate_node_country_format, + partial(must_error, + message="be declared in ISO 3166-1 alpha-2 format") + ), ] for validator in validators: diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py index 005c486bf..af0f895bc 100644 --- a/tests/eidas/test_sp.py +++ b/tests/eidas/test_sp.py @@ -112,3 +112,19 @@ def test_no_keydescriptor(self, config): with pytest.raises(ConfigValidationError): conf.validate() + + def test_no_nodecountry(self, config): + del config["service"]["sp"]["node_country"] + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_nodecountry_wrong_format(self, config): + config["service"]["sp"]["node_country"] = "gr" + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() From 1b8bdbdfdc6173ac58e5b03cbf0a3c1ba7c05d7a Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Tue, 21 Jan 2020 13:51:55 +0200 Subject: [PATCH 05/26] Add eIDAS application identifier support in config - Adds support for setting the application identifier based on eIDAS Message Format v1.1-2 document - Adds validator for the existence of the identifier and the correct formatting - Adds tests for configuration validation related to the identifier and test for the existence as an entity attribute element --- src/saml2/config.py | 24 ++++++++++++++++++++++++ src/saml2/metadata.py | 10 ++++++++++ tests/eidas/sp_conf.py | 3 ++- tests/eidas/test_sp.py | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 1 deletion(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index 927d6ef72..b94e64165 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -8,6 +8,7 @@ import re import sys from functools import partial +import re from iso3166 import countries import six @@ -101,6 +102,7 @@ "sp_type_in_metadata", "requested_attributes", "node_country", + "application_identifier" ] AA_IDP_ARGS = [ @@ -123,6 +125,7 @@ "name_qualifier", "edu_person_targeted_id", "node_country", + "application_identifier" ] PDP_ARGS = ["endpoints", "name_form", "name_id_format"] @@ -595,6 +598,14 @@ def validate_node_country_format(node_country): except KeyError: return False + @staticmethod + def validate_application_identifier_format(application_identifier): + if not application_identifier: + return True + + return re.search(r"([a-zA-Z0-9])+:([a-zA-Z0-9():_\-])+:([0-9])+" + r"(\.([0-9])+){1,2}", application_identifier) + class eIDASSPConfig(SPConfig, eIDASConfig): def get_endpoint_element(self, element): @@ -629,6 +640,19 @@ def validate(self): partial(must_error, message="be declared in ISO 3166-1 alpha-2 format") ), + RuleValidator( + "application_identifier", + getattr(self, "_sp_application_identifier", None), + *self.assert_declared(should_warning) + ), + RuleValidator( + "application_identifier", + getattr(self, "_sp_application_identifier", None), + self.validate_application_identifier_format, + partial(must_error, + message="be in the form :" + ":.[.]”") + ) ] for validator in validators: diff --git a/src/saml2/metadata.py b/src/saml2/metadata.py index 10dee84a8..41dcc70af 100644 --- a/src/saml2/metadata.py +++ b/src/saml2/metadata.py @@ -778,6 +778,16 @@ def entity_descriptor(confd): item = node_country.NodeCountry(text=conf_node_country) entd.extensions.add_extension_element(item) + app_identifer = confd.getattr("application_identifier", confd.context) + if app_identifer: + entd.extensions = entd.extensions or md.Extensions() + ava = AttributeValue(text=app_identifer) + attr = Attribute( + attribute_value=ava, + name="http://eidas.europa.eu/entity-attributes/application-identifier" + ) + _add_attr_to_entity_attributes(entd.extensions, attr) + return entd diff --git a/tests/eidas/sp_conf.py b/tests/eidas/sp_conf.py index e33a5a2d8..edfd0ebd7 100644 --- a/tests/eidas/sp_conf.py +++ b/tests/eidas/sp_conf.py @@ -30,7 +30,8 @@ "sp_type": "public", "sp_type_in_metadata": False, "force_authn": True, - "node_country": "GR" + "node_country": "GR", + "application_identifier": "CEF:eIDAS-ref:2.0" } }, "debug": 1, diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py index af0f895bc..4c6a87a91 100644 --- a/tests/eidas/test_sp.py +++ b/tests/eidas/test_sp.py @@ -65,6 +65,19 @@ def test_node_country_in_metadata(self): assert any(filter(lambda x: x.tag == "NodeCountry", entd.extensions.extension_elements)) + def test_application_identifier_in_metadata(self): + entd = metadata.entity_descriptor(self.conf) + entity_attributes = next(filter(lambda x: x.tag == "EntityAttributes", + entd.extensions.extension_elements)) + app_identifier = [ + x for x in entity_attributes.children + if x.attributes["Name"] == + "http://eidas.europa.eu/entity-attributes/application-identifier" + ] + assert len(app_identifier) == 1 + assert self.conf._sp_application_identifier \ + == next(x.text for y in app_identifier for x in y.children) + class TestSPConfig: @pytest.fixture(scope="function") @@ -128,3 +141,26 @@ def test_nodecountry_wrong_format(self, config): with pytest.raises(ConfigValidationError): conf.validate() + + def test_no_application_identifier_warning(self, config, raise_error_on_warning): + del config["service"]["sp"]["application_identifier"] + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_application_identifier_wrong_format(self, config): + config["service"]["sp"]["application_identifier"] = "TEST:Node.1" + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_application_identifier_ok_format(self, config): + conf = eIDASSPConfig() + conf.load(config) + conf.validate() From faba67f663975d32f6dfb106d88100e934051c9c Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Fri, 24 Jan 2020 11:03:31 +0200 Subject: [PATCH 06/26] Add eIDAs protocol version suppert in config - Adds support for setting the protocol versions supported based on eIDAS Message Format v1.1-2 document - Adds validator for the existence of the protocol version and issues a warning if it doesn't exist - Adds tests for configuration validation and for the existence of protocol version as an entity attribute element - Adds helper functions `make_type` and `make_list` to make parsing the protocol version easier regardless of it being a single element or a list of elements --- src/saml2/config.py | 9 +++++++- src/saml2/metadata.py | 11 ++++++++++ src/saml2/utility/__init__.py | 10 +++++++++ tests/eidas/sp_conf.py | 3 ++- tests/eidas/test_sp.py | 41 ++++++++++++++++++++++++++++++++++- 5 files changed, 71 insertions(+), 3 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index b94e64165..b1974cdd9 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -102,7 +102,8 @@ "sp_type_in_metadata", "requested_attributes", "node_country", - "application_identifier" + "application_identifier", + "protocol_version" ] AA_IDP_ARGS = [ @@ -126,6 +127,7 @@ "edu_person_targeted_id", "node_country", "application_identifier" + "protocol_version" ] PDP_ARGS = ["endpoints", "name_form", "name_id_format"] @@ -652,6 +654,11 @@ def validate(self): partial(must_error, message="be in the form :" ":.[.]”") + ), + RuleValidator( + "protocol_version", + getattr(self, "_sp_protocol_version", None), + *self.assert_declared(should_warning) ) ] diff --git a/src/saml2/metadata.py b/src/saml2/metadata.py index 41dcc70af..5108dd742 100644 --- a/src/saml2/metadata.py +++ b/src/saml2/metadata.py @@ -21,6 +21,7 @@ from saml2 import BINDING_SOAP from saml2 import samlp from saml2 import class_name +from saml2.utility import make_list from saml2 import xmldsig as ds import six @@ -788,6 +789,16 @@ def entity_descriptor(confd): ) _add_attr_to_entity_attributes(entd.extensions, attr) + protocol_version = confd.getattr("protocol_version", confd.context) + if protocol_version: + entd.extensions = entd.extensions or md.Extensions() + ava = [AttributeValue(text=str(c)) for c in make_list(protocol_version)] + attr = Attribute( + attribute_value=ava, + name="http://eidas.europa.eu/entity-attributes/protocol-version" + ) + _add_attr_to_entity_attributes(entd.extensions, attr) + return entd diff --git a/src/saml2/utility/__init__.py b/src/saml2/utility/__init__.py index e69de29bb..5b4f1a9cd 100644 --- a/src/saml2/utility/__init__.py +++ b/src/saml2/utility/__init__.py @@ -0,0 +1,10 @@ +def make_type(mtype, *args): + t_args = [] + similar_type = tuple if mtype is list else list + for x in args: + t_args.extend([x] if not isinstance(x, (list, tuple)) else similar_type(x)) + return mtype(t_args) + + +def make_list(*args): + return make_type(list, *args) diff --git a/tests/eidas/sp_conf.py b/tests/eidas/sp_conf.py index edfd0ebd7..5ebd3e3be 100644 --- a/tests/eidas/sp_conf.py +++ b/tests/eidas/sp_conf.py @@ -31,7 +31,8 @@ "sp_type_in_metadata": False, "force_authn": True, "node_country": "GR", - "application_identifier": "CEF:eIDAS-ref:2.0" + "application_identifier": "CEF:eIDAS-ref:2.0", + "protocol_version": [1.1, 2.2] } }, "debug": 1, diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py index 4c6a87a91..b2c8875a0 100644 --- a/tests/eidas/test_sp.py +++ b/tests/eidas/test_sp.py @@ -71,13 +71,43 @@ def test_application_identifier_in_metadata(self): entd.extensions.extension_elements)) app_identifier = [ x for x in entity_attributes.children - if x.attributes["Name"] == + if getattr(x, "attributes", {}).get("Name") == "http://eidas.europa.eu/entity-attributes/application-identifier" ] assert len(app_identifier) == 1 assert self.conf._sp_application_identifier \ == next(x.text for y in app_identifier for x in y.children) + def test_multiple_protocol_version_in_metadata(self): + entd = metadata.entity_descriptor(self.conf) + entity_attributes = next(filter(lambda x: x.tag == "EntityAttributes", + entd.extensions.extension_elements)) + protocol_version = next( + x for x in entity_attributes.children + if getattr(x, "name", "") == + "http://eidas.europa.eu/entity-attributes/protocol-version" + ) + assert len(protocol_version.attribute_value) == 2 + assert set(str(x) for x in self.conf._sp_protocol_version) \ + == set([x.text for x in protocol_version.attribute_value]) + + def test_protocol_version_in_metadata(self, config): + config["service"]["sp"]["protocol_version"] = 1.2 + + conf = eIDASSPConfig() + conf.load(config) + + entd = metadata.entity_descriptor(conf) + entity_attributes = next(filter(lambda x: x.tag == "EntityAttributes", + entd.extensions.extension_elements)) + protocol_version = next( + x for x in entity_attributes.children + if getattr(x, "name", "") == + "http://eidas.europa.eu/entity-attributes/protocol-version" + ) + assert len(protocol_version.attribute_value) == 1 + assert {str(conf._sp_protocol_version)} \ + == set([x.text for x in protocol_version.attribute_value]) class TestSPConfig: @pytest.fixture(scope="function") @@ -164,3 +194,12 @@ def test_application_identifier_ok_format(self, config): conf = eIDASSPConfig() conf.load(config) conf.validate() + + def test_no_protocol_version_warning(self, config, raise_error_on_warning): + del config["service"]["sp"]["protocol_version"] + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() From 4e3aeb792f4fcf2440e3f370fee6113f99d205ef Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Fri, 31 Jan 2020 14:38:54 +0200 Subject: [PATCH 07/26] Refactor warning/error config checks format - Removes RuleValidator class and uses a dict data structure with key as error message and value as the check - Extracts [endpoint element/protocol version/application identifier/node country] fetching to methods since they are part of the eIDASConfig and will be overriden by the eIDASIdPConfig --- src/saml2/config.py | 113 ++++++++++++++++++------------------ src/saml2/utility/config.py | 41 ------------- tests/eidas/test_sp.py | 3 +- 3 files changed, 58 insertions(+), 99 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index b1974cdd9..e932f07c3 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -24,7 +24,7 @@ from saml2.mdstore import MetadataStore from saml2.saml import NAME_FORMAT_URI from saml2.virtual_org import VirtualOrg -from saml2.utility.config import RuleValidator, should_warning, must_error +from saml2.utility.config import ConfigValidationError logger = logging.getLogger(__name__) @@ -583,15 +583,17 @@ def ecp_endpoint(self, ipaddress): class eIDASConfig(Config): - @classmethod - def assert_not_declared(cls, error_signal): - return (lambda x: x is None, - partial(error_signal, message="not be declared")) + def get_endpoint_element(self, element): + pass + + def get_protocol_version(self): + pass - @classmethod - def assert_declared(cls, error_signal): - return (lambda x: x is not None, - partial(error_signal, message="be declared")) + def get_application_identifier(self): + pass + + def get_node_country(self): + pass @staticmethod def validate_node_country_format(node_country): @@ -613,57 +615,54 @@ class eIDASSPConfig(SPConfig, eIDASConfig): def get_endpoint_element(self, element): return getattr(self, "_sp_endpoints", {}).get(element, None) + def get_application_identifier(self): + return getattr(self, "_sp_application_identifier", None) + + def get_protocol_version(self): + return getattr(self, "_sp_protocol_version", None) + + def get_node_country(self): + return getattr(self, "_sp_node_country", None) + def validate(self): - validators = [ - RuleValidator( - "single_logout_service", - self.get_endpoint_element("single_logout_service"), - *self.assert_not_declared(should_warning) - ), - RuleValidator( - "artifact_resolution_service", - self.get_endpoint_element("artifact_resolution_service"), - *self.assert_not_declared(should_warning) - ), - RuleValidator( - "manage_name_id_service", - self.get_endpoint_element("manage_name_id_service"), - *self.assert_not_declared(should_warning) - ), - RuleValidator( - "KeyDescriptor", - self.cert_file or self.encryption_keypairs, - *self.assert_declared(must_error) - ), - RuleValidator( - "node_country", - getattr(self, "_sp_node_country", None), - self.validate_node_country_format, - partial(must_error, - message="be declared in ISO 3166-1 alpha-2 format") - ), - RuleValidator( - "application_identifier", - getattr(self, "_sp_application_identifier", None), - *self.assert_declared(should_warning) - ), - RuleValidator( - "application_identifier", - getattr(self, "_sp_application_identifier", None), - self.validate_application_identifier_format, - partial(must_error, - message="be in the form :" - ":.[.]”") - ), - RuleValidator( - "protocol_version", - getattr(self, "_sp_protocol_version", None), - *self.assert_declared(should_warning) + warning_validators = { + "single_logout_service SHOULD NOT be declared": + self.get_endpoint_element("single_logout_service") is None, + "artifact_resolution_service SHOULD NOT be declared": + self.get_endpoint_element("artifact_resolution_service") is None, + "manage_name_id_service SHOULD NOT be declared": + self.get_endpoint_element("manage_name_id_service") is None, + "application_identifier SHOULD be declared": + self.get_application_identifier() is not None, + "protocol_version SHOULD be declared": + self.get_protocol_version() is not None, + } + + if not all(warning_validators.values()): + logger.warning( + "Configuration validation warnings occurred: {}".format( + [msg for msg, check in warning_validators.items() + if check is not True] + ) ) - ] - for validator in validators: - validator.validate() + error_validators = { + "KeyDescriptor MUST be declared": + self.cert_file or self.encryption_keypairs, + "node_country MUST be declared in ISO 3166-1 alpha-2 format": + self.validate_node_country_format(self.get_node_country()), + "application_identifier MUST be in the form ::.[.]": + self.validate_application_identifier_format( + self.get_application_identifier()) + } + + if not all(error_validators.values()): + error = "Configuration validation errors occurred:".format( + [msg for msg, check in error_validators.items() + if check is not True]) + logger.error(error) + raise ConfigValidationError(error) class IdPConfig(Config): diff --git a/src/saml2/utility/config.py b/src/saml2/utility/config.py index 5b9d4bb4e..d5b9aeb09 100644 --- a/src/saml2/utility/config.py +++ b/src/saml2/utility/config.py @@ -1,43 +1,2 @@ -import logging - - -logger = logging.getLogger(__name__) - - class ConfigValidationError(Exception): pass - - -class RuleValidator(object): - def __init__(self, element_name, element_value, validator, error_signal): - """ - :param element_name: the name of the element that will be - validated - :param element_value: function to be called - with config as parameter to fetch an element value - :param validator: function to be called - with a config element value as a parameter - :param error_signal: function to be called - with an element name and value to signal an error (can be a log - function, raise an error etc) - """ - self.element_name = element_name - self.element_value = element_value - self.validator = validator - self.error_signal = error_signal - - def validate(self): - if not self.validator(self.element_value): - self.error_signal(self.element_name) - - -def should_warning(element_name, message): - logger.warning("{element} SHOULD {message}".format( - element=element_name, message=message)) - - -def must_error(element_name, message): - error = "{element} MUST {message}".format( - element=element_name, message=message) - logger.error(error) - raise ConfigValidationError(error) diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py index b2c8875a0..1ec2e7dee 100644 --- a/tests/eidas/test_sp.py +++ b/tests/eidas/test_sp.py @@ -109,12 +109,13 @@ def test_protocol_version_in_metadata(self, config): assert {str(conf._sp_protocol_version)} \ == set([x.text for x in protocol_version.attribute_value]) + class TestSPConfig: @pytest.fixture(scope="function") def raise_error_on_warning(self, monkeypatch): def r(*args, **kwargs): raise ConfigValidationError() - monkeypatch.setattr("saml2.utility.config.logger.warning", r) + monkeypatch.setattr("saml2.config.logger.warning", r) @pytest.fixture(scope="function") def config(self): From c8c7864e4a9aeb0749a8e3b8244b34c48fc1287c Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Fri, 31 Jan 2020 21:08:16 +0200 Subject: [PATCH 08/26] Add organization and contact person validations - Adds validation for organization info (name or display name or url) and triggers a warning if none is present - Adds support and technical contact person validation (both should exist and have an email address) and triggers a warning if not both are present - Adds `not_empty` function to do a shallow truthiness check (aka nested values like [{}] will be considered not empty) - Adds test for the above scenarios --- src/saml2/config.py | 26 ++++++- src/saml2/utility/__init__.py | 9 +++ tests/eidas/sp_conf.py | 24 ++++-- tests/eidas/test_sp.py | 134 +++++++++++++++++++++++++++++++++- 4 files changed, 182 insertions(+), 11 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index e932f07c3..a8207dfaa 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -24,6 +24,7 @@ from saml2.mdstore import MetadataStore from saml2.saml import NAME_FORMAT_URI from saml2.virtual_org import VirtualOrg +from saml2.utility import not_empty from saml2.utility.config import ConfigValidationError logger = logging.getLogger(__name__) @@ -610,6 +611,15 @@ def validate_application_identifier_format(application_identifier): return re.search(r"([a-zA-Z0-9])+:([a-zA-Z0-9():_\-])+:([0-9])+" r"(\.([0-9])+){1,2}", application_identifier) + @staticmethod + def get_type_contact_person(contacts, ctype): + return [contact for contact in contacts + if contact.get("contact_type") == ctype] + + @staticmethod + def contact_has_email_address(contact): + return not_empty(contact.get("email_address")) + class eIDASSPConfig(SPConfig, eIDASConfig): def get_endpoint_element(self, element): @@ -633,9 +643,21 @@ def validate(self): "manage_name_id_service SHOULD NOT be declared": self.get_endpoint_element("manage_name_id_service") is None, "application_identifier SHOULD be declared": - self.get_application_identifier() is not None, + not_empty(self.get_application_identifier()), "protocol_version SHOULD be declared": - self.get_protocol_version() is not None, + not_empty(self.get_protocol_version()), + "minimal organization info (name/dname/url) SHOULD be declared": + not_empty(self.organization), + "contact_person with contact_type 'technical' and at least one " + "email_address SHOULD be declared": + any(filter(self.contact_has_email_address, + self.get_type_contact_person(self.contact_person, + ctype="technical"))), + "contact_person with contact_type 'support' and at least one " + "email_address SHOULD be declared": + any(filter(self.contact_has_email_address, + self.get_type_contact_person(self.contact_person, + ctype="support"))) } if not all(warning_validators.values()): diff --git a/src/saml2/utility/__init__.py b/src/saml2/utility/__init__.py index 5b4f1a9cd..7fe70232c 100644 --- a/src/saml2/utility/__init__.py +++ b/src/saml2/utility/__init__.py @@ -8,3 +8,12 @@ def make_type(mtype, *args): def make_list(*args): return make_type(list, *args) + + +def not_empty(element): + if isinstance(element, bool): + return True + + if element: + return True + return False diff --git a/tests/eidas/sp_conf.py b/tests/eidas/sp_conf.py index 5ebd3e3be..fd19fe311 100644 --- a/tests/eidas/sp_conf.py +++ b/tests/eidas/sp_conf.py @@ -61,14 +61,22 @@ "display_name": ("AB Exempel", "se"), "url": "http://www.example.org", }, - "contact_person": [{ - "given_name": "Roland", - "sur_name": "Hedberg", - "telephone_number": "+46 70 100 0000", - "email_address": ["tech@eample.com", - "tech@example.org"], - "contact_type": "technical" - }, + "contact_person": [ + { + "given_name": "Roland", + "sur_name": "Hedberg", + "telephone_number": "+46 70 100 0000", + "email_address": ["tech@eample.com", + "tech@example.org"], + "contact_type": "technical" + }, + { + "given_name": "Roland", + "sur_name": "Hedberg", + "telephone_number": "+46 70 100 0000", + "email_address": ["tech@eample.com", + "tech@example.org"], + "contact_type": "support"} ], "logger": { "rotating": { diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py index 1ec2e7dee..b81af1251 100644 --- a/tests/eidas/test_sp.py +++ b/tests/eidas/test_sp.py @@ -111,6 +111,20 @@ def test_protocol_version_in_metadata(self, config): class TestSPConfig: + @pytest.fixture(scope="function") + def technical_contacts(self, config): + return [ + x for x in config["contact_person"] + if x["contact_type"] == "technical" + ] + + @pytest.fixture(scope="function") + def support_contacts(self, config): + return [ + x for x in config["contact_person"] + if x["contact_type"] == "support" + ] + @pytest.fixture(scope="function") def raise_error_on_warning(self, monkeypatch): def r(*args, **kwargs): @@ -182,6 +196,15 @@ def test_no_application_identifier_warning(self, config, raise_error_on_warning) with pytest.raises(ConfigValidationError): conf.validate() + def test_empty_application_identifier_warning(self, config, raise_error_on_warning): + config["service"]["sp"]["application_identifier"] = "" + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + def test_application_identifier_wrong_format(self, config): config["service"]["sp"]["application_identifier"] = "TEST:Node.1" @@ -191,7 +214,7 @@ def test_application_identifier_wrong_format(self, config): with pytest.raises(ConfigValidationError): conf.validate() - def test_application_identifier_ok_format(self, config): + def test_application_identifier_ok_format(self, config, raise_error_on_warning): conf = eIDASSPConfig() conf.load(config) conf.validate() @@ -204,3 +227,112 @@ def test_no_protocol_version_warning(self, config, raise_error_on_warning): with pytest.raises(ConfigValidationError): conf.validate() + + def test_empty_protocol_version_warning(self, config, raise_error_on_warning): + config["service"]["sp"]["protocol_version"] = "" + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_no_organization_info_warning(self, config, raise_error_on_warning): + del config["organization"] + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_empty_organization_info_warning(self, config, raise_error_on_warning): + config["organization"] = {} + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_no_technical_contact_person(self, + config, + technical_contacts, + raise_error_on_warning): + for contact in technical_contacts: + contact["contact_type"] = "other" + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_technical_contact_person_no_email(self, + config, + technical_contacts, + raise_error_on_warning): + + for contact in technical_contacts: + del contact["email_address"] + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_technical_contact_person_empty_email(self, + config, + technical_contacts, + raise_error_on_warning): + + for contact in technical_contacts: + del contact["email_address"] + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_no_support_contact_person(self, + config, + support_contacts, + raise_error_on_warning): + for contact in support_contacts: + contact["contact_type"] = "other" + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_support_contact_person_no_email(self, + config, + support_contacts, + raise_error_on_warning): + + for contact in support_contacts: + del contact["email_address"] + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() + + def test_support_contact_person_empty_email(self, + config, + support_contacts, + raise_error_on_warning): + + for contact in support_contacts: + del contact["email_address"] + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() From cbaaa8fd5cb93699d7d1a89edb27bf744b7df306 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Mon, 3 Feb 2020 13:28:24 +0200 Subject: [PATCH 09/26] Add SP config validation for https entityid - Adds check that entityid is an https url based on eIDAS SAML Message Format v.1.2. There's no way of really validating that the url is correct (there are many scenarios for false positives and negatives) so we only check that the url scheme is https as the specs define - Adds test for the above rule --- src/saml2/config.py | 6 +++++- tests/eidas/sp_conf.py | 2 +- tests/eidas/test_sp.py | 9 +++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index a8207dfaa..b9612b1bf 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -9,6 +9,7 @@ import sys from functools import partial import re +from urllib import parse from iso3166 import countries import six @@ -676,7 +677,10 @@ def validate(self): "application_identifier MUST be in the form ::.[.]": self.validate_application_identifier_format( - self.get_application_identifier()) + self.get_application_identifier()), + "entityid MUST be an HTTPS URL pointing to the location of its published " + "metadata": + parse.urlparse(self.entityid).scheme == "https" } if not all(error_validators.values()): diff --git a/tests/eidas/sp_conf.py b/tests/eidas/sp_conf.py index fd19fe311..84281ab56 100644 --- a/tests/eidas/sp_conf.py +++ b/tests/eidas/sp_conf.py @@ -2,7 +2,7 @@ from pathutils import xmlsec_path CONFIG = { - "entityid": "urn:mace:example.com:saml:roland:sp", + "entityid": "https://example.org", "name": "urn:mace:example.com:saml:roland:sp", "description": "My own SP", "service": { diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py index b81af1251..3e117c075 100644 --- a/tests/eidas/test_sp.py +++ b/tests/eidas/test_sp.py @@ -336,3 +336,12 @@ def test_support_contact_person_empty_email(self, with pytest.raises(ConfigValidationError): conf.validate() + + def test_entityid_no_https(self, config): + config["entityid"] = "urn:mace:example.com:saml:roland:idp" + + conf = eIDASSPConfig() + conf.load(config) + + with pytest.raises(ConfigValidationError): + conf.validate() From c84a36d36b0c0a6b5ea490670dee85db93a4c9f6 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Thu, 6 Feb 2020 12:28:06 +0200 Subject: [PATCH 10/26] Add SP AuthnRequestsSigned validation - Adds validation check to verify that the eIDAS SP has set the authn_requests_signed to True - Adds tests for the above scenario - Extract `assert_validation_errror` test function for config validation tests --- src/saml2/config.py | 4 +- tests/eidas/sp_conf.py | 1 + tests/eidas/test_sp.py | 136 +++++++++++------------------------------ 3 files changed, 41 insertions(+), 100 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index b9612b1bf..ef2fbfbd7 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -680,7 +680,9 @@ def validate(self): self.get_application_identifier()), "entityid MUST be an HTTPS URL pointing to the location of its published " "metadata": - parse.urlparse(self.entityid).scheme == "https" + parse.urlparse(self.entityid).scheme == "https", + "authn_requests_signed MUST be set to True": + getattr(self, "_sp_authn_requests_signed", None) is True } if not all(error_validators.values()): diff --git a/tests/eidas/sp_conf.py b/tests/eidas/sp_conf.py index 84281ab56..b28b1836d 100644 --- a/tests/eidas/sp_conf.py +++ b/tests/eidas/sp_conf.py @@ -30,6 +30,7 @@ "sp_type": "public", "sp_type_in_metadata": False, "force_authn": True, + "authn_requests_signed": True, "node_country": "GR", "application_identifier": "CEF:eIDAS-ref:2.0", "protocol_version": [1.1, 2.2] diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py index 3e117c075..a7275ca55 100644 --- a/tests/eidas/test_sp.py +++ b/tests/eidas/test_sp.py @@ -111,6 +111,13 @@ def test_protocol_version_in_metadata(self, config): class TestSPConfig: + @staticmethod + def assert_validation_error(config): + conf = eIDASSPConfig() + conf.load(config) + with pytest.raises(ConfigValidationError): + conf.validate() + @pytest.fixture(scope="function") def technical_contacts(self, config): return [ @@ -138,81 +145,45 @@ def config(self): def test_singlelogout_declared(self, config, raise_error_on_warning): config["service"]["sp"]["endpoints"]["single_logout_service"] = \ [("https://example.com", BINDING_HTTP_POST)] - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_artifact_resolution_declared(self, config, raise_error_on_warning): config["service"]["sp"]["endpoints"]["artifact_resolution_service"] = \ [("https://example.com", BINDING_HTTP_POST)] - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_manage_nameid_service_declared(self, config, raise_error_on_warning): config["service"]["sp"]["endpoints"]["manage_name_id_service"] = \ [("https://example.com", BINDING_HTTP_POST)] - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_no_keydescriptor(self, config): del config["cert_file"] del config["encryption_keypairs"] - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_no_nodecountry(self, config): del config["service"]["sp"]["node_country"] - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_nodecountry_wrong_format(self, config): config["service"]["sp"]["node_country"] = "gr" - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_no_application_identifier_warning(self, config, raise_error_on_warning): del config["service"]["sp"]["application_identifier"] - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_empty_application_identifier_warning(self, config, raise_error_on_warning): config["service"]["sp"]["application_identifier"] = "" - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_application_identifier_wrong_format(self, config): config["service"]["sp"]["application_identifier"] = "TEST:Node.1" - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_application_identifier_ok_format(self, config, raise_error_on_warning): conf = eIDASSPConfig() @@ -222,38 +193,22 @@ def test_application_identifier_ok_format(self, config, raise_error_on_warning): def test_no_protocol_version_warning(self, config, raise_error_on_warning): del config["service"]["sp"]["protocol_version"] - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_empty_protocol_version_warning(self, config, raise_error_on_warning): config["service"]["sp"]["protocol_version"] = "" - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_no_organization_info_warning(self, config, raise_error_on_warning): del config["organization"] - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_empty_organization_info_warning(self, config, raise_error_on_warning): config["organization"] = {} - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_no_technical_contact_person(self, config, @@ -262,11 +217,7 @@ def test_no_technical_contact_person(self, for contact in technical_contacts: contact["contact_type"] = "other" - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_technical_contact_person_no_email(self, config, @@ -276,11 +227,7 @@ def test_technical_contact_person_no_email(self, for contact in technical_contacts: del contact["email_address"] - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_technical_contact_person_empty_email(self, config, @@ -290,11 +237,7 @@ def test_technical_contact_person_empty_email(self, for contact in technical_contacts: del contact["email_address"] - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_no_support_contact_person(self, config, @@ -303,11 +246,7 @@ def test_no_support_contact_person(self, for contact in support_contacts: contact["contact_type"] = "other" - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_support_contact_person_no_email(self, config, @@ -317,11 +256,7 @@ def test_support_contact_person_no_email(self, for contact in support_contacts: del contact["email_address"] - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_support_contact_person_empty_email(self, config, @@ -331,17 +266,20 @@ def test_support_contact_person_empty_email(self, for contact in support_contacts: del contact["email_address"] - conf = eIDASSPConfig() - conf.load(config) - - with pytest.raises(ConfigValidationError): - conf.validate() + self.assert_validation_error(config) def test_entityid_no_https(self, config): config["entityid"] = "urn:mace:example.com:saml:roland:idp" - conf = eIDASSPConfig() - conf.load(config) + self.assert_validation_error(config) + + def test_authn_requests_signed_false(self, config): + config["service"]["sp"]["authn_requests_signed"] = False + + self.assert_validation_error(config) + + def test_authn_requests_signed_unassigned(self, config): + del config["service"]["sp"]["authn_requests_signed"] + + self.assert_validation_error(config) - with pytest.raises(ConfigValidationError): - conf.validate() From ff8eba76750f99d896e23dac1a39c832306da5e9 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Fri, 7 Feb 2020 17:28:46 +0200 Subject: [PATCH 11/26] Add eIDAS SP config sp_type validation - Adds validation check for eIDAS SP to verify that sp_type has been set (MUST be set) and that it is set to a valid value (private/public) as stated in eIDAS SAML Message Format v.1.2 spec - Adds tests for the aforementioned checks --- src/saml2/config.py | 4 +++- tests/eidas/test_sp.py | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index ef2fbfbd7..d449617be 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -682,7 +682,9 @@ def validate(self): "metadata": parse.urlparse(self.entityid).scheme == "https", "authn_requests_signed MUST be set to True": - getattr(self, "_sp_authn_requests_signed", None) is True + getattr(self, "_sp_authn_requests_signed", None) is True, + "sp_type MUST be set to 'public' or 'private'": + getattr(self, "_sp_sp_type", None) in ("public", "private") } if not all(error_validators.values()): diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py index a7275ca55..51a9ed6ec 100644 --- a/tests/eidas/test_sp.py +++ b/tests/eidas/test_sp.py @@ -283,3 +283,13 @@ def test_authn_requests_signed_unassigned(self, config): self.assert_validation_error(config) + def test_sp_type_undeclared(self, config): + del config["service"]["sp"]["sp_type"] + + self.assert_validation_error(config) + + def test_sp_type_invalid_value(self, config): + config["service"]["sp"]["sp_type"] = "test value" + + self.assert_validation_error(config) + From 651284d53a8df761b9b619650244fa2d52276aa9 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Fri, 7 Feb 2020 18:29:29 +0200 Subject: [PATCH 12/26] Extract warning and error validators to eIDASConfig - Extracts common (for SP/IdP) warning and error validators - Moves eIDASSPConfig validate method to eIDASConfig class --- src/saml2/config.py | 81 +++++++++++++++++++++++++++------------------ 1 file changed, 48 insertions(+), 33 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index d449617be..48137e3e8 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -621,22 +621,9 @@ def get_type_contact_person(contacts, ctype): def contact_has_email_address(contact): return not_empty(contact.get("email_address")) - -class eIDASSPConfig(SPConfig, eIDASConfig): - def get_endpoint_element(self, element): - return getattr(self, "_sp_endpoints", {}).get(element, None) - - def get_application_identifier(self): - return getattr(self, "_sp_application_identifier", None) - - def get_protocol_version(self): - return getattr(self, "_sp_protocol_version", None) - - def get_node_country(self): - return getattr(self, "_sp_node_country", None) - - def validate(self): - warning_validators = { + @property + def warning_validators(self): + return { "single_logout_service SHOULD NOT be declared": self.get_endpoint_element("single_logout_service") is None, "artifact_resolution_service SHOULD NOT be declared": @@ -661,15 +648,9 @@ def validate(self): ctype="support"))) } - if not all(warning_validators.values()): - logger.warning( - "Configuration validation warnings occurred: {}".format( - [msg for msg, check in warning_validators.items() - if check is not True] - ) - ) - - error_validators = { + @property + def error_validators(self): + return { "KeyDescriptor MUST be declared": self.cert_file or self.encryption_keypairs, "node_country MUST be declared in ISO 3166-1 alpha-2 format": @@ -680,21 +661,55 @@ def validate(self): self.get_application_identifier()), "entityid MUST be an HTTPS URL pointing to the location of its published " "metadata": - parse.urlparse(self.entityid).scheme == "https", - "authn_requests_signed MUST be set to True": - getattr(self, "_sp_authn_requests_signed", None) is True, - "sp_type MUST be set to 'public' or 'private'": - getattr(self, "_sp_sp_type", None) in ("public", "private") + parse.urlparse(self.entityid).scheme == "https" } - if not all(error_validators.values()): + def validate(self): + if not all(self.warning_validators.values()): + logger.warning( + "Configuration validation warnings occurred: {}".format( + [msg for msg, check in self.warning_validators.items() + if check is not True] + ) + ) + + if not all(self.error_validators.values()): error = "Configuration validation errors occurred:".format( - [msg for msg, check in error_validators.items() - if check is not True]) + [msg for msg, check in self.error_validators.items() + if check is not True]) logger.error(error) raise ConfigValidationError(error) +class eIDASSPConfig(SPConfig, eIDASConfig): + def get_endpoint_element(self, element): + return getattr(self, "_sp_endpoints", {}).get(element, None) + + def get_application_identifier(self): + return getattr(self, "_sp_application_identifier", None) + + def get_protocol_version(self): + return getattr(self, "_sp_protocol_version", None) + + def get_node_country(self): + return getattr(self, "_sp_node_country", None) + + @property + def warning_validators(self): + sp_warning_validators = {} + return {**super().warning_validators, **sp_warning_validators} + + @property + def error_validators(self): + sp_error_validators = { + "authn_requests_signed MUST be set to True": + getattr(self, "_sp_authn_requests_signed", None) is True, + "sp_type MUST be set to 'public' or 'private'": + getattr(self, "_sp_sp_type", None) in ("public", "private") + } + return {**super().error_validators, **sp_error_validators} + + class IdPConfig(Config): def_context = "idp" From 851882274a88b99fc8491687b3b5bfcc2d89e360 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Mon, 10 Feb 2020 13:05:03 +0200 Subject: [PATCH 13/26] Fix validator report printing and filtering - Prints the errors that happened during validation (was only printing a message not the actual errors) - Sets all validators to return True/False instead of values. This ensures consistency for the validators structure and also solves issues were when there were errors/warnings printed out validators that weren't returning True/False as values were also printed as errors in the report (specifically the KeyDescriptor check and the application identifier format check) --- src/saml2/config.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index 48137e3e8..668798e22 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -606,11 +606,11 @@ def validate_node_country_format(node_country): @staticmethod def validate_application_identifier_format(application_identifier): - if not application_identifier: + pattern_match = re.search(r"([a-zA-Z0-9])+:([a-zA-Z0-9():_\-])+:([0-9])+" + r"(\.([0-9])+){1,2}", application_identifier) + if not application_identifier or pattern_match: return True - - return re.search(r"([a-zA-Z0-9])+:([a-zA-Z0-9():_\-])+:([0-9])+" - r"(\.([0-9])+){1,2}", application_identifier) + return False @staticmethod def get_type_contact_person(contacts, ctype): @@ -652,7 +652,7 @@ def warning_validators(self): def error_validators(self): return { "KeyDescriptor MUST be declared": - self.cert_file or self.encryption_keypairs, + not_empty(self.cert_file or self.encryption_keypairs), "node_country MUST be declared in ISO 3166-1 alpha-2 format": self.validate_node_country_format(self.get_node_country()), "application_identifier MUST be in the form : Date: Mon, 10 Feb 2020 13:18:02 +0200 Subject: [PATCH 14/26] Adds multiple validators for eIDAS IdP config - Adds inherited validators for eIDAS IdP config. Specifically the following rules related to the metadata: 1. entityid MUST be HTTPs URL 2. SingleLogoutElementService SHOULD NOT be declared 3. ArtifactResolutionService SHOULD NOT be declared 4. ManageNameIDService SHOULD NOT be declared 5. KeyDescriptor MUST be declared 6. Organization with minimal info (name/display name/url) SHOULD be provided 7. Contact Person of contactType technical with an email address SHOULD be provided 8. Contact Person of contactType support with an email address SHOULD be provided 9. eIDAS protocol version implemented SHOULD be provided 10. eIDAS application identifier SHOULD be provided and MUST have a specific format 11. Node country information MUST be declared and MUST follow the ISO 3166-1 alpha-2 format All the above are specified in eIDAS SAML Message Format v.1.2 spec document. --- src/saml2/config.py | 26 ++++- tests/eidas/idp_conf.py | 32 +++--- tests/eidas/test_idp.py | 233 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 274 insertions(+), 17 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index 668798e22..2168fd40f 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -128,7 +128,7 @@ "name_qualifier", "edu_person_targeted_id", "node_country", - "application_identifier" + "application_identifier", "protocol_version" ] @@ -717,8 +717,28 @@ def __init__(self): Config.__init__(self) -class eIDASIdPConfig(IdPConfig): - pass +class eIDASIdPConfig(IdPConfig, eIDASConfig): + def get_endpoint_element(self, element): + return getattr(self, "_idp_endpoints", {}).get(element, None) + + def get_application_identifier(self): + return getattr(self, "_idp_application_identifier", None) + + def get_protocol_version(self): + return getattr(self, "_idp_protocol_version", None) + + def get_node_country(self): + return getattr(self, "_idp_node_country", None) + + @property + def warning_validators(self): + idp_warning_validators = {} + return {**super().warning_validators, **idp_warning_validators} + + @property + def error_validators(self): + idp_error_validators = {} + return {**super().error_validators, **idp_error_validators} def config_factory(_type, config): diff --git a/tests/eidas/idp_conf.py b/tests/eidas/idp_conf.py index 1fd2251a8..24607294b 100644 --- a/tests/eidas/idp_conf.py +++ b/tests/eidas/idp_conf.py @@ -10,16 +10,13 @@ BASE = "http://localhost:8088" CONFIG = { - "entityid": "urn:mace:example.com:saml:roland:idp", + "entityid": "https://example.org", "name": "Rolands IdP", "service": { "idp": { "endpoints": { "single_sign_on_service": [ ("%s/sso" % BASE, BINDING_HTTP_REDIRECT)], - "single_logout_service": [ - ("%s/slo" % BASE, BINDING_SOAP), - ("%s/slop" % BASE, BINDING_HTTP_POST)] }, "policy": { "default": { @@ -38,7 +35,9 @@ } }, "subject_data": full_path("subject_data.db"), - "node_country": "GR" + "node_country": "GR", + "application_identifier": "CEF:eIDAS-ref:2.0", + "protocol_version": [1.1, 2.2] }, }, "debug": 1, @@ -53,16 +52,25 @@ }], "attribute_map_dir": full_path("attributemaps"), "organization": { - "name": "Exempel AB", - "display_name": [("Exempel AB", "se"), ("Example Co.", "en")], - "url": "http://www.example.com/roland", + "name": ("AB Exempel", "se"), + "display_name": ("AB Exempel", "se"), + "url": "http://www.example.org", }, "contact_person": [ { - "given_name": "John", - "sur_name": "Smith", - "email_address": ["john.smith@example.com"], - "contact_type": "technical", + "given_name": "Roland", + "sur_name": "Hedberg", + "telephone_number": "+46 70 100 0000", + "email_address": ["tech@eample.com", + "tech@example.org"], + "contact_type": "technical" }, + { + "given_name": "Roland", + "sur_name": "Hedberg", + "telephone_number": "+46 70 100 0000", + "email_address": ["tech@eample.com", + "tech@example.org"], + "contact_type": "support"} ], } diff --git a/tests/eidas/test_idp.py b/tests/eidas/test_idp.py index eeba4cfcb..1d6e1db39 100644 --- a/tests/eidas/test_idp.py +++ b/tests/eidas/test_idp.py @@ -1,13 +1,242 @@ +import pytest +import copy +from saml2 import BINDING_HTTP_POST from saml2 import metadata -from saml2.config import IdPConfig +from saml2 import samlp +from saml2.client import Saml2Client +from saml2.server import Server +from saml2.config import eIDASSPConfig, eIDASIdPConfig +from eidas.idp_conf import CONFIG +from saml2.utility.config import ConfigValidationError class TestIdP: def setup_class(self): - self.conf = IdPConfig() + self.server = Server("idp_conf") + + self.conf = eIDASIdPConfig() self.conf.load_file("idp_conf") + sp_conf = eIDASSPConfig() + sp_conf.load_file("sp_conf") + + self.client = Saml2Client(sp_conf) + + def teardown_class(self): + self.server.close() + + @pytest.fixture(scope="function") + def config(self): + return copy.deepcopy(CONFIG) + def test_node_country_in_metadata(self): entd = metadata.entity_descriptor(self.conf) assert any(filter(lambda x: x.tag == "NodeCountry", entd.extensions.extension_elements)) + + def test_application_identifier_in_metadata(self): + entd = metadata.entity_descriptor(self.conf) + entity_attributes = next(filter(lambda x: x.tag == "EntityAttributes", + entd.extensions.extension_elements)) + app_identifier = [ + x for x in entity_attributes.children + if getattr(x, "attributes", {}).get("Name") == + "http://eidas.europa.eu/entity-attributes/application-identifier" + ] + assert len(app_identifier) == 1 + assert self.conf._idp_application_identifier \ + == next(x.text for y in app_identifier for x in y.children) + + def test_multiple_protocol_version_in_metadata(self): + entd = metadata.entity_descriptor(self.conf) + entity_attributes = next(filter(lambda x: x.tag == "EntityAttributes", + entd.extensions.extension_elements)) + protocol_version = next( + x for x in entity_attributes.children + if getattr(x, "name", "") == + "http://eidas.europa.eu/entity-attributes/protocol-version" + ) + assert len(protocol_version.attribute_value) == 2 + assert set(str(x) for x in self.conf._idp_protocol_version) \ + == set([x.text for x in protocol_version.attribute_value]) + + def test_protocol_version_in_metadata(self, config): + config["service"]["idp"]["protocol_version"] = 1.2 + + conf = eIDASIdPConfig() + conf.load(config) + + entd = metadata.entity_descriptor(conf) + entity_attributes = next(filter(lambda x: x.tag == "EntityAttributes", + entd.extensions.extension_elements)) + protocol_version = next( + x for x in entity_attributes.children + if getattr(x, "name", "") == + "http://eidas.europa.eu/entity-attributes/protocol-version" + ) + assert len(protocol_version.attribute_value) == 1 + assert {str(conf._idp_protocol_version)} \ + == set([x.text for x in protocol_version.attribute_value]) + + +class TestIdPConfig: + @staticmethod + def assert_validation_error(config): + conf = eIDASIdPConfig() + conf.load(config) + with pytest.raises(ConfigValidationError): + conf.validate() + + @pytest.fixture(scope="function") + def technical_contacts(self, config): + return [ + x for x in config["contact_person"] + if x["contact_type"] == "technical" + ] + + @pytest.fixture(scope="function") + def support_contacts(self, config): + return [ + x for x in config["contact_person"] + if x["contact_type"] == "support" + ] + + @pytest.fixture(scope="function") + def raise_error_on_warning(self, monkeypatch): + def r(*args, **kwargs): + raise ConfigValidationError() + monkeypatch.setattr("saml2.config.logger.warning", r) + + @pytest.fixture(scope="function") + def config(self): + return copy.deepcopy(CONFIG) + + def test_singlelogout_declared(self, config, raise_error_on_warning): + config["service"]["idp"]["endpoints"]["single_logout_service"] = \ + [("https://example.com", BINDING_HTTP_POST)] + self.assert_validation_error(config) + + def test_artifact_resolution_declared(self, config, raise_error_on_warning): + config["service"]["idp"]["endpoints"]["artifact_resolution_service"] = \ + [("https://example.com", BINDING_HTTP_POST)] + self.assert_validation_error(config) + + def test_manage_nameid_service_declared(self, config, raise_error_on_warning): + config["service"]["idp"]["endpoints"]["manage_name_id_service"] = \ + [("https://example.com", BINDING_HTTP_POST)] + self.assert_validation_error(config) + + def test_no_keydescriptor(self, config): + del config["cert_file"] + self.assert_validation_error(config) + + def test_no_nodecountry(self, config): + del config["service"]["idp"]["node_country"] + self.assert_validation_error(config) + + def test_nodecountry_wrong_format(self, config): + config["service"]["idp"]["node_country"] = "gr" + self.assert_validation_error(config) + + def test_no_application_identifier_warning(self, config, raise_error_on_warning): + del config["service"]["idp"]["application_identifier"] + + self.assert_validation_error(config) + + def test_empty_application_identifier_warning(self, config, raise_error_on_warning): + config["service"]["idp"]["application_identifier"] = "" + + self.assert_validation_error(config) + + def test_application_identifier_wrong_format(self, config): + config["service"]["idp"]["application_identifier"] = "TEST:Node.1" + + self.assert_validation_error(config) + + def test_application_identifier_ok_format(self, config, raise_error_on_warning): + conf = eIDASIdPConfig() + conf.load(config) + conf.validate() + + def test_no_protocol_version_warning(self, config, raise_error_on_warning): + del config["service"]["idp"]["protocol_version"] + + self.assert_validation_error(config) + + def test_empty_protocol_version_warning(self, config, raise_error_on_warning): + config["service"]["idp"]["protocol_version"] = "" + + self.assert_validation_error(config) + + def test_no_organization_info_warning(self, config, raise_error_on_warning): + del config["organization"] + + self.assert_validation_error(config) + + def test_empty_organization_info_warning(self, config, raise_error_on_warning): + config["organization"] = {} + + self.assert_validation_error(config) + + def test_no_technical_contact_person(self, + config, + technical_contacts, + raise_error_on_warning): + for contact in technical_contacts: + contact["contact_type"] = "other" + + self.assert_validation_error(config) + + def test_technical_contact_person_no_email(self, + config, + technical_contacts, + raise_error_on_warning): + + for contact in technical_contacts: + del contact["email_address"] + + self.assert_validation_error(config) + + def test_technical_contact_person_empty_email(self, + config, + technical_contacts, + raise_error_on_warning): + + for contact in technical_contacts: + del contact["email_address"] + + self.assert_validation_error(config) + + def test_no_support_contact_person(self, + config, + support_contacts, + raise_error_on_warning): + for contact in support_contacts: + contact["contact_type"] = "other" + + self.assert_validation_error(config) + + def test_support_contact_person_no_email(self, + config, + support_contacts, + raise_error_on_warning): + + for contact in support_contacts: + del contact["email_address"] + + self.assert_validation_error(config) + + def test_support_contact_person_empty_email(self, + config, + support_contacts, + raise_error_on_warning): + + for contact in support_contacts: + del contact["email_address"] + + self.assert_validation_error(config) + + def test_entityid_no_https(self, config): + config["entityid"] = "urn:mace:example.com:saml:roland:idp" + + self.assert_validation_error(config) From a029ce87d050ab26434bea806c05dfd0b588eed1 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Mon, 10 Feb 2020 15:32:38 +0200 Subject: [PATCH 15/26] Rename eidas tests config files - Renames eidas tests config files because there was a clash in paths loaded and it resulted to other irrelevant tests crashing (because they loaded the eidas conf files that had different entity ids) --- .../eidas/{idp_conf.py => eidas_idp_conf.py} | 0 tests/eidas/{sp_conf.py => eidas_sp_conf.py} | 0 tests/eidas/sp2.xml | 40 +++++++++++++++++++ tests/eidas/test_idp.py | 8 ++-- tests/eidas/test_sp.py | 6 +-- 5 files changed, 47 insertions(+), 7 deletions(-) rename tests/eidas/{idp_conf.py => eidas_idp_conf.py} (100%) rename tests/eidas/{sp_conf.py => eidas_sp_conf.py} (100%) create mode 100644 tests/eidas/sp2.xml diff --git a/tests/eidas/idp_conf.py b/tests/eidas/eidas_idp_conf.py similarity index 100% rename from tests/eidas/idp_conf.py rename to tests/eidas/eidas_idp_conf.py diff --git a/tests/eidas/sp_conf.py b/tests/eidas/eidas_sp_conf.py similarity index 100% rename from tests/eidas/sp_conf.py rename to tests/eidas/eidas_sp_conf.py diff --git a/tests/eidas/sp2.xml b/tests/eidas/sp2.xml new file mode 100644 index 000000000..777f40e99 --- /dev/null +++ b/tests/eidas/sp2.xml @@ -0,0 +1,40 @@ +GRCEF:eIDAS-ref:2.01.12.2MIICsDCCAhmgAwIBAgIJAJrzqSSwmDY9MA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNV +BAYTAkFVMRMwEQYDVQQIEwpTb21lLVN0YXRlMSEwHwYDVQQKExhJbnRlcm5ldCBX +aWRnaXRzIFB0eSBMdGQwHhcNMDkxMDA2MTk0OTQxWhcNMDkxMTA1MTk0OTQxWjBF +MQswCQYDVQQGEwJBVTETMBEGA1UECBMKU29tZS1TdGF0ZTEhMB8GA1UEChMYSW50 +ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKB +gQDJg2cms7MqjniT8Fi/XkNHZNPbNVQyMUMXE9tXOdqwYCA1cc8vQdzkihscQMXy +3iPw2cMggBu6gjMTOSOxECkuvX5ZCclKr8pXAJM5cY6gVOaVO2PdTZcvDBKGbiaN +efiEw5hnoZomqZGp8wHNLAUkwtH9vjqqvxyS/vclc6k2ewIDAQABo4GnMIGkMB0G +A1UdDgQWBBRePsKHKYJsiojE78ZWXccK9K4aJTB1BgNVHSMEbjBsgBRePsKHKYJs +iojE78ZWXccK9K4aJaFJpEcwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgTClNvbWUt +U3RhdGUxITAfBgNVBAoTGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZIIJAJrzqSSw +mDY9MAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADgYEAJSrKOEzHO7TL5cy6 +h3qh+3+JAk8HbGBW+cbX6KBCAw/mzU8flK25vnWwXS3dv2FF3Aod0/S7AWNfKib5 +U/SA9nJaz/mWeF9S0farz9AQFc8/NSzAzaVq7YbM4F6f6N2FRl7GikdXRCed45j6 +mrPzGzk3ECbupFnqyREH3+ZPSdk= +MIICITCCAYoCAQEwDQYJKoZIhvcNAQELBQAwWDELMAkGA1UEBhMCenoxCzAJBgNV +BAgMAnp6MQ0wCwYDVQQHDAR6enp6MQ4wDAYDVQQKDAVaenp6ejEOMAwGA1UECwwF +Wnp6enoxDTALBgNVBAMMBHRlc3QwIBcNMTkwNDEyMTk1MDM0WhgPMzAxODA4MTMx +OTUwMzRaMFgxCzAJBgNVBAYTAnp6MQswCQYDVQQIDAJ6ejENMAsGA1UEBwwEenp6 +ejEOMAwGA1UECgwFWnp6enoxDjAMBgNVBAsMBVp6enp6MQ0wCwYDVQQDDAR0ZXN0 +MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDHcj80WU/XBsd9FlyQmfjPUdfm +edhCFDd6TEQmZNNqP/UG+VkGa+BXjRIHMfic/WxPTbGhCjv68ci0UDNomUXagFex +LGNpkwa7+CRVtoc/1xgq+ySE6M4nhcCutScoxNvWNn5eSQ66i3U0sTv91MgsXxqE +dTaiZg0BIufEc3dueQIDAQABMA0GCSqGSIb3DQEBCwUAA4GBAGUV5B+USHvaRa8k +gCNJSuNpo6ARlv0ekrk8bbdNRBiEUdCMyoGJFfuM9K0zybX6Vr25wai3nvaog294 +Vx/jWjX2g5SDbjItH6VGy6C9GCGf1A07VxFRCfJn5tA9HuJjPKiE+g/BmrV5N4Ce +alzFxPHWYkNOzoRU8qI7OqUai1kL +MIICITCCAYoCAQEwDQYJKoZIhvcNAQELBQAwWDELMAkGA1UEBhMCenoxCzAJBgNV +BAgMAnp6MQ0wCwYDVQQHDAR6enp6MQ4wDAYDVQQKDAVaenp6ejEOMAwGA1UECwwF +Wnp6enoxDTALBgNVBAMMBHRlc3QwIBcNMTkwNDEyMTk1MDM0WhgPMzAxODA4MTMx +OTUwMzRaMFgxCzAJBgNVBAYTAnp6MQswCQYDVQQIDAJ6ejENMAsGA1UEBwwEenp6 +ejEOMAwGA1UECgwFWnp6enoxDjAMBgNVBAsMBVp6enp6MQ0wCwYDVQQDDAR0ZXN0 +MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDjW0kJM+4baWKtvO24ZsGXNvNK +KkwTMz7OW5Z6BRqhSOq2WA0c5NCpMk6rD8Z2OTFEolPojEjf8dVyd/Ds/hrjFKQv +8wQgbdXLN51YTIsgd6h+hBJO+vzhl0PT4aT7M0JKo5ALtS6qk4tsworW2BnwyvsG +SAinwfeWt4t/b1J3kwIDAQABMA0GCSqGSIb3DQEBCwUAA4GBAFtj7WArQQBugmh/ +KQjjlfTQ5A052QeXfgTyO9vv1S6MRIi7qgiaEv49cGXnJv/TWbySkMKObPMUApjg +6z8PqcxuShew5FCTkNvwhABFPiyu0fUj3e2FEPHfsBu76jz4ugtmhUqjqhzwFY9c +tnWRkkl6J0AjM3LnHOSgjNIclDZG +urn:mace:example.com:saml:roland:spMy own SPAB ExempelAB Exempelhttp://www.example.orgRolandHedbergtech@eample.comtech@example.org+46 70 100 0000RolandHedbergtech@eample.comtech@example.org+46 70 100 0000 diff --git a/tests/eidas/test_idp.py b/tests/eidas/test_idp.py index 1d6e1db39..8f447688d 100644 --- a/tests/eidas/test_idp.py +++ b/tests/eidas/test_idp.py @@ -6,19 +6,19 @@ from saml2.client import Saml2Client from saml2.server import Server from saml2.config import eIDASSPConfig, eIDASIdPConfig -from eidas.idp_conf import CONFIG +from eidas.eidas_idp_conf import CONFIG from saml2.utility.config import ConfigValidationError class TestIdP: def setup_class(self): - self.server = Server("idp_conf") + self.server = Server("eidas_idp_conf") self.conf = eIDASIdPConfig() - self.conf.load_file("idp_conf") + self.conf.load_file("eidas_idp_conf") sp_conf = eIDASSPConfig() - sp_conf.load_file("sp_conf") + sp_conf.load_file("eidas_sp_conf") self.client = Saml2Client(sp_conf) diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py index 51a9ed6ec..585be694a 100644 --- a/tests/eidas/test_sp.py +++ b/tests/eidas/test_sp.py @@ -6,16 +6,16 @@ from saml2.client import Saml2Client from saml2.server import Server from saml2.config import eIDASSPConfig -from eidas.sp_conf import CONFIG +from eidas.eidas_sp_conf import CONFIG from saml2.utility.config import ConfigValidationError class TestSP: def setup_class(self): - self.server = Server("idp_conf") + self.server = Server("eidas_idp_conf") self.conf = eIDASSPConfig() - self.conf.load_file("sp_conf") + self.conf.load_file("eidas_sp_conf") self.client = Saml2Client(self.conf) From d54432870f01110f66adfdb2d5fc685e3330412d Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Tue, 11 Feb 2020 13:38:57 +0200 Subject: [PATCH 16/26] Add eIDAS IdP validation rule for WantAuthnRequestsSigned - Adds error validation rule to require want_authn_requests_signed to be set to True as defined in the eIDAS SAML Message Format v.1.2 spec document - Adds tests for the above scenario --- src/saml2/config.py | 5 ++++- tests/eidas/eidas_idp_conf.py | 3 ++- tests/eidas/test_idp.py | 10 ++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index 2168fd40f..b3112b170 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -737,7 +737,10 @@ def warning_validators(self): @property def error_validators(self): - idp_error_validators = {} + idp_error_validators = { + "want_authn_requests_signed MUST be set to True": + getattr(self, "_idp_want_authn_requests_signed", None) is True + } return {**super().error_validators, **idp_error_validators} diff --git a/tests/eidas/eidas_idp_conf.py b/tests/eidas/eidas_idp_conf.py index 24607294b..d5afb7333 100644 --- a/tests/eidas/eidas_idp_conf.py +++ b/tests/eidas/eidas_idp_conf.py @@ -37,7 +37,8 @@ "subject_data": full_path("subject_data.db"), "node_country": "GR", "application_identifier": "CEF:eIDAS-ref:2.0", - "protocol_version": [1.1, 2.2] + "protocol_version": [1.1, 2.2], + "want_authn_requests_signed": True }, }, "debug": 1, diff --git a/tests/eidas/test_idp.py b/tests/eidas/test_idp.py index 8f447688d..4d0b9561b 100644 --- a/tests/eidas/test_idp.py +++ b/tests/eidas/test_idp.py @@ -240,3 +240,13 @@ def test_entityid_no_https(self, config): config["entityid"] = "urn:mace:example.com:saml:roland:idp" self.assert_validation_error(config) + + def test_want_authn_requests_signed_unset(self, config): + del config["service"]["idp"]["want_authn_requests_signed"] + + self.assert_validation_error(config) + + def test_want_authn_requests_signed_false(self, config): + config["service"]["idp"]["want_authn_requests_signed"] = False + + self.assert_validation_error(config) From d18673c052ace57694721c573a245c4a784bae13 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Tue, 11 Feb 2020 22:21:41 +0200 Subject: [PATCH 17/26] Add support for exposing IdP supported attributes - Adds support for exposing IdP supported attributes in IDPSSODescriptor as Attribute elements. Support is added in the config file under service->idp->provided_attributes. provided_attributes alread existed as a valid option for idp config but was not used. Supported attributes MUST be published as Attribute elements in the metadata of the eIDAS IdP as stated in eIDAS SAML Message Format v.1.2 spec document - Adds error validation rule in eIDASIdPConfig to ensure provided_attributes MUST be set - Adds test to verify the provided_attributes are exposed as Attribute elements under IDPSSODescriptor and for the error validation rule --- src/saml2/config.py | 5 ++++- src/saml2/metadata.py | 8 ++++++++ tests/eidas/eidas_idp_conf.py | 17 ++++++++++++++++- tests/eidas/test_idp.py | 19 +++++++++++++++++++ 4 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index b3112b170..c3eaa790a 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -739,7 +739,10 @@ def warning_validators(self): def error_validators(self): idp_error_validators = { "want_authn_requests_signed MUST be set to True": - getattr(self, "_idp_want_authn_requests_signed", None) is True + getattr(self, "_idp_want_authn_requests_signed", None) is True, + "provided_attributes MUST be set to denote the supported attributes by " + "the IdP": + not_empty(getattr(self, "_idp_provided_attributes", None)) } return {**super().error_validators, **idp_error_validators} diff --git a/src/saml2/metadata.py b/src/saml2/metadata.py index 5108dd742..ddac542bb 100644 --- a/src/saml2/metadata.py +++ b/src/saml2/metadata.py @@ -594,6 +594,14 @@ def do_idpsso_descriptor(conf, cert=None, enc_cert=None): except KeyError: setattr(idpsso, key, DEFAULTS[key]) + attributes = [ + Attribute(name=attribute.get("name", None), + name_format=attribute.get("name_format", None), + friendly_name=attribute.get("friendly_name", None)) + for attribute in conf.getattr("provided_attributes", "idp") + ] + idpsso.attribute = attributes + return idpsso diff --git a/tests/eidas/eidas_idp_conf.py b/tests/eidas/eidas_idp_conf.py index d5afb7333..ec9e4d530 100644 --- a/tests/eidas/eidas_idp_conf.py +++ b/tests/eidas/eidas_idp_conf.py @@ -38,7 +38,22 @@ "node_country": "GR", "application_identifier": "CEF:eIDAS-ref:2.0", "protocol_version": [1.1, 2.2], - "want_authn_requests_signed": True + "want_authn_requests_signed": True, + "provided_attributes": [ + { + "name": "http://eidas.europa.eu/attributes/naturalperson/PersonIdentifier", + "friendly_name": "PersonIdentifier", + "name_format": "urn:oasis:names:tc:SAML:2.0:attrname-format:uri" + }, + { + "name": "http://eidas.europa.eu/attributes/naturalperson/CurrentFamilyName", + "friendly_name": "FamilyName", + }, + { + "name": "http://eidas.europa.eu/attributes/naturalperson/CurrentGivenName", + "name_format": "urn:oasis:names:tc:SAML:2.0:attrname-format:uri" + } + ], }, }, "debug": 1, diff --git a/tests/eidas/test_idp.py b/tests/eidas/test_idp.py index 4d0b9561b..991f14870 100644 --- a/tests/eidas/test_idp.py +++ b/tests/eidas/test_idp.py @@ -78,6 +78,20 @@ def test_protocol_version_in_metadata(self, config): assert {str(conf._idp_protocol_version)} \ == set([x.text for x in protocol_version.attribute_value]) + def test_supported_attributes(self, config): + entd = metadata.entity_descriptor(self.conf) + attributes_published = [ + set( + filter(lambda x: x is not None, + [attribute.name, attribute.name_format, attribute.friendly_name] + ) + ) + for attribute in entd.idpsso_descriptor.attribute + ] + attributes_stated = [set(x.values()) for x + in self.conf._idp_provided_attributes] + assert all(filter(lambda x: x in attributes_published, attributes_stated)) + class TestIdPConfig: @staticmethod @@ -250,3 +264,8 @@ def test_want_authn_requests_signed_false(self, config): config["service"]["idp"]["want_authn_requests_signed"] = False self.assert_validation_error(config) + + def test_provided_attributes_unset(self, config): + del config["service"]["idp"]["provided_attributes"] + + self.assert_validation_error(config) From b65b841864ef645828f6aee6c366d8f34f499dda Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Fri, 14 Feb 2020 15:49:25 +0200 Subject: [PATCH 18/26] Add support for LoA in eIDAS IdP config - Adds support for defining supported LoA for notified and non-notified eIDAS nodes - Exposes LoA as Attribute element and supported LoA as AttributeValue elements to eIDAS IdP metadata --- src/saml2/config.py | 3 ++- src/saml2/metadata.py | 12 ++++++++++++ tests/eidas/eidas_idp_conf.py | 4 ++++ tests/eidas/test_idp.py | 28 +++++++++++++++++++++++++++- 4 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index c3eaa790a..a78862f76 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -129,7 +129,8 @@ "edu_person_targeted_id", "node_country", "application_identifier", - "protocol_version" + "protocol_version", + "supported_loa" ] PDP_ARGS = ["endpoints", "name_form", "name_id_format"] diff --git a/src/saml2/metadata.py b/src/saml2/metadata.py index ddac542bb..0a423fd52 100644 --- a/src/saml2/metadata.py +++ b/src/saml2/metadata.py @@ -807,6 +807,18 @@ def entity_descriptor(confd): ) _add_attr_to_entity_attributes(entd.extensions, attr) + loa = confd.getattr("supported_loa", confd.context) + if loa: + entd.extensions = entd.extensions or md.Extensions() + ava = [AttributeValue(text=str(c)) for c in make_list( + loa.get("notified", []), loa.get("non_notified", []))] + attr = Attribute( + attribute_value=ava, + name="urn:oasis:names:tc:SAML:attribute:assurance-certification", + name_format="urn:oasis:names:tc:saml2:2.0:attrname-format:uri" + ) + _add_attr_to_entity_attributes(entd.extensions, attr) + return entd diff --git a/tests/eidas/eidas_idp_conf.py b/tests/eidas/eidas_idp_conf.py index ec9e4d530..b97f3d7e3 100644 --- a/tests/eidas/eidas_idp_conf.py +++ b/tests/eidas/eidas_idp_conf.py @@ -54,6 +54,10 @@ "name_format": "urn:oasis:names:tc:SAML:2.0:attrname-format:uri" } ], + "supported_loa": { + "notified": ["http://eidas.europa.eu/LoA/high"], + "non_notified": ["http://eidas.europa.eu/NotNotified/LoA/high"] + } }, }, "debug": 1, diff --git a/tests/eidas/test_idp.py b/tests/eidas/test_idp.py index 991f14870..695c79c76 100644 --- a/tests/eidas/test_idp.py +++ b/tests/eidas/test_idp.py @@ -2,7 +2,7 @@ import copy from saml2 import BINDING_HTTP_POST from saml2 import metadata -from saml2 import samlp +from saml2.utility import make_list from saml2.client import Saml2Client from saml2.server import Server from saml2.config import eIDASSPConfig, eIDASIdPConfig @@ -92,6 +92,32 @@ def test_supported_attributes(self, config): in self.conf._idp_provided_attributes] assert all(filter(lambda x: x in attributes_published, attributes_stated)) + def test_loa_attribute_exposed(self, config): + entd = metadata.entity_descriptor(self.conf) + entity_attributes = next(filter(lambda x: x.tag == "EntityAttributes", + entd.extensions.extension_elements)) + loa_attribute = next( + (x for x in entity_attributes.children + if getattr(x, "name", "") == + "urn:oasis:names:tc:SAML:attribute:assurance-certification"), None + ) + assert loa_attribute is not None + assert loa_attribute.name_format == "urn:oasis:names:tc:saml2:2.0:attrname-format:uri" + + def test_loa_attribute_values_exposes(self, config): + entd = metadata.entity_descriptor(self.conf) + entity_attributes = next(filter(lambda x: x.tag == "EntityAttributes", + entd.extensions.extension_elements)) + loa_attribute = next( + (x for x in entity_attributes.children + if getattr(x, "name", "") == + "urn:oasis:names:tc:SAML:attribute:assurance-certification"), None + ) + assert loa_attribute is not None + loa_values = {x.text for x in loa_attribute.attribute_value} + assert loa_values == set(make_list(*config["service"]["idp"][ + "supported_loa"].values())) + class TestIdPConfig: @staticmethod From f86e464744c240524e65525cddb5675eadbc1735 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Tue, 18 Feb 2020 12:16:37 +0200 Subject: [PATCH 19/26] Add validation rules for LoA configuration - Adds validation checks for LoA configuration of an eIDAS IdP to verify that notified LoA URIs are not published as non-notified and that notified LoA URIs are only the ones that are officially described in the eIDAS SAML Message Format v.1.2 spec document - Adds tests for the above scenarios --- src/saml2/config.py | 24 +++++++++++++++++++++++- tests/eidas/test_idp.py | 13 +++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index a78862f76..70739a406 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -731,6 +731,20 @@ def get_protocol_version(self): def get_node_country(self): return getattr(self, "_idp_node_country", None) + def verify_non_notified_loa(self): + return not any( + [x.startswith("http://eidas.europa.eu/LoA/") + for x in getattr(self, "_idp_supported_loa", {}).get("non_notified")] + ) + + def verify_notified_loa(self): + return all( + [x in ["http://eidas.europa.eu/LoA/low", + "http://eidas.europa.eu/LoA/substantial", + "http://eidas.europa.eu/LoA/high"] + for x in getattr(self, "_idp_supported_loa", {}).get("notified")] + ) + @property def warning_validators(self): idp_warning_validators = {} @@ -743,7 +757,15 @@ def error_validators(self): getattr(self, "_idp_want_authn_requests_signed", None) is True, "provided_attributes MUST be set to denote the supported attributes by " "the IdP": - not_empty(getattr(self, "_idp_provided_attributes", None)) + not_empty(getattr(self, "_idp_provided_attributes", None)), + "supported_loa for non-notified eIDs MUST NOT use an " + "http://eidas.europa.eu/LoA/ prefix": + self.verify_non_notified_loa(), + "supported_loa for notified eID MUST be (at least) one of " + "[http://eidas.europa.eu/LoA/low, " + "http://eidas.europa.eu/LoA/substantial, " + "http://eidas.europa.eu/LoA/high]": + self.verify_notified_loa() } return {**super().error_validators, **idp_error_validators} diff --git a/tests/eidas/test_idp.py b/tests/eidas/test_idp.py index 695c79c76..04c66ada2 100644 --- a/tests/eidas/test_idp.py +++ b/tests/eidas/test_idp.py @@ -295,3 +295,16 @@ def test_provided_attributes_unset(self, config): del config["service"]["idp"]["provided_attributes"] self.assert_validation_error(config) + + def test_notified_loa_in_non_notified(self, config): + config["service"]["idp"]["supported_loa"]["non_notified"] = \ + ["http://eidas.europa.eu/LoA/high"] + + self.assert_validation_error(config) + + def test_notified_loa_wrong(self, config): + config["service"]["idp"]["supported_loa"]["notified"] = \ + config["service"]["idp"]["supported_loa"]["notified"] \ + + ["http://eidas.europa.eu/LoA/something-else"] + + self.assert_validation_error(config) From 53c025193d49358ffd9c3b0771ac3ae6597aacd2 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Tue, 25 Feb 2020 20:05:29 +0200 Subject: [PATCH 20/26] Add warning validation for eIDASSP for AssertionConsumerServiceURL - Adds a config check that warns the user that they SHOULD set the `hide_assertion_consumer_service` to True since the eIDAS SAML Message Format v.1.2 spec document states that during an AssertionConsumerServiceURL attribute SHOULD NOT exist in an AuthnRequest element --- src/saml2/config.py | 5 ++++- tests/eidas/eidas_sp_conf.py | 3 ++- tests/eidas/test_sp.py | 12 +++++++++++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index 70739a406..798f7ecad 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -697,7 +697,10 @@ def get_node_country(self): @property def warning_validators(self): - sp_warning_validators = {} + sp_warning_validators = { + "hide_assertion_consumer_service SHOULD be set to True": + getattr(self, "_sp_hide_assertion_consumer_service", None) is True + } return {**super().warning_validators, **sp_warning_validators} @property diff --git a/tests/eidas/eidas_sp_conf.py b/tests/eidas/eidas_sp_conf.py index b28b1836d..a05757e92 100644 --- a/tests/eidas/eidas_sp_conf.py +++ b/tests/eidas/eidas_sp_conf.py @@ -33,7 +33,8 @@ "authn_requests_signed": True, "node_country": "GR", "application_identifier": "CEF:eIDAS-ref:2.0", - "protocol_version": [1.1, 2.2] + "protocol_version": [1.1, 2.2], + "hide_assertion_consumer_service": True } }, "debug": 1, diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py index 585be694a..f6ffaefb5 100644 --- a/tests/eidas/test_sp.py +++ b/tests/eidas/test_sp.py @@ -185,7 +185,7 @@ def test_application_identifier_wrong_format(self, config): self.assert_validation_error(config) - def test_application_identifier_ok_format(self, config, raise_error_on_warning): + def test_config_ok(self, config, raise_error_on_warning): conf = eIDASSPConfig() conf.load(config) conf.validate() @@ -293,3 +293,13 @@ def test_sp_type_invalid_value(self, config): self.assert_validation_error(config) + def test_hide_assertion_consumer_service_false(self, config, raise_error_on_warning): + config["service"]["sp"]["hide_assertion_consumer_service"] = False + + self.assert_validation_error(config) + + def test_hide_assertion_consumer_service_unset(self, config, + raise_error_on_warning): + del config["service"]["sp"]["hide_assertion_consumer_service"] + + self.assert_validation_error(config) From 0cfc7ac50099be34fbbf88e507564d0b56972233 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Tue, 3 Mar 2020 14:00:08 +0200 Subject: [PATCH 21/26] Add eIDAS IdP validation rule for signed response - Adds error validation rule to for `sign_response` configuration option to True as the authn responses MUST be signed according to eIDAS SAML Message Format v.1.2 spec document - Adds tests to test the aforementioned validation rule --- src/saml2/config.py | 4 +++- tests/eidas/eidas_idp_conf.py | 3 ++- tests/eidas/test_idp.py | 12 +++++++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index 798f7ecad..c74f3f9fc 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -768,7 +768,9 @@ def error_validators(self): "[http://eidas.europa.eu/LoA/low, " "http://eidas.europa.eu/LoA/substantial, " "http://eidas.europa.eu/LoA/high]": - self.verify_notified_loa() + self.verify_notified_loa(), + "sign_response MUST be set to True": + getattr(self, "_idp_sign_response", None) is True } return {**super().error_validators, **idp_error_validators} diff --git a/tests/eidas/eidas_idp_conf.py b/tests/eidas/eidas_idp_conf.py index b97f3d7e3..d3be9a475 100644 --- a/tests/eidas/eidas_idp_conf.py +++ b/tests/eidas/eidas_idp_conf.py @@ -57,7 +57,8 @@ "supported_loa": { "notified": ["http://eidas.europa.eu/LoA/high"], "non_notified": ["http://eidas.europa.eu/NotNotified/LoA/high"] - } + }, + "sign_response": True }, }, "debug": 1, diff --git a/tests/eidas/test_idp.py b/tests/eidas/test_idp.py index 04c66ada2..02fb94583 100644 --- a/tests/eidas/test_idp.py +++ b/tests/eidas/test_idp.py @@ -193,7 +193,7 @@ def test_application_identifier_wrong_format(self, config): self.assert_validation_error(config) - def test_application_identifier_ok_format(self, config, raise_error_on_warning): + def test_config_ok(self, config, raise_error_on_warning): conf = eIDASIdPConfig() conf.load(config) conf.validate() @@ -308,3 +308,13 @@ def test_notified_loa_wrong(self, config): + ["http://eidas.europa.eu/LoA/something-else"] self.assert_validation_error(config) + + def test_sign_response_unset(self, config): + del config["service"]["idp"]["sign_response"] + + self.assert_validation_error(config) + + def test_sign_response_false(self, config): + config["service"]["idp"]["sign_response"] = False + + self.assert_validation_error(config) From 3d912e5de0fb8658c8170738015c550a0f373820 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Tue, 3 Mar 2020 14:03:57 +0200 Subject: [PATCH 22/26] Add eIDAS IdP validation rule for encrypted assertion - Adds error validation rule to for `encrypt_assertion` configuration option to True as the assertions MUST be encrypted according to eIDAS SAML Message Format v.1.2 spec document - Adds tests to test the aforementioned validation rule --- src/saml2/config.py | 4 +++- tests/eidas/eidas_idp_conf.py | 3 ++- tests/eidas/test_idp.py | 10 ++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index c74f3f9fc..0abba6287 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -770,7 +770,9 @@ def error_validators(self): "http://eidas.europa.eu/LoA/high]": self.verify_notified_loa(), "sign_response MUST be set to True": - getattr(self, "_idp_sign_response", None) is True + getattr(self, "_idp_sign_response", None) is True, + "encrypt_assertion MUST be set to True": + getattr(self, "_idp_encrypt_assertion", None) is True, } return {**super().error_validators, **idp_error_validators} diff --git a/tests/eidas/eidas_idp_conf.py b/tests/eidas/eidas_idp_conf.py index d3be9a475..5471b26bf 100644 --- a/tests/eidas/eidas_idp_conf.py +++ b/tests/eidas/eidas_idp_conf.py @@ -58,7 +58,8 @@ "notified": ["http://eidas.europa.eu/LoA/high"], "non_notified": ["http://eidas.europa.eu/NotNotified/LoA/high"] }, - "sign_response": True + "sign_response": True, + "encrypt_assertion": True }, }, "debug": 1, diff --git a/tests/eidas/test_idp.py b/tests/eidas/test_idp.py index 02fb94583..bd1daab49 100644 --- a/tests/eidas/test_idp.py +++ b/tests/eidas/test_idp.py @@ -318,3 +318,13 @@ def test_sign_response_false(self, config): config["service"]["idp"]["sign_response"] = False self.assert_validation_error(config) + + def test_encrypt_assertion_unset(self, config): + del config["service"]["idp"]["encrypt_assertion"] + + self.assert_validation_error(config) + + def test_encrypt_assertion_false(self, config): + config["service"]["idp"]["encrypt_assertion"] = False + + self.assert_validation_error(config) From d3423bb03ddc43caf322940bdfdcba8073e08494 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Thu, 5 Mar 2020 17:08:23 +0200 Subject: [PATCH 23/26] Add eIDAS SP config validation for allow_unsolicited - Adds error validation rule to enforce `allow_unsolicited=False` for an eIDAS SP config since the eIDAS SAML Message Format v.1.2 spec document states that unsolicited responses MUST NOT be accepted - Adds tests for the above validation rule --- src/saml2/config.py | 4 +++- tests/eidas/eidas_sp_conf.py | 3 ++- tests/eidas/test_sp.py | 10 ++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index 0abba6287..7f8202ac0 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -709,7 +709,9 @@ def error_validators(self): "authn_requests_signed MUST be set to True": getattr(self, "_sp_authn_requests_signed", None) is True, "sp_type MUST be set to 'public' or 'private'": - getattr(self, "_sp_sp_type", None) in ("public", "private") + getattr(self, "_sp_sp_type", None) in ("public", "private"), + "allow_unsolicited MUST be set to False": + getattr(self, "_sp_allow_unsolicited", None) is False } return {**super().error_validators, **sp_error_validators} diff --git a/tests/eidas/eidas_sp_conf.py b/tests/eidas/eidas_sp_conf.py index a05757e92..5b9cdde93 100644 --- a/tests/eidas/eidas_sp_conf.py +++ b/tests/eidas/eidas_sp_conf.py @@ -34,7 +34,8 @@ "node_country": "GR", "application_identifier": "CEF:eIDAS-ref:2.0", "protocol_version": [1.1, 2.2], - "hide_assertion_consumer_service": True + "hide_assertion_consumer_service": True, + "allow_unsolicited": False } }, "debug": 1, diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py index f6ffaefb5..4be46323d 100644 --- a/tests/eidas/test_sp.py +++ b/tests/eidas/test_sp.py @@ -303,3 +303,13 @@ def test_hide_assertion_consumer_service_unset(self, config, del config["service"]["sp"]["hide_assertion_consumer_service"] self.assert_validation_error(config) + + def test_allow_unsolicited_true(self, config): + config["service"]["sp"]["allow_unsolicited"] = True + + self.assert_validation_error(config) + + def test_allow_unsolicited_unse(self, config): + del config["service"]["sp"]["allow_unsolicited"] + + self.assert_validation_error(config) From 613a3d0dc76d16edbba8a3eaf9aeb254070eb7ac Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Fri, 6 Mar 2020 11:14:30 +0200 Subject: [PATCH 24/26] Move ConfigValidationError class to config module --- src/saml2/config.py | 5 ++++- src/saml2/utility/config.py | 2 -- tests/eidas/test_idp.py | 3 +-- tests/eidas/test_sp.py | 3 +-- 4 files changed, 6 insertions(+), 7 deletions(-) delete mode 100644 src/saml2/utility/config.py diff --git a/src/saml2/config.py b/src/saml2/config.py index 7f8202ac0..b62d0c805 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -26,7 +26,6 @@ from saml2.saml import NAME_FORMAT_URI from saml2.virtual_org import VirtualOrg from saml2.utility import not_empty -from saml2.utility.config import ConfigValidationError logger = logging.getLogger(__name__) @@ -806,3 +805,7 @@ def config_factory(_type, config): conf.context = _type return conf + + +class ConfigValidationError(Exception): + pass diff --git a/src/saml2/utility/config.py b/src/saml2/utility/config.py deleted file mode 100644 index d5b9aeb09..000000000 --- a/src/saml2/utility/config.py +++ /dev/null @@ -1,2 +0,0 @@ -class ConfigValidationError(Exception): - pass diff --git a/tests/eidas/test_idp.py b/tests/eidas/test_idp.py index bd1daab49..51d72eb9b 100644 --- a/tests/eidas/test_idp.py +++ b/tests/eidas/test_idp.py @@ -5,9 +5,8 @@ from saml2.utility import make_list from saml2.client import Saml2Client from saml2.server import Server -from saml2.config import eIDASSPConfig, eIDASIdPConfig +from saml2.config import eIDASSPConfig, eIDASIdPConfig, ConfigValidationError from eidas.eidas_idp_conf import CONFIG -from saml2.utility.config import ConfigValidationError class TestIdP: diff --git a/tests/eidas/test_sp.py b/tests/eidas/test_sp.py index 4be46323d..3331e23c1 100644 --- a/tests/eidas/test_sp.py +++ b/tests/eidas/test_sp.py @@ -5,9 +5,8 @@ from saml2 import samlp from saml2.client import Saml2Client from saml2.server import Server -from saml2.config import eIDASSPConfig +from saml2.config import eIDASSPConfig, ConfigValidationError from eidas.eidas_sp_conf import CONFIG -from saml2.utility.config import ConfigValidationError class TestSP: From 9a4b0ff9623d7d940c4a04c6aab933a2b3385a8c Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Fri, 6 Mar 2020 14:14:14 +0200 Subject: [PATCH 25/26] Extract LoAs in variable and fix fallback LoA lookups - Extracts the eIDAS LoA prefix for notified eIDs as a variable and constructs a new variable to contain a list of all accepted LoA for notified eIDs - Fixes fallback LoA lookup when `non_notified` or `notified` are unset. Returned None, now returns an empty list - Adds tests to test the aforementioned fixes --- src/saml2/config.py | 25 ++++++++++++++----------- tests/eidas/test_idp.py | 20 +++++++++++++++++--- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index b62d0c805..fcc53c3af 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -188,6 +188,13 @@ "attribute_consuming_service": _RPA } +EIDAS_NOTIFIED_LOA_PREFIX = "http://eidas.europa.eu/LoA/" + +EIDAS_NOTIFIED_LOA = [ + "{prefix}{x}".format(prefix=EIDAS_NOTIFIED_LOA_PREFIX, x=x) + for x in ["low", "substantial", "high"] +] + class ConfigurationError(SAMLError): pass @@ -737,16 +744,14 @@ def get_node_country(self): def verify_non_notified_loa(self): return not any( - [x.startswith("http://eidas.europa.eu/LoA/") - for x in getattr(self, "_idp_supported_loa", {}).get("non_notified")] + x.startswith(EIDAS_NOTIFIED_LOA_PREFIX) + for x in getattr(self, "_idp_supported_loa", {}).get("non_notified", []) ) def verify_notified_loa(self): return all( - [x in ["http://eidas.europa.eu/LoA/low", - "http://eidas.europa.eu/LoA/substantial", - "http://eidas.europa.eu/LoA/high"] - for x in getattr(self, "_idp_supported_loa", {}).get("notified")] + x in EIDAS_NOTIFIED_LOA + for x in getattr(self, "_idp_supported_loa", {}).get("notified", []) ) @property @@ -763,12 +768,10 @@ def error_validators(self): "the IdP": not_empty(getattr(self, "_idp_provided_attributes", None)), "supported_loa for non-notified eIDs MUST NOT use an " - "http://eidas.europa.eu/LoA/ prefix": + "{} prefix".format(EIDAS_NOTIFIED_LOA_PREFIX): self.verify_non_notified_loa(), - "supported_loa for notified eID MUST be (at least) one of " - "[http://eidas.europa.eu/LoA/low, " - "http://eidas.europa.eu/LoA/substantial, " - "http://eidas.europa.eu/LoA/high]": + "supported_loa for notified eID MUST be (at least) one of [{}]" + .format(", ".join(EIDAS_NOTIFIED_LOA)): self.verify_notified_loa(), "sign_response MUST be set to True": getattr(self, "_idp_sign_response", None) is True, diff --git a/tests/eidas/test_idp.py b/tests/eidas/test_idp.py index 51d72eb9b..0b57184cd 100644 --- a/tests/eidas/test_idp.py +++ b/tests/eidas/test_idp.py @@ -119,6 +119,12 @@ def test_loa_attribute_values_exposes(self, config): class TestIdPConfig: + @staticmethod + def config_validate(config): + conf = eIDASIdPConfig() + conf.load(config) + conf.validate() + @staticmethod def assert_validation_error(config): conf = eIDASIdPConfig() @@ -193,9 +199,7 @@ def test_application_identifier_wrong_format(self, config): self.assert_validation_error(config) def test_config_ok(self, config, raise_error_on_warning): - conf = eIDASIdPConfig() - conf.load(config) - conf.validate() + self.config_validate(config) def test_no_protocol_version_warning(self, config, raise_error_on_warning): del config["service"]["idp"]["protocol_version"] @@ -295,12 +299,22 @@ def test_provided_attributes_unset(self, config): self.assert_validation_error(config) + def test_notified_loa_unset(self, config, raise_error_on_warning): + del config["service"]["idp"]["supported_loa"]["notified"] + + self.config_validate(config) + def test_notified_loa_in_non_notified(self, config): config["service"]["idp"]["supported_loa"]["non_notified"] = \ ["http://eidas.europa.eu/LoA/high"] self.assert_validation_error(config) + def test_non_notified_loa_unset(self, config, raise_error_on_warning): + del config["service"]["idp"]["supported_loa"]["non_notified"] + + self.config_validate(config) + def test_notified_loa_wrong(self, config): config["service"]["idp"]["supported_loa"]["notified"] = \ config["service"]["idp"]["supported_loa"]["notified"] \ From 4c93775dd93b7571326d1c225faae4ae625b4179 Mon Sep 17 00:00:00 2001 From: John Paraskevopoulos Date: Mon, 9 Mar 2020 16:53:22 +0200 Subject: [PATCH 26/26] Use assurance_certification for eIDAS LoA config - Replaces supported_loa and uses the already existing assurance_certification for setting the supported LoA of an eIDAS IdP - Refactor tests and validation checks to use this config key --- src/saml2/config.py | 14 +++++------ src/saml2/metadata.py | 12 --------- tests/eidas/eidas_idp_conf.py | 6 ++--- tests/eidas/test_idp.py | 47 +++++++++++------------------------ 4 files changed, 23 insertions(+), 56 deletions(-) diff --git a/src/saml2/config.py b/src/saml2/config.py index fcc53c3af..63353dc4a 100644 --- a/src/saml2/config.py +++ b/src/saml2/config.py @@ -129,7 +129,6 @@ "node_country", "application_identifier", "protocol_version", - "supported_loa" ] PDP_ARGS = ["endpoints", "name_form", "name_id_format"] @@ -744,14 +743,13 @@ def get_node_country(self): def verify_non_notified_loa(self): return not any( - x.startswith(EIDAS_NOTIFIED_LOA_PREFIX) - for x in getattr(self, "_idp_supported_loa", {}).get("non_notified", []) - ) + x.startswith(EIDAS_NOTIFIED_LOA_PREFIX) and x not in EIDAS_NOTIFIED_LOA + for x in getattr(self, "assurance_certification", [])) def verify_notified_loa(self): - return all( + return any( x in EIDAS_NOTIFIED_LOA - for x in getattr(self, "_idp_supported_loa", {}).get("notified", []) + for x in getattr(self, "assurance_certification", []) ) @property @@ -767,10 +765,10 @@ def error_validators(self): "provided_attributes MUST be set to denote the supported attributes by " "the IdP": not_empty(getattr(self, "_idp_provided_attributes", None)), - "supported_loa for non-notified eIDs MUST NOT use an " + "assurance_certification for non-notified eIDs MUST NOT use an " "{} prefix".format(EIDAS_NOTIFIED_LOA_PREFIX): self.verify_non_notified_loa(), - "supported_loa for notified eID MUST be (at least) one of [{}]" + "assurance_certification for notified eID MUST be (at least) one of [{}]" .format(", ".join(EIDAS_NOTIFIED_LOA)): self.verify_notified_loa(), "sign_response MUST be set to True": diff --git a/src/saml2/metadata.py b/src/saml2/metadata.py index 0a423fd52..ddac542bb 100644 --- a/src/saml2/metadata.py +++ b/src/saml2/metadata.py @@ -807,18 +807,6 @@ def entity_descriptor(confd): ) _add_attr_to_entity_attributes(entd.extensions, attr) - loa = confd.getattr("supported_loa", confd.context) - if loa: - entd.extensions = entd.extensions or md.Extensions() - ava = [AttributeValue(text=str(c)) for c in make_list( - loa.get("notified", []), loa.get("non_notified", []))] - attr = Attribute( - attribute_value=ava, - name="urn:oasis:names:tc:SAML:attribute:assurance-certification", - name_format="urn:oasis:names:tc:saml2:2.0:attrname-format:uri" - ) - _add_attr_to_entity_attributes(entd.extensions, attr) - return entd diff --git a/tests/eidas/eidas_idp_conf.py b/tests/eidas/eidas_idp_conf.py index 5471b26bf..515501b99 100644 --- a/tests/eidas/eidas_idp_conf.py +++ b/tests/eidas/eidas_idp_conf.py @@ -10,6 +10,8 @@ BASE = "http://localhost:8088" CONFIG = { + "assurance_certification": ["http://eidas.europa.eu/LoA/high", + "http://eidas.europa.eu/LoA/low"], "entityid": "https://example.org", "name": "Rolands IdP", "service": { @@ -54,10 +56,6 @@ "name_format": "urn:oasis:names:tc:SAML:2.0:attrname-format:uri" } ], - "supported_loa": { - "notified": ["http://eidas.europa.eu/LoA/high"], - "non_notified": ["http://eidas.europa.eu/NotNotified/LoA/high"] - }, "sign_response": True, "encrypt_assertion": True }, diff --git a/tests/eidas/test_idp.py b/tests/eidas/test_idp.py index 0b57184cd..a568cb0ad 100644 --- a/tests/eidas/test_idp.py +++ b/tests/eidas/test_idp.py @@ -39,12 +39,12 @@ def test_application_identifier_in_metadata(self): entd.extensions.extension_elements)) app_identifier = [ x for x in entity_attributes.children - if getattr(x, "attributes", {}).get("Name") == + if getattr(x, "name", "") == "http://eidas.europa.eu/entity-attributes/application-identifier" ] assert len(app_identifier) == 1 assert self.conf._idp_application_identifier \ - == next(x.text for y in app_identifier for x in y.children) + == next(x.attribute_value.text for x in app_identifier) def test_multiple_protocol_version_in_metadata(self): entd = metadata.entity_descriptor(self.conf) @@ -97,25 +97,15 @@ def test_loa_attribute_exposed(self, config): entd.extensions.extension_elements)) loa_attribute = next( (x for x in entity_attributes.children - if getattr(x, "name", "") == + if getattr(x, "attributes", {}).get("Name") == "urn:oasis:names:tc:SAML:attribute:assurance-certification"), None ) assert loa_attribute is not None - assert loa_attribute.name_format == "urn:oasis:names:tc:saml2:2.0:attrname-format:uri" + assert loa_attribute.attributes.get("NameFormat", "") == \ + "urn:oasis:names:tc:SAML:2.0:attrname-format:uri" + loa_values = {x.text for x in loa_attribute.children} - def test_loa_attribute_values_exposes(self, config): - entd = metadata.entity_descriptor(self.conf) - entity_attributes = next(filter(lambda x: x.tag == "EntityAttributes", - entd.extensions.extension_elements)) - loa_attribute = next( - (x for x in entity_attributes.children - if getattr(x, "name", "") == - "urn:oasis:names:tc:SAML:attribute:assurance-certification"), None - ) - assert loa_attribute is not None - loa_values = {x.text for x in loa_attribute.attribute_value} - assert loa_values == set(make_list(*config["service"]["idp"][ - "supported_loa"].values())) + assert loa_values == set(config["assurance_certification"]) class TestIdPConfig: @@ -299,26 +289,19 @@ def test_provided_attributes_unset(self, config): self.assert_validation_error(config) - def test_notified_loa_unset(self, config, raise_error_on_warning): - del config["service"]["idp"]["supported_loa"]["notified"] - - self.config_validate(config) - - def test_notified_loa_in_non_notified(self, config): - config["service"]["idp"]["supported_loa"]["non_notified"] = \ - ["http://eidas.europa.eu/LoA/high"] + def test_assurance_certification_unset(self, config): + del config["assurance_certification"] self.assert_validation_error(config) - def test_non_notified_loa_unset(self, config, raise_error_on_warning): - del config["service"]["idp"]["supported_loa"]["non_notified"] + def test_non_notified_loa_invalid_prefix(self, config): + config["assurance_certification"] = \ + ["http://eidas.europa.eu/LoA/something-else"] - self.config_validate(config) + self.assert_validation_error(config) - def test_notified_loa_wrong(self, config): - config["service"]["idp"]["supported_loa"]["notified"] = \ - config["service"]["idp"]["supported_loa"]["notified"] \ - + ["http://eidas.europa.eu/LoA/something-else"] + def test_no_notified_loa(self, config): + config["assurance_certification"] = ["http://example.com/something-else"] self.assert_validation_error(config)