Page 1 of 1

Do I need to add a token when generating Session?

Posted: Wed Oct 24, 2007 5:09 am
by phpwalker
I'm creating a login page and protected pages.

login.php

Code: Select all

<?php 
require_once "config.inc.php"; // include the database information 

if(isset($_POST['login'])){ 

$clean = array();
$mysql = array();

$clean['user'] = strip_tags($_POST['username']);
$clean['pw'] = $_POST['password'];

$salt = 'alenn';
$password_hash = md5($salt . md5($clean['pw'] . $salt));

$mysql['user'] = mysql_real_escape_string($clean['user']);


// formulate and execute the query
$cmd  = "SELECT * FROM inb_contact WHERE name='{$mysql['user']}' AND password='$password_hash'";

$sql = mysql_query($cmd) or die("username was incorrect. MySQL said".mysql_error()); // this checks to see if the username exists 

$result = mysql_fetch_array($sql); // puts the database information into an array 

// Present results based on validity.
	if (mysql_num_rows($sql) == 1) {
             
				session_start(); 
				$_SESSION['sessioname'] = $mysql['user']; 
				header("location: protected.php"); 

	} else {
				echo "You are not authorized!";
	}

}else{
	echo "Na Tink.";
}

?>

protected.php

Code: Select all

<?php

$authorize = 0;
session_start();

if(isset($_SESSION['sessioname'])){ 
                $authorize = 1;
}else{ 
// Your protected stuff goes here if you wish to echo the username echo $_SESSION[”sessioname”] 
	echo "YOU MUST BE LOGGED IN TO SEE THIS!"; 

if($authorize == 1){
	echo "welcome ".$_SESSION['sessioname']; 
                //show content here
}
}
In the protected.php, I've added $authorize variable to check if the user is the intended one. Is this neccessary or I've overdid something?

Can I use session to stored the $authorize value -> $_SESSION['authorize']; So that it will become...

login.php

Code: Select all

<?php 

//Is this suitable to start session here?
session_start(); 
$_SESSION['authorize'] = 0;

require_once "config.inc.php"; // include the database information 

if(isset($_POST['login'])){ 

$clean = array();
$mysql = array();

$clean['user'] = strip_tags($_POST['username']);
$clean['pw'] = $_POST['password'];

$salt = 'alenn';
$password_hash = md5($salt . md5($clean['pw'] . $salt));

$mysql['user'] = mysql_real_escape_string($clean['user']);


// formulate and execute the query
$cmd  = "SELECT * FROM inb_contact WHERE name='{$mysql['user']}' AND password='$password_hash'";

$sql = mysql_query($cmd) or die("username was incorrect. MySQL said".mysql_error()); // this checks to see if the username exists 

$result = mysql_fetch_array($sql); // puts the database information into an array 

// Present results based on validity.
	if (mysql_num_rows($sql) == 1) {
             
                                                                $_SESSION['authorize'] = 1; //set it to be true if successfully logged in
				$_SESSION['sessioname'] = $mysql['user']; 
				header("location: protected.php"); 

	} else {
				echo "You are not authorized!";
	}

}else{
	echo "Na Tink.";
}

?>
protected.php

Code: Select all

session_start();

if(isset($_SESSION['sessioname'])){ 
     if($_SESSION['authorize'] == 1){
	echo "welcome ".$_SESSION['sessioname']; 
                //show content here
      }
      else{
                echo "unauthorize user";
       }
}else{ 
	echo "YOU MUST BE LOGGED IN TO SEE THIS!"; 


}
Should I regenerate the session ID to make that more secure?

Thanks in advance for the helps..

Posted: Wed Oct 24, 2007 9:50 am
by Mordred
Should I regenerate the session ID to make that more secure?
Yes, every time a user's authorization status changes.

For your other question - yes it seems redundant, if there is data in $_SESSION which is only set when the user has authenticated successfully. I would add two things to this though - session timeout checks and forcing logout on wrong credentials. Optionally, you may implement IP checks on the session owner, but let the users choose if this protection should be active, some of them do not control the IPs of their requests.

Edit:

Code: Select all

$clean['user'] = htmlentities($_POST['username'], ENT_QUOTES, your_encoding_here); //although in your case you may strip_tags first if you so badly want it
...
$_SESSION['sessioname'] = $clean['user']; //not $mysql['user']
It also seems that ti should be called $html, not $clean.

header("location: protected.php"); should use a full URL (http://example.com/protected.php)