Skip to content

Commit 2d5ab84

Browse files
committed
refactor: extract scope/role validation into reusable helper in RoleScopeValidationMixin
1 parent 9d7c26b commit 2d5ab84

1 file changed

Lines changed: 28 additions & 33 deletions

File tree

openedx_authz/rest_api/v1/serializers.py

Lines changed: 28 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -76,28 +76,17 @@ class PermissionValidationResponseSerializer(PermissionValidationSerializer): #
7676
class RoleScopeValidationMixin(serializers.Serializer): # pylint: disable=abstract-method
7777
"""Mixin providing role and scope validation logic."""
7878

79-
def validate(self, attrs) -> dict:
80-
"""Validate that the specified role and scope are valid and that the role exists in the scope.
81-
82-
This method performs the following validations:
83-
1. Validates that the scope is registered in the scope registry
84-
2. Validates that the scope exists in the system
85-
3. Validates that the role is defined into the roles assigned to the scope
79+
def _validate_scope_and_role(self, scope_value: str, role_value: str) -> None:
80+
"""Validate that a single scope exists and the role is defined in it.
8681
8782
Args:
88-
attrs: Dictionary containing 'role' and 'scope' keys with their string values.
89-
90-
Returns:
91-
dict: The validated data dictionary with 'role' and 'scope' keys.
83+
scope_value: The scope string to validate.
84+
role_value: The role string to validate against the scope.
9285
9386
Raises:
9487
serializers.ValidationError: If the scope is not registered, doesn't exist,
9588
or if the role is not defined in the scope.
9689
"""
97-
validated_data = super().validate(attrs)
98-
scope_value = validated_data["scope"]
99-
role_value = validated_data["role"]
100-
10190
try:
10291
scope = api.ScopeData(external_key=scope_value)
10392
except ValueError as exc:
@@ -113,10 +102,31 @@ def validate(self, attrs) -> dict:
113102
if role not in role_definitions:
114103
raise serializers.ValidationError({"role": f"Role '{role_value}' does not exist in scope '{scope_value}'"})
115104

105+
def validate(self, attrs) -> dict:
106+
"""Validate that the specified role and scope are valid and that the role exists in the scope.
107+
108+
This method performs the following validations:
109+
1. Validates that the scope is registered in the scope registry
110+
2. Validates that the scope exists in the system
111+
3. Validates that the role is defined into the roles assigned to the scope
112+
113+
Args:
114+
attrs: Dictionary containing 'role' and 'scope' keys with their string values.
115+
116+
Returns:
117+
dict: The validated data dictionary with 'role' and 'scope' keys.
118+
119+
Raises:
120+
serializers.ValidationError: If the scope is not registered, doesn't exist,
121+
or if the role is not defined in the scope.
122+
"""
123+
validated_data = super().validate(attrs)
124+
self._validate_scope_and_role(validated_data["scope"], validated_data["role"])
116125
return validated_data
117126

118127

119128
class AddUsersToRoleWithScopeSerializer(
129+
RoleScopeValidationMixin,
120130
RoleMixin,
121131
ScopeMixin,
122132
): # pylint: disable=abstract-method
@@ -136,15 +146,15 @@ class AddUsersToRoleWithScopeSerializer(
136146
users = serializers.ListField(child=serializers.CharField(max_length=255), allow_empty=False)
137147

138148
def validate_users(self, value) -> list[str]:
139-
"""Eliminate duplicates preserving order"""
149+
"""Eliminate duplicates preserving order."""
140150
return list(dict.fromkeys(value))
141151

142152
def validate(self, attrs) -> dict:
143153
"""Validate that exactly one of 'scope'/'scopes' is provided and that every
144154
scope exists in the registry, exists in the system, and supports the role.
145155
Returns validated data with a unified ``scopes`` list of strings.
146156
"""
147-
validated_data = super().validate(attrs)
157+
validated_data = super(RoleScopeValidationMixin, self).validate(attrs)
148158
scope = validated_data.get("scope")
149159
scopes = validated_data.get("scopes")
150160
role_value = validated_data["role"]
@@ -161,22 +171,7 @@ def validate(self, attrs) -> dict:
161171
)
162172

163173
for scope_value in scopes_list:
164-
try:
165-
scope_obj = api.ScopeData(external_key=scope_value)
166-
except ValueError as exc:
167-
raise serializers.ValidationError({"scope": str(exc)}) from exc
168-
169-
if not scope_obj.exists():
170-
raise serializers.ValidationError({"scope": f"Scope '{scope_value}' does not exist"})
171-
172-
role_obj = api.RoleData(external_key=role_value)
173-
generic_scope = get_generic_scope(scope_obj)
174-
role_definitions = api.get_role_definitions_in_scope(generic_scope)
175-
176-
if role_obj not in role_definitions:
177-
raise serializers.ValidationError(
178-
{"role": f"Role '{role_value}' does not exist in scope '{scope_value}'"}
179-
)
174+
self._validate_scope_and_role(scope_value, role_value)
180175

181176
validated_data.pop("scope", None)
182177
validated_data["scopes"] = scopes_list

0 commit comments

Comments
 (0)