-2

So I am making a simple login page in PHP. I have 2 tables, one for Staff (admin, manager, etc...) and one for Customer, both have 3 columns in common: username, password, role. When the user login, role will be set in session to redirect the user to the appropriate page. Currently my code is as below:

function.php

function queryMySQL($query)
{
    global $conn;
    $result = $conn->query($query);
    if(!$result)
    {
        die($conn->error);
    }
    return $result;
}
function  passwordToToken($password)
{
    global $salt1;
    global $salt2;
    $token = hash("ripemd128", "$salt1$password$salt2");
    return $token;
}

login.php

<?php
require_once 'function.php'
$user = $_POST['user'];
$pass = $_POST['pass'];
$token = passwordToToken($pass); //encrypt password
$query = "Select userId, username, password, role 
          from users 
          where username = '$user' 
          and password = '$token' 
        union 
          Select customerId, username, password, role 
          from customer 
          where username = '$user' 
          and password = '$token'";
$result = queryMySQL($query); //function return mysqli_query
if($result->num_rows == 0)
{
    $error = "Username/password is invalid";
    echo $error;
} else {
    session_start();
    $_SESSION['uId'] = mysqli_fetch_array($result)[0];
    $_SESSION['user'] = $user;
    $_SESSION['pass'] = $pass;
    $role = mysqli_fetch_array($result)[3];
    if($role != 'admin' || $role != 'staff' || $role != 'manager')
    {
        $_SESSION['role'] = 'customer';
    } else {
        $_SESSION['role'] = $role;
    }
    echo $role;
}
?>

My problem is that when the user login correctly, the $role variable is blank, however if I echo the $_SESSION['uId'] it does return result (the userID), and even if I change the $_SESSION['uId'] value to mysqli_fetch_array($result)[3] I still get the correct result when echo (the user role).

Qirel
  • 25,449
  • 7
  • 45
  • 62
user3676506
  • 102
  • 10
  • What does `queryMySQL()` do? Thats not vanilla PHP so it could be doing anything?? – RiggsFolly Jun 26 '19 at 13:15
  • You really should have ONE Table with a column indicating that this row is a User or a Customer. – RiggsFolly Jun 26 '19 at 13:17
  • Password does not belong or need to be in the session – RiggsFolly Jun 26 '19 at 13:18
  • The second call to `mysqli_fetch_array()` will most likely NEVER have any data, because I assume the user is EITHER a user or a customer but not both. Remember `mysqli_fetch_array()` consumes a complete row from the resultset – RiggsFolly Jun 26 '19 at 13:19
  • 1
    Please dont __roll your own__ password hashing. 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. – RiggsFolly Jun 26 '19 at 13:21
  • I have updated my code for the 2 function `queryMySQL` and `passwordToToken`, but it seem that my understanding of `mysqli_fetch_array()` was wrong. I thought it return an array and we can directly access it like in C#. Also the password is just temporary there for me to check. I will remove it later. Thank you for clarifying. – user3676506 Jun 26 '19 at 13:31

2 Answers2

2

Your problem is that you keep trying to fetch more results. Each time you call mysqli_fetch_array() you ask for the next row - but you only expect one.

Instead, assign the row to a variable $row and use that where you need the columns.

Also, I've changed your role-checks to simplify it a bit. Your current logic was slightly incorrect, as it would always be a customer.

session_start();
$row =  mysqli_fetch_array($result);
$_SESSION['uId'] = $row[0];
$_SESSION['user'] = $user;
$_SESSION['pass'] = $pass;
$role = $row[3];
if (!in_array($role, ['admin', 'staff', 'manager']))
{
    $_SESSION['role'] = 'customer';
} else {
    $_SESSION['role'] = $role;
}
echo $role;

That said, you probably can alter your approach to have one table users, and a column to indicate what role they have. No need to have separate tables.

You should also use a prepared statement, and proper hash of your passwords. See How to use password_hash and How can I prevent SQL injection in PHP?

Qirel
  • 25,449
  • 7
  • 45
  • 62
  • Thank you for your answer. It seem that my understanding of `mysqli_fetch_array()` was wrong. I thought it return an array and we can directly access it like in C#. I am still a beginner at PHP so I might have mixed up with other language. – user3676506 Jun 26 '19 at 13:34
0

Propably just one row is returned from query and you are calling mysqli_fetch_array twice so the second call returns null.

BTW shouldn't that be if($role != 'admin' && $role != 'staff' && $role != 'manager')?