Skip to content

Commit

Permalink
Organizations: Test access from anonymous users and users outside the…
Browse files Browse the repository at this point in the history
… organization

Currently I'm checking the global option
of `ALLOW_PRIVATE_REPOS` to give access to public views
to users outside the org.

We could introduce a privacy level for the organization if we want.
  • Loading branch information
stsewd committed Oct 20, 2021
1 parent fcc7140 commit f81720d
Show file tree
Hide file tree
Showing 9 changed files with 146 additions and 18 deletions.
18 changes: 17 additions & 1 deletion readthedocs/organizations/managers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""Organizations managers."""

from django.conf import settings
from django.db import models

from readthedocs.core.utils.extend import SettingsOverrideObject
Expand All @@ -12,7 +12,12 @@ class TeamManagerBase(models.Manager):
"""Manager to control team's access."""

def teams_for_user(self, user, organization, admin, member):
"""Get the teams where the user is an admin or member."""
teams = self.get_queryset().none()

if not user.is_authenticated:
return teams

if admin:
# Project Team Admin
teams |= user.teams.filter(access=ADMIN_ACCESS)
Expand All @@ -30,6 +35,17 @@ def teams_for_user(self, user, organization, admin, member):

return teams.distinct()

def public(self, user):
"""
Return all teams the user has access to.
If ``ALLOW_PRIVATE_REPOS`` is `True`, all teams are public by default.
Otherwise, we return only the teams where the user is a member.
"""
if not settings.ALLOW_PRIVATE_REPOS:
return self.get_queryset().all()
return self.member(user)

def admin(self, user, organization=None):
return self.teams_for_user(
user,
Expand Down
19 changes: 18 additions & 1 deletion readthedocs/organizations/querysets.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from datetime import timedelta

from django.conf import settings
from django.db import models
from django.db.models import Q
from django.utils import timezone
Expand All @@ -13,13 +14,29 @@ class BaseOrganizationQuerySet(models.QuerySet):

"""Organizations queryset."""

def public(self, user):
"""
Return all organizations the user has access to.
If ``ALLOW_PRIVATE_REPOS`` is `True`, all organizations are public by default.
Otherwise, we return only the organizations where the user is a member or owner.
"""
if not settings.ALLOW_PRIVATE_REPOS:
return self.all()
return self.for_user(user)

def for_user(self, user):
# Never list all for membership
"""List all organizations where the user is a member or owner."""
if not user.is_authenticated:
return self.none()
return self.filter(
Q(owners__in=[user]) | Q(teams__members__in=[user]),
).distinct()

def for_admin_user(self, user):
"""List all organizations where the user an owner."""
if not user.is_authenticated:
return self.none()
return self.filter(owners__in=[user],).distinct()

def created_days_ago(self, days, field='pub_date'):
Expand Down
2 changes: 2 additions & 0 deletions readthedocs/organizations/templatetags/organizations.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ def org_owner(user, obj): # noqa
Any model instance with a relationship with an organization, or an
organization itself.
"""
if not user.is_authenticated:
return False
try:
cls = type(obj)
if cls is Organization:
Expand Down
90 changes: 89 additions & 1 deletion readthedocs/organizations/tests/test_privacy_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ def setUp(self):
}
)

self.another_user = get(User)

def get_url_path_ctx(self):
return self.default_kwargs

Expand All @@ -35,7 +37,7 @@ def get_url_path_ctx(self):
class NoOrganizationsTest(OrganizationMixin, TestCase):

"""Organization views aren't available if organizations aren't allowed."""

default_status_code = 404

def login(self):
Expand Down Expand Up @@ -74,3 +76,89 @@ def test_public_urls(self):
def test_private_urls(self):
from readthedocs.organizations.urls.private import urlpatterns
self._test_url(urlpatterns)


@override_settings(
RTD_ALLOW_ORGANIZATIONS=True,
ALLOW_PRIVATE_REPOS=False,
)
class AnonymousUserWithPublicOrganizationsTest(OrganizationMixin, TestCase):

"""If organizations are public, an anonymous user can access the public views."""

response_data = {
# Places where we 302 on success.
'/organizations/invite/{hash}/redeem/': {'status_code': 302},
}

def login(self):
pass

def test_public_urls(self):
from readthedocs.organizations.urls.public import urlpatterns
self._test_url(urlpatterns)


@override_settings(
RTD_ALLOW_ORGANIZATIONS=True,
ALLOW_PRIVATE_REPOS=True,
)
class AnonymousUserWithPrivateOrganizationsTest(OrganizationMixin, TestCase):

"""If organizations are private, an anonymous user can't access the public views."""

default_status_code = 404
response_data = {
# Places where we 302 on success.
'/organizations/invite/{hash}/redeem/': {'status_code': 302},
}

def login(self):
pass

def test_public_urls(self):
from readthedocs.organizations.urls.public import urlpatterns
self._test_url(urlpatterns)


@override_settings(
RTD_ALLOW_ORGANIZATIONS=True,
ALLOW_PRIVATE_REPOS=False,
)
class AnotherUserWithPublicOrganizationsTest(OrganizationMixin, TestCase):

"""If organizations are public, an user outside the organization can access the public views."""

response_data = {
# Places where we 302 on success.
'/organizations/invite/{hash}/redeem/': {'status_code': 302},
}

def login(self):
self.client.force_login(self.another_user)

def test_public_urls(self):
from readthedocs.organizations.urls.public import urlpatterns
self._test_url(urlpatterns)


@override_settings(
RTD_ALLOW_ORGANIZATIONS=True,
ALLOW_PRIVATE_REPOS=True,
)
class AnotherUserWithPrivateOrganizationsTest(OrganizationMixin, TestCase):

"""If organizations are private, an user outside the organization can't access the public views."""

default_status_code = 404
response_data = {
# Places where we 302 on success.
'/organizations/invite/{hash}/redeem/': {'status_code': 302},
}

def login(self):
self.client.force_login(self.another_user)

def test_public_urls(self):
from readthedocs.organizations.urls.public import urlpatterns
self._test_url(urlpatterns)
5 changes: 5 additions & 0 deletions readthedocs/organizations/urls/private.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@
views.CreateOrganizationSignup.as_view(),
name='organization_create',
),
url(
r'^verify-email/$',
views.OrganizationTemplateView.as_view(template_name='organizations/verify_email.html'),
name='organization_verify_email',
),
url(
r'^(?P<slug>[\w.-]+)/edit/$',
views.EditOrganization.as_view(),
Expand Down
5 changes: 0 additions & 5 deletions readthedocs/organizations/urls/public.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@
from readthedocs.organizations.views import public as views

urlpatterns = [
url(
r'^verify-email/$',
views.OrganizationTemplateView.as_view(template_name='organizations/verify_email.html'),
name='organization_verify_email',
),
url(
r'^(?P<slug>[\w.-]+)/$',
views.DetailOrganization.as_view(),
Expand Down
10 changes: 7 additions & 3 deletions readthedocs/organizations/views/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def get_organization_queryset(self):
"""
if self.admin_only:
return Organization.objects.for_admin_user(user=self.request.user)
return Organization.objects.for_user(user=self.request.user)
return Organization.objects.public(user=self.request.user)

@lru_cache(maxsize=1)
def get_organization(self):
Expand Down Expand Up @@ -109,7 +109,11 @@ def get_team_queryset(self):
This will either be team the user is a member of, or teams where the
user is an owner of the organization.
"""
return Team.objects.member(self.request.user).filter(
if self.admin_only:
queryset = Team.objects.member(self.request.user)
else:
queryset = Team.objects.public(self.request.user)
return queryset.filter(
organization=self.get_organization(),
).order_by('name')

Expand Down Expand Up @@ -141,7 +145,7 @@ class OrganizationView(CheckOrganizationsEnabled):
def get_queryset(self):
if self.admin_only:
return Organization.objects.for_admin_user(user=self.request.user)
return Organization.objects.for_user(user=self.request.user)
return Organization.objects.public(user=self.request.user)

def get_form(self, data=None, files=None, **kwargs):
kwargs['user'] = self.request.user
Expand Down
9 changes: 8 additions & 1 deletion readthedocs/organizations/views/private.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from django.contrib import messages
from django.urls import reverse_lazy
from django.utils.translation import ugettext_lazy as _
from django.views.generic.base import TemplateView
from vanilla import CreateView, DeleteView, ListView, UpdateView

from readthedocs.core.history import UpdateChangeReasonPostView
Expand All @@ -13,13 +14,19 @@
)
from readthedocs.organizations.models import Organization
from readthedocs.organizations.views.base import (
CheckOrganizationsEnabled,
OrganizationOwnerView,
OrganizationTeamMemberView,
OrganizationTeamView,
OrganizationView,
)


class OrganizationTemplateView(PrivateViewMixin, CheckOrganizationsEnabled, TemplateView):

"""Wrapper around `TemplateView` to check if organizations are enabled."""


# Organization views
class CreateOrganizationSignup(PrivateViewMixin, OrganizationView, CreateView):

Expand Down Expand Up @@ -52,9 +59,9 @@ def get_success_url(self):

class ListOrganization(PrivateViewMixin, OrganizationView, ListView):
template_name = 'organizations/organization_list.html'
admin_only = False

def get_queryset(self):
"""Overriden so we always list the organizations of the current user."""
return Organization.objects.for_user(user=self.request.user)


Expand Down
6 changes: 0 additions & 6 deletions readthedocs/organizations/views/public.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from django.db.models import F
from django.http import HttpResponseRedirect
from django.urls import reverse, reverse_lazy
from django.views.generic.base import TemplateView
from vanilla import DetailView, GenericModelView, ListView

from readthedocs.core.permissions import AdminPermission
Expand All @@ -22,11 +21,6 @@
log = logging.getLogger(__name__)


class OrganizationTemplateView(CheckOrganizationsEnabled, TemplateView):

"""Wrapper around `TemplateView` to check if organizations are enabled."""


# Organization

class DetailOrganization(OrganizationView, DetailView):
Expand Down

0 comments on commit f81720d

Please sign in to comment.