[SOLVED] Login script not returning error?

[Dark-Hawk]

Alright so if the login fails, ultimately it's returning zero error for the user, it's just bringing them right back to the login screen without saying anything. I was wanting it to at least display an error at the top of the page. Any help or insight (or if you think something with my code sucks too, that'd be helpful to know) would be great:

<?
include_once("cfg.php");
session_start();

if ($_POST['Submit'] == 'Login') {
$md5pass = md5($_POST['password']);
$eemail = mysql_real_escape_string($_POST['email']);
$sql = "SELECT * FROM $tbl_name WHERE email='$eemail' and password='$md5pass' and confirm='1'";
$result = mysql_query($sql) or die(mysql_error());
$error = mysql_error();
$num = mysql_num_rows($result);

if ($num != 0) {
	// A matching row found (thus the name/pass found) - authenticated
	session_start();
	list($efirst_name) = mysql_fetch_row($result);
	$_SESSION['user'] = $eemail;

	if (isset($_SESSION['user'])) {
		header("Location: calendar.php?msg=Logged In");
	}
	if ($error) {
		header("Location: login.php?msg=Invalid Login");
		//exit();
	}
}
}
?>
<link href="styles.css" rel="stylesheet" type="text/css">

<?php if (isset($_GET['msg'])) { echo "<div class=\"msg\"> $_GET[msg] </div>"; } ?>

[ober]

<?php if (isset($_GET['msg'])) { echo "<div class=\"msg\">".$_GET['msg']."</div>"; } ?>

 

When they get redirected, do you at least see the error message in the URL?

[Dark-Hawk]

No I'm not even seeing the error in the URL. It redirects and works fine if their login succeeded but otherwise it just essentially refreshes the page.

[waterssaz]

forgive my ignorance, bit rusty at PHP, but why are you checkings for mysql errors after a supposed match has been found.

if ($num != 0) {
	// A matching row found (thus the name/pass found) - authenticated
	session_start();
	list($efirst_name) = mysql_fetch_row($result);
	$_SESSION['user'] = $eemail;

	if (isset($_SESSION['user'])) {
		header("Location: calendar.php?msg=Logged In");
	}
	if ($error) {
		header("Location: login.php?msg=Invalid Login");
		//exit();
	}
}

:-)

 

[ober]

if ($num != 0) {
   // A matching row found (thus the name/pass found) - authenticated
   session_start();
   list($efirst_name) = mysql_fetch_row($result);
   $_SESSION['user'] = $eemail;
   header("Location: calendar.php?msg=Logged In");
}
else    
   header("Location: login.php?msg=Invalid Login");

[mrMarcus]

any in cfg.php that we need to see?

 

also, you have 2 session_start()'s in there .. leave the one at the top, remove the other.

[Dark-Hawk]

Great, what ober said did it for me.

 

cfg.php just has the SQL server information (user/pass, etc). Didn't realize I added two session_start()'s, thanks! Any further clean up you'd recommend in terms of securing the code, or..?

[mrMarcus]

play around with this...

<?
#start session;
session_start();

#start output buffering;
ob_start();

#db details;
include_once("cfg.php");

#sanitize function;
function sanitize($input)
{	
$input = @trim($input);
    if (get_magic_quotes_gpc())
{ $input = stripslashes($input); }
    $output = mysql_real_escape_string($input);
}

#has form been submitted;
if ($_POST['Submit'] == 'Login') {
$sql = mysql_query(sprintf("SELECT * FROM $tbl_name WHERE email='%s' and password='%s' and confirm='%d'", sanitize($_POST['email']), sanitize($_POST['password']), (int)1));

#we have a match;
if (mysql_num_rows($sql) > 0)
{
	// A matching row found (thus the name/pass found) - authenticated
	list($efirst_name) = mysql_fetch_row($result);

	#set session;
	$_SESSION['user'] = $eemail;

	#redirect;
	header("Location: calendar.php?msg=Logged In");
	exit;
} else {
	header("Location: login.php?msg=Invalid Login");
	exit;
}
} else { #form not submitted .. do something;
//display form?  redirect?
}
?>
<link href="styles.css" rel="stylesheet" type="text/css">
<?php
$msg = htmlentities(trim($_GET['msg']));
$msg = (isset($msg)) ? true : false;

if ($msg)
{ echo '<div class="msg"> $_GET['msg'] </div>'; }

release output buffer;
ob_end_flush(); ?>

with things like

$result = mysql_num_rows($sql);

unless you are planning on using $result further, you need not put the number of returned rows in a string .. instead, as i did, just do

if (mysql_num_rows($sql) > 0) {

shortens things up.

 

be advised .. i have not tested this code .. aside from any possible syntax errors, it should work.  use it as a base for future coding.

[Dark-Hawk]

I definitely will give that a look, thanks a lot for the help with it and cleaning it up a bit!

[mrMarcus]

quick follow-up .. in the function sanitize() i gave, below this line :

$output = mysql_real_escape_string($input);

add this :

return $output;

 

and, 'release output buffer;' @ the bottom of the script is meant to be a comment, so just add # or // before .. or remove it altogether before using (if you use);

[Dark-Hawk]

Just a heads up I gave that script a try and while I like it, it's not working, I'm really unsure why as it appears to be good:

<?
// start session
session_start();

// start output buffering
ob_start();

// File includes
include_once("cfg.php");
include_once("functions.php");

// sanitize function
function sanitize($input)
{
$input = @trim($input);
if (get_magic_quotes_gpc())
	{ $input = stripslashes($input); }
    $output = mysql_real_escape_string($input);

return $output;
}

// has form been submitted
if ($_POST['Submit'] == 'Login') {
$sql = mysql_query(sprintf("SELECT * FROM $tbl_name WHERE email='%s' and password='%s' and confirm='%d'", sanitize($_POST['email']), sanitize($_POST['password']), (int)1));

// we have a match
if (mysql_num_rows($sql) > 0)
{
// A matching row found (thus the name/pass found) - authenticated
list($email) = mysql_fetch_row($result);
//set session;
$_SESSION['user'] = $email;
//redirect;
header("Location: calendar.php?msg=Logged In");
exit();
} else {
header("Location: login.php?msg=Invalid Login");
exit();
}

} else { 
ShowLogin();
}
?>
<link href="styles.css" rel="stylesheet" type="text/css">
<?php
$msg = htmlentities(trim($_GET['msg']));
$msg = (isset($msg)) ? true : false;

if ($msg)
{ echo "<div class='msg'> $_GET[msg] </div>"; }

// release output buffer
ob_end_flush(); ?>

 

Just to keep things clean I put the login form into a function just to keep it in another file for the time being. It's nothing more than:

function ShowLogin() {
?>
<p> </p><table width="40%" border="0" align="center" cellpadding="0" cellspacing="0">
  <tr> 
    <td bgcolor="#d5e8f9" class="mnuheader" >
<div align="center"><font size="5"><strong>Login 
        Members</strong></font></div></td>
  </tr>
  <tr> 
    <td bgcolor="#e5ecf9" class="mnubody"><form name="form1" method="post" action="">
        <p> </p>
        <p align="center">Your Email 
          <input name="email" type="text">
        </p>
        <p align="center"> Password: 
          <input name="password" type="password">
        </p>
        <p align="center"> 
          <input type="submit" name="Submit" value="Login">
        </p>
        <p align="center"><a href="register.php">Register</a> | <a href="forgot.php">Forgot</a></p>
      </form></td>
  </tr>
</table>
<?
}

[Dark-Hawk]

Whoops, realized the issue, passwords are encrypted with MD5, it was never converting the pass into an MD5 string, so I had to add

$md5pass = md5(sanitize($_POST['password']));

 

Thanks again for the cleaned up code a bit.