Merge commit '277b642da278e484b0b983eea38ee1051feff13e' into feature/fix-friendlyname

Author: jmahoney-eab

.github/workflows/python-package.yml

@@ -0,0 +1,35 @@
+# This workflow will install Python dependencies, run tests and lint with a variety of Python versions
+# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions
+
+name: Python package
+
+on: [push, pull_request]
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        python-version: [2.7,3.5,3.6,3.7, 3.8,3.9]
+
+    steps:
+    - uses: actions/checkout@v2
+    - name: Set up Python ${{ matrix.python-version }}
+      uses: actions/setup-python@v2
+      with:
+        python-version: ${{ matrix.python-version }}
+
+    - name: Install dependencies
+      run: |
+        sudo apt-get update -qq
+        sudo apt-get install -qq swig python-dev libxml2-dev libxmlsec1-dev
+        make install-req
+        make install-test
+
+    - name: Test
+      run: |
+        make pytest
+        make pycodestyle
+        make flake8
+

.gitignore

@@ -14,13 +14,15 @@ __pycache_
 /*.eg
 *.egg-info
 /eggs
+/.eggs
 /build
 /dist
 /venv
 .coverage
 .pypirc
 /.idea
 .mypy_cache/
+.pytest_cache
 
 *.key
 *.crt

Makefile

@@ -0,0 +1,35 @@
+PIP=pip
+FLAKE8=flake8
+PYTEST=pytest
+PYCODESTYLE=pycodestyle
+COVERAGE=coverage
+COVERAGE_CONFIG=tests/coverage.rc
+PEP8_CONFIG=tests/pep8.rc
+MAIN_SOURCE=src/onelogin/saml2
+DEMOS=demo-django demo-flask
+TESTS=tests/src/OneLogin/saml2_tests
+SOURCES=$(MAIN_SOURCE) $(DEMO) $(TESTS)
+
+install-req:
+	$(PIP) install --upgrade 'setuptools<45.0.0'
+	$(PIP) install .
+
+install-test:
+	$(PIP) install -e ".[test]" 
+
+pytest:
+	$(COVERAGE) run --source $(MAIN_SOURCE) --rcfile=$(COVERAGE_CONFIG) -m pytest
+	$(COVERAGE) report -m --rcfile=$(COVERAGE_CONFIG)
+
+pycodestyle:
+	$(PYCODESTYLE) --ignore=E501,E731,W504 $(SOURCES) --config=$(PEP8_CONFIG)
+
+flake8:
+	$(FLAKE8) --ignore=E501,E731,W504 $(SOURCES)
+
+clean: 
+	rm -rf .pytest_cache/
+	rm -rf .eggs/
+	find . -type d -name "__pycache__" -exec rm -r {} +
+	find . -type d -name "*.egg-info" -exec rm -r {} +
+	rm .coverage

README.md

@@ -522,6 +522,13 @@ There's an easier method -- use a metadata exchange.  Metadata is just an XML fi
 
 Using ````parse_remote```` IdP metadata can be obtained and added to the settings without further ado.
 
+Take in mind that the OneLogin_Saml2_IdPMetadataParser class does not validate in any way the URL that is introduced in order to be parsed. 
+
+Usually the same administrator that handles the Service Provider also sets the URL to the IdP, which should be a trusted resource.
+
+But there are other scenarios, like a SAAS app where the administrator of the app delegates this functionality to other users. In this case, extra precaution should be taken in order to validate such URL inputs and avoid attacks like SSRF.
+
+
 ``
 idp_data = OneLogin_Saml2_IdPMetadataParser.parse_remote('https://example.com/auth/saml2/idp/metadata')
 ``
@@ -568,9 +575,10 @@ req = {
 
     # Advanced request options
     "https": "",
-    "lowercase_urlencoding": "",
     "request_uri": "",
-    "query_string": ""
+    "query_string": "",
+    "validate_signature_from_qs": False,
+    "lowercase_urlencoding": False
 }
 ```
 
@@ -602,12 +610,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 ####
 

setup.py

@@ -49,7 +49,8 @@
             'freezegun>=0.3.11, <=1.1.0',
             'pylint==1.9.4',
             'flake8>=3.6.0',
-            'coveralls==1.5.1',
+            'coveralls>=1.11.1',
+            'pytest>=4.6',
         ),
     },
     keywords='saml saml2 xmlsec django flask pyramid python3',

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']:

src/onelogin/saml2/idp_metadata_parser.py

@@ -23,6 +23,9 @@
 class OneLogin_Saml2_IdPMetadataParser(object):
     """
     A class that contain methods related to obtaining and parsing metadata from IdP
+    
+    This class does not validate in any way the URL that is introduced,
+    make sure to validate it properly before use it in a get_metadata method.
     """
 
     @classmethod