In “The Clean Code Talks — Unit Testing”, Miško Hevery mentions that “as little work as possible should be done in constructors [to make classes more easily testable]’. It got me thinking about the way I have implemented my user authentication mechanism.
Having delved into MVC development through CodeIgniter, I designed my first web application to perform user authentication for protected resources in controllers’ constructors in cases where every public function in that controller requires the user to be authenticated.
For controllers with public methods having mixed authentication requirements, I would naturally move the authentication from the constructor to each method requiring authentication (though I don’t currently have a need for this).
I made this choice primarily
- to keep the controller tight, and
- to ensure that all resources in the controller are always covered.
As for code longevity and maintainability: given the application structure, I can’t foresee a situation in which one of the affected controllers would need a public method that didn’t require user authentication, but I can see this as a potential drawback in general with this implementation (i.e., requiring future refactoring).
Is this a good idea?
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.