#24796 closed Cleanup/optimization (fixed)
Middleware ordering hints don't mention SecurityMiddleware
Reported by: | Taymon A. Beal | Owned by: | MZ |
---|---|---|---|
Component: | Documentation | Version: | 1.8 |
Severity: | Normal | Keywords: | |
Cc: | MZ | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The documentation offers 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.
Change History (12)
comment:1 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 9 years ago
comment:4 by , 9 years ago
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.
comment:5 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:6 by , 9 years ago
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.
comment:8 by , 9 years ago
Cc: | added |
---|---|
Has patch: | set |
comment:9 by , 9 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
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!
comment:11 by , 8 years ago
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 :-)
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.