Merge pull request #8117 from mcgachey/mcgachey-lti-migrations
[LTI Provider] Refactoring and clean-up
This commit is contained in:
@@ -0,0 +1,37 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# pylint: disable=invalid-name, missing-docstring, unused-argument, unused-import, line-too-long
|
||||
from south.utils import datetime_utils as datetime
|
||||
from south.db import db
|
||||
from south.v2 import SchemaMigration
|
||||
from django.db import models
|
||||
|
||||
|
||||
class Migration(SchemaMigration):
|
||||
|
||||
def forwards(self, orm):
|
||||
# Adding model 'LtiConsumer'
|
||||
db.create_table('lti_provider_lticonsumer', (
|
||||
('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
|
||||
('consumer_name', self.gf('django.db.models.fields.CharField')(max_length=255)),
|
||||
('consumer_key', self.gf('django.db.models.fields.CharField')(unique=True, max_length=32, db_index=True)),
|
||||
('consumer_secret', self.gf('django.db.models.fields.CharField')(unique=True, max_length=32)),
|
||||
))
|
||||
db.send_create_signal('lti_provider', ['LtiConsumer'])
|
||||
|
||||
|
||||
def backwards(self, orm):
|
||||
# Deleting model 'LtiConsumer'
|
||||
db.delete_table('lti_provider_lticonsumer')
|
||||
|
||||
|
||||
models = {
|
||||
'lti_provider.lticonsumer': {
|
||||
'Meta': {'object_name': 'LtiConsumer'},
|
||||
'consumer_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32', 'db_index': 'True'}),
|
||||
'consumer_name': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
|
||||
'consumer_secret': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32'}),
|
||||
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'})
|
||||
}
|
||||
}
|
||||
|
||||
complete_apps = ['lti_provider']
|
||||
0
lms/djangoapps/lti_provider/migrations/__init__.py
Normal file
0
lms/djangoapps/lti_provider/migrations/__init__.py
Normal file
@@ -1,5 +1,12 @@
|
||||
"""
|
||||
Database models for the LTI provider feature.
|
||||
|
||||
This app uses migrations. If you make changes to this model, be sure to create
|
||||
an appropriate migration file and check it in at the same time as your model
|
||||
changes. To do that,
|
||||
|
||||
1. Go to the edx-platform dir
|
||||
2. ./manage.py lms schemamigration lti_provider --auto "description" --settings=devstack
|
||||
"""
|
||||
from django.db import models
|
||||
from django.dispatch import receiver
|
||||
@@ -13,8 +20,9 @@ class LtiConsumer(models.Model):
|
||||
specific settings, such as the OAuth key/secret pair and any LTI fields
|
||||
that must be persisted.
|
||||
"""
|
||||
key = models.CharField(max_length=32, unique=True, db_index=True)
|
||||
secret = models.CharField(max_length=32, unique=True)
|
||||
consumer_name = models.CharField(max_length=255)
|
||||
consumer_key = models.CharField(max_length=32, unique=True, db_index=True)
|
||||
consumer_secret = models.CharField(max_length=32, unique=True)
|
||||
|
||||
|
||||
@receiver(SCORE_CHANGED)
|
||||
|
||||
@@ -79,7 +79,7 @@ class SignatureValidator(RequestValidator):
|
||||
|
||||
:return: True if the key is valid, False if it is not.
|
||||
"""
|
||||
return LtiConsumer.objects.filter(key=client_key).count() == 1
|
||||
return LtiConsumer.objects.filter(consumer_key=client_key).count() == 1
|
||||
|
||||
def get_client_secret(self, client_key, request):
|
||||
"""
|
||||
@@ -90,7 +90,7 @@ class SignatureValidator(RequestValidator):
|
||||
present, or None if the key does not exist in the database.
|
||||
"""
|
||||
try:
|
||||
return LtiConsumer.objects.get(key=client_key).secret
|
||||
return LtiConsumer.objects.get(consumer_key=client_key).consumer_secret
|
||||
except ObjectDoesNotExist:
|
||||
return None
|
||||
|
||||
|
||||
@@ -78,7 +78,7 @@ class SignatureValidatorTest(TestCase):
|
||||
Verify that validate_client_key succeeds if the client key exists in the
|
||||
database
|
||||
"""
|
||||
LtiConsumer.objects.create(key='client_key', secret='client_secret')
|
||||
LtiConsumer.objects.create(consumer_key='client_key', consumer_secret='client_secret')
|
||||
self.assertTrue(SignatureValidator().validate_client_key('client_key', None))
|
||||
|
||||
def test_validate_missing_key(self):
|
||||
@@ -93,7 +93,7 @@ class SignatureValidatorTest(TestCase):
|
||||
Verify that get_client_secret returns the right value if the key is in
|
||||
the database
|
||||
"""
|
||||
LtiConsumer.objects.create(key='client_key', secret='client_secret')
|
||||
LtiConsumer.objects.create(consumer_key='client_key', consumer_secret='client_secret')
|
||||
secret = SignatureValidator().get_client_secret('client_key', None)
|
||||
self.assertEqual(secret, 'client_secret')
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ from mock import patch, MagicMock
|
||||
|
||||
from lti_provider import views
|
||||
from lti_provider.signature_validator import SignatureValidator
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
from student.tests.factories import UserFactory
|
||||
|
||||
|
||||
@@ -22,10 +23,11 @@ LTI_DEFAULT_PARAMS = {
|
||||
'oauth_nonce': u'OAuth Nonce',
|
||||
}
|
||||
|
||||
|
||||
COURSE_KEY = CourseKey.from_string('some/course/id')
|
||||
USAGE_KEY = UsageKey.from_string('i4x://some/course/problem/uuid').map_into_course(COURSE_KEY)
|
||||
COURSE_PARAMS = {
|
||||
'course_id': 'CourseID',
|
||||
'usage_id': 'UsageID'
|
||||
'course_key': COURSE_KEY,
|
||||
'usage_key': USAGE_KEY
|
||||
}
|
||||
|
||||
|
||||
@@ -72,7 +74,7 @@ class LtiLaunchTest(TestCase):
|
||||
Verifies that the LTI launch succeeds when passed a valid request.
|
||||
"""
|
||||
request = build_launch_request()
|
||||
views.lti_launch(request, COURSE_PARAMS['course_id'], COURSE_PARAMS['usage_id'])
|
||||
views.lti_launch(request, unicode(COURSE_KEY), unicode(USAGE_KEY))
|
||||
render.assert_called_with(request, ALL_PARAMS)
|
||||
|
||||
def launch_with_missing_parameter(self, missing_param):
|
||||
@@ -112,10 +114,10 @@ class LtiLaunchTest(TestCase):
|
||||
properly stored in the session
|
||||
"""
|
||||
request = build_launch_request()
|
||||
views.lti_launch(request, COURSE_PARAMS['course_id'], COURSE_PARAMS['usage_id'])
|
||||
views.lti_launch(request, unicode(COURSE_KEY), unicode(USAGE_KEY))
|
||||
session = request.session[views.LTI_SESSION_KEY]
|
||||
self.assertEqual(session['course_id'], 'CourseID', 'Course ID not set in the session')
|
||||
self.assertEqual(session['usage_id'], 'UsageID', 'Usage ID not set in the session')
|
||||
self.assertEqual(session['course_key'], COURSE_KEY, 'Course key not set in the session')
|
||||
self.assertEqual(session['usage_key'], USAGE_KEY, 'Usage key not set in the session')
|
||||
for key in views.REQUIRED_PARAMETERS:
|
||||
self.assertEqual(session[key], request.POST[key], key + ' not set in the session')
|
||||
|
||||
@@ -126,7 +128,7 @@ class LtiLaunchTest(TestCase):
|
||||
URL
|
||||
"""
|
||||
request = build_launch_request(False)
|
||||
response = views.lti_launch(request, None, None)
|
||||
response = views.lti_launch(request, unicode(COURSE_KEY), unicode(USAGE_KEY))
|
||||
self.assertEqual(response.status_code, 302)
|
||||
self.assertEqual(response['Location'], '/accounts/login?next=/lti_provider/lti_run')
|
||||
|
||||
@@ -139,6 +141,7 @@ class LtiLaunchTest(TestCase):
|
||||
request = build_launch_request()
|
||||
response = views.lti_launch(request, None, None)
|
||||
self.assertEqual(response.status_code, 403)
|
||||
self.assertEqual(response.status_code, 403)
|
||||
|
||||
|
||||
class LtiRunTest(TestCase):
|
||||
@@ -170,7 +173,7 @@ class LtiRunTest(TestCase):
|
||||
Verifies that the lti_run view returns a Forbidden status if the session
|
||||
is missing any of the required LTI parameters or course information.
|
||||
"""
|
||||
extra_keys = ['course_id', 'usage_id']
|
||||
extra_keys = ['course_key', 'usage_key']
|
||||
for key in views.REQUIRED_PARAMETERS + extra_keys:
|
||||
request = build_run_request()
|
||||
del request.session[views.LTI_SESSION_KEY][key]
|
||||
@@ -208,7 +211,6 @@ class RenderCoursewareTest(TestCase):
|
||||
self.module_mock = self.setup_patch('lti_provider.views.get_module_by_usage_id', (self.module_instance, None))
|
||||
self.access_mock = self.setup_patch('lti_provider.views.has_access', 'StaffAccess')
|
||||
self.course_mock = self.setup_patch('lti_provider.views.get_course_with_access', 'CourseWithAccess')
|
||||
self.key_mock = self.setup_patch('lti_provider.views.CourseKey.from_string', 'CourseKey')
|
||||
|
||||
def setup_patch(self, function_name, return_value):
|
||||
"""
|
||||
@@ -228,21 +230,13 @@ class RenderCoursewareTest(TestCase):
|
||||
response = views.render_courseware(request, ALL_PARAMS.copy())
|
||||
self.assertEqual(response, 'Rendered page')
|
||||
|
||||
def test_course_key(self):
|
||||
"""
|
||||
Verify that the correct course key is requested
|
||||
"""
|
||||
request = build_run_request()
|
||||
views.render_courseware(request, ALL_PARAMS.copy())
|
||||
self.key_mock.assert_called_with(ALL_PARAMS['course_id'])
|
||||
|
||||
def test_course_with_access(self):
|
||||
"""
|
||||
Verify that get_course_with_access is called with the right parameters
|
||||
"""
|
||||
request = build_run_request()
|
||||
views.render_courseware(request, ALL_PARAMS.copy())
|
||||
self.course_mock.assert_called_with(request.user, 'load', 'CourseKey')
|
||||
self.course_mock.assert_called_with(request.user, 'load', COURSE_KEY)
|
||||
|
||||
def test_has_access(self):
|
||||
"""
|
||||
@@ -258,7 +252,7 @@ class RenderCoursewareTest(TestCase):
|
||||
"""
|
||||
request = build_run_request()
|
||||
views.render_courseware(request, ALL_PARAMS.copy())
|
||||
self.module_mock.assert_called_with(request, ALL_PARAMS['course_id'], ALL_PARAMS['usage_id'])
|
||||
self.module_mock.assert_called_with(request, unicode(COURSE_KEY), unicode(USAGE_KEY))
|
||||
|
||||
def test_render(self):
|
||||
"""
|
||||
@@ -278,7 +272,7 @@ class RenderCoursewareTest(TestCase):
|
||||
'disable_footer': True,
|
||||
'disable_tabs': True,
|
||||
'staff_access': 'StaffAccess',
|
||||
'xqa_server': 'http://your_xqa_server.com',
|
||||
'xqa_server': 'http://example.com/xqa',
|
||||
}
|
||||
request = build_run_request()
|
||||
views.render_courseware(request, ALL_PARAMS.copy())
|
||||
|
||||
@@ -6,15 +6,21 @@ from django.conf import settings
|
||||
from django.contrib.auth.decorators import login_required
|
||||
from django.contrib.auth.views import redirect_to_login
|
||||
from django.core.urlresolvers import reverse
|
||||
from django.http import HttpResponseBadRequest, HttpResponseForbidden
|
||||
from django.http import HttpResponseBadRequest, HttpResponseForbidden, Http404
|
||||
from django.views.decorators.csrf import csrf_exempt
|
||||
import logging
|
||||
|
||||
from courseware.access import has_access
|
||||
from courseware.courses import get_course_with_access
|
||||
from courseware.module_render import get_module_by_usage_id
|
||||
from edxmako.shortcuts import render_to_response
|
||||
from lti_provider.signature_validator import SignatureValidator
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from lms_xblock.runtime import unquote_slashes
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
from opaque_keys import InvalidKeyError
|
||||
|
||||
log = logging.getLogger("edx.lti_provider")
|
||||
|
||||
|
||||
# LTI launch parameters that must be present for a successful launch
|
||||
REQUIRED_PARAMETERS = [
|
||||
@@ -64,8 +70,18 @@ def lti_launch(request, course_id, usage_id):
|
||||
# Store the course, and usage ID in the session to prevent privilege
|
||||
# escalation if a staff member in one course tries to access material in
|
||||
# another.
|
||||
params['course_id'] = course_id
|
||||
params['usage_id'] = usage_id
|
||||
try:
|
||||
course_key, usage_key = parse_course_and_usage_keys(course_id, usage_id)
|
||||
except InvalidKeyError:
|
||||
log.error(
|
||||
'Invalid course key %s or usage key %s from request %s',
|
||||
course_id,
|
||||
usage_id,
|
||||
request
|
||||
)
|
||||
raise Http404()
|
||||
params['course_key'] = course_key
|
||||
params['usage_key'] = usage_key
|
||||
request.session[LTI_SESSION_KEY] = params
|
||||
|
||||
if not request.user.is_authenticated():
|
||||
@@ -140,7 +156,7 @@ def restore_params_from_session(request):
|
||||
if LTI_SESSION_KEY not in request.session:
|
||||
return None
|
||||
session_params = request.session[LTI_SESSION_KEY]
|
||||
additional_params = ['course_id', 'usage_id']
|
||||
additional_params = ['course_key', 'usage_key']
|
||||
return get_required_parameters(session_params, additional_params)
|
||||
|
||||
|
||||
@@ -154,13 +170,16 @@ def render_courseware(request, lti_params):
|
||||
:return: an HttpResponse object that contains the template and necessary
|
||||
context to render the courseware.
|
||||
"""
|
||||
usage_id = lti_params['usage_id']
|
||||
course_id = lti_params['course_id']
|
||||
course_key = CourseKey.from_string(course_id)
|
||||
usage_key = lti_params['usage_key']
|
||||
course_key = lti_params['course_key']
|
||||
user = request.user
|
||||
course = get_course_with_access(user, 'load', course_key)
|
||||
staff = has_access(request.user, 'staff', course)
|
||||
instance, _ = get_module_by_usage_id(request, course_id, usage_id)
|
||||
instance, _dummy = get_module_by_usage_id(
|
||||
request,
|
||||
unicode(course_key),
|
||||
unicode(usage_key)
|
||||
)
|
||||
|
||||
fragment = instance.render('student_view', context=request.GET)
|
||||
|
||||
@@ -173,7 +192,19 @@ def render_courseware(request, lti_params):
|
||||
'disable_footer': True,
|
||||
'disable_tabs': True,
|
||||
'staff_access': staff,
|
||||
'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://your_xqa_server.com'),
|
||||
'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://example.com/xqa'),
|
||||
}
|
||||
|
||||
return render_to_response('courseware/courseware.html', context)
|
||||
|
||||
|
||||
def parse_course_and_usage_keys(course_id, usage_id):
|
||||
"""
|
||||
Convert course and usage ID strings into key objects. Return a tuple of
|
||||
(course_key, usage_key), or throw an InvalidKeyError if the translation
|
||||
fails.
|
||||
"""
|
||||
course_key = CourseKey.from_string(course_id)
|
||||
usage_id = unquote_slashes(usage_id)
|
||||
usage_key = UsageKey.from_string(usage_id).map_into_course(course_key)
|
||||
return course_key, usage_key
|
||||
|
||||
Reference in New Issue
Block a user