Merge pull request #662 from c00kiemon5ter/fix-xmlsec1-id-attr-param

Fix xmlsec1 --id-attr option

6 files changed, 101 insertions, 165 deletions

diff --git a/src/saml2/sigver.py b/src/saml2/sigver.py
index 6f4b464a..94666b8d 100644
--- a/src/saml2/sigver.py
+++ b/src/saml2/sigver.py
@@ -8,6 +8,7 @@ import hashlib
 import itertools
 import logging
 import os
+import uuid
 import six
 
 from time import mktime
@@ -649,13 +650,13 @@ class CryptoBackend(object):
     def encrypt_assertion(self, statement, enc_key, template, key_type, node_xpath):
         raise NotImplementedError()
 
-    def decrypt(self, enctext, key_file, id_attr):
+    def decrypt(self, enctext, key_file):
         raise NotImplementedError()
 
-    def sign_statement(self, statement, node_name, key_file, node_id, id_attr):
+    def sign_statement(self, statement, node_name, key_file, node_id):
         raise NotImplementedError()
 
-    def validate_signature(self, enctext, cert_file, cert_type, node_name, node_id, id_attr):
+    def validate_signature(self, enctext, cert_file, cert_type, node_name, node_id):
         raise NotImplementedError()
 
 
@@ -767,7 +768,7 @@ class CryptoBackendXmlSec1(CryptoBackend):
 
         return output.decode('utf-8')
 
-    def decrypt(self, enctext, key_file, id_attr):
+    def decrypt(self, enctext, key_file):
         """
 
         :param enctext: XML document containing an encrypted part
@@ -782,8 +783,7 @@ class CryptoBackendXmlSec1(CryptoBackend):
             self.xmlsec,
             '--decrypt',
             '--privkey-pem', key_file,
-            '--id-attr:{id_attr}'.format(id_attr=id_attr),
-            ENC_KEY_CLASS,
+            '--id-attr:Id', ENC_KEY_CLASS,
         ]
 
         try:
@@ -793,7 +793,7 @@ class CryptoBackendXmlSec1(CryptoBackend):
 
         return output.decode('utf-8')
 
-    def sign_statement(self, statement, node_name, key_file, node_id, id_attr):
+    def sign_statement(self, statement, node_name, key_file, node_id):
         """
         Sign an XML statement.
 
@@ -801,8 +801,6 @@ class CryptoBackendXmlSec1(CryptoBackend):
         :param node_name: string like 'urn:oasis:names:...:Assertion'
         :param key_file: The file where the key can be found
         :param node_id:
-        :param id_attr: The attribute name for the identifier, normally one of
-            'id','Id' or 'ID'
         :return: The signed statement
         """
         if isinstance(statement, SamlBase):
@@ -817,8 +815,7 @@ class CryptoBackendXmlSec1(CryptoBackend):
             self.xmlsec,
             '--sign',
             '--privkey-pem', key_file,
-            '--id-attr:{id_attr_name}'.format(id_attr_name=id_attr),
-            node_name,
+            '--id-attr:ID', node_name,
         ]
 
         if node_id:
@@ -836,7 +833,7 @@ class CryptoBackendXmlSec1(CryptoBackend):
             return stdout.decode("utf-8")
         raise SignatureError(stderr)
 
-    def validate_signature(self, signedtext, cert_file, cert_type, node_name, node_id, id_attr):
+    def validate_signature(self, signedtext, cert_file, cert_type, node_name, node_id):
         """
         Validate signature on XML document.
 
@@ -845,8 +842,6 @@ class CryptoBackendXmlSec1(CryptoBackend):
         :param cert_type: The file type of the certificate
         :param node_name: The name of the class that is signed
         :param node_id: The identifier of the node
-        :param id_attr: The attribute name for the identifier, normally one of
-            'id','Id' or 'ID'
         :return: Boolean True if the signature was correct otherwise False.
         """
         if not isinstance(signedtext, six.binary_type):
@@ -862,8 +857,7 @@ class CryptoBackendXmlSec1(CryptoBackend):
             '--verify',
             '--enabled-reference-uris', 'empty,same-doc',
             '--pubkey-cert-{type}'.format(type=cert_type), cert_file,
-            '--id-attr:{id_attr_name}'.format(id_attr_name=id_attr),
-            node_name,
+            '--id-attr:ID', node_name,
         ]
 
         if node_id:
@@ -927,7 +921,7 @@ class CryptoBackendXMLSecurity(CryptoBackend):
         # better than static 0.0 here.
         return 'XMLSecurity 0.0'
 
-    def sign_statement(self, statement, node_name, key_file, node_id, id_attr):
+    def sign_statement(self, statement, node_name, key_file, node_id):
         """
         Sign an XML statement.
 
@@ -950,7 +944,7 @@ class CryptoBackendXMLSecurity(CryptoBackend):
             signed_str = signed_str.decode("utf-8")
         return signed_str
 
-    def validate_signature(self, signedtext, cert_file, cert_type, node_name, node_id, id_attr):
+    def validate_signature(self, signedtext, cert_file, cert_type, node_name, node_id):
         """
         Validate signature on XML document.
 
@@ -989,11 +983,6 @@ def security_context(conf):
     except AttributeError:
         metadata = None
 
-    try:
-        id_attr = conf.id_attr_name
-    except AttributeError:
-        id_attr = None
-
     sec_backend = None
 
     if conf.crypto_backend == 'xmlsec1':
@@ -1054,7 +1043,6 @@ def security_context(conf):
             enc_key_files=enc_key_files,
             encryption_keypairs=conf.encryption_keypairs,
             sec_backend=sec_backend,
-            id_attr=id_attr,
             delete_tmpfiles=conf.delete_tmpfiles)
 
 
@@ -1225,7 +1213,6 @@ class CertHandler(object):
 # openssl x509 -inform pem -noout -in server.crt -pubkey > publickey.pem
 # openssl rsa -inform pem -noout -in publickey.pem -pubin -modulus
 class SecurityContext(object):
-    DEFAULT_ID_ATTR_NAME = 'ID'
     my_cert = None
 
     def __init__(
@@ -1245,11 +1232,8 @@ class SecurityContext(object):
             encryption_keypairs=None,
             enc_cert_type='pem',
             sec_backend=None,
-            id_attr='',
             delete_tmpfiles=True):
 
-        self.id_attr = id_attr or SecurityContext.DEFAULT_ID_ATTR_NAME
-
         self.crypto = crypto
         assert (isinstance(self.crypto, CryptoBackend))
 
@@ -1335,13 +1319,11 @@ class SecurityContext(object):
         return self.crypto.encrypt_assertion(
                 statement, enc_key, template, key_type, node_xpath)
 
-    def decrypt_keys(self, enctext, keys=None, id_attr=''):
+    def decrypt_keys(self, enctext, keys=None):
         """ Decrypting an encrypted text by the use of a private key.
 
         :param enctext: The encrypted text as a string
         :param keys: Keys to try to decrypt enctext with
-        :param id_attr: The attribute name for the identifier, normally one of
-            'id','Id' or 'ID'
         :return: The decrypted text
         """
         key_files = []
@@ -1361,21 +1343,18 @@ class SecurityContext(object):
         key_file_names = list(tmp.name for tmp in key_files)
 
         try:
-            dectext = self.decrypt(enctext, key_file=key_file_names, id_attr=id_attr)
+            dectext = self.decrypt(enctext, key_file=key_file_names)
         except DecryptError as e:
             raise
         else:
             return dectext
 
-    def decrypt(self, enctext, key_file=None, id_attr=''):
+    def decrypt(self, enctext, key_file=None):
         """ Decrypting an encrypted text by the use of a private key.
 
         :param enctext: The encrypted text as a string
         :return: The decrypted text
         """
-        if not id_attr:
-            id_attr = self.id_attr
-
         if not isinstance(key_file, list):
             key_file = [key_file]
 
@@ -1384,7 +1363,7 @@ class SecurityContext(object):
         ]
         for key_file in key_files:
             try:
-                dectext = self.crypto.decrypt(enctext, key_file, id_attr)
+                dectext = self.crypto.decrypt(enctext, key_file)
             except XmlsecError as e:
                 continue
             else:
@@ -1395,7 +1374,7 @@ class SecurityContext(object):
         errmsg = errmsg.format(keys=key_files)
         raise DecryptError(errmsg)
 
-    def verify_signature(self, signedtext, cert_file=None, cert_type='pem', node_name=NODE_NAME, node_id=None, id_attr=''):
+    def verify_signature(self, signedtext, cert_file=None, cert_type='pem', node_name=NODE_NAME, node_id=None):
         """ Verifies the signature of a XML document.
 
         :param signedtext: The XML document as a string
@@ -1403,8 +1382,6 @@ class SecurityContext(object):
         :param cert_type: The file type of the certificate
         :param node_name: The name of the class that is signed
         :param node_id: The identifier of the node
-        :param id_attr: The attribute name for the identifier, normally one of
-            'id','Id' or 'ID'
         :return: Boolean True if the signature was correct otherwise False.
         """
         # This is only for testing purposes, otherwise when would you receive
@@ -1413,18 +1390,15 @@ class SecurityContext(object):
             cert_file = self.cert_file
             cert_type = self.cert_type
 
-        if not id_attr:
-            id_attr = self.id_attr
-
         return self.crypto.validate_signature(
-                signedtext,
-                cert_file=cert_file,
-                cert_type=cert_type,
-                node_name=node_name,
-                node_id=node_id,
-                id_attr=id_attr)
-
-    def _check_signature(self, decoded_xml, item, node_name=NODE_NAME, origdoc=None, id_attr='', must=False, only_valid_cert=False, issuer=None):
+            signedtext,
+            cert_file=cert_file,
+            cert_type=cert_type,
+            node_name=node_name,
+            node_id=node_id,
+        )
+
+    def _check_signature(self, decoded_xml, item, node_name=NODE_NAME, origdoc=None, must=False, only_valid_cert=False, issuer=None):
         try:
             _issuer = item.issuer.text.strip()
         except AttributeError:
@@ -1528,11 +1502,11 @@ class SecurityContext(object):
             try:
                 last_pem_file = pem_fd.name
                 if self.verify_signature(
-                        decoded_xml,
-                        pem_fd.name,
-                        node_name=node_name,
-                        node_id=item.id,
-                        id_attr=id_attr):
+                    decoded_xml,
+                    pem_fd.name,
+                    node_name=node_name,
+                    node_id=item.id,
+                ):
                     verified = True
                     break
             except XmlsecError as exc:
@@ -1550,25 +1524,23 @@ class SecurityContext(object):
 
         return item
 
-    def check_signature(self, item, node_name=NODE_NAME, origdoc=None, id_attr='', must=False, issuer=None):
+    def check_signature(self, item, node_name=NODE_NAME, origdoc=None, must=False, issuer=None):
         """
 
         :param item: Parsed entity
         :param node_name: The name of the node/class/element that is signed
         :param origdoc: The original XML string
-        :param id_attr: The attribute name for the identifier, normally one of
-            'id','Id' or 'ID'
         :param must:
         :return:
         """
         return self._check_signature(
-                origdoc,
-                item,
-                node_name,
-                origdoc,
-                id_attr=id_attr,
-                must=must,
-                issuer=issuer)
+            origdoc,
+            item,
+            node_name,
+            origdoc,
+            must=must,
+            issuer=issuer,
+        )
 
     def correctly_signed_message(self, decoded_xml, msgtype, must=False, origdoc=None, only_valid_cert=False):
         """Check if a request is correctly signed, if we have metadata for
@@ -1683,7 +1655,7 @@ class SecurityContext(object):
         """ Deprecated function. See sign_statement(). """
         return self.sign_statement(statement, **kwargs)
 
-    def sign_statement(self, statement, node_name, key=None, key_file=None, node_id=None, id_attr=''):
+    def sign_statement(self, statement, node_name, key=None, key_file=None, node_id=None):
         """Sign a SAML statement.
 
         :param statement: The statement to be signed
@@ -1691,13 +1663,8 @@ class SecurityContext(object):
         :param key: The key to be used for the signing, either this or
         :param key_file: The file where the key can be found
         :param node_id:
-        :param id_attr: The attribute name for the identifier, normally one of
-            'id','Id' or 'ID'
         :return: The signed statement
         """
-        if not id_attr:
-            id_attr = self.id_attr
-
         if not key_file and key:
             content = str(key).encode()
             tmp = make_temp(content, suffix=".pem", delete_tmpfiles=self.delete_tmpfiles)
@@ -1707,11 +1674,11 @@ class SecurityContext(object):
             key_file = self.key_file
 
         return self.crypto.sign_statement(
-                statement,
-                node_name,
-                key_file,
-                node_id,
-                id_attr)
+            statement,
+            node_name,
+            key_file,
+            node_id,
+        )
 
     def sign_assertion_using_xmlsec(self, statement, **kwargs):
         """ Deprecated function. See sign_assertion(). """
@@ -1749,13 +1716,12 @@ class SecurityContext(object):
         Sign multiple parts of a statement
 
         :param statement: The statement that should be sign, this is XML text
-        :param to_sign: A list of (items, id, id attribute name) tuples that
-            specifies what to sign
+        :param to_sign: A list of (items, id) tuples that specifies what to sign
         :param key: A key that should be used for doing the signing
         :param key_file: A file that contains the key to be used
         :return: A possibly multiple signed statement
         """
-        for (item, sid, id_attr) in to_sign:
+        for (item, sid) in to_sign:
             if not sid:
                 if not item.id:
                     sid = item.id = sid()
@@ -1770,12 +1736,12 @@ class SecurityContext(object):
                         digest_alg=digest_alg)
 
             statement = self.sign_statement(
-                    statement,
-                    class_name(item),
-                    key=key,
-                    key_file=key_file,
-                    node_id=sid,
-                    id_attr=id_attr)
+                statement,
+                class_name(item),
+                key=key,
+                key_file=key_file,
+                node_id=sid,
+            )
 
         return statement
 
@@ -1860,7 +1826,8 @@ def pre_signature_part(ident, public_key=None, identifier=None, digest_alg=None,
 # </EncryptedData>
 
 
-def pre_encryption_part(msg_enc=TRIPLE_DES_CBC, key_enc=RSA_1_5, key_name='my-rsa-key'):
+def pre_encryption_part(msg_enc=TRIPLE_DES_CBC, key_enc=RSA_1_5, key_name='my-rsa-key',
+        encrypted_key_id=None, encrypted_data_id=None):
     """
 
     :param msg_enc:
@@ -1868,10 +1835,12 @@ def pre_encryption_part(msg_enc=TRIPLE_DES_CBC, key_enc=RSA_1_5, key_name='my-rs
     :param key_name:
     :return:
     """
+    ek_id = encrypted_key_id or str(uuid.uuid4())
+    ed_id = encrypted_data_id or str(uuid.uuid4())
     msg_encryption_method = EncryptionMethod(algorithm=msg_enc)
     key_encryption_method = EncryptionMethod(algorithm=key_enc)
     encrypted_key = EncryptedKey(
-            id='EK',
+            id=ek_id,
             encryption_method=key_encryption_method,
             key_info=ds.KeyInfo(
                 key_name=ds.KeyName(text=key_name)),
@@ -1879,7 +1848,7 @@ def pre_encryption_part(msg_enc=TRIPLE_DES_CBC, key_enc=RSA_1_5, key_name='my-rs
                 cipher_value=CipherValue(text='')))
     key_info = ds.KeyInfo(encrypted_key=encrypted_key)
     encrypted_data = EncryptedData(
-        id='ED',
+        id=ed_id,
         type='http://www.w3.org/2001/04/xmlenc#Element',
         encryption_method=msg_encryption_method,
         key_info=key_info,
diff --git a/tests/test_40_sigver.py b/tests/test_40_sigver.py
index 07c919b7..9c6bf87b 100644
--- a/tests/test_40_sigver.py
+++ b/tests/test_40_sigver.py
@@ -196,10 +196,8 @@ class TestSecurity():
     def test_multiple_signatures_assertion(self):
         ass = self._assertion
         # basic test with two of the same
-        to_sign = [(ass, ass.id, ''),
-                   (ass, ass.id, '')
-        ]
-        sign_ass = self.sec.multiple_signatures("%s" % ass, to_sign)
+        to_sign = [(ass, ass.id), (ass, ass.id)]
+        sign_ass = self.sec.multiple_signatures(str(ass), to_sign)
         sass = saml.assertion_from_string(sign_ass)
         assert _eq(sass.keyswv(), ['attribute_statement', 'issue_instant',
                                    'version', 'signature', 'id'])
@@ -223,10 +221,8 @@ class TestSecurity():
 
         # order is important, we can't validate if the signatures are made
         # in the reverse order
-        to_sign = [(self._assertion, self._assertion.id, ''),
-                   (response, response.id, '')]
-
-        s_response = self.sec.multiple_signatures("%s" % response, to_sign)
+        to_sign = [(self._assertion, self._assertion.id), (response, response.id)]
+        s_response = self.sec.multiple_signatures(str(response), to_sign)
         assert s_response is not None
         response = response_from_string(s_response)
 
@@ -491,10 +487,8 @@ class TestSecurityNonAsciiAva():
     def test_multiple_signatures_assertion(self):
         ass = self._assertion
         # basic test with two of the same
-        to_sign = [(ass, ass.id, ''),
-                   (ass, ass.id, '')
-        ]
-        sign_ass = self.sec.multiple_signatures("%s" % ass, to_sign)
+        to_sign = [(ass, ass.id), (ass, ass.id)]
+        sign_ass = self.sec.multiple_signatures(str(ass), to_sign)
         sass = saml.assertion_from_string(sign_ass)
         assert _eq(sass.keyswv(), ['attribute_statement', 'issue_instant',
                                    'version', 'signature', 'id'])
@@ -518,10 +512,8 @@ class TestSecurityNonAsciiAva():
 
         # order is important, we can't validate if the signatures are made
         # in the reverse order
-        to_sign = [(self._assertion, self._assertion.id, ''),
-                   (response, response.id, '')]
-
-        s_response = self.sec.multiple_signatures("%s" % response, to_sign)
+        to_sign = [(self._assertion, self._assertion.id), (response, response.id)]
+        s_response = self.sec.multiple_signatures(str(response), to_sign)
         assert s_response is not None
         response = response_from_string(s_response)
 
diff --git a/tests/test_42_enc.py b/tests/test_42_enc.py
index c29eca1e..1041a19f 100644
--- a/tests/test_42_enc.py
+++ b/tests/test_42_enc.py
@@ -25,7 +25,7 @@ AUTHN = {
 
 
 def test_pre_enc():
-    tmpl = pre_encryption_part()
+    tmpl = pre_encryption_part(encrypted_key_id="EK", encrypted_data_id="ED")
     print(tmpl)
     assert "%s" % tmpl in (TMPL_NO_HEADER, TMPL)
 
diff --git a/tests/test_50_server.py b/tests/test_50_server.py
index 4a262976..e61e050a 100644
--- a/tests/test_50_server.py
+++ b/tests/test_50_server.py
@@ -468,15 +468,13 @@ class TestServer1():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-                                                 node_id=sresponse.id,
-                                                 id_attr="")
+                                                 node_id=sresponse.id)
         assert valid
 
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-                                                 node_id=sresponse.assertion[0].id,
-                                                 id_attr="")
+                                                 node_id=sresponse.assertion[0].id)
         assert valid
 
         self.verify_assertion(sresponse.assertion)
@@ -497,8 +495,7 @@ class TestServer1():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-                                                 node_id=sresponse.id,
-                                                 id_attr="")
+                                                 node_id=sresponse.id)
         assert valid
 
         assert sresponse.assertion[0].signature == None
@@ -523,8 +520,7 @@ class TestServer1():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-                                                 node_id=sresponse.assertion[0].id,
-                                                 id_attr="")
+                                                 node_id=sresponse.assertion[0].id)
         assert valid
 
         self.verify_assertion(sresponse.assertion)
@@ -552,14 +548,16 @@ class TestServer1():
         valid = self.server.sec.verify_signature(
             signed_resp, self.server.config.cert_file,
             node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-            node_id=sresponse.id, id_attr="")
+            node_id=sresponse.id
+        )
 
         assert valid
 
         valid = self.server.sec.verify_signature(
             signed_resp, self.server.config.cert_file,
             node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-            node_id=sresponse.assertion[0].id, id_attr="")
+            node_id=sresponse.assertion[0].id
+        )
 
         assert valid
 
@@ -583,8 +581,7 @@ class TestServer1():
         #valid = self.server.sec.verify_signature(decr_text,
         #                                         self.server.config.cert_file,
         #                                         node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-        #                                         node_id=assertion[0].id,
-        #                                         id_attr="")
+        #                                         node_id=assertion[0].id)
         assert valid
 
     def test_encrypted_signed_response_2(self):
@@ -607,8 +604,7 @@ class TestServer1():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-                                                 node_id=sresponse.id,
-                                                 id_attr="")
+                                                 node_id=sresponse.id)
         assert valid
 
         decr_text_old = copy.deepcopy("%s" % signed_resp)
@@ -653,8 +649,7 @@ class TestServer1():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-                                                 node_id=sresponse.id,
-                                                 id_attr="")
+                                                 node_id=sresponse.id)
         assert valid
 
         key_fd = make_temp(cert_key_str, decode=False)
@@ -667,8 +662,7 @@ class TestServer1():
         valid = self.server.sec.verify_signature(decr_text,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-                                                 node_id=resp.assertion[0].id,
-                                                 id_attr="")
+                                                 node_id=resp.assertion[0].id)
 
         assert valid
 
@@ -700,8 +694,7 @@ class TestServer1():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-                                                 node_id=sresponse.id,
-                                                 id_attr="")
+                                                 node_id=sresponse.id)
         assert valid
 
         decr_text = self.server.sec.decrypt(signed_resp, self.client.config.encryption_keypairs[1]["key_file"])
@@ -713,8 +706,7 @@ class TestServer1():
         valid = self.server.sec.verify_signature(decr_text,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-                                                 node_id=resp.assertion[0].id,
-                                                 id_attr="")
+                                                 node_id=resp.assertion[0].id)
 
         assert valid
 
@@ -733,8 +725,7 @@ class TestServer1():
         #valid = self.server.sec.verify_signature(decr_text,
         #                                         self.server.config.cert_file,
         #                                         node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-        #                                         node_id=assertion[0].id,
-        #                                         id_attr="")
+        #                                         node_id=assertion[0].id)
         assert valid
 
     def test_encrypted_response_1(self):
@@ -1534,15 +1525,13 @@ class TestServer1NonAsciiAva():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-                                                 node_id=sresponse.id,
-                                                 id_attr="")
+                                                 node_id=sresponse.id)
         assert valid
 
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-                                                 node_id=sresponse.assertion[0].id,
-                                                 id_attr="")
+                                                 node_id=sresponse.assertion[0].id)
         assert valid
 
         self.verify_assertion(sresponse.assertion)
@@ -1563,8 +1552,7 @@ class TestServer1NonAsciiAva():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-                                                 node_id=sresponse.id,
-                                                 id_attr="")
+                                                 node_id=sresponse.id)
         assert valid
 
         assert sresponse.assertion[0].signature == None
@@ -1589,8 +1577,7 @@ class TestServer1NonAsciiAva():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-                                                 node_id=sresponse.assertion[0].id,
-                                                 id_attr="")
+                                                 node_id=sresponse.assertion[0].id)
         assert valid
 
         self.verify_assertion(sresponse.assertion)
@@ -1618,14 +1605,16 @@ class TestServer1NonAsciiAva():
         valid = self.server.sec.verify_signature(
             signed_resp, self.server.config.cert_file,
             node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-            node_id=sresponse.id, id_attr="")
+            node_id=sresponse.id,
+        )
 
         assert valid
 
         valid = self.server.sec.verify_signature(
             signed_resp, self.server.config.cert_file,
             node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-            node_id=sresponse.assertion[0].id, id_attr="")
+            node_id=sresponse.assertion[0].id,
+        )
 
         assert valid
 
@@ -1649,8 +1638,7 @@ class TestServer1NonAsciiAva():
         #valid = self.server.sec.verify_signature(decr_text,
         #                                         self.server.config.cert_file,
         #                                         node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-        #                                         node_id=assertion[0].id,
-        #                                         id_attr="")
+        #                                         node_id=assertion[0].id)
         assert valid
 
     def test_encrypted_signed_response_2(self):
@@ -1673,8 +1661,7 @@ class TestServer1NonAsciiAva():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-                                                 node_id=sresponse.id,
-                                                 id_attr="")
+                                                 node_id=sresponse.id)
         assert valid
 
         decr_text_old = copy.deepcopy("%s" % signed_resp)
@@ -1719,8 +1706,7 @@ class TestServer1NonAsciiAva():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-                                                 node_id=sresponse.id,
-                                                 id_attr="")
+                                                 node_id=sresponse.id)
         assert valid
 
         key_fd = make_temp(cert_key_str, decode=False)
@@ -1733,8 +1719,7 @@ class TestServer1NonAsciiAva():
         valid = self.server.sec.verify_signature(decr_text,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-                                                 node_id=resp.assertion[0].id,
-                                                 id_attr="")
+                                                 node_id=resp.assertion[0].id)
 
         assert valid
 
@@ -1766,8 +1751,7 @@ class TestServer1NonAsciiAva():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-                                                 node_id=sresponse.id,
-                                                 id_attr="")
+                                                 node_id=sresponse.id)
         assert valid
 
         decr_text = self.server.sec.decrypt(signed_resp, self.client.config.encryption_keypairs[1]["key_file"])
@@ -1779,8 +1763,7 @@ class TestServer1NonAsciiAva():
         valid = self.server.sec.verify_signature(decr_text,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-                                                 node_id=resp.assertion[0].id,
-                                                 id_attr="")
+                                                 node_id=resp.assertion[0].id)
 
         assert valid
 
@@ -1799,8 +1782,7 @@ class TestServer1NonAsciiAva():
         #valid = self.server.sec.verify_signature(decr_text,
         #                                         self.server.config.cert_file,
         #                                         node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-        #                                         node_id=assertion[0].id,
-        #                                         id_attr="")
+        #                                         node_id=assertion[0].id)
         assert valid
 
     def test_encrypted_response_1(self):
diff --git a/tests/test_51_client.py b/tests/test_51_client.py
index a063dcb4..f6fc2759 100644
--- a/tests/test_51_client.py
+++ b/tests/test_51_client.py
@@ -2679,9 +2679,6 @@ class TestClientNonAsciiAva:
         # Begin with the IdPs side
         _sec = self.server.sec
 
-        nameid_policy = samlp.NameIDPolicy(allow_create="false",
-                                           format=saml.NAMEID_FORMAT_PERSISTENT)
-
         asser_1 = Assertion({"givenName": "Dave"})
 
         farg = add_path(
diff --git a/tests/test_52_default_sign_alg.py b/tests/test_52_default_sign_alg.py
index 6ef8d63e..274ee858 100644
--- a/tests/test_52_default_sign_alg.py
+++ b/tests/test_52_default_sign_alg.py
@@ -117,16 +117,14 @@ class TestSignedResponse():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-                                                 node_id=sresponse.id,
-                                                 id_attr="")
+                                                 node_id=sresponse.id)
         assert valid
         assert ds.SIG_RSA_SHA512 in str(sresponse.assertion[0]), "Not correctly signed!"
         assert ds.DIGEST_SHA512 in str(sresponse.assertion[0]), "Not correctly signed!"
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-                                                 node_id=sresponse.assertion[0].id,
-                                                 id_attr="")
+                                                 node_id=sresponse.assertion[0].id)
         assert valid
 
         self.verify_assertion(sresponse.assertion)
@@ -151,16 +149,14 @@ class TestSignedResponse():
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:protocol:Response',
-                                                 node_id=sresponse.id,
-                                                 id_attr="")
+                                                 node_id=sresponse.id)
         assert valid
         assert ds.SIG_RSA_SHA256 in str(sresponse.assertion[0]), "Not correctly signed!"
         assert ds.DIGEST_SHA256 in str(sresponse.assertion[0]), "Not correctly signed!"
         valid = self.server.sec.verify_signature(signed_resp,
                                                  self.server.config.cert_file,
                                                  node_name='urn:oasis:names:tc:SAML:2.0:assertion:Assertion',
-                                                 node_id=sresponse.assertion[0].id,
-                                                 id_attr="")
+                                                 node_id=sresponse.assertion[0].id)
         assert valid
 
         self.verify_assertion(sresponse.assertion)