Login script using cookies

philweb · Oct 4, 2006

Hi

I am trying to adapt a script that I found, and I am having a bit of trouble. What I want to do is simply to set the cookie for the login and then redirect the person to the logged in page. Here is the script I am using, can anyone see what might be wrong here.

Thanks,

Philweb

if (isset($_POST['submit'])) { // Handle the form.

require_once ('./mysql_connect.php'); // Connect to the db.

// Create a function for escaping the data.
function escape_data ($data) {
	global $dbc; // Need the connection.
	if (ini_get('magic_quotes_gpc')) {
		$data = stripslashes($data);
	}
	return mysql_real_escape_string($data, $dbc);
} // End of function.

$message = NULL; // Create an empty new variable.

// Check for a username.


if ($u && $p) { // If everything's OK.

	// Retrieve the user_id and first_name for that username/password combination.
	$query = "SELECT user_id, first_name FROM employees WHERE first_name='$fn' AND email='$e'";		
	$result = @mysql_query ($query); // Run the query.
	$row = mysql_fetch_array ($result, MYSQL_NUM); // Return a record, if applicable.

	if ($row) { // A record was pulled from the database.

			// Set the cookies & redirect.
			setcookie ('first_name', $row[1]);
			setcookie ('user_id', $row[0]);
			header ("Location:  http://" . $_SERVER['HTTP_HOST'] . dirname($_SERVER['PHP_SELF']) . "/login.php");
			exit(); // Quit the script.

	} else { // No record matched the query.
		$message = '<p>The first name and email entered do not match those on file.</p>'; 
	}

	mysql_close(); // Close the database connection.

} 

} // End of the main Submit conditional.

// Set the page title and include the HTML header.
$page_title = 'Login';


// Print the error message if there is one.
if (isset($message)) {
	echo '<font color="red">', $message, '</font>';
}

<form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
Enter your information in the form below:

<p><b>First Name:</b> <input type="text" name="username" size="10" maxlength="20" value="<?php if (isset($POST['first_name'])) echo $POST['first_name']; ?>" /></p>

<p><b>Email:</b> <input type="text" name="password" size="50" maxlength="100" />
</p>

</form

lozza1978 · Oct 4, 2006

if i were you I would throw some error reporting in there to debug the script.
have a look here http://uk.php.net/error-reporting

Also your using variables without checking if there set, this will throw loads of errors up on your script, you should really only use a variable if its set. other wise, by calling a var that is not set php will throw you an error.

just my 2 cents...

im here to help

lozz

Pehape · Oct 5, 2006

how to use error_reporting..?
should we write it once in the script or write it on every command ?
would be great with a code example.. thank you

lozza1978 · Oct 5, 2006

All you need to do is put this line at the top of each script to catch errors and debug the script..

error_reporting(E_ERROR | E_WARNING | E_PARSE | E_NOTICE);

bpat1434 · Oct 5, 2006

While you are correct lozza, there is a better way. Rather than just outputting possibly sensitive information to everyone, you can catch the errors during execution.

Pehape: You dont' catch any mySQL errors even though your query may return nothing, or error. When that happens, your script will severly malfunction. I'd suggest adding a few or die(); statements to your queries to make sure that there aren't any query issues. These can later be "upgraded" to a less obtrusive error catching.

Also, I'm not sure, but take a look at this code:

header ("Location:  http://" . $_SERVER['HTTP_HOST'] . dirname($_SERVER['PHP_SELF']) . "/login.php");

Are you sure that once they're logged in, you want them to be redirected to the login page? Perhaps you should redirect them to the logged in page.

Rodney_H_ · Oct 6, 2006

This is a bit over-the-top, but it might help...

<?php
// function to make the form:
function makeForm(){

return '<form action="'.$_SERVER['PHP_SELF'].'" method="post">
<p><input type="text" name="fname" size="50" /> First name</p>
<p><input type="text" name="email" size="50" /> Email</p>
<p><input type="submit" name="submit" value="Login" /></p>
</form>';
}


// if the form has NOT been submitted:
if(!isset($_POST['submit']))
{
	echo makeForm();
}

// else process the form:
else
{
	$required = array('first name'=>$_POST['fname'], 'email'=>$_POST['email']);

foreach($required as $key=>$value)
{
	if(!isset($value) || trim($value) == '')
	{
		$errors[] = 'The '.$key.' was left empty';
	}
}

// if there are any errors:
if(isset($errors))
{
	echo '<h2>Error!</h2>
	<p>The following error occurred:</p>
	<ul>';
	foreach($errors as $value)
	{
		echo '<li>'.$value.'</li>';
	}
	echo '</ul>
	<p>Please try again</p>';
	echo makeForm();
}

// else no errors:
else
{
	// if magic quotes, strip slashes:
	if(get_magic_quotes_gpc() == 1)
	{
		$_POST['fname'] = trim(stripslashes($_POST['fname']));
		$_POST['email'] = trim(stripslashes($_POST['email']));
	}

	require_once ('./mysql_connect.php');

	$query = "SELECT `user_id`, `first_name`
			  FROM `employees`
			  WHERE `first_name` = '".
			  	mysql_real_escape_string($_POST['fname'])."'
			  AND `email` = '".
			  	mysql_real_escape_string($_POST['email'])."'
			  LIMIT 1";    

	$result = @mysql_query($query) or die('Could not execute query.');

	if(mysql_num_rows($result) == 0)
	{
		echo '<h2>Error!</h2>
		<p>The first name and email entered do not match those on file.</p>
		<p>Please try again.</p>';
		echo makeForm();
	}

	else
	{
		$row = mysql_fetch_array($result, MYSQL_ASSOC);

		// Set the cookies & redirect.
        setcookie ('first_name', $row['first_name'], time()+60*60*24*30);
        setcookie ('user_id', $row['user_id'], time()+60*60*24*30);

		header ("Location:  http://www.YourDomain.com/Success.php");
		exit();
	}
	mysql_close();
}
}
?>

(edited per BG's point re setting time for cookie)

bradgrafelman · Oct 6, 2006

Am I being really thick here, or has the problem actually been stated? We've all critiqued the code snippet, but what's the actual problem with this code?

Couple general things I noticed that I'll just throw out here:

Your [man]setcookie/man calls don't have expiration times, meaning that as soon as the browser is closed the cookies should be destroyed.
Your [man]setcookie/man calls don't have any path arguements, so they will only be valid in the directory that you've set them in (including sub-directories, e.g. if the login page is at /script/login.php then you can NOT access the cookie in /loggedin.php).

Rodney_H_ · Oct 6, 2006

Well, I was noticing the fact that he is defining a function (escape_data) but never calls it, he uses variables ($fn, $e) but never tells us from whence they came, and the variables that are used and checked in the query are not related to the form input names ("username" and "password"). Those were issues that I tried to address...

It felt like he was copying and pasting two or three code snippets together into a new script but there were/are many dis-connects... of which one is setcookie() function calls as you pointed out, Brad...

Pehape · Oct 6, 2006

i want to ask bout code wrote by Mr.Rodney.H

if(get_magic_quotes_gpc() == 1)
{
$POST['fname'] = trim(stripslashes($POST['fname']));
$POST['email'] = trim(stripslashes($POST['email']));
}

    require_once ('./mysql_connect.php'); 

    $query = "SELECT `user_id`, `first_name` 
              FROM `employees` 
              WHERE `first_name` = '". 
                  [B]mysql_real_escape_string($_POST['fname'])."' [/B]                   AND `email` = '". 
                  [B]mysql_real_escape_string($_POST['email'])."' [/B]                   LIMIT 1";

magic_quote_gpcis on ( automatically addslashes ), why you have to strip the slashes before trim it? and then apply the slashes again using mysql_real_escape_string ..?
if my magic_quote_gpc is on, we don't need to worry bout sql_injection,do we ?

bpat1434 · Oct 6, 2006

if my magic_quote_gpc is on, we don't need to worry bout sql_injection,do we ?

Common misconception about security. That's why Magic quotes is gone in PHP 6 (just check the CVS snapshot).

You remove the slashes because they are characters, and you want to trim the original string.

You then trim the string to the appropriate length.

THen you let mySQL tell PHP which characters to slash (well, PHP uses the mySQL special chars to determine what to escape)

Much safer and much less likely to be susceptible to SQL injection.

But you still haven't stated your problem

Login script using cookies

Pphilweb

Llozza1978

PPehape

Llozza1978

bpat1434

RRodney_H_

Bbradgrafelman

RRodney_H_

PPehape

bpat1434