Why isn't it working?

[3raser]

Why doesn't my code WORK AT ALL? It keeps saying incorrect password, even when a user doesn't exist or the password and username ARE correct.

 

<?php
session_start();
//make sure this is always the very top before the opening php tag
//anytime you want to use a session - this needs to be here
//anytime you try and get information on a user that is logged in, make sure you start the session like this

$username = $_POST['username'];
$password = $_POST['password'];

$session = $_SESSION['user'];

if(!$session)
{

//checks to see if the username and password are set
if(!$username || !$password)
{
//this is the basic form where it allows the users to enter in their username and password

echo "You must fill out all the fields before going any further.";
echo "<br/><br/><center><form action='index.php' method='POST'>Username: <input type='text' name='username'>";
echo "<br/>Password: <input type='text' name='password'><br/><input type='submit'></form></center>";
}
else //if they are set, we continue
{
//this is the details that we use to connect to the database, make sure yours is matching - it's host, user, pass and then db

mysql_connect('', '', '');
mysql_select_db('');

$username = mysql_real_escape_string($username);
$password = mysql_real_escape_string($password);

//check to see if the user exists
$sql_check_user_exist = mysql_query("SELECT * FROM users WHERE username='$username'");
$row_cue = mysql_num_rows($sql_check_user_exist);

//makes sure the user exists before trying to see if the details are correct
if($row_cue['username'] <= 1)
{
	//since the user exists, we are getting the password and username and making sure they are correct
	$dbusername_sql = mysql_query("SELECT password FROM users WHERE username='$username'");
	$dbpassword_sql = mysql_query("SELECT username FROM users WHERE username='$username'");

	$get_pw = mysql_fetch_array($dbpassword_sql);
	$get_user = mysql_fetch_array($dbusername_sql);

	//making sure the password is the same as the password in the database, the same with the username
	if($get_pw['password']==$password && $get_user['username']==$username)
	{
		echo "You have successfully logged in!";

		//creates the session so you can know the user and their details
		$_SESSION['user']=$username;
	}
	else
	{
		//else the details or incorrect so we give them an error
		echo "Sorry, the username or password is incorrect. <a href='index.php'>Back</a>";
	}
}
else
{
	//since no user exists with the username they typed in, we give them this message
	echo "Sorry, no user exists with this username. <a href='index.php'>Back</a>";
}
}
}
else
{
echo "Your already logged in!";
}

?>

[trq]

This part....

 

if($row_cue['username'] <= 1)

 

will be true if the user does or doesn't exist.

 

Thats as far as I went. Your code however is all over the place and needs to be completely rewritten. Why are you executing a query on the same table 3 different times?

[3raser]

This part....

 

if($row_cue['username'] <= 1)

 

will be true if the user does or doesn't exist.

 

Thats as far as I went. Your code however is all over the place and needs to be completely rewritten. Why are you executing a query on the same table 3 different times?

 

First, why do you have to be such an ass? I'm just here for help. And you haven't even explained why the code wouldn't work. You just point it out and say this will always be true. I'm here for the help, and so I would like for it to be explained on why it wont work so I can solve the problem in future problems.

 

And why are you beating down on my code? Just because I use 3 queries on one table doesn't mean my code is crap. If you can point out a few more reasons as to why my code is crap and sloppy, then I'd be happy to listen. Just don't go out and start insulting me just because my code may be crap to you.

[3raser]

Bump...

[Alex]

Just don't go out and start insulting me just because my code may be crap to you.

He never insulted you. Don't get so offended by code critique.

 

Anyway, here's a rewritten copy of your script with comments added to let you know what I did, and why.

 

<?php
error_reporting(E_ALL); // Always develop with error reporting turned all the way up
session_start();
//make sure this is always the very top before the opening php tag
//anytime you want to use a session - this needs to be here
//anytime you try and get information on a user that is logged in, make sure you start the session like this

//$username = $_POST['username'];
//$password = $_POST['password'];

//$session = $_SESSION['user']; // No point in explicitly defining a separate variable for $_SESSION['user'], just use that.

// When checking if variables are set always use isset(), this is to avoid error messages if the variable is not set.
if(!isset($_SESSION['user']))
{

//checks to see if the username and password are set
if(!isset($_POST['username'], $_POST['password'])) // Again using isset()
{
//this is the basic form where it allows the users to enter in their username and password

echo "You must fill out all the fields before going any further.";
echo "<br/><br/><center><form action='index.php' method='POST'>Username: <input type='text' name='username'>";
echo "<br/>Password: <input type='text' name='password'><br/><input type='submit'></form></center>";
}
else //if they are set, we continue
{
//this is the details that we use to connect to the database, make sure yours is matching - it's host, user, pass and then db

mysql_connect('', '', '');
mysql_select_db('');

$username = mysql_real_escape_string($_POST['username']);
$password = mysql_real_escape_string($_POST['password']);

//check to see if the user exists
$sql_check_user_exist = mysql_query("SELECT username, password FROM users WHERE username='$username' AND password='$password' LIMIT 1");
//$row_cue = mysql_num_rows($sql_check_user_exist); // Again really no point in defining a variable for this unless you're using it multiple places

//makes sure the user exists before trying to see if the details are correct
if(mysql_num_rows($sql_check_user_exist) > 0) // You can also just use if(mysql_num_rows($sql_check_user_exist)) because any number other than 1 will evaluate to true, but this way makes things more clear
{
	// No need for these queries, because we can already get the information from the one we did above

	//since the user exists, we are getting the password and username and making sure they are correct
	//$dbusername_sql = mysql_query("SELECT password FROM users WHERE username='$username'"); 
	//$dbpassword_sql = mysql_query("SELECT username FROM users WHERE username='$username'");

	//$get_pw = mysql_fetch_array($dbpassword_sql);
	//$get_user = mysql_fetch_array($dbusername_sql);

	$row = mysql_fetch_assoc($sql_check_user_exist);

	//making sure the password is the same as the password in the database, the same with the username
	if($row['password'] == $_POST['password'] && $row['username'] == $_POST['username'])
	{
		echo "You have successfully logged in!";

		//creates the session so you can know the user and their details
		$_SESSION['user'] = $_POST['username'];
	}
	else
	{
		//else the details or incorrect so we give them an error
		echo "Sorry, the username or password is incorrect. <a href='index.php'>Back</a>";
	}
}
else
{
	//since no user exists with the username they typed in, we give them this message
	echo "Sorry, no user exists with this username. <a href='index.php'>Back</a>";
}
}
}
else
{
echo "Your already logged in!";
}

[trq]

First, why do you have to be such an ass? I'm just here for help. And you haven't even explained why the code wouldn't work. You just point it out and say this will always be true. I'm here for the help, and so I would like for it to be explained on why it wont work so I can solve the problem in future problems.

 

And why are you beating down on my code? Just because I use 3 queries on one table doesn't mean my code is crap. If you can point out a few more reasons as to why my code is crap and sloppy, then I'd be happy to listen. Just don't go out and start insulting me just because my code may be crap to you.

 

Sorry, I didn't realize you where such a sensitive soul.

 

And now to pick on AlexWD's code.

 

This entire section:

 

if($row['password'] == $_POST['password'] && $row['username'] == $_POST['username'])

 

isn't needed, because we know the username & password matched, or we would have already found out because we would have had mysql_num_rows() return 0.

 

If I where to write the OP's code, it would be something like:

 

<?php
session_start();
if (!isset($_SESSION['user'])) {
    if (isset($_POST['username'], $_POST['password']) {
        mysql_connect('', '', '');
        mysql_select_db('');
        $username = mysql_real_escape_string($_POST['username']);
        $password = mysql_real_escape_string($_POST['password']);
        $sql = "SELECT username, password FROM users WHERE username='$username' AND password='$password' LIMIT 1";
        if ($result = mysql_query($result)) {
            if (mysql_num_rows($result)) {
                // user is valid, log in.
                $_SESSION['user'] = $_POST['username'];
                // redirect to somewhere useful.
            } else {
                echo "Sorry, the username or password is incorrect. <a href='index.php'>Back</a>";
            }
        } else {
            // Query failed. We have a problem.
            trigger_error(mysql_error() . "<br />$sql");
            echo "Sorry, there was a problem logging you in, try again later"; 
        }
    } else {
        echo "You must fill out all the fields before going any further.";
        echo "<br/><br/><center><form action='index.php' method='POST'>Username: <input type='text' name='username'>";
        echo "<br/>Password: <input type='text' name='password'><br/><input type='submit'></form></center>";
    }
} else {
    echo "You are already logged in";
}

[Alex]

Bleh, I should have just completely rewritten the entire thing. Bound to make mistakes when I'm working off poor logic in the first place. Anyway, thanks for pointing that out.