🔒️(backend) clarify administrator role checking function names
Rename vague functions to explicitly indicate administrator permission checks, or owner ones. Prevents developer confusion and potential security misuse per auditor recommendations.
This commit is contained in:
committed by
aleb_the_flash
parent
6e48f8f222
commit
64eadadaef
@@ -64,7 +64,7 @@ class RoomPermissions(permissions.BasePermission):
|
|||||||
if request.method == "DELETE":
|
if request.method == "DELETE":
|
||||||
return obj.is_owner(user)
|
return obj.is_owner(user)
|
||||||
|
|
||||||
return obj.is_administrator(user)
|
return obj.is_administrator_or_owner(user)
|
||||||
|
|
||||||
|
|
||||||
class ResourceAccessPermission(IsAuthenticated):
|
class ResourceAccessPermission(IsAuthenticated):
|
||||||
@@ -80,7 +80,7 @@ class ResourceAccessPermission(IsAuthenticated):
|
|||||||
if request.method == "DELETE" and obj.role == RoleChoices.OWNER:
|
if request.method == "DELETE" and obj.role == RoleChoices.OWNER:
|
||||||
return obj.user == user
|
return obj.user == user
|
||||||
|
|
||||||
return obj.resource.is_administrator(user)
|
return obj.resource.is_administrator_or_owner(user)
|
||||||
|
|
||||||
|
|
||||||
class HasAbilityPermission(IsAuthenticated):
|
class HasAbilityPermission(IsAuthenticated):
|
||||||
@@ -98,7 +98,7 @@ class HasPrivilegesOnRoom(IsAuthenticated):
|
|||||||
|
|
||||||
def has_object_permission(self, request, view, obj):
|
def has_object_permission(self, request, view, obj):
|
||||||
"""Determine if user has privileges on room."""
|
"""Determine if user has privileges on room."""
|
||||||
return obj.is_owner(request.user) or obj.is_administrator(request.user)
|
return obj.is_administrator_or_owner(request.user)
|
||||||
|
|
||||||
|
|
||||||
class IsRecordingEnabled(permissions.BasePermission):
|
class IsRecordingEnabled(permissions.BasePermission):
|
||||||
|
|||||||
@@ -58,7 +58,9 @@ class ResourceAccessSerializerMixin:
|
|||||||
request = self.context.get("request", None)
|
request = self.context.get("request", None)
|
||||||
user = getattr(request, "user", None)
|
user = getattr(request, "user", None)
|
||||||
|
|
||||||
if not (user and user.is_authenticated and resource.is_administrator(user)):
|
if not (
|
||||||
|
user and user.is_authenticated and resource.is_administrator_or_owner(user)
|
||||||
|
):
|
||||||
raise PermissionDenied(
|
raise PermissionDenied(
|
||||||
_("You must be administrator or owner of a room to add accesses to it.")
|
_("You must be administrator or owner of a room to add accesses to it.")
|
||||||
)
|
)
|
||||||
@@ -118,9 +120,11 @@ class RoomSerializer(serializers.ModelSerializer):
|
|||||||
return output
|
return output
|
||||||
|
|
||||||
role = instance.get_role(request.user)
|
role = instance.get_role(request.user)
|
||||||
is_admin = models.RoleChoices.check_administrator_role(role)
|
is_admin_or_owner = models.RoleChoices.check_administrator_role(
|
||||||
|
role
|
||||||
|
) or models.RoleChoices.check_owner_role(role)
|
||||||
|
|
||||||
if is_admin:
|
if is_admin_or_owner:
|
||||||
access_serializer = NestedResourceAccessSerializer(
|
access_serializer = NestedResourceAccessSerializer(
|
||||||
instance.accesses.select_related("resource", "user").all(),
|
instance.accesses.select_related("resource", "user").all(),
|
||||||
context=self.context,
|
context=self.context,
|
||||||
@@ -128,7 +132,7 @@ class RoomSerializer(serializers.ModelSerializer):
|
|||||||
)
|
)
|
||||||
output["accesses"] = access_serializer.data
|
output["accesses"] = access_serializer.data
|
||||||
|
|
||||||
if not is_admin:
|
if not is_admin_or_owner:
|
||||||
del output["configuration"]
|
del output["configuration"]
|
||||||
|
|
||||||
should_access_room = (
|
should_access_room = (
|
||||||
@@ -147,7 +151,7 @@ class RoomSerializer(serializers.ModelSerializer):
|
|||||||
room_id=room_id, user=request.user, username=username
|
room_id=room_id, user=request.user, username=username
|
||||||
)
|
)
|
||||||
|
|
||||||
output["is_administrable"] = is_admin
|
output["is_administrable"] = is_admin_or_owner
|
||||||
|
|
||||||
return output
|
return output
|
||||||
|
|
||||||
|
|||||||
@@ -35,7 +35,7 @@ class RoleChoices(models.TextChoices):
|
|||||||
@classmethod
|
@classmethod
|
||||||
def check_administrator_role(cls, role):
|
def check_administrator_role(cls, role):
|
||||||
"""Check if a role is administrator."""
|
"""Check if a role is administrator."""
|
||||||
return role in [cls.ADMIN, cls.OWNER]
|
return role == cls.ADMIN
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def check_owner_role(cls, role):
|
def check_owner_role(cls, role):
|
||||||
@@ -288,13 +288,13 @@ class Resource(BaseModel):
|
|||||||
role = RoleChoices.MEMBER
|
role = RoleChoices.MEMBER
|
||||||
return role
|
return role
|
||||||
|
|
||||||
def is_administrator(self, user):
|
def is_administrator_or_owner(self, user):
|
||||||
"""
|
"""
|
||||||
Check if a user is administrator of the resource.
|
Check if a user is administrator or owner of the resource."""
|
||||||
|
role = self.get_role(user)
|
||||||
Users carrying the "owner" role are considered as administrators a fortiori.
|
return RoleChoices.check_administrator_role(
|
||||||
"""
|
role
|
||||||
return RoleChoices.check_administrator_role(self.get_role(user))
|
) or RoleChoices.check_owner_role(role)
|
||||||
|
|
||||||
def is_owner(self, user):
|
def is_owner(self, user):
|
||||||
"""Check if a user is owner of the resource."""
|
"""Check if a user is owner of the resource."""
|
||||||
|
|||||||
@@ -102,7 +102,7 @@ def test_models_rooms_access_rights_none(django_assert_num_queries):
|
|||||||
with django_assert_num_queries(0):
|
with django_assert_num_queries(0):
|
||||||
assert room.get_role(None) is None
|
assert room.get_role(None) is None
|
||||||
with django_assert_num_queries(0):
|
with django_assert_num_queries(0):
|
||||||
assert room.is_administrator(None) is False
|
assert room.is_administrator_or_owner(None) is False
|
||||||
with django_assert_num_queries(0):
|
with django_assert_num_queries(0):
|
||||||
assert room.is_owner(None) is False
|
assert room.is_owner(None) is False
|
||||||
|
|
||||||
@@ -115,7 +115,7 @@ def test_models_rooms_access_rights_anonymous(django_assert_num_queries):
|
|||||||
with django_assert_num_queries(0):
|
with django_assert_num_queries(0):
|
||||||
assert room.get_role(user) is None
|
assert room.get_role(user) is None
|
||||||
with django_assert_num_queries(0):
|
with django_assert_num_queries(0):
|
||||||
assert room.is_administrator(user) is False
|
assert room.is_administrator_or_owner(user) is False
|
||||||
with django_assert_num_queries(0):
|
with django_assert_num_queries(0):
|
||||||
assert room.is_owner(user) is False
|
assert room.is_owner(user) is False
|
||||||
|
|
||||||
@@ -128,7 +128,7 @@ def test_models_rooms_access_rights_authenticated(django_assert_num_queries):
|
|||||||
with django_assert_num_queries(1):
|
with django_assert_num_queries(1):
|
||||||
assert room.get_role(user) is None
|
assert room.get_role(user) is None
|
||||||
with django_assert_num_queries(1):
|
with django_assert_num_queries(1):
|
||||||
assert room.is_administrator(user) is False
|
assert room.is_administrator_or_owner(user) is False
|
||||||
with django_assert_num_queries(1):
|
with django_assert_num_queries(1):
|
||||||
assert room.is_owner(user) is False
|
assert room.is_owner(user) is False
|
||||||
|
|
||||||
@@ -141,7 +141,7 @@ def test_models_rooms_access_rights_member_direct(django_assert_num_queries):
|
|||||||
with django_assert_num_queries(1):
|
with django_assert_num_queries(1):
|
||||||
assert room.get_role(user) == "member"
|
assert room.get_role(user) == "member"
|
||||||
with django_assert_num_queries(1):
|
with django_assert_num_queries(1):
|
||||||
assert room.is_administrator(user) is False
|
assert room.is_administrator_or_owner(user) is False
|
||||||
with django_assert_num_queries(1):
|
with django_assert_num_queries(1):
|
||||||
assert room.is_owner(user) is False
|
assert room.is_owner(user) is False
|
||||||
|
|
||||||
@@ -154,7 +154,7 @@ def test_models_rooms_access_rights_administrator_direct(django_assert_num_queri
|
|||||||
with django_assert_num_queries(1):
|
with django_assert_num_queries(1):
|
||||||
assert room.get_role(user) == "administrator"
|
assert room.get_role(user) == "administrator"
|
||||||
with django_assert_num_queries(1):
|
with django_assert_num_queries(1):
|
||||||
assert room.is_administrator(user) is True
|
assert room.is_administrator_or_owner(user) is True
|
||||||
with django_assert_num_queries(1):
|
with django_assert_num_queries(1):
|
||||||
assert room.is_owner(user) is False
|
assert room.is_owner(user) is False
|
||||||
|
|
||||||
@@ -167,7 +167,7 @@ def test_models_rooms_access_rights_owner_direct(django_assert_num_queries):
|
|||||||
with django_assert_num_queries(1):
|
with django_assert_num_queries(1):
|
||||||
assert room.get_role(user) == "owner"
|
assert room.get_role(user) == "owner"
|
||||||
with django_assert_num_queries(1):
|
with django_assert_num_queries(1):
|
||||||
assert room.is_administrator(user) is True
|
assert room.is_administrator_or_owner(user) is True
|
||||||
with django_assert_num_queries(1):
|
with django_assert_num_queries(1):
|
||||||
assert room.is_owner(user) is True
|
assert room.is_owner(user) is True
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user