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
1 change: 1 addition & 0 deletions readthedocs/api/v3/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class Meta:
fields = [
"verbose_name",
"privacy_level",
"hidden",
"active",
"built",
"uploaded",
Expand Down
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.

43 changes: 28 additions & 15 deletions readthedocs/api/v3/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,16 @@ class Meta:


class BaseLinksSerializer(serializers.Serializer):
def _absolute_url(self, path):
def _absolute_url(self, path, dashboard=False):
scheme = "http" if settings.DEBUG else "https"
domain = settings.PRODUCTION_DOMAIN
request = self.context.get("request")

if dashboard:
domain = settings.PRODUCTION_DOMAIN
elif request:
domain = request.get_host()
else:
domain = settings.PUBLIC_DOMAIN
return urllib.parse.urlunparse((scheme, domain, path, "", "", ""))


Expand Down Expand Up @@ -128,7 +135,7 @@ class BuildURLsSerializer(BaseLinksSerializer, serializers.Serializer):

def get_project(self, obj):
path = reverse("projects_detail", kwargs={"project_slug": obj.project.slug})
return self._absolute_url(path)
return self._absolute_url(path, dashboard=True)

def get_version(self, obj):
if obj.version:
Expand All @@ -139,7 +146,7 @@ def get_version(self, obj):
"version_slug": obj.version.slug,
},
)
return self._absolute_url(path)
return self._absolute_url(path, dashboard=True)
return None


Expand Down Expand Up @@ -316,7 +323,7 @@ def get_edit(self, obj):
"version_slug": obj.slug,
},
)
return self._absolute_url(path)
return self._absolute_url(path, dashboard=True)


class VersionURLsSerializer(BaseLinksSerializer, serializers.Serializer):
Expand Down Expand Up @@ -358,17 +365,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 +393,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 Expand Up @@ -460,19 +463,19 @@ class ProjectURLsSerializer(BaseLinksSerializer, serializers.Serializer):

def get_home(self, obj):
path = reverse("projects_detail", kwargs={"project_slug": obj.slug})
return self._absolute_url(path)
return self._absolute_url(path, dashboard=True)

def get_builds(self, obj):
path = reverse("builds_project_list", kwargs={"project_slug": obj.slug})
return self._absolute_url(path)
return self._absolute_url(path, dashboard=True)

def get_versions(self, obj):
path = reverse("project_version_list", kwargs={"project_slug": obj.slug})
return self._absolute_url(path)
return self._absolute_url(path, dashboard=True)

def get_downloads(self, obj):
path = reverse("project_downloads", kwargs={"project_slug": obj.slug})
return self._absolute_url(path)
return self._absolute_url(path, dashboard=True)

def get_documentation(self, obj):
version_slug = getattr(self.parent, "version_slug", None)
Expand Down Expand Up @@ -500,6 +503,7 @@ class ProjectLinksSerializer(BaseLinksSerializer):
translations = serializers.SerializerMethodField()
notifications = serializers.SerializerMethodField()
sync_versions = serializers.SerializerMethodField()
filetreediff = serializers.SerializerMethodField()

def get__self(self, obj):
path = reverse("projects-detail", kwargs={"project_slug": obj.slug})
Expand Down Expand Up @@ -541,6 +545,15 @@ def get_builds(self, obj):
)
return self._absolute_url(path)

def get_filetreediff(self, obj):
path = reverse(
"projects-filetreediff",
kwargs={
"project_slug": obj.slug,
},
)
return self._absolute_url(path)

def get_subprojects(self, obj):
path = reverse(
"projects-subprojects-list",
Expand Down
1 change: 1 addition & 0 deletions readthedocs/api/v3/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
# allows /api/v3/projects/pip/
# allows /api/v3/projects/pip/superproject/
# allows /api/v3/projects/pip/sync-versions/
# allows /api/v3/projects/pip/filetreediff/
projects = router.register(
r"projects",
ProjectsViewSet,
Expand Down
Loading