diff --git a/readthedocs/organizations/managers.py b/readthedocs/organizations/managers.py index 9a654739cc8..5e0cabf55f5 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 `False`, 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 c8a61bfeb28..6c6a091f683 100644 --- a/readthedocs/organizations/querysets.py +++ b/readthedocs/organizations/querysets.py @@ -15,13 +15,29 @@ class BaseOrganizationQuerySet(models.QuerySet): """Organizations queryset.""" + def public(self, user): + """ + Return all organizations the user has access to. + + If ``ALLOW_PRIVATE_REPOS`` is `False`, 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 is 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_access.py b/readthedocs/organizations/tests/test_access.py index 41ae4f5f715..04ed971ef4b 100644 --- a/readthedocs/organizations/tests/test_access.py +++ b/readthedocs/organizations/tests/test_access.py @@ -2,11 +2,7 @@ from django.contrib.auth.models import User from django.test import TestCase, override_settings -from readthedocs.organizations.models import ( - Organization, - OrganizationOwner, - Team, -) +from readthedocs.organizations.models import Organization, OrganizationOwner, Team from readthedocs.projects.models import Project from readthedocs.rtd_tests.utils import create_user @@ -34,7 +30,7 @@ def assertResponse(self, path, method=None, data=None, **kwargs): response_attrs.update(kwargs) response_attrs.update(self.url_responses.get(path, {})) for (key, val) in list(response_attrs.items()): - self.assertEqual(getattr(response, key), val) + self.assertEqual(getattr(response, key), val, path) return response def setUp(self): @@ -188,7 +184,7 @@ def test_organization_teams(self): self.assertEqual(self.organization.teams.count(), 1) -@override_settings(RTD_ALLOW_ORGANIZATIONS=True) +@override_settings(RTD_ALLOW_ORGANIZATIONS=True, ALLOW_PRIVATE_REPOS=True) class OrganizationOwnerAccess(OrganizationAccessMixin, TestCase): """Test organization paths with authed org owner.""" @@ -200,7 +196,7 @@ def is_admin(self): return True -@override_settings(RTD_ALLOW_ORGANIZATIONS=True) +@override_settings(RTD_ALLOW_ORGANIZATIONS=True, ALLOW_PRIVATE_REPOS=True) class OrganizationMemberAccess(OrganizationAccessMixin, TestCase): """Test organization paths with authed org member.""" @@ -226,13 +222,13 @@ def is_admin(self): return False -@override_settings(RTD_ALLOW_ORGANIZATIONS=True) +@override_settings(RTD_ALLOW_ORGANIZATIONS=True, ALLOW_PRIVATE_REPOS=True) class OrganizationNonmemberAccess(OrganizationAccessMixin, TestCase): """Test organization paths with authed but non-org user.""" url_responses = { - '/organizations/': {'status_code': 200}, + "/organizations/": {"status_code": 200}, } def assertResponse(self, path, method=None, data=None, **kwargs): diff --git a/readthedocs/organizations/tests/test_privacy_urls.py b/readthedocs/organizations/tests/test_privacy_urls.py index e9109f33bf2..c380338d4ed 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,195 @@ 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 AnonymousUserWithPublicOrganizationsPrivateViewsTest(OrganizationMixin, TestCase): + + """If organizations are public, an anonymous user can't access the private views.""" + + # We get redirected to the login page. + default_status_code = 302 + + def login(self): + pass + + 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=True, +) +class AnonymousUserWithPrivateOrganizationsPrivateViewsTest( + OrganizationMixin, TestCase +): + + """If organizations are private, an anonymous user can't access the private views.""" + + # We get redirected to the login page. + default_status_code = 302 + + def login(self): + pass + + 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 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) + + +@override_settings( + RTD_ALLOW_ORGANIZATIONS=True, + ALLOW_PRIVATE_REPOS=False, +) +class AnotherUserWithPublicOrganizationsPrivateViewsTest(OrganizationMixin, TestCase): + + """If organizations are public, an user outside the organization can access the public views.""" + + default_status_code = 404 + response_data = { + # All users have access to these views. + "/organizations/": {"status_code": 200}, + "/organizations/create/": {"status_code": 200}, + "/organizations/verify-email/": {"status_code": 200}, + # 405's where we should be POST'ing + "/organizations/{slug}/owners/{owner}/delete/": {"status_code": 405}, + "/organizations/{slug}/teams/{team}/members/{member}/revoke/": { + "status_code": 405 + }, + } + + def login(self): + self.client.force_login(self.another_user) + + 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=True, +) +class AnotherUserWithPrivateOrganizationsPrivateViewsTest(OrganizationMixin, TestCase): + + """If organizations are private, an user outside the organization can't access the private views.""" + + default_status_code = 404 + response_data = { + # All users have access to these views. + "/organizations/": {"status_code": 200}, + "/organizations/create/": {"status_code": 200}, + "/organizations/verify-email/": {"status_code": 200}, + # 405's where we should be POST'ing + "/organizations/{slug}/owners/{owner}/delete/": {"status_code": 405}, + "/organizations/{slug}/teams/{team}/members/{member}/revoke/": { + "status_code": 405 + }, + } + + def login(self): + self.client.force_login(self.another_user) + + def test_private_urls(self): + from readthedocs.organizations.urls.private import urlpatterns + + self._test_url(urlpatterns) diff --git a/readthedocs/organizations/urls/private.py b/readthedocs/organizations/urls/private.py index 048633ebc70..c55c5917bf5 100644 --- a/readthedocs/organizations/urls/private.py +++ b/readthedocs/organizations/urls/private.py @@ -14,6 +14,13 @@ views.CreateOrganizationSignup.as_view(), name='organization_create', ), + re_path( + r"^verify-email/$", + views.OrganizationTemplateView.as_view( + template_name="organizations/verify_email.html" + ), + name="organization_verify_email", + ), re_path( r'^(?P[\w.-]+)/edit/$', views.EditOrganization.as_view(), diff --git a/readthedocs/organizations/urls/public.py b/readthedocs/organizations/urls/public.py index 5c9d6a54044..768ada3ccea 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 = [ - re_path( - r'^verify-email/$', - views.OrganizationTemplateView.as_view(template_name='organizations/verify_email.html'), - name='organization_verify_email', - ), re_path( 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 fa8cc2dada7..d13b8d4a25d 100644 --- a/readthedocs/organizations/views/private.py +++ b/readthedocs/organizations/views/private.py @@ -6,6 +6,7 @@ from django.urls import reverse_lazy from django.utils import timezone from django.utils.translation import gettext_lazy as _ +from django.views.generic.base import TemplateView from vanilla import CreateView, DeleteView, ListView, UpdateView from readthedocs.audit.filters import OrganizationSecurityLogFilter @@ -19,6 +20,7 @@ ) from readthedocs.organizations.models import Organization from readthedocs.organizations.views.base import ( + CheckOrganizationsEnabled, OrganizationMixin, OrganizationOwnerView, OrganizationTeamMemberView, @@ -28,6 +30,13 @@ from readthedocs.projects.utils import get_csv_file +class OrganizationTemplateView( + PrivateViewMixin, CheckOrganizationsEnabled, TemplateView +): + + """Wrapper around `TemplateView` to check if organizations are enabled.""" + + # Organization views class CreateOrganizationSignup(PrivateViewMixin, OrganizationView, CreateView): @@ -60,9 +69,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 5458b481547..25ace5d5ae3 100644 --- a/readthedocs/organizations/views/public.py +++ b/readthedocs/organizations/views/public.py @@ -1,11 +1,9 @@ """Views that don't require login.""" # pylint: disable=too-many-ancestors import structlog - 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 +20,6 @@ log = structlog.get_logger(__name__) -class OrganizationTemplateView(CheckOrganizationsEnabled, TemplateView): - - """Wrapper around `TemplateView` to check if organizations are enabled.""" - - # Organization class DetailOrganization(OrganizationView, DetailView):