Merge pull request #256 from eltoder/feature/validate-signature-query-string

Add an option to use query string for validation

Author: pitbulk

README.md

@@ -568,9 +568,10 @@ req = {
 
     # Advanced request options
     "https": "",
-    "lowercase_urlencoding": "",
     "request_uri": "",
-    "query_string": ""
+    "query_string": "",
+    "validate_signature_from_qs": False,
+    "lowercase_urlencoding": False
 }
 ```
 
@@ -602,12 +603,12 @@ An explanation of some advanced request parameters:
 
 * `https` - Defaults to ``off``. Set this to ``on`` if you receive responses over HTTPS.
 
-* `lowercase_urlencoding` - Defaults to `false`. ADFS users should set this to `true`.
-
-* `request_uri` - The path where your SAML server recieves requests. Set this if requests are not recieved at the server's root.
+* `request_uri` - The path where your SAML server receives requests. Set this if requests are not received at the server's root.
 
 * `query_string` - Set this with additional query parameters that should be passed to the request endpoint.
 
+* `validate_signature_from_qs` - If `True`, use `query_string` to validate request and response signatures. Otherwise, use `get_data`. Defaults to `False`. Note that when using `get_data`, query parameters need to be url-encoded for validation. By default we use upper-case url-encoding. Some IdPs, notably Microsoft AD, use lower-case url-encoding, which makes signature validation to fail. To fix this issue, either pass `query_string` and set `validate_signature_from_qs` to `True`, which works for all IdPs, or set `lowercase_urlencoding` to `True`, which only works for AD.
+
 
 #### Initiate SSO ####
 

src/onelogin/saml2/auth.py

@@ -528,6 +528,22 @@ def add_response_signature(self, response_data, sign_algorithm=OneLogin_Saml2_Co
         """
         return self.__build_signature(response_data, 'SAMLResponse', sign_algorithm)
 
+    @staticmethod
+    def __build_sign_query_from_qs(query_string, saml_type):
+        """
+        Build sign query from query string
+
+        :param query_string: The query string
+        :type query_string: str
+
+        :param saml_type: The target URL the user should be redirected to
+        :type saml_type: string SAMLRequest | SAMLResponse
+        """
+        args = ('%s=' % saml_type, 'RelayState=', 'SigAlg=')
+        parts = query_string.split('&')
+        # Join in the order of arguments rather than the original order of parts.
+        return '&'.join(part for arg in args for part in parts if part.startswith(arg))
+
     @staticmethod
     def __build_sign_query(saml_data, relay_state, algorithm, saml_type, lowercase_urlencoding=False):
         """
@@ -660,16 +676,16 @@ def __validate_signature(self, data, saml_type, raise_exceptions=False):
             if isinstance(sign_alg, bytes):
                 sign_alg = sign_alg.decode('utf8')
 
-            lowercase_urlencoding = False
-            if 'lowercase_urlencoding' in self.__request_data.keys():
-                lowercase_urlencoding = self.__request_data['lowercase_urlencoding']
-
-            signed_query = self.__build_sign_query(data[saml_type],
-                                                   data.get('RelayState', None),
-                                                   sign_alg,
-                                                   saml_type,
-                                                   lowercase_urlencoding
-                                                   )
+            query_string = self.__request_data.get('query_string')
+            if query_string and self.__request_data.get('validate_signature_from_qs'):
+                signed_query = self.__build_sign_query_from_qs(query_string, saml_type)
+            else:
+                lowercase_urlencoding = self.__request_data.get('lowercase_urlencoding', False)
+                signed_query = self.__build_sign_query(data[saml_type],
+                                                       data.get('RelayState'),
+                                                       sign_alg,
+                                                       saml_type,
+                                                       lowercase_urlencoding)
 
             if exists_multix509sign:
                 for cert in idp_data['x509certMulti']['signing']: