Verify signed logout requests with the redirect binding

Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com>

3 files changed, 102 insertions, 18 deletions

diff --git a/src/saml2/client.py b/src/saml2/client.py
index aa0bd0c9..a7469d4f 100644
--- a/src/saml2/client.py
+++ b/src/saml2/client.py
@@ -630,7 +630,9 @@ class Saml2Client(Base):
         sign=None,
         sign_alg=None,
         digest_alg=None,
-        relay_state="",
+        relay_state=None,
+        sigalg=None,
+        signature=None,
     ):
         """
         Deal with a LogoutRequest
@@ -639,6 +641,11 @@ class Saml2Client(Base):
         :param name_id: The id of the current user
         :param binding: Which binding the message came in over
         :param sign: Whether the response will be signed or not
+        :param sign_alg: The signing algorithm for the response
+        :param digest_alg: The digest algorithm for the the response
+        :param relay_state: The relay state of the request
+        :param sigalg: The SigAlg query param of the request
+        :param signature: The Signature query param of the request
         :return: Keyword arguments which can be used to send the response
             what's returned follow different patterns for different bindings.
             If the binding is BINDIND_SOAP, what is returned looks like this::
@@ -652,8 +659,13 @@ class Saml2Client(Base):
         """
         logger.info("logout request: %s", request)
 
-        _req = self._parse_request(request, LogoutRequest,
-                                   "single_logout_service", binding)
+        _req = self.parse_logout_request(
+            xmlstr=request,
+            binding=binding,
+            relay_state=relay_state,
+            sigalg=sigalg,
+            signature=signature,
+        )
 
         if _req.message.name_id == name_id:
             try:
diff --git a/src/saml2/entity.py b/src/saml2/entity.py
index e926d2c5..f818b702 100644
--- a/src/saml2/entity.py
+++ b/src/saml2/entity.py
@@ -1584,7 +1584,14 @@ class Entity(HTTPBase):
 
     # ------------------------------------------------------------------------
 
-    def parse_logout_request(self, xmlstr, binding=BINDING_SOAP):
+    def parse_logout_request(
+        self,
+        xmlstr,
+        binding=BINDING_SOAP,
+        relay_state=None,
+        sigalg=None,
+        signature=None,
+    ):
         """ Deal with a LogoutRequest
 
         :param xmlstr: The response as a xml string
@@ -1594,8 +1601,15 @@ class Entity(HTTPBase):
             was not.
         """
 
-        return self._parse_request(xmlstr, saml_request.LogoutRequest,
-                                   "single_logout_service", binding)
+        return self._parse_request(
+            enc_request=xmlstr,
+            request_cls=saml_request.LogoutRequest,
+            service="single_logout_service",
+            binding=binding,
+            relay_state=relay_state,
+            sigalg=sigalg,
+            signature=signature,
+        )
 
     def use_artifact(self, message, endpoint_index=0):
         """
diff --git a/tests/test_51_client.py b/tests/test_51_client.py
index 095d80ab..a323de79 100644
--- a/tests/test_51_client.py
+++ b/tests/test_51_client.py
@@ -1644,14 +1644,68 @@ class TestClient:
         loc = info["headers"][0][1]
         _, _, _, _, qs, _ = parse.urlparse(loc)
         qs = parse.parse_qs(qs)
-        assert _leq(qs.keys(),
-                    ['SigAlg', 'SAMLRequest', 'RelayState', 'Signature'])
+        assert _leq(qs.keys(), ['SigAlg', 'SAMLRequest', 'RelayState', 'Signature'])
 
-        assert verify_redirect_signature(list_values2simpletons(qs),
-                                         client.sec.sec_backend)
+        qs_simple = list_values2simpletons(qs)
+        assert verify_redirect_signature(qs_simple, client.sec.sec_backend)
 
-        res = self.server.parse_logout_request(qs["SAMLRequest"][0],
-                                               BINDING_HTTP_REDIRECT)
+        res = self.server.parse_logout_request(
+            qs_simple["SAMLRequest"],
+            BINDING_HTTP_REDIRECT,
+            relay_state=qs_simple['RelayState'],
+            sigalg=qs_simple['SigAlg'],
+            signature=qs_simple['Signature'],
+        )
+
+    def test_do_logout_signed_redirect_invalid(self):
+        conf = config.SPConfig()
+        conf.load_file("sp_slo_redirect_conf")
+        client = Saml2Client(conf)
+
+        session_info = {
+            "name_id": nid,
+            "issuer": "urn:mace:example.com:saml:roland:idp",
+            "not_on_or_after": in_a_while(minutes=15),
+            "ava": {
+                "givenName": "Anders",
+                "sn": "Andersson",
+                "mail": "anders.andersson@example.com"
+            }
+        }
+        client.users.add_information_about_person(session_info)
+        entity_ids = client.users.issuers_of_info(nid)
+
+        resp = client.do_logout(
+            nid,
+            entity_ids,
+            "Tired",
+            in_a_while(minutes=5),
+            sign=True,
+            expected_binding=BINDING_HTTP_REDIRECT,
+        )
+
+        binding, info = resp[entity_ids[0]]
+        loc = info["headers"][0][1]
+        _, _, _, _, qs, _ = parse.urlparse(loc)
+        qs = parse.parse_qs(qs)
+        qs_simple = list_values2simpletons(qs)
+
+        invalid_signature = 'ZEdMZUQ3SjBjQ2ozWmlGaHhyV3JZSzNkTWhQWU02bjA0dzVNeUd1UWgrVDhnYm1oc1R1TTFjPQo='
+        qs_simple_invalid = {
+            **qs_simple,
+            'Signature': invalid_signature,
+        }
+        assert not verify_redirect_signature(qs_simple_invalid, client.sec.sec_backend)
+
+        self.server.config.setattr("idp", "want_authn_requests_signed", True)
+        with raises(IncorrectlySigned):
+            res = self.server.parse_logout_request(
+                qs_simple["SAMLRequest"],
+                BINDING_HTTP_REDIRECT,
+                relay_state=qs_simple['RelayState'],
+                sigalg=qs_simple['SigAlg'],
+                signature=invalid_signature,
+            )
 
     def test_do_logout_post(self):
         # information about the user from an IdP
@@ -3245,14 +3299,18 @@ class TestClientNonAsciiAva:
         loc = info["headers"][0][1]
         _, _, _, _, qs, _ = parse.urlparse(loc)
         qs = parse.parse_qs(qs)
-        assert _leq(qs.keys(),
-                    ['SigAlg', 'SAMLRequest', 'RelayState', 'Signature'])
+        assert _leq(qs.keys(), ['SigAlg', 'SAMLRequest', 'RelayState', 'Signature'])
 
-        assert verify_redirect_signature(list_values2simpletons(qs),
-                                         client.sec.sec_backend)
+        qs_simple = list_values2simpletons(qs)
+        assert verify_redirect_signature(qs_simple, client.sec.sec_backend)
 
-        res = self.server.parse_logout_request(qs["SAMLRequest"][0],
-                                               BINDING_HTTP_REDIRECT)
+        res = self.server.parse_logout_request(
+            qs_simple["SAMLRequest"],
+            BINDING_HTTP_REDIRECT,
+            relay_state=qs_simple['RelayState'],
+            sigalg=qs_simple['SigAlg'],
+            signature=qs_simple['Signature'],
+        )
 
     def test_do_logout_post(self):
         # information about the user from an IdP