0

I watched a tutorial to build out a registration / login page. The register.php page works fine and writes new users to my mysql database. The index.html page will not load at all for data entry as written (but doesn't give me a specific error). I have checked it against the tutorial. I ran the code through a code checker and it didn't pick up on anything. I have copied the entire page code at the bottom of this question. If I remove the section of code I copied in the top entry, the page will at least load so I assume the error lies in that part of the code.

INDEX.PHP WILL LOAD WITHOUT THIS SECTION OF CODE

if(isset($_REQUEST['login_btn'])){
        $email = filter_var(strtolower($_REQUEST['email']),FILTER_SANITIZE_EMAIL); //strtolower changes email to all lower case
        $password = strip_tags($_REQUEST['password']);

        if(empty($email)){
            $errorMsg[] = 'Must enter email';
        }
        else if(empty($password)){
            $errorMsg[] = 'Must enter password';
        }  
        else{
            try{
            $select_stmt = $db->prepare("SELECT * from users WHERE email = :email LIMIT 1");
            $select_stmt->execute([
                ':email' => $email
            ]);
            $row = $select_stmt->fetch(PDO::FETCH_ASSOC);

            if(select_stmt->rowCount() > 0){
                if(password_verify($password,$row["password"])){

                    $_SESSION['user']['firstname'] = $row["firstname"];
                    $_SESSION['user']['email'] = $row["email"];
                    $_SESSION['user']['user'] = $row["userid"];

                    header("location: welcome.php");
            }
            else{
                $errorMsg[] = "Wrong Email or Password";                
            }
        }
        else{
            $errorMsg[] = "Wrong Email or Password";
        } 
    }        
    catch(PDOException $e){
        echo $e->getMessage();
        }

    }
     
}

THE CODE BELOW IS THE FULL INDEX.PHP PAGE

    <?php
require_once '../../db_php/dbconnection.php';
session_start();

if( isset($_SESSION['user']) ){
    header("location: welcome.php");
}

if(isset($_REQUEST['login_btn'])){
        $email = filter_var(strtolower($_REQUEST['email']),FILTER_SANITIZE_EMAIL); //strtolower changes email to all lower case
        $password = strip_tags($_REQUEST['password']);

        if(empty($email)){
            $errorMsg[] = 'Must enter email';
        }
        else if(empty($password)){
            $errorMsg[] = 'Must enter password';
        }  
        else{
            try{
            $select_stmt = $db->prepare("SELECT * from users WHERE email = :email LIMIT 1");
            $select_stmt->execute([
                ':email' => $email
            ]);
            $row = $select_stmt->fetch(PDO::FETCH_ASSOC);

            if(select_stmt->rowCount() > 0){
                if(password_verify($password,$row["password"])){

                    $_SESSION['user']['firstname'] = $row["firstname"];
                    $_SESSION['user']['email'] = $row["email"];
                    $_SESSION['user']['user'] = $row["userid"];

                    header("location: welcome.php");
            }
            else{
                $errorMsg[] = "Wrong Email or Password";                
            }
        }
        else{
            $errorMsg[] = "Wrong Email or Password";
        } 
    }        
    catch(PDOException $e){
        echo $e->getMessage();
        }

    }
     
}

?>
<html lang="en">

<head>
    <meta charset="UTF-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <link href="https://cdn.jsdelivr.net/npm/bootstrap@5.1.0/dist/css/bootstrap.min.css" rel="stylesheet" integrity="sha384-KyZXEAg3QhqLMpG8r+8fhAXLRk2vvoC2f3B09zVXn8CA5QIVfZOJ3BCsw2P0p/We" crossorigin="anonymous">
    <script src="https://cdn.jsdelivr.net/npm/bootstrap@5.1.0/dist/js/bootstrap.bundle.min.js" integrity="sha384-U1DAWAznBHeqEIlVSCgzq+c9gqGAJn5c/t99JyeKa9xxaYpSvHU5awsuZVVFIhvj" crossorigin="anonymous"></script>
    <title>Fireytech Database Home</title>
    <link rel="stylesheet" href="css/style.css">
</head>
<body>
    <div class="container">
<?php

            if( isset($errorMsg) ){
                foreach($errorMsg as $loginError){
                echo "<p class='alert alert-danger'>".$loginError."</p>";
            }
        }
?>        
        <form action="index.php" method="post">
      <div class="mb-3">
          <label for="email" class="form-label">Email address</label>
          <input type="email" name="email" class="form-control" placeholder="Enter Email">
        </div>
        <div class="mb-3">
          <label for="password" class="form-label">Password</label>
          <input type="password" name="password" class="form-control" placeholder="Enter Password">
        </div>
            <button type="submit" name="login_btn" class="btn btn-primary">Login</button>
        </form>
    </div>
</body>
</html>
Fireytech
  • 23
  • 6
  • `if(select_stmt->rowCount() > 0)` should be `if($select_stmt->rowCount() > 0)` (missing the `$` in front of the variable). However, you should change it to just: `if ($row) {...}` since the [manual](https://www.php.net/manual/en/pdostatement.rowcount.php) says: _"For most databases, PDOStatement::rowCount() does not return the number of rows affected by a SELECT statement."_ – M. Eriksson Mar 01 '22 at 06:37
  • When developing, make sure you [show all errors and warnings](https://stackoverflow.com/questions/1053424/how-do-i-get-php-errors-to-display). – M. Eriksson Mar 01 '22 at 06:41
  • 1
    I would also **strongly** advice you to _not_ run the passwords through `strip_tags()` (or any other function that might change the passwords) before hashing/using them. There's absolutely no reason to do so. Not even sure what you're trying to prevent with it, as you should _never_ output the password anyway? In fact, it can _seriously_ weaken the password. Imagine if someone has `` as password. If you run it through `strip_tags()`, you will be left with a _blank string_ as password. – M. Eriksson Mar 01 '22 at 06:44
  • The tuturial advised running the password through strip tags to avoid injection but I understand what you are saying. I'll read through the show errors link and see if i can figure it out. My site is on a shared web hosting plan so I don't know exactly how I'm going to modify that yet but not seeing errors really hurts my ability to troubleshoot. Regarding the if(select_stmt->rowCount() > 0){ line...I am very new to PHP code. Are you saying to literally change the line of code to "if ($row) {" to replace my current line? – Fireytech Mar 01 '22 at 15:28
  • _"The tuturial advised running the password through strip tags to avoid injection"_ - Then I would recommend you to look for some other tutorial because that's just wrong and _very_ misleading. 1. `strip_tags()` has _nothing_ to do with preventing SQL injections and is not sufficient for that purpose. 2. You're hashing the passwords, so you never even store the original passwords "as is" anyway. 3. Since you're, correctly, using prepared statements with placeholders and aren't injecting the data into the queries directly, you're already safe from SQL injections. – M. Eriksson Mar 01 '22 at 15:33
  • _"Are you saying to literally change the line of code to "if ($row) {" to replace my current line?"_ - Yes. If the query didn't find any match on the email, `$row` will be empty. And if you do `if ($row)`, it will only evaluate as true if it found a record. – M. Eriksson Mar 01 '22 at 15:36
  • 1
    Thanks M. Eriksson. I changed that one line of code you suggested on the row line and the page loaded and WORKED!!!! I also removed the password strip tag as you suggested. Thank you! Now, on to learning the next phase. Eating the elephant one bite at a time LOL. – Fireytech Mar 01 '22 at 15:37

1 Answers1

0

M. Eriksson posted above and it was the correct answer so I'm copying it here to close this question.

if(select_stmt->rowCount() > 0) should be if($select_stmt->rowCount() > 0) (missing the $ in front of the variable).

However, you should change it to just: if ($row) {...} since the manual says:

For most databases, PDOStatement::rowCount() does not return the number of rows affected by a SELECT statement.

Wahyu Kristianto
  • 8,719
  • 6
  • 43
  • 68
Fireytech
  • 23
  • 6