Skip to content

Commit d9ec5be

Browse files
MaferMazumariajgrimaldigithub-actions[bot]
authored
[Backport FC-0099] feat: add openedx-authz and update libraries enforcement points (#37633)
* feat: filter libraries based on user-role scopes (#37564) (cherry picked from commit 6c6fc5d) * feat: add openedx-authz to 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 (cherry picked from commit f4f14a6) * feat: Upgrade Python dependency openedx-authz (#37652) * feat: Upgrade Python dependency openedx-authz handle cache invalidation Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` * fix: update the num of queries in tests --------- Co-authored-by: MaferMazu <[email protected]> Co-authored-by: Maria Fernanda Magallanes Zubillaga <[email protected]> (cherry picked from commit 122b4e0) * chore: update requirements to fix the inconsistency --------- Co-authored-by: Maria Grimaldi (Majo) <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 45f94d4 commit d9ec5be

File tree

16 files changed

+1091
-43
lines changed

16 files changed

+1091
-43
lines changed

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

Lines changed: 112 additions & 11 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]:
@@ -267,7 +277,11 @@ def get_libraries_for_user(user, org=None, text_search=None, order=None) -> Quer
267277
Q(learning_package__description__icontains=text_search)
268278
)
269279

270-
filtered = permissions.perms[permissions.CAN_VIEW_THIS_CONTENT_LIBRARY].filter(user, qs)
280+
# Using distinct() temporarily to avoid duplicate results caused by overlapping permission checks
281+
# between Bridgekeeper and the new authorization framework. This ensures correct results for now,
282+
# but it should be removed once Bridgekeeper support is fully dropped and all permission logic
283+
# is handled through openedx-authz.
284+
filtered = permissions.perms[permissions.CAN_VIEW_THIS_CONTENT_LIBRARY].filter(user, qs).distinct()
271285

272286
if order:
273287
order_query = 'learning_package__'
@@ -332,7 +346,7 @@ def require_permission_for_library_key(library_key: LibraryLocatorV2, user: User
332346
library_obj = ContentLibrary.objects.get_by_key(library_key)
333347
# obj should be able to read any valid model object but mypy thinks it can only be
334348
# "User | AnonymousUser | None"
335-
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):
336350
raise PermissionDenied
337351

338352
return library_obj
@@ -750,3 +764,90 @@ def get_backup_task_status(
750764
result['file'] = artifact.file
751765

752766
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/permissions.py

Lines changed: 156 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,12 @@
22
Permissions for Content Libraries (v2, Learning-Core-based)
33
"""
44
from bridgekeeper import perms, rules
5-
from bridgekeeper.rules import Attribute, ManyRelation, Relation, blanket_rule, in_current_groups
5+
from bridgekeeper.rules import Attribute, ManyRelation, Relation, blanket_rule, in_current_groups, Rule
66
from django.conf import settings
7+
from django.db.models import Q
8+
9+
from openedx_authz import api as authz_api
10+
from openedx_authz.constants.permissions import VIEW_LIBRARY
711

812
from openedx.core.djangoapps.content_libraries.models import ContentLibraryPermission
913

@@ -54,6 +58,154 @@ def is_course_creator(user):
5458

5559
return get_course_creator_status(user) == 'granted'
5660

61+
62+
class HasPermissionInContentLibraryScope(Rule):
63+
"""Bridgekeeper rule that checks content library permissions via the openedx-authz system.
64+
65+
This rule integrates the openedx-authz authorization system (backed by Casbin) with
66+
Bridgekeeper's declarative permission system. It checks if a user has been granted a
67+
specific permission (action) through their role assignments in the authorization system.
68+
69+
The rule works by:
70+
1. Querying the authorization system to find library scopes where the user has this permission
71+
2. Parsing the library keys (org/slug) from the scopes
72+
3. Building database filters to match ContentLibrary models with those org/slug combinations
73+
74+
Attributes:
75+
permission (PermissionData): The permission object representing the action to check
76+
(e.g., 'view', 'edit'). This is used to look up scopes in the authorization system.
77+
78+
filter_keys (list[str]): The Django model fields to use when building QuerySet filters.
79+
Defaults to ['org', 'slug'] for ContentLibrary models.
80+
81+
These fields are used to construct the Q object filters that match libraries
82+
based on the parsed components from library keys in authorization scopes.
83+
84+
For ContentLibrary, library keys have the format 'lib:ORG:SLUG', which maps to:
85+
- 'org' -> filters on org__short_name (related Organization model)
86+
- 'slug' -> filters on slug field
87+
88+
If filtering by different fields is needed, pass a custom list. For example:
89+
- ['org', 'slug'] - default for ContentLibrary (filters by org and slug)
90+
- ['id'] - filter by primary key (for other models)
91+
92+
Examples:
93+
Basic usage with default filter_keys:
94+
>>> from bridgekeeper import perms
95+
>>> from openedx.core.djangoapps.content_libraries.permissions import HasPermissionInContentLibraryScope
96+
>>>
97+
>>> # Uses default filter_keys=['org', 'slug'] for ContentLibrary
98+
>>> can_view = HasPermissionInContentLibraryScope('view_library')
99+
>>> perms['libraries.view_library'] = can_view
100+
101+
Compound permissions with boolean operators:
102+
>>> from bridgekeeper.rules import Attribute
103+
>>>
104+
>>> is_active = Attribute('is_active', True)
105+
>>> is_staff = Attribute('is_staff', True)
106+
>>> can_view = HasPermissionInContentLibraryScope('view_library')
107+
>>>
108+
>>> # User must be active AND (staff OR have explicit permission)
109+
>>> perms['libraries.view_library'] = is_active & (is_staff | can_view)
110+
111+
QuerySet filtering (efficient, database-level):
112+
>>> from openedx.core.djangoapps.content_libraries.models import ContentLibrary
113+
>>>
114+
>>> # Gets all libraries user can view in a single SQL query
115+
>>> visible_libraries = perms['libraries.view_library'].filter(
116+
... request.user,
117+
... ContentLibrary.objects.all()
118+
... )
119+
120+
Individual object checks:
121+
>>> library = ContentLibrary.objects.get(org__short_name='DemoX', slug='CSPROB')
122+
>>> if perms['libraries.view_library'].check(request.user, library):
123+
... # User can view this specific library
124+
125+
Note:
126+
The library keys in authorization scopes must have the format 'lib:ORG:SLUG'
127+
to match the ContentLibrary model's org.short_name and slug fields.
128+
For example, scope 'lib:DemoX:CSPROB' matches a library with
129+
org.short_name='DemoX' and slug='CSPROB'.
130+
"""
131+
132+
def __init__(self, permission: authz_api.PermissionData, filter_keys: list[str] | None = None):
133+
"""Initialize the rule with the action and filter keys to filter on.
134+
135+
Args:
136+
permission (PermissionData): The permission to check (e.g., 'view', 'edit').
137+
filter_keys (list[str]): The model fields to filter on when building QuerySet filters.
138+
Defaults to ['org', 'slug'] for ContentLibrary.
139+
"""
140+
self.permission = permission
141+
self.filter_keys = filter_keys if filter_keys is not None else ["org", "slug"]
142+
143+
def query(self, user):
144+
"""Convert this rule to a Django Q object for QuerySet filtering.
145+
146+
Args:
147+
user: The Django user object (must have a 'username' attribute).
148+
149+
Returns:
150+
Q: A Django Q object that can be used to filter a QuerySet.
151+
The Q object combines multiple conditions using OR (|) operators,
152+
where each condition matches a library's org and slug fields:
153+
Q(org__short_name='OrgA' & slug='lib-a') | Q(org__short_name='OrgB' & slug='lib-b')
154+
155+
Example:
156+
>>> # User has 'view' permission in scopes: ['lib:OrgA:lib-a', 'lib:OrgB:lib-b']
157+
>>> rule = HasPermissionInContentLibraryScope('view', filter_keys=['org', 'slug'])
158+
>>> q = rule.query(user)
159+
>>> # Results in: Q(org__short_name='OrgA', slug='lib-a') | Q(org__short_name='OrgB', slug='lib-b')
160+
>>>
161+
>>> # Apply to queryset
162+
>>> libraries = ContentLibrary.objects.filter(q)
163+
>>> # SQL: SELECT * FROM content_library
164+
>>> # WHERE (org.short_name='OrgA' AND slug='lib-a')
165+
>>> # OR (org.short_name='OrgB' AND slug='lib-b')
166+
"""
167+
scopes = authz_api.get_scopes_for_user_and_permission(
168+
user.username,
169+
self.permission.identifier
170+
)
171+
172+
library_keys = [scope.library_key for scope in scopes]
173+
174+
if not library_keys:
175+
return Q(pk__in=[]) # No access, return Q that matches nothing
176+
177+
# Build Q object: OR together (org AND slug) conditions for each library
178+
query = Q()
179+
for library_key in library_keys:
180+
query |= Q(org__short_name=library_key.org, slug=library_key.slug)
181+
182+
return query
183+
184+
def check(self, user, instance, *args, **kwargs): # pylint: disable=arguments-differ
185+
"""Check if user has permission for a specific object instance.
186+
187+
This method is used for checking permission on individual objects rather
188+
than filtering a QuerySet. It extracts the scope from the object and
189+
checks if the user has the required permission in that scope via Casbin.
190+
191+
Args:
192+
user: The Django user object (must have a 'username' attribute).
193+
instance: The Django model instance to check permission for.
194+
*args: Additional positional arguments (for compatibility with parent signature).
195+
**kwargs: Additional keyword arguments (for compatibility with parent signature).
196+
197+
Returns:
198+
bool: True if the user has the permission in the object's scope,
199+
False otherwise.
200+
201+
Example:
202+
>>> rule = HasPermissionInContentLibraryScope('view')
203+
>>> can_view = rule.check(user, library)
204+
>>> # Checks if user has 'view' permission in scope 'lib:DemoX:CSPROB'
205+
"""
206+
return authz_api.is_user_allowed(user.username, self.permission.identifier, str(instance.library_key))
207+
208+
57209
########################### Permissions ###########################
58210

59211
# Is the user allowed to view XBlocks from the specified content library
@@ -87,7 +239,9 @@ def is_course_creator(user):
87239
is_global_staff |
88240
# Libraries with "public read" permissions can be accessed only by course creators
89241
(Attribute('allow_public_read', True) & is_course_creator) |
90-
# Otherwise the user must be part of the library's team
242+
# Users can access libraries within their authorized scope (via Casbin/role-based permissions)
243+
HasPermissionInContentLibraryScope(VIEW_LIBRARY) |
244+
# Fallback to: the user must be part of the library's team (legacy permission system)
91245
has_explicit_read_permission_for_library
92246
)
93247

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({})

0 commit comments

Comments
 (0)