Skip to content

Commit

Permalink
Merge pull request from GHSA-3q74-3rfh-g37j
Browse files Browse the repository at this point in the history
Co-authored-by: Jesper Hodge <[email protected]>
  • Loading branch information
Feanil Patel and jesperhodge authored Jul 25, 2023
1 parent c540709 commit 5d3eca4
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 22 deletions.
31 changes: 29 additions & 2 deletions cms/djangoapps/contentstore/views/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@
CourseInstructorRole,
CourseStaffRole,
GlobalStaff,
UserBasedRole
UserBasedRole,
OrgStaffRole
)
from common.djangoapps.util.date_utils import get_default_time_display
from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json
Expand Down Expand Up @@ -113,8 +114,8 @@
reverse_library_url,
reverse_url,
reverse_usage_url,
update_course_discussions_settings,
update_course_details,
update_course_discussions_settings,
)
from .component import ADVANCED_COMPONENT_TYPES
from ..helpers import is_content_creator
Expand Down Expand Up @@ -604,6 +605,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),
})

Expand Down Expand Up @@ -631,6 +633,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)
Expand Down Expand Up @@ -1829,13 +1832,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.
Expand Down
14 changes: 8 additions & 6 deletions cms/djangoapps/contentstore/views/library.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import create_xblock_info
from .user import user_with_role

Expand Down Expand Up @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions cms/djangoapps/contentstore/views/tests/test_course_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
96 changes: 90 additions & 6 deletions cms/djangoapps/contentstore/views/tests/test_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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()
Expand All @@ -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'},
Expand Down Expand Up @@ -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'])
5 changes: 5 additions & 0 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down
5 changes: 4 additions & 1 deletion cms/envs/devstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,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'
Expand Down
2 changes: 1 addition & 1 deletion cms/templates/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down

0 comments on commit 5d3eca4

Please sign in to comment.