views:

91

answers:

3

I have created a sign-up and login for my website and all validation works fine for both sign-up and login. After user provides valid credentials he/she is logged into the member area with a welcome message that says Hello first_name last_name.. basically first name and last name is grabbed from database.

Any what I want to do is restrict the member area to only logged in users. Anyone else will be redirected to homepage or login page or where ever I decide they should be redirected to.

I use ci_sessions which are stored in a table "ci_sessions" in my database. Session_id, ip_address, user_agent, last_activity and user_data are the columns. So I guess that's some form form of security rather than have a cookie stored on the users browser alone there is more.

Anyway right now to stop anyone else apart from logged in users to access my website member area e.g. http://mysite.com/member_area I use a simple if statement in my controller for the member area:

if (! $this->session->userdata('first_name'))
    {
    redirect('login');
}

This checks to see whether the person who is attempting to access the member area page has some kind of data stored in user_data in my ci_sessions table such as a first_name and if so allows them to access the page meaning they must have logged in and still have an active session.

If nothing is found in the database they are redirected to the websites login page. What i want to know is if there is a better way of doing this? Is the way I'm doing it now secure enough?

Below is my model code:

<?php
class Current_User {

    private static $user;

    private function __construct() {}

    public static function user() {

        if(!isset(self::$user)) {

            $CI =& get_instance();
            $CI->load->library('session');

            if (!$user_id = $CI->session->userdata('user_id')) {
                return FALSE;
            }

            if (!$u = Doctrine::getTable('User')->find($user_id)) {
                return FALSE;
            }

            self::$user = $u;
        }

        return self::$user;
    }


    public static function login($email, $password) {

        // get User object by email
        if ($u = Doctrine::getTable('User')->findOneByEmail($email)) {


            // to ge the mutated version of the input password
            $u_input = new User();
            $u_input->password = $password;

            // password match
            if ($u->password == $u_input->password) {


                $CI =& get_instance();
                $CI->load->library('session');
                $CI->session->set_userdata('user_id',$u->id);
                $CI->session->set_userdata('username',$u->username);
                $CI->session->set_userdata('first_name',$u->first_name);
                $CI->session->set_userdata('last_name',$u->last_name);

                self::$user = $u;

                return TRUE;
            }

            unset($u_input);
        }

        // login failed
        return FALSE;

    }


    public function __clone() {
        trigger_error('No duplicates allowed.', E_USER_ERROR);
    }

}

All your advice is appreciated.

UPDATE

How about adding this to my model

$CI->session->set_userdata('logged_in', 'TRUE');

This basically adds "logged_in" to my user data in session in DB with the value "TRUE". in my controller for my "member area" I have edited the if statement to say this:

if (! $this->session->userdata('logged_in')==TRUE)
{
redirect('login');

}

If the item doesn't exist "which it won't if a user isn't logged in" then FALSE will be returned and user will be redirected to login page

What do you think?

or I could even make TRUE something secret like dsb453rerfksdhbdsks322 for example. Something random.

+4  A: 

You've hit the nail on the head, but there's a slightly more efficient way to do it.

Extend the base controllers, one way (i believe originally outlined by Phil Sturgeon) but I'll summarise here:

See this article for a very indepth write up.

but in essence:

<?php
class MY_Controller extends Controller
{
    function __construct()
    {
        parent::Controller();
        if (! $this->session->userdata('first_name'))
        {
            redirect('login'); // the user is not logged in, redirect them!
        }
    }
}

so now if you want to restrict access, simply:

class Secret_page extends MY_Controller {

 // your logged in specific controller code
}

and the extended controller will automatically check if the user is logged in in the constructor.

as for how, I'd probably set the user_id as the value to check if its set, or perhaps a user "group" - then you can get user permissions and varying levels of access in your system.

hope this helps a little.

edit

Add this to application/config.php

/*
| -------------------------------------------------------------------
|  Native Auto-load
| -------------------------------------------------------------------
| 
| Nothing to do with cnfig/autoload.php, this allows PHP autoload to work
| for base controllers and some third-party libraries.
|
*/
function __autoload($class)
{
    if(strpos($class, 'CI_') !== 0)
    {
        @include_once( APPPATH . 'core/'. $class . EXT );
    }
}

As you are using CI 2.0, you will need to place the MY_Controllers inside Application/CORE rather than Libraries.

My Application/Core Looks a little like:

Admin_Controller.php
MY_Controller.php
Public_Controller.php
Ross
Yea thanks a lot. I will read the Phil Sturgeon article to get a better understanding. Another thing i have noticed is that even if i'm not logged in session id, user_agent, ip, last_active are stored/updated in the database. Now i can imagine millions of people browsing the website without being logged in. Is this bad? Shouldn't it only logged information of users logged in? if not is there a way I can get it to destroy this data or not log it at all if the user isn't logged in?
Psychonetics
Fatal error: Class 'MY_Controller' not found in ... I think it's a codeignitet 2.0 bug
Psychonetics
nada, you need to make use of PHP5s autoloading magic. see my edited post
Ross
All sorted! Thanks
Psychonetics
You should accept answers that solved your problem.
Mahdi.Montgomery
A: 

i use something like:

    $user_session = $this->model_user_lite->Session_Check();
    if ( $user_session == FALSE )
    {
        redirect('main/about');
        exit();
    }
    $data['auth'] = $user_session;
    $user_id = $this->model_user_lite->Session_UserID ();

to protect not the controller but the functions on their own, which in my opinions is more efficient. The code is pretty self explaining, however i put the session check too:

function Session_Check ( $return_link = FALSE )
{

    $auth   = $this->session->userdata('logged_in');
    if ( $auth == FALSE )
    {
        $return_value = FALSE;
    }
    else
    {
        $return_value = TRUE;
    }

    return $return_value;

}

so where the user logs in, we just set the userdata "logged_in" to TRUE and check it whenever we want.

I hope it helps!

Herr Kaleun