-1

I recently integrated a basic login/register function on my website and need help understanding my login.php code. The basic tutorial I used for the login/register page didnt include password hashing which I am currently trying to integrate myself. Everything was working 100% as expected until I decided to add password hashing. I'm very new to php and how it works so I dont quite get why my code isn't working, but I understand the basic concept/workflow of hashing passwords which is answered in other questions.

A few notes: register is working fine with storing hashed passwords and database connection is successful. Also I have an api service that calls my login.php (code included) which could be the issue.

I tested my SQL querySELECT *, password FROM users where email='jim@gmail.com'and it successfully returns 1 user with all columns.

Edit: I understand this is not SQL injection proof, that is my next step. I am trying to do this correctly, but forgive me if my current code doesn't handle passwords correctly, it is part of the learning process. (this is not a live website)

Applicable Code: Login.php

<?php
include_once("database.php");
$postdata = file_get_contents("php://input");
$request = json_decode($postdata);
if(isset($postdata) && !empty($postdata)){
    $pwd = mysqli_real_escape_string($mysqli, trim($request->password));
    $email = mysqli_real_escape_string($mysqli, trim($request->username));
    $sql = "SELECT *, password FROM users where email='$email'";

    if($result = mysqli_query($mysqli,$sql)){
        $rows = array();
        while($row = mysqli_fetch_assoc($result)){
            $rows[] = $row;
        }
        if(password_verify($pwd, $rows['password'])){
            echo json_encode($rows);
        }
        
    }
    else{
        http_response_code(404);
    }
}
?>

I have tried to use both $row and $rows on line password_verify($pwd, $rows['password']) just in case I dont understand the retrieved object type.

Register.php

<?php
include_once("database.php");
$postdata = file_get_contents("php://input");
if(isset($postdata) && !empty($postdata)){
    $request = json_decode($postdata);
    $name = trim($request->name);
    $pwd = mysqli_real_escape_string($mysqli, trim($request->pwd));
    $email = mysqli_real_escape_string($mysqli, trim($request->email));

    $hash = password_hash($pwd, PASSWORD_BCRYPT);

    $sql = "INSERT INTO users(name,password,email) VALUES ('$name','$hash','$email')";
    if ($mysqli->query($sql) === TRUE) {
        $authdata = [
        'name' => $name,
        'pwd' => '',
        'email' => $email,
        'Id' => mysqli_insert_id($mysqli)
    ];
        echo json_encode($authdata);
    }
}
?>

api.service.ts (shortened)

public userlogin(username, password) {
        //alert(username)
        return this.httpClient.post<any>(this.baseUrl + '/login.php', { username, password })
        .pipe(map(Users => {
            this.setToken(Users[0].name);
            this.getLoggedInName.emit(true);
        return Users;
        }));
    }

    public userregistration(name,email,pwd) {
        return this.httpClient.post<any>(this.baseUrl + '/register.php', { name,email, pwd })
        .pipe(map(Users => {
        return Users;
        }));
    }

I dont completely understand how the api service is listening for output from login.php but it seems like echo json_encode($rows); line from login.php is the output?? Many thanks in advance for any tips, advice, or solutions!

Ian McCleary
  • 809
  • 1
  • 7
  • 14
  • 2
    First off, don't manipulate passwords in any way since this might be doing more harm than good, so remove the escaping function for it. Then check to see what your password column length is. If it isn't 60+ then you'd have to start over. That's the best I can offer so far. Try that. – Funk Forty Niner Aug 03 '20 at 23:36
  • @FunkFortyNiner To be honest I dont fully understand what escaping functions do. Doing something like `if(password_verify($request->password, $rows['password'])){` doesnt seem to work. Would $request->password refer to the typed password? Password column length is 255 to be safe. – Ian McCleary Aug 03 '20 at 23:52
  • 1
    **Warning:** You are wide open to [SQL Injections](https://stackoverflow.com/a/60496/1839439) and should use parameterized **prepared statements** instead of manually building your queries. They are provided by [PDO](https://php.net/manual/pdo.prepared-statements.php) or by [MySQLi](https://php.net/manual/mysqli.quickstart.prepared-statements.php). Never trust any kind of input! Even when your queries are executed only by trusted users, [you are still in risk of corrupting your data](http://bobby-tables.com/). [Escaping is not enough!](https://stackoverflow.com/q/5741187) – Dharman Aug 04 '20 at 00:23
  • @Dharman Thank you for the advice! I realize this is not SQL proof and that is my next step. First, I have decided to work out hashed password storing. – Ian McCleary Aug 04 '20 at 00:27

1 Answers1

0

Well, I figured it out...being new to php, I had a difficult time toubleshooting/debuging. It turns out that the data type for $rows would return null if I tried to reference $rows['password], but $rows is the correct datatype to return my User object. Anyone know why that is?

My solution is to fetch 2 results and create one for the password and one for the user object because running mysqli_fetch_assoc($result); multiple times attempts to fetch the next row in the result-set. I'm not sure if this is good programming practice so feel free to comment your thoughts on this method.

NOTE: THIS CODE IS NOT SQL INJECTION PROOF, DO NOT BLATANTLY IMPLEMENT WITHOUT FURTHER PDO STATEMENTS. AS OTHERS HAVE STATED, PASSWORDS SHOULD NOT BE HANDLED DIRECTLY IN THIS WAY.

login.php

<?php
include_once("database.php");
$postdata = file_get_contents("php://input");
$request = json_decode($postdata);
    if(isset($postdata) && !empty($postdata)){
        $pwd = mysqli_real_escape_string($mysqli, trim($request->password));
        $email = mysqli_real_escape_string($mysqli, trim($request->username));

        $sql = "SELECT * FROM users where email='$email'";

        if($result = mysqli_query($mysqli,$sql)){
            //$passchk = mysqli_fetch_assoc($result);
            $rows = array();
            while($row = mysqli_fetch_assoc($result)){
               $rows[] = $row;
            }
            if($result2 = mysqli_query($mysqli, $sql)){
                $passchk = mysqli_fetch_assoc($result2);

                if (password_verify($pwd, $passchk['password'])){
                    echo json_encode($rows);   
                }
            }
        }
        else{
            http_response_code(404);
        }
}
?>
Ian McCleary
  • 809
  • 1
  • 7
  • 14