From 38d559d31736ebf3e997708372f01771d5218226 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Wed, 13 May 2020 16:44:16 +0200 Subject: [PATCH 01/26] Add basic support for the SAML2 Artifact Binding. --- src/onelogin/saml2/artifact_resolve.py | 93 +++++++++++++++ src/onelogin/saml2/artifact_response.py | 144 ++++++++++++++++++++++++ src/onelogin/saml2/auth.py | 35 ++++++ src/onelogin/saml2/constants.py | 4 +- src/onelogin/saml2/response.py | 55 ++++----- src/onelogin/saml2/xml_templates.py | 19 ++++ 6 files changed, 322 insertions(+), 28 deletions(-) create mode 100644 src/onelogin/saml2/artifact_resolve.py create mode 100644 src/onelogin/saml2/artifact_response.py diff --git a/src/onelogin/saml2/artifact_resolve.py b/src/onelogin/saml2/artifact_resolve.py new file mode 100644 index 00000000..dbfe3b7f --- /dev/null +++ b/src/onelogin/saml2/artifact_resolve.py @@ -0,0 +1,93 @@ +from base64 import b64decode +from binascii import hexlify +from hashlib import sha1 + +import requests +from onelogin.saml2.utils import OneLogin_Saml2_Utils +from onelogin.saml2.xml_templates import OneLogin_Saml2_Templates + +from .errors import OneLogin_Saml2_ValidationError + + +def parse_saml2_artifact(artifact): + # + # SAMLBind - See 3.6.4 Artifact Format, for SAMLart format. + # + decoded = b64decode(artifact) + type_code = b'\x00\x04' + + if decoded[:2] != type_code: + raise OneLogin_Saml2_ValidationError() + + index = str(int.from_bytes(decoded[2:4], byteorder="big")) + sha1_entity_id = decoded[4:24] + message_handle = decoded[24:44] + + return index, sha1_entity_id, message_handle + + +class Artifact_Resolve_Request: + def __init__(self, settings, saml_art): + self.__settings = settings + self.validate_saml2_artifact(saml_art) + self.saml_art = saml_art + + sp_data = self.__settings.get_sp_data() + + uid = OneLogin_Saml2_Utils.generate_unique_id() + self.__id = uid + + issue_instant = OneLogin_Saml2_Utils.parse_time_to_SAML(OneLogin_Saml2_Utils.now()) + + request = OneLogin_Saml2_Templates.ARTIFACT_RESOLVE_REQUEST % \ + { + 'id': uid, + 'issue_instant': issue_instant, + 'entity_id': sp_data['entityId'], + 'artifact': saml_art + } + + self.__artifact_resolve_request = request + + def validate_saml2_artifact(self, saml_art): + index, sha1_entity_id, message_handle = parse_saml2_artifact(saml_art) + + idp = self.__settings.get_idp_data() + if idp['assertionConsumerService']['index'] != index: + raise OneLogin_Saml2_ValidationError( + "The SourceID given in the SAML artifact ({}) does not correspond with a " + "configured ACS index ({})".format( + index, idp['assertionConsumerService']['index'] + ) + ) + + if sha1_entity_id != sha1(idp['entityId'].encode('utf-8')).digest(): + raise OneLogin_Saml2_ValidationError() + + def get_soap_request(self): + request = OneLogin_Saml2_Templates.SOAP_ENVELOPE % \ + { + 'soap_body': self.__artifact_resolve_request + } + return request + + def send(self): + idp = self.__settings.get_idp_data() + headers = {"content-type": "application/soap+xml"} + return requests.post( + url=idp['assertionConsumerService']['url'], + cert=( + idp['assertionConsumerService']['clientKey'], + idp['assertionConsumerService']['clientCert'], + ), + data=self.get_soap_request(), + headers=headers, + ) + + def get_id(self): + """ + Returns the AuthNRequest ID. + :return: AuthNRequest ID + :rtype: string + """ + return self.__id diff --git a/src/onelogin/saml2/artifact_response.py b/src/onelogin/saml2/artifact_response.py new file mode 100644 index 00000000..11b23a3d --- /dev/null +++ b/src/onelogin/saml2/artifact_response.py @@ -0,0 +1,144 @@ +from base64 import b64encode + +from onelogin.saml2.utils import (OneLogin_Saml2_Utils, + OneLogin_Saml2_ValidationError) +from onelogin.saml2.xml_utils import OneLogin_Saml2_XML + + +class Artifact_Response: + def __init__(self, settings, response): + self.__settings = settings + self.__error = None + self.id = None + + self.__artifact_response = response + + soap_envelope = OneLogin_Saml2_XML.to_etree(self.__artifact_response) + + self.document = OneLogin_Saml2_XML.query( + soap_envelope, '/soap:Envelope/soap:Body' + )[0].getchildren()[0] + + self.id = self.document.get('ID', None) + + def get_issuer(self): + """ + Gets the Issuer of the Logout Response Message + :return: The Issuer + :rtype: string + """ + issuer = None + issuer_nodes = self.__query('/samlp:ArtifactResponse/saml:Issuer') + if len(issuer_nodes) == 1: + issuer = OneLogin_Saml2_XML.element_text(issuer_nodes[0]) + return issuer + + def get_status(self): + """ + Gets the Status + :return: The Status + :rtype: string + """ + entries = self.__query('/samlp:ArtifactResponse/samlp:Status/samlp:StatusCode') + if len(entries) == 0: + return None + status = entries[0].attrib['Value'] + return status + + def is_valid(self, request_id, raise_exceptions=False): + """ + Determines if the SAML ArtifactResponse is valid + :param request_id: The ID of the ArtifactResolve sent by this SP to the IdP + :type request_id: string + + :param raise_exceptions: Whether to return false on failure or raise an exception + :type raise_exceptions: Boolean + + :return: Returns if the SAML ArtifactResponse is or not valid + :rtype: boolean + """ + self.__error = None + try: + idp_data = self.__settings.get_idp_data() + idp_entity_id = idp_data['entityId'] + + if self.__settings.is_strict(): + res = OneLogin_Saml2_XML.validate_xml(self.document, 'saml-schema-protocol-2.0.xsd', self.__settings.is_debug_active()) + if isinstance(res, str): + raise OneLogin_Saml2_ValidationError( + 'Invalid SAML ArtifactResponse. Not match the saml-schema-protocol-2.0.xsd', + OneLogin_Saml2_ValidationError.INVALID_XML_FORMAT + ) + + security = self.__settings.get_security_data() + + # Check if the InResponseTo of the Artifact Response matches the ID of the Artifact Resolve Request (requestId) if provided + in_response_to = self.get_in_response_to() + if in_response_to and in_response_to != request_id: + raise OneLogin_Saml2_ValidationError( + 'The InResponseTo of the Logout Response: %s, does not match the ID of the Logout request sent by the SP: %s' % (in_response_to, request_id), + OneLogin_Saml2_ValidationError.WRONG_INRESPONSETO + ) + + # Check issuer + issuer = self.get_issuer() + if issuer is not None and issuer != idp_entity_id: + raise OneLogin_Saml2_ValidationError( + 'Invalid issuer in the Logout Response (expected %(idpEntityId)s, got %(issuer)s)' % + { + 'idpEntityId': idp_entity_id, + 'issuer': issuer + }, + OneLogin_Saml2_ValidationError.WRONG_ISSUER + ) + + return True + # pylint: disable=R0801 + except Exception as err: + self.__error = str(err) + debug = self.__settings.is_debug_active() + if debug: + print(err) + if raise_exceptions: + raise + return False + + def __query(self, query): + """ + Extracts a node from the Etree (Logout Response Message) + :param query: Xpath Expression + :type query: string + :return: The queried node + :rtype: Element + """ + return OneLogin_Saml2_XML.query(self.document, query) + + def get_in_response_to(self): + """ + Gets the ID of the LogoutRequest which this response is in response to + :returns: ID of LogoutRequest this LogoutResponse is in response to or None if it is not present + :rtype: str + """ + return self.document.get('InResponseTo') + + def get_error(self): + """ + After executing a validation process, if it fails this method returns the cause + """ + return self.__error + + def get_xml(self): + """ + Returns the XML that will be sent as part of the response + or that was received at the SP + :return: XML response body + :rtype: string + """ + return self.__artifact_response + + def get_response_xml(self): + return b64encode( + OneLogin_Saml2_XML.to_string( + self.__query('//samlp:ArtifactResponse/samlp:Response')[0] + ) + ) diff --git a/src/onelogin/saml2/auth.py b/src/onelogin/saml2/auth.py index a67e2071..188d8363 100644 --- a/src/onelogin/saml2/auth.py +++ b/src/onelogin/saml2/auth.py @@ -15,6 +15,8 @@ from onelogin.saml2 import compat from onelogin.saml2.authn_request import OneLogin_Saml2_Authn_Request +from onelogin.saml2.artifact_resolve import Artifact_Resolve_Request +from onelogin.saml2.artifact_response import Artifact_Response from onelogin.saml2.constants import OneLogin_Saml2_Constants from onelogin.saml2.logout_request import OneLogin_Saml2_Logout_Request from onelogin.saml2.logout_response import OneLogin_Saml2_Logout_Response @@ -94,6 +96,39 @@ def set_strict(self, value): assert isinstance(value, bool) self.__settings.set_strict(value) + def artifact_resolve(self, saml_art): + """ + Try to resolve the given artifact + + TODO: should be integrated into the process_response method. + """ + resolve_request = Artifact_Resolve_Request(self.__settings, saml_art) + resolve_response = resolve_request.send() + if resolve_response.status_code != 200: + raise OneLogin_Saml2_ValidationError( + "Received a status code {status_code} when trying to resolve the given artifact {saml_art}".format( + status_code=resolve_response.status_code, saml_art=saml_art + ) + ) + + artifact_response = Artifact_Response(self.__settings, resolve_response.content) + if not artifact_response.is_valid(resolve_request.get_id()): + raise OneLogin_Saml2_ValidationError( + "The ArtifactResponse could not be validated due to the following error: {error}".format( + error=artifact_response.get_error() + ) + ) + + saml2_response = OneLogin_Saml2_Response(self.__settings, artifact_response.get_response_xml()) + if not saml2_response.is_valid(self.__request_data, check_signatures=False): + raise OneLogin_Saml2_ValidationError( + "The Response could not be validated due to the following error: {error}".format( + error=artifact_response.get_error() + ) + ) + + return saml2_response + def process_response(self, request_id=None): """ Process the SAML Response sent by the IdP. diff --git a/src/onelogin/saml2/constants.py b/src/onelogin/saml2/constants.py index e85a7fb9..a9d60f25 100644 --- a/src/onelogin/saml2/constants.py +++ b/src/onelogin/saml2/constants.py @@ -56,6 +56,7 @@ class OneLogin_Saml2_Constants(object): NS_PREFIX_XSD = 'xsd' NS_PREFIX_XENC = 'xenc' NS_PREFIX_DS = 'ds' + NS_PREFIX_SOAP = 'soap' # Prefix:Namespace Mappings NSMAP = { @@ -63,7 +64,8 @@ class OneLogin_Saml2_Constants(object): NS_PREFIX_SAML: NS_SAML, NS_PREFIX_DS: NS_DS, NS_PREFIX_XENC: NS_XENC, - NS_PREFIX_MD: NS_MD + NS_PREFIX_MD: NS_MD, + NS_PREFIX_SOAP: NS_SOAP } # Bindings diff --git a/src/onelogin/saml2/response.py b/src/onelogin/saml2/response.py index 04264919..a19aa2b0 100644 --- a/src/onelogin/saml2/response.py +++ b/src/onelogin/saml2/response.py @@ -48,7 +48,7 @@ def __init__(self, settings, response): self.encrypted = True self.decrypted_document = self.__decrypt_assertion(decrypted_document) - def is_valid(self, request_data, request_id=None, raise_exceptions=False): + def is_valid(self, request_data, request_id=None, check_signatures=True, raise_exceptions=False): """ Validates the response object. @@ -287,35 +287,36 @@ def is_valid(self, request_data, request_id=None, raise_exceptions=False): OneLogin_Saml2_ValidationError.NO_SIGNED_MESSAGE ) - if not signed_elements or (not has_signed_response and not has_signed_assertion): - raise OneLogin_Saml2_ValidationError( - 'No Signature found. SAML Response rejected', - OneLogin_Saml2_ValidationError.NO_SIGNATURE_FOUND - ) - else: - cert = self.__settings.get_idp_cert() - fingerprint = idp_data.get('certFingerprint', None) - if fingerprint: - fingerprint = OneLogin_Saml2_Utils.format_finger_print(fingerprint) - fingerprintalg = idp_data.get('certFingerprintAlgorithm', None) - - multicerts = None - if 'x509certMulti' in idp_data and 'signing' in idp_data['x509certMulti'] and idp_data['x509certMulti']['signing']: - multicerts = idp_data['x509certMulti']['signing'] - - # If find a Signature on the Response, validates it checking the original response - if has_signed_response and not OneLogin_Saml2_Utils.validate_sign(self.document, cert, fingerprint, fingerprintalg, xpath=OneLogin_Saml2_Utils.RESPONSE_SIGNATURE_XPATH, multicerts=multicerts, raise_exceptions=False): + if check_signatures: + if not signed_elements or (not has_signed_response and not has_signed_assertion): raise OneLogin_Saml2_ValidationError( - 'Signature validation failed. SAML Response rejected', - OneLogin_Saml2_ValidationError.INVALID_SIGNATURE + 'No Signature found. SAML Response rejected', + OneLogin_Saml2_ValidationError.NO_SIGNATURE_FOUND ) + else: + cert = self.__settings.get_idp_cert() + fingerprint = idp_data.get('certFingerprint', None) + if fingerprint: + fingerprint = OneLogin_Saml2_Utils.format_finger_print(fingerprint) + fingerprintalg = idp_data.get('certFingerprintAlgorithm', None) + + multicerts = None + if 'x509certMulti' in idp_data and 'signing' in idp_data['x509certMulti'] and idp_data['x509certMulti']['signing']: + multicerts = idp_data['x509certMulti']['signing'] + + # If find a Signature on the Response, validates it checking the original response + if has_signed_response and not OneLogin_Saml2_Utils.validate_sign(self.document, cert, fingerprint, fingerprintalg, xpath=OneLogin_Saml2_Utils.RESPONSE_SIGNATURE_XPATH, multicerts=multicerts, raise_exceptions=False): + raise OneLogin_Saml2_ValidationError( + 'Signature validation failed. SAML Response rejected', + OneLogin_Saml2_ValidationError.INVALID_SIGNATURE + ) - document_check_assertion = self.decrypted_document if self.encrypted else self.document - if has_signed_assertion and not OneLogin_Saml2_Utils.validate_sign(document_check_assertion, cert, fingerprint, fingerprintalg, xpath=OneLogin_Saml2_Utils.ASSERTION_SIGNATURE_XPATH, multicerts=multicerts, raise_exceptions=False): - raise OneLogin_Saml2_ValidationError( - 'Signature validation failed. SAML Response rejected', - OneLogin_Saml2_ValidationError.INVALID_SIGNATURE - ) + document_check_assertion = self.decrypted_document if self.encrypted else self.document + if has_signed_assertion and not OneLogin_Saml2_Utils.validate_sign(document_check_assertion, cert, fingerprint, fingerprintalg, xpath=OneLogin_Saml2_Utils.ASSERTION_SIGNATURE_XPATH, multicerts=multicerts, raise_exceptions=False): + raise OneLogin_Saml2_ValidationError( + 'Signature validation failed. SAML Response rejected', + OneLogin_Saml2_ValidationError.INVALID_SIGNATURE + ) return True except Exception as err: diff --git a/src/onelogin/saml2/xml_templates.py b/src/onelogin/saml2/xml_templates.py index 306b1afe..e161851c 100644 --- a/src/onelogin/saml2/xml_templates.py +++ b/src/onelogin/saml2/xml_templates.py @@ -33,6 +33,25 @@ class OneLogin_Saml2_Templates(object): %(requested_authn_context_str)s """ + SOAP_ENVELOPE = """\ + + +%(soap_body)s + + + """ + + ARTIFACT_RESOLVE_REQUEST = """\ + + %(entity_id)s + %(artifact)s + + """ + LOGOUT_REQUEST = """\ Date: Mon, 18 May 2020 16:09:08 +0200 Subject: [PATCH 02/26] Add basic support for the metadata parser to parse the artifactResolutionService --- src/onelogin/saml2/artifact_resolve.py | 10 +++++----- src/onelogin/saml2/idp_metadata_parser.py | 17 ++++++++++++++++- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/onelogin/saml2/artifact_resolve.py b/src/onelogin/saml2/artifact_resolve.py index dbfe3b7f..43257896 100644 --- a/src/onelogin/saml2/artifact_resolve.py +++ b/src/onelogin/saml2/artifact_resolve.py @@ -53,11 +53,11 @@ def validate_saml2_artifact(self, saml_art): index, sha1_entity_id, message_handle = parse_saml2_artifact(saml_art) idp = self.__settings.get_idp_data() - if idp['assertionConsumerService']['index'] != index: + if idp['artifactResolutionService']['index'] != index: raise OneLogin_Saml2_ValidationError( "The SourceID given in the SAML artifact ({}) does not correspond with a " "configured ACS index ({})".format( - index, idp['assertionConsumerService']['index'] + index, idp['artifactResolutionService']['index'] ) ) @@ -75,10 +75,10 @@ def send(self): idp = self.__settings.get_idp_data() headers = {"content-type": "application/soap+xml"} return requests.post( - url=idp['assertionConsumerService']['url'], + url=idp['artifactResolutionService']['url'], cert=( - idp['assertionConsumerService']['clientKey'], - idp['assertionConsumerService']['clientCert'], + idp['artifactResolutionService']['clientKey'], + idp['artifactResolutionService']['clientCert'], ), data=self.get_soap_request(), headers=headers, diff --git a/src/onelogin/saml2/idp_metadata_parser.py b/src/onelogin/saml2/idp_metadata_parser.py index 2d7eec2c..0b770342 100644 --- a/src/onelogin/saml2/idp_metadata_parser.py +++ b/src/onelogin/saml2/idp_metadata_parser.py @@ -126,7 +126,7 @@ def parse( data = {} dom = OneLogin_Saml2_XML.to_etree(idp_metadata) - idp_entity_id = want_authn_requests_signed = idp_name_id_format = idp_sso_url = idp_slo_url = certs = None + idp_entity_id = want_authn_requests_signed = idp_name_id_format = idp_sso_url = idp_slo_url = idp_ars_url = idp_ars_index = certs = None entity_desc_path = '//md:EntityDescriptor' if entity_id: @@ -163,6 +163,15 @@ def parse( if len(slo_nodes) > 0: idp_slo_url = slo_nodes[0].get('Location', None) + ars_nodes = OneLogin_Saml2_XML.query( + idp_descriptor_node, + "./md:ArtifactResolutionService[@Binding='urn:oasis:names:tc:SAML:2.0:bindings:SOAP']" + ) + + if len(ars_nodes) > 0: + idp_ars_url = ars_nodes[0].get('Location', None) + idp_ars_index = ars_nodes[0].get('index', None) + signing_nodes = OneLogin_Saml2_XML.query(idp_descriptor_node, "./md:KeyDescriptor[not(contains(@use, 'encryption'))]/ds:KeyInfo/ds:X509Data/ds:X509Certificate") encryption_nodes = OneLogin_Saml2_XML.query(idp_descriptor_node, "./md:KeyDescriptor[not(contains(@use, 'signing'))]/ds:KeyInfo/ds:X509Data/ds:X509Certificate") @@ -192,6 +201,12 @@ def parse( data['idp']['singleLogoutService']['url'] = idp_slo_url data['idp']['singleLogoutService']['binding'] = required_slo_binding + if idp_ars_url is not None: + data['idp']['artifactResolutionService'] = {} + data['idp']['artifactResolutionService']['url'] = idp_ars_url + data['idp']['artifactResolutionService']['index'] = idp_ars_index + data['idp']['artifactResolutionService']['binding'] = "urn:oasis:names:tc:SAML:2.0:bindings:SOAP" + if want_authn_requests_signed is not None: data['security'] = {} data['security']['authnRequestsSigned'] = want_authn_requests_signed From 3b57911381afe4070d222be470316d4d90ebd4cc Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Tue, 19 May 2020 11:55:44 +0200 Subject: [PATCH 03/26] Oops, I reversed the certs/key options for Requests. --- src/onelogin/saml2/artifact_resolve.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/onelogin/saml2/artifact_resolve.py b/src/onelogin/saml2/artifact_resolve.py index 43257896..0100ce4c 100644 --- a/src/onelogin/saml2/artifact_resolve.py +++ b/src/onelogin/saml2/artifact_resolve.py @@ -77,8 +77,8 @@ def send(self): return requests.post( url=idp['artifactResolutionService']['url'], cert=( - idp['artifactResolutionService']['clientKey'], idp['artifactResolutionService']['clientCert'], + idp['artifactResolutionService']['clientKey'], ), data=self.get_soap_request(), headers=headers, From 34c084d547b6405071dd9970b2de07992572e3b1 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Tue, 19 May 2020 11:56:10 +0200 Subject: [PATCH 04/26] Add signing to the ArtifactResolve message. It looks like it is required for Digi-ID. --- src/onelogin/saml2/artifact_resolve.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/onelogin/saml2/artifact_resolve.py b/src/onelogin/saml2/artifact_resolve.py index 0100ce4c..33024c55 100644 --- a/src/onelogin/saml2/artifact_resolve.py +++ b/src/onelogin/saml2/artifact_resolve.py @@ -5,6 +5,7 @@ import requests from onelogin.saml2.utils import OneLogin_Saml2_Utils from onelogin.saml2.xml_templates import OneLogin_Saml2_Templates +from onelogin.saml2.constants import OneLogin_Saml2_Constants from .errors import OneLogin_Saml2_ValidationError @@ -69,7 +70,13 @@ def get_soap_request(self): { 'soap_body': self.__artifact_resolve_request } - return request + + return OneLogin_Saml2_Utils.add_sign( + request, + self.__settings.get_sp_key(), self.__settings.get_sp_cert(), + sign_algorithm=OneLogin_Saml2_Constants.RSA_SHA256, + digest_algorithm=OneLogin_Saml2_Constants.SHA256, + ) def send(self): idp = self.__settings.get_idp_data() From 68ecd46a122c57339197bf4ef5988f4abc6f3831 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Wed, 20 May 2020 15:28:04 +0200 Subject: [PATCH 05/26] Add logging for requests. --- src/onelogin/saml2/artifact_resolve.py | 19 ++++++++++++++++--- src/onelogin/saml2/artifact_response.py | 1 - src/onelogin/saml2/auth.py | 13 +++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/onelogin/saml2/artifact_resolve.py b/src/onelogin/saml2/artifact_resolve.py index 33024c55..c8c1e0d1 100644 --- a/src/onelogin/saml2/artifact_resolve.py +++ b/src/onelogin/saml2/artifact_resolve.py @@ -1,8 +1,10 @@ +import logging +import requests + from base64 import b64decode from binascii import hexlify from hashlib import sha1 -import requests from onelogin.saml2.utils import OneLogin_Saml2_Utils from onelogin.saml2.xml_templates import OneLogin_Saml2_Templates from onelogin.saml2.constants import OneLogin_Saml2_Constants @@ -10,6 +12,9 @@ from .errors import OneLogin_Saml2_ValidationError +logger = logging.getLogger(__name__) + + def parse_saml2_artifact(artifact): # # SAMLBind - See 3.6.4 Artifact Format, for SAMLart format. @@ -81,13 +86,21 @@ def get_soap_request(self): def send(self): idp = self.__settings.get_idp_data() headers = {"content-type": "application/soap+xml"} + url = idp['artifactResolutionService']['url'] + data = self.get_soap_request() + + logger.debug( + "Doing a ArtifactResolve (POST) request to %s with data %s", + url, data + ) + return requests.post( - url=idp['artifactResolutionService']['url'], + url=url, cert=( idp['artifactResolutionService']['clientCert'], idp['artifactResolutionService']['clientKey'], ), - data=self.get_soap_request(), + data=data, headers=headers, ) diff --git a/src/onelogin/saml2/artifact_response.py b/src/onelogin/saml2/artifact_response.py index 11b23a3d..a724976a 100644 --- a/src/onelogin/saml2/artifact_response.py +++ b/src/onelogin/saml2/artifact_response.py @@ -91,7 +91,6 @@ def is_valid(self, request_id, raise_exceptions=False): }, OneLogin_Saml2_ValidationError.WRONG_ISSUER ) - return True # pylint: disable=R0801 except Exception as err: diff --git a/src/onelogin/saml2/auth.py b/src/onelogin/saml2/auth.py index 188d8363..e1eb9d5e 100644 --- a/src/onelogin/saml2/auth.py +++ b/src/onelogin/saml2/auth.py @@ -11,8 +11,10 @@ """ +import logging import xmlsec + from onelogin.saml2 import compat from onelogin.saml2.authn_request import OneLogin_Saml2_Authn_Request from onelogin.saml2.artifact_resolve import Artifact_Resolve_Request @@ -26,6 +28,9 @@ from onelogin.saml2.xmlparser import tostring +logger = logging.getLogger(__name__) + + class OneLogin_Saml2_Auth(object): """ @@ -102,6 +107,10 @@ def artifact_resolve(self, saml_art): TODO: should be integrated into the process_response method. """ + logger.debug( + "Retrieved the SAMLArt %s via the ACS.", saml_art + ) + resolve_request = Artifact_Resolve_Request(self.__settings, saml_art) resolve_response = resolve_request.send() if resolve_response.status_code != 200: @@ -111,6 +120,10 @@ def artifact_resolve(self, saml_art): ) ) + logger.debug( + "Retrieved a ArtifactResponse with content %s", resolve_response.content + ) + artifact_response = Artifact_Response(self.__settings, resolve_response.content) if not artifact_response.is_valid(resolve_request.get_id()): raise OneLogin_Saml2_ValidationError( From a1140a300dc23d3f5dca0d27879cf70e8333aa04 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Tue, 26 May 2020 15:34:14 +0200 Subject: [PATCH 06/26] Oops, use the proper error message --- src/onelogin/saml2/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/onelogin/saml2/auth.py b/src/onelogin/saml2/auth.py index e1eb9d5e..07705ed1 100644 --- a/src/onelogin/saml2/auth.py +++ b/src/onelogin/saml2/auth.py @@ -136,7 +136,7 @@ def artifact_resolve(self, saml_art): if not saml2_response.is_valid(self.__request_data, check_signatures=False): raise OneLogin_Saml2_ValidationError( "The Response could not be validated due to the following error: {error}".format( - error=artifact_response.get_error() + error=saml2_response.get_error() ) ) From 88ff69242567064caa2dc5bd5fd6a1e583b7c0e8 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Thu, 28 May 2020 15:11:49 +0200 Subject: [PATCH 07/26] Undo the 'check_signatures' hack I introduced earlier. --- src/onelogin/saml2/auth.py | 2 +- src/onelogin/saml2/response.py | 55 +++++++++++++++++----------------- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/onelogin/saml2/auth.py b/src/onelogin/saml2/auth.py index 07705ed1..070320db 100644 --- a/src/onelogin/saml2/auth.py +++ b/src/onelogin/saml2/auth.py @@ -133,7 +133,7 @@ def artifact_resolve(self, saml_art): ) saml2_response = OneLogin_Saml2_Response(self.__settings, artifact_response.get_response_xml()) - if not saml2_response.is_valid(self.__request_data, check_signatures=False): + if not saml2_response.is_valid(self.__request_data): raise OneLogin_Saml2_ValidationError( "The Response could not be validated due to the following error: {error}".format( error=saml2_response.get_error() diff --git a/src/onelogin/saml2/response.py b/src/onelogin/saml2/response.py index a19aa2b0..04264919 100644 --- a/src/onelogin/saml2/response.py +++ b/src/onelogin/saml2/response.py @@ -48,7 +48,7 @@ def __init__(self, settings, response): self.encrypted = True self.decrypted_document = self.__decrypt_assertion(decrypted_document) - def is_valid(self, request_data, request_id=None, check_signatures=True, raise_exceptions=False): + def is_valid(self, request_data, request_id=None, raise_exceptions=False): """ Validates the response object. @@ -287,36 +287,35 @@ def is_valid(self, request_data, request_id=None, check_signatures=True, raise_e OneLogin_Saml2_ValidationError.NO_SIGNED_MESSAGE ) - if check_signatures: - if not signed_elements or (not has_signed_response and not has_signed_assertion): + if not signed_elements or (not has_signed_response and not has_signed_assertion): + raise OneLogin_Saml2_ValidationError( + 'No Signature found. SAML Response rejected', + OneLogin_Saml2_ValidationError.NO_SIGNATURE_FOUND + ) + else: + cert = self.__settings.get_idp_cert() + fingerprint = idp_data.get('certFingerprint', None) + if fingerprint: + fingerprint = OneLogin_Saml2_Utils.format_finger_print(fingerprint) + fingerprintalg = idp_data.get('certFingerprintAlgorithm', None) + + multicerts = None + if 'x509certMulti' in idp_data and 'signing' in idp_data['x509certMulti'] and idp_data['x509certMulti']['signing']: + multicerts = idp_data['x509certMulti']['signing'] + + # If find a Signature on the Response, validates it checking the original response + if has_signed_response and not OneLogin_Saml2_Utils.validate_sign(self.document, cert, fingerprint, fingerprintalg, xpath=OneLogin_Saml2_Utils.RESPONSE_SIGNATURE_XPATH, multicerts=multicerts, raise_exceptions=False): raise OneLogin_Saml2_ValidationError( - 'No Signature found. SAML Response rejected', - OneLogin_Saml2_ValidationError.NO_SIGNATURE_FOUND + 'Signature validation failed. SAML Response rejected', + OneLogin_Saml2_ValidationError.INVALID_SIGNATURE ) - else: - cert = self.__settings.get_idp_cert() - fingerprint = idp_data.get('certFingerprint', None) - if fingerprint: - fingerprint = OneLogin_Saml2_Utils.format_finger_print(fingerprint) - fingerprintalg = idp_data.get('certFingerprintAlgorithm', None) - - multicerts = None - if 'x509certMulti' in idp_data and 'signing' in idp_data['x509certMulti'] and idp_data['x509certMulti']['signing']: - multicerts = idp_data['x509certMulti']['signing'] - - # If find a Signature on the Response, validates it checking the original response - if has_signed_response and not OneLogin_Saml2_Utils.validate_sign(self.document, cert, fingerprint, fingerprintalg, xpath=OneLogin_Saml2_Utils.RESPONSE_SIGNATURE_XPATH, multicerts=multicerts, raise_exceptions=False): - raise OneLogin_Saml2_ValidationError( - 'Signature validation failed. SAML Response rejected', - OneLogin_Saml2_ValidationError.INVALID_SIGNATURE - ) - document_check_assertion = self.decrypted_document if self.encrypted else self.document - if has_signed_assertion and not OneLogin_Saml2_Utils.validate_sign(document_check_assertion, cert, fingerprint, fingerprintalg, xpath=OneLogin_Saml2_Utils.ASSERTION_SIGNATURE_XPATH, multicerts=multicerts, raise_exceptions=False): - raise OneLogin_Saml2_ValidationError( - 'Signature validation failed. SAML Response rejected', - OneLogin_Saml2_ValidationError.INVALID_SIGNATURE - ) + document_check_assertion = self.decrypted_document if self.encrypted else self.document + if has_signed_assertion and not OneLogin_Saml2_Utils.validate_sign(document_check_assertion, cert, fingerprint, fingerprintalg, xpath=OneLogin_Saml2_Utils.ASSERTION_SIGNATURE_XPATH, multicerts=multicerts, raise_exceptions=False): + raise OneLogin_Saml2_ValidationError( + 'Signature validation failed. SAML Response rejected', + OneLogin_Saml2_ValidationError.INVALID_SIGNATURE + ) return True except Exception as err: From 4acbf90d5ae381a36682a181c6fec64f9537efa4 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Thu, 28 May 2020 15:14:03 +0200 Subject: [PATCH 08/26] The metadataparser now sets artifactResolutionService. Adapt the tests accordingly. --- .../saml2_tests/idp_metadata_parser_test.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/src/OneLogin/saml2_tests/idp_metadata_parser_test.py b/tests/src/OneLogin/saml2_tests/idp_metadata_parser_test.py index 4aa653b5..68113ebd 100644 --- a/tests/src/OneLogin/saml2_tests/idp_metadata_parser_test.py +++ b/tests/src/OneLogin/saml2_tests/idp_metadata_parser_test.py @@ -81,6 +81,11 @@ def testParseRemote(self): "singleSignOnService": { "url": "https://idp.testshib.org/idp/profile/SAML2/Redirect/SSO", "binding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" + }, + "artifactResolutionService": { + "url": "https://idp.testshib.org:8443/idp/profile/SAML2/SOAP/ArtifactResolution", + "index": "2", + "binding": "urn:oasis:names:tc:SAML:2.0:bindings:SOAP" } } } @@ -142,6 +147,11 @@ def test_parse_testshib_required_binding_sso_redirect(self): "singleSignOnService": { "url": "https://idp.testshib.org/idp/profile/SAML2/Redirect/SSO", "binding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" + }, + "artifactResolutionService": { + "url": "https://idp.testshib.org:8443/idp/profile/SAML2/SOAP/ArtifactResolution", + "index": "2", + "binding": "urn:oasis:names:tc:SAML:2.0:bindings:SOAP" } } } @@ -180,6 +190,11 @@ def test_parse_testshib_required_binding_sso_post(self): "singleSignOnService": { "url": "https://idp.testshib.org/idp/profile/SAML2/POST/SSO", "binding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" + }, + "artifactResolutionService": { + "url": "https://idp.testshib.org:8443/idp/profile/SAML2/SOAP/ArtifactResolution", + "index": "2", + "binding": "urn:oasis:names:tc:SAML:2.0:bindings:SOAP" } } } From 9c25d0547aca5f9b81b9ccc2bbd518e993c625dd Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Fri, 29 May 2020 11:07:15 +0200 Subject: [PATCH 09/26] Load the soap SSL/TLS key from the security setting instead. --- src/onelogin/saml2/artifact_resolve.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/onelogin/saml2/artifact_resolve.py b/src/onelogin/saml2/artifact_resolve.py index c8c1e0d1..bad39c9e 100644 --- a/src/onelogin/saml2/artifact_resolve.py +++ b/src/onelogin/saml2/artifact_resolve.py @@ -85,6 +85,7 @@ def get_soap_request(self): def send(self): idp = self.__settings.get_idp_data() + security_data = self.__settings.get_security_data() headers = {"content-type": "application/soap+xml"} url = idp['artifactResolutionService']['url'] data = self.get_soap_request() @@ -93,12 +94,11 @@ def send(self): "Doing a ArtifactResolve (POST) request to %s with data %s", url, data ) - return requests.post( url=url, cert=( - idp['artifactResolutionService']['clientCert'], - idp['artifactResolutionService']['clientKey'], + security_data['soapClientCert'], + security_data['soapClientKey'], ), data=data, headers=headers, From aec78a22d5e75dd98899e82faeddb2ee369ffecd Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Fri, 29 May 2020 11:07:25 +0200 Subject: [PATCH 10/26] Cleanup --- src/onelogin/saml2/artifact_resolve.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/onelogin/saml2/artifact_resolve.py b/src/onelogin/saml2/artifact_resolve.py index bad39c9e..bd312491 100644 --- a/src/onelogin/saml2/artifact_resolve.py +++ b/src/onelogin/saml2/artifact_resolve.py @@ -2,7 +2,6 @@ import requests from base64 import b64decode -from binascii import hexlify from hashlib import sha1 from onelogin.saml2.utils import OneLogin_Saml2_Utils From 8047805e7fc02180683e2c5f1168567ed87bbe9d Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Thu, 28 May 2020 15:17:07 +0200 Subject: [PATCH 11/26] Ignore the Advice section when looking for signatures. --- src/onelogin/saml2/response.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/onelogin/saml2/response.py b/src/onelogin/saml2/response.py index 04264919..0d50b471 100644 --- a/src/onelogin/saml2/response.py +++ b/src/onelogin/saml2/response.py @@ -666,8 +666,7 @@ def process_signed_elements(self): :returns: The signed elements tag names :rtype: list """ - sign_nodes = self.__query('//ds:Signature') - + sign_nodes = self.__query('//ds:Signature[not(ancestor::saml:Advice)]') signed_elements = [] verified_seis = [] verified_ids = [] From 41f7cf2fab206cae852561138aab465dc22ac53a Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Fri, 29 May 2020 17:16:16 +0200 Subject: [PATCH 12/26] Refactor the way the SAML Artifact are processed. In the previous setup I used 1 artifactResolutionService (the first) and it was parsed from the metadata file. However, in the metadata file I am using, the same URL is used, with two different indexes. Since the wrong index was returned, this caused errors. Another implementation would be to just ignore the index/url returned in the SAML artifact. --- src/onelogin/saml2/artifact_resolve.py | 26 ++++++++++--------- src/onelogin/saml2/idp_metadata_parser.py | 19 ++++++++------ .../saml2_tests/idp_metadata_parser_test.py | 12 ++++----- 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/src/onelogin/saml2/artifact_resolve.py b/src/onelogin/saml2/artifact_resolve.py index bd312491..28dd1fb5 100644 --- a/src/onelogin/saml2/artifact_resolve.py +++ b/src/onelogin/saml2/artifact_resolve.py @@ -34,7 +34,7 @@ def parse_saml2_artifact(artifact): class Artifact_Resolve_Request: def __init__(self, settings, saml_art): self.__settings = settings - self.validate_saml2_artifact(saml_art) + self.soap_endpoint = self.find_soap_endpoint(saml_art) self.saml_art = saml_art sp_data = self.__settings.get_sp_data() @@ -54,20 +54,23 @@ def __init__(self, settings, saml_art): self.__artifact_resolve_request = request - def validate_saml2_artifact(self, saml_art): + def find_soap_endpoint(self, saml_art): + idp = self.__settings.get_idp_data() index, sha1_entity_id, message_handle = parse_saml2_artifact(saml_art) - idp = self.__settings.get_idp_data() - if idp['artifactResolutionService']['index'] != index: + if sha1_entity_id != sha1(idp['entityId'].encode('utf-8')).digest(): raise OneLogin_Saml2_ValidationError( - "The SourceID given in the SAML artifact ({}) does not correspond with a " - "configured ACS index ({})".format( - index, idp['artifactResolutionService']['index'] - ) + f"The sha1 hash of the entityId returned in the SAML Artifact ({sha1_entity_id})" + f"does not match the sha1 hash of the configured entityId ({idp['entityId']})" ) - if sha1_entity_id != sha1(idp['entityId'].encode('utf-8')).digest(): - raise OneLogin_Saml2_ValidationError() + for ars_node in idp['artifactResolutionService']: + if ars_node['binding'] != "urn:oasis:names:tc:SAML:2.0:bindings:SOAP": + continue + if ars_node['index'] == index: + return ars_node + + return None def get_soap_request(self): request = OneLogin_Saml2_Templates.SOAP_ENVELOPE % \ @@ -83,10 +86,9 @@ def get_soap_request(self): ) def send(self): - idp = self.__settings.get_idp_data() security_data = self.__settings.get_security_data() headers = {"content-type": "application/soap+xml"} - url = idp['artifactResolutionService']['url'] + url = self.soap_endpoint['url'] data = self.get_soap_request() logger.debug( diff --git a/src/onelogin/saml2/idp_metadata_parser.py b/src/onelogin/saml2/idp_metadata_parser.py index 0b770342..ae2273a3 100644 --- a/src/onelogin/saml2/idp_metadata_parser.py +++ b/src/onelogin/saml2/idp_metadata_parser.py @@ -168,9 +168,7 @@ def parse( "./md:ArtifactResolutionService[@Binding='urn:oasis:names:tc:SAML:2.0:bindings:SOAP']" ) - if len(ars_nodes) > 0: - idp_ars_url = ars_nodes[0].get('Location', None) - idp_ars_index = ars_nodes[0].get('index', None) + signing_nodes = OneLogin_Saml2_XML.query(idp_descriptor_node, "./md:KeyDescriptor[not(contains(@use, 'encryption'))]/ds:KeyInfo/ds:X509Data/ds:X509Certificate") encryption_nodes = OneLogin_Saml2_XML.query(idp_descriptor_node, "./md:KeyDescriptor[not(contains(@use, 'signing'))]/ds:KeyInfo/ds:X509Data/ds:X509Certificate") @@ -201,11 +199,16 @@ def parse( data['idp']['singleLogoutService']['url'] = idp_slo_url data['idp']['singleLogoutService']['binding'] = required_slo_binding - if idp_ars_url is not None: - data['idp']['artifactResolutionService'] = {} - data['idp']['artifactResolutionService']['url'] = idp_ars_url - data['idp']['artifactResolutionService']['index'] = idp_ars_index - data['idp']['artifactResolutionService']['binding'] = "urn:oasis:names:tc:SAML:2.0:bindings:SOAP" + for ars_node in ars_nodes: + idp_ars_url = ars_node.get('Location', None) + idp_ars_index = ars_node.get('index', None) + if idp_ars_url is not None: + ars_list = data['idp'].setdefault('artifactResolutionService', []) + ars_list.append({ + 'url': idp_ars_url, + 'index': idp_ars_index, + 'binding': "urn:oasis:names:tc:SAML:2.0:bindings:SOAP", + }) if want_authn_requests_signed is not None: data['security'] = {} diff --git a/tests/src/OneLogin/saml2_tests/idp_metadata_parser_test.py b/tests/src/OneLogin/saml2_tests/idp_metadata_parser_test.py index 68113ebd..13451a32 100644 --- a/tests/src/OneLogin/saml2_tests/idp_metadata_parser_test.py +++ b/tests/src/OneLogin/saml2_tests/idp_metadata_parser_test.py @@ -82,11 +82,11 @@ def testParseRemote(self): "url": "https://idp.testshib.org/idp/profile/SAML2/Redirect/SSO", "binding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" }, - "artifactResolutionService": { + "artifactResolutionService": [{ "url": "https://idp.testshib.org:8443/idp/profile/SAML2/SOAP/ArtifactResolution", "index": "2", "binding": "urn:oasis:names:tc:SAML:2.0:bindings:SOAP" - } + }] } } """ @@ -148,11 +148,11 @@ def test_parse_testshib_required_binding_sso_redirect(self): "url": "https://idp.testshib.org/idp/profile/SAML2/Redirect/SSO", "binding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" }, - "artifactResolutionService": { + "artifactResolutionService": [{ "url": "https://idp.testshib.org:8443/idp/profile/SAML2/SOAP/ArtifactResolution", "index": "2", "binding": "urn:oasis:names:tc:SAML:2.0:bindings:SOAP" - } + }] } } """ @@ -191,11 +191,11 @@ def test_parse_testshib_required_binding_sso_post(self): "url": "https://idp.testshib.org/idp/profile/SAML2/POST/SSO", "binding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" }, - "artifactResolutionService": { + "artifactResolutionService": [{ "url": "https://idp.testshib.org:8443/idp/profile/SAML2/SOAP/ArtifactResolution", "index": "2", "binding": "urn:oasis:names:tc:SAML:2.0:bindings:SOAP" - } + }] } } """ From 3391a69bcef5867ccc27a6d7ce9719785bc9a089 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Tue, 2 Jun 2020 10:06:25 +0200 Subject: [PATCH 13/26] Onelogin's XML to string method messes up the XML, making the signature invalid. --- src/onelogin/saml2/artifact_response.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/onelogin/saml2/artifact_response.py b/src/onelogin/saml2/artifact_response.py index a724976a..0ad93d21 100644 --- a/src/onelogin/saml2/artifact_response.py +++ b/src/onelogin/saml2/artifact_response.py @@ -3,6 +3,7 @@ from onelogin.saml2.utils import (OneLogin_Saml2_Utils, OneLogin_Saml2_ValidationError) from onelogin.saml2.xml_utils import OneLogin_Saml2_XML +from lxml import etree class Artifact_Response: @@ -137,7 +138,7 @@ def get_xml(self): def get_response_xml(self): return b64encode( - OneLogin_Saml2_XML.to_string( + etree.tostring( self.__query('//samlp:ArtifactResponse/samlp:Response')[0] ) ) From db856a782c49f3dae72d639417ab43eb9b8a2435 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Fri, 19 Jun 2020 15:37:56 +0200 Subject: [PATCH 14/26] Make sure the error code is passed on, when validation error is re-raised. --- src/onelogin/saml2/auth.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/onelogin/saml2/auth.py b/src/onelogin/saml2/auth.py index 070320db..e3097ab4 100644 --- a/src/onelogin/saml2/auth.py +++ b/src/onelogin/saml2/auth.py @@ -133,11 +133,13 @@ def artifact_resolve(self, saml_art): ) saml2_response = OneLogin_Saml2_Response(self.__settings, artifact_response.get_response_xml()) - if not saml2_response.is_valid(self.__request_data): + try: + saml2_response.is_valid(self.__request_data, raise_exceptions=True) + except OneLogin_Saml2_ValidationError as e: raise OneLogin_Saml2_ValidationError( "The Response could not be validated due to the following error: {error}".format( error=saml2_response.get_error() - ) + ), code=e.code ) return saml2_response From e5664d7c7d6e05599f2de12da71a378186bc8fd8 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Fri, 19 Jun 2020 15:43:12 +0200 Subject: [PATCH 15/26] Use defusedxml instead of lxml directly. --- src/onelogin/saml2/artifact_response.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/onelogin/saml2/artifact_response.py b/src/onelogin/saml2/artifact_response.py index 0ad93d21..5f9dd8fa 100644 --- a/src/onelogin/saml2/artifact_response.py +++ b/src/onelogin/saml2/artifact_response.py @@ -1,9 +1,8 @@ from base64 import b64encode - +from defusedxml.lxml import tostring from onelogin.saml2.utils import (OneLogin_Saml2_Utils, OneLogin_Saml2_ValidationError) from onelogin.saml2.xml_utils import OneLogin_Saml2_XML -from lxml import etree class Artifact_Response: @@ -138,7 +137,7 @@ def get_xml(self): def get_response_xml(self): return b64encode( - etree.tostring( + tostring( self.__query('//samlp:ArtifactResponse/samlp:Response')[0] ) ) From 1809527d9f22f2714d5c52a2f403ef9c519bc2fc Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Fri, 19 Jun 2020 15:48:18 +0200 Subject: [PATCH 16/26] Add requests as a dependency. --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index f97f0189..8163bc76 100644 --- a/setup.py +++ b/setup.py @@ -42,6 +42,7 @@ 'lxml>=3.3.5', 'xmlsec>=1.0.5', 'defusedxml==0.6.0' + 'requests>=2.24.0' ], dependency_links=['http://github.com/mehcode/python-xmlsec/tarball/master'], extras_require={ From 0fe4fe5fb89cd70782ebeebd7a36cea3952dab64 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Thu, 25 Jun 2020 14:12:25 +0200 Subject: [PATCH 17/26] Various small documentation/text fixes I noticed going through the code. --- src/onelogin/saml2/artifact_resolve.py | 9 ++++++--- src/onelogin/saml2/artifact_response.py | 10 +++++++--- src/onelogin/saml2/errors.py | 1 - 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/onelogin/saml2/artifact_resolve.py b/src/onelogin/saml2/artifact_resolve.py index 28dd1fb5..9ac0e99b 100644 --- a/src/onelogin/saml2/artifact_resolve.py +++ b/src/onelogin/saml2/artifact_resolve.py @@ -22,7 +22,10 @@ def parse_saml2_artifact(artifact): type_code = b'\x00\x04' if decoded[:2] != type_code: - raise OneLogin_Saml2_ValidationError() + raise OneLogin_Saml2_ValidationError( + "The received Artifact does not have the correct header.", + OneLogin_Saml2_ValidationError.WRONG_ARTIFACT_FORMAT + ) index = str(int.from_bytes(decoded[2:4], byteorder="big")) sha1_entity_id = decoded[4:24] @@ -107,8 +110,8 @@ def send(self): def get_id(self): """ - Returns the AuthNRequest ID. - :return: AuthNRequest ID + Returns the ArtifactResolve ID. + :return: ArtifactResolve ID :rtype: string """ return self.__id diff --git a/src/onelogin/saml2/artifact_response.py b/src/onelogin/saml2/artifact_response.py index 5f9dd8fa..72b13654 100644 --- a/src/onelogin/saml2/artifact_response.py +++ b/src/onelogin/saml2/artifact_response.py @@ -23,7 +23,7 @@ def __init__(self, settings, response): def get_issuer(self): """ - Gets the Issuer of the Logout Response Message + Gets the Issuer of the Artifact Response :return: The Issuer :rtype: string """ @@ -114,8 +114,8 @@ def __query(self, query): def get_in_response_to(self): """ - Gets the ID of the LogoutRequest which this response is in response to - :returns: ID of LogoutRequest this LogoutResponse is in response to or None if it is not present + Gets the ID of the ArtifactResolve which this response is in response to + :returns: ID of ArtifactResolve this LogoutResponse is in response to or None if it is not present :rtype: str """ return self.document.get('InResponseTo') @@ -136,6 +136,10 @@ def get_xml(self): return self.__artifact_response def get_response_xml(self): + """ + The response is base64 encoded to make it possible to feed + it to the OneLogin_Saml2_Response class. + """ return b64encode( tostring( self.__query('//samlp:ArtifactResponse/samlp:Response')[0] diff --git a/src/onelogin/saml2/errors.py b/src/onelogin/saml2/errors.py index 6a50f9f1..0be9e1a5 100644 --- a/src/onelogin/saml2/errors.py +++ b/src/onelogin/saml2/errors.py @@ -119,7 +119,6 @@ def __init__(self, message, code=0, errors=None): * (int) code. The code error (defined in the error class). """ assert isinstance(code, int) - if errors is not None: message = message % errors From 2bbccc8e3be5a3576c8223cc3d959b1a3d973e99 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Fri, 3 Jul 2020 12:58:46 +0200 Subject: [PATCH 18/26] Allow the user to distinguish between technical errors and AuthnFailed. --- src/onelogin/saml2/constants.py | 1 + src/onelogin/saml2/errors.py | 1 + src/onelogin/saml2/response.py | 15 +++++++++++---- src/onelogin/saml2/utils.py | 12 +++++++----- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/onelogin/saml2/constants.py b/src/onelogin/saml2/constants.py index a9d60f25..cee93a83 100644 --- a/src/onelogin/saml2/constants.py +++ b/src/onelogin/saml2/constants.py @@ -96,6 +96,7 @@ class OneLogin_Saml2_Constants(object): STATUS_NO_PASSIVE = 'urn:oasis:names:tc:SAML:2.0:status:NoPassive' STATUS_PARTIAL_LOGOUT = 'urn:oasis:names:tc:SAML:2.0:status:PartialLogout' STATUS_PROXY_COUNT_EXCEEDED = 'urn:oasis:names:tc:SAML:2.0:status:ProxyCountExceeded' + STATUS_AUTHN_FAILED = 'urn:oasis:names:tc:SAML:2.0:status:AuthnFailed' # Sign & Crypto SHA1 = 'http://www.w3.org/2000/09/xmldsig#sha1' diff --git a/src/onelogin/saml2/errors.py b/src/onelogin/saml2/errors.py index 0be9e1a5..5c95633e 100644 --- a/src/onelogin/saml2/errors.py +++ b/src/onelogin/saml2/errors.py @@ -110,6 +110,7 @@ class OneLogin_Saml2_ValidationError(Exception): WRONG_NUMBER_OF_SIGNATURES = 43 RESPONSE_EXPIRED = 44 AUTHN_CONTEXT_MISMATCH = 45 + STATUS_CODE_AUTHNFAILED = 46 def __init__(self, message, code=0, errors=None): """ diff --git a/src/onelogin/saml2/response.py b/src/onelogin/saml2/response.py index 0d50b471..4fd873a4 100644 --- a/src/onelogin/saml2/response.py +++ b/src/onelogin/saml2/response.py @@ -334,17 +334,24 @@ def check_status(self): :raises: Exception. If the status is not success """ status = OneLogin_Saml2_Utils.get_status(self.document) - code = status.get('code', None) - if code and code != OneLogin_Saml2_Constants.STATUS_SUCCESS: - splited_code = code.split(':') + codes = status.get('codes', None) + if codes and codes[0] != OneLogin_Saml2_Constants.STATUS_SUCCESS: + splited_code = codes[0].split(':') printable_code = splited_code.pop() status_exception_msg = 'The status code of the Response was not Success, was %s' % printable_code status_msg = status.get('msg', None) if status_msg: status_exception_msg += ' -> ' + status_msg + + exception_code = OneLogin_Saml2_ValidationError.STATUS_CODE_IS_NOT_SUCCESS + + # See SAMLCore - 3.2.2.2. AuthnFailed is a second-level status code. + if len(codes) >= 2 and codes[1] == OneLogin_Saml2_Constants.STATUS_AUTHN_FAILED: + exception_code = OneLogin_Saml2_ValidationError.STATUS_CODE_AUTHNFAILED + raise OneLogin_Saml2_ValidationError( status_exception_msg, - OneLogin_Saml2_ValidationError.STATUS_CODE_IS_NOT_SUCCESS + exception_code ) def check_one_condition(self): diff --git a/src/onelogin/saml2/utils.py b/src/onelogin/saml2/utils.py index 1290033e..8cc21885 100644 --- a/src/onelogin/saml2/utils.py +++ b/src/onelogin/saml2/utils.py @@ -640,7 +640,9 @@ def get_status(dom): :param dom: The Response as XML :type: Document - :returns: The Status, an array with the code and a message. + :returns: The Status, an array with the code and a message. 'code' entry is the + topmost StatusCode, and 'codes' entry contains the StatusCodes in document + order. :rtype: dict """ status = {} @@ -652,14 +654,14 @@ def get_status(dom): OneLogin_Saml2_ValidationError.MISSING_STATUS ) - code_entry = OneLogin_Saml2_XML.query(dom, '/samlp:Response/samlp:Status/samlp:StatusCode', status_entry[0]) - if len(code_entry) != 1: + code_entries = OneLogin_Saml2_XML.query(dom, '/samlp:Response/samlp:Status//samlp:StatusCode', status_entry[0]) + if not code_entries: raise OneLogin_Saml2_ValidationError( 'Missing Status Code on response', OneLogin_Saml2_ValidationError.MISSING_STATUS_CODE ) - code = code_entry[0].values()[0] - status['code'] = code + status['codes'] = [c.get('Value') for c in code_entries] + status['code'] = status['codes'][0] status['msg'] = '' message_entry = OneLogin_Saml2_XML.query(dom, '/samlp:Response/samlp:Status/samlp:StatusMessage', status_entry[0]) From 76e88d4fc4411629df53c7369013ee018edd3de8 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Fri, 3 Jul 2020 16:08:10 +0200 Subject: [PATCH 19/26] Make sure the status code of the ArtifactResponse is checked as well. --- src/onelogin/saml2/artifact_response.py | 40 +++++++++++++++++++++++-- src/onelogin/saml2/utils.py | 23 ++++++++++---- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/src/onelogin/saml2/artifact_response.py b/src/onelogin/saml2/artifact_response.py index 72b13654..e18d96cb 100644 --- a/src/onelogin/saml2/artifact_response.py +++ b/src/onelogin/saml2/artifact_response.py @@ -1,5 +1,6 @@ from base64 import b64encode from defusedxml.lxml import tostring +from onelogin.saml2.constants import OneLogin_Saml2_Constants from onelogin.saml2.utils import (OneLogin_Saml2_Utils, OneLogin_Saml2_ValidationError) from onelogin.saml2.xml_utils import OneLogin_Saml2_XML @@ -28,7 +29,7 @@ def get_issuer(self): :rtype: string """ issuer = None - issuer_nodes = self.__query('/samlp:ArtifactResponse/saml:Issuer') + issuer_nodes = self.__query('//samlp:ArtifactResponse/saml:Issuer') if len(issuer_nodes) == 1: issuer = OneLogin_Saml2_XML.element_text(issuer_nodes[0]) return issuer @@ -39,12 +40,40 @@ def get_status(self): :return: The Status :rtype: string """ - entries = self.__query('/samlp:ArtifactResponse/samlp:Status/samlp:StatusCode') + entries = self.__query('//samlp:ArtifactResponse/samlp:Status/samlp:StatusCode') if len(entries) == 0: return None status = entries[0].attrib['Value'] return status + def check_status(self): + """ + Check if the status of the response is success or not + + :raises: Exception. If the status is not success + """ + doc = OneLogin_Saml2_XML.query(self.document, '//samlp:ArtifactResponse') + if len(doc) != 1: + raise OneLogin_Saml2_ValidationError( + 'Missing Status on response', + OneLogin_Saml2_ValidationError.MISSING_STATUS + ) + status = OneLogin_Saml2_Utils.get_specific_status( + doc[0], + ) + code = status.get('code', None) + if code and code != OneLogin_Saml2_Constants.STATUS_SUCCESS: + splited_code = code.split(':') + printable_code = splited_code.pop() + status_exception_msg = 'The status code of the ArtifactResponse was not Success, was %s' % printable_code + status_msg = status.get('msg', None) + if status_msg: + status_exception_msg += ' -> ' + status_msg + raise OneLogin_Saml2_ValidationError( + status_exception_msg, + OneLogin_Saml2_ValidationError.STATUS_CODE_IS_NOT_SUCCESS + ) + def is_valid(self, request_id, raise_exceptions=False): """ Determines if the SAML ArtifactResponse is valid @@ -80,6 +109,8 @@ def is_valid(self, request_id, raise_exceptions=False): OneLogin_Saml2_ValidationError.WRONG_INRESPONSETO ) + self.check_status() + # Check issuer issuer = self.get_issuer() if issuer is not None and issuer != idp_entity_id: @@ -91,6 +122,11 @@ def is_valid(self, request_id, raise_exceptions=False): }, OneLogin_Saml2_ValidationError.WRONG_ISSUER ) + status = self.get_status() + if status != 'urn:oasis:names:tc:SAML:2.0:status:Success': + raise OneLogin_Saml2_ValidationError( + OneLogin_Saml2_ValidationError.STATUS_CODE_IS_NOT_SUCCESS + ) return True # pylint: disable=R0801 except Exception as err: diff --git a/src/onelogin/saml2/utils.py b/src/onelogin/saml2/utils.py index 8cc21885..1eb8b923 100644 --- a/src/onelogin/saml2/utils.py +++ b/src/onelogin/saml2/utils.py @@ -632,8 +632,8 @@ def generate_name_id(value, sp_nq, sp_format=None, cert=None, debug=False, nq=No else: return OneLogin_Saml2_XML.extract_tag_text(root, "saml:NameID") - @staticmethod - def get_status(dom): + @classmethod + def get_status(cls, dom): """ Gets Status from a Response. @@ -645,16 +645,27 @@ def get_status(dom): order. :rtype: dict """ + doc = OneLogin_Saml2_XML.query(dom, '/samlp:Response') + if len(doc) != 1: + raise OneLogin_Saml2_ValidationError( + 'Missing Status on response', + OneLogin_Saml2_ValidationError.MISSING_STATUS + ) + + return cls.get_specific_status(doc[0]) + + @staticmethod + def get_specific_status(doc): status = {} - status_entry = OneLogin_Saml2_XML.query(dom, '/samlp:Response/samlp:Status') + status_entry = OneLogin_Saml2_XML.query(doc, './samlp:Status') if len(status_entry) != 1: raise OneLogin_Saml2_ValidationError( 'Missing Status on response', OneLogin_Saml2_ValidationError.MISSING_STATUS ) - code_entries = OneLogin_Saml2_XML.query(dom, '/samlp:Response/samlp:Status//samlp:StatusCode', status_entry[0]) + code_entries = OneLogin_Saml2_XML.query(doc, './/samlp:StatusCode', status_entry[0]) if not code_entries: raise OneLogin_Saml2_ValidationError( 'Missing Status Code on response', @@ -664,9 +675,9 @@ def get_status(dom): status['code'] = status['codes'][0] status['msg'] = '' - message_entry = OneLogin_Saml2_XML.query(dom, '/samlp:Response/samlp:Status/samlp:StatusMessage', status_entry[0]) + message_entry = OneLogin_Saml2_XML.query(doc, './samlp:StatusMessage', status_entry[0]) if len(message_entry) == 0: - subcode_entry = OneLogin_Saml2_XML.query(dom, '/samlp:Response/samlp:Status/samlp:StatusCode/samlp:StatusCode', status_entry[0]) + subcode_entry = OneLogin_Saml2_XML.query(doc, './samlp:StatusCode/samlp:StatusCode', status_entry[0]) if len(subcode_entry) == 1: status['msg'] = subcode_entry[0].values()[0] elif len(message_entry) == 1: From f4fd4b92a42d425882e6a78ecd4fb48fde5b7b96 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Fri, 30 Oct 2020 14:57:59 +0100 Subject: [PATCH 20/26] Incorporate 'artifact_resolve' into the normal 'process_response' method. --- src/onelogin/saml2/auth.py | 41 ++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/onelogin/saml2/auth.py b/src/onelogin/saml2/auth.py index e3097ab4..d44f4d78 100644 --- a/src/onelogin/saml2/auth.py +++ b/src/onelogin/saml2/auth.py @@ -104,8 +104,6 @@ def set_strict(self, value): def artifact_resolve(self, saml_art): """ Try to resolve the given artifact - - TODO: should be integrated into the process_response method. """ logger.debug( "Retrieved the SAMLArt %s via the ACS.", saml_art @@ -144,6 +142,21 @@ def artifact_resolve(self, saml_art): return saml2_response + def store_valid_response(self, response): + self.__attributes = response.get_attributes() + self.__friendlyname_attributes = response.get_friendlyname_attributes() + self.__nameid = response.get_nameid() + self.__nameid_format = response.get_nameid_format() + self.__nameid_nq = response.get_nameid_nq() + self.__nameid_spnq = response.get_nameid_spnq() + self.__session_index = response.get_session_index() + self.__session_expiration = response.get_session_not_on_or_after() + self.__last_message_id = response.get_id() + self.__last_assertion_id = response.get_assertion_id() + self.__last_authn_contexts = response.get_authn_contexts() + self.__authenticated = True + self.__last_assertion_not_on_or_after = response.get_assertion_not_on_or_after() + def process_response(self, request_id=None): """ Process the SAML Response sent by the IdP. @@ -162,24 +175,18 @@ def process_response(self, request_id=None): self.__last_response = response.get_xml_document() if response.is_valid(self.__request_data, request_id): - self.__attributes = response.get_attributes() - self.__friendlyname_attributes = response.get_friendlyname_attributes() - self.__nameid = response.get_nameid() - self.__nameid_format = response.get_nameid_format() - self.__nameid_nq = response.get_nameid_nq() - self.__nameid_spnq = response.get_nameid_spnq() - self.__session_index = response.get_session_index() - self.__session_expiration = response.get_session_not_on_or_after() - self.__last_message_id = response.get_id() - self.__last_assertion_id = response.get_assertion_id() - self.__last_authn_contexts = response.get_authn_contexts() - self.__authenticated = True - self.__last_assertion_not_on_or_after = response.get_assertion_not_on_or_after() - + self.store_valid_response(response) else: self.__errors.append('invalid_response') self.__error_reason = response.get_error() - + elif 'get_data' in self.__request_data and 'SAMLArt' in self.__request_data['get_data']: + try: + response = self.artifact_resolve(self.__request_data['get_data']['SAMLArt']) + except OneLogin_Saml2_ValidationError as e: + self.__errors.append('invalid_response') + self.__error_reason = str(e) + else: + self.store_valid_response(response) else: self.__errors.append('invalid_binding') raise OneLogin_Saml2_Error( From 11a8beb755a599a3979443f2b2829cae97e5285b Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Fri, 30 Oct 2020 14:58:54 +0100 Subject: [PATCH 21/26] Add tests for Artifact Resolve/Artifact Binding using the OneLogin_Saml2_Auth class --- .../artifact_response/artifact_response.xml | 56 ++++++++++++ tests/settings/settings11.json | 33 +++++++ tests/src/OneLogin/saml2_tests/auth_test.py | 86 +++++++++++++++++++ 3 files changed, 175 insertions(+) create mode 100644 tests/data/artifact_response/artifact_response.xml create mode 100644 tests/settings/settings11.json diff --git a/tests/data/artifact_response/artifact_response.xml b/tests/data/artifact_response/artifact_response.xml new file mode 100644 index 00000000..7c435cba --- /dev/null +++ b/tests/data/artifact_response/artifact_response.xml @@ -0,0 +1,56 @@ + + + + https://idp.com/saml/idp/metadata + + + + + https://idp.com/saml/idp/metadata + + + + + + + + + + + + + + + + + + + + + + + + + https://idp.com/saml/idp/metadata + + s00000000:12345678 + + + + + + + sp.com + + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + + + + + diff --git a/tests/settings/settings11.json b/tests/settings/settings11.json new file mode 100644 index 00000000..71a1ddd9 --- /dev/null +++ b/tests/settings/settings11.json @@ -0,0 +1,33 @@ +{ + "strict": false, + "debug": false, + "custom_base_path": "../../../tests/data/customPath/", + "sp": { + "entityId": "http://stuff.com/endpoints/metadata.php", + "assertionConsumerService": { + "url": "http://stuff.com/endpoints/endpoints/acs.php", + "binding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact" + }, + "NameIDFormat": "urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified" + }, + "idp": { + "entityId": "https://idp.com/saml/idp/metadata", + "singleSignOnService": { + "url": "https://idp.com/saml/idp/request_authentication", + "binding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" + }, + "artifactResolutionService": [{ + "index": "0", + "url": "https://idp.com/saml/idp/resolve_artifact", + "binding": "urn:oasis:names:tc:SAML:2.0:bindings:SOAP" + }], + "x509cert": "MIICgTCCAeoCCQCbOlrWDdX7FTANBgkqhkiG9w0BAQUFADCBhDELMAkGA1UEBhMCTk8xGDAWBgNVBAgTD0FuZHJlYXMgU29sYmVyZzEMMAoGA1UEBxMDRm9vMRAwDgYDVQQKEwdVTklORVRUMRgwFgYDVQQDEw9mZWlkZS5lcmxhbmcubm8xITAfBgkqhkiG9w0BCQEWEmFuZHJlYXNAdW5pbmV0dC5ubzAeFw0wNzA2MTUxMjAxMzVaFw0wNzA4MTQxMjAxMzVaMIGEMQswCQYDVQQGEwJOTzEYMBYGA1UECBMPQW5kcmVhcyBTb2xiZXJnMQwwCgYDVQQHEwNGb28xEDAOBgNVBAoTB1VOSU5FVFQxGDAWBgNVBAMTD2ZlaWRlLmVybGFuZy5ubzEhMB8GCSqGSIb3DQEJARYSYW5kcmVhc0B1bmluZXR0Lm5vMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDivbhR7P516x/S3BqKxupQe0LONoliupiBOesCO3SHbDrl3+q9IbfnfmE04rNuMcPsIxB161TdDpIesLCn7c8aPHISKOtPlAeTZSnb8QAu7aRjZq3+PbrP5uW3TcfCGPtKTytHOge/OlJbo078dVhXQ14d1EDwXJW1rRXuUt4C8QIDAQABMA0GCSqGSIb3DQEBBQUAA4GBACDVfp86HObqY+e8BUoWQ9+VMQx1ASDohBjwOsg2WykUqRXF+dLfcUH9dWR63CtZIKFDbStNomPnQz7nbK+onygwBspVEbnHuUihZq3ZUdmumQqCw4Uvs/1Uvq3orOo/WJVhTyvLgFVK2QarQ4/67OZfHd7R+POBXhophSMv1ZOo" + }, + "security": { + "soapClientCert": "abc", + "soapClientKey": "abc", + "authnRequestsSigned": false, + "wantAssertionsSigned": false, + "signMetadata": false + } +} diff --git a/tests/src/OneLogin/saml2_tests/auth_test.py b/tests/src/OneLogin/saml2_tests/auth_test.py index ea8cf1d3..49191c14 100644 --- a/tests/src/OneLogin/saml2_tests/auth_test.py +++ b/tests/src/OneLogin/saml2_tests/auth_test.py @@ -4,10 +4,13 @@ # MIT License from base64 import b64decode, b64encode +from hashlib import sha1 import json from os.path import dirname, join, exists import unittest +import responses + from onelogin.saml2 import compat from onelogin.saml2.auth import OneLogin_Saml2_Auth from onelogin.saml2.constants import OneLogin_Saml2_Constants @@ -21,6 +24,14 @@ from urlparse import urlparse, parse_qs +def create_example_artifact(endpoint_url, endpoint_index=b"\x00\x00"): + type_code = b"\x00\x04" + source_id = sha1(endpoint_url.encode("utf-8")).digest() + message_handle = b"01234567890123456789" # something random + + return b64encode(type_code + endpoint_index + source_id + message_handle) + + class OneLogin_Saml2_Auth_Test(unittest.TestCase): data_path = join(dirname(dirname(dirname(dirname(__file__)))), 'data') settings_path = join(dirname(dirname(dirname(dirname(__file__)))), 'settings') @@ -1478,3 +1489,78 @@ def testGetIdFromLogoutResponse(self): auth = OneLogin_Saml2_Auth(message_wrapper, old_settings=settings) auth.process_slo() self.assertIn(auth.get_last_message_id(), '_f9ee61bd9dbf63606faa9ae3b10548d5b3656fb859') + + @unittest.mock.patch('onelogin.saml2.utils.OneLogin_Saml2_Utils.validate_sign') + @responses.activate + def testArtifactResponseSoapRequest(self, mock): + """ + Test that a Artifact Response, makes a SOAP request using the received + saml artifact and returns the response. + """ + mock.return_value = True + + saml_art = create_example_artifact( + "https://idp.com/saml/idp/metadata" + ).decode('utf-8') + response = self.file_contents(join( + self.data_path, 'artifact_response', 'artifact_response.xml' + )) + responses.add( + responses.POST, + "https://idp.com/saml/idp/resolve_artifact", + body=response, + status=200, + ) + + settings_info = self.loadSettingsJSON(name='settings11.json') + request_data = self.get_request() + request_data['get_data'] = { + 'SAMLArt': saml_art + } + auth = OneLogin_Saml2_Auth(request_data, old_settings=settings_info) + auth.process_response() + + self.assertEqual( + responses.calls[0].request.url, + 'https://idp.com/saml/idp/resolve_artifact' + ) + self.assertEqual( + responses.calls[0].request.method, + 'POST' + ) + + self.assertIn( + f'{saml_art}', + responses.calls[0].request.body.decode('utf-8') + ) + + @unittest.mock.patch('onelogin.saml2.utils.OneLogin_Saml2_Utils.validate_sign') + @responses.activate + def testArtifactGetInfoFromLastResponseReceived(self, mock): + mock.return_value = True + + saml_art = create_example_artifact( + "https://idp.com/saml/idp/metadata" + ).decode('utf-8') + response = self.file_contents(join( + self.data_path, 'artifact_response', 'artifact_response.xml' + )) + responses.add( + responses.POST, + "https://idp.com/saml/idp/resolve_artifact", + body=response, + status=200, + ) + + settings_info = self.loadSettingsJSON(name='settings11.json') + request_data = self.get_request() + request_data['get_data'] = { + 'SAMLArt': saml_art + } + auth = OneLogin_Saml2_Auth(request_data, old_settings=settings_info) + auth.process_response() + + self.assertEqual(len(auth.get_errors()), 0) + self.assertEqual(auth.get_last_message_id(), '_1072ee96') + self.assertEqual(auth.get_last_assertion_id(), '_dc9f70e61c') + self.assertEqual(auth.get_last_assertion_not_on_or_after(), None) From 931ba1fd79e5d8e6352ec7866e564af5e43bc369 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Fri, 30 Oct 2020 15:39:50 +0100 Subject: [PATCH 22/26] Typo in ArtifactResponse. --- src/onelogin/saml2/artifact_response.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/onelogin/saml2/artifact_response.py b/src/onelogin/saml2/artifact_response.py index e18d96cb..813701a9 100644 --- a/src/onelogin/saml2/artifact_response.py +++ b/src/onelogin/saml2/artifact_response.py @@ -105,7 +105,7 @@ def is_valid(self, request_id, raise_exceptions=False): in_response_to = self.get_in_response_to() if in_response_to and in_response_to != request_id: raise OneLogin_Saml2_ValidationError( - 'The InResponseTo of the Logout Response: %s, does not match the ID of the Logout request sent by the SP: %s' % (in_response_to, request_id), + 'The InResponseTo of the Artifact Response: %s, does not match the ID of the Artifact Resolve Request sent by the SP: %s' % (in_response_to, request_id), OneLogin_Saml2_ValidationError.WRONG_INRESPONSETO ) From cf687987b70f7ed2d2fe3195391d7f0e529bc13b Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Fri, 30 Oct 2020 15:40:26 +0100 Subject: [PATCH 23/26] Add a rudimentary test for the error case when using OneLogin_Saml2_Auth to do ArtifactResolve/Artifact Binding. --- .../artifact_response_invalid.xml | 56 +++++++++++++++++++ tests/src/OneLogin/saml2_tests/auth_test.py | 38 +++++++++++++ 2 files changed, 94 insertions(+) create mode 100644 tests/data/artifact_response/artifact_response_invalid.xml diff --git a/tests/data/artifact_response/artifact_response_invalid.xml b/tests/data/artifact_response/artifact_response_invalid.xml new file mode 100644 index 00000000..7c435cba --- /dev/null +++ b/tests/data/artifact_response/artifact_response_invalid.xml @@ -0,0 +1,56 @@ + + + + https://idp.com/saml/idp/metadata + + + + + https://idp.com/saml/idp/metadata + + + + + + + + + + + + + + + + + + + + + + + + + https://idp.com/saml/idp/metadata + + s00000000:12345678 + + + + + + + sp.com + + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + + + + + diff --git a/tests/src/OneLogin/saml2_tests/auth_test.py b/tests/src/OneLogin/saml2_tests/auth_test.py index 49191c14..2d6b0cda 100644 --- a/tests/src/OneLogin/saml2_tests/auth_test.py +++ b/tests/src/OneLogin/saml2_tests/auth_test.py @@ -1564,3 +1564,41 @@ def testArtifactGetInfoFromLastResponseReceived(self, mock): self.assertEqual(auth.get_last_message_id(), '_1072ee96') self.assertEqual(auth.get_last_assertion_id(), '_dc9f70e61c') self.assertEqual(auth.get_last_assertion_not_on_or_after(), None) + + @unittest.mock.patch('onelogin.saml2.utils.OneLogin_Saml2_Utils.validate_sign') + @responses.activate + def testArtifactErrorCase(self, mock): + mock.return_value = True + + saml_art = create_example_artifact( + "https://idp.com/saml/idp/metadata" + ).decode('utf-8') + response = self.file_contents(join( + self.data_path, 'artifact_response', 'artifact_response_invalid.xml' + )) + responses.add( + responses.POST, + "https://idp.com/saml/idp/resolve_artifact", + body=response, + status=200, + ) + + settings_info = self.loadSettingsJSON(name='settings11.json') + request_data = self.get_request() + request_data['get_data'] = { + 'SAMLArt': saml_art + } + auth = OneLogin_Saml2_Auth(request_data, old_settings=settings_info) + auth.set_strict(True) + auth.process_response() + + self.assertIn( + 'The ArtifactResponse could not be validated due to the following error: ' + 'The InResponseTo of the Artifact Response: ', + auth.get_last_error_reason(), + ) + + self.assertEqual(len(auth.get_errors()), 1) + self.assertIsNone(auth.get_last_message_id()) + self.assertIsNone(auth.get_last_assertion_id()) + self.assertIsNone(auth.get_last_assertion_not_on_or_after()) From 27d1c2833a3a7a4d238bb153ca42ec4dc5f76126 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Thu, 17 Dec 2020 17:56:04 +0100 Subject: [PATCH 24/26] Add test for ArtifactResponse parsing using the new Artifact_Response class. --- .../artifact_response_invalid.xml | 4 +- .../saml2_tests/artifact_response_test.py | 104 ++++++++++++++++++ 2 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 tests/src/OneLogin/saml2_tests/artifact_response_test.py diff --git a/tests/data/artifact_response/artifact_response_invalid.xml b/tests/data/artifact_response/artifact_response_invalid.xml index 7c435cba..c09a587b 100644 --- a/tests/data/artifact_response/artifact_response_invalid.xml +++ b/tests/data/artifact_response/artifact_response_invalid.xml @@ -3,7 +3,7 @@ https://idp.com/saml/idp/metadata - + https://idp.com/saml/idp/metadata @@ -28,7 +28,7 @@ - + https://idp.com/saml/idp/metadata diff --git a/tests/src/OneLogin/saml2_tests/artifact_response_test.py b/tests/src/OneLogin/saml2_tests/artifact_response_test.py new file mode 100644 index 00000000..4be62cd5 --- /dev/null +++ b/tests/src/OneLogin/saml2_tests/artifact_response_test.py @@ -0,0 +1,104 @@ +from base64 import b64decode, b64encode +from lxml import etree +from datetime import datetime +from datetime import timedelta +from freezegun import freeze_time +import json +from os.path import dirname, join, exists +import unittest +from xml.dom.minidom import parseString + +from onelogin.saml2 import compat +from onelogin.saml2.artifact_response import Artifact_Response +from onelogin.saml2.settings import OneLogin_Saml2_Settings +from onelogin.saml2.utils import OneLogin_Saml2_Utils, OneLogin_Saml2_ValidationError +from onelogin.saml2.xml_utils import OneLogin_Saml2_XML + +class Saml2_Artifact_Response_Test(unittest.TestCase): + data_path = join(dirname(dirname(dirname(dirname(__file__)))), 'data') + settings_path = join(dirname(dirname(dirname(dirname(__file__)))), 'settings') + + def loadSettingsJSON(self, name='settings1.json'): + filename = join(self.settings_path, name) + if exists(filename): + stream = open(filename, 'r') + settings = json.load(stream) + stream.close() + return settings + + def file_contents(self, filename): + f = open(filename, 'r') + content = f.read() + f.close() + return content + + def testConstruct(self): + response = self.file_contents(join( + self.data_path, 'artifact_response', 'artifact_response.xml' + )) + settings = OneLogin_Saml2_Settings(self.loadSettingsJSON()) + response_enc = Artifact_Response(settings, response) + + self.assertIsInstance(response_enc, Artifact_Response) + + def testGetResponseXml(self): + response_data = self.file_contents(join( + self.data_path, 'artifact_response', 'artifact_response.xml' + )) + response_etree = OneLogin_Saml2_XML.to_etree(response_data) + samlp_response = OneLogin_Saml2_XML.query(response_etree, '//samlp:Response')[0] + expected_data = b64encode(OneLogin_Saml2_XML.to_string(samlp_response)) + + settings = OneLogin_Saml2_Settings(self.loadSettingsJSON()) + response = Artifact_Response(settings, response_data) + + self.assertEqual( + response.get_response_xml(), + expected_data + ) + + def testIsValid(self): + response = self.file_contents(join( + self.data_path, 'artifact_response', 'artifact_response.xml' + )) + json_settings = self.loadSettingsJSON(name='settings11.json') + json_settings['strict'] = True + settings = OneLogin_Saml2_Settings(json_settings) + response = Artifact_Response(settings, response) + + self.assertTrue(response.is_valid( + 'ONELOGIN_5ba93c9db0cff93f52b521d7420e43f6eda2784f' + )) + + def testGetIssuer(self): + response = self.file_contents(join( + self.data_path, 'artifact_response', 'artifact_response.xml' + )) + settings = OneLogin_Saml2_Settings(self.loadSettingsJSON()) + response = Artifact_Response(settings, response) + + self.assertEqual(response.get_issuer(), 'https://idp.com/saml/idp/metadata') + + def testGetStatus(self): + response = self.file_contents(join( + self.data_path, 'artifact_response', 'artifact_response.xml' + )) + settings = OneLogin_Saml2_Settings(self.loadSettingsJSON()) + response = Artifact_Response(settings, response) + + self.assertEqual(response.get_status(), 'urn:oasis:names:tc:SAML:2.0:status:Success') + + def testCheckStatus(self): + response = self.file_contents(join( + self.data_path, 'artifact_response', 'artifact_response_invalid.xml' + )) + settings = OneLogin_Saml2_Settings(self.loadSettingsJSON()) + response = Artifact_Response(settings, response) + + with self.assertRaises(OneLogin_Saml2_ValidationError) as context: + response.check_status() + + self.assertEqual( + str(context.exception), + 'The status code of the ArtifactResponse was not Success, was Responder' + ) \ No newline at end of file From 87e91c26ffccd13c735c41d809ae3cd98939e88b Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Thu, 14 Jan 2021 16:00:49 +0100 Subject: [PATCH 25/26] Add responses to the dependencies and add a missing comma. --- setup.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 8163bc76..53d8ccd1 100644 --- a/setup.py +++ b/setup.py @@ -41,7 +41,7 @@ 'isodate>=0.5.0', 'lxml>=3.3.5', 'xmlsec>=1.0.5', - 'defusedxml==0.6.0' + 'defusedxml==0.6.0', 'requests>=2.24.0' ], dependency_links=['http://github.com/mehcode/python-xmlsec/tarball/master'], @@ -52,6 +52,7 @@ 'pylint==1.9.4', 'flake8==3.6.0', 'coveralls==1.5.1', + 'responses>=0.12.0' ), }, keywords='saml saml2 xmlsec django flask pyramid python3', From d7c9126975bccc95476a88c7cec80521268f8992 Mon Sep 17 00:00:00 2001 From: Alexander Schrijver Date: Mon, 11 May 2020 11:18:11 +0200 Subject: [PATCH 26/26] Add the ability to change the ProtocolBinding in the authn request. --- src/onelogin/saml2/authn_request.py | 1 + src/onelogin/saml2/xml_templates.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/onelogin/saml2/authn_request.py b/src/onelogin/saml2/authn_request.py index 48ad9d2a..c0ced249 100644 --- a/src/onelogin/saml2/authn_request.py +++ b/src/onelogin/saml2/authn_request.py @@ -124,6 +124,7 @@ def __init__(self, settings, force_authn=False, is_passive=False, set_nameid_pol 'nameid_policy_str': nameid_policy_str, 'requested_authn_context_str': requested_authn_context_str, 'attr_consuming_service_str': attr_consuming_service_str, + 'acs_binding': sp_data['assertionConsumerService'].get('binding', 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST') } self.__authn_request = request diff --git a/src/onelogin/saml2/xml_templates.py b/src/onelogin/saml2/xml_templates.py index e161851c..4442dd51 100644 --- a/src/onelogin/saml2/xml_templates.py +++ b/src/onelogin/saml2/xml_templates.py @@ -27,7 +27,7 @@ class OneLogin_Saml2_Templates(object): Version="2.0"%(provider_name)s%(force_authn_str)s%(is_passive_str)s IssueInstant="%(issue_instant)s" Destination="%(destination)s" - ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" + ProtocolBinding="%(acs_binding)s" AssertionConsumerServiceURL="%(assertion_url)s"%(attr_consuming_service_str)s> %(entity_id)s%(subject_str)s%(nameid_policy_str)s %(requested_authn_context_str)s