Merge pull request #1 from okfn/up-main

Update main with upstream main

.github/workflows/ci.yml

@@ -8,17 +8,14 @@ on:
   pull_request:
     branches: main
 
-env:
-  CKANVERSION: 2.9
-
 jobs:
   code_quality:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
       - name: Setup Python
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v4
         with:
           python-version: '3.8'
 
@@ -34,8 +31,10 @@ jobs:
   test:
     runs-on: ubuntu-latest
     strategy:
+      fail-fast: false
       matrix:
-        python-version: [ '3.7', '3.8' ]
+        python-version: [ '3.7', '3.8', '3.9']
+        ckan-version: ["2.9", "2.10"]
     name: Python ${{ matrix.python-version }} extension test
 
     services:
@@ -69,10 +68,10 @@ jobs:
           - 8983:8983
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
       - name: Setup Python
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v4
         with:
           python-version: ${{ matrix.python-version }}
           architecture: x64
@@ -86,6 +85,7 @@ jobs:
         env:
           PGPASSWORD: postgres
         run: |
+          export CKANVERSION=${{matrix.ckan-version}}
           bash bin/setup-ckan.bash
 
       - name: Test with pytest
@@ -102,10 +102,10 @@ jobs:
     needs: test
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
       - name: Setup Python
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v4
         with:
           python-version: '3.8'
 

README.md

@@ -228,7 +228,7 @@ to PyPI follow these steps:
   [3]: https://gitter.im/keitaroinc/ckan?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge
   [Pypi]: https://img.shields.io/pypi/v/ckanext-saml2auth
   [4]: https://pypi.org/project/ckanext-saml2auth
-  [Python]: https://img.shields.io/badge/python-3.7%20%7C%203.8-blue
+  [Python]: https://img.shields.io/badge/python-3.7%20%7C%203.8%20%7C%203.9-blue
   [5]: https://www.python.org
-  [CKAN]: https://img.shields.io/badge/ckan-2.9-red
+  [CKAN]: https://img.shields.io/badge/ckan-2.9%20%7C%202.10-yellow
   [6]: https://www.ckan.org

ckanext/saml2auth/plugin.py

@@ -101,13 +101,18 @@ def logout(self):
 
         if response:
             domain = h.get_site_domain_for_cookie()
-
-            # Clear auth cookie in the browser
-            response.set_cookie('auth_tkt', domain=domain, expires=0)
-
             # Clear session cookie in the browser
             response.set_cookie('ckan', domain=domain, expires=0)
 
+            if not toolkit.check_ckan_version(min_version="2.10"):
+                # CKAN <= 2.9.x also sets auth_tkt cookie
+                response.set_cookie('auth_tkt', domain=domain, expires=0)
+
+        if g.userobj:
+            log.info(u'User {0}<{1}> logged out successfully'.format(g.userobj.name, g.userobj.email))
+        else:
+            log.info(u'No user was logged in!')
+
         return response
 
 

ckanext/saml2auth/tests/test_blueprint.py

@@ -20,6 +20,7 @@
 import os
 import pytest
 
+from ckan.plugins import toolkit
 from ckan.plugins.toolkit import url_for
 
 here = os.path.dirname(os.path.abspath(__file__))
@@ -65,7 +66,9 @@ def test_came_from_sent_as_relay_state(self, app):
                              os.path.join(extras_folder, 'provider2', 'idp.xml'))
     @pytest.mark.usefixtures('with_request_context')
     def test_cookies_cleared_on_slo(self, app):
-
+        if toolkit.check_ckan_version(min_version='2.10'):
+            # Remove when dropping support for 2.9
+            pytest.skip("auth_tkt cookie has been deprecated in 2.10")
         url = url_for('user.logout')
 
         import datetime
@@ -93,3 +96,43 @@ def test_cookies_cleared_on_slo(self, app):
             assert cookie[cookie_name]['domain'] == 'test.ckan.net'
             cookie_date = date_parse(cookie[cookie_name]['expires'], ignoretz=True)
             assert cookie_date < datetime.datetime.now()
+
+    @pytest.mark.ckan_config(u'ckanext.saml2auth.idp_metadata.location', u'local')
+    @pytest.mark.ckan_config(u'ckanext.saml2auth.idp_metadata.local_path',
+                             os.path.join(extras_folder, 'provider2', 'idp.xml'))
+    @pytest.mark.usefixtures('with_request_context')
+    def test_ckan_cookie_cleared_on_slo(self, app):
+        if not toolkit.check_ckan_version(min_version='2.10'):
+            # Remove when dropping support for 2.9
+            pytest.skip("This test logic introduced in CKAN 2.10")
+        url = url_for('user.logout')
+
+        import datetime
+        from unittest import mock
+        from http.cookies import SimpleCookie
+        from flask import make_response
+        from dateutil.parser import parse as date_parse
+
+        with mock.patch(
+            'ckanext.saml2auth.plugin._perform_slo',
+                return_value=make_response('')):
+            response = app.get(url=url, follow_redirects=False)
+
+        cookie_headers = [
+            h[1] for h in response.headers
+            if h[0].lower() == 'set-cookie']
+
+        # Starting 2.10, CKAN's SessionMiddleware will append a
+        # new Set-cookie header on every first response from the server.
+        # This includes test requests.
+        assert len(cookie_headers) == 2
+
+        first_cookie = cookie_headers[0]
+
+        cookie = SimpleCookie()
+        cookie.load(first_cookie)
+        cookie_name = [name for name in cookie.keys()][0]
+        assert cookie_name == 'ckan'
+        assert cookie[cookie_name]['domain'] == 'test.ckan.net'
+        cookie_date = date_parse(cookie[cookie_name]['expires'], ignoretz=True)
+        assert cookie_date < datetime.datetime.now()

ckanext/saml2auth/tests/test_blueprint_get_request.py

@@ -22,8 +22,13 @@
 from jinja2 import Template
 import os
 import pytest
+try:
+    from unittest import mock
+except ImportError:
+    import mock
 
 from ckan import model
+from ckan.plugins import toolkit
 
 from saml2.xmldsig import SIG_RSA_SHA256
 from saml2.xmldsig import DIGEST_SHA256
@@ -382,7 +387,9 @@ def test_user_fullname_using_first_last_name(self, app):
         response = app.post(url=url, params=data)
         assert 200 == response.status_code
 
-        user = model.User.by_email('[email protected]')[0]
+        user = model.User.by_email('[email protected]')
+        if isinstance(user, list):
+            user = user[0]
 
         assert user.fullname == 'John Smith'
 
@@ -406,7 +413,9 @@ def test_user_fullname_using_fullname(self, app):
         response = app.post(url=url, params=data)
         assert 200 == response.status_code
 
-        user = model.User.by_email('[email protected]')[0]
+        user = model.User.by_email('[email protected]')
+        if isinstance(user, list):
+            user = user[0]
 
         assert user.fullname == 'John Smith (Operations)'
 
@@ -465,3 +474,36 @@ def test_no_relay_state_redirects_to_fallback_config(self, app):
         response = app.post(url=url, params=data, follow_redirects=False)
 
         assert response.headers['Location'] == 'http://test.ckan.net/dataset/'
+
+    @pytest.mark.ckan_config(u'ckanext.saml2auth.entity_id', u'urn:gov:gsa:SAML:2.0.profiles:sp:sso:test:entity')
+    @pytest.mark.ckan_config(u'ckanext.saml2auth.idp_metadata.location', u'local')
+    @pytest.mark.ckan_config(u'ckanext.saml2auth.idp_metadata.local_path', os.path.join(extras_folder, 'provider0', 'idp.xml'))
+    @pytest.mark.ckan_config(u'ckanext.saml2auth.want_response_signed', u'False')
+    @pytest.mark.ckan_config(u'ckanext.saml2auth.want_assertions_signed', u'False')
+    @pytest.mark.ckan_config(u'ckanext.saml2auth.want_assertions_or_response_signed', u'False')
+    def test_repoze_user_id(self, app):
+        if not toolkit.check_ckan_version(max_version='2.9.6'):
+            # Remove when dropping support for 2.9.6
+            pytest.skip("set_repoze_user has been deprecated in 2.10")
+        encoded_response = _prepare_unsigned_response()
+        url = '/acs'
+
+        data = {
+            'SAMLResponse': encoded_response
+        }
+
+        with mock.patch("ckanext.saml2auth.views.saml2auth.set_repoze_user") as m:
+            app.post(url=url, params=data)
+
+            user = model.User.by_email('[email protected]')
+            if isinstance(user, list):
+                user = user[0]
+
+            assert m.called
+            # Check login worked fine by checking the Response object
+            assert m.call_args[0][1].headers["Location"].endswith("/user/me")
+
+            if toolkit.check_ckan_version(min_version="2.9.6"):
+                assert m.call_args[0][0] == user.id + ",1"
+            else:
+                assert m.call_args[0][0] == user.name

ckanext/saml2auth/tests/test_interface.py

@@ -110,7 +110,9 @@ def test_before_create_is_called(self, app):
 
             assert plugin.calls["before_saml2_user_create"] == 1, plugin.calls
 
-        user = model.User.by_email('[email protected]')[0]
+        user = model.User.by_email('[email protected]')
+        if isinstance(user, list):
+            user = user[0]
 
         assert user.fullname.endswith('TEST CREATE')
 
@@ -145,7 +147,9 @@ def test_before_update_is_called_on_saml_user(self, app):
 
             assert plugin.calls["before_saml2_user_update"] == 1, plugin.calls
 
-        user = model.User.by_email('[email protected]')[0]
+        user = model.User.by_email('[email protected]')
+        if isinstance(user, list):
+            user = user[0]
 
         assert user.fullname.endswith('TEST UPDATE')
 
@@ -172,7 +176,9 @@ def test_before_update_is_called_on_ckan_user(self, app):
 
             assert plugin.calls["before_saml2_user_update"] == 1, plugin.calls
 
-        user = model.User.by_email('[email protected]')[0]
+        user = model.User.by_email('[email protected]')
+        if isinstance(user, list):
+            user = user[0]
 
         assert user.fullname.endswith('TEST UPDATE')
 

ckanext/saml2auth/views/saml2auth.py

@@ -76,13 +76,13 @@ def _get_user_by_saml_id(saml_id):
 
 def _get_user_by_email(email):
 
-    user_obj = model.User.by_email(email)
-    if user_obj:
-        user_obj = user_obj[0]
+    user = model.User.by_email(email)
+    if user and isinstance(user, list):
+        user = user[0]
 
-    h.activate_user_if_deleted(user_obj)
+    h.activate_user_if_deleted(user)
 
-    return _dictize_user(user_obj) if user_obj else None
+    return _dictize_user(user) if user else None
 
 
 def _update_user(user_dict):
@@ -263,8 +263,8 @@ def acs():
 
     resp = toolkit.redirect_to(redirect_target)
 
-    # log the user in programmatically
-    set_repoze_user(g.user, resp)
+    _log_user_into_ckan(resp)
+
     set_saml_session_info(session, session_info)
     set_subject_id(session, session_info['name_id'])
 
@@ -274,6 +274,30 @@ def acs():
     return resp
 
 
+def _log_user_into_ckan(resp):
+    """ Log the user into different CKAN versions.
+
+    CKAN 2.10 introduces flask-login and login_user method.
+
+    CKAN 2.9.6 added a security change and identifies the user
+    with the internal id plus a serial autoincrement (currently static).
+
+    CKAN <= 2.9.5 identifies the user only using the internal id.
+    """
+    if toolkit.check_ckan_version(min_version="2.10"):
+        from ckan.common import login_user
+        login_user(g.userobj)
+        return
+
+    if toolkit.check_ckan_version(min_version="2.9.6"):
+        user_id = "{},1".format(g.userobj.id)
+    else:
+        user_id = g.userobj.name
+    set_repoze_user(user_id, resp)
+
+    log.info(u'User {0}<{1}> logged in successfully'.format(g.userobj.name, g.userobj.email))
+
+
 def saml2login():
     u'''Redirects the user to the
      configured identity provider for authentication

setup.py

@@ -34,7 +34,7 @@
     # Versions should comply with PEP440.  For a discussion on single-sourcing
     # the version across setup.py and the project code, see
     # http://packaging.python.org/en/latest/tutorial.html#version
-    version='1.2.2',
+    version='1.3.0',
 
     description='''An extension to enable Single Sign On(SSO) for CKAN data portals via SAML2 Authentication.''',
     long_description=long_description,
@@ -79,7 +79,7 @@
     packages=find_packages(exclude=['contrib', 'docs', 'tests*']),
     namespace_packages=['ckanext'],
 
-    install_requires=['pysaml2>=6.5.1'],
+    install_requires=['pysaml2>=6.5.1,<7.4'],
 
     # If there are data files included in your packages that need to be
     # installed, specify them here.  If using Python 2.6 or less, then these