0

I'm new to Django but I seem to have nearly identical code working on another site. I can update a record in the Django shell, but in view.py the same code insists on INSERTing a new record when I run this form.

  • I have a "DisciplineEvent" object in the model. I let Django create the "id" field for the primary key. I can see the "id" int column has been created with auto_increment in MySQL
  • The form DisciplineEventEntryForm is created from the "DisciplineEvent" model object.
  • To edit a record, the entry form is populated and the pk is put in a hidden field named "id", which appears to be submitted along with the POST data.

So the relevant part of the view.py is this:

if request.method == 'POST':
    incidentId = request.POST['id']
    editedEvent = DisciplineEvent.objects.get(pk=int(incidentId))
    form = DisciplineEventEntryForm(request.POST, instance=editedEvent)
    form.save()
    variables = Context({
        'account': account,
        'date': request.POST['event_date'],
        'description': request.POST['incident_description'],
        'incident_id':incidentId,
         })
     template = get_template('disciplineform_confirm_entry.html')
     output = template.render(variables)
     response = HttpResponse(output)
     return response

I thought this would pull the record in question, save the new form data into it, and UPDATE the record. Instead it creates a new record with all the data and an incremented primary key.

ScottOrwig
  • 95
  • 1
  • 2
  • 4
  • Can you include your form definition please? – Wogan Mar 04 '10 at 05:02
  • and maybe the template code as well. It might not be a good idea to have 'id' as a POST member. I'm stumped at the moment but your code is a little unconventional so it's hard to guess where the problem might be. – Brian Luft Mar 04 '10 at 05:53
  • I only would save the form if it is valid... are you sure that `DisciplineEvent.objects.get(pk=int(incidentId))` returns an object? Maybe something is broken and `editedEvent` does not contain any object. – Felix Kling Mar 04 '10 at 07:15

1 Answers1

9

What you are trying to do is unconventional and a possible security hole.

You should not get the instance of the object from the hidden id key you populated in the form. Users can easily change this one and get your code to overwrite some other model instance that they may not even have permission for.

The standard way to do it is to obtain the object based on the url.

def view_function(request,id):
    object_to_edit = get_object_or_404(Model,id=id) #Or slug=slug
    form = ModelForm(data = request.POST or None, instance=object_to_edit)
    if form.is_valid():
        form.save()
        redirect()
    return render_to_response('template_name',{},RequestContext(request))

Hope it helps!

lprsd
  • 84,407
  • 47
  • 135
  • 168
  • In what way is using part of the GET data less of a security hole than using part of the POST data? Both are entirely open to modification by the user. – Kylotan Mar 04 '10 at 14:15
  • Kylotan: You got both him and me, wrong. :P Neither is he passing a parameter via GET, nor am I suggesting him to do it via POST. It could perhaps help if you **read** both the question and the answer! – lprsd Mar 04 '10 at 15:38
  • Kylotan: To expand, he is already POSTing the data; but he modifies the content in the db using the POSTed `id` **alone**. That is the risk. – lprsd Mar 04 '10 at 15:41
  • 2
    @becomingGuru: But your code is not fixing this. It is still unsecure. Your remark is right though. – Felix Kling Mar 04 '10 at 17:21
  • This code worked! Still not sure why mine didn't but I'll do it your way from now on. For this application security of individual records isn't really a concern, but if it were I gather the only safe way to go is to code a check for rights before modifying any record, right? Regardless of the way I pass in a primary key (URL, GET, or POST) it would appear hackable. Right? Or is there a better Django solution I haven't learned yet? Thanks to everyone for your help! – ScottOrwig Mar 05 '10 at 01:29
  • 2
    @becomingGuru: you have clearly said "obtain the object based on the url". The URL is part of the GET request and just as easy for the user to modify as the form's hidden POSTed field is. What is important is how that 'id' value is determined and the checks done on it, and you've not addressed that in this answer. – Kylotan Mar 06 '10 at 22:02