WIP works good - set on init use on create_

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

5 files changed, 70 insertions, 51 deletions

diff --git a/src/saml2/client.py b/src/saml2/client.py
index 9809150e..f5033ba2 100644
--- a/src/saml2/client.py
+++ b/src/saml2/client.py
@@ -201,6 +201,8 @@ class Saml2Client(Base):
         return self.do_logout(name_id, entity_ids, reason, expire, sign,
                               sign_alg=sign_alg, digest_alg=digest_alg)
 
+    # XXX calls DefaultSignature
+    # XXX calls self.sign
     def do_logout(self, name_id, entity_ids, reason, expire, sign=None,
                   expected_binding=None, sign_alg=None, digest_alg=None,
                   **kwargs):
diff --git a/src/saml2/client_base.py b/src/saml2/client_base.py
index 0842453c..dd3ddfdf 100644
--- a/src/saml2/client_base.py
+++ b/src/saml2/client_base.py
@@ -56,7 +56,7 @@ from saml2 import BINDING_PAOS
 
 from saml2.xmldsig import SIG_ALLOWED_ALG
 from saml2.xmldsig import DIGEST_ALLOWED_ALG
-from saml2.xmldsig import DefaultSignature
+
 
 logger = logging.getLogger(__name__)
 
@@ -184,10 +184,6 @@ class Base(Entity):
                 val = True
             setattr(self, attr, val)
 
-        # signing and digest algs
-        self.signing_algorithm = self.config.getattr('signing_algorithm', "sp")
-        self.digest_algorithm = self.config.getattr('digest_algorithm', "sp")
-
         if self.entity_type == "sp" and not any(
             [
                 self.want_assertions_signed,
@@ -287,6 +283,7 @@ class Base(Entity):
         else:
             return None
 
+    # XXX sp create
     def create_authn_request(
         self,
         destination,
@@ -451,12 +448,9 @@ class Base(Entity):
         client_crt = kwargs.get("client_crt")
         nsprefix = kwargs.get("nsprefix")
 
-        # XXX will be used to embed the signature to the xml doc - ie, POST binding
-        # XXX always called by the SP, no need to check the context
-        sign = self.authn_requests_signed if sign is None else sign
-        def_sig = DefaultSignature()
-        sign_alg = sign_alg or def_sig.get_sign_alg()
-        digest_alg = digest_alg or def_sig.get_digest_alg()
+        sign = sign if sign is not None else self.should_sign
+        sign_alg = sign_alg or self.signing_algorithm
+        digest_alg = digest_alg or self.digest_algorithm
 
         if sign_alg not in [long_name for short_name, long_name in SIG_ALLOWED_ALG]:
             raise Exception(
@@ -506,6 +500,7 @@ class Base(Entity):
 
         return msg
 
+    # XXX sp create
     def create_attribute_query(self, destination, name_id=None,
             attribute=None, message_id=0, consent=None,
             extensions=None, sign=False, sign_prepare=False, sign_alg=None,
@@ -572,6 +567,7 @@ class Base(Entity):
     # MUST use SOAP for
     # AssertionIDRequest, SubjectQuery,
     # AuthnQuery, AttributeQuery, or AuthzDecisionQuery
+    # XXX sp create
     def create_authz_decision_query(self, destination, action,
             evidence=None, resource=None, subject=None,
             message_id=0, consent=None, extensions=None,
@@ -596,6 +592,7 @@ class Base(Entity):
                              subject=subject, sign_alg=sign_alg,
                              digest_alg=digest_alg, **kwargs)
 
+    # XXX sp create
     def create_authz_decision_query_using_assertion(self, destination,
             assertion, action=None,
             resource=None,
@@ -632,6 +629,7 @@ class Base(Entity):
                 extensions=extensions, sign=sign, nsprefix=nsprefix)
 
     @staticmethod
+    # XXX sp create
     def create_assertion_id_request(assertion_id_refs, **kwargs):
         """
 
@@ -644,6 +642,7 @@ class Base(Entity):
         else:
             return 0, assertion_id_refs[0]
 
+    # XXX sp create
     def create_authn_query(self, subject, destination=None, authn_context=None,
             session_index="", message_id=0, consent=None,
             extensions=None, sign=False, nsprefix=None, sign_alg=None,
@@ -667,6 +666,7 @@ class Base(Entity):
                              nsprefix=nsprefix, sign_alg=sign_alg,
                              digest_alg=digest_alg)
 
+    # XXX sp create
     def create_name_id_mapping_request(self, name_id_policy,
             name_id=None, base_id=None,
             encrypted_id=None, destination=None,
@@ -828,6 +828,7 @@ class Base(Entity):
 
     # ------------------- ECP ------------------------------------------------
 
+    # XXX sp create
     def create_ecp_authn_request(self, entityid=None, relay_state="",
             sign=False, **kwargs):
         """ Makes an authentication request.
@@ -932,6 +933,7 @@ class Base(Entity):
     # ----------------------------------------------------------------------
 
     @staticmethod
+    # XXX sp create
     def create_discovery_service_request(url, entity_id, **kwargs):
         """
         Created the HTTP redirect URL needed to send the user to the
diff --git a/src/saml2/entity.py b/src/saml2/entity.py
index bff8db8d..39bc3dec 100644
--- a/src/saml2/entity.py
+++ b/src/saml2/entity.py
@@ -141,6 +141,24 @@ class Entity(HTTPBase):
         else:
             raise SAMLError("Missing configuration")
 
+        def_sig = DefaultSignature()
+        self.signing_algorithm = (
+            self.config.getattr('signing_algorithm')
+            or def_sig.get_sign_alg()
+        )
+        self.digest_algorithm = (
+            self.config.getattr('digest_algorithm')
+            or def_sig.get_digest_alg()
+        )
+        sign_config = (
+            self.config.getattr("authn_requests_signed", "sp")
+            if self.entity_type == "sp"
+            else self.config.getattr("sign_response", "idp")
+            if self.entity_type == "idp"
+            else False
+        )
+        self.should_sign = sign_config
+
         for item in ["cert_file", "key_file", "ca_certs"]:
             _val = getattr(self.config, item, None)
             if not _val:
@@ -219,20 +237,8 @@ class Entity(HTTPBase):
         :return: A dictionary
         """
 
-        # XXX authn_requests_signed (obj property) applies only to an SP
-        # XXX sign_response (config option) applies to idp/aa
-        # XXX Looking into sp/idp/aa properties should be done in the same way
-        # XXX ^this discrepancy should be fixed
-        sign_config = (
-            self.authn_requests_signed
-            if self.config.context == "sp"
-            else self.config.getattr("sign_response")
-            if self.config.context == "idp"
-            else None
-        )
-        sign = sign_config if sign is None else sign
-        def_sig = DefaultSignature()
-        sigalg = sigalg or def_sig.get_sign_alg()
+        sign = sign if sign is not None else self.should_sign
+        sigalg = sigalg or self.signing_algorithm
 
         # unless if BINDING_HTTP_ARTIFACT
         if response:
@@ -453,10 +459,6 @@ class Entity(HTTPBase):
         sign_alg=None,
         digest_alg=None,
     ):
-        # sign adn digest algs
-        sign_alg = sign_alg or self.signing_algorithm
-        digest_alg = digest_alg or self.digest_algorithm
-
         if msg.signature is None:
             msg.signature = pre_signature_part(
                 msg.id, self.sec.my_cert, 1, sign_alg=sign_alg, digest_alg=digest_alg
@@ -476,6 +478,7 @@ class Entity(HTTPBase):
         logger.info("REQUEST: %s", msg)
         return signed_instance_factory(msg, self.sec, to_sign)
 
+    # XXX calls self.sign
     def _message(
         self,
         request_cls,
@@ -629,6 +632,7 @@ class Entity(HTTPBase):
             raise exception
         return response
 
+    # XXX calls self.sign
     def _response(self, in_response_to, consumer_url=None, status=None,
                   issuer=None, sign=False, to_sign=None, sp_entity_id=None,
                   encrypt_assertion=False,
@@ -806,6 +810,7 @@ class Entity(HTTPBase):
         else:
             return response
 
+    # XXX calls self.sign
     def _status_response(self, response_class, issuer, status, sign=False,
                          sign_alg=None, digest_alg=None,
                          **kwargs):
@@ -914,6 +919,7 @@ class Entity(HTTPBase):
 
     # ------------------------------------------------------------------------
 
+    # XXX ent create
     def create_error_response(self, in_response_to, destination, info,
                               sign=False, issuer=None, sign_alg=None,
                               digest_alg=None, **kwargs):
@@ -936,6 +942,7 @@ class Entity(HTTPBase):
 
     # ------------------------------------------------------------------------
 
+    # XXX ent create
     def create_logout_request(self, destination, issuer_entity_id,
                               subject_id=None, name_id=None,
                               reason=None, expire=None, message_id=0,
@@ -988,6 +995,7 @@ class Entity(HTTPBase):
                              issuer=self._issuer(), sign_alg=sign_alg,
                              digest_alg=digest_alg, **args)
 
+    # XXX ent create
     def create_logout_response(self, request, bindings=None, status=None,
                                sign=False, issuer=None, sign_alg=None,
                                digest_alg=None):
@@ -1015,6 +1023,7 @@ class Entity(HTTPBase):
 
         return response
 
+    # XXX ent create
     def create_artifact_resolve(self, artifact, destination, sessid,
                                 consent=None, extensions=None, sign=False,
                                 sign_alg=None, digest_alg=None):
@@ -1036,6 +1045,7 @@ class Entity(HTTPBase):
                              consent, extensions, sign, artifact=artifact,
                              sign_alg=sign_alg, digest_alg=digest_alg)
 
+    # XXX ent create
     def create_artifact_response(self, request, artifact, bindings=None,
                                  status=None, sign=False, issuer=None,
                                  sign_alg=None, digest_alg=None):
@@ -1056,6 +1066,7 @@ class Entity(HTTPBase):
 
         return response
 
+    # XXX ent create
     def create_manage_name_id_request(self, destination, message_id=0,
                                       consent=None, extensions=None, sign=False,
                                       name_id=None, new_id=None,
@@ -1114,6 +1125,7 @@ class Entity(HTTPBase):
         return self._parse_request(xmlstr, saml_request.ManageNameIDRequest,
                                    "manage_name_id_service", binding)
 
+    # XXX ent create
     def create_manage_name_id_response(self, request, bindings=None,
                                        status=None, sign=False, issuer=None,
                                        sign_alg=None, digest_alg=None,
diff --git a/src/saml2/server.py b/src/saml2/server.py
index 68e04e27..1d4b7543 100644
--- a/src/saml2/server.py
+++ b/src/saml2/server.py
@@ -77,8 +77,15 @@ def _shelve_compat(name, *args, **kwargs):
 class Server(Entity):
     """ A class that does things that IdPs or AAs do """
 
-    def __init__(self, config_file="", config=None, cache=None, stype="idp",
-                 symkey="", msg_cb=None):
+    def __init__(
+        self,
+        config_file="",
+        config=None,
+        cache=None,
+        stype="idp",
+        symkey="",
+        msg_cb=None,
+    ):
         Entity.__init__(self, stype, config, config_file, msg_cb=msg_cb)
         self.eptid = None
         self.init_config(stype)
@@ -218,6 +225,7 @@ class Server(Entity):
         return False
 
     # -------------------------------------------------------------------------
+
     def parse_authn_request(self, enc_request, binding=BINDING_HTTP_REDIRECT):
         """Parse a Authentication Request
 
@@ -438,7 +446,6 @@ class Server(Entity):
         :param encrypt_cert_assertion: Certificate to be used for encryption
         of assertions.
         :param authn_statement: Authentication statement.
-        :param sign_assertion: True if assertions should be signed.
         :param pefim: True if a response according to the PEFIM profile
         should be created.
         :param farg: Argument to pass on to the assertion constructor
@@ -510,7 +517,7 @@ class Server(Entity):
 
     # ------------------------------------------------------------------------
 
-    # noinspection PyUnusedLocal
+    # XXX idp create
     def create_attribute_response(self, identity, in_response_to, destination,
                                   sp_entity_id, userid="", name_id=None,
                                   status=None, issuer=None,
@@ -594,15 +601,15 @@ class Server(Entity):
     def gather_authn_response_args(
         self, sp_entity_id, name_id_policy, userid, **kwargs
     ):
+        kwargs["policy"] = kwargs.get("release_policy")
+
         # collect args and return them
         args = {}
 
-        args["policy"] = kwargs.get(
-            "release_policy", self.config.getattr("policy", "idp")
-        )
-        args['best_effort'] = kwargs.get("best_effort", False)
-
+        # XXX will be passed to _authn_response
         param_defaults = {
+            'policy': None,
+            'best_effort': False,
             'sign_assertion': False,
             'sign_response': False,
             'encrypt_assertion': False,
@@ -610,12 +617,8 @@ class Server(Entity):
             'encrypted_advice_attributes': False,
             'encrypt_cert_advice': None,
             'encrypt_cert_assertion': None,
+            # need to be named sign_alg and digest_alg
         }
-
-        # signing and digest algs
-        self.signing_algorithm = self.config.getattr('signing_algorithm', "idp")
-        self.digest_algorithm = self.config.getattr('digest_algorithm', "idp")
-
         for param, val_default in param_defaults.items():
             val_kw = kwargs.get(param)
             val_config = self.config.getattr(param, "idp")
@@ -687,6 +690,7 @@ class Server(Entity):
 
         return args
 
+    # XXX idp create
     def create_authn_response(
         self,
         identity,
@@ -736,7 +740,6 @@ class Server(Entity):
         assertions in the advice element.
         :param encrypt_cert_assertion: Certificate to be used for encryption
         of assertions.
-        :param sign_assertion: True if assertions should be signed.
         :param pefim: True if a response according to the PEFIM profile
         should be created.
         :return: A response instance
@@ -785,6 +788,7 @@ class Server(Entity):
             return self.create_error_response(in_response_to, destination,
                                               sp_entity_id, exc, name_id)
 
+    # XXX idp create
     def create_authn_request_response(self, identity, in_response_to,
                                       destination, sp_entity_id,
                                       name_id_policy=None, userid=None,
@@ -800,7 +804,7 @@ class Server(Entity):
                                           authn_decl=authn_decl,
                                           session_not_on_or_after=session_not_on_or_after)
 
-    # noinspection PyUnusedLocal
+    # XXX idp create
     def create_assertion_id_request_response(self, assertion_id, sign=False,
                                              sign_alg=None,
                                              digest_alg=None, **kwargs):
@@ -827,7 +831,8 @@ class Server(Entity):
         else:
             return assertion
 
-    # noinspection PyUnusedLocal
+    # XXX calls self.sign => should it call _message (which calls self.sign)?
+    # XXX idp create
     def create_name_id_mapping_response(self, name_id=None, encrypted_id=None,
                                         in_response_to=None,
                                         issuer=None, sign_response=False,
@@ -859,6 +864,7 @@ class Server(Entity):
             logger.info("Message: %s", _resp)
             return _resp
 
+    # XXX idp create
     def create_authn_query_response(self, subject, session_index=None,
                                     requested_context=None, in_response_to=None,
                                     issuer=None, sign_response=False,
@@ -892,6 +898,7 @@ class Server(Entity):
     def parse_ecp_authn_request(self):
         pass
 
+    # XXX idp create
     def create_ecp_authn_request_response(self, acs_url, identity,
                                           in_response_to, destination,
                                           sp_entity_id, name_id_policy=None,
diff --git a/src/saml2/sigver.py b/src/saml2/sigver.py
index ee630340..87441807 100644
--- a/src/saml2/sigver.py
+++ b/src/saml2/sigver.py
@@ -1689,11 +1689,6 @@ class SecurityContext(object):
             node_id,
         )
 
-    def sign_assertion_using_xmlsec(self, statement, **kwargs):
-        """ Deprecated function. See sign_assertion(). """
-        return self.sign_statement(
-                statement, class_name(saml.Assertion()), **kwargs)
-
     def sign_assertion(self, statement, **kwargs):
         """Sign a SAML assertion.
 
@@ -1755,6 +1750,7 @@ class SecurityContext(object):
         return statement
 
 
+# XXX calls DefaultSignature
 def pre_signature_part(ident, public_key=None, identifier=None, digest_alg=None, sign_alg=None):
     """
     If an assertion is to be signed the signature part has to be preset