My first login script

[sKunKbad]

So, in all of the stuff I've done with php, I've never tried to write my own login script, and I'm wondering if what I've got is safe or if anyone has any suggestions:

 

// auto strip slashes and escape for db query
function clean_for_query($data){
global $db;
if(get_magic_quotes_gpc()){ // strip slashes by letting mysql_real_escape_string do the work
	$data = stripslashes($data);
}
$data = mysqli_real_escape_string($db , trim($data));
return $data;
}

// auto clean and extract POST vars
foreach($_POST as $post_key => $post_value){
if($post_key == 'email'){
	$post_email = filter_input(INPUT_POST, 'email' , FILTER_VALIDATE_EMAIL);
}else{
	${"post_$post_key"} = filter_input(INPUT_POST, "$post_key" , FILTER_SANITIZE_STRING);
}
}


$user_is_logged_in = 'false';
$login_error = 0;
if(isset($post_submit_login) && $post_submit_login == 'submit'){
if ($post_login_username == FALSE || $post_login_username == NULL || $post_login_password == FALSE || $post_login_password == NULL){
	$login_error = 1;
	$user_is_logged_in = 'false';
}
if($login_error != 1){
	$clean_login_username = clean_for_query($post_login_username);
	$clean_login_password = clean_for_query($post_login_password);
	$super_password = md5(PASSWORD_SALT . $clean_login_password);
	$a = mysqli_query($db , "SELECT * FROM `users` WHERE username = '$clean_login_username' AND password = '$super_password'");
	$b = mysqli_num_rows($a);
	if($b == 1){
		$row = mysqli_fetch_assoc($a);
		$_SESSION['user_id'] = $row['user_id'];
		$fingerprint = $row['timestamp'];
		session_regenerate_id();
		$_SESSION['user_token'] = md5($fingerprint . session_id() . $_SERVER['HTTP_USER_AGENT']);
		$user_is_logged_in = 'true';
	}else{
		$login_error = 1;
		$user_is_logged_in = 'false';
	}
	mysqli_free_result($a);
}
}
if(isset($_SESSION['user_id']) && isset($_SESSION['user_token'])){
$clean_user_id = clean_for_query($_SESSION['user_id']);
$c = mysqli_query($db , "SELECT * FROM `users` WHERE user_id = '$clean_user_id'");
$d = mysqli_num_rows($c);
if($d == 1){
	$row = mysqli_fetch_assoc($c);
	$fingerprint = $row['timestamp'];
	if($_SESSION['user_token'] = md5($fingerprint . session_id() . $_SERVER['HTTP_USER_AGENT'])){
		session_regenerate_id();
		$user_is_logged_in = 'true';
	}else{
		$login_error = 1;
		$user_is_logged_in = 'false';
	}
}
mysqli_free_result($c);
}

[JonnoTheDev]

Looks clean enough. Does it work?

[Monk3h]

mysqli?

[JonnoTheDev]

mysqli?

Mysqli is the improved mysql extension to make use of the new features of 4.1.3 and above. Part of php5. Whats the issue?

[sKunKbad]

Looks clean enough. Does it work?

 

Yes, it works well. The usage in a page that requires login is like this:

 

require_once LOGIN_STATUS_CHECK ;
if (!isset($user_is_logged_in) || $user_is_logged_in == 'false') { // $user_is_logged_in comes from USER_STATUS_CHECK
echo "data on this page is only accessible if logged in";
require_once "includes/login_form.inc.php";
}else{
     // user content
}

[JonnoTheDev]

Good job.

I would redirect the user to the login page using a header() if not logged in. You could write a function and call on all your secure pages:

 

function checkUserLogin() {

// user not logged in

header("Location:login.php");

exit();

}

[sKunKbad]

Good job.

I would redirect the user to the login page using a header() if not logged in. You could write a function and call on all your secure pages:

 

function checkUserLogin() {

// user not logged in

header("Location:login.php");

exit();

}

 

Is this for security? I only liked the way I did it because a user would be where they want to be once logged in.

[JonnoTheDev]

Do it however you feel is best for you.

I use a login check function on all pages that require the user to be logged in. If they try to access the page by typing the filename in through their web browser then they will be redirected to the login page.

[sKunKbad]

I'm working on making the script recognize if it is a user and/or admin logging in, and will try to refine the process of checking status. I've been reading about login security, and hope that what I have done here will be enough. I realize that by having an SSL it would be a lot more secure.

[Monk3h]

Wouldnt storing the login details in a cookie be more secure?

 

Id just make sure you exclude any characters that can be used to SQL inject, or print the details first.

 

I would also use sleep() to protect against brute force.

 

Md5 as an encrypt.

 

Just shooting ideas here.

[sKunKbad]

Wouldnt storing the login details in a cookie be more secure?

 

Id just make sure you exclude any characters that can be used to SQL inject, or print the details first.

 

I would also use sleep() to protect against brute force.

 

Md5 as an encrypt.

 

Just shooting ideas here.

 

If you take a second look at the code I posted in the original post, maybe it is not obvious, but all post vars are getting cleaned by filter_input() and mysqli_real_escape_string() before being inserted into a query. Is this not enough?

 

If more than one row is retrieved in a database result, then $user_is_logged_in = FALSE

 

sleep() may be a good idea, and I will consider this to help protect against brute force

 

md5 is already being used for both the password and the token

 

Thanks for your input. I think it's important to consider everything carefully.

[premiso]

 

If you take a second look at the code I posted in the original post, maybe it is not obvious, but all post vars are getting cleaned by filter_input() and mysqli_real_escape_string() before being inserted into a query. Is this not enough?

 

That should be plenty. No need to add extra checks when that should work as expected.

 

As for the burte force part, instead of sleep use session to keep track of # of entries and if it reaches X place a hold on the account for X amount of minutes letting the user know that the threshold has been reached and they need to wait before retrying.

 

 

Other than that I think it looks good and secure to me. Nice job on it.

[sKunKbad]

As for the burte force part, instead of sleep use session to keep track of # of entries and if it reaches X place a hold on the account for X amount of minutes letting the user know that the threshold has been reached and they need to wait before retrying.

 

I also think this is the better solution, but I am concerned that a true attacker would know to purge cookies (or not have them enabled at all). Login attempts could be stored in a database, but checking IP addresses is unreliable, and there really isn't anything else to check that really IDs a connection. I suppose ALL of this could be done, and it would make things tougher for an attacker, but I'm guessing that the old saying, "locks keep honest people honest" sort of applies to login security. I think I will store the login attempts in a cookie, and consider the script done.

[premiso]

Yea, how I intended it to be used is that in the user table you would have a column, loginhold.

 

If loginhold is set to a timestamp you check that time to see if enough time has passed, when that is entered the timestamp is set to X minutes of now's time. This way if time is equal to or greater than that stamp the user is allowed to try to login again.

 

I would also have another column with login attempts, so you can store that in the DB to, after the X one you reset it and set that loginhold. If the user logs in successfully you simply set login attempts to 0.

 

Anyhow, it can be done in the future and really is not a huge issue until you encounter it =)

 

Maybe you can implement that in the next version =P

[sKunKbad]

Yea, how I intended it to be used is that in the user table you would have a column, loginhold.

 

If loginhold is set to a timestamp you check that time to see if enough time has passed, when that is entered the timestamp is set to X minutes of now's time. This way if time is equal to or greater than that stamp the user is allowed to try to login again.

 

I would also have another column with login attempts, so you can store that in the DB to, after the X one you reset it and set that loginhold. If the user logs in successfully you simply set login attempts to 0.

 

Anyhow, it can be done in the future and really is not a huge issue until you encounter it =)

 

Maybe you can implement that in the next version =P

 

Well, I'm not sure if I did it exactly how you would have done it, but it works. I'm not sure if my database tables are as finely tuned as they could be, in terms of length and type. I'm going to include the whole script, minus the app_top.inc. What do you think now?

 

<?php
/*
This script is used to check if a person is a user or admin.
Cookies, must be enabled on the client's browser for this 
script to work. Another option would be to have the 
session ID appended to the URL by setting session.use_trans_sid
to true in the server's php.ini file.

Example usage for a user page:

	if (!isset($user_is_logged_in) || $user_is_logged_in == 'false') { 
		//show login form
	}else{
		// display user's content
		// the specific user is determined by the $clean_user_id variable
	}

Example usage for an admin page:

	if (!isset($admin_is_logged_in) || $admin_is_logged_in == 'false') {
		//show login form
	}else{
		// display admin's content
	}

SQL to get started:

	 CREATE TABLE `database_name`.`users` (
	`user_id` SMALLINT NOT NULL AUTO_INCREMENT ,
	`username` VARCHAR( 12 ) NOT NULL ,
	`password` VARCHAR( 32 ) NOT NULL ,
	`timestamp` VARCHAR( 26 ) NOT NULL ,
	`login_hold` INT NOT NULL DEFAULT '0',
	PRIMARY KEY ( `user_id` ) ,
	UNIQUE (
	`username`
	)
	) ENGINE = InnoDB;

	 CREATE TABLE `database_name`.`admins` (
	`admin_id` SMALLINT NOT NULL AUTO_INCREMENT ,
	`username` VARCHAR( 12 ) NOT NULL ,
	`password` VARCHAR( 32 ) NOT NULL ,
	`timestamp` VARCHAR( 26 ) NOT NULL ,
	`login_hold` INT NOT NULL DEFAULT '0',
	PRIMARY KEY ( `admin_id` ) ,
	UNIQUE (
	`username`
	)
	) ENGINE = InnoDB;

	 CREATE TABLE `database_name`.`user_login_attempts` (
	`user_id` SMALLINT NOT NULL ,
	`attempt_count` TINYINT NOT NULL DEFAULT '0',
	`last_attempt` INT NOT NULL
	) ENGINE = InnoDB;

	 CREATE TABLE `database_name`.`ip_login_attempts` (
	`IPaddr` VARCHAR( 16 ) NOT NULL ,
	`attempt_count` TINYINT( 2) NOT NULL ,
	`login_hold` INT NULL DEFAULT '0',
	`last_attempt` INT NOT NULL
	) ENGINE = InnoDB;

	 CREATE TABLE `database_name`.`admin_login_attempts` (
	`admin_id` SMALLINT NOT NULL ,
	`attempt_count` TINYINT NOT NULL DEFAULT '0',
	`last_attempt` INT NOT NULL
	) ENGINE = InnoDB;

Users and admins will still need to be able to log out.
Logging out usually requires that the session is unset,
and that the user or admin is redirected to somewhere.
*/
// auto strip slashes and escape for db query
function clean_for_query($data){
global $db;
if(get_magic_quotes_gpc()){ // strip slashes and let mysql_real_escape_string do the work
	$data = stripslashes($data);
}
$data = mysqli_real_escape_string($db , trim($data));
return $data;
}

// auto clean and extract POST vars
foreach($_POST as $post_key => $post_value){
if($post_key == 'email'){
	$post_email = filter_input(INPUT_POST, 'email' , FILTER_VALIDATE_EMAIL);
}else{
	${"post_$post_key"} = filter_input(INPUT_POST, "$post_key" , FILTER_SANITIZE_STRING);
}
}

// initialize some variables
$user_is_logged_in = 'false';
$login_error = 0;
$admin_is_logged_in = 'false';
$admin_login_error = 0;

// check if this is a login form submission
if(isset($post_submit_login) && $post_submit_login == 'submit'){

// If the form post did not come from the same page on the development or production server, according to the HTTP_REFERER value, then the form post is an attack
if (isset($_SERVER['HTTP_REFERER']) && !stristr($_SERVER['HTTP_REFERER'], $_SERVER["SERVER_NAME"] . $_SERVER["REQUEST_URI"] )){
	die();
}

// If the POST vars did not pass validation
if ($post_login_username == FALSE || $post_login_username == NULL || $post_login_password == FALSE || $post_login_password == NULL){
	$login_error = 1;
	$user_is_logged_in = 'false';
	$admin_login_error = 1;
	$admin_is_logged_in = 'false';
}

// A username or password can not be more than 12 characters, and if either posted values are larger than 12 characters, login fails
if( strlen( $post_login_username ) > MAX_USERNAME_LENGTH || strlen( $post_login_password ) > MAX_PASSWORD_LENGTH ){
	$login_error = 1;
	$user_is_logged_in = 'false';
	$admin_login_error = 1;
	$admin_is_logged_in = 'false';
}

// check if the remote IP address is on hold
$ip_on_hold = 0;
if( isset($_SERVER['REMOTE_ADDR']) && $_SERVER['REMOTE_ADDR'] != '' && $_SERVER['REMOTE_ADDR'] != NULL ){
	$clean_remote_ip = clean_for_query($_SERVER['REMOTE_ADDR']);
	$k = mysqli_query($db , "SELECT login_hold FROM `ip_login_attempts` WHERE IPaddr = '$clean_remote_ip' LIMIT 1");
	$l = mysqli_num_rows($k);
	if($l == 1){
		$row = mysqli_fetch_assoc($k);
		if ($row['login_hold'] > time() ){
			$user_is_logged_in = 'false';
			$login_error = 1;
			$ip_on_hold = 1;
		}
	}
	mysqli_free_result($k);
}

//  check if the user is on hold
$user_on_hold = 0;
$clean_login_username = clean_for_query($post_login_username);
$m = mysqli_query($db , "SELECT login_hold FROM `users` WHERE username = '$clean_login_username' LIMIT 1");
$n = mysqli_num_rows($m);
if($l == 1){
	$row = mysqli_fetch_assoc($m);
	if ($row['login_hold'] > time() ){
		$user_is_logged_in = 'false';
		$login_error = 1;
		$user_on_hold = 1;
	}
}
mysqli_free_result($m);

// check if the admin is on hold
$admin_on_hold = 0;
$clean_login_username = clean_for_query($post_login_username);
$m = mysqli_query($db , "SELECT login_hold FROM `admins` WHERE username = '$clean_login_username' LIMIT 1");
$n = mysqli_num_rows($m);
if($l == 1){
	$row = mysqli_fetch_assoc($m);
	if ($row['login_hold'] > time() ){
		$admin_is_logged_in = 'false';
		$admin_login_error = 1;
		$admin_on_hold = 1;
	}
}
mysqli_free_result($m);

// if there was no error with login form submission validation, check if the user's username and password match a database row
if($login_error != 1){
	$clean_login_username = clean_for_query($post_login_username);
	$clean_login_password = clean_for_query($post_login_password);
	$super_password = md5(PASSWORD_SALT . $clean_login_password);
	$a = mysqli_query($db , "SELECT * FROM `users` WHERE username = '$clean_login_username' AND password = '$super_password' LIMIT 1");
	$b = mysqli_num_rows($a);
	if($b == 1){ // if they match, then add some session keys, and $user_is_logged_in = 'true'
		$row = mysqli_fetch_assoc($a);
		$_SESSION['user_id'] = $row['user_id'];
		$fingerprint = $row['timestamp'];
		session_regenerate_id();
		$_SESSION['user_token'] = md5($fingerprint . session_id() . $_SERVER['HTTP_USER_AGENT']);
		$user_is_logged_in = 'true';
		mysqli_query($db , "UPDATE `users` SET login_hold = 0 WHERE user_id = '{$row['user_id']}' LIMIT 1");
		mysqli_query($db , "DELETE FROM `user_login_attempts` WHERE user_id = '{$row['user_id']}' LIMIT 1");
		if( isset($_SERVER['REMOTE_ADDR']) && $_SERVER['REMOTE_ADDR'] != '' && $_SERVER['REMOTE_ADDR'] != NULL ){ // clean up the ip_login_attempts log
			$clean_remote_ip = clean_for_query($_SERVER['REMOTE_ADDR']);
			mysqli_query($db , "DELETE FROM `ip_login_attempts` WHERE IPaddr = '$clean_remote_ip' LIMIT 1");
		}
	}else{ // if they don't match, then the form submission matched no user
		$login_error = 1;
		$user_is_logged_in = 'false';
	}
	mysqli_free_result($a);
}

// if there was no error with login form submission validation, check if the admin's username and password match a database row
if($admin_login_error != 1){ 
	$clean_login_username = clean_for_query($post_login_username);
	$clean_login_password = clean_for_query($post_login_password);
	$super_password = md5(PASSWORD_SALT . $clean_login_password);
	$a = mysqli_query($db , "SELECT * FROM `admins` WHERE username = '$clean_login_username' AND password = '$super_password' LIMIT 1");
	$b = mysqli_num_rows($a);
	if($b == 1){ // if they match, then add some session keys, and $admin_is_logged_in = 'true'
		$row = mysqli_fetch_assoc($a);
		$_SESSION['admin_id'] = $row['admin_id'];
		$fingerprint = $row['timestamp'];
		session_regenerate_id();
		$_SESSION['admin_token'] = md5($fingerprint . session_id() . $_SERVER['HTTP_USER_AGENT']);
		$admin_is_logged_in = 'true';
		mysqli_query($db , "UPDATE `admins` SET login_hold = 0 WHERE admin_id = '{$row['admin_id']}' LIMIT 1");
		mysqli_query($db , "DELETE FROM `admin_login_attempts` WHERE admin_id = '{$row['admin_id']}' LIMIT 1");
		if( isset($_SERVER['REMOTE_ADDR']) && $_SERVER['REMOTE_ADDR'] != '' && $_SERVER['REMOTE_ADDR'] != NULL ){ // clean up the ip_login_attempts log
			$clean_remote_ip = clean_for_query($_SERVER['REMOTE_ADDR']);
			mysqli_query($db , "DELETE FROM `ip_login_attempts` WHERE IPaddr = '$clean_remote_ip' LIMIT 1");
		}
	}else{ // if they don't match, then the form submission matched no admin
		$admin_login_error = 1;
		$admin_is_logged_in = 'false';
	}
	mysqli_free_result($a);
}

$try_ip = 0;
// If a login attempt was made with an actual user's username, but the password was wrong, log the attempt
if( $login_error == 1 ){ 
	$clean_login_username = clean_for_query($post_login_username);
	$e = mysqli_query($db , "SELECT user_id FROM users WHERE username ='$clean_login_username' LIMIT 1");
	$f = mysqli_num_rows($e);
	if ( $f == 1 ){ // If the post contains a real username
		$row = mysqli_fetch_assoc($e);
		$failed_user_id = $row['user_id'];
		mysqli_free_result($e);
		$i = mysqli_query($db , "SELECT attempt_count FROM user_login_attempts WHERE user_id ='$failed_user_id' LIMIT 1");
		$j = mysqli_num_rows($i);
		if ( $j == 1 ){ // If the user already has a failed login attempt
			$row = mysqli_fetch_assoc($i);
			$previous_attempts = $row['attempt_count'];
			$current_attempts = $previous_attempts + 1;
			$hold_time = time() + ( LOGIN_RESTRICTION_MINS * 60);
			if ($current_attempts >= LOGIN_ATTEMPT_LIMIT) { // set or reset the login hold if the user has LOGIN_ATTEMPT_LIMIT failed login attempts or greater
				mysqli_query($db, "UPDATE `users` SET login_hold = '$hold_time' WHERE user_id = '$failed_user_id'");
			}
			mysqli_query($db, "UPDATE `user_login_attempts` SET attempt_count = '$current_attempts' , last_attempt = " . time() . " WHERE user_id = '$failed_user_id'");
		}else{ // Else create a database row to start loggin failed login attempts
			mysqli_query($db, "INSERT INTO `user_login_attempts` (user_id,attempt_count,last_attempt) VALUES ('$failed_user_id',1," . time() . ")");
		}
		mysqli_free_result($i);
	}else{
		$try_ip = 1;
	}
}

// If a login attempt was made with an actual admin's username, but the password was wrong, log the attempt
if( $admin_login_error == 1 ){ 
	$clean_login_username = clean_for_query($post_login_username);
	$e = mysqli_query($db , "SELECT admin_id FROM admins WHERE username ='$clean_login_username' LIMIT 1");
	$f = mysqli_num_rows($e);
	if ( $f == 1 ){ // If the post contains a real admin's username
		$row = mysqli_fetch_assoc($e);
		$failed_admin_id = $row['admin_id'];
		mysqli_free_result($e);
		$i = mysqli_query($db , "SELECT attempt_count FROM admin_login_attempts WHERE admin_id ='$failed_admin_id' LIMIT 1");
		$j = mysqli_num_rows($i);
		if ( $j == 1 ){ // If the admin already has a failed login attempt
			$row = mysqli_fetch_assoc($i);
			$previous_attempts = $row['attempt_count'];
			$current_attempts = $previous_attempts + 1;
			$hold_time = time() + ( LOGIN_RESTRICTION_MINS * 60);
			if ($current_attempts >= LOGIN_ATTEMPT_LIMIT) { // set or reset the login hold if the admin has LOGIN_ATTEMPT_LIMIT failed login attempts or greater
				mysqli_query($db, "UPDATE `admins` SET login_hold = '$hold_time' WHERE admin_id = '$failed_admin_id'");
			}
			mysqli_query($db, "UPDATE `admin_login_attempts` SET attempt_count = '$current_attempts' , last_attempt = " . time() . " WHERE admin_id = '$failed_admin_id'");
		}else{ // Else create a database row to start loggin failed login attempts
			mysqli_query($db, "INSERT INTO `admin_login_attempts` (admin_id,attempt_count,last_attempt) VALUES ('$failed_admin_id',1," . time() . ")");
		}
		mysqli_free_result($i);
	}else{
		$try_ip += $try_ip;
	}
}

// If a login attempt was made with no match for user's username or admin's username, log the attempt by IP address (if possible)
if($try_ip == 2){ 
		if(isset($_SERVER['REMOTE_ADDR']) && $_SERVER['REMOTE_ADDR'] != '' && $_SERVER['REMOTE_ADDR'] != NULL){
			$clean_remote_ip = clean_for_query($_SERVER['REMOTE_ADDR']);
			$g = mysqli_query($db , "SELECT attempt_count FROM ip_login_attempts WHERE IPaddr = '$clean_remote_ip'");
			$h = mysqli_num_rows($g);
			if ( $h == 1 ){
				$row = mysqli_fetch_assoc($g);
				$previous_attempts = $row['attempt_count'];
				$current_attempts = $previous_attempts + 1;
				$hold_time = time() + ( LOGIN_RESTRICTION_MINS * 60);
				if ($current_attempts >= LOGIN_ATTEMPT_LIMIT) {
					mysqli_query($db, "UPDATE `ip_login_attempts` SET attempt_count = '$current_attempts', last_attempt = '" . time() . "', login_hold = '$hold_time' WHERE IPaddr = '$clean_remote_ip'");
				}else{
					mysqli_query($db, "UPDATE `ip_login_attempts` SET attempt_count = '$current_attempts', last_attempt = '" . time() . "' WHERE IPaddr = '$clean_remote_ip'");
				}
			}else{
				mysqli_query($db, "INSERT INTO `ip_login_attempts` (IPaddr,attempt_count,last_attempt) VALUES ( '$clean_remote_ip' , 1 , " . time() . ")");
			}
			mysqli_free_result($g);
		}
	}
}

// if the user or admin is not logging in through the form, we need to check who they are

// the session variables are used to see if the person is a user
if(isset($_SESSION['user_id']) && isset($_SESSION['user_token'])){
$clean_user_id = clean_for_query($_SESSION['user_id']);
$c = mysqli_query($db , "SELECT * FROM `users` WHERE user_id = '$clean_user_id' LIMIT 1");
$d = mysqli_num_rows($c);
if($d == 1){ // if there was a match for the user's ID, we proceed to check the token
	$row = mysqli_fetch_assoc($c);
	$fingerprint = $row['timestamp'];
	if($_SESSION['user_token'] = md5($fingerprint . session_id() . $_SERVER['HTTP_USER_AGENT'])){ // user's timestamp, session ID, and user agent must stay the same
		session_regenerate_id();
		$user_is_logged_in = 'true';
	}else{ // if user's timestamp, session ID, and user agent are not the same, they will have to log in again
		$login_error = 1;
		$user_is_logged_in = 'false';
	}
}
mysqli_free_result($c);
}

// the session variables are used to see if the person is an admin
if(isset($_SESSION['admin_id']) && isset($_SESSION['admin_token'])){
$clean_admin_id = clean_for_query($_SESSION['admin_id']);
$c = mysqli_query($db , "SELECT * FROM `admins` WHERE admin_id = '$clean_admin_id' LIMIT 1");
$d = mysqli_num_rows($c);
if($d == 1){ // if there was a match for the admin's ID, we proceed to check the token
	$row = mysqli_fetch_assoc($c);
	$fingerprint = $row['timestamp'];
	if($_SESSION['admin_token'] = md5($fingerprint . session_id() . $_SERVER['HTTP_USER_AGENT'])){ // admin's timestamp, session ID, and user agent must stay the same
		session_regenerate_id();
		$admin_is_logged_in = 'true';
	}else{ // if admin's timestamp, session ID, and user agent are not the same, they will have to log in again
		$admin_login_error = 1;
		$admin_is_logged_in = 'false';
	}
}
mysqli_free_result($c);
}

// If max logins are preventing a user / admin / attacker from logging in, show this message
if ((isset($user_on_hold) && $user_on_hold == 1) || (isset($admin_on_hold) && $admin_on_hold == 1) || (isset($ip_on_hold) && $ip_on_hold == 1)){
echo "You have exceeded the login attempts allowed and must wait " . LOGIN_RESTRICTION_MINS . " minutes before trying again. Please come back later and try again.";
}
?>

 

I also made a little cron for cleanup:

 

<?php
/*
This script is used to clean up the old
login attempts in the database.

*/
require_once("app_top.inc.php");
$old_hold = time() - ( LOGIN_RESTRICTION_MINS * 60);
echo $old_hold;
mysqli_query($db , "DELETE FROM `user_login_attempts` WHERE last_attempt < '$old_hold'");
mysqli_query($db , "DELETE FROM `admin_login_attempts` WHERE last_attempt < '$old_hold'");
mysqli_query($db , "DELETE FROM `ip_login_attempts` WHERE last_attempt < '$old_hold'");
?>

 

 

[premiso]

Yea a little different than I would have done.

 

For example, I would have kept the admins in the same table as users, just have a level or maybe isadmin field in the database. This prevents multiple queries and saves some hassels for a few key users who are deemed as admins.

 

For the user table SQL, I would use this:

CREATE TABLE `database_name`.`users` (
      `user_id` SMALLINT NOT NULL AUTO_INCREMENT ,
      `username` VARCHAR( 12 ) NOT NULL ,
      `password` VARCHAR( 32 ) NOT NULL ,
      `timestamp` VARCHAR( 26 ) NOT NULL ,
      `admin` TINYINT NOT NULL DEFAULT '0',
      PRIMARY KEY ( `user_id` ) ,
      UNIQUE (
      `username`
      )
      ) ENGINE = InnoDB;

 

Since the tables are identical, no need to have 2 separate. Then you can easily set, isAdmin or something similar to signify they are an admin or not.

 

As for the code I have yet to look closely at it, but from an overview it seems pretty good, as with all code it can use some cleanup, but it looks good to me. Anyhow nice job, if you need anymore help/advice don't hesitate to ask.

[sKunKbad]

Since the tables are identical, no need to have 2 separate. Then you can easily set, isAdmin or something similar to signify they are an admin or not.

 

I had read that having a separate table for admins was a good thing for security. This was my only reason for doing this. I really don't know if it matters, and it would obviously be way easier to code up this script if I had a column that designated an admin as an admin.

 

I've never had one of my sites hacked, but I fear this, and try to code with security in mind. I suppose if some hacker had the ability to get at my database of users, then they would easily be able to see the admin database too.

 

When considering security, there are opinions, and a lot to consider. I've spent a lot of time on this script, and I'm willing to spend more time on it, but at some point I just have to be done!

[premiso]

Yea, I can sort have see it. But honestly if they can access the admin part of the table script than they can even access the admin table. Its all about SQL injection.

 

I have never separated the admins/mods/users in any of my script and have yet to be hacked.

 

The key is to make sure you cannot be SQL Injected, thats when the bad stuff comes. As long as the code does not allow for that I fail to see how a hacker can get into your site without having the right credentials.

 

At least that is my 2cents, I think it is just extra work and not more secure.

[corbin]

Yea a little different than I would have done.

 

For example, I would have kept the admins in the same table as users, just have a level or maybe isadmin field in the database. This prevents multiple queries and saves some hassels for a few key users who are deemed as admins.

 

For the user table SQL, I would use this:

CREATE TABLE `database_name`.`users` (
      `user_id` SMALLINT NOT NULL AUTO_INCREMENT ,
      `username` VARCHAR( 12 ) NOT NULL ,
      `password` VARCHAR( 32 ) NOT NULL ,
      `timestamp` VARCHAR( 26 ) NOT NULL ,
      `admin` TINYINT NOT NULL DEFAULT '0',
      PRIMARY KEY ( `user_id` ) ,
      UNIQUE (
      `username`
      )
      ) ENGINE = InnoDB;

 

Since the tables are identical, no need to have 2 separate. Then you can easily set, isAdmin or something similar to signify they are an admin or not.

 

As for the code I have yet to look closely at it, but from an overview it seems pretty good, as with all code it can use some cleanup, but it looks good to me. Anyhow nice job, if you need anymore help/advice don't hesitate to ask.

 

 

This is kind of offtopic, but this caught my eye.

 

 

 

Why a varchar field for a date?

 

Have you ever benchmarked selecting rows between certain times based off of a varchar column when you have a couple thousand rows?  It would be slow as can be.

 

 

I would stick with either a datetime column or an int column with a unix timestamp.

[sKunKbad]

This is kind of offtopic, but this caught my eye.

 

 

 

Why a varchar field for a date?

 

Have you ever benchmarked selecting rows between certain times based off of a varchar column when you have a couple thousand rows?  It would be slow as can be.

 

 

I would stick with either a datetime column or an int column with a unix timestamp.

 

I had it varchar because I was storing the date formatted as date('c'), however, I switched now to time(), and made it int(10). I had taken a look at DATETIME, which is the same as date('c'), but it auto updates, which isn't really what I wanted for this field. It might not even matter, because this date only serves to be a user type salt in $_SESSION['token'].

 

While I was at it, I changed some of the other column types too.

 

Thanks for your input.