Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security fix GHSA-3q74-3rfh-g37j #32838

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 28 additions & 1 deletion cms/djangoapps/contentstore/views/course.py
Expand Up @@ -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
Expand Down Expand Up @@ -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),
})

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -1943,13 +1946,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
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 .block 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
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
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
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
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion cms/templates/index.html
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