Skip to content
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

API V3: change permissions to allow anonymous access to public resources #11485

Merged
merged 5 commits into from
Aug 5, 2024
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
86 changes: 26 additions & 60 deletions readthedocs/api/v3/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand All @@ -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():
Expand All @@ -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):
Expand Down Expand Up @@ -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)
91 changes: 5 additions & 86 deletions readthedocs/api/v3/permissions.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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."""
Expand All @@ -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
33 changes: 33 additions & 0 deletions readthedocs/api/v3/tests/responses/projects-builds-list.json
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
Loading