API V3: change permissions to allow anonymous access to public resour…

…ces (#11485)

* API V3: change permissions to allow anonymous access to public resources

Ref readthedocs/ext-theme#154

* Exclude external versions from API

* Fix organizations queryset

* Fix tests

get_permissions overrides all declarations of permissions, even in
actions

readthedocs/api/v3/mixins.py

@@ -94,9 +94,19 @@ def _get_parent_project(self):
         return get_object_or_404(Project, slug=slug)
 
     def _get_parent_build(self):
+        """
+        Filter the build by the permissions of the current user.
+
+        Build permissions depend not only on the project, but also on
+        the version, Build.objects.api takes all that into consideration.
+        """
         project_slug = self._get_parent_object_lookup(self.PROJECT_LOOKUP_NAMES)
         build_pk = self._get_parent_object_lookup(self.BUILD_LOOKUP_NAMES)
-        return get_object_or_404(Build, pk=build_pk, project__slug=project_slug)
+        return get_object_or_404(
+            Build.objects.api(user=self.request.user),
+            pk=build_pk,
+            project__slug=project_slug,
+        )
 
     def _get_parent_version(self):
         project_slug = self._get_parent_object_lookup(self.PROJECT_LOOKUP_NAMES)
@@ -137,18 +147,13 @@ class ProjectQuerySetMixin(NestedParentObjectMixin):
 
     All APIv3 ViewSet should inherit this mixin, unless specific permissions
     required. In that case, a specific mixin for that case should be defined.
-    """
-
-    def detail_objects(self, queryset, user):
-        # Filter results by user
-        return queryset.api(user=user)
 
-    def listing_objects(self, queryset, user):
-        project = self._get_parent_project()
-        if self.has_admin_permission(user, project):
-            return queryset
+    .. note::
 
-        return queryset.none()
+       When using nested views, the ``NestedViewSetMixin`` should be
+       used and should be before this mixin in the inheritance list.
+       So it can properly filter the queryset based on the parent object.
+    """
 
     def has_admin_permission(self, user, project):
         # Use .only for small optimization
@@ -163,25 +168,8 @@ def admin_projects(self, user):
         return Project.objects.for_admin_user(user=user)
 
     def get_queryset(self):
-        """
-        Filter results based on user permissions.
-
-        1. returns ``Projects`` where the user is admin if ``/projects/`` is hit
-        2. filters by parent ``project_slug`` (NestedViewSetMixin)
-        2. returns ``detail_objects`` results if it's a detail view
-        3. returns ``listing_objects`` results if it's a listing view
-        4. raise a ``NotFound`` exception otherwise
-        """
-
-        # We need to have defined the class attribute as ``queryset = Model.objects.all()``
-        queryset = super().get_queryset()
-
-        # Detail requests are public
-        if self.detail:
-            return self.detail_objects(queryset, self.request.user)
-
-        # List view are only allowed if user is owner of parent project
-        return self.listing_objects(queryset, self.request.user)
+        """Filter projects or related resources based on the permissions of the current user."""
+        return self.model.objects.api(user=self.request.user)
 
 
 class OrganizationQuerySetMixin(NestedParentObjectMixin):
@@ -191,18 +179,13 @@ class OrganizationQuerySetMixin(NestedParentObjectMixin):
 
     All APIv3 organizations' ViewSet should inherit this mixin, unless specific permissions
     required. In that case, a specific mixin for that case should be defined.
-    """
-
-    def detail_objects(self, queryset, user):
-        # Filter results by user
-        return queryset.for_user(user=user)
 
-    def listing_objects(self, queryset, user):
-        organization = self._get_parent_organization()
-        if self.has_admin_permission(user, organization):
-            return queryset
+    .. note::
 
-        return queryset.none()
+       When using nested views, the ``NestedViewSetMixin`` should be
+       used and should be before this mixin in the inheritance list.
+       So it can properly filter the queryset based on the parent object.
+    """
 
     def has_admin_permission(self, user, organization):
         if self.admin_organizations(user).filter(pk=organization.pk).exists():
@@ -221,25 +204,8 @@ def admin_organizations(self, user):
         return Organization.objects.for_admin_user(user=user)
 
     def get_queryset(self):
-        """
-        Filter results based on user permissions.
-
-        1. returns ``Organizations`` where the user is admin if ``/organizations/`` is hit
-        2. filters by parent ``organization_slug`` (NestedViewSetMixin)
-        2. returns ``detail_objects`` results if it's a detail view
-        3. returns ``listing_objects`` results if it's a listing view
-        4. raise a ``NotFound`` exception otherwise
-        """
-
-        # We need to have defined the class attribute as ``queryset = Model.objects.all()``
-        queryset = super().get_queryset()
-
-        # Detail requests are public
-        if self.detail:
-            return self.detail_objects(queryset, self.request.user)
-
-        # List view are only allowed if user is owner of parent project
-        return self.listing_objects(queryset, self.request.user)
+        """Filter organizations or related resources based on the permissions of the current user."""
+        return self.model.objects.api(user=self.request.user)
 
 
 class UserQuerySetMixin(NestedParentObjectMixin):
@@ -273,4 +239,4 @@ def update(self, request, *args, **kwargs):
 
 class RemoteQuerySetMixin:
     def get_queryset(self):
-        return super().get_queryset().api(self.request.user)
+        return self.model.objects.api(self.request.user)

readthedocs/api/v3/permissions.py

@@ -1,6 +1,5 @@
-from rest_framework.permissions import BasePermission, IsAuthenticated
+from rest_framework.permissions import BasePermission
 
-from readthedocs.core.utils.extend import SettingsOverrideObject
 from readthedocs.subscriptions.constants import TYPE_EMBED_API
 from readthedocs.subscriptions.products import get_feature
 
@@ -29,63 +28,16 @@ def has_permission(self, request, view):
         return True
 
 
-class UserProjectsListing(BasePermission):
+class IsCurrentUser(BasePermission):
 
-    """Allow access to ``/projects`` (user's projects listing)."""
+    """Grant permission if user is the same as the one being accessed."""
 
     def has_permission(self, request, view):
-        if view.basename == "projects" and view.action in (
-            "list",
-            "create",  # used to create Form in BrowsableAPIRenderer
-            None,  # needed for BrowsableAPIRenderer
-        ):
-            # hitting ``/projects/``, allowing
+        user = view._get_parent_user()
+        if user == request.user:
             return True
 
 
-class PublicDetailPrivateListing(BasePermission):
-
-    """
-    Permission class for our custom use case.
-
-    * Always give permission for a ``detail`` request
-    * Only give permission for ``listing`` request if user is admin of the project
-
-    However, for notification endpoints we only allow users with access to the project.
-    """
-
-    def has_permission(self, request, view):
-        # NOTE: ``superproject`` is an action name, defined by the class
-        # method under ``ProjectViewSet``. We should apply the same
-        # permissions restrictions than for a detail action (since it only
-        # returns one superproject if exists). ``list`` and ``retrieve`` are
-        # DRF standard action names (same as ``update`` or ``partial_update``).
-        if view.detail and view.action in ("list", "retrieve", "superproject"):
-            # detail view is only allowed on list/retrieve actions (not
-            # ``update`` or ``partial_update``).
-            if view.basename != "projects-notifications":
-                # We don't want to give detail access to projects'
-                # notifications to users that don't have access to the project.
-                return True
-
-        if view.basename.startswith("projects"):
-            project = view._get_parent_project()
-            if view.has_admin_permission(request.user, project):
-                return True
-
-        if view.basename.startswith("organizations"):
-            organization = view._get_parent_organization()
-            if view.has_admin_permission(request.user, organization):
-                return True
-
-        if view.basename.startswith("users"):
-            user = view._get_parent_user()
-            if view.has_admin_permission(request.user, user):
-                return True
-
-        return False
-
-
 class IsProjectAdmin(BasePermission):
 
     """Grant permission if user has admin rights on the Project."""
@@ -108,36 +60,3 @@ def has_permission(self, request, view):
         organization = view._get_parent_organization()
         if view.is_admin_member(request.user, organization):
             return True
-
-
-class UserOrganizationsListing(BasePermission):
-    def has_permission(self, request, view):
-        if view.basename == "organizations" and view.action in (
-            "list",
-            None,  # needed for BrowsableAPIRenderer
-        ):
-            # hitting ``/organizations/``, allowing
-            return True
-
-
-class CommonPermissionsBase(BasePermission):
-
-    """
-    Common permission class used for most APIv3 endpoints.
-
-    This class should be used by ``APIv3Settings.permission_classes`` to define
-    the permissions for most APIv3 endpoints. It has to be overridden from
-    corporate to define proper permissions there.
-    """
-
-    def has_permission(self, request, view):
-        if not IsAuthenticated().has_permission(request, view):
-            return False
-
-        return UserProjectsListing().has_permission(
-            request, view
-        ) or PublicDetailPrivateListing().has_permission(request, view)
-
-
-class CommonPermissions(SettingsOverrideObject):
-    _default_class = CommonPermissionsBase

readthedocs/api/v3/tests/responses/projects-builds-list.json

@@ -0,0 +1,33 @@
+{
+  "count": 1,
+  "next": null,
+  "previous": null,
+  "results": [
+    {
+      "commit": "a1b2c3",
+      "created": "2019-04-29T10:00:00Z",
+      "duration": 60,
+      "error": "",
+      "finished": "2019-04-29T10:01:00Z",
+      "id": 1,
+      "_links": {
+        "_self": "https://readthedocs.org/api/v3/projects/project/builds/1/",
+        "project": "https://readthedocs.org/api/v3/projects/project/",
+        "version": "https://readthedocs.org/api/v3/projects/project/versions/v1.0/",
+        "notifications": "https://readthedocs.org/api/v3/projects/project/builds/1/notifications/"
+      },
+      "urls": {
+        "build": "https://readthedocs.org/projects/project/builds/1/",
+        "project": "https://readthedocs.org/projects/project/",
+        "version": "https://readthedocs.org/dashboard/project/version/v1.0/edit/"
+      },
+      "project": "project",
+      "state": {
+        "code": "finished",
+        "name": "Finished"
+      },
+      "success": true,
+      "version": "v1.0"
+    }
+  ]
+}