The purpose of the MVC pattern is to achieve separation of domain logic from the user interface, following the principle of Separation of Concerns:
In computer science, separation of concerns (SoC) is the process of separating a computer program into distinct features that overlap in functionality as little as possible. A concern is any piece of interest or focus in a program. Typically, concerns are synonymous with features or behaviors. Progress towards SoC is traditionally achieved through modularity of programming and encapsulation (or "transparency" of operation), with the help of information hiding. Layered designs in information systems are also often based on separation of concerns (e.g., presentation layer, business logic layer, data access layer, database layer).
While this is great and it's beautifully taken care of by CodeIgniter, it's not enough. The same principles apply to the individual components of the MVC triad (Model, View and Controller). Concerns should be as separate as possible, it's up to you to identify what those concerns are for your application, but if you put everything in one controller and one model you are probably doing it wrong.
The MVC pattern dictates that the controller's only job should be:
The controller receives user input and initiates a response by making calls on model objects. A controller accepts input from the user and instructs the model and a view port to perform actions based on that input.
So it shouldn't be that fat. It's job is to accept input, validate it and pass it on to the model and / or the view. As an oversimplification on what the concerns might be:
- A controller should vaguely represent a page in your website,
- A model should represent a table in your database.
So a controller should be concerned for one page and a model for one table. If your website has other pages than home
, let's say a contact page then you should consider a page
controller. That way it will be a lot easier for you to mentally connect parts of your application with the actual code, and greatly help with maintenance. By separating code in smaller logical parts, your code will be more robust and easier to test as per the Single Responsibility principle:
In object-oriented programming, the single responsibility principle states that every object should have a single responsibility, and that responsibility should be entirely encapsulated by the class. All its services should be narrowly aligned with that responsibility.
The numbers you have in your question are a little much. I can't safely comment on whether what you are doing could be improved, you should post the actual code for peer-review at Code Review Stack Exchange.
There is a small performance issue as well. Everytime you use a class, the class is loaded fully. The overhead is minuscule, but worth noting.
Further reading:
As suggested by @melee in the comments you should look into CodeIgniter helpers, as a better place to put a lot of commonly used functionality. You can easily create your own helpers and have some of your functions in them and not in your controllers. And you should learn about hooks to easily extend the framework core.
Let's clarify a few things first:
PHP sessions, in their default form are cookie based, I have an overview of how they work in another answer. But since they are cookie based, to me that says that if you are going to base your authentication and authorization workflow around cookies, go with normal cookies for everything - and just use sessions for what they are good for: session data persistence.
The circular login pattern you noticed is exactly why you should delete cookies on logout. Also, as @Morons writes, when a user clicks log out, expects to actually log out. It doesn't cancel out the auto-login feature, but one should be able to log out at any time.
Now, my auto login-approach goes something like this:
Which of course is pretty basic, but what is really important is the whole "cookie is valid" check. When a user successfully logins with the "remember me" option checked, I generate an auto-login token: A sensibly unique random string, most probably a salted hash.
That's the only thing I store in the cookie, nothing else nothing more. There is no actual need to store anything that could possibly identify an individual user in a cookie, you should avoid storing stuff like usernames and emails (even hashed). In my database, I have a simple token
table that basically stores:
- A user id,
- The token,
- An expiration date
And my check involves just checking against this table. The expiration date may seem like an overkill, as you can expire the cookie whenever you want but: You should treat everything that comes along with the cookie as user input, unsafe and easily faked. That includes it's expiration. And of course at log out I don't bother killing the cookie, I just set a NOW()
as the expiration date. Easy as pie, and I keep most elements of the check server-side, where it feels a little bit more safe.
In a recent project, I went a step further and added the check at every page request instead of storing something in a session to indicate an authorized session. Some may argue that the call to the database will lead to performance issues, but my 'token' table has grown to about 10 million tokens1 and still I haven't noticed any actual issue. Keep in mind that sessions involve file system access, that's almost never faster than a simple select
.
Of course, any cookie that stores anything related to authentication and authorization should be encrypted. And I'd recommend you always enforce secure cookies, with setcookie()
is as easy as setting the secure
parameter to true:
Indicates that the cookie should only be transmitted over a secure HTTPS connection from the client. When set to TRUE, the cookie will only be set if a secure connection exists. On the server-side, it's on the programmer to send this kind of cookie only on secure connection (e.g. with respect to $_SERVER["HTTPS"]).
1 I don't delete expired tokens for a variety of reasons.
Best Answer
This is what I do too.
The simplicity of the code for checking authenticaton in the construct is more favorable to me (the construct is not too fat either).
I check the minimal permission required in the construct and individual permissions in the individual methods.
If all the method of the controller require same and only minimal permissions, i just put them in the construct.
One other form of the implementation is separating the controller for Authenticated users.
The backend controllers would extend 'Admin_Controller' which checks authentication in its construct, thereby restricting any operations it the user is not authenticated.
This makes the code more reusable and reliable.