Skip to content

Commit 0accbde

Browse files
committed
squash!: Support specifying multiple admin view and manage permissions on scope data
1 parent 7583ced commit 0accbde

6 files changed

Lines changed: 259 additions & 86 deletions

File tree

openedx_authz/api/data.py

Lines changed: 68 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -355,31 +355,37 @@ def validate_external_key(cls, _: str) -> bool:
355355

356356
@classmethod
357357
@abstractmethod
358-
def get_admin_view_permission(cls) -> PermissionData:
359-
"""Get the permission required to view this scope
358+
def get_admin_view_permissions(cls) -> list[PermissionData]:
359+
"""Get the permissions required to view this scope.
360360
361361
This method should be implemented on every ScopeData subclass to define
362-
which permission to check against when a user tries to see assignations
362+
which permissions to check against when a user tries to see assignations
363363
related to this scope in the Admin Console.
364364
365+
The consumer uses OR logic: if the user has any one of the returned
366+
permissions, access is granted. An empty list means access is denied.
367+
365368
Returns:
366-
PermissionData: The permission required to view this scope in the admin console.
369+
list[PermissionData]: The permissions required to view this scope in the admin console.
367370
"""
368-
raise NotImplementedError("Subclasses must implement get_admin_view_permission method.")
371+
raise NotImplementedError("Subclasses must implement get_admin_view_permissions method.")
369372

370373
@classmethod
371374
@abstractmethod
372-
def get_admin_manage_permission(cls) -> PermissionData:
373-
"""Get the permission required to manage this scope
375+
def get_admin_manage_permissions(cls) -> list[PermissionData]:
376+
"""Get the permissions required to manage this scope.
374377
375378
This method should be implemented on every ScopeData subclass to define
376-
which permission to check against when a user tries to manage assignations
379+
which permissions to check against when a user tries to manage assignations
377380
related to this scope in the Admin Console.
378381
382+
The consumer uses OR logic: if the user has any one of the returned
383+
permissions, access is granted. An empty list means access is denied.
384+
379385
Returns:
380-
PermissionData: The permission required to manage this scope in the admin console.
386+
list[PermissionData]: The permissions required to manage this scope in the admin console.
381387
"""
382-
raise NotImplementedError("Subclasses must implement get_admin_manage_permission method.")
388+
raise NotImplementedError("Subclasses must implement get_admin_manage_permissions method.")
383389

384390
@abstractmethod
385391
def get_object(self) -> Any | None:
@@ -452,28 +458,28 @@ def validate_external_key(cls, external_key: str) -> bool:
452458
return external_key == GLOBAL_SCOPE_WILDCARD
453459

454460
@classmethod
455-
def get_admin_view_permission(cls) -> PermissionData:
456-
"""No admin view permission for the global scope.
461+
def get_admin_view_permissions(cls) -> list[PermissionData]:
462+
"""Get the permissions required to view the global scope.
457463
458-
Global scope assignments are managed exclusively by superadmins
459-
at the REST API layer, so no granular permission is needed.
464+
Since the global scope spans all resource types, a user needs any one
465+
of the scope-specific view permissions to be granted access.
460466
461467
Returns:
462-
None
468+
list[PermissionData]: VIEW_LIBRARY_TEAM or COURSES_VIEW_COURSE_TEAM (OR logic).
463469
"""
464-
return VIEW_LIBRARY_TEAM
470+
return [VIEW_LIBRARY_TEAM, COURSES_VIEW_COURSE_TEAM]
465471

466472
@classmethod
467-
def get_admin_manage_permission(cls) -> PermissionData:
468-
"""No admin manage permission for the global scope.
473+
def get_admin_manage_permissions(cls) -> list[PermissionData]:
474+
"""No manage permissions for the global scope.
469475
470-
Global scope assignments are managed exclusively by superadmins
471-
at the REST API layer, so no granular permission is needed.
476+
Global scope management is restricted to superadmins at the REST API
477+
layer, so no granular permission grants access.
472478
473479
Returns:
474-
None
480+
list[PermissionData]: Empty list — access is always denied via permissions.
475481
"""
476-
return None
482+
return []
477483

478484
def get_object(self) -> None:
479485
"""The global wildcard scope does not map to a concrete domain object.
@@ -566,22 +572,22 @@ def validate_external_key(cls, external_key: str) -> bool:
566572
return False
567573

568574
@classmethod
569-
def get_admin_view_permission(cls) -> PermissionData:
570-
"""Get the permission required to view this scope
575+
def get_admin_view_permissions(cls) -> list[PermissionData]:
576+
"""Get the permissions required to view this scope.
571577
572578
Returns:
573-
PermissionData: The permission required to view this scope in the admin console.
579+
list[PermissionData]: The permissions required to view this scope in the admin console.
574580
"""
575-
return VIEW_LIBRARY_TEAM
581+
return [VIEW_LIBRARY_TEAM]
576582

577583
@classmethod
578-
def get_admin_manage_permission(cls) -> PermissionData:
579-
"""Get the permission required to manage this scope
584+
def get_admin_manage_permissions(cls) -> list[PermissionData]:
585+
"""Get the permissions required to manage this scope.
580586
581587
Returns:
582-
PermissionData: The permission required to manage this scope in the admin console.
588+
list[PermissionData]: The permissions required to manage this scope in the admin console.
583589
"""
584-
return MANAGE_LIBRARY_TEAM
590+
return [MANAGE_LIBRARY_TEAM]
585591

586592
def get_object(self) -> ContentLibrary | None:
587593
"""Retrieve the ContentLibrary instance associated with this scope.
@@ -697,22 +703,22 @@ def validate_external_key(cls, external_key: str) -> bool:
697703
return False
698704

699705
@classmethod
700-
def get_admin_view_permission(cls) -> PermissionData:
701-
"""Get the permission required to view this scope
706+
def get_admin_view_permissions(cls) -> list[PermissionData]:
707+
"""Get the permissions required to view this scope.
702708
703709
Returns:
704-
PermissionData: The permission required to view this scope in the admin console.
710+
list[PermissionData]: The permissions required to view this scope in the admin console.
705711
"""
706-
return COURSES_VIEW_COURSE_TEAM
712+
return [COURSES_VIEW_COURSE_TEAM]
707713

708714
@classmethod
709-
def get_admin_manage_permission(cls) -> PermissionData:
710-
"""Get the permission required to manage this scope
715+
def get_admin_manage_permissions(cls) -> list[PermissionData]:
716+
"""Get the permissions required to manage this scope.
711717
712718
Returns:
713-
PermissionData: The permission required to manage this scope in the admin console.
719+
list[PermissionData]: The permissions required to manage this scope in the admin console.
714720
"""
715-
return COURSES_MANAGE_COURSE_TEAM
721+
return [COURSES_MANAGE_COURSE_TEAM]
716722

717723
def get_object(self) -> CourseOverview | None:
718724
"""Retrieve the CourseOverview instance associated with this scope.
@@ -821,13 +827,13 @@ def validate_external_key(cls, external_key: str) -> bool:
821827
return True
822828

823829
@classmethod
824-
def get_admin_view_permission(cls) -> PermissionData:
825-
"""Get the permission required to view this scope
830+
def get_admin_view_permissions(cls) -> list[PermissionData]:
831+
"""Get the permissions required to view this scope.
826832
827833
Returns:
828-
PermissionData: The permission required to view this scope in the admin console.
834+
list[PermissionData]: The permissions required to view this scope in the admin console.
829835
"""
830-
raise NotImplementedError("Subclasses must implement get_admin_view_permission method.")
836+
raise NotImplementedError("Subclasses must implement get_admin_view_permissions method.")
831837

832838
@classmethod
833839
def build_external_key(cls, org: str) -> str:
@@ -848,13 +854,13 @@ def build_external_key(cls, org: str) -> str:
848854
return f"{cls.NAMESPACE}{EXTERNAL_KEY_SEPARATOR}{org}{cls.ID_SEPARATOR}{GLOBAL_SCOPE_WILDCARD}"
849855

850856
@classmethod
851-
def get_admin_manage_permission(cls) -> PermissionData:
852-
"""Get the permission required to manage this scope
857+
def get_admin_manage_permissions(cls) -> list[PermissionData]:
858+
"""Get the permissions required to manage this scope.
853859
854860
Returns:
855-
PermissionData: The permission required to manage this scope in the admin console.
861+
list[PermissionData]: The permissions required to manage this scope in the admin console.
856862
"""
857-
raise NotImplementedError("Subclasses must implement get_admin_manage_permission method.")
863+
raise NotImplementedError("Subclasses must implement get_admin_manage_permissions method.")
858864

859865
@classmethod
860866
def get_org(cls, external_key: str) -> str | None:
@@ -946,22 +952,22 @@ class OrgContentLibraryGlobData(OrgGlobData):
946952
ID_SEPARATOR: ClassVar[str] = ":"
947953

948954
@classmethod
949-
def get_admin_view_permission(cls) -> PermissionData:
950-
"""Get the permission required to view this scope
955+
def get_admin_view_permissions(cls) -> list[PermissionData]:
956+
"""Get the permissions required to view this scope.
951957
952958
Returns:
953-
PermissionData: The permission required to view this scope in the admin console.
959+
list[PermissionData]: The permissions required to view this scope in the admin console.
954960
"""
955-
return VIEW_LIBRARY_TEAM
961+
return [VIEW_LIBRARY_TEAM]
956962

957963
@classmethod
958-
def get_admin_manage_permission(cls) -> PermissionData:
959-
"""Get the permission required to manage this scope
964+
def get_admin_manage_permissions(cls) -> list[PermissionData]:
965+
"""Get the permissions required to manage this scope.
960966
961967
Returns:
962-
PermissionData: The permission required to manage this scope in the admin console.
968+
list[PermissionData]: The permissions required to manage this scope in the admin console.
963969
"""
964-
return MANAGE_LIBRARY_TEAM
970+
return [MANAGE_LIBRARY_TEAM]
965971

966972

967973
@define
@@ -1004,22 +1010,22 @@ class OrgCourseOverviewGlobData(OrgGlobData):
10041010
ID_SEPARATOR: ClassVar[str] = "+"
10051011

10061012
@classmethod
1007-
def get_admin_view_permission(cls) -> PermissionData:
1008-
"""Get the permission required to view this scope
1013+
def get_admin_view_permissions(cls) -> list[PermissionData]:
1014+
"""Get the permissions required to view this scope.
10091015
10101016
Returns:
1011-
PermissionData: The permission required to view this scope in the admin console.
1017+
list[PermissionData]: The permissions required to view this scope in the admin console.
10121018
"""
1013-
return COURSES_VIEW_COURSE_TEAM
1019+
return [COURSES_VIEW_COURSE_TEAM]
10141020

10151021
@classmethod
1016-
def get_admin_manage_permission(cls) -> PermissionData:
1017-
"""Get the permission required to manage this scope
1022+
def get_admin_manage_permissions(cls) -> list[PermissionData]:
1023+
"""Get the permissions required to manage this scope.
10181024
10191025
Returns:
1020-
PermissionData: The permission required to manage this scope in the admin console.
1026+
list[PermissionData]: The permissions required to manage this scope in the admin console.
10211027
"""
1022-
return COURSES_MANAGE_COURSE_TEAM
1028+
return [COURSES_MANAGE_COURSE_TEAM]
10231029

10241030

10251031
class CCXCourseOverviewData(CourseOverviewData):

openedx_authz/api/users.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -272,21 +272,24 @@ def _filter_allowed_assignments(
272272
) -> list[RoleAssignmentData]:
273273
"""
274274
Filter the given role assignments to only include those that the user has permission to view.
275+
276+
Uses OR logic: if the user has any one of the scope's admin view permissions, the
277+
assignment is included.
275278
"""
276279
if not user_external_key:
277280
# If no user is specified, return all assignments
278281
return assignments
279282
allowed_assignments: list[RoleAssignmentData] = []
280283
for assignment in assignments:
281-
permission = None
282-
283-
# Get the permission needed to view the specific scope in the admin console
284-
permission = assignment.scope.get_admin_view_permission().identifier
284+
view_permissions = assignment.scope.get_admin_view_permissions()
285285

286-
if permission and is_user_allowed(
287-
user_external_key=user_external_key,
288-
action_external_key=permission,
289-
scope_external_key=assignment.scope.external_key,
286+
if view_permissions and any(
287+
is_user_allowed(
288+
user_external_key=user_external_key,
289+
action_external_key=perm.identifier,
290+
scope_external_key=assignment.scope.external_key,
291+
)
292+
for perm in view_permissions
290293
):
291294
allowed_assignments.append(assignment)
292295

openedx_authz/rest_api/v1/views.py

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -781,7 +781,7 @@ def _get_allowed_scope_queryset(
781781
username: str,
782782
scope_cls: type,
783783
glob_cls: type,
784-
get_permission: callable,
784+
get_permissions: callable,
785785
queryset_builder: callable,
786786
extract_ids: callable,
787787
search: str = "",
@@ -790,15 +790,15 @@ def _get_allowed_scope_queryset(
790790
"""Resolve allowed scopes from Casbin and return a filtered queryset.
791791
792792
This helper encapsulates the shared pattern of:
793-
1. Fetching allowed scopes for a user and permission.
793+
1. Fetching allowed scopes for a user across any of the scope's permissions (OR logic).
794794
2. Partitioning them into specific IDs vs org-level globs.
795795
3. Delegating to the appropriate queryset builder.
796796
797797
Args:
798798
username: The username to check permissions for.
799799
scope_cls: The concrete scope data class (e.g., CourseOverviewData).
800800
glob_cls: The org-level glob class (e.g., OrgCourseOverviewGlobData).
801-
get_permission: Callable that returns the permission for a scope class.
801+
get_permissions: Callable that returns a list of permissions for a scope class.
802802
queryset_builder: Callable that builds the filtered queryset (e.g., _get_courses_queryset).
803803
extract_ids: Callable that extracts specific IDs from non-glob scopes.
804804
search: Optional search term to filter by display name.
@@ -807,10 +807,19 @@ def _get_allowed_scope_queryset(
807807
Returns:
808808
QuerySet: The filtered queryset projected to the unified scope shape.
809809
"""
810-
allowed_scopes = get_scopes_for_user_and_permission(username, get_permission(scope_cls).identifier)
811-
specific_scopes = [s for s in allowed_scopes if not isinstance(s, glob_cls)]
810+
# Collect allowed scopes across all permissions (OR logic)
811+
all_allowed_scopes = []
812+
seen = set()
813+
for perm in get_permissions(scope_cls):
814+
for scope in get_scopes_for_user_and_permission(username, perm.identifier):
815+
key = scope.namespaced_key
816+
if key not in seen:
817+
seen.add(key)
818+
all_allowed_scopes.append(scope)
819+
820+
specific_scopes = [s for s in all_allowed_scopes if not isinstance(s, glob_cls)]
812821
allowed_ids = extract_ids(specific_scopes)
813-
allowed_orgs = {s.org for s in allowed_scopes if isinstance(s, glob_cls)}
822+
allowed_orgs = {s.org for s in all_allowed_scopes if isinstance(s, glob_cls)}
814823
return queryset_builder(allowed_ids, allowed_orgs, search=search, org=org)
815824

816825
def _build_queryset(self, courses_qs: QuerySet | None, libraries_qs: QuerySet | None) -> QuerySet:
@@ -852,9 +861,11 @@ def get_queryset(self) -> QuerySet:
852861

853862
management_only = params_serializer.validated_data["management_permission_only"]
854863

855-
# Determine which permission to check based on the query parameter.
856-
def get_permission(scope_cls):
857-
return scope_cls.get_admin_manage_permission() if management_only else scope_cls.get_admin_view_permission()
864+
# Determine which permissions to check based on the query parameter.
865+
def get_permissions(scope_cls):
866+
return (
867+
scope_cls.get_admin_manage_permissions() if management_only else scope_cls.get_admin_view_permissions()
868+
)
858869

859870
# Resolve allowed scopes from Casbin and build filtered querysets.
860871
courses_qs = None
@@ -863,7 +874,7 @@ def get_permission(scope_cls):
863874
username=user.username,
864875
scope_cls=CourseOverviewData,
865876
glob_cls=OrgCourseOverviewGlobData,
866-
get_permission=get_permission,
877+
get_permissions=get_permissions,
867878
queryset_builder=self._get_courses_queryset,
868879
extract_ids=lambda scopes: {s.external_key for s in scopes},
869880
search=search,
@@ -876,7 +887,7 @@ def get_permission(scope_cls):
876887
username=user.username,
877888
scope_cls=ContentLibraryData,
878889
glob_cls=OrgContentLibraryGlobData,
879-
get_permission=get_permission,
890+
get_permissions=get_permissions,
880891
queryset_builder=self._get_libraries_queryset,
881892
extract_ids=lambda scopes: {
882893
(s.external_key.split(":")[1], s.external_key.split(":")[2]) for s in scopes

openedx_authz/tests/api/test_data.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ def test_scope_validate_external_key(self, external_key, expected_valid, expecte
370370
"unknown:DemoX:*",
371371
)
372372
def test_get_subclass_by_external_key_unknown_scope_raises_value_error(self, external_key):
373-
"""Inknown namespace should raise ValueError, including wildcard keys."""
373+
"""Unknown namespace should raise ValueError, including wildcard keys."""
374374
with self.assertRaises(ValueError):
375375
ScopeMeta.get_subclass_by_external_key(external_key)
376376

@@ -410,7 +410,7 @@ def exists(self) -> bool:
410410
return False
411411

412412
@classmethod
413-
def get_admin_view_permission(cls):
413+
def get_admin_view_permissions(cls):
414414
raise NotImplementedError("Not implemented for TempScope")
415415

416416
# Metaclass should have recreated the registries on the class

0 commit comments

Comments
 (0)