Skip to content

Commit

Permalink
fix: content libraries permissions
Browse files Browse the repository at this point in the history
  • Loading branch information
rpenido committed Dec 3, 2024
1 parent 7ac03a2 commit 7a446a2
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 28 deletions.
15 changes: 12 additions & 3 deletions openedx/core/djangoapps/content_libraries/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ def is_studio_request(_):
return settings.SERVICE_VARIANT == "cms"


@blanket_rule
def is_course_creator(user):
from cms.djangoapps.course_creators.views import get_course_creator_status

return get_course_creator_status(user) == 'granted'

########################### Permissions ###########################

# Is the user allowed to view XBlocks from the specified content library
Expand All @@ -68,16 +74,19 @@ def is_studio_request(_):

# Is the user allowed to create content libraries?
CAN_CREATE_CONTENT_LIBRARY = 'content_libraries.create_library'
perms[CAN_CREATE_CONTENT_LIBRARY] = is_user_active
if settings.FEATURES.get('ENABLE_CREATOR_GROUP', False):
perms[CAN_CREATE_CONTENT_LIBRARY] = is_global_staff | (is_user_active & is_course_creator)
else:
perms[CAN_CREATE_CONTENT_LIBRARY] = is_global_staff

# Is the user allowed to view the specified content library in Studio,
# including to view the raw OLX and asset files?
CAN_VIEW_THIS_CONTENT_LIBRARY = 'content_libraries.view_library'
perms[CAN_VIEW_THIS_CONTENT_LIBRARY] = is_user_active & (
# Global staff can access any library
is_global_staff |
# Some libraries allow anyone to view them in Studio:
Attribute('allow_public_read', True) |
# Libraries with "public read" permissions can be accessed only by course creators
(Attribute('allow_public_read', True) & is_course_creator) |
# Otherwise the user must be part of the library's team
has_explicit_read_permission_for_library
)
Expand Down
2 changes: 1 addition & 1 deletion openedx/core/djangoapps/content_libraries/tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class ContentLibrariesRestApiTest(APITransactionTestCase):

def setUp(self):
super().setUp()
self.user = UserFactory.create(username="Bob", email="[email protected]", password="edx")
self.user = UserFactory.create(username="Bob", email="[email protected]", password="edx", is_staff=True)
# Create an organization
self.organization, _ = Organization.objects.get_or_create(
short_name="CL-TEST",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,8 @@ def test_library_blocks(self): # pylint: disable=too-many-statements
Tests with some non-ASCII chars in slugs, titles, descriptions.
"""
admin = UserFactory.create(username="Admin", email="[email protected]", is_staff=True)

lib = self._create_library(slug="téstlꜟط", title="A Tést Lꜟطrary", description="Tésting XBlocks")
lib_id = lib["id"]
assert lib['has_unpublished_changes'] is False
Expand Down Expand Up @@ -531,7 +533,7 @@ def test_library_permissions(self): # pylint: disable=too-many-statements
Learning Core data models.
"""
# Create a few users to use for all of these tests:
admin = UserFactory.create(username="Admin", email="[email protected]")
admin = UserFactory.create(username="Admin", email="[email protected]", is_staff=True)
author = UserFactory.create(username="Author", email="[email protected]")
reader = UserFactory.create(username="Reader", email="[email protected]")
group = Group.objects.create(name="group1")
Expand Down Expand Up @@ -653,14 +655,15 @@ def test_library_permissions(self): # pylint: disable=too-many-statements
self._get_library_block_asset(block3_key, file_name="static/whatever.png", expect_response=403)
# Nor can they preview the block:
self._render_block_view(block3_key, view_name="student_view", expect_response=403)
# But if we grant allow_public_read, then they can:
# Even if we grant allow_public_read, then they can't:
with self.as_user(admin):
self._update_library(lib_id, allow_public_read=True)
self._set_library_block_asset(block3_key, "static/whatever.png", b"data")
with self.as_user(random_user):
self._get_library_block_olx(block3_key)
self._get_library_block_olx(block3_key, expect_response=403)
self._get_library_block_fields(block3_key, expect_response=403)
# But he can preview the block:
self._render_block_view(block3_key, view_name="student_view")
f = self._get_library_block_fields(block3_key)
# self._get_library_block_assets(block3_key)
# self._get_library_block_asset(block3_key, file_name="whatever.png")

Expand Down Expand Up @@ -702,7 +705,7 @@ def test_no_lockout(self):
"""
Test that administrators cannot be removed if they are the only administrator granted access.
"""
admin = UserFactory.create(username="Admin", email="[email protected]")
admin = UserFactory.create(username="Admin", email="[email protected]", is_staff=True)
successor = UserFactory.create(username="Successor", email="[email protected]")
with self.as_user(admin):
lib = self._create_library(slug="permtest", title="Permission Test Library", description="Testing")
Expand Down Expand Up @@ -1026,7 +1029,7 @@ def test_library_paste_clipboard(self):
from openedx.core.djangoapps.content_staging.api import save_xblock_to_user_clipboard

# Create user to perform tests on
author = UserFactory.create(username="Author", email="[email protected]")
author = UserFactory.create(username="Author", email="[email protected]", is_staff=True)
with self.as_user(author):
lib = self._create_library(
slug="test_lib_paste_clipboard",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import ddt
from django.contrib.auth import get_user_model
from django.core.files.uploadedfile import SimpleUploadedFile
from edx_django_utils.cache import RequestCache
from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator, LibraryCollectionLocator
from openedx_tagging.core.tagging.models import Tag, Taxonomy
from openedx_tagging.core.tagging.models.system_defined import SystemDefinedTaxonomy
Expand Down Expand Up @@ -289,8 +290,8 @@ def setUp(self):
self._setUp_taxonomies()
self._setUp_collection()

# Clear the rules cache in between test runs to keep query counts consistent.
rules_cache.clear()
# Clear all request caches in between test runs to keep query counts consistent.
RequestCache.clear_all_namespaces()


@skip_unless_cms
Expand Down Expand Up @@ -510,12 +511,12 @@ def test_create_taxonomy(self, user_attr: str, expected_status: int) -> None:

@ddt.data(
('staff', 11),
("content_creatorA", 16),
("library_staffA", 16),
("library_userA", 16),
("instructorA", 16),
("course_instructorA", 16),
("course_staffA", 16),
("content_creatorA", 17),
("library_staffA", 17),
("library_userA", 17),
("instructorA", 17),
("course_instructorA", 17),
("course_staffA", 17),
)
@ddt.unpack
def test_list_taxonomy_query_count(self, user_attr: str, expected_queries: int):
Expand Down Expand Up @@ -1879,16 +1880,16 @@ def test_get_copied_tags(self):
('staff', 'courseA', 8),
('staff', 'libraryA', 8),
('staff', 'collection_key', 8),
("content_creatorA", 'courseA', 11, False),
("content_creatorA", 'libraryA', 11, False),
("content_creatorA", 'collection_key', 11, False),
("library_staffA", 'libraryA', 11, False), # Library users can only view objecttags, not change them?
("library_staffA", 'collection_key', 11, False),
("library_userA", 'libraryA', 11, False),
("library_userA", 'collection_key', 11, False),
("instructorA", 'courseA', 11),
("course_instructorA", 'courseA', 11),
("course_staffA", 'courseA', 11),
("content_creatorA", 'courseA', 12, False),
("content_creatorA", 'libraryA', 12, False),
("content_creatorA", 'collection_key', 12, False),
("library_staffA", 'libraryA', 12, False), # Library users can only view objecttags, not change them?
("library_staffA", 'collection_key', 12, False),
("library_userA", 'libraryA', 12, False),
("library_userA", 'collection_key', 12, False),
("instructorA", 'courseA', 12),
("course_instructorA", 'courseA', 12),
("course_staffA", 'courseA', 12),
)
@ddt.unpack
def test_object_tags_query_count(
Expand Down

0 comments on commit 7a446a2

Please sign in to comment.