Skip to content

Commit f4f14a6

Browse files
authored
[FC-0099] feat: add openedx-authz to library apis user_can_create_library and require_permission_for_library_key (#37501)
* feat: add the authz check to the library api function feat: add the authz publish check in rest_api blocks and containers feat: add the authz checks in libraries and refactor feat: add collections checks feat: update enforcement in serializer file refactor: refactor the permission check functions fix: fix value error fix: calling the queries twice * test: add structure for test and apply feedback refactor: refactor the tests and apply feedback fix: apply feedback Revert "refactor: refactor the tests and apply feedback" This reverts commit aa0bd52. refactor: use constants and avoid mapping test: fix the test to have them in order docs: about we rely on bridgekeeper and the old check for two cases docs: update openedx/core/djangoapps/content_libraries/api/libraries.py Co-authored-by: Maria Grimaldi (Majo) <[email protected]> refactor: use global scope wildcard instead of * refactor: allow receiving PermissionData objects refactor: do not inherit from BaseRolesTestCase to favor CL setup methods If both BaseRolesTestCase and ContentLibrariesRestApiTest define a method with the same name (e.g., setUp()), Python will use the one found first in the MRO, which is the one in BaseRolesTestCase because it is listed first in the class definition leading to unexpected behavior. refactor: remove unnecessary imports and indent * chore: bump openedx-authz version
1 parent 6c6fc5d commit f4f14a6

File tree

12 files changed

+447
-22
lines changed

12 files changed

+447
-22
lines changed

openedx/core/djangoapps/content_libraries/api/libraries.py

Lines changed: 107 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,11 @@
5353
from django.db import IntegrityError, transaction
5454
from django.db.models import Q, QuerySet
5555
from django.utils.translation import gettext as _
56-
from opaque_keys.edx.locator import (
57-
LibraryLocatorV2,
58-
LibraryUsageLocatorV2,
59-
)
60-
from openedx_events.content_authoring.data import (
61-
ContentLibraryData,
62-
)
56+
from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
57+
from openedx_authz import api as authz_api
58+
from openedx_authz.api import assign_role_to_user_in_scope
59+
from openedx_authz.constants import permissions as authz_permissions
60+
from openedx_events.content_authoring.data import ContentLibraryData
6361
from openedx_events.content_authoring.signals import (
6462
CONTENT_LIBRARY_CREATED,
6563
CONTENT_LIBRARY_DELETED,
@@ -70,14 +68,14 @@
7068
from organizations.models import Organization
7169
from user_tasks.models import UserTaskArtifact, UserTaskStatus
7270
from xblock.core import XBlock
73-
from openedx_authz.api import assign_role_to_user_in_scope
7471

7572
from openedx.core.types import User as UserType
7673

7774
from .. import permissions, tasks
7875
from ..constants import ALL_RIGHTS_RESERVED
7976
from ..models import ContentLibrary, ContentLibraryPermission
8077
from .exceptions import LibraryAlreadyExists, LibraryPermissionIntegrityError
78+
from .permissions import LEGACY_LIB_PERMISSIONS
8179

8280
log = logging.getLogger(__name__)
8381

@@ -109,6 +107,7 @@
109107
"revert_changes",
110108
"get_backup_task_status",
111109
"assign_library_role_to_user",
110+
"user_has_permission_across_lib_authz_systems",
112111
]
113112

114113

@@ -245,7 +244,18 @@ def user_can_create_library(user: AbstractUser) -> bool:
245244
"""
246245
Check if the user has permission to create a content library.
247246
"""
248-
return user.has_perm(permissions.CAN_CREATE_CONTENT_LIBRARY)
247+
library_permission = permissions.CAN_CREATE_CONTENT_LIBRARY
248+
lib_permission_in_authz = _transform_legacy_lib_permission_to_authz_permission(library_permission)
249+
# The authz_api.is_user_allowed check only validates permissions within a specific library context. Since
250+
# creating a library is not tied to an existing one, we use user.has_perm (via Bridgekeeper) to check if the user
251+
# can create libraries, meaning they have the course creator role. In the future, this should rely on a global (*)
252+
# role defined in the Authorization Framework for instance-level resource creation.
253+
has_perms = user.has_perm(library_permission) or authz_api.is_user_allowed(
254+
user,
255+
lib_permission_in_authz,
256+
authz_api.data.GLOBAL_SCOPE_WILDCARD,
257+
)
258+
return has_perms
249259

250260

251261
def get_libraries_for_user(user, org=None, text_search=None, order=None) -> QuerySet[ContentLibrary]:
@@ -336,7 +346,7 @@ def require_permission_for_library_key(library_key: LibraryLocatorV2, user: User
336346
library_obj = ContentLibrary.objects.get_by_key(library_key)
337347
# obj should be able to read any valid model object but mypy thinks it can only be
338348
# "User | AnonymousUser | None"
339-
if not user.has_perm(permission, obj=library_obj): # type:ignore[arg-type]
349+
if not user_has_permission_across_lib_authz_systems(user, permission, library_obj):
340350
raise PermissionDenied
341351

342352
return library_obj
@@ -754,3 +764,90 @@ def get_backup_task_status(
754764
result['file'] = artifact.file
755765

756766
return result
767+
768+
769+
def _transform_legacy_lib_permission_to_authz_permission(permission: str) -> str:
770+
"""
771+
Transform a legacy content library permission to an openedx-authz permission.
772+
"""
773+
# There is no dedicated permission or role for can_create_content_library in openedx-authz yet,
774+
# so we reuse the same permission to rely on user.has_perm via Bridgekeeper.
775+
return {
776+
permissions.CAN_CREATE_CONTENT_LIBRARY: permissions.CAN_CREATE_CONTENT_LIBRARY,
777+
permissions.CAN_DELETE_THIS_CONTENT_LIBRARY: authz_permissions.DELETE_LIBRARY.identifier,
778+
permissions.CAN_EDIT_THIS_CONTENT_LIBRARY: authz_permissions.EDIT_LIBRARY_CONTENT.identifier,
779+
permissions.CAN_EDIT_THIS_CONTENT_LIBRARY_TEAM: authz_permissions.MANAGE_LIBRARY_TEAM.identifier,
780+
permissions.CAN_VIEW_THIS_CONTENT_LIBRARY: authz_permissions.VIEW_LIBRARY.identifier,
781+
permissions.CAN_VIEW_THIS_CONTENT_LIBRARY_TEAM: authz_permissions.VIEW_LIBRARY_TEAM.identifier,
782+
}.get(permission, permission)
783+
784+
785+
def _transform_authz_permission_to_legacy_lib_permission(permission: str) -> str:
786+
"""
787+
Transform an openedx-authz permission to a legacy content library permission.
788+
"""
789+
return {
790+
authz_permissions.PUBLISH_LIBRARY_CONTENT.identifier: permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
791+
authz_permissions.CREATE_LIBRARY_COLLECTION.identifier: permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
792+
authz_permissions.EDIT_LIBRARY_COLLECTION.identifier: permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
793+
authz_permissions.DELETE_LIBRARY_COLLECTION.identifier: permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
794+
}.get(permission, permission)
795+
796+
797+
def user_has_permission_across_lib_authz_systems(
798+
user: UserType,
799+
permission: str | authz_api.data.PermissionData,
800+
library_obj: ContentLibrary,
801+
) -> bool:
802+
"""
803+
Check whether a user has a given permission on a content library across both the
804+
legacy edx-platform permission system and the newer openedx-authz system.
805+
806+
The provided permission name is normalized to both systems (legacy and authz), and
807+
authorization is granted if either:
808+
- the user holds the legacy object-level permission on the ContentLibrary instance, or
809+
- the openedx-authz API allows the user for the corresponding permission on the library.
810+
811+
**Note:**
812+
Temporary: this function uses Bridgekeeper-based logic for cases not yet modeled in openedx-authz.
813+
814+
Current gaps covered here:
815+
- CAN_CREATE_CONTENT_LIBRARY: we call user.has_perm via Bridgekeeper to verify the user is a course creator.
816+
- CAN_VIEW_THIS_CONTENT_LIBRARY: we respect the allow_public_read flag via Bridgekeeper.
817+
818+
Replace these with authz_api.is_user_allowed once openedx-authz supports
819+
these conditions natively (including global (*) roles).
820+
821+
Args:
822+
user: The Django user (or user-like object) to check.
823+
permission: The permission identifier (either a legacy codename or an openedx-authz name).
824+
library_obj: The ContentLibrary instance to check against.
825+
826+
Returns:
827+
bool: True if the user is authorized by either system; otherwise False.
828+
"""
829+
if isinstance(permission, authz_api.data.PermissionData):
830+
permission = permission.identifier
831+
if _is_legacy_permission(permission):
832+
legacy_permission = permission
833+
authz_permission = _transform_legacy_lib_permission_to_authz_permission(permission)
834+
else:
835+
authz_permission = permission
836+
legacy_permission = _transform_authz_permission_to_legacy_lib_permission(permission)
837+
return (
838+
# Check both the legacy and the new openedx-authz permissions
839+
user.has_perm(perm=legacy_permission, obj=library_obj)
840+
or authz_api.is_user_allowed(
841+
user,
842+
authz_permission,
843+
str(library_obj.library_key),
844+
)
845+
)
846+
847+
848+
def _is_legacy_permission(permission: str) -> bool:
849+
"""
850+
Determine if the specified library permission is part of the legacy
851+
or the new openedx-authz system.
852+
"""
853+
return permission in LEGACY_LIB_PERMISSIONS

openedx/core/djangoapps/content_libraries/api/permissions.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,13 @@
1212
CAN_VIEW_THIS_CONTENT_LIBRARY,
1313
CAN_VIEW_THIS_CONTENT_LIBRARY_TEAM
1414
)
15+
16+
LEGACY_LIB_PERMISSIONS = frozenset({
17+
CAN_CREATE_CONTENT_LIBRARY,
18+
CAN_DELETE_THIS_CONTENT_LIBRARY,
19+
CAN_EDIT_THIS_CONTENT_LIBRARY,
20+
CAN_EDIT_THIS_CONTENT_LIBRARY_TEAM,
21+
CAN_LEARN_FROM_THIS_CONTENT_LIBRARY,
22+
CAN_VIEW_THIS_CONTENT_LIBRARY,
23+
CAN_VIEW_THIS_CONTENT_LIBRARY_TEAM,
24+
})

openedx/core/djangoapps/content_libraries/rest_api/blocks.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from django.utils.decorators import method_decorator
1010
from drf_yasg.utils import swagger_auto_schema
1111
from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
12+
from openedx_authz.constants import permissions as authz_permissions
1213
from openedx_learning.api import authoring as authoring_api
1314
from rest_framework import status
1415
from rest_framework.exceptions import NotFound, ValidationError
@@ -238,7 +239,7 @@ def post(self, request, usage_key_str):
238239
api.require_permission_for_library_key(
239240
key.lib_key,
240241
request.user,
241-
permissions.CAN_EDIT_THIS_CONTENT_LIBRARY
242+
authz_permissions.PUBLISH_LIBRARY_CONTENT
242243
)
243244
api.publish_component_changes(key, request.user)
244245
return Response({})

openedx/core/djangoapps/content_libraries/rest_api/collections.py

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from rest_framework.status import HTTP_204_NO_CONTENT
1414

1515
from opaque_keys.edx.locator import LibraryLocatorV2
16+
from openedx_authz.constants import permissions as authz_permissions
1617
from openedx_learning.api import authoring as authoring_api
1718
from openedx_learning.api.authoring_models import Collection
1819

@@ -56,7 +57,6 @@ def get_content_library(self) -> ContentLibrary:
5657
if self.request.method in ['OPTIONS', 'GET']
5758
else permissions.CAN_EDIT_THIS_CONTENT_LIBRARY
5859
)
59-
6060
self._content_library = api.require_permission_for_library_key(
6161
library_key,
6262
self.request.user,
@@ -110,6 +110,11 @@ def create(self, request: RestRequest, *args, **kwargs) -> Response:
110110
Create a Collection that belongs to a Content Library
111111
"""
112112
content_library = self.get_content_library()
113+
api.require_permission_for_library_key(
114+
content_library.library_key,
115+
request.user,
116+
authz_permissions.CREATE_LIBRARY_COLLECTION
117+
)
113118
create_serializer = ContentLibraryCollectionUpdateSerializer(data=request.data)
114119
create_serializer.is_valid(raise_exception=True)
115120

@@ -144,6 +149,11 @@ def partial_update(self, request: RestRequest, *args, **kwargs) -> Response:
144149
Update a Collection that belongs to a Content Library
145150
"""
146151
content_library = self.get_content_library()
152+
api.require_permission_for_library_key(
153+
content_library.library_key,
154+
request.user,
155+
authz_permissions.EDIT_LIBRARY_COLLECTION
156+
)
147157
collection_key = kwargs["key"]
148158

149159
update_serializer = ContentLibraryCollectionUpdateSerializer(
@@ -165,6 +175,12 @@ def destroy(self, request: RestRequest, *args, **kwargs) -> Response:
165175
"""
166176
Soft-deletes a Collection that belongs to a Content Library
167177
"""
178+
content_library = self.get_content_library()
179+
api.require_permission_for_library_key(
180+
content_library.library_key,
181+
request.user,
182+
authz_permissions.DELETE_LIBRARY_COLLECTION
183+
)
168184
collection = super().get_object()
169185
assert collection.learning_package_id
170186
authoring_api.delete_collection(
@@ -181,6 +197,11 @@ def restore(self, request: RestRequest, *args, **kwargs) -> Response:
181197
Restores a soft-deleted Collection that belongs to a Content Library
182198
"""
183199
content_library = self.get_content_library()
200+
api.require_permission_for_library_key(
201+
content_library.library_key,
202+
request.user,
203+
authz_permissions.EDIT_LIBRARY_COLLECTION
204+
)
184205
assert content_library.learning_package_id
185206
collection_key = kwargs["key"]
186207
authoring_api.restore_collection(
@@ -198,6 +219,11 @@ def update_items(self, request: RestRequest, *args, **kwargs) -> Response:
198219
Collection and items must all be part of the given library/learning package.
199220
"""
200221
content_library = self.get_content_library()
222+
api.require_permission_for_library_key(
223+
content_library.library_key,
224+
request.user,
225+
authz_permissions.EDIT_LIBRARY_COLLECTION
226+
)
201227
collection_key = kwargs["key"]
202228

203229
serializer = ContentLibraryItemKeysSerializer(data=request.data)

openedx/core/djangoapps/content_libraries/rest_api/containers.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from drf_yasg import openapi
1313

1414
from opaque_keys.edx.locator import LibraryLocatorV2, LibraryContainerLocator
15+
from openedx_authz.constants import permissions as authz_permissions
1516
from openedx_learning.api import authoring as authoring_api
1617
from rest_framework.generics import GenericAPIView
1718
from rest_framework.response import Response
@@ -379,7 +380,7 @@ def post(self, request: RestRequest, container_key: LibraryContainerLocator) ->
379380
api.require_permission_for_library_key(
380381
container_key.lib_key,
381382
request.user,
382-
permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
383+
authz_permissions.PUBLISH_LIBRARY_CONTENT
383384
)
384385
api.publish_container_changes(container_key, request.user.id)
385386
# If we need to in the future, we could return a list of all the child containers/components that were

openedx/core/djangoapps/content_libraries/rest_api/libraries.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
from user_tasks.models import UserTaskStatus
8383

8484
from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
85+
from openedx_authz.constants import permissions as authz_permissions
8586
from organizations.api import ensure_organization
8687
from organizations.exceptions import InvalidOrganizationException
8788
from organizations.models import Organization
@@ -219,7 +220,7 @@ def post(self, request):
219220
"""
220221
Create a new content library.
221222
"""
222-
if not request.user.has_perm(permissions.CAN_CREATE_CONTENT_LIBRARY):
223+
if not api.user_can_create_library(request.user):
223224
raise PermissionDenied
224225
serializer = ContentLibraryMetadataSerializer(data=request.data)
225226
serializer.is_valid(raise_exception=True)
@@ -479,7 +480,11 @@ def post(self, request, lib_key_str):
479480
descendants.
480481
"""
481482
key = LibraryLocatorV2.from_string(lib_key_str)
482-
api.require_permission_for_library_key(key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY)
483+
api.require_permission_for_library_key(
484+
key,
485+
request.user,
486+
authz_permissions.PUBLISH_LIBRARY_CONTENT
487+
)
483488
api.publish_changes(key, request.user.id)
484489
return Response({})
485490

@@ -838,7 +843,7 @@ def post(self, request):
838843
"""
839844
Restore a library from a backup file.
840845
"""
841-
if not request.user.has_perm(permissions.CAN_CREATE_CONTENT_LIBRARY):
846+
if not api.user_can_create_library(request.user):
842847
raise PermissionDenied
843848

844849
serializer = LibraryRestoreFileSerializer(data=request.data)

openedx/core/djangoapps/content_libraries/rest_api/serializers.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from user_tasks.models import UserTaskStatus
1515

1616
from openedx.core.djangoapps.content_libraries.tasks import LibraryRestoreTask
17+
from openedx.core.djangoapps.content_libraries import api
1718
from openedx.core.djangoapps.content_libraries.api.containers import ContainerType
1819
from openedx.core.djangoapps.content_libraries.constants import ALL_RIGHTS_RESERVED, LICENSE_OPTIONS
1920
from openedx.core.djangoapps.content_libraries.models import (
@@ -75,7 +76,8 @@ def get_can_edit_library(self, obj):
7576
return False
7677

7778
library_obj = ContentLibrary.objects.get_by_key(obj.key)
78-
return user.has_perm(permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, obj=library_obj)
79+
return api.user_has_permission_across_lib_authz_systems(
80+
user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, library_obj)
7981

8082

8183
class ContentLibraryUpdateSerializer(serializers.Serializer):

0 commit comments

Comments
 (0)