#27686 closed Bug (invalid)
calls to request.user.is_authenticated returns vary by cookie header for all users
Reported by: | Jeff Willette | Owned by: | Jeff Willette |
---|---|---|---|
Component: | contrib.sessions | Version: | 1.10 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
If request.user.is_authenticated() is called in a view, a Vary: "Cookie"
Http header is returned, even if the user is an anonymous user. The anonymous user has no sessionid
and no other cookies set. This means that any other inconsequential cookies that are in the request (such as google analytics) will cause downstream caches to cache separate pages for each user.
I think that is the user is not_authenticated, then there should be no Vary
header sent back from django.
You can recreate this problem by creating a new django project and creating a view that returns an HttpResponse after calling to is_authenticated with an anonymous user, and also calling another view that does not call to is_authenticated and comparing the HttpHeaders. I have done so here: https://github.com/deltaskelta/django-is-authenticated-vary-headers.git
Change History (9)
comment:1 by , 8 years ago
Component: | Uncategorized → contrib.auth |
---|---|
Type: | Uncategorized → Bug |
comment:2 by , 8 years ago
comment:3 by , 8 years ago
Component: | contrib.auth → contrib.sessions |
---|
I think I have a working fix here: https://github.com/django/django/pull/7802/commits/1777eac1da084577fb10020c685d21f0dfcf71a8
I changed one other test that tested for the vary cookie to be sent when I don't think it should be (on an empty session)
comment:4 by , 8 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:5 by , 8 years ago
Resolution: | → invalid |
---|---|
Status: | assigned → closed |
I don't think the analysis in this ticket is correct. If we leave out the Vary: Cookie
for responses to unauthenticated users, when the view checked the session (e.g. even just by calling request.is_authenticated()
), then a cache could incorrectly serve the unauthed version of the response to an authenticated user.
Fundamentally, if your view ever checks the session and modifies the response according to its contents (or lack thereof), then that response is cookie-dependent and should have Vary: Cookie
set. Whether the session was empty or not is irrelevant; if a different session cookie could have resulted in a different response, we need Vary: Cookie
.
comment:6 by , 8 years ago
If you need a shared-cacheable response, then you simply need to avoid ever checking the session at all: the response must be the same for ALL users, regardless of their session or lack thereof. If you need a small user-specific portion (e.g. a logged-in notice in the top right or whatnot) in an otherwise shared-cacheable e.g. landing page, one technique is to populate that user-specific portion with a separate Ajax request.
comment:7 by , 4 years ago
Resolution: | invalid |
---|---|
Status: | closed → new |
I'm running into a similar issue but have a different idea for a fix. I may be missing some reason why this also doesn't work but thought worth checking:
what if get_user
( django/contrib/auth/__init__.py
) checks if request.session.session_key is None
(and if so just returns AnonymousUser()
) before accessing the session?
comment:8 by , 4 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
Hi David.
As I read the discussion, the issue is that the is_authenticated()
check could return True
, i.e. if there's any point in checking it at all, then we really should set the Vary
header.
(Hence the ticket being closed as invalid.)
This is independent of whether there's a way we could detect this without triggering it being set, which would be to introduce a regression on this understanding.
Please see the Triage Flow guidelines and TicketClosingReasons/DontReopenTickets — if there's doubt about a resolution, much better that it's addressed on the DevelopersMailingList where it can be properly discussed. Thanks.
comment:9 by , 4 years ago
yes, i was just reaching that conclusion myself. thanks, and apologies for the noise
Isn't the
Vary: Cookie
header added theSessionMiddleware
if the session is accessed? Anonymous users can have sessions. Feel free to propose a patch, otherwise I'm not sure what to do.