Skip to content

APIv3: proxy these URLs to be served from El Proxito /_/api/v3/ #11831

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

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft
3 changes: 3 additions & 0 deletions readthedocs/api/v3/proxied_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@
from readthedocs.api.v3.proxied_views import ProxiedEmbedAPI
from readthedocs.search.api.v3.views import ProxiedSearchAPI

from .urls import router

api_proxied_urls = [
path("embed/", ProxiedEmbedAPI.as_view(), name="embed_api_v3"),
path("search/", ProxiedSearchAPI.as_view(), name="search_api_v3"),
]

urlpatterns = api_proxied_urls
urlpatterns += router.urls
Copy link
Member

@stsewd stsewd Jan 14, 2025

Choose a reason for hiding this comment

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

We are proxying the whole API here instead of what addons needs, this has several implications:

  • Duplicate endpoints
  • Attack surface is greater. Preventing XSS vulnerabilities in doc pages is not under our total control (as it depends on the user more). We won't be exposing write operations, but the attack surface is greater now (but you can argue that the most important information is docs content, project, and version, which is already possible to get, but it's harder to list all projects for example).
  • Permissions need to be adapted, especially for .com, as addons needs to work with password and token access, we can't assume we always have a user.
  • Caching as well will need to handle per-view depending on what data gets staled.
  • I think we did lots of optimizations in the addons response, which we may be loosing when migrating everything to API v3.

I'd suggest we explicitly bring what we need and adapt it to work under proxito (as we have done with search and embed APIs) instead of trying to bring the whole API v3 at once. For example, we don't need to list all projects from docs pages, or redirects.

8 changes: 2 additions & 6 deletions readthedocs/api/v3/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,17 +358,13 @@ class Meta:
"privacy_level",
]

def __init__(self, *args, resolver=None, version_serializer=None, **kwargs):
def __init__(self, *args, resolver=None, **kwargs):
super().__init__(*args, **kwargs)

# Use a shared resolver to reduce the amount of DB queries while
# resolving version URLs.
self.resolver = kwargs.pop("resolver", Resolver())

# Allow passing a specific serializer when initializing it.
# This is required to pass ``VersionSerializerNoLinks`` from the addons API.
self.version_serializer = version_serializer or VersionSerializer

def get_downloads(self, obj):
downloads = obj.get_downloads()
data = {}
Expand All @@ -390,7 +386,7 @@ def get_aliases(self, obj):
if obj.slug == LATEST:
alias_version = obj.project.get_original_latest_version()
if alias_version and alias_version.active:
return [self.version_serializer(alias_version).data]
return [VersionSerializer(alias_version).data]
return []


Expand Down
12 changes: 12 additions & 0 deletions readthedocs/api/v3/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
BuildsCreateViewSet,
BuildsViewSet,
EnvironmentVariablesViewSet,
FileTreeDiffViewSet,
NotificationsBuildViewSet,
NotificationsForUserViewSet,
NotificationsOrganizationViewSet,
Expand Down Expand Up @@ -77,6 +78,17 @@
],
)

# allows /api/v3/projects/pip/versions/v3.6.2/filetreediff/
versions.register(
r"filetreediff",
FileTreeDiffViewSet,
basename="projects-versions-filetreediff",
parents_query_lookups=[
"project__slug",
"version__slug",
],
)

# allows /api/v3/projects/pip/builds/
# allows /api/v3/projects/pip/builds/1053/
builds = projects.register(
Expand Down
125 changes: 123 additions & 2 deletions readthedocs/api/v3/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@
from rest_framework.permissions import IsAuthenticated
from rest_framework.renderers import BrowsableAPIRenderer
from rest_framework.response import Response
from rest_framework.throttling import AnonRateThrottle, UserRateThrottle
from rest_framework.viewsets import GenericViewSet, ModelViewSet, ReadOnlyModelViewSet
from rest_framework_extensions.mixins import NestedViewSetMixin

from readthedocs.api.v2.permissions import ReadOnlyPermission
from readthedocs.builds.constants import EXTERNAL
from readthedocs.builds.models import Build, Version
from readthedocs.core.resolver import Resolver
from readthedocs.core.utils import trigger_build
from readthedocs.core.utils.extend import SettingsOverrideObject
from readthedocs.core.views.hooks import trigger_sync_versions
from readthedocs.filetreediff import get_diff
from readthedocs.notifications.models import Notification
from readthedocs.oauth.models import (
RemoteOrganization,
Expand Down Expand Up @@ -110,7 +111,7 @@ class APIv3Settings:
LimitOffsetPagination.default_limit = 10

renderer_classes = (AlphabeticalSortedJSONRenderer, BrowsableAPIRenderer)
throttle_classes = (UserRateThrottle, AnonRateThrottle)
# throttle_classes = (UserRateThrottle, AnonRateThrottle)
Copy link
Member

Choose a reason for hiding this comment

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

Why did we remove this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I temporary commented this because I was blocked immediately since we have the throttle very low in our settings.

"DEFAULT_THROTTLE_RATES": {
"anon": "5/minute",
"user": "60/minute",
},

We should increase these values to be able to use APIv3 in the way we want with addons.

filter_backends = (filters.DjangoFilterBackend,)
metadata_class = SimpleMetadata

Expand Down Expand Up @@ -382,6 +383,126 @@ def get_queryset(self):
return super().get_queryset().exclude(type=EXTERNAL)


class FileTreeDiffViewSet(
APIv3Settings,
NestedViewSetMixin,
ProjectQuerySetMixin,
FlexFieldsMixin,
ReadOnlyModelViewSet,
):
model = Version
lookup_field = "slug"
lookup_url_kwarg = "version_slug"

# Allow ``.`` (dots) on version slug
lookup_value_regex = r"[^/]+"

permission_classes = [ReadOnlyPermission | (IsAuthenticated & IsProjectAdmin)]

def _get_filetreediff_response(self, *, request, project, version, resolver):
"""
Get the file tree diff response for the given version.

This response is only enabled for external versions,
we do the comparison between the current version and the latest version.
"""
if not version.is_external:
return None

if not project.addons.filetreediff_enabled:
return None

base_version = (
project.addons.options_base_version or project.get_latest_version()
)
# TODO: check if `self._has_permission` is important after the migration here.
# if not base_version or not self._has_permission(
# request=request, version=base_version
# ):
if not base_version:
return None

diff = get_diff(version_a=version, version_b=base_version)
if not diff:
return None

return {
"outdated": diff.outdated,
"diff": {
"added": [
{
"filename": filename,
"urls": {
"current": resolver.resolve_version(
project=project,
filename=filename,
version=version,
),
"base": resolver.resolve_version(
project=project,
filename=filename,
version=base_version,
),
},
}
for filename in diff.added
],
"deleted": [
{
"filename": filename,
"urls": {
"current": resolver.resolve_version(
project=project,
filename=filename,
version=version,
),
"base": resolver.resolve_version(
project=project,
filename=filename,
version=base_version,
),
},
}
for filename in diff.deleted
],
"modified": [
{
"filename": filename,
"urls": {
"current": resolver.resolve_version(
project=project,
filename=filename,
version=version,
),
"base": resolver.resolve_version(
project=project,
filename=filename,
version=base_version,
),
},
}
for filename in diff.modified
],
},
}

def list(self, request, **kwargs):
project = self._get_parent_project()
version = self._get_parent_version()
resolver = Resolver()

data = (
self._get_filetreediff_response(
request=request,
project=project,
version=version,
resolver=resolver,
)
or {}
)
return Response(data=data)


class BuildsViewSet(
APIv3Settings,
NestedViewSetMixin,
Expand Down
Loading