[Django] #24796: Middleware ordering hints don't mention SecurityMiddleware
Django
-------------------------------+--------------------
Reporter: TaymonB | Owner: nobody
Type: Uncategorized | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
The documentation offers
[https://docs.djangoproject.com/en/1.8/ref/middleware/#middleware-ordering
hints] on how to order different middleware classes, but doesn't say
anything about where one should put SecurityMiddleware relative to the
other classes. The default project template puts it at the bottom, but the
old django-secure documentation suggests putting it near the top.
--
Ticket URL: <https://code.djangoproject.com/ticket/24796>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: TaymonB | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* type: Uncategorized => Cleanup/optimization
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24796#comment:1>
Django
Reporter: TaymonB | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
Tim, do you recall why you put it at the bottom of the list when you did
the integration?
I think I recommended top of the list in the django-secure docs just
because I figured if you're going to turn on the redirect-to-HTTPS, it may
as well happen sooner rather than after running through a bunch of other
unnecessary middleware.
Trying to think what the reasoning would be for having it at the bottom of
the list: I guess if you had other middleware that wanted to access the
value of some headers set by SecurityMiddleware?
Mostly I think it just really doesn't matter very much where you put it,
unless you're in an unusual situation.
--
Ticket URL: <https://code.djangoproject.com/ticket/24796#comment:2>
Django
Reporter: TaymonB | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I don't recall putting any thought into its position in the list.
--
Ticket URL: <https://code.djangoproject.com/ticket/24796#comment:3>
Django
Reporter: TaymonB | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by yamila-moreno):
Hi! I would like to help here, but I'm not sure what's the decission. As
far as I see:
- This middleware is not documented in the "ordering middleware" part.
Should be?
- This middleware appears in a different possition than older versions. It
would'n affect the documentation, and ¿maybe? is another ticket.
I guess the documentation should follow the code, and I'm not sure if it's
going to be changed.
--
Ticket URL: <https://code.djangoproject.com/ticket/24796#comment:4>
Django
Reporter: TaymonB | Owner:
Type: | marissazhou
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by marissazhou):
* owner: nobody => marissazhou
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/24796#comment:5>
Django
Reporter: TaymonB | Owner:
Type: | marissazhou
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
I don't have strong feelings about this, but if we're going to clarify the
situation, the only reasoning I have for putting it anywhere is what I
mentioned; that it should go near the top of the list if you're using the
SSL redirect, for efficiency. So my inclination would be to a) mention
that in the docs, and b) change the startproject template to move it up
top, just so we're following our own advice.
--
Ticket URL: <https://code.djangoproject.com/ticket/24796#comment:6>
Django
Reporter: TaymonB | Owner:
Type: | marissazhou
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by marissazhou):
Fixed the ticket
https://github.com/django/django/pull/4814
--
Ticket URL: <https://code.djangoproject.com/ticket/24796#comment:7>
Django
Reporter: TaymonB | Owner:
Type: | marissazhou
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by marissazhou):
* cc: marissazhou (added)
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24796#comment:8>
Django
Reporter: TaymonB | Owner:
Type: | marissazhou
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by carljm):
* stage: Accepted => Ready for checkin
Comment:
I think the docs addition sentence could probably be reworded slightly for
better flow, but that can be handled by whoever merges - I think this
patch is basically ready to go. Thanks for the pull request!
--
Ticket URL: <https://code.djangoproject.com/ticket/24796#comment:9>
Django
Reporter: TaymonB | Owner:
Type: | marissazhou
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"8b1f39a727be91aab40bdb37235718ed63ae1d50" 8b1f39a7]:
{{{
#!CommitTicketReference repository=""
revision="8b1f39a727be91aab40bdb37235718ed63ae1d50"
Fixed #24796 -- Added a hint on placement of SecurityMiddleware in
MIDDLEWARE_CLASSES.
Also moved it in the project template.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24796#comment:10>
Django
Reporter: TaymonB | Owner:
Type: | marissazhou
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by edmorley):
I don't suppose this could be backported to 1.8? (or at least just the
docs part)
The current 1.8 docs caused some confusion in:
https://github.com/evansd/whitenoise/issues/100
Thanks :-)
--
Ticket URL: <https://code.djangoproject.com/ticket/24796#comment:11>
Django
Reporter: TaymonB | Owner:
Type: | marissazhou
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"358ae4a687729a0f8dc23e71616f90649e111231" 358ae4a]:
{{{
#!CommitTicketReference repository=""
revision="358ae4a687729a0f8dc23e71616f90649e111231"
[1.8.x] Fixed #24796 -- Moved SecurityMiddleware in MIDDLEWARE_CLASSES
docs.
Partial backport of 8b1f39a727be91aab40bdb37235718ed63ae1d50 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24796#comment:12>