• 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Passing ID in url without letting others users manipulate it when editing forms

#1
[eluser]jleequeen[/eluser]
Hello All,

I've got a quick little question regarding edit forms. When I pass the "id" from the database table to the view which decides which field to edit, how I keep someone from manipulating the ID in the URL and updating something they should. For example I have a form with the URL of:

Code:
sales/edit/123

Obviously I would love to pass just the ID through to the edit method of the sales controller, but what keeps someone from just adjusting the id parameter in the URL and editing someone elses record? Do I need to do additional checks against say customer_id to make sure they are who they are? Look for the customer_id in a session variable somewhere? My simplified model looks like this:

Code:
Class Sales_model extends Model
{
    function edit($id, $data)
    {
        $this->db->where('id', $id);
        $this->db->update('sales', $data);
        return TRUE;
    }
}

Maybe I'm not seeing something, but I guess I just don't see what keeps one customer from fudging the URL and updating another companies record.

#2
[eluser]mdowns[/eluser]
I can think of two ways. The pretty way (IMHO) is to implement an ACL. There a lot of posts regarding ACL's and CI so just search the forums.

The second way would be to store the id in a session. Then when the user navigates to edit, it uses the id from the session. I don't really like that solution, though.

#3
[eluser]LuckyFella73[/eluser]
Just add a hidden input field in the edit-form
containing the id. In the controller you can check
the POST value of the hidden field value and do
all the validation and db updates or whatever you
have to do.

#4
[eluser]Rick Jolly[/eluser]
[quote author="jleequeen" date="1219869540"]Do I need to do additional checks against say customer_id to make sure they are who they are? Look for the customer_id in a session variable somewhere?[/quote]
Yes. There should be a link between sales records and customers. For example, each sales record could have a customer id as the foreign key. So you'd get the customer id from the session and do something like this:
Code:
Class Sales_model extends Model
{
    function edit($id, $customer_id, $data)
    {
        $this->db->where('id', $id);
        $this->db->where('customer_id', $customer_id);
        $this->db->update('sales', $data);
    }
}

Also, don't trust a hidden form field any more than the url.

#5
[eluser]jleequeen[/eluser]
Thanks for your suggestions.

#6
[eluser]shaffick[/eluser]
Instead of passing the ID in the URL, you could generate a hash (x characters) and add that to your products table in the db. (edit: each product or item in that table will have a unique hash btw)

And pass that hash in the URL. That might be more secure if you have to pass the reference id in the url.

cheers Smile

#7
[eluser]Michael Wales[/eluser]
The real problem is the insecurity within your form processor. It doesn't really matter if people can change the ID within the URL and it doesn't matter how you hash it or obfuscate it - when you get to the root of the problem: users should not be allowed to view/edit other user's records, yet they can.

Solve that and you solve your problem.

Since we don't really know anything about your application I'm just going to assume there is a user_id field associated with each record. That user_id acts as a foreign key to your users table, ensuring that the record in question belongs to a real, legitimate, authenticated user.

If this is the case, it's as simple as an if...then statement within the first line of your method. Psuedo-code below:

Code:
function view($id) {
  $this->load->model('record');
  $record = $this->record->get($id);  // Using our model to return an object representing our record
  // Is this one of our authenticated user's records?
  if ($record->user_id !== $this->session->userdata('user_id')) {
    // Nope - GTFO!
    redirect('');
    return;
  }
  // This is one of our user's records - show them the form
  $this->load->view('records/view');
}

Copy/paste and your edit method has a great starting point.

#8
[eluser]Rick Jolly[/eluser]
Then we agree Michael ;-). That might be my first emoticon.

Watch out for that reserved "view" method name.

#9
[eluser]sikkle[/eluser]
hehe cheap shot ! Smile

#10
[eluser]drewbee[/eluser]
Also, when I am updating data in a form (editing), I always pull in the record in question. During the initial page load the query is run to get the default values. This same query is run even during the post-submit page to verify that the record is indeed theres, and they can update it.

With that being said, if the user messes with the url and tries to edit anything that isn't theres (even after the post) they are auto matically redirected to the create screen of the form.

And Yes I know this creates additional overhead, but it is in the name of integrity, and I am willing to take the risk of that one additional query.

Something else I do as well. Lets say I have a drop down and that drop down is populated from a database lookup. I also call these lookups on the post-submit page. I don't want the user submited values in the database that are not exactly what they are suppose to be. Once again -- additional overhead in the name of integrity.


Digg   Delicious   Reddit   Facebook   Twitter   StumbleUpon  


  Theme © 2014 iAndrew  
Powered By MyBB, © 2002-2019 MyBB Group.