Merge pull request from GHSA-3q74-3rfh-g37j (openedx#32838)

This is a security fix that restricts capabilities to author libraries in studio to course authors/ members of an organization.
Security advisory: GHSA-3q74-3rfh-g37j

cms/djangoapps/contentstore/views/course.py

@@ -53,7 +53,8 @@
     CourseInstructorRole,
     CourseStaffRole,
     GlobalStaff,
-    UserBasedRole
+    UserBasedRole,
+    OrgStaffRole
 )
 from common.djangoapps.util.course import get_link_for_about_page
 from common.djangoapps.util.date_utils import get_default_time_display
@@ -597,6 +598,7 @@ def format_in_process_course_view(uca):
         'optimization_enabled': optimization_enabled,
         'active_tab': 'courses',
         'allowed_organizations': get_allowed_organizations(user),
+        'allowed_organizations_for_libraries': get_allowed_organizations_for_libraries(user),
         'can_create_organizations': user_can_create_organizations(user),
     })
 
@@ -624,6 +626,7 @@ def library_listing(request):
         'split_studio_home': split_library_view_on_dashboard(),
         'active_tab': 'libraries',
         'allowed_organizations': get_allowed_organizations(request.user),
+        'allowed_organizations_for_libraries': get_allowed_organizations_for_libraries(request.user),
         'can_create_organizations': user_can_create_organizations(request.user),
     }
     return render_to_response('index.html', data)
@@ -1943,13 +1946,37 @@ def get_allowed_organizations(user):
         return []
 
 
+def get_allowed_organizations_for_libraries(user):
+    """
+    Helper method for returning the list of organizations for which the user is allowed to create libraries.
+    """
+    if settings.FEATURES.get('ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES', False):
+        return get_organizations_for_non_course_creators(user)
+    elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False):
+        return get_organizations(user)
+    else:
+        return []
+
+
 def user_can_create_organizations(user):
     """
     Returns True if the user can create organizations.
     """
     return user.is_staff or not settings.FEATURES.get('ENABLE_CREATOR_GROUP', False)
 
 
+def get_organizations_for_non_course_creators(user):
+    """
+    Returns the list of organizations which the user is a staff member of, as a list of strings.
+    """
+    orgs_map = set()
+    orgs = OrgStaffRole().get_orgs_for_user(user)
+    # deduplicate
+    for org in orgs:
+        orgs_map.add(org)
+    return list(orgs_map)
+
+
 def get_organizations(user):
     """
     Returns the list of organizations for which the user is allowed to create courses.

cms/djangoapps/contentstore/views/library.py

@@ -35,14 +35,15 @@
 from common.djangoapps.student.roles import (
     CourseInstructorRole,
     CourseStaffRole,
-    LibraryUserRole
+    LibraryUserRole,
+    OrgStaffRole,
+    UserBasedRole,
 )
 from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json
 
 from ..config.waffle import REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND
 from ..utils import add_instructor, reverse_library_url
 from .component import CONTAINER_TEMPLATES, get_component_templates
-from .helpers import is_content_creator
 from .block import create_xblock_info
 from .user import user_with_role
 
@@ -79,10 +80,11 @@ def user_can_create_library(user, org=None):
     elif user.is_staff:
         return True
     elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False):
-        has_course_creator_role = True
-        if org:
-            has_course_creator_role = is_content_creator(user, org)
-        return get_course_creator_status(user) == 'granted' and has_course_creator_role
+        is_course_creator = get_course_creator_status(user) == 'granted'
+        has_org_staff_role = OrgStaffRole().get_orgs_for_user(user).exists()
+        has_course_staff_role = UserBasedRole(user=user, role=CourseStaffRole.ROLE).courses_with_role().exists()
+
+        return is_course_creator or has_org_staff_role or has_course_staff_role
     else:
         # EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present.
         disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None)

cms/djangoapps/contentstore/views/tests/test_course_index.py

@@ -423,13 +423,13 @@ def check_index_page(self, separate_archived_courses, org):
 
     @ddt.data(
         # Staff user has course staff access
-        (True, 'staff', None, 0, 20),
-        (False, 'staff', None, 0, 20),
+        (True, 'staff', None, 0, 21),
+        (False, 'staff', None, 0, 21),
         # Base user has global staff access
-        (True, 'user', ORG, 2, 20),
-        (False, 'user', ORG, 2, 20),
-        (True, 'user', None, 2, 20),
-        (False, 'user', None, 2, 20),
+        (True, 'user', ORG, 2, 21),
+        (False, 'user', ORG, 2, 21),
+        (True, 'user', None, 2, 21),
+        (False, 'user', None, 2, 21),
     )
     @ddt.unpack
     def test_separate_archived_courses(self, separate_archived_courses, username, org, mongo_queries, sql_queries):

cms/djangoapps/contentstore/views/tests/test_library.py

@@ -19,11 +19,15 @@
 from cms.djangoapps.contentstore.tests.utils import AjaxEnabledTestClient, CourseTestCase, parse_json
 from cms.djangoapps.contentstore.utils import reverse_course_url, reverse_library_url
 from cms.djangoapps.course_creators.views import add_user_with_status_granted as grant_course_creator_status
-from common.djangoapps.student.roles import LibraryUserRole
+from common.djangoapps.student.roles import LibraryUserRole, CourseStaffRole
 from xmodule.modulestore.tests.factories import LibraryFactory  # lint-amnesty, pylint: disable=wrong-import-order
+from cms.djangoapps.course_creators.models import CourseCreator
+
+from common.djangoapps.student import auth
 
 from ..component import get_component_templates
 from ..library import user_can_create_library
+from ..course import get_allowed_organizations_for_libraries
 
 LIBRARY_REST_URL = '/library/'  # URL for GET/POST requests involving libraries
 
@@ -51,26 +55,51 @@ def setUp(self):
     ######################################################
     # Tests for /library/ - list and create libraries:
 
+    # When libraries are disabled, nobody can create libraries
     @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", False)
     def test_library_creator_status_libraries_not_enabled(self):
         _, nostaff_user = self.create_non_staff_authed_user_client()
         self.assertEqual(user_can_create_library(nostaff_user), False)
 
+    # When creator group is disabled, non-staff users can create libraries
+    @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True)
+    def test_library_creator_status_with_no_course_creator_role(self):
+        _, nostaff_user = self.create_non_staff_authed_user_client()
+        self.assertEqual(user_can_create_library(nostaff_user), True)
+
+    # When creator group is enabled, Non staff users cannot create libraries
+    @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True)
+    def test_library_creator_status_for_enabled_creator_group_setting_for_non_staff_users(self):
+        _, nostaff_user = self.create_non_staff_authed_user_client()
+        with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}):
+            self.assertEqual(user_can_create_library(nostaff_user), False)
+
+    # Global staff can create libraries
     @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True)
     def test_library_creator_status_with_is_staff_user(self):
         self.assertEqual(user_can_create_library(self.user), True)
 
+    # When creator groups are enabled, global staff can create libraries
+    @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True)
+    def test_library_creator_status_for_enabled_creator_group_setting_with_is_staff_user(self):
+        with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}):
+            self.assertEqual(user_can_create_library(self.user), True)
+
+    # When creator groups are enabled, course creators can create libraries
     @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True)
-    def test_library_creator_status_with_course_creator_role(self):
+    def test_library_creator_status_with_course_creator_role_for_enabled_creator_group_setting(self):
         _, nostaff_user = self.create_non_staff_authed_user_client()
         with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}):
             grant_course_creator_status(self.user, nostaff_user)
             self.assertEqual(user_can_create_library(nostaff_user), True)
 
+    # When creator groups are enabled, course staff members can create libraries
     @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True)
-    def test_library_creator_status_with_no_course_creator_role(self):
+    def test_library_creator_status_with_course_staff_role_for_enabled_creator_group_setting(self):
         _, nostaff_user = self.create_non_staff_authed_user_client()
-        self.assertEqual(user_can_create_library(nostaff_user), True)
+        with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}):
+            auth.add_users(self.user, CourseStaffRole(self.course.id), nostaff_user)
+            self.assertEqual(user_can_create_library(nostaff_user), True)
 
     @ddt.data(
         (False, False, True),
@@ -188,9 +217,9 @@ def test_lib_create_permission_no_course_creator_role_and_no_course_creator_grou
         self.assertEqual(response.status_code, 200)
 
     @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True})
-    def test_lib_create_permission_no_course_creator_role_and_course_creator_group(self):
+    def test_lib_create_permission_no_course_creator_role_and_no_course_creator_group_and_no_course_staff_role(self):
         """
-        Users who are not given course creator roles should not be able to create libraries
+        Users who are not given course creator roles or course staff role should not be able to create libraries
         if ENABLE_CREATOR_GROUP is enabled.
         """
         self.client.logout()
@@ -201,6 +230,23 @@ def test_lib_create_permission_no_course_creator_role_and_course_creator_group(s
         })
         self.assertEqual(response.status_code, 403)
 
+    @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True})
+    def test_lib_create_permission_course_staff_role(self):
+        """
+        Users who are staff on any existing course should able to create libraries
+        if ENABLE_CREATOR_GROUP is enabled.
+        """
+        self.client.logout()
+        ns_user, password = self.create_non_staff_user()
+        self.client.login(username=ns_user.username, password=password)
+
+        auth.add_users(self.user, CourseStaffRole(self.course.id), ns_user)
+        self.assertTrue(auth.user_has_role(ns_user, CourseStaffRole(self.course.id)))
+        response = self.client.ajax_post(LIBRARY_REST_URL, {
+            'org': 'org', 'library': 'lib', 'display_name': "New Library",
+        })
+        self.assertEqual(response.status_code, 200)
+
     @ddt.data(
         {},
         {'org': 'org'},
@@ -405,3 +451,41 @@ def test_component_limits(self):
             response = self.client.ajax_post(reverse('xblock_handler'), data)
             self.assertEqual(response.status_code, 400)
             self.assertIn('cannot have more than 1 component', parse_json(response)['error'])
+
+    def test_allowed_organizations_for_library(self):
+        """
+        Test the different organizations that a user can select for creating a library, depending
+        on Feature Flags and on user role.
+        With organization staff access enabled, a user should be able to select organizations they
+        are a staff member of. Else, with creator groups enabled, the user should be able to select
+        organizations they are course creator for.
+        """
+        course_creator = CourseCreator.objects.create(user=self.user, all_organizations=True)
+        with patch('cms.djangoapps.course_creators.models.CourseCreator.objects.filter') as mock_filter:
+            mock_filter.return_value.first.return_value = course_creator
+            with patch('organizations.models.Organization.objects.all') as mock_all:
+                mock_all.return_value.values_list.return_value = ['org1', 'org2']
+                with patch('common.djangoapps.student.roles.OrgStaffRole.get_orgs_for_user') as get_user_orgs:
+                    get_user_orgs.return_value = ['org3']
+                    # Call the method under test
+                    with mock.patch.dict(
+                        'django.conf.settings.FEATURES',
+                        {"ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES": False}
+                    ):
+                        with mock.patch.dict(
+                            'django.conf.settings.FEATURES',
+                            {"ENABLE_CREATOR_GROUP": False}
+                        ):
+                            organizations = get_allowed_organizations_for_libraries(self.user)
+                            # Assert that the method returned the expected value
+                            self.assertEqual(organizations, [])
+                        with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}):
+                            organizations = get_allowed_organizations_for_libraries(self.user)
+                            # Assert that the method returned the expected value
+                            self.assertEqual(organizations, ['org1', 'org2'])
+                    with mock.patch.dict(
+                        'django.conf.settings.FEATURES',
+                        {"ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES": True}
+                    ):
+                        organizations = get_allowed_organizations_for_libraries(self.user)
+                        self.assertEqual(organizations, ['org3'])

cms/envs/common.py

@@ -215,6 +215,11 @@
     # an Open edX admin has added them to the course creator group.
     'ENABLE_CREATOR_GROUP': True,
 
+    # If set to True, organization staff members can create libraries for their specific
+    # organization and no other organizations. They do not need to be course creators,
+    # even when ENABLE_CREATOR_GROUP is True.
+    'ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES': True,
+
     # Turn off account locking if failed login attempts exceeds a limit
     'ENABLE_MAX_FAILED_LOGIN_ATTEMPTS': False,
 

cms/envs/devstack.py

@@ -156,7 +156,10 @@ def should_show_debug_toolbar(request):  # lint-amnesty, pylint: disable=missing
 FEATURES['CERTIFICATES_HTML_VIEW'] = True
 
 ########################## AUTHOR PERMISSION #######################
-FEATURES['ENABLE_CREATOR_GROUP'] = False
+FEATURES['ENABLE_CREATOR_GROUP'] = True
+
+########################## Library creation organizations restriction #######################
+FEATURES['ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES'] = True
 
 ################### FRONTEND APPLICATION PUBLISHER URL ###################
 FEATURES['FRONTEND_APP_PUBLISHER_URL'] = 'http://localhost:18400'

cms/templates/index.html

@@ -167,7 +167,7 @@ <h3 class="title">${_("Create a New Library")}</h3>
                     <input class="new-library-org" id="new-library-org" type="text" name="new-library-org" required placeholder="${_('e.g. UniversityX or OrganizationX')}" aria-describedby="tip-new-library-org tip-error-new-library-org" />
                   % else:
                     <select class="new-library-org" id="new-library-org" name="new-library-org" required aria-describedby="tip-new-library-org tip-error-new-library-org">
-                      % for org in allowed_organizations:
+                      % for org in allowed_organizations_for_libraries:
                         <option value="${org}">${org}</option>
                       % endfor
                     </select>