refactor: replace deprecated rebind_noauth_module_to_user, get_real_user, runtime.hostname, runtime.course_id [BD-13]

CHANGELOG.rst

@@ -15,6 +15,15 @@ Please See the [releases tab](https://github.com/openedx/xblock-lti-consumer/rel
 
 Unreleased
 ~~~~~~~~~~
+
+7.0.0 - 2022-11-29
+------------------
+* Refactor anonymous user to real user rebinding function to use `rebind_user` service.
+* Refactor accessing hostname from runtime attribute to using `settings.LMS_BASE`.
+* Refactor usage of `get_real_user` with `UserService`.
+* Refactor deprecated usage of `runtime.course_id` and replace with `runtime.scope_ids.usage_id.context_key`.
+* Refactor deprecated usage of `block.location` with `block.scope_ids.usage_id`.
+
 6.4.0 - 2022-11-18
 ------------------
 Adds support for sending an external_user_id in LTI 1.1 XBlock launches. When the

lti_consumer/__init__.py

@@ -4,4 +4,4 @@
 from .apps import LTIConsumerApp
 from .lti_xblock import LtiConsumerXBlock
 
-__version__ = '6.4.0'
+__version__ = '7.0.0'

lti_consumer/api.py

@@ -65,24 +65,24 @@ def _get_lti_config_for_block(block):
     if block.config_type == 'database':
         lti_config = _get_or_create_local_lti_config(
             block.lti_version,
-            block.location,
+            block.scope_ids.usage_id,
             LtiConfiguration.CONFIG_ON_DB,
         )
     elif block.config_type == 'external':
         config = get_external_config_from_filter(
-            {"course_key": block.location.course_key},
+            {"course_key": block.scope_ids.usage_id.context_key},
             block.external_config
         )
         lti_config = _get_or_create_local_lti_config(
             config.get("version"),
-            block.location,
+            block.scope_ids.usage_id,
             LtiConfiguration.CONFIG_EXTERNAL,
             external_id=block.external_config,
         )
     else:
         lti_config = _get_or_create_local_lti_config(
             block.lti_version,
-            block.location,
+            block.scope_ids.usage_id,
             LtiConfiguration.CONFIG_ON_XBLOCK,
         )
     return lti_config

lti_consumer/lti_1p1/oauth.py

@@ -147,7 +147,7 @@ def log_authorization_header(request, client_key, client_secret):
     oauth_body_hash = str(base64.b64encode(sha1.digest()))
     log.debug("[LTI] oauth_body_hash = %s", oauth_body_hash)
     client = oauth1.Client(client_key, client_secret)
-    params = client.get_oauth_params(request)
+    params = oauth1.rfc5849.signature.collect_parameters(headers=request.headers, exclude_oauth_signature=False)
     params.append(('oauth_body_hash', oauth_body_hash))
     mock_request = SignedRequest(
         uri=str(urllib.parse.unquote(request.url)),

lti_consumer/lti_xblock.py

@@ -139,10 +139,10 @@ def valid_config_type_values(block):
         {"display_name": _("Configuration on block"), "value": "new"}
     ]
 
-    if database_config_enabled(block.location.course_key):
+    if database_config_enabled(block.scope_ids.usage_id.context_key):
         values.append({"display_name": _("Database Configuration"), "value": "database"})
 
-    if external_config_filter_enabled(block.location.course_key):
+    if external_config_filter_enabled(block.scope_ids.usage_id.context_key):
         values.append({"display_name": _("Reusable Configuration"), "value": "external"})
 
     return values
@@ -161,6 +161,7 @@ class LaunchTarget:
 
 
 @XBlock.needs('i18n')
+@XBlock.needs('rebind_user')
 @XBlock.wants('user')
 @XBlock.wants('settings')
 @XBlock.wants('lti-configuration')
@@ -695,8 +696,8 @@ def editable_fields(self):
         editable_fields = self.editable_field_names
         noneditable_fields = []
 
-        is_database_config_enabled = database_config_enabled(self.location.course_key)  # pylint: disable=no-member
-        is_external_config_filter_enabled = external_config_filter_enabled(self.location.course_key)  # pylint: disable=no-member
+        is_database_config_enabled = database_config_enabled(self.scope_ids.usage_id.context_key)
+        is_external_config_filter_enabled = external_config_filter_enabled(self.scope_ids.usage_id.context_key)
 
         # If neither additional config_types are enabled, do not display the "config_type" field to users, as "new" is
         # the only option and does not make sense without other options.
@@ -713,7 +714,7 @@ def editable_fields(self):
         if config_service:
             is_already_sharing_learner_info = self.ask_to_send_email or self.ask_to_send_username
             if not config_service.configuration.lti_access_to_learners_editable(
-                    self.course_id,
+                    self.scope_ids.usage_id.context_key,
                     is_already_sharing_learner_info,
             ):
                 noneditable_fields.extend(['ask_to_send_username', 'ask_to_send_email'])
@@ -745,7 +746,7 @@ def context_id(self):
         context_id is an opaque identifier that uniquely identifies the context (e.g., a course)
         that contains the link being launched.
         """
-        return str(self.course_id)
+        return str(self.scope_ids.usage_id.context_key)
 
     @property
     def role(self):
@@ -763,7 +764,7 @@ def course(self):
         """
         Return course by course id.
         """
-        return self.runtime.modulestore.get_course(self.runtime.course_id)
+        return self.runtime.modulestore.get_course(self.scope_ids.usage_id.context_key)
 
     @property
     def lti_provider_key_secret(self):
@@ -843,7 +844,7 @@ def get_lti_1p1_user_id(self):
         toggling this flag in a running course carries the risk of breaking the LTI integrations in the course. This
         flag should also only be enabled for new courses in which no LTI attempts have been made.
         """
-        if external_user_id_1p1_launches_enabled(self.location.course_key):  # pylint: disable=no-member
+        if external_user_id_1p1_launches_enabled(self.scope_ids.usage_id.context_key):
             return self.external_user_id
 
         return self.anonymous_user_id
@@ -880,9 +881,7 @@ def resource_link_id(self):
         i4x-2-3-lti-31de800015cf4afb973356dbe81496df this part of resource_link_id:
         makes resource_link_id to be unique among courses inside same system.
         """
-        return str(urllib.parse.quote(
-            f"{self.runtime.hostname}-{self.location.html_id()}"  # pylint: disable=no-member
-        ))
+        return str(urllib.parse.quote(f"{settings.LMS_BASE}-{self.scope_ids.usage_id.html_id()}"))
 
     @property
     def lis_result_sourcedid(self):
@@ -1223,7 +1222,7 @@ def lti_1p3_access_token(self, request, suffix=''):  # pylint: disable=unused-ar
         # Runtime import because this can only be run in the LMS/Studio Django
         # environments. Importing the views on the top level will cause RuntimeErorr
         from lti_consumer.plugin.views import access_token_endpoint  # pylint: disable=import-outside-toplevel
-        return access_token_endpoint(request, usage_id=str(self.location))  # pylint: disable=no-member
+        return access_token_endpoint(request, usage_id=str(self.scope_ids.usage_id))
 
     @XBlock.handler
     def outcome_service_handler(self, request, suffix=''):  # pylint: disable=unused-argument
@@ -1294,7 +1293,7 @@ def result_service_handler(self, request, suffix=''):
         except LtiError:
             return Response(status=401)  # Unauthorized in this case.  401 is right
 
-        user = self.runtime.get_real_user(anon_id)
+        user = self.runtime.service(self, 'user').get_user_by_anonymous_id(anon_id)
         if not user:  # that means we can't save to database, as we do not have real user id.
             msg = _("[LTI]: Real user not found against anon_id: {}").format(anon_id)
             log.info(msg)
@@ -1332,7 +1331,7 @@ def _result_service_get(self, lti_consumer, user):
         Returns:
             dict:  response to this request as dictated by the LtiConsumer
         """
-        self.runtime.rebind_noauth_module_to_user(self, user)
+        self.runtime.service(self, 'rebind_user').rebind_noauth_module_to_user(self, user)
         args = []
         if self.module_score:
             args.extend([self.module_score, self.score_comment])
@@ -1420,7 +1419,7 @@ def set_user_module_score(self, user, score, max_score, comment=''):
         else:
             scaled_score = None
 
-        self.runtime.rebind_noauth_module_to_user(self, user)
+        self.runtime.service(self, 'rebind_user').rebind_noauth_module_to_user(self, user)
 
         # have to publish for the progress page...
         self.runtime.publish(
@@ -1459,8 +1458,8 @@ def get_lti_1p3_launch_data(self):
         from lti_consumer.api import config_id_for_block
         config_id = config_id_for_block(self)
 
-        location = self.location  # pylint: disable=no-member
-        course_key = str(location.course_key)
+        location = self.scope_ids.usage_id
+        course_key = str(location.context_key)
 
         launch_data = Lti1p3LaunchData(
             user_id=self.lms_user_id,
@@ -1482,7 +1481,7 @@ def get_context_title(self):
         Return the title attribute of the context_claim for LTI 1.3 launches. This information is included in the
         launch_data query or form parameter of the LTI 1.3 third-party login initiation request.
         """
-        course_key = self.location.course_key  # pylint: disable=no-member
+        course_key = self.scope_ids.usage_id.context_key
         course = compat.get_course_by_id(course_key)
 
         return " - ".join([
@@ -1555,7 +1554,7 @@ def _get_context_for_template(self):
         return {
             'launch_url': launch_url.strip(),
             'lti_1p3_launch_url': lti_1p3_launch_url,
-            'element_id': self.location.html_id(),  # pylint: disable=no-member
+            'element_id': self.scope_ids.usage_id.html_id(),
             'element_class': self.category,
             'launch_target': self.launch_target,
             'display_name': self.display_name,

lti_consumer/models.py

@@ -241,7 +241,7 @@ def clean(self):
             })
         if self.version == self.LTI_1P3 and self.config_store == self.CONFIG_ON_DB:
             block = compat.load_enough_xblock(self.location)
-            if not database_config_enabled(block.location.course_key):
+            if not database_config_enabled(block.scope_ids.usage_id.context_key):
                 raise ValidationError({
                     "config_store": _("LTI Configuration stores on database is not enabled."),
                 })

lti_consumer/outcomes.py

@@ -6,8 +6,8 @@
 """
 
 import logging
-import urllib.parse
 from xml.sax.saxutils import escape
+from urllib.parse import unquote
 
 from lxml import etree
 from xblockutils.resources import ResourceLoader
@@ -183,7 +183,8 @@ def handle_request(self, request):
             log.debug("[LTI]: %s", error_message)
             return response_xml_template.format(**failure_values)
 
-        real_user = self.xblock.runtime.get_real_user(urllib.parse.unquote(sourced_id.split(':')[-1]))
+        anon_id = unquote(sourced_id.split(':')[-1])
+        real_user = self.xblock.runtime.service(self, 'user').get_user_by_anonymous_id(anon_id)
         if not real_user:  # that means we can't save to database, as we do not have real user id.
             failure_values['imsx_messageIdentifier'] = escape(imsx_message_identifier)
             failure_values['imsx_description'] = "User not found."

lti_consumer/signals/signals.py

@@ -46,7 +46,7 @@ def publish_grade_on_score_update(sender, instance, **kwargs):  # pylint: disabl
                 # the LMS database.
                 log.info(
                     "Publishing LTI grade from block %s to LMS. User: %s (score: %s)",
-                    block.location,
+                    block.scope_ids.usage_id,
                     user,
                     score,
                 )

lti_consumer/tests/test_utils.py

@@ -5,7 +5,8 @@
 from unittest.mock import Mock
 import urllib
 
-from opaque_keys.edx.keys import UsageKey
+from opaque_keys.edx.keys import CourseKey
+from opaque_keys.edx.locator import LocalId
 from webob import Request
 from workbench.runtime import WorkbenchRuntime
 from xblock.fields import ScopeIds
@@ -22,30 +23,27 @@ def make_xblock(xblock_name, xblock_cls, attributes):
     runtime = WorkbenchRuntime()
     key_store = DictKeyValueStore()
     db_model = KvsFieldData(key_store)
-    ids = generate_scope_ids(runtime, xblock_name)
+    course_id = 'course-v1:edX+DemoX+Demo_Course'
+    course_key = CourseKey.from_string(course_id)
+    ids = generate_scope_ids(course_key, xblock_name)
+
     xblock = xblock_cls(runtime, db_model, scope_ids=ids)
     xblock.category = Mock()
 
-    xblock.location = UsageKey.from_string(
-        'block-v1:edX+DemoX+Demo_Course+type@problem+block@466f474fa4d045a8b7bde1b911e095ca'
-    )
-
     xblock.runtime = Mock(
         hostname='localhost',
     )
-    xblock.course_id = 'course-v1:edX+DemoX+Demo_Course'
     for key, value in attributes.items():
         setattr(xblock, key, value)
     return xblock
 
 
-def generate_scope_ids(runtime, block_type):
+def generate_scope_ids(course_key, block_type):
     """
     Helper to generate scope IDs for an XBlock
     """
-    def_id = runtime.id_generator.create_definition(block_type)
-    usage_id = runtime.id_generator.create_usage(def_id)
-    return ScopeIds('user', block_type, def_id, usage_id)
+    usage_key = course_key.make_usage_key(block_type, str(LocalId()))
+    return ScopeIds('user', block_type, usage_key, usage_key)
 
 
 def make_request(body, method='POST'):

lti_consumer/tests/unit/plugin/test_views.py

@@ -333,7 +333,7 @@ def test_launch_callback_endpoint_deep_linking_database_config(self, dl_enabled)
         self.xblock.config_type = 'database'
 
         LtiConfiguration.objects.filter(id=self.config.id).update(
-            location=self.xblock.location,  # pylint: disable=no-member
+            location=self.xblock.scope_ids.usage_id,
             version=LtiConfiguration.LTI_1P3,
             config_store=LtiConfiguration.CONFIG_ON_DB,
             lti_advantage_deep_linking_enabled=dl_enabled,

lti_consumer/tests/unit/plugin/test_views_lti_ags.py

@@ -51,7 +51,7 @@ def setUp(self):
 
         # Create configuration
         self.lti_config = LtiConfiguration.objects.create(
-            location=self.xblock.location,  # pylint: disable=no-member
+            location=self.xblock.scope_ids.usage_id,
             version=LtiConfiguration.LTI_1P3,
         )
 
@@ -171,7 +171,7 @@ def test_lti_ags_list(self):
         line_item = LtiAgsLineItem.objects.create(
             lti_configuration=self.lti_config,
             resource_id="test",
-            resource_link_id=self.xblock.location,  # pylint: disable=no-member
+            resource_link_id=self.xblock.scope_ids.usage_id,
             label="test label",
             score_maximum=100
         )
@@ -192,7 +192,7 @@ def test_lti_ags_list(self):
                     'scoreMaximum': 100,
                     'label': 'test label',
                     'tag': '',
-                    'resourceLinkId': str(self.xblock.location),  # pylint: disable=no-member
+                    'resourceLinkId': str(self.xblock.scope_ids.usage_id),
                     'startDateTime': None,
                     'endDateTime': None,
                 }
@@ -209,7 +209,7 @@ def test_lti_ags_retrieve(self):
         line_item = LtiAgsLineItem.objects.create(
             lti_configuration=self.lti_config,
             resource_id="test",
-            resource_link_id=self.xblock.location,  # pylint: disable=no-member
+            resource_link_id=self.xblock.scope_ids.usage_id,
             label="test label",
             score_maximum=100
         )
@@ -236,7 +236,7 @@ def test_lti_ags_retrieve(self):
                 'scoreMaximum': 100,
                 'label': 'test label',
                 'tag': '',
-                'resourceLinkId': str(self.xblock.location),  # pylint: disable=no-member
+                'resourceLinkId': str(self.xblock.scope_ids.usage_id),
                 'startDateTime': None,
                 'endDateTime': None,
             }
@@ -256,7 +256,7 @@ def test_create_lineitem(self):
                 'scoreMaximum': 100,
                 'label': 'test',
                 'tag': 'score',
-                'resourceLinkId': str(self.xblock.location),  # pylint: disable=no-member
+                'resourceLinkId': str(self.xblock.scope_ids.usage_id),
             }),
             content_type="application/vnd.ims.lis.v2.lineitem+json",
         )
@@ -270,7 +270,7 @@ def test_create_lineitem(self):
                 'scoreMaximum': 100,
                 'label': 'test',
                 'tag': 'score',
-                'resourceLinkId': str(self.xblock.location),  # pylint: disable=no-member
+                'resourceLinkId': str(self.xblock.scope_ids.usage_id),
                 'startDateTime': None,
                 'endDateTime': None,
             }
@@ -281,7 +281,7 @@ def test_create_lineitem(self):
         self.assertEqual(line_item.score_maximum, 100)
         self.assertEqual(line_item.label, 'test')
         self.assertEqual(line_item.tag, 'score')
-        self.assertEqual(str(line_item.resource_link_id), str(self.xblock.location))  # pylint: disable=no-member
+        self.assertEqual(str(line_item.resource_link_id), str(self.xblock.scope_ids.usage_id))
 
     def test_create_lineitem_invalid_resource_link_id(self):
         """
@@ -318,7 +318,7 @@ def setUp(self):
         self.line_item = LtiAgsLineItem.objects.create(
             lti_configuration=self.lti_config,
             resource_id="test",
-            resource_link_id=self.xblock.location,  # pylint: disable=no-member
+            resource_link_id=self.xblock.scope_ids.usage_id,
             label="test label",
             score_maximum=100
         )
@@ -850,7 +850,7 @@ def setUp(self):
         self.line_item = LtiAgsLineItem.objects.create(
             lti_configuration=self.lti_config,
             resource_id="test",
-            resource_link_id=self.xblock.location,  # pylint: disable=no-member
+            resource_link_id=self.xblock.scope_ids.usage_id,
             label="test label",
             score_maximum=100
         )

lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py

@@ -31,7 +31,7 @@ def setUp(self):
 
         # Create configuration
         self.lti_config = LtiConfiguration.objects.create(
-            location=self.xblock.location,  # pylint: disable=no-member
+            location=self.xblock.scope_ids.usage_id,
             version=LtiConfiguration.LTI_1P3,
         )