Repository: IdentityPython/pysaml2

Issue:
make_metadata.py fails to generate signed metadata with Python 3
When using Python 3, `make_metadata.py` and the examples error out when trying to generate signed metadata.

The problem is that [`sign_entity_descriptor()`](https://github.com/IdentityPython/pysaml2/blob/master/src/saml2/metadata.py#L810) returns `str` (from a backend's [`sign_statement()`](https://github.com/IdentityPython/pysaml2/blob/master/src/saml2/sigver.py#L796), which calls `decode()` on the data), and then passes it to [`metadata_tostring_fix()`](https://github.com/IdentityPython/pysaml2/blob/master/src/saml2/metadata.py#L65), which expects it to be a `bytes` in Python 3.

## Code Version
At least 5.0.0, 5.1.0 and current master are affected.

## Expected Behavior
It should not crash.

## Current Behavior
```
Traceback (most recent call last):
  File "./sp.py", line 830, in metadata
    _args.sign,
  File "/home/asdf/pysaml2/venv/lib/python3.7/site-packages/pysaml2-5.1.0-py3.7.egg/saml2/metadata.py", line 113, in create_metadata_string
    return metadata_tostring_fix(eid, nspair, xmldoc)
  File "/home/asdf/pysaml2/venv/lib/python3.7/site-packages/pysaml2-5.1.0-py3.7.egg/saml2/metadata.py", line 73, in metadata_tostring_fix
    if b"\"xs:string\"" in xmlstring and bXMLNSXS not in xmlstring:
TypeError: 'in <string>' requires string as left operand, not bytes
```

## Possible Solution

The backwards-compatible change would probably be to make `metadata_tostring_fix()` accept `str` for Python 3.

The library contains two uses of that function, in `make_metadata.py` and `create_metadata_string()`, both after a call to `sign_entity_descriptor()`. However, in case someone is using `metadata_tostring_fix()` externally, it might be a good idea to make it allow both `bytes` and `str`.

## Steps to Reproduce
1. Run the `sp-wsgi` example using Python 3.
2. Access `http://localhost:8087/metadata`.
3. Observe the error message in the console.


Discussion:

Patch:
diff --git a/src/saml2/metadata.py b/src/saml2/metadata.py
index 5c465032..b2317131 100644
--- a/src/saml2/metadata.py
+++ b/src/saml2/metadata.py
@@ -66,10 +66,10 @@ def metadata_tostring_fix(desc, nspair, xmlstring=""):
     if not xmlstring:
         xmlstring = desc.to_string(nspair)
 
-    if six.PY2:
+    try:
         if "\"xs:string\"" in xmlstring and XMLNSXS not in xmlstring:
             xmlstring = xmlstring.replace(MDNS, MDNS + XMLNSXS)
-    else:
+    except TypeError:
         if b"\"xs:string\"" in xmlstring and bXMLNSXS not in xmlstring:
             xmlstring = xmlstring.replace(bMDNS, bMDNS + bXMLNSXS)
 


Test patch:
diff --git a/tests/test_39_metadata.py b/tests/test_39_metadata.py
index a3796083..8ab6dfa5 100644
--- a/tests/test_39_metadata.py
+++ b/tests/test_39_metadata.py
@@ -1,7 +1,10 @@
 import copy
 from saml2.config import SPConfig
-from saml2.metadata import entity_descriptor
+from saml2.metadata import create_metadata_string, entity_descriptor
 from saml2.saml import NAME_FORMAT_URI, NAME_FORMAT_BASIC
+from saml2 import sigver
+
+from pathutils import full_path
 
 __author__ = 'roland'
 
@@ -47,5 +50,17 @@ def test_requested_attribute_name_format():
         assert req_attr.name_format == NAME_FORMAT_BASIC
 
 
+def test_signed_metadata_proper_str_bytes_handling():
+    sp_conf_2 = sp_conf.copy()
+    sp_conf_2['key_file'] = full_path("test.key")
+    sp_conf_2['cert_file'] = full_path("inc-md-cert.pem")
+    # requires xmlsec binaries per https://pysaml2.readthedocs.io/en/latest/examples/sp.html
+    sp_conf_2['xmlsec_binary'] = sigver.get_xmlsec_binary(["/opt/local/bin"])
+    cnf = SPConfig().load(sp_conf_2, metadata_construction=True)
+
+    # This will raise TypeError if string/bytes handling is not correct
+    sp_metadata = create_metadata_string('', config=cnf, sign=True)
+
+
 if __name__ == '__main__':
-    test_requested_attribute_name_format()
\ No newline at end of file
+    test_requested_attribute_name_format()