From f81720d1316a01e8cfa4ce6a0783471c1d445498 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 19 Oct 2021 15:20:52 -0500 Subject: [PATCH] Organizations: Test access from anonymous users and users outside the 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. --- readthedocs/organizations/managers.py | 18 +++- readthedocs/organizations/querysets.py | 19 +++- .../templatetags/organizations.py | 2 + .../organizations/tests/test_privacy_urls.py | 90 ++++++++++++++++++- readthedocs/organizations/urls/private.py | 5 ++ readthedocs/organizations/urls/public.py | 5 -- readthedocs/organizations/views/base.py | 10 ++- readthedocs/organizations/views/private.py | 9 +- readthedocs/organizations/views/public.py | 6 -- 9 files changed, 146 insertions(+), 18 deletions(-) diff --git a/readthedocs/organizations/managers.py b/readthedocs/organizations/managers.py index 9a654739cc8..0787bc2d25d 100644 --- a/readthedocs/organizations/managers.py +++ b/readthedocs/organizations/managers.py @@ -1,5 +1,5 @@ """Organizations managers.""" - +from django.conf import settings from django.db import models from readthedocs.core.utils.extend import SettingsOverrideObject @@ -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) @@ -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, diff --git a/readthedocs/organizations/querysets.py b/readthedocs/organizations/querysets.py index ddd2b692888..ce61bb3e074 100644 --- a/readthedocs/organizations/querysets.py +++ b/readthedocs/organizations/querysets.py @@ -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 @@ -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'): diff --git a/readthedocs/organizations/templatetags/organizations.py b/readthedocs/organizations/templatetags/organizations.py index 1e3d0822f33..1e4b55544ce 100644 --- a/readthedocs/organizations/templatetags/organizations.py +++ b/readthedocs/organizations/templatetags/organizations.py @@ -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: diff --git a/readthedocs/organizations/tests/test_privacy_urls.py b/readthedocs/organizations/tests/test_privacy_urls.py index e9109f33bf2..62f620191ab 100644 --- a/readthedocs/organizations/tests/test_privacy_urls.py +++ b/readthedocs/organizations/tests/test_privacy_urls.py @@ -27,6 +27,8 @@ def setUp(self): } ) + self.another_user = get(User) + def get_url_path_ctx(self): return self.default_kwargs @@ -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): @@ -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) diff --git a/readthedocs/organizations/urls/private.py b/readthedocs/organizations/urls/private.py index b3b77327c7e..cd29c91f1a6 100644 --- a/readthedocs/organizations/urls/private.py +++ b/readthedocs/organizations/urls/private.py @@ -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[\w.-]+)/edit/$', views.EditOrganization.as_view(), diff --git a/readthedocs/organizations/urls/public.py b/readthedocs/organizations/urls/public.py index 8ee0e17f2f1..bba55f85ab0 100644 --- a/readthedocs/organizations/urls/public.py +++ b/readthedocs/organizations/urls/public.py @@ -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[\w.-]+)/$', views.DetailOrganization.as_view(), diff --git a/readthedocs/organizations/views/base.py b/readthedocs/organizations/views/base.py index 0595d9043ba..20656421604 100644 --- a/readthedocs/organizations/views/base.py +++ b/readthedocs/organizations/views/base.py @@ -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): @@ -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') @@ -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 diff --git a/readthedocs/organizations/views/private.py b/readthedocs/organizations/views/private.py index 2e1a712ae03..a179aa548f9 100644 --- a/readthedocs/organizations/views/private.py +++ b/readthedocs/organizations/views/private.py @@ -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 @@ -13,6 +14,7 @@ ) from readthedocs.organizations.models import Organization from readthedocs.organizations.views.base import ( + CheckOrganizationsEnabled, OrganizationOwnerView, OrganizationTeamMemberView, OrganizationTeamView, @@ -20,6 +22,11 @@ ) +class OrganizationTemplateView(PrivateViewMixin, CheckOrganizationsEnabled, TemplateView): + + """Wrapper around `TemplateView` to check if organizations are enabled.""" + + # Organization views class CreateOrganizationSignup(PrivateViewMixin, OrganizationView, CreateView): @@ -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) diff --git a/readthedocs/organizations/views/public.py b/readthedocs/organizations/views/public.py index fd427a7f0a5..557b8adf4a6 100644 --- a/readthedocs/organizations/views/public.py +++ b/readthedocs/organizations/views/public.py @@ -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 @@ -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):