Create the signer along with the sigalg allowance check

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

4 files changed, 32 insertions, 48 deletions

diff --git a/src/saml2/entity.py b/src/saml2/entity.py
index 57d2a55a..f9f2eec1 100644
--- a/src/saml2/entity.py
+++ b/src/saml2/entity.py
@@ -71,6 +71,7 @@ from saml2.sigver import pre_signature_part
 from saml2.sigver import pre_encrypt_assertion
 from saml2.sigver import signed_instance_factory
 from saml2.virtual_org import VirtualOrg
+from saml2.pack import http_redirect_message
 
 import saml2.xmldsig as ds
 
@@ -251,19 +252,14 @@ class Entity(HTTPBase):
             info["method"] = "POST"
         elif binding == BINDING_HTTP_REDIRECT:
             logger.info("HTTP REDIRECT")
-            signer = (
-                self.sec.sec_backend.get_signer(sigalg)
-                if sign and sigalg
-                else None
-            )
-            info = self.use_http_get(
-                msg_str,
-                destination,
-                relay_state,
-                typ,
-                signer=signer,
+            info = http_redirect_message(
+                message=msg_str,
+                location=destination,
+                relay_state=relay_state,
+                typ=typ,
+                sign=sign,
                 sigalg=sigalg,
-                **kwargs,
+                backend=self.sec.sec_backend,
             )
             info["url"] = str(destination)
             info["method"] = "GET"
diff --git a/src/saml2/httpbase.py b/src/saml2/httpbase.py
index a6846dab..6a0d1ce7 100644
--- a/src/saml2/httpbase.py
+++ b/src/saml2/httpbase.py
@@ -388,25 +388,3 @@ class HTTPBase(object):
     def add_credentials(self, user, passwd):
         self.user = user
         self.passwd = passwd
-
-    @staticmethod
-    def use_http_get(message, destination, relay_state,
-                     typ="SAMLRequest", sigalg="", signer=None, **kwargs):
-        """
-        Send a message using GET, this is the HTTP-Redirect case so
-        no direct response is expected to this request.
-
-        :param message:
-        :param destination:
-        :param relay_state:
-        :param typ: Whether a Request, Response or Artifact
-        :param sigalg: Which algorithm the signature function will use to sign
-            the message
-        :param signer: A signing function that can be used to sign the message
-        :return: dictionary
-        """
-        if not isinstance(message, six.string_types):
-            message = "%s" % (message,)
-
-        return http_redirect_message(message, destination, relay_state, typ,
-                                     sigalg, signer)
diff --git a/src/saml2/pack.py b/src/saml2/pack.py
index e2cb2a8d..50f35dcf 100644
--- a/src/saml2/pack.py
+++ b/src/saml2/pack.py
@@ -141,8 +141,15 @@ def http_post_message(message, relay_state="", typ="SAMLRequest", **kwargs):
             "status": 200}
 
 
-def http_redirect_message(message, location, relay_state="", typ="SAMLRequest",
-                          sigalg='', signer=None, **kwargs):
+def http_redirect_message(
+    message,
+    location,
+    relay_state="",
+    typ="SAMLRequest",
+    sigalg=None,
+    sign=None,
+    backend=None,
+):
     """The HTTP Redirect binding defines a mechanism by which SAML protocol
     messages can be transmitted within URL parameters.
     Messages are encoded for use with this binding using a URL encoding
@@ -156,7 +163,7 @@ def http_redirect_message(message, location, relay_state="", typ="SAMLRequest",
     :param typ: What type of message it is SAMLRequest/SAMLResponse/SAMLart
     :param sigalg: Which algorithm the signature function will use to sign
         the message
-    :param signer: A signature function that can be used to sign the message
+    :param sign: Whether the message should be signed
     :return: A tuple containing header information and a HTML message.
     """
 
@@ -178,19 +185,18 @@ def http_redirect_message(message, location, relay_state="", typ="SAMLRequest",
     if relay_state:
         args["RelayState"] = relay_state
 
-    # XXX !should not depend on signer, but on sign
-    # XXX if both signalg and signer are here they have to match
-    # XXX now we allow them to differ
-    # XXX signer should be created here; not passed in
-    if signer:
+    if sign:
         # XXX check for allowed algo -- should do the same for POST binding
         # sigalgs, should be one defined in xmldsig
         if sigalg not in [long_name for short_name, long_name in SIG_ALLOWED_ALG]:
             raise Exception(
                 "Signature algo not in allowed list: {algo}".format(algo=sigalg)
             )
-        args["SigAlg"] = sigalg
+        signer = backend.get_signer(sigalg) if sign and sigalg else None
+        if not signer:
+            raise Exception("Could not init signer fro algo {algo}".format(algo=sigalg))
 
+        args["SigAlg"] = sigalg
         string = "&".join(urlencode({k: args[k]}) for k in _order if k in args)
         string_enc = string.encode('ascii')
         args["Signature"] = base64.b64encode(signer.sign(string_enc))
diff --git a/tests/test_70_redirect_signing.py b/tests/test_70_redirect_signing.py
index a079d6cb..5286d4c6 100644
--- a/tests/test_70_redirect_signing.py
+++ b/tests/test_70_redirect_signing.py
@@ -30,11 +30,15 @@ def test():
         destination = srvs[0]["location"]
         req_id, req = sp.create_authn_request(destination, id="id1")
 
-        signer = sp.sec.sec_backend.get_signer(SIG_RSA_SHA1)
-
-        info = http_redirect_message(req, destination, relay_state="RS",
-                                     typ="SAMLRequest", sigalg=SIG_RSA_SHA1,
-                                     signer=signer)
+        info = http_redirect_message(
+            req,
+            destination,
+            relay_state="RS",
+            typ="SAMLRequest",
+            sigalg=SIG_RSA_SHA1,
+            sign=True,
+            backend=sp.sec.sec_backend,
+        )
 
         verified_ok = False