Merge lp:~julian-edwards/maas/clickjacking-bug-1298784 into lp:maas/trunk

Proposed by Julian Edwards on 2014-05-01
Status: Merged
Approved by: Julian Edwards on 2014-05-05
Approved revision: 2297
Merged at revision: 2299
Proposed branch: lp:~julian-edwards/maas/clickjacking-bug-1298784
Merge into: lp:maas/trunk
Diff against target: 11 lines (+1/-0)
1 file modified
src/maas/ (+1/-0)
To merge this branch: bzr merge lp:~julian-edwards/maas/clickjacking-bug-1298784
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) 2014-05-01 Approve on 2014-05-05
Review via email:

Commit message

Enable Django's clickjacking middleware.

Description of the change

Not sure if it warrants a unit test but I verified that the header is present after enabling this.

To post a comment you must log in.
Jeroen T. Vermeulen (jtv) wrote :

Testing is a grey area with these things, but I think you're OK. My feeling is that code which merely states configuration only needs to be tested for:

1. Running at all. I'm sure that's covered already.
2. Stating things correctly, if stating them incorrectly is plausible. Not here though.
3. Producing the right interaction within the scope of the product.

For #3 I would say the interaction is completely between Django and the browser. The only thing you do here is tell Django that it should happen.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maas/'
2--- src/maas/ 2014-02-14 12:12:28 +0000
3+++ src/maas/ 2014-05-01 06:10:35 +0000
4@@ -249,6 +249,7 @@
5 'maasserver.middleware.ExternalComponentsMiddleware',
6 'metadataserver.middleware.MetadataErrorsMiddleware',
7 'django.middleware.transaction.TransactionMiddleware',
8+ 'django.middleware.clickjacking.XFrameOptionsMiddleware',
9 'django.middleware.csrf.CsrfViewMiddleware',
10 'maasserver.middleware.ExceptionLoggerMiddleware',
11 'django.contrib.auth.middleware.AuthenticationMiddleware',