Is Session a safe place to store data ?

Hello All !

I am working on project to build a webapp to manage the finances of multiple companys.

My question is: Is it safe to store imnportant user related data in session ?

Example: I want to allow users to create invoices in my app.

In my DB I have a table called invoices, and it has a column called company_id.

When the user logs in my application (using Ion Auth) its company_id is stored in session...

Than every time I want to retrieve a record I will add a where statement like this:


        $this->db->where('company_id', $this->session->company_id);
        $query = $this->db->get('invoices'); 


My point is that if a user can manipulate session data, he will be able to see other company invoices... and I dont want that....

Yes you can use sessions for that.

The session data is stored on the server, the client's browser is linked to its session on the server cased on the contents of a cookie containing only a reference ID. There are techniques where a hacker can 'hijack' someone else his session. You should have a look here: https://www.owasp.org/index.php/Session_...ing_attack to get familiarized with the methods that can be used to hijack a session and make sure your app does not allow any of those techniques.

Codeigniter also give you the option to link a session id to a particular IP address. This can prevent for example some evil hacker from the UK to take over the session from a client in the US. But this will also mean that a genuine user cannot use it's own session on his phone for example when switching from his mobile 3g connection to his wifi beacuse he will have a different IP address. So he must login again when switching networks.

PHP Code:

$config['sess_match_ip'] = TRUE; 


It is safe, but the approach is not good. Better keep only the user_id and retrieve the additional information by asking it from the models.

What you do now is introducing session dependency within your models. This limits the context where these models can be used. Imagine that in the future you would need to do something under CLI with these models - it would be impossible without heavy rework.

Edit: Remove all the $this->session->something from all the models and pass them as method parameters.

Good - only store the user id in the session - which is a long randomized string - not an incrementing ID
Better - have a db table for current logged in users - the customer logs in - you verify the log in - you create a long randomized string JUST for the session,
that string is stored in Logged in users table along with the actual customer id, date time, - etc etc

then the randomized id you are storing in the session is only good for that specific session and will not work at all for the real company tables etc - and if they try to use it a day later they will get logged out, etc Bonus is that it gives you an easy record of which customers are logging in, for how long, etc
you could put other things in there like last page visited etc.

Thank you very much for those answers guys !

Since I´ve learned programming and PHP by myself, I miss some of those tricks...

Now that you pointed me that making a DB query for everything is the safer way to go, WHAT ABOUT PERFORMANCE ?

I mean... If I only store the user_id in session, everytime a user make a request I would have to performance all these queries:

* Get the USER group
* Get his GROUP permissions
* Get the USER ACL permissions
* Get the USER company_ID

Before every request my code would run 4 queries....

Why not save those data in session ? Wouldnt the performance be increased in a high usage program ?

THANK YOU !

If they are all linked by user_id with a 1:1 relationship, just do a single query joining all the tables, and get all the data in one go. Storing all that stuff in the session can be done, it is just not a good idea as your end user can easily manipulate the data in the cookie, changing their permissions, their company or their group.

With a well structured database, and well written queries, this additional query will not adversely affect performance. I doubt you will even notice it.

Best wishes,

Paul.

PS Cartalot and ivantcholakov are right, you should not store the user ID directly, nor refer to the session in the models.

Thank you very much for those tips !

I will keep what I´ve learned from you in my mind !

Another question....

All my security verifications are made in the construct__ of my controller.

Is it ok to pass the user variable with all the user info to other functions in the controller ?

Like:

PHP Code:

function __construct() {
  $this->user = $this->access_model->get_user_data();

  //security verifications...
}

function index(){
  $data['user'] = $this->user->id
} 


That is fine.

You can also do it in create a pre-controller or post-controller hook that checks authorization and redirects to the login if not authorized. Of course you have to exclude your login contollers from the hook otherwise you get a never ending loop.

You will still have to collect your user data from your controller if you do that but if you make your authorisation routine as slick as possible, you can then just collect the user data you need per controller rather than collecting everything by default.

But the way you are doing it is fine.

Paul.

http://www.codeigniter.com/user_guide/ge...hooks.html

PS I presume your access model is doing something on authorisation fail? Such as a redirect to login? I prefer my models to return results only, so when I do something like this I call an access library, which calls the model to get the user data, and redirects on fail. Alternatively you could just do the check in the constructor, or call a function to do it. The advantage of the library is that it can then have other access methods in it, and logic to deal with failed logins, like update session with illegal access attempt, check an attempts count and initialise a 1 minute, 3 minute, 5 minute etc escalating access delay.

About:

* Get the USER group
* Get his GROUP permissions
* Get the USER ACL permissions
* Get the USER company_ID

This kind of data you can read once from database at the beginning of the request processing, when the user gets authenticated, and then you can just keep it in memory. You can make a model Current_user that within its properties keeps this kind of data and that has getter/setter methods. Thus you can avoid repetitive database queries. No session storage needs to be involved.

$group = $this->current_user->get_group();
$permissions = $this->current_user->get_group_permissions();

I don't understand why the users table must not be auto incremented. What's the problem in that? Ion Auth from Bed Edmunds is considered insecured?