WIP works good - set on init use on create_

c00kiemon5ter · c00kiemon5ter · commit c0410837a5ee · 2020-11-24T17:46:32.000+02:00
Signed-off-by: Ivan Kanakarakis &lt;ivan.kanak@gmail.com&gt;
diff --git a/src/saml2/client.py b/src/saml2/client.py
@@ -201,6 +201,8 @@ def global_logout(self, name_id, reason="", expire=None, sign=None,
         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
@@ -56,7 +56,7 @@
 
 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 @@ def __init__(self, config=None, identity_cache=None, state_cache=None,
                 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 @@ def service_urls(self, binding=BINDING_HTTP_POST):
         else:
             return None
 
+    # XXX sp create
     def create_authn_request(
         self,
         destination,
@@ -451,12 +448,9 @@ def create_authn_request(
         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 @@ def create_authn_request(
 
         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 @@ def create_attribute_query(self, destination, name_id=None,
     # 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 @@ def create_authz_decision_query(self, destination, action,
                              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 @@ def create_authz_decision_query_using_assertion(self, destination,
                 extensions=extensions, sign=sign, nsprefix=nsprefix)
 
     @staticmethod
+    # XXX sp create
     def create_assertion_id_request(assertion_id_refs, **kwargs):
         """
 
@@ -644,6 +642,7 @@ def create_assertion_id_request(assertion_id_refs, **kwargs):
         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 @@ def create_authn_query(self, subject, destination=None, authn_context=None,
                              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 @@ def parse_name_id_mapping_request_response(self, txt, binding=BINDING_SOAP):
 
     # ------------------- 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 @@ def can_handle_ecp_response(response):
     # ----------------------------------------------------------------------
 
     @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
@@ -141,6 +141,24 @@ def __init__(self, entity_type, config=None, config_file="",
         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 @@ def apply_binding(
         :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 @@ def sign(
         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 @@ def sign(
         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 @@ def _encrypt_assertion(self, encrypt_cert, sp_entity_id, response,
             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 @@ def _response(self, in_response_to, consumer_url=None, status=None,
         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 @@ def _parse_request(self, enc_request, request_cls, service, binding):
 
     # ------------------------------------------------------------------------
 
+    # 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 @@ def create_error_response(self, in_response_to, destination, info,
 
     # ------------------------------------------------------------------------
 
+    # 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 @@ def create_logout_request(self, destination, issuer_entity_id,
                              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 @@ def create_logout_response(self, request, bindings=None, status=None,
 
         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 @@ def create_artifact_resolve(self, artifact, destination, sessid,
                              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 @@ def create_artifact_response(self, request, artifact, bindings=None,
 
         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 @@ def parse_manage_name_id_request(self, xmlstr, binding=BINDING_SOAP):
         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
@@ -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 @@ def verify_assertion_consumer_service(self, request):
         return False
 
     # -------------------------------------------------------------------------
+
     def parse_authn_request(self, enc_request, binding=BINDING_HTTP_REDIRECT):
         """Parse a Authentication Request
 
@@ -438,7 +446,6 @@ def _authn_response(
         :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 @@ def _authn_response(
 
     # ------------------------------------------------------------------------
 
-    # 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,28 +601,24 @@ def create_attribute_response(self, identity, in_response_to, destination,
     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,
             'encrypt_assertion_self_contained': True,
             '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 @@ def gather_authn_response_args(
 
         return args
 
+    # XXX idp create
     def create_authn_response(
         self,
         identity,
@@ -736,7 +740,6 @@ def create_authn_response(
         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 @@ def create_authn_response(
             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 @@ def create_authn_request_response(self, identity, in_response_to,
                                           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 @@ def create_assertion_id_request_response(self, assertion_id, sign=False,
         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 @@ def create_name_id_mapping_response(self, name_id=None, encrypted_id=None,
             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 @@ def create_authn_query_response(self, subject, session_index=None,
     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
