Fix #119. Allow AuthnRequest with no NameIDPolicy

Author: pitbulk

README.md

@@ -518,10 +518,11 @@ We can set a 'return_to' url parameter to the login function and that will be co
 target_url = 'https://example.com'
 auth.login(return_to=target_url)
 ```
-The login method can recieve 2 more optional parameters:
+The login method can recieve 3 more optional parameters:
 
-* force_authn  When true the AuthNReuqest will set the ForceAuthn='true'
-* is_passive   When true the AuthNReuqest will set the Ispassive='true'
+* force_authn       When true the AuthNReuqest will set the ForceAuthn='true'
+* is_passive        When true the AuthNReuqest will set the Ispassive='true'
+* set_nameid_policy When true the AuthNReuqest will set a nameIdPolicy element.
 
 If a match on the future SAMLResponse ID and the AuthNRequest ID to be sent is required, that AuthNRequest ID must to be extracted and stored for future validation, we can get that ID by
 

src/onelogin/saml2/auth.py

@@ -267,22 +267,26 @@ def get_last_request_id(self):
         """
         return self.__last_request_id
 
-    def login(self, return_to=None, force_authn=False, is_passive=False):
+    def login(self, return_to=None, force_authn=False, is_passive=False, set_nameid_policy=True):
         """
         Initiates the SSO process.
 
         :param return_to: Optional argument. The target URL the user should be redirected to after login.
         :type return_to: string
 
         :param force_authn: Optional argument. When true the AuthNReuqest will set the ForceAuthn='true'.
-        :type force_authn: string
+        :type force_authn: bool
 
         :param is_passive: Optional argument. When true the AuthNReuqest will set the Ispassive='true'.
-        :type is_passive: string
+        :type is_passive: bool
+
+        :param set_nameid_policy: Optional argument. When true the AuthNReuqest will set a nameIdPolicy element.
+        :type set_nameid_policy: bool
 
         :returns: Redirection url
+        :rtype: string
         """
-        authn_request = OneLogin_Saml2_Authn_Request(self.__settings, force_authn, is_passive)
+        authn_request = OneLogin_Saml2_Authn_Request(self.__settings, force_authn, is_passive, set_nameid_policy)
 
         self.__last_request_id = authn_request.get_id()
 

src/onelogin/saml2/authn_request.py

@@ -22,7 +22,7 @@ class OneLogin_Saml2_Authn_Request(object):
 
     """
 
-    def __init__(self, settings, force_authn=False, is_passive=False):
+    def __init__(self, settings, force_authn=False, is_passive=False, set_nameid_policy=True):
         """
         Constructs the AuthnRequest object.
 
@@ -34,6 +34,9 @@ def __init__(self, settings, force_authn=False, is_passive=False):
 
         :param is_passive: Optional argument. When true the AuthNReuqest will set the Ispassive='true'.
         :type is_passive: bool
+
+        :param set_nameid_policy: Optional argument. When true the AuthNReuqest will set a nameIdPolicy element.
+        :type set_nameid_policy: bool
         """
         self.__settings = settings
 
@@ -47,10 +50,6 @@ def __init__(self, settings, force_authn=False, is_passive=False):
 
         destination = idp_data['singleSignOnService']['url']
 
-        name_id_policy_format = sp_data['NameIDFormat']
-        if 'wantNameIdEncrypted' in security and security['wantNameIdEncrypted']:
-            name_id_policy_format = OneLogin_Saml2_Constants.NAMEID_ENCRYPTED
-
         provider_name_str = ''
         organization_data = settings.get_organization()
         if isinstance(organization_data, dict) and organization_data:
@@ -70,6 +69,17 @@ def __init__(self, settings, force_authn=False, is_passive=False):
         if is_passive is True:
             is_passive_str = 'IsPassive="true"'
 
+        nameid_policy_str = ''
+        if set_nameid_policy:
+            name_id_policy_format = sp_data['NameIDFormat']
+            if 'wantNameIdEncrypted' in security and security['wantNameIdEncrypted']:
+                name_id_policy_format = OneLogin_Saml2_Constants.NAMEID_ENCRYPTED
+
+            nameid_policy_str = """
+    <samlp:NameIDPolicy
+        Format="%s"
+        AllowCreate="true" />""" % name_id_policy_format
+
         requested_authn_context_str = ''
         if 'requestedAuthnContext' in security.keys() and security['requestedAuthnContext'] is not False:
             authn_comparison = 'exact'
@@ -104,9 +114,7 @@ def __init__(self, settings, force_authn=False, is_passive=False):
     AssertionConsumerServiceURL="%(assertion_url)s"
     %(attr_consuming_service_str)s>
     <saml:Issuer>%(entity_id)s</saml:Issuer>
-    <samlp:NameIDPolicy
-        Format="%(name_id_policy)s"
-        AllowCreate="true" />
+%(nameid_policy_str)s
 %(requested_authn_context_str)s
 </samlp:AuthnRequest>""" % \
             {
@@ -118,7 +126,7 @@ def __init__(self, settings, force_authn=False, is_passive=False):
                 'destination': destination,
                 'assertion_url': sp_data['assertionConsumerService']['url'],
                 'entity_id': sp_data['entityId'],
-                'name_id_policy': name_id_policy_format,
+                'nameid_policy_str': nameid_policy_str,
                 'requested_authn_context_str': requested_authn_context_str,
                 'attr_consuming_service_str': attr_consuming_service_str
             }

src/onelogin/saml2/utils.py

@@ -1051,14 +1051,12 @@ def validate_node_sign(signature_node, elem, cert=None, fingerprint=None, finger
                     if fingerprint == x509_fingerprint_value:
                         cert = OneLogin_Saml2_Utils.format_cert(x509_cert_value)
 
-
             # Check if Reference URI is empty
             # reference_elem = OneLogin_Saml2_Utils.query(signature_node, '//ds:Reference')
             # if len(reference_elem) > 0:
             #    if reference_elem[0].get('URI') == '':
             #        reference_elem[0].set('URI', '#%s' % signature_node.getparent().get('ID'))
 
-
             if cert is None or cert == '':
                 return False
 

tests/src/OneLogin/saml2_tests/auth_test.py

@@ -608,7 +608,7 @@ def testLoginSigned(self):
     def testLoginForceAuthN(self):
         """
         Tests the login method of the OneLogin_Saml2_Auth class
-        Case Logout with no parameters. A AuthN Request is built with ForceAuthn and redirect executed
+        Case Login with no parameters. A AuthN Request is built with ForceAuthn and redirect executed
         """
         settings_info = self.loadSettingsJSON()
         return_to = u'http://example.com/returnto'
@@ -642,7 +642,7 @@ def testLoginForceAuthN(self):
     def testLoginIsPassive(self):
         """
         Tests the login method of the OneLogin_Saml2_Auth class
-        Case Logout with no parameters. A AuthN Request is built with IsPassive and redirect executed
+        Case Login with no parameters. A AuthN Request is built with IsPassive and redirect executed
         """
         settings_info = self.loadSettingsJSON()
         return_to = u'http://example.com/returnto'
@@ -673,6 +673,40 @@ def testLoginIsPassive(self):
         request_3 = OneLogin_Saml2_Utils.decode_base64_and_inflate(parsed_query_3['SAMLRequest'][0])
         self.assertIn('IsPassive="true"', request_3)
 
+    def testLoginSetNameIDPolicy(self):
+        """
+        Tests the login method of the OneLogin_Saml2_Auth class
+        Case Logout with no parameters. A AuthN Request is built with and without NameIDPolicy
+        """
+        settings_info = self.loadSettingsJSON()
+        return_to = u'http://example.com/returnto'
+        sso_url = settings_info['idp']['singleSignOnService']['url']
+
+        auth = OneLogin_Saml2_Auth(self.get_request(), old_settings=settings_info)
+        target_url = auth.login(return_to)
+        parsed_query = parse_qs(urlparse(target_url)[4])
+        sso_url = settings_info['idp']['singleSignOnService']['url']
+        self.assertIn(sso_url, target_url)
+        self.assertIn('SAMLRequest', parsed_query)
+        request = OneLogin_Saml2_Utils.decode_base64_and_inflate(parsed_query['SAMLRequest'][0])
+        self.assertIn('<samlp:NameIDPolicy', request)
+
+        auth_2 = OneLogin_Saml2_Auth(self.get_request(), old_settings=settings_info)
+        target_url_2 = auth_2.login(return_to, False, False, True)
+        parsed_query_2 = parse_qs(urlparse(target_url_2)[4])
+        self.assertIn(sso_url, target_url_2)
+        self.assertIn('SAMLRequest', parsed_query_2)
+        request_2 = OneLogin_Saml2_Utils.decode_base64_and_inflate(parsed_query_2['SAMLRequest'][0])
+        self.assertIn('<samlp:NameIDPolicy', request_2)
+
+        auth_3 = OneLogin_Saml2_Auth(self.get_request(), old_settings=settings_info)
+        target_url_3 = auth_3.login(return_to, False, False, False)
+        parsed_query_3 = parse_qs(urlparse(target_url_3)[4])
+        self.assertIn(sso_url, target_url_3)
+        self.assertIn('SAMLRequest', parsed_query_3)
+        request_3 = OneLogin_Saml2_Utils.decode_base64_and_inflate(parsed_query_3['SAMLRequest'][0])
+        self.assertNotIn('<samlp:NameIDPolicy', request_3)
+
     def testLogout(self):
         """
         Tests the logout method of the OneLogin_Saml2_Auth class

tests/src/OneLogin/saml2_tests/authn_request_test.py

@@ -208,6 +208,34 @@ def testCreateRequestIsPassive(self):
         self.assertRegexpMatches(inflated_3, '^<samlp:AuthnRequest')
         self.assertIn('IsPassive="true"', inflated_3)
 
+    def testCreateRequestSetNameIDPolicy(self):
+        """
+        Tests the OneLogin_Saml2_Authn_Request Constructor.
+        The creation of a deflated SAML Request with and without NameIDPolicy
+        """
+        saml_settings = self.loadSettingsJSON()
+        settings = OneLogin_Saml2_Settings(saml_settings)
+        authn_request = OneLogin_Saml2_Authn_Request(settings)
+        authn_request_encoded = authn_request.get_request()
+        decoded = b64decode(authn_request_encoded)
+        inflated = decompress(decoded, -15)
+        self.assertRegexpMatches(inflated, '^<samlp:AuthnRequest')
+        self.assertIn('<samlp:NameIDPolicy', inflated)
+
+        authn_request_2 = OneLogin_Saml2_Authn_Request(settings, False, False, True)
+        authn_request_encoded_2 = authn_request_2.get_request()
+        decoded_2 = b64decode(authn_request_encoded_2)
+        inflated_2 = decompress(decoded_2, -15)
+        self.assertRegexpMatches(inflated_2, '^<samlp:AuthnRequest')
+        self.assertIn('<samlp:NameIDPolicy', inflated_2)
+
+        authn_request_3 = OneLogin_Saml2_Authn_Request(settings, False, False, False)
+        authn_request_encoded_3 = authn_request_3.get_request()
+        decoded_3 = b64decode(authn_request_encoded_3)
+        inflated_3 = decompress(decoded_3, -15)
+        self.assertRegexpMatches(inflated_3, '^<samlp:AuthnRequest')
+        self.assertNotIn('<samlp:NameIDPolicy', inflated_3)
+
     def testCreateDeflatedSAMLRequestURLParameter(self):
         """
         Tests the OneLogin_Saml2_Authn_Request Constructor.