Re: Consider reverting or adding guidelines on how to use class based views for security sensitive features

2016-11-21 Thread Yoong Kang Lim
> I think that patch was just an example of bad abstraction. For instance,
_log_and_response was strange and confusingly named, and seemed to be there
mostly for vanity, to mask the imperative nature of the top level of
control.

Proposed patch author here. Yes, I agree that wasn't a very successful
attempt at the abstraction, and I think that's why I abandoned that effort.

Perhaps the best abstraction isn't a Class Based View at all? I'm sure we
could write classes other than Views, instead of forcing the abstraction to
be a subclass of a View.



On Tue, Nov 22, 2016 at 3:29 PM, Yo-Yo Ma  wrote:

> > I found it much more difficult to follow to the point where I didn't
> feel it was an improvement.
>
> I think that patch was just an example of bad abstraction. For instance,
> _log_and_response was strange and confusingly named, and seemed to be there
> mostly for vanity, to mask the imperative nature of the top level of
> control.
>
> When "properly" abstracted, class-based views, IMHO, are much simpler to
> reason about. And, despite the one extra indentation level of a method (vs
> a module "function"), CBV tend to offer flatter logic and better separation
> of concerns.
>
> Somewhat ironically, I find better examples of functional style code in
> CBV than I do in view functions.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers  (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-developers.
> To view this discussion on the web visit https://groups.google.com/d/
> msgid/django-developers/6acd4821-d51f-4bf4-9550-
> f3843eb28805%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAKL8yL4J-42Tn%3D_Lhi%2BAvAoY8nbG8-gUE47jAwc7xpZCCCm_Wg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Consider reverting or adding guidelines on how to use class based views for security sensitive features

2016-11-21 Thread Yo-Yo Ma
> I found it much more difficult to follow to the point where I didn't feel it 
> was an improvement.

I think that patch was just an example of bad abstraction. For instance, 
_log_and_response was strange and confusingly named, and seemed to be there 
mostly for vanity, to mask the imperative nature of the top level of control.

When "properly" abstracted, class-based views, IMHO, are much simpler to reason 
about. And, despite the one extra indentation level of a method (vs a module 
"function"), CBV tend to offer flatter logic and better separation of concerns.

Somewhat ironically, I find better examples of functional style code in CBV 
than I do in view functions.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/6acd4821-d51f-4bf4-9550-f3843eb28805%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Consider reverting or adding guidelines on how to use class based views for security sensitive features

2016-11-21 Thread Tim Graham
I haven't extended these views much, so I can't talk about the pain points 
of extending the function-based views compared to the ease of extending the 
classes. I'm certainly more confident about reasoning with function-based 
code. There was a draft patch [0] a few months ago that converted some 
admin views to class-based views. I found it much more difficult to follow 
to the point where I didn't feel it was an improvement.

When reviewing the patch for the auth class-based view additions, I made 
the mistake of assuming that the existing tests would catch this type of 
obviously incorrect issue. Perhaps with the function-based views, the code 
was "too obviously correct" that a test for token verification on POST 
wasn't considered. I'd like to think that future work on the class-based 
views would be more careful about testing, especially after we made this 
mistake.

If you feel the class-based views aren't reliable enough to deprecate the 
function-based views immediately, then I would say we shouldn't ship those 
views in Django in the first place. contrib.auth isn't a good place to ship 
"experimental API." The class-based views would certainly benefit from 
further audit now that we realize this type of mistake is possible.

[0] https://github.com/django/django/pull/6045

On Monday, November 21, 2016 at 2:11:34 PM UTC-5, Markus Holtermann wrote:
>
> Hi all,
>
> As it turned out [1], due to their complexity, using class-based generic 
> views for security-sensitive functionality can result in unintended 
> behavior. Essentially, the reset token was only checked on GET requests, 
> not on POST. This was due to the check being in `get_context_data()` (which 
> is only called on GET but not POST except for invalid forms) and not higher 
> up the stack. Validation could happen in the SetPasswordForm but doesn't 
> really belong there either. The form is being used by the admin to allow 
> superusers to change other users' password. Also, password resets could 
> probably happen via other ways that want to leverage a form that doesn't 
> require a token. In the end, from my perspective the check for the correct 
> token does belong in the view.
>
> While the reported issue was fixed [2] it raises the question if the added 
> functionality of class-based generic views is worth the danger of shooting 
> ourselves in the foot. I see the benefits of GCBVs. But given that the 
> reported issue stayed unnoticed for 4 months makes me think that those 
> views are not the best for these use cases and easily underpin the security 
> functionality. Hence I suggest to revert the patch (including all 
> additional features they gained) unless they are integrated in the 
> function-based views and add guidelines on how to use class-based generic 
> views for security sensitive feature.
>
> This is the thread to get the discussion about this started.
>
> One thing I want to suggest regardless if the class-based generic views 
> are removed again or not, is to hold off the deprecation of the 
> function-based views. This allows users who feel the same to not use 
> class-based generic views without having deprecation warnings. At least 
> until the next LTS release.
>
> Furthermore, myself and Florian Apolloner, who discovered the issue, are 
> leaning +0 to +1 on the revert of the class-based views.
>
> Cheers,
>
> Markus Holtermann
>
> [1] 
> https://www.djangoproject.com/weblog/2016/nov/21/passwordresetconfirmview-security-advisory/
> [2] https://github.com/django/django/pull/7591
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/491d8668-2024-4dd4-bf1e-5dfca54aab08%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[ANNOUNCE] Django security advisory: Vulnerability in password reset (master branch only)

2016-11-21 Thread Tim Graham
We don't normally give security advisories for issues that affect only
the master branch, but in this case we've made an exception as the issue
could be high impact.

Please see the blog post for details:
https://www.djangoproject.com/weblog/2016/nov/21/passwordresetconfirmview-security-advisory/

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/3b5835b7-d14a-4c0e-9cd5-eab9fcec5fc5%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Consider reverting or adding guidelines on how to use class based views for security sensitive features

2016-11-21 Thread Tom Christie
Just to be absolutely clear, in case it's needed...

> is to hold off the deprecation of the function-based views.

Markus is specifically referring to the FBV implementations of the 
contrib.auth views here.
(Not to FBVs generally, which we've no intention of deprecating whatsoever)

  - Tom

>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/ee70f431-412d-45ee-b646-37adb1d1f464%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Consider reverting or adding guidelines on how to use class based views for security sensitive features

2016-11-21 Thread Markus Holtermann
Hi all,

As it turned out [1], due to their complexity, using class-based generic 
views for security-sensitive functionality can result in unintended 
behavior. Essentially, the reset token was only checked on GET requests, 
not on POST. This was due to the check being in `get_context_data()` (which 
is only called on GET but not POST except for invalid forms) and not higher 
up the stack. Validation could happen in the SetPasswordForm but doesn't 
really belong there either. The form is being used by the admin to allow 
superusers to change other users' password. Also, password resets could 
probably happen via other ways that want to leverage a form that doesn't 
require a token. In the end, from my perspective the check for the correct 
token does belong in the view.

While the reported issue was fixed [2] it raises the question if the added 
functionality of class-based generic views is worth the danger of shooting 
ourselves in the foot. I see the benefits of GCBVs. But given that the 
reported issue stayed unnoticed for 4 months makes me think that those 
views are not the best for these use cases and easily underpin the security 
functionality. Hence I suggest to revert the patch (including all 
additional features they gained) unless they are integrated in the 
function-based views and add guidelines on how to use class-based generic 
views for security sensitive feature.

This is the thread to get the discussion about this started.

One thing I want to suggest regardless if the class-based generic views are 
removed again or not, is to hold off the deprecation of the function-based 
views. This allows users who feel the same to not use class-based generic 
views without having deprecation warnings. At least until the next LTS 
release.

Furthermore, myself and Florian Apolloner, who discovered the issue, are 
leaning +0 to +1 on the revert of the class-based views.

Cheers,

Markus Holtermann

[1] 
https://www.djangoproject.com/weblog/2016/nov/21/passwordresetconfirmview-security-advisory/
[2] https://github.com/django/django/pull/7591

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/19b675f5-c25a-43e8-ac73-2a31b9e351d6%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Newbie's questions

2016-11-21 Thread ranvir singh


On Saturday, November 19, 2016 at 3:44:11 AM UTC+5:30, Jeremy Spencer wrote:
>
> There is extensive details on the django project website on this topic: 
>
>- https://docs.djangoproject.com/en/dev/internals/contributing/
>- 
>
> https://docs.djangoproject.com/en/dev/internals/contributing/new-contributors/
>
> Thank you very much for your help, I already have read this. I want to 
know the importance of contributors licence that the community want the 
people to fill before contributing. Can't this process be automated as the 
person have to take a print out and then send the signed copy to the 
community. 

Also I have been working with github issues for a while so it is a little 
difficult to understand the ticket system and bug tracking system that the 
community uses. Can you please shed some light on this too.

--
Ranvir Singh
https://ranvirsinghprojects.wordpress.com
https://github.com/singh1114
  

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/27263773-3e5a-4d59-8501-3e1d8553c447%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Newbie's questions

2016-11-21 Thread ranvir singh


On Saturday, November 19, 2016 at 5:01:45 AM UTC+5:30, sste...@gmail.com 
wrote:
>
> Probably want to change that 'secret key' you used on the live site a bit 
> more private:
>
> # SECURITY WARNING: keep the secret key used in production secret!
> S
> S 
>

Thank you for your response. Please elaborate more on what you want to 
convey. Being newbie I can't understand whatever you wanted to say.
 
--
Ranvir Singh
https://ranvirsinghprojects.wordpress.com
https://github.com/singh1114
 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/42bd44bb-9eec-4cfc-be1b-52d3dbf115d2%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


debugging the test suite of django itself

2016-11-21 Thread Aleksej Manaev
I'm working on a django ticket and my changes lead to errors in the django 
test suite. How can you debug the test suite of django itself in PyCharm?

I can run the tests:
runtests.py staticfiles_tests

I tried Python tests -> Unittests and it says:
django.core.exceptions.ImproperlyConfigured: Requested settings, but 
settings are not configured. You must either define the environment 
variable DJANGO_SETTINGS_MODULE or call settings.configure() before 
accessing settings.

Using Django tests as Configurations asks for Django project root, settings 
and a manage script.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/1270cf2a-ce04-4ead-b4a0-70d49cdf3f57%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.