TLT-4135 (feat) add setting to exclude views

.github/workflows/ci.yaml

@@ -0,0 +1,24 @@
+name: Quality Checks
+on: [push]
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        django-version: ['3.2', '3.1', '3.0', '2.2', '2.1', '2.0']
+        python-version: ['3.7', '3.8', '3.9']
+    steps:
+      - uses: actions/checkout@v3
+      - name: Set up Python
+        uses: actions/setup-python@v3
+        with:
+          python-version: ${{ matrix.python-version }}
+          cache: 'pip'
+          cache-dependency-path: 'setup.py'
+      - name: Install Python Dependencies
+        run: |
+            pip install --upgrade pip wheel
+            pip install Django~=${{ matrix.django-version }}
+            python setup.py install
+      - name: Run tests
+        run: python run_tests.py

.gitignore

@@ -5,6 +5,12 @@ dist
 *.egg-info
 *.egg
 .idea
-.tox/
 .eggs/
 .vscode/
+
+.venv
+.tox
+
+# created by `pyenv local`; this allows for customization of
+# versions to test under e.g. `tox`
+.python-version

LICENSE

@@ -1,4 +1,4 @@
-Copyright 2016 The President and Fellows of Harvard College
+Copyright 2021 The President and Fellows of Harvard College
 
    Licensed under the Apache License, Version 2.0 (the "License");
    you may not use this file except in compliance with the License.

README.md

@@ -1,28 +1,88 @@
 
 # django-auth-lti
 
-django_auth_lti is a package that provides Django authentication middleware and backend classes for building tools that work with an LTI consumer. 
+django_auth_lti is a package that provides Django authentication middleware and backend classes for building tools that work with an LTI consumer.
 
 To use LTI authentication with a Django app, edit settings.py as follows:
 
-* add 'django_auth_lti.middleware_patched.MultiLTILaunchAuthMiddleware' to your MIDDLEWARE (Django >= 1.10) or MIDDLEWARE_CLASSES (Django < 1.10), making sure that it appears AFTER 'django.contrib.auth.middleware.AuthenticationMiddleware'
+* add `django_auth_lti.middleware_patched.MultiLTILaunchAuthMiddleware` to your MIDDLEWARE (Django >= 1.10), making sure that it appears AFTER `django.contrib.auth.middleware.AuthenticationMiddleware`
 
-* add 'django_auth_lti.backends.LTIAuthBackend' to your BACKEND_CLASSES
+* add 'django_auth_lti.backends.LTIAuthBackend' to your `BACKEND_CLASSES`
 
 * configure the OAuth credentials - add something like this to your project configuration:
-```
+```python
 LTI_OAUTH_CREDENTIALS = {
     'test': 'secret',
     'test2': 'reallysecret'
 }
 ```
 
 * OPTIONALLY, you can define a custom role key at the project level. Doing so will cause the middleware to look for any roles associated with that key during the launch request and merge them into the default LTI roles list.  You can declare such a key by adding this to your project configuration:
-```
+```python
 LTI_CUSTOM_ROLE_KEY = 'my-custom-role-key-change-me'
 ```
 
-The MultiLTILaunchAuthMiddleware will ensure that all users of your app are authenticated before they can access any page.  Upon successful authentication, a Django user record is created (or updated) and the user is allowed to access the application.  The middleware will also make the LTI launch parameters available to any request via a 'LTI' parameter on the request object.
-```
+The `MultiLTILaunchAuthMiddleware` will ensure that all users of your app are authenticated before they can access any page.  Upon successful authentication, a Django user record is created (or updated) and the user is allowed to access the application.  The middleware will also make the LTI launch parameters available to any request via a 'LTI' parameter on the request object.
+```python
 request.LTI.get('resource_link_id')
 ```
+
+# Excluding paths
+
+The middleware and reverse monkeypatch will skip checks for the `LTI` parameter if:
+
+* `request.path` is blank (i.e. the empty string `""`)
+* `request.path` exactly matches one of the paths in the `EXCLUDE_PATHS` setting.
+
+To provide custom paths to exclude (e.g. `/w/ping/` for watchman, or `/app/tool_config/`), add the following in your django project condfiguration:
+
+```python
+DJANGO_AUTH_LTI_EXCLUDE_PATHS = [
+    '/lti_tool/tool_config/',
+    '/w/ping/',
+]
+```
+
+# Local development
+
+Bootstrapping a local Python development environment on your host machine for testing (`USE_PYTHON_VERSION` can correspond to the Python version under test):
+
+```sh
+USE_PYTHON_VERSION="3.9.10"
+VENV_DIR=".venv"
+pyenv install --skip-existing ${USE_PYTHON_VERSION}
+rm -Rf "${VENV_DIR}" && PYENV_VERSION=${USE_PYTHON_VERSION} python -m venv "${VENV_DIR}"
+. "${VENV_DIR}"/bin/activate && pip install --upgrade pip wheel
+. "${VENV_DIR}"/bin/activate && python setup.py install
+```
+
+To test against a specific version of Django, you can `pip install` it before running `python setup.py install`.
+
+# Testing
+
+To run tests in a single environment:
+
+```sh
+python run_tests.py
+```
+
+To run a test matrix across Python and Django versions:
+
+```sh
+pip install tox
+
+# You'll need to have all the Python versions specified in tox installed.
+# For pyenv, you can use e.g.
+#
+#    pyenv install --skip-existing 3.7.x
+#    pyenv install --skip-existing 3.8.x
+#    pyenv install --skip-existing 3.9.x
+#    pyenv install --skip-existing 3.10.x
+#
+# ... where x is the specific version available to you in pyenv.
+# Then make them available to tox like so:
+#
+#    pyenv local 3.7.x 3.8.x 3.9.x 3.10.x
+
+tox  # --parallel (optional)
+```

django_auth_lti/conf.py

@@ -0,0 +1,9 @@
+from django.conf import settings
+
+
+def get_excluded_paths():
+    excluded = getattr(settings, "DJANGO_AUTH_LTI_EXCLUDE_PATHS", [])
+    excluded.append("")
+    # add a blank path to the list by default, as `path` can sometimes not
+    # exist, e.g. when rendering django-debug-toolbar templates
+    return excluded

django_auth_lti/middleware_patched.py

@@ -1,19 +1,17 @@
 import logging
 
-import django_auth_lti.patch_reverse
-
 from django.contrib import auth
 from django.core.exceptions import ImproperlyConfigured
 from django.conf import settings
+from django.utils.deprecation import MiddlewareMixin
 
+from .conf import get_excluded_paths
 from .timer import Timer
-
 from .thread_local import set_current_request
-
-try:
-    from django.utils.deprecation import MiddlewareMixin
-except ImportError:  # Django < 1.10
-    MiddlewareMixin = object
+# importing here will ensure that django.urls.reverse is patched
+# for other libraries and parts of django, e.g. `url` template tag,
+# when the middleware is loaded
+import django_auth_lti.patch_reverse
 
 
 logger = logging.getLogger(__name__)
@@ -41,6 +39,12 @@ class MultiLTILaunchAuthMiddleware(MiddlewareMixin):
     def process_request(self, request):
         logger.debug('inside process_request %s' % request.path)
 
+        # note that `path` can sometimes not exist, e.g. when rendering
+        # django-debug-toolbar templates
+        if getattr(request, "path", "") in get_excluded_paths():
+            set_current_request(request)
+            return
+
         # AuthenticationMiddleware is required so that request.user exists.
         if not hasattr(request, 'user'):
             logger.debug('improperly configured: request has no user attr')

django_auth_lti/patch_reverse.py

@@ -4,6 +4,8 @@
 from urllib.parse import urlparse, urlunparse, parse_qs
 from urllib.parse import urlencode
 
+from django_auth_lti.conf import get_excluded_paths
+
 from .thread_local import get_current_request
 
 django_reverse = None
@@ -23,9 +25,10 @@ def reverse(*args, **kwargs):
     # Check for custom exclude_resource_link_id kwarg and remove it before
     # passing kwargs to django reverse
     exclude_resource_link_id = kwargs.pop('exclude_resource_link_id', False)
+    excluded_path = getattr(request, "path", "") in get_excluded_paths()
 
     url = django_reverse(*args, **kwargs)
-    if not exclude_resource_link_id:
+    if not exclude_resource_link_id and not excluded_path:
         # Append resource_link_id query param if exclude_resource_link_id kwarg
         # was not passed or is False
         parsed = urlparse(url)

django_auth_lti/request_validator.py

@@ -1,9 +1,7 @@
 import logging
-import time
 
 from django.conf import settings
 from django.core.cache import cache
-from django.core.exceptions import ImproperlyConfigured
 from oauthlib.common import to_unicode
 from oauthlib.oauth1 import RequestValidator
 

django_auth_lti/verification.py

@@ -1,4 +1,4 @@
-from django.core.exceptions import ImproperlyConfigured, PermissionDenied
+from django.core.exceptions import PermissionDenied
 
 
 def is_allowed(request, allowed_roles, raise_exception):
@@ -11,10 +11,10 @@ def is_allowed(request, allowed_roles, raise_exception):
 
     user_roles = request.LTI.get('roles', [])
     is_user_allowed = set(allowed) & set(user_roles)
-    
+
     if not is_user_allowed and raise_exception:
         raise PermissionDenied
-    
+
     return is_user_allowed
 
 

setup.py

@@ -8,7 +8,7 @@
 
 setup(
     name='django-auth-lti',
-    version='2.0.4',
+    version='2.1.0',
     packages=['django_auth_lti'],
     include_package_data=True,
     license='TBD License',  # example license
@@ -24,8 +24,9 @@
         'License :: OSI Approved :: BSD License',  # example license
         'Operating System :: OS Independent',
         'Programming Language :: Python',
-        'Programming Language :: Python :: 3.6',
         'Programming Language :: Python :: 3.7',
+        'Programming Language :: Python :: 3.8',
+        'Programming Language :: Python :: 3.9',
         'Topic :: Internet :: WWW/HTTP',
         'Topic :: Internet :: WWW/HTTP :: Dynamic Content',
     ],
@@ -36,8 +37,5 @@
         "oauthlib==3.1.1",
         "requests_oauthlib"
     ],
-    tests_require=[
-        'mock',
-    ],
     zip_safe=False,
 )

tests/helpers.py

@@ -2,7 +2,7 @@
 from django.contrib.auth import models
 from unittest import mock
 
-def build_lti_launch_request(post_data):
+def build_lti_launch_request(post_data, url='/fake/lti/launch'):
     """
     Utility method that builds a fake lti launch request with custom data.
     """
@@ -12,7 +12,7 @@ def build_lti_launch_request(post_data):
     if 'resource_link_id' not in post_data:
         post_data.update(resource_link_id='d202fb112a14f27107149ed874bf630aa8e029a5')
 
-    request = RequestFactory().post('/fake/lti/launch', post_data)
+    request = RequestFactory().post(url, post_data)
     request.user = mock.Mock(name='User', spec=models.User)
     request.session = {}
     return request

tests/test_lti_auth_middleware_patched.py

@@ -1,7 +1,9 @@
 import unittest
-from unittest.mock import patch
+from unittest.mock import Mock, PropertyMock, patch
+
 from django_auth_lti.middleware_patched import MultiLTILaunchAuthMiddleware
-from django.test import TestCase, override_settings
+from django.test import override_settings, RequestFactory
+
 from . import helpers
 
 LTI_AUTH_MAX_LAUNCHES=3
@@ -16,6 +18,30 @@ def setUp(self):
     def build_lti_launch_request(self, post_data):
         return helpers.build_lti_launch_request(post_data)
 
+    @override_settings(DJANGO_AUTH_LTI_EXCLUDE_PATHS=['/skip_lti/'])
+    @patch('django_auth_lti.middleware_patched.auth')
+    def test_exclude_path(self, mock_auth, mock_logger):
+        """
+        Skip LTI session processing if `request.path` is excluded by project settings
+        """
+        request = RequestFactory().get('/skip_lti/')
+        self.mw.process_request(request)
+        self.assertIsNone(getattr(request, 'LTI', None))
+        self.assertEqual(mock_auth.login.call_count, 0)
+
+    @patch('django_auth_lti.middleware_patched.auth')
+    def test_exclude_blank_path(self, mock_auth, mock_logger):
+        """
+        Skip LTI session processing if `request.path` is blank, as is the case with some
+        local requests, like those initiated by the django-debug-toolbar library
+        """
+        mock_request = Mock(request=Mock(path=''))
+        mock_request_LTI_attribute_mock = PropertyMock()
+        type(mock_request).LTI = mock_request_LTI_attribute_mock
+        self.mw.process_request(mock_request)
+        self.assertEqual(mock_request.LTI.call_count, 0)
+        self.assertEqual(mock_auth.login.call_count, 0)
+
     @override_settings(LTI_AUTH_MAX_LAUNCHES=LTI_AUTH_MAX_LAUNCHES)
     @patch('django_auth_lti.middleware_patched.auth')
     @patch('django_auth_lti.middleware_patched.set_current_request')

tests/test_reverse.py

@@ -0,0 +1,55 @@
+from unittest import TestCase
+from unittest.mock import patch
+
+from django.test import override_settings, RequestFactory
+from django.urls import reverse
+
+from . import helpers
+from django_auth_lti.middleware_patched import MultiLTILaunchAuthMiddleware
+
+class TestReverse(TestCase):
+    def setUp(self):
+        self.mw = MultiLTILaunchAuthMiddleware()
+
+    def build_lti_launch_request(self, post_data, url):
+        return helpers.build_lti_launch_request(post_data, url)
+
+    @patch('django_auth_lti.middleware.logger')
+    @patch('django_auth_lti.middleware_patched.auth')
+    def test_patched_reverse_adds_resource_link_id(self, mock_auth, mock_logger):
+        """
+        `django.urls.reverse()` should add the `resource_link_id` from the LTI session to the URL
+        (`django.urls.reverse` should be patched automatically by importing MultiLTILaunchAuthMiddleware)
+        """
+        mock_auth.authenticate.return_value = True
+        request = self.build_lti_launch_request({"resource_link_id": 'abc123'}, url='/lti_launch/')
+        self.mw.process_request(request)
+        url = reverse('lti_launch')
+        self.assertEqual(url, '/lti_launch/?resource_link_id=abc123')
+
+    @override_settings(DJANGO_AUTH_LTI_EXCLUDE_PATHS=['/skip_lti/'])
+    @patch('django_auth_lti.middleware.logger')
+    @patch('django_auth_lti.middleware_patched.auth')
+    def test_patched_reverse_exclude_paths_settings(self, mock_auth, mock_logger):
+        """
+        `django.urls.reverse()` should not check the LTI session for the `resource_link_id` if the `request.path` is excluded by project settings
+        (`django.urls.reverse` should be patched automatically by importing MultiLTILaunchAuthMiddleware)
+        """
+        mock_auth.authenticate.return_value = True
+        request = RequestFactory().get('/skip_lti/')
+        self.mw.process_request(request)
+        url = reverse('lti_launch')
+        self.assertEqual(url, '/lti_launch/')
+
+    @patch('django_auth_lti.middleware.logger')
+    @patch('django_auth_lti.middleware_patched.auth')
+    def test_patched_reverse_exclude_resource_link_id_param(self, mock_auth, mock_logger):
+        """
+        `django.urls.reverse()` should not check the LTI session for the `resource_link_id` if the `exclude_resource_link_id` is set to `True` when called
+        (`django.urls.reverse` should be patched automatically by importing MultiLTILaunchAuthMiddleware)
+        """
+        mock_auth.authenticate.return_value = True
+        request = self.build_lti_launch_request({"resource_link_id": 'abc123'}, url='/lti_launch/')
+        self.mw.process_request(request)
+        url = reverse('lti_launch', exclude_resource_link_id=True)
+        self.assertEqual(url, '/lti_launch/')

tests/test_settings.py

@@ -14,6 +14,8 @@
     'django_auth_lti',
 ]
 
+ROOT_URLCONF = 'tests.urls'
+
 TEMPLATES = [
     {
         'BACKEND': 'django.template.backends.django.DjangoTemplates',