0

I'm trying to implement a login system, and it mostly works except for this PHP script that's been returning 0:


// $username_err & $password_err is empty, gets reset every time as "".

if(empty($username_err) && empty($password_err)){
    $query = "SELECT rowid, username, password FROM admin_login WHERE username = ?";

    if($stmt = mysqli_prepare($db, $query)){
        mysqli_stmt_bind_param($stmt, "s", $param_username);
        $param_username = $username;

            if(mysqli_stmt_execute($stmt)){
                mysqli_stmt_store_result($stmt);

                // Doesn't work here, works up until here.
                if(mysqli_stmt_num_rows($stmt) == 1){
                    mysqli_stmt_bind_result($stmt, $rowid, $username, $hashed_password);

                    if(mysqli_stmt_fetch($stmt)){
                        if(password_verify($password, $hashed_password)){

                        session_start();

                        $_SESSION["loggedin"] = true;
                        $_SESSION["id"] = $rowid;
                        $_SESSION["username"] = $username;

                        header("location: index.php");

                        }else{
                           $password_err = "Invalid password";
                        }
                    }
                }else{
                    $username_err = "No such account exists.";
            }
        }else{
    echo "An error occurred.";
    }

  }

mysqli_stmt_close($stmt);

}

mysqli_close($db);

This never works, as it gets stuck since it doesn't meet the condition mysqli_stmt_num_rows == 1, rather it returns a 0. It does work via MySQL directly through the Workbench:

SELECT rowid, username, password FROM admin_login WHERE username = "admin";

does return 1 row with the matching criteria.

** EDIT: Here's my new code; **

<?php
require_once "_php/login/config.php";

$username = $password = "";
$err = "";

if ($_SERVER['REQUEST_METHOD'] == "POST") {
    if (empty(trim($_POST["username"]))) {
        $err = "Enter a username";
    } else {
        $password = trim($_POST["password"]);
    }

    if (empty(trim($_POST["password"]))) {
        $err = "Enter a password";
    } else {
        $password = trim($_POST["password"]);
    }

    $stmt = $db->prepare("SELECT rowid, username, password FROM admin_login WHERE username = ?");
    $stmt->bind_param("s", $_POST["username"]);
    $stmt->execute();
    $stmt->store_result();
    $user = $stmt->get_result()->fetch_assoc();

    if ($user && password_verify($_POST["password"], $user["password"])) {
        session_start();

        $_SESSION["loggedin"] = true;
        $_SESSION["username"] = $username;

        header("location: index.php");
    } else {
        $err = "Wrong password";
    }
}

//end of program

And it throws:

Fatal error: Uncaught Error: Call to a member function fetch_assoc() on boolean in directory/login_script.php:25 Stack trace: #0 directory/login.php(2): require() #1 {main} thrown in directory/login_script.php on line 25

For reference, login_script is the PHP script, login is the frontend as php.

Dharman
  • 30,962
  • 25
  • 85
  • 135
Dragonsnap
  • 834
  • 10
  • 25
  • MySqli is horribly old. Have you looked at trying PDO? https://www.php.net/manual/en/book.pdo.php -- Its much simpler, safer and binding params is simpler. Also shouldnt $param_username = $username; be above the bind statement? – daveBM Jun 30 '19 at 12:31
  • 3
    @daveBM Uhh, what?? `mysql_*` is old and should be avoided - yes. `mysqli_` (note the `i`) is perfectly fine. PDO is no more safe than MySQLi is, as long as you bind parameters (goes for both MySQLi and PDO). – Qirel Jun 30 '19 at 12:37
  • @daveBM I'll take a look at PDO, but isn't MySQLi a modern successor to MySQL commands in php? Also, I did modify the code to have $param_username = $username on top of bind, doesn't really change anything. – Dragonsnap Jun 30 '19 at 12:37
  • Do you get any errors? `var_dump($stmt->error);` after `mysqli_stmt_store_result()`? What does `mysqli_stmt_store_result()` return? How do you know it executes (meaning, entering the `if` block for the execute? – Qirel Jun 30 '19 at 12:41
  • Why don't you just use `$username` directly instead of creating a copy of it as `$param_username`? – M. Eriksson Jun 30 '19 at 12:41
  • @daveBM - In what way is PDO "safer" than Mysqli? – M. Eriksson Jun 30 '19 at 12:43
  • @Qirei When I added echo mysqli_stmt_num_rows($stmt) before the if block for the comparison, I got a 0. – Dragonsnap Jun 30 '19 at 12:58
  • @MagnusEriksson No real reason except I was following a tutorial. I could change $param_username to $username, doesn't really change the results. – Dragonsnap Jun 30 '19 at 12:59
  • Why is `$password = trim($_POST["password"]);` repeated twice? – Dharman Jun 30 '19 at 15:19

1 Answers1

0
  1. You never need that useless num_rows thing. You already have a row from fetch(). Use it wherever you have an idea to use num_rows.
  2. When it's 0 (or, rather, the only fetched row is false), it means your query found no rows given the input values and the current database contents. As simple as that. This is the answer to your question. Simple, direct and logical.
  3. That said, this code three times more bloated than it needs be. See the canonical code I wrote for checking the password using mysqli. Verify your input values, the database contents and check for errors - and it would work flawless.
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345