Re: [Django] #32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and csrf_cookie_set logic isn't right

[Django]

#32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and
csrf_cookie_set logic isn't right
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 |  Jerdonek
 Type:  Bug  |   Status:  assigned
Component:  CSRF |  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:   | 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 Mariusz Felisiak ):

 In [changeset:"311401d9a278339795a7ab3f1617c4da9077fdcc" 311401d9]:
 {{{
 #!CommitTicketReference repository=""
 revision="311401d9a278339795a7ab3f1617c4da9077fdcc"
 Refs #32902 -- Added CSRF test when rotate_token() is called between
 resetting the token and processing response.
 }}}

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.c3993cf9fb9ca94b9251b3c7db3de654%40djangoproject.com.

Re: [Django] #32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and csrf_cookie_set logic isn't right

[Django]

#32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and
csrf_cookie_set logic isn't right
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 |  Jerdonek
 Type:  Bug  |   Status:  closed
Component:  CSRF |  Version:  dev
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | 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 Mariusz Felisiak ):

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


Comment:

 In [changeset:"a2e1f1e2954c8144f2acf83402ed1f55ea9692a4" a2e1f1e2]:
 {{{
 #!CommitTicketReference repository=""
 revision="a2e1f1e2954c8144f2acf83402ed1f55ea9692a4"
 Fixed #32902 -- Fixed CsrfViewMiddleware.process_response()'s cookie reset
 logic.

 Thanks Florian Apolloner and Shai Berger for reviews.
 }}}

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.aed9750edfd696e08926d4a1cf5860c8%40djangoproject.com.

Re: [Django] #32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and csrf_cookie_set logic isn't right

[Django]

#32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and
csrf_cookie_set logic isn't right
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 |  Jerdonek
 Type:  Bug  |   Status:  closed
Component:  CSRF |  Version:  dev
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | 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 Mariusz Felisiak ):

 In [changeset:"3cfcb8cbc83cfe58307b87435d55c27180cb4f94" 3cfcb8cb]:
 {{{
 #!CommitTicketReference repository=""
 revision="3cfcb8cbc83cfe58307b87435d55c27180cb4f94"
 Refs #32902 -- Moved ensure_csrf_cookie_view after protected_view.
 }}}

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.15cd26883ffc51456d903e6f6d5a4f62%40djangoproject.com.

Re: [Django] #32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and csrf_cookie_set logic isn't right

[Django]

#32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and
csrf_cookie_set logic isn't right
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 |  Jerdonek
 Type:  Bug  |   Status:  assigned
Component:  CSRF |  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:   | 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 Mariusz Felisiak):

 * stage:  Accepted => Ready for checkin


-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.af3b264b9a411dbe7d4239366f5b4b10%40djangoproject.com.

Re: [Django] #32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and csrf_cookie_set logic isn't right

[Django]

#32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and
csrf_cookie_set logic isn't right
+--
 Reporter:  Chris Jerdonek  |Owner:  Chris Jerdonek
 Type:  Bug |   Status:  assigned
Component:  CSRF|  Version:  dev
 Severity:  Normal  |   Resolution:
 Keywords:  | Triage Stage:  Accepted
Has patch:  1   |  Needs documentation:  0
  Needs tests:  0   |  Patch needs improvement:  0
Easy pickings:  0   |UI/UX:  0
+--
Changes (by Mariusz Felisiak):

 * stage:  Ready for checkin => Accepted


-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.dc72681b1ea7b106fa41398882b726a0%40djangoproject.com.

Re: [Django] #32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and csrf_cookie_set logic isn't right

[Django]

#32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and
csrf_cookie_set logic isn't right
-+-
 Reporter:  Chris Jerdonek   |Owner:  Chris
 |  Jerdonek
 Type:  Bug  |   Status:  assigned
Component:  CSRF |  Version:  dev
 Severity:  Normal   |   Resolution:
 Keywords:   | 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 Shai Berger):

 * stage:  Accepted => Ready for checkin


Comment:

 Would appreciate at least one more pair of eyes on this, but LGTM.

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.e74c804fb946866a87d3e84f847196ae%40djangoproject.com.

Re: [Django] #32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and csrf_cookie_set logic isn't right

[Django]

#32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and
csrf_cookie_set logic isn't right
+--
 Reporter:  Chris Jerdonek  |Owner:  Chris Jerdonek
 Type:  Bug |   Status:  assigned
Component:  CSRF|  Version:  dev
 Severity:  Normal  |   Resolution:
 Keywords:  | Triage Stage:  Accepted
Has patch:  1   |  Needs documentation:  0
  Needs tests:  0   |  Patch needs improvement:  0
Easy pickings:  0   |UI/UX:  0
+--
Changes (by Chris Jerdonek):

 * has_patch:  0 => 1


Comment:

 PR: https://github.com/django/django/pull/14599

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.ebbe146f280bb8c5e8070e4c0fca8f2b%40djangoproject.com.

Re: [Django] #32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and csrf_cookie_set logic isn't right

[Django]

#32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and
csrf_cookie_set logic isn't right
+--
 Reporter:  Chris Jerdonek  |Owner:  Chris Jerdonek
 Type:  Bug |   Status:  assigned
Component:  CSRF|  Version:  dev
 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 Mariusz Felisiak):

 * cc: Shai Berger, Florian Apolloner (added)
 * stage:  Unreviewed => Accepted


-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.2d52a8b7dd2710998775ce661c5775aa%40djangoproject.com.

Re: [Django] #32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and csrf_cookie_set logic isn't right

[Django]

#32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and
csrf_cookie_set logic isn't right
+--
 Reporter:  Chris Jerdonek  |Owner:  Chris Jerdonek
 Type:  Bug |   Status:  assigned
Component:  CSRF|  Version:  dev
 Severity:  Normal  |   Resolution:
 Keywords:  | Triage Stage:  Unreviewed
Has patch:  0   |  Needs documentation:  0
  Needs tests:  0   |  Patch needs improvement:  0
Easy pickings:  0   |UI/UX:  0
+--

Comment (by Chris Jerdonek):

 The code under question was originally added 5 years in this large-ish
 commit here:
 
https://github.com/django/django/commit/5112e65ef2df1dbb95ff83026b6a962fb2688661

 Indeed, part of my proposed fix involves restoring the initial
 `csrf_cookie_set` lines prior to that commit (`csrf_cookie_set` was called
 `csrf_processing_done` before this commit).

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.23d6a92fdbf0e8a3cc57eddd2dac71fa%40djangoproject.com.

Re: [Django] #32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and csrf_cookie_set logic isn't right

[Django]

#32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and
csrf_cookie_set logic isn't right
+--
 Reporter:  Chris Jerdonek  |Owner:  Chris Jerdonek
 Type:  Bug |   Status:  assigned
Component:  CSRF|  Version:  dev
 Severity:  Normal  |   Resolution:
 Keywords:  | Triage Stage:  Unreviewed
Has patch:  0   |  Needs documentation:  0
  Needs tests:  0   |  Patch needs improvement:  0
Easy pickings:  0   |UI/UX:  0
+--

Comment (by Chris Jerdonek):

 I believe the fix should look something like this:

 {{{
 diff --git a/django/middleware/csrf.py b/django/middleware/csrf.py
 index c2a9470ab1..bf97e50146 100644
 --- a/django/middleware/csrf.py
 +++ b/django/middleware/csrf.py
 @@ -437,15 +437,14 @@ class CsrfViewMiddleware(MiddlewareMixin):
  return self._accept(request)

  def process_response(self, request, response):
 -if not getattr(request, 'csrf_cookie_needs_reset', False):
 -if getattr(response, 'csrf_cookie_set', False):
 -return response
 -
 -if not request.META.get("CSRF_COOKIE_USED", False):
 +# Prevent the cookie from being set twice.
 +if getattr(response, 'csrf_cookie_set', False):
  return response
 +if (getattr(request, 'csrf_cookie_needs_reset', False) or
 +request.META.get("CSRF_COOKIE_USED")):
 +# Set the CSRF cookie even if it's already set so that the
 +# expiry timer gets renewed.
 +self._set_token(request, response)
 +response.csrf_cookie_set = True

 -# Set the CSRF cookie even if it's already set, so we renew
 -# the expiry timer.
 -self._set_token(request, response)
 -response.csrf_cookie_set = True
  return response
 }}}

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.839c1419e00876e5ab1230eead5490ac%40djangoproject.com.

[Django] #32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and csrf_cookie_set logic isn't right

[Django]

#32902: CsrfViewMiddleware.process_response()'s csrf_cookie_needs_reset and
csrf_cookie_set logic isn't right
--+
   Reporter:  Chris Jerdonek  |  Owner:  Chris Jerdonek
   Type:  Bug | Status:  assigned
  Component:  CSRF|Version:  dev
   Severity:  Normal  |   Keywords:
   Triage Stage:  Unreviewed  |  Has patch:  0
Needs documentation:  0   |Needs tests:  0
Patch needs improvement:  0   |  Easy pickings:  0
  UI/UX:  0   |
--+
 I noticed that the `csrf_cookie_needs_reset` and `csrf_cookie_set` logic
 inside `CsrfViewMiddleware.process_response()` isn't right:
 
https://github.com/django/django/blob/fa35c8bdbc6aca65d94d6280fa463d5bc7baa5c0/django/middleware/csrf.py#L439-L451

 Consequently--

 1. `self._set_token(request, response)` can get called twice in some
 circumstances, even if `response.csrf_cookie_set` is true at the
 beginning, and
 2. the cookie can fail to be reset in some circumstances, even if
 `csrf_cookie_needs_reset` is true at the beginning.

 (I previously let `secur...@djangoproject.com` know about this issue, and
 they said it was okay to resolve this publicly.)

-- 
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/052.f116460b8bbb69f19427f1d37c7c4a7e%40djangoproject.com.