Add clickjacking protection (X-Frame-Options header)

[rniemeyer]

Overview

For security reasons, many sites implement some form of ​clickjacking protection. Now that most of the modern browsers (IE8+, Firefox 3.6.9+, Chrome 4.1+, Safari 4+, Opera 10.5+) support the X-Frame-Options header, it seems to make sense for Django to support it as well.

Details

Included is a patch for a middleware (based off ​Paul Osman's work) that will set the X-Frame-Options header for all responses. By default, sets it to 'DENY', but allows for a settings.py value if 'SAMEORIGIN' is desired instead.

I stuck this in a new clickjacking middleware module, but it could obviously go somewhere else if that's not the best location.

Why?

While this is a rather trivial piece of code, it still feels like a worthwhile addition to Django for PR and "batteries included" reasons. If that's not generally agreed upon, I can open up a discussion on django dev. If this is deemed a good idea, then I can add docs to go along with the code/tests.

[rniemeyer]

XFrameOptionsMiddleware + tests

[Łukasz Rekucki]

It would be good to have a decorator to disable this - something that works like csrf_excempt, but it should allow you to choose between 'deny', 'sameorigin' or disabling it all together. I have at least one form that's embedded in client's page via an IFrame, so if I understand correctly adding this would prevent it from working correctly.

[rniemeyer]

Improved patch: added decorators, tests and some flexibility

[rniemeyer]

The documentation is nearly complete. Will include in updated patch sometime during Pycon.

[Luke Plant]

+1, I was going to suggest it myself. The patch looks pretty good. After Django 1.3 is out, we should have some discussion on django-devs about:

• what the default value should be (I think SAMEORIGIN would make it better for general use, with very little decrease in security).
• whether we can avoid a new setting
• whether the middleware should be on by default or in the project template.

[rniemeyer]

Patch, tests and docs

[rniemeyer]

Renamed patch to .diff i h8 eclipse

[rniemeyer]

Updated patch with docs (thanks to Sir @elliotthoffman) and changed default from DENY to SAMEORIGIN.

Opened a discussion: ​http://groups.google.com/group/django-developers/browse_thread/thread/12a590c6fadc0a61

Does the version of this ticket need to be changed? I don't recall setting it, so not sure how that works...

[Luke Plant]

Thanks for the great patch, I will commit it shortly.

I did have to change a fair number of things - it was easier for me to do so because I'm familiar with all our standards etc. Since I'd like to encourage you to continue contributing to Django, but want to make it easier to add your contributions, I've included below the list of things I changed, mainly to highlight things you probably wouldn't have been aware of:

• Docs:
  • wrapped to 80 characters, as per our ​guidelines for writing docs
  • added some blanklines after headers. This makes it easier to re-wrap in editors, and is consistent with the rest of our docs.
  • corrected some indentation, so that lists appeared correctly formatted.
  • moved the main docs file out of the contrib directory - that's for contrib apps. (The CSRF docs are currently in the wrong place, because that used to be a contrib app, but now is core, and I don't think we've got a good way of doing re-direction yet).
  • added a 'versionadded' Sphinx directive
  • fixed some Python syntax errors (noticed from the lack of code highlighting in the built docs.)
  • used standard capitalization of X-Frame-Options throughout.
  • other misc spelling corrections to docs.
  • added section to ref/middleware.txt
  • added section to ref/settings.txt
• Code:
  • added setting to django/conf/global_settings.py
  • removed trailing whitespace in various files. (You can often configure your editor to show this, and most colorized diff tools, like ​colordiff, will highlight this - they can often be used in conjuction with 'svn diff' etc.).
  • used standard capitalization of X-Frame-Options in response header.
  • changed tearDown in tests so that it restores the original value of the setting (which will now always be there, since we added it to gloab_settings.py), rather than remove any setting.
  • removed a Python 2.4 fallback that's no longer necessary.

And also:

• as discussed, I added the middleware to the project template, commented out.
• added this to the release notes, as a worthy feature addition.
• added you to AUTHORS. I put your Google profile as the link - please say if you'd prefer your e-mail address or a different website.

Finally, as a matter of process, you should update the 'Patch needs improvement/Needs docs/Needs tests' flags when you think you've addressed any issues - otherwise the committers will generally wait for the original author to fix the patch up unless they are particularly motivated about the feature/bug.

[Luke Plant]

In [16298]:

Fixed #14261 - Added clickjacking protection (X-Frame-Options header)

Many thanks to rniemeyer for the patch!

[jfialkoff]

Hopefully I'm not missing it, but it seems like this page (​https://docs.djangoproject.com/en/dev/ref/clickjacking/) should mention the exempt decorator but does not.

[Tim Graham]

I see @xframe_options_exempt on that page.