Security: When is a good time to check for permission?

[eluser]Peter Bowen[/eluser]
A logged in user should only be able to access records (eg contacts, invoices etc) belonging to them.

The record id is passed from the view to the controller either in the URI as controller/method/[id] or as a hidden form field. This means that the record id is visible to a logged in user.

Someone could log in and then try to guess the id and do horrible things. (It would be difficult because it's a long random string) but that's not enough security.

Right now the owner's id is stored with each record. The model checks that the record belongs to the user before doing anything with it. This seems wasteful - an extra database call every time a record is read, updated or deleted.

Is there a better way to do it? I'd appreciate your thoughts.

Kind regards

Pete Bowen

[eluser]James McMurray[/eluser]
You could retrieve a list of the user's records when they log in and keep it in the session, but that's going to be wasteful in a different direction (session size vs. database access). Which is better will depend on how many records someone can have and how often they're expected to change pages.

[eluser]Frank Rocco[/eluser]
Why not keep the owner id in a session variable.
Filter records based on the owner id in session.

If a different user changes the record id on the url line, they will not be able to see the record, as it does not belong to them.

[eluser]pickupman[/eluser]
Frank's got the right approach. If you are requiring every model call be validated against the owner id, and you are using ActiveRecord for you queries, you can use something like:

Code:

class Customers extends Model{

  function __construct(){
     parent::Model();
     $this->_where();
  }

  function _where(){
     $this->db->where('owner_id', (int)$this->session->userdata('owner_id'));
  }

//Rest of model
}

Now all of your DB queries will have AND owner_id='(int)', so you don't have to write it on all of them. Or if you don't want it across the entire model, then remove the statement from the constructor, and only call it at the top of each method.