Merge pull request #834 from mheuwes/soap-fixes

Fix AttributeError and signature mangling during construction of SOAP request

5 files changed, 66 insertions, 12 deletions

diff --git a/src/saml2/client.py b/src/saml2/client.py
index 5f82c6bc..e8642dfa 100644
--- a/src/saml2/client.py
+++ b/src/saml2/client.py
@@ -152,8 +152,8 @@ class Saml2Client(Base):
             # XXX   ^through self.create_authn_request(...)
             # XXX - sign_redirect will add the signature to the query params
             # XXX   ^through self.apply_binding(...)
-            sign_post = False if binding == BINDING_HTTP_REDIRECT else sign
-            sign_redirect = False if binding == BINDING_HTTP_POST and sign else sign
+            sign_redirect = sign and binding == BINDING_HTTP_REDIRECT
+            sign_post = sign and not sign_redirect
 
             reqid, request = self.create_authn_request(
                 destination=destination,
@@ -318,10 +318,8 @@ class Saml2Client(Base):
                 session_indexes = None
 
             sign = sign if sign is not None else self.logout_requests_signed
-            sign_post = sign and (
-                binding == BINDING_HTTP_POST or binding == BINDING_SOAP
-            )
             sign_redirect = sign and binding == BINDING_HTTP_REDIRECT
+            sign_post = sign and not sign_redirect
 
             log_report = {
                 "message": "Invoking SLO on entity",
diff --git a/src/saml2/httpbase.py b/src/saml2/httpbase.py
index 5860992d..f8393639 100644
--- a/src/saml2/httpbase.py
+++ b/src/saml2/httpbase.py
@@ -315,7 +315,7 @@ class HTTPBase(object):
 
         if sign and self.sec:
             _signed = self.sec.sign_statement(soap_message,
-                                              class_name=class_name(request),
+                                              node_name=class_name(request),
                                               node_id=request.id)
             soap_message = _signed
 
diff --git a/src/saml2/pack.py b/src/saml2/pack.py
index f0890471..36480743 100644
--- a/src/saml2/pack.py
+++ b/src/saml2/pack.py
@@ -240,7 +240,7 @@ def make_soap_enveloped_saml_thingy(thingy, header_parts=None):
         if thingy[0:5].lower() == '<?xml':
             logger.debug("thingy0: %s", thingy)
             _part = thingy.split("\n")
-            thingy = "".join(_part[1:])
+            thingy = "\n".join(_part[1:])
         thingy = thingy.replace(PREFIX, "")
         logger.debug("thingy: %s", thingy)
         _child = ElementTree.Element('')
diff --git a/src/saml2/request.py b/src/saml2/request.py
index 200a1ff8..787af78f 100644
--- a/src/saml2/request.py
+++ b/src/saml2/request.py
@@ -2,7 +2,6 @@ import logging
 
 from saml2 import time_util
 from saml2 import BINDING_HTTP_REDIRECT
-from saml2 import BINDING_HTTP_POST
 from saml2.attribute_converter import to_local
 from saml2.s_utils import OtherError
 
@@ -55,22 +54,22 @@ class Request(object):
         logger.debug("xmlstr: %s, relay_state: %s, sigalg: %s, signature: %s",
                      self.xmlstr, relay_state, sigalg, signature)
 
-        signed_post = must and binding == BINDING_HTTP_POST
-        signed_redirect = must and binding == BINDING_HTTP_REDIRECT
+        sign_redirect = must and binding == BINDING_HTTP_REDIRECT
+        sign_post = must and not sign_redirect
         incorrectly_signed = IncorrectlySigned("Request was not signed correctly")
 
         try:
             self.message = self.signature_check(
                 xmldata,
                 origdoc=origdoc,
-                must=signed_post,
+                must=sign_post,
                 only_valid_cert=only_valid_cert,
             )
         except Exception as e:
             self.message = None
             raise incorrectly_signed from e
 
-        if signed_redirect:
+        if sign_redirect:
             if sigalg is None or signature is None:
                 raise incorrectly_signed
 
diff --git a/tests/test_50_server.py b/tests/test_50_server.py
index 07ef4b91..2344481f 100644
--- a/tests/test_50_server.py
+++ b/tests/test_50_server.py
@@ -12,6 +12,7 @@ from saml2.cert import OpenSSLWrapper
 from saml2.sigver import make_temp, DecryptError, EncryptError, CertificateError
 from saml2.assertion import Policy
 from saml2.authn_context import INTERNETPROTOCOLPASSWORD
+from saml2.response import IncorrectlySigned
 from saml2.saml import NameID, NAMEID_FORMAT_TRANSIENT
 from saml2.samlp import response_from_string
 
@@ -32,6 +33,7 @@ from saml2.s_utils import sid
 from saml2.soap import make_soap_enveloped_saml_thingy
 from saml2 import BINDING_HTTP_POST
 from saml2 import BINDING_HTTP_REDIRECT
+from saml2 import BINDING_SOAP
 from saml2.time_util import instant
 
 from pytest import raises
@@ -2246,9 +2248,64 @@ class TestServer1NonAsciiAva():
 
         with closing(Server("idp_soap_conf")) as idp:
             request = idp.parse_logout_request(saml_soap)
+            assert request
+
+            idp.config.setattr("idp", "want_authn_requests_signed", True)
+            assert idp.config.getattr("want_authn_requests_signed", "idp")
+
+            with raises(IncorrectlySigned):
+                # check unsigned requests over SOAP to fail
+                request = idp.parse_logout_request(saml_soap)
+                assert not request
+
+            idp.ident.close()
+
+    def test_slo_soap_signed(self):
+        soon = time_util.in_a_while(days=1)
+        sinfo = {
+            "name_id": nid,
+            "issuer": "urn:mace:example.com:saml:roland:idp",
+            "not_on_or_after": soon,
+            "user": {
+                "givenName": "Leo",
+                "sn": "Laport",
+            }
+        }
+
+        sp = client.Saml2Client(config_file="server_conf")
+        sp.users.add_information_about_person(sinfo)
+
+        req_id, logout_request = sp.create_logout_request(
+            name_id=nid, destination="http://localhost:8088/slo",
+            issuer_entity_id="urn:mace:example.com:saml:roland:idp",
+            reason="I'm tired of this", sign=True, sign_alg=ds.SIG_RSA_SHA512,
+            digest_alg=ds.DIGEST_SHA512,
+        )
+
+        saml_soap = sp.apply_binding(BINDING_SOAP, logout_request, sign=False)
+        saml_soap = saml_soap["data"]
+        self.server.ident.close()
+
+        with closing(Server("idp_soap_conf")) as idp:
+            idp.config.setattr("idp", "want_authn_requests_signed", True)
+            assert idp.config.getattr("want_authn_requests_signed", "idp")
+
+            with raises(IncorrectlySigned):
+                # idp_soap_conf has invalid certificate for sp
+                request = idp.parse_logout_request(saml_soap)
+                assert not request
+
+            idp.ident.close()
+
+        with closing(Server("idp_conf_verify_cert")) as idp:
+            idp.config.setattr("idp", "want_authn_requests_signed", True)
+            assert idp.config.getattr("want_authn_requests_signed", "idp")
+
+            request = idp.parse_logout_request(saml_soap)
             idp.ident.close()
             assert request
 
+
 # ------------------------------------------------------------------------