Differentiate between metadata NameIDFormat and AuthnRequest NameIDPolicy Format

The `name_id_format` configuration option is used to define

1. the value of the `<NameIDFormat>` metadata element
2. and the value of the `<NameIDPolicy>` `Format` attribute in an `AuthnRequest`

The configuration option to set what the value of `<NameIDFormat>` element is in the
metadata should be different from the configuration option to specify what should be
requested in an `AuthnRequest` through the `<NameIDPolicy Format="...">` attribute.

Introduce a new option (`name_id_policy_format`), or use the same name but scoped in a
specific section for metadata and AuthnRequest.

On the side of this, pysaml2 defaults to _transient_ as the `<NameIDPolicy
Format="...">` attribute value. To omit requesting a value for the `<NameIDPolicy
Format="">` attribute the value `"None"` (a string) must be set in the configuration.
This is unintuitive. It is better to be explicit and set transient to request a
transient NameID, than not setting a value and requesting transient by default. If no
value is set, no specific `<NameIDPolicy Format="...">` should be requested.

- Refactor the name_id_format usage
- Add name_id_policy_format configuration option
- Remove the "None" convention value

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

7 files changed, 89 insertions, 46 deletions

diff --git a/docs/howto/config.rst b/docs/howto/config.rst
index 70bd1bd5..6e2bb635 100644
--- a/docs/howto/config.rst
+++ b/docs/howto/config.rst
@@ -536,10 +536,26 @@ Example::
     }
 
 
+name_id_policy_format
+"""""""""""""""""""""
+
+A string value that will be used to set the ``Format`` attribute of the
+``<NameIDPolicy>`` element of an ``<AuthnRequest>``.
+
+Example::
+
+    "service": {
+        "sp": {
+            "name_id_policy_format": "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent",
+        }
+    }
+
+
 name_id_format_allow_create
 """""""""""""""""""""""""""
 
-Enable AllowCreate in NameIDPolicy.
+A boolean value (``True`` or ``False``) that will be used to set the ``AllowCreate``
+attribute of the ``<NameIDPolicy>`` element of an ``<AuthnRequest>``.
 
 Example::
 
@@ -550,6 +566,24 @@ Example::
     }
 
 
+name_id_format
+""""""""""""""
+
+A list of string values that will be used to set the ``<NameIDFormat>`` element of the
+metadata of an entity.
+
+Example::
+
+    "service": {
+        "idp": {
+            "name_id_format": [
+                "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent",
+                "urn:oasis:names:tc:SAML:2.0:nameid-format:transient",
+            ]
+        }
+    }
+
+
 allow_unsolicited
 """""""""""""""""
 
diff --git a/src/saml2/client_base.py b/src/saml2/client_base.py
index ad7de4ef..564f657e 100644
--- a/src/saml2/client_base.py
+++ b/src/saml2/client_base.py
@@ -288,7 +288,7 @@ class Base(Entity):
         :param vorg: The virtual organization the service belongs to.
         :param scoping: The scope of the request
         :param binding: The protocol to use for the Response !!
-        :param nameid_format: Format of the NameID
+        :param nameid_format: Format of the NameIDPolicy
         :param service_url_binding: Where the reply should be sent dependent
             on reply binding.
         :param message_id: The identifier for this request
@@ -351,29 +351,20 @@ class Base(Entity):
                 raise ValueError("Wrong type for param {name}".format(name=param))
 
         # NameIDPolicy
-        nameid_format_config = self.config.getattr("name_id_format", "sp")
-        nameid_format_config = (
-            nameid_format_config[0]
-            if isinstance(nameid_format_config, list)
-            else nameid_format_config
-        )
-        nameid_format = (
+        nameid_policy_format_config = self.config.getattr("name_id_policy_format", "sp")
+        nameid_policy_format = (
             nameid_format
-            if nameid_format is not None
-            else NAMEID_FORMAT_TRANSIENT
-            if nameid_format_config is None
-            else None
-            if nameid_format_config == 'None'
-            else nameid_format_config
+            or nameid_policy_format_config
+            or None
         )
 
         allow_create_config = self.config.getattr("name_id_format_allow_create", "sp")
         allow_create = (
             None
             # SAML 2.0 errata says AllowCreate MUST NOT be used for transient ids
-            if nameid_format == NAMEID_FORMAT_TRANSIENT
+            if nameid_policy_format == NAMEID_FORMAT_TRANSIENT
             else allow_create
-            if allow_create is not None
+            if allow_create
             else str(bool(allow_create_config)).lower()
         )
 
@@ -381,13 +372,15 @@ class Base(Entity):
             kwargs.pop("name_id_policy", None)
             if "name_id_policy" in kwargs
             else None
-            if nameid_format == ""
-            else samlp.NameIDPolicy(allow_create=allow_create, format=nameid_format)
+            if not nameid_policy_format
+            else samlp.NameIDPolicy(
+                allow_create=allow_create, format=nameid_policy_format
+            )
         )
 
         if name_id_policy and vorg:
             name_id_policy.sp_name_qualifier = vorg
-            name_id_policy.format = nameid_format or NAMEID_FORMAT_PERSISTENT
+            name_id_policy.format = nameid_policy_format or NAMEID_FORMAT_PERSISTENT
 
         args["name_id_policy"] = name_id_policy
 
diff --git a/src/saml2/config.py b/src/saml2/config.py
index 147d1bdf..278aba16 100644
--- a/src/saml2/config.py
+++ b/src/saml2/config.py
@@ -89,6 +89,7 @@ SP_ARGS = [
     "allow_unsolicited",
     "ecp",
     "name_id_format",
+    "name_id_policy_format",
     "name_id_format_allow_create",
     "logout_requests_signed",
     "requested_attribute_name_format",
@@ -209,6 +210,7 @@ class Config(object):
         self.contact_person = None
         self.name_form = None
         self.name_id_format = None
+        self.name_id_policy_format = None
         self.name_id_format_allow_create = None
         self.virtual_organization = None
         self.only_use_keys_in_metadata = True
diff --git a/src/saml2/metadata.py b/src/saml2/metadata.py
index b2317131..d80b41ac 100644
--- a/src/saml2/metadata.py
+++ b/src/saml2/metadata.py
@@ -379,13 +379,15 @@ def do_extensions(mname, item):
 
 
 def _do_nameid_format(cls, conf, typ):
-    namef = conf.getattr("name_id_format", typ)
-    if namef:
-        if isinstance(namef, six.string_types):
-            ids = [md.NameIDFormat(namef)]
-        else:
-            ids = [md.NameIDFormat(text=form) for form in namef]
-        setattr(cls, "name_id_format", ids)
+    name_id_format = conf.getattr("name_id_format", typ)
+    if not name_id_format:
+        return
+
+    if isinstance(name_id_format, six.string_types):
+        name_id_format = [name_id_format]
+
+    formats = [md.NameIDFormat(text=format) for format in name_id_format]
+    setattr(cls, "name_id_format", formats)
 
 
 def do_endpoints(conf, endpoints):
diff --git a/tests/sp_conf_nameidpolicy.py b/tests/sp_conf_nameidpolicy.py
index d15989c2..c2e81c1d 100644
--- a/tests/sp_conf_nameidpolicy.py
+++ b/tests/sp_conf_nameidpolicy.py
@@ -14,7 +14,7 @@ CONFIG = {
             "required_attributes": ["surName", "givenName", "mail"],
             "optional_attributes": ["title"],
             "idp": ["urn:mace:example.com:saml:roland:idp"],
-            "name_id_format": "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent",
+            "name_id_policy_format": "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent",
             "name_id_format_allow_create": "true"
         }
     },
diff --git a/tests/test_50_server.py b/tests/test_50_server.py
index e61e050a..589890cc 100644
--- a/tests/test_50_server.py
+++ b/tests/test_50_server.py
@@ -251,7 +251,10 @@ class TestServer1():
 
     def test_parse_ok_request(self):
         req_id, authn_request = self.client.create_authn_request(
-            message_id="id1", destination="http://localhost:8088/sso")
+            message_id="id1",
+            destination="http://localhost:8088/sso",
+            nameid_format=saml.NAMEID_FORMAT_TRANSIENT,
+        )
 
         print(authn_request)
         binding = BINDING_HTTP_REDIRECT
@@ -1308,7 +1311,10 @@ class TestServer1NonAsciiAva():
 
     def test_parse_ok_request(self):
         req_id, authn_request = self.client.create_authn_request(
-            message_id="id1", destination="http://localhost:8088/sso")
+            message_id="id1",
+            destination="http://localhost:8088/sso",
+            nameid_format=saml.NAMEID_FORMAT_TRANSIENT,
+        )
 
         print(authn_request)
         binding = BINDING_HTTP_REDIRECT
diff --git a/tests/test_51_client.py b/tests/test_51_client.py
index 302b459d..3b00865c 100644
--- a/tests/test_51_client.py
+++ b/tests/test_51_client.py
@@ -242,7 +242,7 @@ class TestClient:
         req_id, req = self.client.create_attribute_query(
             "https://aai-demo-idp.switch.ch/idp/shibboleth",
             "_e7b68a04488f715cda642fbdd90099f5",
-            format=saml.NAMEID_FORMAT_TRANSIENT,
+            format=NAMEID_FORMAT_TRANSIENT,
             message_id="id1")
 
         assert isinstance(req, samlp.AttributeQuery)
@@ -253,12 +253,15 @@ class TestClient:
         assert req.issue_instant
         assert req.issuer.text == "urn:mace:example.com:saml:roland:sp"
         nameid = req.subject.name_id
-        assert nameid.format == saml.NAMEID_FORMAT_TRANSIENT
+        assert nameid.format == NAMEID_FORMAT_TRANSIENT
         assert nameid.text == "_e7b68a04488f715cda642fbdd90099f5"
 
     def test_create_auth_request_0(self):
         ar_str = "%s" % self.client.create_authn_request(
-            "http://www.example.com/sso", message_id="id1")[1]
+            "http://www.example.com/sso",
+            message_id="id1",
+            nameid_format=NAMEID_FORMAT_TRANSIENT,
+        )[1]
 
         ar = samlp.authn_request_from_string(ar_str)
         assert ar.assertion_consumer_service_url == ("http://lingon.catalogix"
@@ -270,7 +273,7 @@ class TestClient:
         assert ar.issuer.text == "urn:mace:example.com:saml:roland:sp"
         nid_policy = ar.name_id_policy
         assert nid_policy.allow_create is None
-        assert nid_policy.format == saml.NAMEID_FORMAT_TRANSIENT
+        assert nid_policy.format == NAMEID_FORMAT_TRANSIENT
 
         node_requested_attributes = None
         for e in ar.extensions.extension_elements:
@@ -892,7 +895,7 @@ class TestClient:
             subject=factory(saml.Subject, text="_aaa",
                             name_id=factory(
                                 saml.NameID,
-                                format=saml.NAMEID_FORMAT_TRANSIENT)),
+                                format=NAMEID_FORMAT_TRANSIENT)),
             attribute_statement=do_attribute_statement(
                 {
                     ("", "", "sn"): ("Jeter", ""),
@@ -976,7 +979,7 @@ class TestClient:
             self.client.config.entityid,
             self.server.config.attribute_converters,
             self.server.config.getattr("policy", "idp"),
-            name_id=factory(saml.NameID, format=saml.NAMEID_FORMAT_TRANSIENT),
+            name_id=factory(saml.NameID, format=NAMEID_FORMAT_TRANSIENT),
             issuer=self.server._issuer(),
             authn_class=INTERNETPROTOCOLPASSWORD,
             authn_auth="http://www.example.com/login",
@@ -1037,7 +1040,7 @@ class TestClient:
             'in_response_to': "_012345",
             'subject_confirmation_method': saml.SCM_BEARER
         }
-        name_id = factory(saml.NameID, format=saml.NAMEID_FORMAT_TRANSIENT)
+        name_id = factory(saml.NameID, format=NAMEID_FORMAT_TRANSIENT)
 
         farg = add_path(
             {},
@@ -1149,7 +1152,7 @@ class TestClient:
             farg['assertion']['subject']['subject_confirmation'],
             ['subject_confirmation_data', 'recipient',
              "http://lingon.catalogix.se:8087/"])
-        name_id = factory(saml.NameID, format=saml.NAMEID_FORMAT_TRANSIENT)
+        name_id = factory(saml.NameID, format=NAMEID_FORMAT_TRANSIENT)
 
         assertion_1 = asser_1.construct(
             self.client.config.entityid,
@@ -1796,7 +1799,7 @@ class TestClientNonAsciiAva:
         req_id, req = self.client.create_attribute_query(
             "https://aai-demo-idp.switch.ch/idp/shibboleth",
             "_e7b68a04488f715cda642fbdd90099f5",
-            format=saml.NAMEID_FORMAT_TRANSIENT,
+            format=NAMEID_FORMAT_TRANSIENT,
             message_id="id1")
 
         assert isinstance(req, samlp.AttributeQuery)
@@ -1807,12 +1810,15 @@ class TestClientNonAsciiAva:
         assert req.issue_instant
         assert req.issuer.text == "urn:mace:example.com:saml:roland:sp"
         nameid = req.subject.name_id
-        assert nameid.format == saml.NAMEID_FORMAT_TRANSIENT
+        assert nameid.format == NAMEID_FORMAT_TRANSIENT
         assert nameid.text == "_e7b68a04488f715cda642fbdd90099f5"
 
     def test_create_auth_request_0(self):
         ar_str = "%s" % self.client.create_authn_request(
-            "http://www.example.com/sso", message_id="id1")[1]
+            "http://www.example.com/sso",
+            message_id="id1",
+            nameid_format=NAMEID_FORMAT_TRANSIENT,
+        )[1]
 
         ar = samlp.authn_request_from_string(ar_str)
         assert ar.assertion_consumer_service_url == ("http://lingon.catalogix"
@@ -1824,7 +1830,7 @@ class TestClientNonAsciiAva:
         assert ar.issuer.text == "urn:mace:example.com:saml:roland:sp"
         nid_policy = ar.name_id_policy
         assert nid_policy.allow_create is None
-        assert nid_policy.format == saml.NAMEID_FORMAT_TRANSIENT
+        assert nid_policy.format == NAMEID_FORMAT_TRANSIENT
 
         node_requested_attributes = None
         for e in ar.extensions.extension_elements:
@@ -2464,7 +2470,7 @@ class TestClientNonAsciiAva:
             subject=factory(saml.Subject, text="_aaa",
                             name_id=factory(
                                 saml.NameID,
-                                format=saml.NAMEID_FORMAT_TRANSIENT)),
+                                format=NAMEID_FORMAT_TRANSIENT)),
             attribute_statement=do_attribute_statement(
                 {
                     ("", "", "sn"): ("Jeter", ""),
@@ -2548,7 +2554,7 @@ class TestClientNonAsciiAva:
             self.client.config.entityid,
             self.server.config.attribute_converters,
             self.server.config.getattr("policy", "idp"),
-            name_id=factory(saml.NameID, format=saml.NAMEID_FORMAT_TRANSIENT),
+            name_id=factory(saml.NameID, format=NAMEID_FORMAT_TRANSIENT),
             issuer=self.server._issuer(),
             authn_class=INTERNETPROTOCOLPASSWORD,
             authn_auth="http://www.example.com/login",
@@ -2609,7 +2615,7 @@ class TestClientNonAsciiAva:
             'in_response_to': "_012345",
             'subject_confirmation_method': saml.SCM_BEARER
         }
-        name_id = factory(saml.NameID, format=saml.NAMEID_FORMAT_TRANSIENT)
+        name_id = factory(saml.NameID, format=NAMEID_FORMAT_TRANSIENT)
 
         farg = add_path(
             {},
@@ -2722,7 +2728,7 @@ class TestClientNonAsciiAva:
             farg['assertion']['subject']['subject_confirmation'],
             ['subject_confirmation_data', 'recipient',
              "http://lingon.catalogix.se:8087/"])
-        name_id = factory(saml.NameID, format=saml.NAMEID_FORMAT_TRANSIENT)
+        name_id = factory(saml.NameID, format=NAMEID_FORMAT_TRANSIENT)
 
         assertion_1 = asser_1.construct(
             self.client.config.entityid,