Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: allow optionally passing PII in course LTI tab (#26982)

Browse Source
This commit is contained in:
Kshitij Sobti
2021-07-09 18:43:42 +05:30
committed by GitHub
parent a409cccc75
commit 666a50d298
8 changed files with 183 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cms/djangoapps/contentstore/views/item.py
View File

@@ -20,7 +20,7 @@ from edx_proctoring.api import (
)
from edx_proctoring.exceptions import ProctoredExamNotFoundException
from help_tokens.core import HelpUrlExpert
from lti_consumer.models import CourseEditLTIFieldsEnabledFlag
from lti_consumer.models import CourseAllowPIISharingInLTIFlag
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import LibraryUsageLocator
from pytz import UTC
@@ -311,7 +311,7 @@ class StudioEditModuleRuntime:
if service_name == "settings":
return SettingsService()
if service_name == "lti-configuration":
return ConfigurationService(CourseEditLTIFieldsEnabledFlag)
return ConfigurationService(CourseAllowPIISharingInLTIFlag)
if service_name == "teams_configuration":
return TeamsConfigurationService()
if service_name == "library_tools":

39
openedx/core/djangoapps/discussions/serializers.py
View File

@@ -2,15 +2,16 @@
Serializers for Discussion views.
"""
from lti_consumer.api import get_lti_pii_sharing_state_for_course
from lti_consumer.models import LtiConfiguration
from opaque_keys.edx.keys import CourseKey
from rest_framework import serializers
from xmodule.modulestore.django import modulestore
from lms.djangoapps.discussion.rest_api.serializers import DiscussionSettingsSerializer
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.lib.courses import get_course_by_id
from xmodule.modulestore.django import modulestore
from .models import DEFAULT_PROVIDER_TYPE, AVAILABLE_PROVIDER_MAP, DiscussionsConfiguration, Features
from .models import AVAILABLE_PROVIDER_MAP, DEFAULT_PROVIDER_TYPE, DiscussionsConfiguration, Features
class LtiSerializer(serializers.ModelSerializer):
@@ -20,6 +21,8 @@ class LtiSerializer(serializers.ModelSerializer):
class Meta:
model = LtiConfiguration
fields = [
'pii_share_username',
'pii_share_email',
'lti_1p1_client_key',
'lti_1p1_client_secret',
'lti_1p1_launch_url',
@@ -38,14 +41,26 @@ class LtiSerializer(serializers.ModelSerializer):
}
return payload
def to_representation(self, instance):
representation = super().to_representation(instance)
if not self.context.get('pii_sharing_allowed'):
representation.pop('pii_share_username')
representation.pop('pii_share_email')
return representation
def update(self, instance: LtiConfiguration, validated_data: dict) -> LtiConfiguration:
"""
Create/update a model-backed instance
"""
instance = instance or LtiConfiguration()
instance.config_store = LtiConfiguration.CONFIG_ON_DB
pii_sharing_allowed = self.context.get('pii_sharing_allowed', False)
if validated_data:
for key, value in validated_data.items():
if key.startswith('pii_') and not pii_sharing_allowed:
raise serializers.ValidationError(
"Cannot enable sending PII data until PII sharing for LTI is enabled for the course."
)
if key in self.Meta.fields:
setattr(instance, key, value)
instance.save()
@@ -185,16 +200,18 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer):
"""
Serialize data into a dictionary, to be used as a response
"""
course_key = instance.context_key
payload = super().to_representation(instance)
lti_configuration_data = {}
supports_lti = instance.supports('lti')
if supports_lti:
lti_configuration = LtiSerializer(instance.lti_configuration)
lti_configuration = LtiSerializer(instance.lti_configuration, context={
'pii_sharing_allowed': get_lti_pii_sharing_state_for_course(course_key),
})
lti_configuration_data = lti_configuration.data
provider_type = instance.provider_type or DEFAULT_PROVIDER_TYPE
plugin_configuration = instance.plugin_configuration
if provider_type == 'legacy':
course_key = instance.context_key
course = get_course_by_id(course_key)
legacy_settings = LegacySettingsSerializer(
course,
@@ -224,12 +241,17 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer):
setattr(instance, key, value)
# _update_* helpers assume `enabled` and `provider_type`
# have already been set
instance = self._update_lti(instance, validated_data)
instance = self._update_lti(instance, validated_data, instance.context_key)
instance = self._update_plugin_configuration(instance, validated_data)
instance.save()
return instance
def _update_lti(self, instance: DiscussionsConfiguration, validated_data: dict) -> DiscussionsConfiguration:
def _update_lti(
self,
instance: DiscussionsConfiguration,
validated_data: dict,
course_key: CourseKey
) -> DiscussionsConfiguration:
"""
Update LtiConfiguration
"""
@@ -243,6 +265,9 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer):
lti_configuration,
data=lti_configuration_data,
partial=True,
context={
'pii_sharing_allowed': get_lti_pii_sharing_state_for_course(course_key),
}
)
if lti_serializer.is_valid(raise_exception=True):
lti_serializer.save()

55
openedx/core/djangoapps/discussions/tests/test_views.py
View File

@@ -4,11 +4,13 @@ Test app view logic
# pylint: disable=test-inherits-tests
from datetime import datetime, timedelta, timezone
import unittest
from contextlib import contextmanager
import ddt
from django.conf import settings
from django.core.exceptions import ValidationError
from django.urls import reverse
from lti_consumer.models import CourseAllowPIISharingInLTIFlag
from rest_framework import status
from rest_framework.test import APITestCase
@@ -182,6 +184,59 @@ class DataTest(AuthorizedApiTest):
response = self._get()
self._assert_defaults(response)
@contextmanager
def _pii_sharing_for_course(self, enabled):
instance = CourseAllowPIISharingInLTIFlag.objects.create(course_id=self.course.id, enabled=enabled)
yield
instance.delete()
@ddt.data(
{"pii_share_username": True},
{"pii_share_email": True},
{"pii_share_email": True, "pii_share_username": True},
)
def test_post_pii_fields_fail(self, pii_fields):
"""
If no record exists, defaults should be returned.
"""
data = self._setup_lti()
data['lti_configuration'].update(pii_fields)
response = self._post(data)
assert response.status_code == 400
@ddt.data(
{"pii_share_username": True},
{"pii_share_email": True},
{"pii_share_email": True, "pii_share_username": True},
)
def test_post_pii_fields(self, pii_fields):
"""
Only if PII sharing is enabled should a user be able to set pii fields.
"""
data = self._setup_lti()
data['lti_configuration'].update(pii_fields)
with self._pii_sharing_for_course(enabled=False):
response = self._post(data)
assert response.status_code == 400
with self._pii_sharing_for_course(enabled=True):
response = self._post(data)
assert response.status_code == 200
@ddt.data(
True, False
)
def test_get_pii_fields(self, pii_sharing):
"""
Only if PII is enabled should pii fields be returned.
"""
self._setup_lti()
with self._pii_sharing_for_course(enabled=pii_sharing):
response = self._get()
data = response.json()
# If pii_sharing is true, then the fields should be present, and absent otherwise
assert ("pii_share_email" in data["lti_configuration"]) == pii_sharing
assert ("pii_share_username" in data["lti_configuration"]) == pii_sharing
def test_post_everything(self):
"""
API should accept requests to update _all_ fields at once

39
openedx/features/lti_course_tab/tab.py
View File

@@ -8,6 +8,7 @@ from django.contrib.auth.models import AbstractBaseUser
from django.contrib.sites.shortcuts import get_current_site
from django.http import HttpRequest
from django.utils.translation import get_language, to_locale, ugettext_lazy
from lti_consumer.api import get_lti_pii_sharing_state_for_course
from lti_consumer.lti_1p1.contrib.django import lti_embed
from lti_consumer.models import LtiConfiguration
from opaque_keys.edx.keys import CourseKey
@@ -34,7 +35,43 @@ class LtiCourseLaunchMixin:
}
DEFAULT_ROLE = 'Student'
def _get_pii_lti_parameters(self, course: CourseBlock, request: HttpRequest) -> Dict[str, str]:
"""
Get LTI parameters that contain PII.
Args:
course (CourseBlock): CourseBlock object.
request (HttpRequest): Request object for view in which LTI will be embedded.
Returns:
Dictionary with LTI parameters containing PII.
"""
pii_sharing_allowed = get_lti_pii_sharing_state_for_course(course.id)
if not pii_sharing_allowed:
return {}
lti_config = self._get_lti_config(course)
# Currently only LTI 1.1 is supported by the tab
if lti_config.version != lti_config.LTI_1P1:
return {}
pii_config = {}
if lti_config.pii_share_username:
pii_config['person_sourcedid'] = request.user.username
if lti_config.pii_share_email:
pii_config['person_contact_email_primary'] = request.user.email
return pii_config
def _get_additional_lti_parameters(self, course: CourseBlock, request: HttpRequest) -> Dict[str, str]:
"""
Get additional misc LTI parameters.
Args:
course (CourseBlock): CourseBlock object.
request (HttpRequest): Request object for view in which LTI will be embedded.
Returns:
Dictionary with additional LTI parameters.
"""
lti_config = self._get_lti_config(course)
additional_config = lti_config.lti_config.get('additional_parameters', {})
return additional_config
@@ -98,6 +135,7 @@ class LtiCourseLaunchMixin:
context_title = self._get_context_title(course)
result_sourcedid = quote(self._get_result_sourcedid(context_id, resource_link_id, user_id))
additional_params = self._get_additional_lti_parameters(course, request)
pii_params = self._get_pii_lti_parameters(course, request)
locale = to_locale(get_language())
return lti_embed(
@@ -111,6 +149,7 @@ class LtiCourseLaunchMixin:
context_label=context_id,
result_sourcedid=result_sourcedid,
launch_presentation_locale=locale,
**pii_params,
**additional_params,
)

67
openedx/features/lti_course_tab/tests.py
View File

@@ -1,39 +1,76 @@
"""
Tests for LTI Course tabs.
"""
import itertools
from unittest.mock import Mock, patch
import ddt
from django.test import RequestFactory
from lti_consumer.models import CourseAllowPIISharingInLTIFlag, LtiConfiguration
from lms.djangoapps.courseware.tests.test_tabs import TabTestCase
from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration
from openedx.features.lti_course_tab.tab import DiscussionLtiCourseTab
@ddt.ddt
class DiscussionLtiCourseTabTestCase(TabTestCase):
"""Test cases for LTI Discussion Tab."""
def setUp(self):
super().setUp()
self.discussion_config = DiscussionsConfiguration.objects.create(context_key=self.course.id, enabled=False)
self.discussion_config.lti_configuration = LtiConfiguration.objects.create(
config_store=LtiConfiguration.CONFIG_ON_DB,
lti_1p1_launch_url='http://test.url',
lti_1p1_client_key='test_client_key',
lti_1p1_client_secret='test_client_secret',
)
self.discussion_config.save()
self.url = self.reverse('course_tab_view', args=[str(self.course.id), DiscussionLtiCourseTab.type])
def check_discussion_tab(self):
"""Helper function for verifying the LTI discussion tab."""
return self.check_tab(
tab_class=DiscussionLtiCourseTab,
dict_tab={'type': DiscussionLtiCourseTab.type, 'name': 'same'},
expected_link=self.reverse('course_tab_view', args=[str(self.course.id), DiscussionLtiCourseTab.type]),
expected_link=self.url,
expected_tab_id=DiscussionLtiCourseTab.type,
invalid_dict_tab=None,
)
@patch('openedx.features.lti_course_tab.tab.DiscussionsConfiguration.get')
@patch('common.djangoapps.student.models.CourseEnrollment.is_enrolled')
def test_discussion_lti_tab(self, is_enrolled, discussion_config_get):
is_enrolled.return_value = True
mock_config = Mock()
mock_config.lti_configuration = {}
mock_config.enabled = False
discussion_config_get.return_value = mock_config
@ddt.data(True, False)
@patch('common.djangoapps.student.models.CourseEnrollment.is_enrolled', Mock(return_value=True))
def test_pii_params_on_discussion_lti_tab(self, discussion_config_enabled):
self.discussion_config.enabled = discussion_config_enabled
self.discussion_config.save()
tab = self.check_discussion_tab()
self.check_can_display_results(
tab, for_staff_only=True, for_enrolled_users_only=True, expected_value=False
)
mock_config.enabled = True
self.check_discussion_tab()
self.check_can_display_results(
tab, for_staff_only=True, for_enrolled_users_only=True
tab,
for_staff_only=True,
for_enrolled_users_only=True,
expected_value=discussion_config_enabled,
)
@ddt.data(*itertools.product((True, False), repeat=3))
@ddt.unpack
def test_discussion_lti_tab_pii(self, enable_sending_pii, share_username, share_email):
CourseAllowPIISharingInLTIFlag.objects.create(course_id=self.course.id, enabled=enable_sending_pii)
self.discussion_config.lti_configuration.lti_config = {
"pii_share_username": share_username,
"pii_share_email": share_email,
}
self.discussion_config.lti_configuration.save()
tab = self.check_discussion_tab()
request = RequestFactory().get(self.url)
user = self.create_mock_user(is_enrolled=True)
request.user = user
embed_code = tab._get_lti_embed_code(self.course, request) # pylint: disable=protected-access
if enable_sending_pii and share_username:
assert user.username in embed_code
else:
assert user.username not in embed_code
if enable_sending_pii and share_email:
assert user.email in embed_code
else:
assert user.email not in embed_code

2
requirements/edx/base.txt
View File

@@ -596,7 +596,7 @@ libsass==0.10.0
# ora2
loremipsum==1.0.5
# via ora2
lti-consumer-xblock==2.11.0
lti-consumer-xblock==3.0.0
# via -r requirements/edx/base.in
lxml==4.5.0
# via

2
requirements/edx/development.txt
View File

@@ -776,7 +776,7 @@ loremipsum==1.0.5
# via
# -r requirements/edx/testing.txt
# ora2
lti-consumer-xblock==2.11.0
lti-consumer-xblock==3.0.0
# via -r requirements/edx/testing.txt
lxml==4.5.0
# via

2
requirements/edx/testing.txt
View File

@@ -741,7 +741,7 @@ loremipsum==1.0.5
# via
# -r requirements/edx/base.txt
# ora2
lti-consumer-xblock==2.11.0
lti-consumer-xblock==3.0.0
# via -r requirements/edx/base.txt
lxml==4.5.0
# via