Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Per-page X-Frame-Options/Clickjacking protection #2477

Merged
merged 4 commits into from
Feb 21, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 19 additions & 2 deletions cms/admin/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def __init__(self, *args, **kwargs):
def clean(self):
cleaned_data = self.cleaned_data
slug = cleaned_data.get('slug', '')

page = self.instance
lang = cleaned_data.get('language', None)
# No language, can not go further, but validation failed already
Expand Down Expand Up @@ -181,6 +181,13 @@ class AdvancedSettingsForm(forms.ModelForm):
overwrite_url = forms.CharField(label=_('Overwrite URL'), max_length=255, required=False,
help_text=_('Keep this field empty if standard path should be used.'))

xframe_options = forms.ChoiceField(
choices=Page._meta.get_field('xframe_options').choices,
label=_('X Frame Options'),
help_text=_('Whether this page can be embedded in other pages or websites'),
initial=Page._meta.get_field('xframe_options').default,
required=False
)
redirect = forms.CharField(label=_('Redirect'), max_length=255, required=False,
help_text=_('Redirects to this URL.'))
language = forms.ChoiceField(label=_("Language"), choices=get_language_tuple(),
Expand Down Expand Up @@ -230,6 +237,16 @@ def clean_application_namespace(self):
raise ValidationError(_('A instance name with this name already exists.'))
return namespace

def clean_xframe_options(self):
if 'xframe_options' not in self.fields:
return # nothing to do, field isn't present

xframe_options = self.cleaned_data['xframe_options']
if xframe_options == '':
return Page._meta.get_field('xframe_options').default

return xframe_options

def clean_overwrite_url(self):
if 'overwrite_url' in self.fields:
url = self.cleaned_data['overwrite_url']
Expand All @@ -240,7 +257,7 @@ class Meta:
model = Page
fields = [
'site', 'template', 'reverse_id', 'overwrite_url', 'redirect', 'soft_root', 'navigation_extenders',
'application_urls', 'application_namespace'
'application_urls', 'application_namespace', "xframe_options",
]


Expand Down
3 changes: 2 additions & 1 deletion cms/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def create_page(title, template, language, menu_title=None, slug=None,
in_navigation=False, soft_root=False, reverse_id=None,
navigation_extenders=None, published=False, site=None,
login_required=False, limit_visibility_in_menu=VISIBILITY_ALL,
position="last-child", overwrite_url=None):
position="last-child", overwrite_url=None, xframe_options=Page.X_FRAME_OPTIONS_INHERIT):
"""
Create a CMS Page and it's title for the given language

Expand Down Expand Up @@ -194,6 +194,7 @@ def create_page(title, template, language, menu_title=None, slug=None,
site=site,
login_required=login_required,
limit_visibility_in_menu=limit_visibility_in_menu,
xframe_options=xframe_options,
)
page.insert_at(parent, position)
page.save()
Expand Down
210 changes: 210 additions & 0 deletions cms/migrations/0053_auto__add_field_page_xframe_options.py

Large diffs are not rendered by default.

45 changes: 43 additions & 2 deletions cms/models/pagemodel.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
from cms.utils.helpers import reversion_register
from django.contrib.sites.models import Site
from django.core.urlresolvers import reverse
from django.core.cache import cache
from django.conf import settings
from django.db import models
from django.db.models import Q
from django.shortcuts import get_object_or_404
Expand All @@ -39,7 +41,12 @@ class Page(with_metaclass(PageMetaClass, MPTTModel)):
(2, _('for anonymous users only')),
)

template_choices = [(x, _(y)) for x, y in get_cms_setting('TEMPLATES')]
X_FRAME_OPTIONS_INHERIT = 0
X_FRAME_OPTIONS_DENY = 1
X_FRAME_OPTIONS_SAMEORIGIN = 2
X_FRAME_OPTIONS_ALLOW= 3

template_choices = [(x, _(y)) for x, y in get_cms_setting('TEMPLATES')]

created_by = models.CharField(_("created by"), max_length=70, editable=False)
changed_by = models.CharField(_("changed by"), max_length=70, editable=False)
Expand Down Expand Up @@ -87,6 +94,19 @@ class Page(with_metaclass(PageMetaClass, MPTTModel)):

# If the draft is loaded from a reversion version save the revision id here.
revision_id = models.PositiveIntegerField(default=0, editable=False)

# X Frame Options for clickjacking protection
xframe_options = models.IntegerField(
choices=(
(X_FRAME_OPTIONS_INHERIT, _('Inherit from parent page')),
(X_FRAME_OPTIONS_DENY, _('Deny')),
(X_FRAME_OPTIONS_SAMEORIGIN, _('Only this website')),
(X_FRAME_OPTIONS_ALLOW, _('Allow'))
),
default=getattr(settings, 'CMS_DEFAULT_X_FRAME_OPTIONS', X_FRAME_OPTIONS_INHERIT)
)


# Managers
objects = PageManager()
permissions = PagePermissionsPermissionManager()
Expand Down Expand Up @@ -250,6 +270,7 @@ def _copy_attributes(self, target):
target.application_namespace = self.application_namespace
target.template = self.template
target.site_id = self.site_id
target.xframe_options = self.xframe_options

def copy_page(self, target, site, position='first-child',
copy_permissions=True):
Expand Down Expand Up @@ -1141,7 +1162,27 @@ def rescan_placeholders(self):
self.placeholders.add(placeholder)
found[placeholder_name] = placeholder
return found


def get_xframe_options(self):
""" Finds X_FRAME_OPTION from tree if inherited """
xframe_options = cache.get('cms:xframe_options:%s' % self.pk)
if xframe_options is None:
ancestors = self.get_ancestors(ascending=True, include_self=True)

# Ignore those pages which just inherit their value
ancestors = ancestors.exclude(xframe_options=self.X_FRAME_OPTIONS_INHERIT)

# Now just give me the clickjacking setting (not anything else)
xframe_options = ancestors.values_list('xframe_options', flat=True)

if len(xframe_options) <= 0:
# No ancestors were found
return None

xframe_options = xframe_options[0]
cache.set('cms:xframe_options:%s' % self.pk, xframe_options)

return xframe_options

def _reversion():
exclude_fields = ['publisher_is_draft', 'publisher_public', 'publisher_state']
Expand Down
6 changes: 3 additions & 3 deletions cms/tests/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ def test_delete(self):
page.publish('en')
with self.login_user_context(admin):
data = {'post': 'yes'}
with self.assertNumQueries(FuzzyInt(300, 380)):
with self.assertNumQueries(FuzzyInt(300, 382)):
response = self.client.post(URL_CMS_PAGE_DELETE % page.pk, data)
self.assertRedirects(response, URL_CMS_PAGE)

Expand Down Expand Up @@ -1205,9 +1205,9 @@ def test_render_edit_mode(self):
with self.assertNumQueries(FuzzyInt(40, 60)):
output = force_unicode(self.client.get('/en/?edit').content)
self.assertIn('<b>Test</b>', output)
with self.assertNumQueries(FuzzyInt(18, 33)):
with self.assertNumQueries(FuzzyInt(18, 34)):
output = force_unicode(self.client.get('/en/?edit').content)
with self.assertNumQueries(FuzzyInt(18, 19)):
with self.assertNumQueries(FuzzyInt(18, 21)):
output = force_unicode(self.client.get('/en/').content)

def test_tree_view_queries(self):
Expand Down
69 changes: 69 additions & 0 deletions cms/tests/page.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import os.path

from django.conf import settings
from django.core.cache import cache
from django.contrib.sites.models import Site
from django.contrib import admin
from django.core.exceptions import ValidationError
Expand Down Expand Up @@ -43,6 +44,9 @@ def test_content_type(self):


class PagesTestCase(CMSTestCase):
def tearDown(self):
cache.clear()

def test_add_page(self):
"""
Test that the add admin page could be displayed via the admin
Expand Down Expand Up @@ -820,6 +824,71 @@ def test_plugin_loading_queries(self):
self.assertIn('text-%d-%d' % (i, j), content)
self.assertIn('link-%d-%d' % (i, j), content)

def test_xframe_options_allow(self):
"""Test that no X-Frame-Options is set when page's xframe_options is set to allow"""
page = create_page(
title='home',
template='nav_playground.html',
language='en',
published=True,
slug='home',
xframe_options=Page.X_FRAME_OPTIONS_ALLOW
)

resp = self.client.get(page.get_absolute_url('en'))
self.assertEqual(resp.get('X-Frame-Options'), None)

def test_xframe_options_sameorigin(self):
"""Test that X-Frame-Options is 'SAMEORIGIN' when xframe_options is set to origin"""
page = create_page(
title='home',
template='nav_playground.html',
language='en',
published=True,
slug='home',
xframe_options=Page.X_FRAME_OPTIONS_SAMEORIGIN
)

resp = self.client.get(page.get_absolute_url('en'))
self.assertEqual(resp.get('X-Frame-Options'), 'SAMEORIGIN')

def test_xframe_options_deny(self):
"""Test that X-Frame-Options is 'DENY' when xframe_options is set to deny"""
page = create_page(
title='home',
template='nav_playground.html',
language='en',
published=True,
slug='home',
xframe_options=Page.X_FRAME_OPTIONS_DENY
)

resp = self.client.get(page.get_absolute_url('en'))
self.assertEqual(resp.get('X-Frame-Options'), 'DENY')

def test_xframe_options_inherit_with_parent(self):
"""Test that X-Frame-Options is set to parent page's setting when inherit is set"""
parent = create_page(
title='home',
template='nav_playground.html',
language='en',
published=True,
slug='home',
xframe_options=Page.X_FRAME_OPTIONS_DENY
)

page = create_page(
title='subpage',
template='nav_playground.html',
language='en',
published=True,
slug='subpage',
parent=parent,
xframe_options=Page.X_FRAME_OPTIONS_INHERIT
)

resp = self.client.get(page.get_absolute_url('en'))
self.assertEqual(resp.get('X-Frame-Options'), 'DENY')

class PageAdminTestBase(CMSTestCase):
"""
Expand Down
23 changes: 21 additions & 2 deletions cms/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from django.template.response import TemplateResponse
from cms.apphook_pool import apphook_pool
from cms.appresolver import get_app_urls
from cms.models import Title
from cms.models import Title, Page
from cms.utils import get_template_from_request, get_language_from_request
from cms.utils.i18n import get_fallback_languages, force_language, get_public_languages, get_redirect_on_fallback, \
get_language_list, is_language_prefix_patterns_used
Expand Down Expand Up @@ -148,4 +148,23 @@ def details(request, slug):
if not context['has_view_permissions']:
return _handle_no_page(request, slug)

return TemplateResponse(request, template_name, context)
response = TemplateResponse(request, template_name, context)

# Add headers for X Frame Options - this really should be changed upon moving to class based views
xframe_options = page.get_xframe_options()
if xframe_options == Page.X_FRAME_OPTIONS_INHERIT:
# This is when we defer to django's own clickjacking handling
return response

# We want to prevent django setting this in their middlewear
response.xframe_options_exempt = True

if xframe_options == Page.X_FRAME_OPTIONS_ALLOW:
# Do nothing, allowed is no header.
return response
elif xframe_options == Page.X_FRAME_OPTIONS_SAMEORIGIN:
response['X-Frame-Options'] = 'SAMEORIGIN'
elif xframe_options == Page.X_FRAME_OPTIONS_DENY:
response['X-Frame-Options'] = 'DENY'

return response
14 changes: 14 additions & 0 deletions docs/getting_started/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -725,3 +725,17 @@ Example::

.. _django-reversion: https://github.com/etianen/django-reversion
.. _unihandecode.js: https://github.com/ojii/unihandecode.js

CMS_DEFAULT_X_FRAME_OPTIONS
===========================

Default: ``Page.X_FRAME_OPTIONS_INHERIT``

This setting is the default value for a Page's X Frame Options setting.
This should be an integer preferably taken from the Page object e.g.

- X_FRAME_OPTIONS_INHERIT
- X_FRAME_OPTIONS_ALLOW
- X_FRAME_OPTIONS_SAMEORIGIN
- X_FRAME_OPTIONS_DENY

7 changes: 7 additions & 0 deletions docs/upgrade/3.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -357,3 +357,10 @@ Using ``placeholder_tags`` will cause a ``DeprecationWarning`` to occur.

``placeholder_tags`` will be removed in version 3.1.

Per-page Clickjacking protection
================================

An advanced option has been added which controls, on a per-page basis, the
``X-Frame-Options`` header. The default setting is to inherit from the parent
page. If no ancestor specifies a value, no header will be set, allowing Django's
own middleware to handle it (if enabled).