diff --git a/readthedocs/api/v2/admin.py b/readthedocs/api/v2/admin.py new file mode 100644 index 00000000000..2ca7373fc12 --- /dev/null +++ b/readthedocs/api/v2/admin.py @@ -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"] diff --git a/readthedocs/api/v2/apps.py b/readthedocs/api/v2/apps.py new file mode 100644 index 00000000000..952b608cc05 --- /dev/null +++ b/readthedocs/api/v2/apps.py @@ -0,0 +1,6 @@ +from django.apps import AppConfig + + +class APIV2Config(AppConfig): + name = "readthedocs.api.v2" + verbose_name = "API V2" diff --git a/readthedocs/api/v2/client.py b/readthedocs/api/v2/client.py index 9e153b8ecbd..53defcfef6d 100644 --- a/readthedocs/api/v2/client.py +++ b/readthedocs/api/v2/client.py @@ -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 @@ -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( @@ -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) diff --git a/readthedocs/api/v2/migrations/0001_initial.py b/readthedocs/api/v2/migrations/0001_initial.py new file mode 100644 index 00000000000..c3a95da5aae --- /dev/null +++ b/readthedocs/api/v2/migrations/0001_initial.py @@ -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, + }, + ), + ] diff --git a/readthedocs/api/v2/migrations/__init__.py b/readthedocs/api/v2/migrations/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/readthedocs/api/v2/models.py b/readthedocs/api/v2/models.py new file mode 100644 index 00000000000..8fa986c851f --- /dev/null +++ b/readthedocs/api/v2/models.py @@ -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, + 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") diff --git a/readthedocs/api/v2/permissions.py b/readthedocs/api/v2/permissions.py index 130f2bf75aa..4044af52813 100644 --- a/readthedocs/api/v2/permissions.py +++ b/readthedocs/api/v2/permissions.py @@ -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 @@ -65,3 +67,37 @@ def has_permission(self, request, view): .exists() ) return has_access + + +class TokenKeyParser(KeyParser): + + """ + 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) + 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 diff --git a/readthedocs/api/v2/urls.py b/readthedocs/api/v2/urls.py index a5090a6af6d..8b6edafbfa9 100644 --- a/readthedocs/api/v2/urls.py +++ b/readthedocs/api/v2/urls.py @@ -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"), diff --git a/readthedocs/api/v2/views/core_views.py b/readthedocs/api/v2/views/core_views.py index dbff892f2e2..e358ce62fde 100644 --- a/readthedocs/api/v2/views/core_views.py +++ b/readthedocs/api/v2/views/core_views.py @@ -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,)) diff --git a/readthedocs/api/v2/views/model_views.py b/readthedocs/api/v2/views/model_views.py index 73c506857b3..94f2cb9d908 100644 --- a/readthedocs/api/v2/views/model_views.py +++ b/readthedocs/api/v2/views/model_views.py @@ -1,11 +1,12 @@ """Endpoints for listing Projects, Versions, Builds, etc.""" - import json import structlog from allauth.socialaccount.models import SocialAccount from django.conf import settings +from django.core.exceptions import PermissionDenied from django.db.models import BooleanField, Case, Value, When +from django.http import Http404 from django.shortcuts import get_object_or_404 from django.template.loader import render_to_string from rest_framework import decorators, permissions, status, viewsets @@ -14,6 +15,11 @@ from rest_framework.renderers import BaseRenderer, JSONRenderer from rest_framework.response import Response +from readthedocs.api.v2.permissions import ( + APIRestrictedPermission, + HasBuildAPIKey, + IsOwner, +) from readthedocs.api.v2.utils import normalize_build_command from readthedocs.builds.constants import INTERNAL from readthedocs.builds.models import Build, BuildCommandResult, Version @@ -22,7 +28,6 @@ from readthedocs.projects.models import Domain, Project from readthedocs.storage import build_commands_storage -from ..permissions import APIRestrictedPermission, IsOwner from ..serializers import ( BuildAdminReadOnlySerializer, BuildAdminSerializer, @@ -134,16 +139,29 @@ class UserSelectViewSet(viewsets.ReadOnlyModelViewSet): def get_serializer_class(self): try: if ( - self.request.user.is_staff and - self.admin_serializer_class is not None - ): + self.request.user.is_staff or self.request.build_api_key + ) and self.admin_serializer_class is not None: return self.admin_serializer_class except AttributeError: pass return self.serializer_class + def get_queryset_for_api_key(self, api_key): + """Queryset used when an API key is used in the request.""" + raise NotImplementedError + def get_queryset(self): - """Use our API manager method to determine authorization on queryset.""" + """ + Filter objects by user or API key. + + If an API key is present, we filter by the project associated with the key. + Otherwise, we filter using our API manager method. + + With this we check if the user/api key is authorized to acccess the object. + """ + api_key = getattr(self.request, "build_api_key", None) + if api_key: + return self.get_queryset_for_api_key(api_key) return self.model.objects.api(self.request.user) @@ -151,7 +169,7 @@ class ProjectViewSet(DisableListEndpoint, UpdateModelMixin, UserSelectViewSet): """List, filter, etc, Projects.""" - permission_classes = [APIRestrictedPermission] + permission_classes = [HasBuildAPIKey | APIRestrictedPermission] renderer_classes = (JSONRenderer,) serializer_class = ProjectSerializer admin_serializer_class = ProjectAdminSerializer @@ -168,10 +186,7 @@ def translations(self, *_, **__): @decorators.action(detail=True) def subprojects(self, request, **kwargs): - project = get_object_or_404( - Project.objects.api(request.user), - pk=kwargs['pk'], - ) + project = self.get_object() rels = project.subprojects.all() children = [rel.child for rel in rels] return Response({ @@ -180,10 +195,7 @@ def subprojects(self, request, **kwargs): @decorators.action(detail=True) def active_versions(self, request, **kwargs): - project = get_object_or_404( - Project.objects.api(request.user), - pk=kwargs['pk'], - ) + project = self.get_object() versions = project.versions(manager=INTERNAL).filter(active=True) return Response({ 'versions': VersionSerializer(versions, many=True).data, @@ -191,18 +203,18 @@ def active_versions(self, request, **kwargs): @decorators.action(detail=True) def canonical_url(self, request, **kwargs): - project = get_object_or_404( - Project.objects.api(request.user), - pk=kwargs['pk'], - ) + project = self.get_object() return Response({ 'url': project.get_docs_url(), }) + def get_queryset_for_api_key(self, api_key): + return self.model.objects.filter(pk=api_key.project.pk) + class VersionViewSet(DisableListEndpoint, UpdateModelMixin, UserSelectViewSet): - permission_classes = [APIRestrictedPermission] + permission_classes = [HasBuildAPIKey | APIRestrictedPermission] renderer_classes = (JSONRenderer,) serializer_class = VersionSerializer admin_serializer_class = VersionAdminSerializer @@ -212,9 +224,12 @@ class VersionViewSet(DisableListEndpoint, UpdateModelMixin, UserSelectViewSet): 'project__slug', ) + def get_queryset_for_api_key(self, api_key): + return self.model.objects.filter(project=api_key.project) + class BuildViewSet(DisableListEndpoint, UpdateModelMixin, UserSelectViewSet): - permission_classes = [APIRestrictedPermission] + permission_classes = [HasBuildAPIKey | APIRestrictedPermission] renderer_classes = (JSONRenderer, PlainTextBuildRenderer) model = Build filterset_fields = ('project__slug', 'commit') @@ -227,7 +242,7 @@ def get_serializer_class(self): pre-process the `command` field before returning it to the user, and we also want to have a specific serializer for admins. """ - if self.request.user.is_staff: + if self.request.user.is_staff or self.request.build_api_key: # Logic copied from `UserSelectViewSet.get_serializer_class` # and extended to choose serializer from self.action if self.action not in ["list", "retrieve"]: @@ -237,12 +252,22 @@ def get_serializer_class(self): @decorators.action( detail=False, - permission_classes=[permissions.IsAdminUser], + permission_classes=[HasBuildAPIKey | permissions.IsAdminUser], methods=['get'], ) def concurrent(self, request, **kwargs): project_slug = request.GET.get('project__slug') - project = get_object_or_404(Project, slug=project_slug) + build_api_key = request.build_api_key + if build_api_key: + if project_slug != build_api_key.project.slug: + log.info( + "Project slug doesn't match the one attached to the API key.", + project_slug=project_slug, + ) + raise Http404() + project = build_api_key.project + else: + project = get_object_or_404(Project, slug=project_slug) limit_reached, concurrent, max_concurrent = Build.objects.concurrent(project) data = { 'limit_reached': limit_reached, @@ -291,7 +316,7 @@ def retrieve(self, *args, **kwargs): @decorators.action( detail=True, - permission_classes=[permissions.IsAdminUser], + permission_classes=[HasBuildAPIKey | permissions.IsAdminUser], methods=['post'], ) def reset(self, request, **kwargs): @@ -300,14 +325,30 @@ def reset(self, request, **kwargs): instance.reset() return Response(status=status.HTTP_204_NO_CONTENT) + def get_queryset_for_api_key(self, api_key): + return self.model.objects.filter(project=api_key.project) + class BuildCommandViewSet(DisableListEndpoint, CreateModelMixin, UserSelectViewSet): parser_classes = [JSONParser, MultiPartParser] - permission_classes = [APIRestrictedPermission] + permission_classes = [HasBuildAPIKey | APIRestrictedPermission] renderer_classes = (JSONRenderer,) serializer_class = BuildCommandSerializer model = BuildCommandResult + def perform_create(self, serializer): + """Restrict creation to builds attached to the project from the api key.""" + build_pk = serializer.validated_data["build"].pk + api_key = self.request.build_api_key + if api_key and not api_key.project.builds.filter(pk=build_pk).exists(): + raise PermissionDenied() + # If the request isn't attached to a build api key, + # the user doing the request is a superuser, so it has access to all projects. + return super().perform_create(serializer) + + def get_queryset_for_api_key(self, api_key): + return self.model.objects.filter(build__project=api_key.project) + class DomainViewSet(DisableListEndpoint, UserSelectViewSet): permission_classes = [APIRestrictedPermission] diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index f37d1a4083a..0395528f7e3 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -43,6 +43,7 @@ def prepare_build( :rtype: tuple """ # Avoid circular import + from readthedocs.api.v2.models import BuildAPIKey from readthedocs.builds.models import Build from readthedocs.builds.tasks import send_build_notifications from readthedocs.projects.models import Project, WebHookEvent @@ -157,6 +158,8 @@ def prepare_build( ) build.save() + _, build_api_key = BuildAPIKey.objects.create_key(project=project) + return ( update_docs_task.signature( args=( @@ -164,7 +167,8 @@ def prepare_build( build.pk, ), kwargs={ - 'build_commit': commit, + "build_commit": commit, + "build_api_key": build_api_key, }, options=options, immutable=True, diff --git a/readthedocs/core/views/hooks.py b/readthedocs/core/views/hooks.py index 49832a05339..267ac60f553 100644 --- a/readthedocs/core/views/hooks.py +++ b/readthedocs/core/views/hooks.py @@ -2,6 +2,7 @@ import structlog +from readthedocs.api.v2.models import BuildAPIKey from readthedocs.builds.constants import ( EXTERNAL, EXTERNAL_VERSION_STATE_CLOSED, @@ -106,13 +107,16 @@ def trigger_sync_versions(project): # respect the queue for this project options['queue'] = project.build_queue + _, build_api_key = BuildAPIKey.objects.create_key(project=project) + log.debug( 'Triggering sync repository.', project_slug=version.project.slug, version_slug=version.slug, ) sync_repository_task.apply_async( - (version.pk,), + args=[version.pk], + kwargs={"build_api_key": build_api_key}, **options, ) return version.slug diff --git a/readthedocs/projects/tasks/builds.py b/readthedocs/projects/tasks/builds.py index 5143405de72..70fdfb4114d 100644 --- a/readthedocs/projects/tasks/builds.py +++ b/readthedocs/projects/tasks/builds.py @@ -156,7 +156,7 @@ def before_start(self, task_id, args, kwargs): # argument self.data.version_pk = args[0] - self.data.api_client = setup_api() + self.data.api_client = setup_api(kwargs["build_api_key"]) # load all data from the API required for the build self.data.version = self.get_version(self.data.version_pk) @@ -241,7 +241,7 @@ def execute(self): base=SyncRepositoryTask, bind=True, ) -def sync_repository_task(self, version_id, **kwargs): +def sync_repository_task(self, version_id, *, build_api_key, **kwargs): # In case we pass more arguments than expected, log them and ignore them, # so we don't break builds while we deploy a change that requires an extra argument. if kwargs: @@ -395,7 +395,7 @@ def before_start(self, task_id, args, kwargs): # anymore and we are not using it self.data.environment_class = LocalBuildEnvironment - self.data.api_client = setup_api() + self.data.api_client = setup_api(kwargs["build_api_key"]) self.data.build = self.get_build(self.data.build_pk) self.data.version = self.get_version(self.data.version_pk) @@ -700,6 +700,11 @@ def after_return(self, status, retval, task_id, args, kwargs, einfo): if self.data.version: clean_build(self.data.version) + try: + self.data.api_client.revoke.post() + except Exception: + log.exception("Failed to revoke build api key.", exc_info=True) + log.info( 'Build finished.', length=self.data.build['length'], @@ -940,7 +945,9 @@ def send_notifications(self, version_pk, build_pk, event): bind=True, ignore_result=True, ) -def update_docs_task(self, version_id, build_id, build_commit=None, **kwargs): +def update_docs_task( + self, version_id, build_id, *, build_api_key, build_commit=None, **kwargs +): # In case we pass more arguments than expected, log them and ignore them, # so we don't break builds while we deploy a change that requires an extra argument. if kwargs: diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index e57dc18d200..7a085004ff0 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -67,6 +67,7 @@ def _trigger_update_docs_task(self): return update_docs_task.delay( self.version.pk, self.build.pk, + build_api_key="1234", build_commit=self.build.commit, ) @@ -578,6 +579,9 @@ def test_successful_build( "error": "", } + assert self.requests_mock.request_history[10]._request.method == "POST" + assert self.requests_mock.request_history[10].path == "/api/v2/revoke/" + assert BuildData.objects.all().exists() self.mocker.mocks["build_media_storage"].rclone_sync_directory.assert_has_calls( @@ -643,10 +647,10 @@ def test_failed_build( assert not BuildData.objects.all().exists() # Test we are updating the DB by calling the API with the updated build object - api_request = self.requests_mock.request_history[ - -1 - ] # the last one should be the PATCH for the build + # The second last one should be the PATCH for the build + api_request = self.requests_mock.request_history[-2] assert api_request._request.method == "PATCH" + assert api_request.path == "/api/v2/build/1/" assert api_request.json() == { "builder": mock.ANY, "commit": self.build.commit, @@ -657,6 +661,11 @@ def test_failed_build( "success": False, } + # The last request is to revoke the API build key. + revoke_key_request = self.requests_mock.request_history[-1] + assert revoke_key_request._request.method == "POST" + assert revoke_key_request.path == "/api/v2/revoke/" + @mock.patch("readthedocs.doc_builder.director.load_yaml_config") def test_build_commands_executed( self, @@ -1739,9 +1748,10 @@ def test_config_file_exception(self, load_yaml_config): # This is a known exceptions. We hit the API saving the correct error # in the Build object. In this case, the "error message" coming from # the exception will be shown to the user - assert self.requests_mock.request_history[-1]._request.method == "PATCH" - assert self.requests_mock.request_history[-1].path == "/api/v2/build/1/" - assert self.requests_mock.request_history[-1].json() == { + api_request = self.requests_mock.request_history[-2] + assert api_request._request.method == "PATCH" + assert api_request.path == "/api/v2/build/1/" + assert api_request.json() == { "id": 1, "state": "finished", "commit": "a1b2c3", @@ -1751,10 +1761,14 @@ def test_config_file_exception(self, load_yaml_config): "length": 0, } + revoke_key_request = self.requests_mock.request_history[-1] + assert revoke_key_request._request.method == "POST" + assert revoke_key_request.path == "/api/v2/revoke/" + class TestSyncRepositoryTask(BuildEnvironmentBase): def _trigger_sync_repository_task(self): - sync_repository_task.delay(self.version.pk) + sync_repository_task.delay(self.version.pk, build_api_key="1234") @mock.patch('readthedocs.projects.tasks.builds.clean_build') def test_clean_build_after_sync_repository(self, clean_build): @@ -1794,7 +1808,9 @@ def test_check_duplicate_reserved_version_latest(self, on_failure, verbose_name) mock.ANY, mock.ANY, [self.version.pk], - {}, + { + "build_api_key": mock.ANY, + }, mock.ANY, ) diff --git a/readthedocs/projects/tests/test_docker_environment.py b/readthedocs/projects/tests/test_docker_environment.py index bbff15170af..aa5f007f22a 100644 --- a/readthedocs/projects/tests/test_docker_environment.py +++ b/readthedocs/projects/tests/test_docker_environment.py @@ -31,7 +31,7 @@ def setup(self, requests_mock): project=self.project, version=self.version, build={'id': self.build.pk}, - api_client=setup_api(), + api_client=setup_api("1234"), ) def test_container_id(self): diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 8f56d209fc6..d70c5f11b56 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -14,6 +14,7 @@ from rest_framework import status from rest_framework.test import APIClient +from readthedocs.api.v2.models import BuildAPIKey from readthedocs.api.v2.views.integrations import ( GITHUB_CREATE, GITHUB_DELETE, @@ -604,6 +605,31 @@ def test_build_filter_by_commit(self): class APITests(TestCase): fixtures = ['eric.json', 'test_data.json'] + def test_revoke_build_api_key(self): + user = get(User) + project = get(Project, users=[user]) + _, build_api_key = BuildAPIKey.objects.create_key(project) + client = APIClient() + revoke_url = "/api/v2/revoke/" + self.assertTrue(BuildAPIKey.objects.is_valid(build_api_key)) + + # Anonymous request. + client.logout() + resp = client.post(revoke_url) + self.assertEqual(resp.status_code, 403) + self.assertTrue(BuildAPIKey.objects.is_valid(build_api_key)) + + # Using user/password. + client.force_login(user) + resp = client.post(revoke_url) + self.assertEqual(resp.status_code, 403) + self.assertTrue(BuildAPIKey.objects.is_valid(build_api_key)) + + client.logout() + resp = client.post(revoke_url, HTTP_AUTHORIZATION=f"Token {build_api_key}") + self.assertEqual(resp.status_code, 204) + self.assertFalse(BuildAPIKey.objects.is_valid(build_api_key)) + def test_user_doesnt_get_full_api_return(self): user_normal = get(User, is_staff=False) user_admin = get(User, is_staff=True) @@ -699,6 +725,52 @@ def test_project_read_and_write_endpoints_for_staff_user(self): resp = client.patch(f"/api/v2/project/{project.pk}/") self.assertEqual(resp.status_code, 200) + def test_project_read_and_write_endpoints_for_build_api_token(self): + user_normal = get(User, is_staff=False) + user_admin = get(User, is_staff=True) + + project_a = get(Project, users=[user_normal], privacy_level=PUBLIC) + project_b = get(Project, users=[user_admin], privacy_level=PUBLIC) + project_c = get(Project, privacy_level=PUBLIC) + client = APIClient() + + _, build_api_key = BuildAPIKey.objects.create_key(project_a) + client.credentials(HTTP_AUTHORIZATION=f"Token {build_api_key}") + + # List operations without a filter aren't allowed. + resp = client.get("/api/v2/project/") + self.assertEqual(resp.status_code, 410) + + # We don't allow creating projects. + resp = client.post("/api/v2/project/") + self.assertEqual(resp.status_code, 405) + + # The key grants access to project_a only. + resp = client.get(f"/api/v2/project/{project_a.pk}/") + self.assertEqual(resp.status_code, 200) + + # We don't allow deleting projects. + resp = client.delete(f"/api/v2/project/{project_a.pk}/") + self.assertEqual(resp.status_code, 405) + + # Update is fine. + resp = client.patch(f"/api/v2/project/{project_a.pk}/") + self.assertEqual(resp.status_code, 200) + + disallowed_projects = [ + project_b, + project_c, + ] + for project in disallowed_projects: + resp = client.get(f"/api/v2/project/{project.pk}/") + self.assertEqual(resp.status_code, 404) + + resp = client.delete(f"/api/v2/project/{project.pk}/") + self.assertEqual(resp.status_code, 405) + + resp = client.patch(f"/api/v2/project/{project.pk}/") + self.assertEqual(resp.status_code, 404) + def test_build_read_only_endpoints_for_normal_user(self): user_normal = get(User, is_staff=False) user_admin = get(User, is_staff=True) @@ -775,6 +847,55 @@ def test_build_read_and_write_endpoints_for_staff_user(self): resp = client.patch(f"/api/v2/build/{build.pk}/") self.assertEqual(resp.status_code, 200) + def test_build_read_and_write_endpoints_for_build_api_token(self): + user_normal = get(User, is_staff=False) + user_admin = get(User, is_staff=True) + + project_a = get(Project, users=[user_normal], privacy_level=PUBLIC) + project_b = get(Project, users=[user_admin], privacy_level=PUBLIC) + project_c = get(Project, privacy_level=PUBLIC) + client = APIClient() + + _, build_api_key = BuildAPIKey.objects.create_key(project_a) + client.credentials(HTTP_AUTHORIZATION=f"Token {build_api_key}") + + # List operations without a filter aren't allowed. + resp = client.get("/api/v2/build/") + self.assertEqual(resp.status_code, 410) + + # We don't allow to create builds. + resp = client.post("/api/v2/build/") + self.assertEqual(resp.status_code, 405) + + Version.objects.all().update(privacy_level=PUBLIC) + + # The key grants access to builds form project_a only. + build = get(Build, project=project_a, version=project_a.versions.first()) + resp = client.get(f"/api/v2/build/{build.pk}/") + self.assertEqual(resp.status_code, 200) + + # We don't allow deleting builds. + resp = client.delete(f"/api/v2/build/{build.pk}/") + self.assertEqual(resp.status_code, 405) + + # Update them is fine. + resp = client.patch(f"/api/v2/build/{build.pk}/") + self.assertEqual(resp.status_code, 200) + + disallowed_builds = [ + get(Build, project=project_b, version=project_b.versions.first()), + get(Build, project=project_c, version=project_c.versions.first()), + ] + for build in disallowed_builds: + resp = client.get(f"/api/v2/build/{build.pk}/") + self.assertEqual(resp.status_code, 404) + + resp = client.delete(f"/api/v2/build/{build.pk}/") + self.assertEqual(resp.status_code, 405) + + resp = client.patch(f"/api/v2/build/{build.pk}/") + self.assertEqual(resp.status_code, 404) + def test_build_commands_read_only_endpoints_for_normal_user(self): user_normal = get(User, is_staff=False) user_admin = get(User, is_staff=True) @@ -865,6 +986,82 @@ def test_build_commands_read_and_write_endpoints_for_staff_user(self): resp = client.patch(f"/api/v2/command/{command.pk}/") self.assertEqual(resp.status_code, 405) + def test_build_commands_read_and_write_endpoints_for_build_api_token(self): + user_normal = get(User, is_staff=False) + user_admin = get(User, is_staff=True) + + project_a = get(Project, users=[user_normal], privacy_level=PUBLIC) + project_b = get(Project, users=[user_admin], privacy_level=PUBLIC) + project_c = get(Project, privacy_level=PUBLIC) + client = APIClient() + + _, build_api_key = BuildAPIKey.objects.create_key(project_a) + client.credentials(HTTP_AUTHORIZATION=f"Token {build_api_key}") + + # List operations without a filter aren't allowed. + resp = client.get("/api/v2/command/") + self.assertEqual(resp.status_code, 410) + + Version.objects.all().update(privacy_level=PUBLIC) + + build = get(Build, project=project_a, version=project_a.versions.first()) + command = get(BuildCommandResult, build=build) + + # We allow creating build commands. + resp = client.post( + "/api/v2/command/", + { + "build": build.pk, + "command": "test", + "output": "test", + "exit_code": 0, + "start_time": datetime.datetime.utcnow(), + "end_time": datetime.datetime.utcnow(), + }, + ) + self.assertEqual(resp.status_code, 201) + + resp = client.get(f"/api/v2/command/{command.pk}/") + self.assertEqual(resp.status_code, 200) + + # We don't allow deleting commands. + resp = client.delete(f"/api/v2/command/{command.pk}/") + self.assertEqual(resp.status_code, 405) + + # Neither updating them. + resp = client.patch(f"/api/v2/command/{command.pk}/") + self.assertEqual(resp.status_code, 405) + + disallowed_builds = [ + get(Build, project=project_b, version=project_b.versions.first()), + get(Build, project=project_c, version=project_c.versions.first()), + ] + disallowed_build_commands = [ + get(BuildCommandResult, build=build) for build in disallowed_builds + ] + for command in disallowed_build_commands: + resp = client.post( + "/api/v2/command/", + { + "build": command.build.pk, + "command": "test", + "output": "test", + "exit_code": 0, + "start_time": datetime.datetime.utcnow(), + "end_time": datetime.datetime.utcnow(), + }, + ) + self.assertEqual(resp.status_code, 403) + + resp = client.get(f"/api/v2/command/{command.pk}/") + self.assertEqual(resp.status_code, 404) + + resp = client.delete(f"/api/v2/command/{command.pk}/") + self.assertEqual(resp.status_code, 405) + + resp = client.patch(f"/api/v2/command/{command.pk}/") + self.assertEqual(resp.status_code, 405) + def test_versions_read_only_endpoints_for_normal_user(self): user_normal = get(User, is_staff=False) user_admin = get(User, is_staff=True) @@ -943,6 +1140,55 @@ def test_versions_read_and_write_endpoints_for_staff_user(self): resp = client.patch(f"/api/v2/version/{version.pk}/") self.assertEqual(resp.status_code, 200) + def test_versions_read_and_write_endpoints_for_build_api_token(self): + user_normal = get(User, is_staff=False) + user_admin = get(User, is_staff=True) + + project_a = get(Project, users=[user_normal], privacy_level=PUBLIC) + project_b = get(Project, users=[user_admin], privacy_level=PUBLIC) + project_c = get(Project, privacy_level=PUBLIC) + Version.objects.all().update(privacy_level=PUBLIC) + + client = APIClient() + _, build_api_key = BuildAPIKey.objects.create_key(project_a) + client.credentials(HTTP_AUTHORIZATION=f"Token {build_api_key}") + + # List operations without a filter aren't allowed. + resp = client.get("/api/v2/version/") + self.assertEqual(resp.status_code, 410) + + # We don't allow to create versions. + resp = client.post("/api/v2/version/") + self.assertEqual(resp.status_code, 405) + + version = project_a.versions.first() + resp = client.get(f"/api/v2/version/{version.pk}/") + self.assertEqual(resp.status_code, 200) + + # We don't allow deleting versions. + resp = client.delete(f"/api/v2/version/{version.pk}/") + self.assertEqual(resp.status_code, 405) + + # Update them is fine. + resp = client.patch(f"/api/v2/version/{version.pk}/") + self.assertEqual(resp.status_code, 200) + + disallowed_versions = [ + project_b.versions.first(), + project_c.versions.first(), + ] + for version in disallowed_versions: + resp = client.get(f"/api/v2/version/{version.pk}/") + self.assertEqual(resp.status_code, 404) + + # We don't allow deleting versions. + resp = client.delete(f"/api/v2/version/{version.pk}/") + self.assertEqual(resp.status_code, 405) + + # Update them is fine. + resp = client.patch(f"/api/v2/version/{version.pk}/") + self.assertEqual(resp.status_code, 404) + def test_domains_read_only_endpoints_for_normal_user(self): user_normal = get(User, is_staff=False) user_admin = get(User, is_staff=True) @@ -1021,6 +1267,45 @@ def test_domains_read_and_write_endpoints_for_staff_user(self): resp = client.patch(f"/api/v2/domain/{domain.pk}/") self.assertEqual(resp.status_code, 405) + def test_domains_read_and_write_endpoints_for_build_api_token(self): + # Build API tokens don't grant access to the domain endpoints. + user_normal = get(User, is_staff=False) + user_admin = get(User, is_staff=True) + + project_a = get(Project, users=[user_normal], privacy_level=PUBLIC) + project_b = get(Project, users=[user_admin], privacy_level=PUBLIC) + project_c = get(Project, privacy_level=PUBLIC) + Version.objects.all().update(privacy_level=PUBLIC) + + client = APIClient() + _, build_api_key = BuildAPIKey.objects.create_key(project_a) + client.credentials(HTTP_AUTHORIZATION=f"Token {build_api_key}") + + # List operations without a filter aren't allowed. + resp = client.get("/api/v2/domain/") + self.assertEqual(resp.status_code, 410) + + # We don't allow to create domains. + resp = client.post("/api/v2/domain/") + self.assertEqual(resp.status_code, 403) + + domains = [ + get(Domain, project=project_a), + get(Domain, project=project_b), + get(Domain, project=project_c), + ] + for domain in domains: + resp = client.get(f"/api/v2/domain/{domain.pk}/") + self.assertEqual(resp.status_code, 200) + + # We don't allow deleting domains. + resp = client.delete(f"/api/v2/domain/{domain.pk}/") + self.assertEqual(resp.status_code, 403) + + # Neither update them. + resp = client.patch(f"/api/v2/domain/{domain.pk}/") + self.assertEqual(resp.status_code, 403) + def test_project_features(self): user = get(User, is_staff=True) project = get(Project, main_language_project=None) @@ -1386,7 +1671,12 @@ def test_sync_repository_custom_project_queue(self, sync_repository_task, trigge trigger_build.assert_not_called() latest_version = self.project.versions.get(slug=LATEST) sync_repository_task.apply_async.assert_called_with( - (latest_version.pk,), + args=[ + latest_version.pk, + ], + kwargs={ + "build_api_key": mock.ANY, + }, queue='specific-build-queue', ) @@ -1470,7 +1760,9 @@ def test_github_webhook_no_build_on_delete(self, sync_repository_task, trigger_b self.assertEqual(resp.data['versions'], [LATEST]) trigger_build.assert_not_called() latest_version = self.project.versions.get(slug=LATEST) - sync_repository_task.apply_async.assert_called_with((latest_version.pk,)) + sync_repository_task.apply_async.assert_called_with( + args=[latest_version.pk], kwargs={"build_api_key": mock.ANY} + ) @mock.patch("readthedocs.core.views.hooks.sync_repository_task") def test_github_ping_event(self, sync_repository_task, trigger_build): @@ -1505,7 +1797,9 @@ def test_github_create_event(self, sync_repository_task, trigger_build): self.assertEqual(resp.data['versions'], [LATEST]) trigger_build.assert_not_called() latest_version = self.project.versions.get(slug=LATEST) - sync_repository_task.apply_async.assert_called_with((latest_version.pk,)) + sync_repository_task.apply_async.assert_called_with( + args=[latest_version.pk], kwargs={"build_api_key": mock.ANY} + ) @mock.patch('readthedocs.core.utils.trigger_build') def test_github_pull_request_opened_event(self, trigger_build, core_trigger_build): @@ -1726,7 +2020,9 @@ def test_github_delete_event(self, sync_repository_task, trigger_build): self.assertEqual(resp.data['versions'], [LATEST]) trigger_build.assert_not_called() latest_version = self.project.versions.get(slug=LATEST) - sync_repository_task.apply_async.assert_called_with((latest_version.pk,)) + sync_repository_task.apply_async.assert_called_with( + args=[latest_version.pk], kwargs={"build_api_key": mock.ANY} + ) def test_github_parse_ref(self, trigger_build): wh = GitHubWebhookView() @@ -2026,7 +2322,9 @@ def test_gitlab_push_hook_creation( self.assertEqual(resp.data['versions'], [LATEST]) trigger_build.assert_not_called() latest_version = self.project.versions.get(slug=LATEST) - sync_repository_task.apply_async.assert_called_with((latest_version.pk,)) + sync_repository_task.apply_async.assert_called_with( + args=[latest_version.pk], kwargs={"build_api_key": mock.ANY} + ) @mock.patch('readthedocs.core.views.hooks.sync_repository_task') def test_gitlab_push_hook_deletion( @@ -2048,7 +2346,9 @@ def test_gitlab_push_hook_deletion( self.assertEqual(resp.data['versions'], [LATEST]) trigger_build.assert_not_called() latest_version = self.project.versions.get(slug=LATEST) - sync_repository_task.apply_async.assert_called_with((latest_version.pk,)) + sync_repository_task.apply_async.assert_called_with( + args=[latest_version.pk], kwargs={"build_api_key": mock.ANY} + ) @mock.patch('readthedocs.core.views.hooks.sync_repository_task') def test_gitlab_tag_push_hook_creation( @@ -2071,7 +2371,9 @@ def test_gitlab_tag_push_hook_creation( self.assertEqual(resp.data['versions'], [LATEST]) trigger_build.assert_not_called() latest_version = self.project.versions.get(slug=LATEST) - sync_repository_task.apply_async.assert_called_with((latest_version.pk,)) + sync_repository_task.apply_async.assert_called_with( + args=[latest_version.pk], kwargs={"build_api_key": mock.ANY} + ) @mock.patch('readthedocs.core.views.hooks.sync_repository_task') def test_gitlab_tag_push_hook_deletion( @@ -2094,7 +2396,9 @@ def test_gitlab_tag_push_hook_deletion( self.assertEqual(resp.data['versions'], [LATEST]) trigger_build.assert_not_called() latest_version = self.project.versions.get(slug=LATEST) - sync_repository_task.apply_async.assert_called_with((latest_version.pk,)) + sync_repository_task.apply_async.assert_called_with( + args=[latest_version.pk], kwargs={"build_api_key": mock.ANY} + ) def test_gitlab_invalid_webhook(self, trigger_build): """GitLab webhook unhandled event.""" @@ -2530,7 +2834,9 @@ def test_bitbucket_push_hook_creation( self.assertEqual(resp.data['versions'], [LATEST]) trigger_build.assert_not_called() latest_version = self.project.versions.get(slug=LATEST) - sync_repository_task.apply_async.assert_called_with((latest_version.pk,)) + sync_repository_task.apply_async.assert_called_with( + args=[latest_version.pk], kwargs={"build_api_key": mock.ANY} + ) @mock.patch('readthedocs.core.views.hooks.sync_repository_task') def test_bitbucket_push_hook_deletion( @@ -2549,7 +2855,9 @@ def test_bitbucket_push_hook_deletion( self.assertEqual(resp.data['versions'], [LATEST]) trigger_build.assert_not_called() latest_version = self.project.versions.get(slug=LATEST) - sync_repository_task.apply_async.assert_called_with((latest_version.pk,)) + sync_repository_task.apply_async.assert_called_with( + args=[latest_version.pk], kwargs={"build_api_key": mock.ANY} + ) def test_bitbucket_invalid_webhook(self, trigger_build): """Bitbucket webhook unhandled event.""" diff --git a/readthedocs/rtd_tests/tests/test_core_utils.py b/readthedocs/rtd_tests/tests/test_core_utils.py index d57e9293ca9..a6e1e9b7821 100644 --- a/readthedocs/rtd_tests/tests/test_core_utils.py +++ b/readthedocs/rtd_tests/tests/test_core_utils.py @@ -59,6 +59,7 @@ def test_trigger_build_when_version_not_provided_default_version_exist(self, upd ), kwargs={ 'build_commit': None, + "build_api_key": mock.ANY, }, options=mock.ANY, immutable=True, @@ -80,6 +81,7 @@ def test_trigger_build_when_version_not_provided_default_version_doesnt_exist(se ), kwargs={ 'build_commit': None, + "build_api_key": mock.ANY, }, options=mock.ANY, immutable=True, @@ -166,6 +168,7 @@ def test_trigger_build_rounded_time_limit(self, update_docs): ), kwargs={ 'build_commit': None, + "build_api_key": mock.ANY, }, options=options, immutable=True, diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index d400b5c1741..073367c86e1 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -219,6 +219,7 @@ def INSTALLED_APPS(self): # noqa 'django_gravatar', 'rest_framework', 'rest_framework.authtoken', + "rest_framework_api_key", 'corsheaders', 'annoying', 'django_extensions', diff --git a/requirements/deploy.txt b/requirements/deploy.txt index 2e4e278f942..61bb1b5362f 100644 --- a/requirements/deploy.txt +++ b/requirements/deploy.txt @@ -168,6 +168,8 @@ djangorestframework==3.14.0 # via # -r requirements/pip.txt # drf-extensions +djangorestframework-api-key==2.3.0 + # via -r requirements/pip.txt djangorestframework-jsonp==1.0.2 # via -r requirements/pip.txt docker==6.1.2 @@ -353,7 +355,6 @@ sentry-sdk==1.25.0 six==1.16.0 # via # -r requirements/pip.txt - # asttokens # click-repl # django-annoying # django-elasticsearch-dsl diff --git a/requirements/docker.txt b/requirements/docker.txt index 10c0ea0ada9..d6673e8d2c7 100644 --- a/requirements/docker.txt +++ b/requirements/docker.txt @@ -177,6 +177,8 @@ djangorestframework==3.14.0 # via # -r requirements/pip.txt # drf-extensions +djangorestframework-api-key==2.3.0 + # via -r requirements/pip.txt djangorestframework-jsonp==1.0.2 # via -r requirements/pip.txt docker==6.1.2 @@ -381,7 +383,6 @@ selectolax==0.3.14 six==1.16.0 # via # -r requirements/pip.txt - # asttokens # click-repl # django-annoying # django-elasticsearch-dsl diff --git a/requirements/pip.in b/requirements/pip.in index 0ae4c81b8dd..aee6737f8b0 100644 --- a/requirements/pip.in +++ b/requirements/pip.in @@ -12,6 +12,7 @@ django-autoslug django-simple-history==3.0.0 djangorestframework +djangorestframework-api-key # For intersphinx during builds Sphinx diff --git a/requirements/pip.txt b/requirements/pip.txt index 003198b6601..0961c3eaaae 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -124,6 +124,8 @@ djangorestframework==3.14.0 # via # -r requirements/pip.in # drf-extensions +djangorestframework-api-key==2.3.0 + # via -r requirements/pip.in djangorestframework-jsonp==1.0.2 # via -r requirements/pip.in docker==6.1.2 diff --git a/requirements/testing.txt b/requirements/testing.txt index 3c8a5230cad..362b8b5f241 100644 --- a/requirements/testing.txt +++ b/requirements/testing.txt @@ -165,6 +165,8 @@ djangorestframework==3.14.0 # via # -r requirements/pip.txt # drf-extensions +djangorestframework-api-key==2.3.0 + # via -r requirements/pip.txt djangorestframework-jsonp==1.0.2 # via -r requirements/pip.txt docker==6.1.2 @@ -412,9 +414,7 @@ toml==0.10.2 # -r requirements/pip.txt # dparse tomli==2.0.1 - # via - # coverage - # pytest + # via pytest typing-extensions==4.6.3 # via # -r requirements/pip.txt