🔒️(backend) remove owner as valid role for ask_for_access serializer

When a ask_for_access creation is made, we explicitly remove the owner
role to prevent role escalation.
This commit is contained in:
Manuel Raynaud
2025-11-12 11:58:50 +01:00
parent 8799b4aa2f
commit bf68a5ae40
3 changed files with 33 additions and 6 deletions

View File

@@ -23,6 +23,10 @@ and this project adheres to
- 🐛(frontend) fix pdf embed to use full width #1526 - 🐛(frontend) fix pdf embed to use full width #1526
- 🐛(pdf) fix table cell alignment issue in exported documents #1582 - 🐛(pdf) fix table cell alignment issue in exported documents #1582
### Security
- mitigate role escalation in the ask_for_access viewset #1580
## [3.9.0] - 2025-11-10 ## [3.9.0] - 2025-11-10
### Added ### Added

View File

@@ -786,7 +786,9 @@ class DocumentAskForAccessCreateSerializer(serializers.Serializer):
"""Serializer for creating a document ask for access.""" """Serializer for creating a document ask for access."""
role = serializers.ChoiceField( role = serializers.ChoiceField(
choices=models.RoleChoices.choices, choices=[
role for role in choices.RoleChoices if role != models.RoleChoices.OWNER
],
required=False, required=False,
default=models.RoleChoices.READER, default=models.RoleChoices.READER,
) )
@@ -810,11 +812,11 @@ class DocumentAskForAccessSerializer(serializers.ModelSerializer):
] ]
read_only_fields = ["id", "document", "user", "role", "created_at", "abilities"] read_only_fields = ["id", "document", "user", "role", "created_at", "abilities"]
def get_abilities(self, invitation) -> dict: def get_abilities(self, instance) -> dict:
"""Return abilities of the logged-in user on the instance.""" """Return abilities of the logged-in user on the instance."""
request = self.context.get("request") request = self.context.get("request")
if request: if request:
return invitation.get_abilities(request.user) return instance.get_abilities(request.user)
return {} return {}

View File

@@ -115,7 +115,10 @@ def test_api_documents_ask_for_access_create_authenticated_non_root_document():
assert response.status_code == 404 assert response.status_code == 404
def test_api_documents_ask_for_access_create_authenticated_specific_role(): @pytest.mark.parametrize(
"role", [role for role in RoleChoices if role != RoleChoices.OWNER]
)
def test_api_documents_ask_for_access_create_authenticated_specific_role(role):
""" """
Authenticated users should be able to create a document ask for access with a specific role. Authenticated users should be able to create a document ask for access with a specific role.
""" """
@@ -127,17 +130,35 @@ def test_api_documents_ask_for_access_create_authenticated_specific_role():
response = client.post( response = client.post(
f"/api/v1.0/documents/{document.id}/ask-for-access/", f"/api/v1.0/documents/{document.id}/ask-for-access/",
data={"role": RoleChoices.EDITOR}, data={"role": role},
) )
assert response.status_code == 201 assert response.status_code == 201
assert DocumentAskForAccess.objects.filter( assert DocumentAskForAccess.objects.filter(
document=document, document=document,
user=user, user=user,
role=RoleChoices.EDITOR, role=role,
).exists() ).exists()
def test_api_documents_ask_for_access_create_authenticated_owner_role():
"""
Authenticated users should not be able to create a document ask for access with the owner role.
"""
document = DocumentFactory()
user = UserFactory()
client = APIClient()
client.force_login(user)
response = client.post(
f"/api/v1.0/documents/{document.id}/ask-for-access/",
data={"role": RoleChoices.OWNER},
)
assert response.status_code == 400
assert response.json() == {"role": ['"owner" is not a valid choice.']}
def test_api_documents_ask_for_access_create_authenticated_already_has_access(): def test_api_documents_ask_for_access_create_authenticated_already_has_access():
"""Authenticated users with existing access can ask for access with a different role.""" """Authenticated users with existing access can ask for access with a different role."""
user = UserFactory() user = UserFactory()