Help with simple login script

Hi all,

I'm quite new to PHP still so as an exercise I moved a script (which worked) into a function. I've been working at it a bit and I'm going being nailed by

Warning: mysqli_fetch_array() expects parameter 1 to be mysqli_result, object given in : $row = mysqli_fetch_array($this->conn, $result);

This is my full function

function logcon($user, $password )
{
   
  $esc_user = mysqli_real_escape_string($this->conn, $user);
  $esc_password = mysqli_real_escape_string($this->conn,$password); 
$sql = "SELECT * ALL from USERS WHERE username  ='{$user}' AND password='{$password}'";
 
$result = mysqli_query($this->conn, $sql);
$row = mysqli_fetch_array($this->conn, $result);
            return $row;
            }

Code (markup):

This is my login page (minus CSS and form)

if(isset($_POST['submit'])){
 
$user=$_POST['user'];
$password=$_POST['password'];
 
 
//To ensure that none of the fields are blank when submitting the form if
if(isset($_POST['user']) && isset($_POST['password']))
    {   
   
   
$user = stripslashes($user);
$password = stripslashes($password);
$db1=new dbmember();
$db1->openDB();               
$result=$db1->logcon($user, $password);
 
 
if($row[0]==1)
{
    session_start();
    $_SESSION['user'] = $user;
    $_SESSION['password'] = $password;
    $_SESSION['loggedin'] = "true";
    header("location:index.php");
}

PHP:

Any thoughts on where I'm going wrong? Thanks.

 

Only pass the result into mysqli_fetch_array(); You don't need the connection.

Also in your query I believe 'SELECT * ALL' should be 'SELECT *' and

on your login page on line 19 you need to change $row to $result.

 

You have the following fixes that need to be done :
1)
mysqli_fetch_array() takes only 2 parameters , first : a result set identifier which in your case is returned by mysqli_query() , second : the type of result array you want , which is a constant.
You are passing the connection link to mysqli_fetch_array() which does not know how to interpret it.

So change mysqli_fetch_array($this->conn, $result) to mysqli_fetch_array($result).

2)
You have another error , which is your MySQL query syntax. * means ALL and you have SELECT * ALL... , so change it to SELECT ALL.

So this should be your code after editing :

function logcon($user, $password )
{
 
  $esc_user = mysqli_real_escape_string($this->conn, $user);
  $esc_password = mysqli_real_escape_string($this->conn,$password);
$sql = "SELECT * from USERS WHERE username  ='{$user}' AND password='{$password}'";
$result = mysqli_query($this->conn, $sql);
$row = mysqli_fetch_array($result);
            return $row;
            }

Code (markup):

 

Also, you shouldn't be selecting * for security reasons. Never pull out the password in a query. You shouldn't pull out * anyway in any query unless you intend to use all of the data. In the above case you should just select ID to make sure the user exists.

function logcon($user, $password )
{
  $esc_user = mysqli_real_escape_string($this->conn, $user);
  $esc_password = mysqli_real_escape_string($this->conn,$password);
$sql = "SELECT id from USERS WHERE username  ='{$user}' AND password='{$password}'";
$result = mysqli_query($this->conn, $sql);
while ($row = mysqli_fetch_array($result)){
            $loggedin = true;
            }

PHP:

I'm assuming that you've already hashed the password but that may not be the case otherwise you'd have already escaped it before this function...

 

You seem to be escaping your user and password input but not using those values in your query.
Think about changing the WHERE clause of your query to:

WHERE username  ='{$esc_user}' AND password='{$esc_password}'

Code (markup):

These are the variables you assigned the escaped values of the user input to.

 

escaping the inputs is useless. You should always your prepared statement for 100% safety, again sql injections