Add role parameter to LTI. BLD-583.
This commit is contained in:
@@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes,
|
||||
in roughly chronological order, most recent first. Add your entries at or near
|
||||
the top. Include a label indicating the component affected.
|
||||
|
||||
Blades: Add role parameter to LTI. BLD-583.
|
||||
|
||||
Blades: Bugfix "In Firefox YouTube video with start time plays from 00:00:00".
|
||||
BLD-708.
|
||||
|
||||
|
||||
42
cms/djangoapps/contentstore/tests/test_access.py
Normal file
42
cms/djangoapps/contentstore/tests/test_access.py
Normal file
@@ -0,0 +1,42 @@
|
||||
"""
|
||||
Tests access.py
|
||||
"""
|
||||
from django.test import TestCase
|
||||
from django.contrib.auth.models import User
|
||||
from xmodule.modulestore import Location
|
||||
|
||||
from student.roles import CourseInstructorRole, CourseStaffRole
|
||||
from student.tests.factories import AdminFactory
|
||||
from student.auth import add_users
|
||||
from contentstore.views.access import get_user_role
|
||||
|
||||
class RolesTest(TestCase):
|
||||
"""
|
||||
Tests for user roles.
|
||||
"""
|
||||
def setUp(self):
|
||||
""" Test case setup """
|
||||
self.global_admin = AdminFactory()
|
||||
self.instructor = User.objects.create_user('testinstructor', 'testinstructor+courses@edx.org', 'foo')
|
||||
self.staff = User.objects.create_user('teststaff', 'teststaff+courses@edx.org', 'foo')
|
||||
self.location = Location('i4x', 'mitX', '101', 'course', 'test')
|
||||
|
||||
def test_get_user_role_instructor(self):
|
||||
"""
|
||||
Verifies if user is instructor.
|
||||
"""
|
||||
add_users(self.global_admin, CourseInstructorRole(self.location), self.instructor)
|
||||
self.assertEqual(
|
||||
'instructor',
|
||||
get_user_role(self.instructor, self.location, self.location.course_id)
|
||||
)
|
||||
|
||||
def test_get_user_role_staff(self):
|
||||
"""
|
||||
Verifies if user is staff.
|
||||
"""
|
||||
add_users(self.global_admin, CourseStaffRole(self.location), self.staff)
|
||||
self.assertEqual(
|
||||
'staff',
|
||||
get_user_role(self.staff, self.location, self.location.course_id)
|
||||
)
|
||||
@@ -1,6 +1,6 @@
|
||||
from ..utils import get_course_location_for_item
|
||||
from xmodule.modulestore.locator import CourseLocator
|
||||
from student.roles import CourseStaffRole, GlobalStaff
|
||||
from student.roles import CourseStaffRole, GlobalStaff, CourseInstructorRole
|
||||
from student import auth
|
||||
|
||||
|
||||
@@ -20,3 +20,17 @@ def has_course_access(user, location, role=CourseStaffRole):
|
||||
# this can be expensive if location is not category=='course'
|
||||
location = get_course_location_for_item(location)
|
||||
return auth.has_access(user, role(location))
|
||||
|
||||
|
||||
def get_user_role(user, location, context):
|
||||
"""
|
||||
Return corresponding string if user has staff or instructor role in Studio.
|
||||
This will not return student role because its purpose for using in Studio.
|
||||
|
||||
:param location: a descriptor.location
|
||||
:param context: a course_id
|
||||
"""
|
||||
if auth.has_access(user, CourseInstructorRole(location, context)):
|
||||
return 'instructor'
|
||||
else:
|
||||
return 'staff'
|
||||
|
||||
@@ -26,6 +26,8 @@ from .session_kv_store import SessionKeyValueStore
|
||||
from .helpers import render_from_lms
|
||||
from ..utils import get_course_for_item
|
||||
|
||||
from contentstore.views.access import get_user_role
|
||||
|
||||
__all__ = ['preview_handler']
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
@@ -132,6 +134,7 @@ def _preview_module_system(request, descriptor):
|
||||
),
|
||||
),
|
||||
error_descriptor_class=ErrorDescriptor,
|
||||
get_user_role=lambda: get_user_role(request.user, descriptor.location, course_id),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -56,7 +56,6 @@ from pkg_resources import resource_string
|
||||
from xblock.core import String, Scope, List, XBlock
|
||||
from xblock.fields import Boolean, Float
|
||||
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
|
||||
@@ -328,6 +327,25 @@ class LTIModule(LTIFields, XModule):
|
||||
"""
|
||||
return u':'.join(urllib.quote(i) for i in (self.lti_id, self.get_resource_link_id(), self.get_user_id()))
|
||||
|
||||
def get_course(self):
|
||||
"""
|
||||
Return course by course id.
|
||||
"""
|
||||
course_location = CourseDescriptor.id_to_location(self.course_id)
|
||||
course = self.descriptor.runtime.modulestore.get_item(course_location)
|
||||
return course
|
||||
|
||||
@property
|
||||
def role(self):
|
||||
"""
|
||||
Get system user role and convert it to LTI role.
|
||||
"""
|
||||
roles = {
|
||||
'student': u'Student',
|
||||
'staff': u'Administrator',
|
||||
'instructor': u'Instructor',
|
||||
}
|
||||
return roles.get(self.system.get_user_role(), u'Student')
|
||||
|
||||
def oauth_params(self, custom_parameters, client_key, client_secret):
|
||||
"""
|
||||
@@ -351,7 +369,7 @@ class LTIModule(LTIFields, XModule):
|
||||
u'launch_presentation_return_url': '',
|
||||
u'lti_message_type': u'basic-lti-launch-request',
|
||||
u'lti_version': 'LTI-1p0',
|
||||
u'role': u'student',
|
||||
u'roles': self.role,
|
||||
|
||||
# Parameters required for grading:
|
||||
u'resource_link_id': self.get_resource_link_id(),
|
||||
@@ -607,10 +625,7 @@ oauth_consumer_key="", oauth_signature="frVp4JuvT1mVXlxktiAUjQ7%2F1cw%3D"'}
|
||||
"""
|
||||
Obtains client_key and client_secret credentials from current course.
|
||||
"""
|
||||
course_id = self.course_id
|
||||
course_location = CourseDescriptor.id_to_location(course_id)
|
||||
course = self.descriptor.runtime.modulestore.get_item(course_location)
|
||||
|
||||
course = self.get_course()
|
||||
for lti_passport in course.lti_passports:
|
||||
try:
|
||||
lti_id, key, secret = [i.strip() for i in lti_passport.split(':')]
|
||||
|
||||
@@ -77,6 +77,7 @@ def get_test_system(course_id=''):
|
||||
open_ended_grading_interface=open_ended_grading_interface,
|
||||
course_id=course_id,
|
||||
error_descriptor_class=ErrorDescriptor,
|
||||
get_user_role=Mock(is_staff=False),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -1029,7 +1029,7 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable=abs
|
||||
open_ended_grading_interface=None, s3_interface=None,
|
||||
cache=None, can_execute_unsafe_code=None, replace_course_urls=None,
|
||||
replace_jump_to_id_urls=None, error_descriptor_class=None, get_real_user=None,
|
||||
field_data=None,
|
||||
field_data=None, get_user_role=None,
|
||||
**kwargs):
|
||||
"""
|
||||
Create a closure around the system environment.
|
||||
@@ -1082,6 +1082,9 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable=abs
|
||||
get_real_user - function that takes `anonymous_student_id` and returns real user_id,
|
||||
associated with `anonymous_student_id`.
|
||||
|
||||
get_user_role - A function that returns user role. Implementation is different
|
||||
for LMS and Studio.
|
||||
|
||||
field_data - the `FieldData` to use for backing XBlock storage.
|
||||
"""
|
||||
|
||||
@@ -1119,6 +1122,8 @@ class ModuleSystem(ConfigurableFragmentWrapper, Runtime): # pylint: disable=abs
|
||||
|
||||
self.get_real_user = get_real_user
|
||||
|
||||
self.get_user_role = get_user_role
|
||||
|
||||
def get(self, attr):
|
||||
""" provide uniform access to attributes (like etree)."""
|
||||
return self.__dict__.get(attr)
|
||||
|
||||
@@ -466,3 +466,20 @@ def _has_staff_access_to_descriptor(user, descriptor, course_context=None):
|
||||
descriptor: something that has a location attribute
|
||||
"""
|
||||
return _has_staff_access_to_location(user, descriptor.location, course_context)
|
||||
|
||||
|
||||
def get_user_role(user, course_id):
|
||||
"""
|
||||
Return corresponding string if user has staff, instructor or student
|
||||
course role in LMS.
|
||||
"""
|
||||
from courseware.courses import get_course
|
||||
course = get_course(course_id)
|
||||
if is_masquerading_as_student(user):
|
||||
return 'student'
|
||||
elif has_access(user, course, 'instructor'):
|
||||
return 'instructor'
|
||||
elif has_access(user, course, 'staff'):
|
||||
return 'staff'
|
||||
else:
|
||||
return 'student'
|
||||
|
||||
@@ -56,6 +56,17 @@ Feature: LMS.LTI component
|
||||
And I see in the gradebook table that "Total" is "5"
|
||||
|
||||
#7
|
||||
Scenario: Graded LTI component in LMS role's masquerading correctly works
|
||||
Given the course has correct LTI credentials with registered Instructor
|
||||
And the course has an LTI component with correct fields:
|
||||
| open_in_a_new_page | has_score |
|
||||
| False | True |
|
||||
And I view the LTI and it is rendered in iframe
|
||||
And I see in iframe that LTI role is Instructor
|
||||
And I switch to Student view
|
||||
Then I see in iframe that LTI role is Student
|
||||
|
||||
#8
|
||||
Scenario: Graded LTI component in LMS is correctly works with beta testers
|
||||
Given the course has correct LTI credentials with registered BetaTester
|
||||
And the course has an LTI component with correct fields:
|
||||
@@ -65,5 +76,3 @@ Feature: LMS.LTI component
|
||||
And I click on the "Progress" tab
|
||||
Then I see text "Problem Scores: 5/10"
|
||||
And I see graph with total progress "5%"
|
||||
|
||||
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
#pylint: disable=C0111
|
||||
|
||||
import datetime
|
||||
import os
|
||||
import pytz
|
||||
from mock import patch
|
||||
from pytz import UTC
|
||||
from nose.tools import assert_equal
|
||||
from splinter.exceptions import ElementDoesNotExist
|
||||
|
||||
from django.contrib.auth.models import User
|
||||
from django.core.urlresolvers import reverse
|
||||
@@ -12,9 +14,13 @@ from lettuce.django import django_url
|
||||
|
||||
from common import course_id, visit_scenario_item
|
||||
from courseware.tests.factories import InstructorFactory, BetaTesterFactory
|
||||
|
||||
from courseware.access import has_access
|
||||
from student.tests.factories import UserFactory
|
||||
|
||||
from nose.tools import assert_equals
|
||||
from common import course_id, visit_scenario_item
|
||||
|
||||
|
||||
@step('I view the LTI and error is shown$')
|
||||
def lti_is_not_rendered(_step):
|
||||
@@ -66,6 +72,7 @@ def incorrect_lti_is_rendered(_step):
|
||||
assert world.is_css_present('iframe', wait_time=2)
|
||||
assert not world.is_css_present('.link_lti_new_window', wait_time=0)
|
||||
assert not world.is_css_present('.error_message', wait_time=0)
|
||||
|
||||
#inside iframe test content is presented
|
||||
check_lti_iframe_content("Wrong LTI signature")
|
||||
|
||||
@@ -79,6 +86,7 @@ def set_correct_lti_passport(_step, user='Instructor'):
|
||||
world.lti_server.oauth_settings['client_secret']
|
||||
)]
|
||||
}
|
||||
|
||||
i_am_registered_for_the_course(coursenum, metadata, user)
|
||||
|
||||
|
||||
@@ -91,8 +99,10 @@ def set_incorrect_lti_passport(_step):
|
||||
"incorrect_lti_secret_key"
|
||||
)]
|
||||
}
|
||||
|
||||
i_am_registered_for_the_course(coursenum, metadata)
|
||||
|
||||
|
||||
@step('the course has an LTI component with (.*) fields(?:\:)?$') #, new_page is(.*), is_graded is(.*)
|
||||
def add_correct_lti_to_course(_step, fields):
|
||||
category = 'lti'
|
||||
@@ -100,6 +110,7 @@ def add_correct_lti_to_course(_step, fields):
|
||||
'lti_id': 'correct_lti_id',
|
||||
'launch_url': world.lti_server.oauth_settings['lti_base'] + world.lti_server.oauth_settings['lti_endpoint'],
|
||||
}
|
||||
|
||||
if fields.strip() == 'incorrect_lti_id': # incorrect fields
|
||||
metadata.update({
|
||||
'lti_id': 'incorrect_lti_id'
|
||||
@@ -132,7 +143,6 @@ def add_correct_lti_to_course(_step, fields):
|
||||
|
||||
|
||||
def create_course_for_lti(course, metadata):
|
||||
|
||||
# First clear the modulestore so we don't try to recreate
|
||||
# the same course twice
|
||||
# This also ensures that the necessary templates are loaded
|
||||
@@ -202,10 +212,12 @@ def i_am_registered_for_the_course(coursenum, metadata, user='Instructor'):
|
||||
user = BetaTesterFactory(course=course_location)
|
||||
normal_student = UserFactory()
|
||||
instructor = InstructorFactory(course=course_location)
|
||||
|
||||
assert not has_access(normal_student, course_descriptor, 'load')
|
||||
assert has_access(user, course_descriptor, 'load')
|
||||
assert has_access(instructor, course_descriptor, 'load')
|
||||
else:
|
||||
metadata.update({'start': datetime.datetime(1970, 1, 1, tzinfo=UTC)})
|
||||
create_course_for_lti(coursenum, metadata)
|
||||
course_descriptor = world.scenario_dict['COURSE']
|
||||
course_location = world.scenario_dict['COURSE'].location
|
||||
@@ -214,6 +226,7 @@ def i_am_registered_for_the_course(coursenum, metadata, user='Instructor'):
|
||||
# Enroll the user in the course and log them in
|
||||
if has_access(user, course_descriptor, 'load'):
|
||||
world.enroll_user(user, course_id(coursenum))
|
||||
|
||||
world.log_in(username=user.username, password='test')
|
||||
|
||||
|
||||
@@ -229,11 +242,11 @@ def check_lti_popup():
|
||||
url = world.browser.url
|
||||
basename = os.path.basename(url)
|
||||
pathname = os.path.splitext(basename)[0]
|
||||
|
||||
if pathname == u'correct_lti_endpoint':
|
||||
break
|
||||
|
||||
result = world.css_find('.result').first.text
|
||||
|
||||
assert result == u'This is LTI tool. Success.'
|
||||
|
||||
world.browser.driver.close() # Close the pop-up window
|
||||
@@ -279,3 +292,26 @@ def click_grade(_step):
|
||||
iframe.find_by_name('submit-button').first.click()
|
||||
assert iframe.is_text_present('LTI consumer (edX) responded with XML content')
|
||||
|
||||
|
||||
@step('I see in iframe that LTI role is (.*)$')
|
||||
def check_role(_step, role):
|
||||
world.is_css_present('iframe')
|
||||
location = world.scenario_dict['LTI'].location.html_id()
|
||||
iframe_name = 'ltiFrame-' + location
|
||||
with world.browser.get_iframe(iframe_name) as iframe:
|
||||
expected_role = 'Role: ' + role
|
||||
role = world.retry_on_exception(
|
||||
lambda: iframe.find_by_tag('h5').first.value,
|
||||
max_attempts=5,
|
||||
ignored_exceptions=ElementDoesNotExist
|
||||
)
|
||||
assert_equal(expected_role, role)
|
||||
|
||||
|
||||
@step('I switch to (.*)$')
|
||||
def switch_view(_step, view):
|
||||
staff_status = world.css_find('#staffstatus').first
|
||||
if staff_status.text != view:
|
||||
world.css_click('#staffstatus')
|
||||
world.wait_for_ajax_complete()
|
||||
|
||||
|
||||
@@ -185,7 +185,7 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
|
||||
'''
|
||||
self._send_head(status_code)
|
||||
if getattr(self.server, 'grade_data', False): # lti can be graded
|
||||
url = "//{}:{}".format(self.server.server_host, self.server.server_port)
|
||||
url = "//%s:%s" % self.server.server_address
|
||||
response_str = textwrap.dedent("""
|
||||
<html>
|
||||
<head>
|
||||
@@ -196,13 +196,14 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
|
||||
<h2>Graded IFrame loaded</h2>
|
||||
<h3>Server response is:</h3>
|
||||
<h3 class="result">{}</h3>
|
||||
<h5>Role: {role}</h5>
|
||||
</div>
|
||||
<form action="{url}/grade" method="post">
|
||||
<input type="submit" name="submit-button" value="Submit">
|
||||
</form>
|
||||
</body>
|
||||
</html>
|
||||
""").format(message, url=url)
|
||||
""").format(message, role=self.post_dict['roles'], url=url)
|
||||
else: # lti can't be graded
|
||||
response_str = textwrap.dedent("""
|
||||
<html>
|
||||
@@ -214,6 +215,7 @@ class MockLTIRequestHandler(BaseHTTPRequestHandler):
|
||||
<h2>IFrame loaded</h2>
|
||||
<h3>Server response is:</h3>
|
||||
<h3 class="result">{}</h3>
|
||||
|
||||
</div>
|
||||
</body>
|
||||
</html>
|
||||
|
||||
@@ -57,7 +57,7 @@ class MockLTIServerTest(unittest.TestCase):
|
||||
#wrong number of params and no signature
|
||||
payload = {
|
||||
'user_id': 'default_user_id',
|
||||
'role': 'student',
|
||||
'roles': 'Student',
|
||||
'oauth_nonce': '',
|
||||
'oauth_timestamp': '',
|
||||
}
|
||||
@@ -73,7 +73,7 @@ class MockLTIServerTest(unittest.TestCase):
|
||||
"""
|
||||
payload = {
|
||||
'user_id': 'default_user_id',
|
||||
'role': 'student',
|
||||
'roles': 'Student',
|
||||
'oauth_nonce': '',
|
||||
'oauth_timestamp': '',
|
||||
'oauth_consumer_key': 'test_client_key',
|
||||
@@ -100,7 +100,7 @@ class MockLTIServerTest(unittest.TestCase):
|
||||
"""
|
||||
payload = {
|
||||
'user_id': 'default_user_id',
|
||||
'role': 'student',
|
||||
'roles': 'Student',
|
||||
'oauth_nonce': '',
|
||||
'oauth_timestamp': '',
|
||||
'oauth_consumer_key': 'test_client_key',
|
||||
@@ -126,7 +126,7 @@ class MockLTIServerTest(unittest.TestCase):
|
||||
|
||||
payload = {
|
||||
'user_id': 'default_user_id',
|
||||
'role': 'student',
|
||||
'roles': 'Student',
|
||||
'oauth_nonce': '',
|
||||
'oauth_timestamp': '',
|
||||
'oauth_consumer_key': 'test_client_key',
|
||||
|
||||
@@ -17,7 +17,7 @@ import django.utils
|
||||
from django.views.decorators.csrf import csrf_exempt
|
||||
|
||||
from capa.xqueue_interface import XQueueInterface
|
||||
from courseware.access import has_access
|
||||
from courseware.access import has_access, get_user_role
|
||||
from courseware.masquerade import setup_masquerade
|
||||
from courseware.model_data import FieldDataCache, DjangoKeyValueStore
|
||||
from lms.lib.xblock.field_data import LmsFieldData
|
||||
@@ -432,6 +432,7 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
|
||||
# directly as the runtime i18n service.
|
||||
'i18n': django.utils.translation,
|
||||
},
|
||||
get_user_role=lambda: get_user_role(user, course_id),
|
||||
)
|
||||
|
||||
# pass position specified in URL to module through ModuleSystem
|
||||
|
||||
@@ -102,3 +102,48 @@ class AccessTestCase(TestCase):
|
||||
def test__user_passed_as_none(self):
|
||||
"""Ensure has_access handles a user being passed as null"""
|
||||
access.has_access(None, 'global', 'staff', None)
|
||||
|
||||
class UserRoleTestCase(TestCase):
|
||||
"""
|
||||
Tests for user roles.
|
||||
"""
|
||||
def setUp(self):
|
||||
self.course = Location('i4x://edX/toy/course/2012_Fall')
|
||||
self.anonymous_user = AnonymousUserFactory()
|
||||
self.student = UserFactory()
|
||||
self.global_staff = UserFactory(is_staff=True)
|
||||
self.course_staff = StaffFactory(course=self.course)
|
||||
self.course_instructor = InstructorFactory(course=self.course)
|
||||
|
||||
def test_user_role_staff(self):
|
||||
"""Ensure that user role is student for staff masqueraded as student."""
|
||||
self.assertEqual(
|
||||
'staff',
|
||||
access.get_user_role(self.course_staff, self.course.course_id)
|
||||
)
|
||||
# Masquerade staff
|
||||
self.course_staff.masquerade_as_student = True
|
||||
self.assertEqual(
|
||||
'student',
|
||||
access.get_user_role(self.course_staff, self.course.course_id)
|
||||
)
|
||||
|
||||
def test_user_role_instructor(self):
|
||||
"""Ensure that user role is student for instructor masqueraded as student."""
|
||||
self.assertEqual(
|
||||
'instructor',
|
||||
access.get_user_role(self.course_instructor, self.course.course_id)
|
||||
)
|
||||
# Masquerade instructor
|
||||
self.course_instructor.masquerade_as_student = True
|
||||
self.assertEqual(
|
||||
'student',
|
||||
access.get_user_role(self.course_instructor, self.course.course_id)
|
||||
)
|
||||
|
||||
def test_user_role_anonymous(self):
|
||||
"""Ensure that user role is student for anonymous user."""
|
||||
self.assertEqual(
|
||||
'student',
|
||||
access.get_user_role(self.anonymous_user, self.course.course_id)
|
||||
)
|
||||
|
||||
@@ -43,7 +43,7 @@ class TestLTI(BaseTestXmodule):
|
||||
u'launch_presentation_return_url': '',
|
||||
u'lti_message_type': u'basic-lti-launch-request',
|
||||
u'lti_version': 'LTI-1p0',
|
||||
u'role': u'student',
|
||||
u'roles': u'Student',
|
||||
|
||||
u'resource_link_id': module_id,
|
||||
u'lis_result_sourcedid': sourcedId,
|
||||
|
||||
Reference in New Issue
Block a user