Merge pull request #307 from openedx/mroytman/MST-1717-external-id-LTI-1.1-launches

Add course flag to send external_user_id as user_id in LTI 1.1 XBlock launches

Author: michaelroytman

CHANGELOG.rst

@@ -15,6 +15,13 @@ Please See the [releases tab](https://github.com/edx/xblock-lti-consumer/release
 
 Unreleased
 ~~~~~~~~~~
+6.4.0 - 2022-11-18
+------------------
+Adds support for sending an external_user_id in LTI 1.1 XBlock launches. When the
+lti_consumer.enable_external_user_id_1p1_launches CourseWaffleFlag is enabled, the LTI 1.1 launch will send an
+external_user_id as the user_id attribute of the launch. When the lti_consumer.enable_external_user_id_1p1_launches
+CourseWaffleFlag is disabled, the LTI 1.1 launch will continue to send the anonymous_user_id. The external_user_id is
+defined, created, and stored by the external_user_ids Djangoapp in edx-platform.
 
 6.3.0 - 2022-11-16
 ------------------

lti_consumer/__init__.py

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

lti_consumer/lti_xblock.py

@@ -75,7 +75,13 @@
 from .outcomes import OutcomeService
 from .plugin import compat
 from .track import track_event
-from .utils import _, resolve_custom_parameter_template, external_config_filter_enabled, database_config_enabled
+from .utils import (
+    _,
+    resolve_custom_parameter_template,
+    external_config_filter_enabled,
+    external_user_id_1p1_launches_enabled,
+    database_config_enabled,
+)
 
 log = logging.getLogger(__name__)
 
@@ -826,6 +832,22 @@ def external_user_id(self):
             raise LtiError(self.ugettext("Could not get user id for current request"))
         return str(user_id)
 
+    def get_lti_1p1_user_id(self):
+        """
+        Returns the user ID to send to an LTI tool during an LTI 1.1 launch. If the
+        enable_external_user_id_1p1_launches CourseWaffleFlag is enabled for the course, returns the external_user_id
+        defined by the external_user_ids Djangoapp. Otherwise, returns the anonymous_user_id.
+
+        This addresses cases where LTI tools require a static, opaque user_id that is consistent across contexts. On an
+        opt-in basis, courses can be set up to send the external_user_id instead of the anonymous_user_id. Note that
+        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
+            return self.external_user_id
+
+        return self.anonymous_user_id
+
     @property
     def resource_link_id(self):
         """
@@ -875,7 +897,7 @@ def lis_result_sourcedid(self):
         return "{context}:{resource_link}:{user_id}".format(
             context=urllib.parse.quote(self.context_id),
             resource_link=self.resource_link_id,
-            user_id=self.anonymous_user_id
+            user_id=self.get_lti_1p1_user_id()
         )
 
     @property
@@ -1112,7 +1134,7 @@ def lti_launch_handler(self, request, suffix=''):  # pylint: disable=unused-argu
         # return a 400 response with an appropriate error template.
         try:
             real_user_data = self.extract_real_user_data()
-            user_id = self.anonymous_user_id
+            user_id = self.get_lti_1p1_user_id()
             role = self.role
 
             # Convert the LMS role into an LTI 1.1 role.

lti_consumer/plugin/compat.py

@@ -31,6 +31,17 @@
 # .. toggle_warning: None.
 ENABLE_EXTERNAL_CONFIG_FILTER = 'enable_external_config_filter'
 
+# .. toggle_name: lti_consumer.enable_external_user_id_1p1_launches
+# .. toggle_implementation: CourseWaffleFlag
+# .. toggle_default: False
+# .. toggle_description: Enables sending a user's external user ID, as created and stored by the external_user_ids
+#    Djangoapp, instead of an anonymous user ID in LTI 1.1 launches.
+# .. toggle_use_cases: open_edx
+# .. toggle_creation_date: 2022-11-18
+# .. toggle_tickets: https://github.com/openedx/xblock-lti-consumer/pull/307
+# .. toggle_warning: None.
+ENABLE_EXTERNAL_USER_ID_1P1_LAUNCHES = 'enable_external_user_id_1p1_launches'
+
 # Waffle Flags
 # .. toggle_name: lti_consumer.enable_database_config
 # .. toggle_implementation: CourseWaffleFlag
@@ -54,6 +65,15 @@ def get_external_config_waffle_flag():
     return CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.{ENABLE_EXTERNAL_CONFIG_FILTER}', __name__)
 
 
+def get_external_user_id_1p1_launches_waffle_flag():
+    """
+    Import and return Waffle flag for enabling sending external user IDs in LTI 1.1 launches.
+    """
+    # pylint: disable=import-error,import-outside-toplevel
+    from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag
+    return CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.{ENABLE_EXTERNAL_USER_ID_1P1_LAUNCHES}', __name__)
+
+
 def get_database_config_waffle_flag():
     # pylint: disable=import-error,import-outside-toplevel
     from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag

lti_consumer/tests/unit/test_lti_xblock.py

@@ -85,7 +85,6 @@ class TestProperties(TestLtiConsumerXBlock):
     """
     Unit tests for LtiConsumerXBlock properties
     """
-
     def test_descriptor(self):
         """
         Test `descriptor` returns the XBLock object
@@ -304,13 +303,14 @@ def test_resource_link_id(self):
 
     @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.context_id')
     @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.resource_link_id')
-    @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.anonymous_user_id', PropertyMock(return_value=FAKE_USER_ID))
-    def test_lis_result_sourcedid(self, mock_resource_link_id, mock_context_id):
+    @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.get_lti_1p1_user_id')
+    def test_lis_result_sourcedid(self, mock_get_external_user_id, mock_resource_link_id, mock_context_id):
         """
         Test `lis_result_sourcedid` returns appropriate string
         """
         mock_resource_link_id.__get__ = Mock(return_value='resource_link_id')
         mock_context_id.__get__ = Mock(return_value='context_id')
+        mock_get_external_user_id.return_value = FAKE_USER_ID
 
         self.assertEqual(self.xblock.lis_result_sourcedid, f"context_id:resource_link_id:{FAKE_USER_ID}")
 
@@ -668,6 +668,35 @@ def test_unauthenticated_user(self):
             self.xblock.extract_real_user_data()
 
 
+@ddt.ddt
+class TestGetLti1p1UserId(TestLtiConsumerXBlock):
+    """ Unit tests for the get_lti_1p1_user_id method"""
+    def setUp(self):
+        super().setUp()
+
+        # Mock out the anonymous_user_id and external_user_id properties.
+        fake_user = Mock()
+        fake_user.opt_attrs = {
+            'edx-platform.user_id': 1,
+            'edx-platform.user_role': 'studnent',
+            'edx-platform.is_authenticated': True,
+            'edx-platform.anonymous_user_id': 'anonymous_user_id',
+        }
+        self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user)
+        self.xblock.runtime.service(self, 'user').get_external_user_id = Mock(return_value="external_user_id")
+
+    @ddt.data(
+        (True, 'external_user_id'),
+        (False, 'anonymous_user_id'),
+    )
+    @ddt.unpack
+    def test_external_user_id_flag_enabled(self, external_user_id_1p1_launches_enabled_value, expected_value):
+        with patch('lti_consumer.lti_xblock.external_user_id_1p1_launches_enabled') as \
+                external_user_id_1p1_launches_enabled:
+            external_user_id_1p1_launches_enabled.return_value = external_user_id_1p1_launches_enabled_value
+            self.assertEqual(self.xblock.get_lti_1p1_user_id(), expected_value)
+
+
 class TestStudentView(TestLtiConsumerXBlock):
     """
     Unit tests for LtiConsumerXBlock.student_view()
@@ -783,6 +812,11 @@ def setUp(self):
 
         self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user)
 
+        self.mock_external_user_ids_patcher = patch("lti_consumer.lti_xblock.external_user_id_1p1_launches_enabled")
+        self.mock_external_user_ids_patcher_enabled = self.mock_external_user_ids_patcher.start()
+        self.mock_external_user_ids_patcher_enabled.return_value = False
+        self.addCleanup(self.mock_external_user_ids_patcher.stop)
+
     @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.course')
     @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.anonymous_user_id', PropertyMock(return_value=FAKE_USER_ID))
     def test_generate_launch_request_called(self, mock_course):

lti_consumer/utils.py

@@ -8,7 +8,11 @@
 from django.conf import settings
 from edx_django_utils.cache import get_cache_key, TieredCache
 
-from lti_consumer.plugin.compat import get_external_config_waffle_flag, get_database_config_waffle_flag
+from lti_consumer.plugin.compat import (
+    get_external_config_waffle_flag,
+    get_external_user_id_1p1_launches_waffle_flag,
+    get_database_config_waffle_flag,
+)
 from lti_consumer.lti_1p3.constants import LTI_1P3_CONTEXT_TYPE
 from lti_consumer.lti_1p3.exceptions import InvalidClaimValue, MissingRequiredClaim
 
@@ -187,6 +191,17 @@ def external_config_filter_enabled(course_key):
     return get_external_config_waffle_flag().is_enabled(course_key)
 
 
+def external_user_id_1p1_launches_enabled(course_key):
+    """
+    Returns whether the lti_consumer.enable_external_user_id_1p1_launches CourseWaffleFlag is enabled.
+    Returns True if sending external user IDs in LTI 1.1 launches is enabled for the course via the CourseWaffleFlag.
+
+    Arguments:
+        course_key (opaque_keys.edx.locator.CourseLocator): Course Key
+    """
+    return get_external_user_id_1p1_launches_waffle_flag().is_enabled(course_key)
+
+
 def database_config_enabled(course_key):
     """
     Return whether the lti_consumer.enable_database_config WaffleFlag is enabled. Return True if it is enabled;