Simple login returning blank page

Question

I'm learning PHP and I have made a simple login script but the problem is that it only redirects me to a blank page. It's meant to redirect to index.php if user credentials are correct but this is apparently not the case? There are also validation so that if the user enters blank, an error is returned. This doesn't appear to have been executed.

login.php

<form id="login-form" method="post" action="logininc.php"> <fieldset> 
  <legend>Login </legend> 
  <p>Please enter your username and password to access the administrator's panel</p>

   <label for="user"> <input type="text" name="user" placeholder="Type your username here" id="user" /></label> 
   <label for="password"> <input type="password" name="password" placeholder="Type your password here" id="password" /></label>
   <label for="submit"> <input type="submit" class="btn btn-primary"name="submit" id="submit" value="Login" /> </label> </fieldset> </form>

logininc.php // my processing page

<?php

require_once("assets/configs/db_config.php");
$user=$_POST['user']; 
$password=$_POST['password'];

if(isset($_POST['login']))
{
//To ensure that none of the fields are blank when submitting the form if
if($user || $password != NULL)
    {
        $user = stripslashes($user);
        $password = stripslashes($password);
        $user = mysqli_real_escape_string($user);
        $password = mysqli_real_escape_string($password);

        $sql="SELECT * FROM $test_db WHERE user='$user' and password='$password'";
        $result=mysqli_query($sql);

        $row=mysql_fetch_array($result);

        if($row['user'] == $user && $row['password'] == $password)
        {
            session_start();
            $_SESSION['user'] = $user;
            $_SESSION['password'] = $password;
            $_SESSION['loggedin'] = "true";
            header("location:index.php");
        }
        else
        {
            print ('<div id="error">Computer says no.</div>');

        }
            print ('<div id="error">Enter something!</div>');

}
}



    ?>

index.php // success page

 <?php //module to check logins
 session_start();

 if(!isset($_SESSION["loggedIn"])){
     header("Location: login.php");
     exit;
 }
 Echo 'Congratulations <b>'.$_SESSION['user'].'</b> you successfully logged in!!<br />
         Your Password is: <b>'.$_SESSION['password'].'</b><br />
         <a href="login.php">Logout</a>';
 ?>

Proper answers were already given, but wanted to comment on your check. Your conditional is if $user || $password != NULL. That's the equivalent of saying if $user returns true OR if $password isn't NULL. So it does not check if both $user and $password are filled in. You need two separate conditions connected by &&. (&& = both have to return true, || is either one of them has to return true) So it should be ($user != NULL && $password != NULL) — vanamerongen, Jul 24 '13 at 09:05
Great code for an [SQL injection](http://stackoverflow.com/questions/60174/how-to-prevent-sql-injection-in-php). — jor, Jul 24 '13 at 09:08

qsi · Answer 1 · 2013-07-25T09:33:06.423

$row = mysql_fetch_array should be $row = mysqli_fetch_array

and as the others have already mentioned, use

if(isset($_POST['user']) && isset($_POST['password'])) {


// your code here


}

and btw: using a session where you only say "loggedin = true", or "login = yes", etc. is anything but secure

EDIT (security discussion):

passwords should always be saved encrypted (registration):

function login($email, $password)   {
    $email = mysql_real_escape_string($email);
    $q = "SELECT id, email, password, salt FROM members WHERE email='" . $email . "'";
    $result = mysql_query($q, $this->connection);
        $output = mysql_fetch_assoc($result);
        $user_id = $output['id'];
        $database_username = $output['username'];
        $database_email = $output['email'];
        $database_password = $output['password'];


        $password = hash('sha512', $password);

            if($database_password == $password) {
                $user_browser = $_SERVER['HTTP_USER_AGENT']; 
                $user_id = preg_replace("/[^0-9]+/", "", $user_id);
                $_SESSION['user_id'] = $user_id;
                $_SESSION['username'] = $email;
                $login_hash = hash('sha512', $password.$user_browser);
                $_SESSION['login_hash'] = $login_hash;
        }   else    {
            return false;
        }
} // function

function login_check()  {
    $user_id = $_SESSION["user_id"];
    $login_hash = $_SESSION["login_hash"];
    $email = $_SESSION["username"];
    $user_browser = $_SERVER['HTTP_USER_AGENT'];

    $q = "SELECT password FROM members WHERE id ='" . $user_id . "'";
    $result = mysql_query($q, $this->connection);
    $output = mysql_fetch_assoc($result);
    $database_password = $output['password'];

    if(mysql_num_rows($result) == 1)    {

        $login_check = hash('sha512', $database_password.$user_browser);
        if($login_check == $login_hash) {
                return true;
            } else { 
                return false; 
        }
    } else  {
        return false; 
    }
}

In addition you could create a random salt (registration) for each user, to set your security level even a bit higher (Note: hash(hash(hash(...))) lowers your security level since you lose information during a hash process)

NOTE: This is just a (working) example login/-check script with a high security level. Still this script can be improved (bruteforce,mysqli/prepared statements,hashing passwords directly in forms,secure session, ...)

Someone could log into your website by session manipulation or session hijacking, since the information in your session that is checked only says "true", it doesn't bind to any user at all. — qsi, Jul 24 '13 at 09:47
I agree on the session hijacking, but how should one manipulate the session object which is stored on the server (by default) ? — DarkBee, Jul 24 '13 at 09:56
Indeed sessions can't just be set by a user himself. The data for $_SESSION is kept on the server itself, and it's looked up by a key stored as a cookie, but if you are assigning sessions directly by unfiltered user input, one could set $_session['loggedin'] = true. So I suggest to always make things perhaps a little bit too secure, e.g. use secure information to check for the logged in state — qsi, Jul 24 '13 at 10:11
I see and what would u consider secure information ? Or what session would u set to keep track of an user that is logged on ? — DarkBee, Jul 25 '13 at 08:08
I see. Seems bit like the same approach I use to store a `remember me`-cookie on the clientside. Never considerd to do the same for validating my sessions. Thx for the information +1 — DarkBee, Jul 25 '13 at 09:53
FYI [password_hash](http://be2.php.net/password_hash) for php5.5.X and [password_hash from irc_maxell](https://github.com/ircmaxell/password_compat) for PHP5.3.7+ — DarkBee, Jul 25 '13 at 17:51

score 1 · Accepted Answer · answered Jul 24 '13 at 09:04

1

What happens when you change if(isset($_POST['login'])) to if(isset($_POST['submit']))?

answered Jul 24 '13 at 09:04

Simon King

453
3
9

score 0 · Answer 3 · answered Jul 24 '13 at 09:03

The problem is if(isset($_POST['login']))

You never set a "login" entry in your form.

You can do:

if(isset($_POSt["user"]) && isset($_POST["password"])) {
$user=$_POST['user']; 
$password=$_POST['password'];
//To ensure that none of the fields are blank when submitting the form if
if($user && $password) {

score 0 · Answer 4 · edited Jul 24 '13 at 10:36

First of all the name of your submit button is "submit". And you are checking if the "login" is post or not.

if(isset($_POST['login']))
{

it should have been:

if(isset($_POST['submit']))
{

You have written :

if($user || $password != NULL)
{

it should have been:

if($user != NULL || $password != NULL)
{

You have used mysqli and mysql command which is not a good practice

$result=mysqli_query($sql);

    $row=mysql_fetch_array($result);

Instead of

if($row['user'] == $user && $row['password'] == $password)
    {
//this code again check for condition which is already checked in the sql statement

it is better practice to write :

if($row->num_rows==1)
    {

in index.php you have written

if(!isset($_SESSION["loggedIn"])){

it should have been

if(!isset($_SESSION["loggedin"])){

since you have stored the lowercase index while storing into session.

Simple login returning blank page

4 Answers4

Linked

Related