Skip to content

Commit 4c8fdb3

Browse files
committed
feat: validate if scope and role exist
1 parent 8708679 commit 4c8fdb3

3 files changed

Lines changed: 101 additions & 7 deletions

File tree

openedx_authz/api/data.py

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
"""Data classes and enums for representing roles, permissions, and policies."""
22

33
import re
4+
from abc import abstractmethod
45
from enum import Enum
56
from typing import ClassVar, Literal, Type
67

78
from attrs import define
89
from opaque_keys import InvalidKeyError
910
from opaque_keys.edx.locator import LibraryLocatorV2
1011

12+
try:
13+
from openedx.core.djangoapps.content_libraries.models import ContentLibrary
14+
except ImportError:
15+
ContentLibrary = None
16+
1117
__all__ = [
1218
"UserData",
1319
"PermissionData",
@@ -302,6 +308,15 @@ def validate_external_key(cls, _: str) -> bool:
302308
"""
303309
return True
304310

311+
@abstractmethod
312+
def exists(self) -> bool:
313+
"""Check if the scope exists.
314+
315+
Returns:
316+
bool: True if the scope exists, False otherwise.
317+
"""
318+
raise NotImplementedError("Subclasses must implement exists method.")
319+
305320

306321
@define
307322
class ContentLibraryData(ScopeData):
@@ -356,6 +371,19 @@ def validate_external_key(cls, external_key: str) -> bool:
356371
except InvalidKeyError:
357372
return False
358373

374+
def exists(self) -> bool:
375+
"""Check if the content library exists.
376+
377+
Returns:
378+
bool: True if the content library exists, False otherwise.
379+
"""
380+
try:
381+
library_key = LibraryLocatorV2.from_string(self.library_id)
382+
ContentLibrary.objects.get_by_key(library_key=library_key)
383+
return True
384+
except ContentLibrary.DoesNotExist:
385+
return False
386+
359387
def __str__(self):
360388
"""Human readable string representation of the content library."""
361389
return self.library_id
@@ -365,6 +393,32 @@ def __repr__(self):
365393
return self.namespaced_key
366394

367395

396+
class CourseData(ScopeData):
397+
"""A course scope for authorization in the Open edX platform.
398+
399+
This class represents a course scope for authorization in the Open edX platform.
400+
"""
401+
402+
NAMESPACE: ClassVar[str] = "course-v1"
403+
404+
@classmethod
405+
def validate_external_key(cls, _: str) -> bool:
406+
"""Validate the external_key format for CourseData.
407+
408+
Args:
409+
external_key: The external key to validate.
410+
"""
411+
return True
412+
413+
def exists(self) -> bool:
414+
"""Check if the course exists.
415+
416+
Returns:
417+
bool: True if the course exists, False otherwise.
418+
"""
419+
return True
420+
421+
368422
class SubjectMeta(type):
369423
"""Metaclass for SubjectData to handle dynamic subclass instantiation based on namespace."""
370424

@@ -581,7 +635,7 @@ def __repr__(self):
581635
return f"{self.action.namespaced_key} => {self.effect}"
582636

583637

584-
@define
638+
@define(eq=False)
585639
class RoleData(AuthZData):
586640
"""A role is a named collection of permissions that can be assigned to subjects.
587641
@@ -610,6 +664,12 @@ class RoleData(AuthZData):
610664
NAMESPACE: ClassVar[str] = "role"
611665
permissions: list[PermissionData] = []
612666

667+
def __eq__(self, other):
668+
"""Compare roles based on their namespaced_key."""
669+
if not isinstance(other, RoleData):
670+
return False
671+
return self.namespaced_key == other.namespaced_key
672+
613673
@property
614674
def name(self) -> str:
615675
"""The human-readable name of the role (e.g., 'Library Admin', 'Course Instructor').

openedx_authz/rest_api/v1/serializers.py

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from django.contrib.auth import get_user_model
44
from rest_framework import serializers
55

6-
from openedx_authz.api.data import RoleAssignmentData
6+
from openedx_authz import api
77
from openedx_authz.rest_api.enums import SortField, SortOrder
88
from openedx_authz.rest_api.v1.fields import CommaSeparatedListField, LowercaseCharField
99

@@ -38,13 +38,48 @@ class PermissionValidationResponseSerializer(PermissionValidationSerializer): #
3838
allowed = serializers.BooleanField()
3939

4040

41-
class AddUsersToRoleWithScopeSerializer(RoleMixin, ScopeMixin): # pylint: disable=abstract-method
41+
class RoleScopeValidationMixin(serializers.Serializer): # pylint: disable=abstract-method
42+
"""Mixin providing role and scope validation logic."""
43+
44+
def validate(self, attrs):
45+
"""Validate that role exists in scope."""
46+
validated_data = super().validate(attrs)
47+
scope_value = validated_data["scope"]
48+
role_value = validated_data["role"]
49+
50+
try:
51+
scope = api.ScopeData(external_key=scope_value)
52+
except ValueError as exc:
53+
raise serializers.ValidationError(exc) from exc
54+
55+
if not scope.exists():
56+
raise serializers.ValidationError(f"Scope '{scope_value}' does not exist")
57+
58+
role = api.RoleData(external_key=role_value)
59+
general_scope = api.ScopeData(namespaced_key=f"{scope.NAMESPACE}{scope.SEPARATOR}*")
60+
role_definitions = api.get_role_definitions_in_scope(general_scope)
61+
62+
if role not in role_definitions:
63+
raise serializers.ValidationError(f"Role '{role_value}' does not exist in scope '{scope_value}'")
64+
65+
return validated_data
66+
67+
68+
class AddUsersToRoleWithScopeSerializer(
69+
RoleScopeValidationMixin,
70+
RoleMixin,
71+
ScopeMixin,
72+
): # pylint: disable=abstract-method
4273
"""Serializer for adding users to a role with a scope."""
4374

4475
users = serializers.ListField(child=serializers.CharField(max_length=255), allow_empty=False)
4576

4677

47-
class RemoveUsersFromRoleWithScopeSerializer(RoleMixin, ScopeMixin): # pylint: disable=abstract-method
78+
class RemoveUsersFromRoleWithScopeSerializer(
79+
RoleScopeValidationMixin,
80+
RoleMixin,
81+
ScopeMixin,
82+
): # pylint: disable=abstract-method
4883
"""Serializer for removing users from a role with a scope."""
4984

5085
users = CommaSeparatedListField(allow_blank=False)
@@ -100,7 +135,7 @@ def _get_user(self, obj) -> User | None:
100135
user_map = self.context.get("user_map", {})
101136
return user_map.get(obj.subject.username)
102137

103-
def get_username(self, obj: RoleAssignmentData) -> str:
138+
def get_username(self, obj: api.RoleAssignmentData) -> str:
104139
"""Get the username for the given role assignment."""
105140
return obj.subject.username
106141

@@ -114,6 +149,6 @@ def get_email(self, obj) -> str:
114149
user = self._get_user(obj)
115150
return getattr(user, "email", "") if user else ""
116151

117-
def get_roles(self, obj: RoleAssignmentData) -> list[str]:
152+
def get_roles(self, obj: api.RoleAssignmentData) -> list[str]:
118153
"""Get the roles for the given role assignment."""
119154
return [role.external_key for role in obj.roles]

openedx_authz/rest_api/v1/views.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,6 @@ def put(self, request: HttpRequest) -> Response:
225225
serializer = AddUsersToRoleWithScopeSerializer(data=request.data)
226226
serializer.is_valid(raise_exception=True)
227227

228-
# TODO: Should we validate that the role or scope exists?
229228
role_name = serializer.validated_data["role"]
230229
scope = serializer.validated_data["scope"]
231230

0 commit comments

Comments
 (0)