Where to do my session's check ?

[eluser]koskoz[/eluser]
Hi everybody,
I'm building an administration for my website, and I met an issue about CI.

Basically, if the user try to access to any admin class, I've got a little piece of code checking if he's correctly auth.

But I don't know where to put this piece of code.
For the moment, as I'm using a template library, I've got one global view for the admin so I put my session check in it, but it's not the aim of a view.

I could do my session check in every admin controller but it's really boring, and if I want to change something I have to do it in every class.

Where do I put my session check ?

Thanks.

[eluser]InsiteFX[/eluser]
Why not extend the Controller add your auth code to it then when you write new Controllers extend your auth contoller.

one Controller extended to all your other Contollers.

Enjoy
InsiteFX

[eluser]jedd[/eluser]
As InsiteFX suggested, MY_Controller is an ideal place - though I wouldn't have that purely for admin-related controllers to extend from - instead I'd have a private function there that was told which controllers are auth-requiring, and which are not.

[quote author="koskoz" date="1249046820"]
But I don't know where to put this piece of code.

For the moment, as I'm using a template library, I've got one global view for the admin so I put my session check in it, but it's not the aim of a view.
[/quote]

You are correct that the view is now the right place to get access to the controller (but yes, to being the right place to control 'display logic').

I could do my session check in every admin controller but it's really boring, and if I want to change something I have to do it in every class.

Boring?

How big is this 'little piece of code'?

My auth checks in *every* controller that I care controlling access to are simply this for a normal logged-in-user requirement:

Code:

// Kick out anyone who isn't authenticated, for starters
$this->_ensure_authenticated_user( "Forum" );

.. or this, for admin requirement:

Code:

// Kick out anyone who isn't admin
if (! $this->_is_admin())
        redirect('/people/login');

And I could probably get that first one down to one line by assuming that the reader could glean the meaning of a function called _ensure_authenticated_user without my help.

[eluser]koskoz[/eluser]
Yeah but you add it to each controller that need it right ?

So if you want to change something in your little piece of code you need to change it in every controller that use it.

So I tried to write another controller, like this :

Code:

class My_Controller extends Controller
{

    function __constructor()
    {
        parent::Controller();

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

}

And every admin controller extends this one.
But it's not working.
Do I need to do something else in my admin controller ?

[eluser]jedd[/eluser]
[quote author="koskoz" date="1249058456"]Yeah but you add it to each controller that need it right ?

So if you want to change something in your little piece of code you need to change it in every controller that use it.[/quote]

I have a 1 or 2 line piece of code in my controllers, yes - but that code never changes. It simply calls a function that will come back if the user is logged in / is admin - or it redirects off to wherever it needs to go if they are not admin / not logged in.

So .. my Admin controller has this in its constructor:

Code:

if (! $this->_is_admin())
    redirect('/people/login');

In MY_Controller I have a couple of auth functions - _is_admin and the one I mentioned before, _ensure_authenticated_user. My _is_admin function looks like this:

Code:

function  _is_admin ( )  {
    return ($this->session->userdata('admin'))  ?  TRUE  :  FALSE;
    }

The session data is registered at login (authentication) time. So I could just have checks of the user data whenever I want to check if it's an admin, but having this function being in-directed through MY_Controller like this means I can, relatively easily, extend this to more complex admin-style (are they a moderator, a vice-admin, etc) checks in the future.

But any future change will always involve code changes .. otherwise it wouldn't be a change. My point is that there's always a trade-off between complexity now versus complexity later.


With the problem you have cited with your code, are you setting userdata('logged_in') when the user is authenticated?

I wouldn't have it in the constructor of my MY_Controller - I'd separate it out (as I've done above) into a private function and then call it in each controller. That way you can have more control over which of your controllers need which level of authentication.

[eluser]koskoz[/eluser]
For the moment I've got that in my My_Controller :

Code:

class Auth_Admin extends Controller
{

    function __construct()
    {
        parent::Controller();

        $logged = $this->session->userdata('logged_in');
        if ( $logged != TRUE) {
            redirect(site_url().'/admin');
        }
    }

}

If I've right understood, you suggest that I do something like this instead ?

Code:

class Auth_Admin extends Controller
{

    function __construct()
    {
        parent::Controller();
        $this->check_auth();
    }
    
    function check_auth()
    {
        $logged = $this->session->userdata('logged_in');
        if ( $logged != TRUE) {
            redirect(site_url().'/admin');
        }        
    }

}

Or I remove the $this->check_auth(); from the constructor and I put it in each admin controller ?

[eluser]jedd[/eluser]
[quote author="koskoz" date="1249070146"]
If I've right understood, you suggest that I do something like this instead ?

Code:

class Auth_Admin extends Controller
{

    function __construct()
    {
        parent::Controller();
        $this->check_auth();
    }
    
    function check_auth()
    {
        $logged = $this->session->userdata('logged_in');
        if ( $logged != TRUE) {
            redirect(site_url().'/admin');
        }        
    }

}

Or I remove the $this->check_auth(); from the constructor and I put it in each admin controller ?[/quote]

Yes - that second way I think will work better for you.

Note #1 : if you name a function starting with a _ - it becomes a private function, so a user can't call it. I recommend doing this with this kind of function (or indeed, any function you never want users to know about).

Note #2 : a redirect() does not need the site_url - so you can just do this:

Code:

function check_auth()  {
    if  ( ($this->session->userdata('logged_in'))  != TRUE )
        redirect('/admin');
    }

Note #3 : usually I wouldn't have a 'logged_in' flag set - I'd just rely on login_name (or whatever wording you prefer) being set to prove that the user had authenticated as a (at least) normal user. For admin stuff, as you are doing, then yes (of course) you'll need a specific flag there.

[eluser]koskoz[/eluser]
Ok, thanks for all these advises.
If I use the "private" keyword before the method's name, do I still need the underscore ?

[eluser]jedd[/eluser]
At the end of the day, the [url="http://ellislab.com/codeigniter/user-guide/general/controllers.html#private"]CI User Guide[/url] is always right.

There may be other values of right, but that's neither here nor there. If you want your code to work sensibly with future releases of CI, and to make sense to other CI users that may follow you .. well, that's entirely up to you.

[eluser]koskoz[/eluser]
Yes but that don't tell if we can or not add the keyword "private" before the method's name.