Skip to content

Commit bc99706

Browse files
committed
refactor: refactor the permission check functions
1 parent 767beb4 commit bc99706

7 files changed

Lines changed: 75 additions & 53 deletions

File tree

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

Lines changed: 55 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
from ..constants import ALL_RIGHTS_RESERVED
7474
from ..models import ContentLibrary, ContentLibraryPermission
7575
from .exceptions import LibraryAlreadyExists, LibraryPermissionIntegrityError
76+
from .permissions import LEGACY_LIB_PERMISSIONS
7677

7778
log = logging.getLogger(__name__)
7879

@@ -103,8 +104,7 @@
103104
"publish_changes",
104105
"revert_changes",
105106
"get_backup_task_status",
106-
"require_authz_lib_permission",
107-
"user_has_permission_in_library",
107+
"user_has_permission_across_lib_authz_systems",
108108
]
109109

110110

@@ -236,7 +236,7 @@ def user_can_create_library(user: AbstractUser) -> bool:
236236
Check if the user has permission to create a content library.
237237
"""
238238
library_permission = permissions.CAN_CREATE_CONTENT_LIBRARY
239-
lib_permission_in_authz = _transform_lib_permission_to_authz_permission(library_permission)
239+
lib_permission_in_authz = _transform_legacy_lib_permission_to_authz_permission(library_permission)
240240
has_perms = user.has_perm(library_permission) or authz_api.is_user_allowed(
241241
user,
242242
lib_permission_in_authz,
@@ -329,15 +329,7 @@ def require_permission_for_library_key(library_key: LibraryLocatorV2, user: User
329329
library_obj = ContentLibrary.objects.get_by_key(library_key)
330330
# obj should be able to read any valid model object but mypy thinks it can only be
331331
# "User | AnonymousUser | None"
332-
authz_permission = _transform_lib_permission_to_authz_permission(permission)
333-
if not (
334-
user.has_perm(permission, obj=library_obj)
335-
or authz_api.is_user_allowed(
336-
user,
337-
authz_permission,
338-
str(library_key),
339-
)
340-
): # type:ignore[arg-type]
332+
if not user_has_permission_across_lib_authz_systems(user, permission, library_key):
341333
raise PermissionDenied
342334

343335
return library_obj
@@ -732,9 +724,9 @@ def get_backup_task_status(
732724
return result
733725

734726

735-
def _transform_lib_permission_to_authz_permission(permission: str) -> str:
727+
def _transform_legacy_lib_permission_to_authz_permission(permission: str) -> str:
736728
"""
737-
Transform a content library permission to an openedx-authz permission.
729+
Transform a legacy content library permission to an openedx-authz permission.
738730
"""
739731
mapping = {
740732
permissions.CAN_CREATE_CONTENT_LIBRARY: 'create_library',
@@ -747,45 +739,65 @@ def _transform_lib_permission_to_authz_permission(permission: str) -> str:
747739
return mapping.get(permission, permission)
748740

749741

750-
def require_authz_lib_permission(
751-
library_key: LibraryLocatorV2,
752-
user: UserType,
753-
permission: str
754-
) -> ContentLibrary:
742+
def _transform_authz_permission_to_legacy_lib_permission(permission: str) -> str:
755743
"""
756-
This function checks the new permissions if needed using openedx-authz and also checks
757-
for the old ones to maintain compatibility.
744+
Transform an openedx-authz permission to a legacy content library permission.
758745
"""
759-
# Check the new permission along with the edit permission (the old one).
760-
# This should apply only for publish, and crud over collections.
761-
library_obj = ContentLibrary.objects.get_by_key(library_key)
762-
if not (
763-
user.has_perm(permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, obj=library_obj)
764-
or authz_api.is_user_allowed(
765-
user,
766-
permission,
767-
str(library_key),
768-
)
769-
):
770-
raise PermissionDenied
771-
return library_obj
746+
mapping = {
747+
'publish_library_content': permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
748+
'create_library_collection': permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
749+
'edit_library_collection': permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
750+
'delete_library_collection': permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
751+
}
752+
return mapping.get(permission, permission)
772753

773-
def user_has_permission_in_library(
774-
library_key: LibraryLocatorV2,
754+
755+
def user_has_permission_across_lib_authz_systems(
775756
user: UserType,
776-
permission: str
757+
permission: str,
758+
library_key: LibraryLocatorV2,
777759
) -> bool:
778760
"""
779-
This function checks if the user has the specified permission in the library.
761+
Check whether a user has a given permission on a content library across both the
762+
legacy edx-platform permission system and the newer openedx-authz system.
763+
764+
The provided permission name is normalized to both systems (legacy and authz), and
765+
authorization is granted if either:
766+
- the user holds the legacy object-level permission on the ContentLibrary instance, or
767+
- the openedx-authz API allows the user for the corresponding permission on the library.
768+
769+
Args:
770+
user: The Django user (or user-like object) to check.
771+
permission: The permission identifier (either a legacy codename or an openedx-authz name).
772+
library_key: The LibraryLocatorV2 identifying the target content library.
773+
774+
Returns:
775+
bool: True if the user is authorized by either system; otherwise False.
776+
777+
Raises:
778+
ContentLibrary.DoesNotExist: If a library does not exist for the given key.
780779
"""
781780
library_obj = ContentLibrary.objects.get_by_key(library_key)
782-
# Identify if the permission is old or for authz (in this moment we are using old, for the serializer)
783-
permission_in_authz = _transform_lib_permission_to_authz_permission(permission)
781+
if _is_legacy_permission(permission):
782+
legacy_permission = permission
783+
authz_permission = _transform_legacy_lib_permission_to_authz_permission(permission)
784+
else:
785+
authz_permission = permission
786+
legacy_permission = _transform_authz_permission_to_legacy_lib_permission(permission)
784787
return (
785-
user.has_perm(permission, obj=library_obj)
788+
# Check both the legacy and the new openedx-authz permissions
789+
user.has_perm(perm=legacy_permission, obj=library_obj)
786790
or authz_api.is_user_allowed(
787791
user,
788-
permission_in_authz,
792+
authz_permission,
789793
str(library_key),
790794
)
791-
)
795+
)
796+
797+
798+
def _is_legacy_permission(permission: str) -> bool:
799+
"""
800+
Determine if the specified library permission is part of the legacy
801+
or the new openedx-authz system.
802+
"""
803+
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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ def post(self, request, usage_key_str):
235235
Publish the draft changes made to this component.
236236
"""
237237
key = LibraryUsageLocatorV2.from_string(usage_key_str)
238-
api.require_authz_lib_permission(
238+
api.require_permission_for_library_key(
239239
key.lib_key,
240240
request.user,
241241
'publish_library_content'

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ def create(self, request: RestRequest, *args, **kwargs) -> Response:
104104
Create a Collection that belongs to a Content Library
105105
"""
106106
content_library = self.get_content_library()
107-
api.require_authz_lib_permission(
107+
api.require_permission_for_library_key(
108108
content_library.library_key,
109109
request.user,
110110
'create_library_collection'
@@ -143,7 +143,7 @@ def partial_update(self, request: RestRequest, *args, **kwargs) -> Response:
143143
Update a Collection that belongs to a Content Library
144144
"""
145145
content_library = self.get_content_library()
146-
api.require_authz_lib_permission(
146+
api.require_permission_for_library_key(
147147
content_library.library_key,
148148
request.user,
149149
'edit_library_collection'
@@ -170,7 +170,7 @@ def destroy(self, request: RestRequest, *args, **kwargs) -> Response:
170170
Soft-deletes a Collection that belongs to a Content Library
171171
"""
172172
content_library = self.get_content_library()
173-
api.require_authz_lib_permission(
173+
api.require_permission_for_library_key(
174174
content_library.library_key,
175175
request.user,
176176
'delete_library_collection'
@@ -191,7 +191,7 @@ def restore(self, request: RestRequest, *args, **kwargs) -> Response:
191191
Restores a soft-deleted Collection that belongs to a Content Library
192192
"""
193193
content_library = self.get_content_library()
194-
api.require_authz_lib_permission(
194+
api.require_permission_for_library_key(
195195
content_library.library_key,
196196
request.user,
197197
'edit_library_collection'
@@ -213,7 +213,7 @@ def update_items(self, request: RestRequest, *args, **kwargs) -> Response:
213213
Collection and items must all be part of the given library/learning package.
214214
"""
215215
content_library = self.get_content_library()
216-
api.require_authz_lib_permission(
216+
api.require_permission_for_library_key(
217217
content_library.library_key,
218218
request.user,
219219
'edit_library_collection'

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ def post(self, request: RestRequest, container_key: LibraryContainerLocator) ->
376376
"""
377377
Publish the container and its children
378378
"""
379-
api.require_authz_lib_permission(
379+
api.require_permission_for_library_key(
380380
container_key.lib_key,
381381
request.user,
382382
'publish_library_content'

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ def post(self, request, lib_key_str):
473473
descendants.
474474
"""
475475
key = LibraryLocatorV2.from_string(lib_key_str)
476-
api.require_authz_lib_permission(
476+
api.require_permission_for_library_key(
477477
key,
478478
request.user,
479479
'publish_library_content'

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ def get_can_edit_library(self, obj):
7676
return False
7777

7878
library_obj = ContentLibrary.objects.get_by_key(obj.key)
79-
return api.user_has_permission_in_library(
80-
library_obj.library_key, user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY)
79+
return api.user_has_permission_across_lib_authz_systems(
80+
user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, library_obj.library_key)
8181

8282

8383
class ContentLibraryUpdateSerializer(serializers.Serializer):

0 commit comments

Comments
 (0)