Login page using PHP and MYSQL based on user role

Question

I created a login page.

I want customer and manager to login from this same page. I separated customer and manager in db by their role "","manager" respectively (customer represented as null value).

I wrote this code but its not working as i expected. When I type name and password and press login button it doesn't go to desired page rather it remains on login page.

<?php
session_start();

$db=mysqli_connect("localhost","root","","authentication");

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

    $customer_name = mysql_real_escape_string($_POST['customer_name']);
    $password = mysql_real_escape_string($_POST['password']);
    $password=md5($password);
    if(empty($customer_name)&&empty($password)) {
        $error="Fields are Mandatory";
    } else {
        $sql="SELECT * 
              FROM customers 
              WHERE customer_name='$customer_name' AND password='$password'";
        $result=mysqli_query($db,$sql);
        $row=mysqli_fetch_array($result);
        $_SESSION['id']=$row['id'];
        $_SESSION['role']=$row['role'];
        $count=mysqli_num_rows($result);

        if($count==1){
            if ($row['role']=="manager"){
                header("location:manager.php");      
            }
            else if($row['role']==""){
                header("location:ueser.php");      
            }
        }else{
            $error='Customer_name/password combination incorrect';
        }
    }
}
?>

This doesn't seem smart. You never want a customer to be able to log in as a manager, not even if someone makes a stupid mistake. Seperating the login pages, routines, and data tables, for customers and managers seems wise. It also allows you to have two different login methods, where you could, for instance, use two factor authentication for managers. (me mutters: "SQL-injection") — KIKO Software, Jul 10 '18 at 13:08
Your script is wide open to [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) Use [prepared parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) in either the `MYSQLI_` or `PDO` API's — RiggsFolly, Jul 10 '18 at 13:25
Please dont __roll your own__ password hashing specially not using MD5(). PHP provides [`password_hash()`](http://php.net/manual/en/function.password-hash.php) and [`password_verify()`](http://php.net/manual/en/function.password-verify.php) please use them. And here are some [good ideas about passwords](https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet) If you are using a PHP version prior to 5.5 [there is a compatibility pack available here](https://github.com/ircmaxell/password_compat) — RiggsFolly, Jul 10 '18 at 13:25
You are also setting up the SESSION before you check if the query actually returned anything — RiggsFolly, Jul 10 '18 at 13:26

score 1 · Answer 1 · answered Jul 10 '18 at 14:38

There is a lot to be desired code wise. I left some additional comments where you could better this code. I think your main issue is that you need to use the header redirect a bit different. The L in Location I believe needs to be capital and should be Location: url white space needed and the page needs to be a url.

Also I would use a switch over if else for when more roles are potentially added. HOWEVER doing this redirect is useless if you do not check that the user accessing the page has the rights to be there. Authentication vs Authorization.

<?php

session_start();

$db=mysqli_connect("localhost","root","","authentication");

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

    if(empty($_POST['customer_name'])&&empty($_POST['password'])) {
        $error="Fields are Mandatory";
    } else {

        //You cannot mess with them before check if empty md5 will hash a space and null

        //Use prepared statements and the mysql_real_escape_string is not needed
        $customer_name = mysql_real_escape_string();

        //I really am not sure that mysql_real_escape_string before your hash is a good idea...
        //You will be hashing the escaped version of it.
        $password = mysql_real_escape_string($_POST['password']);

        //This is not secure either check out.
        //http://php.net/manual/en/function.password-hash.php
        //http://php.net/manual/en/function.password-verify.php
        $password=md5($password);

        //http://php.net/manual/en/mysqli.quickstart.prepared-statements.php
        $sql="SELECT * 
              FROM customers 
              WHERE customer_name='$customer_name' AND password='$password'";

        //You can drop the password from the query and check what is returned
        //In the results with using password_verify)
        $result=mysqli_query($db,$sql);

        //Check this upfront and do work if you meet it.
        //I assume that username is unique so you should
        //Want to do something if one is returned
        if(mysqli_num_rows($result) == 1){;
            $row=mysqli_fetch_array($result);
            $_SESSION['id']=$row['id'];
            $_SESSION['role']=$row['role'];

            //For the page I would use a switch
            switch ($_SESSION['role']) {
                case "manager":
                    //Capital L and a space
                    //Also it should be a url so example
                    header("Location: http://localhost/manager.php"); 
                    break;
                default:
                    //Capital L and a space
                    //Also it should be a url so example
                    header("Location: http://localhost/user.php"); 
                    break;
            }
        }else{
            $error='Customer_name/password combination incorrect';
        }
    }
}

score 0 · Answer 2 · answered Jul 10 '18 at 13:07

0

Use elseif instead else if

/* Incorrect Method: */
if ($a > $b):
    echo $a." is greater than ".$b;
else if ($a == $b): // Will not compile.
    echo "The above line causes a parse error.";
endif;


/* Correct Method: */
if ($a > $b):
    echo $a." is greater than ".$b;
elseif ($a == $b): // Note the combination of the words.
    echo $a." equals ".$b;
else:
    echo $a." is neither greater than or equal to ".$b;
endif;

For More http://php.net/manual/en/control-structures.elseif.php#control-structures.elseif

answered Jul 10 '18 at 13:07

TarangP

2,682
5
20
37

1

It would be much more helpful if you answered the question using the OP's information rather than just copying pieces of the PHP manual – RiggsFolly Jul 10 '18 at 13:24
Likewise using the same format that they used instead of the `:` and `endif` – nerdlyist Jul 10 '18 at 13:38
`Use elseif instead else if` – TarangP Jul 10 '18 at 13:57

Login page using PHP and MYSQL based on user role

2 Answers2