views:

56

answers:

1

The Problem

Funny problem. Funny because it looks casual until you start thinking about it. Let's say I allow people to create items based on other items. You can open /items/new?id=3 and unlike your regular new action, instead of seeing an empty form, you will be seeing a form pre-populated with values from item-3. So unlike your average default new action, mine looks like this:

def new
  @item = params[:id] ? Item.find(params[:id]) : Item.new
end

So you can access the action with the url like /items/new?id=1 and the form will be pre-populated with data from item-1.

So far so good.

Now you submit this data. My create action is pretty standard.

def create
  @item = Item.new(params[:item])

  if @item.save
    # happy
  else
    # sad
  end
end

There is just one issue here. The new item that was just created (let's call it item-2) needs to know who the daddy is. In our case, the daddy is item-1 (since the original form was pre-populated with data from item-1) and so the item-2 must have parent_id set to 1. Only, this is not just a nice-to-have. It has to be rock-solid, impossible to tamper with. You must not be able to spoof the parent_id.

So my question is — how can I ensure that parent_id is set correctly when creating new item based on existing item?

Some help will be greatly appreciated, but just so you don't think this is a simple casual issue, here are some non-solutions that you're probably thinking of.

Non-solution #1

So the usual solution to saving parent_id is just have it as hidden field in the form.

<% form_for @item do |f| %>
  ...
  <%= f.hidden_field :parent_id, :value => params[:id] %>
  ...
<% end %>

But here I can easily switch it in my html. When form is submitted server wouldn't have a clue. Server will simply "believe" that the item was created with parent [whatever], when it was supposed to be parent 1.

Non-solution #2

On the server, I could use session to store which item you're editing. Then my new action will look like this.

def new
  if params[:id]
    @item = Item.find(params[:id])
    session[:creating_item] = params[:id].to_i
  else
    @item = Item.new
    session[:creating_item] = :mine # this prevents loophole if session is cleared
  end
end

And my create action will take this value and set it to parent_id.

def create
  @item = Item.new(params[:item])

  if session[:creating_item] == :mine
    @item.parent_id = nil
  elsif session[:creating_item].is_a?(Fixnum)
    @item.parent_id = session[:creating_item]
  else
    # session was probably cleared
    redirect_to root_url and return
  end

  if @item.save
    # happy
  else
    # sad
  end
end

This will not work if you open /items/new multiple times (e.g. in multiple tabs in browser). The session value will be overwritten every time. So if you submit a form opened at /items/new?id=1 but you also opened /items/new?id=2, then parent_id in session will be 2. Fail.

Non-solution #3

What if you save all opened parents in array in session, instead of just overwriting single value. Well, I'm not gonna write out long examples, I'm just gonna say that when any of the open forms are submitted — you will know that it must be one of the parents you saved in session (e.g. 1, 2 or 3), but you still don't know which one. You could then save data from parent 1 as if it was from parent 3.

A: 

How about storing the parent in the session, but using a hash instead of an array. You can use an authenticity token as the key to the hash, so that when the form is submitted you lookup the parent based on this value. Multiple forms will each have their own token, so you can have many parent ids being tracked in the session at the same time.

An alternative would be to change the workflow a little. Have the action actually clone and create the new element before allowing the user to edit. This way the whole problem becomes moot. You may need to confirm with the user before they create to ensure you don't have cloned objects all through the system, or provide a simple delete function if they change their mind.

Toby Hede
The different workflow approach is definitely not something I'd like to do. It's way too easy to create tons of objects that way, and in my domain, that's exactly what's gonna happen. :) As per using auth-token, it's almost helping, but... you can still copy the auth token from one form to another, can't you? It protects from CSRF, but if you open 2 forms and switch tokens — rails won't mind, or would it?
hakunin
You can create some sort of hash based on the parent_id itself and check this against the expected value, similar to how the CSRF stuff works. How big an issue is this stuff actually going to be? Is there other data you can check as a validation step? So cross reference the parent against the new child?
Toby Hede
Any hash solution is easily hacked by swapping tokens from 2 open forms, each meant for different parent_id. Other data can all change, so not really. :( Nothing to x-reference. It will be an ugly and unsolvable issue as soon as somebody finds a way to spoof parent and posts a script somewhere (in my target market this is a reasonable expectation.)
hakunin