tags:

views:

64

answers:

1

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.

+2  A: 

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!

Lakshman Prasad
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
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!
Lakshman Prasad
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.
Lakshman Prasad
@becomingGuru: But your code is not fixing this. It is still unsecure. Your remark is right though.
Felix Kling
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
@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