diff --git a/readthedocs/api/v2/permissions.py b/readthedocs/api/v2/permissions.py index 3ed5c655229..a9cbbeec0ca 100644 --- a/readthedocs/api/v2/permissions.py +++ b/readthedocs/api/v2/permissions.py @@ -38,30 +38,6 @@ def has_object_permission(self, request, view, obj): ) -class APIPermission(permissions.IsAuthenticatedOrReadOnly): - - """ - Control users access to the API. - - This permission should allow authenticated users readonly access to the API, - and allow admin users write access. This should be used on API resources - that need to implement write operations to resources that were based on the - ReadOnlyViewSet - """ - - def has_permission(self, request, view): - has_perm = super().has_permission(request, view) - return has_perm or (request.user and request.user.is_staff) - - def has_object_permission(self, request, view, obj): - has_perm = super().has_object_permission( - request, - view, - obj, - ) - return has_perm or (request.user and request.user.is_staff) - - class APIRestrictedPermission(permissions.BasePermission): """ diff --git a/readthedocs/api/v2/views/model_views.py b/readthedocs/api/v2/views/model_views.py index cc8bfd8aae8..73c506857b3 100644 --- a/readthedocs/api/v2/views/model_views.py +++ b/readthedocs/api/v2/views/model_views.py @@ -9,6 +9,7 @@ from django.shortcuts import get_object_or_404 from django.template.loader import render_to_string from rest_framework import decorators, permissions, status, viewsets +from rest_framework.mixins import CreateModelMixin, UpdateModelMixin from rest_framework.parsers import JSONParser, MultiPartParser from rest_framework.renderers import BaseRenderer, JSONRenderer from rest_framework.response import Response @@ -21,7 +22,7 @@ from readthedocs.projects.models import Domain, Project from readthedocs.storage import build_commands_storage -from ..permissions import APIPermission, APIRestrictedPermission, IsOwner +from ..permissions import APIRestrictedPermission, IsOwner from ..serializers import ( BuildAdminReadOnlySerializer, BuildAdminSerializer, @@ -117,7 +118,7 @@ def list(self, *args, **kwargs): ) -class UserSelectViewSet(viewsets.ModelViewSet): +class UserSelectViewSet(viewsets.ReadOnlyModelViewSet): """ View set that varies serializer class based on request user credentials. @@ -125,6 +126,9 @@ class UserSelectViewSet(viewsets.ModelViewSet): Viewsets using this class should have an attribute `admin_serializer_class`, which is a serializer that might have more fields that only admin/staff users require. If the user is staff, this class will be returned instead. + + By default read-only endpoints will be allowed, + to allow write endpoints, inherit from the proper ``rest_framework.mixins.*`` classes. """ def get_serializer_class(self): @@ -143,11 +147,11 @@ def get_queryset(self): return self.model.objects.api(self.request.user) -class ProjectViewSet(DisableListEndpoint, UserSelectViewSet): +class ProjectViewSet(DisableListEndpoint, UpdateModelMixin, UserSelectViewSet): """List, filter, etc, Projects.""" - permission_classes = [APIPermission] + permission_classes = [APIRestrictedPermission] renderer_classes = (JSONRenderer,) serializer_class = ProjectSerializer admin_serializer_class = ProjectAdminSerializer @@ -196,7 +200,7 @@ def canonical_url(self, request, **kwargs): }) -class VersionViewSet(DisableListEndpoint, UserSelectViewSet): +class VersionViewSet(DisableListEndpoint, UpdateModelMixin, UserSelectViewSet): permission_classes = [APIRestrictedPermission] renderer_classes = (JSONRenderer,) @@ -209,7 +213,7 @@ class VersionViewSet(DisableListEndpoint, UserSelectViewSet): ) -class BuildViewSet(DisableListEndpoint, UserSelectViewSet): +class BuildViewSet(DisableListEndpoint, UpdateModelMixin, UserSelectViewSet): permission_classes = [APIRestrictedPermission] renderer_classes = (JSONRenderer, PlainTextBuildRenderer) model = Build @@ -297,7 +301,7 @@ def reset(self, request, **kwargs): return Response(status=status.HTTP_204_NO_CONTENT) -class BuildCommandViewSet(DisableListEndpoint, UserSelectViewSet): +class BuildCommandViewSet(DisableListEndpoint, CreateModelMixin, UserSelectViewSet): parser_classes = [JSONParser, MultiPartParser] permission_classes = [APIRestrictedPermission] renderer_classes = (JSONRenderer,) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 5d6b0703ce6..d0d7901c1ef 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -54,8 +54,10 @@ RemoteRepository, RemoteRepositoryRelation, ) +from readthedocs.projects.constants import PUBLIC from readthedocs.projects.models import ( APIProject, + Domain, EnvironmentVariable, Feature, Project, @@ -74,33 +76,6 @@ def setUp(self): self.project = get(Project, users=[self.user]) self.version = self.project.versions.get(slug=LATEST) - def test_make_build(self): - """Test that a superuser can use the API.""" - client = APIClient() - client.login(username='super', password='test') - resp = client.post( - '/api/v2/build/', - { - 'project': 1, - 'version': 1, - 'success': True, - 'output': 'Test Output', - 'error': 'Test Error', - 'state': 'cloning', - }, - format='json', - ) - self.assertEqual(resp.status_code, status.HTTP_201_CREATED) - build = resp.data - self.assertEqual(build['state_display'], 'Cloning') - self.assertIsNone(build['config']) - - resp = client.get('/api/v2/build/%s/' % build['id']) - self.assertEqual(resp.status_code, 200) - build = resp.data - self.assertEqual(build['output'], 'Test Output') - self.assertEqual(build['state_display'], 'Cloning') - def test_reset_build(self): build = get( Build, @@ -164,72 +139,6 @@ def test_api_does_not_have_private_config_key_normal_user(self): self.assertIn('config', resp.data) self.assertNotIn('_config', resp.data) - def test_save_config(self): - client = APIClient() - client.login(username='super', password='test') - resp = client.post( - '/api/v2/build/', - { - 'project': 1, - 'version': 1, - 'config': {'one': 'two'}, - }, - format='json', - ) - self.assertEqual(resp.status_code, status.HTTP_201_CREATED) - build_one = resp.data - self.assertEqual(build_one['config'], {'one': 'two'}) - - resp = client.get('/api/v2/build/%s/' % build_one['id']) - self.assertEqual(resp.status_code, 200) - build = resp.data - self.assertEqual(build['config'], {'one': 'two'}) - - def test_save_same_config(self): - client = APIClient() - client.login(username='super', password='test') - resp = client.post( - '/api/v2/build/', - { - 'project': 1, - 'version': 1, - 'config': {'one': 'two'}, - }, - format='json', - ) - self.assertEqual(resp.status_code, status.HTTP_201_CREATED) - build_one = resp.data - self.assertEqual(build_one['config'], {'one': 'two'}) - - resp = client.post( - '/api/v2/build/', - { - 'project': 1, - 'version': 1, - 'config': {'one': 'two'}, - }, - format='json', - ) - self.assertEqual(resp.status_code, status.HTTP_201_CREATED) - build_two = resp.data - self.assertEqual(build_two['config'], {'one': 'two'}) - - resp = client.get('/api/v2/build/%s/' % build_one['id']) - self.assertEqual(resp.status_code, 200) - build = resp.data - self.assertEqual(build['config'], {'one': 'two'}) - - # Checking the values from the db, just to be sure the - # api isn't lying. - self.assertEqual( - Build.objects.get(pk=build_one['id'])._config, - {'one': 'two'}, - ) - self.assertEqual( - Build.objects.get(pk=build_two['id'])._config, - {Build.CONFIG_KEY: build_one['id']}, - ) - def test_save_same_config_using_patch(self): client = APIClient() client.login(username='super', password='test') @@ -463,27 +372,17 @@ def test_make_build_protected_fields(self): self.assertIn('builder', resp.data) def test_make_build_commands(self): - """Create build and build commands.""" + """Create build commands.""" client = APIClient() client.login(username='super', password='test') - resp = client.post( - '/api/v2/build/', - { - 'project': 1, - 'version': 1, - 'success': True, - }, - format='json', - ) - self.assertEqual(resp.status_code, status.HTTP_201_CREATED) - build = resp.data + build = get(Build, project=self.project, version=self.version, success=True) now = timezone.now() start_time = now - datetime.timedelta(seconds=5) end_time = now resp = client.post( '/api/v2/command/', { - "build": build["id"], + "build": build.pk, "command": "$CONDA_ENVS_PATH/$CONDA_DEFAULT_ENV/bin/python -m sphinx", "description": "Conda and Sphinx command", "exit_code": 0, @@ -495,7 +394,7 @@ def test_make_build_commands(self): resp = client.post( "/api/v2/command/", { - "build": build["id"], + "build": build.pk, "command": "$READTHEDOCS_VIRTUALENV_PATH/bin/python -m sphinx", "description": "Python and Sphinx command", "exit_code": 0, @@ -505,7 +404,7 @@ def test_make_build_commands(self): format='json', ) self.assertEqual(resp.status_code, status.HTTP_201_CREATED) - resp = client.get('/api/v2/build/%s/' % build['id']) + resp = client.get(f"/api/v2/build/{build.pk}/") self.assertEqual(resp.status_code, 200) build = resp.data self.assertEqual(len(build["commands"]), 2) @@ -727,6 +626,398 @@ def test_user_doesnt_get_full_api_return(self): self.assertIn("readthedocs_yaml_path", resp.data) self.assertEqual(resp.data["readthedocs_yaml_path"], "bar") + def test_project_read_only_endpoints_for_normal_user(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() + + client.force_authenticate(user=user_normal) + + # 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, 403) + + projects = [ + project_a, + project_b, + project_c, + ] + for project in projects: + resp = client.get(f"/api/v2/project/{project.pk}/") + self.assertEqual(resp.status_code, 200) + + resp = client.delete(f"/api/v2/project/{project.pk}/") + self.assertEqual(resp.status_code, 403) + + resp = client.patch(f"/api/v2/project/{project.pk}/") + self.assertEqual(resp.status_code, 403) + + def test_project_read_and_write_endpoints_for_staff_user(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() + + client.force_authenticate(user=user_admin) + + # 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) + + projects = [ + project_a, + project_b, + project_c, + ] + for project in projects: + resp = client.get(f"/api/v2/project/{project.pk}/") + self.assertEqual(resp.status_code, 200) + + # We don't allow deleting projects. + resp = client.delete(f"/api/v2/project/{project.pk}/") + self.assertEqual(resp.status_code, 405) + + # Update them is fine. + resp = client.patch(f"/api/v2/project/{project.pk}/") + self.assertEqual(resp.status_code, 200) + + def test_build_read_only_endpoints_for_normal_user(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() + + client.force_authenticate(user=user_normal) + + # List operations without a filter aren't allowed. + resp = client.get("/api/v2/build/") + self.assertEqual(resp.status_code, 410) + + # We don't allow creating builds for normal users. + resp = client.post("/api/v2/build/") + self.assertEqual(resp.status_code, 403) + + Version.objects.all().update(privacy_level=PUBLIC) + + builds = [ + get(Build, project=project_a, version=project_a.versions.first()), + get(Build, project=project_b, version=project_b.versions.first()), + get(Build, project=project_c, version=project_c.versions.first()), + ] + for build in builds: + 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, 403) + + # Neither update them. + resp = client.patch(f"/api/v2/build/{build.pk}/") + self.assertEqual(resp.status_code, 403) + + def test_build_read_and_write_endpoints_for_staff_user(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() + + client.force_authenticate(user=user_admin) + + # 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) + + builds = [ + get(Build, project=project_a, version=project_a.versions.first()), + get(Build, project=project_b, version=project_b.versions.first()), + get(Build, project=project_c, version=project_c.versions.first()), + ] + for build in builds: + 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) + + 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) + + 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() + + client.force_authenticate(user=user_normal) + + # List operations without a filter aren't allowed. + resp = client.get("/api/v2/build/") + self.assertEqual(resp.status_code, 410) + + # We don't allow creating commands for normal users. + resp = client.post("/api/v2/command/") + self.assertEqual(resp.status_code, 403) + + Version.objects.all().update(privacy_level=PUBLIC) + + builds = [ + get(Build, project=project_a, version=project_a.versions.first()), + get(Build, project=project_b, version=project_b.versions.first()), + get(Build, project=project_c, version=project_c.versions.first()), + ] + build_commands = [get(BuildCommandResult, build=build) for build in builds] + + for command in build_commands: + resp = client.get(f"/api/v2/command/{command.pk}/") + self.assertEqual(resp.status_code, 200) + + # We don't allow deleting builds. + resp = client.delete(f"/api/v2/command/{command.pk}/") + self.assertEqual(resp.status_code, 403) + + # Neither update them. + resp = client.patch(f"/api/v2/command/{command.pk}/") + self.assertEqual(resp.status_code, 403) + + def test_build_commands_read_and_write_endpoints_for_staff_user(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() + + client.force_authenticate(user=user_admin) + + # 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) + + builds = [ + get(Build, project=project_a, version=project_a.versions.first()), + get(Build, project=project_b, version=project_b.versions.first()), + get(Build, project=project_c, version=project_c.versions.first()), + ] + build_commands = [get(BuildCommandResult, build=build) for build in builds] + + # We do allow to create build commands from the API for staff users. + resp = client.post( + "/api/v2/command/", + { + "build": builds[0].pk, + "command": "test", + "output": "test", + "exit_code": 0, + "start_time": datetime.datetime.utcnow(), + "end_time": datetime.datetime.utcnow(), + }, + ) + self.assertEqual(resp.status_code, 201) + + for command in build_commands: + 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) + + def test_versions_read_only_endpoints_for_normal_user(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() + + client.force_authenticate(user=user_normal) + + # List operations without a filter aren't allowed. + resp = client.get("/api/v2/version/") + self.assertEqual(resp.status_code, 410) + + # We don't allow creating versions. + resp = client.post("/api/v2/version/") + self.assertEqual(resp.status_code, 403) + + versions = [ + project_a.versions.first(), + project_b.versions.first(), + project_c.versions.first(), + ] + + for version in versions: + 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, 403) + + # Neither update them. + resp = client.patch(f"/api/v2/version/{version.pk}/") + self.assertEqual(resp.status_code, 403) + + def test_versions_read_and_write_endpoints_for_staff_user(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() + + client.force_authenticate(user=user_admin) + + # 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) + + versions = [ + project_a.versions.first(), + project_b.versions.first(), + project_c.versions.first(), + ] + + for version in versions: + 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) + + def test_domains_read_only_endpoints_for_normal_user(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() + + client.force_authenticate(user=user_normal) + + # List operations without a filter aren't allowed. + resp = client.get("/api/v2/domain/") + self.assertEqual(resp.status_code, 410) + + # We don't allow creating 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_domains_read_and_write_endpoints_for_staff_user(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() + + client.force_authenticate(user=user_admin) + + # 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, 405) + + 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, 405) + + # Neither update them. + resp = client.patch(f"/api/v2/domain/{domain.pk}/") + self.assertEqual(resp.status_code, 405) + def test_project_features(self): user = get(User, is_staff=True) project = get(Project, main_language_project=None)