From 6e32b6cf9b296edc48730fba5ae3640f47b3bef5 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Mon, 2 Jun 2014 14:18:19 +0100 Subject: [PATCH 01/33] Update Page.route API so that it returns a Page instance rather than an HttpResponse; catch the latter in the 'serve' view for backward compatibility --- wagtail/tests/fixtures/test.json | 25 +++++++++++++++++++- wagtail/tests/models.py | 13 ++++++++++ wagtail/wagtailcore/models.py | 2 +- wagtail/wagtailcore/tests/test_page_model.py | 21 ++++++++++++++-- wagtail/wagtailcore/views.py | 14 +++++++++-- 5 files changed, 69 insertions(+), 6 deletions(-) diff --git a/wagtail/tests/fixtures/test.json b/wagtail/tests/fixtures/test.json index 2d3db6e35..b769b7960 100644 --- a/wagtail/tests/fixtures/test.json +++ b/wagtail/tests/fixtures/test.json @@ -23,7 +23,7 @@ "model": "wagtailcore.page", "fields": { "title": "Welcome to the Wagtail test site!", - "numchild": 3, + "numchild": 4, "show_in_menus": false, "live": true, "depth": 2, @@ -257,6 +257,29 @@ } }, +{ + "pk": 10, + "model": "wagtailcore.page", + "fields": { + "title": "Old style route method", + "numchild": 0, + "show_in_menus": true, + "live": true, + "depth": 3, + "content_type": ["tests", "pagewitholdstyleroutemethod"], + "path": "000100010004", + "url_path": "/home/old-style-route/", + "slug": "old-style-route" + } +}, +{ + "pk": 10, + "model": "tests.pagewitholdstyleroutemethod", + "fields": { + "content": "

Test with old style route method

" + } +}, + { "pk": 1, "model": "wagtailcore.site", diff --git a/wagtail/tests/models.py b/wagtail/tests/models.py index ff6dc2dcc..0c369dd34 100644 --- a/wagtail/tests/models.py +++ b/wagtail/tests/models.py @@ -106,6 +106,19 @@ class SimplePage(Page): content = models.TextField() +class PageWithOldStyleRouteMethod(Page): + """ + Prior to Wagtail 0.4, the route() method on Page returned an HttpResponse + rather than a Page instance. As subclasses of Page may override route, + we need to continue accepting this convention (albeit as a deprecated API). + """ + content = models.TextField() + template = 'tests/simple_page.html' + + def route(self, request, path_components): + return self.serve(request) + + # Event page class EventPageCarouselItem(Orderable, CarouselItem): diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index cf33bcbfb..d8edbfc72 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -415,7 +415,7 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, Indexed)): else: # request is for this very page if self.live: - return self.serve(request) + return self else: raise Http404 diff --git a/wagtail/wagtailcore/tests/test_page_model.py b/wagtail/wagtailcore/tests/test_page_model.py index da9fc6174..0543bf32c 100644 --- a/wagtail/wagtailcore/tests/test_page_model.py +++ b/wagtail/wagtailcore/tests/test_page_model.py @@ -2,7 +2,7 @@ from django.test import TestCase, Client from django.http import HttpRequest, Http404 from wagtail.wagtailcore.models import Page, Site -from wagtail.tests.models import EventPage, EventIndex, SimplePage +from wagtail.tests.models import EventPage, EventIndex, SimplePage, PageWithOldStyleRouteMethod class TestSiteRouting(TestCase): @@ -136,8 +136,13 @@ class TestRouting(TestCase): request = HttpRequest() request.path = '/events/christmas/' - response = homepage.route(request, ['events', 'christmas']) + found_page = homepage.route(request, ['events', 'christmas']) + self.assertEqual(found_page, christmas_page) + def test_request_serving(self): + christmas_page = EventPage.objects.get(url_path='/home/events/christmas/') + request = HttpRequest() + response = christmas_page.serve(request) self.assertEqual(response.status_code, 200) self.assertEqual(response.context_data['self'], christmas_page) used_template = response.resolve_template(response.template_name) @@ -226,6 +231,18 @@ class TestServeView(TestCase): self.assertContains(response, 'Christmas') + def test_old_style_routing(self): + """ + Test that route() methods that return an HttpResponse are correctly handled + """ + response = self.client.get('/old-style-route/') + expected_page = PageWithOldStyleRouteMethod.objects.get(url_path='/home/old-style-route/') + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.context['self'], expected_page) + self.assertEqual(response.templates[0].name, 'tests/simple_page.html') + + class TestStaticSitePaths(TestCase): def setUp(self): self.root_page = Page.objects.get(id=1) diff --git a/wagtail/wagtailcore/views.py b/wagtail/wagtailcore/views.py index 1b1f6cf43..7377cf761 100644 --- a/wagtail/wagtailcore/views.py +++ b/wagtail/wagtailcore/views.py @@ -1,4 +1,6 @@ -from django.http import Http404 +import warnings + +from django.http import HttpResponse, Http404 def serve(request, path): @@ -8,4 +10,12 @@ def serve(request, path): raise Http404 path_components = [component for component in path.split('/') if component] - return request.site.root_page.specific.route(request, path_components) + page = request.site.root_page.specific.route(request, path_components) + if isinstance(page, HttpResponse): + warnings.warn( + "Page.route should return a Page, not an HttpResponse", + DeprecationWarning + ) + return page + + return page.serve(request) From 9cc3614097b1ea645eabf63a45fe6e2338a641af Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Mon, 2 Jun 2014 23:00:20 +0100 Subject: [PATCH 02/33] Catch the deprecation warning for old-style routes during unit tests --- wagtail/wagtailcore/tests/test_page_model.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/wagtail/wagtailcore/tests/test_page_model.py b/wagtail/wagtailcore/tests/test_page_model.py index 0543bf32c..34f8356f2 100644 --- a/wagtail/wagtailcore/tests/test_page_model.py +++ b/wagtail/wagtailcore/tests/test_page_model.py @@ -1,3 +1,5 @@ +import warnings + from django.test import TestCase, Client from django.http import HttpRequest, Http404 @@ -235,9 +237,15 @@ class TestServeView(TestCase): """ Test that route() methods that return an HttpResponse are correctly handled """ - response = self.client.get('/old-style-route/') - expected_page = PageWithOldStyleRouteMethod.objects.get(url_path='/home/old-style-route/') + with warnings.catch_warnings(record=True) as w: + response = self.client.get('/old-style-route/') + # Check that a DeprecationWarning has been triggered + self.assertEqual(len(w), 1) + self.assertTrue(issubclass(w[-1].category, DeprecationWarning)) + self.assertTrue("Page.route should return a Page" in str(w[-1].message)) + + expected_page = PageWithOldStyleRouteMethod.objects.get(url_path='/home/old-style-route/') self.assertEqual(response.status_code, 200) self.assertEqual(response.context['self'], expected_page) self.assertEqual(response.templates[0].name, 'tests/simple_page.html') From df15ece5dcef3962b5c0b2ee914018491bf20e9a Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Mon, 2 Jun 2014 23:30:00 +0100 Subject: [PATCH 03/33] Implement before_serve_page hook --- wagtail/tests/wagtail_hooks.py | 8 ++++++++ wagtail/wagtailcore/tests/test_page_model.py | 4 ++++ wagtail/wagtailcore/views.py | 7 +++++++ 3 files changed, 19 insertions(+) diff --git a/wagtail/tests/wagtail_hooks.py b/wagtail/tests/wagtail_hooks.py index 13e3e46d2..4ad63e881 100644 --- a/wagtail/tests/wagtail_hooks.py +++ b/wagtail/tests/wagtail_hooks.py @@ -1,3 +1,5 @@ +from django.http import HttpResponse + from wagtail.wagtailcore import hooks from wagtail.wagtailcore.whitelist import attribute_rule, check_url, allow_without_attributes @@ -17,3 +19,9 @@ def whitelister_element_rules(): 'a': attribute_rule({'href': check_url, 'target': True}), } hooks.register('construct_whitelister_element_rules', whitelister_element_rules) + + +def block_googlebot(page, request): + if request.META.get('HTTP_USER_AGENT') == 'GoogleBot': + return HttpResponse("

bad googlebot no cookie

") +hooks.register('before_serve_page', block_googlebot) diff --git a/wagtail/wagtailcore/tests/test_page_model.py b/wagtail/wagtailcore/tests/test_page_model.py index 34f8356f2..ab5b9c148 100644 --- a/wagtail/wagtailcore/tests/test_page_model.py +++ b/wagtail/wagtailcore/tests/test_page_model.py @@ -250,6 +250,10 @@ class TestServeView(TestCase): self.assertEqual(response.context['self'], expected_page) self.assertEqual(response.templates[0].name, 'tests/simple_page.html') + def test_before_serve_hook(self): + response = self.client.get('/events/', HTTP_USER_AGENT='GoogleBot') + self.assertContains(response, 'bad googlebot no cookie') + class TestStaticSitePaths(TestCase): def setUp(self): diff --git a/wagtail/wagtailcore/views.py b/wagtail/wagtailcore/views.py index 7377cf761..0872dd68f 100644 --- a/wagtail/wagtailcore/views.py +++ b/wagtail/wagtailcore/views.py @@ -2,6 +2,8 @@ import warnings from django.http import HttpResponse, Http404 +from wagtail.wagtailcore import hooks + def serve(request, path): # we need a valid Site object corresponding to this request (set in wagtail.wagtailcore.middleware.SiteMiddleware) @@ -18,4 +20,9 @@ def serve(request, path): ) return page + for fn in hooks.get_hooks('before_serve_page'): + result = fn(page, request) + if isinstance(result, HttpResponse): + return result + return page.serve(request) From 4af5f3d24d50ebaa0f65dba76b678115c3afca07 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 3 Jun 2014 11:22:46 +0100 Subject: [PATCH 04/33] Add PageViewRestriction model, and apply permission check in the 'serve' view --- .../0005_create_page_view_restriction.py | 117 ++++++++++++++++++ wagtail/wagtailcore/models.py | 7 ++ wagtail/wagtailcore/wagtail_hooks.py | 11 ++ 3 files changed, 135 insertions(+) create mode 100644 wagtail/wagtailcore/migrations/0005_create_page_view_restriction.py create mode 100644 wagtail/wagtailcore/wagtail_hooks.py diff --git a/wagtail/wagtailcore/migrations/0005_create_page_view_restriction.py b/wagtail/wagtailcore/migrations/0005_create_page_view_restriction.py new file mode 100644 index 000000000..f2d79573f --- /dev/null +++ b/wagtail/wagtailcore/migrations/0005_create_page_view_restriction.py @@ -0,0 +1,117 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from south.utils import datetime_utils as datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding model 'PageViewRestriction' + db.create_table('wagtailcore_pageviewrestriction', ( + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('page', self.gf('django.db.models.fields.related.ForeignKey')(related_name='view_restrictions', to=orm['wagtailcore.Page'])), + ('password', self.gf('django.db.models.fields.CharField')(max_length=255)), + )) + db.send_create_signal('wagtailcore', ['PageViewRestriction']) + + + def backwards(self, orm): + # Deleting model 'PageViewRestriction' + db.delete_table('wagtailcore_pageviewrestriction') + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'symmetrical': 'False', 'related_name': "'user_set'", 'blank': 'True', 'to': "orm['auth.Group']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'symmetrical': 'False', 'related_name': "'user_set'", 'blank': 'True', 'to': "orm['auth.Permission']"}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'wagtailcore.grouppagepermission': { + 'Meta': {'object_name': 'GroupPagePermission'}, + 'group': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'page_permissions'", 'to': "orm['auth.Group']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'page': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'group_permissions'", 'to': "orm['wagtailcore.Page']"}), + 'permission_type': ('django.db.models.fields.CharField', [], {'max_length': '20'}) + }, + 'wagtailcore.page': { + 'Meta': {'object_name': 'Page'}, + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'pages'", 'to': "orm['contenttypes.ContentType']"}), + 'depth': ('django.db.models.fields.PositiveIntegerField', [], {}), + 'expire_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'blank': 'True'}), + 'expired': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'go_live_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'blank': 'True'}), + 'has_unpublished_changes': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'live': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'numchild': ('django.db.models.fields.PositiveIntegerField', [], {'default': '0'}), + 'owner': ('django.db.models.fields.related.ForeignKey', [], {'blank': 'True', 'related_name': "'owned_pages'", 'null': 'True', 'to': "orm['auth.User']"}), + 'path': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}), + 'search_description': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'seo_title': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}), + 'show_in_menus': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'slug': ('django.db.models.fields.SlugField', [], {'max_length': '50'}), + 'title': ('django.db.models.fields.CharField', [], {'max_length': '255'}), + 'url_path': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}) + }, + 'wagtailcore.pagerevision': { + 'Meta': {'object_name': 'PageRevision'}, + 'approved_go_live_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'blank': 'True'}), + 'content_json': ('django.db.models.fields.TextField', [], {}), + 'created_at': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'page': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'revisions'", 'to': "orm['wagtailcore.Page']"}), + 'submitted_for_moderation': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True', 'blank': 'True'}) + }, + 'wagtailcore.pageviewrestriction': { + 'Meta': {'object_name': 'PageViewRestriction'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'page': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'view_restrictions'", 'to': "orm['wagtailcore.Page']"}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '255'}) + }, + 'wagtailcore.site': { + 'Meta': {'unique_together': "(('hostname', 'port'),)", 'object_name': 'Site'}, + 'hostname': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_default_site': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'port': ('django.db.models.fields.IntegerField', [], {'default': '80'}), + 'root_page': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'sites_rooted_here'", 'to': "orm['wagtailcore.Page']"}) + } + } + + complete_apps = ['wagtailcore'] \ No newline at end of file diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index d8edbfc72..b035a0856 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -9,6 +9,8 @@ from modelcluster.models import ClusterableModel from django.db import models, connection, transaction from django.db.models import get_model, Q +from django.db.models.signals import post_save +from django.dispatch import receiver from django.http import Http404 from django.core.cache import cache from django.core.handlers.wsgi import WSGIRequest @@ -1150,3 +1152,8 @@ class PagePermissionTester(object): else: # no publishing required, so the already-tested 'add' permission is sufficient return True + + +class PageViewRestriction(models.Model): + page = models.ForeignKey('Page', related_name='view_restrictions') + password = models.CharField(max_length=255) diff --git a/wagtail/wagtailcore/wagtail_hooks.py b/wagtail/wagtailcore/wagtail_hooks.py new file mode 100644 index 000000000..1dd480001 --- /dev/null +++ b/wagtail/wagtailcore/wagtail_hooks.py @@ -0,0 +1,11 @@ +from django.http import HttpResponse + +from wagtail.wagtailcore import hooks +from wagtail.wagtailcore.models import PageViewRestriction + +def check_view_restrictions(page, request): + ancestors_and_self = list(page.get_ancestors().values_list('id', flat=True)) + [page] + restrictions = PageViewRestriction.objects.filter(page__in=ancestors_and_self) + for restriction in restrictions: + return HttpResponse("

Blocked due to view restriction

") +hooks.register('before_serve_page', check_view_restrictions) From 6f855d7fd86556956248ded90c7e27832b3a713d Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 3 Jun 2014 15:34:27 +0100 Subject: [PATCH 05/33] simplified get_ancestors logic --- wagtail/wagtailcore/wagtail_hooks.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/wagtail/wagtailcore/wagtail_hooks.py b/wagtail/wagtailcore/wagtail_hooks.py index 1dd480001..73c98a01d 100644 --- a/wagtail/wagtailcore/wagtail_hooks.py +++ b/wagtail/wagtailcore/wagtail_hooks.py @@ -4,8 +4,7 @@ from wagtail.wagtailcore import hooks from wagtail.wagtailcore.models import PageViewRestriction def check_view_restrictions(page, request): - ancestors_and_self = list(page.get_ancestors().values_list('id', flat=True)) + [page] - restrictions = PageViewRestriction.objects.filter(page__in=ancestors_and_self) + restrictions = PageViewRestriction.objects.filter(page__in=page.get_ancestors(inclusive=True)) for restriction in restrictions: return HttpResponse("

Blocked due to view restriction

") hooks.register('before_serve_page', check_view_restrictions) From d0e93f997f5caf8be28989d1907413f12c3e9b56 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 3 Jun 2014 17:24:04 +0100 Subject: [PATCH 06/33] Delegate serving of the 'password required' response to the Page model --- wagtail/wagtailcore/models.py | 8 ++++++++ .../templates/wagtailcore/password_required.html | 9 +++++++++ wagtail/wagtailcore/wagtail_hooks.py | 4 +--- 3 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 wagtail/wagtailcore/templates/wagtailcore/password_required.html diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index b035a0856..59c9eb861 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -811,6 +811,14 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, Indexed)): def get_prev_siblings(self, inclusive=False): return self.get_siblings(inclusive).filter(path__lte=self.path).order_by('-path') + password_required_template = getattr(settings, 'PASSWORD_REQUIRED_TEMPLATE', 'wagtailcore/password_required.html') + def serve_password_required_response(self, request, form): + return TemplateResponse(request, self.password_required_template, { + 'self': self, + 'request': request, + 'form': form, + }) + def get_navigation_menu_items(): # Get all pages that appear in the navigation menu: ones which have children, diff --git a/wagtail/wagtailcore/templates/wagtailcore/password_required.html b/wagtail/wagtailcore/templates/wagtailcore/password_required.html new file mode 100644 index 000000000..d4ee0b1fc --- /dev/null +++ b/wagtail/wagtailcore/templates/wagtailcore/password_required.html @@ -0,0 +1,9 @@ + + + + Password required + + +

Password required

+ + diff --git a/wagtail/wagtailcore/wagtail_hooks.py b/wagtail/wagtailcore/wagtail_hooks.py index 73c98a01d..347194c32 100644 --- a/wagtail/wagtailcore/wagtail_hooks.py +++ b/wagtail/wagtailcore/wagtail_hooks.py @@ -1,10 +1,8 @@ -from django.http import HttpResponse - from wagtail.wagtailcore import hooks from wagtail.wagtailcore.models import PageViewRestriction def check_view_restrictions(page, request): restrictions = PageViewRestriction.objects.filter(page__in=page.get_ancestors(inclusive=True)) for restriction in restrictions: - return HttpResponse("

Blocked due to view restriction

") + return page.serve_password_required_response(request, None) hooks.register('before_serve_page', check_view_restrictions) From e4008a814824a24f36c288f7309c4867b8919870 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Wed, 4 Jun 2014 15:28:03 +0100 Subject: [PATCH 07/33] implement form submission logic for the password form --- wagtail/wagtailcore/forms.py | 16 +++++++++++++ wagtail/wagtailcore/models.py | 18 +++++++++----- .../wagtailcore/password_required.html | 6 +++++ wagtail/wagtailcore/urls.py | 5 +++- wagtail/wagtailcore/views.py | 24 +++++++++++++++++++ wagtail/wagtailcore/wagtail_hooks.py | 10 +++++++- 6 files changed, 71 insertions(+), 8 deletions(-) create mode 100644 wagtail/wagtailcore/forms.py diff --git a/wagtail/wagtailcore/forms.py b/wagtail/wagtailcore/forms.py new file mode 100644 index 000000000..1b83c8c94 --- /dev/null +++ b/wagtail/wagtailcore/forms.py @@ -0,0 +1,16 @@ +from django import forms + +class PasswordPageViewRestrictionForm(forms.Form): + password = forms.CharField(label="Password", widget=forms.PasswordInput) + return_url = forms.CharField(widget=forms.HiddenInput) + + def __init__(self, *args, **kwargs): + self.restriction = kwargs.pop('instance') + super(PasswordPageViewRestrictionForm, self).__init__(*args, **kwargs) + + def clean_password(self): + data = self.cleaned_data['password'] + if data != self.restriction.password: + raise forms.ValidationError("The password you have entered is not correct. Please try again.") + + return data diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 59c9eb861..08e1b612b 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -812,12 +812,18 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, Indexed)): return self.get_siblings(inclusive).filter(path__lte=self.path).order_by('-path') password_required_template = getattr(settings, 'PASSWORD_REQUIRED_TEMPLATE', 'wagtailcore/password_required.html') - def serve_password_required_response(self, request, form): - return TemplateResponse(request, self.password_required_template, { - 'self': self, - 'request': request, - 'form': form, - }) + def serve_password_required_response(self, request, form, action_url): + """ + Serve a response indicating that the user has been denied access to view this page, + and must supply a password. + form = a Django form object containing the password input + (and zero or more hidden fields that also need to be output on the template) + action_url = URL that this form should be POSTed to + """ + context = self.get_context(request) + context['form'] = form + context['action_url'] = action_url + return TemplateResponse(request, self.password_required_template, context) def get_navigation_menu_items(): diff --git a/wagtail/wagtailcore/templates/wagtailcore/password_required.html b/wagtail/wagtailcore/templates/wagtailcore/password_required.html index d4ee0b1fc..d7601ca69 100644 --- a/wagtail/wagtailcore/templates/wagtailcore/password_required.html +++ b/wagtail/wagtailcore/templates/wagtailcore/password_required.html @@ -5,5 +5,11 @@

Password required

+

You need a password to access this page.

+
+ {% csrf_token %} + {{ form.as_p }} + +
diff --git a/wagtail/wagtailcore/urls.py b/wagtail/wagtailcore/urls.py index f0336fec4..9777dea8a 100644 --- a/wagtail/wagtailcore/urls.py +++ b/wagtail/wagtailcore/urls.py @@ -2,7 +2,10 @@ from django.conf.urls import url from wagtail.wagtailcore import views urlpatterns = [ - # All front-end views are handled through Wagtail's core.views.serve mechanism. + url(r'^_util/authenticate_with_password/(\d+)/(\d+)/$', views.authenticate_with_password, + name='wagtailcore_authenticate_with_password'), + + # Front-end page views are handled through Wagtail's core.views.serve mechanism. # Here we match a (possibly empty) list of path segments, each followed by # a '/'. If a trailing slash is not present, we leave CommonMiddleware to # handle it as usual (i.e. redirect it to the trailing slash version if diff --git a/wagtail/wagtailcore/views.py b/wagtail/wagtailcore/views.py index 0872dd68f..bd12d7f97 100644 --- a/wagtail/wagtailcore/views.py +++ b/wagtail/wagtailcore/views.py @@ -1,8 +1,12 @@ import warnings from django.http import HttpResponse, Http404 +from django.shortcuts import get_object_or_404, redirect +from django.core.urlresolvers import reverse from wagtail.wagtailcore import hooks +from wagtail.wagtailcore.models import Page, PageViewRestriction +from wagtail.wagtailcore.forms import PasswordPageViewRestrictionForm def serve(request, path): @@ -26,3 +30,23 @@ def serve(request, path): return result return page.serve(request) + + +def authenticate_with_password(request, page_view_restriction_id, page_id): + """ + Handle a submission of PasswordPageViewRestrictionForm to grant view access over a + subtree that is protected by a PageViewRestriction + """ + restriction = get_object_or_404(PageViewRestriction, id=page_view_restriction_id) + page = get_object_or_404(Page, id=page_id).specific + + if request.POST: + form = PasswordPageViewRestrictionForm(request.POST, instance=restriction) + if form.is_valid(): + # TODO: record 'has authenticated against this page view restriction' flag in the session + return redirect(form.cleaned_data['return_url']) + else: + form = PasswordPageViewRestrictionForm(instance=restriction) + + action_url = reverse('wagtailcore_authenticate_with_password', args=[restriction.id, page.id]) + return page.serve_password_required_response(request, form, action_url) diff --git a/wagtail/wagtailcore/wagtail_hooks.py b/wagtail/wagtailcore/wagtail_hooks.py index 347194c32..ccac0bd43 100644 --- a/wagtail/wagtailcore/wagtail_hooks.py +++ b/wagtail/wagtailcore/wagtail_hooks.py @@ -1,8 +1,16 @@ +from django.core.urlresolvers import reverse + from wagtail.wagtailcore import hooks from wagtail.wagtailcore.models import PageViewRestriction +from wagtail.wagtailcore.forms import PasswordPageViewRestrictionForm def check_view_restrictions(page, request): restrictions = PageViewRestriction.objects.filter(page__in=page.get_ancestors(inclusive=True)) + for restriction in restrictions: - return page.serve_password_required_response(request, None) + form = PasswordPageViewRestrictionForm(instance=restriction, + initial={'return_url': request.get_full_path()}) + action_url = reverse('wagtailcore_authenticate_with_password', args=[restriction.id, page.id]) + return page.serve_password_required_response(request, form, action_url) + hooks.register('before_serve_page', check_view_restrictions) From 1e71e10cd22d18e170d45d05349b8427358c7153 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Wed, 4 Jun 2014 16:02:43 +0100 Subject: [PATCH 08/33] Keep track of passed page view restrictions in the session --- wagtail/wagtailcore/views.py | 11 ++++++++++- wagtail/wagtailcore/wagtail_hooks.py | 13 ++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/wagtail/wagtailcore/views.py b/wagtail/wagtailcore/views.py index bd12d7f97..78e0476b6 100644 --- a/wagtail/wagtailcore/views.py +++ b/wagtail/wagtailcore/views.py @@ -3,6 +3,7 @@ import warnings from django.http import HttpResponse, Http404 from django.shortcuts import get_object_or_404, redirect from django.core.urlresolvers import reverse +from django.conf import settings from wagtail.wagtailcore import hooks from wagtail.wagtailcore.models import Page, PageViewRestriction @@ -43,7 +44,15 @@ def authenticate_with_password(request, page_view_restriction_id, page_id): if request.POST: form = PasswordPageViewRestrictionForm(request.POST, instance=restriction) if form.is_valid(): - # TODO: record 'has authenticated against this page view restriction' flag in the session + has_existing_session = (settings.SESSION_COOKIE_NAME in request.COOKIES) + passed_restrictions = request.session.setdefault('passed_page_view_restrictions', []) + if restriction.id not in passed_restrictions: + passed_restrictions.append(restriction.id) + if not has_existing_session: + # if this is a session we've created, set it to expire at the end + # of the browser session + request.session.set_expiry(0) + return redirect(form.cleaned_data['return_url']) else: form = PasswordPageViewRestrictionForm(instance=restriction) diff --git a/wagtail/wagtailcore/wagtail_hooks.py b/wagtail/wagtailcore/wagtail_hooks.py index ccac0bd43..b2ab31248 100644 --- a/wagtail/wagtailcore/wagtail_hooks.py +++ b/wagtail/wagtailcore/wagtail_hooks.py @@ -7,10 +7,13 @@ from wagtail.wagtailcore.forms import PasswordPageViewRestrictionForm def check_view_restrictions(page, request): restrictions = PageViewRestriction.objects.filter(page__in=page.get_ancestors(inclusive=True)) - for restriction in restrictions: - form = PasswordPageViewRestrictionForm(instance=restriction, - initial={'return_url': request.get_full_path()}) - action_url = reverse('wagtailcore_authenticate_with_password', args=[restriction.id, page.id]) - return page.serve_password_required_response(request, form, action_url) + if restrictions: + passed_restrictions = request.session.get('passed_page_view_restrictions', []) + for restriction in restrictions: + if restriction.id not in passed_restrictions: + form = PasswordPageViewRestrictionForm(instance=restriction, + initial={'return_url': request.get_full_path()}) + action_url = reverse('wagtailcore_authenticate_with_password', args=[restriction.id, page.id]) + return page.serve_password_required_response(request, form, action_url) hooks.register('before_serve_page', check_view_restrictions) From cae0d8fecbcfb428ecc0130045ab6bdfee463269 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 5 Jun 2014 11:04:37 +0100 Subject: [PATCH 09/33] add public/private indicator on explorer and page editor --- .../wagtailadmin/pages/_view_permission_indicator.html | 6 ++++++ wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html | 3 +++ wagtail/wagtailadmin/templates/wagtailadmin/pages/list.html | 5 ++++- wagtail/wagtailcore/models.py | 3 +++ wagtail/wagtailcore/wagtail_hooks.py | 3 +-- 5 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html new file mode 100644 index 000000000..7f9c7d82b --- /dev/null +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html @@ -0,0 +1,6 @@ +{% load i18n %} +{% if page.get_view_restrictions %} +
{% trans 'Private' %}
+{% else %} +
{% trans 'Public' %}
+{% endif %} diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html index 8cc7fdca6..e56ad170c 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html @@ -16,6 +16,9 @@
{% trans "Status:" %} {% if page.live %}{{ page.status_string }}{% else %}{{ page.status_string }}{% endif %}
+ + {% include "wagtailadmin/pages/_view_permission_indicator.html" with page=page only %} + diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/list.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/list.html index 7258c5429..5fe32838e 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/list.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/list.html @@ -48,6 +48,9 @@ {% else %}

{{ parent_page.title }}

{% endif %} + + {% include "wagtailadmin/pages/_view_permission_indicator.html" with page=parent_page only %} +
    {% if parent_page_perms.can_edit and 'edit' not in hide_actions|default:'' %}
  • {% trans 'Edit' %}
    • @@ -249,4 +252,4 @@
-{% endif %} \ No newline at end of file +{% endif %} diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 08e1b612b..8670f0315 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -811,6 +811,9 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, Indexed)): def get_prev_siblings(self, inclusive=False): return self.get_siblings(inclusive).filter(path__lte=self.path).order_by('-path') + def get_view_restrictions(self): + return PageViewRestriction.objects.filter(page__in=self.get_ancestors(inclusive=True)) + password_required_template = getattr(settings, 'PASSWORD_REQUIRED_TEMPLATE', 'wagtailcore/password_required.html') def serve_password_required_response(self, request, form, action_url): """ diff --git a/wagtail/wagtailcore/wagtail_hooks.py b/wagtail/wagtailcore/wagtail_hooks.py index b2ab31248..e92971c99 100644 --- a/wagtail/wagtailcore/wagtail_hooks.py +++ b/wagtail/wagtailcore/wagtail_hooks.py @@ -1,11 +1,10 @@ from django.core.urlresolvers import reverse from wagtail.wagtailcore import hooks -from wagtail.wagtailcore.models import PageViewRestriction from wagtail.wagtailcore.forms import PasswordPageViewRestrictionForm def check_view_restrictions(page, request): - restrictions = PageViewRestriction.objects.filter(page__in=page.get_ancestors(inclusive=True)) + restrictions = page.get_view_restrictions() if restrictions: passed_restrictions = request.session.get('passed_page_view_restrictions', []) From eaee2b9695842f4c38a79874a55eec1e0aeb4065 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Fri, 6 Jun 2014 12:52:34 +0100 Subject: [PATCH 10/33] Implement (mostly-empty) views for setting view restrictions, accessible from the permission indicator --- .../js/view-permission-indicator.js | 9 +++++ .../ancestor_restriction.html | 8 +++++ .../set_view_restrictions.html | 7 ++++ .../wagtailadmin/pages/_editor_js.html | 1 + .../pages/_view_permission_indicator.html | 25 ++++++++++--- .../templates/wagtailadmin/pages/edit.html | 6 ++-- .../templates/wagtailadmin/pages/index.html | 4 +++ .../templates/wagtailadmin/pages/list.html | 2 +- wagtail/wagtailadmin/urls.py | 4 ++- .../views/page_view_restrictions.py | 35 +++++++++++++++++++ wagtail/wagtailcore/models.py | 3 ++ 11 files changed, 94 insertions(+), 10 deletions(-) create mode 100644 wagtail/wagtailadmin/static/wagtailadmin/js/view-permission-indicator.js create mode 100644 wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/ancestor_restriction.html create mode 100644 wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/set_view_restrictions.html create mode 100644 wagtail/wagtailadmin/views/page_view_restrictions.py diff --git a/wagtail/wagtailadmin/static/wagtailadmin/js/view-permission-indicator.js b/wagtail/wagtailadmin/static/wagtailadmin/js/view-permission-indicator.js new file mode 100644 index 000000000..52b7442a7 --- /dev/null +++ b/wagtail/wagtailadmin/static/wagtailadmin/js/view-permission-indicator.js @@ -0,0 +1,9 @@ +$(function() { + /* Interface to set view permissions from the explorer / editor */ + $('a.action-set-view-permissions').click(function() { + ModalWorkflow({ + 'url': this.href + }); + return false; + }); +}); diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/ancestor_restriction.html b/wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/ancestor_restriction.html new file mode 100644 index 000000000..d8b652f32 --- /dev/null +++ b/wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/ancestor_restriction.html @@ -0,0 +1,8 @@ +{% load i18n %} +{% trans "Access restricted" as title_str %} +{% include "wagtailadmin/shared/header.html" with title=title_str %} + +
+ {% trans "Access to this page is restricted because it is within the section:" %} + {{ page_with_restriction.title }} +
diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/set_view_restrictions.html b/wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/set_view_restrictions.html new file mode 100644 index 000000000..82e928c68 --- /dev/null +++ b/wagtail/wagtailadmin/templates/wagtailadmin/page_view_restrictions/set_view_restrictions.html @@ -0,0 +1,7 @@ +{% load i18n %} +{% trans "Set access restrictions" as title_str %} +{% include "wagtailadmin/shared/header.html" with title=title_str %} + +
+

form for setting access restrictions goes here

+
diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/_editor_js.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_editor_js.html index 17bcbd8bf..f494f75ec 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/_editor_js.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_editor_js.html @@ -21,6 +21,7 @@ + {% hook_output 'insert_editor_js' %} {% endcompress %} diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html index 7f9c7d82b..030703aab 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_view_permission_indicator.html @@ -1,6 +1,21 @@ -{% load i18n %} -{% if page.get_view_restrictions %} -
{% trans 'Private' %}
-{% else %} -
{% trans 'Public' %}
+{% load i18n wagtailadmin_tags %} + +{% if not page_perms %} + {% page_permissions page as page_perms %} {% endif %} + +{% with page.get_view_restrictions as has_view_restrictions %} + {% if has_view_restrictions %} + {% trans 'Private' as label %} + {% else %} + {% trans 'Public' as label %} + {% endif %} + + {% if page_perms.can_set_view_restrictions %} + + {% else %} +
{{ label }}
+ {% endif %} +{% endwith %} diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html index e56ad170c..a1b108933 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html @@ -6,6 +6,7 @@ {% block bodyclass %}menu-explorer page-editor{% endblock %} {% block content %} + {% page_permissions page as page_perms %}
{% include "wagtailadmin/shared/breadcrumb.html" with page=page %} @@ -17,7 +18,7 @@ {% trans "Status:" %} {% if page.live %}{{ page.status_string }}{% else %}{{ page.status_string }}{% endif %} - {% include "wagtailadmin/pages/_view_permission_indicator.html" with page=page only %} + {% include "wagtailadmin/pages/_view_permission_indicator.html" with page=page page_perms=page_perms only %}
@@ -25,8 +26,7 @@
{% csrf_token %} {{ edit_handler.render_form_content }} - - {% page_permissions page as page_perms %} +