Skip to content

Commit 02d300d

Browse files
committed
🔒️(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.
1 parent 066c26f commit 02d300d

File tree

2 files changed

+29
-6
lines changed

2 files changed

+29
-6
lines changed

src/backend/core/api/serializers.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -786,7 +786,9 @@ class DocumentAskForAccessCreateSerializer(serializers.Serializer):
786786
"""Serializer for creating a document ask for access."""
787787

788788
role = serializers.ChoiceField(
789-
choices=models.RoleChoices.choices,
789+
choices=[
790+
role for role in choices.RoleChoices if role != models.RoleChoices.OWNER
791+
],
790792
required=False,
791793
default=models.RoleChoices.READER,
792794
)
@@ -810,11 +812,11 @@ class Meta:
810812
]
811813
read_only_fields = ["id", "document", "user", "role", "created_at", "abilities"]
812814

813-
def get_abilities(self, invitation) -> dict:
815+
def get_abilities(self, instance) -> dict:
814816
"""Return abilities of the logged-in user on the instance."""
815817
request = self.context.get("request")
816818
if request:
817-
return invitation.get_abilities(request.user)
819+
return instance.get_abilities(request.user)
818820
return {}
819821

820822

src/backend/core/tests/documents/test_api_documents_ask_for_access.py

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,10 @@ def test_api_documents_ask_for_access_create_authenticated_non_root_document():
115115
assert response.status_code == 404
116116

117117

118-
def test_api_documents_ask_for_access_create_authenticated_specific_role():
118+
@pytest.mark.parametrize(
119+
"role", [role for role in RoleChoices if role != RoleChoices.OWNER]
120+
)
121+
def test_api_documents_ask_for_access_create_authenticated_specific_role(role):
119122
"""
120123
Authenticated users should be able to create a document ask for access with a specific role.
121124
"""
@@ -127,17 +130,35 @@ def test_api_documents_ask_for_access_create_authenticated_specific_role():
127130

128131
response = client.post(
129132
f"/api/v1.0/documents/{document.id}/ask-for-access/",
130-
data={"role": RoleChoices.EDITOR},
133+
data={"role": role},
131134
)
132135
assert response.status_code == 201
133136

134137
assert DocumentAskForAccess.objects.filter(
135138
document=document,
136139
user=user,
137-
role=RoleChoices.EDITOR,
140+
role=role,
138141
).exists()
139142

140143

144+
def test_api_documents_ask_for_access_create_authenticated_owner_role():
145+
"""
146+
Authenticated users should not be able to create a document ask for access with the owner role.
147+
"""
148+
document = DocumentFactory()
149+
user = UserFactory()
150+
151+
client = APIClient()
152+
client.force_login(user)
153+
154+
response = client.post(
155+
f"/api/v1.0/documents/{document.id}/ask-for-access/",
156+
data={"role": RoleChoices.OWNER},
157+
)
158+
assert response.status_code == 400
159+
assert response.json() == {"role": ['"owner" is not a valid choice.']}
160+
161+
141162
def test_api_documents_ask_for_access_create_authenticated_already_has_access():
142163
"""Authenticated users with existing access can ask for access with a different role."""
143164
user = UserFactory()

0 commit comments

Comments
 (0)