I’ve got my first real PHP web application up and running. It is working as expected, but being my first time, I’m sure there is plenty of room for improvement.
Right now, there is one particular aspect that I’d like some Code Review love on: my session handling class.
I haven’t used any frameworks this time around, because I’m afraid I’d miss out on learning what is really going on and that I wouldn't appreciate what the framework is doing for me without doing it the ‘hard way’ first.
My app consists of about 8 pages that require the user to be logged in to view.
My goal with this was to create a chunk of code that I could plop onto the top of each page, that would take care of:
- starting the php session
- ensuring the user is logged in
- loading the user’s permissions
- if not logged in, send them to the login page
- after logging in, send them to the original destination page
- if they are already logged in when at the login page, bypass and go to the default page
The main areas I’m interested in getting feedback are:
- Are there any functional flaws I haven’t noticed yet?
- What problems am I going to run into doing it this way?
- Should this class be doing more?
- Should it be doing less?
- What aspects could be more efficient?
- Should I be passing my
$db
object to the$session
object?
I’m looking forward to getting feedback and learning from the wisdom of the community.
Thanks!
EDIT: I'm aware that passwords should never be stored in plain text. I'll be fixing that soon.
There are 3 different ‘types’ of pages:
- Login page - this is the only page accessible when a user is not logged in
- Logout page - logs the user out and redirects to login page
- Regular page - all other pages are this type. user must be logged in to view these pages (I’ll use dashboard.php as the example)
_classes.php
<?php
sessionHandler::$config['loginPage'] = "login.php";
sessionHandler::$config['logoutPage'] = "logout.php";
sessionHandler::$config['afterLoginPage'] = "dashboard.php";
sessionHandler::$config['afterLogoutPage'] = "login.php";
sessionHandler::$config['permissionsRefreshInterval'] = 300; // seconds
class sessionHandler
{
public static $config;
function __construct($special = NULL)
{
session_start();
if ($special == 'LOGOUT') {
// This is the logout page, clear the session and
// send the user to the afterLogout page
session_destroy(); // clear session files on server
$_SESSION = NULL; // clear session variable for this session
$this->sendToPage(sessionHandler::$config['afterLogoutPage']);
}
$userID = $_SESSION['userID'];
if ($special == 'LOGIN' && $userID) {
// user is already logged in, so bypass the login page
$this->sendToPage(sessionHandler::$config['afterLoginPage']);
}
if (!$userID && ($special != 'LOGIN')) {
// If there's no session (no $userID) AND this
// isn't the login page, send them to the login page
sessionHandler::sendToLoginPage();
}
$nextRefreshTime = sessionHandler::$config['permissionsRefreshInterval'] + $_SESSION['lastPermissionRefreshTime'];
if (time() > $nextRefreshTime) {
sessionHandler::loadUserPermissions($userID);
}
}
function sendToPage($page)
{
header("Location: $page");
}
function sendToLoginPage()
{
// redirect user to login page.
// pass along current page, so we can
// redirect them after a successful login
$sourcePage = basename($_SERVER['REQUEST_URI']); // #TODO: make this work with deeper urls example.com/path/file.php
if ($sourcePage == sessionHandler::$config['logoutPage']) {
// don't do this if they are coming from the logout page
$sourcePage = NULL;
} else {
$sourcePage = '?d=' . urlencode($sourcePage);
}
$loginPage = sessionHandler::$config['loginPage'] . $sourcePage;
$this->sendToPage($loginPage);
}
static function loadUserPermissions($userid)
{
if (!$userid) {
return FALSE;
} else {
$query = "SELECT userID, firstName, lastName, email, phone, permissions FROM users WHERE userID = ? AND active = 1;";
$user = $GLOBALS['db']->SQL_SINGLE($query, Array($userid));
if ($user) {
$_SESSION['userID'] = $user['userID'];
$_SESSION['displayName'] = $user['firstName'] . ' ' . $user['lastName'];
$_SESSION['userEmail'] = $user['email'];
$_SESSION['userPhone'] = $user['phone'];
// currently I'm just storing user permissions as a comma separated
// list in the DB. I'll make it more elegant in the future
$permissions = explode(",", $user['permissions']);
foreach ($permissions as $p) {
$_SESSION['permissions'][$p] = 1;
}
$_SESSION['lastPermissionRefreshTime'] = time();
return TRUE;
} else {
return FALSE;
}
}
}
function p_check($permission, $echo = NULL)
{
// if user has the named permission, then echo the supplied HTML code
if ($_SESSION['permissions'][$permission]) {
echo $echo;
return TRUE;
} else {
return FALSE;
}
}
static function login($username, $password)
{
// validates user credentials and loads up user properties
$user = $GLOBALS['db']->SQL_SINGLE("SELECT userID FROM users WHERE username = ? AND password = ? AND active = 1;", Array($username, $password));
if ($user) {
$_SESSION['sessionStartTime'] = time();
sessionHandler::loadUserPermissions($user['userID']);
return TRUE;
} else {
return FALSE;
}
}
}
login.php
<?PHP
require_once('_classes.php');
$db = new db_controller(); // use this globally for all DB work.
$session = new sessionHandler('LOGIN');
?>
<html>
<body>
<div>
<div>
<div>
<img src="img/logo.png">
</div>
<h3>Welcome to myApp</h3>
<p>Login in to see it in action!</p>
<form id="loginForm">
<input name="username">
<input type="password" name="password">
<button type="submit">Login</button>
</form>
</div>
</div>
<!-- scripts -->
<script src="js/jquery-2.1.1.js"></script>
<script src="js/bootstrap.min.js"></script>
<script>
$('#loginForm').submit(function (e) {
e.preventDefault();
$.ajax({
type: 'POST',
url: '_functions.php?function=login',
data: $('form').serialize(),
dataType: 'JSON',
success: function (data) {
if (data['success']) {
// determine if the user has been redirected from another
// page prior to getting to the login page, if so, send them
// back to it instead of the default page
var urlParams = {};window.location.search.replace(/[?&]+([^=&]+)=([^&]*)/gi, function (str, key, value) {urlParams[key] = value; });
var redirectPage = decodeURIComponent(urlParams['d']);
if (redirectPage == 'undefined') {
// if there is not redirect parameter on the URL,
// then send them to the default page
redirectPage = data['redirectPage'];
}
window.location.replace(redirectPage);
} else {
alert(data['error']);
}
}
});
});
</script>
</body>
</html>
logout.php
<?PHP
require_once('_classes.php');
$db = new db_controller();
$session = new sessionHandler('LOGOUT');
?>
dashboard.php
<?PHP
require_once('_classes.php');
$db = new db_controller(); // use this globally for all DB work.
$session = new sessionHandler();
?>
<html>
<body>
<div>
<nav role="navigation">
<div class="sidebar-collapse">
<ul>
<li><a href="#"><i class="fa fa-user"></i> <span>Home</span><span class="fa arrow"></span></a>
<ul class="nav nav-second-level collapse">
<li><a href="dashboard.php">My Dashboard</a></li>
<li><a href="record-multiple.php">Record Data</a></li>
</ul>
</li>
<?php $session->p_check('maintenance', '
<li>
<a href="#"><i class="fa fa-wrench"></i> <span class="nav-label">Maintenance</span><span class="fa arrow"></span></a>
<ul class="nav nav-second-level collapse">
<li><a href="maintenance.php?filter=overdue">Overdue <span>' . $statusCount['overdue'] . '</span></a></li>
<li><a href="maintenance.php?filter=due-soon">Due Soon <span>' . $statusCount['due soon'] . '</span></a></li>
<li><a href="maintenance.php?filter=good">Good <span>' . $statusCount['good'] . '</span></a></li>
<li><a href="maintenance.php?filter=no-history">No History <span>' . $statusCount['no history'] . '</span></a></li>
<li><a href="maintenance.php">All Equipment <span>' . $statusCount['all'] . '</span></a></li>
</ul>
</li>
'); ?>
<?php $session->p_check('equipment', '
<li>
<a href="#"><i class="fa fa-truck"></i> <span class="nav-label">Equipment</span><span class="fa arrow"></span></a>
<ul class="nav nav-second-level collapse">
<li><a href="equipment.php">Equipment List</a></li>
<li><a href="maintenance-plans.php">Maintenance Plans</a></li>
<li><a href="maintenance-log.php">Maintenance Log</a></li>
</ul>
</li>
'); ?>
<li><a href="#"><i class="fa fa-sign-out"></i> <span>Exit</span><span class="fa arrow"></span></a>
<ul class="nav nav-second-level collapse">
<li><a href="logout.php" id="logOut">Log Out</a></li>
</ul>
</li>
</ul>
</div>
</nav>
<div id="page-wrapper" class="gray-bg">
<div class="row wrapper wrapper-content">
<div class="col-lg-12">
<!-- Main Page Content......-->
<!-- Main Page Content......-->
<!-- Main Page Content......-->
<!-- Main Page Content......-->
<!-- Main Page Content......-->
</div>
</div>
</div>
</div>
</body>
</html>