2

I have a few functions that manage the access levels of a certain controllers within the controller file.

I decided to take them out either to a library or to a helper.

Since they are mostly procedural I decided to use helpers.

Here is the original method in the controller file (others are access_level_member, access_level_moderator etc)

  function access_level_admin() {

    if (!$this->session->userdata('loggedin')) {
        $this->session->set_flashdata('warning', 'Not logged in.');
        redirect('/start_page', 'refresh');
    }
    if (!$this->session->userdata('admin')) {
        $this->session->set_flashdata('warning', 'Access Denied!');
        redirect('/start_page', 'refresh');
    }  

Here is it withing a helper.

function access_level_admin() {
    $CI =& get_instance();
    if (!$CI->session->userdata('loggedin')) {
        $CI->session->set_flashdata('warning', 'Not logged in.');
        redirect('/start_page', 'refresh');
    }
    if (!$CI->session->userdata('admin')) {
        $CI->session->set_flashdata('warning', 'Access Denied!');
        redirect('/start_page', 'refresh');
    }  

So my question is, is it a better idea to place it in a library or a helper and is it ok to use $CI =& get_instance(); within a helper.

Any better ideas or recommendations?

ignite1688846
  • 147
  • 3
  • 12
  • 2
    I would put them in a library (or a model), because they're not "helper functions" - which helps you doing something - but those functions are vital to your application - I'd place them in some authentication library, or authentication model. As for the use of get_instance() in an helper, it's quite mandatory as you don't have access to the CI superobject there (no $this available) – Damien Pirsy Dec 02 '12 at 08:50
  • @Damien Pirsy I respect the reasoning but, helpers are as crucial as a library in any application if your application depends on it. For example I build my forms with form_helper. For now, I see helpers as a place where you place functions that are more or less independent from each other and don't require an object to be utilized. However I might be wrong. Also thanks for the get_instance() clarification. – ignite1688846 Dec 02 '12 at 09:03
  • I agree with @Damien Pirsy. I would put them in some sort of `User` model. `$user->isAdmin()` is readable, simple and straight forward. – Maxime Morin Dec 02 '12 at 13:24
  • That sort of functionality belongs in a MY_Controller or even MY_Session if you ask me. It's not database functionality so I don't see it in a model personally and you want it called every time a page loads, I really don't get the comments that it belongs in a model to be honest. – Rick Calder Dec 02 '12 at 16:09
  • @Rick Calder Even if CodeIgniter suggests to use models as database interaction classes, I don't see them that way. I prefer to see models as holders of the business logic. In my opinion, a user is part of the website's business. That's also why I posted as a comment and not as an answer. It's an opinion and I don't think there's a definitive answer. (Beside the `$CI` part; which as been answered already.) Hopefully this helps you understand why I would create a model for it. :) – Maxime Morin Dec 02 '12 at 21:34
  • @MaximeMorin I definately agree that one should not be limited by the strict MVC definitions, especially since not counting the fact that you load a model, there are almost none of restrictions to use a model as a holders of buisiness logic. – ignite1688846 Dec 03 '12 at 19:02
  • https://stackoverflow.com/a/63914758/2943403 – mickmackusa Oct 12 '20 at 10:13

1 Answers1

3

I would place the logic in a parent controller and have your controllers extend it.

class Authenticated_Controller extends CI_Controller {
    public function __construct() {
        parent::__construct();
        if (!$this->session->userdata('loggedin')) {
            $this->session->set_flashdata('warning', 'Not logged in.');
            redirect('/start_page', 'refresh');
        }
}

class Admin_Controller extends Authenticated_Controller {
    public function __construct() {
        parent::__construct();
        if (!$this->session->userdata('admin')) {
            $this->session->set_flashdata('warning', 'Access Denied!');
            redirect('/start_page', 'refresh');
        }
}
Stanley
  • 5,057
  • 4
  • 34
  • 44
  • Thank you for answering. So I should extend Authenticated_Controller with my controllers? Also how will I then keep people for accessing that controller (the authenticated and admin etc)? Even though it is interesting I think it complicates things, I mean some controllers won't need the login part since that page you could for example see without any access restrictions. How evern I will look into the posibility more. – ignite1688846 Dec 03 '12 at 18:59
  • 1
    You can create a `Front_Controller` which extends `CI_Controller`. `Front_Controller` will not perform any authentication. As what @Damien Pirsy suggested you can put the authentication logic in a authentication library. `Authenticated_Controller` will call the authentication library. There may be cases where some functions in your controller require authentication but some are not. You can have that controller extends `Front_Controller` and call the authentication library only in those functions which require authentication. – Stanley Dec 04 '12 at 02:30
  • This is a good answer, and the exact method I use to authenticate for my admin pages. Authentication shouldn't be in a helper. – Jeemusu Dec 21 '12 at 09:00