0

I was trying to make a login page in php connected with database of mysql.. below is the html code of the page for login where values are entered and then directed to a second page of php where they are checked..

<html>
<head>
    <title>Library Login</title>
    <link rel="stylesheet" type="text/css" href="css/reset.css">
    <link rel="stylesheet" type="text/css" href="css/structure.css">
</head>
<body>
    <form class="box login" method="GET" action="http://localhost/redirect.php">
        <label align="center"><font size="6" color="grey">Library System</font></label>
        <fieldset class="boxBody">
        <label>Username</label>
        <input type="text"  placeholder="Username" required name="username">
        <label><a href="#" class="rLink" tabindex="5" ></a>Password</label>
        <input type="password" placeholder="Password" required name="password">
        <input type="submit" class="btnLogin" value="Login"  name="login">
        <input type="reset" class="btnLogin" value="Reset"  name="reset" >
        <label>
    </form>
</html>
</div>

And below is the code for second page where only else condition is executed whatever entry is input... I am new to Php and Mysql... Please help me out...

<?php
$con=mysqli_connect("localhost","root","","project");

if(mysqli_connect_errno())
{
    echo "failed".mysqli_connect_errno();
}

$uid=$_GET['username'];
$pass=$_GET['password'];
$sql="SELECT *FROM login";
$result=mysqli_query($con,$sql);

while($data=mysqli_fetch_array($result))
{
    if($uid==$data['user'] and $pass==$data['pass'])
    {
        header('location:http://localhost/error/index.html');
    }
    else
    {
        header('location:http://localhost/mam.html');
    }
}

mysqli_close($con);
?>
sampathsris
  • 21,564
  • 12
  • 71
  • 98
beginner
  • 13
  • 3
  • No.. Every time else condition is executed while checking the value irrespective of the fact that i have entered correct input or not – beginner Jul 22 '14 at 23:08
  • I know it's not an answer but you really should try and move to [PDO](http://php.net/manual/en/book.pdo.php) instead of mysqli. – pid Jul 22 '14 at 23:08
  • 1
    If the first row of the result does not match, then you will be in the else part and with header() call you are away to mam.html and don't get back to this script. You should use a WHERE clause to get the relevant row, if this exists. And you should read about prepared statements with placeholders before you put your input values into the WHERE clause. – VMai Jul 22 '14 at 23:10
  • 3
    Add error reporting to the top of your file(s) `error_reporting(E_ALL); ini_set('display_errors', 1);` see if it yields anything. – Funk Forty Niner Jul 22 '14 at 23:12
  • @pid: can you argue why to use PDO instead of mysqli? – Cristian Ciocău Jul 22 '14 at 23:13
  • Sidenote: Considering you're using plaintext as password storage (which is highly discouraged), you're best using POST instead of GET, not to mention an up-to-date (password) hashing algo such as [**CRYPT_BLOWFISH**](http://security.stackexchange.com/q/36471) or PHP 5.5's [`password_hash()`](http://www.php.net/manual/en/function.password-hash.php) function. For PHP < 5.5 use the [`password_hash() compatibility pack`](https://github.com/ircmaxell/password_compat). – Funk Forty Niner Jul 22 '14 at 23:16
  • @beginner The order of a result is undefined if there's no ORDER BY clause. You can't be sure that the row with your input data will be the first one in the query result. – VMai Jul 22 '14 at 23:17
  • 1
    @ceakki: PDO is an abstraction layer helping to reason on a higher level while mysqli is implementation-specific and creates coupling with MySQL implementation details. Generally, in the open source community mysqli is considered superseded by PDO. Also look here: http://stackoverflow.com/questions/13569/mysqli-or-pdo-what-are-the-pros-and-cons – pid Jul 22 '14 at 23:17
  • This is an excellent comparison between PDO and mysqli: http://code.tutsplus.com/tutorials/pdo-vs-mysqli-which-should-you-use--net-24059 – pid Jul 22 '14 at 23:19
  • Off topic: interesting to see unclosed `body` and `fieldset` tags, empty `label` tags, and `div` tags after the closing `html` tag. – sampathsris Jul 22 '14 at 23:20
  • @Krumia: also the `` tag. It's since XHTML I don't see someone using that tag... – pid Jul 22 '14 at 23:23

1 Answers1

0

OK, as you are dealing with authentication, let's improve your code a little.

<?php

// Do not connect using root, especially when not setting a password:
$con=mysqli_connect("localhost","projectuser","password","project");
if(mysqli_connect_errno())
{
echo "failed".mysqli_connect_errno();
}

$uid = $_GET['username'];
$pass = $_GET['password'];

// This is the main problem, there was a typo:
$sql = "SELECT * FROM login";

// Directly ask the DB if the credentials are correct.
// Then you do not need the loop below.
// BUT: Do not forget to escape the data in this case!
$sql .= " WHERE uid = '" . mysqli_real_escape_string($uid) . "' AND pass = '" . mysqli_real_escape_string($pass) . "'";

$result=mysqli_query($con,$sql);
if ($result->num_rows === 1) {
    header('location:http://localhost/mam.html');
} else {
    header('location:http://localhost/error/index.html');
}
mysqli_close($con);
?>

A further improvement would be to hash (and salt) the password in the database.

Also, as VMai pointed out, the use of prepared statements would be appropriate.

brainbowler
  • 667
  • 5
  • 17
  • 1
    `mysql_real_escape_string` that's a `mysql_` function; careful ;-) – Funk Forty Niner Jul 22 '14 at 23:18
  • You're mixing mysqli_* functions (used by OP) with mysql_* functions. That won't work. Please improve the code in the lines of mysqli with a prepared statement and placeholders and bind the inputs to the placeholders instead of string concatenation. – VMai Jul 22 '14 at 23:18
  • Missed the i, fixed that. However, it would work in case both modules are available - at least for the escaping. – brainbowler Jul 22 '14 at 23:20
  • 1
    *"However, it would work in case both modules are available"* - Can you elaborate on that? `mysql_` and `mysqli_` do not mix together inside the same page/code used. Those are seperate MySQL APIs, as is PDO. – Funk Forty Niner Jul 22 '14 at 23:24