Login script not working

[nVee]

Hey Guys

Well I have a login script I wrote today, and it kinda took a turn for the worse as I cannot find the problem. I will try explain it as I go:

<?php
// CHECK IF SESSION IS ALREADY SET
if($_SESSION["id"] == "1") 
{
echo "<p>Welcome back ".$uname."! <a href='news.php'>News</a> |<a href='profile.php'>Profile</a> |<a href='logout.php'>Logout</a></p>";
}
// CHECK IF THE USER PRESSED SUBMIT TO ATTEMPT A LOGIN
if($_POST["userlogin"] == "submit") {
$username = $_POST["email"];
$password = substr(md5($_POST["password"]),0,16);
connectdb();
$query = mysql_query("SELECT name, email, password, account_type FROM ov_users WHERE email = '".$email."' AND password = '".$password."' AND account_type = '2'");
if(!$query) {
echo "<p>Oops, this is strange ... we cannot seem to log you in at the moment! Please try again in 5 minutes. If this problem occurs again, please contact our support department at <a href='mailto:[e-mail]'>[e-mail]</a></p>";
}
// This just assigns the users name to $uname so that I can use it as a message to welcome the user.
while($result = mysql_fetch_array($query)) {
$uname = $result["name"];
}
$num = mysql_num_rows($query);
// CHECK IF THE USER DID NOT SELECT REMEMBER ME, OBVIOUSLY CREATING A SESSION AS APPOSE TO A COOKIE.
if($num > 0) {
$_SESSION["username"] = $username;
$_SESSION["id"] = session_id();
$_SESSION["active"] = "1";
echo "<p>Welcome back ".$uname." Click <a href='profile.php'>here</a> to view your profile!</a></p>";
}
// CHECK IF THE USER DID SELECT REMEMBER ME. THIS CREATES A COOKIE CALLED cookie_id WITH A RANDOM STRING AND MD5. THIS THEN GETS SAVED IN THE DATABASE AND WILL BE RECALLED LATER.
if($num > 0 && $rememberme == "remember") {
setcookie("username",$username,time()[phone]);
$rand = rand(0,10000000);
set_cookie("cookie_id",$rand,time()[phone]);
$mdrand = md5($rand);
$query = mysql_query("UPDATE ov_users SET cookie_id='".$mdrand."' WHERE email='".$username."'");
echo "<p>Welcome back ".$username."! Click <a href='profile.php'>here</a> to view your profile!</a></p>";
if(!$query) {
echo "<p>Oops, this is strange ... we cannot seem to log you in at the moment! Please try again in 5 minutes. If this problem occurs again, please contact our support department at <a href='mailto:[e-mail]'>[e-mail]</a></p>";
}
}
// THIS IS TRUE IF THE USERNAME AND PASSWORD DOES NOT MATCH
if($num != 0) {
echo "<p>The username and password you entered does not exist or your account needs to be verified. Please check your details and try again. | <a href='index.php'>TRY AGAIN</a> | <a href='forgotpass.php'>FORGOT MY PASSWORD</a> | <a href='register.php'>REGISTER A FREE ACCOUNT</a></p>";
}
// THIS IS TRUE IF THE USER DID NOT PRESS SUBMIT. THIS JUST SHOWS THE LOGIN FORM
} else {
?>
   <form action="<?php echo htmlentities($_SERVER['PHP_SELF']); ?>" method="post">
<p>
Email: <input type="text" name="email" />
Password: <input type="text" name="password" />
   Remember me:<input name="rememberme" type="checkbox" value="remember" /></input>
<input name="userlogin" type="submit" value="submit"></input> 
| Forgot my password
</p>
</form>
   <?php
}
?>

[edited by: eelixduppy at 3:07 pm (utc) on Feb 11, 2010]
[edit reason] exemplified [/edit]

[Matthew1980]

Hi there nVee,

Have you echoed your $_POST array to screen after the submit button is pressed to see what variables are being sent to the processing part of the script?

print_r($_POST);
exit;

Use the exit; just to terminate the script when the array is echoed to screen..

Cheers,

MRb

[CyBerAliEn]

What exactly is the problem?

User login fails? Scripting doesn't process as intended/expected?

If that is your whole script, I see a number of "logical" errors, namely: variables are called for use at places before they ever assigned or set... and these variables are then assigned/set later. This is a flaw. For example, at the very beginning you check:

if($_SESSION["id"] == "1")

But if you look further down the code, you set the "PHP session ID" to the index 'id'. And you set '1' to the index 'active'. So you should change the IF condition to:

if($_SESSION["active"] == "1")

In this same block of code, you attempt to use the variable 'uname'... but it hasn't been assigned or set yet? You need to set it up before you use it! Such as follows:

$uname = $_SESSION['username'];

Aside from this, there are further concerns further down. For example, why are you setting a cookie for "remember me"? No where in your code do you try to check or utilize the cookie value. Additionally, you give the cookie the name '$rand' but you then update the database to show the cookie ID as 'md5($rand)'. How is this suppose to work?

What I would recommend is being simple.

Create a form to collect the user information. Send this form to a processor (a separate script). This script should grab the form fields and check if the user is a valid user. IF the user is invalid; reject and display an error message. IF the user is valid; set a session variable such as follows:

$_SESSION['validuser'] = true;
$_SESSION['username'] = $_POST['email'];

Then, have the processor redirect the user to the "authorized only" page/area.

Inside this authorized area, just setup some code like the following:

<?php
if ($_SESSION['validuser']!==true) { include('my_login_page.html'); exit(); }
?>
(page for member zone is down here)

This is very simple way to do it. Setting up the form, the checking, and the end-purpose (user zone) all in one file is too chaotic.

[nVee]

Thank you both for your replies. I realise now I never told you what the problem is. I have modified the code a bit however, so it works slightly better than yesterday.

At the moment, and I will paste the revised code below, it logs the user in, and the session gets set, but it would on login say that I am logged in, but right below that say incorrect username and password. I predict, I have brackets in the wrong place and my if/else statements dont match up.

I am taking to note your points. I however want to say that the cookie md5 thing is part of a security feature. It prevents a possible hacker to get the session ID. On authentication, it will decrypt the session ID and compare does 2.

Anyways, I will play around with the code again, but here is the last revised version:

<?php
if($_SESSION["active"] == "1") 
{
echo "<p>Welcome back ".$result["name"]."! <a href='news.php'>News</a> |<a href='profile.php'>Profile</a> |<a href='logout.php'>Logout</a></p>";
}

if($_POST["userlogin"] == "submit") {
 $username = $_POST["email"];
 $password = substr(md5($_POST["password"]),0,16);
 connectdb();
 $query = mysql_query("SELECT name, email, password, account_type FROM ov_users WHERE email = '".$email."' AND password = '".$password."' AND account_type = '2'");
 if(!$query) {
echo "<p>Oops, this is strange ... we cannot seem to log you in at the moment! Please try again in 5 minutes. If this problem occurs again, please contact our support department at <a href='mailto:[e-mail]'>[e-mail]</a></p>";
}
$num = mysql_num_rows($query);
while($result = mysql_fetch_array($query)) {
$uname = $result["name"];
}

// This indicates that the user logged in successfully but did not select to remember me.

if($num > 0) {
$_SESSION["username"] = $username;
$_SESSION["id"] = session_id();
$_SESSION["active"] = "1";
echo "<p>Welcome back ".$username." Click <a href='profile.php'>here</a> to view your profile!</a></p>";
}

// This indicates that the user logged in successfully but selected to be remembered.

if($num > 0 && $rememberme == "remember") {
setcookie("username",$username,time()[phone]);
$rand = rand(0,1000000);
setcookie("cookie_id",$rand,time()[phone]);
$mdrand = md5($rand);
$query = mysql_query("UPDATE ov_users SET cookie_id='".$mdrand."' WHERE email='".$username."'");
if(!$query) {
echo "<p>Oops, this is strange ... we cannot seem to log you in at the moment! Please try again in 5 minutes. If this problem occurs again, please contact our support department at <a href='mailto:[e-mail]'>[e-mail]</a></p>";
exit();
}
echo "<p>Welcome back ".$username."! Click <a href='profile.php'>here</a> to view your profile!</a></p>";

}
 } 
 if($num != 0) {
echo "<p>The username and password you entered does not exist or your account needs to be verified. Please check your details and try again. | <a href='index.php'>TRY AGAIN</a> | <a href='forgotpass.php'>FORGOT MY PASSWORD</a> | <a href='register.php'>REGISTER A FREE ACCOUNT</a></p>";
}
 if($_SESSION["active"] != "1")
 {
?>
      <form action="<?php echo htmlentities($_SERVER['PHP_SELF']); ?>" method="post">
<p>
Email: <input type="text" name="email" />
Password: <input type="text" name="password" />
      Remember me:<input name="rememberme" type="checkbox" value="remember" /></input>
<input name="userlogin" type="submit" value="submit"></input> 
| Forgot my password
</p>
</form>
      <?php
}
?>

[Matthew1980]

Hi there nVee,

Just a quick note, as I have just noticed:-

$username = $_POST["email"];

All of your Super globals are in double quotes, whereas, they only need single quotes as they are keys in arrays:-

$username = $_POST['email'];
and the $_SESSION['active'], $_SESSION['id'],$_SESSION['username']

This is just good practice to do this.

Is this all in one file then, as cyberalien asks? If it is you will have to see if the sessions are set, and if they are then proceed or:-

if(isset($_SESSION['active']) && ($_SESSION['active'] == "1"))
{
//its set, do stuff - etc
}

Just a thought, but have you got your error messaging turned on?

place it right by the first opening <?php tag ie:

<?php
error_reporting(E_ALL);

You will then be told what (if anything) is wrong with the script, when developing this tool is important.

Cheers,

MRb