How to check username and password matches the database values

Question

I'm really sorry if the question looks silly. But I've been trying for days to check my username and password in the database matches what I'm typing in the html page... This is my Login form...

<form method="POST" action="Dashboard/Dashboard.php">

    <div class="form-group md-form">
        <!--<input type="email" class="form-control" id="email" value="" placeholder="Enter email address">-->
        <i class="fa fa-user prefix grey-text"></i>
        <input name="username" id="username" type="text" class="form-control" required>
        <label for="defaultForm-email">Username</label>
    </div>
    <div class="form-group md-form">
        <!--<input type="password" class="form-control" id="password" value="" placeholder="Enter password">-->
        <i class="fa fa-lock prefix grey-text"></i>
        <input name="password" id="password" type="password"  class="form-control" required>
        <label for="defaultForm-pass">Your password</label>
    </div>
    <div class="text-center">
        <button type="reset" class="btn btn-amber btn-sm"><strong>Reset</strong></button>
        <input type="submit" name="submit" id="submit" class="btn btn-green btn-sm" value="Sign in">                                           
    </div>

</form>

And this is the code(php) I'm using in Dashboard.php

<?php
    $servername = "localhost";
    $username = "root";
    $password = "";
    $databaseName = "test";

    $conn = mysqli_connect($servername, $username, $password, $databaseName);

    $un = $_POST['username'];
    $pw = $_POST['password'];
    print $pass . "_" . $email;

    $query = mysqli_query($conn, "SELECT log_username,log_password FROM login WHERE log_username='$un' AND log_password='$pw'");

    $result_can = mysqli_query($conn, $query);


    while ($row = mysql_fetch_assoc($result_can)) {


        $check_username = $row['username'];
        $check_password = $row['password'];
    }
    if ($un == $check_username && $pw == $check_password) {
        $message = "ok";
        echo "<script type='text/javascript'>alert('$message');</script>";
        header("Location: Doctors.php");
    } else {
        $message = "No";
        echo "<script type='text/javascript'>alert('$message');</script>";
        header("Location: Doctors.php");
    }
    ?>

I really tried like thousands of times, but couldn't figure out where I went wrong... Can anyone please help me?

I know my code is open to SQL injection, but I don't care about it as this is a example I needed to show to my friends So neglect that part.

Answer 1

Stack Overflow is for "professional and enthusiast programmers." With respect, you've shown us code in your question that isn't even close to being worthy of either name. It's grossly insecure, and if you put it on the public internet, your site will be cracked by cybercriminals.

StackOverflow people don't have much of a sense of humor about bad security code. You get strong reactions to code like yours because, well, Equifax, and Ashley Madison, and Adobe, and all the rest of the places that have been cracked by cybercriminals. Why do we jump on you? Because we don't like cybercriminals and we don't want to make life easy for them. Friends don't let friends do bad password security. Friends don't show friends grossly insecure password-validation code.

What's wrong with your code? You're storing passwords as plain text, and you're vulnerable to SQL injection. I will address the first of these issues.

Fortunately, php has outstanding industry-leading facilities to do password security. Read about them here. http://php.net/manual/en/faq.passwords.php Use them. How do you handle passwords?

1. When a user registers on your site and first presents a password, you hash it, in your code running on your server, something like this.

  $usersPassword = $_POST['password']);
  $hash = password_hash( $usersPassword , PASSWORD_DEFAULT );
  // you then store the username and the hash in your dbms. 
  // the column holding the hash should be VARCHAR(255) for future-proofing
  // NEVER! store the plain text (unhashed) password in your database

2. When a user tries to log in, you do a query like this on your server:

    SELECT log_password FROM log_user WHERE log_username = TheUsernameGiven
   

   You then put the retrieved password into a variable named $hash.

   You then use php's password_verify() function, again on your server, to check whether the password your would-be user just gave you matches the password in your database.

   Finally, on your server you check whether the user's password needs to be rehashed, because the method you used previously to hash it has become obsolete.

 $usersPassword = $_POST['password']);
 $valid = password_verify ( $usersPassword, $hash );
 if ( $valid ) {
   if ( password_needs_rehash ( $hash, PASSWORD_DEFAULT ) ) {
     $newHash = password_hash( $usersPassword, PASSWORD_DEFAULT );
     /* UPDATE the user's row in `log_user` to store $newHash */
   }
   /* log the user in, have fun! */
 }
 else {
  /* tell the would-be user the username/password combo is invalid */
 }

This sequence is futureproof, because it can rehash passwords later if the old hashing method gets too easy for cybercreeps to crack. Many user accounts have lifetimes far longer than versions of packages like php.

For credentials like passwords to remain secret, you must use https, not http, to connect between browser and server. Otherwise cybercriminals can intercept the traffic from your user to your server and grab her password. It can be a pain in the xxx neck to rig up an https-enabled server, but it's a critical part of deploying a web application. (Services like Heroku allow you to test your apps with https easily.)

Answer 2

Several problems, some were mentioned by comments above.

Mixing mysql_* vs. mysqli_* API

You call the query with mysqli_query() but you try to fetch results with mysql_fetch_assoc(). You can't mix these different APIs. The mysql_* functions will not use the connection you opened with mysqli_connect(), and vice-versa. Pick one MySQL extension and stick with it.

Hint: Don't use mysql_* at all. It's deprecated, and has been removed from PHP 7.0+

Querying with conditions for both username and password

Just search for the username, and fetch the password. If you search for both, then the search will return zero rows, unless the correct password was used.

You don't want that. You want to avoid putting the plaintext password in the SQL query. Just search on the username, and fetch the stored password and then compare what you fetch to the user input password.

Uninitialized variables

If you fetch zero rows from your query, then $check_username and $check_password are never set. Then you compare those variables in your if statement. Not a fatal error, but bad style.

No password hashing

You appear to be comparing the user input, which I assume is plaintext, directly to what's stored in the database. You're Probably Storing Passwords Incorrectly.

Instead, when you store your password, use password_hash() first.

No query parameters

I know you said you don't care about your SQL injection vulnerability, but this is like being an electrician and saying you don't care that your electrical panel is stuffed with oily rags. Be sure to post your disregard for safety on your LinkedIn profile, so employers know who to avoid.

Recommended implementation

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); // enable exceptions

$conn = new mysqli($servername, $mysql_username, $mysql_password, $databaseName);

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

$sql = "SELECT log_username, log_password_hash FROM login WHERE log_username=?";
$stmt = $conn->prepare($sql);
$stmt->bind_param('s', $log_username);
$stmt->execute();
$result = $stmt->get_result();

while ($row = $result->fetch_assoc()) {
    if (password_verify($log_password, $row['log_password_hash'])) {
        $message = "ok";
        // header must be called before any other output
        header("Location: Doctors.php");
        exit();
    }
}
$message = "No";
// header must be called before any other output
header("Location: Doctors.php");

Answer 3

There are several problems here, both in your code and in the thought process. Let's work our way down:

$un = $_POST['username'];
$pw = $_POST['password'];
print $pass . "_" . $email;

That print line should be giving you a warning. The variables $pass and $email do not exist. You should remove that line, unless what you were trying to do is to print $un and $pw instead.

$query = mysqli_query($conn, "SELECT log_username,log_password FROM login WHERE log_username='$un' AND log_password='$pw'");

There's no need to select both the username and password column. If there is a match, they will always be the same as $un and $pw, which you already have. You're only checking whether the username and password are correct or not, so selecting a single column is good enough. Preferably the user id, but only the username will be sufficient.

Keep in mind that -- assuming the query executes successfully -- $query will contain a mysqli_result object.

$result_can = mysqli_query($conn, $query);

This line needs to be removed. You have already executed your query and $query is its result, what you're doing here makes no sense and should be giving you a warning, or perhaps even a fatal error.

while ($row = mysql_fetch_assoc($result_can)) {
    $check_username = $row['username'];
    $check_password = $row['password'];
}
if ($un == $check_username && $pw == $check_password) {
    $message = "ok";
    echo "<script type='text/javascript'>alert('$message');</script>";
    header("Location: Doctors.php");
} else {
    $message = "No";
    echo "<script type='text/javascript'>alert('$message');</script>";
    header("Location: Doctors.php");
}

You cannot mix mysql_* and mysqli_* functions. Using mysql_fetch_assoc() here should give you a fatal error. You should use mysqli_fetch_assoc() instead (on $query instead of $result_can), however:

Since you're only interested in whether or not there was any result at all, this whole section can be changed to:

if (mysqli_num_rows($query) > 0) {
    $message = "ok";
    echo "<script type='text/javascript'>alert('$message');</script>";
    header("Location: Doctors.php");
} else {
    $message = "No";
    echo "<script type='text/javascript'>alert('$message');</script>";
    header("Location: Doctors.php");
}

This will pose other problems, because you cannot use header() to redirect the user after echo'ing your <script> tag (you will get a "headers already sent" error). If you want the Javascript alert, perform the redirect with Javascript as well. Also, that $message variable is rather useless, you might as well put the message directly into the alert:

if (mysqli_num_rows($query) > 0) {
    echo "<script type='text/javascript'>alert('ok'); window.location.href='Doctors.php';</script>";
} else {
    echo "<script type='text/javascript'>alert('No'); window.location.href='Doctors.php';</script>";
}

Once you fix all of these issues, you still have some thinking to do.

• You should never store passwords in plain text in your database.
• You may not care about SQL injection right now, but with your current query I can log in as any valid user (e.g. "admin") by typing their username as admin' AND 1 --, or if I just want access I can use a username of any' OR 1 -- and be logged in as the first user in your table. Look into prepared statements and how they work.
• You have no error handling at all. You should add checks to see if the database connection was opened successfully, the query executed properly, the form was posted and the username/password fields were filled in and think about how you want to present a useful error message to the user.

The main lesson here should be: when you're developing and it doesn't work, always check the error logs to see if it contains any hints and turn on PHP's error reporting features so you can see what you did wrong right in your browser.