From a7f25f3c0d666d9da128d09d1e07668836e86ff8 Mon Sep 17 00:00:00 2001 From: Cesar Montilla Date: Thu, 22 Dec 2022 16:21:26 -0400 Subject: [PATCH] feat: Import required LTI 1.3 auth utilities feat: Add auth, models, urls and views module feat: Add base dependencies feat: Add pylti1p3 dependency feat: Upgrade dependencies feat: Include base test classes feat: Improve docstrings fix: Remove unused dependencies fix: Apply suggestions from code review Co-authored-by: Javier Torres <40271196+Jacatove@users.noreply.github.com> Co-authored-by: AlexJMP <62396424+alexjmpb@users.noreply.github.com> --- openedx_lti_tool_plugin/apps.py | 7 +- openedx_lti_tool_plugin/auth.py | 24 +++++++ openedx_lti_tool_plugin/models.py | 22 ++++++ openedx_lti_tool_plugin/settings/test.py | 2 + openedx_lti_tool_plugin/tests/test_apps.py | 7 +- openedx_lti_tool_plugin/tests/test_auth.py | 11 +++ openedx_lti_tool_plugin/tests/test_models.py | 12 ++++ openedx_lti_tool_plugin/tests/test_views.py | 36 ++++++++++ openedx_lti_tool_plugin/urls.py | 14 ++-- openedx_lti_tool_plugin/views.py | 55 ++++++++++++++ requirements/base.in | 3 +- requirements/base.txt | 32 ++++++++- requirements/constraints.txt | 3 + requirements/pip-tools.txt | 2 +- requirements/test.in | 1 - requirements/test.txt | 75 +++++++++++++++----- 16 files changed, 267 insertions(+), 39 deletions(-) create mode 100644 openedx_lti_tool_plugin/auth.py create mode 100644 openedx_lti_tool_plugin/models.py create mode 100644 openedx_lti_tool_plugin/tests/test_auth.py create mode 100644 openedx_lti_tool_plugin/tests/test_models.py create mode 100644 openedx_lti_tool_plugin/tests/test_views.py create mode 100644 openedx_lti_tool_plugin/views.py diff --git a/openedx_lti_tool_plugin/apps.py b/openedx_lti_tool_plugin/apps.py index e72bc0f..8bbc409 100644 --- a/openedx_lti_tool_plugin/apps.py +++ b/openedx_lti_tool_plugin/apps.py @@ -1,9 +1,4 @@ -""" -App configuration for `openedx_lti_tool_plugin`. - -For more information on this file, see: -https://docs.djangoproject.com/en/3.2/ref/applications -""" +"""App configuration for `openedx_lti_tool_plugin`.""" from django.apps import AppConfig diff --git a/openedx_lti_tool_plugin/auth.py b/openedx_lti_tool_plugin/auth.py new file mode 100644 index 0000000..c38160c --- /dev/null +++ b/openedx_lti_tool_plugin/auth.py @@ -0,0 +1,24 @@ +"""Authentication for `openedx_lti_tool_plugin`.""" +from django.contrib.auth.backends import ModelBackend + +from .models import LtiProfile # pylint: disable=unused-import + + +class LtiAuthenticationBackend(ModelBackend): + """Custom LTI 1.3 Django authentication backend. + + Returns a user platform if any LTI profile instance matches + with the requested LTI user identity claims (iss, aud, sub). + Returns None if no user profile is found. + """ + + # pylint: disable=arguments-renamed + def authenticate(self, request, iss=None, aud=None, sub=None, **kwargs): + """Authenticate using LTI launch claims corresponding to a LTIProfile instance. + + Args: + request: HTTP request object + iss (str, optional): LTI issuer claim. Defaults to None. + aud (str, optional): LTI audience claim. Defaults to None. + sub (str, optional): LTI subject claim. Defaults to None. + """ diff --git a/openedx_lti_tool_plugin/models.py b/openedx_lti_tool_plugin/models.py new file mode 100644 index 0000000..371f78e --- /dev/null +++ b/openedx_lti_tool_plugin/models.py @@ -0,0 +1,22 @@ +"""Models for `openedx_lti_tool_plugin`.""" +from django.contrib.auth import get_user_model +from django.db import models + + +class LtiProfileManager(models.Manager): + """LTI 1.3 profile model manager.""" + + +class LtiProfile(models.Model): + """LTI 1.3 profile for Open edX users. + + A unique representation of the LTI subject + that initiated an LTI launch. + """ + + objects = LtiProfileManager() + user = models.OneToOneField(get_user_model(), on_delete=models.CASCADE) + + def __str__(self): + """Get a string representation of this model instance.""" + return f'' diff --git a/openedx_lti_tool_plugin/settings/test.py b/openedx_lti_tool_plugin/settings/test.py index 3acdb3f..4bf9479 100644 --- a/openedx_lti_tool_plugin/settings/test.py +++ b/openedx_lti_tool_plugin/settings/test.py @@ -27,3 +27,5 @@ 'NAME': 'db.sqlite3', }, } + +DEFAULT_AUTO_FIELD = 'django.db.models.BigAutoField' diff --git a/openedx_lti_tool_plugin/tests/test_apps.py b/openedx_lti_tool_plugin/tests/test_apps.py index c2daf42..5b8fff1 100644 --- a/openedx_lti_tool_plugin/tests/test_apps.py +++ b/openedx_lti_tool_plugin/tests/test_apps.py @@ -1,9 +1,4 @@ -""" -Tests for the `openedx_lti_tool_plugin` apps module. - -For more information on this file, see: -https://docs.python.org/3/library/unittest.html -""" +"""Tests for the `openedx_lti_tool_plugin` apps module.""" from django.test import TestCase from jsonschema import validate diff --git a/openedx_lti_tool_plugin/tests/test_auth.py b/openedx_lti_tool_plugin/tests/test_auth.py new file mode 100644 index 0000000..ad79b38 --- /dev/null +++ b/openedx_lti_tool_plugin/tests/test_auth.py @@ -0,0 +1,11 @@ +"""Tests for the `openedx_lti_tool_plugin` auth module.""" +from django.test import TestCase + +from openedx_lti_tool_plugin.auth import LtiAuthenticationBackend # pylint: disable=unused-import + + +class TestLtiAuthenticationBackend(TestCase): + """Test LTI 1.3 profile authentication backend.""" + + def test_authenticate(self): + """Test authenticate method.""" diff --git a/openedx_lti_tool_plugin/tests/test_models.py b/openedx_lti_tool_plugin/tests/test_models.py new file mode 100644 index 0000000..68cdf29 --- /dev/null +++ b/openedx_lti_tool_plugin/tests/test_models.py @@ -0,0 +1,12 @@ +"""Tests for the `openedx_lti_tool_plugin` models module.""" +from django.test import TestCase + +from openedx_lti_tool_plugin.models import LtiProfile, LtiProfileManager # pylint: disable=unused-import + + +class TestLtiProfileManager(TestCase): + """Test LTI profile model manager.""" + + +class TestLtiProfile(TestCase): + """Test LTI 1.3 profile model.""" diff --git a/openedx_lti_tool_plugin/tests/test_views.py b/openedx_lti_tool_plugin/tests/test_views.py new file mode 100644 index 0000000..1d1238f --- /dev/null +++ b/openedx_lti_tool_plugin/tests/test_views.py @@ -0,0 +1,36 @@ +"""Tests for the `openedx_lti_tool_plugin` views module.""" +from django.test import TestCase + +from openedx_lti_tool_plugin.views import ( # pylint: disable=unused-import + LtiToolBaseView, + LtiToolLaunchView, + LtiToolLoginView, +) + + +class TestLtiToolBaseView(TestCase): + """Test base LTI 1.3 view.""" + + +class TestLtiToolLoginView(TestCase): + """Test LTI 1.3 third-party login view.""" + + def test_get(self): + """Test GET method.""" + + def test_post(self): + """Test POST method.""" + + +class TestLtiToolLaunchView(TestCase): + """Test LTI 1.3 platform tool launch view.""" + + def test_authenticate_and_login(self): + """Test LTI 1.3 launch user authentication and authorization.""" + + +class TestLtiToolJwksView(TestCase): + """Test LTI 1.3 JSON Web Key Sets view.""" + + def test_get(self): + """Test GET method.""" diff --git a/openedx_lti_tool_plugin/urls.py b/openedx_lti_tool_plugin/urls.py index b2c1fee..9d91c44 100644 --- a/openedx_lti_tool_plugin/urls.py +++ b/openedx_lti_tool_plugin/urls.py @@ -1,8 +1,10 @@ -""" -URL configuration for `openedx_lti_tool_plugin`. +"""URL configuration for `openedx_lti_tool_plugin`.""" +from django.urls import path -For more information on this file, see: -https://docs.djangoproject.com/en/3.2/topics/http/urls/ -""" +from openedx_lti_tool_plugin import views -urlpatterns = [] +urlpatterns = [ + path('1.3/login/', views.LtiToolLoginView.as_view(), name='lti1p3-login'), + path('1.3/launch/', views.LtiToolLaunchView.as_view(), name='lti1p3-launch'), + path('1.3/pub/jwks/', views.LtiToolJwksView.as_view(), name='lti1p3-pub-jwks'), +] diff --git a/openedx_lti_tool_plugin/views.py b/openedx_lti_tool_plugin/views.py new file mode 100644 index 0000000..9886dbe --- /dev/null +++ b/openedx_lti_tool_plugin/views.py @@ -0,0 +1,55 @@ +"""Views for `openedx_lti_tool_plugin`.""" +from django.contrib.auth import authenticate # pylint: disable=unused-import +from django.views.generic.base import TemplateResponseMixin, View +from pylti1p3.contrib.django import DjangoCacheDataStorage # pylint: disable=unused-import +from pylti1p3.contrib.django import DjangoDbToolConf # pylint: disable=unused-import +from pylti1p3.contrib.django import DjangoMessageLaunch # pylint: disable=unused-import +from pylti1p3.contrib.django import DjangoOIDCLogin # pylint: disable=unused-import +from pylti1p3.exception import LtiException # pylint: disable=unused-import +from pylti1p3.exception import OIDCException # pylint: disable=unused-import + + +class LtiToolBaseView(View): + """Base LTI view initializing common LTI tool attributes.""" + + def setup(self, request, *args, **kwargs): + """Initialize attributes shared by all LTI views.""" + + +class LtiToolLoginView(LtiToolBaseView): + """ + LTI 1.3 third-party login view. + + The LTI platform will start the OpenID Connect flow by redirecting the User + Agent (UA) to this view. The redirect may be a form POST or a GET. On + success the view should redirect the UA to the LTI platform's authentication + URL. + """ + + def get(self, request): + """Get request.""" + return self.post(request) + + def post(self, request): + """Initialize 3rd-party login requests to redirect.""" + + +class LtiToolLaunchView(TemplateResponseMixin, LtiToolBaseView): + """LTI 1.3 platform tool launch view. + + Returns a rendered view of a requested XBlock LTI launch, + unless authentication or authorization fails. + """ + + def _authenticate_and_login(self): + """Authenticate and authorize the user for this LTI message launch.""" + + +class LtiToolJwksView(LtiToolBaseView): + """LTI 1.3 JSON Web Key Sets view. + + Returns the LTI tool public key. + """ + + def get(self, request): + """Return the public JWKS.""" diff --git a/requirements/base.in b/requirements/base.in index 52d808e..fa1c11a 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -2,4 +2,5 @@ -c constraints.txt -Django # Web application framework +Django # Web application framework +pylti1p3 # LTI 1.3 Advantage Tool implementation diff --git a/requirements/base.txt b/requirements/base.txt index 99c652c..d6fdd79 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -4,13 +4,41 @@ # # pip-compile --output-file=requirements/base.txt requirements/base.in # -asgiref==3.5.2 +asgiref==3.6.0 # via django +certifi==2022.12.7 + # via requests +cffi==1.15.1 + # via cryptography +charset-normalizer==2.1.1 + # via requests +cryptography==38.0.4 + # via jwcrypto +deprecated==1.2.13 + # via jwcrypto django==3.2.16 # via # -c requirements/constraints.txt # -r requirements/base.in -pytz==2022.6 +idna==3.4 + # via requests +jwcrypto==1.4.2 + # via pylti1p3 +pycparser==2.21 + # via cffi +pyjwt==2.6.0 + # via pylti1p3 +pylti1p3==1.10.0 + # via + # -c requirements/constraints.txt + # -r requirements/base.in +pytz==2022.7 # via django +requests==2.28.1 + # via pylti1p3 sqlparse==0.4.3 # via django +urllib3==1.26.13 + # via requests +wrapt==1.14.1 + # via deprecated diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 2acb451..e26639b 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -22,3 +22,6 @@ tox<4.0.0 # pylint==2.15.0 includes changes that makes pylint-django to break: # https://github.com/PyCQA/pylint-django/issues/370 pylint<2.15.0 + +# use edx-platform version pylti1p3==1.10.0. +pylti1p3<1.11.0 diff --git a/requirements/pip-tools.txt b/requirements/pip-tools.txt index 739f3e4..3ef46a6 100644 --- a/requirements/pip-tools.txt +++ b/requirements/pip-tools.txt @@ -16,7 +16,7 @@ pip==22.3.1 # via # -r requirements/pip-tools.in # pip-tools -pip-tools==6.12.0 +pip-tools==6.12.1 # via -r requirements/pip-tools.in setuptools==59.8.0 # via diff --git a/requirements/test.in b/requirements/test.in index e05c485..6be3988 100644 --- a/requirements/test.in +++ b/requirements/test.in @@ -12,5 +12,4 @@ isort # to standardize order of imports tox # Virtualenv management for tests pytest-django # pytest extension for better Django support -codecov # Code coverage reporting jsonschema # JSON Schema data validation diff --git a/requirements/test.txt b/requirements/test.txt index c55ced0..5e5a632 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -4,24 +4,36 @@ # # pip-compile --output-file=requirements/test.txt requirements/test.in # -asgiref==3.5.2 +asgiref==3.6.0 # via # -r requirements/base.txt # django astroid==2.11.7 # via pylint -attrs==22.1.0 +attrs==22.2.0 # via # jsonschema # pytest certifi==2022.12.7 - # via requests + # via + # -r requirements/base.txt + # requests +cffi==1.15.1 + # via + # -r requirements/base.txt + # cryptography charset-normalizer==2.1.1 - # via requests -codecov==2.1.12 - # via -r requirements/test.in -coverage==6.5.0 - # via codecov + # via + # -r requirements/base.txt + # requests +cryptography==38.0.4 + # via + # -r requirements/base.txt + # jwcrypto +deprecated==1.2.13 + # via + # -r requirements/base.txt + # jwcrypto dill==0.3.6 # via pylint distlib==0.3.6 @@ -29,22 +41,30 @@ distlib==0.3.6 # via # -c requirements/constraints.txt # -r requirements/base.txt -exceptiongroup==1.0.4 +exceptiongroup==1.1.0 # via pytest filelock==3.8.2 # via # tox # virtualenv idna==3.4 - # via requests + # via + # -r requirements/base.txt + # requests +importlib-resources==5.10.1 + # via jsonschema iniconfig==1.1.1 # via pytest -isort==5.11.2 +isort==5.11.4 # via # -r requirements/test.in # pylint jsonschema==4.17.3 # via -r requirements/test.in +jwcrypto==1.4.2 + # via + # -r requirements/base.txt + # pylti1p3 lazy-object-proxy==1.8.0 # via astroid mccabe==0.7.0 @@ -53,6 +73,8 @@ packaging==22.0 # via # pytest # tox +pkgutil-resolve-name==1.3.10 + # via jsonschema platformdirs==2.6.0 # via # pylint @@ -65,8 +87,16 @@ py==1.11.0 # via tox pycodestyle==2.10.0 # via -r requirements/test.in +pycparser==2.21 + # via + # -r requirements/base.txt + # cffi pydocstyle==6.1.1 # via -r requirements/test.in +pyjwt==2.6.0 + # via + # -r requirements/base.txt + # pylti1p3 pylint==2.14.5 # via # -c requirements/constraints.txt @@ -77,18 +107,24 @@ pylint-django==2.5.3 # via -r requirements/test.in pylint-plugin-utils==0.7 # via pylint-django +pylti1p3==1.10.0 + # via + # -c requirements/constraints.txt + # -r requirements/base.txt pyrsistent==0.19.2 # via jsonschema pytest==7.2.0 # via pytest-django pytest-django==4.5.2 # via -r requirements/test.in -pytz==2022.6 +pytz==2022.7 # via # -r requirements/base.txt # django requests==2.28.1 - # via codecov + # via + # -r requirements/base.txt + # pylti1p3 six==1.16.0 # via tox snowballstemmer==2.2.0 @@ -104,7 +140,7 @@ tomli==2.0.1 # tox tomlkit==0.11.6 # via pylint -tox==3.27.1 +tox==3.28.0 # via # -c requirements/constraints.txt # -r requirements/test.in @@ -113,11 +149,18 @@ typing-extensions==4.4.0 # astroid # pylint urllib3==1.26.13 - # via requests + # via + # -r requirements/base.txt + # requests virtualenv==20.17.1 # via tox wrapt==1.14.1 - # via astroid + # via + # -r requirements/base.txt + # astroid + # deprecated +zipp==3.11.0 + # via importlib-resources # The following packages are considered to be unsafe in a requirements file: # setuptools