From fa230de03bb950f4c6b5c8e38f16803efcbff3f5 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 21 Jul 2015 15:58:34 +0100 Subject: [PATCH 1/3] Fix app label for site permissions - should be 'wagtailcore', not 'site' --- wagtail/wagtailsites/templates/wagtailsites/edit.html | 2 +- wagtail/wagtailsites/templates/wagtailsites/index.html | 2 +- wagtail/wagtailsites/views.py | 9 +++++---- wagtail/wagtailsites/wagtail_hooks.py | 7 ++++++- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/wagtail/wagtailsites/templates/wagtailsites/edit.html b/wagtail/wagtailsites/templates/wagtailsites/edit.html index 96e06e2b0..5bb27278c 100644 --- a/wagtail/wagtailsites/templates/wagtailsites/edit.html +++ b/wagtail/wagtailsites/templates/wagtailsites/edit.html @@ -21,7 +21,7 @@
  • - {% if perms.site.delete_site %} + {% if perms.wagtailcore.delete_site %} {% trans "Delete site" %} {% endif %}
    • diff --git a/wagtail/wagtailsites/templates/wagtailsites/index.html b/wagtail/wagtailsites/templates/wagtailsites/index.html index 9481ca77d..ddf0abc02 100644 --- a/wagtail/wagtailsites/templates/wagtailsites/index.html +++ b/wagtail/wagtailsites/templates/wagtailsites/index.html @@ -3,7 +3,7 @@ {% block titletag %}{% trans "Sites" %}{% endblock %} {% block content %} {% trans "Sites" as sites_str %} - {% if perms.site.add_site %} + {% if perms.wagtailcore.add_site %} {% trans "Add a site" as add_a_site_str %} {% include "wagtailadmin/shared/header.html" with title=sites_str add_link="wagtailsites:add" add_text=add_a_site_str icon="site" %} {% else %} diff --git a/wagtail/wagtailsites/views.py b/wagtail/wagtailsites/views.py index 8d98ad431..b88d03b54 100644 --- a/wagtail/wagtailsites/views.py +++ b/wagtail/wagtailsites/views.py @@ -7,9 +7,10 @@ from wagtail.wagtailcore.models import Site from wagtail.wagtailsites.forms import SiteForm from wagtail.wagtailadmin import messages + def user_has_site_model_perm(user): for verb in ['add', 'change', 'delete']: - if user.has_perm('site.%s_site' % verb): + if user.has_perm('wagtailcore.%s_site' % verb): return True return False @@ -22,7 +23,7 @@ def index(request): }) -@permission_required('site.add_site') +@permission_required('wagtailcore.add_site') def create(request): if request.POST: form = SiteForm(request.POST) @@ -42,7 +43,7 @@ def create(request): }) -@permission_required('site.change_site') +@permission_required('wagtailcore.change_site') def edit(request, site_id): site = get_object_or_404(Site, id=site_id) @@ -65,7 +66,7 @@ def edit(request, site_id): }) -@permission_required('site.delete_site') +@permission_required('wagtailcore.delete_site') def delete(request, site_id): site = get_object_or_404(Site, id=site_id) diff --git a/wagtail/wagtailsites/wagtail_hooks.py b/wagtail/wagtailsites/wagtail_hooks.py index 410ba0c51..ae837d5c6 100644 --- a/wagtail/wagtailsites/wagtail_hooks.py +++ b/wagtail/wagtailsites/wagtail_hooks.py @@ -17,7 +17,12 @@ def register_admin_urls(): class SitesMenuItem(MenuItem): def is_shown(self, request): - return request.user.is_superuser + return ( + request.user.has_perm('wagtailcore.add_site') + or request.user.has_perm('wagtailcore.edit_site') + or request.user.has_perm('wagtailcore.delete_site') + ) + @hooks.register('register_settings_menu_item') def register_sites_menu_item(): From bf21a0cbe5ea9d8b009910a4072b61f38dab772a Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 21 Jul 2015 17:03:55 +0100 Subject: [PATCH 2/3] Add tests for accessing site admin with non-superuser permissions --- wagtail/wagtailsites/tests.py | 83 +++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/wagtail/wagtailsites/tests.py b/wagtail/wagtailsites/tests.py index ab97b401f..84769bd18 100644 --- a/wagtail/wagtailsites/tests.py +++ b/wagtail/wagtailsites/tests.py @@ -2,6 +2,8 @@ from __future__ import unicode_literals from django.test import TestCase from django.core.urlresolvers import reverse from django.utils import six +from django.contrib.auth import get_user_model +from django.contrib.auth.models import Permission from wagtail.tests.utils import WagtailTestUtils from wagtail.wagtailcore.models import Site, Page @@ -252,3 +254,84 @@ class TestSiteDeleteView(TestCase, WagtailTestUtils): # Check that the site was edited with self.assertRaises(Site.DoesNotExist): Site.objects.get(id=self.localhost.id) + + +class TestLimitedPermissions(TestCase, WagtailTestUtils): + def setUp(self): + # Create a user + user = get_user_model().objects.create_user(username='test', email='test@email.com', password='password') + user.user_permissions.add( + Permission.objects.get(codename='access_admin'), + Permission.objects.get(codename='add_site'), + Permission.objects.get(codename='change_site'), + Permission.objects.get(codename='delete_site') + ) + + # Login + self.client.login(username='test', password='password') + + self.home_page = Page.objects.get(id=2) + self.localhost = Site.objects.all()[0] + + def test_get_index(self): + response = self.client.get(reverse('wagtailsites:index')) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailsites/index.html') + + def test_get_create_view(self): + response = self.client.get(reverse('wagtailsites:add')) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailsites/create.html') + + def test_create(self): + response = self.client.post(reverse('wagtailsites:add'), { + 'hostname': "testsite", + 'port': "80", + 'root_page': str(self.home_page.id), + }) + + # Should redirect back to index + self.assertRedirects(response, reverse('wagtailsites:index')) + + # Check that the site was created + self.assertEqual(Site.objects.filter(hostname='testsite').count(), 1) + + def test_get_edit_view(self): + edit_url = reverse('wagtailsites:edit', args=(self.localhost.id,)) + response = self.client.get(edit_url) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailsites/edit.html') + + def test_edit(self): + edit_url = reverse('wagtailsites:edit', args=(self.localhost.id,)) + edited_hostname = 'edited' + response = self.client.post(edit_url, { + 'hostname': edited_hostname, + 'port': 80, + 'root_page': self.home_page.id, + }) + + # Should redirect back to index + self.assertRedirects(response, reverse('wagtailsites:index')) + + # Check that the site was edited + self.assertEqual(Site.objects.get(id=self.localhost.id).hostname, edited_hostname) + + def test_get_delete_view(self): + delete_url = reverse('wagtailsites:delete', args=(self.localhost.id,)) + response = self.client.get(delete_url) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailsites/confirm_delete.html') + + def test_delete(self): + delete_url = reverse('wagtailsites:delete', args=(self.localhost.id,)) + response = self.client.post(delete_url, { + 'trivial_key': 'trivial_value' + }) + + # Should redirect back to index + self.assertRedirects(response, reverse('wagtailsites:index')) + + # Check that the site was edited + with self.assertRaises(Site.DoesNotExist): + Site.objects.get(id=self.localhost.id) From 41b6a7a9c8ffa3956051d125ed81bf8d9dfc4743 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 21 Jul 2015 17:06:21 +0100 Subject: [PATCH 3/3] Fix request.POST checks so that we don't need a dummy value in tests --- wagtail/wagtailsites/tests.py | 8 ++------ wagtail/wagtailsites/views.py | 6 +++--- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/wagtail/wagtailsites/tests.py b/wagtail/wagtailsites/tests.py index 84769bd18..c506bc7a0 100644 --- a/wagtail/wagtailsites/tests.py +++ b/wagtail/wagtailsites/tests.py @@ -244,9 +244,7 @@ class TestSiteDeleteView(TestCase, WagtailTestUtils): self.assertEqual(self.get(site_id=100000).status_code, 404) def test_posting_deletes_site(self): - response = self.post({ - 'trivial_key': 'trivial_value' - }) + response = self.post() # Should redirect back to index self.assertRedirects(response, reverse('wagtailsites:index')) @@ -325,9 +323,7 @@ class TestLimitedPermissions(TestCase, WagtailTestUtils): def test_delete(self): delete_url = reverse('wagtailsites:delete', args=(self.localhost.id,)) - response = self.client.post(delete_url, { - 'trivial_key': 'trivial_value' - }) + response = self.client.post(delete_url) # Should redirect back to index self.assertRedirects(response, reverse('wagtailsites:index')) diff --git a/wagtail/wagtailsites/views.py b/wagtail/wagtailsites/views.py index b88d03b54..2475be0a2 100644 --- a/wagtail/wagtailsites/views.py +++ b/wagtail/wagtailsites/views.py @@ -25,7 +25,7 @@ def index(request): @permission_required('wagtailcore.add_site') def create(request): - if request.POST: + if request.method == 'POST': form = SiteForm(request.POST) if form.is_valid(): site = form.save() @@ -47,7 +47,7 @@ def create(request): def edit(request, site_id): site = get_object_or_404(Site, id=site_id) - if request.POST: + if request.method == 'POST': form = SiteForm(request.POST, instance=site) if form.is_valid(): site = form.save() @@ -70,7 +70,7 @@ def edit(request, site_id): def delete(request, site_id): site = get_object_or_404(Site, id=site_id) - if request.POST: + if request.method == 'POST': site.delete() messages.success(request, _("Site '{0}' deleted.").format(site.hostname)) return redirect('wagtailsites:index')