diff --git a/perma_web/conftest.py b/perma_web/conftest.py index 2d9a07578..43a8c0e3c 100644 --- a/perma_web/conftest.py +++ b/perma_web/conftest.py @@ -41,12 +41,6 @@ def _create_server(self, connections_override=None): HTTPSLiveServerThread._create_server = _create_server -# shadow this fixture from pytest_django_liveserver_ssl so that it doesn't request the admin client (which doesn't work with our fixture situation) -@pytest.fixture() -def live_server_ssl_clients_for_patch(client): - return [client] - - @pytest.fixture(scope="session") def set_up_certs(): certs = [ @@ -519,6 +513,11 @@ def org_user(org_user_factory): return org_user_factory() +@pytest.fixture +def admin_user(link_user_factory): + return link_user_factory(is_staff=True) + + @pytest.fixture def perma_client(): """ diff --git a/perma_web/perma/models.py b/perma_web/perma/models.py index 59d09c92f..bcfc551b0 100755 --- a/perma_web/perma/models.py +++ b/perma_web/perma/models.py @@ -1072,7 +1072,12 @@ def can_edit_registrar(self, registrar): return self.is_staff or self.registrar == registrar def can_edit_organization(self, organization): - return self.organizations.filter(pk=organization.pk).exists() + if self.is_staff: + return True + elif self.registrar: + return self.registrar == organization.registrar + else: + return self.organizations.filter(pk=organization.pk).exists() ### subscriptions ### diff --git a/perma_web/perma/tests/test_permissions.py b/perma_web/perma/tests/test_permissions.py index 31d504205..d433dafbd 100644 --- a/perma_web/perma/tests/test_permissions.py +++ b/perma_web/perma/tests/test_permissions.py @@ -3,144 +3,179 @@ from perma.urls import urlpatterns -from .utils import PermaTestCase - - -class PermissionsTestCase(PermaTestCase): - - @override_settings(SECURE_SSL_REDIRECT=False) - def test_permissions(self): - """Test who can log into restricted pages.""" - all_users = { - 'test_admin_user@example.com', - 'test_registrar_user@example.com', - 'test_org_user@example.com', - 'test_user@example.com' - } - views = [ - { - 'urls': [ - ['user_management_stats'], - ['user_management_manage_admin_user'], - ['user_management_admin_user_add_user'], - ['user_management_manage_single_admin_user_delete', {'kwargs':{'user_id': 1}}], - ['user_management_manage_single_admin_user_remove', {'kwargs':{'user_id': 1}}], - ['user_management_manage_registrar'], - ['user_management_manage_single_registrar_user', {'kwargs':{'user_id': 2}}], - ['user_management_manage_single_registrar_user_delete', {'kwargs':{'user_id': 2}}], - ['user_management_manage_single_registrar_user_reactivate', {'kwargs':{'user_id': 2}}], - ['user_management_approve_pending_registrar', {'kwargs':{'registrar_id': 2}}], - ['user_management_manage_user'], - ['user_management_user_add_user'], - ['user_management_manage_single_user', {'kwargs':{'user_id': 4}}], - ['user_management_manage_single_user_delete', {'kwargs':{'user_id': 4}}], - ['user_management_manage_single_organization_user_delete', {'kwargs':{'user_id': 3}}], - ['user_management_manage_single_organization_user_reactivate', {'kwargs':{'user_id': 3}}], - ['user_management_manage_single_user_reactivate', {'kwargs':{'user_id': 4}}], - ['user_management_manage_single_sponsored_user_delete', {'kwargs':{'user_id': 20}}], - ['user_management_manage_single_sponsored_user_reactivate', {'kwargs':{'user_id': 20}}] - ], - 'allowed': {'test_admin_user@example.com'}, - }, - { - 'urls': [ - ['user_management_manage_registrar_user'], - ['user_management_registrar_user_add_user'], - ['user_management_manage_single_registrar', {'kwargs':{'registrar_id': 1}}], - ['user_management_manage_sponsored_user'], - ['user_management_manage_sponsored_user_export_user_list'], - ['user_management_sponsored_user_add_user'], - ['user_management_manage_single_sponsored_user', {'kwargs':{'user_id': 20}}], - ['user_management_manage_single_sponsored_user_remove', {'kwargs':{'user_id': 20, 'registrar_id': 1}}], - ['user_management_manage_single_sponsored_user_readd', {'kwargs':{'user_id': 20, 'registrar_id': 1}}], - ['user_management_manage_single_sponsored_user_links', {'kwargs':{'user_id': 20, 'registrar_id': 1}}] - ], - 'allowed': {'test_admin_user@example.com', 'test_registrar_user@example.com'}, - }, - { - 'urls': [ - ['user_management_manage_single_organization_user', {'kwargs':{'user_id': 3}}], - ['user_management_manage_organization_user'], - ['user_management_manage_organization'], - ['user_management_manage_single_organization', {'kwargs':{'org_id':1}}], - ['user_management_manage_single_organization_export_user_list', {'kwargs': {'org_id': 1}}], - ['user_management_manage_single_organization_delete', {'kwargs':{'org_id':1}}], - ['user_management_organization_user_add_user'], - ['user_management_manage_single_organization_user_remove', {'kwargs':{'user_id': 3}, - 'success_status': 302}], - ], - 'allowed': {'test_admin_user@example.com', 'test_registrar_user@example.com', - 'test_org_user@example.com'}, - }, - { - 'urls': [ - ['user_management_manage_single_registrar_user_remove', {'kwargs':{'user_id': 2}}], - ], - 'allowed': {'test_registrar_user@example.com'} - }, - - { - 'urls': [ - ['user_management_organization_user_leave_organization', {'kwargs':{'org_id': 1}}], - ], - 'allowed': {'test_org_user@example.com'} - }, - - { - 'urls': [ - ['user_management_settings_profile'], - ['user_management_settings_password'], - ['user_management_settings_tools'], - ['create_link'], - ['user_delete_link', {'kwargs':{'guid':'1234-1234'},'success_status':404}], - ], - 'allowed': {'test_user@example.com'}, - 'disallowed': set(), - }, - ] - - views_tested = set() - for view in views: - for url in view['urls']: - view_name = url[0] - opts = url[1] if len(url)>1 else {} - views_tested.add(view_name) - url = reverse(view_name, kwargs=opts.get('kwargs', None)) - success_status = opts.get('success_status', 200) - success_test = opts.get('success_test', None) - - # try while logged out - self.client.logout() - resp = self.client.get(url) - self.assertRedirects(resp, f"{reverse('user_management_limited_login')}?next={url}") - - # try with valid users - for user in view['allowed']: - self.log_in_user(user) - resp = self.client.get(url, secure=True) - if success_test: - success_test(resp) - else: - self.assertEqual(resp.status_code, success_status, - "View %s returned status %s for user %s; expected %s." % (view_name, resp.status_code, user, success_status)) - - # try with invalid users - for user in view.get('disallowed', all_users - view['allowed']): - self.log_in_user(user) - resp = self.client.get(url) - self.assertEqual(resp.status_code, 403, - "View %s returned status %s for user %s; expected %s." % (view_name, resp.status_code, user, success_status)) - # self.assertRedirects(resp, settings.LOGIN_URL+"?next="+url, target_status_code=302, - # msg_prefix="Error while confirming that %s can't view %s: " % (user, view_name)) - - # make sure that all ^manage/ views were tested - for urlpattern in urlpatterns: - - # Things that are no longer needed and have become redirects or other special cases - excluded_names = ['create_link_with_org', - 'link_browser', - 'user_management_resend_activation'] - - if urlpattern.pattern._regex.startswith(r'^manage/') and urlpattern.pattern._regex != '^manage/?$' and urlpattern.name not in excluded_names: - self.assertTrue(urlpattern.name in views_tested, - "Permissions not checked for view '%s' -- add to 'views' or 'any_user_allowed'." % urlpattern.name) +import pytest + + +@override_settings(SECURE_SSL_REDIRECT=False) +@pytest.mark.django_db +def test_permissions(client, admin_user, registrar_user, org_user, link_user_factory, sponsored_user, pending_registrar): + """Test who can log into restricted pages.""" + + regular_user = link_user_factory() + + # Nickname for convenience + org_user_org = org_user.organizations.first() + registrar_user_registrar = registrar_user.registrar + sponsored_user_registrar = sponsored_user.sponsorships.first().registrar + + # Retrieve the registrar user who supports the sponsored user + sponsored_user_registrar_user = sponsored_user_registrar.users.first() + assert sponsored_user_registrar_user + + # Create the registrar user who supports the org user + assert not org_user_org.registrar.users.first() + org_user_org.registrar.users.add(link_user_factory()) + org_user_registrar_user = org_user_org.registrar.users.first() + assert org_user_registrar_user + + all_users = { + admin_user, + registrar_user, + org_user_registrar_user, + sponsored_user_registrar_user, + org_user, + sponsored_user, + regular_user + } + + views = [ + { + 'urls': [ + ['user_management_stats'], + ['user_management_manage_admin_user'], + ['user_management_admin_user_add_user'], + ['user_management_manage_single_admin_user_delete', {'kwargs':{'user_id': admin_user.id}}], + ['user_management_manage_single_admin_user_remove', {'kwargs':{'user_id': admin_user.id}}], + ['user_management_manage_registrar'], + ['user_management_manage_single_registrar_user', {'kwargs':{'user_id': registrar_user.id}}], + ['user_management_manage_single_registrar_user_delete', {'kwargs':{'user_id': registrar_user.id}}], + ['user_management_manage_single_registrar_user_reactivate', {'kwargs':{'user_id': registrar_user.id}}], + ['user_management_approve_pending_registrar', {'kwargs':{'registrar_id': pending_registrar.id}}], + ['user_management_manage_user'], + ['user_management_user_add_user'], + ['user_management_manage_single_user', {'kwargs':{'user_id': regular_user.id}}], + ['user_management_manage_single_user_delete', {'kwargs':{'user_id': regular_user.id}}], + ['user_management_manage_single_organization_user_delete', {'kwargs':{'user_id': org_user.id}}], + ['user_management_manage_single_organization_user_reactivate', {'kwargs':{'user_id': org_user.id}}], + ['user_management_manage_single_user_reactivate', {'kwargs':{'user_id': regular_user.id}}], + ['user_management_manage_single_sponsored_user_delete', {'kwargs':{'user_id': sponsored_user.id}}], + ['user_management_manage_single_sponsored_user_reactivate', {'kwargs':{'user_id': sponsored_user.id}}] + ], + 'allowed': {admin_user}, + }, + { + 'urls': [ + ['user_management_manage_registrar_user'], + ['user_management_registrar_user_add_user'], + ['user_management_manage_sponsored_user'], + ['user_management_manage_sponsored_user_export_user_list'], + ['user_management_sponsored_user_add_user'] + ], + 'allowed': {admin_user, registrar_user, org_user_registrar_user, sponsored_user_registrar_user}, + }, + { + 'urls': [ + ['user_management_manage_single_registrar', {'kwargs':{'registrar_id': registrar_user_registrar.id}}], + ], + 'allowed': {admin_user, registrar_user}, + }, + { + 'urls': [ + ['user_management_manage_single_sponsored_user', {'kwargs':{'user_id': sponsored_user.id}}], + ['user_management_manage_single_sponsored_user_remove', {'kwargs':{'user_id': sponsored_user.id, 'registrar_id': sponsored_user_registrar.id}}], + ['user_management_manage_single_sponsored_user_readd', {'kwargs':{'user_id': sponsored_user.id, 'registrar_id': sponsored_user_registrar.id}}], + ['user_management_manage_single_sponsored_user_links', {'kwargs':{'user_id': sponsored_user.id, 'registrar_id': sponsored_user_registrar.id}}] + ], + 'allowed': {admin_user, sponsored_user_registrar_user}, + }, + { + 'urls': [ + ['user_management_manage_organization_user'], + ['user_management_manage_organization'], + ['user_management_organization_user_add_user'], + ], + 'allowed': {admin_user, registrar_user, org_user_registrar_user, sponsored_user_registrar_user, org_user}, + }, + { + 'urls': [ + ['user_management_manage_single_organization_user', {'kwargs':{'user_id': org_user.id}}], + ['user_management_manage_single_organization', {'kwargs':{'org_id': org_user_org.id}}], + ['user_management_manage_single_organization_export_user_list', {'kwargs': {'org_id': org_user_org.id}}], + ['user_management_manage_single_organization_delete', {'kwargs':{'org_id': org_user_org.id}}], + ['user_management_manage_single_organization_user_remove', {'kwargs':{'user_id': org_user.id}, + 'success_status': 302}], + ], + 'allowed': {admin_user, org_user_registrar_user, org_user}, + }, + { + 'urls': [ + ['user_management_manage_single_registrar_user_remove', {'kwargs':{'user_id': registrar_user.id}}], + ], + 'allowed': {registrar_user} + }, + + { + 'urls': [ + ['user_management_organization_user_leave_organization', {'kwargs':{'org_id': org_user_org.id}}], + ], + 'allowed': {org_user} + }, + + { + 'urls': [ + ['user_management_settings_profile'], + ['user_management_settings_password'], + ['user_management_settings_tools'], + ['create_link'], + ['user_delete_link', {'kwargs':{'guid':'1234-1234'},'success_status':404}], + ], + 'allowed': {regular_user, sponsored_user}, + 'disallowed': set(), + }, + ] + + views_tested = set() + for view in views: + for url in view['urls']: + view_name = url[0] + opts = url[1] if len(url)>1 else {} + views_tested.add(view_name) + url = reverse(view_name, kwargs=opts.get('kwargs', None)) + success_status = opts.get('success_status', 200) + success_test = opts.get('success_test', None) + + # try while logged out + client.logout() + resp = client.get(url) + + assert resp.status_code == 302 + assert resp['Location'] == f"{reverse('user_management_limited_login')}?next={url}" + + # try with valid users + for user in view['allowed']: + client.force_login(user) + resp = client.get(url, secure=True) + if success_test: + success_test(resp) + else: + assert resp.status_code == success_status, \ + "View %s returned status %s for user %s; expected %s." % (view_name, resp.status_code, user, success_status) + + # try with invalid users + for user in view.get('disallowed', all_users - view['allowed']): + client.force_login(user) + resp = client.get(url) + assert resp.status_code == 403, \ + "View %s returned status %s for user %s; expected %s." % (view_name, resp.status_code, user, 403) + + # make sure that all ^manage/ views were tested + for urlpattern in urlpatterns: + + # Things that are no longer needed and have become redirects or other special cases + excluded_names = ['create_link_with_org', + 'link_browser', + 'user_management_resend_activation'] + + if urlpattern.pattern._regex.startswith(r'^manage/') and urlpattern.pattern._regex != '^manage/?$' and urlpattern.name not in excluded_names: + assert urlpattern.name in views_tested, \ + "Permissions not checked for view '%s' -- add to 'views' or 'any_user_allowed'." % urlpattern.name diff --git a/perma_web/perma/tests/test_views_user_management.py b/perma_web/perma/tests/test_views_user_management.py index 45232395d..2de94a4e9 100644 --- a/perma_web/perma/tests/test_views_user_management.py +++ b/perma_web/perma/tests/test_views_user_management.py @@ -227,7 +227,7 @@ def test_registrar_cannot_update_unrelated_registrar(self): self.get('user_management_manage_single_registrar', user=self.registrar_user, reverse_kwargs={'args': [self.unrelated_registrar.pk]}, - require_status_code=404) + require_status_code=403) def test_admin_can_approve_pending_registrar(self): self.submit_form('user_management_approve_pending_registrar', @@ -538,13 +538,13 @@ def test_registrar_cannot_update_unrelated_organization(self): self.get('user_management_manage_single_organization', user=self.registrar_user, reverse_kwargs={'args': [self.unrelated_organization.pk]}, - require_status_code=404) + require_status_code=403) def test_org_user_cannot_update_unrelated_organization(self): self.get('user_management_manage_single_organization', user=self.organization_user, reverse_kwargs={'args': [self.unrelated_organization.pk]}, - require_status_code=404) + require_status_code=403) def _delete_organization(self, user, should_succeed=True): if should_succeed: @@ -833,14 +833,14 @@ def test_registrar_can_edit_org_user(self): # User from another registrar's org fails self.get('user_management_manage_single_organization_user', reverse_kwargs={'args': [self.another_unrelated_organization_user.pk]}, - require_status_code=404) + require_status_code=403) # Repeat with the other registrar, to confirm we're # getting 404s because of permission reasons, not because the # test fixtures are broken. self.log_in_user(self.unrelated_registrar_user) self.get('user_management_manage_single_organization_user', reverse_kwargs={'args': [self.organization_user.pk]}, - require_status_code=404) + require_status_code=403) self.get('user_management_manage_single_organization_user', reverse_kwargs={'args': [self.another_unrelated_organization_user.pk]}) @@ -855,15 +855,13 @@ def test_org_can_edit_org_user(self): # User from another org fails self.get('user_management_manage_single_organization_user', reverse_kwargs={'args': [self.pk_from_email(org_two_users[0])]}, - require_status_code=404) - # Repeat in reverse, to confirm we're - # getting 404s because of permission reasons, not because the - # test fixtures are broken. + require_status_code=403) + + # Repeat with another org self.log_in_user(org_two_users[1]) self.get('user_management_manage_single_organization_user', reverse_kwargs={'args': [self.pk_from_email(org_one_users[1])]}, - require_status_code=404) - # User from another org fails + require_status_code=403) self.get('user_management_manage_single_organization_user', reverse_kwargs={'args': [self.pk_from_email(org_two_users[0])]}) diff --git a/perma_web/perma/views/user_management.py b/perma_web/perma/views/user_management.py index f88627020..152e57e0a 100755 --- a/perma_web/perma/views/user_management.py +++ b/perma_web/perma/views/user_management.py @@ -465,7 +465,7 @@ def manage_single_registrar(request, registrar_id): target_registrar = get_object_or_404(Registrar, id=registrar_id) if not request.user.can_edit_registrar(target_registrar): - raise Http404 + return HttpResponseForbidden() form = RegistrarForm(get_form_data(request), prefix = "a", instance=target_registrar) if request.method == 'POST': @@ -580,10 +580,9 @@ def manage_organization(request): @user_passes_test_or_403(lambda user: user.is_staff or user.is_registrar_user() or user.is_organization_user) def manage_single_organization(request, org_id): """ Edit organization details. """ - try: - target_org = Organization.objects.accessible_to(request.user).get(pk=org_id) - except Organization.DoesNotExist: - raise Http404 + target_org = get_object_or_404(Organization, id=org_id) + if not request.user.can_edit_organization(target_org): + return HttpResponseForbidden() if request.user.is_staff: form = OrganizationWithRegistrarForm(get_form_data(request), prefix = "a", instance=target_org) @@ -607,14 +606,13 @@ def manage_single_organization_delete(request, org_id): """ Delete an empty org """ - try: - target_org = Organization.objects.accessible_to(request.user).get(pk=org_id) - except Organization.DoesNotExist: - raise Http404 + target_org = get_object_or_404(Organization, id=org_id) + if not request.user.can_edit_organization(target_org): + return HttpResponseForbidden() if request.method == 'POST': if target_org.links.count() > 0: - raise Http404 + return HttpResponseForbidden() target_org.safe_delete() target_org.save() @@ -743,6 +741,10 @@ def manage_single_organization_export_user_list( request: HttpRequest, org_id: int ) -> HttpResponse | JsonResponse: """Return a file listing all users belonging to an organization.""" + target_org = get_object_or_404(Organization, id=org_id) + if not request.user.can_edit_organization(target_org): + return HttpResponseForbidden() + org_users = ( LinkUser.objects.filter(organizations__id=org_id) .annotate(organization_name=F('organizations__name')) @@ -1004,7 +1006,7 @@ def edit_user_in_group(request, user_id, group_name): affiliations = user_org_affiliations.filter(organization__in=registrar_orgs) if not sponsorships and len(affiliations) == 0: - raise Http404 + return HttpResponseForbidden() elif request.user.is_organization_user: # org users can only edit their members in the same orgs @@ -1013,7 +1015,7 @@ def edit_user_in_group(request, user_id, group_name): affiliations = user_org_affiliations.filter(organization__in=request.user.organizations.all()) if len(affiliations) == 0: - raise Http404 + return HttpResponseForbidden() else: # Must be admin user @@ -1276,14 +1278,15 @@ def manage_single_organization_user_remove(request, user_id): """ Remove an organization user from an org. """ + target_user = get_object_or_404(LinkUser, id=user_id) + if not request.user.shares_scope_with_user(target_user): + return HttpResponseForbidden() if request.method == 'POST': - try: - org = Organization.objects.accessible_to(request.user).get(pk=request.POST.get('org')) - except Organization.DoesNotExist: - raise Http404 + org = get_object_or_404(Organization, id=request.POST.get('org')) + if not request.user.can_edit_organization(org): + return HttpResponseForbidden() - target_user = get_object_or_404(LinkUser, id=user_id) target_user.organizations.remove(org) # special case -- user demoted themselves, can't see page anymore @@ -1303,7 +1306,7 @@ def manage_single_registrar_user_remove(request, user_id): # Registrar users can only edit their own registrar users if request.user.registrar_id != target_user.registrar_id: - raise Http404 + return HttpResponseForbidden() context = {'target_user': target_user, 'this_page': 'organization_user'} @@ -1331,7 +1334,7 @@ def toggle_status(request, user_id, registrar_id, status): if request.user.is_registrar_user() and \ (request.user.registrar not in target_user.sponsoring_registrars.all() or str(request.user.registrar_id) != registrar_id): - raise Http404 + return HttpResponseForbidden() if request.method == 'POST': sponsorship.status = status @@ -1366,7 +1369,7 @@ def manage_single_sponsored_user_links(request, user_id, registrar_id): if request.user.is_registrar_user() and \ (request.user.registrar not in target_user.sponsoring_registrars.all() or str(request.user.registrar_id) != registrar_id): - raise Http404 + return HttpResponseForbidden() folders = Folder.objects.filter(owned_by=target_user, sponsored_by=registrar) links = Link.objects.filter(folders__in=folders).select_related('capture_job').prefetch_related('captures').order_by('-creation_timestamp')