-1

Hello on my first post!

Here I am trying to make a very simple login form because in a few weeks I have an assignment where I need to create something similar and I wanted to know the basics first. Now I figured this question might make you slap your head by it's easy solution but I just can't seem to figure it out myself.

So what I tried to do was just put my POST values in a MySQL query, and when a user is found display it in a while loop or else display the a warning but my code only result in an empty space.

I will spare you the details about the index.php because it really is just a POST form.

<!DOCTYPE html>
<html>
<head>
    <title></title>
    <link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>

<div id="wrapper">
<?php

include "connect.php";

$query = "SELECT * FROM members WHERE username = '".mysql_real_escape_string($_POST['username'])."' AND password = '".mysql_real_escape_string($_POST['password'])."'";
$result = mysql_query($query);

 if (mysql_fetch_assoc($result) > 0) {
    while ($row = mysql_fetch_assoc($result)) {
        $username = $row['username'];
        $password = $row['password'];

    ?><h3>Username - <?php echo $username; ?></h3><?php
        ?><h3>Password - <?php echo $password; ?></h3><?php

        unset($_POST['username']);
    unset($_POST['password']);
    } // endwhile
} // endif
else {
    ?><h3>Wrong Username of Password. Try again.</h3><?php

    unset($_POST['username']);
    unset($_POST['password']);
}
?>
<input type="button" name="back" value="Back" onclick="location.href='index.php'">
</div>

</body>
</html>
Roy
  • 11
  • 1
  • This is totally wrong! Don't try to build a login system if you have no idea how to do so! Your application can be hacked in milliseconds. – Sliq Feb 20 '14 at 16:51
  • I'm getting a definite sense of duja vu here - see : http://stackoverflow.com/questions/21858047/php-login-form-with-html-form/21859859 (as the most recent example I've encountered of a virtually identical question) – CD001 Feb 20 '14 at 17:00
  • Hello @Roy and welcome to SO. When posting questions here, please give full information regarding: what are you trying to achieve, what have you tried so far(i.e. code), what's wrong (i.e. error messages, wrong program behavior, etc). – tftd Feb 20 '14 at 17:00

2 Answers2

1

You should avoid using the mysql_* functions as they are deprecated and will not be supported any more! Please consider using PDO instead, as it provides a common way to connect to all types of databases. Mysqli is also an option, but that limits you to just MySQL.

In your code example, I see you're using plaintext passwords. This is considered to be extremely bad practice - it provides an easy way of people who have access to your database to know the passwords of all users of the application. You should consider using a hash of some kind, i.e md5 or sha1 to secure your user's passwords. It is a good idea to add a password salt to the equation to make things a bit harder for hackers to crack.

Here is an example of how your code should look using PDO

// connect.php
$db_host = '127.0.0.1';
$db_user = 'user';
$db_pass = 'pass';
$db_name = 'database_name';
$db = new PDO('mysql:host='.$db_host.';dbname='.$db_name, $db_user, $db_pass);

// login.php 
<!DOCTYPE html>
<html>
<head>
    <title></title>
    <link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>

<div id="wrapper">
    <?php

    include('connect.php');

    // Using prepared statements almost eliminates the possibility of SQL Injection.
    $preparedQuery = $db->prepare("SELECT * FROM members WHERE username = :username AND password = :password");
    $preparedQuery->bindValue(":username", $_POST['username']);
    $preparedQuery->bindValue(":password", sha1($_POST['password']));
    $preparedQuery->execute();

    // Retrieve the results from the database
    $user = $preparedQuery->fetch(PDO::FETCH_ASSOC);

    // If there is a user record print the user & pass... 
    if($user != ""){
        $message = "<h3>Username - ".$user['username']."</h3>";
        $message .= "<h3>Password - ".$user['password']."</h3>";
        // start user session?
    } else {
        $message = "<h3>Wrong Username of Password. Try again.</h3>";
        unset( $_POST[ 'username' ] );
        unset( $_POST[ 'password' ] );
    }
    ?>
    <?php echo $message; ?>
    <input type="button" name="back" value="Back" onclick="location.href='index.php'">
</div>

</body>
</html>
tftd
  • 16,203
  • 11
  • 62
  • 106
  • Thank you very much, while researching I today I discovered that the mysql_* functions are about to lose their support like you said so I will follow your advice and use PDO! – Roy Feb 20 '14 at 17:35
0

you need to use mysql_num_rows(), Try this,

if (mysql_num_rows($result) > 0) {

instead of

if (mysql_fetch_assoc($result) > 0) {

NOTE: Use mysqli_* functions or PDO instead of mysql_* functions(deprecated)

Krish R
  • 22,583
  • 7
  • 50
  • 59