Skip to content

Use project-scoped temporal tokens to interact with the API from the builders #10378

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

Merged
merged 25 commits into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions readthedocs/api/v2/admin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from django.contrib import admin
from rest_framework_api_key.admin import APIKeyModelAdmin

from readthedocs.api.v2.models import BuildAPIKey


@admin.register(BuildAPIKey)
class BuildAPIKeyAdmin(APIKeyModelAdmin):
raw_id_fields = ["project"]
search_fields = [*APIKeyModelAdmin.search_fields, "project__slug"]
6 changes: 6 additions & 0 deletions readthedocs/api/v2/apps.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from django.apps import AppConfig


class APIV2Config(AppConfig):
name = "readthedocs.api.v2"
verbose_name = "API V2"
14 changes: 3 additions & 11 deletions readthedocs/api/v2/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def dumps(self, data):
return JSONRenderer().render(data)


def setup_api():
def setup_api(build_api_key):
session = requests.Session()
if settings.SLUMBER_API_HOST.startswith('https'):
# Only use the HostHeaderSSLAdapter for HTTPS connections
Expand All @@ -48,7 +48,8 @@ def setup_api():
settings.SLUMBER_API_HOST,
adapter_class(max_retries=retry),
)
session.headers.update({'Host': settings.PRODUCTION_DOMAIN})
session.headers.update({"Host": settings.PRODUCTION_DOMAIN})
session.headers["Authorization"] = f"Token {build_api_key}"
api_config = {
'base_url': '%s/api/v2/' % settings.SLUMBER_API_HOST,
'serializer': serialize.Serializer(
Expand All @@ -60,13 +61,4 @@ def setup_api():
),
'session': session,
}
if settings.SLUMBER_USERNAME and settings.SLUMBER_PASSWORD:
log.debug(
'Using slumber v2.',
username=settings.SLUMBER_USERNAME,
api_host=settings.SLUMBER_API_HOST,
)
session.auth = (settings.SLUMBER_USERNAME, settings.SLUMBER_PASSWORD)
else:
log.warning('SLUMBER_USERNAME/PASSWORD settings are not set')
return API(**api_config)
73 changes: 73 additions & 0 deletions readthedocs/api/v2/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# Generated by Django 3.2.18 on 2023-05-31 20:40

import django.db.models.deletion
from django.db import migrations, models


class Migration(migrations.Migration):
initial = True

dependencies = [
("projects", "0100_project_readthedocs_yaml_path"),
]

operations = [
migrations.CreateModel(
name="BuildAPIKey",
fields=[
(
"id",
models.CharField(
editable=False,
max_length=150,
primary_key=True,
serialize=False,
unique=True,
),
),
("prefix", models.CharField(editable=False, max_length=8, unique=True)),
("hashed_key", models.CharField(editable=False, max_length=150)),
("created", models.DateTimeField(auto_now_add=True, db_index=True)),
(
"name",
models.CharField(
default=None,
help_text="A free-form name for the API key. Need not be unique. 50 characters max.",
max_length=50,
),
),
(
"revoked",
models.BooleanField(
blank=True,
default=False,
help_text="If the API key is revoked, clients cannot use it anymore. (This cannot be undone.)",
),
),
(
"expiry_date",
models.DateTimeField(
blank=True,
help_text="Once API key expires, clients cannot use it anymore.",
null=True,
verbose_name="Expires",
),
),
(
"project",
models.ForeignKey(
help_text="Project that this API key grants access to",
on_delete=django.db.models.deletion.CASCADE,
related_name="build_api_keys",
to="projects.project",
),
),
],
options={
"verbose_name": "Build API key",
"verbose_name_plural": "Build API keys",
"ordering": ("-created",),
"abstract": False,
},
),
]
Empty file.
48 changes: 48 additions & 0 deletions readthedocs/api/v2/models.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
from datetime import timedelta

from django.db import models
from django.utils import timezone
from django.utils.translation import gettext_lazy as _
from rest_framework_api_key.models import AbstractAPIKey, BaseAPIKeyManager

from readthedocs.projects.models import Project


class BuildAPIKeyManager(BaseAPIKeyManager):
def create_key(self, project):
"""
Create a new API key for a project.

Build API keys are valid for 3 hours,
and can be revoked at any time by hitting the /api/v2/revoke/ endpoint.
"""
expiry_date = timezone.now() + timedelta(hours=3)
return super().create_key(
# Name is required, so we use the project slug for it.
name=project.slug,
expiry_date=expiry_date,
project=project,
)


class BuildAPIKey(AbstractAPIKey):

"""
API key for securely interacting with the API from the builders.

The key is attached to a single project,
it can be used to have write access to the API V2.
"""

project = models.ForeignKey(
Project,
on_delete=models.CASCADE,
Copy link
Member

Choose a reason for hiding this comment

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

This adds more overhead to the deletion of the project. We may want to make this SET_NULL or similar. See #10040

related_name="build_api_keys",
help_text=_("Project that this API key grants access to"),
)

objects = BuildAPIKeyManager()

class Meta(AbstractAPIKey.Meta):
verbose_name = _("Build API key")
verbose_name_plural = _("Build API keys")
36 changes: 36 additions & 0 deletions readthedocs/api/v2/permissions.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
"""Defines access permissions for the API."""

from rest_framework import permissions
from rest_framework_api_key.permissions import BaseHasAPIKey, KeyParser

from readthedocs.api.v2.models import BuildAPIKey
from readthedocs.builds.models import Version


Expand Down Expand Up @@ -65,3 +67,37 @@ def has_permission(self, request, view):
.exists()
)
return has_access


class TokenKeyParser(KeyParser):
Copy link
Member

Choose a reason for hiding this comment

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

Odd this isn't builtin... seems pretty standard?

Copy link
Member Author

Choose a reason for hiding this comment

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

They default to Api-Key {token}


"""
Custom key parser to use ``Token {TOKEN}`` as format.

This is the same format we use in API V3 for auth/authz.
"""

keyword = "Token"


class HasBuildAPIKey(BaseHasAPIKey):

"""
Custom permission to inject the build API key into the request.

This avoids having to parse the key again on each view.
The key is injected in the ``request.build_api_key`` attribute
only if it's valid, otherwise it's set to ``None``.
"""

model = BuildAPIKey
key_parser = TokenKeyParser()

def has_permission(self, request, view):
build_api_key = None
has_permission = super().has_permission(request, view)
Copy link
Member

Choose a reason for hiding this comment

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

I don't fully understand what code this is calling... What is actually being validated here? Is it in our code, or a third-party package? And is it checking the project matches the API key somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

We rely on the permission class from the package to make sure that the API key given is valid, we just override it to attach the key to the request, otherwise we need to parse the key and get it from the db every time we want to use from a view.

"""
Custom permission to inject the build API key into the request.
This avoids having to parse the key again on each view.
The key is injected in the ``request.build_api_key`` attribute
only if it's valid, otherwise it's set to ``None``.
"""

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha, so it's basically just checking that the key exists? Not checking it against any specific user or anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, api keys aren't attached to a user, authorization for a given project is done via the queryset.

if has_permission:
key = self.get_key(request)
build_api_key = self.model.objects.get_from_key(key)
request.build_api_key = build_api_key
return has_permission
8 changes: 8 additions & 0 deletions readthedocs/api/v2/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@
re_path(r'^', include(router.urls)),
]

urlpatterns += [
path(
"revoke/",
core_views.RevokeBuildAPIKeyView.as_view(),
name="revoke_build_api_key",
),
]

function_urls = [
path("docurl/", core_views.docurl, name="docurl"),
path("footer_html/", footer_views.FooterHTML.as_view(), name="footer_html"),
Expand Down
23 changes: 22 additions & 1 deletion readthedocs/api/v2/views/core_views.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,37 @@
"""Utility endpoints relating to canonical urls, embedded content, etc."""

from django.shortcuts import get_object_or_404
from rest_framework import decorators, permissions, status
from rest_framework.renderers import JSONRenderer
from rest_framework.response import Response
from rest_framework.views import APIView

from readthedocs.api.v2.permissions import HasBuildAPIKey
from readthedocs.builds.constants import LATEST
from readthedocs.builds.models import Version
from readthedocs.core.templatetags.core_tags import make_document_url
from readthedocs.projects.models import Project


class RevokeBuildAPIKeyView(APIView):

"""
Revoke a build API key.

This is done by hitting the /api/v2/revoke/ endpoint with a POST request,
while using the API key to be revoked as the authorization key.
"""

http_method_names = ["post"]
permission_classes = [HasBuildAPIKey]
renderer_classes = [JSONRenderer]

def post(self, request, *args, **kwargs):
build_api_key = request.build_api_key
build_api_key.revoked = True
build_api_key.save()
return Response(status=status.HTTP_204_NO_CONTENT)


@decorators.api_view(['GET'])
@decorators.permission_classes((permissions.AllowAny,))
@decorators.renderer_classes((JSONRenderer,))
Expand Down
Loading