[Django] #29471: Set-Cookie response is cached for deleting invalid session cookies

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #29471: Set-Cookie response is cached for deleting invalid session cookies

Django
#29471: Set-Cookie response is cached for deleting invalid session cookies
-------------------------------------+-------------------------------------
               Reporter:  Duane      |          Owner:  nobody
  Hutchins                           |
                   Type:  Bug        |         Status:  new
              Component:             |        Version:  2.0
  contrib.sessions                   |
               Severity:  Normal     |       Keywords:  empty session cache
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 Regarding:
 [https://docs.djangoproject.com/en/2.0/_modules/django/contrib/sessions/middleware/#SessionMiddleware
 django.contrib.sessions.middleware.SessionMiddleware]
 and
 [https://docs.djangoproject.com/en/2.0/_modules/django/utils/cache/#learn_cache_key
 django.utils.cache]

 Using the SessionMiddleware and caching, if an invalid sessionid (i.e. an
 empty session) is passed via cookie, the SessionMiddleware responds by
 deleting the cookie, and this response is cached without Vary: Cookie, so
 all subsequent authenticated requests also receive that delete-session-
 cookie header.

 The end result is that if a cache-enabled page receives a request using an
 invalid session id, that page will log out all users who visit that page
 until that cache expires/purges.

 To duplicate:

 * Enable caching and sessions as normal.
 * Visit a cache-enabled page which shows different content for users who
 are logged in vs not logged in.
 * Log in via browser
 * Change your browser's sessionid cookie value via Chrome Developer Tools
 or whatever.
 * Refresh your browser
 * The browser is told to delete the invalid cookie (correct).
 * Log in again. Refresh again (if necessary).
 * The login is successful, however, the browser is instructed to delete
 the session cookie (incorrect).
 * The user is logged out
 * The same logout happens for all users visit that page -- until cache
 expires

 If I decorate the aforementioned page view with @vary_on_cookie (which
 inserts the Vary:Cookie header), then the issue is resolved.

 Possible solutions I can think of:

 * SessionMiddleware adds Vary:Cookie when deleting invalid session cookies
 * Cache Util assumes Vary:Cookie when response includes Set-Cookie header.

--
Ticket URL: <https://code.djangoproject.com/ticket/29471>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/056.32b5571849c92df51a63f01b9ae93ab1%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29471: Set-Cookie response is cached for deleting invalid session cookies

Django
#29471: Set-Cookie response is cached for deleting invalid session cookies
-------------------------------------+-------------------------------------
     Reporter:  Duane Hutchins       |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  2.0
     Severity:  Normal               |               Resolution:
     Keywords:  empty session cache  |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tim Graham):

 Isn't the solution to use `@vary_on_cookie` as you said? If you have "a
 cache-enabled page which shows different content for users who are logged
 in vs not logged in" then you need to use the `Vary: Cookie` header, don't
 you?

--
Ticket URL: <https://code.djangoproject.com/ticket/29471#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/071.35a2e368a9fb0e18c0bcd3821a3e7f45%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29471: Set-Cookie response is cached for deleting invalid session cookies

Django
In reply to this post by Django
#29471: Set-Cookie response is cached for deleting invalid session cookies
-------------------------------------+-------------------------------------
     Reporter:  Duane Hutchins       |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  2.0
     Severity:  Normal               |               Resolution:
     Keywords:  empty session cache  |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Duane Hutchins):

 Replying to [comment:1 Tim Graham]:
 > Isn't the solution to use `@vary_on_cookie` as you said? If you have "a
 cache-enabled page which shows different content for users who are logged
 in vs not logged in" then you need to use the `Vary: Cookie` header, don't
 you?

 The SessionMiddleware sets `Vary: Cookie` automatically if the session is
 accessed. The bug is that SessionMiddleware is not doing that if the
 session cookie is invalid.

 When accessing the session object:

 * No session cookie => `Vary: Cookie` is set
 * Valid session cookie => `Vary: Cookie` is set
 * Invalid session cookie => `Vary: Cookie` is not set

 Examining
 [https://docs.djangoproject.com/en/2.0/_modules/django/contrib/sessions/middleware/#SessionMiddleware
 the source code], you can confirm this as true because the
 `patch_vary_headers` only happens if the session cookie is valid or
 missing.

 {{{
             # First check if we need to delete this cookie.
             # The session should be deleted only if the session is
 entirely empty
             if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
                 response.delete_cookie(
                     settings.SESSION_COOKIE_NAME,
                     path=settings.SESSION_COOKIE_PATH,
                     domain=settings.SESSION_COOKIE_DOMAIN,
                 )
             else:
                 if accessed:
                     patch_vary_headers(response, ('Cookie',))
 }}}


 The end result is that this causes the cache handler to cache invalid-
 session responses without `Vary: Cookie`. So, if the cached invalid-
 session response was to delete the session cookie, then the cookie is
 always deleted on that page -- until the cache is updated.

--
Ticket URL: <https://code.djangoproject.com/ticket/29471#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/071.cad84ef5b86184074624d72cd5c0d75a%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29471: Set-Cookie response is cached for deleting invalid session cookies

Django
In reply to this post by Django
#29471: Set-Cookie response is cached for deleting invalid session cookies
-------------------------------------+------------------------------------
     Reporter:  Duane Hutchins       |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  2.0
     Severity:  Normal               |               Resolution:
     Keywords:  empty session cache  |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+------------------------------------
Changes (by Tim Graham):

 * stage:  Unreviewed => Accepted


--
Ticket URL: <https://code.djangoproject.com/ticket/29471#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/071.65ec6451ce85f5491b410b438fcf7b23%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29471: Set-Cookie response is cached for deleting invalid session cookies

Django
In reply to this post by Django
#29471: Set-Cookie response is cached for deleting invalid session cookies
-------------------------------------+------------------------------------
     Reporter:  Duane Hutchins       |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  2.0
     Severity:  Normal               |               Resolution:
     Keywords:  empty session cache  |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+------------------------------------
Changes (by Herbert Fortes):

 * cc: Herbert Fortes (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/29471#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/071.0c708a8427765f962a99593732380ed0%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29471: Set-Cookie response is cached for deleting invalid session cookies

Django
In reply to this post by Django
#29471: Set-Cookie response is cached for deleting invalid session cookies
-------------------------------------+-------------------------------------
     Reporter:  Duane Hutchins       |                    Owner:
                                     |  birthdaysgift
         Type:  Bug                  |                   Status:  assigned
    Component:  contrib.sessions     |                  Version:  2.0
     Severity:  Normal               |               Resolution:
     Keywords:  empty session cache  |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by birthdaysgift):

 * owner:  nobody => birthdaysgift
 * status:  new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/29471#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/071.e7ff77e45e6f315264dd4a87eee8e725%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29471: Set-Cookie response is cached for deleting invalid session cookies

Django
In reply to this post by Django
#29471: Set-Cookie response is cached for deleting invalid session cookies
-------------------------------------+-------------------------------------
     Reporter:  Duane Hutchins       |                    Owner:
                                     |  birthdaysgift
         Type:  Bug                  |                   Status:  assigned
    Component:  contrib.sessions     |                  Version:  2.0
     Severity:  Normal               |               Resolution:
     Keywords:  empty session cache  |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by birthdaysgift):

 * has_patch:  0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/29471#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/071.70a8662e2deeee8950fb7238ffadb5aa%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29471: Set-Cookie response is cached for deleting invalid session cookies

Django
In reply to this post by Django
#29471: Set-Cookie response is cached for deleting invalid session cookies
-------------------------------------+-------------------------------------
     Reporter:  Duane Hutchins       |                    Owner:
                                     |  birthdaysgift
         Type:  Bug                  |                   Status:  assigned
    Component:  contrib.sessions     |                  Version:  2.0
     Severity:  Normal               |               Resolution:
     Keywords:  empty session cache  |             Triage Stage:  Ready for
                                     |  checkin
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

 * stage:  Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/29471#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/071.49e32c32c7a8821481fe4dec6c21a2b4%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29471: Set-Cookie response is cached for deleting invalid session cookies

Django
In reply to this post by Django
#29471: Set-Cookie response is cached for deleting invalid session cookies
-------------------------------------+-------------------------------------
     Reporter:  Duane Hutchins       |                    Owner:
                                     |  birthdaysgift
         Type:  Bug                  |                   Status:  closed
    Component:  contrib.sessions     |                  Version:  2.0
     Severity:  Normal               |               Resolution:  fixed
     Keywords:  empty session cache  |             Triage Stage:  Ready for
                                     |  checkin
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"dc740dde50873e82f761386fd73ca17d9eaa008b" dc740dd]:
 {{{
 #!CommitTicketReference repository=""
 revision="dc740dde50873e82f761386fd73ca17d9eaa008b"
 Fixed #29471 -- Added 'Vary: Cookie' to invalid/empty session cookie
 responses.
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29471#comment:8>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/071.84d03c1dee166295926020afe5fdc543%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29471: Set-Cookie response is cached for deleting invalid session cookies

Django
In reply to this post by Django
#29471: Set-Cookie response is cached for deleting invalid session cookies
-------------------------------------+-------------------------------------
     Reporter:  Duane Hutchins       |                    Owner:
                                     |  birthdaysgift
         Type:  Bug                  |                   Status:  closed
    Component:  contrib.sessions     |                  Version:  2.0
     Severity:  Normal               |               Resolution:  fixed
     Keywords:  empty session cache  |             Triage Stage:  Ready for
                                     |  checkin
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Martin H.):

 * cc: Martin H. (added)


Comment:

 AFAICS, this bug is already present in the 2.x versions. Shouldn't it be
 patched there as well?

--
Ticket URL: <https://code.djangoproject.com/ticket/29471#comment:9>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/071.23f409ad6a43a85e23ba651b545536b8%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29471: Set-Cookie response is cached for deleting invalid session cookies

Django
In reply to this post by Django
#29471: Set-Cookie response is cached for deleting invalid session cookies
-------------------------------------+-------------------------------------
     Reporter:  Duane Hutchins       |                    Owner:
                                     |  birthdaysgift
         Type:  Bug                  |                   Status:  closed
    Component:  contrib.sessions     |                  Version:  2.0
     Severity:  Normal               |               Resolution:  fixed
     Keywords:  empty session cache  |             Triage Stage:  Ready for
                                     |  checkin
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by felixxm):

 This patch doesn't qualify for a backport, because it's not a regression
 or a new feature in Django 2.2.

--
Ticket URL: <https://code.djangoproject.com/ticket/29471#comment:10>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/071.17ee892152596e6579694f463e0294fa%40djangoproject.com.