Hello all, Is there any chance I could get some feedback on why this was not accepted? I am a little surprised since Jacob Kaplan-Moss seemed to indicate to me over IRC that this was very likely to get an award for GSoC. Thanks, Connor
On Saturday, March 26, 2016 at 12:47:52 AM UTC+8, Connor Boyle wrote: > > > On your last point: This may be a very bad idea from the beginning, but > I'd hope to experiment with making a wrapper object for form.cleaned_data > whose (the wrapper object's) .__getattribute__() returns the value for that > key in form.cleaned_data. > > For example: > calling wrapper.name would get the value stored in > form.cleaned_data['name'] > > On Friday, March 25, 2016 at 4:21:09 AM UTC+8, is_null wrote: >> >> Hi Connor, >> >> Overall I find it pretty cool that work on this has been started. >> There are a few questions I'd like to ask on this proposal. >> >> In this example: >> >> class ReadingDelete(UserPassesTestMixin, DeleteView): >> model = Reading >> >> def test_func(self): >> user = self.request.user >> reading = self.get_object() >> return reading in user.leader.club.readings.all() >> >> Should we really fetch the object from all objects and then check it >> against a "secure" queryset ? After all, there's absolutely no >> security here between the "get_object()" call and the "reading in ..." >> test. Wouldn't it be more efficient and safer to try to fetch the >> object directly from a secure queryset ? ie: self.queryset = >> user.leader.club.readings.all(); reading = self.get_object(). This >> somehow relates to the Form Choices Filter sections of the proposal: >> >> > running a Condition against every instance in a queryset can quickly >> become very inefficient. For cases when it would be necessary, the mixin >> would provide a callback to allow the developer to use whatever means they >> want to more efficiently narrow down the queryset before the Conditions are >> run against its instances. This may seem like redundant code, however the >> purposes of the two different "narrowing" methods are not the same, one is >> for efficiency, one is for security. Rough implementation: >> >> The implementation is pretty rough indeed: it seems like the queryset >> is narrowed only if method == 'GET'. Doesn't that mean that a >> malicious user can validate a form with selected choice that was not >> displayed in the initial rendering ? >> >> IMHO this is the most challenging part of the subject this proposal is >> about. Perhaps if Conditions could translate to combinations of Q >> objects it would be easier to get the best of both worlds. >> >> >> > As is probably fairly clear from the above code, a user attempting to >> access the above view would have to have theiris_staff attribute set to >> True, otherwise the view will (if the following behavior is not overridden) >> raise a PermissionDeniederror with an appropriate message provided >> automatically by conditions.RequestUserCondition. >> >> What kind of messages would be appropriate to use here ? Wouldn't it >> be tricky to find safe messages, that wouldn't help a malicious user >> gain information about the security they are trying to circumvent ? >> For me, this raises the same problem as with login forms: if an >> attacker gets a "incorrect username or passord" message on a failed >> login test then they don't learn if the username exist, whereas having >> an "incorrect password" message on a failed login test does leak the >> information that the username exists. That said, this kind of feature >> would *definitely* by useful for logs. >> >> >> > Since Clubs no longer have .leader (they now have .leaders), the if >> statement's condition becomes nonsense, and due to Django templates' policy >> of silent failure simply doesn't show the contents of the if statement, >> thus not showing the links to the update and delete pages for that Reading. >> >> If such a bug is found, then a the DoD of the bugfix should include a >> regression test to ensure that leaders always see the edit/delete >> links in the template response. Should Ahmad have duplicate logic in >> templates like this in the first place ? I recon it's the same problem >> everybody has to solve in their projects though: perhaps it would be >> interresting to encapsulate this particular logic in a template filter >> ? >> >> {% if reading.club.leader.user == request.user %} >> <a href="{% url 'reading:update' pk=reading.pk %}">Update</a> >> <a href="{% url 'reading:delete' pk=reading.pk %}">Delete</a> >> {% endif %} >> >> Becomes: >> >> {% if reading|can_edit:request.user %} >> <a href="{% url 'reading:update' pk=reading.pk %}">Update</a> >> <a href="{% url 'reading:delete' pk=reading.pk %}">Delete</a> >> {% endif %} >> >> Or, with a context variable, like in the case of django.contrib.admin: >> {% if has_change_permission %}. That said, I'm all for providing a >> framework for Ahmad to have a convenient, consistent, secure and >> forward-compatible way to encapsulate these permission checks. >> >> >> > Combiners >> > There would also of course be classes for combining multiple conditions >> into one. The two "combiners" would beEveryCondition and AnyCondition. >> >> Would it be nice to leverage & and | operators and perhaps have ~ for >> negation like with Q objects ? >> >> >> In this example: >> >> class Book(models.Model): >> class Meta: >> conditions = { >> 'access': (IsInLibrary,), >> 'create': (IsAuthor, CanWrite), >> 'read': (CanRead, OwnsBook), >> 'update': (OwnsReadPen,), >> 'delete': (IsABadPerson,), >> } >> >> What is the difference between access and read ? Is 'access' here as >> an example of non-default permission type that Django users could >> implement, such as 'borrow', or would this be used by any feature >> provided by the framework itself ? >> >> >> > Eventually, the Conditions API could provide a mixin for views to take >> Conditions that were originally designed to be run on objects and apply >> them to the .cleaned_data of forms. It is uncertain how reliably this could >> really work, but it would certainly increase the usefulness of the >> Conditions API if it did. For example, it would allow a developer to use >> the same Conditions they would to limit the kind of object a user can >> access to limit the kind of object the user can create. >> >> How would that work exactly ? Could you add an example ? >> >> >> Also note that I'm just an amateur django-developer so perhaps don't >> take my review too seriously, I'm just trying to learn and help ;) >> >> Best >> >> James >> > -- 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/a08236d3-31c3-44d0-81ab-a958be0f6d3b%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.