I'm +1 with Baptiste, Ben, Josh and João.
Also, Luke, you said :
> 1. Recognise that CBVs are much harder to reason about, and therefore
> require much more care.
2. Avoid using CBVs unless you really need them.
>
Just wanted to note that this means never. FBV vs CBV is a choice, there's
really few cases, if any, where one or the other is *really needed*.
>From my (highly personal) point of view, some people will always be more at
ease coding with CBV than with FBV and vice versa. FBVs are simpler at
start, CBVs are more customizable, but in the end, you'll find a way to
customize your FBV (a.k.a copy/pasting code) and the added complexity to
your GCBV subclass probably won't be that high (chances are : you'll be
adding some variable in the get_context_data(), while still calling super
and that's it)
Maybe what we should be providing is more a way to put the complicated
security-important logic out of a view altogether. A PasswordChanger
class/module/... that would do the necessary work but just this work,
independantly from how a view works. and then CBVs/FBVs would just call
functions/methods on that. But then if there's already too much
abstractions in CBV for the taste of some...
My 2 cents.
Joachim
Le mercredi 23 novembre 2016 06:16:24 UTC+1, Luke Plant a écrit :
>
> Hi all,
>
> First, I want to say that complex things fail in complex ways, so there
> it's probably a fallacy to look for a single root cause. I agree with
> various other points about mistakes that were made, but not others.
> Particularly:
>
> On 22/11/16 12:41, Florian Apolloner wrote:
>
> Hi,
>
> On Monday, November 21, 2016 at 11:56:56 PM UTC+1, Tim Graham wrote:
>>
>> … that the existing tests would catch this type of obviously incorrect
>> issue.
>>
>
> I think that is the main issue here. I was also really surprised to
> discover that the tests were missing cases like this -- then again, writing
> good tests is hard and I know that I am personally one not to write good
> tests usually. Either way, I think the way forward is to improve the
> testsuite in this case.
>
>
> I disagree with this, there was no issue with the existing test suite for
> the FBV version. *Tests are never close to exhaustive and we should never
> think they are or can be.* Tests are spot checks of correctness in an
> infinite universe of possible input values. When trying to work out what
> you should test in this vast space, there are a few sensible ways:
>
> 1. Look at the requirements, and pick spots that match each requirement.
> In this case:
>
> - If a request has the required token, it should be allowed through
> - If a request does not have the required token, it should not be
> allowed through
>
> 2. Make some guesses about well known edge cases and test them (e.g the
> empty string case etc.). However this could be a complete waste of time
>
> 3. Look at the implementation at work out where it might go wrong. This
> one is really important in reality.
>
> And this is what the existing tests did. Out of the infinite number of
> HTTP requests to test under point 1, they happened to pick a GET request,
> which was perfectly sensible. In the case of FBVs, the control flow is so
> clear that all of the following are equally silly tests to add:
>
> - Check that requests with query string parameter 'xyz' doesn't
> introduce a back door
> - Check that on leap years tokens are checked
> - Check that the security is correct for requests that are
> specifically GET, POST, HEAD, OPTIONS, DELETE....
>
>
> The conclusion is this: in CBVs control flow is much, much less clear. In
> the FBV the mistake would have been immediately obvious, the CBV base
> classes obfuscated things to the point where it wasn't at all clear. To
> write adequate tests for the CBV version, under point 3 above, you have
> *much,
> much more work* to do.
>
> I disagree that forcing people to write their own auth views is *
> necessarily* worse than providing a CBV that people can customise. Given
> this demonstration of how CBVs obfuscate control flow, it's quite possible
> that writing an FBV from scratch will be less error prone than using a CBV
> and subclassing, especially when we have encapsulated things like the token
> checking. Personally I would be massively happier auditing a custom FBV
> than a subclassed CBV, especially when you consider that the CBV subclass
> is inheriting from a stack of classes that could change in later versions
> of Django in some subtle way that breaks the code. Subclassing is not
> necessarily safer, it just feels safer ("I'm only changing this one little
> method"), and *that feeling of greater safety is itself a huge danger*.
>
> Going forward, I think we need to:
>
> 1. Recognise that CBVs are much harder to reason about, and therefore
> require much more care.
> 2. Avoid using CBVs unless you really need them.
> 3. Recognise that tests for FBVs are inadequate against the CBV
> translation.
> 4. Not deprecate the FBVs. At the very least, they provide a sensible and
> easy to understand starting point for people that want to copy the logic to
> create their own, and do so in a way that they actually understand and can
> audit.
>
> I'd be +0 on reverting to an FBV only, but it's part of a much bigger
> discussion
>
> Regards,
>
> Luke
>
>
--
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/0b1012db-c44e-4aee-826a-da9fcbb7069b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
