From 34567944eb22830f4d9eac4dd8b07281ab9f98a6 Mon Sep 17 00:00:00 2001
From: Santos Gallegos <stsewd@proton.me>
Date: Tue, 9 Jul 2024 17:08:32 -0500
Subject: [PATCH] API V3: avoid leaking information through expandable fields
 (#11381)

* API V3: avoid leaking information through expandable fields

There are still more things to do, but first I'm removing the ones
that aren't documented, so this shouldn't be a breaking change,
and the information from those fields can already be obtained
from the main object, and if another user with lower permissions was relying on those
fields, that was actually a bug.

Ref https://github.com/readthedocs/readthedocs-corporate/issues/1736.

Expandable fields may look cool, but once you have to deal with
permissions, they are a pain. Sorry, had to vent.

* Missing class inheritance
---
 readthedocs/api/v3/serializers.py | 19 +++++++++++++++----
 readthedocs/api/v3/views.py       | 11 +++++------
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/readthedocs/api/v3/serializers.py b/readthedocs/api/v3/serializers.py
index 2c3aaa2e753..6efd5c2290d 100644
--- a/readthedocs/api/v3/serializers.py
+++ b/readthedocs/api/v3/serializers.py
@@ -778,18 +778,23 @@ class Meta:
 
         expandable_fields = {
             # NOTE: this has to be a Model method, can't be a
-            # ``SerializerMethodField`` as far as I know
+            # ``SerializerMethodField`` as far as I know.
+            # NOTE: this lists public versions only.
             "active_versions": (
                 VersionSerializer,
                 {
                     "many": True,
                 },
             ),
+            # NOTE: we use a serializer without expandable fields to avoid
+            # leaking information about the organization through the project.
             "organization": (
-                "readthedocs.api.v3.serializers.OrganizationSerializer",
+                "readthedocs.api.v3.serializers.OrganizationSerializerWithoutExpandableFields",
                 # NOTE: we cannot have a Project with multiple organizations.
                 {"source": "organizations.first"},
             ),
+            # NOTE: we are leaking the slugs of all teams linked to this project
+            # to anyone with access to this prtoject. It's only the slug, but still.
             "teams": (
                 serializers.SlugRelatedField,
                 {
@@ -1165,7 +1170,7 @@ class Meta:
         }
 
 
-class OrganizationSerializer(FlexFieldsModelSerializer):
+class OrganizationSerializerWithoutExpandableFields(FlexFieldsModelSerializer):
     created = serializers.DateTimeField(source="pub_date")
     modified = serializers.DateTimeField(source="modified_date")
     owners = UserSerializer(many=True)
@@ -1187,8 +1192,14 @@ class Meta:
             "_links",
         )
 
+
+class OrganizationSerializerWithExpandableFields(
+    OrganizationSerializerWithoutExpandableFields
+):
+    class Meta(OrganizationSerializerWithoutExpandableFields.Meta):
         expandable_fields = {
-            "projects": (ProjectSerializer, {"many": True}),
+            # TODO: we are leaking all teams and its members to anyone who is
+            # an admin member of the organization.
             "teams": (TeamSerializer, {"many": True}),
         }
 
diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py
index dbf6bee4554..6cbdbc22722 100644
--- a/readthedocs/api/v3/views.py
+++ b/readthedocs/api/v3/views.py
@@ -69,7 +69,7 @@
     BuildSerializer,
     EnvironmentVariableSerializer,
     NotificationSerializer,
-    OrganizationSerializer,
+    OrganizationSerializerWithExpandableFields,
     ProjectCreateSerializer,
     ProjectSerializer,
     ProjectUpdateSerializer,
@@ -646,7 +646,7 @@ class OrganizationsViewSetBase(
     # Also note that Read the Docs for Business expose this endpoint already.
 
     model = Organization
-    serializer_class = OrganizationSerializer
+    serializer_class = OrganizationSerializerWithExpandableFields
     queryset = Organization.objects.none()
     permission_classes = (IsAuthenticated,)
 
@@ -659,6 +659,7 @@ class OrganizationsProjectsViewSet(
     APIv3Settings,
     NestedViewSetMixin,
     OrganizationQuerySetMixin,
+    FlexFieldsMixin,
     ReadOnlyModelViewSet,
 ):
     model = Project
@@ -667,10 +668,8 @@ class OrganizationsProjectsViewSet(
     queryset = Project.objects.all()
     serializer_class = ProjectSerializer
     permission_classes = [IsAuthenticated & IsOrganizationAdminMember]
-    permit_list_expands = [
-        "organization",
-        "organization.teams",
-    ]
+    # We don't need to expand the organization, it's already known.
+    permit_list_expands = []
 
     def get_view_name(self):
         return f"Organizations Projects {self.suffix}"