Commit ce17c1cd by jagonzalr

make check of creation of library a true/false for forntend, add security in api call, clean tests

parent fe2c92e6
...@@ -100,18 +100,6 @@ class CourseTestCase(ProceduralCourseTestMixin, ModuleStoreTestCase): ...@@ -100,18 +100,6 @@ class CourseTestCase(ProceduralCourseTestMixin, ModuleStoreTestCase):
nonstaff.is_authenticated = lambda: authenticate nonstaff.is_authenticated = lambda: authenticate
return client, nonstaff return client, nonstaff
def create_staff_authed_user_client(self, authenticate=True):
"""
Create a staff user, log them in (if authenticate=True), and return the client, user to use for testing.
"""
staff, password = self.create_staff_user()
client = AjaxEnabledTestClient()
if authenticate:
client.login(username=staff.username, password=password)
staff.is_authenticated = lambda: authenticate
return client, staff
def reload_course(self): def reload_course(self):
""" """
Reloads the course object from the database Reloads the course object from the database
......
...@@ -519,18 +519,18 @@ def course_listing(request): ...@@ -519,18 +519,18 @@ def course_listing(request):
return render_to_response('index.html', { return render_to_response('index.html', {
'courses': courses, 'courses': courses,
'in_process_course_actions': in_process_course_actions, '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': _get_library_creator_status(user),
'user': user, 'user': user,
'request_course_creator_url': reverse('contentstore.views.request_course_creator'), 'request_course_creator_url': reverse('contentstore.views.request_course_creator'),
'course_creator_status': _get_course_creator_status(user), 'course_creator_status': _get_course_creator_status(user),
'rerun_creator_status': GlobalStaff().has_user(user), 'rerun_creator_status': GlobalStaff().has_user(user),
'allow_unicode_course_id': settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID', False), 'allow_unicode_course_id': settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID', False),
'allow_course_reruns': settings.FEATURES.get('ALLOW_COURSE_RERUNS', True), 'allow_course_reruns': settings.FEATURES.get('ALLOW_COURSE_RERUNS', True),
'libraries_enabled': LIBRARIES_ENABLED,
'libraries': [format_library_for_view(lib) for lib in libraries],
'library_creator_status': _get_library_creator_status(user),
'is_programs_enabled': programs_config.is_studio_tab_enabled and user.is_staff, 'is_programs_enabled': programs_config.is_studio_tab_enabled and user.is_staff,
'programs': programs, 'programs': programs,
'program_authoring_url': reverse('programs') 'program_authoring_url': reverse('programs'),
}) })
...@@ -1655,19 +1655,15 @@ def _get_course_creator_status(user): ...@@ -1655,19 +1655,15 @@ def _get_course_creator_status(user):
def _get_library_creator_status(user): def _get_library_creator_status(user):
""" """
Helper method for returning the library creation status for a particular user, Helper method for returning the library creation status for a particular user,
taking into account the values of DISABLE_LIBRARY_CREATION and LIBRARIES_ENABLED. taking into account the value LIBRARIES_ENABLED.
""" """
if not LIBRARIES_ENABLED: if not LIBRARIES_ENABLED:
library_creator_status = 'disallowed_for_this_site' return False
elif user.is_staff: elif user.is_staff:
library_creator_status = 'granted' return True
elif CourseCreatorRole().has_user(user): elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False):
library_creator_status = 'granted' return CourseCreatorRole().has_user(user)
elif settings.FEATURES.get('DISABLE_LIBRARY_CREATION', False):
library_creator_status = 'disallowed_for_this_site'
else: else:
library_creator_status = 'disallowed_for_this_site' return False
return library_creator_status
...@@ -9,7 +9,7 @@ import logging ...@@ -9,7 +9,7 @@ import logging
from contentstore.views.item import create_xblock_info from contentstore.views.item import create_xblock_info
from contentstore.utils import reverse_library_url, add_instructor 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.contrib.auth.decorators import login_required
from django.core.exceptions import PermissionDenied from django.core.exceptions import PermissionDenied
from django.conf import settings from django.conf import settings
...@@ -29,7 +29,7 @@ from .component import get_component_templates, CONTAINER_TEMPLATES ...@@ -29,7 +29,7 @@ from .component import get_component_templates, CONTAINER_TEMPLATES
from student.auth import ( from student.auth import (
STUDIO_VIEW_USERS, STUDIO_EDIT_ROLES, get_user_permissions, has_studio_read_access, has_studio_write_access STUDIO_VIEW_USERS, STUDIO_EDIT_ROLES, get_user_permissions, has_studio_read_access, has_studio_write_access
) )
from student.roles import CourseInstructorRole, CourseStaffRole, LibraryUserRole from student.roles import CourseInstructorRole, CourseStaffRole, LibraryUserRole, CourseCreatorRole
from util.json_request import expect_json, JsonResponse, JsonResponseBadRequest from util.json_request import expect_json, JsonResponse, JsonResponseBadRequest
__all__ = ['library_handler', 'manage_library_users'] __all__ = ['library_handler', 'manage_library_users']
...@@ -50,6 +50,11 @@ def library_handler(request, library_key_string=None): ...@@ -50,6 +50,11 @@ def library_handler(request, library_key_string=None):
log.exception("Attempted to use the content library API when the libraries feature is disabled.") 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 raise Http404 # Should never happen because we test the feature in urls.py also
if not CourseCreatorRole().has_user(request.user):
if not request.user.is_staff:
return HttpResponseForbidden()
if library_key_string is not None and request.method == 'POST': if library_key_string is not None and request.method == 'POST':
return HttpResponseNotAllowed(("POST",)) return HttpResponseNotAllowed(("POST",))
......
...@@ -16,7 +16,7 @@ from contentstore.courseware_index import CoursewareSearchIndexer, SearchIndexin ...@@ -16,7 +16,7 @@ from contentstore.courseware_index import CoursewareSearchIndexer, SearchIndexin
from contentstore.tests.utils import CourseTestCase from contentstore.tests.utils import CourseTestCase
from contentstore.utils import reverse_course_url, reverse_library_url, add_instructor, reverse_usage_url from contentstore.utils import reverse_course_url, reverse_library_url, add_instructor, reverse_usage_url
from contentstore.views.course import ( from contentstore.views.course import (
course_outline_initial_state, reindex_course_and_check_access, _deprecated_blocks_info, _get_library_creator_status course_outline_initial_state, reindex_course_and_check_access, _deprecated_blocks_info
) )
from contentstore.views.item import create_xblock_info, VisibilityState from contentstore.views.item import create_xblock_info, VisibilityState
from course_action_state.managers import CourseRerunUIStateManager from course_action_state.managers import CourseRerunUIStateManager
...@@ -30,7 +30,6 @@ from xmodule.modulestore import ModuleStoreEnum ...@@ -30,7 +30,6 @@ from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, LibraryFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, LibraryFactory
from student import auth
from student.roles import CourseCreatorRole from student.roles import CourseCreatorRole
...@@ -50,36 +49,6 @@ class TestCourseIndex(CourseTestCase): ...@@ -50,36 +49,6 @@ class TestCourseIndex(CourseTestCase):
display_name='dotted.course.name-2', display_name='dotted.course.name-2',
) )
@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), "disallowed_for_this_site")
@mock.patch("contentstore.views.library.LIBRARIES_ENABLED", True)
def test_library_creator_status_with_is_staff_user(self):
_, staff_user = self.create_staff_authed_user_client()
self.assertEqual(_get_library_creator_status(staff_user), "granted")
@mock.patch("contentstore.views.library.LIBRARIES_ENABLED", True)
def test_library_creator_status_with_disable_library_creation(self):
_, nostaff_user = self.create_non_staff_authed_user_client()
with mock.patch.dict('django.conf.settings.FEATURES', {"DISABLE_LIBRARY_CREATION": True}):
self.assertEqual(_get_library_creator_status(nostaff_user), "disallowed_for_this_site")
@mock.patch("contentstore.views.library.LIBRARIES_ENABLED", True)
def test_library_creator_status_with_course_creator_role(self):
_, staff_user = self.create_staff_authed_user_client()
_, nostaff_user = self.create_non_staff_authed_user_client()
auth.add_users(staff_user, CourseCreatorRole(), nostaff_user)
self.assertEqual(_get_library_creator_status(nostaff_user), "granted")
@mock.patch("contentstore.views.library.LIBRARIES_ENABLED", True)
def test_library_creator_status_with_no_course_creator_role_no_is_staff_no_disable_library_creation(self):
_, nostaff_user = self.create_non_staff_authed_user_client()
with mock.patch.dict('django.conf.settings.FEATURES', {"DISABLE_LIBRARY_CREATION": False}):
self.assertEqual(_get_library_creator_status(nostaff_user), "disallowed_for_this_site")
def check_index_and_outline(self, authed_client): def check_index_and_outline(self, authed_client):
""" """
Test getting the list of courses and then pulling up their outlines Test getting the list of courses and then pulling up their outlines
......
...@@ -5,13 +5,17 @@ More important high-level tests are in contentstore/tests/test_libraries.py ...@@ -5,13 +5,17 @@ More important high-level tests are in contentstore/tests/test_libraries.py
""" """
from contentstore.tests.utils import AjaxEnabledTestClient, parse_json from contentstore.tests.utils import AjaxEnabledTestClient, parse_json
from contentstore.utils import reverse_course_url, reverse_library_url 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 contentstore.views.component import get_component_templates
from contentstore.views.course import _get_library_creator_status
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import LibraryFactory from xmodule.modulestore.tests.factories import LibraryFactory
from mock import patch from mock import patch
from opaque_keys.edx.locator import CourseKey, LibraryLocator from opaque_keys.edx.locator import CourseKey, LibraryLocator
import ddt import ddt
from student.roles import LibraryUserRole import mock
from student.auth import add_users
from student.roles import CourseCreatorRole, LibraryUserRole
LIBRARY_REST_URL = '/library/' # URL for GET/POST requests involving libraries LIBRARY_REST_URL = '/library/' # URL for GET/POST requests involving libraries
...@@ -24,7 +28,7 @@ def make_url_for_lib(key): ...@@ -24,7 +28,7 @@ def make_url_for_lib(key):
@ddt.ddt @ddt.ddt
class UnitTestLibraries(ModuleStoreTestCase): class UnitTestLibraries(CourseTestCase):
""" """
Unit tests for library views Unit tests for library views
""" """
...@@ -38,6 +42,28 @@ class UnitTestLibraries(ModuleStoreTestCase): ...@@ -38,6 +42,28 @@ class UnitTestLibraries(ModuleStoreTestCase):
###################################################### ######################################################
# Tests for /library/ - list and create libraries: # 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}):
add_users(self.user, CourseCreatorRole(), 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), False)
@patch("contentstore.views.library.LIBRARIES_ENABLED", False) @patch("contentstore.views.library.LIBRARIES_ENABLED", False)
def test_with_libraries_disabled(self): def test_with_libraries_disabled(self):
""" """
...@@ -93,7 +119,7 @@ class UnitTestLibraries(ModuleStoreTestCase): ...@@ -93,7 +119,7 @@ class UnitTestLibraries(ModuleStoreTestCase):
self.client.logout() self.client.logout()
ns_user, password = self.create_non_staff_user() ns_user, password = self.create_non_staff_user()
self.client.login(username=ns_user.username, password=password) self.client.login(username=ns_user.username, password=password)
add_users(self.user, CourseCreatorRole(), ns_user)
response = self.client.ajax_post(LIBRARY_REST_URL, { response = self.client.ajax_post(LIBRARY_REST_URL, {
'org': 'org', 'library': 'lib', 'display_name': "New Library", 'org': 'org', 'library': 'lib', 'display_name': "New Library",
}) })
......
...@@ -168,7 +168,6 @@ FEATURES = { ...@@ -168,7 +168,6 @@ FEATURES = {
# Enable support for content libraries. Note that content libraries are # Enable support for content libraries. Note that content libraries are
# only supported in courses using split mongo. # only supported in courses using split mongo.
'ENABLE_CONTENT_LIBRARIES': True, 'ENABLE_CONTENT_LIBRARIES': True,
'DISABLE_LIBRARY_CREATION': False,
# Milestones application flag # Milestones application flag
'MILESTONES_APP': False, 'MILESTONES_APP': False,
......
...@@ -33,8 +33,7 @@ from openedx.core.djangolib.markup import HTML, Text ...@@ -33,8 +33,7 @@ from openedx.core.djangolib.markup import HTML, Text
% elif course_creator_status=='disallowed_for_this_site' and settings.FEATURES.get('STUDIO_REQUEST_EMAIL',''): % elif course_creator_status=='disallowed_for_this_site' and settings.FEATURES.get('STUDIO_REQUEST_EMAIL',''):
<a href="mailto:${settings.FEATURES.get('STUDIO_REQUEST_EMAIL','')}">${_("Email staff to create course")}</a> <a href="mailto:${settings.FEATURES.get('STUDIO_REQUEST_EMAIL','')}">${_("Email staff to create course")}</a>
% endif % endif
% if show_new_library_button:
% if library_creator_status=='granted':
<a href="#" class="button new-button new-library-button"><span class="icon fa fa-plus icon-inline" aria-hidden="true"></span> <a href="#" class="button new-button new-library-button"><span class="icon fa fa-plus icon-inline" aria-hidden="true"></span>
${_("New Library")}</a> ${_("New Library")}</a>
% endif % endif
...@@ -130,7 +129,7 @@ from openedx.core.djangolib.markup import HTML, Text ...@@ -130,7 +129,7 @@ from openedx.core.djangolib.markup import HTML, Text
% endif % endif
%if libraries_enabled and library_creator_status=='granted': %if libraries_enabled and show_new_library_button:
<div class="wrapper-create-element wrapper-create-library"> <div class="wrapper-create-element wrapper-create-library">
<form class="form-create create-library library-info" id="create-library-form" name="create-library-form"> <form class="form-create create-library library-info" id="create-library-form" name="create-library-form">
<div class="wrap-error"> <div class="wrap-error">
...@@ -497,7 +496,7 @@ from openedx.core.djangolib.markup import HTML, Text ...@@ -497,7 +496,7 @@ from openedx.core.djangolib.markup import HTML, Text
</div> </div>
</div> </div>
</div> </div>
% if library_creator_status=='granted': % if show_new_library_button:
<div class="notice-item has-actions"> <div class="notice-item has-actions">
<div class="msg"> <div class="msg">
<h3 class="title">${_('Create Your First Library')}</h3> <h3 class="title">${_('Create Your First Library')}</h3>
......
...@@ -436,22 +436,6 @@ class ModuleStoreTestCase(ModuleStoreIsolationMixin, TestCase): ...@@ -436,22 +436,6 @@ class ModuleStoreTestCase(ModuleStoreIsolationMixin, TestCase):
nonstaff_user.save() nonstaff_user.save()
return nonstaff_user, password return nonstaff_user, password
def create_staff_user(self):
"""
Creates a staff test user.
Returns the staff test user and its password.
"""
uname = 'teststaff'
password = 'bar'
staff_user = User.objects.create_user(uname, 'test+staff@edx.org', password)
# Note that we do not actually need to do anything
# for registration if we directly mark them active.
staff_user.is_active = True
staff_user.is_staff = True
staff_user.save()
return staff_user, password
def update_course(self, course, user_id): def update_course(self, course, user_id):
""" """
Updates the version of course in the modulestore Updates the version of course in the modulestore
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment