I’ve implemented a simple DeleteView implmentation which works fine. However, I need to do some additional checks to make sure that that the email value in the object being deleted matches the logged in users email and if not, abort delete. So, after looking at DeleteView I thought I could override delete and to test I simply put a breakpoint() inside but my method never gets called. Not sure why? I also tried overriding post which did get called however, super().post seems to call delete so I can’t see why my override for delete doesn’t work
class ManageEventPeopleDelete(DeleteView):
model = EmailToEvent
def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context['category_id'] = self.kwargs['category_id']
context['association_id'] = self.kwargs['association_id']
context['event_id'] = self.kwargs['event_id']
return context
def delete(request, *args, **kwargs):
breakpoint()
def get_success_url(self):
return reverse("showorderofplay:manage-event-people", args=[self.kwargs["category_id"], self.kwargs["association_id"], self.kwargs["event_id"]])
This is actually a change in Django 4.0.
See the release notes at Django 4.0 release notes | Django documentation | Django
While the class hierarchy has not changed, the flow through the methods has. The “delete” method is no longer used.
In practice, this type of pre-validation should be in the view that preceeds this. This view is not intended to be the view allowing you to choose what object to delete, but to confirm that a deletion should occur.
In other words, you should have a different view allowing the email address to be entered. That view would accept the POST of the form containing the email address. If the view accepts that email address as valid, it can then redirect the user to your DeleteView for confirmation.
But the fact the delete url is still exposed is a risk though, someone malocious could find out what the url is for the delete confirmation and then start adding random id numbers on the end meaning records could get spuriously deleted. For example this is part of the url for the delete confirmation form I have
/delete_person_to_event/11/
where 11 is the object id
Doing this across two steps does not remove the absolute and definite requirement of always validating data being submitted.
The first step, of testing the submitted email address in the selection view, is only a matter of UI/UX convenience.
However, that does not remove the necessity of verifying what’s being done in the second view. You must never, under any circumstance trust what is being submitted from a browser.
What that means in this specific example is that you probably want to implement something like the UserPassesTestMixin to verify the user’s authority to even request the GET for the confirmation.
Sure, that makes perfect sense. I’ve protected against people not logged in trying to use create/update/delete url’s with the use of the @login_required decorator in the classes. Now I’m protecting against logged in people fiddling the integers within the url to access records that they shouldn’t have access to. I’ve implemented a check within form_valid for one of my CreateView’s which works but when I call form_invalid it doesn’t render the CreateView form again but instead seems to fall back to the url before the form. My questions today are:
- Why is form valid not completing render_to_response()
- Could I actually redirect to a totally different url from form_valid()
def form_invalid(self, form):
return self.render_to_response(self.get_context_data(form=form))
def form_valid(self, form):
# Before we save, double check that someone hasn't fiddled the URL to save an event in
# an association they don't have access
association = Association.objects.get(pk=self.kwargs['association_id'])
if not self.request.user.has_perm('check_in_associations',association):
self.form_invalid(form)
...
Absolutely. That’s actually the design intent of the edit CBVs. (That’s the purpose of the success_url attribute.)
I’m kind of lost here with what view and logic you’re trying to refer to.
First, you’re performing this test in the wrong location. The form_valid method should be able to rely upon the fact that the form has been completely tested and validated.
The form_invalid method is intented to redisplay the existing form, rendering any error messages that may be applicable.
Your form_invalid method is not returning a response. It looks like it’s trying to return a dict, which would throw an error. Also your attempt to call form_invalid isn’t doing anything with the data being returned from the call.
I do the test in form_valid because the form itself (form.py) does not have the field ‘association’ (foriegn key) as there is no need to send this to the client to then come back again. Active association id is defined from the integer in the url so I want to test that someone hasn’t altered the integer for association to trick an edit/create/delete in somewhere they would not have accessed if they had navigated naturally. Are you saying though that I could use def clean(self): in forms.py to check for this problem because association_id is in kwargs and I have user info in self.requests.user ?
ah but kwargs aren’t available in ModelForm 
Briefly, yes. Your clean method in your form is not limited to what is submitted by the form.
You can pass additional data to the form from the view. One of the easiest ways is to override get_form, and add attributes to the form that is returned by super().get_form(...).
Or, you can override get_form_kwargs such that these parameters are passed to __init__. You then define a custom __init__ method on the form to save those parameters.
Either way, you would then have those variables available to you in your clean method of the form.
I’ve selected the easy option and passed kwarg value and request.user in get_form() but it seems clumsy to me. Please could you show me what the equivalent implmentation would be using init as this is something I have not yet tried?
def get_form(self, form_class=None):
form = super(EventCreate, self).get_form(form_class)
form.fields['when'].widget = AdminDateWidget(attrs={'type': 'date'})
form.association_id = self.kwargs['association_id']
form.current_user = self.request.user
return form
It’s actually the less-clumsy of the two options.
See the code in the thread at django custom form - removing fields based on user for a sample implementation. (You don’t need to pay a lot of attention to the text, you’re primarily interested in the code.)