From 11ea567d4dd6a9533103554f1e3d7bbf7bcfae25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar=20Montilla?= Date: Wed, 22 May 2024 03:45:57 +0000 Subject: [PATCH] feat: Add CourseAccessConfiguration filter to DeepLinkingForm --- openedx_lti_tool_plugin/deep_linking/forms.py | 85 ++++++--- .../deep_linking/tests/test_forms.py | 166 ++++++++++++++---- .../deep_linking/tests/test_views.py | 77 ++++++-- openedx_lti_tool_plugin/deep_linking/views.py | 36 +++- .../resource_link_launch/views.py | 4 + openedx_lti_tool_plugin/tests/test_utils.py | 69 +++++++- openedx_lti_tool_plugin/utils.py | 29 ++- 7 files changed, 386 insertions(+), 80 deletions(-) diff --git a/openedx_lti_tool_plugin/deep_linking/forms.py b/openedx_lti_tool_plugin/deep_linking/forms.py index 14a2c05..02a0ae5 100644 --- a/openedx_lti_tool_plugin/deep_linking/forms.py +++ b/openedx_lti_tool_plugin/deep_linking/forms.py @@ -1,59 +1,71 @@ """Django Forms.""" -from typing import List, Optional, Set, Tuple +import json +from typing import Optional, Set, Tuple from django import forms from django.http.request import HttpRequest from django.urls import reverse from django.utils.translation import gettext as _ +from pylti1p3.contrib.django.lti1p3_tool_config.models import LtiTool from pylti1p3.deep_link_resource import DeepLinkResource from openedx_lti_tool_plugin.apps import OpenEdxLtiToolPluginConfig as app_config +from openedx_lti_tool_plugin.deep_linking.exceptions import DeepLinkingException from openedx_lti_tool_plugin.edxapp_wrapper.learning_sequences import course_context +from openedx_lti_tool_plugin.models import CourseAccessConfiguration +from openedx_lti_tool_plugin.waffle import COURSE_ACCESS_CONFIGURATION class DeepLinkingForm(forms.Form): """Deep Linking Form.""" - def __init__(self, *args: tuple, request=None, **kwargs: dict): + content_items = forms.MultipleChoiceField( + required=False, + widget=forms.CheckboxSelectMultiple, + label=_('Courses'), + ) + + def __init__( + self, + *args: tuple, + request: HttpRequest, + lti_tool: LtiTool, + **kwargs: dict, + ): """Class __init__ method. - Initialize class instance attributes and add `content_items` field. + Initialize class instance attributes and add field choices + to the `content_items` field. Args: *args: Variable length argument list. request: HttpRequest object. + lti_tool: LtiTool model instance. **kwargs: Arbitrary keyword arguments. """ super().__init__(*args, **kwargs) - self.fields['content_items'] = forms.MultipleChoiceField( - choices=self.get_content_items_choices(request), - required=False, - widget=forms.CheckboxSelectMultiple, - label=_('Courses'), - ) + self.request = request + self.lti_tool = lti_tool + self.fields['content_items'].choices = self.get_content_items_choices() - def get_content_items_choices(self, request: HttpRequest) -> List[Tuple[str, str]]: + def get_content_items_choices(self) -> Set[Optional[Tuple[str, str]]]: """Get `content_items` field choices. - Args: - request: HttpRequest object. - Returns: - List of tuples with choices for the `content_items` field. + Set of tuples with choices for the `content_items` field or an empty set. """ - return [ - self.get_content_items_choice(course, request) - for course in course_context().objects.all() - ] + return { + self.get_content_items_choice(course) + for course in self.get_course_contexts() + } - def get_content_items_choice(self, course, request: HttpRequest) -> Tuple[str, str]: + def get_content_items_choice(self, course) -> Tuple[str, str]: """Get `content_items` field choice. Args: course (CourseContext): Course object. - request: HttpRequest object. Returns: Tuple containing the choice value and name. @@ -68,10 +80,41 @@ def get_content_items_choice(self, course, request: HttpRequest) -> Tuple[str, s ) return ( - request.build_absolute_uri(relative_url), + self.request.build_absolute_uri(relative_url), course.learning_context.title, ) + def get_course_contexts(self): + """Get CourseContext objects. + + Returns:self.cleaned_data + All CourseContext objects if COURSE_ACCESS_CONFIGURATION switch + is disabled or all CourseContext objects matching the IDs in + the CourseAccessConfiguration `allowed_course_ids` field. + + Raises: + CourseAccessConfiguration.DoesNotExist: If CourseAccessConfiguration + does not exist for this form `lti_tool` attribute. + + """ + if not COURSE_ACCESS_CONFIGURATION.is_enabled(): + return course_context().objects.all() + + try: + course_access_config = CourseAccessConfiguration.objects.get( + lti_tool=self.lti_tool, + ) + except CourseAccessConfiguration.DoesNotExist as exc: + raise DeepLinkingException( + _(f'Course access configuration not found: {self.lti_tool.title}.'), + ) from exc + + return course_context().objects.filter( + learning_context__context_key__in=json.loads( + course_access_config.allowed_course_ids, + ), + ) + def get_deep_link_resources(self) -> Set[Optional[DeepLinkResource]]: """Get DeepLinkResource objects from this form `cleaned_data` attribute. diff --git a/openedx_lti_tool_plugin/deep_linking/tests/test_forms.py b/openedx_lti_tool_plugin/deep_linking/tests/test_forms.py index 2e512ba..e50d30c 100644 --- a/openedx_lti_tool_plugin/deep_linking/tests/test_forms.py +++ b/openedx_lti_tool_plugin/deep_linking/tests/test_forms.py @@ -1,80 +1,87 @@ """Tests forms module.""" from unittest.mock import MagicMock, patch -from django import forms from django.test import TestCase from openedx_lti_tool_plugin.apps import OpenEdxLtiToolPluginConfig as app_config +from openedx_lti_tool_plugin.deep_linking.exceptions import DeepLinkingException from openedx_lti_tool_plugin.deep_linking.forms import DeepLinkingForm from openedx_lti_tool_plugin.deep_linking.tests import MODULE_PATH +from openedx_lti_tool_plugin.models import CourseAccessConfiguration MODULE_PATH = f'{MODULE_PATH}.forms' -class TestDeepLinkingForm(TestCase): - """Test DeepLinkingForm class.""" +class DeepLinkingFormBaseTestCase(TestCase): + """DeepLinkingForm TestCase.""" def setUp(self): """Set up test fixtures.""" super().setUp() self.form_class = DeepLinkingForm self.request = MagicMock() + self.lti_tool = MagicMock() + self.form_kwargs = {'request': self.request, 'lti_tool': self.lti_tool} self.learning_context = MagicMock(context_key='random-course-key', title='Test') self.course = MagicMock(learning_context=self.learning_context) - @patch(f'{MODULE_PATH}.forms.MultipleChoiceField') - @patch(f'{MODULE_PATH}._') - @patch.object(DeepLinkingForm, 'get_content_items_choices') + +@patch.object(DeepLinkingForm, 'get_content_items_choices', return_value=[]) +class TestDeepLinkingFormInit(DeepLinkingFormBaseTestCase): + """Test DeepLinkingForm `__init__` method.""" + def test_init( self, get_content_items_choices_mock: MagicMock, - gettext_mock: MagicMock, - multiple_choice_field_mock: MagicMock, ): """Test `__init__` method.""" + form = self.form_class(request=self.request, lti_tool=self.lti_tool) + + self.assertEqual(form.request, self.request) + self.assertEqual(form.lti_tool, self.lti_tool) self.assertEqual( - self.form_class(request=self.request).fields, - {'content_items': multiple_choice_field_mock.return_value}, - ) - get_content_items_choices_mock.assert_called_once_with(self.request) - gettext_mock.assert_called_once_with('Courses') - multiple_choice_field_mock.assert_called_once_with( - choices=get_content_items_choices_mock(), - required=False, - widget=forms.CheckboxSelectMultiple, - label=gettext_mock(), + list(form.fields['content_items'].choices), + get_content_items_choices_mock.return_value, ) + get_content_items_choices_mock.assert_called_once_with() + + +@patch.object(DeepLinkingForm, 'get_course_contexts') +@patch.object(DeepLinkingForm, 'get_content_items_choice') +@patch.object(DeepLinkingForm, '__init__', return_value=None) +class TestDeepLinkingFormGetContentItemsChoices(DeepLinkingFormBaseTestCase): + """Test DeepLinkingForm `get_content_items_choices` method.""" - @patch(f'{MODULE_PATH}.course_context') - @patch.object(DeepLinkingForm, 'get_content_items_choice') - @patch.object(DeepLinkingForm, '__init__', return_value=None) def test_get_content_items_choices( self, - deep_linking_form_init: MagicMock, # pylint: disable=unused-argument + init_mock: MagicMock, # pylint: disable=unused-argument get_content_items_choice_mock: MagicMock, - course_context_mock: MagicMock, + get_course_contexts_mock: MagicMock, ): """Test `get_content_items_choices` method.""" - course_context_mock.return_value.objects.all.return_value = [self.course] + get_course_contexts_mock.return_value = [self.course] self.assertEqual( - self.form_class().get_content_items_choices(self.request), - [get_content_items_choice_mock.return_value], + self.form_class(**self.form_kwargs).get_content_items_choices(), + {get_content_items_choice_mock.return_value}, ) - course_context_mock.assert_called_once_with() - course_context_mock().objects.all.assert_called_once_with() - get_content_items_choice_mock.assert_called_once_with(self.course, self.request) + get_course_contexts_mock.assert_called_once_with() + get_content_items_choice_mock.assert_called_once_with(self.course) + + +@patch(f'{MODULE_PATH}.reverse') +@patch.object(DeepLinkingForm, 'get_content_items_choices') +class TestDeepLinkingFormGetContentItemsChoice(DeepLinkingFormBaseTestCase): + """Test DeepLinkingForm `get_content_items_choice` method.""" - @patch(f'{MODULE_PATH}.reverse') - @patch.object(DeepLinkingForm, '__init__', return_value=None) def test_get_content_items_choice( self, - deep_linking_form_init: MagicMock, # pylint: disable=unused-argument + get_content_items_choices_mock: MagicMock, # pylint: disable=unused-argument reverse_mock: MagicMock, ): """Test `get_content_items_choice` method.""" self.assertEqual( - self.form_class().get_content_items_choice(self.course, self.request), + self.form_class(**self.form_kwargs).get_content_items_choice(self.course), ( self.request.build_absolute_uri.return_value, self.course.learning_context.title, @@ -86,16 +93,101 @@ def test_get_content_items_choice( ) self.request.build_absolute_uri.assert_called_once_with(reverse_mock()) - @patch(f'{MODULE_PATH}.DeepLinkResource') - @patch.object(DeepLinkingForm, '__init__', return_value=None) + +@patch(f'{MODULE_PATH}.course_context') +@patch(f'{MODULE_PATH}.json.loads') +@patch.object(CourseAccessConfiguration.objects, 'get') +@patch(f'{MODULE_PATH}.COURSE_ACCESS_CONFIGURATION') +@patch.object(DeepLinkingForm, 'get_content_items_choices') +class TestDeepLinkingFormGetCourseContexts(DeepLinkingFormBaseTestCase): + """Test DeepLinkingForm `get_course_contexts` method.""" + + def test_get_course_contexts( + self, + get_content_items_choices_mock: MagicMock, # pylint: disable=unused-argument + course_access_configuration_switch_mock: MagicMock, + course_access_configuration_get_mock: MagicMock, + json_loads_mock: MagicMock, + course_context: MagicMock, + ): + """Test `get_course_contexts` method.""" + self.assertEqual( + self.form_class(**self.form_kwargs).get_course_contexts(), + course_context.return_value.objects.filter.return_value, + ) + course_access_configuration_switch_mock.is_enabled.assert_called_once_with() + course_access_configuration_get_mock.assert_called_once_with(lti_tool=self.lti_tool) + course_context.assert_called_once_with() + json_loads_mock.assert_called_once_with( + course_access_configuration_get_mock().allowed_course_ids, + ) + course_context().objects.filter.assert_called_once_with( + learning_context__context_key__in=json_loads_mock() + ) + + def test_with_disabled_course_access_configuration_switch( + self, + get_content_items_choices_mock: MagicMock, # pylint: disable=unused-argument + course_access_configuration_switch_mock: MagicMock, + course_access_configuration_get_mock: MagicMock, + json_loads_mock: MagicMock, + course_context: MagicMock, + ): + """Test with disabled `COURSE_ACCESS_CONFIGURATION` switch.""" + course_access_configuration_switch_mock.is_enabled.return_value = False + + self.assertEqual( + self.form_class(**self.form_kwargs).get_course_contexts(), + course_context.return_value.objects.all.return_value, + ) + course_access_configuration_switch_mock.is_enabled.assert_called_once_with() + course_context.assert_called_once_with() + course_context().objects.all.assert_called_once_with() + course_access_configuration_get_mock.assert_not_called() + json_loads_mock.assert_not_called() + course_context().objects.filter.assert_not_called() + + @patch(f'{MODULE_PATH}._') + def test_without_course_access_configuration( + self, + gettext_mock: MagicMock, + get_content_items_choices_mock: MagicMock, # pylint: disable=unused-argument + course_access_configuration_switch_mock: MagicMock, + course_access_configuration_get_mock: MagicMock, + json_loads_mock: MagicMock, + course_context: MagicMock, + ): + """Test without CourseAccessConfiguration instance.""" + course_access_configuration_get_mock.side_effect = CourseAccessConfiguration.DoesNotExist + + with self.assertRaises(DeepLinkingException) as ctxm: + self.form_class(**self.form_kwargs).get_course_contexts() + + course_access_configuration_switch_mock.is_enabled.assert_called_once_with() + course_access_configuration_get_mock.assert_called_once_with(lti_tool=self.lti_tool) + gettext_mock.assert_called_once_with( + f'Course access configuration not found: {self.lti_tool.title}.', + ) + self.assertEqual(str(gettext_mock()), str(ctxm.exception)) + course_context.assert_not_called() + course_context().objects.all.assert_not_called() + json_loads_mock.assert_not_called() + course_context().objects.filter.assert_not_called() + + +@patch(f'{MODULE_PATH}.DeepLinkResource') +@patch.object(DeepLinkingForm, 'get_content_items_choices') +class TestDeepLinkingFormGetDeepLinkResources(DeepLinkingFormBaseTestCase): + """Test DeepLinkingForm `get_deep_link_resources` method.""" + def test_get_deep_link_resources( self, - deep_linking_form_init: MagicMock, # pylint: disable=unused-argument + get_content_items_choices_mock: MagicMock, # pylint: disable=unused-argument deep_link_resource_mock: MagicMock, ): """Test `get_deep_link_resources` method.""" content_item = 'https://example.com' - form = self.form_class() + form = self.form_class(**self.form_kwargs) form.cleaned_data = {'content_items': [content_item]} self.assertEqual( diff --git a/openedx_lti_tool_plugin/deep_linking/tests/test_views.py b/openedx_lti_tool_plugin/deep_linking/tests/test_views.py index a7ada99..f0632b6 100644 --- a/openedx_lti_tool_plugin/deep_linking/tests/test_views.py +++ b/openedx_lti_tool_plugin/deep_linking/tests/test_views.py @@ -15,6 +15,7 @@ DeepLinkingView, validate_deep_linking_message, ) +from openedx_lti_tool_plugin.tests import AUD, ISS MODULE_PATH = f'{MODULE_PATH}.views' @@ -155,6 +156,8 @@ def test_class_attributes(self): @patch.object(DeepLinkingFormView, 'get_message_from_cache') @patch(f'{MODULE_PATH}.validate_deep_linking_message') +@patch(f'{MODULE_PATH}.get_identity_claims', return_value=(ISS, AUD, None, None)) +@patch.object(DeepLinkingFormView, 'tool_config', new_callable=PropertyMock) @patch.object(DeepLinkingFormView, 'form_class') class TestDeepLinkingFormViewGet(TestCase): """Test DeepLinkingFormView get method.""" @@ -173,6 +176,8 @@ def test_get( self, render_mock: MagicMock, form_class_mock: MagicMock, + tool_config_mock: MagicMock, + get_identity_claims_mock: MagicMock, validate_deep_linking_message_mock: MagicMock, get_message_from_cache_mock: MagicMock, ): @@ -183,7 +188,13 @@ def test_get( ) get_message_from_cache_mock.assert_called_once_with(self.request, self.launch_id) validate_deep_linking_message_mock.assert_called_once_with(get_message_from_cache_mock()) - form_class_mock.assert_called_once_with(request=self.request) + get_message_from_cache_mock().get_launch_data.assert_called_once_with() + get_identity_claims_mock.assert_called_once_with(get_message_from_cache_mock().get_launch_data()) + tool_config_mock().get_lti_tool.assert_called_once_with(ISS, AUD) + form_class_mock.assert_called_once_with( + request=self.request, + lti_tool=tool_config_mock().get_lti_tool(), + ) render_mock.assert_called_once_with( self.request, 'openedx_lti_tool_plugin/deep_linking/form.html', @@ -199,6 +210,8 @@ def test_raises_lti_exception( self, http_response_error_mock: MagicMock, form_class_mock: MagicMock, + tool_config_mock: MagicMock, + get_identity_claims_mock: MagicMock, validate_deep_linking_message_mock: MagicMock, get_message_from_cache_mock: MagicMock, ): @@ -212,6 +225,9 @@ def test_raises_lti_exception( ) get_message_from_cache_mock.assert_called_once_with(self.request, self.launch_id) validate_deep_linking_message_mock.assert_not_called() + get_message_from_cache_mock.return_value.get_launch_data.assert_not_called() + get_identity_claims_mock.assert_not_called() + tool_config_mock().get_lti_tool.assert_not_called() form_class_mock.assert_not_called() http_response_error_mock.assert_called_once_with(exception) @@ -220,6 +236,8 @@ def test_raises_deep_linking_exception( self, http_response_error_mock: MagicMock, form_class_mock: MagicMock, + tool_config_mock: MagicMock, + get_identity_claims_mock: MagicMock, validate_deep_linking_message_mock: MagicMock, get_message_from_cache_mock: MagicMock, ): @@ -233,12 +251,17 @@ def test_raises_deep_linking_exception( ) get_message_from_cache_mock.assert_called_once_with(self.request, self.launch_id) validate_deep_linking_message_mock.assert_called_once_with(get_message_from_cache_mock()) + get_message_from_cache_mock().get_launch_data.assert_not_called() + get_identity_claims_mock.assert_not_called() + tool_config_mock().get_lti_tool.assert_not_called() form_class_mock.assert_not_called() http_response_error_mock.assert_called_once_with(exception) @patch.object(DeepLinkingFormView, 'get_message_from_cache') @patch(f'{MODULE_PATH}.validate_deep_linking_message') +@patch(f'{MODULE_PATH}.get_identity_claims', return_value=(ISS, AUD, None, None)) +@patch.object(DeepLinkingFormView, 'tool_config', new_callable=PropertyMock) @patch.object(DeepLinkingFormView, 'form_class') class TestDeepLinkingFormViewPost(TestCase): """Test DeepLinkingFormView post method.""" @@ -257,6 +280,8 @@ def test_post( self, http_response_mock: MagicMock, form_class_mock: MagicMock, + tool_config_mock: MagicMock, + get_identity_claims_mock: MagicMock, validate_deep_linking_message_mock: MagicMock, get_message_from_cache_mock: MagicMock, ): @@ -265,10 +290,17 @@ def test_post( self.view_class.as_view()(self.request, self.launch_id), http_response_mock.return_value, ) - form_class_mock.assert_called_once_with(self.request.POST, request=self.request) - form_class_mock().is_valid.assert_called_once_with() get_message_from_cache_mock.assert_called_once_with(self.request, self.launch_id) validate_deep_linking_message_mock.assert_called_once_with(get_message_from_cache_mock()) + get_message_from_cache_mock().get_launch_data.assert_called_once_with() + get_identity_claims_mock.assert_called_once_with(get_message_from_cache_mock().get_launch_data()) + tool_config_mock().get_lti_tool.assert_called_once_with(ISS, AUD) + form_class_mock.assert_called_once_with( + self.request.POST, + request=self.request, + lti_tool=tool_config_mock().get_lti_tool(), + ) + form_class_mock().is_valid.assert_called_once_with() form_class_mock().get_deep_link_resources.assert_called_once_with() get_message_from_cache_mock().get_deep_link.assert_called_once_with() get_message_from_cache_mock().get_deep_link().output_response_form.assert_called_once_with( @@ -283,6 +315,8 @@ def test_with_invalid_form( self, http_response_error_mock: MagicMock, form_class_mock: MagicMock, + tool_config_mock: MagicMock, + get_identity_claims_mock: MagicMock, validate_deep_linking_message_mock: MagicMock, get_message_from_cache_mock: MagicMock, ): @@ -293,10 +327,17 @@ def test_with_invalid_form( self.view_class.as_view()(self.request, self.launch_id), http_response_error_mock.return_value, ) - form_class_mock.assert_called_once_with(self.request.POST, request=self.request) + get_message_from_cache_mock.assert_called_once_with(self.request, self.launch_id) + validate_deep_linking_message_mock.assert_called_once_with(get_message_from_cache_mock()) + get_message_from_cache_mock().get_launch_data.assert_called_once_with() + get_identity_claims_mock.assert_called_once_with(get_message_from_cache_mock().get_launch_data()) + tool_config_mock().get_lti_tool.assert_called_once_with(ISS, AUD) + form_class_mock.assert_called_once_with( + self.request.POST, + request=self.request, + lti_tool=tool_config_mock().get_lti_tool(), + ) form_class_mock().is_valid.assert_called_once_with() - get_message_from_cache_mock.assert_not_called() - validate_deep_linking_message_mock.assert_not_called() form_class_mock().get_deep_link_resources.assert_not_called() get_message_from_cache_mock().get_deep_link.assert_not_called() get_message_from_cache_mock().get_deep_link().output_response_form.assert_not_called() @@ -307,6 +348,8 @@ def test_raises_lti_exception( self, http_response_error_mock: MagicMock, form_class_mock: MagicMock, + tool_config_mock: MagicMock, + get_identity_claims_mock: MagicMock, validate_deep_linking_message_mock: MagicMock, get_message_from_cache_mock: MagicMock, ): @@ -318,10 +361,16 @@ def test_raises_lti_exception( self.view_class.as_view()(self.request, self.launch_id), http_response_error_mock.return_value, ) - form_class_mock.assert_called_once_with(self.request.POST, request=self.request) - form_class_mock().is_valid.assert_called_once_with() get_message_from_cache_mock.assert_called_once_with(self.request, self.launch_id) validate_deep_linking_message_mock.assert_not_called() + get_message_from_cache_mock.return_value.get_launch_data.assert_not_called() + get_identity_claims_mock.assert_not_called() + tool_config_mock().get_lti_tool.assert_not_called() + form_class_mock.assert_not_called() + form_class_mock().is_valid.assert_not_called() + form_class_mock().get_deep_link_resources.assert_not_called() + get_message_from_cache_mock.return_value.get_deep_link.assert_not_called() + get_message_from_cache_mock.return_value.get_deep_link().output_response_form.assert_not_called() http_response_error_mock.assert_called_once_with(exception) @patch.object(DeepLinkingFormView, 'http_response_error') @@ -329,6 +378,8 @@ def test_raises_deep_linking_exception( self, http_response_error_mock: MagicMock, form_class_mock: MagicMock, + tool_config_mock: MagicMock, + get_identity_claims_mock: MagicMock, validate_deep_linking_message_mock: MagicMock, get_message_from_cache_mock: MagicMock, ): @@ -340,10 +391,16 @@ def test_raises_deep_linking_exception( self.view_class.as_view()(self.request, self.launch_id), http_response_error_mock.return_value, ) - form_class_mock.assert_called_once_with(self.request.POST, request=self.request) - form_class_mock().is_valid.assert_called_once_with() get_message_from_cache_mock.assert_called_once_with(self.request, self.launch_id) validate_deep_linking_message_mock.assert_called_once_with(get_message_from_cache_mock()) + get_message_from_cache_mock().get_launch_data.assert_not_called() + get_identity_claims_mock.assert_not_called() + tool_config_mock().get_lti_tool.assert_not_called() + form_class_mock.assert_not_called() + form_class_mock().is_valid.assert_not_called() + form_class_mock().get_deep_link_resources.assert_not_called() + get_message_from_cache_mock().get_deep_link.assert_not_called() + get_message_from_cache_mock().get_deep_link().output_response_form.assert_not_called() http_response_error_mock.assert_called_once_with(exception) diff --git a/openedx_lti_tool_plugin/deep_linking/views.py b/openedx_lti_tool_plugin/deep_linking/views.py index bfe87d7..1f770f7 100644 --- a/openedx_lti_tool_plugin/deep_linking/views.py +++ b/openedx_lti_tool_plugin/deep_linking/views.py @@ -16,6 +16,7 @@ from openedx_lti_tool_plugin.deep_linking.exceptions import DeepLinkingException from openedx_lti_tool_plugin.deep_linking.forms import DeepLinkingForm from openedx_lti_tool_plugin.http import LoggedHttpResponseBadRequest +from openedx_lti_tool_plugin.utils import get_identity_claims from openedx_lti_tool_plugin.views import LtiToolBaseView @@ -69,13 +70,15 @@ def post( """ try: + # Get launch message. message = DjangoMessageLaunch( request, self.tool_config, launch_data_storage=self.tool_storage, ) + # Check launch message type. validate_deep_linking_message(message) - + # Redirect to DeepLinkingForm view. return redirect( f'{app_config.name}:1.3:deep-linking:form', launch_id=message.get_launch_id().replace('lti1p3-launch-', ''), @@ -122,14 +125,21 @@ def get( """ try: + # Get message from cache. message = self.get_message_from_cache(request, launch_id) + # Validate message. validate_deep_linking_message(message) - + # Get identity claims from launch data. + iss, aud, _sub, _pii = get_identity_claims(message.get_launch_data()) + # Render form template. return render( request, 'openedx_lti_tool_plugin/deep_linking/form.html', { - 'form': self.form_class(request=request), + 'form': self.form_class( + request=request, + lti_tool=self.tool_config.get_lti_tool(iss, aud), + ), 'form_url': f'{app_config.name}:1.3:deep-linking:form', 'launch_id': launch_id, }, @@ -156,14 +166,22 @@ def post( """ try: - form = self.form_class(request.POST, request=request) - - if not form.is_valid(): - raise DeepLinkingException(form.errors) - + # Get message from cache. message = self.get_message_from_cache(request, launch_id) + # Validate message. validate_deep_linking_message(message) - + # Get identity claims from launch data. + iss, aud, _sub, _pii = get_identity_claims(message.get_launch_data()) + # Initialize form. + form = self.form_class( + request.POST, + request=request, + lti_tool=self.tool_config.get_lti_tool(iss, aud), + ) + # Validate form. + if not form.is_valid(): + raise DeepLinkingException(form.errors) + # Render Deep Linking response. return HttpResponse( message.get_deep_link().output_response_form( form.get_deep_link_resources(), diff --git a/openedx_lti_tool_plugin/resource_link_launch/views.py b/openedx_lti_tool_plugin/resource_link_launch/views.py index 8d3c0c7..3ea1049 100644 --- a/openedx_lti_tool_plugin/resource_link_launch/views.py +++ b/openedx_lti_tool_plugin/resource_link_launch/views.py @@ -99,8 +99,12 @@ def post( _('Message type is not LtiResourceLinkRequest.'), ) # Get launch data. + # TODO: Replace redundant get_launch_data method with + # the DjangoMessageLaunch get_launch_data method. launch_data = self.get_launch_data(launch_message) # Get identity claims from launch data + # TODO: Replace get_identity_claims method with + # openedx_lti_tool.utils:get_identity_claims function. iss, aud, sub, pii = self.get_identity_claims(launch_data) # Check course access permission. self.check_course_access_permission(course_id, iss, aud) diff --git a/openedx_lti_tool_plugin/tests/test_utils.py b/openedx_lti_tool_plugin/tests/test_utils.py index 4e485ae..42b2bb7 100644 --- a/openedx_lti_tool_plugin/tests/test_utils.py +++ b/openedx_lti_tool_plugin/tests/test_utils.py @@ -1,8 +1,12 @@ -"""Tests for the openedx_lti_tool_plugin utils module.""" +"""Tests utils module.""" +from unittest.mock import MagicMock, patch + from django.test import TestCase -from openedx_lti_tool_plugin.utils import get_client_id, get_pii_from_claims +from openedx_lti_tool_plugin.tests import AUD, ISS, SUB +from openedx_lti_tool_plugin.utils import get_client_id, get_identity_claims, get_pii_from_claims +MODULE_PATH = 'openedx_lti_tool_plugin.utils' CLIENT_ID = 'random-client-id' @@ -56,3 +60,64 @@ def test_with_missing_pii_claims(self): 'locale': '', } ) + + +@patch(f'{MODULE_PATH}.get_client_id') +@patch(f'{MODULE_PATH}.get_pii_from_claims') +@patch(f'{MODULE_PATH}.SAVE_PII_DATA') +class TestGetIdentityClaims(TestCase): + """Test `get_identity_claims` function.""" + + def setUp(self): + """Set up test fixtures.""" + super().setUp() + self.launch_data = { + 'iss': ISS, + 'aud': AUD, + 'sub': SUB, + 'azp': AUD, + } + + def test_with_enabled_save_pii_data_switch( + self, + save_pii_data_mock: MagicMock, + get_pii_from_claims_mock: MagicMock, + get_client_id_mock: MagicMock, + ): + """Test with enabled `SAVE_PII_DATA` switch.""" + save_pii_data_mock.is_enabled.return_value = True + + self.assertEqual( + get_identity_claims(self.launch_data), + ( + self.launch_data['iss'], + get_client_id_mock.return_value, + self.launch_data['sub'], + get_pii_from_claims_mock.return_value, + ) + ) + get_client_id_mock.assert_called_once_with(self.launch_data['aud'], self.launch_data['azp']) + save_pii_data_mock.is_enabled.assert_called_once_with() + get_pii_from_claims_mock.assert_called_once_with(self.launch_data) + + def test_with_disabled_save_pii_data_switch( + self, + save_pii_data_mock: MagicMock, + get_pii_from_claims_mock: MagicMock, + get_client_id_mock: MagicMock, + ): + """Test with disabled `SAVE_PII_DATA` switch.""" + save_pii_data_mock.is_enabled.return_value = False + + self.assertEqual( + get_identity_claims(self.launch_data), + ( + self.launch_data['iss'], + get_client_id_mock.return_value, + self.launch_data['sub'], + {}, + ) + ) + get_client_id_mock.assert_called_once_with(self.launch_data['aud'], self.launch_data['azp']) + save_pii_data_mock.is_enabled.assert_called_once_with() + get_pii_from_claims_mock.assert_not_called() diff --git a/openedx_lti_tool_plugin/utils.py b/openedx_lti_tool_plugin/utils.py index cf90d29..ac827a4 100644 --- a/openedx_lti_tool_plugin/utils.py +++ b/openedx_lti_tool_plugin/utils.py @@ -1,8 +1,10 @@ """Utilities.""" -from typing import Optional +from typing import Optional, Tuple from django.conf import settings +from openedx_lti_tool_plugin.waffle import SAVE_PII_DATA + def is_plugin_enabled() -> bool: """Check 'OLTITP_ENABLE_LTI_TOOL' setting value. @@ -68,3 +70,28 @@ def get_pii_from_claims(claims: dict) -> dict: 'family_name': claims.get('family_name', ''), 'locale': claims.get('locale', ''), } + + +def get_identity_claims(launch_data: dict) -> Tuple[str, str, str, dict]: + """Get identity claims from launch data. + + Args: + launch_data: Launch data dictionary. + + Returns: + A tuple containing the `iss`, `aud`, `sub` and + PII (Personal Identifiable Information) claims. + + .. _OpenID Connect Core 1.0 - ID Token: + https://openid.net/specs/openid-connect-core-1_0.html#IDToken + + .. _OpenID Connect Core 1.0 - Standard Claims: + https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims + + """ + return ( + launch_data.get('iss'), + get_client_id(launch_data.get('aud'), launch_data.get('azp')), + launch_data.get('sub'), + get_pii_from_claims(launch_data) if SAVE_PII_DATA.is_enabled() else {}, + )