diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 023cb07199fe..664d1d34da4f 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/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,6 +1946,18 @@ 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. @@ -1950,6 +1965,18 @@ def user_can_create_organizations(user): 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. diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 8ec1e79d1932..328c92687b2c 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/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) diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index cf9f2a4197ae..9fdbb425e51d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/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): diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index 746e61cb3126..bac450b3bd26 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/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']) diff --git a/cms/envs/common.py b/cms/envs/common.py index 70b4f0781a0a..a9ea495b0841 100644 --- a/cms/envs/common.py +++ b/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, diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 1466d15b7e2c..58030a2fa914 100644 --- a/cms/envs/devstack.py +++ b/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' diff --git a/cms/templates/index.html b/cms/templates/index.html index 12e50215ba6b..dbd83b2ff518 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -167,7 +167,7 @@