Please help me revise this log in code.

shinichi_nguyen · January 13, 2010

I had it on another post but after the first piece of code posted, then it got changed, and changed again due to sugestions, then the post just get kind of messy and the problem is not yet solved. This is the latest code that I got now and it does not work. After I type in username and password and hit log in, it gets me back to the login form with blank username and password. Please help telling me what is wrong with it?

checklogin.php

<?php


if (!isset($_POST['myusername']) || !isset($_POST['mypassword'])) {
header("location:http://www.mysite.com/login.html");

} 
//check that the form fields are not empty, and redirect back to the login page if they are
elseif (empty($_POST['myusername']) || empty($_POST['mypassword'])) {
header( "location:http://www.mysite.com/login.html" );

												}
else{

$host="localhost"; // Host name 
$username="mod"; // Mysql username 
$password="modpass"; // Mysql password 
$db_name="mydbname"; // Database name 
$tbl_name="users"; // Table name 

// Connect to server and select databse.
mysql_connect("$host", "$username", "$password")or die("cannot connect"); 
mysql_select_db("$db_name")or die("cannot select DB");

// Define $myusername and $mypassword 
$myusername=$_POST['myusername']; 
$mypassword=$_POST['mypassword']; 

// To protect MySQL injection (more detail about MySQL injection)
$myusername = stripslashes($myusername);
$mypassword = stripslashes($mypassword);
$myusername = mysql_real_escape_string($myusername);
$mypassword = mysql_real_escape_string($mypassword);

$sql="SELECT * FROM $tbl_name WHERE username='$myusername' and password='$mypassword'";
$result=mysql_query($sql);

// Mysql_num_row is counting table row
$count=mysql_num_rows($result);
// If result matched $myusername and $mypassword, table row must be 1 row

if($count == 1){
while($row = mysql_fetch_array($result))	{
//start the session and register a variable
session_start();
$_SESSION['mysession']="mysession";
//successful login code will go here...

//we will redirect the user to another page where we will make sure they're logged in
  	header( "location:http://www.mysite.com/administrative.php" );  
echo 'Success!';												}

			  }
else {

  //if nothing is returned by the query, unsuccessful login code goes here...
  header( "location:http://www.mysite.com/login.html" );  
  echo 'Incorrect login name or password. Please try again.';
  }
} 
?>

administrative.php

<? 
if($_SESSION["mysession"]<>"mysession"){
header("location:http://www.mysite.com/login.html");
}
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>

<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Administrative page</title>

</head>

<body>
<h2>Log in successful!!</h2>
</body>
</html>

ram4nd · January 13, 2010

You might want to use && instead ||:

if (!isset($_POST['myusername']) || !isset($_POST['mypassword'])) {

Same here:

elseif (empty($_POST['myusername']) || empty($_POST['mypassword'])) {

Worst, but funniest way I have seen someone to connect to sql:

mysql_connect("$host", "$username", "$password")or die("cannot connect");

mikesta707 · January 13, 2010

No, he should use OR. Think about it, you want an error to occur if 1 OR the other is not set (or, 1 OR the other is empty).

I don't see much of a problem (other than the fact that your code is formatted very strangely)

Can you post the form you use to log in? try doing

print_r($_POST);

and see what $_POST data is being sent (and if the data being sent is what you expect it to be)

also remember if you use an md5 (or other algorithm) hash on your passwords, you have to hash the password before you check it in the query

laffin · January 13, 2010

No page outputs allowed on header redirects,

created a processpost, to do away with repetitive code.

<?php

function processpost($vars=array())
{
foreach($var as $item)
	$_GLOBAL[$item]=isset($_POST[$item])?trim($_POST[$item]):'';
}

// process our form fields, and make them as variables
// Define $myusername and $mypassword 
processposts(array('myusername','mypassword'));

//check that the form fields are not empty, and redirect back to the login page if they are
if(empty($myusername) || empty($mypassword))
{
header( "location:http://www.mysite.com/login.html" );
die();
}
$host="localhost"; // Host name 
$username="mod"; // Mysql username 
$password="modpass"; // Mysql password 
$db_name="mydbname"; // Database name 
$tbl_name="users"; // Table name 

// Connect to server and select databse.
mysql_connect("$host", "$username", "$password") or die("cannot connect"); 
mysql_select_db("$db_name") or die("cannot select DB");

// To protect MySQL injection (more detail about MySQL injection)
$myusername = stripslashes($myusername);
$mypassword = stripslashes($mypassword);
$myusername = mysql_real_escape_string($myusername);
$mypassword = mysql_real_escape_string($mypassword);

$sql="SELECT * FROM $tbl_name WHERE username='$myusername' and password='$mypassword'";
$result=mysql_query($sql);

// Mysql_num_row is counting table row
$count=mysql_num_rows($result);
// If result matched $myusername and $mypassword, table row must be 1 row

if($count == 1){
   $row = mysql_fetch_array($result);
   //start the session and register a variable
   session_start();
   $_SESSION['mysession']="mysession";
   //successful login code will go here...

   //we will redirect the user to another page where we will make sure they're logged in
     header( "location:http://www.mysite.com/administrative.php" );  
   // Cant user header location with page output
   // echo 'Success!';

} else {

  //if nothing is returned by the query, unsuccessful login code goes here...
  header( "location:http://www.mysite.com/login.html" );  
   // Cant user header location with page output
  // echo 'Incorrect login name or password. Please try again.';
}
?>

Avoid using php short tags,

u must start a session, in order to use session variables

<?php
session_start();
if($_SESSION["mysession"]<>"mysession"){
header("location:http://www.mysite.com/login.html");
}
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>

<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Administrative page</title>

</head>

<body>
<h2>Log in successful!!</h2>
</body>
</html>

ram4nd · January 13, 2010

Yes, but he should use AND also, if he is trying to do such thing. What if both are empty? Still my bad.

You get redirected back because $count is not 1 when you try to login.

if($count == 1){

if($_SESSION["mysession"]<>"mysession"){

<> works in php?

Wow php is surprising.

mikesta707 · January 13, 2010

if both are empty then the or will still be true. And will only be true if both are empty, and he wants it to redirect if 1 OR both are empty.

ram4nd · January 13, 2010

if both are empty then the or will still be true. And will only be true if both are empty, and he wants it to redirect if 1 OR both are empty.

I have never though about it like that, but that was the thing that made me confused. But I can't believe how stupid I am :S

laffin · January 13, 2010

Nope, he got redirected, because he didnt use start_session in the second script.

the first script, if correctly logged in, would send him to the second script, which in turn sent him back to the first script, because the session variables werent available, so failed the first comparison.

And it should be OR, which is either or both are not set or empty. would be silly to use AND, if you gave it a password with no username, or vice versa.

shinichi_nguyen · January 13, 2010

This is the code for check log in after read all your replies. Thank you, guys! But it's not working, yet.

Here is what appeared when I hit log in:

Warning: Invalid argument supplied for foreach() in /home/blabla/public_html/checklogin.php on line 4

Warning: Cannot modify header information - headers already sent by (output started at /home/blabla/public_html/checklogin.php:4) in /home/blabla/public_html/checklogin.php on line 15

Just a side question: what does the processpost do? why dont we use: $myusername= $_POST('myusername'), to catch the username sent and set to variable $myusename?

<?php
function processpost($vars=array())
{
   foreach($var as $item)
      $_GLOBAL[$item]=isset($_POST[$item])?trim($_POST[$item]):'';

}

// process our form fields, and make them as variables
// Define $myusername and $mypassword 
processpost(array('myusername','mypassword'));

if (empty($myusername) || empty($mypassword)) 
{
header("location:http://www.mysite.com/login.html");
die();
} 

$host="localhost"; // Host name 
$username="mod"; // Mysql username 
$password="pw"; // Mysql password 
$db_name="dbname"; // Database name 
$tbl_name="users"; // Table name 

// Connect to server and select databse.
mysql_connect("$host", "$username", "$password")or die("cannot connect"); 
mysql_select_db("$db_name")or die("cannot select DB");

// Define $myusername and $mypassword 
//$myusername=$_POST['myusername']; 
//$mypassword=$_POST['mypassword']; 

// To protect MySQL injection (more detail about MySQL injection)
$myusername = stripslashes($myusername);
$mypassword = stripslashes($mypassword);
$myusername = mysql_real_escape_string($myusername);
$mypassword = mysql_real_escape_string($mypassword);

$sql="SELECT * FROM $tbl_name WHERE username='$myusername' and password='$mypassword'";
$result=mysql_query($sql);

// Mysql_num_row is counting table row
$count=mysql_num_rows($result);
// If result matched $myusername and $mypassword, table row must be 1 row

if($count == 1){
$row = mysql_fetch_array($result);

//start the session and register a variable
session_start();
$_SESSION['mysession']="mysession";
//successful login code will go here...

//we will redirect the user to another page where we will make sure they're logged in
  	header( "location:http://www.mysite.com/administrative.php" );  
//echo 'Success!';												}

			  }
else {

  //if nothing is returned by the query, unsuccessful login code goes here...
  header( "location:http://www.mysite.com/login.html" );  
// Cant user header location with page output
  // echo 'Incorrect login name or password. Please try again.';	  }
} 
?>

laffin · January 13, 2010

its a typo

replace

foreach($var as $item)

with

foreach($vars as $item)

Just a side question: what does the processpost do? why dont we use: $myusername= $_POST('myusername'), to catch the username sent and set to variable $myusename?

its an example of how to avoid redundant code. for example if we expand the function to:

function processpost($vars=array())
{
   foreach($vars as $item)
      $_GLOBAL[$item]=isset($_POST[$item])?mysql_real_escape_string(stripslashes(trim($_POST[$item]))):'';
                  
}
processpost(array('myusername','mypassword'));

if (empty($myusername) || empty($mypassword)) 
{
   header("location:http://www.mysite.com/login.html");
   die();
}

You can do away with these lines

if (!isset($_POST['myusername']) || !isset($_POST['mypassword'])) {
header("location:http://www.mysite.com/login.html");

} 
//check that the form fields are not empty, and redirect back to the login page if they are
elseif (empty($_POST['myusername']) || empty($_POST['mypassword'])) {
header( "location:http://www.mysite.com/login.html" );
// Define $myusername and $mypassword 
$myusername=$_POST['myusername']; 
$mypassword=$_POST['mypassword']; 

// To protect MySQL injection (more detail about MySQL injection)
$myusername = stripslashes($myusername);
$mypassword = stripslashes($mypassword);
$myusername = mysql_real_escape_string($myusername);
$mypassword = mysql_real_escape_string($mypassword);

Its basicly the same code, but more generic to handle a range of $_POST variables and assign them into a global variable.

The odd thing you will notice is the

isset($_POST[$item])?

the question mark, designates this as a trenary operator.

which is basicly an if statement

if(isset($_POST[$item]))
  $_GLOBAL[$item]=mysql_real_escape_string(stripslashes(trim($_POST[$item])));
else
  $_GLOBAL[$item]='';

so if $_POST is set, it does all the extra functions, otherwise, it just makes an empty string.

shinichi_nguyen · January 14, 2010

Ok, so i fixed that typo, now no warning appears but it just redirect me back to the log in page again with blank username and password field. I'm positive the username and password value in database are correct!

shinichi_nguyen · January 14, 2010

I used the print_r($_POST) and i see that the values sent are correct as in database.

Here is the log in form

<form name="login" id="login" method="post" action="checklogin.php">
<table align="center">
<tr>
<td><label for="username">Username</label></td>
<td><input type="text" name="myusername" id="myusername" /></td>
</tr>
<tr>
<td><label for="password">Password</label></td>
<td><input type="password" name="mypassword" id="mypassword" /></td>
<tr>
<td></td>
<td><input type="submit" name="submit" id="submit" value="Submit" /></td>
</tr>
</tr>
</table>
</form>

Here is the latest checklogin.php

<?php
print_r($_POST);
function processpost($var=array())
{
   foreach($var as $item)
      $_GLOBAL[$item]=isset($_POST[$item])?trim($_POST[$item]):'';

}

// process our form fields, and make them as variables
// Define $myusername and $mypassword 
processpost(array('myusername','mypassword'));

if (empty($myusername) || empty($mypassword)) 
{
header("location:http://www.mysite.com/login.html");
die();
} 

$host="localhost"; // Host name 
$username="mymod"; // Mysql username 
$password="mypassword"; // Mysql password 
$db_name="mydbname"; // Database name 
$tbl_name="users"; // Table name 

// Connect to server and select databse.
mysql_connect("$host", "$username", "$password")or die("cannot connect"); 
mysql_select_db("$db_name")or die("cannot select DB");

// Define $myusername and $mypassword 
//$myusername=$_POST['myusername']; 
//$mypassword=$_POST['mypassword']; 

// To protect MySQL injection (more detail about MySQL injection)
$myusername = stripslashes($myusername);
$mypassword = stripslashes($mypassword);
$myusername = mysql_real_escape_string($myusername);
$mypassword = mysql_real_escape_string($mypassword);

$sql="SELECT * FROM $tbl_name WHERE username='$myusername' and password='$mypassword'";
$result=mysql_query($sql);

// Mysql_num_row is counting table row
$count=mysql_num_rows($result);
// If result matched $myusername and $mypassword, table row must be 1 row

if($count == 1){
$row = mysql_fetch_array($result);

//start the session and register a variable
session_start();
$_SESSION['mysession']="mysession";
//successful login code will go here...

//we will redirect the user to another page where we will make sure they're logged in
  	header( "location:http://www.mysite.com/administrative.php" );  
//echo 'Success!';												}

			  }
else {

  //if nothing is returned by the query, unsuccessful login code goes here...
  header( "location:http://www.mysite.com/login.html" );  
// Cant user header location with page output
  // echo 'Incorrect login name or password. Please try again.';	  }
} 
?>

and here is the administrative.php

<?php
session_start();
if($_SESSION["mysession"]<>"mysession"){
header("location:http://www.mysite.com/login.html");
}
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>

<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Administrative page</title>

</head>

<body>
<h2>Log in successful!!</h2>
</body>
</html>

bombsquad · January 14, 2010

Instead of using this code

//check that the form fields are not empty, and redirect back to the login page if they are

elseif (empty($_POST['myusername']) || empty($_POST['mypassword'])) {

header( "location:http://www.mysite.com/login.html" );

}

try this one to avoid redirect page

<?php

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

//check that the form fields are not empty, and redirect back to the login page if they are

if (empty($_POST['myusername']) || empty($_POST['mypassword'])) {

$msg = "Empty field found!";

}else{

//your code goes here

}

?>

<table>

<tr>

<td>

<?php

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

echo $msg;

}

?>

</td>

</tr>

<tr>

<td>YOUR DESIGN GOES HERE..</td>

</tr>

</table>

</form>

shinichi_nguyen · January 14, 2010

Thank you, Bomb (If I can call you that), but your code is too vague at least for me, a php new born newbie . Can you look at the latest code of the 3 pages and tell me why it keeps taking me back to the login page after I submit username and password? like what's wrong with the code?

Thank you for all your help, guys! I'm pretty much looking to get this project done asap! So...please bear with me!

Thanks

bombsquad · January 15, 2010

Thank you, Bomb (If I can call you that), but your code is too vague at least for me, a php new born newbie . Can you look at the latest code of the 3 pages and tell me why it keeps taking me back to the login page after I submit username and password? like what's wrong with the code?

Thank you for all your help, guys! I'm pretty much looking to get this project done asap! So...please bear with me!

Thanks

your don't have a session_start(); in your administrative.php page. and include this code at the top of your page to see all possible errors error_reporting(E_ALL);ini_set('display_errors', '1');

shinichi_nguyen · January 15, 2010

Bombsquad, to what I see, I do have a session start in my administrative.php page.

bombsquad · January 25, 2010

I don't see any session_start() in your administrative.php

you only have this if($_SESSION["mysession"]<>"mysession"){ this won't work until you start your session..

Sign In

Please help me revise this log in code.

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation

Important Information