php login form always takes me to editor.php page

Question

My current login form doesn't work correctly. It always logs me in no matter what is entered into the login form. Since the first question, I decided to change my code up lots.

Here is the up to date code:

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

    if(isset($_POST['loginSub'])) {
        //Connect to DB
        include_once("db_connect.php");

        //Gets whatever is inside input box and prevents sql injection
        $username=($_POST['user']);
        $password=($_POST['password']);

        $query = "SELECT * FROM user WHERE user= 'kent' AND password = 'password'";
        $rows = mysqli_query($conn, $query);

        if ($rows->num_rows == 1) {
            $_SESSION['user']=$username; // Initializing Session
            header("location: loginAuth.php"); // Redirecting To Other Page
        } else {
            $error = "Username or Password is invalid!!!!";
        }
            mysql_close($conn); // Closing Connection

    }       

?>

Here is the HTML form:

<form id="login" method="POST" action="loginAuth.php">

                    <label for="user"><strong>Login:</strong></label>
                    <input type="text"size=20 autocorrect=off autocapitalize=words name="user">
                    <!--<label for="loginPassword" name="password"> <strong>Password:</strong></label>
                    <input type="password" name="Password"> -->

                    <label for="password" > <strong>Password:</strong></label> 
                    <input type="password" name="password">

                    <input name="loginSub" type="submit" value="login"> 
                </form>  

The form action loginAuth.php basically takes you to the editor page and has no php in it yet.

Each snippet of code comes from the same php page. My question is, how can I make it so it doesn't log you in when the credentials are incorrect?

Answer 1

mysqli_real_escape_string requires an connection parameter, else it will be blank:

$username = mysqli_real_escape_string($db, $username);
$password = mysqli_real_escape_string($db, $password);

Since, both variables are blank, your query will automatically evaluates to true.

---

Also, you won't need quotes for mysqli_query:

$query = mysqli_query($db,$sql);

---

You would also need to specify the form method which is POST:

<form id="login" method="POST" action="loginAuth.php">

This is because you are getting the data in PHP as $_POST:

An associative array of variables passed to the current script via the HTTP POST method when using application/x-www-form-urlencoded or multipart/form-data as the HTTP Content-Type in the request.

$username=strip_tags($_POST['user']);
$password=strip_tags($_POST['password']);

---

Just a tip: You shouldn't store users' passwords as plain-text, you should hash/ crypt it.

In PHP 5.5, there's a Password Hash function: http://php.net/manual/en/ref.password.php.

---

Updated Logic:

$sql = "SELECT * FROM users WHERE username='$username' AND password='$password' LIMIT 1";
$query = mysqli_query($db, $sql);
$result = mysqli_result($query);

if(mysqli_num_rows($result) > 0) {
    $_SESSION['username']= $username;
    $_SESSION['id']= $id;
    //useless code doesnt work header('Location: editor.php');

// { other code }

Answer 2

Your first mistake is you haven specify form method when you not specify from method it should always $_GET but you are checking with $_POST so first please specify form method and this your login page so my advice set post method in form tag for security purpose.

<form id="login" action="loginAuth.php" method="post">

Then try may it helps you.

Answer 3

To summarize other answers and improve code:

In your form you need to set the method. So <form id="login" action="loginAuth.php"> becomes <form id="login" action="loginAuth.php" method="post">

To improve your form HTML:

<div id="loginform">
   <form id="login" action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
        <label for="username"><strong>Username:</strong></label>
        <input type="text" size=20 autocorrect="off" autocapitalize="words" name="username" id="username">

         <label for="password" name="password"><strong>Password:</strong></label>
         <input type="password" name="password" id="password">

         <input name="loginSub" type="submit" value="login"> 
   </form> 
</div>

Note in order to make the labels clickable the for attribute needs to match the id attribute of the input. Als the name attribute needs to match the name of the $_POST. Both of these weren't correct in your form.

To shorten your code and make it more maintainable just use the following:

First instead of if(isset($_POST['loginSub'])) { you can also just say if( $_POST ) This makes sure the rest of the code is only executed when the page is posted.

To shorten your code even more instead of doing all the sql injection prevention on seperate lines you can combine them into one. This makes maintaining a lot easier.

$username = mysqli_real_escape_string($db, stripslashes( strip_tags( $_POST['username'] ) ) );
$password = mysqli_real_escape_string($db, stripslashes( strip_tags( $_POST['password'] ) ) );

Also don't forget to add the database connection to mysqli_real_escape_string().

To see if the username and password combination match use

$sql = "SELECT * FROM `users` WHERE `username`= '$username' AND `password` = '$password' LIMIT 1";

Note the backticks around each column name. The backticks are there to prevent an error called mysql reserved words. Altough the columns names aren't reserved word but in general it's good practice to use them.

To execute the query you don't need the quotes. So $query = mysqli_query("$db,$sql"); becomes $query = mysqli_query($db, $sql);

After that you can simply do:

$query = mysqli_query($db, $sql);

if( $query ) {
  if( mysqli_num_rows( $query ) >= 1 ) {
        $_SESSION['username'] = base64_encode( $username ); 
        header('Location: home.php');
        exit(); // To prevent further exection of code
  } else {
        echo "Incorrect details.Try again!";
  }
} else {
  echo "Error in query";      
}

base64_encode makes it harder to read a session. Use base64_decode() to read it

Notice that you shouldn't be storing passwords in plain text use a function like password_hash() and password_verify().

You also shouldn't just use md5() as it is easily cracked. What you can do is use in in combination. Something like below. These are just 2 example of what you could do, I'm not saying one is better than the other. Just don't use md5() on it's own.

$password = password_hash( md5( $password) );
$password = password_hash( hash('sha512', sha1( md5( $password ) ) );

Answer 4

First of all the query should be checking both the username and password. It should be like this:

$sql = "SELECT * FROM users WHERE username = '$username' and password = '$password'";

Then check the results to see if you're not getting any records, then show a message like "invalid credentials". If not you should redirect or show what part you want to.

Answer 5

Some quick fixes for you could be:

1) Specify method to form method="POST"

2) mysqli_real_escape_string expects two parameters. Please check this link for more information. Change the following code from

$username = mysqli_real_escape_string($username);
$password = mysqli_real_escape_string($password);

to

$username = mysqli_real_escape_string($db, $username);
$password = mysqli_real_escape_string($db, $password);

3) Change $query = mysqli_query("$db, $sql"); to $query = mysqli_query($db, $sql);

4) Also check for the updated query for checking username and password.

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

    if(isset($_POST['loginSub'])) 
    {
        //Connect to DB
        include_once("db_connect.php");

        //Gets whatever is inside input box and prevents sql injection
        $username = strip_tags($_POST['user']);
        $password = strip_tags($_POST['password']);

        //Gets whatever is inside input box and prevents sql injection
        $username = stripslashes($username);
        $password = stripslashes($password);

        //Gets whatever is inside input box and prevents sql injection
        $username = mysqli_real_escape_string($db, $username);
        $password = mysqli_real_escape_string($db, $password);

        // Look in users table and filter those who have same username is the one inputed.
        $sql = "SELECT id, username  FROM users WHERE username = '$username' and password = '$password' ";
        $query = mysqli_query($db, $sql);

        if($query)
        {
            if(mysqli_num_rows($query) == 1)
            {
                $_SESSION['username']= $username;
                $_SESSION['id']= $id;
                header('Location: home.php');
            }
            else
            {
                echo "Incorrect details.Try again!";
            }
        }
        else
        {
            echo "Error in query";
        }
    }   
?>


<div id="loginform">
    <form id="login" action="loginAuth.php" method="POST">
        <label for="login"><strong>Login:</strong></label>
        <input type="text"size=20 autocorrect=off autocapitalize=words name="user">
        <label for="loginPassword" name="password"> <strong>Password:</strong></label>
        <input type="password" name="Password">
        <input name="loginSub" type="submit" value="login"> 
    </form> 
</div>