From 293bcde73b12b7374f120752680c05a399b76774 Mon Sep 17 00:00:00 2001
From: Eugene Toder <eltoder@gmail.com>
Date: Sat, 27 Mar 2021 17:41:59 -0400
Subject: [PATCH] Add an option to use query string for validation

When validating request or response signature in process_slo() we
currently rebuild query string from 'get_data' elements. This requires
URL encoding components of the string. Unfortunately, some IdPs (Azure
AD, ADFS) use lower-case encoding. To handle this, one needs to pass
lowercase_urlencoding=True. This complicates code that needs to support
different IdPs.

Instead, if 'query_string' is passed, take parts from it directly. This
avoids the need to URL encode them. This is similar to the
`retrieveParametersFromServer` argument in the PHP version.

This feature is disabled by default. Pass validate_signature_from_qs=True
to enable it.
---
 README.md                  | 11 ++++++-----
 src/onelogin/saml2/auth.py | 36 ++++++++++++++++++++++++++----------
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/README.md b/README.md
index dc854bbb..bb67791d 100644
--- a/README.md
+++ b/README.md
@@ -562,9 +562,10 @@ req = {
 
     # Advanced request options
     "https": "",
-    "lowercase_urlencoding": "",
     "request_uri": "",
-    "query_string": ""
+    "query_string": "",
+    "validate_signature_from_qs": False,
+    "lowercase_urlencoding": False
 }
 ```
 
@@ -596,12 +597,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 ####
 
diff --git a/src/onelogin/saml2/auth.py b/src/onelogin/saml2/auth.py
index 33de8724..cfaee5fd 100644
--- a/src/onelogin/saml2/auth.py
+++ b/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']: