Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API V3: Don't allow leaking teams through expandable fields #11471

Merged
merged 5 commits into from
Aug 5, 2024
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 56 additions & 2 deletions docs/user/api/v3.rst
Original file line number Diff line number Diff line change
@@ -202,7 +202,8 @@ Projects list

:Query Parameters:

* **expand** (*string*) -- with ``organization`` and ``teams``.
* **expand** (*string*) -- Allows to add/expand some extra fields in the response.
Allowed values are: ``organization``.



@@ -314,7 +315,8 @@ Project details

:Query Parameters:

* **expand** (*string*) -- with ``organization`` and ``teams``.
* **expand** (*string*) -- Allows to add/expand some extra fields in the response.
Allowed values are: ``organization``.

.. note::

@@ -1730,6 +1732,58 @@ Organization projects list
]
}

Organization teams list
+++++++++++++++++++++++

.. http:get:: /api/v3/organizations/(string:organization_slug)/teams/

Retrieve list of teams under an organization.

**Example request**:

.. tabs::

.. code-tab:: bash

$ curl -H "Authorization: Token <token>" https://readthedocs.com/api/v3/organizations/pypa/teams/

.. code-tab:: python

import requests
URL = 'https://readthedocs.com/api/v3/organizations/pypa/teams/'
TOKEN = '<token>'
HEADERS = {'Authorization': f'token {TOKEN}'}
response = requests.get(URL, headers=HEADERS)
print(response.json())

**Example response**:

.. sourcecode:: json

{
"count": 2,
"next": null,
"previous": null,
"results": [
{
"access": "admin",
"created": "2024-07-01T21:19:55.573819Z",
"modified": "2024-07-01T21:19:55.573837Z",
"name": "Admins",
"slug": "admins"
},
{
"access": "readonly",
"created": "2024-07-01T21:19:55.593815Z",
"modified": "2024-07-01T21:19:55.593829Z",
"name": "Read Only",
"slug": "read-only"
}
]
}

:query string expand: Allows to add/expand some extra fields in the response.
Allowed values are: ``members``.

Remote organizations
~~~~~~~~~~~~~~~~~~~~
5 changes: 5 additions & 0 deletions readthedocs/api/v3/mixins.py
Original file line number Diff line number Diff line change
@@ -188,12 +188,17 @@ class OrganizationQuerySetMixin(NestedParentObjectMixin):
"""

def has_admin_permission(self, user, organization):
"""Check if user is an owner of the organization."""
if self.admin_organizations(user).filter(pk=organization.pk).exists():
return True

return False

def is_admin_member(self, user, organization):
"""Check if user is an owner or belongs to a team with admin permissions of the organization."""
if self.has_admin_permission(user, organization):
return True
Comment on lines +199 to +200
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the owner didn't belong to any admin teams, we were not granting permissions :_


return (
Project.objects.for_admin_user(user=user)
.filter(organizations__in=[organization])
25 changes: 2 additions & 23 deletions readthedocs/api/v3/serializers.py
Original file line number Diff line number Diff line change
@@ -802,20 +802,10 @@ class Meta:
# NOTE: we use a serializer without expandable fields to avoid
# leaking information about the organization through the project.
"organization": (
"readthedocs.api.v3.serializers.OrganizationSerializerWithoutExpandableFields",
"readthedocs.api.v3.serializers.OrganizationSerializer",
# 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,
{
"slug_field": "slug",
"many": True,
"read_only": True,
},
),
}

def __init__(self, *args, **kwargs):
@@ -1183,7 +1173,7 @@ class Meta:
}


class OrganizationSerializerWithoutExpandableFields(FlexFieldsModelSerializer):
class OrganizationSerializer(serializers.ModelSerializer):
created = serializers.DateTimeField(source="pub_date")
modified = serializers.DateTimeField(source="modified_date")
owners = UserSerializer(many=True)
@@ -1206,17 +1196,6 @@ class Meta:
)


class OrganizationSerializerWithExpandableFields(
OrganizationSerializerWithoutExpandableFields
):
class Meta(OrganizationSerializerWithoutExpandableFields.Meta):
expandable_fields = {
# TODO: we are leaking all teams and its members to anyone who is
# an admin member of the organization.
"teams": (TeamSerializer, {"many": True}),
}


class RemoteOrganizationSerializer(serializers.ModelSerializer):
class Meta:
model = RemoteOrganization
8 changes: 8 additions & 0 deletions readthedocs/api/v3/urls.py
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@
NotificationsOrganizationViewSet,
NotificationsProjectViewSet,
NotificationsUserViewSet,
OrganizationsTeamsViewSet,
OrganizationsViewSet,
ProjectsViewSet,
RedirectsViewSet,
@@ -141,6 +142,13 @@
parents_query_lookups=["organization__slug"],
)

organizations.register(
"teams",
OrganizationsTeamsViewSet,
basename="organizations-teams",
parents_query_lookups=["organization__slug"],
)

# allows /api/v3/notifications/
router.register(
r"notifications",
22 changes: 19 additions & 3 deletions readthedocs/api/v3/views.py
Original file line number Diff line number Diff line change
@@ -35,7 +35,7 @@
RemoteRepository,
RemoteRepositoryRelation,
)
from readthedocs.organizations.models import Organization
from readthedocs.organizations.models import Organization, Team
from readthedocs.projects.models import (
EnvironmentVariable,
Project,
@@ -72,7 +72,7 @@
BuildSerializer,
EnvironmentVariableSerializer,
NotificationSerializer,
OrganizationSerializerWithExpandableFields,
OrganizationSerializer,
ProjectCreateSerializer,
ProjectSerializer,
ProjectUpdateSerializer,
@@ -83,6 +83,7 @@
SubprojectCreateSerializer,
SubprojectDestroySerializer,
SubprojectSerializer,
TeamSerializer,
UserSerializer,
VersionSerializer,
VersionUpdateSerializer,
@@ -692,7 +693,7 @@ class OrganizationsViewSetBase(
# Also note that Read the Docs for Business expose this endpoint already.

model = Organization
serializer_class = OrganizationSerializerWithExpandableFields
serializer_class = OrganizationSerializer
queryset = Organization.objects.none()
permission_classes = (IsAuthenticated,)

@@ -721,6 +722,21 @@ def get_view_name(self):
return f"Organizations Projects {self.suffix}"


class OrganizationsTeamsViewSet(
APIv3Settings,
NestedViewSetMixin,
OrganizationQuerySetMixin,
FlexFieldsMixin,
ListModelMixin,
GenericViewSet,
):
model = Team
queryset = Team.objects.all()
serializer_class = TeamSerializer
permission_classes = [IsAuthenticated & IsOrganizationAdmin]
permit_list_expands = ["members"]


class NotificationsOrganizationViewSet(
APIv3Settings,
NestedViewSetMixin,