diff --git a/lti_consumer/models.py b/lti_consumer/models.py index e8736d2ea6148743ee4cfcff7471453381d12935..acf8eb267b3e0cac38f6a7fd398f3f2ddf908dc5 100644 --- a/lti_consumer/models.py +++ b/lti_consumer/models.py @@ -264,7 +264,7 @@ class LtiConfiguration(models.Model): if block is None: if self.location is None: raise ValueError(_("Block location not set, it's not possible to retrieve the block.")) - block = self._block = compat.load_block_as_anonymous_user(self.location) + block = self._block = compat.load_block_as_user(self.location) return block @block.setter diff --git a/lti_consumer/plugin/compat.py b/lti_consumer/plugin/compat.py index e7a1b1561841072bd9fc2035c9059a800da0d8a7..7335627e5471017e41505e353604391276b5b608 100644 --- a/lti_consumer/plugin/compat.py +++ b/lti_consumer/plugin/compat.py @@ -78,9 +78,43 @@ def run_xblock_handler_noauth(*args, **kwargs): return handle_xblock_callback_noauth(*args, **kwargs) -def load_block_as_anonymous_user(location): +def load_block_as_user(location): """ - Load a block as anonymous user. + Load a block as the current user, or load as the anonymous user if no user is available. + """ + # pylint: disable=import-error,import-outside-toplevel + from crum import get_current_user, get_current_request + from xmodule.modulestore.django import modulestore + from lms.djangoapps.courseware.module_render import get_module_for_descriptor_internal + from openedx.core.lib.xblock_utils import request_token + + # Retrieve descriptor from modulestore + descriptor = modulestore().get_item(location) + user = get_current_user() + request = get_current_request() + if user and request: + # If we're in request scope, the descriptor may already be a block bound to a user + # and we don't need to do any more loading + if descriptor.scope_ids.user_id is not None and user.id == descriptor.scope_ids.user_id: + return descriptor + + # If not load this block to bind it onto the user + get_module_for_descriptor_internal( + user=user, + descriptor=descriptor, + student_data=None, + course_id=location.course_key, + track_function=None, + request_token=request_token(request), + ) + return descriptor + else: + return _load_block_as_anonymous_user(location, descriptor) + + +def _load_block_as_anonymous_user(location, descriptor): + """ + Load a block as the anonymous user because no user is available. This uses a few internal courseware methods to retrieve the descriptor and bind an AnonymousUser to it, in a similar fashion as a `noauth` XBlock @@ -89,12 +123,8 @@ def load_block_as_anonymous_user(location): # pylint: disable=import-error,import-outside-toplevel from crum import impersonate from django.contrib.auth.models import AnonymousUser - from xmodule.modulestore.django import modulestore from lms.djangoapps.courseware.module_render import get_module_for_descriptor_internal - # Retrieve descriptor from modulestore - descriptor = modulestore().get_item(location) - # ensure `crum.get_current_user` returns AnonymousUser. It returns None when outside # of request scope which causes error during block loading. user = AnonymousUser() diff --git a/lti_consumer/signals.py b/lti_consumer/signals.py index b239349326834701c53b176b6faa7ee6c9f2a72a..e9d928e9faea722b96f05febe3f48d4cb1cd6082 100644 --- a/lti_consumer/signals.py +++ b/lti_consumer/signals.py @@ -31,7 +31,7 @@ def publish_grade_on_score_update(sender, instance, **kwargs): # pylint: disabl and instance.score_given: try: # Load block using LMS APIs and check if the block is graded and still accept grades. - block = compat.load_block_as_anonymous_user(instance.line_item.resource_link_id) + block = compat.load_block_as_user(instance.line_item.resource_link_id) if block.has_score and (not block.is_past_due() or block.accept_grades_past_due): # Map external ID to platform user user = compat.get_user_from_external_user_id(instance.user_id) diff --git a/lti_consumer/tests/unit/plugin/test_views.py b/lti_consumer/tests/unit/plugin/test_views.py index 17db1dcfbd32600ada6a8aba2fdb96c94e268bee..4d7e9e99818fae94675a3ce2d9514d58d4e19d19 100644 --- a/lti_consumer/tests/unit/plugin/test_views.py +++ b/lti_consumer/tests/unit/plugin/test_views.py @@ -144,7 +144,7 @@ class TestLti1p3LaunchGateEndpoint(TestCase): self.compat.get_external_id_for_user.return_value = "12345" model_compat_patcher = patch("lti_consumer.models.compat") model_compat = model_compat_patcher.start() - model_compat.load_block_as_anonymous_user.return_value = self.xblock + model_compat.load_block_as_user.return_value = self.xblock self.addCleanup(model_compat_patcher.stop) def test_invalid_lti_version(self): diff --git a/lti_consumer/tests/unit/plugin/test_views_lti_ags.py b/lti_consumer/tests/unit/plugin/test_views_lti_ags.py index 7a3e7f94441eb56327d7f93735752bd4547ce8c1..856419b9bfc9e73f5639ee4fbc090538867f3e4e 100644 --- a/lti_consumer/tests/unit/plugin/test_views_lti_ags.py +++ b/lti_consumer/tests/unit/plugin/test_views_lti_ags.py @@ -71,7 +71,7 @@ class LtiAgsLineItemViewSetTestCase(APITransactionTestCase): self.addCleanup(compat_mock.stop) self._compat_mock = compat_mock.start() self._compat_mock.get_user_from_external_user_id.return_value = self._mock_user - self._compat_mock.load_block_as_anonymous_user.return_value = self.xblock + self._compat_mock.load_block_as_user.return_value = self.xblock def _set_lti_token(self, scopes=None): """ @@ -431,7 +431,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase): "gradingProgress": grading_progress, }) - self._compat_mock.load_block_as_anonymous_user.assert_not_called() + self._compat_mock.load_block_as_user.assert_not_called() self._compat_mock.get_user_from_external_user_id.assert_not_called() def test_xblock_grade_publish_on_score_save(self): @@ -439,7 +439,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase): Test that the grade is submitted when gradingProgress is `FullyGraded`. """ # Set up LMS mocks - self._compat_mock.load_block_as_anonymous_user.return_value = self.xblock + self._compat_mock.load_block_as_user.return_value = self.xblock self._compat_mock.get_user_from_external_user_id.return_value = 'user_mock' self.xblock.set_user_module_score = Mock() @@ -452,7 +452,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase): # Check if publish grade was called self.xblock.set_user_module_score.assert_called_once() self._compat_mock.get_user_from_external_user_id.assert_called_once() - self._compat_mock.load_block_as_anonymous_user.assert_called_once() + self._compat_mock.load_block_as_user.assert_called_once() call_args = self.xblock.set_user_module_score.call_args.args self.assertEqual(call_args, ('user_mock', 0.83, 1, 'This is exceptional work.')) @@ -462,7 +462,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase): Test when given score is bigger than maximum score. """ # Return block bypassing LMS API - self._compat_mock.load_block_as_anonymous_user.return_value = self.xblock + self._compat_mock.load_block_as_user.return_value = self.xblock self._compat_mock.get_user_from_external_user_id.return_value = 'user_mock' self.xblock.set_user_module_score = Mock() @@ -494,7 +494,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase): self.xblock.runtime.publish.side_effect = LmsException # Return block bypassing LMS API - self._compat_mock.load_block_as_anonymous_user.return_value = self.xblock + self._compat_mock.load_block_as_user.return_value = self.xblock # Check that the except statement catches the exception with self.assertRaises(LmsException): @@ -511,7 +511,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase): self._post_lti_score() # Check that the block wasn't set if due date is past - self._compat_mock.load_block_as_anonymous_user.assert_called_once() + self._compat_mock.load_block_as_user.assert_called_once() self._compat_mock.get_user_from_external_user_id.assert_not_called() self.xblock.set_user_module_score.assert_not_called() @@ -521,7 +521,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase): Test grade publish after due date when accept_grades_past_due is True. Grade should publish. """ # Return block bypassing LMS API - self._compat_mock.load_block_as_anonymous_user.return_value = self.xblock + self._compat_mock.load_block_as_user.return_value = self.xblock self._compat_mock.get_user_from_external_user_id.return_value = 'user_mock' self.xblock.set_user_module_score = Mock() diff --git a/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py b/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py index b19794d6bc831b93663b26ed9f7033299572ccd3..445730f038f58697d8a8bbb5559019eaa80a86f6 100644 --- a/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py +++ b/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py @@ -80,7 +80,7 @@ class LtiDeepLinkingTestCase(APITransactionTestCase): self.addCleanup(compat_mock.stop) self._compat_mock = compat_mock.start() self._compat_mock.get_user_from_external_user_id.return_value = self._mock_user - self._compat_mock.load_block_as_anonymous_user.return_value = self.xblock + self._compat_mock.load_block_as_user.return_value = self.xblock @ddt.ddt diff --git a/lti_consumer/tests/unit/test_lti_xblock.py b/lti_consumer/tests/unit/test_lti_xblock.py index 7f535df62952f7db0cc11cf9ebe6393bc40a3a48..59c27f406c0e3176317c93ffe800eafb69f868ae 100644 --- a/lti_consumer/tests/unit/test_lti_xblock.py +++ b/lti_consumer/tests/unit/test_lti_xblock.py @@ -1576,7 +1576,7 @@ class TestLti1p3AccessTokenEndpoint(TestLtiConsumerXBlock): patcher = patch( 'lti_consumer.models.compat', - **{'load_block_as_anonymous_user.return_value': self.xblock} + **{'load_block_as_user.return_value': self.xblock} ) patcher.start() self.addCleanup(patcher.stop) @@ -1751,7 +1751,7 @@ class TestLti1p3AccessTokenJWK(TestCase): self.request = make_jwt_request(jwt) patcher = patch( 'lti_consumer.models.compat', - **{'load_block_as_anonymous_user.return_value': self.xblock} + **{'load_block_as_user.return_value': self.xblock} ) patcher.start() self.addCleanup(patcher.stop) diff --git a/lti_consumer/tests/unit/test_models.py b/lti_consumer/tests/unit/test_models.py index aa06952c8ea2ed1fa7115ec9cb3621cc4873c528..f1562bc4989cc126e8e64b23b3c8770b6694e463 100644 --- a/lti_consumer/tests/unit/test_models.py +++ b/lti_consumer/tests/unit/test_models.py @@ -318,7 +318,7 @@ class TestLtiConfigurationModel(TestCase): """ Check if a block is properly loaded when calling the `block` property. """ - compat_mock.load_block_as_anonymous_user.return_value = self.xblock + compat_mock.load_block_as_user.return_value = self.xblock block = self.lti_1p3_config.block self.assertEqual(block, self.xblock) @@ -435,7 +435,7 @@ class TestLtiAgsScoreModel(TestCase): compat_mock = patch("lti_consumer.signals.compat") self.addCleanup(compat_mock.stop) self._compat_mock = compat_mock.start() - self._compat_mock.load_block_as_anonymous_user.return_value = make_xblock( + self._compat_mock.load_block_as_user.return_value = make_xblock( 'lti_consumer', LtiConsumerXBlock, { 'due': datetime.now(timezone.utc), 'graceperiod': timedelta(days=2),