Skip to content

Commit

Permalink
fix: protect get environment document endpoint (#4459)
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewelwell authored Aug 7, 2024
1 parent fb27b57 commit bee01c7
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 3 deletions.
2 changes: 2 additions & 0 deletions api/environments/permissions/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ def has_permission(self, request, view):
def has_object_permission(self, request, view, obj):
if view.action == "clone":
return request.user.has_project_permission(CREATE_ENVIRONMENT, obj.project)
elif view.action == "get_document":
return request.user.has_environment_permission(VIEW_ENVIRONMENT, obj)

return request.user.is_environment_admin(obj) or view.action in [
"user_permissions"
Expand Down
5 changes: 4 additions & 1 deletion api/environments/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,10 @@ def user_permissions(self, request, *args, **kwargs):
@swagger_auto_schema(responses={200: SDKEnvironmentDocumentModel})
@action(detail=True, methods=["GET"], url_path="document")
def get_document(self, request, api_key: str):
return Response(Environment.get_environment_document(api_key))
environment = (
self.get_object()
) # use get_object to ensure permissions check is performed
return Response(Environment.get_environment_document(environment.api_key))

@swagger_auto_schema(request_body=no_body, responses={202: ""})
@action(detail=True, methods=["POST"], url_path="enable-v2-versioning")
Expand Down
21 changes: 19 additions & 2 deletions api/tests/unit/environments/test_unit_environments_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -766,11 +766,13 @@ def test_audit_log_entry_created_when_environment_updated(
def test_get_document(
environment: Environment,
project: Project,
admin_client_new: APIClient,
staff_client: APIClient,
feature: Feature,
segment: Segment,
with_environment_permissions: WithEnvironmentPermissionsCallable,
) -> None:
# Given
with_environment_permissions([VIEW_ENVIRONMENT])

# and some sample data to make sure we're testing all of the document
segment_rule = SegmentRule.objects.create(
Expand All @@ -786,13 +788,28 @@ def test_get_document(
)

# When
response = admin_client_new.get(url)
response = staff_client.get(url)

# Then
assert response.status_code == status.HTTP_200_OK
assert response.json()


def test_cannot_get_environment_document_without_permission(
staff_client: APIClient, environment: Environment
) -> None:
# Given
url = reverse(
"api-v1:environments:environment-get-document", args=[environment.api_key]
)

# When
response = staff_client.get(url)

# Then
assert response.status_code == status.HTTP_403_FORBIDDEN


def test_get_all_trait_keys_for_environment_only_returns_distinct_keys(
identity: Identity,
admin_client_new: APIClient,
Expand Down

0 comments on commit bee01c7

Please sign in to comment.