From cb463c0d1194528e97a640f11e0b77662189ce9f Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Wed, 29 Nov 2017 15:49:30 -0500 Subject: [PATCH 1/2] Revert "Revert "Merge pull request #16447 from appsembler/omar/hide-library-button"" This reverts commit 7696727fda1127b7c61f9d6299784dae291d54fa. --- cms/djangoapps/contentstore/views/library.py | 2 +- .../contentstore/views/tests/test_library.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index bce8079f62..1d1667a268 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -58,7 +58,7 @@ def get_library_creator_status(user): elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): return get_course_creator_status(user) == 'granted' else: - return True + return not settings.FEATURES.get('DISABLE_COURSE_CREATION', False) @login_required diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index 102d0778af..ab1441a37f 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -28,6 +28,7 @@ def make_url_for_lib(key): @ddt.ddt +@mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_COURSE_CREATION': False}) class UnitTestLibraries(CourseTestCase): """ Unit tests for library views @@ -63,6 +64,17 @@ class UnitTestLibraries(CourseTestCase): _, nostaff_user = self.create_non_staff_authed_user_client() self.assertEqual(get_library_creator_status(nostaff_user), True) + @mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_COURSE_CREATION': True}) + @mock.patch("contentstore.views.library.LIBRARIES_ENABLED", True) + def test_library_creator_status_with_no_course_creator_role_and_disabled_nonstaff_course_creation(self): + """ + Ensure that `DISABLE_COURSE_CREATION` feature works with libraries as well. + """ + nostaff_client, nostaff_user = self.create_non_staff_authed_user_client() + self.assertFalse(get_library_creator_status(nostaff_user)) + response = nostaff_client.get_json(LIBRARY_REST_URL) + self.assertEqual(response.status_code, 200) + @patch("contentstore.views.library.LIBRARIES_ENABLED", False) def test_with_libraries_disabled(self): """ From a09866622da347b6bfb4013bd2c7c1cfb0177bb3 Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Wed, 29 Nov 2017 16:11:27 -0500 Subject: [PATCH 2/2] Move library creator checks to POST-only --- cms/djangoapps/contentstore/views/library.py | 17 ++++++++--------- .../contentstore/views/tests/test_library.py | 10 ++++++++-- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 1d1667a268..440d537d04 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -72,21 +72,20 @@ 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: + if request.method == 'POST': + if not get_library_creator_status(request.user): return HttpResponseForbidden() - if library_key_string is not None and request.method == 'POST': - return HttpResponseNotAllowed(("POST",)) + if library_key_string is not None: + return HttpResponseNotAllowed(("POST",)) - if request.method == 'POST': return _create_library(request) - # request method is get, since only GET and POST are allowed by @require_http_methods(('GET', 'POST')) - if library_key_string: - return _display_library(library_key_string, request) + else: + if library_key_string: + return _display_library(library_key_string, request) - return _list_libraries(request) + return _list_libraries(request) def _display_library(library_key_string, request): diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index ab1441a37f..30714ffb42 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -72,8 +72,14 @@ class UnitTestLibraries(CourseTestCase): """ nostaff_client, nostaff_user = self.create_non_staff_authed_user_client() self.assertFalse(get_library_creator_status(nostaff_user)) - response = nostaff_client.get_json(LIBRARY_REST_URL) - self.assertEqual(response.status_code, 200) + + # To be explicit, this user can GET, but not POST + get_response = nostaff_client.get_json(LIBRARY_REST_URL) + post_response = nostaff_client.ajax_post(LIBRARY_REST_URL, { + 'org': 'org', 'library': 'lib', 'display_name': "New Library", + }) + self.assertEqual(get_response.status_code, 200) + self.assertEqual(post_response.status_code, 403) @patch("contentstore.views.library.LIBRARIES_ENABLED", False) def test_with_libraries_disabled(self):