diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index 6f95cd26da..cf8f113a0d 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -11,7 +11,7 @@ import ddt from mock import patch from student.auth import has_studio_read_access, has_studio_write_access from student.roles import ( - CourseInstructorRole, CourseStaffRole, CourseCreatorRole, LibraryUserRole, + CourseInstructorRole, CourseStaffRole, LibraryUserRole, OrgStaffRole, OrgInstructorRole, OrgLibraryUserRole, ) from xmodule.modulestore import ModuleStoreEnum @@ -25,6 +25,7 @@ from xblock_django.user_service import DjangoXBlockUserService from xmodule.x_module import STUDIO_VIEW from student import auth from student.tests.factories import UserFactory +from course_creators.views import add_user_with_status_granted class LibraryTestCase(ModuleStoreTestCase): @@ -533,9 +534,13 @@ class TestLibraryAccess(SignalDisconnectTestMixin, LibraryTestCase): self.client.logout() self._assert_cannot_create_library(expected_code=302) # 302 redirect to login expected - # Now check that logged-in users without CourseCreator role can still create libraries + # Now check that logged-in users without CourseCreator role cannot create libraries self._login_as_non_staff_user(logout_first=False) - self.assertFalse(CourseCreatorRole().has_user(self.non_staff_user)) + with patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}): + self._assert_cannot_create_library(expected_code=403) # 403 user is not CourseCreator + + # Now check that logged-in users with CourseCreator role can create libraries + add_user_with_status_granted(self.user, self.non_staff_user) with patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}): lib_key2 = self._create_library(library="lib2", display_name="Test Library 2") library2 = modulestore().get_library(lib_key2) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 08666660f3..df5ba575ae 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -26,7 +26,7 @@ from .component import ( ADVANCED_COMPONENT_TYPES, ) from .item import create_xblock_info -from .library import LIBRARIES_ENABLED +from .library import LIBRARIES_ENABLED, get_library_creator_status from ccx_keys.locator import CCXLocator from contentstore.course_group_config import ( COHORT_SCHEME, @@ -465,6 +465,7 @@ def course_listing(request): List all courses available to the logged in user """ courses, in_process_course_actions = get_courses_accessible_to_user(request) + user = request.user libraries = _accessible_libraries_list(request.user) if LIBRARIES_ENABLED else [] def format_in_process_course_view(uca): @@ -509,11 +510,11 @@ def course_listing(request): 'in_process_course_actions': in_process_course_actions, 'libraries_enabled': LIBRARIES_ENABLED, 'libraries': [format_library_for_view(lib) for lib in libraries], - 'show_new_library_button': LIBRARIES_ENABLED and request.user.is_active, - 'user': request.user, + 'show_new_library_button': get_library_creator_status(user), + 'user': user, 'request_course_creator_url': reverse('contentstore.views.request_course_creator'), - 'course_creator_status': _get_course_creator_status(request.user), - 'rerun_creator_status': GlobalStaff().has_user(request.user), + 'course_creator_status': _get_course_creator_status(user), + 'rerun_creator_status': GlobalStaff().has_user(user), 'allow_unicode_course_id': settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID', False), 'allow_course_reruns': settings.FEATURES.get('ALLOW_COURSE_RERUNS', True), }) @@ -1619,6 +1620,7 @@ def _get_course_creator_status(user): If the user passed in has not previously visited the index page, it will be added with status 'unrequested' if the course creator group is in use. """ + if user.is_staff: course_creator_status = 'granted' elif settings.FEATURES.get('DISABLE_COURSE_CREATION', False): diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index cb3cd4e18b..7a642d474a 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -9,7 +9,7 @@ import logging from contentstore.views.item import create_xblock_info from contentstore.utils import reverse_library_url, add_instructor -from django.http import HttpResponseNotAllowed, Http404 +from django.http import HttpResponseNotAllowed, Http404, HttpResponseForbidden from django.contrib.auth.decorators import login_required from django.core.exceptions import PermissionDenied from django.conf import settings @@ -25,6 +25,7 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from .user import user_with_role +from course_creators.views import get_course_creator_status from .component import get_component_templates, CONTAINER_TEMPLATES from student.auth import ( STUDIO_VIEW_USERS, STUDIO_EDIT_ROLES, get_user_permissions, has_studio_read_access, has_studio_write_access @@ -39,6 +40,22 @@ log = logging.getLogger(__name__) LIBRARIES_ENABLED = settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES', False) +def get_library_creator_status(user): + """ + Helper method for returning the library creation status for a particular user, + taking into account the value LIBRARIES_ENABLED. + """ + + if not LIBRARIES_ENABLED: + return False + elif user.is_staff: + return True + elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): + return get_course_creator_status(user) == 'granted' + else: + return True + + @login_required @ensure_csrf_cookie @require_http_methods(('GET', 'POST')) @@ -50,6 +67,10 @@ def library_handler(request, library_key_string=None): log.exception("Attempted to use the content library API when the libraries feature is disabled.") raise Http404 # Should never happen because we test the feature in urls.py also + if not get_library_creator_status(request.user): + if not request.user.is_staff: + return HttpResponseForbidden() + if library_key_string is not None and request.method == 'POST': return HttpResponseNotAllowed(("POST",)) diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index 70b3e6387f..337335654a 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -24,6 +24,7 @@ from course_action_state.models import CourseRerunState from opaque_keys.edx.locator import CourseLocator from search.api import perform_search from student.auth import has_course_author_access +from student.roles import LibraryUserRole from student.tests.factories import UserFactory from util.date_utils import get_default_time_display from xmodule.modulestore import ModuleStoreEnum @@ -75,22 +76,37 @@ class TestCourseIndex(CourseTestCase): """ Test getting the list of libraries from the course listing page """ + def _assert_library_link_present(response, library): + """ + Asserts there's a valid library link on libraries tab. + """ + parsed_html = lxml.html.fromstring(response.content) + library_link_elements = parsed_html.find_class('library-link') + self.assertEqual(len(library_link_elements), 1) + link = library_link_elements[0] + self.assertEqual( + link.get("href"), + reverse_library_url('library_handler', library.location.library_key), + ) + # now test that url + outline_response = self.client.get(link.get("href"), {}, HTTP_ACCEPT='text/html') + self.assertEqual(outline_response.status_code, 200) + # Add a library: lib1 = LibraryFactory.create() index_url = '/home/' index_response = self.client.get(index_url, {}, HTTP_ACCEPT='text/html') - parsed_html = lxml.html.fromstring(index_response.content) - library_link_elements = parsed_html.find_class('library-link') - self.assertEqual(len(library_link_elements), 1) - link = library_link_elements[0] - self.assertEqual( - link.get("href"), - reverse_library_url('library_handler', lib1.location.library_key), - ) - # now test that url - outline_response = self.client.get(link.get("href"), {}, HTTP_ACCEPT='text/html') - self.assertEqual(outline_response.status_code, 200) + _assert_library_link_present(index_response, lib1) + + # Make sure libraries are visible to non-staff users too + self.client.logout() + non_staff_user, non_staff_userpassword = self.create_non_staff_user() + lib2 = LibraryFactory.create(user_id=non_staff_user.id) + LibraryUserRole(lib2.location.library_key).add_users(non_staff_user) + self.client.login(username=non_staff_user.username, password=non_staff_userpassword) + index_response = self.client.get(index_url, {}, HTTP_ACCEPT='text/html') + _assert_library_link_present(index_response, lib2) def test_is_staff_access(self): """ diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index 2a704c002d..0e4c7b5521 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -5,12 +5,15 @@ More important high-level tests are in contentstore/tests/test_libraries.py """ from contentstore.tests.utils import AjaxEnabledTestClient, parse_json from contentstore.utils import reverse_course_url, reverse_library_url +from contentstore.tests.utils import CourseTestCase from contentstore.views.component import get_component_templates -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from contentstore.views.library import get_library_creator_status +from course_creators.views import add_user_with_status_granted as grant_course_creator_status from xmodule.modulestore.tests.factories import LibraryFactory from mock import patch from opaque_keys.edx.locator import CourseKey, LibraryLocator import ddt +import mock from student.roles import LibraryUserRole LIBRARY_REST_URL = '/library/' # URL for GET/POST requests involving libraries @@ -24,7 +27,7 @@ def make_url_for_lib(key): @ddt.ddt -class UnitTestLibraries(ModuleStoreTestCase): +class UnitTestLibraries(CourseTestCase): """ Unit tests for library views """ @@ -38,6 +41,27 @@ class UnitTestLibraries(ModuleStoreTestCase): ###################################################### # Tests for /library/ - list and create libraries: + @mock.patch("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(get_library_creator_status(nostaff_user), False) + + @mock.patch("contentstore.views.library.LIBRARIES_ENABLED", True) + def test_library_creator_status_with_is_staff_user(self): + self.assertEqual(get_library_creator_status(self.user), True) + + @mock.patch("contentstore.views.library.LIBRARIES_ENABLED", True) + def test_library_creator_status_with_course_creator_role(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(get_library_creator_status(nostaff_user), True) + + @mock.patch("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(get_library_creator_status(nostaff_user), True) + @patch("contentstore.views.library.LIBRARIES_ENABLED", False) def test_with_libraries_disabled(self): """ @@ -87,18 +111,45 @@ class UnitTestLibraries(ModuleStoreTestCase): @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}) def test_lib_create_permission(self): """ - Users who are not given course creator roles should still be able to - create libraries. + Users who are given course creator roles should be able to create libraries. """ self.client.logout() ns_user, password = self.create_non_staff_user() self.client.login(username=ns_user.username, password=password) - + grant_course_creator_status(self.user, ns_user) response = self.client.ajax_post(LIBRARY_REST_URL, { 'org': 'org', 'library': 'lib', 'display_name': "New Library", }) self.assertEqual(response.status_code, 200) + @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': False}) + def test_lib_create_permission_no_course_creator_role_and_no_course_creator_group(self): + """ + Users who are not given course creator roles should still be able to create libraries + if ENABLE_CREATOR_GROUP is not enabled. + """ + self.client.logout() + ns_user, password = self.create_non_staff_user() + self.client.login(username=ns_user.username, password=password) + response = self.client.ajax_post(LIBRARY_REST_URL, { + 'org': 'org', 'library': 'lib', 'display_name': "New Library", + }) + 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): + """ + Users who are not given course creator roles should not be 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) + response = self.client.ajax_post(LIBRARY_REST_URL, { + 'org': 'org', 'library': 'lib', 'display_name': "New Library", + }) + self.assertEqual(response.status_code, 403) + @ddt.data( {}, {'org': 'org'}, diff --git a/cms/templates/index.html b/cms/templates/index.html index ccf85c37f5..977443d545 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -33,7 +33,6 @@ from openedx.core.djangolib.markup import HTML, Text % elif course_creator_status=='disallowed_for_this_site' and settings.FEATURES.get('STUDIO_REQUEST_EMAIL',''): ${_("Email staff to create course")} % endif - % if show_new_library_button: ${_("New Library")}