Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BD-24] [TNL-7318] BB-2355: Add LTI support and Django authentication extension. #105

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lti_consumer/lti_1p3/consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ def access_token(self, token_request_data):
"access_token": self.key_handler.encode_and_sign(
{
"sub": self.client_id,
"iss": self.iss,
giovannicimolin marked this conversation as resolved.
Show resolved Hide resolved
"scopes": scopes_str
},
# Create token valid for 3600 seconds (1h) as per specification
Expand Down
4 changes: 4 additions & 0 deletions lti_consumer/lti_1p3/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ class TokenSignatureExpired(Lti1p3Exception):
pass


class UnauthorizedToken(Lti1p3Exception):
pass


class NoSuitableKeys(Lti1p3Exception):
pass

Expand Down
Empty file.
Empty file.
79 changes: 79 additions & 0 deletions lti_consumer/lti_1p3/extensions/rest_framework/authentication.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
"""
Django REST Framework extensions for LTI 1.3 & LTI Advantage implementation.

Implements a custom authentication class to be used by LTI Advantage extensions.
"""
from django.utils.translation import ugettext as _
from rest_framework import authentication
from rest_framework import exceptions

from lti_consumer.models import LtiConfiguration


class Lti1p3ApiAuthentication(authentication.BaseAuthentication):
"""
LTI 1.3 Token based authentication.

Clients should authenticate by passing the token key in the "Authorization".
LTI 1.3 expects a token like the following:
Authorization: Bearer jwt-token

Since the base implementation of this library uses JWT tokens, we expect
a RSA256 signed token that contains the allowed scopes.
"""
keyword = 'Bearer'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
keyword = 'Bearer'
KEYWORD = 'Bearer'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've based this class on https://github.com/encode/django-rest-framework/blob/master/rest_framework/authentication.py#L148 which uses a lowercase variable. I don't see any issue changing it to uppercase though.

Do you want me to change it?


def authenticate(self, request):
"""
Authenticate an LTI 1.3 Tool.

This doesn't return a user, but let's the external access and commit
changes.
TODO: Consider creating an user for LTI operations, both to keep track
of changes and to use Django's authorization flow.
"""
auth = request.headers.get('Authorization', '').split()
lti_config_id = request.parser_context['kwargs'].get('lti_config_id')

# Check if auth token is present on request and is correctly formatted.
if not auth or auth[0].lower() != self.keyword.lower():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if not auth or auth[0].lower() != self.keyword.lower():
if not auth or auth[0].lower() != self.KEYWORD.lower():

msg = _('Missing LTI 1.3 authentication token.')
raise exceptions.AuthenticationFailed(msg)

if len(auth) == 1:
msg = _('Invalid token header. No credentials provided.')
raise exceptions.AuthenticationFailed(msg)

if len(auth) > 2:
msg = _('Invalid token header. Token string should not contain spaces.')
raise exceptions.AuthenticationFailed(msg)

# Retrieve LTI configuration or fail if it doesn't exist
try:
lti_configuration = LtiConfiguration.objects.get(pk=lti_config_id)
lti_consumer = lti_configuration.get_lti_consumer()
except:
giovannicimolin marked this conversation as resolved.
Show resolved Hide resolved
giovannicimolin marked this conversation as resolved.
Show resolved Hide resolved
msg = _('LTI configuration not found.')
raise exceptions.AuthenticationFailed(msg)

# Verify token validity
# This doesn't validate specific permissions, just checks if the token
# is valid or not.
try:
lti_consumer.check_token(auth[1])
except Exception:
msg = _('Invalid token signature.')
raise exceptions.AuthenticationFailed(msg)

# Passing parameters back to the view through the request in order
# to avoid implementing a separate authentication backend or
# keeping track of LTI "sessions" through a custom model.
# With the LTI Configuration and consumer attached to the request
# the views and permissions classes can make use of the
# current LTI context to retrieve settings and decode the token passed.
request.lti_configuration = lti_configuration
request.lti_consumer = lti_consumer

# Return (None, None) since this isn't tied to any authentication
# backend on Django, and it's just used for LTI endpoints.
return (None, None)
giovannicimolin marked this conversation as resolved.
Show resolved Hide resolved
Empty file.
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
"""
Unit tests for LTI 1.3 consumer implementation
"""
from __future__ import absolute_import, unicode_literals

import ddt
from mock import MagicMock, patch

from Cryptodome.PublicKey import RSA
from django.test.testcases import TestCase
from rest_framework import exceptions

from lti_consumer.models import LtiConfiguration
from lti_consumer.lti_1p3.consumer import LtiConsumer1p3
from lti_consumer.lti_1p3.extensions.rest_framework.authentication import Lti1p3ApiAuthentication


# Variables required for testing and verification
ISS = "http://test-platform.example/"
OIDC_URL = "http://test-platform/oidc"
LAUNCH_URL = "http://test-platform/launch"
CLIENT_ID = "1"
DEPLOYMENT_ID = "1"
NONCE = "1234"
STATE = "ABCD"
# Consider storing a fixed key
RSA_KEY_ID = "1"
RSA_KEY = RSA.generate(2048).export_key('PEM')


@ddt.ddt
class TestLtiAuthentication(TestCase):
"""
Unit tests for Lti1p3ApiAuthentication class
"""
def setUp(self):
super(TestLtiAuthentication, self).setUp()

# Set up consumer
self.lti_consumer = LtiConsumer1p3(
iss=ISS,
lti_oidc_url=OIDC_URL,
lti_launch_url=LAUNCH_URL,
client_id=CLIENT_ID,
deployment_id=DEPLOYMENT_ID,
rsa_key=RSA_KEY,
rsa_key_id=RSA_KEY_ID,
# Use the same key for testing purposes
tool_key=RSA_KEY,
)

# Create LTI Configuration
self.lti_configuration = LtiConfiguration.objects.create(
version=LtiConfiguration.LTI_1P3,
)

# Patch call that retrieves config from modulestore
# We're not testing the model here
self._lti_block_patch = patch(
'lti_consumer.models.LtiConfiguration.get_lti_consumer',
return_value=self.lti_consumer,
)
self.addCleanup(self._lti_block_patch.stop)
self._lti_block_patch.start()

def _make_request(self):
"""
Returns a Mock Request that can be used to test the LTI auth.
"""
mock_request = MagicMock()

# Generate a valid access token
token = self.lti_consumer.key_handler.encode_and_sign(
{
"sub": self.lti_consumer.client_id,
"iss": self.lti_consumer.iss,
"scopes": "",
},
expiration=3600
)
mock_request.headers = {
"Authorization": "Bearer {}".format(token),
}

# Set the lti config id in the "url"
mock_request.parser_context = {"kwargs": {
"lti_config_id": self.lti_configuration.id,
}}

return mock_request

@ddt.data(
None,
"",
"Bearer",
"Bearer invalid token",
# Valid token format, but cannot be decoded
"Bearer invalid",
)
def test_invalid_auth_token(self, token):
"""
Test invalid and auth token in auth mechanism.
"""
mock_request = self._make_request()

# Either set invalid token or clear headers
if token is not None:
mock_request.headers = {
"Authorization": token,
}
else:
mock_request.headers = {}

with self.assertRaises(exceptions.AuthenticationFailed):
auth = Lti1p3ApiAuthentication()
auth.authenticate(mock_request)

def test_no_lti_config(self):
"""
Test that the login is invalid is LTI config doesn't exist.
giovannicimolin marked this conversation as resolved.
Show resolved Hide resolved
"""
mock_request = self._make_request()
mock_request.parser_context = {"kwargs": {
"lti_config_id": 0, # Django id field is never zero
}}

with self.assertRaises(exceptions.AuthenticationFailed):
auth = Lti1p3ApiAuthentication()
auth.authenticate(mock_request)

def test_lti_login_succeeds(self):
"""
Test if login successful and that the LTI Consumer and token
are attached to request.
"""
mock_request = self._make_request()

# Run auth
auth = Lti1p3ApiAuthentication()
auth.authenticate(mock_request)

# Check request
self.assertEqual(mock_request.lti_consumer, self.lti_consumer)
14 changes: 12 additions & 2 deletions lti_consumer/plugin/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,21 @@
from __future__ import absolute_import

from django.conf import settings
from django.conf.urls import url
from django.conf.urls import url, include

from .views import (
from rest_framework import routers

from lti_consumer.plugin.views import (
public_keyset_endpoint,
launch_gate_endpoint,
access_token_endpoint
)


# LTI 1.3 APIs router
router = routers.SimpleRouter(trailing_slash=False)


urlpatterns = [
url(
'lti_consumer/v1/public_keysets/{}$'.format(settings.USAGE_ID_PATTERN),
Expand All @@ -29,5 +35,9 @@
'lti_consumer/v1/token/{}$'.format(settings.USAGE_ID_PATTERN),
access_token_endpoint,
name='lti_consumer.access_token'
),
url(
r'lti_consumer/v1/lti/(?P<lti_config_id>[-\w]+)/',
include(router.urls)
)
]
3 changes: 3 additions & 0 deletions requirements/constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,6 @@ zipp<1.2.0

# Newer versions not available in python 3.5
stevedore<=1.32.0

# Same as in edx-platform
djangorestframework==3.9.4
1 change: 1 addition & 0 deletions requirements/test.in
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ mock
django-pyfs
edx_lint
pycodestyle
djangorestframework
xblock-sdk
8 changes: 4 additions & 4 deletions requirements/test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,17 @@
appdirs==1.4.4 # via -r requirements/base.txt, fs
astroid==2.3.3 # via pylint, pylint-celery
bleach==3.1.5 # via -r requirements/base.txt
boto3==1.14.53 # via fs-s3fs
botocore==1.17.53 # via boto3, s3transfer
boto3==1.14.62 # via fs-s3fs
botocore==1.17.62 # via boto3, s3transfer
certifi==2020.6.20 # via -r requirements/base.txt, requests
chardet==3.0.4 # via -r requirements/base.txt, requests
click-log==0.3.2 # via edx-lint
click==7.1.2 # via click-log, edx-lint
coverage==5.2.1 # via coveralls
coverage==5.3 # via coveralls
coveralls==2.1.2 # via -r requirements/test.in
ddt==1.4.1 # via -r requirements/test.in
django-pyfs==2.2 # via -r requirements/test.in
djangorestframework==3.9.4 # via -c requirements/constraints.txt, -r requirements/test.in
docopt==0.6.2 # via coveralls
docutils==0.15.2 # via botocore
edx-lint==1.5.2 # via -r requirements/test.in
Expand Down Expand Up @@ -56,7 +57,6 @@ six==1.15.0 # via -r requirements/base.txt, astroid, bleach, edx-l
sqlparse==0.3.1 # via -r requirements/base.txt, django
stevedore==1.32.0 # via -c requirements/constraints.txt, -r requirements/base.txt, edx-opaque-keys
typed-ast==1.4.1 # via astroid
typing==3.7.4.3 # via -r requirements/base.txt, fs
urllib3==1.25.10 # via -r requirements/base.txt, botocore, requests
web-fragments==0.3.2 # via -r requirements/base.txt, xblock, xblock-utils
webencodings==0.5.1 # via -r requirements/base.txt, bleach
Expand Down
8 changes: 4 additions & 4 deletions requirements/travis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,19 @@
appdirs==1.4.4 # via -r requirements/test.txt, -r requirements/tox.txt, fs, virtualenv
astroid==2.3.3 # via -r requirements/test.txt, pylint, pylint-celery
bleach==3.1.5 # via -r requirements/test.txt
boto3==1.14.53 # via -r requirements/test.txt, fs-s3fs
botocore==1.17.53 # via -r requirements/test.txt, boto3, s3transfer
boto3==1.14.62 # via -r requirements/test.txt, fs-s3fs
botocore==1.17.62 # via -r requirements/test.txt, boto3, s3transfer
certifi==2020.6.20 # via -r requirements/test.txt, requests
chardet==3.0.4 # via -r requirements/test.txt, requests
click-log==0.3.2 # via -r requirements/test.txt, edx-lint
click==7.1.2 # via -r requirements/test.txt, click-log, edx-lint
coverage==5.2.1 # via -r requirements/test.txt, coveralls
coverage==5.3 # via -r requirements/test.txt, coveralls
coveralls==2.1.2 # via -r requirements/test.txt
ddt==1.4.1 # via -r requirements/test.txt
distlib==0.3.1 # via -r requirements/tox.txt, virtualenv
django-pyfs==2.2 # via -r requirements/test.txt
django==2.2.16 # via -c requirements/constraints.txt, -r requirements/test.txt, django-pyfs, edx-opaque-keys, xblock-sdk
djangorestframework==3.9.4 # via -c requirements/constraints.txt, -r requirements/test.txt
docopt==0.6.2 # via -r requirements/test.txt, coveralls
docutils==0.15.2 # via -r requirements/test.txt, botocore
edx-lint==1.5.2 # via -r requirements/test.txt
Expand Down Expand Up @@ -65,7 +66,6 @@ stevedore==1.32.0 # via -c requirements/constraints.txt, -r requirements
toml==0.10.1 # via -r requirements/tox.txt, tox
tox==3.20.0 # via -r requirements/tox.txt
typed-ast==1.4.1 # via -r requirements/test.txt, astroid
typing==3.7.4.3 # via -r requirements/test.txt, fs
urllib3==1.25.10 # via -r requirements/test.txt, botocore, requests
virtualenv==20.0.31 # via -r requirements/tox.txt, tox
web-fragments==0.3.2 # via -r requirements/test.txt, xblock, xblock-utils
Expand Down