Merge branch 'master' into chris/FAL-4266-add-title-to-course-details

Author: ChrisChV

cms/djangoapps/contentstore/views/preview.py

@@ -18,6 +18,7 @@
 from xblock.exceptions import NoSuchHandlerError
 from xblock.runtime import KvsFieldData
 
+from openedx.core.djangoapps.video_config.services import VideoConfigService
 from xmodule.contentstore.django import contentstore
 from xmodule.exceptions import NotFoundError, ProcessingError
 from xmodule.modulestore.django import XBlockI18nService, modulestore
@@ -214,7 +215,8 @@ def _prepare_runtime_for_preview(request, block):
         "teams_configuration": TeamsConfigurationService(),
         "sandbox": SandboxService(contentstore=contentstore, course_id=course_id),
         "cache": CacheService(cache),
-        'replace_urls': ReplaceURLService
+        'replace_urls': ReplaceURLService,
+        'video_config': VideoConfigService(),
     }
 
     block.runtime.get_block_for_descriptor = partial(_load_preview_block, request)

common/djangoapps/third_party_auth/api/serializers.py

@@ -20,4 +20,7 @@ def get_username(self, social_user):
 
     def get_remote_id(self, social_user):
         """ Gets remote id from social user based on provider """
+        remote_id_field_name = self.context.get('remote_id_field_name', None)
+        if remote_id_field_name:
+            return self.provider.get_remote_id_from_field_name(social_user, remote_id_field_name)
         return self.provider.get_remote_id_from_social_auth(social_user)

common/djangoapps/third_party_auth/api/tests/test_views.py

@@ -38,8 +38,10 @@
 PASSWORD = "edx"
 
 
-def get_mapping_data_by_usernames(usernames):
+def get_mapping_data_by_usernames(usernames, remote_id_field_name=False):
     """ Generate mapping data used in response """
+    if remote_id_field_name:
+        return [{'username': username, 'remote_id': 'external_' + username} for username in usernames]
     return [{'username': username, 'remote_id': 'remote_' + username} for username in usernames]
 
 
@@ -76,11 +78,13 @@ def setUp(self):  # pylint: disable=arguments-differ
                 provider=google.backend_name,
                 uid=f'{username}@gmail.com',
             )
-            UserSocialAuth.objects.create(
+            usa = UserSocialAuth.objects.create(
                 user=user,
                 provider=testshib.backend_name,
                 uid=f'{testshib.slug}:remote_{username}',
             )
+            usa.set_extra_data({'external_user_id': f'external_{username}'})
+            usa.refresh_from_db()
         # Create another user not linked to any providers:
         UserFactory.create(username=CARL_USERNAME, email=f'{CARL_USERNAME}@example.com', password=PASSWORD)
 
@@ -304,12 +308,20 @@ def test_list_all_user_mappings_oauth2(self, valid_call, expect_code, expect_dat
     @ddt.data(
         ({'username': [ALICE_USERNAME, STAFF_USERNAME]}, 200,
          get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME])),
+        ({'username': [ALICE_USERNAME, STAFF_USERNAME], 'remote_id_field_name': 'external_user_id'}, 200,
+         get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME], remote_id_field_name=True)),
         ({'remote_id': ['remote_' + ALICE_USERNAME, 'remote_' + STAFF_USERNAME, 'remote_' + CARL_USERNAME]}, 200,
          get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME])),
+        ({'remote_id': ['remote_' + ALICE_USERNAME, 'remote_' + STAFF_USERNAME, 'remote_' + CARL_USERNAME],
+          'remote_id_field_name': 'external_user_id'}, 200,
+         get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME], remote_id_field_name=True)),
         ({'username': [ALICE_USERNAME, CARL_USERNAME, STAFF_USERNAME]}, 200,
          get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME])),
         ({'username': [ALICE_USERNAME], 'remote_id': ['remote_' + STAFF_USERNAME]}, 200,
          get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME])),
+        ({'username': [ALICE_USERNAME], 'remote_id': ['remote_' + STAFF_USERNAME],
+          'remote_id_field_name': 'external_user_id'}, 200,
+         get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME], remote_id_field_name=True)),
     )
     @ddt.unpack
     def test_user_mappings_with_query_params_comma_separated(self, query_params, expect_code, expect_data):
@@ -321,19 +333,29 @@ def test_user_mappings_with_query_params_comma_separated(self, query_params, exp
         for attr in ['username', 'remote_id']:
             if attr in query_params:
                 params.append('{}={}'.format(attr, ','.join(query_params[attr])))
+        if 'remote_id_field_name' in query_params:
+            params.append('remote_id_field_name={}'.format(query_params['remote_id_field_name']))
         url = "{}?{}".format(base_url, '&'.join(params))
         response = self.client.get(url, HTTP_X_EDX_API_KEY=VALID_API_KEY)
         self._verify_response(response, expect_code, expect_data)
 
     @ddt.data(
         ({'username': [ALICE_USERNAME, STAFF_USERNAME]}, 200,
          get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME])),
+        ({'username': [ALICE_USERNAME, STAFF_USERNAME], 'remote_id_field_name': 'external_user_id'}, 200,
+         get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME], remote_id_field_name=True)),
         ({'remote_id': ['remote_' + ALICE_USERNAME, 'remote_' + STAFF_USERNAME, 'remote_' + CARL_USERNAME]}, 200,
          get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME])),
+        ({'remote_id': ['remote_' + ALICE_USERNAME, 'remote_' + STAFF_USERNAME, 'remote_' + CARL_USERNAME],
+          'remote_id_field_name': 'external_user_id'}, 200,
+         get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME], remote_id_field_name=True)),
         ({'username': [ALICE_USERNAME, CARL_USERNAME, STAFF_USERNAME]}, 200,
          get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME])),
         ({'username': [ALICE_USERNAME], 'remote_id': ['remote_' + STAFF_USERNAME]}, 200,
          get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME])),
+        ({'username': [ALICE_USERNAME], 'remote_id': ['remote_' + STAFF_USERNAME],
+          'remote_id_field_name': 'external_user_id'}, 200,
+         get_mapping_data_by_usernames([ALICE_USERNAME, STAFF_USERNAME], remote_id_field_name=True)),
     )
     @ddt.unpack
     def test_user_mappings_with_query_params_multi_value_key(self, query_params, expect_code, expect_data):
@@ -345,6 +367,8 @@ def test_user_mappings_with_query_params_multi_value_key(self, query_params, exp
         for attr in ['username', 'remote_id']:
             if attr in query_params:
                 params.setlist(attr, query_params[attr])
+        if 'remote_id_field_name' in query_params:
+            params['remote_id_field_name'] = query_params['remote_id_field_name']
         url = f"{base_url}?{params.urlencode()}"
         response = self.client.get(url, HTTP_X_EDX_API_KEY=VALID_API_KEY)
         self._verify_response(response, expect_code, expect_data)

common/djangoapps/third_party_auth/api/views.py

@@ -323,6 +323,9 @@ class UserMappingView(ListAPIView):
 
           GET /api/third_party_auth/v0/providers/{provider_id}/users?username={username1},{username2}
 
+          GET /api/third_party_auth/v0/providers/{provider_id}/users?username={username1}&
+            remote_id_field_name={external_id_field_name}
+
           GET /api/third_party_auth/v0/providers/{provider_id}/users?username={username1}&usernames={username2}
 
           GET /api/third_party_auth/v0/providers/{provider_id}/users?remote_id={remote_id1},{remote_id2}
@@ -346,6 +349,9 @@ class UserMappingView(ListAPIView):
         * usernames: Optional. List of comma separated edX usernames to filter the result set.
           e.g. ?usernames=bob123,jane456
 
+        * remote_id_field_name: Optional. The field name to use for the remote id lookup.
+          Useful when learners are coming from external LMS. e.g. ?remote_id_field_name=ext_userid_sf
+
         * page, page_size: Optional. Used for paging the result set, especially when getting
           an unfiltered list.
 
@@ -415,6 +421,7 @@ def get_serializer_context(self):
         remove idp_slug from the remote_id if there is any
         """
         context = super().get_serializer_context()
+        context['remote_id_field_name'] = self.request.query_params.get('remote_id_field_name', None)
         context['provider'] = self.provider
 
         return context

common/djangoapps/third_party_auth/models.py

@@ -810,6 +810,17 @@ def match_social_auth(self, social_auth):
         prefix = self.slug + ":"
         return self.backend_name == social_auth.provider and social_auth.uid.startswith(prefix)
 
+    def get_remote_id_from_field_name(self, social_auth, field_name):
+        """ Given a UserSocialAuth object, return the user remote ID against the field name provided. """
+        if not self.match_social_auth(social_auth):
+            raise ValueError(
+                f"UserSocialAuth record does not match given provider {self.provider_id}"
+            )
+        field_value = social_auth.extra_data.get(field_name, None)
+        if field_value and isinstance(field_value, list):
+            return field_value[0]
+        return field_value
+
     def get_remote_id_from_social_auth(self, social_auth):
         """ Given a UserSocialAuth object, return the remote ID used by this provider. """
         assert self.match_social_auth(social_auth)

lms/djangoapps/courseware/block_render.py

@@ -42,6 +42,7 @@
 from xblock.runtime import KvsFieldData
 
 from lms.djangoapps.teams.services import TeamsService
+from openedx.core.djangoapps.video_config.services import VideoConfigService
 from openedx.core.lib.xblock_services.call_to_action import CallToActionService
 from xmodule.contentstore.django import contentstore
 from xmodule.exceptions import NotFoundError, ProcessingError
@@ -635,6 +636,7 @@ def inner_get_block(block: XBlock) -> XBlock | None:
         'call_to_action': CallToActionService(),
         'publish': EventPublishingService(user, course_id, track_function),
         'enrollments': EnrollmentsService(),
+        'video_config': VideoConfigService(),
     }
 
     runtime.get_block_for_descriptor = inner_get_block

lms/djangoapps/courseware/tests/test_video_mongo.py

@@ -37,7 +37,7 @@
 from lxml import etree
 from path import Path as path
 from xmodule.contentstore.content import StaticContent
-from xmodule.course_block import (
+from openedx.core.djangoapps.video_config.sharing import (
     COURSE_VIDEO_SHARING_ALL_VIDEOS,
     COURSE_VIDEO_SHARING_NONE,
     COURSE_VIDEO_SHARING_PER_VIDEO
@@ -57,6 +57,7 @@
 from common.djangoapps.xblock_django.constants import ATTR_KEY_REQUEST_COUNTRY_CODE
 from lms.djangoapps.courseware.tests.helpers import get_context_dict_from_string
 from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE
+from openedx.core.djangoapps.video_config import sharing
 from openedx.core.djangoapps.video_pipeline.config.waffle import DEPRECATE_YOUTUBE
 from openedx.core.djangoapps.waffle_utils.models import WaffleFlagCourseOverrideModel
 from openedx.core.djangolib.testing.utils import CacheIsolationTestCase
@@ -260,14 +261,14 @@ def test_is_public_sharing_enabled(self, feature_enabled):
         """Test public video url."""
         assert self.block.public_access is True
         with self.mock_feature_toggle(enabled=feature_enabled):
-            assert self.block.is_public_sharing_enabled() == feature_enabled
+            assert sharing.is_public_sharing_enabled(self.block.location, self.block.public_access) == feature_enabled
 
     def test_is_public_sharing_enabled__not_public(self):
         self.block.public_access = False
         with self.mock_feature_toggle():
-            assert not self.block.is_public_sharing_enabled()
+            assert not sharing.is_public_sharing_enabled(self.block.location, self.block.public_access)
 
-    @patch('xmodule.video_block.video_block.VideoBlock.get_course_video_sharing_override')
+    @patch('openedx.core.djangoapps.video_config.sharing.get_course_video_sharing_override')
     def test_is_public_sharing_enabled_by_course_override(self, mock_course_sharing_override):
 
         # Given a course overrides all videos to be shared
@@ -276,47 +277,47 @@ def test_is_public_sharing_enabled_by_course_override(self, mock_course_sharing_
 
         # When I try to determine if public sharing is enabled
         with self.mock_feature_toggle():
-            is_public_sharing_enabled = self.block.is_public_sharing_enabled()
+            is_public_sharing_enabled = sharing.is_public_sharing_enabled(self.block.location, self.block.public_access)
 
         # Then I will get that course value
         self.assertTrue(is_public_sharing_enabled)
 
-    @patch('xmodule.video_block.video_block.VideoBlock.get_course_video_sharing_override')
+    @patch('openedx.core.djangoapps.video_config.sharing.get_course_video_sharing_override')
     def test_is_public_sharing_disabled_by_course_override(self, mock_course_sharing_override):
         # Given a course overrides no videos to be shared
         mock_course_sharing_override.return_value = COURSE_VIDEO_SHARING_NONE
         self.block.public_access = 'some-arbitrary-value'
 
         # When I try to determine if public sharing is enabled
         with self.mock_feature_toggle():
-            is_public_sharing_enabled = self.block.is_public_sharing_enabled()
+            is_public_sharing_enabled = sharing.is_public_sharing_enabled(self.block.location, self.block.public_access)
 
         # Then I will get that course value
         self.assertFalse(is_public_sharing_enabled)
 
     @ddt.data(COURSE_VIDEO_SHARING_PER_VIDEO, None)
-    @patch('xmodule.video_block.video_block.VideoBlock.get_course_video_sharing_override')
+    @patch('openedx.core.djangoapps.video_config.sharing.get_course_video_sharing_override')
     def test_is_public_sharing_enabled_per_video(self, mock_override_value, mock_course_sharing_override):
         # Given a course does not override per-video settings
         mock_course_sharing_override.return_value = mock_override_value
         self.block.public_access = 'some-arbitrary-value'
 
         # When I try to determine if public sharing is enabled
         with self.mock_feature_toggle():
-            is_public_sharing_enabled = self.block.is_public_sharing_enabled()
+            is_public_sharing_enabled = sharing.is_public_sharing_enabled(self.block.location, self.block.public_access)
 
         # I will get the per-video value
         self.assertEqual(self.block.public_access, is_public_sharing_enabled)
 
-    @patch('xmodule.video_block.video_block.get_course_by_id')
+    @patch('openedx.core.lib.courses.get_course_by_id')
     def test_is_public_sharing_course_not_found(self, mock_get_course):
         # Given a course does not override per-video settings
         mock_get_course.side_effect = Http404()
         self.block.public_access = 'some-arbitrary-value'
 
         # When I try to determine if public sharing is enabled
         with self.mock_feature_toggle():
-            is_public_sharing_enabled = self.block.is_public_sharing_enabled()
+            is_public_sharing_enabled = sharing.is_public_sharing_enabled(self.block.location, self.block.public_access)
 
         # I will fall-back to per-video values
         self.assertEqual(self.block.public_access, is_public_sharing_enabled)
@@ -325,7 +326,7 @@ def test_is_public_sharing_course_not_found(self, mock_get_course):
     def test_context(self, is_public_sharing_enabled):
         with self.mock_feature_toggle():
             with patch.object(
-                self.block,
+                sharing,
                 'is_public_sharing_enabled',
                 return_value=is_public_sharing_enabled
             ):

lms/djangoapps/courseware/views/views.py

@@ -138,6 +138,7 @@
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.util.user_messages import PageLevelMessages
 from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE
+from openedx.core.djangoapps.video_config.sharing import is_public_sharing_enabled
 from openedx.core.djangoapps.zendesk_proxy.utils import create_zendesk_ticket
 from openedx.core.djangolib.markup import HTML, Text
 from openedx.core.lib.courses import get_course_by_id
@@ -1869,7 +1870,7 @@ def get_course_and_video_block(self, usage_key_string):
             )
 
             # Block must be marked as public to be viewed
-            if not video_block.is_public_sharing_enabled():
+            if not is_public_sharing_enabled(video_block.location, video_block.public_access):
                 raise Http404("Video not found.")
 
         return course, video_block

openedx/core/djangoapps/content_libraries/api/libraries.py

@@ -267,7 +267,11 @@ def get_libraries_for_user(user, org=None, text_search=None, order=None) -> Quer
             Q(learning_package__description__icontains=text_search)
         )
 
-    filtered = permissions.perms[permissions.CAN_VIEW_THIS_CONTENT_LIBRARY].filter(user, qs)
+    # Using distinct() temporarily to avoid duplicate results caused by overlapping permission checks
+    # between Bridgekeeper and the new authorization framework. This ensures correct results for now,
+    # but it should be removed once Bridgekeeper support is fully dropped and all permission logic
+    # is handled through openedx-authz.
+    filtered = permissions.perms[permissions.CAN_VIEW_THIS_CONTENT_LIBRARY].filter(user, qs).distinct()
 
     if order:
         order_query = 'learning_package__'