PHP reservation? more secure or unnecessary
I am using the MVC model (I think it is called) and I have split my site into smaller pages and included ... Is it safer / better or worse (no benefit) for checking the same conditional twice?
For example, I have an accounts page that looks something like this:
// Must be logged in
if(isset($_SESSION['userID'])){
include('edit_user.php');
}
and then in my page edit_user.php
I have something like this:
// Must be logged in
if(isset($_SESSION['userID'])){
if(isset($_POST['editUser'])){
//Validate the form
}
?>
<form>
// Display the form
</form>
<?php
} // End main IF
In general, I check if the user ID is set to two ... I still do the same with all of my pages (which require users to be logged in). Is it really necessary? My initial thought was to prevent unregistered users from directly accessing the form edit_user.php
and doing something (I also thought of just redirecting if users access the page directly). What do you guys think or suggest?
Edit
I don't think I have explained myself too clearly ... It was just an example ... Here is a better example to better understand the reasons for my question:
... Account page
if(isset$_SESSION['userID'])){
include('edit_user.php');// edit user form
include('change_password.php');// change password form
include('change_pic.php');// change photo form
}
and from within each of my inclusions, I ask again SESSION['userID']
... So what are you suggesting now?
Well, that's overkill, which violates the Don't Repeat Yourself (DRY) design principle. If your file is edit_user.php
publicly available, you definitely need the checks there, so you could remove the other checks if you are confident in the functionality.
It could be argued that your code is cleaner with in-place checks, however in the long run redundancy as this will lead to more maintenance problems.
You don't need to have multiple checks. If all requests go through the controller, you only need to add validation to the controller. This is the point of the controller to route the request. The view displays data. The model interacts with the database (and applies business logic).
In many cases, redundancy is beneficial, but in your example it is not necessary. This also goes against the DRY (Do not Repeat Yourself) principle. The more you repeat the same code, the more time you waste (you also face the possibility of adding bugs to your code due to repetition). You should be fine with validation in edit_user.php only.
Since you are using MVC, here's what I suggest: Define some of the authentication features available globally. Then, in your controller's constructor methods, use them to see if the user should be able to access that section. If they are not logged in, you can redirect them to another page or display an error message, for example. Of course, you can use a finer methodology and place calls to your authentication functions at the beginning of certain controller methods.
You should always check for input that comes in outside of your control, so every php script that the user can access should check.
Even if you don't expect it, if a user can see a link in the URL to a page that is 10 forms deep in your site, then they can still go directly to it.
Joomla uses a nice technique, which is to put this right at the top of every PHP file:
defined('_JEXEC') or die('Restricted access');
_JEXEC
is defined at the main entry point. You can do something like:
if ( isset($_SESSION['userID']) ) {
define('LOGGED_IN', 1);
include('edit_user.php');
}
With this in edit_user.php and other files:
defined('LOGGED_IN') or die('You must be logged in.');