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

Feanil/backporting #35180

Merged
merged 5 commits into from
Jul 25, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1670,7 +1670,7 @@ def get_home_context(request, no_course=False):
LIBRARY_AUTHORING_MICROFRONTEND_URL,
LIBRARIES_ENABLED,
should_redirect_to_library_authoring_mfe,
user_can_create_library,
user_can_view_create_library_button,
)

active_courses = []
Expand Down Expand Up @@ -1699,7 +1699,8 @@ def get_home_context(request, no_course=False):
'library_authoring_mfe_url': LIBRARY_AUTHORING_MICROFRONTEND_URL,
'taxonomy_list_mfe_url': get_taxonomy_list_url(),
'libraries': libraries,
'show_new_library_button': user_can_create_library(user) and not should_redirect_to_library_authoring_mfe(),
'show_new_library_button': user_can_view_create_library_button(user)
and not should_redirect_to_library_authoring_mfe(),
'user': user,
'request_course_creator_url': reverse('request_course_creator'),
'course_creator_status': _get_course_creator_status(user),
Expand Down
60 changes: 51 additions & 9 deletions cms/djangoapps/contentstore/views/library.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from django.conf import settings
from django.contrib.auth.decorators import login_required
from django.core.exceptions import PermissionDenied
from django.http import Http404, HttpResponseForbidden, HttpResponseNotAllowed
from django.http import Http404, HttpResponseNotAllowed
from django.utils.translation import gettext as _
from django.views.decorators.csrf import ensure_csrf_cookie
from django.views.decorators.http import require_http_methods
Expand Down Expand Up @@ -69,12 +69,10 @@ def should_redirect_to_library_authoring_mfe():
)


def user_can_create_library(user, org=None):
def user_can_view_create_library_button(user):
"""
Helper method for returning the library creation status for a particular user,
taking into account the value LIBRARIES_ENABLED.
Helper method for displaying the visibilty of the create_library_button.
"""

if not LIBRARIES_ENABLED:
return False
elif user.is_staff:
Expand All @@ -84,8 +82,56 @@ def user_can_create_library(user, org=None):
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()
has_course_admin_role = UserBasedRole(user=user, role=CourseInstructorRole.ROLE).courses_with_role().exists()
return is_course_creator or has_org_staff_role or has_course_staff_role or has_course_admin_role
else:
# EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present.
disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None)
disable_course_creation = settings.FEATURES.get('DISABLE_COURSE_CREATION', False)
if disable_library_creation is not None:
return not disable_library_creation
else:
return not disable_course_creation


def user_can_create_library(user, org):
"""
Helper method for returning the library creation status for a particular user,
taking into account the value LIBRARIES_ENABLED.

if the ENABLE_CREATOR_GROUP value is False, then any user can create a library (in any org),
if library creation is enabled.

if the ENABLE_CREATOR_GROUP value is true, then what a user can do varies by thier role.

Global Staff: can make libraries in any org.
Course Creator Group Members: can make libraries in any org.
Organization Staff: Can make libraries in the organization for which they are staff.
Course Staff: Can make libraries in the organization which has courses of which they are staff.
Course Admin: Can make libraries in the organization which has courses of which they are Admin.
"""
if org is None:
return False
if not LIBRARIES_ENABLED:
return False
elif user.is_staff:
return True
if settings.FEATURES.get('ENABLE_CREATOR_GROUP', False):
is_course_creator = get_course_creator_status(user) == 'granted'
has_org_staff_role = org in OrgStaffRole().get_orgs_for_user(user)
has_course_staff_role = (
UserBasedRole(user=user, role=CourseStaffRole.ROLE)
.courses_with_role()
.filter(org=org)
.exists()
)
has_course_admin_role = (
UserBasedRole(user=user, role=CourseInstructorRole.ROLE)
.courses_with_role()
.filter(org=org)
.exists()
)
return is_course_creator or has_org_staff_role or has_course_staff_role or has_course_admin_role

else:
# EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present.
disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None)
Expand All @@ -108,12 +154,8 @@ def library_handler(request, library_key_string=None):
raise Http404 # Should never happen because we test the feature in urls.py also

if request.method == 'POST':
if not user_can_create_library(request.user):
return HttpResponseForbidden()

if library_key_string is not None:
return HttpResponseNotAllowed(("POST",))

return _create_library(request)

else:
Expand Down
39 changes: 25 additions & 14 deletions cms/djangoapps/contentstore/views/tests/test_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,55 +59,66 @@ def setUp(self):
@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)
self.assertEqual(user_can_create_library(nostaff_user, None), 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)
self.assertEqual(user_can_create_library(nostaff_user, 'An Org'), 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)
self.assertEqual(user_can_create_library(nostaff_user, None), False)

# Global staff can create libraries
# Global staff can create libraries for any org, even ones that don't exist.
@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)
print(self.user.is_staff)
self.assertEqual(user_can_create_library(self.user, 'aNyOrg'), True)

# When creator groups are enabled, global staff can create libraries
# Global staff can create libraries for any org, but an org has to be supplied.
@mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True)
def test_library_creator_status_with_is_staff_user_no_org(self):
print(self.user.is_staff)
self.assertEqual(user_can_create_library(self.user, None), False)

# When creator groups are enabled, global staff can create libraries in any org
@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)
self.assertEqual(user_can_create_library(self.user, 'RandomOrg'), True)

# When creator groups are enabled, course creators can create libraries
# When creator groups are enabled, course creators can create libraries in any org.
@mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True)
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)
self.assertEqual(user_can_create_library(nostaff_user, 'soMeRandOmoRg'), True)

# When creator groups are enabled, course staff members can create libraries
# but only in the org they are course staff for.
@mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True)
def test_library_creator_status_with_course_staff_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}):
auth.add_users(self.user, CourseStaffRole(self.course.id), nostaff_user)
self.assertEqual(user_can_create_library(nostaff_user), True)
self.assertEqual(user_can_create_library(nostaff_user, self.course.org), True)
self.assertEqual(user_can_create_library(nostaff_user, 'SomEOtherOrg'), False)

# When creator groups are enabled, course instructor members can create libraries
# but only in the org they are course staff for.
@mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True)
def test_library_creator_status_with_course_instructor_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}):
auth.add_users(self.user, CourseInstructorRole(self.course.id), nostaff_user)
self.assertEqual(user_can_create_library(nostaff_user), True)
self.assertEqual(user_can_create_library(nostaff_user, self.course.org), True)
self.assertEqual(user_can_create_library(nostaff_user, 'SomEOtherOrg'), False)

@ddt.data(
(False, False, True),
Expand All @@ -131,7 +142,7 @@ def test_library_creator_status_settings(self, disable_course, disable_library,
"DISABLE_LIBRARY_CREATION": disable_library
}
):
self.assertEqual(user_can_create_library(nostaff_user), expected_status)
self.assertEqual(user_can_create_library(nostaff_user, 'SomEOrg'), expected_status)

@mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_COURSE_CREATION': True})
@mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True)
Expand All @@ -140,7 +151,7 @@ def test_library_creator_status_with_no_course_creator_role_and_disabled_nonstaf
Ensure that `DISABLE_COURSE_CREATION` feature works with libraries as well.
"""
nostaff_client, nostaff_user = self.create_non_staff_authed_user_client()
self.assertFalse(user_can_create_library(nostaff_user))
self.assertFalse(user_can_create_library(nostaff_user, 'SomEOrg'))

# To be explicit, this user can GET, but not POST
get_response = nostaff_client.get_json(LIBRARY_REST_URL)
Expand Down Expand Up @@ -251,7 +262,7 @@ def test_lib_create_permission_course_staff_role(self):
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",
'org': self.course.org, 'library': 'lib', 'display_name': "New Library",
})
self.assertEqual(response.status_code, 200)

Expand Down
11 changes: 10 additions & 1 deletion common/djangoapps/util/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from django.utils.translation import gettext as _
from django.utils.translation import ngettext
from pytz import UTC
from storages.backends.s3boto3 import S3Boto3Storage


class FileValidationException(Exception):
Expand All @@ -23,7 +24,7 @@ class FileValidationException(Exception):


def store_uploaded_file(
request, file_key, allowed_file_types, base_storage_filename, max_file_size, validator=None,
request, file_key, allowed_file_types, base_storage_filename, max_file_size, validator=None, is_private=False,
):
"""
Stores an uploaded file to django file storage.
Expand All @@ -45,6 +46,8 @@ def store_uploaded_file(
a `FileValidationException` if the file is not properly formatted. If any exception is thrown, the stored
file will be deleted before the exception is re-raised. Note that the implementor of the validator function
should take care to close the stored file if they open it for reading.
is_private (Boolean): an optional boolean which if True and the storage backend is S3,
sets the ACL for the file object to be private.

Returns:
Storage: the file storage object where the file can be retrieved from
Expand Down Expand Up @@ -75,6 +78,12 @@ def store_uploaded_file(
file_storage = DefaultStorage()
# If a file already exists with the supplied name, file_storage will make the filename unique.
stored_file_name = file_storage.save(stored_file_name, uploaded_file)
if is_private and settings.DEFAULT_FILE_STORAGE == 'storages.backends.s3boto3.S3Boto3Storage':
S3Boto3Storage().connection.meta.client.put_object_acl(
ACL='private',
Bucket=settings.AWS_STORAGE_BUCKET_NAME,
Key=stored_file_name,
)

if validator:
try:
Expand Down
3 changes: 2 additions & 1 deletion lms/djangoapps/instructor/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1603,7 +1603,8 @@ def post(self, request, course_key_string):
request, 'uploaded-file', ['.csv'],
course_and_time_based_filename_generator(course_key, 'cohorts'),
max_file_size=2000000, # limit to 2 MB
validator=_cohorts_csv_validator
validator=_cohorts_csv_validator,
is_private=True
)
task_api.submit_cohort_students(request, course_key, file_name)
except (FileValidationException, ValueError) as e:
Expand Down
Loading